Skip to content

test(fuzz): structured CLI/Maestro generators that reach command validation and assert error codes (#1781 B2) - #1866

Draft
thymikee wants to merge 7 commits into
mainfrom
test/1781-b2-fuzz-v2
Draft

test(fuzz): structured CLI/Maestro generators that reach command validation and assert error codes (#1781 B2)#1866
thymikee wants to merge 7 commits into
mainfrom
test/1781-b2-fuzz-v2

Conversation

@thymikee

@thymikee thymikee commented Aug 19, 2026

Copy link
Copy Markdown
Member

Summary

Closes the B2 half of #1781. CLI-arg and Maestro mutations used to die in the tokenizer, so nothing behind it was exercised, and every case was judged only by "did it fail well" — which cannot see a parser that silently accepts input it should refuse (the #1433 class).

Two targets, cli-validation and maestro-validation, build cases from the real command surface (the CLI schema registry, the Maestro command shapes) so they tokenize cleanly, and each case carries the outcome its generator planted (scripts/fuzz/validation-case.ts). The judge therefore reports two kinds the old invariant could not:

  • silent-accept — an input built to be invalid parsed cleanly (fix(cli): reject excess positionals #1433);
  • wrong-code — rejected, but not with the AppError.code the generator planted, so no "some error" passes.

Each CLI class declares the layer that refuses it. command-validation classes (excess positionals, unsupported-for-command flags, unknown commands) survive the argv scan into finalizeParsedArgs — the reach this PR adds — and hold 80% of the mutated budget. token-scan classes (bad enum, out-of-range int, missing value, valued boolean) are refused inside parseFlagValue during the scan, which the classic cli-args target already reaches; they are weighted down to under 25% and kept only for the error-code assertion cli-args cannot make. Both layers are asserted per class in validation-arbitraries.test.ts, so the reach is disclosed rather than implied.

Rules whose entire input space is a few strings (batch's step-source rule; the --in-app/--system conflict) are pinned seed cases, not generated classes — generating them re-executed ~15 literals thousands of times a night for no added reach.

15 files.

Validation

Rediscovery from a clean corpus (planted red)

Seeded by reverting the fixing hunk onto current main: the pre-fix trees predate the fuzz harness (#1438 landed a day after #1433) and the .ad parser has since moved into packages/ad-script, so a literal checkout cannot run these generators. Each fix and its pre-fix parent are named; the seeded behaviour is the parent's.

#1433 excess positionals — fix 4c02b6ad2, pre-fix parent fcaa6c995. assertCommandPositionalArity neutralized, --target cli-validation --iterations 38000, three seeds:

cli-validation: 15 cases, 1 failures (131ms)      [seed 303; also 16 @ 101, 22 @ 202]

1 invariant violation(s):

[cli-validation] silent-accept: excess-positional: expected INVALID_ARGS, parser accepted the input
  input:    {"payload":["alert","p","p","com.example.app"],"mutation":"excess-positional","expect":{"outcome":"reject","code":"INVALID_ARGS"}}

The A/B that makes this new reach rather than a restatement: against the same seeded tree, the untouched cli-args target ran a full 38,000 cases and found nothing — a silent acceptance is invisible to a rejection-only invariant. Case pinned in scripts/fuzz/corpus/regressions.json via the real --append-corpus path, and green again once the fix was restored.

Replay parse leak — fix 006c4cadc (#1438), pre-fix parent e545544df. The readQuotedReplayToken JSON guard reverted so the quoted literal reaches JSON.parse raw:

replay-script: 133 cases, 1 failures (329ms)      [seed 202; also 276 @ 101, 1239 @ 303]

[replay-script] untyped-throw: SyntaxError: Bad control character in string literal in JSON at position 1 (line 1 column 2) (at at JSON.parse (<anonymous>))

This one is found by the pre-existing replay-script target: it is evidence the harness still works, not evidence for the new generators.

What the calibration actually shows (#1781 B3, corrected)

Eight seeded defects, all rediscovered, but only two by generation the classic targets cannot match:

genuine generated new reach new detection class, seed-caught already within classic reach
rows 2 — #1433 (cli-validation), silently-accepted Maestro field (maestro-validation; classic maestro blind at 38,000 cases) 2 — batch step-source, --in-app/--system conflict (pinned seeds, so "time-to-find" is 0 by construction) 4 — replay parse leak, #1792-class plain Error at the enum seam (classic cli-args: 114/179/114 cases), Maestro unsupported-command plain Error (classic maestro: 27/36/49), raw YAMLParseError escape

An earlier version of this PR credited two of the fourth column to cli-validation/maestro-validation; that was wrong, and an independent review caught it. Full table and method on #1781.

This measures reach, not yield. The seeded set is drawn from the bug shapes these generators aim at, so it shows the lane can see this class — it does not predict how many unknown defects it will find. #1869 is the ledger that tests yield.

Nightly runtime

38,000 cases/target, chosen by measurement rather than rounding. Paired repeats on a quiet host, before (5 classic x 50k) vs after (7 x 38k): 17.07s / 16.48s before, 17.04s / 16.88s after — parity. A flat 33k would have been ~2s cheaper while costing the five untouched targets a third of their depth, so it was not worth taking. Current nightly for reference (run 32211761708): Fuzz parsers step 21s, job 47s, against a 20-minute timeout.

Local gates: pnpm check:affected --run green.

Catches / Evidence / Cost / Kill-criterion

  • Catches: validation-layer defects behind the tokenizer — a bounded command silently swallowing extra positionals or a conflicting flag pair (fix(cli): reject excess positionals #1433 class, invisible to a rejection-only invariant), a rejection carrying the wrong AppError.code, and Maestro command/field rules that stop rejecting or lose their typed boundary.
  • Evidence: fix(cli): reject excess positionals #1433 rediscovered in 15-22 generated cases while the classic target stayed blind across 38,000; a silently-accepted Maestro field in 26-69 cases against the same blind comparison; both counterexamples deterministic and replayable (--seed + path; note fast-check shrinks the record pre-image here, not the rendered payload). 8/8 seeded defects rediscovered overall, of which 2 are new generated reach, 2 are new detection class via pinned seeds, and 4 were already reachable — measured, not asserted.
  • Cost: nightly at parity (7x38k vs 5x50k, ~17s local, 21s CI step in a 20-minute job); no new job, no runner minutes, no macOS occupancy; +1 PR-time unit file (~0.4s, in-process, unit-core, not a subprocess-stub member); ~530 lines of harness/generator plus ~120 of test. Reach traded away deliberately: numeric flag values are sampled at {min, max, midpoint} rather than swept (float modulo drifted past a fractional max and produced a phantom), and the four token-scan classes are capped under 25% of the mutated budget.
  • Kill-criterion: tracked in Parser fuzz validation targets: findings ledger and kill criterion (#1781 B2) #1869 — delete both validation targets (keep the classic five) if a phantom finding reaches a nightly twice, or if neither target finds a real defect by 2027-02-19. The ledger there records every nightly finding; without it the criterion is unfalsifiable.

Related: #1414 (the original lane), #1823 (sibling tracker; this PR adds no subprocess-stub member), #1792 (plain-Error residue; PR #1853 owns that fix, untouched here).

@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.31 MB 2.31 MB 0 B
JS gzip 759.3 kB 759.3 kB 0 B
npm tarball 882.6 kB 882.6 kB 0 B
npm unpacked 3.08 MB 3.08 MB 0 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.9 ms 27.9 ms -1.1 ms
CLI --help 70.0 ms 69.0 ms -1.0 ms

Top changed chunks: no changes in the largest emitted chunks.

@thymikee

Copy link
Copy Markdown
Member Author

CI note (first push): Coverage failed with Worker exited unexpectedly on scripts/fuzz/corpus-replay.test.ts — the #1824 fork-death signature (983/984 files passed, no test failed), but on a file this PR had made heavier, so I treated it as mine rather than as the known flake.

Cause: corpus-replay.test.ts imports arbitraries.ts, which imported the validation generators, which pull the CLI schema/command-metadata registry — a large extra module graph inside a V8-instrumented worker, for generators that file never samples (it only ever generates for the selector target).

Fix (f421b6a): the validation generators are resolved in the run path (generate.ts) instead of inside arbitraryForTarget, so corpus replay keeps its original small graph. Coverage now passes in 5m40s, and the rest of the lane is green apart from the device smoke jobs still running.

Two related mitigations already in the PR: the schema-derived CLI surface is built lazily and memoized (an eager build at import timed out the coverage-instrumented promotion test), and the harness self-check assertions were merged into one run rather than two so the serialized subprocess-stub project (#1823) gains no extra worker startups.

@thymikee

Copy link
Copy Markdown
Member Author

Thanks — the two evidence-integrity findings were both right, and one of them was worse than reported. Every measurement below is re-run against the current generator; the PR body and the #1781 table are restated, and the original B3 comment is marked superseded rather than edited away.

Fixed

  1. Constants dressed as generation. back-mode-conflict and batch-step-source produced ~15 distinct payloads between them (7 and 8 after I first tried to widen them) because the grammar has exactly one command with a step-source rule and one flag key with two tokens. Widening them was the wrong fix, so they are now pinned seed cases on the target — run verbatim once per run, before any generated case — and both are dropped from the new-reach column of the calibration table.
  2. S4 mis-credited — and so was S5. Verified your claim and extended it: the untouched cli-args finds the utils: app-log-files and verified-file throw plain Error instead of AppError #1792-class defect at 114/179/114 cases, and the untouched maestro finds the plain-Error unsupported-command defect at 27/36/49. Both are now in the "already within classic reach" column. To avoid the same error in the other direction I added the missing A/B: for every row credited to a validation target, the classic target was run against the same seeded tree at full nightly depth. Against seeded fix(cli): reject excess positionals #1433, cli-args finds nothing in 38,000 cases while cli-validation finds it at 15/16/22 — and I added an eighth seed (a silently-accepted Maestro field) where maestro is blind across 38,000 and maestro-validation finds it at 26/47/69. Honest split is now 2 generated new reach · 2 new-class-via-seed · 4 pre-existing, stated in both the PR body and the table.
  3. Token-scan share. Cannot be pushed past the scan — parseFlagValue is where flag values are parsed — so it is weighted down and disclosed instead: 33% → 19.8% of the mutated budget. Every class now declares its layer (token-scan / command-validation), all seven CLI classes are asserted against the layer they claim (the four you flagged included, which the old test skipped), and a ratio test fails if a weight edit hands the budget back to the scan.
  4. The Coverage job: "Worker exited unexpectedly" kills one fork in the last ~40s of the run (4 of 17 recent CI failures) #1824 comment was wrong. You are right that the instrumented set is identical either way and that the lazy memoize() in cd736441e was the actual fix. The comment in generate.ts now says so explicitly, and the guard is a real one: validationSurfaceBuildCount() must read 0 after a fresh import and 1 after the first sample, so re-hoisting cliSurfaces() to module scope fails a test rather than a nightly.
  5. Per-mutation expected code instead of one surface-wide constant — each CLI and Maestro class carries its own code, so a class whose contract changes moves alone. (Every value is still INVALID_ARGS; that is what these parsers throw, and the PR now says so rather than implying variety.)
  6. PR-time sample 500 → 3,000, i.e. 7.9% of a nightly rather than 1.5%.
  7. Kill criterion now owned: Parser fuzz validation targets: findings ledger and kill criterion (#1781 B2) #1869 carries the findings ledger and both criteria (a phantom reaching a nightly twice; no real defect by 2027-02-19), modelled on subprocess-stub vitest project: members and kill criterion (#1781 A4) #1823.

Declined / changed differently

  • Widening the two constant classes into real families — declined; their input space is genuinely ~15 strings, so seeds are the honest home. They stay covered, just not counted as generation.
  • Budget — moved to 38,000, not 40,000: paired repeats on a quiet host put 7×38k at 17.04/16.88s against 5×50k at 17.07/16.48s, i.e. parity, where 40k measured a consistent +0.8s. Your arithmetic that 33k was a deeper cut than needed was right; the five untouched targets get back to 76% of their depth.

Nits taken: CI job is 47s (I had read step times, not job times); "shrunk to a minimal case" replaced with "deterministic and replayable", with a note that fast-check shrinks the record pre-image rather than the rendered payload; the numeric-sampling narrowing from the float-phantom fix is now stated as a deliberate reach trade in the Cost line; the reach-not-yield caveat is in the PR body, not only on #1781.

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