From 0ea254486e1f8105da44c525e8bd261ea6c58753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 18 Aug 2026 20:11:34 +0200 Subject: [PATCH 01/11] test(fuzz): structured CLI/Maestro generators that reach command validation and assert error codes (#1781 B2) --- .github/workflows/replays-nightly.yml | 21 +- docs/agents/testing.md | 14 +- .../validation-arbitraries.test.ts.snap | 29 ++ scripts/fuzz/arbitraries.ts | 13 + scripts/fuzz/envelope.ts | 2 + scripts/fuzz/harness.test.ts | 64 ++- scripts/fuzz/invariant.ts | 25 +- scripts/fuzz/self-check-targets.ts | 35 ++ scripts/fuzz/target-types.ts | 31 +- scripts/fuzz/targets.ts | 66 +++ scripts/fuzz/validation-arbitraries.test.ts | 105 +++++ scripts/fuzz/validation-arbitraries.ts | 432 ++++++++++++++++++ scripts/fuzz/validation-case.ts | 150 ++++++ vitest.config.ts | 3 + 14 files changed, 950 insertions(+), 40 deletions(-) create mode 100644 scripts/fuzz/__snapshots__/validation-arbitraries.test.ts.snap create mode 100644 scripts/fuzz/validation-arbitraries.test.ts create mode 100644 scripts/fuzz/validation-arbitraries.ts create mode 100644 scripts/fuzz/validation-case.ts diff --git a/.github/workflows/replays-nightly.yml b/.github/workflows/replays-nightly.yml index 623f3be336..40b2e354b0 100644 --- a/.github/workflows/replays-nightly.yml +++ b/.github/workflows/replays-nightly.yml @@ -12,7 +12,7 @@ on: fuzz-iterations: description: 'Parser fuzz cases per target' required: false - default: '50000' + default: '33000' fuzz-seed: description: 'Parser fuzz PRNG seed (defaults to the run number)' required: false @@ -26,12 +26,17 @@ concurrency: cancel-in-progress: true jobs: - # Parser fuzz lane (#1414): hostile input into parseArgs, selector parsing, .ad replay - # scripts, batch --steps JSON, and the Maestro compat parser. One invariant — every - # rejection is a typed AppError with a non-empty hint, and no case hangs. Device-free, so - # it rides this nightly rather than owning a workflow; the seed varies per run so the lane - # keeps exploring, and anything it catches is appended to the checked-in corpus that the - # unit lane replays (scripts/fuzz/corpus/regressions.json). + # Parser fuzz lane (#1414, validation targets #1781 B2): hostile input into parseArgs, + # selector parsing, .ad replay scripts, batch --steps JSON, and the Maestro compat parser, + # plus schema-derived CLI/Maestro validation cases that assert a planted outcome (specific + # error code, or acceptance) so a silent acceptance is a finding too. Every rejection must + # be a typed AppError with a non-empty hint, and no case hangs. Device-free, so it rides + # this nightly rather than owning a workflow; the seed varies per run so the lane keeps + # exploring, and anything it catches is appended to the checked-in corpus that the unit + # lane replays (scripts/fuzz/corpus/regressions.json). + # + # 33,000 cases/target holds the job's wall-clock at the pre-B2 five-target/50k budget + # (~2min measured) now that seven targets share it — the budget is flat, not the depth. nightly-parser-fuzz: name: Parser Fuzz Lane runs-on: ubuntu-latest @@ -60,7 +65,7 @@ jobs: - name: Fuzz parsers if: always() env: - FUZZ_ITERATIONS: ${{ github.event.inputs.fuzz-iterations || '50000' }} + FUZZ_ITERATIONS: ${{ github.event.inputs.fuzz-iterations || '33000' }} FUZZ_SEED: ${{ github.event.inputs.fuzz-seed || github.run_number }} uses: ./.github/actions/run-gate with: diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 44d5e6841f..efaaf3f3a1 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -378,6 +378,15 @@ pnpm mutation:check # score an existing .tmp/mutation/mutati invariant: every rejection is a typed `AppError` whose normalized `hint` is non-empty, and no case hangs (a worker-thread watchdog attributes a stall to the exact input). +Two **validation targets** (#1781 B2) — `cli-validation` and `maestro-validation` — go further: +their cases are built from the real command surface (the CLI schema registry, the Maestro command +shapes) so they tokenize cleanly and die in command validation, and each case carries the outcome +its generator planted (`scripts/fuzz/validation-case.ts`). The judge then also fails a **silent +acceptance** of an input built to be invalid (the #1433 excess-positionals class) and a rejection +with the **wrong `AppError.code`** — not just "some error". Their generator expectations are gated +at PR time by `scripts/fuzz/validation-arbitraries.test.ts` (unit-core: in-process, no worker), so +a drifted generator fails in seconds instead of producing phantom nightly findings. + ```sh pnpm fuzz:parsers # all targets, 2,000 cases each, seed 1 pnpm fuzz:parsers --target selector --iterations 50000 --seed 7 @@ -397,7 +406,8 @@ parsers changed". The self-check and fuzz steps write to separate artifact subdi run unconditionally; the step summary prints each envelope it finds and never fails on a missing file. -Cases come from fast-check arbitraries (`scripts/fuzz/arbitraries.ts`) built on the hazard vocabulary +Cases come from fast-check arbitraries (`scripts/fuzz/arbitraries.ts`, validation envelopes in +`scripts/fuzz/validation-arbitraries.ts`) built on the hazard vocabulary shared with `src/__tests__/test-utils/property-arbitraries.ts`, so a hazard added for the property suite reaches the fuzz lane too — and a counterexample is reported **shrunk**, with fast-check's seed and replay path printed alongside the saved artifact. @@ -407,7 +417,7 @@ A nightly discovery reaches the unit lane by promotion, not hand-editing: the pr `scripts/fuzz/corpus/regressions.json`, which `scripts/fuzz/corpus-replay.test.ts` replays on every PR — through the same worker watchdog, so a promoted hang case fails against its per-case budget instead of wedging the unit job. `scripts/fuzz/harness.test.ts` covers the harness itself — an -untyped throw, an empty hint, and a wedged worker must each be reported, startup time is never charged against the per-case budget, and +untyped throw, an empty hint, a wedged worker, a silent acceptance, and a wrong error code must each be reported, startup time is never charged against the per-case budget, and every mode writes an envelope — using the broken-on-purpose targets in `scripts/fuzz/self-check-targets.ts` (also what `--self-check` runs in CI), so a regressed classifier or watchdog cannot pass silently. Adding a parser to the lane means adding a target to diff --git a/scripts/fuzz/__snapshots__/validation-arbitraries.test.ts.snap b/scripts/fuzz/__snapshots__/validation-arbitraries.test.ts.snap new file mode 100644 index 0000000000..e61b690670 --- /dev/null +++ b/scripts/fuzz/__snapshots__/validation-arbitraries.test.ts.snap @@ -0,0 +1,29 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`cli-validation generator > exercises every mutation class, including valid accept cases 1`] = ` +[ + "back-mode-conflict", + "bad-enum-value", + "batch-step-source", + "boolean-with-value", + "excess-positional", + "int-out-of-range", + "missing-flag-value", + "unknown-command", + "unsupported-flag", + "valid", +] +`; + +exports[`maestro-validation generator > exercises every mutation class, including valid accept cases 1`] = ` +[ + "bad-press-key", + "config-unknown-key", + "missing-required", + "multi-key-command", + "scroll-options", + "unsupported-command", + "unsupported-field", + "valid", +] +`; diff --git a/scripts/fuzz/arbitraries.ts b/scripts/fuzz/arbitraries.ts index 204a72a144..9e221c7c5f 100644 --- a/scripts/fuzz/arbitraries.ts +++ b/scripts/fuzz/arbitraries.ts @@ -10,6 +10,7 @@ import fc from 'fast-check'; import { replayScriptArb } from '../../src/__tests__/test-utils/property-arbitraries.ts'; import type { FuzzTarget, FuzzTargetName } from './target-types.ts'; +import { cliValidationArb, maestroValidationArb } from './validation-arbitraries.ts'; const SELECTOR_VALUE_HAZARDS = [ '', @@ -182,8 +183,20 @@ const STRUCTURED_BASES: Partial>> = 'batch-steps': fc.json({ maxDepth: 3 }), }; +/** + * Validation targets generate their own expectation-carrying envelopes (#1781 B2); splicing + * hazards into the envelope JSON would corrupt the envelope, not the payload, so they are + * exempt from the corruption/noise mix. Payload-level hostility lives inside their generators. + */ +const VALIDATION_ARBITRARIES: Partial>> = { + 'cli-validation': cliValidationArb, + 'maestro-validation': maestroValidationArb, +}; + /** The case distribution for one target: mostly near-miss, some valid, some pure noise. */ export function arbitraryForTarget(target: FuzzTarget): fc.Arbitrary { + const validation = VALIDATION_ARBITRARIES[target.name]; + if (validation !== undefined) return validation; const seeded = fc.constantFrom(...target.seeds); const structured = STRUCTURED_BASES[target.name]; const base = structured === undefined ? seeded : fc.oneof(seeded, structured); diff --git a/scripts/fuzz/envelope.ts b/scripts/fuzz/envelope.ts index 0a549a78bc..9286ad8adb 100644 --- a/scripts/fuzz/envelope.ts +++ b/scripts/fuzz/envelope.ts @@ -88,6 +88,8 @@ const CASE_GENERATION_INPUTS = [ 'generate.ts', 'targets.ts', 'invariant.ts', + 'validation-arbitraries.ts', + 'validation-case.ts', ] as const; /** Content hash of `CASE_GENERATION_INPUTS`. */ diff --git a/scripts/fuzz/harness.test.ts b/scripts/fuzz/harness.test.ts index 447efef9e7..4a32e4bbd6 100644 --- a/scripts/fuzz/harness.test.ts +++ b/scripts/fuzz/harness.test.ts @@ -52,16 +52,49 @@ describe('fuzz invariant classifier', () => { const failure = checkCase(targetNamed('self-check-empty-hint'), 'case'); expect(failure?.kind).toBe('empty-hint'); }); + + // The two validation-only kinds (#1781 B2): without them a parser that silently accepts + // invalid input, or rejects with the wrong code, would read as a pass forever. + it('reports a silent acceptance of a case marked invalid as silent-accept', () => { + const failure = checkCase(targetNamed('self-check-silent-accept'), 'case'); + expect(failure?.kind).toBe('silent-accept'); + }); + + it('reports a rejection with an unexpected code as wrong-code', () => { + const failure = checkCase(targetNamed('self-check-wrong-code'), 'case'); + expect(failure?.kind).toBe('wrong-code'); + expect(failure?.detail).toContain('expected INVALID_ARGS, got COMMAND_FAILED'); + }); }); describe('fuzz harness self-check', () => { - it('catches an untyped throw, an empty hint, and a wedged worker', () => { - const { status, stdout } = runHarness(['--self-check', '--case-timeout-ms', '750']); - expect(stdout).toContain('ok self-check-untyped-throw: expected untyped-throw'); - expect(stdout).toContain('ok self-check-empty-hint: expected empty-hint'); - expect(stdout).toContain('ok self-check-hang: expected hang, got hang'); - expect(status).toBe(0); - }); + // One run asserts both the report and its envelope: a second full self-check would cost five + // more worker startups in the serialized subprocess-stub project (#1823) for no new signal. + it( + 'catches every seeded violation kind and writes the self-check envelope', + () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'fuzz-selfcheck-')); + const { status, stdout } = runHarness([ + '--self-check', + '--case-timeout-ms', + '750', + '--artifact-dir', + dir, + ]); + expect(stdout).toContain('ok self-check-untyped-throw: expected untyped-throw'); + expect(stdout).toContain('ok self-check-empty-hint: expected empty-hint'); + expect(stdout).toContain('ok self-check-hang: expected hang, got hang'); + expect(stdout).toContain('ok self-check-silent-accept: expected silent-accept'); + expect(stdout).toContain('ok self-check-wrong-code: expected wrong-code'); + expect(status).toBe(0); + const envelope = JSON.parse(fs.readFileSync(path.join(dir, 'run-envelope.json'), 'utf8')); + expect(envelope.result).toBe('pass'); + expect(envelope.data.mode).toBe('self-check'); + expect(envelope.data.targetRuns).toHaveLength(5); + fs.rmSync(dir, { recursive: true, force: true }); + }, + 30_000, + ); }); describe('worker startup budget', () => { @@ -93,7 +126,15 @@ describe('worker startup budget', () => { */ function hashWithout(skip: string): string { const digest = crypto.createHash('sha256'); - for (const name of ['arbitraries.ts', 'generate.ts', 'targets.ts', 'invariant.ts']) { + const inputs = [ + 'arbitraries.ts', + 'generate.ts', + 'targets.ts', + 'invariant.ts', + 'validation-arbitraries.ts', + 'validation-case.ts', + ]; + for (const name of inputs) { if (name !== skip) digest.update(fs.readFileSync(path.join(FUZZ_DIR, name))); } return `sha256:${digest.digest('hex').slice(0, 16)}`; @@ -159,13 +200,6 @@ describe('run envelope', () => { expect(envelope.data.stage).toBe('error'); }); - it('is written for a self-check run', () => { - const { envelope, status } = envelopeFrom(['--self-check', '--case-timeout-ms', '750']); - expect(status).toBe(0); - expect(envelope.result).toBe('pass'); - expect(envelope.data.mode).toBe('self-check'); - expect(envelope.data.targetRuns).toHaveLength(3); - }); }); describe('artifact promotion', () => { diff --git a/scripts/fuzz/invariant.ts b/scripts/fuzz/invariant.ts index e8984e8af8..cbbb2ae096 100644 --- a/scripts/fuzz/invariant.ts +++ b/scripts/fuzz/invariant.ts @@ -1,30 +1,29 @@ -// The single invariant the parser fuzz lane enforces (#1414). +// The invariants the parser fuzz lane enforces (#1414, validation targets #1781 B2). // -// Parsers are the front door for agent-authored input, so the contract is not "parses -// correctly" (nobody can say what a mutated string should mean) but "fails well": +// Parsers are the front door for agent-authored input. For classic targets the contract is not +// "parses correctly" (nobody can say what a mutated string should mean) but "fails well": // // 1. a rejection is an `AppError` — never a bare Error, TypeError, string, or undefined; // 2. the normalized error carries a non-empty `hint`, so the caller is told what to do; // 3. the case terminates — enforced by the harness watchdog, not by this module, because // synchronous parsers cannot be interrupted from inside their own tick. +// +// A validation target (`target.check`) additionally knows what each case SHOULD do, because its +// generator constructed the case with a planted violation or none: it judges silent acceptances +// and wrong error codes too (validation-case.ts). import { AppError, normalizeError } from '@agent-device/kernel/errors'; -import type { FuzzTarget, FuzzTargetName } from './target-types.ts'; - -export type FuzzFailureKind = 'untyped-throw' | 'empty-hint' | 'hang'; +import type { FuzzFailure, FuzzFailureKind, FuzzTarget } from './target-types.ts'; -export type FuzzFailure = { - target: FuzzTargetName; - input: string; - kind: FuzzFailureKind; - detail: string; -}; +export type { FuzzFailure, FuzzFailureKind }; /** * Runs one case and returns the invariant violation it produced, or `null`. - * Accepting the parse is a pass: the lane judges rejections, not results. + * For classic targets, accepting the parse is a pass: they judge rejections, not results. + * A validation target owns its whole judgment via `check`. */ export function checkCase(target: FuzzTarget, input: string): FuzzFailure | null { + if (target.check) return target.check(input); try { target.run(input); return null; diff --git a/scripts/fuzz/self-check-targets.ts b/scripts/fuzz/self-check-targets.ts index 65b1e42698..2351039f73 100644 --- a/scripts/fuzz/self-check-targets.ts +++ b/scripts/fuzz/self-check-targets.ts @@ -8,6 +8,7 @@ import { AppError } from '@agent-device/kernel/errors'; import type { FuzzTarget, SelfCheckTargetName } from './target-types.ts'; +import { judgeValidationCase } from './validation-case.ts'; const SEEDS = ['self-check']; @@ -38,6 +39,38 @@ export const SELF_CHECK_TARGETS: readonly FuzzTarget[] = [ }, seeds: SEEDS, }, + { + // A validation judge that stops reporting silent acceptances would leave the #1433 class + // undetectable again; this target accepts an input its expectation marks invalid. + name: 'self-check-silent-accept', + description: 'accepts a case marked invalid (expects a silent-accept failure)', + run: () => {}, + check: (input) => + judgeValidationCase( + 'self-check-silent-accept', + input, + { mutation: 'seeded', expect: { outcome: 'reject', code: 'INVALID_ARGS' } }, + () => {}, + ), + seeds: SEEDS, + }, + { + name: 'self-check-wrong-code', + description: 'rejects with a different code than expected (expects a wrong-code failure)', + run: () => { + throw new AppError('COMMAND_FAILED', 'self-check wrong code', { hint: 'seeded' }); + }, + check: (input) => + judgeValidationCase( + 'self-check-wrong-code', + input, + { mutation: 'seeded', expect: { outcome: 'reject', code: 'INVALID_ARGS' } }, + () => { + throw new AppError('COMMAND_FAILED', 'self-check wrong code', { hint: 'seeded' }); + }, + ), + seeds: SEEDS, + }, ]; /** The failure kind each self-check target must produce, keyed by target name. */ @@ -45,4 +78,6 @@ export const SELF_CHECK_EXPECTATIONS = { 'self-check-untyped-throw': 'untyped-throw', 'self-check-empty-hint': 'empty-hint', 'self-check-hang': 'hang', + 'self-check-silent-accept': 'silent-accept', + 'self-check-wrong-code': 'wrong-code', } as const satisfies Record; diff --git a/scripts/fuzz/target-types.ts b/scripts/fuzz/target-types.ts index 773b7373f4..8199e8eed8 100644 --- a/scripts/fuzz/target-types.ts +++ b/scripts/fuzz/target-types.ts @@ -7,25 +7,52 @@ /** Names of the real parser targets the lane fuzzes. */ export type ParserTargetName = | 'cli-args' + | 'cli-validation' | 'selector' | 'replay-script' | 'batch-steps' - | 'maestro'; + | 'maestro' + | 'maestro-validation'; /** Names of the broken-on-purpose targets that prove the harness can still fail. */ export type SelfCheckTargetName = | 'self-check-untyped-throw' | 'self-check-empty-hint' - | 'self-check-hang'; + | 'self-check-hang' + | 'self-check-silent-accept' + | 'self-check-wrong-code'; export type FuzzTargetName = ParserTargetName | SelfCheckTargetName; +/** + * `silent-accept` and `wrong-code` only arise from validation targets, whose cases carry an + * expected outcome (validation-case.ts); the classic targets can only produce the first three. + */ +export type FuzzFailureKind = + | 'untyped-throw' + | 'empty-hint' + | 'hang' + | 'silent-accept' + | 'wrong-code'; + +export type FuzzFailure = { + target: FuzzTargetName; + input: string; + kind: FuzzFailureKind; + detail: string; +}; + export type FuzzTarget = { name: FuzzTargetName; /** Human-readable description used in failure reports. */ description: string; /** Runs the parser on one case; may throw. */ run: (input: string) => void; + /** + * Full case judgment for expectation-carrying targets; overrides the default + * rejection-only invariant when present. Still runs under the worker watchdog. + */ + check?: (input: string) => FuzzFailure | null; /** Valid-ish inputs the mutator derives cases from. */ seeds: string[]; }; diff --git a/scripts/fuzz/targets.ts b/scripts/fuzz/targets.ts index 29f8fe1381..1eeee018fa 100644 --- a/scripts/fuzz/targets.ts +++ b/scripts/fuzz/targets.ts @@ -13,6 +13,7 @@ import { parseReplayScriptDetailed } from '@agent-device/ad-script'; import { readCliBatchStepsJson } from '../../src/cli/batch-steps.ts'; import { inspectMaestroFlow } from '@agent-device/maestro'; import type { FuzzTarget } from './target-types.ts'; +import { encodeValidationCase, makeValidationCheck } from './validation-case.ts'; // argv is carried as one string so a case (and its corpus entry, artifact, and repro // command) stays a single copy-pasteable value. Splitting on spaces is deliberate: the @@ -91,6 +92,67 @@ export const FUZZ_TARGETS: readonly FuzzTarget[] = [ '', ], }, + { + // Expectation-carrying cases built from the CLI schema registry (#1781 B2): each case is a + // JSON envelope (validation-case.ts) whose argv tokenizes cleanly, so the planted violation + // surfaces in command validation and is asserted against its specific error code. `run` + // exists for the classic path only; `check` owns the judgment. + name: 'cli-validation', + description: 'parseArgs via schema-derived command lines with expected outcomes', + run: (input) => void runCliValidationPayload(JSON.parse(input).payload as string[]), + check: makeValidationCheck('cli-validation', (payload) => + runCliValidationPayload(payload as string[]), + ), + seeds: [ + encodeValidationCase({ + payload: ['open', 'com.example.app'], + mutation: 'valid', + expect: { outcome: 'accept' }, + }), + encodeValidationCase({ + payload: ['click', 'text=Login', '--json'], + mutation: 'valid', + expect: { outcome: 'accept' }, + }), + encodeValidationCase({ + payload: ['devices', '--platform=bogus'], + mutation: 'bad-enum-value', + expect: { outcome: 'reject', code: 'INVALID_ARGS' }, + }), + encodeValidationCase({ + payload: ['snapshot', '--depth'], + mutation: 'missing-flag-value', + expect: { outcome: 'reject', code: 'INVALID_ARGS' }, + }), + ], + }, + { + // Same contract for Maestro flows: shape-valid YAML with one planted violation, so the + // failure surfaces in the command-shape validation behind the YAML tokenizer. + name: 'maestro-validation', + description: 'inspectMaestroFlow via shape-derived flows with expected outcomes', + run: (input) => void inspectMaestroFlow(JSON.parse(input).payload as string, 'fuzz.yaml'), + check: makeValidationCheck('maestro-validation', (payload) => + void inspectMaestroFlow(payload as string, 'fuzz.yaml'), + ), + seeds: [ + encodeValidationCase({ + payload: 'appId: com.example.app\n---\n- launchApp\n- tapOn: "Login"\n', + mutation: 'valid', + expect: { outcome: 'accept' }, + }), + encodeValidationCase({ + payload: 'appId: com.example.app\n---\n- clickOn: "Login"\n', + mutation: 'unsupported-command', + expect: { outcome: 'reject', code: 'INVALID_ARGS' }, + }), + encodeValidationCase({ + payload: 'appId: com.example.app\n---\n- tapOn:\n bogusField: "x"\n', + mutation: 'unsupported-field', + expect: { outcome: 'reject', code: 'INVALID_ARGS' }, + }), + ], + }, { name: 'maestro', description: 'parseMaestroProgram (Maestro compat)', @@ -108,3 +170,7 @@ export const FUZZ_TARGETS: readonly FuzzTarget[] = [ ], }, ]; + +function runCliValidationPayload(argv: readonly string[]): void { + void parseArgs([...argv], { strictFlags: true }); +} diff --git a/scripts/fuzz/validation-arbitraries.test.ts b/scripts/fuzz/validation-arbitraries.test.ts new file mode 100644 index 0000000000..20552bb90d --- /dev/null +++ b/scripts/fuzz/validation-arbitraries.test.ts @@ -0,0 +1,105 @@ +// Generator-expectation gate for the validation fuzz targets (#1781 B2). +// +// A validation case asserts a specific outcome the generator planted, so a drifted generator is +// worse than a weak one: it would report phantom findings nightly (or mark real bugs expected). +// This suite replays fixed-seed samples against the real parsers in-process — every planted +// expectation must hold on a healthy tree, every mutation class must appear, and the planted +// violations must surface as validation-layer errors, past the tokenizer. + +import fc from 'fast-check'; +import { describe, expect, it } from 'vitest'; +import { checkCase, describeFailure } from './invariant.ts'; +import { getFuzzTarget } from './registry.ts'; +import { decodeValidationCase } from './validation-case.ts'; +import { cliValidationArb, maestroValidationArb } from './validation-arbitraries.ts'; + +const SAMPLE_SIZE = 500; +const SEED = 1; + +const TARGETS = [ + ['cli-validation', cliValidationArb], + ['maestro-validation', maestroValidationArb], +] as const; + +describe.each(TARGETS)('%s generator', (targetName, arbitrary) => { + const target = getFuzzTarget(targetName); + const sample = fc.sample(arbitrary, { numRuns: SAMPLE_SIZE, seed: SEED }); + + it('is deterministic for a seed, so a reported counterexample replays', () => { + const again = fc.sample(arbitrary, { numRuns: 32, seed: 7 }); + expect(again).toEqual(fc.sample(arbitrary, { numRuns: 32, seed: 7 })); + expect(again).not.toEqual(fc.sample(arbitrary, { numRuns: 32, seed: 8 })); + }); + + it('produces decodable envelopes whose expectations hold on a healthy tree', () => { + const failures = []; + for (const input of sample) { + expect(decodeValidationCase(input)).not.toBeNull(); + const failure = target.check!(input); + if (failure) failures.push(describeFailure(failure)); + } + expect(failures).toEqual([]); + }); + + it('exercises every mutation class, including valid accept cases', () => { + const mutations = new Set(sample.map((input) => decodeValidationCase(input)!.mutation)); + expect(mutations).toContain('valid'); + // Every rule the generator declares shows up in a nightly-scale slice of the space; a rule + // that stops firing (surface drift, weight bug) is dead coverage and fails here. + expect([...mutations].sort()).toMatchSnapshot(); + }); +}); + +describe('planted violations surface past the tokenizer', () => { + function rejectionMessageFor(targetName: (typeof TARGETS)[number][0], mutation: string): string { + const arbitrary = targetName === 'cli-validation' ? cliValidationArb : maestroValidationArb; + const sample = fc.sample(arbitrary, { numRuns: SAMPLE_SIZE, seed: SEED }); + const input = sample.find((entry) => decodeValidationCase(entry)!.mutation === mutation); + expect(input, `no ${mutation} case in the fixed-seed sample`).toBeDefined(); + const decoded = decodeValidationCase(input!)!; + const target = getFuzzTarget(targetName); + try { + target.run(input!); + } catch (error) { + return (error as Error).message; + } + throw new Error(`expected ${mutation} case to reject: ${JSON.stringify(decoded.payload)}`); + } + + it('CLI excess positionals die in positional-arity validation (#1433)', () => { + expect(rejectionMessageFor('cli-validation', 'excess-positional')).toMatch( + /accepts at most \d+ positional argument/, + ); + }); + + it('CLI enum violations die in flag-value validation', () => { + expect(rejectionMessageFor('cli-validation', 'bad-enum-value')).toMatch(/^Invalid /); + }); + + it('CLI unsupported flags die in per-command flag support validation', () => { + expect(rejectionMessageFor('cli-validation', 'unsupported-flag')).toMatch( + /is not supported for command/, + ); + }); + + it('Maestro unknown commands die in command-shape validation, not the YAML tokenizer', () => { + expect(rejectionMessageFor('maestro-validation', 'unsupported-command')).toMatch( + /Maestro command ".+" is not supported/, + ); + }); + + it('Maestro unknown fields die in per-command field validation', () => { + expect(rejectionMessageFor('maestro-validation', 'unsupported-field')).toMatch( + /field ".+" is not supported/, + ); + }); +}); + +describe('validation envelope guard', () => { + it('reports a malformed envelope as a finding instead of crashing the worker', () => { + const target = getFuzzTarget('cli-validation'); + const failure = checkCase(target, 'not an envelope'); + expect(failure?.kind).toBe('untyped-throw'); + expect(failure?.detail).toContain('malformed validation case envelope'); + }); +}); diff --git a/scripts/fuzz/validation-arbitraries.ts b/scripts/fuzz/validation-arbitraries.ts new file mode 100644 index 0000000000..26ce911404 --- /dev/null +++ b/scripts/fuzz/validation-arbitraries.ts @@ -0,0 +1,432 @@ +// Structured case generators for the validation fuzz targets (#1781 B2). +// +// The classic mutators splice hazards into flat strings, so a CLI or Maestro case almost always +// dies in the tokenizer ("Unknown command", YAML error) and the validation layer behind it goes +// unexercised. These generators build cases FROM the real command surface — the CLI schema +// registry and the Maestro command shapes — so a case tokenizes cleanly and its planted +// violation surfaces in command validation (positional arity, flag support, enum/range checks, +// unsupported Maestro commands and fields). Each case records the outcome it was built to have; +// validation-case.ts judges the parser against it, which is what makes a silent acceptance +// (the #1433 class) reportable at all. + +import fc from 'fast-check'; +import { isKnownCliCommandName, listCliCommandNames } from '../../src/command-catalog.ts'; +import { + getCliCommandSchema, + getFlagDefinitions, + type FlagDefinition, +} from '../../src/cli-schema/command-schema.ts'; +import { isFlagSupportedForCommand } from '../../src/cli-schema/option-schema.ts'; +import { encodeValidationCase, type ValidationCase } from './validation-case.ts'; + +const REJECT = { outcome: 'reject', code: 'INVALID_ARGS' } as const; +const ACCEPT = { outcome: 'accept' } as const; + +// Benign, occasionally hostile positional/flag values. No leading '-': a dash token would be +// read as a flag and the case would die before validation, which is the classic targets' job. +const SAFE_VALUES = [ + 'com.example.app', + 'text=Login', + '@e1', + 'hello world', + '123', + 'Ünïcøde', + '😀 emoji', + 'say "hi"', + 'a\\b', + '', +] as const; + +// --------------------------------------------------------------------------------------------- +// CLI: the surface is derived from the schema registry, never hand-listed. +// --------------------------------------------------------------------------------------------- + +type CliCommandSurface = { + name: string; + /** `null` when the schema allows extra positionals — the arity mutation does not apply. */ + maxPositionals: number | null; + /** Flags supported for this command, restricted to the unambiguous pool. */ + flags: readonly FlagDefinition[]; +}; + +// Commands whose parse path is deliberately special: cdp preserves post-command args verbatim, +// react-devtools passes unknown flags through, batch enforces a step-source invariant covered +// by its own fixed mutation below. +const EXCLUDED_CLI_COMMANDS = new Set(['cdp', 'react-devtools', 'batch']); + +// help/version reroute parsing, snapshotDiff rewrites the command, steps/stepsFile carry the +// batch step-source invariant. All are exercised elsewhere; here they would blur expectations. +const EXCLUDED_FLAG_KEYS = new Set(['help', 'version', 'snapshotDiff', 'steps', 'stepsFile']); + +function collidingFlagNames(definitions: readonly FlagDefinition[]): Set { + const counts = new Map(); + for (const definition of definitions) { + for (const name of definition.names) counts.set(name, (counts.get(name) ?? 0) + 1); + } + return new Set([...counts.entries()].filter(([, count]) => count > 1).map(([name]) => name)); +} + +const COLLIDING_FLAG_NAMES = collidingFlagNames(getFlagDefinitions()); + +/** Flag definitions with a single unambiguous long name, excluding the special keys. */ +const SAFE_FLAG_POOL: readonly FlagDefinition[] = getFlagDefinitions().filter( + (definition) => + !EXCLUDED_FLAG_KEYS.has(definition.key) && + definition.names.some((name) => name.startsWith('--') && !COLLIDING_FLAG_NAMES.has(name)), +); + +function flagToken(definition: FlagDefinition): string { + return definition.names.find( + (name) => name.startsWith('--') && !COLLIDING_FLAG_NAMES.has(name), + )!; +} + +const CLI_SURFACES: readonly CliCommandSurface[] = listCliCommandNames() + .filter((name) => !EXCLUDED_CLI_COMMANDS.has(name)) + .map((name) => { + const schema = getCliCommandSchema(name); + return { + name, + maxPositionals: schema.allowsExtraPositionals ? null : (schema.positionalArgs?.length ?? 0), + flags: SAFE_FLAG_POOL.filter((definition) => + isFlagSupportedForCommand(definition.key, name), + ), + }; + }); + +/** A schema-valid value for one flag, salt-selected so shrinking stays deterministic. */ +function validFlagValue(definition: FlagDefinition, salt: number): string { + if (definition.type === 'enum') { + const values = definition.enumValues ?? []; + return values[salt % Math.max(values.length, 1)] ?? '1'; + } + if (definition.type === 'int' || definition.type === 'number') { + const low = definition.min ?? 0; + const high = definition.max ?? low + 1000; + return String(low + (salt % (high - low + 1))); + } + const value = SAFE_VALUES[salt % SAFE_VALUES.length]!; + return value.length === 0 ? 'value' : value; +} + +/** Renders one flag as argv tokens; value flags use `--flag=value` so no token is consumed. */ +function renderFlag(definition: FlagDefinition, salt: number): string { + const token = flagToken(definition); + if (definition.type === 'boolean' || definition.setValue !== undefined) return token; + if (definition.type === 'booleanOrString') return token; + return `${token}=${validFlagValue(definition, salt)}`; +} + +type CliBase = { + surface: CliCommandSurface; + positionals: string[]; + flags: FlagDefinition[]; + salt: number; +}; + +function validArgv(base: CliBase): string[] { + return [ + base.surface.name, + ...base.positionals, + ...base.flags.map((definition, index) => renderFlag(definition, base.salt + index)), + ]; +} + +type CliMutation = { + name: string; + apply: (base: CliBase) => ValidationCase | null; +}; + +const isValueFlag = (definition: FlagDefinition) => + definition.setValue === undefined && + (definition.type === 'string' || + definition.type === 'enum' || + definition.type === 'int' || + definition.type === 'number'); + +const FAKE_COMMANDS = ['frobnicate', 'tapp', 'navigate', 'clik', 'snapshoot', 'opne'].filter( + (name) => !isKnownCliCommandName(name), +); + +const CLI_MUTATIONS: readonly CliMutation[] = [ + { + // #1433: a bounded command must refuse extra positionals instead of swallowing them. + name: 'excess-positional', + apply: (base) => { + if (base.surface.maxPositionals === null) return null; + const filler = SAFE_VALUES[base.salt % SAFE_VALUES.length] || 'extra'; + const positionals = [ + ...Array.from({ length: base.surface.maxPositionals }, () => 'p'), + ...Array.from({ length: 1 + (base.salt % 2) }, () => filler), + ]; + return { + payload: [base.surface.name, ...positionals], + mutation: 'excess-positional', + expect: REJECT, + }; + }, + }, + { + name: 'unsupported-flag', + apply: (base) => { + const foreign = SAFE_FLAG_POOL.filter( + (definition) => !isFlagSupportedForCommand(definition.key, base.surface.name), + ); + const definition = foreign[base.salt % Math.max(foreign.length, 1)]; + if (!definition) return null; + return { + payload: [...validArgv(base), renderFlag(definition, base.salt)], + mutation: 'unsupported-flag', + expect: REJECT, + }; + }, + }, + { + name: 'bad-enum-value', + apply: (base) => { + const enums = base.surface.flags.filter((definition) => definition.type === 'enum'); + const definition = enums[base.salt % Math.max(enums.length, 1)]; + if (!definition) return null; + return { + payload: [base.surface.name, `${flagToken(definition)}=bogus-${base.salt % 7}`], + mutation: 'bad-enum-value', + expect: REJECT, + }; + }, + }, + { + name: 'int-out-of-range', + apply: (base) => { + const bounded = base.surface.flags.filter( + (definition) => + (definition.type === 'int' || definition.type === 'number') && + (definition.min !== undefined || definition.max !== undefined), + ); + const definition = bounded[base.salt % Math.max(bounded.length, 1)]; + if (!definition) return null; + const value = + definition.min !== undefined ? String(definition.min - 1) : String(definition.max! + 1); + return { + payload: [base.surface.name, `${flagToken(definition)}=${value}`], + mutation: 'int-out-of-range', + expect: REJECT, + }; + }, + }, + { + name: 'missing-flag-value', + apply: (base) => { + const valued = base.surface.flags.filter(isValueFlag); + const definition = valued[base.salt % Math.max(valued.length, 1)]; + if (!definition) return null; + return { + payload: [base.surface.name, flagToken(definition)], + mutation: 'missing-flag-value', + expect: REJECT, + }; + }, + }, + { + name: 'boolean-with-value', + apply: (base) => { + const booleans = base.surface.flags.filter( + (definition) => definition.type === 'boolean' || definition.setValue !== undefined, + ); + const definition = booleans[base.salt % Math.max(booleans.length, 1)]; + if (!definition) return null; + return { + payload: [...validArgv(base), `${flagToken(definition)}=true`], + mutation: 'boolean-with-value', + expect: REJECT, + }; + }, + }, + { + name: 'unknown-command', + apply: (base) => { + const name = FAKE_COMMANDS[base.salt % FAKE_COMMANDS.length]!; + return { payload: [name, ...base.positionals], mutation: 'unknown-command', expect: REJECT }; + }, + }, + { + name: 'batch-step-source', + apply: (base) => ({ + payload: + base.salt % 2 === 0 ? ['batch'] : ['batch', '--steps=[]', '--steps-file=steps.json'], + mutation: 'batch-step-source', + expect: REJECT, + }), + }, + { + name: 'back-mode-conflict', + apply: () => ({ + payload: ['back', '--in-app', '--system'], + mutation: 'back-mode-conflict', + expect: REJECT, + }), + }, +]; + +const cliBaseArb: fc.Arbitrary = fc + .record({ + surfaceIndex: fc.nat(), + positionals: fc.array(fc.constantFrom(...SAFE_VALUES), { maxLength: 4 }), + flagIndices: fc.uniqueArray(fc.nat({ max: 200 }), { maxLength: 3 }), + salt: fc.nat({ max: 10_000 }), + }) + .map(({ surfaceIndex, positionals, flagIndices, salt }) => { + const surface = CLI_SURFACES[surfaceIndex % CLI_SURFACES.length]!; + const maxPositionals = surface.maxPositionals ?? 3; + const flags = [ + ...new Map( + flagIndices + .filter(() => surface.flags.length > 0) + .map((index) => surface.flags[index % surface.flags.length]!) + .map((definition) => [definition.key, definition]), + ).values(), + ]; + return { surface, positionals: positionals.slice(0, maxPositionals), flags, salt }; + }); + +/** Encoded CLI validation cases: ~1/3 valid (expect accept), the rest planted violations. */ +export const cliValidationArb: fc.Arbitrary = fc + .record({ base: cliBaseArb, mutationIndex: fc.nat() }) + .map(({ base, mutationIndex }) => { + // A third of the space stays valid so a false rejection is discoverable too. + if (mutationIndex % (CLI_MUTATIONS.length + 4) >= CLI_MUTATIONS.length) { + return encodeValidationCase({ payload: validArgv(base), mutation: 'valid', expect: ACCEPT }); + } + // Rotate to the first applicable mutation so the map stays total. + for (let step = 0; step < CLI_MUTATIONS.length; step += 1) { + const mutation = CLI_MUTATIONS[(mutationIndex + step) % CLI_MUTATIONS.length]!; + const validationCase = mutation.apply(base); + if (validationCase) return encodeValidationCase(validationCase); + } + return encodeValidationCase({ payload: validArgv(base), mutation: 'valid', expect: ACCEPT }); + }); + +// --------------------------------------------------------------------------------------------- +// Maestro: cases are built from the command shapes the converter accepts, then one shape rule +// is violated. The soundness test replays samples against the real parser, so a drifted shape +// fails at PR time rather than as a phantom nightly finding. +// --------------------------------------------------------------------------------------------- + +const yamlText = (salt: number): string => + JSON.stringify(SAFE_VALUES[salt % SAFE_VALUES.length] || 'Login'); + +/** One valid command rendered as YAML list-entry lines. */ +function validMaestroCommand(pick: number, salt: number): string[] { + const text = yamlText(salt); + const options: (() => string[])[] = [ + () => ['- back'], + () => ['- hideKeyboard'], + () => ['- stopApp'], + () => ['- scroll'], + () => ['- waitForAnimationToEnd'], + () => ['- launchApp'], + () => [`- launchApp: ${text}`], + () => [`- tapOn: ${text}`], + () => ['- tapOn:', ` id: ${text}`], + () => ['- tapOn:', ` text: ${text}`], + () => [`- doubleTapOn: ${text}`], + () => [`- longPressOn: ${text}`], + () => [`- inputText: ${text}`], + () => ['- openLink: "https://example.com/page"'], + () => [`- assertVisible: ${text}`], + () => [`- assertNotVisible: ${text}`], + () => [`- takeScreenshot: ${text}`], + () => ['- swipe:', ` direction: ${['UP', 'DOWN', 'LEFT', 'RIGHT'][salt % 4]}`], + () => [`- pressKey: ${['back', 'enter', 'return', 'home'][salt % 4]}`], + () => ['- extendedWaitUntil:', ` visible: ${text}`, ' timeout: 500'], + () => ['- scrollUntilVisible:', ' element:', ` text: ${text}`], + () => ['- repeat:', ' times: 2', ' commands:', ' - back'], + () => ['- runFlow: other.yaml'], + ]; + return options[pick % options.length]!(); +} + +const FAKE_MAESTRO_COMMANDS = [ + 'clickOn', + 'tapOnPoint', + 'assertTrue', + 'evalScript', + 'launchActivity', + 'inputTextt', +] as const; + +type MaestroMutation = { name: string; lines: (salt: number) => string[] }; + +const MAESTRO_MUTATIONS: readonly MaestroMutation[] = [ + { + // The B2 headline: an unknown command name must die in command validation, not the + // YAML tokenizer — mutated names used to be unreachable because the YAML never parsed. + name: 'unsupported-command', + lines: (salt) => { + const name = FAKE_MAESTRO_COMMANDS[salt % FAKE_MAESTRO_COMMANDS.length]!; + return salt % 2 === 0 ? [`- ${name}`] : [`- ${name}: ${yamlText(salt)}`]; + }, + }, + { + name: 'unsupported-field', + lines: (salt) => + salt % 2 === 0 + ? ['- tapOn:', ` bogusField: ${yamlText(salt)}`] + : ['- launchApp:', ` appId: ${yamlText(salt)}`, ' bogus: 1'], + }, + { + name: 'multi-key-command', + lines: (salt) => [`- tapOn: ${yamlText(salt)}`, ` inputText: ${yamlText(salt + 1)}`], + }, + { + name: 'missing-required', + lines: (salt) => + salt % 2 === 0 ? ['- inputText:'] : ['- extendedWaitUntil:', ' timeout: 500'], + }, + { name: 'bad-press-key', lines: () => ['- pressKey: sleep'] }, + { name: 'scroll-options', lines: () => ['- scroll:', ' direction: UP'] }, +]; + +type MaestroBase = { commandPicks: number[]; salt: number; withConfig: boolean }; + +function renderMaestroFlow(base: MaestroBase, mutatedLines?: string[], at?: number): string { + const commands = base.commandPicks.map((pick, index) => + validMaestroCommand(pick, base.salt + index), + ); + if (mutatedLines) commands.splice(Math.min(at ?? 0, commands.length), 0, mutatedLines); + const body = commands.flat().join('\n'); + const config = base.withConfig ? `appId: ${yamlText(base.salt)}\n---\n` : ''; + return `${config}${body}\n`; +} + +const maestroBaseArb: fc.Arbitrary = fc.record({ + commandPicks: fc.array(fc.nat({ max: 100 }), { minLength: 1, maxLength: 4 }), + salt: fc.nat({ max: 10_000 }), + withConfig: fc.boolean(), +}); + +/** Encoded Maestro validation cases: ~1/3 valid flows, the rest one planted shape violation. */ +export const maestroValidationArb: fc.Arbitrary = fc + .record({ base: maestroBaseArb, mutationIndex: fc.nat(), insertAt: fc.nat({ max: 4 }) }) + .map(({ base, mutationIndex, insertAt }) => { + if (mutationIndex % (MAESTRO_MUTATIONS.length + 3) >= MAESTRO_MUTATIONS.length) { + return encodeValidationCase({ + payload: renderMaestroFlow(base), + mutation: 'valid', + expect: ACCEPT, + }); + } + const mutation = MAESTRO_MUTATIONS[mutationIndex % MAESTRO_MUTATIONS.length]!; + // A config-level violation replaces the flow body mutation for a slice of the space. + if (mutation.name === 'unsupported-field' && base.salt % 3 === 0) { + return encodeValidationCase({ + payload: `appId: com.example.app\nbogusKey: 1\n---\n${renderMaestroFlow({ ...base, withConfig: false })}`, + mutation: 'config-unknown-key', + expect: REJECT, + }); + } + return encodeValidationCase({ + payload: renderMaestroFlow(base, mutation.lines(base.salt), insertAt), + mutation: mutation.name, + expect: REJECT, + }); + }); diff --git a/scripts/fuzz/validation-case.ts b/scripts/fuzz/validation-case.ts new file mode 100644 index 0000000000..bfc438a938 --- /dev/null +++ b/scripts/fuzz/validation-case.ts @@ -0,0 +1,150 @@ +// Expectation-carrying cases for the validation fuzz targets (#1781 B2). +// +// The classic targets judge only rejections ("fails well"), which cannot see the bug class +// #1433 belonged to: a parser that silently ACCEPTS input it should refuse. A validation case +// therefore carries its own expected outcome, decided by the generator that constructed it — +// it knows whether it built a valid command line or planted a specific violation. The case +// travels as one JSON string so the existing corpus, artifact, repro, and worker plumbing +// carry it unchanged. + +import { AppError, normalizeError } from '@agent-device/kernel/errors'; +import type { FuzzFailure, FuzzTargetName } from './target-types.ts'; + +export type ValidationExpectation = + | { outcome: 'accept' } + | { outcome: 'reject'; code: string }; + +export type ValidationCase = { + /** CLI argv vector or Maestro flow source, depending on the target. */ + payload: string[] | string; + /** Which generator rule produced the case — names the finding and the calibration row. */ + mutation: string; + expect: ValidationExpectation; +}; + +export function encodeValidationCase(validationCase: ValidationCase): string { + return JSON.stringify(validationCase); +} + +/** `null` for anything that is not a well-formed validation envelope. */ +export function decodeValidationCase(input: string): ValidationCase | null { + let parsed: unknown; + try { + parsed = JSON.parse(input); + } catch { + return null; + } + if (parsed === null || typeof parsed !== 'object') return null; + const { payload, mutation, expect } = parsed as Record; + if (typeof mutation !== 'string') return null; + const validPayload = + typeof payload === 'string' || + (Array.isArray(payload) && payload.every((entry) => typeof entry === 'string')); + if (!validPayload) return null; + const expectation = readExpectation(expect); + if (!expectation) return null; + return { payload: payload as string[] | string, mutation, expect: expectation }; +} + +function readExpectation(value: unknown): ValidationExpectation | null { + if (value === null || typeof value !== 'object') return null; + const { outcome, code } = value as Record; + if (outcome === 'accept') return { outcome: 'accept' }; + if (outcome === 'reject' && typeof code === 'string' && code.length > 0) { + return { outcome: 'reject', code }; + } + return null; +} + +/** + * Runs one validation case and judges the outcome against its expectation. + * + * The classic rejection invariant still applies (typed AppError, non-empty hint); on top of it: + * - an input built to be invalid that parses cleanly is a `silent-accept` finding — the #1433 + * class, invisible to a rejection-only judge; + * - a rejection with a different `AppError.code` than the generator planted for — or any + * rejection of an input built to be valid — is a `wrong-code` finding. + */ +export function judgeValidationCase( + target: FuzzTargetName, + input: string, + validationCase: Pick, + run: () => void, +): FuzzFailure | null { + const { mutation, expect } = validationCase; + try { + run(); + } catch (error) { + if (!(error instanceof AppError)) { + return failure(target, input, 'untyped-throw', `${mutation}: ${describeThrown(error)}`); + } + const hint = normalizeError(error).hint; + if (typeof hint !== 'string' || hint.trim().length === 0) { + return failure( + target, + input, + 'empty-hint', + `${mutation}: AppError ${error.code} has no hint: ${error.message}`, + ); + } + if (expect.outcome === 'accept') { + return failure( + target, + input, + 'wrong-code', + `${mutation}: expected accept, rejected with ${error.code}: ${error.message}`, + ); + } + if (error.code !== expect.code) { + return failure( + target, + input, + 'wrong-code', + `${mutation}: expected ${expect.code}, got ${error.code}: ${error.message}`, + ); + } + return null; + } + if (expect.outcome === 'reject') { + return failure( + target, + input, + 'silent-accept', + `${mutation}: expected ${expect.code}, parser accepted the input`, + ); + } + return null; +} + +/** + * Wraps a payload runner as a target `check`. A malformed envelope is a harness or corpus + * defect, not a parser bug, but it must still surface red rather than crash the run. + */ +export function makeValidationCheck( + target: FuzzTargetName, + runPayload: (payload: string[] | string) => void, +): (input: string) => FuzzFailure | null { + return (input) => { + const validationCase = decodeValidationCase(input); + if (!validationCase) { + return failure(target, input, 'untyped-throw', 'malformed validation case envelope'); + } + return judgeValidationCase(target, input, validationCase, () => + runPayload(validationCase.payload), + ); + }; +} + +function failure( + target: FuzzTargetName, + input: string, + kind: FuzzFailure['kind'], + detail: string, +): FuzzFailure { + return { target, input, kind, detail }; +} + +function describeThrown(error: unknown): string { + if (error instanceof Error) return `${error.name}: ${error.message}`; + return `non-Error throw: ${typeof error} ${String(error)}`; +} diff --git a/vitest.config.ts b/vitest.config.ts index e15533161a..e22a39457d 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -61,6 +61,9 @@ export default defineConfig({ include: [ 'src/**/*.test.ts', 'packages/*/src/**/*.test.ts', + // The validation fuzz generators' expectation gate (#1781 B2): in-process, no + // subprocess or worker, so it rides the fast lane unlike its serialized siblings. + 'scripts/fuzz/validation-arbitraries.test.ts', 'scripts/__tests__/help-conformance-bench.test.ts', 'scripts/__tests__/help-conformance-error-recovery-coverage.test.ts', 'scripts/__tests__/help-conformance-sample-outputs.test.ts', From b02a01fda71167902f33c5f7a047e217e11ea9a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 07:52:01 +0200 Subject: [PATCH 02/11] test(fuzz): pin the rediscovered #1433 excess-positional case and keep numeric flag samples inside their range --- scripts/fuzz/corpus/regressions.json | 5 +++++ scripts/fuzz/validation-arbitraries.ts | 9 ++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/scripts/fuzz/corpus/regressions.json b/scripts/fuzz/corpus/regressions.json index 719c853384..d59f16695b 100644 --- a/scripts/fuzz/corpus/regressions.json +++ b/scripts/fuzz/corpus/regressions.json @@ -9,6 +9,11 @@ "input": "click --selector", "note": "seed: flag with a missing value must reject as INVALID_ARGS with a hint" }, + { + "target": "cli-validation", + "input": "{\"payload\":[\"alert\",\"p\",\"p\",\"com.example.app\"],\"mutation\":\"excess-positional\",\"expect\":{\"outcome\":\"reject\",\"code\":\"INVALID_ARGS\"}}", + "note": "silent-accept: #1433 excess positionals — rediscovered against the pre-fix behaviour of assertCommandPositionalArity (fix 4c02b6ad2), 11 cases in" + }, { "target": "maestro", "input": "appId: com.example.app\n---\n- tapOn: \"unterminated\n", diff --git a/scripts/fuzz/validation-arbitraries.ts b/scripts/fuzz/validation-arbitraries.ts index 26ce911404..1410986ce9 100644 --- a/scripts/fuzz/validation-arbitraries.ts +++ b/scripts/fuzz/validation-arbitraries.ts @@ -100,11 +100,18 @@ function validFlagValue(definition: FlagDefinition, salt: number): string { const values = definition.enumValues ?? []; return values[salt % Math.max(values.length, 1)] ?? '1'; } - if (definition.type === 'int' || definition.type === 'number') { + if (definition.type === 'int') { const low = definition.min ?? 0; const high = definition.max ?? low + 1000; return String(low + (salt % (high - low + 1))); } + if (definition.type === 'number') { + // Endpoints and midpoint only: float modulo arithmetic can drift past a fractional `max` + // (a 33k-case nightly slice caught `--scale=1.110000000000017` as a phantom rejection). + const low = definition.min ?? 0; + const high = definition.max ?? low + 1000; + return String([low, high, (low + high) / 2][salt % 3]); + } const value = SAFE_VALUES[salt % SAFE_VALUES.length]!; return value.length === 0 ? 'value' : value; } From a2a0aaf56392ce4aa7daae2c2edeccf7c6156986 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 07:58:06 +0200 Subject: [PATCH 03/11] style: apply oxfmt to the new fuzz modules --- scripts/fuzz/harness.test.ts | 47 ++++++++++++-------------- scripts/fuzz/targets.ts | 5 +-- scripts/fuzz/validation-arbitraries.ts | 11 ++---- scripts/fuzz/validation-case.ts | 4 +-- 4 files changed, 28 insertions(+), 39 deletions(-) diff --git a/scripts/fuzz/harness.test.ts b/scripts/fuzz/harness.test.ts index 4a32e4bbd6..0c922deb44 100644 --- a/scripts/fuzz/harness.test.ts +++ b/scripts/fuzz/harness.test.ts @@ -70,31 +70,27 @@ describe('fuzz invariant classifier', () => { describe('fuzz harness self-check', () => { // One run asserts both the report and its envelope: a second full self-check would cost five // more worker startups in the serialized subprocess-stub project (#1823) for no new signal. - it( - 'catches every seeded violation kind and writes the self-check envelope', - () => { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'fuzz-selfcheck-')); - const { status, stdout } = runHarness([ - '--self-check', - '--case-timeout-ms', - '750', - '--artifact-dir', - dir, - ]); - expect(stdout).toContain('ok self-check-untyped-throw: expected untyped-throw'); - expect(stdout).toContain('ok self-check-empty-hint: expected empty-hint'); - expect(stdout).toContain('ok self-check-hang: expected hang, got hang'); - expect(stdout).toContain('ok self-check-silent-accept: expected silent-accept'); - expect(stdout).toContain('ok self-check-wrong-code: expected wrong-code'); - expect(status).toBe(0); - const envelope = JSON.parse(fs.readFileSync(path.join(dir, 'run-envelope.json'), 'utf8')); - expect(envelope.result).toBe('pass'); - expect(envelope.data.mode).toBe('self-check'); - expect(envelope.data.targetRuns).toHaveLength(5); - fs.rmSync(dir, { recursive: true, force: true }); - }, - 30_000, - ); + it('catches every seeded violation kind and writes the self-check envelope', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'fuzz-selfcheck-')); + const { status, stdout } = runHarness([ + '--self-check', + '--case-timeout-ms', + '750', + '--artifact-dir', + dir, + ]); + expect(stdout).toContain('ok self-check-untyped-throw: expected untyped-throw'); + expect(stdout).toContain('ok self-check-empty-hint: expected empty-hint'); + expect(stdout).toContain('ok self-check-hang: expected hang, got hang'); + expect(stdout).toContain('ok self-check-silent-accept: expected silent-accept'); + expect(stdout).toContain('ok self-check-wrong-code: expected wrong-code'); + expect(status).toBe(0); + const envelope = JSON.parse(fs.readFileSync(path.join(dir, 'run-envelope.json'), 'utf8')); + expect(envelope.result).toBe('pass'); + expect(envelope.data.mode).toBe('self-check'); + expect(envelope.data.targetRuns).toHaveLength(5); + fs.rmSync(dir, { recursive: true, force: true }); + }, 30_000); }); describe('worker startup budget', () => { @@ -199,7 +195,6 @@ describe('run envelope', () => { expect(envelope.result).toBe('fail'); expect(envelope.data.stage).toBe('error'); }); - }); describe('artifact promotion', () => { diff --git a/scripts/fuzz/targets.ts b/scripts/fuzz/targets.ts index 1eeee018fa..ea1d0260f4 100644 --- a/scripts/fuzz/targets.ts +++ b/scripts/fuzz/targets.ts @@ -132,8 +132,9 @@ export const FUZZ_TARGETS: readonly FuzzTarget[] = [ name: 'maestro-validation', description: 'inspectMaestroFlow via shape-derived flows with expected outcomes', run: (input) => void inspectMaestroFlow(JSON.parse(input).payload as string, 'fuzz.yaml'), - check: makeValidationCheck('maestro-validation', (payload) => - void inspectMaestroFlow(payload as string, 'fuzz.yaml'), + check: makeValidationCheck( + 'maestro-validation', + (payload) => void inspectMaestroFlow(payload as string, 'fuzz.yaml'), ), seeds: [ encodeValidationCase({ diff --git a/scripts/fuzz/validation-arbitraries.ts b/scripts/fuzz/validation-arbitraries.ts index 1410986ce9..6a3e605d93 100644 --- a/scripts/fuzz/validation-arbitraries.ts +++ b/scripts/fuzz/validation-arbitraries.ts @@ -76,9 +76,7 @@ const SAFE_FLAG_POOL: readonly FlagDefinition[] = getFlagDefinitions().filter( ); function flagToken(definition: FlagDefinition): string { - return definition.names.find( - (name) => name.startsWith('--') && !COLLIDING_FLAG_NAMES.has(name), - )!; + return definition.names.find((name) => name.startsWith('--') && !COLLIDING_FLAG_NAMES.has(name))!; } const CLI_SURFACES: readonly CliCommandSurface[] = listCliCommandNames() @@ -88,9 +86,7 @@ const CLI_SURFACES: readonly CliCommandSurface[] = listCliCommandNames() return { name, maxPositionals: schema.allowsExtraPositionals ? null : (schema.positionalArgs?.length ?? 0), - flags: SAFE_FLAG_POOL.filter((definition) => - isFlagSupportedForCommand(definition.key, name), - ), + flags: SAFE_FLAG_POOL.filter((definition) => isFlagSupportedForCommand(definition.key, name)), }; }); @@ -258,8 +254,7 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ { name: 'batch-step-source', apply: (base) => ({ - payload: - base.salt % 2 === 0 ? ['batch'] : ['batch', '--steps=[]', '--steps-file=steps.json'], + payload: base.salt % 2 === 0 ? ['batch'] : ['batch', '--steps=[]', '--steps-file=steps.json'], mutation: 'batch-step-source', expect: REJECT, }), diff --git a/scripts/fuzz/validation-case.ts b/scripts/fuzz/validation-case.ts index bfc438a938..eb9fc16b64 100644 --- a/scripts/fuzz/validation-case.ts +++ b/scripts/fuzz/validation-case.ts @@ -10,9 +10,7 @@ import { AppError, normalizeError } from '@agent-device/kernel/errors'; import type { FuzzFailure, FuzzTargetName } from './target-types.ts'; -export type ValidationExpectation = - | { outcome: 'accept' } - | { outcome: 'reject'; code: string }; +export type ValidationExpectation = { outcome: 'accept' } | { outcome: 'reject'; code: string }; export type ValidationCase = { /** CLI argv vector or Maestro flow source, depending on the target. */ From 2f0b523995ac7fcc5bd032d719485cd17c9c61eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 08:05:07 +0200 Subject: [PATCH 04/11] perf(fuzz): derive the CLI validation surface lazily so unrelated harness paths keep their startup --- scripts/fuzz/invariant.ts | 6 +- scripts/fuzz/validation-arbitraries.ts | 92 +++++++++++++++++--------- 2 files changed, 64 insertions(+), 34 deletions(-) diff --git a/scripts/fuzz/invariant.ts b/scripts/fuzz/invariant.ts index cbbb2ae096..a54e7cb4b7 100644 --- a/scripts/fuzz/invariant.ts +++ b/scripts/fuzz/invariant.ts @@ -13,9 +13,11 @@ // and wrong error codes too (validation-case.ts). import { AppError, normalizeError } from '@agent-device/kernel/errors'; -import type { FuzzFailure, FuzzFailureKind, FuzzTarget } from './target-types.ts'; +import type { FuzzFailure, FuzzTarget } from './target-types.ts'; -export type { FuzzFailure, FuzzFailureKind }; +// Re-exported because every existing consumer imports the failure type from the invariant it +// belongs to; the declaration moved to target-types.ts only to keep targets cycle-free. +export type { FuzzFailure }; /** * Runs one case and returns the invariant violation it produced, or `null`. diff --git a/scripts/fuzz/validation-arbitraries.ts b/scripts/fuzz/validation-arbitraries.ts index 6a3e605d93..20bdf1bedd 100644 --- a/scripts/fuzz/validation-arbitraries.ts +++ b/scripts/fuzz/validation-arbitraries.ts @@ -66,29 +66,62 @@ function collidingFlagNames(definitions: readonly FlagDefinition[]): Set return new Set([...counts.entries()].filter(([, count]) => count > 1).map(([name]) => name)); } -const COLLIDING_FLAG_NAMES = collidingFlagNames(getFlagDefinitions()); +/** + * Deriving the surface walks the whole command-metadata registry, so it is computed on first use + * rather than at import: a `--input-file` replay or a classic-target run must not pay for a + * generator it never asks for (it did, and timed out the coverage-instrumented promotion test). + */ +function memoize(build: () => T): () => T { + let value: T | undefined; + return () => (value ??= build()); +} + +const collidingNames = memoize(() => collidingFlagNames(getFlagDefinitions())); /** Flag definitions with a single unambiguous long name, excluding the special keys. */ -const SAFE_FLAG_POOL: readonly FlagDefinition[] = getFlagDefinitions().filter( - (definition) => - !EXCLUDED_FLAG_KEYS.has(definition.key) && - definition.names.some((name) => name.startsWith('--') && !COLLIDING_FLAG_NAMES.has(name)), +const safeFlagPool = memoize(() => + getFlagDefinitions().filter( + (definition) => + !EXCLUDED_FLAG_KEYS.has(definition.key) && + definition.names.some((name) => name.startsWith('--') && !collidingNames().has(name)), + ), ); function flagToken(definition: FlagDefinition): string { - return definition.names.find((name) => name.startsWith('--') && !COLLIDING_FLAG_NAMES.has(name))!; + return definition.names.find((name) => name.startsWith('--') && !collidingNames().has(name))!; } -const CLI_SURFACES: readonly CliCommandSurface[] = listCliCommandNames() - .filter((name) => !EXCLUDED_CLI_COMMANDS.has(name)) - .map((name) => { - const schema = getCliCommandSchema(name); - return { - name, - maxPositionals: schema.allowsExtraPositionals ? null : (schema.positionalArgs?.length ?? 0), - flags: SAFE_FLAG_POOL.filter((definition) => isFlagSupportedForCommand(definition.key, name)), - }; - }); +const cliSurfaces = memoize(() => + listCliCommandNames() + .filter((name) => !EXCLUDED_CLI_COMMANDS.has(name)) + .map((name) => { + const schema = getCliCommandSchema(name); + return { + name, + maxPositionals: schema.allowsExtraPositionals ? null : (schema.positionalArgs?.length ?? 0), + flags: safeFlagPool().filter((definition) => + isFlagSupportedForCommand(definition.key, name), + ), + }; + }), +); + +/** Inclusive bounds for a numeric flag, defaulted when the schema leaves an end open. */ +function numericBounds(definition: FlagDefinition): { low: number; high: number } { + const low = definition.min ?? 0; + return { low, high: definition.max ?? low + 1000 }; +} + +/** + * A schema-valid numeric value. Floats take endpoints and the midpoint only: modulo arithmetic + * can drift past a fractional `max` (a 33k-case slice produced `--scale=1.110000000000017`, + * which the parser rightly rejected — a phantom finding, not a bug). + */ +function validNumericValue(definition: FlagDefinition, salt: number): string { + const { low, high } = numericBounds(definition); + if (definition.type === 'int') return String(low + (salt % (high - low + 1))); + return String([low, high, (low + high) / 2][salt % 3]); +} /** A schema-valid value for one flag, salt-selected so shrinking stays deterministic. */ function validFlagValue(definition: FlagDefinition, salt: number): string { @@ -96,17 +129,8 @@ function validFlagValue(definition: FlagDefinition, salt: number): string { const values = definition.enumValues ?? []; return values[salt % Math.max(values.length, 1)] ?? '1'; } - if (definition.type === 'int') { - const low = definition.min ?? 0; - const high = definition.max ?? low + 1000; - return String(low + (salt % (high - low + 1))); - } - if (definition.type === 'number') { - // Endpoints and midpoint only: float modulo arithmetic can drift past a fractional `max` - // (a 33k-case nightly slice caught `--scale=1.110000000000017` as a phantom rejection). - const low = definition.min ?? 0; - const high = definition.max ?? low + 1000; - return String([low, high, (low + high) / 2][salt % 3]); + if (definition.type === 'int' || definition.type === 'number') { + return validNumericValue(definition, salt); } const value = SAFE_VALUES[salt % SAFE_VALUES.length]!; return value.length === 0 ? 'value' : value; @@ -147,8 +171,10 @@ const isValueFlag = (definition: FlagDefinition) => definition.type === 'int' || definition.type === 'number'); -const FAKE_COMMANDS = ['frobnicate', 'tapp', 'navigate', 'clik', 'snapshoot', 'opne'].filter( - (name) => !isKnownCliCommandName(name), +const fakeCommands = memoize(() => + ['frobnicate', 'tapp', 'navigate', 'clik', 'snapshoot', 'opne'].filter( + (name) => !isKnownCliCommandName(name), + ), ); const CLI_MUTATIONS: readonly CliMutation[] = [ @@ -172,7 +198,7 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ { name: 'unsupported-flag', apply: (base) => { - const foreign = SAFE_FLAG_POOL.filter( + const foreign = safeFlagPool().filter( (definition) => !isFlagSupportedForCommand(definition.key, base.surface.name), ); const definition = foreign[base.salt % Math.max(foreign.length, 1)]; @@ -247,7 +273,8 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ { name: 'unknown-command', apply: (base) => { - const name = FAKE_COMMANDS[base.salt % FAKE_COMMANDS.length]!; + const names = fakeCommands(); + const name = names[base.salt % names.length]!; return { payload: [name, ...base.positionals], mutation: 'unknown-command', expect: REJECT }; }, }, @@ -277,7 +304,8 @@ const cliBaseArb: fc.Arbitrary = fc salt: fc.nat({ max: 10_000 }), }) .map(({ surfaceIndex, positionals, flagIndices, salt }) => { - const surface = CLI_SURFACES[surfaceIndex % CLI_SURFACES.length]!; + const surfaces = cliSurfaces(); + const surface = surfaces[surfaceIndex % surfaces.length]!; const maxPositionals = surface.maxPositionals ?? 3; const flags = [ ...new Map( From 96cb40423b285167c67eab5b2fdebf8d3d7e15b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 08:32:20 +0200 Subject: [PATCH 05/11] test(fuzz): resolve validation generators in the run path so corpus replay keeps its small module graph --- scripts/fuzz/arbitraries.ts | 13 ------------- scripts/fuzz/generate.ts | 13 ++++++++++++- scripts/fuzz/validation-arbitraries.ts | 8 ++++++++ 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/scripts/fuzz/arbitraries.ts b/scripts/fuzz/arbitraries.ts index 9e221c7c5f..204a72a144 100644 --- a/scripts/fuzz/arbitraries.ts +++ b/scripts/fuzz/arbitraries.ts @@ -10,7 +10,6 @@ import fc from 'fast-check'; import { replayScriptArb } from '../../src/__tests__/test-utils/property-arbitraries.ts'; import type { FuzzTarget, FuzzTargetName } from './target-types.ts'; -import { cliValidationArb, maestroValidationArb } from './validation-arbitraries.ts'; const SELECTOR_VALUE_HAZARDS = [ '', @@ -183,20 +182,8 @@ const STRUCTURED_BASES: Partial>> = 'batch-steps': fc.json({ maxDepth: 3 }), }; -/** - * Validation targets generate their own expectation-carrying envelopes (#1781 B2); splicing - * hazards into the envelope JSON would corrupt the envelope, not the payload, so they are - * exempt from the corruption/noise mix. Payload-level hostility lives inside their generators. - */ -const VALIDATION_ARBITRARIES: Partial>> = { - 'cli-validation': cliValidationArb, - 'maestro-validation': maestroValidationArb, -}; - /** The case distribution for one target: mostly near-miss, some valid, some pure noise. */ export function arbitraryForTarget(target: FuzzTarget): fc.Arbitrary { - const validation = VALIDATION_ARBITRARIES[target.name]; - if (validation !== undefined) return validation; const seeded = fc.constantFrom(...target.seeds); const structured = STRUCTURED_BASES[target.name]; const base = structured === undefined ? seeded : fc.oneof(seeded, structured); diff --git a/scripts/fuzz/generate.ts b/scripts/fuzz/generate.ts index 6ddd2737f4..d02341a882 100644 --- a/scripts/fuzz/generate.ts +++ b/scripts/fuzz/generate.ts @@ -7,6 +7,7 @@ import fc from 'fast-check'; import { arbitraryForTarget } from './arbitraries.ts'; +import { validationArbitraryFor } from './validation-arbitraries.ts'; import { type CaseRunner, createCaseRunner } from './execute.ts'; import type { FuzzFailure } from './invariant.ts'; import type { FuzzTarget } from './target-types.ts'; @@ -68,6 +69,16 @@ export async function generateAndCheck( } } +/** + * Validation targets carry their own expectation-encoding generators; splicing hazards into an + * envelope would corrupt the envelope rather than the payload. They are resolved here rather + * than inside `arbitraryForTarget` so the corpus-replay unit file — which only ever samples the + * classic targets — does not pull the CLI schema registry into its instrumented worker (#1824). + */ +function casesFor(target: FuzzTarget): fc.Arbitrary { + return validationArbitraryFor(target.name) ?? arbitraryForTarget(target); +} + /** The generated half of a run: fast-check picks the inputs and shrinks any counterexample. */ async function checkGenerated( target: FuzzTarget, @@ -76,7 +87,7 @@ async function checkGenerated( ): Promise { let cases = target.seeds.length; const details = await fc.check( - fc.asyncProperty(arbitraryForTarget(target), async (input) => { + fc.asyncProperty(casesFor(target), async (input) => { cases += 1; return (await runner.run(input)) === null; }), diff --git a/scripts/fuzz/validation-arbitraries.ts b/scripts/fuzz/validation-arbitraries.ts index 20bdf1bedd..e947962264 100644 --- a/scripts/fuzz/validation-arbitraries.ts +++ b/scripts/fuzz/validation-arbitraries.ts @@ -17,6 +17,7 @@ import { type FlagDefinition, } from '../../src/cli-schema/command-schema.ts'; import { isFlagSupportedForCommand } from '../../src/cli-schema/option-schema.ts'; +import type { FuzzTargetName } from './target-types.ts'; import { encodeValidationCase, type ValidationCase } from './validation-case.ts'; const REJECT = { outcome: 'reject', code: 'INVALID_ARGS' } as const; @@ -460,3 +461,10 @@ export const maestroValidationArb: fc.Arbitrary = fc expect: REJECT, }); }); + +/** The generator for a validation target, or `undefined` for the classic targets. */ +export function validationArbitraryFor(target: FuzzTargetName): fc.Arbitrary | undefined { + if (target === 'cli-validation') return cliValidationArb; + if (target === 'maestro-validation') return maestroValidationArb; + return undefined; +} From 502952c685e4cd19b30b905bc80e37f93bc23ff2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 09:23:32 +0200 Subject: [PATCH 06/11] test(fuzz): weight the CLI budget toward command validation, pin the finite classes as seeds, guard lazy surface derivation --- .github/workflows/replays-nightly.yml | 10 +- .../validation-arbitraries.test.ts.snap | 2 - scripts/fuzz/generate.ts | 12 +- scripts/fuzz/targets.ts | 25 ++++ scripts/fuzz/validation-arbitraries.test.ts | 68 ++++++--- scripts/fuzz/validation-arbitraries.ts | 132 ++++++++++++------ 6 files changed, 180 insertions(+), 69 deletions(-) diff --git a/.github/workflows/replays-nightly.yml b/.github/workflows/replays-nightly.yml index 40b2e354b0..6ba826a47c 100644 --- a/.github/workflows/replays-nightly.yml +++ b/.github/workflows/replays-nightly.yml @@ -12,7 +12,7 @@ on: fuzz-iterations: description: 'Parser fuzz cases per target' required: false - default: '33000' + default: '38000' fuzz-seed: description: 'Parser fuzz PRNG seed (defaults to the run number)' required: false @@ -35,8 +35,10 @@ jobs: # exploring, and anything it catches is appended to the checked-in corpus that the unit # lane replays (scripts/fuzz/corpus/regressions.json). # - # 33,000 cases/target holds the job's wall-clock at the pre-B2 five-target/50k budget - # (~2min measured) now that seven targets share it — the budget is flat, not the depth. + # 38,000 cases/target holds the job's wall-clock at the pre-B2 five-target/50k budget now that + # seven targets share it: measured on a quiet host, 7x38k = 16.5s against 5x50k = 16.6-16.9s. + # Chosen by measurement rather than rounding — 33k would have been 2s cheaper but cost the + # five untouched targets a third of their depth for no wall-clock reason. nightly-parser-fuzz: name: Parser Fuzz Lane runs-on: ubuntu-latest @@ -65,7 +67,7 @@ jobs: - name: Fuzz parsers if: always() env: - FUZZ_ITERATIONS: ${{ github.event.inputs.fuzz-iterations || '33000' }} + FUZZ_ITERATIONS: ${{ github.event.inputs.fuzz-iterations || '38000' }} FUZZ_SEED: ${{ github.event.inputs.fuzz-seed || github.run_number }} uses: ./.github/actions/run-gate with: diff --git a/scripts/fuzz/__snapshots__/validation-arbitraries.test.ts.snap b/scripts/fuzz/__snapshots__/validation-arbitraries.test.ts.snap index e61b690670..61976935b4 100644 --- a/scripts/fuzz/__snapshots__/validation-arbitraries.test.ts.snap +++ b/scripts/fuzz/__snapshots__/validation-arbitraries.test.ts.snap @@ -2,9 +2,7 @@ exports[`cli-validation generator > exercises every mutation class, including valid accept cases 1`] = ` [ - "back-mode-conflict", "bad-enum-value", - "batch-step-source", "boolean-with-value", "excess-positional", "int-out-of-range", diff --git a/scripts/fuzz/generate.ts b/scripts/fuzz/generate.ts index d02341a882..436befaa09 100644 --- a/scripts/fuzz/generate.ts +++ b/scripts/fuzz/generate.ts @@ -71,9 +71,15 @@ export async function generateAndCheck( /** * Validation targets carry their own expectation-encoding generators; splicing hazards into an - * envelope would corrupt the envelope rather than the payload. They are resolved here rather - * than inside `arbitraryForTarget` so the corpus-replay unit file — which only ever samples the - * classic targets — does not pull the CLI schema registry into its instrumented worker (#1824). + * envelope would corrupt the envelope rather than the payload, so they bypass `arbitraryForTarget`. + * + * The split is organizational only. It was first committed claiming it kept the CLI schema + * registry out of corpus-replay's instrumented module graph; that claim was wrong. + * `corpus-replay.test.ts` imports `targets.ts`, which imports `src/cli/parser/args.ts`, which + * already pulls `command-schema`, `option-schema`, and `command-catalog`, and coverage instruments + * `src/**` only — the instrumented set is identical either way. What actually fixed the + * coverage-instrumented startup was deriving the CLI surface lazily in `validation-arbitraries.ts`; + * `validationSurfaceBuildCount()` is the guard against that regressing. */ function casesFor(target: FuzzTarget): fc.Arbitrary { return validationArbitraryFor(target.name) ?? arbitraryForTarget(target); diff --git a/scripts/fuzz/targets.ts b/scripts/fuzz/targets.ts index ea1d0260f4..65b9a1cb64 100644 --- a/scripts/fuzz/targets.ts +++ b/scripts/fuzz/targets.ts @@ -124,6 +124,31 @@ export const FUZZ_TARGETS: readonly FuzzTarget[] = [ mutation: 'missing-flag-value', expect: { outcome: 'reject', code: 'INVALID_ARGS' }, }), + // Two command-validation rules whose entire input space is a handful of strings: `batch` is + // the only command with a step-source rule, and `backMode` the only flag key reachable + // through two tokens. Generating them re-executed ~15 payloads thousands of times a night + // for no added reach, so they are pinned here and run verbatim once per run, before any + // generated case. They are seed regressions, not generated reach — #1781's table says so. + encodeValidationCase({ + payload: ['batch'], + mutation: 'batch-step-source-none', + expect: { outcome: 'reject', code: 'INVALID_ARGS' }, + }), + encodeValidationCase({ + payload: ['batch', '--steps=[]', '--steps-file=steps.json'], + mutation: 'batch-step-source-both', + expect: { outcome: 'reject', code: 'INVALID_ARGS' }, + }), + encodeValidationCase({ + payload: ['back', '--in-app', '--system'], + mutation: 'conflicting-flag-tokens', + expect: { outcome: 'reject', code: 'INVALID_ARGS' }, + }), + encodeValidationCase({ + payload: ['back', '--system', '--in-app'], + mutation: 'conflicting-flag-tokens', + expect: { outcome: 'reject', code: 'INVALID_ARGS' }, + }), ], }, { diff --git a/scripts/fuzz/validation-arbitraries.test.ts b/scripts/fuzz/validation-arbitraries.test.ts index 20552bb90d..8d09f7e570 100644 --- a/scripts/fuzz/validation-arbitraries.test.ts +++ b/scripts/fuzz/validation-arbitraries.test.ts @@ -7,13 +7,19 @@ // violations must surface as validation-layer errors, past the tokenizer. import fc from 'fast-check'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { checkCase, describeFailure } from './invariant.ts'; import { getFuzzTarget } from './registry.ts'; import { decodeValidationCase } from './validation-case.ts'; -import { cliValidationArb, maestroValidationArb } from './validation-arbitraries.ts'; +import { + cliValidationArb, + maestroValidationArb, + validationSurfaceBuildCount, +} from './validation-arbitraries.ts'; -const SAMPLE_SIZE = 500; +// A mismatch that fires at ~1-in-1000 must not pass here and then phantom nightly: the nightly +// draws 38,000 cases per target, so this samples 7.9% of it rather than the 1.5% it began at. +const SAMPLE_SIZE = 3_000; const SEED = 1; const TARGETS = [ @@ -50,7 +56,7 @@ describe.each(TARGETS)('%s generator', (targetName, arbitrary) => { }); }); -describe('planted violations surface past the tokenizer', () => { +describe('planted violations surface where the generator says they do', () => { function rejectionMessageFor(targetName: (typeof TARGETS)[number][0], mutation: string): string { const arbitrary = targetName === 'cli-validation' ? cliValidationArb : maestroValidationArb; const sample = fc.sample(arbitrary, { numRuns: SAMPLE_SIZE, seed: SEED }); @@ -66,20 +72,38 @@ describe('planted violations surface past the tokenizer', () => { throw new Error(`expected ${mutation} case to reject: ${JSON.stringify(decoded.payload)}`); } - it('CLI excess positionals die in positional-arity validation (#1433)', () => { - expect(rejectionMessageFor('cli-validation', 'excess-positional')).toMatch( - /accepts at most \d+ positional argument/, - ); - }); - - it('CLI enum violations die in flag-value validation', () => { - expect(rejectionMessageFor('cli-validation', 'bad-enum-value')).toMatch(/^Invalid /); + // The layer each class actually reaches, asserted rather than implied. `command-validation` + // classes survive the argv scan and are refused by finalizeParsedArgs — the reach B2 adds. + // `token-scan` classes are refused inside parseFlagValue while argv is still being scanned: + // the layer the classic `cli-args` target already reaches. They stay for the error-code + // assertion cli-args cannot make, and are not claimed as new reach anywhere. + it.each([ + ['excess-positional', 'command-validation', /accepts at most \d+ positional argument/], + ['unsupported-flag', 'command-validation', /is not supported for command/], + ['unknown-command', 'command-validation', /^Unknown command:/], + ['bad-enum-value', 'token-scan', /^Invalid /], + ['int-out-of-range', 'token-scan', /^Invalid /], + ['missing-flag-value', 'token-scan', /requires a value\./], + ['boolean-with-value', 'token-scan', /does not take a value\./], + ])('CLI %s is refused in %s', (mutation, _layer, expected) => { + expect(rejectionMessageFor('cli-validation', mutation)).toMatch(expected); }); - it('CLI unsupported flags die in per-command flag support validation', () => { - expect(rejectionMessageFor('cli-validation', 'unsupported-flag')).toMatch( - /is not supported for command/, - ); + it('spends most of the CLI budget on classes that survive the argv scan', () => { + const sample = fc.sample(cliValidationArb, { numRuns: SAMPLE_SIZE, seed: SEED }); + const tokenScan = new Set([ + 'bad-enum-value', + 'int-out-of-range', + 'missing-flag-value', + 'boolean-with-value', + ]); + const mutated = sample + .map((input) => decodeValidationCase(input)!.mutation) + .filter((mutation) => mutation !== 'valid'); + const scanned = mutated.filter((mutation) => tokenScan.has(mutation)).length; + // Weighted down rather than removed, so the ratio is the lane's disclosed reach: a weight + // edit that quietly hands the budget back to the token scan fails here. + expect(scanned / mutated.length).toBeLessThan(0.25); }); it('Maestro unknown commands die in command-shape validation, not the YAML tokenizer', () => { @@ -95,6 +119,18 @@ describe('planted violations surface past the tokenizer', () => { }); }); +describe('generator startup', () => { + // The eager version of this derivation charged every harness path the command-metadata + // registry and timed out the coverage-instrumented promotion test. Importing must stay free. + it('does not derive the CLI surface until a case is actually generated', async () => { + vi.resetModules(); + const fresh = await import('./validation-arbitraries.ts'); + expect(fresh.validationSurfaceBuildCount()).toBe(0); + fc.sample(fresh.cliValidationArb, { numRuns: 1, seed: SEED }); + expect(fresh.validationSurfaceBuildCount()).toBe(1); + }); +}); + describe('validation envelope guard', () => { it('reports a malformed envelope as a finding instead of crashing the worker', () => { const target = getFuzzTarget('cli-validation'); diff --git a/scripts/fuzz/validation-arbitraries.ts b/scripts/fuzz/validation-arbitraries.ts index e947962264..b53e506b0b 100644 --- a/scripts/fuzz/validation-arbitraries.ts +++ b/scripts/fuzz/validation-arbitraries.ts @@ -20,7 +20,6 @@ import { isFlagSupportedForCommand } from '../../src/cli-schema/option-schema.ts import type { FuzzTargetName } from './target-types.ts'; import { encodeValidationCase, type ValidationCase } from './validation-case.ts'; -const REJECT = { outcome: 'reject', code: 'INVALID_ARGS' } as const; const ACCEPT = { outcome: 'accept' } as const; // Benign, occasionally hostile positional/flag values. No leading '-': a dash token would be @@ -77,6 +76,18 @@ function memoize(build: () => T): () => T { return () => (value ??= build()); } +let surfaceBuilds = 0; + +/** + * How many times the schema-derived surface has been built. It must still be 0 after importing + * this module: an eager build cost every harness path — `--input-file` and the classic targets + * included — the whole command-metadata registry, and timed out the coverage-instrumented + * promotion test. Asserted in validation-arbitraries.test.ts, which is the actual #1824 guard. + */ +export function validationSurfaceBuildCount(): number { + return surfaceBuilds; +} + const collidingNames = memoize(() => collidingFlagNames(getFlagDefinitions())); /** Flag definitions with a single unambiguous long name, excluding the special keys. */ @@ -92,8 +103,9 @@ function flagToken(definition: FlagDefinition): string { return definition.names.find((name) => name.startsWith('--') && !collidingNames().has(name))!; } -const cliSurfaces = memoize(() => - listCliCommandNames() +const cliSurfaces = memoize(() => { + surfaceBuilds += 1; + return listCliCommandNames() .filter((name) => !EXCLUDED_CLI_COMMANDS.has(name)) .map((name) => { const schema = getCliCommandSchema(name); @@ -104,8 +116,8 @@ const cliSurfaces = memoize(() => isFlagSupportedForCommand(definition.key, name), ), }; - }), -); + }); +}); /** Inclusive bounds for a numeric flag, defaulted when the schema leaves an end open. */ function numericBounds(definition: FlagDefinition): { low: number; high: number } { @@ -160,9 +172,24 @@ function validArgv(base: CliBase): string[] { ]; } +/** + * Which parser layer refuses the planted violation. `token-scan` violations are refused inside + * `parseRawArgs`/`parseFlagValue` while argv is still being scanned — the layer the classic + * `cli-args` target already reaches; `command-validation` violations survive the scan and are + * refused by `finalizeParsedArgs` (arity, per-command flag support, command identity), the layer + * B2 exists to reach. Asserted per class in validation-arbitraries.test.ts, so the lane's real + * reach stays disclosed rather than implied. + */ +type CliMutationLayer = 'token-scan' | 'command-validation'; + type CliMutation = { name: string; - apply: (base: CliBase) => ValidationCase | null; + layer: CliMutationLayer; + /** Relative share of the mutated budget; command-validation classes are weighted up. */ + weight: number; + /** The `AppError.code` this class must be refused with. */ + code: string; + apply: (base: CliBase) => Omit | null; }; const isValueFlag = (definition: FlagDefinition) => @@ -182,6 +209,9 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ { // #1433: a bounded command must refuse extra positionals instead of swallowing them. name: 'excess-positional', + layer: 'command-validation', + weight: 6, + code: 'INVALID_ARGS', apply: (base) => { if (base.surface.maxPositionals === null) return null; const filler = SAFE_VALUES[base.salt % SAFE_VALUES.length] || 'extra'; @@ -192,12 +222,14 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ return { payload: [base.surface.name, ...positionals], mutation: 'excess-positional', - expect: REJECT, }; }, }, { name: 'unsupported-flag', + layer: 'command-validation', + weight: 6, + code: 'INVALID_ARGS', apply: (base) => { const foreign = safeFlagPool().filter( (definition) => !isFlagSupportedForCommand(definition.key, base.surface.name), @@ -207,12 +239,14 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ return { payload: [...validArgv(base), renderFlag(definition, base.salt)], mutation: 'unsupported-flag', - expect: REJECT, }; }, }, { name: 'bad-enum-value', + layer: 'token-scan', + weight: 1, + code: 'INVALID_ARGS', apply: (base) => { const enums = base.surface.flags.filter((definition) => definition.type === 'enum'); const definition = enums[base.salt % Math.max(enums.length, 1)]; @@ -220,12 +254,14 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ return { payload: [base.surface.name, `${flagToken(definition)}=bogus-${base.salt % 7}`], mutation: 'bad-enum-value', - expect: REJECT, }; }, }, { name: 'int-out-of-range', + layer: 'token-scan', + weight: 1, + code: 'INVALID_ARGS', apply: (base) => { const bounded = base.surface.flags.filter( (definition) => @@ -239,12 +275,14 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ return { payload: [base.surface.name, `${flagToken(definition)}=${value}`], mutation: 'int-out-of-range', - expect: REJECT, }; }, }, { name: 'missing-flag-value', + layer: 'token-scan', + weight: 1, + code: 'INVALID_ARGS', apply: (base) => { const valued = base.surface.flags.filter(isValueFlag); const definition = valued[base.salt % Math.max(valued.length, 1)]; @@ -252,12 +290,14 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ return { payload: [base.surface.name, flagToken(definition)], mutation: 'missing-flag-value', - expect: REJECT, }; }, }, { name: 'boolean-with-value', + layer: 'token-scan', + weight: 1, + code: 'INVALID_ARGS', apply: (base) => { const booleans = base.surface.flags.filter( (definition) => definition.type === 'boolean' || definition.setValue !== undefined, @@ -267,34 +307,20 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ return { payload: [...validArgv(base), `${flagToken(definition)}=true`], mutation: 'boolean-with-value', - expect: REJECT, }; }, }, { name: 'unknown-command', + layer: 'command-validation', + weight: 3, + code: 'INVALID_ARGS', apply: (base) => { const names = fakeCommands(); const name = names[base.salt % names.length]!; - return { payload: [name, ...base.positionals], mutation: 'unknown-command', expect: REJECT }; + return { payload: [name, ...base.positionals], mutation: 'unknown-command' }; }, }, - { - name: 'batch-step-source', - apply: (base) => ({ - payload: base.salt % 2 === 0 ? ['batch'] : ['batch', '--steps=[]', '--steps-file=steps.json'], - mutation: 'batch-step-source', - expect: REJECT, - }), - }, - { - name: 'back-mode-conflict', - apply: () => ({ - payload: ['back', '--in-app', '--system'], - mutation: 'back-mode-conflict', - expect: REJECT, - }), - }, ]; const cliBaseArb: fc.Arbitrary = fc @@ -319,21 +345,34 @@ const cliBaseArb: fc.Arbitrary = fc return { surface, positionals: positionals.slice(0, maxPositionals), flags, salt }; }); -/** Encoded CLI validation cases: ~1/3 valid (expect accept), the rest planted violations. */ +/** Mutation indices expanded by weight, so command-validation classes take the larger share. */ +const weightedCliMutations = memoize(() => + CLI_MUTATIONS.flatMap((mutation, index) => Array.from({ length: mutation.weight }, () => index)), +); + +function validCase(base: CliBase): string { + return encodeValidationCase({ payload: validArgv(base), mutation: 'valid', expect: ACCEPT }); +} + +/** Encoded CLI validation cases: ~1/4 valid (expect accept), the rest planted violations. */ export const cliValidationArb: fc.Arbitrary = fc .record({ base: cliBaseArb, mutationIndex: fc.nat() }) .map(({ base, mutationIndex }) => { - // A third of the space stays valid so a false rejection is discoverable too. - if (mutationIndex % (CLI_MUTATIONS.length + 4) >= CLI_MUTATIONS.length) { - return encodeValidationCase({ payload: validArgv(base), mutation: 'valid', expect: ACCEPT }); - } + const weighted = weightedCliMutations(); + // A quarter of the space stays valid so a false rejection is discoverable too. + if (mutationIndex % (weighted.length + 6) >= weighted.length) return validCase(base); // Rotate to the first applicable mutation so the map stays total. - for (let step = 0; step < CLI_MUTATIONS.length; step += 1) { - const mutation = CLI_MUTATIONS[(mutationIndex + step) % CLI_MUTATIONS.length]!; - const validationCase = mutation.apply(base); - if (validationCase) return encodeValidationCase(validationCase); + for (let step = 0; step < weighted.length; step += 1) { + const mutation = CLI_MUTATIONS[weighted[(mutationIndex + step) % weighted.length]!]!; + const built = mutation.apply(base); + if (built) { + return encodeValidationCase({ + ...built, + expect: { outcome: 'reject', code: mutation.code }, + }); + } } - return encodeValidationCase({ payload: validArgv(base), mutation: 'valid', expect: ACCEPT }); + return validCase(base); }); // --------------------------------------------------------------------------------------------- @@ -385,13 +424,15 @@ const FAKE_MAESTRO_COMMANDS = [ 'inputTextt', ] as const; -type MaestroMutation = { name: string; lines: (salt: number) => string[] }; +/** `code` is per class, like the CLI table: a class whose contract changes moves alone. */ +type MaestroMutation = { name: string; code: string; lines: (salt: number) => string[] }; const MAESTRO_MUTATIONS: readonly MaestroMutation[] = [ { // The B2 headline: an unknown command name must die in command validation, not the // YAML tokenizer — mutated names used to be unreachable because the YAML never parsed. name: 'unsupported-command', + code: 'INVALID_ARGS', lines: (salt) => { const name = FAKE_MAESTRO_COMMANDS[salt % FAKE_MAESTRO_COMMANDS.length]!; return salt % 2 === 0 ? [`- ${name}`] : [`- ${name}: ${yamlText(salt)}`]; @@ -399,6 +440,7 @@ const MAESTRO_MUTATIONS: readonly MaestroMutation[] = [ }, { name: 'unsupported-field', + code: 'INVALID_ARGS', lines: (salt) => salt % 2 === 0 ? ['- tapOn:', ` bogusField: ${yamlText(salt)}`] @@ -406,15 +448,17 @@ const MAESTRO_MUTATIONS: readonly MaestroMutation[] = [ }, { name: 'multi-key-command', + code: 'INVALID_ARGS', lines: (salt) => [`- tapOn: ${yamlText(salt)}`, ` inputText: ${yamlText(salt + 1)}`], }, { name: 'missing-required', + code: 'INVALID_ARGS', lines: (salt) => salt % 2 === 0 ? ['- inputText:'] : ['- extendedWaitUntil:', ' timeout: 500'], }, - { name: 'bad-press-key', lines: () => ['- pressKey: sleep'] }, - { name: 'scroll-options', lines: () => ['- scroll:', ' direction: UP'] }, + { name: 'bad-press-key', code: 'INVALID_ARGS', lines: () => ['- pressKey: sleep'] }, + { name: 'scroll-options', code: 'INVALID_ARGS', lines: () => ['- scroll:', ' direction: UP'] }, ]; type MaestroBase = { commandPicks: number[]; salt: number; withConfig: boolean }; @@ -452,13 +496,13 @@ export const maestroValidationArb: fc.Arbitrary = fc return encodeValidationCase({ payload: `appId: com.example.app\nbogusKey: 1\n---\n${renderMaestroFlow({ ...base, withConfig: false })}`, mutation: 'config-unknown-key', - expect: REJECT, + expect: { outcome: 'reject', code: 'INVALID_ARGS' }, }); } return encodeValidationCase({ payload: renderMaestroFlow(base, mutation.lines(base.salt), insertAt), mutation: mutation.name, - expect: REJECT, + expect: { outcome: 'reject', code: mutation.code }, }); }); From 18354bd6eaf27f1dbd5bdcab1f8ef3fb8c841115 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 09:28:45 +0200 Subject: [PATCH 07/11] docs(testing): describe the validation lane's layer split, seed-pinned classes, and PR-time gates --- docs/agents/testing.md | 46 ++++++++++----------- scripts/fuzz/validation-arbitraries.test.ts | 6 +-- 2 files changed, 22 insertions(+), 30 deletions(-) diff --git a/docs/agents/testing.md b/docs/agents/testing.md index efaaf3f3a1..029bf00a01 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -380,31 +380,27 @@ hangs (a worker-thread watchdog attributes a stall to the exact input). Two **validation targets** (#1781 B2) — `cli-validation` and `maestro-validation` — go further: their cases are built from the real command surface (the CLI schema registry, the Maestro command -shapes) so they tokenize cleanly and die in command validation, and each case carries the outcome -its generator planted (`scripts/fuzz/validation-case.ts`). The judge then also fails a **silent -acceptance** of an input built to be invalid (the #1433 excess-positionals class) and a rejection -with the **wrong `AppError.code`** — not just "some error". Their generator expectations are gated -at PR time by `scripts/fuzz/validation-arbitraries.test.ts` (unit-core: in-process, no worker), so -a drifted generator fails in seconds instead of producing phantom nightly findings. - -```sh -pnpm fuzz:parsers # all targets, 2,000 cases each, seed 1 -pnpm fuzz:parsers --target selector --iterations 50000 --seed 7 -pnpm fuzz:parsers --input-file .tmp/fuzz/.json # repro a saved case -pnpm fuzz:parsers --input-file .tmp/fuzz/.json --append-corpus # …and pin it -pnpm fuzz:parsers --self-check # require the harness to still fail -``` - -The generating run is nightly (`Parser Fuzz Lane` in `.github/workflows/replays-nightly.yml`, seeded -by the run number). Every terminal path — pass, fail, `--self-check`, or a crash in the harness -itself — writes `/run-envelope.json` on the shared lane contract -(`scripts/lib/lane-envelope.ts`, #1430), with the lane's own facts under `data`: mode, per-target -cases/failures/durations, failures, repro commands, and `stage` (`error` marks a run that could not -complete itself, since the shared `result` is only `pass`/`fail`). `configHash` hashes the modules -that decide a case set, so "the same seed means different inputs now" is distinguishable from "the -parsers changed". The self-check and fuzz steps write to separate artifact subdirectories and both -run unconditionally; the step summary prints each envelope it finds and never fails on a missing -file. +shapes) so they tokenize cleanly, and each case carries the outcome its generator planted +(`scripts/fuzz/validation-case.ts`). The judge then also fails a **silent acceptance** of an input +built to be invalid (the #1433 excess-positionals class, which a rejection-only invariant cannot +see at all) and a rejection with the **wrong `AppError.code`** — not just "some error". + +Each CLI mutation class declares the parser layer it is refused by. `command-validation` classes +(excess positionals, unsupported-for-command flags, unknown commands) survive the argv scan and +reach `finalizeParsedArgs` — the reach these targets exist to add; `token-scan` classes (bad enum +values, out-of-range ints, missing flag values, valued booleans) are refused inside `parseFlagValue` +while argv is still being scanned, which is where the classic `cli-args` target already reaches. +They are weighted down to under a quarter of the mutated budget and kept only for the error-code +assertion `cli-args` cannot make. Rules whose whole input space is a few strings (`batch`'s +step-source rule, the `--in-app`/`--system` conflict) are **pinned seed cases** rather than +generated classes, so the nightly does not re-execute a handful of literals thousands of times. + +`scripts/fuzz/validation-arbitraries.test.ts` (unit-core, in-process) gates all of this at PR time: +fixed-seed samples must hold every planted expectation, every mutation class must still fire, each +class must be refused in its declared layer, the token-scan share must stay under 25%, and the +schema surface must not be derived until a case is generated (an eager derivation timed out the +coverage-instrumented promotion test). A drifted generator fails in seconds instead of producing +phantom nightly findings. Cases come from fast-check arbitraries (`scripts/fuzz/arbitraries.ts`, validation envelopes in `scripts/fuzz/validation-arbitraries.ts`) built on the hazard vocabulary diff --git a/scripts/fuzz/validation-arbitraries.test.ts b/scripts/fuzz/validation-arbitraries.test.ts index 8d09f7e570..a722654269 100644 --- a/scripts/fuzz/validation-arbitraries.test.ts +++ b/scripts/fuzz/validation-arbitraries.test.ts @@ -11,11 +11,7 @@ import { describe, expect, it, vi } from 'vitest'; import { checkCase, describeFailure } from './invariant.ts'; import { getFuzzTarget } from './registry.ts'; import { decodeValidationCase } from './validation-case.ts'; -import { - cliValidationArb, - maestroValidationArb, - validationSurfaceBuildCount, -} from './validation-arbitraries.ts'; +import { cliValidationArb, maestroValidationArb } from './validation-arbitraries.ts'; // A mismatch that fires at ~1-in-1000 must not pass here and then phantom nightly: the nightly // draws 38,000 cases per target, so this samples 7.9% of it rather than the 1.5% it began at. From bb8b04e8ddc0987fa035836b2b0ab8dcca6e1ba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 13:37:42 +0200 Subject: [PATCH 08/11] refactor(fuzz): split the validation generator into CLI and Maestro modules, mirrored in tests --- .../validation-arbitraries-cli.test.ts.snap | 14 + ...lidation-arbitraries-maestro.test.ts.snap} | 13 - .../fuzz/validation-arbitraries-cli.test.ts | 105 ++++ scripts/fuzz/validation-arbitraries-cli.ts | 353 ++++++++++++ .../validation-arbitraries-maestro.test.ts | 74 +++ .../fuzz/validation-arbitraries-maestro.ts | 138 +++++ scripts/fuzz/validation-arbitraries.test.ts | 143 +---- scripts/fuzz/validation-arbitraries.ts | 511 +----------------- scripts/fuzz/validation-case.test.ts | 39 ++ scripts/fuzz/validation-values.ts | 21 + vitest.config.ts | 7 +- 11 files changed, 768 insertions(+), 650 deletions(-) create mode 100644 scripts/fuzz/__snapshots__/validation-arbitraries-cli.test.ts.snap rename scripts/fuzz/__snapshots__/{validation-arbitraries.test.ts.snap => validation-arbitraries-maestro.test.ts.snap} (55%) create mode 100644 scripts/fuzz/validation-arbitraries-cli.test.ts create mode 100644 scripts/fuzz/validation-arbitraries-cli.ts create mode 100644 scripts/fuzz/validation-arbitraries-maestro.test.ts create mode 100644 scripts/fuzz/validation-arbitraries-maestro.ts create mode 100644 scripts/fuzz/validation-case.test.ts create mode 100644 scripts/fuzz/validation-values.ts diff --git a/scripts/fuzz/__snapshots__/validation-arbitraries-cli.test.ts.snap b/scripts/fuzz/__snapshots__/validation-arbitraries-cli.test.ts.snap new file mode 100644 index 0000000000..0836946777 --- /dev/null +++ b/scripts/fuzz/__snapshots__/validation-arbitraries-cli.test.ts.snap @@ -0,0 +1,14 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`cli-validation generator > exercises every mutation class, including valid accept cases 1`] = ` +[ + "bad-enum-value", + "boolean-with-value", + "excess-positional", + "int-out-of-range", + "missing-flag-value", + "unknown-command", + "unsupported-flag", + "valid", +] +`; diff --git a/scripts/fuzz/__snapshots__/validation-arbitraries.test.ts.snap b/scripts/fuzz/__snapshots__/validation-arbitraries-maestro.test.ts.snap similarity index 55% rename from scripts/fuzz/__snapshots__/validation-arbitraries.test.ts.snap rename to scripts/fuzz/__snapshots__/validation-arbitraries-maestro.test.ts.snap index 61976935b4..ecbaef8f44 100644 --- a/scripts/fuzz/__snapshots__/validation-arbitraries.test.ts.snap +++ b/scripts/fuzz/__snapshots__/validation-arbitraries-maestro.test.ts.snap @@ -1,18 +1,5 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`cli-validation generator > exercises every mutation class, including valid accept cases 1`] = ` -[ - "bad-enum-value", - "boolean-with-value", - "excess-positional", - "int-out-of-range", - "missing-flag-value", - "unknown-command", - "unsupported-flag", - "valid", -] -`; - exports[`maestro-validation generator > exercises every mutation class, including valid accept cases 1`] = ` [ "bad-press-key", diff --git a/scripts/fuzz/validation-arbitraries-cli.test.ts b/scripts/fuzz/validation-arbitraries-cli.test.ts new file mode 100644 index 0000000000..282b6a2ee2 --- /dev/null +++ b/scripts/fuzz/validation-arbitraries-cli.test.ts @@ -0,0 +1,105 @@ +// Expectation gate for the CLI validation generator (#1781 B2). +// +// A validation case asserts a specific outcome the generator planted, so a drifted generator is +// worse than a weak one: it would report phantom findings nightly (or mark real bugs expected). +// This replays fixed-seed samples against the real parser in-process — every planted expectation +// must hold on a healthy tree, every class must still fire, and each class must be refused in the +// parser layer it claims, so the lane's real reach stays disclosed rather than implied. + +import fc from 'fast-check'; +import { describe, expect, it, vi } from 'vitest'; +import { describeFailure } from './invariant.ts'; +import { getFuzzTarget } from './registry.ts'; +import { decodeValidationCase } from './validation-case.ts'; +import { cliValidationArb } from './validation-arbitraries-cli.ts'; + +// A mismatch that fires at ~1-in-1000 must not pass here and then phantom nightly: the nightly +// draws 38,000 cases per target, so this samples 7.9% of it rather than the 1.5% it began at. +const SAMPLE_SIZE = 3_000; +const SEED = 1; + +const target = getFuzzTarget('cli-validation'); +const sample = fc.sample(cliValidationArb, { numRuns: SAMPLE_SIZE, seed: SEED }); + +describe('cli-validation generator', () => { + it('is deterministic for a seed, so a reported counterexample replays', () => { + const again = fc.sample(cliValidationArb, { numRuns: 32, seed: 7 }); + expect(again).toEqual(fc.sample(cliValidationArb, { numRuns: 32, seed: 7 })); + expect(again).not.toEqual(fc.sample(cliValidationArb, { numRuns: 32, seed: 8 })); + }); + + it('produces decodable envelopes whose expectations hold on a healthy tree', () => { + const failures = []; + for (const input of sample) { + expect(decodeValidationCase(input)).not.toBeNull(); + const failure = target.check!(input); + if (failure) failures.push(describeFailure(failure)); + } + expect(failures).toEqual([]); + }); + + it('exercises every mutation class, including valid accept cases', () => { + const mutations = new Set(sample.map((input) => decodeValidationCase(input)!.mutation)); + expect(mutations).toContain('valid'); + // A class that stops firing (surface drift, weight bug) is dead coverage and fails here. + expect([...mutations].sort()).toMatchSnapshot(); + }); +}); + +describe('planted CLI violations are refused where the generator says they are', () => { + function rejectionMessageFor(mutation: string): string { + const input = sample.find((entry) => decodeValidationCase(entry)!.mutation === mutation); + expect(input, `no ${mutation} case in the fixed-seed sample`).toBeDefined(); + const decoded = decodeValidationCase(input!)!; + try { + target.run(input!); + } catch (error) { + return (error as Error).message; + } + throw new Error(`expected ${mutation} case to reject: ${JSON.stringify(decoded.payload)}`); + } + + // `command-validation` classes survive the argv scan and reach finalizeParsedArgs — the reach + // these targets add. `token-scan` classes are refused inside parseFlagValue while argv is still + // being scanned: the layer the classic `cli-args` target already reaches, kept here for the + // error-code assertion cli-args cannot make and never claimed as new reach. + it.each([ + ['excess-positional', 'command-validation', /accepts at most \d+ positional argument/], + ['unsupported-flag', 'command-validation', /is not supported for command/], + ['unknown-command', 'command-validation', /^Unknown command:/], + ['bad-enum-value', 'token-scan', /^Invalid /], + ['int-out-of-range', 'token-scan', /^Invalid /], + ['missing-flag-value', 'token-scan', /requires a value\./], + ['boolean-with-value', 'token-scan', /does not take a value\./], + ])('%s is refused in %s', (mutation, _layer, expected) => { + expect(rejectionMessageFor(mutation)).toMatch(expected); + }); + + it('spends most of the budget on classes that survive the argv scan', () => { + const tokenScan = new Set([ + 'bad-enum-value', + 'int-out-of-range', + 'missing-flag-value', + 'boolean-with-value', + ]); + const mutated = sample + .map((input) => decodeValidationCase(input)!.mutation) + .filter((mutation) => mutation !== 'valid'); + const scanned = mutated.filter((mutation) => tokenScan.has(mutation)).length; + // Weighted down rather than removed, so the ratio is the lane's disclosed reach: a weight + // edit that quietly hands the budget back to the token scan fails here. + expect(scanned / mutated.length).toBeLessThan(0.25); + }); +}); + +describe('generator startup', () => { + // The eager version of this derivation charged every harness path the command-metadata registry + // and timed out the coverage-instrumented promotion test (#1824). Importing must stay free. + it('does not derive the CLI surface until a case is actually generated', async () => { + vi.resetModules(); + const fresh = await import('./validation-arbitraries-cli.ts'); + expect(fresh.validationSurfaceBuildCount()).toBe(0); + fc.sample(fresh.cliValidationArb, { numRuns: 1, seed: SEED }); + expect(fresh.validationSurfaceBuildCount()).toBe(1); + }); +}); diff --git a/scripts/fuzz/validation-arbitraries-cli.ts b/scripts/fuzz/validation-arbitraries-cli.ts new file mode 100644 index 0000000000..79b474562d --- /dev/null +++ b/scripts/fuzz/validation-arbitraries-cli.ts @@ -0,0 +1,353 @@ +// CLI command-line case generation for the `cli-validation` fuzz target (#1781 B2). +// +// The classic mutator splices hazards into a flat string, so a CLI case almost always dies in the +// token scan and the validation layer behind it goes unexercised. These cases are built FROM the +// schema registry — command catalog, per-command positional arity, per-command flag support, flag +// types — so argv tokenizes cleanly and the planted violation surfaces where the generator says +// it does. Each case records the outcome it was built to have; validation-case.ts judges the +// parser against it, which is what makes a silent acceptance (the #1433 class) reportable at all. + +import fc from 'fast-check'; +import { isKnownCliCommandName, listCliCommandNames } from '../../src/command-catalog.ts'; +import { + getCliCommandSchema, + getFlagDefinitions, + type FlagDefinition, +} from '../../src/cli-schema/command-schema.ts'; +import { isFlagSupportedForCommand } from '../../src/cli-schema/option-schema.ts'; +import { encodeValidationCase, type ValidationCase } from './validation-case.ts'; +import { ACCEPT, SAFE_VALUES } from './validation-values.ts'; + +type CliCommandSurface = { + name: string; + /** `null` when the schema allows extra positionals — the arity mutation does not apply. */ + maxPositionals: number | null; + /** Flags supported for this command, restricted to the unambiguous pool. */ + flags: readonly FlagDefinition[]; +}; + +// Commands whose parse path is deliberately special: cdp preserves post-command args verbatim, +// react-devtools passes unknown flags through, batch enforces a step-source invariant covered +// by its own fixed mutation below. +const EXCLUDED_CLI_COMMANDS = new Set(['cdp', 'react-devtools', 'batch']); + +// help/version reroute parsing, snapshotDiff rewrites the command, steps/stepsFile carry the +// batch step-source invariant. All are exercised elsewhere; here they would blur expectations. +const EXCLUDED_FLAG_KEYS = new Set(['help', 'version', 'snapshotDiff', 'steps', 'stepsFile']); + +function collidingFlagNames(definitions: readonly FlagDefinition[]): Set { + const counts = new Map(); + for (const definition of definitions) { + for (const name of definition.names) counts.set(name, (counts.get(name) ?? 0) + 1); + } + return new Set([...counts.entries()].filter(([, count]) => count > 1).map(([name]) => name)); +} + +/** + * Deriving the surface walks the whole command-metadata registry, so it is computed on first use + * rather than at import: a `--input-file` replay or a classic-target run must not pay for a + * generator it never asks for (it did, and timed out the coverage-instrumented promotion test). + */ +function memoize(build: () => T): () => T { + let value: T | undefined; + return () => (value ??= build()); +} + +let surfaceBuilds = 0; + +/** + * How many times the schema-derived surface has been built. It must still be 0 after importing + * this module: an eager build cost every harness path — `--input-file` and the classic targets + * included — the whole command-metadata registry, and timed out the coverage-instrumented + * promotion test. Asserted in validation-arbitraries.test.ts, which is the actual #1824 guard. + */ +export function validationSurfaceBuildCount(): number { + return surfaceBuilds; +} + +const collidingNames = memoize(() => collidingFlagNames(getFlagDefinitions())); + +/** Flag definitions with a single unambiguous long name, excluding the special keys. */ +const safeFlagPool = memoize(() => + getFlagDefinitions().filter( + (definition) => + !EXCLUDED_FLAG_KEYS.has(definition.key) && + definition.names.some((name) => name.startsWith('--') && !collidingNames().has(name)), + ), +); + +function flagToken(definition: FlagDefinition): string { + return definition.names.find((name) => name.startsWith('--') && !collidingNames().has(name))!; +} + +const cliSurfaces = memoize(() => { + surfaceBuilds += 1; + return listCliCommandNames() + .filter((name) => !EXCLUDED_CLI_COMMANDS.has(name)) + .map((name) => { + const schema = getCliCommandSchema(name); + return { + name, + maxPositionals: schema.allowsExtraPositionals ? null : (schema.positionalArgs?.length ?? 0), + flags: safeFlagPool().filter((definition) => + isFlagSupportedForCommand(definition.key, name), + ), + }; + }); +}); + +/** Inclusive bounds for a numeric flag, defaulted when the schema leaves an end open. */ +function numericBounds(definition: FlagDefinition): { low: number; high: number } { + const low = definition.min ?? 0; + return { low, high: definition.max ?? low + 1000 }; +} + +/** + * A schema-valid numeric value. Floats take endpoints and the midpoint only: modulo arithmetic + * can drift past a fractional `max` (a 33k-case slice produced `--scale=1.110000000000017`, + * which the parser rightly rejected — a phantom finding, not a bug). + */ +function validNumericValue(definition: FlagDefinition, salt: number): string { + const { low, high } = numericBounds(definition); + if (definition.type === 'int') return String(low + (salt % (high - low + 1))); + return String([low, high, (low + high) / 2][salt % 3]); +} + +/** A schema-valid value for one flag, salt-selected so shrinking stays deterministic. */ +function validFlagValue(definition: FlagDefinition, salt: number): string { + if (definition.type === 'enum') { + const values = definition.enumValues ?? []; + return values[salt % Math.max(values.length, 1)] ?? '1'; + } + if (definition.type === 'int' || definition.type === 'number') { + return validNumericValue(definition, salt); + } + const value = SAFE_VALUES[salt % SAFE_VALUES.length]!; + return value.length === 0 ? 'value' : value; +} + +/** Renders one flag as argv tokens; value flags use `--flag=value` so no token is consumed. */ +function renderFlag(definition: FlagDefinition, salt: number): string { + const token = flagToken(definition); + if (definition.type === 'boolean' || definition.setValue !== undefined) return token; + if (definition.type === 'booleanOrString') return token; + return `${token}=${validFlagValue(definition, salt)}`; +} + +type CliBase = { + surface: CliCommandSurface; + positionals: string[]; + flags: FlagDefinition[]; + salt: number; +}; + +function validArgv(base: CliBase): string[] { + return [ + base.surface.name, + ...base.positionals, + ...base.flags.map((definition, index) => renderFlag(definition, base.salt + index)), + ]; +} + +/** + * Which parser layer refuses the planted violation. `token-scan` violations are refused inside + * `parseRawArgs`/`parseFlagValue` while argv is still being scanned — the layer the classic + * `cli-args` target already reaches; `command-validation` violations survive the scan and are + * refused by `finalizeParsedArgs` (arity, per-command flag support, command identity), the layer + * B2 exists to reach. Asserted per class in validation-arbitraries.test.ts, so the lane's real + * reach stays disclosed rather than implied. + */ +type CliMutationLayer = 'token-scan' | 'command-validation'; + +type CliMutation = { + name: string; + layer: CliMutationLayer; + /** Relative share of the mutated budget; command-validation classes are weighted up. */ + weight: number; + /** The `AppError.code` this class must be refused with. */ + code: string; + apply: (base: CliBase) => Omit | null; +}; + +const isValueFlag = (definition: FlagDefinition) => + definition.setValue === undefined && + (definition.type === 'string' || + definition.type === 'enum' || + definition.type === 'int' || + definition.type === 'number'); + +const fakeCommands = memoize(() => + ['frobnicate', 'tapp', 'navigate', 'clik', 'snapshoot', 'opne'].filter( + (name) => !isKnownCliCommandName(name), + ), +); + +const CLI_MUTATIONS: readonly CliMutation[] = [ + { + // #1433: a bounded command must refuse extra positionals instead of swallowing them. + name: 'excess-positional', + layer: 'command-validation', + weight: 6, + code: 'INVALID_ARGS', + apply: (base) => { + if (base.surface.maxPositionals === null) return null; + const filler = SAFE_VALUES[base.salt % SAFE_VALUES.length] || 'extra'; + const positionals = [ + ...Array.from({ length: base.surface.maxPositionals }, () => 'p'), + ...Array.from({ length: 1 + (base.salt % 2) }, () => filler), + ]; + return { + payload: [base.surface.name, ...positionals], + mutation: 'excess-positional', + }; + }, + }, + { + name: 'unsupported-flag', + layer: 'command-validation', + weight: 6, + code: 'INVALID_ARGS', + apply: (base) => { + const foreign = safeFlagPool().filter( + (definition) => !isFlagSupportedForCommand(definition.key, base.surface.name), + ); + const definition = foreign[base.salt % Math.max(foreign.length, 1)]; + if (!definition) return null; + return { + payload: [...validArgv(base), renderFlag(definition, base.salt)], + mutation: 'unsupported-flag', + }; + }, + }, + { + name: 'bad-enum-value', + layer: 'token-scan', + weight: 1, + code: 'INVALID_ARGS', + apply: (base) => { + const enums = base.surface.flags.filter((definition) => definition.type === 'enum'); + const definition = enums[base.salt % Math.max(enums.length, 1)]; + if (!definition) return null; + return { + payload: [base.surface.name, `${flagToken(definition)}=bogus-${base.salt % 7}`], + mutation: 'bad-enum-value', + }; + }, + }, + { + name: 'int-out-of-range', + layer: 'token-scan', + weight: 1, + code: 'INVALID_ARGS', + apply: (base) => { + const bounded = base.surface.flags.filter( + (definition) => + (definition.type === 'int' || definition.type === 'number') && + (definition.min !== undefined || definition.max !== undefined), + ); + const definition = bounded[base.salt % Math.max(bounded.length, 1)]; + if (!definition) return null; + const value = + definition.min !== undefined ? String(definition.min - 1) : String(definition.max! + 1); + return { + payload: [base.surface.name, `${flagToken(definition)}=${value}`], + mutation: 'int-out-of-range', + }; + }, + }, + { + name: 'missing-flag-value', + layer: 'token-scan', + weight: 1, + code: 'INVALID_ARGS', + apply: (base) => { + const valued = base.surface.flags.filter(isValueFlag); + const definition = valued[base.salt % Math.max(valued.length, 1)]; + if (!definition) return null; + return { + payload: [base.surface.name, flagToken(definition)], + mutation: 'missing-flag-value', + }; + }, + }, + { + name: 'boolean-with-value', + layer: 'token-scan', + weight: 1, + code: 'INVALID_ARGS', + apply: (base) => { + const booleans = base.surface.flags.filter( + (definition) => definition.type === 'boolean' || definition.setValue !== undefined, + ); + const definition = booleans[base.salt % Math.max(booleans.length, 1)]; + if (!definition) return null; + return { + payload: [...validArgv(base), `${flagToken(definition)}=true`], + mutation: 'boolean-with-value', + }; + }, + }, + { + name: 'unknown-command', + layer: 'command-validation', + weight: 3, + code: 'INVALID_ARGS', + apply: (base) => { + const names = fakeCommands(); + const name = names[base.salt % names.length]!; + return { payload: [name, ...base.positionals], mutation: 'unknown-command' }; + }, + }, +]; + +const cliBaseArb: fc.Arbitrary = fc + .record({ + surfaceIndex: fc.nat(), + positionals: fc.array(fc.constantFrom(...SAFE_VALUES), { maxLength: 4 }), + flagIndices: fc.uniqueArray(fc.nat({ max: 200 }), { maxLength: 3 }), + salt: fc.nat({ max: 10_000 }), + }) + .map(({ surfaceIndex, positionals, flagIndices, salt }) => { + const surfaces = cliSurfaces(); + const surface = surfaces[surfaceIndex % surfaces.length]!; + const maxPositionals = surface.maxPositionals ?? 3; + const flags = [ + ...new Map( + flagIndices + .filter(() => surface.flags.length > 0) + .map((index) => surface.flags[index % surface.flags.length]!) + .map((definition) => [definition.key, definition]), + ).values(), + ]; + return { surface, positionals: positionals.slice(0, maxPositionals), flags, salt }; + }); + +/** Mutation indices expanded by weight, so command-validation classes take the larger share. */ +const weightedCliMutations = memoize(() => + CLI_MUTATIONS.flatMap((mutation, index) => Array.from({ length: mutation.weight }, () => index)), +); + +function validCase(base: CliBase): string { + return encodeValidationCase({ payload: validArgv(base), mutation: 'valid', expect: ACCEPT }); +} + +/** Encoded CLI validation cases: ~1/4 valid (expect accept), the rest planted violations. */ +export const cliValidationArb: fc.Arbitrary = fc + .record({ base: cliBaseArb, mutationIndex: fc.nat() }) + .map(({ base, mutationIndex }) => { + const weighted = weightedCliMutations(); + // A quarter of the space stays valid so a false rejection is discoverable too. + if (mutationIndex % (weighted.length + 6) >= weighted.length) return validCase(base); + // Rotate to the first applicable mutation so the map stays total. + for (let step = 0; step < weighted.length; step += 1) { + const mutation = CLI_MUTATIONS[weighted[(mutationIndex + step) % weighted.length]!]!; + const built = mutation.apply(base); + if (built) { + return encodeValidationCase({ + ...built, + expect: { outcome: 'reject', code: mutation.code }, + }); + } + } + return validCase(base); + }); diff --git a/scripts/fuzz/validation-arbitraries-maestro.test.ts b/scripts/fuzz/validation-arbitraries-maestro.test.ts new file mode 100644 index 0000000000..fb0c3e4361 --- /dev/null +++ b/scripts/fuzz/validation-arbitraries-maestro.test.ts @@ -0,0 +1,74 @@ +// Expectation gate for the Maestro validation generator (#1781 B2). +// +// The Maestro shapes are rendered by hand rather than derived from a registry, so this file is +// the only thing standing between a drifted shape and a nightly full of phantom findings: it +// replays fixed-seed samples against the real parser and requires every planted expectation to +// hold, every class to fire, and the violations to be refused in command-shape validation rather +// than by the YAML tokenizer. + +import fc from 'fast-check'; +import { describe, expect, it } from 'vitest'; +import { describeFailure } from './invariant.ts'; +import { getFuzzTarget } from './registry.ts'; +import { decodeValidationCase } from './validation-case.ts'; +import { maestroValidationArb } from './validation-arbitraries-maestro.ts'; + +const SAMPLE_SIZE = 3_000; +const SEED = 1; + +const target = getFuzzTarget('maestro-validation'); +const sample = fc.sample(maestroValidationArb, { numRuns: SAMPLE_SIZE, seed: SEED }); + +describe('maestro-validation generator', () => { + it('is deterministic for a seed, so a reported counterexample replays', () => { + const again = fc.sample(maestroValidationArb, { numRuns: 32, seed: 7 }); + expect(again).toEqual(fc.sample(maestroValidationArb, { numRuns: 32, seed: 7 })); + expect(again).not.toEqual(fc.sample(maestroValidationArb, { numRuns: 32, seed: 8 })); + }); + + it('produces decodable envelopes whose expectations hold on a healthy tree', () => { + const failures = []; + for (const input of sample) { + expect(decodeValidationCase(input)).not.toBeNull(); + const failure = target.check!(input); + if (failure) failures.push(describeFailure(failure)); + } + expect(failures).toEqual([]); + }); + + it('exercises every mutation class, including valid accept cases', () => { + const mutations = new Set(sample.map((input) => decodeValidationCase(input)!.mutation)); + expect(mutations).toContain('valid'); + expect([...mutations].sort()).toMatchSnapshot(); + }); +}); + +describe('planted Maestro violations are refused by command-shape validation', () => { + function rejectionMessageFor(mutation: string): string { + const input = sample.find((entry) => decodeValidationCase(entry)!.mutation === mutation); + expect(input, `no ${mutation} case in the fixed-seed sample`).toBeDefined(); + const decoded = decodeValidationCase(input!)!; + try { + target.run(input!); + } catch (error) { + return (error as Error).message; + } + throw new Error(`expected ${mutation} case to reject: ${JSON.stringify(decoded.payload)}`); + } + + it('unknown commands die in command-shape validation, not the YAML tokenizer', () => { + expect(rejectionMessageFor('unsupported-command')).toMatch( + /Maestro command ".+" is not supported/, + ); + }); + + it('unknown fields die in per-command field validation', () => { + expect(rejectionMessageFor('unsupported-field')).toMatch(/field ".+" is not supported/); + }); + + it('unknown config keys die in flow-config validation', () => { + expect(rejectionMessageFor('config-unknown-key')).toMatch( + /Maestro flow config field ".+" is not supported/, + ); + }); +}); diff --git a/scripts/fuzz/validation-arbitraries-maestro.ts b/scripts/fuzz/validation-arbitraries-maestro.ts new file mode 100644 index 0000000000..40a2eb8f29 --- /dev/null +++ b/scripts/fuzz/validation-arbitraries-maestro.ts @@ -0,0 +1,138 @@ +// Maestro flow case generation for the `maestro-validation` fuzz target (#1781 B2). +// +// Cases are built from the command shapes the converter accepts, then one shape rule is violated, +// so the failure surfaces in command-shape validation rather than in the YAML tokenizer. The +// shapes are rendered here rather than derived from a registry — Maestro's accepted surface is a +// parser table, not exported data — so the generator test replays samples against the real parser +// and a drifted shape fails at PR time instead of phantoming nightly. + +import fc from 'fast-check'; +import { encodeValidationCase } from './validation-case.ts'; +import { ACCEPT, SAFE_VALUES } from './validation-values.ts'; + +// --------------------------------------------------------------------------------------------- + +const yamlText = (salt: number): string => + JSON.stringify(SAFE_VALUES[salt % SAFE_VALUES.length] || 'Login'); + +/** One valid command rendered as YAML list-entry lines. */ +function validMaestroCommand(pick: number, salt: number): string[] { + const text = yamlText(salt); + const options: (() => string[])[] = [ + () => ['- back'], + () => ['- hideKeyboard'], + () => ['- stopApp'], + () => ['- scroll'], + () => ['- waitForAnimationToEnd'], + () => ['- launchApp'], + () => [`- launchApp: ${text}`], + () => [`- tapOn: ${text}`], + () => ['- tapOn:', ` id: ${text}`], + () => ['- tapOn:', ` text: ${text}`], + () => [`- doubleTapOn: ${text}`], + () => [`- longPressOn: ${text}`], + () => [`- inputText: ${text}`], + () => ['- openLink: "https://example.com/page"'], + () => [`- assertVisible: ${text}`], + () => [`- assertNotVisible: ${text}`], + () => [`- takeScreenshot: ${text}`], + () => ['- swipe:', ` direction: ${['UP', 'DOWN', 'LEFT', 'RIGHT'][salt % 4]}`], + () => [`- pressKey: ${['back', 'enter', 'return', 'home'][salt % 4]}`], + () => ['- extendedWaitUntil:', ` visible: ${text}`, ' timeout: 500'], + () => ['- scrollUntilVisible:', ' element:', ` text: ${text}`], + () => ['- repeat:', ' times: 2', ' commands:', ' - back'], + () => ['- runFlow: other.yaml'], + ]; + return options[pick % options.length]!(); +} + +const FAKE_MAESTRO_COMMANDS = [ + 'clickOn', + 'tapOnPoint', + 'assertTrue', + 'evalScript', + 'launchActivity', + 'inputTextt', +] as const; + +/** `code` is per class, like the CLI table: a class whose contract changes moves alone. */ +type MaestroMutation = { name: string; code: string; lines: (salt: number) => string[] }; + +const MAESTRO_MUTATIONS: readonly MaestroMutation[] = [ + { + // The B2 headline: an unknown command name must die in command validation, not the + // YAML tokenizer — mutated names used to be unreachable because the YAML never parsed. + name: 'unsupported-command', + code: 'INVALID_ARGS', + lines: (salt) => { + const name = FAKE_MAESTRO_COMMANDS[salt % FAKE_MAESTRO_COMMANDS.length]!; + return salt % 2 === 0 ? [`- ${name}`] : [`- ${name}: ${yamlText(salt)}`]; + }, + }, + { + name: 'unsupported-field', + code: 'INVALID_ARGS', + lines: (salt) => + salt % 2 === 0 + ? ['- tapOn:', ` bogusField: ${yamlText(salt)}`] + : ['- launchApp:', ` appId: ${yamlText(salt)}`, ' bogus: 1'], + }, + { + name: 'multi-key-command', + code: 'INVALID_ARGS', + lines: (salt) => [`- tapOn: ${yamlText(salt)}`, ` inputText: ${yamlText(salt + 1)}`], + }, + { + name: 'missing-required', + code: 'INVALID_ARGS', + lines: (salt) => + salt % 2 === 0 ? ['- inputText:'] : ['- extendedWaitUntil:', ' timeout: 500'], + }, + { name: 'bad-press-key', code: 'INVALID_ARGS', lines: () => ['- pressKey: sleep'] }, + { name: 'scroll-options', code: 'INVALID_ARGS', lines: () => ['- scroll:', ' direction: UP'] }, +]; + +type MaestroBase = { commandPicks: number[]; salt: number; withConfig: boolean }; + +function renderMaestroFlow(base: MaestroBase, mutatedLines?: string[], at?: number): string { + const commands = base.commandPicks.map((pick, index) => + validMaestroCommand(pick, base.salt + index), + ); + if (mutatedLines) commands.splice(Math.min(at ?? 0, commands.length), 0, mutatedLines); + const body = commands.flat().join('\n'); + const config = base.withConfig ? `appId: ${yamlText(base.salt)}\n---\n` : ''; + return `${config}${body}\n`; +} + +const maestroBaseArb: fc.Arbitrary = fc.record({ + commandPicks: fc.array(fc.nat({ max: 100 }), { minLength: 1, maxLength: 4 }), + salt: fc.nat({ max: 10_000 }), + withConfig: fc.boolean(), +}); + +/** Encoded Maestro validation cases: ~1/3 valid flows, the rest one planted shape violation. */ +export const maestroValidationArb: fc.Arbitrary = fc + .record({ base: maestroBaseArb, mutationIndex: fc.nat(), insertAt: fc.nat({ max: 4 }) }) + .map(({ base, mutationIndex, insertAt }) => { + if (mutationIndex % (MAESTRO_MUTATIONS.length + 3) >= MAESTRO_MUTATIONS.length) { + return encodeValidationCase({ + payload: renderMaestroFlow(base), + mutation: 'valid', + expect: ACCEPT, + }); + } + const mutation = MAESTRO_MUTATIONS[mutationIndex % MAESTRO_MUTATIONS.length]!; + // A config-level violation replaces the flow body mutation for a slice of the space. + if (mutation.name === 'unsupported-field' && base.salt % 3 === 0) { + return encodeValidationCase({ + payload: `appId: com.example.app\nbogusKey: 1\n---\n${renderMaestroFlow({ ...base, withConfig: false })}`, + mutation: 'config-unknown-key', + expect: { outcome: 'reject', code: 'INVALID_ARGS' }, + }); + } + return encodeValidationCase({ + payload: renderMaestroFlow(base, mutation.lines(base.salt), insertAt), + mutation: mutation.name, + expect: { outcome: 'reject', code: mutation.code }, + }); + }); diff --git a/scripts/fuzz/validation-arbitraries.test.ts b/scripts/fuzz/validation-arbitraries.test.ts index a722654269..5a17ac1b93 100644 --- a/scripts/fuzz/validation-arbitraries.test.ts +++ b/scripts/fuzz/validation-arbitraries.test.ts @@ -1,137 +1,18 @@ -// Generator-expectation gate for the validation fuzz targets (#1781 B2). -// -// A validation case asserts a specific outcome the generator planted, so a drifted generator is -// worse than a weak one: it would report phantom findings nightly (or mark real bugs expected). -// This suite replays fixed-seed samples against the real parsers in-process — every planted -// expectation must hold on a healthy tree, every mutation class must appear, and the planted -// violations must surface as validation-layer errors, past the tokenizer. +// The validation generator lookup (#1781 B2). -import fc from 'fast-check'; -import { describe, expect, it, vi } from 'vitest'; -import { checkCase, describeFailure } from './invariant.ts'; -import { getFuzzTarget } from './registry.ts'; -import { decodeValidationCase } from './validation-case.ts'; -import { cliValidationArb, maestroValidationArb } from './validation-arbitraries.ts'; +import { describe, expect, it } from 'vitest'; +import { validationArbitraryFor } from './validation-arbitraries.ts'; -// A mismatch that fires at ~1-in-1000 must not pass here and then phantom nightly: the nightly -// draws 38,000 cases per target, so this samples 7.9% of it rather than the 1.5% it began at. -const SAMPLE_SIZE = 3_000; -const SEED = 1; - -const TARGETS = [ - ['cli-validation', cliValidationArb], - ['maestro-validation', maestroValidationArb], -] as const; - -describe.each(TARGETS)('%s generator', (targetName, arbitrary) => { - const target = getFuzzTarget(targetName); - const sample = fc.sample(arbitrary, { numRuns: SAMPLE_SIZE, seed: SEED }); - - it('is deterministic for a seed, so a reported counterexample replays', () => { - const again = fc.sample(arbitrary, { numRuns: 32, seed: 7 }); - expect(again).toEqual(fc.sample(arbitrary, { numRuns: 32, seed: 7 })); - expect(again).not.toEqual(fc.sample(arbitrary, { numRuns: 32, seed: 8 })); - }); - - it('produces decodable envelopes whose expectations hold on a healthy tree', () => { - const failures = []; - for (const input of sample) { - expect(decodeValidationCase(input)).not.toBeNull(); - const failure = target.check!(input); - if (failure) failures.push(describeFailure(failure)); - } - expect(failures).toEqual([]); - }); - - it('exercises every mutation class, including valid accept cases', () => { - const mutations = new Set(sample.map((input) => decodeValidationCase(input)!.mutation)); - expect(mutations).toContain('valid'); - // Every rule the generator declares shows up in a nightly-scale slice of the space; a rule - // that stops firing (surface drift, weight bug) is dead coverage and fails here. - expect([...mutations].sort()).toMatchSnapshot(); - }); -}); - -describe('planted violations surface where the generator says they do', () => { - function rejectionMessageFor(targetName: (typeof TARGETS)[number][0], mutation: string): string { - const arbitrary = targetName === 'cli-validation' ? cliValidationArb : maestroValidationArb; - const sample = fc.sample(arbitrary, { numRuns: SAMPLE_SIZE, seed: SEED }); - const input = sample.find((entry) => decodeValidationCase(entry)!.mutation === mutation); - expect(input, `no ${mutation} case in the fixed-seed sample`).toBeDefined(); - const decoded = decodeValidationCase(input!)!; - const target = getFuzzTarget(targetName); - try { - target.run(input!); - } catch (error) { - return (error as Error).message; - } - throw new Error(`expected ${mutation} case to reject: ${JSON.stringify(decoded.payload)}`); - } - - // The layer each class actually reaches, asserted rather than implied. `command-validation` - // classes survive the argv scan and are refused by finalizeParsedArgs — the reach B2 adds. - // `token-scan` classes are refused inside parseFlagValue while argv is still being scanned: - // the layer the classic `cli-args` target already reaches. They stay for the error-code - // assertion cli-args cannot make, and are not claimed as new reach anywhere. - it.each([ - ['excess-positional', 'command-validation', /accepts at most \d+ positional argument/], - ['unsupported-flag', 'command-validation', /is not supported for command/], - ['unknown-command', 'command-validation', /^Unknown command:/], - ['bad-enum-value', 'token-scan', /^Invalid /], - ['int-out-of-range', 'token-scan', /^Invalid /], - ['missing-flag-value', 'token-scan', /requires a value\./], - ['boolean-with-value', 'token-scan', /does not take a value\./], - ])('CLI %s is refused in %s', (mutation, _layer, expected) => { - expect(rejectionMessageFor('cli-validation', mutation)).toMatch(expected); +describe('validationArbitraryFor', () => { + it('resolves a generator for each validation target', () => { + expect(validationArbitraryFor('cli-validation')).toBeDefined(); + expect(validationArbitraryFor('maestro-validation')).toBeDefined(); }); - it('spends most of the CLI budget on classes that survive the argv scan', () => { - const sample = fc.sample(cliValidationArb, { numRuns: SAMPLE_SIZE, seed: SEED }); - const tokenScan = new Set([ - 'bad-enum-value', - 'int-out-of-range', - 'missing-flag-value', - 'boolean-with-value', - ]); - const mutated = sample - .map((input) => decodeValidationCase(input)!.mutation) - .filter((mutation) => mutation !== 'valid'); - const scanned = mutated.filter((mutation) => tokenScan.has(mutation)).length; - // Weighted down rather than removed, so the ratio is the lane's disclosed reach: a weight - // edit that quietly hands the budget back to the token scan fails here. - expect(scanned / mutated.length).toBeLessThan(0.25); - }); - - it('Maestro unknown commands die in command-shape validation, not the YAML tokenizer', () => { - expect(rejectionMessageFor('maestro-validation', 'unsupported-command')).toMatch( - /Maestro command ".+" is not supported/, - ); - }); - - it('Maestro unknown fields die in per-command field validation', () => { - expect(rejectionMessageFor('maestro-validation', 'unsupported-field')).toMatch( - /field ".+" is not supported/, - ); - }); -}); - -describe('generator startup', () => { - // The eager version of this derivation charged every harness path the command-metadata - // registry and timed out the coverage-instrumented promotion test. Importing must stay free. - it('does not derive the CLI surface until a case is actually generated', async () => { - vi.resetModules(); - const fresh = await import('./validation-arbitraries.ts'); - expect(fresh.validationSurfaceBuildCount()).toBe(0); - fc.sample(fresh.cliValidationArb, { numRuns: 1, seed: SEED }); - expect(fresh.validationSurfaceBuildCount()).toBe(1); - }); -}); - -describe('validation envelope guard', () => { - it('reports a malformed envelope as a finding instead of crashing the worker', () => { - const target = getFuzzTarget('cli-validation'); - const failure = checkCase(target, 'not an envelope'); - expect(failure?.kind).toBe('untyped-throw'); - expect(failure?.detail).toContain('malformed validation case envelope'); + // `undefined` is what routes a classic target back to the hazard-splicing mutator, so this is + // the difference between a corrupted-string case and an expectation-carrying one. + it('returns undefined for the classic targets', () => { + expect(validationArbitraryFor('selector')).toBeUndefined(); + expect(validationArbitraryFor('cli-args')).toBeUndefined(); }); }); diff --git a/scripts/fuzz/validation-arbitraries.ts b/scripts/fuzz/validation-arbitraries.ts index b53e506b0b..81799b7a08 100644 --- a/scripts/fuzz/validation-arbitraries.ts +++ b/scripts/fuzz/validation-arbitraries.ts @@ -1,510 +1,13 @@ -// Structured case generators for the validation fuzz targets (#1781 B2). +// Target name to validation generator (#1781 B2). // -// The classic mutators splice hazards into flat strings, so a CLI or Maestro case almost always -// dies in the tokenizer ("Unknown command", YAML error) and the validation layer behind it goes -// unexercised. These generators build cases FROM the real command surface — the CLI schema -// registry and the Maestro command shapes — so a case tokenizes cleanly and its planted -// violation surfaces in command validation (positional arity, flag support, enum/range checks, -// unsupported Maestro commands and fields). Each case records the outcome it was built to have; -// validation-case.ts judges the parser against it, which is what makes a silent acceptance -// (the #1433 class) reportable at all. +// The one place the two domain generators meet: `generate.ts` asks for a target's cases by name +// and gets the classic mutator when the answer is `undefined`. Keeping the lookup here means +// neither domain module imports the other, and adding a validation target is one line. -import fc from 'fast-check'; -import { isKnownCliCommandName, listCliCommandNames } from '../../src/command-catalog.ts'; -import { - getCliCommandSchema, - getFlagDefinitions, - type FlagDefinition, -} from '../../src/cli-schema/command-schema.ts'; -import { isFlagSupportedForCommand } from '../../src/cli-schema/option-schema.ts'; +import type fc from 'fast-check'; import type { FuzzTargetName } from './target-types.ts'; -import { encodeValidationCase, type ValidationCase } from './validation-case.ts'; - -const ACCEPT = { outcome: 'accept' } as const; - -// Benign, occasionally hostile positional/flag values. No leading '-': a dash token would be -// read as a flag and the case would die before validation, which is the classic targets' job. -const SAFE_VALUES = [ - 'com.example.app', - 'text=Login', - '@e1', - 'hello world', - '123', - 'Ünïcøde', - '😀 emoji', - 'say "hi"', - 'a\\b', - '', -] as const; - -// --------------------------------------------------------------------------------------------- -// CLI: the surface is derived from the schema registry, never hand-listed. -// --------------------------------------------------------------------------------------------- - -type CliCommandSurface = { - name: string; - /** `null` when the schema allows extra positionals — the arity mutation does not apply. */ - maxPositionals: number | null; - /** Flags supported for this command, restricted to the unambiguous pool. */ - flags: readonly FlagDefinition[]; -}; - -// Commands whose parse path is deliberately special: cdp preserves post-command args verbatim, -// react-devtools passes unknown flags through, batch enforces a step-source invariant covered -// by its own fixed mutation below. -const EXCLUDED_CLI_COMMANDS = new Set(['cdp', 'react-devtools', 'batch']); - -// help/version reroute parsing, snapshotDiff rewrites the command, steps/stepsFile carry the -// batch step-source invariant. All are exercised elsewhere; here they would blur expectations. -const EXCLUDED_FLAG_KEYS = new Set(['help', 'version', 'snapshotDiff', 'steps', 'stepsFile']); - -function collidingFlagNames(definitions: readonly FlagDefinition[]): Set { - const counts = new Map(); - for (const definition of definitions) { - for (const name of definition.names) counts.set(name, (counts.get(name) ?? 0) + 1); - } - return new Set([...counts.entries()].filter(([, count]) => count > 1).map(([name]) => name)); -} - -/** - * Deriving the surface walks the whole command-metadata registry, so it is computed on first use - * rather than at import: a `--input-file` replay or a classic-target run must not pay for a - * generator it never asks for (it did, and timed out the coverage-instrumented promotion test). - */ -function memoize(build: () => T): () => T { - let value: T | undefined; - return () => (value ??= build()); -} - -let surfaceBuilds = 0; - -/** - * How many times the schema-derived surface has been built. It must still be 0 after importing - * this module: an eager build cost every harness path — `--input-file` and the classic targets - * included — the whole command-metadata registry, and timed out the coverage-instrumented - * promotion test. Asserted in validation-arbitraries.test.ts, which is the actual #1824 guard. - */ -export function validationSurfaceBuildCount(): number { - return surfaceBuilds; -} - -const collidingNames = memoize(() => collidingFlagNames(getFlagDefinitions())); - -/** Flag definitions with a single unambiguous long name, excluding the special keys. */ -const safeFlagPool = memoize(() => - getFlagDefinitions().filter( - (definition) => - !EXCLUDED_FLAG_KEYS.has(definition.key) && - definition.names.some((name) => name.startsWith('--') && !collidingNames().has(name)), - ), -); - -function flagToken(definition: FlagDefinition): string { - return definition.names.find((name) => name.startsWith('--') && !collidingNames().has(name))!; -} - -const cliSurfaces = memoize(() => { - surfaceBuilds += 1; - return listCliCommandNames() - .filter((name) => !EXCLUDED_CLI_COMMANDS.has(name)) - .map((name) => { - const schema = getCliCommandSchema(name); - return { - name, - maxPositionals: schema.allowsExtraPositionals ? null : (schema.positionalArgs?.length ?? 0), - flags: safeFlagPool().filter((definition) => - isFlagSupportedForCommand(definition.key, name), - ), - }; - }); -}); - -/** Inclusive bounds for a numeric flag, defaulted when the schema leaves an end open. */ -function numericBounds(definition: FlagDefinition): { low: number; high: number } { - const low = definition.min ?? 0; - return { low, high: definition.max ?? low + 1000 }; -} - -/** - * A schema-valid numeric value. Floats take endpoints and the midpoint only: modulo arithmetic - * can drift past a fractional `max` (a 33k-case slice produced `--scale=1.110000000000017`, - * which the parser rightly rejected — a phantom finding, not a bug). - */ -function validNumericValue(definition: FlagDefinition, salt: number): string { - const { low, high } = numericBounds(definition); - if (definition.type === 'int') return String(low + (salt % (high - low + 1))); - return String([low, high, (low + high) / 2][salt % 3]); -} - -/** A schema-valid value for one flag, salt-selected so shrinking stays deterministic. */ -function validFlagValue(definition: FlagDefinition, salt: number): string { - if (definition.type === 'enum') { - const values = definition.enumValues ?? []; - return values[salt % Math.max(values.length, 1)] ?? '1'; - } - if (definition.type === 'int' || definition.type === 'number') { - return validNumericValue(definition, salt); - } - const value = SAFE_VALUES[salt % SAFE_VALUES.length]!; - return value.length === 0 ? 'value' : value; -} - -/** Renders one flag as argv tokens; value flags use `--flag=value` so no token is consumed. */ -function renderFlag(definition: FlagDefinition, salt: number): string { - const token = flagToken(definition); - if (definition.type === 'boolean' || definition.setValue !== undefined) return token; - if (definition.type === 'booleanOrString') return token; - return `${token}=${validFlagValue(definition, salt)}`; -} - -type CliBase = { - surface: CliCommandSurface; - positionals: string[]; - flags: FlagDefinition[]; - salt: number; -}; - -function validArgv(base: CliBase): string[] { - return [ - base.surface.name, - ...base.positionals, - ...base.flags.map((definition, index) => renderFlag(definition, base.salt + index)), - ]; -} - -/** - * Which parser layer refuses the planted violation. `token-scan` violations are refused inside - * `parseRawArgs`/`parseFlagValue` while argv is still being scanned — the layer the classic - * `cli-args` target already reaches; `command-validation` violations survive the scan and are - * refused by `finalizeParsedArgs` (arity, per-command flag support, command identity), the layer - * B2 exists to reach. Asserted per class in validation-arbitraries.test.ts, so the lane's real - * reach stays disclosed rather than implied. - */ -type CliMutationLayer = 'token-scan' | 'command-validation'; - -type CliMutation = { - name: string; - layer: CliMutationLayer; - /** Relative share of the mutated budget; command-validation classes are weighted up. */ - weight: number; - /** The `AppError.code` this class must be refused with. */ - code: string; - apply: (base: CliBase) => Omit | null; -}; - -const isValueFlag = (definition: FlagDefinition) => - definition.setValue === undefined && - (definition.type === 'string' || - definition.type === 'enum' || - definition.type === 'int' || - definition.type === 'number'); - -const fakeCommands = memoize(() => - ['frobnicate', 'tapp', 'navigate', 'clik', 'snapshoot', 'opne'].filter( - (name) => !isKnownCliCommandName(name), - ), -); - -const CLI_MUTATIONS: readonly CliMutation[] = [ - { - // #1433: a bounded command must refuse extra positionals instead of swallowing them. - name: 'excess-positional', - layer: 'command-validation', - weight: 6, - code: 'INVALID_ARGS', - apply: (base) => { - if (base.surface.maxPositionals === null) return null; - const filler = SAFE_VALUES[base.salt % SAFE_VALUES.length] || 'extra'; - const positionals = [ - ...Array.from({ length: base.surface.maxPositionals }, () => 'p'), - ...Array.from({ length: 1 + (base.salt % 2) }, () => filler), - ]; - return { - payload: [base.surface.name, ...positionals], - mutation: 'excess-positional', - }; - }, - }, - { - name: 'unsupported-flag', - layer: 'command-validation', - weight: 6, - code: 'INVALID_ARGS', - apply: (base) => { - const foreign = safeFlagPool().filter( - (definition) => !isFlagSupportedForCommand(definition.key, base.surface.name), - ); - const definition = foreign[base.salt % Math.max(foreign.length, 1)]; - if (!definition) return null; - return { - payload: [...validArgv(base), renderFlag(definition, base.salt)], - mutation: 'unsupported-flag', - }; - }, - }, - { - name: 'bad-enum-value', - layer: 'token-scan', - weight: 1, - code: 'INVALID_ARGS', - apply: (base) => { - const enums = base.surface.flags.filter((definition) => definition.type === 'enum'); - const definition = enums[base.salt % Math.max(enums.length, 1)]; - if (!definition) return null; - return { - payload: [base.surface.name, `${flagToken(definition)}=bogus-${base.salt % 7}`], - mutation: 'bad-enum-value', - }; - }, - }, - { - name: 'int-out-of-range', - layer: 'token-scan', - weight: 1, - code: 'INVALID_ARGS', - apply: (base) => { - const bounded = base.surface.flags.filter( - (definition) => - (definition.type === 'int' || definition.type === 'number') && - (definition.min !== undefined || definition.max !== undefined), - ); - const definition = bounded[base.salt % Math.max(bounded.length, 1)]; - if (!definition) return null; - const value = - definition.min !== undefined ? String(definition.min - 1) : String(definition.max! + 1); - return { - payload: [base.surface.name, `${flagToken(definition)}=${value}`], - mutation: 'int-out-of-range', - }; - }, - }, - { - name: 'missing-flag-value', - layer: 'token-scan', - weight: 1, - code: 'INVALID_ARGS', - apply: (base) => { - const valued = base.surface.flags.filter(isValueFlag); - const definition = valued[base.salt % Math.max(valued.length, 1)]; - if (!definition) return null; - return { - payload: [base.surface.name, flagToken(definition)], - mutation: 'missing-flag-value', - }; - }, - }, - { - name: 'boolean-with-value', - layer: 'token-scan', - weight: 1, - code: 'INVALID_ARGS', - apply: (base) => { - const booleans = base.surface.flags.filter( - (definition) => definition.type === 'boolean' || definition.setValue !== undefined, - ); - const definition = booleans[base.salt % Math.max(booleans.length, 1)]; - if (!definition) return null; - return { - payload: [...validArgv(base), `${flagToken(definition)}=true`], - mutation: 'boolean-with-value', - }; - }, - }, - { - name: 'unknown-command', - layer: 'command-validation', - weight: 3, - code: 'INVALID_ARGS', - apply: (base) => { - const names = fakeCommands(); - const name = names[base.salt % names.length]!; - return { payload: [name, ...base.positionals], mutation: 'unknown-command' }; - }, - }, -]; - -const cliBaseArb: fc.Arbitrary = fc - .record({ - surfaceIndex: fc.nat(), - positionals: fc.array(fc.constantFrom(...SAFE_VALUES), { maxLength: 4 }), - flagIndices: fc.uniqueArray(fc.nat({ max: 200 }), { maxLength: 3 }), - salt: fc.nat({ max: 10_000 }), - }) - .map(({ surfaceIndex, positionals, flagIndices, salt }) => { - const surfaces = cliSurfaces(); - const surface = surfaces[surfaceIndex % surfaces.length]!; - const maxPositionals = surface.maxPositionals ?? 3; - const flags = [ - ...new Map( - flagIndices - .filter(() => surface.flags.length > 0) - .map((index) => surface.flags[index % surface.flags.length]!) - .map((definition) => [definition.key, definition]), - ).values(), - ]; - return { surface, positionals: positionals.slice(0, maxPositionals), flags, salt }; - }); - -/** Mutation indices expanded by weight, so command-validation classes take the larger share. */ -const weightedCliMutations = memoize(() => - CLI_MUTATIONS.flatMap((mutation, index) => Array.from({ length: mutation.weight }, () => index)), -); - -function validCase(base: CliBase): string { - return encodeValidationCase({ payload: validArgv(base), mutation: 'valid', expect: ACCEPT }); -} - -/** Encoded CLI validation cases: ~1/4 valid (expect accept), the rest planted violations. */ -export const cliValidationArb: fc.Arbitrary = fc - .record({ base: cliBaseArb, mutationIndex: fc.nat() }) - .map(({ base, mutationIndex }) => { - const weighted = weightedCliMutations(); - // A quarter of the space stays valid so a false rejection is discoverable too. - if (mutationIndex % (weighted.length + 6) >= weighted.length) return validCase(base); - // Rotate to the first applicable mutation so the map stays total. - for (let step = 0; step < weighted.length; step += 1) { - const mutation = CLI_MUTATIONS[weighted[(mutationIndex + step) % weighted.length]!]!; - const built = mutation.apply(base); - if (built) { - return encodeValidationCase({ - ...built, - expect: { outcome: 'reject', code: mutation.code }, - }); - } - } - return validCase(base); - }); - -// --------------------------------------------------------------------------------------------- -// Maestro: cases are built from the command shapes the converter accepts, then one shape rule -// is violated. The soundness test replays samples against the real parser, so a drifted shape -// fails at PR time rather than as a phantom nightly finding. -// --------------------------------------------------------------------------------------------- - -const yamlText = (salt: number): string => - JSON.stringify(SAFE_VALUES[salt % SAFE_VALUES.length] || 'Login'); - -/** One valid command rendered as YAML list-entry lines. */ -function validMaestroCommand(pick: number, salt: number): string[] { - const text = yamlText(salt); - const options: (() => string[])[] = [ - () => ['- back'], - () => ['- hideKeyboard'], - () => ['- stopApp'], - () => ['- scroll'], - () => ['- waitForAnimationToEnd'], - () => ['- launchApp'], - () => [`- launchApp: ${text}`], - () => [`- tapOn: ${text}`], - () => ['- tapOn:', ` id: ${text}`], - () => ['- tapOn:', ` text: ${text}`], - () => [`- doubleTapOn: ${text}`], - () => [`- longPressOn: ${text}`], - () => [`- inputText: ${text}`], - () => ['- openLink: "https://example.com/page"'], - () => [`- assertVisible: ${text}`], - () => [`- assertNotVisible: ${text}`], - () => [`- takeScreenshot: ${text}`], - () => ['- swipe:', ` direction: ${['UP', 'DOWN', 'LEFT', 'RIGHT'][salt % 4]}`], - () => [`- pressKey: ${['back', 'enter', 'return', 'home'][salt % 4]}`], - () => ['- extendedWaitUntil:', ` visible: ${text}`, ' timeout: 500'], - () => ['- scrollUntilVisible:', ' element:', ` text: ${text}`], - () => ['- repeat:', ' times: 2', ' commands:', ' - back'], - () => ['- runFlow: other.yaml'], - ]; - return options[pick % options.length]!(); -} - -const FAKE_MAESTRO_COMMANDS = [ - 'clickOn', - 'tapOnPoint', - 'assertTrue', - 'evalScript', - 'launchActivity', - 'inputTextt', -] as const; - -/** `code` is per class, like the CLI table: a class whose contract changes moves alone. */ -type MaestroMutation = { name: string; code: string; lines: (salt: number) => string[] }; - -const MAESTRO_MUTATIONS: readonly MaestroMutation[] = [ - { - // The B2 headline: an unknown command name must die in command validation, not the - // YAML tokenizer — mutated names used to be unreachable because the YAML never parsed. - name: 'unsupported-command', - code: 'INVALID_ARGS', - lines: (salt) => { - const name = FAKE_MAESTRO_COMMANDS[salt % FAKE_MAESTRO_COMMANDS.length]!; - return salt % 2 === 0 ? [`- ${name}`] : [`- ${name}: ${yamlText(salt)}`]; - }, - }, - { - name: 'unsupported-field', - code: 'INVALID_ARGS', - lines: (salt) => - salt % 2 === 0 - ? ['- tapOn:', ` bogusField: ${yamlText(salt)}`] - : ['- launchApp:', ` appId: ${yamlText(salt)}`, ' bogus: 1'], - }, - { - name: 'multi-key-command', - code: 'INVALID_ARGS', - lines: (salt) => [`- tapOn: ${yamlText(salt)}`, ` inputText: ${yamlText(salt + 1)}`], - }, - { - name: 'missing-required', - code: 'INVALID_ARGS', - lines: (salt) => - salt % 2 === 0 ? ['- inputText:'] : ['- extendedWaitUntil:', ' timeout: 500'], - }, - { name: 'bad-press-key', code: 'INVALID_ARGS', lines: () => ['- pressKey: sleep'] }, - { name: 'scroll-options', code: 'INVALID_ARGS', lines: () => ['- scroll:', ' direction: UP'] }, -]; - -type MaestroBase = { commandPicks: number[]; salt: number; withConfig: boolean }; - -function renderMaestroFlow(base: MaestroBase, mutatedLines?: string[], at?: number): string { - const commands = base.commandPicks.map((pick, index) => - validMaestroCommand(pick, base.salt + index), - ); - if (mutatedLines) commands.splice(Math.min(at ?? 0, commands.length), 0, mutatedLines); - const body = commands.flat().join('\n'); - const config = base.withConfig ? `appId: ${yamlText(base.salt)}\n---\n` : ''; - return `${config}${body}\n`; -} - -const maestroBaseArb: fc.Arbitrary = fc.record({ - commandPicks: fc.array(fc.nat({ max: 100 }), { minLength: 1, maxLength: 4 }), - salt: fc.nat({ max: 10_000 }), - withConfig: fc.boolean(), -}); - -/** Encoded Maestro validation cases: ~1/3 valid flows, the rest one planted shape violation. */ -export const maestroValidationArb: fc.Arbitrary = fc - .record({ base: maestroBaseArb, mutationIndex: fc.nat(), insertAt: fc.nat({ max: 4 }) }) - .map(({ base, mutationIndex, insertAt }) => { - if (mutationIndex % (MAESTRO_MUTATIONS.length + 3) >= MAESTRO_MUTATIONS.length) { - return encodeValidationCase({ - payload: renderMaestroFlow(base), - mutation: 'valid', - expect: ACCEPT, - }); - } - const mutation = MAESTRO_MUTATIONS[mutationIndex % MAESTRO_MUTATIONS.length]!; - // A config-level violation replaces the flow body mutation for a slice of the space. - if (mutation.name === 'unsupported-field' && base.salt % 3 === 0) { - return encodeValidationCase({ - payload: `appId: com.example.app\nbogusKey: 1\n---\n${renderMaestroFlow({ ...base, withConfig: false })}`, - mutation: 'config-unknown-key', - expect: { outcome: 'reject', code: 'INVALID_ARGS' }, - }); - } - return encodeValidationCase({ - payload: renderMaestroFlow(base, mutation.lines(base.salt), insertAt), - mutation: mutation.name, - expect: { outcome: 'reject', code: mutation.code }, - }); - }); +import { cliValidationArb } from './validation-arbitraries-cli.ts'; +import { maestroValidationArb } from './validation-arbitraries-maestro.ts'; /** The generator for a validation target, or `undefined` for the classic targets. */ export function validationArbitraryFor(target: FuzzTargetName): fc.Arbitrary | undefined { diff --git a/scripts/fuzz/validation-case.test.ts b/scripts/fuzz/validation-case.test.ts new file mode 100644 index 0000000000..75baa05098 --- /dev/null +++ b/scripts/fuzz/validation-case.test.ts @@ -0,0 +1,39 @@ +// The validation case envelope and its judge (#1781 B2). +// +// The envelope is what carries a planted expectation from the generator to the judge, through the +// corpus, the artifact, and the worker boundary. A malformed one is a harness or corpus defect +// rather than a parser bug, but it must still surface red instead of crashing a nightly. + +import { describe, expect, it } from 'vitest'; +import { checkCase } from './invariant.ts'; +import { getFuzzTarget } from './registry.ts'; +import { decodeValidationCase, encodeValidationCase } from './validation-case.ts'; + +describe('validation case envelope', () => { + it('round-trips a case through the string form the corpus and artifacts carry', () => { + const original = { + payload: ['open', 'com.example.app'], + mutation: 'valid', + expect: { outcome: 'accept' }, + } as const; + expect(decodeValidationCase(encodeValidationCase(original))).toEqual(original); + }); + + it.each([ + ['not json at all', 'plain text'], + ['valid json that is not an envelope', '{"nope":1}'], + ['an envelope without an expectation', '{"payload":["open"],"mutation":"x"}'], + [ + 'a reject expectation without a code', + '{"payload":[],"mutation":"x","expect":{"outcome":"reject"}}', + ], + ])('rejects %s', (_label, input) => { + expect(decodeValidationCase(input)).toBeNull(); + }); + + it('reports a malformed envelope as a finding instead of crashing the worker', () => { + const failure = checkCase(getFuzzTarget('cli-validation'), 'not an envelope'); + expect(failure?.kind).toBe('untyped-throw'); + expect(failure?.detail).toContain('malformed validation case envelope'); + }); +}); diff --git a/scripts/fuzz/validation-values.ts b/scripts/fuzz/validation-values.ts new file mode 100644 index 0000000000..bf869eaa9b --- /dev/null +++ b/scripts/fuzz/validation-values.ts @@ -0,0 +1,21 @@ +// Payload value vocabulary shared by the validation generators (#1781 B2). +// +// Benign, occasionally hostile values used for CLI positionals/flag values and Maestro scalars. +// No leading '-': a dash token would be read as a flag and the case would die in the token scan, +// which is the classic targets' job — these generators exist to reach past it. + +export const SAFE_VALUES = [ + 'com.example.app', + 'text=Login', + '@e1', + 'hello world', + '123', + 'Ünïcøde', + '😀 emoji', + 'say "hi"', + 'a\\b', + '', +] as const; + +/** The accept expectation, shared so a valid case reads the same in both generators. */ +export const ACCEPT = { outcome: 'accept' } as const; diff --git a/vitest.config.ts b/vitest.config.ts index e22a39457d..a4637ea674 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -61,9 +61,12 @@ export default defineConfig({ include: [ 'src/**/*.test.ts', 'packages/*/src/**/*.test.ts', - // The validation fuzz generators' expectation gate (#1781 B2): in-process, no - // subprocess or worker, so it rides the fast lane unlike its serialized siblings. + // The validation fuzz generators' expectation gates (#1781 B2): in-process, no + // subprocess or worker, so they ride the fast lane unlike their serialized siblings. 'scripts/fuzz/validation-arbitraries.test.ts', + 'scripts/fuzz/validation-arbitraries-cli.test.ts', + 'scripts/fuzz/validation-arbitraries-maestro.test.ts', + 'scripts/fuzz/validation-case.test.ts', 'scripts/__tests__/help-conformance-bench.test.ts', 'scripts/__tests__/help-conformance-error-recovery-coverage.test.ts', 'scripts/__tests__/help-conformance-sample-outputs.test.ts', From 2ec67b0780248a8addade7aed18d6b8b0055f59a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 18:39:07 +0200 Subject: [PATCH 09/11] refactor(fuzz): collapse the flag-shaped mutation classes and seed literals, derive class coverage from declarations --- docs/agents/testing.md | 32 ++-- .../validation-arbitraries-cli.test.ts.snap | 14 -- ...alidation-arbitraries-maestro.test.ts.snap | 14 -- scripts/fuzz/invariant.ts | 2 +- scripts/fuzz/targets.ts | 68 ++------- .../fuzz/validation-arbitraries-cli.test.ts | 32 +--- scripts/fuzz/validation-arbitraries-cli.ts | 143 +++++++++--------- .../validation-arbitraries-maestro.test.ts | 59 +++++--- .../fuzz/validation-arbitraries-maestro.ts | 15 +- scripts/fuzz/validation-case.ts | 23 ++- scripts/fuzz/validation-generator-contract.ts | 51 +++++++ 11 files changed, 221 insertions(+), 232 deletions(-) delete mode 100644 scripts/fuzz/__snapshots__/validation-arbitraries-cli.test.ts.snap delete mode 100644 scripts/fuzz/__snapshots__/validation-arbitraries-maestro.test.ts.snap create mode 100644 scripts/fuzz/validation-generator-contract.ts diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 029bf00a01..e19463637a 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -382,25 +382,19 @@ Two **validation targets** (#1781 B2) — `cli-validation` and `maestro-validati their cases are built 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 then also fails a **silent acceptance** of an input -built to be invalid (the #1433 excess-positionals class, which a rejection-only invariant cannot -see at all) and a rejection with the **wrong `AppError.code`** — not just "some error". - -Each CLI mutation class declares the parser layer it is refused by. `command-validation` classes -(excess positionals, unsupported-for-command flags, unknown commands) survive the argv scan and -reach `finalizeParsedArgs` — the reach these targets exist to add; `token-scan` classes (bad enum -values, out-of-range ints, missing flag values, valued booleans) are refused inside `parseFlagValue` -while argv is still being scanned, which is where the classic `cli-args` target already reaches. -They are weighted down to under a quarter of the mutated budget and kept only for the error-code -assertion `cli-args` cannot make. Rules whose whole input space is a few strings (`batch`'s -step-source rule, the `--in-app`/`--system` conflict) are **pinned seed cases** rather than -generated classes, so the nightly does not re-execute a handful of literals thousands of times. - -`scripts/fuzz/validation-arbitraries.test.ts` (unit-core, in-process) gates all of this at PR time: -fixed-seed samples must hold every planted expectation, every mutation class must still fire, each -class must be refused in its declared layer, the token-scan share must stay under 25%, and the -schema surface must not be derived until a case is generated (an eager derivation timed out the -coverage-instrumented promotion test). A drifted generator fails in seconds instead of producing -phantom nightly findings. +built to be invalid (the #1433 class, which a rejection-only invariant cannot see at all) and a +rejection carrying the **wrong `AppError.code`**. + +Each CLI mutation class declares the parser layer that refuses it — `command-validation` (past the +argv scan, the reach these targets add) or `token-scan` (inside `parseFlagValue`, where the classic +`cli-args` target already reaches, capped under a quarter of the mutated budget). Rules whose whole +input space is a few strings are pinned seed cases rather than generated classes. + +The declarations above are executable: `scripts/fuzz/validation-arbitraries-{cli,maestro}.test.ts` +replay fixed-seed samples against the real parsers and fail on a drifted generator, an unfired +class, a class refused in a layer it does not claim, a token-scan share over 25%, or a schema +surface derived at import (#1824). Read those files for the current guarantees rather than a list +here, which would only age. Cases come from fast-check arbitraries (`scripts/fuzz/arbitraries.ts`, validation envelopes in `scripts/fuzz/validation-arbitraries.ts`) built on the hazard vocabulary diff --git a/scripts/fuzz/__snapshots__/validation-arbitraries-cli.test.ts.snap b/scripts/fuzz/__snapshots__/validation-arbitraries-cli.test.ts.snap deleted file mode 100644 index 0836946777..0000000000 --- a/scripts/fuzz/__snapshots__/validation-arbitraries-cli.test.ts.snap +++ /dev/null @@ -1,14 +0,0 @@ -// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html - -exports[`cli-validation generator > exercises every mutation class, including valid accept cases 1`] = ` -[ - "bad-enum-value", - "boolean-with-value", - "excess-positional", - "int-out-of-range", - "missing-flag-value", - "unknown-command", - "unsupported-flag", - "valid", -] -`; diff --git a/scripts/fuzz/__snapshots__/validation-arbitraries-maestro.test.ts.snap b/scripts/fuzz/__snapshots__/validation-arbitraries-maestro.test.ts.snap deleted file mode 100644 index ecbaef8f44..0000000000 --- a/scripts/fuzz/__snapshots__/validation-arbitraries-maestro.test.ts.snap +++ /dev/null @@ -1,14 +0,0 @@ -// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html - -exports[`maestro-validation generator > exercises every mutation class, including valid accept cases 1`] = ` -[ - "bad-press-key", - "config-unknown-key", - "missing-required", - "multi-key-command", - "scroll-options", - "unsupported-command", - "unsupported-field", - "valid", -] -`; diff --git a/scripts/fuzz/invariant.ts b/scripts/fuzz/invariant.ts index a54e7cb4b7..0514ead4db 100644 --- a/scripts/fuzz/invariant.ts +++ b/scripts/fuzz/invariant.ts @@ -51,7 +51,7 @@ export function checkCase(target: FuzzTarget, input: string): FuzzFailure | null } } -function describeThrown(error: unknown): string { +export function describeThrown(error: unknown): string { if (error instanceof Error) { const stackLine = error.stack?.split('\n')[1]?.trim(); return `${error.name}: ${error.message}${stackLine ? ` (at ${stackLine})` : ''}`; diff --git a/scripts/fuzz/targets.ts b/scripts/fuzz/targets.ts index 65b9a1cb64..d166b810e3 100644 --- a/scripts/fuzz/targets.ts +++ b/scripts/fuzz/targets.ts @@ -13,7 +13,7 @@ import { parseReplayScriptDetailed } from '@agent-device/ad-script'; import { readCliBatchStepsJson } from '../../src/cli/batch-steps.ts'; import { inspectMaestroFlow } from '@agent-device/maestro'; import type { FuzzTarget } from './target-types.ts'; -import { encodeValidationCase, makeValidationCheck } from './validation-case.ts'; +import { acceptCase, makeValidationCheck, rejectCase } from './validation-case.ts'; // argv is carried as one string so a case (and its corpus entry, artifact, and repro // command) stays a single copy-pasteable value. Splitting on spaces is deliberate: the @@ -104,51 +104,19 @@ export const FUZZ_TARGETS: readonly FuzzTarget[] = [ runCliValidationPayload(payload as string[]), ), seeds: [ - encodeValidationCase({ - payload: ['open', 'com.example.app'], - mutation: 'valid', - expect: { outcome: 'accept' }, - }), - encodeValidationCase({ - payload: ['click', 'text=Login', '--json'], - mutation: 'valid', - expect: { outcome: 'accept' }, - }), - encodeValidationCase({ - payload: ['devices', '--platform=bogus'], - mutation: 'bad-enum-value', - expect: { outcome: 'reject', code: 'INVALID_ARGS' }, - }), - encodeValidationCase({ - payload: ['snapshot', '--depth'], - mutation: 'missing-flag-value', - expect: { outcome: 'reject', code: 'INVALID_ARGS' }, - }), + acceptCase(['open', 'com.example.app']), + acceptCase(['click', 'text=Login', '--json']), + rejectCase(['devices', '--platform=bogus'], 'bad-enum-value'), + rejectCase(['snapshot', '--depth'], 'missing-flag-value'), // Two command-validation rules whose entire input space is a handful of strings: `batch` is // the only command with a step-source rule, and `backMode` the only flag key reachable // through two tokens. Generating them re-executed ~15 payloads thousands of times a night // for no added reach, so they are pinned here and run verbatim once per run, before any // generated case. They are seed regressions, not generated reach — #1781's table says so. - encodeValidationCase({ - payload: ['batch'], - mutation: 'batch-step-source-none', - expect: { outcome: 'reject', code: 'INVALID_ARGS' }, - }), - encodeValidationCase({ - payload: ['batch', '--steps=[]', '--steps-file=steps.json'], - mutation: 'batch-step-source-both', - expect: { outcome: 'reject', code: 'INVALID_ARGS' }, - }), - encodeValidationCase({ - payload: ['back', '--in-app', '--system'], - mutation: 'conflicting-flag-tokens', - expect: { outcome: 'reject', code: 'INVALID_ARGS' }, - }), - encodeValidationCase({ - payload: ['back', '--system', '--in-app'], - mutation: 'conflicting-flag-tokens', - expect: { outcome: 'reject', code: 'INVALID_ARGS' }, - }), + rejectCase(['batch'], 'batch-step-source-none'), + rejectCase(['batch', '--steps=[]', '--steps-file=steps.json'], 'batch-step-source-both'), + rejectCase(['back', '--in-app', '--system'], 'conflicting-flag-tokens'), + rejectCase(['back', '--system', '--in-app'], 'conflicting-flag-tokens'), ], }, { @@ -162,21 +130,9 @@ export const FUZZ_TARGETS: readonly FuzzTarget[] = [ (payload) => void inspectMaestroFlow(payload as string, 'fuzz.yaml'), ), seeds: [ - encodeValidationCase({ - payload: 'appId: com.example.app\n---\n- launchApp\n- tapOn: "Login"\n', - mutation: 'valid', - expect: { outcome: 'accept' }, - }), - encodeValidationCase({ - payload: 'appId: com.example.app\n---\n- clickOn: "Login"\n', - mutation: 'unsupported-command', - expect: { outcome: 'reject', code: 'INVALID_ARGS' }, - }), - encodeValidationCase({ - payload: 'appId: com.example.app\n---\n- tapOn:\n bogusField: "x"\n', - mutation: 'unsupported-field', - expect: { outcome: 'reject', code: 'INVALID_ARGS' }, - }), + acceptCase('appId: com.example.app\n---\n- launchApp\n- tapOn: "Login"\n'), + rejectCase('appId: com.example.app\n---\n- clickOn: "Login"\n', 'unsupported-command'), + rejectCase('appId: com.example.app\n---\n- tapOn:\n bogusField: "x"\n', 'unsupported-field'), ], }, { diff --git a/scripts/fuzz/validation-arbitraries-cli.test.ts b/scripts/fuzz/validation-arbitraries-cli.test.ts index 282b6a2ee2..c4e612a7e2 100644 --- a/scripts/fuzz/validation-arbitraries-cli.test.ts +++ b/scripts/fuzz/validation-arbitraries-cli.test.ts @@ -8,10 +8,10 @@ import fc from 'fast-check'; import { describe, expect, it, vi } from 'vitest'; -import { describeFailure } from './invariant.ts'; import { getFuzzTarget } from './registry.ts'; import { decodeValidationCase } from './validation-case.ts'; -import { cliValidationArb } from './validation-arbitraries-cli.ts'; +import { describeGeneratorContract } from './validation-generator-contract.ts'; +import { CLI_MUTATION_NAMES, cliValidationArb } from './validation-arbitraries-cli.ts'; // A mismatch that fires at ~1-in-1000 must not pass here and then phantom nightly: the nightly // draws 38,000 cases per target, so this samples 7.9% of it rather than the 1.5% it began at. @@ -21,29 +21,11 @@ const SEED = 1; const target = getFuzzTarget('cli-validation'); const sample = fc.sample(cliValidationArb, { numRuns: SAMPLE_SIZE, seed: SEED }); -describe('cli-validation generator', () => { - it('is deterministic for a seed, so a reported counterexample replays', () => { - const again = fc.sample(cliValidationArb, { numRuns: 32, seed: 7 }); - expect(again).toEqual(fc.sample(cliValidationArb, { numRuns: 32, seed: 7 })); - expect(again).not.toEqual(fc.sample(cliValidationArb, { numRuns: 32, seed: 8 })); - }); - - it('produces decodable envelopes whose expectations hold on a healthy tree', () => { - const failures = []; - for (const input of sample) { - expect(decodeValidationCase(input)).not.toBeNull(); - const failure = target.check!(input); - if (failure) failures.push(describeFailure(failure)); - } - expect(failures).toEqual([]); - }); - - it('exercises every mutation class, including valid accept cases', () => { - const mutations = new Set(sample.map((input) => decodeValidationCase(input)!.mutation)); - expect(mutations).toContain('valid'); - // A class that stops firing (surface drift, weight bug) is dead coverage and fails here. - expect([...mutations].sort()).toMatchSnapshot(); - }); +describeGeneratorContract({ + targetName: 'cli-validation', + arbitrary: cliValidationArb, + declaredClasses: CLI_MUTATION_NAMES, + sample, }); describe('planted CLI violations are refused where the generator says they are', () => { diff --git a/scripts/fuzz/validation-arbitraries-cli.ts b/scripts/fuzz/validation-arbitraries-cli.ts index 79b474562d..3dbad5063a 100644 --- a/scripts/fuzz/validation-arbitraries-cli.ts +++ b/scripts/fuzz/validation-arbitraries-cli.ts @@ -35,14 +35,6 @@ const EXCLUDED_CLI_COMMANDS = new Set(['cdp', 'react-devtools', 'batch']); // batch step-source invariant. All are exercised elsewhere; here they would blur expectations. const EXCLUDED_FLAG_KEYS = new Set(['help', 'version', 'snapshotDiff', 'steps', 'stepsFile']); -function collidingFlagNames(definitions: readonly FlagDefinition[]): Set { - const counts = new Map(); - for (const definition of definitions) { - for (const name of definition.names) counts.set(name, (counts.get(name) ?? 0) + 1); - } - return new Set([...counts.entries()].filter(([, count]) => count > 1).map(([name]) => name)); -} - /** * Deriving the surface walks the whole command-metadata registry, so it is computed on first use * rather than at import: a `--input-file` replay or a classic-target run must not pay for a @@ -65,7 +57,16 @@ export function validationSurfaceBuildCount(): number { return surfaceBuilds; } -const collidingNames = memoize(() => collidingFlagNames(getFlagDefinitions())); +/** Names carried by more than one definition (`--port`, `--scope`): which one the parser picks + * depends on the command, so a case built on one would assert an outcome the parser never owed. */ +const collidingNames = memoize(() => { + const seen = new Set(); + const colliding = new Set(); + for (const definition of getFlagDefinitions()) { + for (const name of definition.names) (seen.has(name) ? colliding : seen).add(name); + } + return colliding; +}); /** Flag definitions with a single unambiguous long name, excluding the special keys. */ const safeFlagPool = memoize(() => @@ -116,8 +117,8 @@ function validNumericValue(definition: FlagDefinition, salt: number): string { /** A schema-valid value for one flag, salt-selected so shrinking stays deterministic. */ function validFlagValue(definition: FlagDefinition, salt: number): string { if (definition.type === 'enum') { - const values = definition.enumValues ?? []; - return values[salt % Math.max(values.length, 1)] ?? '1'; + const values = definition.enumValues!; + return values[salt % values.length]!; } if (definition.type === 'int' || definition.type === 'number') { return validNumericValue(definition, salt); @@ -182,6 +183,31 @@ const fakeCommands = memoize(() => ), ); +/** + * Five of the seven classes are the same shape: pick a flag the schema says matches, render argv + * around it. Writing that out per class duplicated the picker and restated each class name inside + * its own body, where it could drift from the declared `name`; the factory owns both. + */ +function flagMutation(spec: { + name: string; + layer: CliMutationLayer; + weight: number; + code: string; + /** Flags this class can plant, from the schema — empty means the class does not apply. */ + candidates: (base: CliBase) => readonly FlagDefinition[]; + argv: (definition: FlagDefinition, base: CliBase) => string[]; +}): CliMutation { + const { candidates, argv, ...declaration } = spec; + return { + ...declaration, + apply: (base) => { + const pool = candidates(base); + const definition = pool[base.salt % pool.length]; + return definition ? { payload: argv(definition, base), mutation: spec.name } : null; + }, + }; +} + const CLI_MUTATIONS: readonly CliMutation[] = [ { // #1433: a bounded command must refuse extra positionals instead of swallowing them. @@ -202,91 +228,63 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ }; }, }, - { + flagMutation({ name: 'unsupported-flag', layer: 'command-validation', weight: 6, code: 'INVALID_ARGS', - apply: (base) => { - const foreign = safeFlagPool().filter( + candidates: (base) => + safeFlagPool().filter( (definition) => !isFlagSupportedForCommand(definition.key, base.surface.name), - ); - const definition = foreign[base.salt % Math.max(foreign.length, 1)]; - if (!definition) return null; - return { - payload: [...validArgv(base), renderFlag(definition, base.salt)], - mutation: 'unsupported-flag', - }; - }, - }, - { + ), + argv: (definition, base) => [...validArgv(base), renderFlag(definition, base.salt)], + }), + flagMutation({ name: 'bad-enum-value', layer: 'token-scan', weight: 1, code: 'INVALID_ARGS', - apply: (base) => { - const enums = base.surface.flags.filter((definition) => definition.type === 'enum'); - const definition = enums[base.salt % Math.max(enums.length, 1)]; - if (!definition) return null; - return { - payload: [base.surface.name, `${flagToken(definition)}=bogus-${base.salt % 7}`], - mutation: 'bad-enum-value', - }; - }, - }, - { + candidates: (base) => base.surface.flags.filter((definition) => definition.type === 'enum'), + argv: (definition, base) => [ + base.surface.name, + `${flagToken(definition)}=bogus-${base.salt % 7}`, + ], + }), + flagMutation({ name: 'int-out-of-range', layer: 'token-scan', weight: 1, code: 'INVALID_ARGS', - apply: (base) => { - const bounded = base.surface.flags.filter( + candidates: (base) => + base.surface.flags.filter( (definition) => (definition.type === 'int' || definition.type === 'number') && (definition.min !== undefined || definition.max !== undefined), - ); - const definition = bounded[base.salt % Math.max(bounded.length, 1)]; - if (!definition) return null; - const value = - definition.min !== undefined ? String(definition.min - 1) : String(definition.max! + 1); - return { - payload: [base.surface.name, `${flagToken(definition)}=${value}`], - mutation: 'int-out-of-range', - }; - }, - }, - { + ), + argv: (definition, base) => [ + base.surface.name, + `${flagToken(definition)}=${definition.min !== undefined ? definition.min - 1 : definition.max! + 1}`, + ], + }), + flagMutation({ name: 'missing-flag-value', layer: 'token-scan', weight: 1, code: 'INVALID_ARGS', - apply: (base) => { - const valued = base.surface.flags.filter(isValueFlag); - const definition = valued[base.salt % Math.max(valued.length, 1)]; - if (!definition) return null; - return { - payload: [base.surface.name, flagToken(definition)], - mutation: 'missing-flag-value', - }; - }, - }, - { + candidates: (base) => base.surface.flags.filter(isValueFlag), + argv: (definition, base) => [base.surface.name, flagToken(definition)], + }), + flagMutation({ name: 'boolean-with-value', layer: 'token-scan', weight: 1, code: 'INVALID_ARGS', - apply: (base) => { - const booleans = base.surface.flags.filter( + candidates: (base) => + base.surface.flags.filter( (definition) => definition.type === 'boolean' || definition.setValue !== undefined, - ); - const definition = booleans[base.salt % Math.max(booleans.length, 1)]; - if (!definition) return null; - return { - payload: [...validArgv(base), `${flagToken(definition)}=true`], - mutation: 'boolean-with-value', - }; - }, - }, + ), + argv: (definition, base) => [...validArgv(base), `${flagToken(definition)}=true`], + }), { name: 'unknown-command', layer: 'command-validation', @@ -300,6 +298,9 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ }, ]; +/** The classes this generator declares, so its coverage test needs no hand-kept list. */ +export const CLI_MUTATION_NAMES: readonly string[] = CLI_MUTATIONS.map((mutation) => mutation.name); + const cliBaseArb: fc.Arbitrary = fc .record({ surfaceIndex: fc.nat(), diff --git a/scripts/fuzz/validation-arbitraries-maestro.test.ts b/scripts/fuzz/validation-arbitraries-maestro.test.ts index fb0c3e4361..68f2d18c52 100644 --- a/scripts/fuzz/validation-arbitraries-maestro.test.ts +++ b/scripts/fuzz/validation-arbitraries-maestro.test.ts @@ -8,10 +8,11 @@ import fc from 'fast-check'; import { describe, expect, it } from 'vitest'; -import { describeFailure } from './invariant.ts'; +import { SUPPORTED_MAESTRO_COMMAND_NAMES } from '../../packages/maestro/src/internal/program-ir-command-parser.ts'; import { getFuzzTarget } from './registry.ts'; import { decodeValidationCase } from './validation-case.ts'; -import { maestroValidationArb } from './validation-arbitraries-maestro.ts'; +import { describeGeneratorContract } from './validation-generator-contract.ts'; +import { MAESTRO_MUTATION_NAMES, maestroValidationArb } from './validation-arbitraries-maestro.ts'; const SAMPLE_SIZE = 3_000; const SEED = 1; @@ -19,28 +20,11 @@ const SEED = 1; const target = getFuzzTarget('maestro-validation'); const sample = fc.sample(maestroValidationArb, { numRuns: SAMPLE_SIZE, seed: SEED }); -describe('maestro-validation generator', () => { - it('is deterministic for a seed, so a reported counterexample replays', () => { - const again = fc.sample(maestroValidationArb, { numRuns: 32, seed: 7 }); - expect(again).toEqual(fc.sample(maestroValidationArb, { numRuns: 32, seed: 7 })); - expect(again).not.toEqual(fc.sample(maestroValidationArb, { numRuns: 32, seed: 8 })); - }); - - it('produces decodable envelopes whose expectations hold on a healthy tree', () => { - const failures = []; - for (const input of sample) { - expect(decodeValidationCase(input)).not.toBeNull(); - const failure = target.check!(input); - if (failure) failures.push(describeFailure(failure)); - } - expect(failures).toEqual([]); - }); - - it('exercises every mutation class, including valid accept cases', () => { - const mutations = new Set(sample.map((input) => decodeValidationCase(input)!.mutation)); - expect(mutations).toContain('valid'); - expect([...mutations].sort()).toMatchSnapshot(); - }); +describeGeneratorContract({ + targetName: 'maestro-validation', + arbitrary: maestroValidationArb, + declaredClasses: MAESTRO_MUTATION_NAMES, + sample, }); describe('planted Maestro violations are refused by command-shape validation', () => { @@ -72,3 +56,30 @@ describe('planted Maestro violations are refused by command-shape validation', ( ); }); }); + +describe('shape coverage against the converter', () => { + // The valid Maestro shapes are rendered by hand here, so the one thing that cannot be left to + // drift is which commands they cover: a command the converter accepts but this generator never + // emits is a hole the nightly cannot see. Waivers are explicit and must say why. + const NOT_GENERATED = new Set([ + // Both take a nested command list whose bodies the repeat/runFlow cases already cover. + 'retry', + // Needs a JS file on disk to resolve, which a generated case has no way to provide. + 'runScript', + ]); + + it('emits every command the converter supports, or waives it explicitly', () => { + const emitted = new Set( + sample.flatMap((entry) => { + const { payload } = decodeValidationCase(entry)!; + return typeof payload === 'string' ? payload.split('\n') : []; + }), + ); + const covered = (command: string) => + [...emitted].some((line) => line.trim().startsWith(`- ${command}`)); + const missing = SUPPORTED_MAESTRO_COMMAND_NAMES.filter( + (command) => !NOT_GENERATED.has(command) && !covered(command), + ); + expect(missing).toEqual([]); + }); +}); diff --git a/scripts/fuzz/validation-arbitraries-maestro.ts b/scripts/fuzz/validation-arbitraries-maestro.ts index 40a2eb8f29..5ba4617723 100644 --- a/scripts/fuzz/validation-arbitraries-maestro.ts +++ b/scripts/fuzz/validation-arbitraries-maestro.ts @@ -24,6 +24,8 @@ function validMaestroCommand(pick: number, salt: number): string[] { () => ['- stopApp'], () => ['- scroll'], () => ['- waitForAnimationToEnd'], + () => ['- eraseText'], + () => [`- eraseText: ${1 + (salt % 9)}`], () => ['- launchApp'], () => [`- launchApp: ${text}`], () => [`- tapOn: ${text}`], @@ -92,13 +94,20 @@ const MAESTRO_MUTATIONS: readonly MaestroMutation[] = [ { name: 'scroll-options', code: 'INVALID_ARGS', lines: () => ['- scroll:', ' direction: UP'] }, ]; +/** Declared classes plus the config-level variant `unsupported-field` renders for a salt slice. */ +export const MAESTRO_MUTATION_NAMES: readonly string[] = [ + ...MAESTRO_MUTATIONS.map((mutation) => mutation.name), + 'config-unknown-key', +]; + type MaestroBase = { commandPicks: number[]; salt: number; withConfig: boolean }; -function renderMaestroFlow(base: MaestroBase, mutatedLines?: string[], at?: number): string { +/** `planted` travels as one value: a mutation's lines and where they go are never separable. */ +function renderMaestroFlow(base: MaestroBase, planted?: { lines: string[]; at: number }): string { const commands = base.commandPicks.map((pick, index) => validMaestroCommand(pick, base.salt + index), ); - if (mutatedLines) commands.splice(Math.min(at ?? 0, commands.length), 0, mutatedLines); + if (planted) commands.splice(Math.min(planted.at, commands.length), 0, planted.lines); const body = commands.flat().join('\n'); const config = base.withConfig ? `appId: ${yamlText(base.salt)}\n---\n` : ''; return `${config}${body}\n`; @@ -131,7 +140,7 @@ export const maestroValidationArb: fc.Arbitrary = fc }); } return encodeValidationCase({ - payload: renderMaestroFlow(base, mutation.lines(base.salt), insertAt), + payload: renderMaestroFlow(base, { lines: mutation.lines(base.salt), at: insertAt }), mutation: mutation.name, expect: { outcome: 'reject', code: mutation.code }, }); diff --git a/scripts/fuzz/validation-case.ts b/scripts/fuzz/validation-case.ts index eb9fc16b64..8f59421df9 100644 --- a/scripts/fuzz/validation-case.ts +++ b/scripts/fuzz/validation-case.ts @@ -8,6 +8,7 @@ // carry it unchanged. import { AppError, normalizeError } from '@agent-device/kernel/errors'; +import { describeThrown } from './invariant.ts'; import type { FuzzFailure, FuzzTargetName } from './target-types.ts'; export type ValidationExpectation = { outcome: 'accept' } | { outcome: 'reject'; code: string }; @@ -24,6 +25,23 @@ export function encodeValidationCase(validationCase: ValidationCase): string { return JSON.stringify(validationCase); } +/** A pinned case the parser must accept. */ +export function acceptCase(payload: ValidationCase['payload']): string { + return encodeValidationCase({ payload, mutation: 'valid', expect: { outcome: 'accept' } }); +} + +/** + * A pinned case the parser must refuse with `code`. Seeds run verbatim before any generated case, + * which is where rules whose whole input space is a few strings belong (#1781 B2). + */ +export function rejectCase( + payload: ValidationCase['payload'], + mutation: string, + code = 'INVALID_ARGS', +): string { + return encodeValidationCase({ payload, mutation, expect: { outcome: 'reject', code } }); +} + /** `null` for anything that is not a well-formed validation envelope. */ export function decodeValidationCase(input: string): ValidationCase | null { let parsed: unknown; @@ -141,8 +159,3 @@ function failure( ): FuzzFailure { return { target, input, kind, detail }; } - -function describeThrown(error: unknown): string { - if (error instanceof Error) return `${error.name}: ${error.message}`; - return `non-Error throw: ${typeof error} ${String(error)}`; -} diff --git a/scripts/fuzz/validation-generator-contract.ts b/scripts/fuzz/validation-generator-contract.ts new file mode 100644 index 0000000000..c5edd59fad --- /dev/null +++ b/scripts/fuzz/validation-generator-contract.ts @@ -0,0 +1,51 @@ +// The contract every validation generator owes, as a reusable suite (#1781 B2). +// +// Both generators must be replayable, must hold every expectation they plant against the real +// parser, and must keep firing every class they declare. That is one contract, so it is asserted +// from one place — the domain-specific reach and layer assertions stay in each generator's own +// test file, which is where the knowledge that differs lives. + +import type fc from 'fast-check'; +import { sample } from 'fast-check'; +import { describe, expect, it } from 'vitest'; +import { describeFailure } from './invariant.ts'; +import { getFuzzTarget } from './registry.ts'; +import type { ParserTargetName } from './target-types.ts'; +import { decodeValidationCase } from './validation-case.ts'; + +export function describeGeneratorContract(input: { + targetName: ParserTargetName; + arbitrary: fc.Arbitrary; + /** The classes the generator declares; coverage is checked against it, not a snapshot. */ + declaredClasses: readonly string[]; + sample: readonly string[]; +}): void { + const { targetName, arbitrary, declaredClasses, sample: cases } = input; + const target = getFuzzTarget(targetName); + + describe(`${targetName} generator`, () => { + it('is deterministic for a seed, so a reported counterexample replays', () => { + const again = sample(arbitrary, { numRuns: 32, seed: 7 }); + expect(again).toEqual(sample(arbitrary, { numRuns: 32, seed: 7 })); + expect(again).not.toEqual(sample(arbitrary, { numRuns: 32, seed: 8 })); + }); + + it('produces decodable envelopes whose expectations hold on a healthy tree', () => { + const failures = []; + for (const entry of cases) { + expect(decodeValidationCase(entry)).not.toBeNull(); + const failure = target.check!(entry); + if (failure) failures.push(describeFailure(failure)); + } + expect(failures).toEqual([]); + }); + + // Checked against the generator's own declaration rather than a pinned snapshot: a class that + // stops firing is dead coverage, and a snapshot of the class list only restates the source. + it('fires every class it declares, and still generates valid accept cases', () => { + const fired = new Set(cases.map((entry) => decodeValidationCase(entry)!.mutation)); + expect([...declaredClasses].sort().filter((name) => !fired.has(name))).toEqual([]); + expect(fired).toContain('valid'); + }); + }); +} From 92b0edca1ac779eedb1de4a2db3eb2f1c3b55f0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 21:43:39 +0200 Subject: [PATCH 10/11] fix(fuzz): hash every case-generation module in configHash, guarded by an import-closure test --- scripts/fuzz/envelope.test.ts | 68 +++++++++++++++++++ scripts/fuzz/envelope.ts | 17 ++++- scripts/fuzz/harness.test.ts | 11 +-- scripts/fuzz/validation-arbitraries-cli.ts | 58 +++++++--------- .../fuzz/validation-arbitraries-maestro.ts | 6 +- vitest.config.ts | 1 + 6 files changed, 115 insertions(+), 46 deletions(-) create mode 100644 scripts/fuzz/envelope.test.ts diff --git a/scripts/fuzz/envelope.test.ts b/scripts/fuzz/envelope.test.ts new file mode 100644 index 0000000000..94cdcb02eb --- /dev/null +++ b/scripts/fuzz/envelope.test.ts @@ -0,0 +1,68 @@ +// The run envelope's provenance guarantees (#1414, #1781 B2). +// +// `configHash` exists so "the same seed means different inputs now" is distinguishable from "the +// parsers changed": a stale corpus that reads as confidence is the failure it prevents. That only +// holds while the hash covers every module deciding what a case contains — and the domain split +// broke it silently, because the modules it hashed stopped being where generation lived. This +// test derives the answer from the import graph instead of trusting a hand-kept list. + +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; +import { CASE_GENERATION_INPUTS, NON_GENERATING_MODULES } from './envelope.ts'; + +const FUZZ_DIR = path.dirname(fileURLToPath(import.meta.url)); + +/** Roots of case generation: the arbitraries, the seeds, the loop, and the violation rule. */ +const ROOTS = ['arbitraries.ts', 'generate.ts', 'targets.ts', 'invariant.ts'] as const; + +function localImportsOf(file: string): string[] { + const source = fs.readFileSync(path.join(FUZZ_DIR, file), 'utf8'); + return [...source.matchAll(/from '\.\/([\w-]+\.ts)'/g)].map((match) => match[1]!); +} + +/** + * Walk stops at a waived module: what a non-generating module imports cannot reach a case either + * (the runner imports the target registry, which would otherwise drag the whole harness in). + */ +function generationClosure(): Set { + const seen = new Set(); + const queue = [...ROOTS]; + while (queue.length > 0) { + const file = queue.pop()!; + if (seen.has(file)) continue; + seen.add(file); + if (file in NON_GENERATING_MODULES) continue; + queue.push(...localImportsOf(file)); + } + return seen; +} + +describe('configHash coverage', () => { + it('hashes every module reachable from the generation roots, or waives it with a reason', () => { + const covered = new Set([ + ...CASE_GENERATION_INPUTS, + ...Object.keys(NON_GENERATING_MODULES), + ]); + const uncovered = [...generationClosure()].filter((file) => !covered.has(file)).sort(); + expect(uncovered).toEqual([]); + }); + + it('waives nothing it also hashes, nothing unreachable, and explains every waiver', () => { + const closure = generationClosure(); + for (const [file, reason] of Object.entries(NON_GENERATING_MODULES)) { + expect(CASE_GENERATION_INPUTS).not.toContain(file); + // A waiver for a module the roots no longer reach is dead configuration, and dead + // configuration is how the next reader learns the wrong thing about what is covered. + expect(closure, `${file} is waived but unreachable`).toContain(file); + expect(reason.trim().length).toBeGreaterThan(10); + } + }); + + it('lists only files that exist, so a renamed module fails here rather than hashing nothing', () => { + for (const file of [...CASE_GENERATION_INPUTS, ...Object.keys(NON_GENERATING_MODULES)]) { + expect(fs.existsSync(path.join(FUZZ_DIR, file)), file).toBe(true); + } + }); +}); diff --git a/scripts/fuzz/envelope.ts b/scripts/fuzz/envelope.ts index 9286ad8adb..3df2744773 100644 --- a/scripts/fuzz/envelope.ts +++ b/scripts/fuzz/envelope.ts @@ -83,15 +83,30 @@ export function writeFuzzEnvelope(input: { * and the invariant itself. Hashing a subset would let a changed case set look like an unchanged * lane, which is exactly the drift this field exists to catch. */ -const CASE_GENERATION_INPUTS = [ +export const CASE_GENERATION_INPUTS = [ 'arbitraries.ts', 'generate.ts', 'targets.ts', 'invariant.ts', 'validation-arbitraries.ts', + 'validation-arbitraries-cli.ts', + 'validation-arbitraries-maestro.ts', + 'validation-values.ts', 'validation-case.ts', ] as const; +/** + * Modules reachable from the generation roots that deliberately do NOT feed the hash, because + * they cannot change what a case contains. Kept as data so `envelope.test.ts` can prove the list + * above still covers everything else: the domain split moved case generation out of + * `validation-arbitraries.ts` into two new modules and the hash silently stopped covering them, + * which is exactly the drift a stale corpus reads as confidence. + */ +export const NON_GENERATING_MODULES: Readonly> = { + 'target-types.ts': 'types only — erased at runtime, so no value of it reaches a case', + 'execute.ts': 'runs cases under the watchdog; decides how a case executes, not what it contains', +}; + /** Content hash of `CASE_GENERATION_INPUTS`. */ function harnessHash(): string { const here = path.dirname(new URL(import.meta.url).pathname); diff --git a/scripts/fuzz/harness.test.ts b/scripts/fuzz/harness.test.ts index 0c922deb44..ae0e9058f9 100644 --- a/scripts/fuzz/harness.test.ts +++ b/scripts/fuzz/harness.test.ts @@ -13,6 +13,7 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vitest'; import { LANE_ENVELOPE_SCHEMA_VERSION } from '../lib/lane-envelope.ts'; +import { CASE_GENERATION_INPUTS } from './envelope.ts'; import { checkCase } from './invariant.ts'; import { SELF_CHECK_TARGETS } from './self-check-targets.ts'; @@ -122,15 +123,7 @@ describe('worker startup budget', () => { */ function hashWithout(skip: string): string { const digest = crypto.createHash('sha256'); - const inputs = [ - 'arbitraries.ts', - 'generate.ts', - 'targets.ts', - 'invariant.ts', - 'validation-arbitraries.ts', - 'validation-case.ts', - ]; - for (const name of inputs) { + for (const name of CASE_GENERATION_INPUTS) { if (name !== skip) digest.update(fs.readFileSync(path.join(FUZZ_DIR, name))); } return `sha256:${digest.digest('hex').slice(0, 16)}`; diff --git a/scripts/fuzz/validation-arbitraries-cli.ts b/scripts/fuzz/validation-arbitraries-cli.ts index 3dbad5063a..1423e99b46 100644 --- a/scripts/fuzz/validation-arbitraries-cli.ts +++ b/scripts/fuzz/validation-arbitraries-cli.ts @@ -27,12 +27,12 @@ type CliCommandSurface = { }; // Commands whose parse path is deliberately special: cdp preserves post-command args verbatim, -// react-devtools passes unknown flags through, batch enforces a step-source invariant covered -// by its own fixed mutation below. +// react-devtools passes unknown flags through, and batch's step-source rule has an input space of +// two strings, so it is pinned as a seed case on the target instead of generated. const EXCLUDED_CLI_COMMANDS = new Set(['cdp', 'react-devtools', 'batch']); -// help/version reroute parsing, snapshotDiff rewrites the command, steps/stepsFile carry the -// batch step-source invariant. All are exercised elsewhere; here they would blur expectations. +// help/version reroute parsing and snapshotDiff rewrites the command, so a case carrying one +// asserts an outcome the generator did not plant; steps/stepsFile belong to the pinned batch seeds. const EXCLUDED_FLAG_KEYS = new Set(['help', 'version', 'snapshotDiff', 'steps', 'stepsFile']); /** @@ -97,42 +97,34 @@ const cliSurfaces = memoize(() => { }); }); -/** Inclusive bounds for a numeric flag, defaulted when the schema leaves an end open. */ -function numericBounds(definition: FlagDefinition): { low: number; high: number } { - const low = definition.min ?? 0; - return { low, high: definition.max ?? low + 1000 }; -} - /** - * A schema-valid numeric value. Floats take endpoints and the midpoint only: modulo arithmetic - * can drift past a fractional `max` (a 33k-case slice produced `--scale=1.110000000000017`, - * which the parser rightly rejected — a phantom finding, not a bug). + * A value the schema accepts, salt-selected so a case replays. Floats take endpoints and the + * midpoint only — modulo arithmetic drifts past a fractional `max` (a 33k slice produced + * `--scale=1.110000000000017`, which the parser rightly rejected: a phantom, not a bug). */ -function validNumericValue(definition: FlagDefinition, salt: number): string { - const { low, high } = numericBounds(definition); - if (definition.type === 'int') return String(low + (salt % (high - low + 1))); - return String([low, high, (low + high) / 2][salt % 3]); -} - -/** A schema-valid value for one flag, salt-selected so shrinking stays deterministic. */ -function validFlagValue(definition: FlagDefinition, salt: number): string { - if (definition.type === 'enum') { - const values = definition.enumValues!; - return values[salt % values.length]!; - } - if (definition.type === 'int' || definition.type === 'number') { - return validNumericValue(definition, salt); +function flagValue(definition: FlagDefinition, salt: number): string { + const low = definition.min ?? 0; + const high = definition.max ?? low + 1000; + switch (definition.type) { + case 'enum': + return definition.enumValues![salt % definition.enumValues!.length]!; + case 'int': + return String(low + (salt % (high - low + 1))); + case 'number': + return String([low, high, (low + high) / 2][salt % 3]); + default: + return SAFE_VALUES[salt % SAFE_VALUES.length] || 'value'; } - const value = SAFE_VALUES[salt % SAFE_VALUES.length]!; - return value.length === 0 ? 'value' : value; } -/** Renders one flag as argv tokens; value flags use `--flag=value` so no token is consumed. */ +/** One flag as argv: a bare token for the kinds that take no value, `--flag=value` otherwise. */ function renderFlag(definition: FlagDefinition, salt: number): string { const token = flagToken(definition); - if (definition.type === 'boolean' || definition.setValue !== undefined) return token; - if (definition.type === 'booleanOrString') return token; - return `${token}=${validFlagValue(definition, salt)}`; + const bare = + definition.setValue !== undefined || + definition.type === 'boolean' || + definition.type === 'booleanOrString'; + return bare ? token : `${token}=${flagValue(definition, salt)}`; } type CliBase = { diff --git a/scripts/fuzz/validation-arbitraries-maestro.ts b/scripts/fuzz/validation-arbitraries-maestro.ts index 5ba4617723..27fa4036b4 100644 --- a/scripts/fuzz/validation-arbitraries-maestro.ts +++ b/scripts/fuzz/validation-arbitraries-maestro.ts @@ -2,9 +2,9 @@ // // Cases are built from the command shapes the converter accepts, then one shape rule is violated, // so the failure surfaces in command-shape validation rather than in the YAML tokenizer. The -// shapes are rendered here rather than derived from a registry — Maestro's accepted surface is a -// parser table, not exported data — so the generator test replays samples against the real parser -// and a drifted shape fails at PR time instead of phantoming nightly. +// shapes are rendered here because Maestro's accepted surface is a parser table rather than +// exported data; the test asserts this file covers `SUPPORTED_MAESTRO_COMMAND_NAMES`, so the copy +// cannot silently drift from the converter. import fc from 'fast-check'; import { encodeValidationCase } from './validation-case.ts'; diff --git a/vitest.config.ts b/vitest.config.ts index a4637ea674..9e75027c68 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -67,6 +67,7 @@ export default defineConfig({ 'scripts/fuzz/validation-arbitraries-cli.test.ts', 'scripts/fuzz/validation-arbitraries-maestro.test.ts', 'scripts/fuzz/validation-case.test.ts', + 'scripts/fuzz/envelope.test.ts', 'scripts/__tests__/help-conformance-bench.test.ts', 'scripts/__tests__/help-conformance-error-recovery-coverage.test.ts', 'scripts/__tests__/help-conformance-sample-outputs.test.ts', From 9238162d7a4b9899063970c08d9f349f6b9010c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 22:07:48 +0200 Subject: [PATCH 11/11] test(fuzz): assert CLI command and flag-key coverage against the registry, and close the six gaps it found --- .../fuzz/validation-arbitraries-cli.test.ts | 52 ++++++++- scripts/fuzz/validation-arbitraries-cli.ts | 101 ++++++++++++------ 2 files changed, 121 insertions(+), 32 deletions(-) diff --git a/scripts/fuzz/validation-arbitraries-cli.test.ts b/scripts/fuzz/validation-arbitraries-cli.test.ts index c4e612a7e2..f4953f3b1b 100644 --- a/scripts/fuzz/validation-arbitraries-cli.test.ts +++ b/scripts/fuzz/validation-arbitraries-cli.test.ts @@ -8,10 +8,18 @@ import fc from 'fast-check'; import { describe, expect, it, vi } from 'vitest'; +import { listCliCommandNames } from '../../src/command-catalog.ts'; +import { getFlagDefinitions } from '../../src/cli-schema/command-schema.ts'; import { getFuzzTarget } from './registry.ts'; import { decodeValidationCase } from './validation-case.ts'; import { describeGeneratorContract } from './validation-generator-contract.ts'; -import { CLI_MUTATION_NAMES, cliValidationArb } from './validation-arbitraries-cli.ts'; +import { + CLI_MUTATION_NAMES, + cliValidationArb, + generatableCliSurface, + UNGENERATED_COMMANDS, + UNGENERATED_FLAG_KEYS, +} from './validation-arbitraries-cli.ts'; // A mismatch that fires at ~1-in-1000 must not pass here and then phantom nightly: the nightly // draws 38,000 cases per target, so this samples 7.9% of it rather than the 1.5% it began at. @@ -85,3 +93,45 @@ describe('generator startup', () => { expect(fresh.validationSurfaceBuildCount()).toBe(1); }); }); + +describe('surface coverage against the registry', () => { + // The same shape as the Maestro converter-coverage assertion, and it found the same class of + // bug: hand-kept knowledge about which parts of the surface are skipped goes stale silently. + // Both sides are derived — the catalog and the flag registry on one, the generator's own + // reachable surface on the other — so a new command or flag key is covered or named, never + // ignored. Reachability, not sampling: a flag drawn once per few thousand cases would otherwise + // make this gate depend on seed luck. + const generatable = generatableCliSurface(); + + it('can emit every command in the catalog, or waives it with a reason', () => { + const commands = new Set(generatable.commands); + const missing = listCliCommandNames().filter( + (command) => !commands.has(command) && !(command in UNGENERATED_COMMANDS), + ); + expect(missing).toEqual([]); + }); + + it('can emit every flag key in the schema, or waives it with a reason', () => { + const emitted = new Set(generatable.flagKeys); + const keys = [...new Set(getFlagDefinitions().map((definition) => definition.key))]; + const missing = keys.filter((key) => !emitted.has(key) && !(key in UNGENERATED_FLAG_KEYS)); + expect(missing).toEqual([]); + }); + + it('waives nothing it can emit, and nothing the registry no longer has', () => { + const catalog = new Set(listCliCommandNames()); + const commands = new Set(generatable.commands); + for (const [command, reason] of Object.entries(UNGENERATED_COMMANDS)) { + expect(catalog, `${command} is waived but not in the catalog`).toContain(command); + expect(commands, `${command} is waived but generatable`).not.toContain(command); + expect(reason.trim().length).toBeGreaterThan(10); + } + const keys = new Set(getFlagDefinitions().map((definition) => definition.key)); + const emitted = new Set(generatable.flagKeys); + for (const [key, reason] of Object.entries(UNGENERATED_FLAG_KEYS)) { + expect(keys, `${key} is waived but not in the schema`).toContain(key); + expect(emitted, `${key} is waived but generatable`).not.toContain(key); + expect(reason.trim().length).toBeGreaterThan(10); + } + }); +}); diff --git a/scripts/fuzz/validation-arbitraries-cli.ts b/scripts/fuzz/validation-arbitraries-cli.ts index 1423e99b46..5dd45a13c2 100644 --- a/scripts/fuzz/validation-arbitraries-cli.ts +++ b/scripts/fuzz/validation-arbitraries-cli.ts @@ -26,14 +26,29 @@ type CliCommandSurface = { flags: readonly FlagDefinition[]; }; -// Commands whose parse path is deliberately special: cdp preserves post-command args verbatim, -// react-devtools passes unknown flags through, and batch's step-source rule has an input space of -// two strings, so it is pinned as a seed case on the target instead of generated. -const EXCLUDED_CLI_COMMANDS = new Set(['cdp', 'react-devtools', 'batch']); +/** + * Commands this generator never emits, each with the reason it is out. Declared as data because + * `validation-arbitraries-cli.test.ts` walks the catalog and fails on any command that is neither + * reachable nor waived here — and on any waiver the catalog no longer has, so dead entries cannot + * accumulate the way the Maestro shape table drifted before its coverage assertion existed. + */ +export const UNGENERATED_COMMANDS: Readonly> = { + cdp: 'preserves every post-command argument verbatim, so no planted violation can be refused', + 'react-devtools': 'passes unknown flags through to the local tool instead of refusing them', + batch: 'its step-source rule has an input space of two strings, pinned as seed cases instead', +}; -// help/version reroute parsing and snapshotDiff rewrites the command, so a case carrying one -// asserts an outcome the generator did not plant; steps/stepsFile belong to the pinned batch seeds. -const EXCLUDED_FLAG_KEYS = new Set(['help', 'version', 'snapshotDiff', 'steps', 'stepsFile']); +/** Flag keys this generator never emits, same contract as the commands above. */ +export const UNGENERATED_FLAG_KEYS: Readonly> = { + help: 'reroutes parsing to help output, so the planted outcome never happens', + version: 'reroutes parsing to version output, same as help', + snapshotDiff: 'rewrites the command to `diff`, so the case no longer asserts what it planted', + steps: 'belongs to the pinned batch step-source seeds', + stepsFile: 'belongs to the pinned batch step-source seeds', + installSource: 'carries no CLI token at all (`names: []`) — reachable only through config', + batchOnError: 'supported only on `batch`, which is waived above, so no case can carry it', + batchMaxSteps: 'supported only on `batch`, which is waived above, so no case can carry it', +}; /** * Deriving the surface walks the whole command-metadata registry, so it is computed on first use @@ -57,42 +72,46 @@ export function validationSurfaceBuildCount(): number { return surfaceBuilds; } -/** Names carried by more than one definition (`--port`, `--scope`): which one the parser picks - * depends on the command, so a case built on one would assert an outcome the parser never owed. */ -const collidingNames = memoize(() => { - const seen = new Set(); - const colliding = new Set(); - for (const definition of getFlagDefinitions()) { - for (const name of definition.names) (seen.has(name) ? colliding : seen).add(name); - } - return colliding; -}); - -/** Flag definitions with a single unambiguous long name, excluding the special keys. */ +/** Every definition that can be written as a token at all, minus the waived keys. */ const safeFlagPool = memoize(() => getFlagDefinitions().filter( - (definition) => - !EXCLUDED_FLAG_KEYS.has(definition.key) && - definition.names.some((name) => name.startsWith('--') && !collidingNames().has(name)), + (definition) => !(definition.key in UNGENERATED_FLAG_KEYS) && definition.names.length > 0, ), ); +/** Long token when there is one; short-only definitions (`-i`) are part of the surface too. */ function flagToken(definition: FlagDefinition): string { - return definition.names.find((name) => name.startsWith('--') && !collidingNames().has(name))!; + return definition.names.find((name) => name.startsWith('--')) ?? definition.names[0]!; +} + +/** + * Flags this command can carry unambiguously. A token two definitions share (`--port` on `proxy` + * versus `metro`, `--scope` on `record` versus the snapshot family) is fine as long as only one of + * them is supported *here* — which is how `resolveFlagDefinition` reads it too. Excluding those + * tokens outright left six keys silently ungenerated until the coverage assertion said so. + */ +function unambiguousFlagsFor(command: string): readonly FlagDefinition[] { + const supported = safeFlagPool().filter((definition) => + isFlagSupportedForCommand(definition.key, command), + ); + const perToken = new Map(); + for (const definition of supported) { + const token = flagToken(definition); + perToken.set(token, (perToken.get(token) ?? 0) + 1); + } + return supported.filter((definition) => perToken.get(flagToken(definition)) === 1); } const cliSurfaces = memoize(() => { surfaceBuilds += 1; return listCliCommandNames() - .filter((name) => !EXCLUDED_CLI_COMMANDS.has(name)) + .filter((name) => !(name in UNGENERATED_COMMANDS)) .map((name) => { const schema = getCliCommandSchema(name); return { name, maxPositionals: schema.allowsExtraPositionals ? null : (schema.positionalArgs?.length ?? 0), - flags: safeFlagPool().filter((definition) => - isFlagSupportedForCommand(definition.key, name), - ), + flags: unambiguousFlagsFor(name), }; }); }); @@ -225,10 +244,17 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ layer: 'command-validation', weight: 6, code: 'INVALID_ARGS', - candidates: (base) => - safeFlagPool().filter( - (definition) => !isFlagSupportedForCommand(definition.key, base.surface.name), - ), + // A foreign flag must also be a foreign *token*: `--port` is unsupported on `proxy` as the + // metro key, but the parser resolves that token to proxy's own `--port` and accepts it, so the + // case would assert a violation the parser never owed. + candidates: (base) => { + const ownTokens = new Set(base.surface.flags.map(flagToken)); + return safeFlagPool().filter( + (definition) => + !isFlagSupportedForCommand(definition.key, base.surface.name) && + !ownTokens.has(flagToken(definition)), + ); + }, argv: (definition, base) => [...validArgv(base), renderFlag(definition, base.salt)], }), flagMutation({ @@ -290,6 +316,19 @@ const CLI_MUTATIONS: readonly CliMutation[] = [ }, ]; +/** + * What the generator can emit at all: the commands it builds cases for and the flag keys those + * commands can carry. Reachability rather than a sample, because a flag drawn about once per + * 3,000 cases would make a coverage gate depend on seed luck instead of on the surface. + */ +export function generatableCliSurface(): { commands: string[]; flagKeys: string[] } { + const surfaces = cliSurfaces(); + return { + commands: surfaces.map((surface) => surface.name), + flagKeys: [...new Set(surfaces.flatMap((surface) => surface.flags.map((flag) => flag.key)))], + }; +} + /** The classes this generator declares, so its coverage test needs no hand-kept list. */ export const CLI_MUTATION_NAMES: readonly string[] = CLI_MUTATIONS.map((mutation) => mutation.name);