Repository navigation
test(audit): cover the shared deny note and the safe-probe suffix - #185
Open
brettdavies wants to merge 2 commits into
Open
brettdavies wants to merge 2 commits into
brettdavies wants to merge 2 commits into
Conversation
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.
brettdavies
force-pushed
the
test/help-flags-review-coverage
branch
from
October 9, 2026 17:25
dcff7c5 to
14ef85f
Compare
12 of 13 tasks
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
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.p3-examples-subcommand,p1-rich-tui,p6-no-pager-behavioral,p2-schema-print,p7-cursor-paginationandp7-timeout-behavioral. Each has a help that declares the flag with one dash beside a double-dash name.p2-json-outputmust never run a target without a--helpor--versionsuffix. A stand-in prints JSON for any call that carries the output flag and neither suffix, and the row has to stay a skip.Usage:line that names only the program. Each stays a definition.Code review of the stack
Three
ce-code-reviewpasses, 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: statuscomplete, verdictReady with fixes, run20261008-175155-34bd41cc. The adversarial lens ran in-process: the cross-model pass through thecodexCLI returnedYour workspace is out of creditsbefore reviewing anything.The validator confirmed all three findings. Each is fixed in the stack:
p7-quietgraded the partial output of a--helpthat crashed or timed outAlso taken from that pass:
confirm_flagsentries without dashes stopped matching*rows) were not readp1-must-env-varpassed on a value placeholderPass 2, the fixes. Run
20261009-104233-76b8bcd7over 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$NAMEwritten beside a flag's names. Its testing reviewer named three untested branches. All of that is fixed in #181 (the narrowing), #184 (the$NAMErule 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: statuscomplete, verdictReady with fixes, run20261009-112707-a595126d. The cross-model review ran throughcursor-agent(requestedcomposer-2.5-fast; served model unverified, so its findings count as attributed evidence, not independent agreement).The validator confirmed the one finding that survived:
-iL <inputfilename>: Input from list of hosts/networksand three more)Also taken from that pass:
p7-quietafter a--helpthat timed outconfirm_flagsentry spelled+xor/xwas given dashes and stopped matchingFour other candidates were rejected with evidence. One, raised by two reviewers, said
p7-quietruns--helptwice; 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):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(inrefresh,taint,untaintandimport) 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.src/audits/behavioral/error_probe.rs:49, pre-existing:p2-json-errors,p4-json-error-outputandp2-consistent-envelopestill gate on the characters--outputplusjsonin the raw help, and probe with a literal--output. The plan names this gate under Scope Boundaries as deliberately lenient. Now thatp2-json-outputreads definitions, one scorecard can disagree with itself: opt_out on that row while these three still probe. Suggested fix: route the gate throughfind_flagand pass the declared spelling. It moves rows and needs its own expected-moves list.src/audits/behavioral/limit_flag.rs:156: a-ncounts 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 containnumber oforlines. 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:-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:line, at or right of its arguments column, with a one-space description or none, are read as the synopsis wrapping.-n NUMthen-1 means no limit.) is read as a flag.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_TOKENthere, 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, becausecmdreads 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 testsRelated Issues/Stories
docs/plans/2026-10-06-0034-fix-help-flag-names-across-frameworks-plan.md(the review gate after U10)Testing
Test Summary:
cargo test --test dogfoodpasses.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_ruleand fromsubcommand_help::dash_rule_notes, and the suffix dropped from the probe arguments injson_output.rs. These eight new tests failed, beside the existing ones that cover the same guards:The other three new tests cover
pass_or_warn's pass, its plain warn, and the Goflagpass.The three classifier cases, each against the one guard it covers. Without the set-off guard on the sentence rule:
Without the empty-description guard on the sentence rule:
With a usage line of fewer than three words taking its last word as the arguments column:
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:
Renamed:
Deleted:
Breaking Changes
Deployment Notes
Checklist