Repository navigation
feat(p3): add SHOULD for command-list entries that repeat the binary name - #53
Merged
Merged
Conversation
13 of 25 tasks
…name Hand-written help often lists commands as `tool status`, `tool server stop`, with the binary name at the head of every entry. A reader or agent scanning the block has to strip that prefix before it can read the command token, and an auditor that reads the block literally sees the binary name repeated instead of the commands. `p3-should-unprefixed-command-list` asks for the command token to sit in the left column on its own, with the bare invocation line left as the one legitimate carrier of the binary name alone. The requirement is a SHOULD, not a MUST: the behavioral layer already grades this surface with warnings rather than failures, and a strict rule would penalize the hand-written help that much of the audited population ships. It applies only to CLIs that use subcommands.
brettdavies
force-pushed
the
feat/p3-should-unprefixed-command-list
branch
from
September 17, 2026 04:59
e31129f to
d593b3d
Compare
14 of 27 tasks
brettdavies
added a commit
to brettdavies/agentnative-cli
that referenced
this pull request
Sep 17, 2026
…their real subcommands (#94) ## Summary A CLI whose `--help` lists its commands under a hand-written heading such as `Common commands:`, with every entry led by the binary name, parsed as having no subcommands. Fifteen behavioral audits read that empty list: ten degraded to skip and left the score denominator, and five answered on no evidence, so anc reported "binary has no subcommands" about herdr, a tool with sixteen. This PR teaches the shared help parser the hand-written shape, collapses the JSON-output audit's private copy of the parser onto it, and guards a substring matcher the fix would otherwise turn into false MUST failures. The parser opens a block on any header whose last word is `commands:` or `subcommands:`, takes the tool's own name from the `Usage:` line and from the binary's file stem, and strips that token from a block's entries only when every entry leads with it. A line indented more deeply than the block's first entry is a continuation of the previous entry (a wrapped description or a nested subcommand), not an entry of its own, so it can neither break the prefix agreement nor become a name. A name is the first token of the invocation part of each entry, so `herdr server stop` contributes `server`, `herdr machine <subcommand>` contributes `machine`, and the bare `herdr` line contributes nothing. Duplicates collapse and `.subcommands()` keeps its type, so none of the fifteen consumers changes. A new `command_blocks()` accessor exposes each block's header, raw entries, and stripped prefix, and `missing_subcommands_reason()` lets an audit say whether it found no block or found one it could not read. Destructive-verb matching moves from substring to segment-prefix so `format`, `transform`, `perform`, `confirm`, and `firmware` stop classifying as destructive while `delete-all`, `dropdb`, `rmdir`, `cleanup`, `reset-keys`, and `force-push` still do. This lands ahead of the parser change because the parser is what makes the substring rule reachable on hand-written-help CLIs. Plan: `docs/plans/2026-09-16-1542-fix-hand-written-help-subcommand-parsing-plan.md` (units U3, U1, U2; U4 is #95). ## Changelog ### Changed - Change scores for CLIs with a hand-written command list: ten audits that skipped on "no subcommands" now evaluate real names, and five that answered on no evidence now answer on real ones, so scores can move in either direction. herdr moves from 77 to 72 and stays badge-eligible. ### Fixed - Fix the help parser so a hand-written `... commands:` block whose entries repeat the binary name yields the real top-level command names instead of no subcommands. - Fix `p5-force-yes` and `p5-read-write-distinction` classifying `format`, `transform`, `perform`, `confirm`, and `firmware` as destructive because they contain `rm`. - Fix `p3-subcommand-examples` and `p6-standard-names` evidence to say whether no command block was found or a block was found but could not be read, instead of asserting the tool has no subcommands. - Fix `p2-json-output` opting out without probing any subcommand on a CLI whose command list is hand-written. ## Type of Change - [x] `fix`: Bug fix (non-breaking change which adds functionality) - [x] `refactor`: Code refactoring (no functional changes) - [ ] `feat`: New feature (non-breaking change which adds functionality) - [ ] `perf`: Performance improvement - [ ] `docs`: Documentation update - [ ] `test`: Adding or updating tests - [ ] `chore`: Maintenance tasks (dependencies, config, etc.) - [ ] `ci`: CI/CD configuration changes - [ ] `style`: Code style/formatting changes - [ ] `build`: Build system changes - [ ] `BREAKING CHANGE`: Breaking API change (requires major version bump) ## Related Issues/Stories - Story: `docs/plans/2026-09-16-1542-fix-hand-written-help-subcommand-parsing-plan.md` - Issue: n/a - Architecture: `docs/solutions/workflow-issues/anc-pager-substring-false-positive-2026-06-02.md` (the recorded sibling defect: a heuristic tuned to one vendor's output shape, applied to third-party text, failing silently) - Related PRs: follows #92 and #93, both merged ahead of this one; #95 (U4) follows, with its requirement from `brettdavies/agentnative` PR brettdavies/agentnative#53 (merged) ## Testing - [x] Unit tests added/updated - [x] Integration tests added/updated - [x] Manual testing completed - [x] All tests passing **Test Summary:** - Unit tests: 763 passing (1 ignored, pre-existing) - Integration tests: 123 passing across the seven integration binaries, including the new `test_handwritten_help_fixture_reports_real_subcommands` - Coverage: n/a (not measured in this repo) Each new test was observed failing against the unfixed code before the fix landed. The destructive-verb guard (U3), against the substring matcher: ```text ---- audits::behavioral::destructive_ops::tests::verb_inside_a_word_is_not_destructive stdout ---- thread '...' panicked at src/audits/behavioral/destructive_ops.rs:105:13: format should not be destructive ``` The hand-written block parse (U1), against the three-header parser: ```text ---- runner::help_probe::tests::hand_written_block_yields_top_level_names_without_the_tool_prefix stdout ---- assertion `left == right` failed left: [] right: ["status", "update", "completion", "server", "channel", "config", "machine", "api"] ``` The JSON-output probe on a hand-written block (U2), against the private parser: ```text ---- audits::behavioral::json_output::tests::json_output_probes_hand_written_command_block stdout ---- assertion `left == right` failed: got OptOut("no --output/--format flag detected — tool does not ship structured output. ...") left: OptOut("...") right: Pass ``` The four parser tests added from code review, against the parser as first written: ```text single_space_bare_entry_in_a_prefixed_block_is_description_only left: ["Launch", "status"] right: ["status"] deeper_indented_nested_lines_are_continuations_of_their_parent_entry left: ["server", "start", "stop"] right: ["server"] wrapped_description_continuation_keeps_the_prefixed_block_intact left: ["herdr", "server"] right: ["status", "update", "completion", "server", "channel", "config", "machine", "api"] probe_strips_the_binary_stem_when_the_usage_line_leads_with_a_launcher left: ["test"] right: ["status", "server"] ``` Manual verification: `anc audit .` on this repo changes no scorecard row against the base branch. `anc audit $(command -v herdr)` moves four rows from skip to real verdicts (`p6-standard-names` warn, `p6-consistent-naming` warn, `p3-subcommand-examples` fail, `p5-read-write-distinction` warn) and every other row keeps its status. `anc audit tests/fixtures/handwritten-help/tally` reports the fixture's seven real commands, passes `p2-json-output` through the `count` subcommand probe, and skips `p5-force-yes` because `format` is no longer destructive. ## Files Modified **Modified:** - `src/runner/help_probe/mod.rs`: command-block parser, `CommandBlock`, `command_blocks()`, `missing_subcommands_reason()`, usage-line tool name, stale allows and doc claims removed - `src/runner/mod.rs`: `BinaryRunner::binary_stem()` for the probe's tool-name fallback - `src/audits/behavioral/destructive_ops.rs`: segment-prefix destructive-verb matching - `src/audits/behavioral/json_output.rs`: shared parser via the project's cached help probe, built-ins filtered at the call site - `src/audits/behavioral/subcommand_help.rs`: `should_skip` shared with the JSON-output audit - `src/audits/behavioral/subcommand_examples.rs`: evidence wording on an empty parse - `src/audits/behavioral/standard_names.rs`: evidence wording on an empty parse - `tests/integration.rs`: end-to-end audit of the hand-written fixture **Created:** - `tests/fixtures/handwritten-help/tally`: shell fixture in the herdr shape (usage line, two prefixed `... commands:` blocks, per-subcommand help, `--output json` on one subcommand) **Renamed:** - None. **Deleted:** - None. ## Key Features - One parser serves every consumer of subcommand names, and it now reads clap, cobra (`Available Commands:`), and hand-written (`Common commands:`) blocks alike. - Evidence tells the truth about what the parser saw: "no command block found" or "a block was found but no names could be parsed", never "the tool has no subcommands". ## Benefits - Tools with hand-written help are graded on what they do. Ten audits re-enter the denominator and five stop answering on no evidence. - No CLI gains a MUST failure that is not a true statement about it: the destructive-verb guard lands in the same PR as the parser that would have exposed it. ## Breaking Changes - [x] No breaking changes - [ ] Breaking changes described below: ## Deployment Notes - [x] No special deployment steps required - [ ] Deployment steps documented below: ## Checklist - [x] Code follows project conventions and style guidelines - [x] Commit messages follow [Conventional Commits](https://www.conventionalcommits.org/) - [x] Self-review of code completed - [x] Tests added/updated and passing - [x] No new warnings or errors introduced - [x] Changes are backward compatible (or breaking changes documented) ## Additional Context ### Post-Deploy Monitoring & Validation No production runtime is deployed by this repo; the artifact is the `anc` binary. After the next release, re-audit every tool on the site registry and record each badge-eligibility crossing: a score that falls because a hand-written-help tool is now graded on real names is the intended change, not a regression. A regression signal would be a clap-shaped tool whose scorecard rows change between the previous release and this one; `anc audit .` on this repo is the committed guard for that and changes no row. ### Review Code review ran through the compound-engineering review skill (run `20260916-220231-1a8ade92`, status complete, verdict "Ready with fixes") with correctness, project-standards, testing, maintainability, learnings, and adversarial lenses. The cross-model adversarial peer did not run: the installed `grok` CLI rejects the runner's `--json-schema` argument at launch, so no content left the machine and the in-process adversarial reviewer covered that lens. Every actionable finding was confirmed by an independent validator and applied in this PR: continuation lines in a prefixed block, the binary stem as a second prefix candidate, the single-space bare-invocation entry, the delimiter-branch destructive-verb test, and the module doc's view list. ### Unapplied review findings Both were demoted to residual risks by the review and are deferred as follow-ups rather than applied here. - [ ] P3, `src/audits/behavioral/destructive_ops.rs:39`: Segment-prefix matching no longer catches fused verbs such as `autoremove` and `autoclean`, which substring matching did. The plan's R9 defines the matcher as start-of-name or start-of-segment; adding the fused forms to the verb list with a failing-first test is the fix if recall matters. - [ ] P3, `src/runner/help_probe/mod.rs:401`: Cobra group headers with a parenthetical before the colon (`Basic Commands (Beginner):`, as kubectl prints) are not recognized, so such tools parse a partial name set. Not a regression, and outside R1 as written; stripping a trailing parenthetical before testing the last word would cover it. Residual risks the review recorded and this PR does not change: every parsed name is spawned as `<bin> <name> --help`, and a hand-written subcommand that ignores `--help` runs until the runner's timeout; the pager audit's substring matcher is the still-open sibling of the destructive-verb defect this PR fixes.
brettdavies
added a commit
to brettdavies/agentnative-cli
that referenced
this pull request
Sep 17, 2026
…their real subcommands (#94) ## Summary A CLI whose `--help` lists its commands under a hand-written heading such as `Common commands:`, with every entry led by the binary name, parsed as having no subcommands. Fifteen behavioral audits read that empty list: ten degraded to skip and left the score denominator, and five answered on no evidence, so anc reported "binary has no subcommands" about herdr, a tool with sixteen. This PR teaches the shared help parser the hand-written shape, collapses the JSON-output audit's private copy of the parser onto it, and guards a substring matcher the fix would otherwise turn into false MUST failures. The parser opens a block on any header whose last word is `commands:` or `subcommands:`, takes the tool's own name from the `Usage:` line and from the binary's file stem, and strips that token from a block's entries only when every entry leads with it. A line indented more deeply than the block's first entry is a continuation of the previous entry (a wrapped description or a nested subcommand), not an entry of its own, so it can neither break the prefix agreement nor become a name. A name is the first token of the invocation part of each entry, so `herdr server stop` contributes `server`, `herdr machine <subcommand>` contributes `machine`, and the bare `herdr` line contributes nothing. Duplicates collapse and `.subcommands()` keeps its type, so none of the fifteen consumers changes. A new `command_blocks()` accessor exposes each block's header, raw entries, and stripped prefix, and `missing_subcommands_reason()` lets an audit say whether it found no block or found one it could not read. Destructive-verb matching moves from substring to segment-prefix so `format`, `transform`, `perform`, `confirm`, and `firmware` stop classifying as destructive while `delete-all`, `dropdb`, `rmdir`, `cleanup`, `reset-keys`, and `force-push` still do. This lands ahead of the parser change because the parser is what makes the substring rule reachable on hand-written-help CLIs. Plan: `docs/plans/2026-09-16-1542-fix-hand-written-help-subcommand-parsing-plan.md` (units U3, U1, U2; U4 is #95). ## Changelog ### Changed - Change scores for CLIs with a hand-written command list: ten audits that skipped on "no subcommands" now evaluate real names, and five that answered on no evidence now answer on real ones, so scores can move in either direction. herdr moves from 77 to 72 and stays badge-eligible. ### Fixed - Fix the help parser so a hand-written `... commands:` block whose entries repeat the binary name yields the real top-level command names instead of no subcommands. - Fix `p5-force-yes` and `p5-read-write-distinction` classifying `format`, `transform`, `perform`, `confirm`, and `firmware` as destructive because they contain `rm`. - Fix `p3-subcommand-examples` and `p6-standard-names` evidence to say whether no command block was found or a block was found but could not be read, instead of asserting the tool has no subcommands. - Fix `p2-json-output` opting out without probing any subcommand on a CLI whose command list is hand-written. ## Type of Change - [x] `fix`: Bug fix (non-breaking change which adds functionality) - [x] `refactor`: Code refactoring (no functional changes) - [ ] `feat`: New feature (non-breaking change which adds functionality) - [ ] `perf`: Performance improvement - [ ] `docs`: Documentation update - [ ] `test`: Adding or updating tests - [ ] `chore`: Maintenance tasks (dependencies, config, etc.) - [ ] `ci`: CI/CD configuration changes - [ ] `style`: Code style/formatting changes - [ ] `build`: Build system changes - [ ] `BREAKING CHANGE`: Breaking API change (requires major version bump) ## Related Issues/Stories - Story: `docs/plans/2026-09-16-1542-fix-hand-written-help-subcommand-parsing-plan.md` - Issue: n/a - Architecture: `docs/solutions/workflow-issues/anc-pager-substring-false-positive-2026-06-02.md` (the recorded sibling defect: a heuristic tuned to one vendor's output shape, applied to third-party text, failing silently) - Related PRs: follows #92 and #93, both merged ahead of this one; #95 (U4) follows, with its requirement from `brettdavies/agentnative` PR brettdavies/agentnative#53 (merged) ## Testing - [x] Unit tests added/updated - [x] Integration tests added/updated - [x] Manual testing completed - [x] All tests passing **Test Summary:** - Unit tests: 763 passing (1 ignored, pre-existing) - Integration tests: 123 passing across the seven integration binaries, including the new `test_handwritten_help_fixture_reports_real_subcommands` - Coverage: n/a (not measured in this repo) Each new test was observed failing against the unfixed code before the fix landed. The destructive-verb guard (U3), against the substring matcher: ```text ---- audits::behavioral::destructive_ops::tests::verb_inside_a_word_is_not_destructive stdout ---- thread '...' panicked at src/audits/behavioral/destructive_ops.rs:105:13: format should not be destructive ``` The hand-written block parse (U1), against the three-header parser: ```text ---- runner::help_probe::tests::hand_written_block_yields_top_level_names_without_the_tool_prefix stdout ---- assertion `left == right` failed left: [] right: ["status", "update", "completion", "server", "channel", "config", "machine", "api"] ``` The JSON-output probe on a hand-written block (U2), against the private parser: ```text ---- audits::behavioral::json_output::tests::json_output_probes_hand_written_command_block stdout ---- assertion `left == right` failed: got OptOut("no --output/--format flag detected — tool does not ship structured output. ...") left: OptOut("...") right: Pass ``` The four parser tests added from code review, against the parser as first written: ```text single_space_bare_entry_in_a_prefixed_block_is_description_only left: ["Launch", "status"] right: ["status"] deeper_indented_nested_lines_are_continuations_of_their_parent_entry left: ["server", "start", "stop"] right: ["server"] wrapped_description_continuation_keeps_the_prefixed_block_intact left: ["herdr", "server"] right: ["status", "update", "completion", "server", "channel", "config", "machine", "api"] probe_strips_the_binary_stem_when_the_usage_line_leads_with_a_launcher left: ["test"] right: ["status", "server"] ``` Manual verification: `anc audit .` on this repo changes no scorecard row against the base branch. `anc audit $(command -v herdr)` moves four rows from skip to real verdicts (`p6-standard-names` warn, `p6-consistent-naming` warn, `p3-subcommand-examples` fail, `p5-read-write-distinction` warn) and every other row keeps its status. `anc audit tests/fixtures/handwritten-help/tally` reports the fixture's seven real commands, passes `p2-json-output` through the `count` subcommand probe, and skips `p5-force-yes` because `format` is no longer destructive. ## Files Modified **Modified:** - `src/runner/help_probe/mod.rs`: command-block parser, `CommandBlock`, `command_blocks()`, `missing_subcommands_reason()`, usage-line tool name, stale allows and doc claims removed - `src/runner/mod.rs`: `BinaryRunner::binary_stem()` for the probe's tool-name fallback - `src/audits/behavioral/destructive_ops.rs`: segment-prefix destructive-verb matching - `src/audits/behavioral/json_output.rs`: shared parser via the project's cached help probe, built-ins filtered at the call site - `src/audits/behavioral/subcommand_help.rs`: `should_skip` shared with the JSON-output audit - `src/audits/behavioral/subcommand_examples.rs`: evidence wording on an empty parse - `src/audits/behavioral/standard_names.rs`: evidence wording on an empty parse - `tests/integration.rs`: end-to-end audit of the hand-written fixture **Created:** - `tests/fixtures/handwritten-help/tally`: shell fixture in the herdr shape (usage line, two prefixed `... commands:` blocks, per-subcommand help, `--output json` on one subcommand) **Renamed:** - None. **Deleted:** - None. ## Key Features - One parser serves every consumer of subcommand names, and it now reads clap, cobra (`Available Commands:`), and hand-written (`Common commands:`) blocks alike. - Evidence tells the truth about what the parser saw: "no command block found" or "a block was found but no names could be parsed", never "the tool has no subcommands". ## Benefits - Tools with hand-written help are graded on what they do. Ten audits re-enter the denominator and five stop answering on no evidence. - No CLI gains a MUST failure that is not a true statement about it: the destructive-verb guard lands in the same PR as the parser that would have exposed it. ## Breaking Changes - [x] No breaking changes - [ ] Breaking changes described below: ## Deployment Notes - [x] No special deployment steps required - [ ] Deployment steps documented below: ## Checklist - [x] Code follows project conventions and style guidelines - [x] Commit messages follow [Conventional Commits](https://www.conventionalcommits.org/) - [x] Self-review of code completed - [x] Tests added/updated and passing - [x] No new warnings or errors introduced - [x] Changes are backward compatible (or breaking changes documented) ## Additional Context ### Post-Deploy Monitoring & Validation No production runtime is deployed by this repo; the artifact is the `anc` binary. After the next release, re-audit every tool on the site registry and record each badge-eligibility crossing: a score that falls because a hand-written-help tool is now graded on real names is the intended change, not a regression. A regression signal would be a clap-shaped tool whose scorecard rows change between the previous release and this one; `anc audit .` on this repo is the committed guard for that and changes no row. ### Review Code review ran through the compound-engineering review skill (run `20260916-220231-1a8ade92`, status complete, verdict "Ready with fixes") with correctness, project-standards, testing, maintainability, learnings, and adversarial lenses. The cross-model adversarial peer did not run: the installed `grok` CLI rejects the runner's `--json-schema` argument at launch, so no content left the machine and the in-process adversarial reviewer covered that lens. Every actionable finding was confirmed by an independent validator and applied in this PR: continuation lines in a prefixed block, the binary stem as a second prefix candidate, the single-space bare-invocation entry, the delimiter-branch destructive-verb test, and the module doc's view list. ### Unapplied review findings Both were demoted to residual risks by the review and are deferred as follow-ups rather than applied here. - [ ] P3, `src/audits/behavioral/destructive_ops.rs:39`: Segment-prefix matching no longer catches fused verbs such as `autoremove` and `autoclean`, which substring matching did. The plan's R9 defines the matcher as start-of-name or start-of-segment; adding the fused forms to the verb list with a failing-first test is the fix if recall matters. - [ ] P3, `src/runner/help_probe/mod.rs:401`: Cobra group headers with a parenthetical before the colon (`Basic Commands (Beginner):`, as kubectl prints) are not recognized, so such tools parse a partial name set. Not a regression, and outside R1 as written; stripping a trailing parenthetical before testing the last word would cover it. Residual risks the review recorded and this PR does not change: every parsed name is spawned as `<bin> <name> --help`, and a hand-written subcommand that ignores `--help` runs until the runner's timeout; the pager audit's substring matcher is the still-open sibling of the destructive-verb defect this PR fixes.
brettdavies
added a commit
to brettdavies/agentnative-cli
that referenced
this pull request
Sep 17, 2026
## Summary Adds the P3 SHOULD check for command lists that repeat the binary name. A hand-written `Common commands:` block whose entries read `tool status`, `tool server stop` makes a reader or agent strip the prefix before the command token is visible. The new `p3-unprefixed-command-list` audit reads the command blocks the help probe already parses (from the parser change in #94): a block whose entries all led with the tool name is prefixed, and every entry that names a command beyond that bare prefix is an offender. The bare-invocation entry, which documents what the tool does with no arguments, is not one, and a block whose header names examples (`Example commands:`) is not graded, since examples are where the spec expects full invocations. The audit warns and names the offending invocations (the first five, then a count), passes when every graded block is unprefixed, and resolves not-applicable when no subcommand names were parsed, on the same condition its P3 and P6 siblings use, with the shared reason saying whether no block was found or a block was found but could not be read. The requirement `p3-should-unprefixed-command-list` is authored upstream in the spec and vendored here from `8b84077`, the squash-merge of spec PR #53 on the spec's `dev` branch (see Deployment Notes). The registry grows from 59 to 60 requirements (22 SHOULDs) and the coverage matrix is regenerated to match. Plan: `docs/plans/2026-09-16-1542-fix-hand-written-help-subcommand-parsing-plan.md`, unit U4. ## Changelog ### Added - Add the `p3-unprefixed-command-list` audit for the new SHOULD `p3-should-unprefixed-command-list`: a `--help` command list whose entries repeat the binary name warns with the offending entries named; a clap-shaped list passes; a tool whose help yields no subcommand names is not applicable; an examples section is never graded. ### Changed - Change scores for every CLI that lists commands in `--help`: the new SHOULD row enters the denominator, as a pass for clap-shaped lists and a warn for prefixed ones. herdr moves from 72 to 71 and stays badge-eligible; this repository gains a passing row. ## Type of Change - [x] `feat`: New feature (non-breaking change which adds functionality) - [ ] `fix`: Bug fix (non-breaking change which adds functionality) - [ ] `refactor`: Code refactoring (no functional changes) - [ ] `perf`: Performance improvement - [ ] `docs`: Documentation update - [ ] `test`: Adding or updating tests - [ ] `chore`: Maintenance tasks (dependencies, config, etc.) - [ ] `ci`: CI/CD configuration changes - [ ] `style`: Code style/formatting changes - [ ] `build`: Build system changes - [ ] `BREAKING CHANGE`: Breaking API change (requires major version bump) ## Related Issues/Stories - Story: `docs/plans/2026-09-16-1542-fix-hand-written-help-subcommand-parsing-plan.md` (U4) - Issue: n/a - Architecture: `CLAUDE.md` § Principle Registry (requirements are generated from the vendored spec frontmatter; the counter tests are bumped deliberately) - Related PRs: follows #94, merged ahead of this one; the requirement comes from `brettdavies/agentnative` PR brettdavies/agentnative#53 (merged) ## Testing - [x] Unit tests added/updated - [x] Integration tests added/updated - [x] Manual testing completed - [x] All tests passing **Test Summary:** - Unit tests: 772 passing (1 ignored, pre-existing) - Integration tests: 123 passing across the seven integration binaries - Coverage: n/a (not measured in this repo) Each guard was observed failing before the audit and counters landed. With the vendored requirement in place and nothing else changed: ```text thread 'principles::registry::tests::registry_size_matches_spec' panicked at src/principles/registry.rs:379:9: left: 60 right: 59 thread 'principles::registry::tests::level_counts_match_spec' panicked at src/principles/registry.rs:385:9: left: 22 right: 21 error: docs/coverage-matrix.md is out of date; run `anc emit coverage-matrix` error: coverage/matrix.json is out of date; run `anc emit coverage-matrix` ``` The fixture assertion for the new row, before the audit was registered: ```text thread 'test_handwritten_help_fixture_reports_real_subcommands' panicked at tests/integration.rs:284:32: no row for p3-unprefixed-command-list ``` The three tests added from code review, against the audit as first written: ```text bare_invocation_alone_does_not_warn: assertion failed: matches!(audit_unprefixed_command_list(&help), AuditStatus::NotApplicable(_)) block_with_no_parseable_names_is_not_applicable: expected NotApplicable, got Warn("command-list entries repeat the binary name `modes`: `modes --init`, `modes --build`. ...") example_block_is_not_graded: left: Warn("command-list entries repeat the binary name `exa`: `exa init --name foo`, `exa build --release`. ...") right: Pass ``` The bare-invocation guards in the unit test and the fixture test were proven live by removing the empty-command filter, observing both fail, and restoring it: ```text prefixed_block_warns_and_names_the_offending_lines: bare invocation must not be listed: ... `herdr`: `herdr `, `herdr status [server|client]`, ... test_handwritten_help_fixture_reports_real_subcommands: prefix evidence should name prefixed entries and not the bare invocation: ... `tally`: `tally `, `tally count <path>`, ... and 3 more. ``` Manual verification: `anc emit coverage-matrix --check` exits 0 after regeneration. `anc audit .` on this repository adds exactly one row against #94, `p3-unprefixed-command-list` as pass, and changes no other row. `anc audit tests/fixtures/handwritten-help/tally` warns once on the new row and names `tally count <path>` while leaving the bare `tally` entry out of the evidence. `anc audit $(command -v herdr)` warns once, naming its prefixed entries and counting the rest. ## Files Modified **Modified:** - `src/principles/spec/principles/p3-progressive-help-discovery.md`: vendored spec at `8b84077` with the new SHOULD - `src/audits/behavioral/mod.rs`: audit registered - `src/runner/mod.rs`: `CommandBlock` re-exported alongside `HelpOutput` - `src/principles/registry.rs`: counter tests bumped to 60 requirements and 22 SHOULDs - `tests/build_parser.rs`: vendored-spec count bumped to 60 - `tests/integration.rs`: the hand-written fixture asserts the new warn row and its evidence - `docs/coverage-matrix.md`, `coverage/matrix.json`: regenerated **Created:** - `src/audits/behavioral/unprefixed_command_list.rs`: the audit and its unit tests **Renamed:** - None. **Deleted:** - None. ## Key Features - The evidence names the offending invocations as written up to the description gap (`herdr status [server|client]`), so the remediation is legible without reopening the help text. - The check reads the same parsed blocks the parser produces, so the audit and the parser cannot disagree about what counts as a prefix. ## Benefits - Tools with hand-written, binary-prefixed command lists get one actionable warning instead of silently losing their subcommand grades. - Clap-shaped tools gain a full-credit row. ## Breaking Changes - [x] No breaking changes - [ ] Breaking changes described below: ## Deployment Notes - [ ] No special deployment steps required - [x] Deployment steps documented below: The requirement is vendored from the spec's `dev` branch at `8b84077` (the squash-merge of spec PR #53), not from a tagged spec release, because the audit cannot compile until the requirement id resolves. The commit on this branch was vendored from the spec branch head `d593b3d` before that merge; `scripts/sync-spec.sh --ref 8b84077` run after the merge produces a byte-identical tree, so there is nothing further to commit here. The vendored `VERSION` stays `0.5.0`, which the scorecard reports as `spec_version`, while the published v0.5.0 spec has 59 requirements; cut a spec release that includes this SHOULD before the next CLI release, so the pin names a published spec that contains it. The normal tag sync picks it up at that point. ## Screenshots/Recordings n/a ## Checklist - [x] Code follows project conventions and style guidelines - [x] Commit messages follow [Conventional Commits](https://www.conventionalcommits.org/) - [x] Self-review of code completed - [x] Tests added/updated and passing - [x] No new warnings or errors introduced - [x] Changes are backward compatible (or breaking changes documented) ## Additional Context ### Post-Deploy Monitoring & Validation No production runtime is deployed by this repo; the artifact is the `anc` binary. After the next release, every tool on the site registry gains one new SHOULD row. A prefixed hand-written list warning is the intended change. A regression signal would be a clap-shaped tool whose new row is anything but pass, or a tool with no command list whose row is anything but not-applicable; `anc audit .` on this repository is the committed guard for the pass case. ### Review Code review ran through the compound-engineering review skill (run `20260916-231619-bf850931`, status complete, verdict "Ready with fixes") with correctness, project-standards, testing, maintainability, learnings, and adversarial lenses; the cross-model adversarial peer was skipped on the route failure observed earlier in the session (the installed `grok` CLI rejects the runner's arguments at launch, so nothing left the machine) and the in-process adversarial reviewer covered that lens. All three confirmed findings were applied in this PR: the applicability gate now matches the sibling audits (not applicable when no subcommand names were parsed), example-style blocks are excluded from grading, and the bare-invocation guards in both tests now assert on a rendering the formatter can produce. ### Unapplied review findings None of the review's actionable findings were deferred. Residual risks the review recorded and this PR does not change: - The warn header names the prefix of the first offending entry; two blocks resolving to different prefixes would show only the first in the header (each quoted entry carries its own prefix). - An identical entry repeated across two blocks counts twice in the "and N more" tally. - The requirement is vendored from the spec's `dev` branch ahead of a spec release; see Deployment Notes for the release ordering, and the site's scorecard regeneration is where badge eligibility shifts for tools sitting at the 70% floor. - No `--audit-profile` category suppresses the new SHOULD; whether one should is a judgment call the drift tests do not make.
brettdavies
added a commit
to brettdavies/agentnative-cli
that referenced
this pull request
Sep 17, 2026
…hipped The UTF-8 evidence-preview plan and the hand-written-help subcommand-parsing plan both carry `status: completed`, and their bodies state what landed: the UTF-8 fix as PR #92 with its guard as PR #93, and the parser work as PR #94 (U3, U1, U2) with the new SHOULD check as PR #95, which vendors spec PR brettdavies/agentnative#53. The parsing plan's requirements and decisions now match the shipped parser: a block strips only when every entry leads with the tool's own name from the Usage line or the binary stem, `subcommands:` headers count, deeper-indented lines continue the previous entry, and a single-space bare entry is description only. Unit file lists name the files each PR touched, the audit is `p3-unprefixed-command-list`, the vendored VERSION stays at 0.5.0, and the follow-up list gains the fused verbs, kubectl's parenthetical headers, the release-time re-audit, and the spec release the scorecard's spec_version pin needs.
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.
Summary
Adds a SHOULD to P3 for the shape of the command list in
--help. Hand-written help often lists commands astool status,tool server stop, with the binary name at the head of every entry. A reader or agent scanning the block has to strip that prefix before it can read the command token, and an auditor that reads the block literally sees the binary name repeated instead of the commands.p3-should-unprefixed-command-listasks for the command token to sit in the left column on its own; the bare invocation line, which documents what the tool does with no arguments, stays the one legitimate carrier of the binary name alone.The requirement is a SHOULD rather than a MUST: the behavioral layer already grades this surface with warnings rather than failures, and a strict rule would penalize the hand-written help that much of the audited population ships. It applies only to CLIs that use subcommands, matching
p3-must-subcommand-examples. The Evidence and Anti-Patterns sections gain one bullet each, andlast-revisedmoves to 2026-09-17.Changelog
Added
p3-should-unprefixed-command-list(SHOULD, applies when the CLI uses subcommands): command-list entries name the command directly rather than repeating the binary name as a prefix.Linked audit review
brettdavies/agentnative-cli#95 (vendors this branch at
d593b3dand adds thep3-unprefixed-command-listaudit)Human reviewer
Reviewer: @brettdavies
AI disclosure
The requirement's tier and firing condition were decided by the maintainer during planning; the frontmatter entry, prose bullets, and this PR body were drafted by Claude Code (Fable 5.1) from that plan and are pending the maintainer's review.