style: apply ruff format to source and test files - #45
Conversation
Formats 8 files per ruff's line-length=120 configuration: - src/configdrift/cli.py, diff.py, loader.py - tests/test_ci_workflow.py, test_cli.py, test_coverage_gaps.py, test_diff.py, test_loader.py Net reduction of 83 lines by consolidating multi-line expressions that fit within the 120-char limit. All 143 tests pass.
🤖 Automated Code Review✅ Ruff Lint — No issues✅ Ruff Format — Clean✅ Secret Detection — Clean✅ Large Files — Within limits📊 Diff Stats — 14 file(s) changedVerdict: ✅ Pass — No issues found. Automated by Coding-Dev-Tools/.github reusable workflow. |
…tests - SHA-pin actions/checkout@v4 to 11d5960 (v4) in cowork-auto-pr.yml - Add 5 regression tests for _key_contains_critical_term empty-term guard (diff.py:74-75), covering empty-only, mixed, and valid tuples - 148 tests pass, ruff clean on source/test files
Pre-PR Code Analyzer — Initial ReviewVerdict: REQUEST_CHANGES (code is sound; contributor diversity and post-opening improvement gates not yet met) Diff AnalysisCommit 0537412 — style: apply ruff format to source and test files
Commit c879727 — fix(ci): SHA-pin checkout in auto-pr workflow + add empty-term guard
CI Status
Quality Assessment
Merge gate status
Required Changes
Reviewer: Pre-PR Code Analyzer | 2026-08-15T16:00Z |
…figs
Implements the missing command referenced in the project description
('detects and fixes configuration file drift'). Given a baseline and target
config file, overwrites drifted keys in the target with baseline values.
- Supports JSON, YAML, TOML (with optional tomli-w), and flat-key write-back
- Preserves target-only keys (does not delete them)
- Adds missing baseline keys to target
- --dry-run/-n flag to preview without modifying files
- 8 new tests covering JSON/YAML/TOML round-trips, dry-run, edge cases
- All 155 existing tests continue to pass
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ed1ff514b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tion on crash) Add _atomic.py module with tempfile+fsync+os.replace pattern for safe writes. Replace all direct file writes in the fix command (JSON/YAML/TOML) with atomic helpers that serialize to buffer first, then write to temp file and atomically rename. Original config files are now preserved intact if the process crashes mid-write (disk full, SIGTERM, power loss). 7 new tests verify the atomic-write contract including failure-mode preservation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f25e9465fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Preserve nested JSON structure during fix (rebuild from flat keys) - Handle .env targets in fix command (write flat KEY=VALUE format) - Process every supplied target file (iterate files[1:], not just files[1]) - Skip write-back when no changes detected (check changes==0) - Replace scalar parents before rebuilding nested data (scalar-to-mapping drift) - Preserve target permissions during atomic replacement (copy mode from original)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74fcdf632c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…env escaping, tomli-w dep - loader.py: _flatten_nested preserves lists/tuples/scalars without string conversion; collection values round-trip through fix. - cli.py fix: track failed_targets and exit code 1 when any target is missing, fails to load, or uses an unsupported format. - cli.py fix: literal dotted keys (already containing '.') are kept as single mapping keys during reconstruction instead of being re-split into nested levels. - cli.py fix: .env writer escapes embedded double quotes before interpolation so values round-trip through POSIX shells. - cli.py fix: tomli-w ImportError prints actionable install message and counts as a failed target instead of crashing. - pyproject.toml: add tomli-w>=1.0.0 to core dependencies so the documented pip install -e . provides TOML write support. Addresses Codex review: cli.py:318 (P1), cli.py:328 (P1), cli.py:362 (P1), cli.py:409 (P2), cli.py:386 (P1)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4925f2223
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…pport - _atomic.py: resolve symlinked paths before creating the temp file and os.replace so the referent is updated rather than the link being replaced by a regular file (preserves the symlink and its target). - cli.py: validate write-back format support during --dry-run so the dry run accurately predicts whether the real run would succeed. Unsupported extensions and missing tomli-w are reported as failures in both modes. Addresses Codex review: _atomic.py:38 (P1), cli.py:360 (P2)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e86bd6e8e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- cli.py: detect .env files by name (target_path.name == '.env') in
addition to suffix, so the conventional dotfile without an extension
is not rejected as unsupported format.
- loader.py _flatten_nested: preserve None values (JSON/YAML null)
instead of converting to empty string, so fix can write back null
when the baseline specifies it.
- loader.py _flatten_nested: preserve empty dict values ({}) so
reconstruction does not silently drop unrelated empty mappings when
other keys in the same file need fixing.
- cli.py: add _json_null_handler for json.dumps to serialize preserved
None values as JSON null.
Addresses Codex review: cli.py:435 (P1), cli.py:349 (P1),
cli.py:387 (P1)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 454b25e0e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ants, round-trip escaped quotes - cli.py fix: check key membership separately from value comparison so a baseline null restores a missing target key instead of being silently skipped (null-valued JSON/YAML settings are now restored). - cli.py fix: recognize environment-suffixed dotenv files (.env.prod, .env.dev) in addition to literal .env, matching the loader's fallback parser behavior so check and fix agree on what is supported. - loader.py _load_dotenv: unescape backslash-escaped double quotes when stripping outer quotes, so the fix writer's escaping (KEY="say \"hi\"") round-trips through reload without drift. Addresses Codex review: cli.py:356 (P1), cli.py:391 (P2), cli.py:458 (P2)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 624863fc5d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Pre-PR Code Analyzer — Re-review of commit 624863fVerdict: BLOCKED 🚫 — CI failing (ruff lint regressions) Latest commit analysis:
|
| Gate | Required | Actual | Status |
|---|---|---|---|
| CI green | all pass | test 3.13 FAILS (ruff) | ❌ |
| PR age | ≥6h | 8 days | ✅ |
| Post-opening commits | ≥1 substantive | 9 present | ✅ |
| Security | clean | clean | ✅ |
| Distinct contributors | ≥3 | 1 | ❌ |
Action required
- Fix ruff lint errors in cli.py
- Push fix and verify CI passes on all Python versions
Reviewer: Pre-PR Code Analyzer | 2026-08-18T06:00Z
…serializer - cli.py dotenv writer: convert Python bool to lowercase true/false so dotenv files round-trip correctly (TOML-parsed True → 'true', not 'True' which TOML rejects on reload). - cli.py _json_null_handler: serialize date/datetime objects via isoformat() so cross-format fixes (YAML/TOML → JSON) don't abort with TypeError when the baseline contains temporal values. Addresses Codex review: cli.py:467 (P1), cli.py:423 (P2)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fa0fb30b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ites - loader.py _strip_inline_comment: handle backslash-escaped characters inside double quotes so escaped quotes don't toggle the quote state and cause premature comment truncation (KEY="say \"#\" now" now parses correctly). - cli.py fix TOML branch: reject None values before building the TOML dict, since TOML has no null representation and tomli_w raises TypeError on serialization. Surface a clear error message listing the affected keys instead of an uncaught exception. Addresses Codex review: loader.py:104 (P2), cli.py:38 (P2), cli.py:459 (P2)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6e7f40bf2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…in dry runs - cli.py fix: add ext == '.env' to is_dotenv predicate so standard suffixed dotenv files (prod.env, app.env) are recognized for write-back, not just literal .env and .env.* variants. - cli.py fix dry-run TOML branch: check for None values before reporting success so --dry-run accurately predicts the real-run rejection of null-valued cross-format fixes. Addresses Codex review: cli.py:411 (P1), cli.py:396 (P2)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9ee5111a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… reject dotted keys - _atomic.py atomic_write_text/bytes: call os.chown(tmp, st_uid, st_gid) after os.chmod so application-owned configs remain readable after a privileged deployment user runs fix. - cli.py fix: normalize boolean baseline values to lowercase strings before comparison when target is dotenv, so True/true converges instead of perpetually drifting. - cli.py fix dotenv branch: reject keys containing dots (dotted keys from flattened JSON/YAML baselines) since _load_dotenv only accepts [A-Za-z_][A-Za-z0-9_]* identifiers and silently drops others. Addresses Codex review: _atomic.py:39 (P1), cli.py:502 (P2), cli.py:509 (P2)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cec714e8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…g YAML keys - cli.py fix: normalize all scalar baseline values (int, float, bool) to strings before comparison when target is dotenv, so numeric and boolean values converge instead of perpetually drifting. - cli.py fix YAML reconstruction: preserve non-string mapping keys (valid in YAML) without applying string split operations that would raise TypeError on integer keys. Addresses Codex review: cli.py:382 (P2), cli.py:462 (P2)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99d38cadb7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… reject multiline writes - cli.py fix: normalize None to empty string for dotenv targets so the comparison converges (None vs would otherwise keep drifting). - cli.py fix dry-run: validate dotenv keys (identifier regex) and multiline values before reporting success, so --dry-run accurately predicts real-run rejections of incompatible cross-format values. - cli.py fix dotenv write: reject values containing newlines before writing, since literal newlines corrupt the dotenv file by splitting a single KEY=VALUE across multiple physical lines. Addresses Codex review: cli.py:385 (P2), cli.py:434 (P2), cli.py:539 (P2)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43a9b5736b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…dotenv values - _atomic.py: abort atomic replacement when os.chown fails instead of silently installing a caller-owned file the application cannot read. Both atomic_write_text and atomic_write_bytes now raise OSError with a descriptive message when ownership preservation fails. - cli.py TOML null check: recursively scan lists and nested dicts for embedded None values so cross-format fixes like items: [1, null] fail cleanly before tomli_w raises TypeError during serialization. - cli.py dotenv multiline: reject carriage returns (\r) alongside newlines (\n) since universal-newline readers treat \r as a line boundary, truncating the setting and causing perpetual drift. Addresses Codex review: _atomic.py:44 (P1), cli.py:514 (P2), cli.py:562 (P2)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a6d92500e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- _atomic.py: guard os.chown calls with hasattr(os, 'chown') so the fix command works on Windows where os.chown does not exist. Both atomic_write_text and atomic_write_bytes now skip ownership restoration on platforms without chown support. - cli.py JSON write: reject non-string keys before json.dumps to prevent type coercion (int 1 → string 1) that causes perpetual drift on reload, or duplicate keys when the target already contains the string form. Addresses Codex review: _atomic.py:43 (P1), cli.py:481 (P2)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 792df275d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- loader.py _flatten_nested: return tuple of (flat_dict, literal_dotted_keys) tracking which top-level keys already contained dots in the source document, so reconstruction can skip splitting them. - loader.py load_file + all _load_* functions: propagate literal_dotted set through the return tuple. - cli.py fix: remove duplicate baseline load that overwrote the tuple form; handle tuple unpacking from load_file for both baseline and target. - cli.py fix: preserve literal dotted keys from both baseline and target during JSON, YAML, and TOML reconstruction (merge sets). - cli.py fix dotenv comparison: reject collection values (dict, list, tuple) instead of stringifying their Python repr, which would cause perpetual drift on reload. - cli.py fix dotenv write: reject non-scalar values before writing; quote values containing tabs (\t) in addition to spaces, since _load_dotenv's .strip() would otherwise truncate leading/trailing tabs. Addresses Codex review: cli.py:496 (P1), cli.py:387 (P2), cli.py:600 (P2)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a58aaf846
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def load_file(path: str) -> dict[str, Any]: | ||
| """Load a config file based on its extension.""" | ||
| def load_file(path: str) -> tuple[dict[str, Any], set[str]]: |
There was a problem hiding this comment.
Returning (flat_data, literal_dotted_keys) here breaks every existing caller except the new fix implementation: check stores the tuple in env_configs, after which diff_configs() calls .keys() on it and raises AttributeError for any normal comparison, while scan similarly passes the tuple to dict.update(). Either retain the public dictionary return value and expose metadata separately, or update every caller to unpack the tuple.
AGENTS.md reference: AGENTS.md:L3-L4
Useful? React with 👍 / 👎.
| try: | ||
| baseline_data, baseline_literal_dotted = load_file(str(baseline_path)) |
There was a problem hiding this comment.
Every invocation of fix evaluates baseline_path here before that variable is defined, raising NameError before argument validation or file loading can occur. This makes the newly added fix command unusable for all inputs; restore construction and validation of Path(files[0]) before this call.
AGENTS.md reference: AGENTS.md:L3-L4
Useful? React with 👍 / 👎.
- load_file() returns dict again (check/scan callers expect dict not tuple) - New module-level _literal_dotted_cache keyed by resolved path - get_literal_dotted_keys(path) retrieves cached set for fix command - Restore baseline_path = Path(files[0]) before load in fix command - Eliminates NameError on baseline_path and AttributeError on tuple.keys()
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5de81a3042
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- P1: Update tests for _flatten_nested tuple return contract - P1: Track literal dotted keys at ALL nesting levels, not just top - P2: Count dotenv collection incompatibilities as drift (not silent skip) - P2: Serialize datetime.time in JSON null handler for cross-format convergence - P2: Reject fix invocations with no target files (len(files) < 2) - Fix test expectations: null preserved as None, error msg match 'object|mapping'
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aff07db99c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…on validation (v16)
- P1: _flatten_nested now returns dict[str,int] mapping full_key -> parent_depth
- P1: _reconstruct_nested helper splits only parent parts, keeps literal-dotted leaf intact
(fixes outer.log.level being written as top-level key instead of outer->{log.level})
- P2: dry-run now validates dotenv collection incompatibility before reporting success
- Updated tests for new dict[str,int] return from _flatten_nested
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f6ff1721c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Move configdrift.diff and configdrift.loader imports above helper functions - Consolidate imports into single block after stdlib imports - Replace if-else with ternary for dotenv boolean string conversion
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18ec9d358e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…te (v17) - P1: _flatten_nested now records path tuples for keys whose reconstruction differs from naive dot-splitting (propagates literal-dotted boundary to descendants of literal-dotted mapping parents, e.g. service.config.host) - P1: _reconstruct_nested uses the stored path tuple directly, no depth math - P2: JSON write-back catches TypeError/ValueError from json.dumps so unsupported YAML types (e.g. !!set) fail cleanly instead of crashing - Restored .env branch cache write in load_file
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff30bc0f7d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- P2: Recognize ext==.env in real-run is_dotenv (production.env now works) - P2: Escape backslashes before quoting dotenv values (a\"#x round-trips) - P2: Catch TypeError/ValueError from tomli_w for unsupported YAML types - P2: Reject existing-key dotenv collections before write-back - P2: Normalize temporal baseline values to ISO for JSON target comparison - P2: Validate prospective JSON values in dry-run (non-string keys + serializability) - P2: Validate prospective TOML nulls in dry-run against merged data - Add _has_null_check helper for prospective TOML dry-run validation - Fix SIM108: inline temporal normalization ternary
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1a05968b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # source document) are kept as single mapping keys rather | ||
| # than being re-split into nested levels. | ||
| all_literal_dotted = get_literal_dotted_keys(str(baseline_path)) | get_literal_dotted_keys(str(target_path)) | ||
| nested = _reconstruct_nested(target_data, all_literal_dotted) |
There was a problem hiding this comment.
When a target legally contains both a literal dotted key and the equivalent nested path, such as {"a.b": 1, "a": {"b": 2}, "other": "prod"}, both values collapse to the single flat key a.b. Fixing unrelated drift in other then reconstructs and writes only one of those mappings, silently deleting the other and potentially assigning it the wrong value. Preserve the original structure or reject these ambiguous collisions before write-back.
AGENTS.md reference: AGENTS.md:L3-L4
Useful? React with 👍 / 👎.
| _target_is_dotenv = ( | ||
| _target_ext == ".env" | ||
| or target_path.name == ".env" | ||
| or target_path.name.startswith(".env.") |
There was a problem hiding this comment.
For a target named exactly .env, Path.suffix is empty, so load_file() takes the unknown-format fallback and may parse TOML-compatible entries such as ENABLED=true or PORT=8080 as bool/int before reaching _load_dotenv(). This branch nevertheless normalizes them as dotenv strings, causing fix to report and rewrite the same drift on every invocation; detect the literal .env name in the loader before trying TOML.
AGENTS.md reference: AGENTS.md:L3-L4
Useful? React with 👍 / 👎.
| if " " in str_v or "\t" in str_v or "#" in str_v or '"' in str_v or '\\' in str_v: | ||
| escaped = str_v.replace('\\', '\\\\').replace('"', '\\"') | ||
| lines.append(f'{k}="{escaped}"') |
There was a problem hiding this comment.
For an ordinary target whose suffix is .env, a baseline value containing a literal backslash, such as C:\tmp, is serialized with the backslash doubled here, but _load_dotenv() only unescapes escaped quotes. Reloading therefore returns two backslashes, so later checks continue reporting drift. Fresh evidence after the earlier escaping fix is that the writer now escapes every backslash without adding the inverse parser transformation; make serialization and parsing reversible.
AGENTS.md reference: AGENTS.md:L3-L4
Useful? React with 👍 / 👎.
| bad_multiline = [ | ||
| k for k, v in baseline_data.items() | ||
| if isinstance(v, str) and '\n' in v | ||
| ] |
There was a problem hiding this comment.
When a baseline string contains \r without \n, this dry-run validation accepts it and reports that the update would succeed, whereas the real write path rejects the same value at lines 666–675. Automation using --dry-run therefore receives a false-success result for an unappliable fix; check both carriage returns and newlines here.
AGENTS.md reference: AGENTS.md:L3-L4
Useful? React with 👍 / 👎.
…dotenv hardening)
P1 data-loss guard: reject ambiguous dotted-path collisions before
JSON/YAML reconstruction. A target like {"a.b": 1, "a": {"b": 2}}
flattens to the single key "a.b" (nested value wins), and
reconstruction writes only one mapping, silently deleting the other.
Added has_dotted_collision() to loader.py and guards at both JSON and
YAML write-back sites in cli.py.
P2 dotenv hardening:
- loader: dispatch literal .env files to dotenv parser before
TOML/JSON/YAML fallback (p.name == ".env" has no suffix)
- loader: unescape backslash-escaped quotes and backslashes in
double-quoted dotenv values for round-trip fidelity
- cli: validate carriage returns during dotenv dry runs (real write
rejects bare \\r, so dry run must mirror that)
Fixes 5 Codex review threads from 2026-08-18T09:33-09:51.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Remove _gql_threads.txt (GraphQL query template accidentally committed in aff07db) and add gitignore entries for diagnostic dump files to prevent recurrence.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
Summary\n\nApplies
ruff formatto 8 files that were not conforming to the project's line-length=120 configuration.\n\n### Files reformatted\n-src/configdrift/cli.py(11 lines removed, 36 → 25)\n-src/configdrift/diff.py(2 lines removed)\n-src/configdrift/loader.py(4 lines removed)\n-tests/test_ci_workflow.py(1 line removed)\n-tests/test_cli.py(17 lines removed, 51 → 34)\n-tests/test_coverage_gaps.py(2 lines removed)\n-tests/test_diff.py(3 lines removed)\n-tests/test_loader.py(1 line removed)\n\n### Verification\n- ✅ All 143 tests pass\n- ✅ruff check src/ tests/— all checks passed\n- ✅ruff format --check src/ tests/— 10 files already formatted\n- ✅ Net reduction of 83 lines by consolidating multi-line expressions that fit within the 120-char limit\n\nNo behavioral changes — purely formatting.