Skip to content

feat(sessions): add auto-retry logic for failed connections#2030

Merged
jonathanlab merged 2 commits intomainfrom
posthog-code/implement-automatic-retry-for-connection-errors
May 5, 2026
Merged

feat(sessions): add auto-retry logic for failed connections#2030
jonathanlab merged 2 commits intomainfrom
posthog-code/implement-automatic-retry-for-connection-errors

Conversation

@jonathanlab
Copy link
Copy Markdown
Contributor

Add automatic retry logic for failed agent connections that intermittently resolve on retry. The session will attempt to reconnect up to 2 times with a 10-second delay between attempts before showing an error to the user


Created with PostHog Code

Generated-By: PostHog Code
Task-Id: 84fbe425-c84e-4f1b-94d4-72773db32287
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 5, 2026

Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
apps/code/src/renderer/features/sessions/service/service.ts:475-498
**No connectivity re-check between retry attempts**

`getIsOnline()` is evaluated once before the loop, but the loop waits 10 seconds per attempt. If the device goes offline during that gap, the retry still fires — and `clearSessionError``reconnectInPlace` will throw "Unable to reach server…", overwriting `lastRetryMessage` with a misleading error while consuming both retry slots. Checking `getIsOnline()` at the top of each loop iteration would short-circuit cleanly and avoid corrupting the final error message.

### Issue 2 of 3
apps/code/src/renderer/features/sessions/service/service.ts:475-506
**No tests for the retry flow — and existing test will break**

The repo rule requires comprehensive tests for retry logic covering multiple attempts. None are included. Additionally, the existing test `"creates error session when auth is missing"` (which runs with `getIsOnline()` returning `true`) now hits the retry path: `setSession` is called with `status: "connecting"` instead of `status: "error"`, and each of the two `setTimeout(resolve, 10_000)` calls runs for real since that describe block does not use `vi.useFakeTimers()`. The test will hang for ~20 s and then fail because the error state is now emitted via `updateSession`, not `setSession`.

### Issue 3 of 3
apps/code/src/renderer/features/sessions/service/service.ts:499-506
**Stale-session update after a long async gap**

Up to 20 seconds pass while the retry loop runs. In that window the user can navigate to a different task, dismiss the session, or a new `connectToTask` call can create a fresh session under the same `taskId`. The `getSessionByTaskId` lookup and subsequent `updateSession` call have no guard against this — they may stomp an unrelated session. Checking whether the current session's `taskRunId` still matches the one captured at the start of the catch block before calling `updateSession` would prevent this.

Reviews (1): Last reviewed commit: "feat(sessions): add auto-retry logic for..." | Re-trigger Greptile

Comment thread apps/code/src/renderer/features/sessions/service/service.ts
Comment thread apps/code/src/renderer/features/sessions/service/service.ts
Comment thread apps/code/src/renderer/features/sessions/service/service.ts
- Re-check getIsOnline() inside the retry loop so a mid-retry
  disconnect short-circuits cleanly with a "disconnected" state
  instead of consuming retry slots with misleading errors.
- Guard the final updateSession against a missing session (user
  dismissed the task during the retry gap).
- Add tests covering the auto-retry path: success on retry, failure
  after both retries, going offline mid-retry, and dismissed session.

Generated-By: PostHog Code
Task-Id: 84fbe425-c84e-4f1b-94d4-72773db32287
@jonathanlab jonathanlab added the Create release This will trigger a new release label May 5, 2026
@jonathanlab jonathanlab merged commit 93faf30 into main May 5, 2026
15 checks passed
@jonathanlab jonathanlab deleted the posthog-code/implement-automatic-retry-for-connection-errors branch May 5, 2026 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Create release This will trigger a new release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants