Skip to content

test(audit): cover the shared deny note and the safe-probe suffix - #185

Open
brettdavies wants to merge 2 commits into
fix/help-flags-env-hint-placeholdersfrom
test/help-flags-review-coverage
Open

brettdavies wants to merge 2 commits into
fix/help-flags-env-hint-placeholdersfrom
test/help-flags-review-coverage

Conversation

@brettdavies

@brettdavies brettdavies commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

The last PR of the stack, and the record of its code review. It adds the tests the review found missing, and lists what the review raised and where each item went.

Tests added:

  • flag_presence::pass_or_warn, which seven audits share, is tested directly: a declared name passes, a miss warns with the message alone, and a single-dash spelling beside double-dash names adds the reason it does not count.
  • Six audits append that reason to a deny and none asserted it reached the message: p3-examples-subcommand, p1-rich-tui, p6-no-pager-behavioral, p2-schema-print, p7-cursor-pagination and p7-timeout-behavioral. Each has a help that declares the flag with one dash beside a double-dash name.
  • p2-json-output must never run a target without a --help or --version suffix. A stand-in prints JSON for any call that carries the output flag and neither suffix, and the row has to stay a skip.
  • Three guards on the classifier's sentence and usage rules (fix(audit): keep example commands, usage wraps and wrapped sentences out of the flag definitions #181) had no case of their own: a described row under a same-indent sentence that does not end, a name alone on its line under such a sentence, and one-space rows under a Usage: line that names only the program. Each stays a definition.

Code review of the stack

Three ce-code-review passes, each with an independent validator.

Pass 1, the stack. origin/dev... at the top of the stack (8134d09), with correctness, security, testing, maintainability, project-standards and adversarial reviewers and two searches of the solutions corpus. Receipt: status complete, verdict Ready with fixes, run 20261008-175155-34bd41cc. The adversarial lens ran in-process: the cross-model pass through the codex CLI returned Your workspace is out of credits before reviewing anything.

The validator confirmed all three findings. Each is fixed in the stack:

# finding fixed in
1 (P1) Under a clap short flag with no description, the long-only rows below it were read as its description #181
2 (P2) Indented example continuations, wrapped usage lines and wrapped sentences that start with a flag were read as definitions #181, in part (see below)
3 (P3) p7-quiet graded the partial output of a --help that crashed or timed out #182

Also taken from that pass:

item fixed in
confirm_flags entries without dashes stopped matching #182
typer and rich-click required options (* rows) were not read #183
The Thor gem in the probe image had no integrity check #183
rsync's p1-must-env-var passed on a value placeholder #184
Missing tests: the shared deny note, and the safe-probe suffix this PR

Pass 2, the fixes. Run 20261009-104233-76b8bcd7 over the first version of #181 to #185. Its correctness reviewer found a real layout that each of the three new text rules dropped (three findings, P2, P2 and P3) and named two risks: the long-column rule turning a wrapped description into a flag, and the env-hint rule hiding a $NAME written beside a flag's names. Its testing reviewer named three untested branches. All of that is fixed in #181 (the narrowing), #184 (the $NAME rule and the TAB-indented case) and this PR (the guard cases). The run has no receipt: its adversarial reviewer stalled and was stopped at its time limit, and pass 3 reviewed the result in its place.

Pass 3, the fixes again. 8134d09...2956e20, with correctness, testing, maintainability and project-standards reviewers and a cross-model adversarial review. Receipt: status complete, verdict Ready with fixes, run 20261009-112707-a595126d. The cross-model review ran through cursor-agent (requested composer-2.5-fast; served model unverified, so its findings count as attributed evidence, not independent agreement).

The validator confirmed the one finding that survived:

# finding fixed in
3 (P2) The wrapped-sentence rule dropped nmap's colon-described rows (-iL <inputfilename>: Input from list of hosts/networks and three more) #181: a colon that ends the names sets the description off

Also taken from that pass:

item fixed in
No test pinned a sentence that wraps onto two flag-led lines #181
No test for p7-quiet after a --help that timed out #182
A confirm_flags entry spelled +x or /x was given dashes and stopped matching #182
A placeholder on a TAB-indented line or in a box-table row was still read as an environment variable #184

