fix(server): "Edit from here" works on a message after a stopped run - #13883
juliusmarminge wants to merge 2 commits into
Conversation
Only completed runs captured a checkpoint, so a rollback targeting a stopped run's ordinal failed client-side with "Rollback target run ordinal N is unavailable". Interrupted and cancelled runs now capture one too, keeping their terminal status. Rollback also marks stopped and failed runs after the target as rolled back, since the provider drops them from its conversation. Adds a live-recorded Codex 0.156.1 fixture: turn 1, turn 2 stopped mid-tool, turn 3, rollback to turn 2, turn 4. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| completedAt: capturedAt, | ||
| checkpointScopeId: scope.id, | ||
| }, | ||
| payload: stopped |
There was a problem hiding this comment.
🟠 High orchestration-v2/CheckpointCaptureService.ts:200
A concurrent rollback is overwritten here: this stale run payload changes the run from rolled_back back to interrupted and commits a checkpoint the rollback did not see, making the discarded turn available again while the workspace remains restored. Because rollback can run after the initial read and before commitCommand, capture must conditionally commit only if the run is still the same stopped version (or otherwise serialize/recheck against rollback).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration-v2/CheckpointCaptureService.ts around line 200:
A concurrent rollback is overwritten here: this stale `run` payload changes the run from `rolled_back` back to `interrupted` and commits a checkpoint the rollback did not see, making the discarded turn available again while the workspace remains restored. Because rollback can run after the initial read and before `commitCommand`, capture must conditionally commit only if the run is still the same stopped version (or otherwise serialize/recheck against rollback).
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: unavailable · PR result: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This is a focused server bug fix that preserves stopped-run state while making existing rollback behavior work correctly, with targeted replay coverage and no schema, default, security, or deployment changes. A concurrency correctness risk and missing cancelled-path coverage remain noted in the review findings. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
Stopped runs now capture a checkpoint, so a capture can find its run already rolled back: rollback and capture share the thread's effect lane, and a rollback can commit while a failed capture waits out its retry, or ahead of a capture a restart requeued. Committing then would write the run back to interrupted with a checkpoint the rollback never saw, bringing the discarded turn back. Capture now settles without committing when the run is already rolled back. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| yield* projections.getCheckpointCaptureContext(input.threadId, input); | ||
| // A stopped run is already terminal. Its checkpoint is the rollback point | ||
| // for the message after it, so capture leaves its status alone. | ||
| const stopped = run?.status === "interrupted" || run?.status === "cancelled"; |
There was a problem hiding this comment.
The new cancelled capture path has no focused test: the stopped-turn replay covers interrupted, while the cancelled replay covers a queued run that never reaches capture. Could you add a test using the real capture service with test layers that starts from a running turn finalized as cancelled and verifies its checkpoint is recorded without completing the run or root node?
Posted via Macroscope — Effect Service Conventions
"Edit from here" on a message that follows an interrupted or cancelled run failed client-side with "Rollback target run ordinal N is unavailable". Only completed runs captured a checkpoint (
appRunOrdinal), so the client had nothing to target for the stopped run.What changed
RunExecutionServicenow enqueues thecheckpoint.captureeffect forinterruptedandcancelledterminals as well ascompleted.CheckpointCaptureServiceaccepts a stopped run. It writes the checkpoint and itscheckpointIdbut keeps the run's terminal status (nocompletedflip, no node update). The capture is enqueued together with the terminal events, so it runs on the thread's effect lane before any later run's start. The workspace is therefore still in the stopped run's state when it is captured.CheckpointRollbackServicecounted onlycompletedruns after the target asrolled_back. Stopped and failed runs after the target are dropped from the provider conversation too, so they now leave the transcript as well. The existing Pithread_rollback_after_stopfixture had silently left the stopped turn 2 visible after rolling back to turn 1. Its assertions now cover that.Failed runs still don't capture. The maintainer decision was about stopped runs, and a failed run's workspace state is less trustworthy.
Providers
Rollback targets are now provider turns that may be partial. Here is what I checked for each adapter:
thread/revert. Interrupted turns keep a strong native turn id. The live recording confirms Codex keeps the stopped turn and drops only the turn after it.CommandPolicy.Verification
thread_rollback_to_stopped_turn, recorded live against Codex 0.156.1 (gpt-6-luna) with a new recorder scenario. Steps: turn 1; turn 2 stopped mid-tool; turn 3; rollback to turn 2's checkpoint; turn 4. The recall in turn 4 includes turns 1 and 2 and excludes turn 3.expected 3 to be at least 4provider turns, run 4failed).vp test runpassed 215/215 acrossOrchestratorReplayFixtures.integration,CodexReplayFixtures.integration,OrchestratorReplayFixtures.contract,CheckpointCaptureService,CheckpointRollbackService,RunExecutionService,runtimeLayer, andProviderRuntimeRecoveryService.ThreadFork.integration,ThreadMergeBack.integration,OrchestratorReplayRecovery.integration, andEffectWorker.tsc --noEmit -p apps/serveris clean.vp linton the touched files is clean. knip reports nothing new.Model: Claude Opus 5.5 (Claude Code)
🤖 Generated with Claude Code