The automatic restart guard forgets older crashes after five minutes, so an unfinished turn can keep restarting the server indefinitely. This preserves the existing three-recovery limit across unplanned restarts.
Persist the cumulative process-loss count on the existing continuation intent, forward it through the worker, and decline the fourth recovery. Planned shutdowns and provider-completed turns reset the allowance; live continuations retain the predecessor intent past outbox cleanup. Legacy intents retain their existing guard.
Compared with #24177, this omits environment-wide admission, child-first scheduling, parent-wake suppression, their migration, and policy-specific tests/docs. Those are separate policy decisions. The pruning lookup uses the existing thread/status index. No UI or client contract changes.
Addresses #24059. Maria explicitly requested this takeover and a smaller fix in the task brief. This keeps the existing three-recovery allowance and fixes its elapsed-time loophole; the environment-wide admission and parent-wake policies are excluded. The task authorization is not a public approval comment. Macroscope requests human approval of the remaining stateful recovery behavior before merge.
220eb73524ca). After: all 111 tests in the three focused suites passed.vp run check:changed passed format, lint, both knip checks, generated GraphQL, server/scripts typechecks, and 125 tests in five files.0475b3da8e.This bounds successive recovery runs. Crashes before a successor run commits, child-result parent wakes, host overload, and orphaned subprocesses remain outside scope. Historical crashes without a saved count are not reconstructed.
Replaces #24177, with thanks to @Zheruel for the original implementation.
Built with GPT-6.1-Sol in the Codex harness on T3 Code.
Added 1 commit
Verdict: Needs Human Approval
Macroscope's review found this PR requires human approval — The PR changes production crash-recovery policy across restarts, persisting a cumulative loss count and suppressing the fourth recovery of an unfinished turn. Because this gate determines whether an existing provider workflow resumes, the stateful behavior warrants human review.
You can add or adjust custom eligibility rules. Learn more.
✅ Thread transfer remains within every enforced ceiling.
ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful
mainmeasurement shown below.
| Provider | Metric | Main baseline | This PR | Impact | PR ceiling | |
|---|---|---|---|---|---|---|
| Codex | Total thread wire | 5.0 KiB | 5.0 KiB | +7 B (+0.1%) | 6.8 KiB | ✅ |
| Codex | Thread snapshot wire | 3.8 KiB | 3.8 KiB | 0 B (0.0%) | 4.9 KiB | ✅ |
| Codex | Live turn WebSocket wire | 1.2 KiB | 1.2 KiB | +7 B (+0.6%) | 2.0 KiB | ✅ |
| Codex | Live turn WebSocket decoded | 20.9 KiB | 20.9 KiB | 0 B (0.0%) | 29.3 KiB | ✅ |
| Codex | Live turn messages | 2 | 2 | 0 (0.0%) | 8 | ✅ |
| Claude | Total thread wire | 5.0 KiB | 5.0 KiB | 0 B (0.0%) | 6.8 KiB | ✅ |
| Claude | Thread snapshot wire | 3.8 KiB | 3.8 KiB | 0 B (0.0%) | 4.9 KiB | ✅ |
| Claude | Live turn WebSocket wire | 1.2 KiB | 1.2 KiB | 0 B (0.0%) | 2.0 KiB | ✅ |
| Claude | Live turn WebSocket decoded | 21.2 KiB | 21.2 KiB | 0 B (0.0%) | 29.3 KiB | ✅ |
| Claude | Live turn messages | 2 | 2 | 0 (0.0%) | 8 | ✅ |
Baseline: 4e42eaf · PR result: 0475b3d · Source CI: success
10 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.
No actionable comments were generated in the recent review.
b4602c22-d264-4557-870e-febf043a3d57Reviewing files that changed from the base of the PR and between 220eb73524ca7882d53b6a12c824ccbcd97ad279 and 0475b3da8e27d0e8667f8955978642c6742201f3.
apps/server/src/orchestration-v2/EffectOutbox.tsapps/server/src/orchestration-v2/EffectWorker.test.tsapps/server/src/orchestration-v2/EffectWorker.tsapps/server/src/orchestration-v2/FoundationPersistence.test.tsapps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.tsapps/server/src/orchestration-v2/RestartContinuation.test.tsapps/server/src/orchestration-v2/RestartContinuation.tsdocs/internals/server-updates.mddocs/user/updating.mdIncluded review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
Restart continuations now carry a cumulative process-loss count. Recovery increments or resets the count based on run state, and delivery applies a count-based limit. Outbox pruning retains continuation effects while matching runs remain active. Tests and documentation cover these rules.
Changes: Restart continuation recovery
| Layer / File(s) | Summary |
|---|---|
Continuation intent and retention apps/server/src/orchestration-v2/EffectOutbox.ts, apps/server/src/orchestration-v2/FoundationPersistence.test.ts | Continuation requests can include a nonnegative processLossCount. Pruning retains settled continuation effects while a matching run is queued, preparing, starting, running, or waiting. |
Recovery count and delivery guard apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.ts, apps/server/src/orchestration-v2/EffectWorker.ts, apps/server/src/orchestration-v2/RestartContinuation.ts, apps/server/src/orchestration-v2/*test.ts, docs/internals/server-updates.md, docs/user/updating.md | Recovery increments the count for unfinished work and resets it for completed or waiting work. Shutdown continuations use zero. The worker passes the count to continueRestartedRun, which applies the count-based limit; legacy requests without a count retain the chain-length check. |
Priority: External image Normal
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix
| 1 | sequenceDiagram |
| 2 | participant ProviderRuntimeRecoveryService |
| 3 | participant EffectOutbox |
| 4 | participant EffectWorker |
| 5 | participant RestartContinuation |
| 6 | ProviderRuntimeRecoveryService->>EffectOutbox: Read prior continuation effect |
| 7 | EffectOutbox-->>ProviderRuntimeRecoveryService: Return retained processLossCount |
| 8 | ProviderRuntimeRecoveryService->>EffectWorker: Submit continuation with updated count |
| 9 | EffectWorker->>RestartContinuation: Call continueRestartedRun with count |
| 10 | RestartContinuation-->>EffectWorker: Apply continuation limit |
Possibly related PRs
processLossCount and update restart-continuation recovery behavior.Suggested reviewers: t3dotgg, yash-singh1
0475bThe restart-recovery limit has no identified merge-blocking issue; merge after normal checks.
Failed Failed checks: 1 warning
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | Warning | The description clearly covers the problem, change, scope, exclusions, verification results, limitations, and tooling. The Scope and approval section links issue #24059, but it does not provide the re… | Add a link to the maintainer approval comment that explicitly approves the direction and scope. If no prior approval exists, explain why this focused fix qualifies for the exception and provide the required maintainer approval before merge. |
| Check name | Status | Explanation |
|---|---|---|
| Title check | Passed | The title "fix(server): preserve the crash recovery budget across restarts" follows conventional commit format and matches the main change. The PR persists a cumulative process-loss count on the conti… |
| Linked Issues check | Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | Passed | Check skipped because no linked issues were found for this pull request. |
Explanation
The title "fix(server): preserve the crash recovery budget across restarts" follows conventional commit format and matches the main change. The PR persists a cumulative process-loss count on the continuation intent so the three-recovery limit survives unplanned restarts, and declines the fourth recovery. The title is specific and concise, and it names the server scope.
Explanation
The description clearly covers the problem, change, scope, exclusions, verification results, limitations, and tooling. The Scope and approval section links issue #24059, but it does not provide the required public maintainer approval comment; it explicitly states that the task authorization is not public approval.
Resolution
Add a link to the maintainer approval comment that explicitly approves the direction and scope. If no prior approval exists, explain why this focused fix qualifies for the exception and provide the required maintainer approval before merge.
Comment @coderabbitai help to get the list of available commands.
Note
Written by gpt-6.1-sol on behalf of Maria
verified as-is at 0475b3da8e27d0e8667f8955978642c6742201f3 on a linux boat vm. no code changes.
github/main (48a292364a) in a separate worktree. after three automatic recoveries, waited over 326 seconds before the fourth crash. main incorrectly started another real codex run./usr/bin/google-chrome, a project added through the app pointing at a real git repo, and real codex gpt-6.1-sol turns. no mocks or database/timestamp injection.MANUAL_RESUME_OK.RESET_RECOVERY_COMPLETE.vp run check:changed against the pr base passed format, lint, both knip checks, generated graphql, affected typechecks, and 125 related tests. hosted checks: 28/28 green.main after the fourth crash, still running:
main incorrectly resumes a fourth time after the five-minute window
pr after the fourth crash, stopped after three recoveries:
pr stays idle after its fourth crash with only three automatic resumes
new user message after the cap, real terminal read, and provider completion (20 seconds):
manual continuation after the recovery cap completes through codex
command transcript, persisted counts, and verification limits
native desktop/iOS/Android, macOS/Windows, other providers, and remote/relay/tunnel connections were not exercised on this linux vm. week-long outbox retention and completed/waiting background-work recovery were covered by existing tests, not live waits.
an initial root launch selected the old runner's default home; it was stopped and excluded, then the pr scenario was recreated with an explicit isolated home. one recording attempt failed during vm cpu overload (a 19-second freeze notification); the replacement recording was inspected and its uploaded playback and seeking were verified in t3 preview. browser reloads recovered; no ten-minute provider/app stall occurred.