Skip to content

fix: improve SSE heartbeat and session retry logic in TUI#22151

Closed
rudalsgecko wants to merge 1 commit intoanomalyco:devfrom
rudalsgecko:fix/sse-heartbeat-retry
Closed

fix: improve SSE heartbeat and session retry logic in TUI#22151
rudalsgecko wants to merge 1 commit intoanomalyco:devfrom
rudalsgecko:fix/sse-heartbeat-retry

Conversation

@rudalsgecko
Copy link
Copy Markdown

Summary

This PR improves the reliability of SSE connections and session error handling in the TUI by:

  • Implementing a 20s heartbeat timeout in SDKProvider to detect stalled connections.
  • Adding a 250ms delay between SSE reconnection attempts to prevent tight CPU-bound loops.
  • Adding a retry limit (10 attempts) to SessionProcessor to avoid infinite retry cycles.
  • Gracefully halting sessions with a resumeable AbortedError when the retry limit is reached.
  • Automatically re-syncing the client state in SyncProvider upon reconnection.

How to use this branch locally

To test or use these improvements, install this branch directly from GitHub:

Using bun:

bun add https://github.com/rudalsgecko/opencode#fix/sse-heartbeat-retry

Using npm:

npm install https://github.com/rudalsgecko/opencode#fix/sse-heartbeat-retry

- Implement 20s heartbeat timeout in TUI SDKProvider
- Add 250ms delay between SSE reconnection attempts to avoid tight loops
- Add retry limit (10 attempts) to SessionProcessor
- Handle retry exhaustion by halting session with a resumeable AbortedError
- Trigger re-sync in SyncProvider on server.connected or reconnectToken change
@github-actions github-actions bot added needs:compliance This means the issue will auto-close after 2 hours. needs:issue labels Apr 12, 2026
@github-actions
Copy link
Copy Markdown
Contributor

This PR doesn't fully meet our contributing guidelines and PR template.

What needs to be fixed:

  • PR description is missing required template sections. Please use the PR template.

Please edit this PR description to address the above within 2 hours, or it will be automatically closed.

If you believe this was flagged incorrectly, please let a maintainer know.

@github-actions
Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions
Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

Found 1 potential related PR:

PR #15436: fix(tui): recover event stream after lock/sleep disconnect
#15436

This PR addresses similar SSE connection reliability concerns - specifically recovering event streams when the connection is lost (e.g., after lock/sleep). It may have overlapping solutions or context relevant to the heartbeat and retry logic improvements in PR #22151.

@rudalsgecko
Copy link
Copy Markdown
Author

리뷰 의견 (임시)

작성자 본인의 셀프 리뷰입니다. 주요 확인 필요 항목을 남깁니다.

🔴 Blocker

1. processor.ts 타입 캐스팅

create: (input: Input) => create(input) as Effect.Effect<Handle, never, never>,

never, never로 강제 캐스팅하면 실제 발생 가능한 에러/요구사항이 타입 시스템에서 사라집니다. Effect.fnEffect.gen 전환 과정에서 추론 타입이 바뀐 것이 원인으로 추정되는데, 캐스팅으로 덮기보다는 시그니처를 맞추는 방향으로 고치는 것이 안전합니다.

🟠 의심

2. Schedule.recurs 조합 방식

SessionRetry.exponentialBackoff(...).pipe(Schedule.recurs(RETRY_MAX_ATTEMPTS))

Effect에서 두 Schedule을 교차 적용하려면 보통 Schedule.intersect(Schedule.recurs(n))를 사용합니다. 현 코드가 실제로 10회에서 멈추는지 테스트로 검증이 필요합니다.

🟡 개선 포인트

3. 들여쓰기 불균형
processor.ts 하단의 닫는 }) 들여쓰기가 어긋나 있어 포매터/린터 통과 여부 확인 필요.

4. 고정 250ms 재연결 지연
서버가 장기간 다운이면 250ms 간격으로 계속 재시도합니다. SessionRetry의 지수 백오프 + 지터를 재사용하는 쪽이 리소스에 더 안전합니다.

5. 20s heartbeat 타임아웃의 서버 의존성
서버의 heartbeat 방출 주기가 20s 미만임을 암묵적으로 가정합니다. 상수/주석으로 서버 측 주기와 연결해두면 좋겠습니다. 네트워크 스파이크 시 false positive 가능성도 고려 필요.

6. server.heartbeat 타입 캐스팅