Four other candidates were rejected with evidence. One, raised by two reviewers, said p7-quiet runs --help twice; the runner caches by arguments, so it is one run. One gave a trigger that is read as a definition. One asked for definitions from usage wraps, which the plan's requirements rule out. One, about bullets under a flag, showed no input where a flag is lost.

The fixes for pass 3 are the last commits on #181, #182 and #184. No review pass has read them. Each was written test-first, and the corpus before/after in each PR was run on the final heads.

Unapplied review findings

Each of these needs a decision that is not this stack's to make, or rests on a layout no real help has shown.

From pass 1 (run 20261008-175155-34bd41cc):

  • P2, src/runner/help_probe/flags/classify.rs, finding feat: complete v0.1 — 30 checks, 3 layers, test fixtures, README #2, the remainder: a flag-led sentence that opens its own paragraph is still read as a definition. terraform's -state, state-out, and -backup are legacy options supported for the local (in refresh, taint, untaint and import) is the known case, and no audit asks for -state. One reviewer proposed rejecting a line whose description opens with a connective (and, or, is, are, flag, option). Not applied: the plan recognizes layouts by shape, and a word list is a different kind of rule. aws prints real definitions as flag-led paragraphs, so shape alone cannot tell the two apart.
  • P2, src/audits/behavioral/error_probe.rs:49, pre-existing: p2-json-errors, p4-json-error-output and p2-consistent-envelope still gate on the characters --output plus json in the raw help, and probe with a literal --output. The plan names this gate under Scope Boundaries as deliberately lenient. Now that p2-json-output reads definitions, one scorecard can disagree with itself: opt_out on that row while these three still probe. Suggested fix: route the gate through find_flag and pass the declared spelling. It moves rows and needs its own expected-moves list.
  • P3, src/audits/behavioral/limit_flag.rs:156: a -n counts as a limit flag when its description names a count. Descriptions now include their wrapped lines (fix(audit): stop reading wrapped description lines as flag definitions #168), so a longer description is more likely to contain number of or lines. No corpus row moved for it. It belongs with the plan's deferred work on what a short letter means, which names this guard as its seed.

From pass 3 (run 20261009-112707-a595126d), residual risks. None is a finding; the reviewers reproduced each on a made-up layout only, and none occurs in the 71 fixtures, the 910 registry captures or 122 other help texts:

  • Wrapped-sentence rule, rows with no colon: a row whose description follows its names after one space ( -a do all the things), directly under same-indent text that does not end in ., :, ! or ?, is read as the rest of a sentence, and so are the rows under it until one ends a sentence. A gap or a colon before the description keeps the row.
  • Usage-wrap rule: rows printed directly under a usage: line, at or right of its arguments column, with a one-space description or none, are read as the synopsis wrapping.
  • Long-column rule: under a short flag with no description, a next-line description that starts with a dash-led name four columns right of the short ( -n NUM then -1 means no limit.) is read as a flag.
  • Usage wraps the rule does not reach, unchanged from before the stack: a synopsis under Usage: on a line of its own, and argparse's wrap to column 7 for a long program name, still declare their dash-led lines.
  • p1-must-env-var: a variable printed bare in a column of its own between the names and the description (--token GH_TOKEN API token) is blanked with the placeholders, because a placeholder column looks the same (--config-file -c CONFIG_FILE Path). A $GH_TOKEN there, or the name in the description, still counts. Whether a bare upper-case name in its own column should count is a product call; fix(audit): stop counting a flag's value placeholder as an environment variable #184 pins the current answer in a test.

Found while fixing, and older than the stack: python3's rows with a placeholder and a spaced colon (-c cmd : program passed in as string) are not read, because cmd reads as the start of a sentence at column 0. Its -b : issue warnings ... rows are read.

Changelog

No user-facing changes.

Type of Change

  • test: Adding or updating tests

Related Issues/Stories

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing completed
  • All tests passing

Test Summary:

  • 11 new tests and three new cases in an existing table, 1344 passing across the suite. Self-audit: cargo test --test dogfood passes.
  • No production code changes, so there is no corpus before/after for this PR.

Negatives observed failing. The tests pin behavior the code already has, so each was run against the code with its guard removed: the note dropped from HelpOutput::noting_dash_rule and from subcommand_help::dash_rule_notes, and the suffix dropped from the probe arguments in json_output.rs. These eight new tests failed, beside the existing ones that cover the same guards:

audits::behavioral::cursor_pagination::tests::a_single_dash_cursor_in_a_list_subcommand_warns_and_says_why
audits::behavioral::examples_subcommand::tests::a_single_dash_examples_beside_double_dash_names_warns_and_says_why
audits::behavioral::flag_presence::tests::a_single_dash_spelling_beside_double_dash_names_warns_and_says_why
audits::behavioral::json_output::tests::no_probe_is_sent_without_a_help_or_version_suffix
audits::behavioral::no_pager_behavioral::tests::a_single_dash_no_pager_beside_double_dash_names_warns_and_says_why
audits::behavioral::rich_tui::tests::a_single_dash_ui_beside_double_dash_names_warns_and_says_why
audits::behavioral::schema_print::tests::a_single_dash_schema_beside_double_dash_names_fails_and_says_why
audits::behavioral::timeout_behavioral::tests::a_single_dash_timeout_in_a_long_running_subcommand_warns_and_says_why

The other three new tests cover pass_or_warn's pass, its plain warn, and the Go flag pass.

The three classifier cases, each against the one guard it covers. Without the set-off guard on the sentence rule:

    "a described row under a sentence at the same indent that does not end: read [], declares [[\"--null\"]]",

Without the empty-description guard on the sentence rule:

    "a name alone on its line, under a sentence at the same indent that does not end: read [], declares [[\"--null\"]]",

With a usage line of fewer than three words taking its last word as the arguments column:

    "one-space rows under a usage line that names only the program: read [], declares [[\"-a\"], [\"-v\"]]",

Files Modified

Modified:

  • src/audits/behavioral/: flag_presence.rs, examples_subcommand.rs, rich_tui.rs, no_pager_behavioral.rs, schema_print.rs, cursor_pagination.rs, timeout_behavioral.rs, json_output.rs (tests only)
  • src/runner/help_probe/flags/classify.rs (test cases only)

Created:

  • None.

Renamed:

  • None.

Deleted:

  • None.

Breaking Changes

  • No breaking changes

Deployment Notes

  • No special deployment steps required

Checklist

  • Code follows project conventions and style guidelines
  • Commit messages follow Conventional Commits
  • Self-review of code completed
  • Tests added/updated and passing
  • No new warnings or errors introduced
  • Changes are backward compatible (or breaking changes documented)

@brettdavies
brettdavies added this pull request to stack #166 October 9, 2026 15:41
Three things the audits rely on had no test of their own:

- `flag_presence::pass_or_warn`, which seven audits share, is tested directly: a declared name passes, a miss warns with the message alone, and a single-dash spelling beside double-dash names adds the reason it does not count.
- Six audits append that reason to a deny and none asserted it reached the message: `p3-examples-subcommand`, `p1-rich-tui`, `p6-no-pager-behavioral`, `p2-schema-print`, `p7-cursor-pagination` and `p7-timeout-behavioral`. Each now has a help that declares the flag with one dash beside a double-dash name.
- `p2-json-output` must never run a target without a `--help` or `--version` suffix. A stand-in now prints JSON for any call that carries the output flag and neither suffix, and the row has to stay a skip.

Each new test was run against the code with its guard removed (the note dropped from `noting_dash_rule` and `dash_rule_notes`, the suffix dropped from the probe arguments) and failed.
Three cases join the table of definitions that sit beside prose and usage lines. Each was observed failing under a mutation of the guard it covers.

- A described row directly under a same-indent sentence that does not end stays a definition, because a gap sets its description off. Dropping the set-off guard from the sentence rule reads it as prose.
- A name alone on its line under such a sentence stays a definition. Dropping the empty-description guard reads it as prose.
- One-space rows under `Usage: tool` stay definitions: a usage line that names only the program has no arguments column. Falling back to its last word as that column reads the rows as a synopsis wrap.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant