feat: strengthen agent help benchmarks

[thymikee]

Summary

Improve agent-facing CLI help with a compact, validated starting point and explicit --settle command boundaries.

Harden help-conformance research with parser-backed syntax validation, metamorphic cases, repeat trials, infrastructure-error separation, token-based case scoring, and aggregate failure summaries. SkillGym output-interpretation cases now prove that a real local help command was consumed before scoring; permitted wrapper commands cannot satisfy that requirement by themselves.

Scope: 17 files. The work expanded from CLI help and SkillGym prompts into their benchmark oracle and harness; it does not change device runtime behavior.

Validation

• Rebased onto origin/main at 56b72c5cf; formatting, typecheck, lint, tooling/build checks, and the 17-file fallow audit are green on a93e4741f.
• Review-specific deterministic regressions: 24/24 pass, covering unquoted line breaks, local-help bypasses, quote-insensitive selector scoring, quoting-invariant command scoring, and runner-envelope classification.
• The quoted-selector regression uses the exact reviewed plan, agent-device press label="@react.dev" --settle, and verifies that parsed-token canonicalization still satisfies the semantic matcher.
• Earlier model-backed evidence remains visible: the four affected SkillGym cases passed on both configured runners, for 8/8 executions across GPT-5.4-mini and Claude Haiku.
• A full local unit run hit the repository's documented host-contention pattern across unrelated upload, Android, router, replay, and Apple files. Representative failures passed in isolation; the remaining Apple timeout passed alone in 738 ms. GitHub CI on the pushed head remains authoritative.
• Earlier Haiku stability sampling remains visible rather than collapsed into a single headline: the nine-case pass was 6/9; targeted three-trial repeats were 3/3 for engineering-validate-mode, 3/3 for settle-diff-is-observation, and 2/3 for sample-output-not-settled-needs-observe.

Known boundary: parser-backed validation proves syntax, positional bounds, and interaction grammar, but it cannot prove runtime meaning for intentionally free-form values such as open <appOrUrl>.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-27 08:18 UTC

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.85 MB	1.85 MB	+806 B
JS gzip	592.4 kB	592.7 kB	+333 B
npm tarball	706.5 kB	706.9 kB	+435 B
npm unpacked	2.47 MB	2.47 MB	+1.5 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	28.4 ms	27.6 ms	-0.9 ms
CLI --help	63.1 ms	58.0 ms	-5.1 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/cli-help.js	+806 B	+333 B

[thymikee]

Review finding (actionable): the new grammar oracle still accepts excess positionals for command readers that silently ignore them. validateParsedCommand() delegates to readInputFromCli(), but readers such as openCliReader consume only the first two positionals. Thus agent-device open com.example.community close is reported valid, while close is merely the second open positional; the benchmark's regex-based opensAndCloses check can count it as cleanup even though no close command runs. This leaves a false-green path in the core benchmark claim. Please validate positional arity from the production command schema (or otherwise reject unconsumed tokens) and add a regression proving the malformed plan fails.

Validation is also incomplete for the changed SkillGym assertions/prompts: the PR reports only sample-output-settled-diff-next-target. Per docs/agents/testing.md, run and report the focused SkillGym cases affected here (settle-diff-is-observation, sample-output-settled-diff-next-target, sample-output-not-settled-needs-observe, and sample-output-private-ax-recovery-continues). CI is otherwise green at e3f086c9bd6e7f2c671ac7eaa1ea606ff5acbbc6.

[thymikee]

Reviewed and benchmarked at e3f086c in a cloud container (no simulator; all cases below are planning-only).

Validation run

Check	Result
vitest run scripts/__tests__/help-conformance-bench.test.ts src/cli/parser/__tests__ src/cli-schema/command-schema-guards.test.ts	196/196 pass — matches the PR claim
pnpm lint / pnpm format:check / pnpm check:tooling	green
Help bench, all 9 cases, --runner claude:claude-haiku-4-5	6 PASS / 3 FAIL
Help bench --repeat 3 on those 3	engineering-validate-mode 3/3, settle-diff-is-observation 3/3, sample-output-not-settled-needs-observe 2/3
SkillGym settle-diff-is-observation, sample-output-settled-diff-next-target, sample-output-not-settled-needs-observe, sample-output-private-ax-recovery-continues @ claude-haiku	4/4 pass

