Repository navigation
Conversation
An agent run could apply a long sequence of edits, verify them by
rendering frames, and write a confident summary while none of the work
was durable. The server had no way to tell: the editor answered "success"
and the tool result carried no durability information, unlike the
external MCP path which does track baseRevision.
The editor now reports { revision, pending, failed } with each tool
result, derived from state SaveCoordinator already keeps, and the server
stores it on the run. executeBrowserTool then fails mutating tools when
the editor's writes are settled-failed, telling the agent to stop rather
than build further work on state that cannot be saved.
The distinctions matter as much as the check:
- reads never fail on a save error; the agent still has to read state in
order to report the problem
- a pending save is not a failure. Autosave debounces by 500ms and
normally settles after the tool returns, so treating pending as loss
would fail nearly every edit
- a missing signal is unknown, not failed, so an older editor keeps
working
Project writes are also flushed at terminal settle, including failed and
cancelled runs, which is exactly when the last edits used to be stranded
inside the autosave debounce window. A checkpoint flush every few tool
results bounds exposure further; neither is awaited during the run, so
tool throughput is unchanged.
editor-persistence.verify.ts pins the decision table.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`handleDraftStore` called `readJson(req)` with no limit, inheriting the 1 MiB default, while the artifact contract is 8 MiB everywhere else: the client guard in serverRunDraftStore, agentRuntimeSnapshotValidation, and storeAgentArtifact all use MAX_ARTIFACT_BYTES. Its sibling handleToolResult passes an explicit limit; the draft route was simply never given one. Any draft over 1 MiB was therefore built, accepted by the client's own size check, sent, and rejected at the door with 'request body too large' → HTTP 413, surfacing as 'Server run draft could not be persisted (HTTP 413)'. The envelope embeds the already-serialized artifact body as a JSON string, so re-escaping pushes even sub-1 MiB drafts over the line. The new limit leaves room for that re-escaping; storeAgentArtifact still recomputes the real size after parsing and rejects over-cap artifacts with 409, so the transport bound only keeps unbounded uploads out of memory. Covered by a routes.verify assertion tying the transport limit to MAX_ARTIFACT_BYTES; verified to fail against the previous code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
routes.ts crossed the 500-line source ceiling once the durability signal and the draft body limit landed (510 lines). Deferred admission is the most self-contained responsibility in the file: a timeout map, a started-run set and four small functions that decide whether a queued run may execute. It has no HTTP surface of its own — the handlers call into it — so moving it leaves routing, validation and response shaping behind in routes.ts. boundRun and handleDraftStore deliberately stay put: routes.verify.ts asserts against routes.ts source text for their exact status contracts. No behaviour change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SSKUltra
force-pushed
the
fix/agent-run-durability
branch
from
September 24, 2026 06:51
1d446e3 to
3fe4089
Compare
0xsline
pushed a commit
that referenced
this pull request
Sep 26, 2026
`handleDraftStore` called `readJson(req)` with no limit, inheriting the 1 MiB default, while the artifact contract is 8 MiB everywhere else: the client guard in serverRunDraftStore, agentRuntimeSnapshotValidation, and storeAgentArtifact all use MAX_ARTIFACT_BYTES. Its sibling handleToolResult passes an explicit limit; the draft route was simply never given one. Any draft over 1 MiB was therefore built, accepted by the client's own size check, sent, and rejected at the door with 'request body too large' → HTTP 413, surfacing as 'Server run draft could not be persisted (HTTP 413)'. The envelope embeds the already-serialized artifact body as a JSON string, so re-escaping pushes even sub-1 MiB drafts over the line. The new limit leaves room for that re-escaping; storeAgentArtifact still recomputes the real size after parsing and rejects over-cap artifacts with 409, so the transport bound only keeps unbounded uploads out of memory. Covered by a routes.verify assertion tying the transport limit to MAX_ARTIFACT_BYTES; verified to fail against the previous code. Closes #153. Cherry-picked from #164; the durability gate in that PR is still under review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 713778f)
0xsline
requested changes
Sep 26, 2026
0xsline
left a comment
0xsline
left a comment
Owner
There was a problem hiding this comment.
Thanks for this. The draft-size fix is exactly right: I've cherry-picked 713778f onto main with your authorship kept (9a858bf). That fixes the 413 in #153.
I'm holding back the durability gate (47830fa) for now, because review turned up some false failures:
- The gate checks a project-wide flag, not this call's save. It keys off "the last save failed" rather than whether this call's own edit was saved. With auto-apply off, the agent's edits go into the proposal and never trigger a project save. So one earlier failed autosave makes every non-read tool fail for the rest of the run, and the model is told the edit "was applied in the editor".
- Tools that never write the project get failed after they ran. Only read tools are exempt.
web_search,run_code,submit_export,install_skill/manage_skilland the paidgenerate_*tools are marked failed after they already ran. That invites a retry the tool policy forbids, and paid tools can bill twice. - Auto-apply can duplicate clips. In auto-apply mode a failed save leaves the edit queued, and it is re-applied once storage recovers. Reporting it as failed can lead the model to retry, which duplicates the clip (e.g.
add_audio). flush()doesn't force a save. It only awaits saves that are already queued, and never triggers the editor's 500 ms debounced save. So the settle flush and the 8-result checkpoint don't actually force anything to disk.
Points 1–3 would go away if the check used whether this call's edit was persisted, and only gated calls that actually produced project edits. I'd be glad to review that as a follow-up PR on top of current main; the deferred-admission refactor (3fe4089) is fine to carry along with it.
Owner
|
Main has moved on: #200, #203, #207, #209, #211 and #213 are merged now, plus a fix for the >500-line verify gate that was failing If you want, I can merge main into your branch and push the resolution back here (maintainer edits are enabled) so the diff is reviewable again — just say the word. The remaining review points from the earlier CHANGES_REQUESTED are still open on our side and we would like to see how they are addressed before merging. |
…ility Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # package.json
… failing the call Replaces the run-wide durability gate from 47830fa, which review showed produced false failures: - It keyed off the project's last save, not this call's. With auto-apply off, edits go into the proposal and never save, so one earlier failed autosave failed every non-read tool for the rest of the run. - Tools that never write the project (web_search, run_code, submit_export, skills, paid generate_* tools) were marked failed after they ran, inviting a retry the tool policy forbids and double billing. - In auto-apply a failed save keeps the edit queued and lands it on the next successful save; reporting it as failed led to retries that duplicated clips. - flushProjectSaves() only awaits queued saves and never triggers the debounced autosave, so the settle flush and checkpoint forced nothing. Landing already saves before it applies and keeps a failed batch queued on the turn, so persistToolAction now reports what happened to this call's own landing edits: none (reads, proposal-only edits, tools outside the project), saved, or queued. A queued call stays successful and its result carries projectSaveWarning, telling the model not to repeat it and to stop editing. The run-wide signal, server gate, checkpoint and settle flush are removed. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
Thanks @0xsline, and for cherry-picking the draft-size fix. I merged current main into the branch (31eeb8c) and reworked the durability gate along the lines you suggested (b0c6e3c). I've also updated the PR description. The key observation: on current main,
The server-side gate ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent agent-run defects, both reproducible on
main. The draft-size fix (2) already landed onmainas 9a858bf. This PR now carries the reworked durability signal (1) and the deferred-admission refactor (3fe4089).1. The agent wasn't told when its own edits could not be saved
When a live landing of an agent edit fails to save (auto-apply timeline edits or pool imports), the edit is not applied. It stays queued on the run and lands with the next successful save. The model was never told: it got a plain success and kept editing, while the user only saw an error row.
The browser now reports what happened to this call's own project edits:
none: the call landed nothing. This covers reads, proposal-only edits with auto-apply off, and tools that never touch the project (web_search,run_code,submit_export, skills, paidgenerate_*).saved: this call's edits landed. Landing saves before it applies, so a landed edit is on disk.queued: this call's edits are waiting because their save failed.A
queuedcall stays successful. Its result carries aprojectSaveWarningtelling the model not to repeat the call, since the queued edit would be applied twice, and to stop editing and tell the user to check storage. Nothing is marked failed, so there's no retry, no duplicated clips and no double billing.This replaces the earlier run-wide gate (47830fa), which keyed off the project's last save rather than the call's own. The periodic checkpoint and the settle-time flush are removed as well:
flushProjectSaves()only awaits saves already queued, and agent landings already save synchronously.2. Drafts over 1 MiB were rejected (HTTP 413)
Landed on
mainas 9a858bf (cherry-pick of 713778f).Verification
src/agent/serverRunProjectWrite.verify.ts(new) covers the per-call decision table: proposal-only edits and earlier failures never taint a call, and a saved pool import isn't blamed for timeline edits an earlier call left queued. It also checks that aqueuedresult is posted as a success carrying the warning, whilesaved/noneresults are untouched. The test fails if the executor stops annotating.npm run verify:server-runs, app typecheck, lint,verify-registrationandverify-gate-coveragepass.