fix(sessions): recover and resend prompts after "session not found"#3170
fix(sessions): recover and resend prompts after "session not found"#3170richardsolomou wants to merge 2 commits into
Conversation
Local agent sessions are idle-killed after 15 minutes. When the renderer misses the idle-kill event (sleep, dropped subscription), replying to the task hit the dead session, surfaced "Session not found", and the composer had already cleared the typed prompt. Now a fatal prompt failure awaits the existing in-place recovery and resends the prompt once on the recovered session, so stale replies land instead of erroring. If a send still fails for any reason, the typed prompt is restored into the composer instead of being lost. Generated-By: PostHog Code Task-Id: 03ebf8b9-1bd2-49a9-ae91-d8f5e86ed683
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
|
Reviews (1): Last reviewed commit: "fix(sessions): recover and resend prompt..." | Re-trigger Greptile |
|
|
||
| expect(result).toEqual({ stopReason: "end_turn" }); | ||
| expect(recoverSpy).toHaveBeenCalledTimes(1); | ||
| expect(promptMutate).toHaveBeenCalledTimes(2); | ||
| }); | ||
|
|
||
| it("surfaces the error state when recovery fails", async () => { | ||
| const { service, store, promptMutate, recoverSpy } = createHarness(); | ||
| promptMutate.mockRejectedValue( | ||
| new Error(`Session not found: ${TASK_RUN_ID}`), | ||
| ); | ||
| recoverSpy.mockResolvedValue(false); | ||
|
|
||
| await expect(service.sendPrompt(TASK_ID, "hello again")).rejects.toThrow( | ||
| "Session not found", | ||
| ); | ||
| expect(promptMutate).toHaveBeenCalledTimes(1); | ||
| expect(store.setSession).toHaveBeenCalledWith( | ||
| expect.objectContaining({ taskRunId: TASK_RUN_ID, status: "error" }), | ||
| ); | ||
| }); | ||
|
|
||
| it("does not loop when the resend hits another fatal error", async () => { | ||
| const { service, promptMutate, recoverSpy } = createHarness(); | ||
| promptMutate.mockRejectedValue( | ||
| new Error(`Session not found: ${TASK_RUN_ID}`), | ||
| ); | ||
| recoverSpy.mockResolvedValue(true); | ||
|
|
||
| await expect(service.sendPrompt(TASK_ID, "hello again")).rejects.toThrow( | ||
| "Session not found", | ||
| ); | ||
| expect(recoverSpy).toHaveBeenCalledTimes(1); | ||
| expect(promptMutate).toHaveBeenCalledTimes(2); | ||
| }); | ||
|
|
||
| it("swallows recovery exceptions and rethrows the original error", async () => { | ||
| const { service, promptMutate, recoverSpy } = createHarness(); | ||
| promptMutate.mockRejectedValue( | ||
| new Error(`Session not found: ${TASK_RUN_ID}`), | ||
| ); | ||
| recoverSpy.mockRejectedValue(new Error("auth restoring")); | ||
|
|
||
| await expect(service.sendPrompt(TASK_ID, "hello again")).rejects.toThrow( | ||
| "Session not found", | ||
| ); | ||
| expect(promptMutate).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Prefer parameterised tests per project conventions
The four test cases share identical scaffolding — mock promptMutate, mock recoverSpy, call sendPrompt, assert. Per the team's style guide ("We always prefer parameterised tests"), these should be expressed as a single it.each table that varies the mock return values and the expected outcome, rather than four separate it(...) blocks.
Context Used: Do not attempt to comment on incorrect alphabetica... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
The three failure cases exercise the same fatal-error path with different recovery behaviour, so express them as one it.each table per review feedback. All failure cases now also assert the session is surfaced in the error state. Generated-By: PostHog Code Task-Id: 03ebf8b9-1bd2-49a9-ae91-d8f5e86ed683
Problem
Replying to a local (or worktree) task that has been idle for a while sometimes fails with "Session not found" — and the typed prompt is lost, forcing the user to re-type it.
Local agent sessions are idle-killed by the workspace server after 15 minutes. The renderer normally gets notified and reconnects, but when it misses that event (laptop sleep, dropped subscription, host restart) the UI still shows the session as connected. The reply then hits the dead session, the server throws
Session not found, and the error path only showed a toast — while the composer had already cleared the prompt optimistically on submit.Changes
@posthog/coreSessionService.sendLocalPrompt: on a fatal prompt failure (e.g. "session not found"), await the existing in-place auto-recovery and resend the prompt once on the recovered session, so replies to stale sessions land instead of erroring. AnisRecoveryResendguard prevents retry loops; if recovery fails, the session is put into the error state with the Retry UI as before.@posthog/uiuseSessionCallbacks.handleSendPrompt: if a send still fails for any reason, restore the typed prompt into the composer (xmlToContent+setPendingContent, the same mechanism the cancel flow uses) and refocus it — skipped if the user has already started typing a new message.How did you test this?
packages/core/src/sessions/sessionServicePromptRecovery.test.ts: recover-and-resend succeeds, recovery failure surfaces the error state, no retry loop when the resend also hits a fatal error, and recovery exceptions don't mask the original error.pnpm --filter @posthog/core typecheck,pnpm --filter @posthog/ui typecheck, andbiome lint packages/core(zeronoRestrictedImports) — all clean.Automatic notifications
🤖 Generated with Claude Code