Repository navigation
Conversation
baron
left a comment
There was a problem hiding this comment.
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.
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_outterminal reason is published for unattended agent_run/lane callers instead of the generic transienttimeout. 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— bothRepoPromptandrepoprompt-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.windowssucceeded; 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-readylane.Passed: mandatory
preflight.sh commitandpreflight.sh push(staged/outgoing secret scans, guardrails, clean push boundary).Review follow-up
agent_run starthandoff option. No claim is made that aninitializeor provisioning timeout necessarily occurred while loading history.resume_timed_out.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
resumeFrameTooLargeandresumeTimedOutdiagnostic reasons.Semantic follow-ups (not a blind conflict resolution):
resumeFreshStartFallbackReason, andallowResumeTimeoutFallbackparameter/call propagation. Do not accidentally restore the timeout-to-fresh behavior.resume_history_too_largefor the blocked oversized target, thenresume_timed_outfor the consumed timeout outcome. Preserve both enum cases and wire labels.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=allandmake dev-lint.Targeted static follow-up review: no remaining findings.
No additional live smoke/provider run, app lifecycle action, full-suite, release, or
pr-readyexecution; 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).