Skip to content

fix(codex): preserve conversations when resume times out - #1457

Draft
baron wants to merge 2 commits into
mainfrom
fix/1299-codex-resume-timeout-fresh-thread
Draft

baron wants to merge 2 commits into
mainfrom
fix/1299-codex-resume-timeout-fresh-thread

Conversation

@baron

@baron baron commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fixes #1299.

A resume timeout must not authorize discarding a conversation. Keep the committed Codex conversation ID, rollout path, and cleanup handle, retire the timed-out process, and stop dispatch. The error explicitly offers two choices: resend the message to retry the same resume, or use Handoff to New Chat on an assistant message to carry the task into a new chat.

Root cause and fix

The coordinator previously started fresh on the second resume timeout, and separately skipped that same resume target on later attempts. Both timeout-driven fresh-thread paths are removed, including their routing-readiness classification. Each explicit retry still uses the original reference; there is no automatic timeout-to-fresh retry loop.

A typed resume_timed_out terminal reason is published for unattended agent_run/lane callers instead of the generic transient timeout. Startup disposition resets on the next attempt, so a different startup failure cannot inherit the reason. Send failures return the original actionable notice, and the terminal barrier uses the existing failure text for notifications rather than generating an empty/generic failure.

Relationship to #1443

This is independent of #1443's deterministic oversized-history handling. The frame-too-large and missing-rollout policies are intentionally unchanged here. The notice uses the same preservation/Handoff style, but timeout recovery also permits an explicit retry. The terminal-notification and reported-send-failure fixes overlap with #1443. This PR will need a rebase over #1443 if #1443 merges first, preserving both typed reasons and removing the now-obsolete timeout fallback from that branch.

