Uh oh!
There was an error while loading. Please reload this page.
[orchestrator-v2] fix(orchestrator): Resume persisted Claude sessions after provider session recycle - #3750
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
3c8b95b to
fb7f966CompareApprovabilityVerdict: Approved Straightforward bug fix that adds a condition to properly resume Claude sessions after provider session idle-release. The change is minimal (one new condition in existing logic), well-commented, and thoroughly tested. You can customize Macroscope's approvability policy. Learn more. |
f1ffac0 to
33656adCompare33656ad to
97bf0d0Compare
Summary
Claude threads that sit idle for more than 30 minutes fail instantly on the
next message with
transport_error "Claude Agent SDK query failed."(the CLIreports
Session ID ... is already in use). This PR makes the adapter resumethe persisted native session instead of trying to recreate it.
One commit on
fix/claude-session-resume:ClaudeAdapterV2.ts(the fix) andClaudeAdapterV2.test.ts(two new tests). Everything else comes from the basebranch.
Problem and Fix
ProviderSessionManageridle-releases a provider session (30 min default), the adapter's in-memoryopenedNativeThreadsset is lost. The next turn'sopenQuerytherefore opens the native thread with{sessionId}(create semantics,--session-id) instead of{resume}, and the Claude CLI fails fast because the session id already exists on disk.openQuery, treat a persisted provider turn as proof the native session exists:providerTurnOrdinal > 1(computed byProviderTurnStartServiceas max persisted turn ordinal for the provider thread plus 1) forces{resume}alongside the existing in-memory and rollback signals.Edge cases checked: run attempt retries (session created on first attempt),
steering (
steerTurnnever callsopenQuery),thread_startrollback (newprovider thread id resets the ordinal to 1, so create is used for the fresh
native session), mid-thread rollback (
resumeSessionAtalready forcesresume), forks in the same session instance (covered by
openedWithResume),and first-run failures before CLI spawn (no turn persisted, ordinal stays 1).
Validation
vp check: passvp run typecheck: passvp test apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts:14/14 pass, including two new tests (
ClaudeAdapterV2 native session identity) asserting ordinal 1 opens withsessionIdand ordinal 2 openswith
resumestashing the fix and rerunning)
OrchestratorReplayFixtures.integration.test.ts(claude fixtures): passKnown limitations
the native session (or the reverse: session created, persistence failed),
the identity choice can still be wrong on retry. Both traps pre-date this
change (the in-memory set had the same blind spots within a session
instance); this PR keeps the fix minimal and does not add auto-retry
identity flipping.
persisted for the forked thread. If the process restarts between the fork
and the forked thread's first turn, the ordinal is still 1 and
openQuerysends
{sessionId}for an existing native session. The ordinal signaldoes not engage there, so behavior is identical to before this change
(pre-existing hole, found by review; a
forkedFrom-based signal wasrejected because
thread_startrollback preservesforkedFromwhileminting a fresh, uncreated native session id).
Note
Resume persisted Claude sessions on subsequent provider turns in
ClaudeAdapterV2When a provider session is recycled (idle release), the adapter previously opened a new native Claude session on the next turn instead of resuming the existing one.
openSessionnow checksproviderTurnOrdinal > 1as an additional condition to setshouldResume, ensuring the query opens in resume mode for any turn after the first. Tests are added to assert new-session behavior on the first provider turn and resume behavior on subsequent turns.Macroscope summarized 97bf0d0.
Note
Medium Risk
Changes how Claude Agent SDK queries are opened after session recycle; wrong ordinal or identity edge cases could still mis-create vs resume, but scope is limited to resume detection in the adapter.
Overview
Fixes immediate transport failures on the next message when a Claude thread was idle long enough for the provider session to be released (~30 min). After recycle, the adapter no longer tries to create the native CLI session with a fixed
sessionId(which errors with “Session ID … is already in use”); it resumes the on-disk session instead.In
ClaudeAdapterV2.openQuery,shouldResumenow also becomes true whenproviderTurnOrdinal > 1, using persisted turn history as proof the native session already exists when the per-instanceopenedNativeThreadsset was cleared. Tests cover first turn (sessionId, noresume) vs a later turn on a fresh session instance (resume, nosessionId).Reviewed by Cursor Bugbot for commit 97bf0d0. Bugbot is set up for automated code reviews on this repo. Configure here.