Keep malformed file edits from aborting agent turns

[deregtd]

User scenario

While creating or editing an app, the model makes an invalid file-tool call—for example, writeFile with an empty filename. That mistake should return a tool error the model can correct, not terminate the whole turn when the step is saved.

Previously the malformed change could enter the in-memory edit buffer and fail schema validation only at the persistence boundary. By then the normal tool-error recovery opportunity had passed.

What changes

Validate each file change before it mutates the step buffer. An invalid call is recorded as a tool error; the agent can retry with a corrected write and continue. Existing persistence-boundary validation remains as a final safeguard.

Read knowledge is settled against a gadget's pin only when an edit successfully buffers. Rejected writes, unmatched edits, and no-op edits leave committed read stamps intact, so the next valid edit can proceed without an unnecessary re-read.

Rejected changes emit bounded stage/count diagnostics without file paths or contents. Existing size limits and OT representation are unchanged; the oversized-blueprint test now expects rejection at the earlier validation boundary.

Boundary and dependencies

Targets main independently. The validation runs in the kernel because that is where agent file tools buffer and commit their edits.

This PR no longer contains the persistence queue, shared file/replica permission-helper extraction, or inference/persistence timing instrumentation. Replica concurrency belongs to #774; timing-only diagnostics are in #776.

Verification

177 tests across the edit-validation, blueprint, chat-change, worktree, and worktree-session suites passed. Regression cases cover a rejected write, an unmatched edit, and a no-op edit followed by successful edits to existing committed files in the same agent step. All three cases reproduced the read-stamp failure before the fix. Full lint, generated-config checks, and workspace build/type checks passed.

Extracted from #729. No container capability, integration grant, scheduling change, or OT algebra change.

[ask-bonk]

LGTM!

github run

[github-actions]

Preview: pr772-ddr-split-age-2b3dffd9

https://pr772-ddr-split-age-2b3dffd9-router.cloudflare-os-previews.workers.dev

Dashboard · deleted when this PR closes

[ask-bonk]

LGTM!

github run

[ask-bonk]

Review: 1 findings.

Posted one P2 inline comment, verified with a focused reproduction using the PR’s tool implementations.

github run

[ask-bonk]

LGTM!

github run

[github-actions]

Eval results

Verdict: ⚪ Unchanged. No task moved beyond what 10 runs can tell apart from noise.

Task	Score	Δ score	Fisher test	Cache hits	Avg min	Avg steps
change-calendar	90% → 80%	−10 pp	p = 1.00	95% → 94% −1 pp	3.6 → 4.6	26.6 → 24.1
chess	90% → 100%	+10 pp	p = 1.00	98% 0 pp	10.4 → 12.6	65.4 → 60.9
incident-desk	90%	0 pp	p = 1.00	96% → 95% −1 pp	4.5 → 5.1	31.6 → 27.0
worker-logs	80% → 90%	+10 pp	p = 1.00	96% → 95% −1 pp	4.2 → 4.9	26.6 → 26.3

Failed checks

Task	Check	Failed
change-calendar	t4 earliest-available-applies-every-rule	1/10 → 0/10
change-calendar	t4 earliest-available-matches-the-reference	1/10 → 0/10
change-calendar	t4 earliest-available-slots-are-bookable	1/10 → 0/10
change-calendar	t4 names-the-booked-windows-the-new-rules-reject	0/10 → 2/10
chess	t3 detects-insufficient-material-after-a-capture	1/10 → 0/10
chess	t3 judges-material-and-terminal-positions-like-the-oracle	1/10 → 0/10
incident-desk	t1 opens-acknowledges-and-resolves-in-order	1/10 → 0/10
incident-desk	t2 existing-incidents-survive-and-race-still-holds	0/9 → 1/10
incident-desk	t2 escalation-moves-toward-severity-one-and-stops	0/9 → 1/10
worker-logs	t1 hourly-buckets-cover-every-hour-including-empty-ones	2/10 → 1/10
worker-logs	t1 ranges-are-half-open-and-filter-by-worker	2/10 → 1/10
worker-logs	t1 ingests-resets-and-summarises-per-worker	1/10

Run · trajectories and raw results

[ask-bonk]

🔬 Eval runs review

Performance

Against main, pass rates were change-calendar 90% → 80%, chess 90% → 100%, incident-desk 90% → 90%, and worker-logs 80% → 90%; comparison.json judges all changes within noise. Candidate runs used fewer model steps and lower mean costs on every task, including incident-desk $0.0258 → $0.0211, but took 14–28% longer; cache hits stayed around 94–98%, with no additional prefix misses behind incident-desk’s significant break-rate increase. Incorrect calendar audits lost the most candidate runs (2/10), while chess remained the largest time/step expense and its unmatched-edit errors rose from 3 to 8.

