fix(batch): spec honesty at every boundary and attribution visibility

[joctaTorres]

Summary

Stacked on #95. Closes all 8 findings of the Fable review of #95 (verdict: request-changes), driven end-to-end by the review-fixes ratchet batch (2 phases, 6 changes, both proof-of-work gates green) — with per-stage models dogfooded mid-batch (propose: claude:opus, apply: opencode, verify: claude:sonnet).

Phase 1 — spec honesty at every boundary (findings 2, 3, 4, 7, 8)

• Parser (spec-parser-hygiene): parseAgentSpec rejects whitespace-bearing agent/model parts ("claude: opus", "claude :m") and models starting with -, naming the offending value. Reject, never auto-trim.
• Write path (config-write-load-honesty): batch config set agent <value> validates through the shared AgentSettingSchema — the loader's own authority — and refuses to persist what the loader rejects. Inline {apply: opencode, ...} values persist as real YAML maps (closing a pre-existing quoted-string trap).
• Load path: readProjectConfig validates the batch section per key: warnings name batch.<key>[.<stage>] and the offending value (derived from the zod issues; authToken never echoed), and valid siblings are preserved — no more silently reverting gate/locus/permissions behind the generic gate/strategy/proofOfWork text. The e2e now drives the real readProjectConfig/setProjectBatchSetting seams instead of a hand-rolled YAML re-parse.
• CLI flag (standalone-agent-flag-error): malformed --agent fails before any spawn with an actionable error naming the value; the residual skill-locus parse seam rethrows SkillLocusError so even a slipped-through spec maps to a structured failed step, never a raw throw.

Phase 2 — attribution visibility (findings 1, 5, 6)