Validation

  • Fail-first: the new terminal-barrier regression failed against the original implementation for the returned guidance, failure notification, and typed reason. Its automatic cold-build retry was canceled after those expected failures were captured.

  • Passed: make dev-test FILTER='CodexResumeWedgeCommitTests|AgentSessionLinkLaneBoardTests|AgentRunLifecycleContractsTests|AgentRunCancellationOutcomeTests' — 67 tests, 0 failures (43 Codex, 14 lane-board, 4 lifecycle, 6 cancellation).

  • Coverage includes three consecutive timeouts without replacement, persisted identity preservation, explicit same-thread retry, fresh routing admission rather than stale routed signals, one actionable error with a terminal barrier installed, stale-reason reset, and lane-board wire serialization.

  • Passed: make dev-swift-build PRODUCT=all — both RepoPrompt and repoprompt-mcp.

  • Passed: make dev-lint; only changed in-scope Swift files were formatted with repository-pinned SwiftFormat 0.61.1. Domain-model files retain their existing formatting.

  • Interim compilation failures (one remaining removed-helper reference and a new test's renderer type name) were corrected before the final passing run.

  • Attempted: non-launching make dev-smoke. windows succeeded; workspace switching failed with [-32000] Connection closed. This exercised the existing debug app, not a newly packaged build.

  • Not run: full test suite, live Codex/provider turns, app launch/relaunch/stop, packaged/release validation, or the heavyweight pr-ready lane.

  • Passed: mandatory preflight.sh commit and preflight.sh push (staged/outgoing secret scans, guardrails, clean push boundary).

Review follow-up

  • Repeated same-reference timeouts now make Handoff the primary recommendation after the second failure; retries remain explicit and the fixed deadline is not inflated for every caller.
  • Timeout notices retain the original bounded error/method and include an agent-facing agent_run start handoff option. No claim is made that an initialize or provisioning timeout necessarily occurred while loading history.
  • Reuses fix(codex): stop oversized resumes without losing the current task #1443's error-kind recovery-diagnostic carrier, including legacy decoding compatibility, so the stopped-resume notice survives summary-tier compaction and restoration as an error.
  • Terminalization consumes the timeout disposition; new attempts also clear it before any provider-specific admission path. Legacy snapshot text classification recognizes the stopped Codex resume notice as resume_timed_out.
  • An auth refresh followed by resume timeout keeps the timeout notice/reason instead of appending contradictory manual-login guidance.
  • Restores frame-overflow replacement coverage for bootstrap permit renewal, stale routing-signal rejection, and retained successor admission while that fallback still exists on main.
  • Barrier regression coverage verifies that another provider's later turn cannot notify an earlier turn's failure; the production binding's failure-text reader remains current-turn-only.

Rebase over #1443

Reviewer map: expect roughly ten mechanical conflict hunks, especially the coordinator fallback enum/helper/log/source, startup guards and outcome reset, catch/error emission, no-active-thread reporting, terminal request, and failure-reason enum/display/help additions. Keep the shared diagnostic carrier/decoder/projection and its compatibility test once; retain both resumeFrameTooLarge and resumeTimedOut diagnostic reasons.

Semantic follow-ups (not a blind conflict resolution):

  1. After fix(codex): stop oversized resumes without losing the current task #1443 removes the frame-overflow fallback, delete this branch's now-dead fresh-fallback block (reviewed around coordinator lines 7535–7629), resumeFreshStartFallbackReason, and allowResumeTimeoutFallback parameter/call propagation. Do not accidentally restore the timeout-to-fresh behavior.
  2. In terminal settlement prefer resume_history_too_large for the blocked oversized target, then resume_timed_out for the consumed timeout outcome. Preserve both enum cases and wire labels.
  3. In the startup catch check oversized history before generic timeout classification; only the timeout path should increment timeout accounting or offer a conditional retry.
  4. Keep fix(codex): stop oversized resumes without losing the current task #1443's conversation/rollout-pair block and Handoff guidance. Keep this branch's repeated-timeout Handoff preference and agent-facing instructions. Clear both independent startup states only at their owning lifecycle boundaries: the oversized-target block must survive retries of the same conversation/rollout pair and must not be cleared with the timeout disposition on every new attempt.
  5. Drop the now-obsolete frame-overflow fresh-replacement test only when its production path is removed; keep explicit timeout retry/routing-admission coverage and fix(codex): stop oversized resumes without losing the current task #1443's blocked-oversized tests.

Review follow-up validation

  • Passed: make dev-test FILTER='CodexResumeWedgeCommitTests|AgentRetentionCompactionTests|AgentTranscriptRecoveryDiagnosticTests|AgentSessionLinkLaneBoardTests|AgentRunLifecycleContractsTests|AgentRunCancellationOutcomeTests' — 81 tests, 0 failures.

  • Passed: make dev-swift-build PRODUCT=all and make dev-lint.

  • Targeted static follow-up review: no remaining findings.

  • No additional live smoke/provider run, app lifecycle action, full-suite, release, or pr-ready execution; the earlier existing-app smoke limitation above remains.

  • Passed: mandatory commit and push preflights for the review-fix commit (staged/outgoing secret scans and guardrails).

@baron baron left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed a0cf702d9fe5c9a7921cb1838fd0fd0a251bfeb9 against its merge base.

No actionable defect found in the timeout-specific changes. I checked that repeated resume timeouts keep the conversation/rollout/cleanup tuple, stop before dispatch, retire the timed-out controller, renew admission on explicit retry, and publish a single retained error with the resume_timed_out classification. The tests also cover auth-recovery timeout, stale routing signals, transcript compaction/serialization, and clearing the startup disposition for later attempts.

Integration note: #1443 removes the oversized-resume fallback while this PR removes the timeout fallback. Both edit the same coordinator branches and recovery-diagnostic schema; this head deliberately still tests automatic fresh-thread fallback for oversized responses. Combine the two behaviors explicitly and run both regression matrices after integration so neither protection is lost during conflict resolution.

CI for this head was queued at review time: https://github.com/repoprompt/repoprompt-ce/actions/runs/39878285370. Static source/test review only; no native macOS XCTest run here.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Codex: after a timed-out resume, a fresh thread starts silently and the conversation is lost

1 participant