diff --git a/.github/workflows/replays-nightly.yml b/.github/workflows/replays-nightly.yml index 623f3be336..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: '50000' + default: '38000' fuzz-seed: description: 'Parser fuzz PRNG seed (defaults to the run number)' required: false @@ -26,12 +26,19 @@ 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). + # + # 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 @@ -60,7 +67,7 @@ jobs: - name: Fuzz parsers if: always() env: - FUZZ_ITERATIONS: ${{ github.event.inputs.fuzz-iterations || '50000' }} + 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/docs/agents/testing.md b/docs/agents/testing.md index 44d5e6841f..e19463637a 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -378,26 +378,26 @@ 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). -```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. - -Cases come from fast-check arbitraries (`scripts/fuzz/arbitraries.ts`) built on the hazard vocabulary +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 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 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 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 +407,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/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/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 0a549a78bc..3df2744773 100644 --- a/scripts/fuzz/envelope.ts +++ b/scripts/fuzz/envelope.ts @@ -83,13 +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/generate.ts b/scripts/fuzz/generate.ts index 6ddd2737f4..436befaa09 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,22 @@ 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, 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); +} + /** The generated half of a run: fast-check picks the inputs and shrinks any counterexample. */ async function checkGenerated( target: FuzzTarget, @@ -76,7 +93,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/harness.test.ts b/scripts/fuzz/harness.test.ts index 447efef9e7..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'; @@ -52,16 +53,45 @@ 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']); + // 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 +123,7 @@ 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']) { + 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)}`; @@ -158,14 +188,6 @@ describe('run envelope', () => { expect(envelope.result).toBe('fail'); 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..0514ead4db 100644 --- a/scripts/fuzz/invariant.ts +++ b/scripts/fuzz/invariant.ts @@ -1,30 +1,31 @@ -// 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, FuzzTarget } from './target-types.ts'; -export type FuzzFailure = { - target: FuzzTargetName; - input: string; - kind: FuzzFailureKind; - detail: string; -}; +// 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`. - * 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; @@ -50,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/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..d166b810e3 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 { 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 @@ -91,6 +92,49 @@ 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: [ + 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. + 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'), + ], + }, + { + // 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: [ + 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'), + ], + }, { name: 'maestro', description: 'parseMaestroProgram (Maestro compat)', @@ -108,3 +152,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-cli.test.ts b/scripts/fuzz/validation-arbitraries-cli.test.ts new file mode 100644 index 0000000000..f4953f3b1b --- /dev/null +++ b/scripts/fuzz/validation-arbitraries-cli.test.ts @@ -0,0 +1,137 @@ +// 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 { 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, + 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. +const SAMPLE_SIZE = 3_000; +const SEED = 1; + +const target = getFuzzTarget('cli-validation'); +const sample = fc.sample(cliValidationArb, { numRuns: SAMPLE_SIZE, seed: SEED }); + +describeGeneratorContract({ + targetName: 'cli-validation', + arbitrary: cliValidationArb, + declaredClasses: CLI_MUTATION_NAMES, + sample, +}); + +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); + }); +}); + +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 new file mode 100644 index 0000000000..5dd45a13c2 --- /dev/null +++ b/scripts/fuzz/validation-arbitraries-cli.ts @@ -0,0 +1,385 @@ +// 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 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', +}; + +/** 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 + * 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; +} + +/** Every definition that can be written as a token at all, minus the waived keys. */ +const safeFlagPool = memoize(() => + getFlagDefinitions().filter( + (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('--')) ?? 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) => !(name in UNGENERATED_COMMANDS)) + .map((name) => { + const schema = getCliCommandSchema(name); + return { + name, + maxPositionals: schema.allowsExtraPositionals ? null : (schema.positionalArgs?.length ?? 0), + flags: unambiguousFlagsFor(name), + }; + }); +}); + +/** + * 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 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'; + } +} + +/** 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); + const bare = + definition.setValue !== undefined || + definition.type === 'boolean' || + definition.type === 'booleanOrString'; + return bare ? token : `${token}=${flagValue(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), + ), +); + +/** + * 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. + 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', + }; + }, + }, + flagMutation({ + name: 'unsupported-flag', + layer: 'command-validation', + weight: 6, + code: 'INVALID_ARGS', + // 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({ + name: 'bad-enum-value', + layer: 'token-scan', + weight: 1, + code: 'INVALID_ARGS', + 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', + candidates: (base) => + base.surface.flags.filter( + (definition) => + (definition.type === 'int' || definition.type === 'number') && + (definition.min !== undefined || definition.max !== undefined), + ), + 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', + 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', + candidates: (base) => + base.surface.flags.filter( + (definition) => definition.type === 'boolean' || definition.setValue !== undefined, + ), + argv: (definition, base) => [...validArgv(base), `${flagToken(definition)}=true`], + }), + { + 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' }; + }, + }, +]; + +/** + * 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); + +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..68f2d18c52 --- /dev/null +++ b/scripts/fuzz/validation-arbitraries-maestro.test.ts @@ -0,0 +1,85 @@ +// 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 { 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 { describeGeneratorContract } from './validation-generator-contract.ts'; +import { MAESTRO_MUTATION_NAMES, 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 }); + +describeGeneratorContract({ + targetName: 'maestro-validation', + arbitrary: maestroValidationArb, + declaredClasses: MAESTRO_MUTATION_NAMES, + sample, +}); + +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/, + ); + }); +}); + +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 new file mode 100644 index 0000000000..27fa4036b4 --- /dev/null +++ b/scripts/fuzz/validation-arbitraries-maestro.ts @@ -0,0 +1,147 @@ +// 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 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'; +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'], + () => ['- eraseText'], + () => [`- eraseText: ${1 + (salt % 9)}`], + () => ['- 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'] }, +]; + +/** 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 }; + +/** `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 (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`; +} + +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, { lines: mutation.lines(base.salt), at: 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 new file mode 100644 index 0000000000..5a17ac1b93 --- /dev/null +++ b/scripts/fuzz/validation-arbitraries.test.ts @@ -0,0 +1,18 @@ +// The validation generator lookup (#1781 B2). + +import { describe, expect, it } from 'vitest'; +import { validationArbitraryFor } from './validation-arbitraries.ts'; + +describe('validationArbitraryFor', () => { + it('resolves a generator for each validation target', () => { + expect(validationArbitraryFor('cli-validation')).toBeDefined(); + expect(validationArbitraryFor('maestro-validation')).toBeDefined(); + }); + + // `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 new file mode 100644 index 0000000000..81799b7a08 --- /dev/null +++ b/scripts/fuzz/validation-arbitraries.ts @@ -0,0 +1,17 @@ +// Target name to validation generator (#1781 B2). +// +// 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 type fc from 'fast-check'; +import type { FuzzTargetName } from './target-types.ts'; +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 { + if (target === 'cli-validation') return cliValidationArb; + if (target === 'maestro-validation') return maestroValidationArb; + return 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-case.ts b/scripts/fuzz/validation-case.ts new file mode 100644 index 0000000000..8f59421df9 --- /dev/null +++ b/scripts/fuzz/validation-case.ts @@ -0,0 +1,161 @@ +// 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 { describeThrown } from './invariant.ts'; +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); +} + +/** 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; + 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 }; +} 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'); + }); + }); +} 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 e15533161a..9e75027c68 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -61,6 +61,13 @@ export default defineConfig({ include: [ 'src/**/*.test.ts', 'packages/*/src/**/*.test.ts', + // 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/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',