• Rendered surfaces (attribution-rendered-surfaces): the stage/agent/model/scope hint threads into blocker/message in the mapper's single attribution branch, so the non-JSON blocked line, parked reason on resume, journal entry, and standalone renderer all carry it — zero renderer edits. Display is asserted at the rendered surfaces (stdout/park/journal), and bare-name/progressed/scope-less shapes are pinned byte-for-byte.
• pr/decompose coverage (pr-decompose-stage-attribution): agentStageScopes reaches the batch-driven PR and decompose spawns (uniformAgentScope handles decompose's scalar-covered case), so agent: {pr: X:bad} fast-failures get the same attributed surface.
• Signal-kill gate (signal-kill-hint-gate): the hint requires a real non-zero exit code — a timeout SIGKILL under a valid model is never told its model id may be invalid.

Tests

Full suite green: 203 files / 2879 tests, coverage 95.54% over the enforced 95% gate. Phase proofs: agent-model-selection.test.ts (now through the real load/write/flag seams) and model-failure-attribution.test.ts (33 tests incl. rendered-surface assertions).

Also removes a stale tracked ratchet-rex-*.done runtime artifact dating back to the eval commits.

🤖 Generated with Claude Code

https://claude.ai/code/session_019S4t1YHtiCS4H35XtyEC1b

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	ratchet	09e3c38	Commit Preview URL Branch Preview URL	Jul 06 2026, 11:05 PM

[joctaTorres]

Verdict: Request changes — 8/8 prior findings genuinely closed, but the new per-key config loader introduces one confirmed (Low) regression

All eight findings from the PR #95 review are closed by real code with tests that pin the claimed behavior at the actual seams (renderers, park store, journal, write/load paths — not just StepResult fields). Full suite passes (2879/2879) and tsc --noEmit is clean on the branch. However, the finding-2 fix (parseBatchPerKey) has a prototype-chain key bug that regresses previously-passing configs, verified by an end-to-end reproduction. One trivial fix and this is an approve.

Per-finding closure

#	Finding	Closed?	Evidence
1	Hint never reached rendered surfaces	✅	src/core/batch/engine/outcome.ts:200-210 threads the hint into blocker/message inside the attribution branch only. Tests exercise the real renderer (renderStepResult), the parked reason, the journal entry message, and the JSON surface (test/batch-engine/model-failure-attribution.test.ts, "rendered surfaces carry the attribution hint"). Non-attributed shapes are pinned byte-for-byte via deep-equal against a no-attribution baseline.
2	Whole batch section dropped on any invalid key	⚠️ mostly	parseBatchPerKey (src/core/project-config.ts:277-312) warns per key naming batch.<key>[.<path>] + JSON-stringified value, redacts authToken, preserves valid siblings — all pinned in test/core/project-config.test.ts ("per-key batch load (resilient)") and through the real readProjectConfig seam in agent-model-selection.test.ts. The batch schema has no cross-key refinements (validateRemoteSettings runs at engine time), so per-key parsing loses no load-time constraint. But see New Finding 1 below.
3	config set agent "claude:" persisted what the loader rejects	✅	validateSetting routes agent through the shared AgentSettingSchema (src/core/batch/config.ts:807-843); {-prefixed values parse as YAML flow maps and persist as real maps via SetResult.parsedValue (setProjectBatchSetting, config.ts:975-981). Write refusal pinned: file byte-for-byte unchanged on reject (test/core/batch/config.test.ts), command-level rejection (test/commands/batch/config.test.ts), loader round-trips the persisted map with no warning.
4	Standalone --agent "claude:" crashed with raw throw	✅	resolveChangeStepSettings validates before any mutation and throws naming the value; apply/propose/verify command tests assert the injected spawner is never invoked. ensureCommandInSpawnLocus (skill-locus.ts:201-227) wraps parseAgentSpec into SkillLocusError (asserted via instanceof and .name), and engine-skill-locus.test.ts pins the engine mapping it to a structured blocked step with zero spawns — no raw escape.
5	No attribution for batch-driven pr/decompose spawns	✅	agentStageScopes threaded through runPr/runStackedPr/runDecomposition (src/commands/batch/apply.ts) into PrStepContext/DecompositionStepContext. PR uses scopes.pr; decompose uses uniformAgentScope gated on spec?.model !== undefined (engine.ts:503-507 / engine.ts:680-684). Attributed + unchanged shapes both pinned on every surface for both paths.
6	Hint fired on signal kills	✅	Gate spawn.signal === null at outcome.ts:200; AgentSpawnResult.signal is typed NodeJS.Signals | null (never undefined) so the gate can't mis-suppress. Signal-kill suppression pinned unit-level and engine-level on every surface, plus an exit-code regression guard that the hint still fires for code-2 failures.
7	Whitespace accepted in specs	✅	parseAgentSpec rejects leading/trailing whitespace in bare-agent, agent-part, and model-part, naming the full spec and the offending part (agent-setting.ts:85-113); previously-valid specs (opencode:zai/glm-5.2, codex:vendor:tagged-1) pinned byte-for-byte unchanged.
8	Leading-dash model rode into argv	✅	model.startsWith('-') rejected naming the value (agent-setting.ts:114-118), incl. claude:--dangerously-skip-permissions; interior dashes (codex:gpt-5.2-codex) stay valid.

New findings

1. [Low, confirmed — the request-changes item] parseBatchPerKey crashes on prototype-chain key names, nuking the ENTIRE config to null (regression vs. base).

src/core/project-config.ts:292:

if (!(key in shape)) continue; // unknown key: ignored (schema is partial)

The zod shape object inherits from Object.prototype, so in matches inherited properties. A batch section containing a key named constructor, toString, hasOwnProperty, valueOf, etc. passes the guard, shape[key] resolves to the inherited function, and fieldSchema.safeParse(value) throws TypeError: fieldSchema.safeParse is not a function. That escapes to readProjectConfig's outer catch (project-config.ts:255-258), which returns null — dropping not just the batch section but schema, context, rules, and eval too, with only a generic "Failed to parse" warning.

Reproduced end-to-end on this branch:

schema: ratchet
context: hello
batch:
  constructor: 1
  gate: after-propose

→ readProjectConfig(...) returns null; warning: Failed to parse .ratchet/config.yaml: TypeError: fieldSchema.safeParse is not a function.

On base (batch/model), this exact config loads fine: the whole-section safeParse strips the unknown key and keeps gate: after-propose. So the resilience fix regresses a previously-passing input into total config loss — the precise failure mode finding 2 set out to eliminate, amplified from one section to the whole file.

Fix is one line:

if (!Object.hasOwn(shape, key)) continue;
// or: if (!Object.prototype.hasOwnProperty.call(shape, key)) continue;

Please also add a test pinning that a prototype-named unknown batch key is ignored with siblings preserved.

2. [Nit, non-blocking] Vague error for non-string / unknown-stage map entries in validateAgentSetting. For --set 'agent={apply: 123}' or {deploy: x}, the union fails before the superRefine runs, so zod emits a single invalid_union issue at path [] with message Invalid input → Invalid value for 'agent': agent: Invalid input — neither the stage key nor the offending value is named (config.ts:826-840 doesn't unpack unionErrors). The common case (malformed spec string per stage) is named correctly. Consider unpacking unionErrors for the map branch.

3. [Nit, observation] The signal gate doesn't cover the sidecar's synthesized timeout. The ReX sidecar's internal timeout resolves { exitCode: exitCode ?? 1, signal: null } (rex-sidecar-runtime.ts:254, :295), so an in-sidecar timeout under an explicit model with zero journal entries still fires the hint. The detail does carry the ReX sidecar timed out after Xms stderr line, and an agent that hangs on an invalid model is a plausible cause, so this is arguably within the hint's "if this model id is invalid…" phrasing — but it's outside the "signal kill ⇒ model likely valid" rationale of finding 6. Fine to leave; noting for the record.

Checked and clean

• Full test suite on the branch: 2879/2879 pass; tsc --noEmit clean.
• YAML flow-map edge cases verified empirically against the repo's yaml version: {apply: claude:fable} parses the colon-bearing plain scalar correctly; provider/model slashes, quoted values, and multi-entry maps round-trip; unclosed braces, trailing garbage ({a: b} extra), and block-in-flow ({apply: claude: fable}) all throw and are converted to clean errors; nested braces ({apply: {agent: claude}}) are rejected by the schema (non-string stage value).
• uniformAgentScope correctness: a scalar agent setting always materializes a full uniform scope map, and any map-typed setting makes scalarAgent return undefined (no model ⇒ attribution gated off), so a coincidentally-uniform scope map can never mis-attribute the decompose spawn. Partial/mixed/empty/undefined maps all return undefined (pinned in agent-scope-resolution.test.ts).
• No blocker/message enrichment leaks into non-attributed branches: bare-name, progressed, scope-less, signal-kill, and zero-exit shapes deep-equal their no-attribution baselines.
• No double-hint on any single rendered line (blocked line prints blocker only; journal prints message only; JSON carries it in three fields by design).
• Empty inline map (agent={}) is accepted at write and by the loader alike (behaves as unset) — no write/load divergence.
• git status on the review checkout left clean; no tracked files modified.

🤖 Fable 5 review via Claude Code