fix: lost provider session recovery#1938
fix: lost provider session recovery#1938ashvinnihalani wants to merge 1 commit intopingdotgg:mainfrom
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 |
ApprovabilityVerdict: Approved This is a small, well-scoped bug fix that adds a defensive check to verify an active session actually exists before attempting to reuse it. The change consists of moving one function call and adding one condition check, with a comprehensive test covering the fix. You can customize Macroscope's approvability policy. Learn more. |
|
Can you please use a more conventional PR title? eg:
|
|
@binbandit Done |
- Require an active provider session before treating thread.session as reusable - Add regression coverage for starting a fresh session when only projected state exists
c11ff5d to
9c66c3d
Compare
What Changed
Why
Starting a new turn could fail after the provider session was lost because the app still believed the old session was reusable.
This change makes the next turn recover automatically by starting a fresh provider session instead of trying to reuse stale session state.
UI Changes
None.
Checklist
Note
Medium Risk
Changes session reuse logic for turn start, which can affect whether sessions are restarted vs reused and could cause unexpected extra session creation if the active-session check is wrong. Scope is limited to
ProviderCommandReactorplus a focused regression test.Overview
Fixes lost provider-session recovery by only reusing
thread.sessionwhen there is also a live provider session.ensureSessionForThreadnow resolves the active provider session up front and treats projected (non-stopped) session state as reusable only if it matches an active session, otherwise it starts and binds a fresh session.Adds a regression test ensuring a new
thread.turn.startwill callstartSession+sendTurnwhen the thread has only projected session state (e.g., after restart) and no active provider session.Reviewed by Cursor Bugbot for commit 9c66c3d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix
ProviderCommandReactorto start a fresh session when provider session is lostWhen a thread has a projected session state but no active provider session, the reactor was incorrectly treating it as an existing session and skipping session initialization. The fix resolves the active provider session earlier in ProviderCommandReactor.ts and requires both a non-stopped projected session and a live active session before reusing an existing session.
Macroscope summarized 9c66c3d.