That last row closes the second half of the earlier review comment — the four focused SkillGym cases are green with Haiku.

Environment note: the SkillGym claude-code runner needs IS_SANDBOX=1 when the container runs as root, since it passes --dangerously-skip-permissions and the CLI refuses that as root.

Findings

1. Positional arity — confirmed, and the schema already carries the fix

Direct probe of validateAgentDeviceCommand:

open com.example.community close      -> valid
press @e12 --settle extra junk        -> valid
close a b c                           -> valid
fill 'label="Q"' text --settle boom   -> valid

The false-green reproduces end to end: a five-command plan whose only close is open's swallowed second positional scores validPlanCommands: true and opensAndCloses: true.

cliSchema already carries allowsExtraPositionals, and open/close do not set it. Only click, press, fill, longpress, type, is, find, gesture, wait, clipboard, replay, test, logs, cdp, react-devtools do — where extra tokens are legitimately reassembled into multi-token selectors. So the rule is precise rather than a blanket cap:

const schema = getCliCommandOverride(command);
if (!schema?.allowsExtraPositionals &&
    positionals.length > (schema?.positionalArgs?.length ?? 0)) -> invalid

No command in the repo (53 total) uses a variadic ... positional, so positionalArgs.length is a sound bound. This also subsumes validateSnapshotPositionals — snapshot declares no positionalArgs, so the hand-written special case falls out for free and can be deleted.

2. Runner/API errors are scored as model failures

This bit the run above. The first settle-diff-is-observation FAIL (6/9, commands: []) was actually:

{"is_error": true, "result": "API Error: Unable to connect to API: Self-signed certificate detected..."}

runCaseRawOutput sets runnerError only when the process throws or stdout is empty, and normalizeParsedJson unwraps .result while ignoring is_error. Re-running gave 3/3 PASS.

This matters more after this PR than before it: validPlanCommands is now on every case and fails closed on empty commands, and the new summarizeResults taxonomy reports runnerErrors: 0 while inflating failedChecks.validPlanCommands with pure infrastructure noise — which defeats the purpose of the stability aggregate. Suggest treating is_error === true (and the codex equivalent) as a runnerError.

3. A real Haiku regression the reported 12/12 hides

sample-output-not-settled-needs-observe is 2/3 under the de-coached task. Trial 2 answered agent-device snapshot without -i, failing observesBeforeActing; trials 1 and 3 used -i. Worth reporting the --repeat number in Validation rather than a single-trial score — the feature this PR adds is exactly what surfaces it.

4. usesValidationPrep is position-locked

isPnpmScript(commands[0], 'build') && isPnpmScript(commands[1], 'clean:daemon') grades one exact opening rather than validity. It passed 4/4 for Haiku, but any plan inserting e.g. agent-device doctor first fails, and it can never be satisfied by the Android path the help text prescribes (build:android -> clean:daemon). Consider order-relative instead of index-based.

5. Related false-green the oracle still cannot see

The one genuine engineering-validate-mode miss was agent-device open "appname=Settings" — a selector expression where open expects an app name or bundle id. It validates because appOrUrl accepts any string. Not fixable by arity; noting it so the "production CLI grammar validation" line in the README is not read as stronger than it is.

Nits

• ROOT = new URL('..', import.meta.url).pathname in the new plan validator percent-encodes spaces and breaks on Windows; fileURLToPath is the fix. Consistent with the existing bench script, so pre-existing.
• printSummary computes validationIssues but never prints it (it is in the JSON).
• The 14-key selector list in AGENT_START_LINES is hand-duplicated from SELECTOR_KEY_NAMES. It is exactly correct today; a test asserting the help line matches that export would stop it drifting.

Verdict

Direction is good — the tokenizer plus shell-projection guard is solid and well tested, the fail-fast case-definition asserts are a nice touch, de-coaching the quiz prompts is clearly right, and --repeat proved its worth immediately by catching finding 3. Findings 1 and 2 are the ones worth blocking on, since both let the benchmark report green where it should not.

---

Generated by Claude Code

[thymikee]

Addressed in 983249c91:

