Skip to content

Guard help-conformance correctness at production and CI boundaries #1406

Description

@thymikee

Context

PR #1404 and its review fixes exposed a recurring class of false-green paths around agent-facing help:

  • The help-conformance benchmark validates positional arity, while the production CLI can still silently discard extra positionals.
  • A benchmark scorer can exist and pass without proving that it rejects the bad plan that motivated it.
  • Runner/API failures are reconstructed from raw strings and can be reported alongside model-validation failures, making infrastructure instability look like model quality.
  • Changes to the help-conformance harness fall through to an overly broad affected-check set, including paid/stochastic SkillGym work.
  • The main harnesses have grown beyond the repository's context-safety guidance, but that guidance is not ratcheted.

These are different symptoms of the same gap: important claims are benchmark-local conventions instead of fail-closed contracts at the boundary that owns them.

Concrete production examples today include:

snapshot ignored.json   # extra positional is ignored
close first second      # `second` is ignored
open app url close      # `close` is swallowed instead of being a lifecycle command

Goal

Make it difficult to ship another help/benchmark false green:

  1. Production and benchmark parsing share one positional contract.
  2. Every important oracle demonstrates that it can fail.
  3. Infrastructure failures cannot enter model scoring.
  4. Deterministic help-conformance checks have explicit affected-check ownership.
  5. Guidance harnesses remain cheap enough for an agent to read and change safely.

Proposed work

1. Enforce positional bounds at the production CLI boundary

  • Add one shared positional-arity assertion derived from CommandSchema.
  • Apply it to normal CLI parsing and legacy batch positionals.
  • Require an explicit schema trait for any command that intentionally accepts additional positionals.
  • Add an exhaustive test that supplies max + 1 positionals to every bounded command and expects rejection.
  • Remove the benchmark's private arity implementation once the production contract is reusable.
  • Update CLI help/docs and add a behavioral SkillGym case if rejecting ignored input changes the user-visible contract.

Relevant seams:

  • src/cli/parser/args.ts
  • src/commands/cli-grammar/registry.ts
  • command readers such as src/commands/management/app.ts
  • scripts/help-conformance-command-validator.ts

2. Require falsification fixtures for benchmark oracles

Extend case/expectation definitions so each important oracle carries:

  • a minimal passing witness;
  • at least one known-bad counterexample;
  • where useful, a metamorphic variant that changes irrelevant nouns/values.

Add a completeness test that rejects a named expectation without falsification fixtures. Include regressions for swallowed lifecycle commands, unsupported flags/selectors, pseudo refs, shell operators, and invalid positional ordering.

3. Make runner outcomes typed and evidence honest

Replace raw-string success inference with a discriminated outcome:

type RunnerOutcome =
  | { kind: 'success'; raw: string; commands: string[] }
  | { kind: 'runner-error'; raw: string; message: string; reason: RunnerErrorReason };
  • Only success outcomes may reach command validation and scoring.
  • A result must not contain both runnerError and model-validation failures.
  • All-error aggregates render N/A, not 0/0 (0%).
  • Add an --evidence mode that requires at least three evaluated trials and emits paste-ready Markdown containing commit SHA, runner/model, evaluated trials, runner errors, pass rates, and help-document hashes.
  • Keep single-trial mode for exploration, but do not label it stability evidence.

4. Give help-conformance tooling explicit affected-check ownership

  • Add a deterministic help-conformance check to check:affected.
  • Give it explicit ownership of scripts/help-conformance-* and focused tests.
  • Treat test/skillgym/README.md as documentation rather than model input.
  • Keep stochastic/paid Claude and Codex calls non-gating; gate schemas, counterexamples, adapters, and reporting deterministically.

5. Split oversized harnesses and add a merge-base ratchet

Current sizes:

  • scripts/help-conformance-bench.mjs: 846 lines
  • test/skillgym/suites/agent-device-smoke-suite.ts: 2,807 lines
  • scripts/__tests__/help-conformance-bench.test.ts: 449 lines

Split by question: case definitions, runners, reporting, scoring, shared SkillGym harness, fixture cases, and guidance cases. Then add a merge-base-aware size guard:

  • New implementation files cannot exceed 500 lines.
  • Existing implementation files over 500 lines cannot grow.
  • Files over 1,000 lines require a reviewed waiver or must shrink.
  • Generated fixtures/data receive documented exemptions.
  • Source/test topology stays aligned when modules split.

The ratchet should report the file, baseline/current size, applicable rule, and recovery action. It should not require a repository-wide cleanup before landing.

Suggested delivery order

  1. Production positional contract.
  2. Harness split.
  3. Typed outcomes, falsification fixtures, and evidence mode.
  4. Affected-check ownership and module-size ratchet.

Separate PRs are expected; later work should build on the production contract rather than duplicating it.

Acceptance criteria

  • Extra positionals fail consistently in the production CLI, legacy batch parsing, and the benchmark.
  • Positional behavior is derived from CommandSchema; no parallel command/arity map remains.
  • Every named/important benchmark expectation has a passing witness and known-bad counterexample enforced by a completeness test.
  • Runner/API errors are structurally separate from model-plan failures and cannot affect pass-rate denominators.
  • All-infrastructure-error reports show N/A.
  • Evidence reports are reproducible and include trials, errors, commit/model identity, rates, and document hashes.
  • check:affected selects a deterministic help-conformance check for harness changes without automatically requiring paid model runs.
  • test/skillgym/README.md changes remain documentation-only.
  • Oversized harnesses are split by responsibility and a merge-base size ratchet prevents regression.
  • Relevant deterministic tests, affected-check tests, CLI help/docs, and behavioral SkillGym coverage are updated.

Non-goals

  • Making stochastic model calls a required PR gate.
  • Treating one model trial as stability evidence.
  • Duplicating command grammar or help contracts inside the benchmark.
  • A blanket line-count gate without grandfathering and explicit generated-data exemptions.

Related: #1404

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions