fix(doctor): stop printing remediation hints for flags that do not exist - #198
Open
TAJD wants to merge 4 commits into
Open
fix(doctor): stop printing remediation hints for flags that do not exist#198TAJD wants to merge 4 commits into
TAJD wants to merge 4 commits into
Conversation
`cofferdam doctor` told users to run two commands that exit 2. - The suppression scan-limit warning said "pass --paths to scope" and "add --paths". `doctor` takes no positional args and no --paths; its only options are --pretty, --robot and --help. - The stale-directive warning said "see `cofferdam explain --list`". `explain` requires a <CHECK_ID> argument and defines no --list, so that command fails with "unexpected argument". A remediation hint is read at the moment the user is already stuck. One that names a flag that was never implemented costs more than no hint at all. Both hints are now named constants with a test asserting they reference no CLI flag, so the invariant is pinned rather than restated in prose. docs/doctor.md repeated both errors verbatim and is corrected to match. docs/suppression.md recommended `--ignore-path`, which no subcommand defines; it now points at cofferdam.toml's `exclude` and .cofferdamignore, the two mechanisms that exist. Found by diffing every flag named in the hand-written docs against the output of `cofferdam <subcommand> --help`, per CD-299. CD-301 has the full sweep, including seven shipped-but-undocumented flags handled separately. Verified: cargo test -p cofferdam-cli (21 passed), clippy -D warnings clean, cargo fmt --check clean, gen-docs --check exits 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019afugU2bTYCtXn6Vy9dn1v
`ProjectConfig` has no `exclude` field and no `[ignore]` table; discovery exclusion is `.cofferdamignore` / `.gitignore` only. The previous commit replaced one impossible flag with an impossible config key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019afugU2bTYCtXn6Vy9dn1v
…s not exist `UnsupportedSchemaVersion` told the user to run `cofferdam invariants migrate`. There is no `cofferdam invariants` subcommand — the clap enum has 20 variants and none of them is `Invariants`. Same defect as the two doctor hints in the previous commit, and worse placed: this message is read at the moment a spec has already been rejected and the build has stopped. Currently unreachable, since MIN_SUPPORTED equals CURRENT so no declared version can fall below the minimum. It arms itself at the first MAJOR bump, which is exactly when a wrong hint costs the most. Adds a regression test over both schema-version variants. 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) |
… future work `Consistency.UnusedSuppression` and `Consistency.BroadSuppression` both ship in consistency.rs, but the page described the first as "a future check (separate bead)" in two places. Name them. 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) |
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
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-301 (the code half).
The bug
cofferdam doctorprinted two remediation hints telling the user to run commands that exit 2.skipped — N files exceeds scan limit; pass --paths to scopedoctortakes no positional args and no--paths. Usage isdoctor [OPTIONS]; options are--pretty,--robot,--help.run cofferdam doctor from a narrower directory or add --pathssee `cofferdam explain --list`explainrequires<CHECK_ID>and defines no--list. Running it giveserror: unexpected argument '--list' found, exit 2.A remediation hint is read at the exact moment the user is already stuck. One naming a flag that was never implemented is worse than no hint, because it costs a second failed command before the user stops trusting the output.
docs/doctor.mdrepeated both verbatim, so the binary and the docs were consistently wrong together — which is why neither caught the other.The fix
Both hints are now named constants (
SCAN_LIMIT_HINT,STALE_ID_HINT) carrying accurate advice: rundoctorfrom a subdirectory, and reach the catalog viacofferdam explain <CHECK_ID>or the checks index.A test asserts neither hint names a CLI flag. That pins the invariant instead of restating it in a comment — the previous strings would fail it.
Also corrected
docs/suppression.md, which recommended--ignore-path. No subcommand defines that flag; the real mechanisms arecofferdam.toml'sexcludeand.cofferdamignore.How it was found
Per CD-299, I diffed every
--flagnamed in the hand-written docs againstcofferdam <subcommand> --helpfor all 13 subcommands. The generatedreference/cli.mdhad to be excluded from the comparison — it is produced from clap and therefore always agrees with the binary, which is exactly why it hides drift in the pages humans write.The same sweep found seven shipped-but-undocumented flags (
check --no-cache,--cache-dir,--hide-baselined,--time-checks,init --force,watch --debounce,gen-docs --out). Those are documentation-only and are tracked on CD-301 for the docs branch;--no-cachematters most, since cofferdam's ownCLAUDE.mdrequires it for real-repo validation and no public page mentions it.Verification
cargo test -p cofferdam-cli— 21 passedcargo clippy --all-targets -- -D warnings— cleancargo fmt --all -- --check— cleancofferdam gen-docs --check— exit 0🤖 Generated with Claude Code
https://claude.ai/code/session_019afugU2bTYCtXn6Vy9dn1v