• Positional bounds now come from the production command schema for commands that do not allow extra positionals; the snapshot special case is gone. Lifecycle scoring also requires distinct top-level agent-device open and agent-device close commands, so a close token swallowed by open cannot produce a false green.
• Claude/Codex error envelopes and empty output are classified as runner errors. They are reported separately and excluded from model pass-rate and failure-taxonomy denominators.
• Validation prep is order-relative, accepts intervening checks, and recognizes both build and build:android.
• Both benchmark entry points use fileURLToPath; terminal summaries print validation issues; selector-help vocabulary is drift-tested against SELECTOR_KEY_NAMES.
• The SkillGym README now states the parser oracle’s semantic boundary for free-form open <appOrUrl> values.

Regression coverage is 43/43 focused tests. Formatting, typecheck, lint, tooling/build, and fallow are green on the rebased head. The PR validation section now reports the independently verified SkillGym 4/4 result and the Haiku repeat variance (including the 2/3 not-settled case) instead of the earlier opaque 12/12 headline.

[thymikee]

Re-reviewed at 983249c. All seven items from my previous comment are addressed. CI is green (25/25), and locally pnpm lint, pnpm format:check, and the focused suite (199/199, up from 196) all pass.

Item	Status
1. Positional arity	Addressed (two layers — see note below)
2. Runner errors scored as model failures	Addressed
3. Reported 12/12 hid variance	Addressed in PR body
4. usesValidationPrep position-locked	Addressed
5. open <appOrUrl> semantic ceiling	Documented in README + PR body
Nit: ROOT via .pathname	Fixed (fileURLToPath, both modules)
Nit: printSummary dropped validationIssues	Fixed
Nit: selector-key drift	Fixed (asserts against SELECTOR_KEY_NAMES)

One precision note on finding 1

validatePositionalArity is exactly the right shape, and dropping validateSnapshotPositionals for it is a clean generalization. Verified:

close a b c              -> rejected (at most 1, received 3)
snapshot -i ./ev.json    -> rejected (at most 0, received 1)
press role=button label="Submit" --settle  -> still valid (allowsExtraPositionals)

But the literal command from the original review comment still validates:

agent-device open com.example.community close  -> valid

open declares ['appOrUrl?', 'url?'], so two positionals are within bounds — arity can't catch this one. What actually closes the false-green is the opensAndCloses rewrite, and it does close it. Verified end to end on the head:

opensAndCloses(['agent-device open com.example.community close', ...])  -> false

So the benchmark claim is sound now; I just want the record to be accurate about which change does the work, since a future reader might otherwise assume arity covers it. The residual — a syntactically valid open whose second positional is meaningless — is the free-form-value boundary you documented, and I agree that's the right place to leave it.

Finding 2 verified too: an all-error group now yields evaluatedTrials: 0, runnerErrors: 2, passRate: 0 instead of silently counting as model failures.

New Haiku run on this head

Full nine-case pass: 7/9. Three-trial repeats on the two failures:

dogfood-mode:             1/3 (33%); failed capturesStrongIssueEvidence=2, opensKnownDogfoodApp=1
engineering-validate-mode: 3/3 (100%)

engineering-validate-mode is now stable — the earlier opensSettings miss was variance, and the usesValidationPrep rewrite holds across all three trials. The richer summary line is doing real work here; that taxonomy is what isolated the next item.

New finding: case matchers run on raw text, not tokens

dogfood-mode trial 3 differs from trials 1 and 2 by exactly one character pair:

trial 1, 2:  agent-device open com.example.shop --platform ios    -> opensKnownDogfoodApp passes
trial 3:     agent-device open "com.example.shop" --platform ios  -> opensKnownDogfoodApp FAILS

opensKnownDogfoodApp is /\bagent-device\s+open\s+com\.example\.shop\b/i, so legitimate shell quoting breaks it. This is the false-negative twin of the false-positive class you just fixed: the plan validator tokenizes properly and strips quotes, but resolveChecks matchers still regex the raw string. opensSettings and fillsExpectedSearch have the same exposure.

Since validatePlanCommands already returns tokens per command, matchers could score against those instead of raw text — that would make quoting irrelevant and remove a chunk of noise from the --repeat aggregates. The other 2/3 capturesStrongIssueEvidence failures look like genuine Haiku variance (plain screenshots instead of --overlay-refs / record start), not a harness artifact.

Nit: detectRunnerError can misfire on model-authored JSON

