fix: clear stale agentSessionId on session_not_found loop#544
fix: clear stale agentSessionId on session_not_found loop#544openasocket wants to merge 1 commit intoRunMaestro:0.16.0-RCfrom
Conversation
…ite loop When a Claude session expires or is deleted, subsequent prompts would try --resume with the stale session ID, receive session_not_found, and repeat endlessly. Now clears agentSessionId at both the tab and session level so the next prompt starts a fresh session. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds error handling for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR fixes an infinite-loop bug where a stale Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as UI / Prompt
participant Hook as useAgentListeners
participant Store as SessionStore
participant Agent as Claude Agent
Note over UI,Agent: Before fix — infinite loop
UI->>Agent: --resume stale-session-abc123
Agent-->>Hook: onAgentError(session_not_found)
Hook->>Store: set state='error' (agentSessionId unchanged)
UI->>Agent: --resume stale-session-abc123 (loop)
Note over UI,Agent: After fix — recovery
UI->>Agent: --resume stale-session-abc123
Agent-->>Hook: onAgentError(session_not_found)
Hook->>Store: clear tab.agentSessionId + session.agentSessionId
Agent-->>Hook: onAgentExit
Hook->>Store: set state='idle' (exit handler)
UI->>Agent: (no --resume flag) start fresh session
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it('clears agentSessionId on session_not_found so next prompt starts fresh', () => { | ||
| const deps = createMockDeps(); | ||
| const tab = createMockTab({ | ||
| id: 'tab-1', | ||
| agentSessionId: 'stale-session-abc123', | ||
| }); | ||
| const session = createMockSession({ | ||
| id: 'sess-1', | ||
| state: 'busy', | ||
| aiTabs: [tab], | ||
| activeTabId: 'tab-1', | ||
| agentSessionId: 'stale-session-abc123', | ||
| }); | ||
| useSessionStore.setState({ | ||
| sessions: [session], | ||
| activeSessionId: 'sess-1', | ||
| }); | ||
|
|
||
| renderHook(() => useAgentListeners(deps)); | ||
|
|
||
| onAgentErrorHandler?.('sess-1-ai-tab-1', { | ||
| ...baseError, | ||
| type: 'session_not_found', | ||
| }); | ||
|
|
||
| const updated = useSessionStore.getState().sessions.find((s) => s.id === 'sess-1'); | ||
| const updatedTab = updated?.aiTabs.find((t) => t.id === 'tab-1'); | ||
| // Tab-level agentSessionId must be cleared | ||
| expect(updatedTab?.agentSessionId).toBeUndefined(); | ||
| // Session-level agentSessionId must also be cleared | ||
| expect(updated?.agentSessionId).toBeUndefined(); | ||
| }); |
There was a problem hiding this comment.
Test doesn't assert session state remains usable after the error
The test verifies agentSessionId is cleared (the primary goal), but the session was created with state: 'busy' and is never checked afterward. If the exit handler doesn't fire in a test environment, the session could remain 'busy', leaving the UI input disabled.
While the production exit event should always follow the error event (transitioning state to 'idle'), it would strengthen the test to also assert the session state is not left in an unusable state, or to document that the state cleanup is delegated to the exit handler:
// Verify agentSessionId is cleared
expect(updatedTab?.agentSessionId).toBeUndefined();
expect(updated?.agentSessionId).toBeUndefined();
// State cleanup is handled by onAgentExit; verify it is NOT set to 'error'
// (which would block the execution queue from dequeuing the next item)
expect(updated?.state).not.toBe('error');![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
pedramamini
force-pushed
the
0.16.0-RC
branch
4 times, most recently
from
1883595 to
24f4bce
Compare
1 participant
Summary
agentSessionIdwas never cleared — every subsequent prompt tried--resumewith the dead ID, receivedsession_not_found, and looped endlesslyagentSessionIdat both tab and session level onsession_not_founderrors, so the next prompt starts a fresh session automaticallyTest plan
agentSessionIdis cleared at both tab and session level onsession_not_found🤖 Generated with Claude Code
Summary by CodeRabbit