⚪ VERDICT: NO REGRESSION FROM THIS PR

Pass-rate differences remain within noise, and neither side’s trajectories connect the failures or performance shifts to the diff’s earlier transform validation and deferred read-stamp settlement.

Triage

Failure modes

• Incorrect spacing audit · change-calendar 2/10 · model error · this PR: no — Turn 4, trials 3 and 6: after executeCode fetched all bookings, the final reply incorrectly added mw-101 TOO_CLOSE and mw-103 TOO_CLOSE, despite their 45-hour end-to-start gap. Their availability implementations passed every check; main’s failed run instead omitted allowed-start-hour filtering in earliestAvailable.
• Deleting records mistaken for test fixtures · incident-desk 1/10 · model error · this PR: no — Turn 2, trial 9: executeCode showed seven existing incidents, then the agent added and invoked cleanupVerificationFixtures() to delete them. It had created none of those records and was explicitly told everything already on the board must stay; deletion also caused the later escalation check’s null-record failure. Main’s failure was different: its unknown-incident lookup used .one() and threw.
• Oversized SQL binding batches · worker-logs 1/10 · system prompt · this PR: no — Turn 1, trial 5: writeFile(server.js) implemented ingestion with 100 six-parameter rows per statement, assuming that stayed below SQLite’s limit. Bulk ingestion failed with “too many SQL variables,” leaving subsequent queries empty; main trial 2 used the same batching and failed identically. The unchanged system prompt advertises SQLite storage without explaining its binding limits.

Tool errors

• editFile: No matching text was found in the file. · change-calendar 4 → 0, chess 3 → 8, incident-desk 2 → 0, worker-logs 2 → 3 · model error — Agents supplied text with incorrect escaping or whitespace. Candidate chess trial 5, turn 2 repeated a rejected full-function patch before narrowing it; main calendar trial 6, turn 3 recovered by rereading and matching the actual whitespace.
• editFile: Validation failed for tool "editFile" · change-calendar 2 → 1, chess 1 → 3, incident-desk 6 → 2, worker-logs 2 → 1 · model error — Calls omitted required filename or replacement, or supplied .replace instead. Candidate incident-desk trial 2, turn 2 corrected the missing filename on its next step; main trial 2 made five such malformed calls together. These are existing argument-validation errors, not the PR’s new transform-validation boundary.
• editFile: Multiple matches were found. The text to match must be unique. · change-calendar 1 → 2, chess 4 → 3, incident-desk 2 → 1, worker-logs 3 → 1 · model error — Agents chose snippets repeated elsewhere despite the parameter’s uniqueness requirement. Candidate calendar trial 4, turn 1 matched several ORDER BY clauses and recovered with longer surrounding SQL.
• readFile: File does not exist. · change-calendar 0 → 4, incident-desk 0 → 2 · model error — Agents tried reading server.js and client.js immediately after creating a gadget without a blueprint, although the prompt and creation description say such gadgets are empty. They subsequently wrote the files.
• executeCode: This step already made code changes… · change-calendar 0 → 1, worker-logs 0 → 1 · tool design — Candidate calendar trial 1, turn 4 and worker-logs trial 4, turn 2 combined edits and execution in one step. The unchanged tool description omits this restriction; the error explains it, and both calls succeeded when retried on the next step.

Prompt cache

• incident-desk · cache hits 96.14% → 95.16% · breaks 0.00953% → 0.01163% · this PR: no — Break-rate p = 0.0052, but no prefix-loss step stands out: both sides consistently report three uncached tokens. At trial 1’s first turn-2 readFile step, main retained 18,241 cache-read tokens and PR retained 17,760; the appended escalation request and differing turn-1 transcripts account for different prompt sizes, not a new cache reset. Mean model steps also fell 31.6 → 27.0, so the rate change is not evidence of worse caching.

What to do

• Optional: add packages/workshop-evals/evals/edit-recovery.eval.ts to exercise an empty-filename write followed by a corrected write and an edit of previously read committed code. These runs never hit the new transform-validation boundary, so they do not demonstrate the intended recovery improvement.
• Optional, unrelated to this PR: in packages/workshop-backend/src/agent.ts, add to EXECUTE_CODE_TOOL_DESCRIPTION: “After making file changes, call executeCode in your next model step, not in the same tool-call batch.”
• Optional, unrelated to this PR: add Workers-specific SQL binding-limit guidance to SYSTEM_PROMPT in packages/workshop-backend/src/agent.ts, recommending fixed-size insert statements rather than assuming ordinary SQLite batch limits.

github run