isErrorPayload accepts status === 'failed' and type === 'error', and for codex runCodex returns the assistant message rather than an API envelope. So a model answer like {"status": "failed", "commands": [...]} is classified as infrastructure error and dropped from scoring entirely — the mirror image of finding 2. Cheap guard: skip envelope detection when the payload carries a commands array. Unlikely in practice, but it fails silently, which is the same property that made finding 2 hard to notice.

Neither of these is blocking. The two things I flagged as blockers are genuinely resolved.

---

Generated by Claude Code

[thymikee]

Not ready; two P2 validation bypasses remain:

1. scripts/help-conformance-plan-validator.mjs treats unquoted \r/\n as ordinary whitespace. A plan like agent-device press @e1 --settle\ncat is accepted as one token stream even though a shell executes two commands. Reject unquoted newlines as shell-projection and add regression coverage (quoted newlines may retain their current semantics).

2. requireLocalCliHelp does not prove help was consumed. assertLocalCliHelpUsed() calls isLocalCliHelpCommand(), whose segment predicate accepts bare cat and printf; therefore a run containing only cat satisfies the requirement. Separate “allowed wrapper segment” from “actual agent-device local help probe,” require at least one real help segment, and add a bypass regression.

The rest of the scope and production trace is coherent; all current CI is green. No ready-for-human label applied.

[thymikee]

Addressed every actionable item from the latest reviews in 7a918915b:

• Unquoted line-break projection: LF, CR, and escaped LF now fail as shell-projection before command grammar is evaluated. Quoted multiline values retain their existing token semantics. Regression coverage includes all four shapes.
• Local-help proof bypass: the SkillGym policy now separates an allowed wrapper segment (cat/printf) from an actual agent-device local-help probe. Bare cat remains permitted where composition requires it but cannot satisfy requireLocalCliHelp; help mixed with a forbidden runtime segment also cannot pass.
• Raw-text matcher noise: positive and forbidden case patterns now score a canonical plan reconstructed from parser tokens, so open com.example.shop and open "com.example.shop" are equivalent. The exact dogfood app, Settings, and multi-word fill cases are regression-tested.
• Runner-envelope false positive: a model-authored payload carrying a commands array remains a model result even when it uses fields such as status: "failed"; it is no longer discarded as infrastructure failure.

The scorer and local-help policy were extracted into focused modules while changing them, reducing the benchmark entry point from 846 to 743 lines and the SkillGym suite from 2,807 to 2,743 lines.

Validation:

• 24/24 deterministic review regressions.
• Formatting, typecheck, lint, tooling/build, and fallow pass.
• All four affected SkillGym cases pass on both configured runners: 8/8 executions across GPT-5.4-mini and Claude Haiku.
• The broad local unit run showed unrelated host-contention failures; representative failures passed in isolation, including the final Apple timeout at 738 ms. CI is running on the pushed head.

[thymikee]

Re-reviewed 7a91891. Both prior bypasses are fixed: unquoted/escaped CR/LF are rejected while quoted newlines remain valid, and requireLocalCliHelp now requires a genuine agent-device help probe rather than bare cat/printf. Runner-envelope separation is also corrected and all checks are green.

One P2 scorer regression remains: help-conformance-case-checks.mjs canonicalizes parsed tokens by dropping quotes from tokens without whitespace. A valid plan such as agent-device press label="@react.dev" --settle becomes label=@react.dev, so the existing usesLiteralHandleSelector matcher—which requires quoted selector text—now falsely fails. Make selector matchers/canonicalization quote-insensitive without reintroducing raw-text noise, and add this exact regression case.

No ready-for-human label applied.

[thymikee]

Addressed the remaining P2 on the rebased head in a93e474.

• Rebased cleanly onto origin/main at 56b72c5.
• Made usesLiteralHandleSelector match the parsed token's canonical semantic form rather than requiring shell quote characters that tokenization intentionally removes.
• Added the exact regression plan from review: agent-device press label="@react.dev" --settle.
• Verified the semantic matcher remains true after parsing/canonicalization.

Validation is green: format, typecheck, lint, tooling/build checks, 17-file fallow audit, and the focused benchmark/policy suites (24/24).

[thymikee]

Re-reviewed a93e4741f after the rebase onto 56b72c5c. The first three commits are patch-equivalent to the previously reviewed series; the new commit correctly fixes quote-sensitive selector scoring after tokenization and adds the exact label="@react.dev" regression plan. No production device route changed, so device evidence is not required. The PR is clean/mergeable and all current checks are green. Ready for human merge.