feat(cli): cofferdam invariants subcommand (CD-308) + fix inert cofferdam.toml [layers] (CD-312) - #200
Merged
Merged
Conversation
The parsed architectural spec was reachable only through the MCP server's `cofferdam.invariants` tool. That inverted the server's own contract — every other tool there is a thin wrapper over a CLI command — and left a user whose rule was not firing with no way to ask the CLI what it had actually loaded. The docs twice promised verbs of this subcommand that did not exist, and an error message in cofferdam-core sent users to one. `show` prints the resolved spec: which files were read, which of them the layers in force came from, the public API, boundaries, invariants and scripted rules, plus load warnings. `validate` parses without running the engine and exits 1 on failure, so CI can gate on the config separately from the findings it produces; `--strict` extends that to warnings. `normalize` emits the canonical TOML. No `migrate` — it earns its keep at the first MAJOR schema bump and not before. fix(config): load [layers] from cofferdam.toml (CD-312) Found while writing `show`, which reports which file the layers in force came from and could not answer honestly. `loader::load` built the config with an unconditional `layers: None`, having already deserialised the block; `parse_layers`, the function that turns it into a `LayersConfig`, had no callers anywhere in the workspace. So `Design.LayerViolation` fired only for layers declared in cofferdam.invariants.toml, while the cofferdam.toml spelling parsed, validated and was discarded — accepted, therefore silent. It also made the "declared in both files" deprecation hint unreachable, since the condition it tests could never be true. Both now work. The change reports violations that were always there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019afugU2bTYCtXn6Vy9dn1v
Criterion benchmark comparison (PR head vs base branch) |
Criterion benchmark comparison (PR head vs base branch) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes CD-308. Closes CD-312.
CD-308 —
cofferdam invariantsThe parsed architectural spec was reachable only through the MCP server's
cofferdam.invariantstool.docs/mcp.mdsaid so plainly: "No CLI equivalent exists — this is MCP-only." That inverts the server's own contract — every other tool there is a thin wrapper over a CLI command — and it matters because the spec is the load-bearing config surface. It merges two files, resolves globs, and validates a predicate DSL whose failure aborts the run. A user with a rule that was not firing had no way to ask the CLI what it had loaded.Three verbs:
show— the resolved, merged spec. Which files were read, which of them the layers in force came from, public API, boundaries, invariants, scripted rules, load warnings. Text by default,--robot/--format jsonfor machines,--prettyto indent. Exits 0 whenever the spec loads, including when nothing is declared — but says "Nothing declared" rather than printing an empty skeleton, because "you have no spec" and "your spec is empty" send a user to different fixes.validate— parse and report without running the engine. Exits 1 on a malformed predicate or an unsupported schema version, so CI can gate on the config alone.--strictextends that to warnings (missing/deprecatedschema_version,[layers]in both files).normalize— canonical TOML to stdout. Round-trip tested: normalising an already-normalised spec is a no-op.No
migrate. It earns its keep at the first MAJOR schema bump and not before, and the docs have already been wrong about it twice.CD-312 —
[layers]incofferdam.tomldid nothingFound while writing
show, which reports which file the layers in force came from and could not answer honestly while one of the two sources did not work.loader::loadbuilt the config with an unconditionallayers: None, having already deserialised the block intoTomlDoc::layers.parse_layers— the function that turns it into aLayersConfig— had no callers anywhere in the workspace. SoDesign.LayerViolationfired only for layers declared incofferdam.invariants.toml; thecofferdam.tomlspelling parsed, validated, and was thrown away.Reproduced before the fix on a two-layer fixture with a deliberate violation: layers in
cofferdam.toml→ no finding; byte-identical layers incofferdam.invariants.toml→Design.LayerViolation. Same failure shape as theexcludekey from the docs sweep, and silent for the same reason — the config is accepted, so nothing tells you.It also made the
[layers] declared in both filesdeprecation hint unreachable:merge_invariants_fromtestscfg.layers.is_some(), which was always false. Both work now, verified end to end.This is a behaviour change in the honest direction. A project whose
cofferdam.toml[layers]block has been quietly inert will start reporting the violations it always had. Filed under Fixed rather than Changed for that reason.Verification
cargo test --workspace— 1334 passed (12 new: 9 CLI integration tests for the subcommand, 3 config unit tests for the layers fix)cargo clippy --workspace --all-targets -- -D warnings— cleancargo fmt --all -- --check— cleangen-docs --check— clean (the llms.txt drift gate caught the missing subcommand entry, which is the gate doing its job)Note
docs/schema-versioning.mdstill describescofferdam invariants normalizeas future work andmigrateas existing. That file is owned by #197, which is rewriting the same section; it gets corrected in whichever of the two merges second rather than conflicting here.🤖 Generated with Claude Code
https://claude.ai/code/session_019afugU2bTYCtXn6Vy9dn1v