Skip to content

Commit 724ad43

Browse files
committed
cowork-bot: scan intra-env duplicate-key collision warning
scan merges all config files in an environment directory via dict.update(), so two files defining the same flattened key with different values silently resolved to glob-order winner. Now emits a stderr warning naming both files (stderr so --output json stdout stays machine-readable) and globs files in sorted order for deterministic winner selection. +3 tests (158 pass, ruff clean).
1 parent 51f31da commit 724ad43

2 files changed

Lines changed: 74 additions & 4 deletions

File tree

src/configdrift/cli.py

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ def require_license(product: str) -> None: # type: ignore[misc]
3636
invoke_without_command=True,
3737
)
3838
console = Console()
39+
# Warnings that must not pollute machine-readable (JSON) stdout go to stderr.
40+
_warn_console = Console(stderr=True)
3941

4042
_require_license_strict: bool = False
4143

@@ -283,21 +285,35 @@ def scan(
283285
files_loaded = 0
284286
for env_name, dir_path in dir_mapping.items():
285287
env_configs[env_name] = {}
288+
key_sources: dict[str, str] = {} # flattened key -> file that provided it
286289
p = Path(dir_path)
287290
if not p.is_dir():
288291
console.print(
289292
f"[yellow]Warning: '{dir_path}' is not a directory, skipping.[/yellow]"
290293
)
291294
continue
292-
# Load all supported config files in the directory and merge
295+
# Load all supported config files in the directory and merge.
296+
# Later files silently overwrite earlier ones via dict.update(); surface
297+
# conflicting duplicate keys instead of letting glob order pick a winner.
293298
for ext in ("*.yaml", "*.yml", "*.json", "*.toml", "*.env"):
294-
for f in p.glob(ext):
299+
for f in sorted(p.glob(ext)):
295300
try:
296301
data = load_file(str(f))
297-
env_configs[env_name].update(data)
298-
files_loaded += 1
299302
except Exception as e:
300303
console.print(f"[yellow]Warning: could not load {f}: {e}[/yellow]")
304+
continue
305+
for k, v in data.items():
306+
prev_file = key_sources.get(k)
307+
if prev_file is not None and env_configs[env_name].get(k) != v:
308+
_warn_console.print(
309+
f"[yellow]Warning: key '{k}' is defined with conflicting "
310+
f"values in {prev_file} and {f} (environment "
311+
f"'{env_name}'); using the value from {f} "
312+
"(alphabetical filename order).[/yellow]"
313+
)
314+
key_sources[k] = str(f)
315+
env_configs[env_name].update(data)
316+
files_loaded += 1
301317

302318
# Silent-failure guard: if nothing was actually loaded, any "no drift"
303319
# result would be a false green. Fail loudly instead.

tests/test_cli.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,60 @@ def test_scan_baseline_loaded_nothing_exits_1(self):
521521
assert result.exit_code == 1
522522
assert "loaded no config" in result.stdout
523523

524+
class TestScanKeyCollisionWarning:
525+
"""Scan must warn when two files in one env define the same key differently."""
526+
527+
def test_scan_conflicting_duplicate_key_warns(self):
528+
with tempfile.TemporaryDirectory() as tmpdir:
529+
dev = Path(tmpdir) / "dev"
530+
prod = Path(tmpdir) / "prod"
531+
dev.mkdir()
532+
prod.mkdir()
533+
(dev / "a_app.yaml").write_text(yaml.dump({"host": "localhost"}))
534+
(dev / "b_extra.yaml").write_text(yaml.dump({"port": 8080}))
535+
(prod / "a_app.yaml").write_text(yaml.dump({"host": "one.example.com"}))
536+
(prod / "b_extra.yaml").write_text(
537+
yaml.dump({"host": "two.example.com", "port": 8080})
538+
)
539+
result = runner.invoke(app, ["scan", str(dev), str(prod)])
540+
assert result.exit_code == 0, f"STDOUT: {result.stdout}"
541+
# Warning goes to stderr so JSON/table stdout stays machine-readable.
542+
assert "conflicting" in result.stderr
543+
assert "'host'" in result.stderr
544+
545+
def test_scan_identical_duplicate_keys_no_warning(self):
546+
"""Same key+value in two files is redundant but not conflicting."""
547+
with tempfile.TemporaryDirectory() as tmpdir:
548+
dev = Path(tmpdir) / "dev"
549+
prod = Path(tmpdir) / "prod"
550+
dev.mkdir()
551+
prod.mkdir()
552+
(dev / "a.yaml").write_text(yaml.dump({"host": "localhost"}))
553+
(dev / "b.yaml").write_text(yaml.dump({"host": "localhost"}))
554+
(prod / "a.yaml").write_text(yaml.dump({"host": "prod.example.com"}))
555+
(prod / "b.yaml").write_text(yaml.dump({"host": "prod.example.com"}))
556+
result = runner.invoke(app, ["scan", str(dev), str(prod)])
557+
assert result.exit_code == 0
558+
assert "conflicting" not in result.stdout
559+
560+
def test_scan_collision_uses_alphabetical_order(self):
561+
"""The surviving value comes from the alphabetically-last file."""
562+
with tempfile.TemporaryDirectory() as tmpdir:
563+
dev = Path(tmpdir) / "dev"
564+
prod = Path(tmpdir) / "prod"
565+
dev.mkdir()
566+
prod.mkdir()
567+
(dev / "c.yaml").write_text(yaml.dump({"host": "localhost"}))
568+
(prod / "a.yaml").write_text(yaml.dump({"host": "from-a"}))
569+
(prod / "z.yaml").write_text(yaml.dump({"host": "from-z"}))
570+
result = runner.invoke(
571+
app, ["scan", str(dev), str(prod), "--output", "json"]
572+
)
573+
assert result.exit_code == 0
574+
data = json.loads(result.stdout)
575+
host_change = next(c for c in data["prod"]["changes"] if c["key"] == "host")
576+
assert host_change["new_value"] == "from-z"
577+
524578
def test_scan_healthy_still_works(self):
525579
with tempfile.TemporaryDirectory() as tmpdir:
526580
dev = Path(tmpdir) / "dev"

0 commit comments

Comments
 (0)