if ((event.payload.type as string) === \"server.heartbeat\") continue

as string 대신 SDK 이벤트 타입에 정식으로 heartbeat를 추가하는 쪽이 타입 안전성 측면에서 바람직합니다.

🟢 minor

7. disconnected 플래그와 reconnectToken 이중화
두 상태가 의미상 겹칩니다. reconnectToken 증가 조건을 connectionState() 기반으로 통일하거나, disconnected 플래그의 역할을 주석으로 명확히 해두면 유지보수성이 개선됩니다.

8. sync.tsx의 bootstrap 중복 호출 가능성
server.connected 이벤트와 reconnectToken 증가가 모두 bootstrap()을 트리거합니다. 재연결 직후 짧은 시간에 두 번 호출될 여지가 있는지 dedupe 확인 필요.

👍 좋은 점

  • 무한 재시도 루프 방지(retry cap) 도입
  • 재시도 한도 도달 시 사용자 친화적 메시지 제공(AbortedError 변환)
  • 재연결 시 클라이언트 상태 재동기화 경로 추가

우선순위: 1 → 2 → 3 순서로 확인 권장합니다.

@rudalsgecko rudalsgecko reopened this Apr 13, 2026
@rudalsgecko rudalsgecko marked this pull request as draft April 13, 2026 03:31
@rudalsgecko
Copy link
Copy Markdown
Author

Blocker / 의심 항목 상세 조사 (업데이트)

로컬에서 PR 브랜치를 체크아웃해 bunx tsc --noEmit로 실측한 결과, 원본 PR은 processor.ts에서 6개의 TS 컴파일 에러가 발생합니다.

실측 에러

TS2322 L534: Effect<..., unknown, unknown> → Effect<Result, never, never> 할당 불가
TS2345 L572: Schedule<number> 을 .pipe() 인자 자리에 전달 (함수 아님)
TS2339 L574: Property 'catchAll' does not exist on Effect
TS7006 L574: implicit any
TS2339 L586: Property 'catchAll' does not exist on Effect
TS7006 L586: implicit any

근본 원인

  1. Effect.catchAll 제거됨 — Effect 4 beta에서 Effect.catch로 리네임되었습니다. (effect/dist/Effect.d.ts L3964: catch_ as catch, 주석: "This API replaces Effect.catchAll")
  2. Schedule.recurs(N)는 함수가 아님(times: number) => Schedule<number> 형태이므로 .pipe(Schedule.recurs(N))에 넣으면 타입/런타임 에러.
  3. Effect 4에는 Schedule.intersect가 없음 — AND 조합은 Schedule.both로 해야 합니다.
  4. createas Effect.Effect<Handle, never, never> 캐스트는 위 에러들의 원인인 process의 잘못된 리턴 타입(never, never)이 상위로 누수된 결과를 덮어버리는 용도였습니다.

제안 수정 (diff)

-        const process = (streamInput: LLM.StreamInput): Effect.Effect<Result, never, never> => {
-          return Effect.gen(function* () {
-            yield* slog.info(\"process\")
-            ctx.needsCompaction = false
-            ctx.shouldBreak = (yield* config.get()).experimental?.continue_loop_on_deny !== true
+        const process = Effect.fn(\"SessionProcessor.process\")(function* (streamInput: LLM.StreamInput) {
+          yield* slog.info(\"process\")
+          ctx.needsCompaction = false
+          ctx.shouldBreak = (yield* config.get()).experimental?.continue_loop_on_deny !== true
 
+          return yield* Effect.gen(function* () {
             yield* Effect.gen(function* () {
@@
-                }).pipe(Schedule.recurs(SessionRetry.RETRY_MAX_ATTEMPTS)),
+                }).pipe(Schedule.both(Schedule.recurs(SessionRetry.RETRY_MAX_ATTEMPTS))),
               ),
-              Effect.catchAll((e) => {
+              Effect.catch((e) => {
                 const error = parse(e)
                 if (MessageV2.APIError.isInstance(error) && error.data.isRetryable) {
                   return halt(
                     new MessageV2.AbortedError(
                       { message: \"Retry limit reached. Please check your network and continue.\" },
                       { cause: e },
                     ),
                   )
                 }
-                return Effect.fail(e)
+                return halt(e)
               }),
-              Effect.catchAll((e) => halt(e)),
               Effect.ensuring(cleanup()),
             )
@@
-        }
+        })
 
         return {
           ...
         } satisfies Handle
-        })
+      })
 
-        return Service.of({
-        create: (input: Input) => create(input) as Effect.Effect<Handle, never, never>,
-        })
+      return Service.of({ create })

수정 요지

  • processEffect.fn(\"SessionProcessor.process\")로 복원하여 tracing span을 유지하고 타입 추론을 정상화.
  • Schedule.both(Schedule.recurs(N))로 AND 조합해 재시도 한도를 실제로 걸리게 함.
  • Effect.catchAllEffect.catch (Effect 4 API).
  • 두 번째 catch는 첫 번째로 통합 (return Effect.fail(e) 대신 바로 halt(e)).
  • 캐스트 제거 → Service.of({ create }).

검증

수정 후 bunx tsc --noEmit 실행 시 processor.ts 관련 TS 에러 0개 (사전 존재하는 drizzle.config.ts/proxy.ts 에러만 남음).

@github-actions
Copy link
Copy Markdown
Contributor

This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 2-hour window.

Feel free to open a new pull request that follows our guidelines.

@github-actions github-actions bot removed the needs:compliance This means the issue will auto-close after 2 hours. label Apr 13, 2026
@github-actions github-actions bot closed this Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant