Difficulty: Advanced
Type: feature (resilience)
Background
lib/api/live.ts exposes subscribeWebhookEvents(), which opens a GET /v1/admin/events/stream SSE connection and parses text/event-stream frames into WebhookEventLog objects. The method is explicitly documented as PROVISIONAL, since the backend SSE endpoint doesn't exist in guildpass-core yet, and the intent is that "failures are intentionally reported to the caller so the UI can silently resume the existing /v1/admin/events polling behavior." Elsewhere in the same file, REST calls already benefit from a shared retry/backoff (backoffDelayMs) and circuit-breaker (CircuitEntry) mechanism.
Problem
As written, subscribeWebhookEvents() makes exactly one connection attempt. If the stream is dropped (network blip, backend restart, proxy timeout) — including the case explicitly coded as throw new Error('Admin events stream closed before unsubscribe') when the reader reports done — the caller's onError fires once and there is no attempt to reconnect. Any consumer wanting live-ish updates has to reimplement reconnect/backoff logic itself, and there's no reuse of the retry primitives already built for REST calls in the same module.
Expected Outcome
subscribeWebhookEvents() automatically attempts to reconnect after an unexpected stream close or connection failure, using capped exponential backoff consistent with the existing REST retry constants, and stops attempting once the caller calls the returned unsubscribe function. onError is still invoked so the UI's polling fallback keeps working, but transient drops no longer require the caller to manually resubscribe.
Suggested Implementation
- Wrap the existing
fetch(...).then(...).catch(...) stream-reading logic in a reconnect loop, reusing backoffDelayMs() (or a similarly shaped helper) for delay calculation between attempts.
- Track reconnect attempts per subscription instance (not shared with the REST circuit breaker's per-path state, since a stream is stateful and long-lived) and cap the maximum retry delay and/or maximum attempt count before giving up and calling
onError with a terminal error.
- Ensure the returned unsubscribe function (
() => controller.abort()) fully stops the reconnect loop — an in-flight backoff timer must be cleared, not just the current fetch aborted.
- Reset the reconnect/backoff counter after a period of stable connection (e.g., once frames have been successfully received for some duration), so a long-lived healthy connection that drops once doesn't inherit a stale backoff state from an earlier flaky period.
- Add an optional
onReconnecting?: (attempt: number, delayMs: number) => void callback so the admin UI (app/admin/*) can surface connection status if desired (optional — check if any consumer currently uses this method before wiring UI).
- Add unit tests mocking
fetch to simulate: a stream that closes mid-read, a stream that never opens (network error), and a stream that stays healthy — asserting reconnect attempts, backoff delays, and correct cleanup on unsubscribe.
Acceptance Criteria
Likely Affected Files/Directories
lib/api/live.ts
docs/admin-events-stream-contract.md
test/ (new test file, e.g. test/webhook-stream-reconnect.test.ts)
Difficulty: Advanced
Type: feature (resilience)
Background
lib/api/live.tsexposessubscribeWebhookEvents(), which opens aGET /v1/admin/events/streamSSE connection and parsestext/event-streamframes intoWebhookEventLogobjects. The method is explicitly documented as PROVISIONAL, since the backend SSE endpoint doesn't exist inguildpass-coreyet, and the intent is that "failures are intentionally reported to the caller so the UI can silently resume the existing/v1/admin/eventspolling behavior." Elsewhere in the same file, REST calls already benefit from a shared retry/backoff (backoffDelayMs) and circuit-breaker (CircuitEntry) mechanism.Problem
As written,
subscribeWebhookEvents()makes exactly one connection attempt. If the stream is dropped (network blip, backend restart, proxy timeout) — including the case explicitly coded asthrow new Error('Admin events stream closed before unsubscribe')when the reader reportsdone— the caller'sonErrorfires once and there is no attempt to reconnect. Any consumer wanting live-ish updates has to reimplement reconnect/backoff logic itself, and there's no reuse of the retry primitives already built for REST calls in the same module.Expected Outcome
subscribeWebhookEvents()automatically attempts to reconnect after an unexpected stream close or connection failure, using capped exponential backoff consistent with the existing REST retry constants, and stops attempting once the caller calls the returned unsubscribe function.onErroris still invoked so the UI's polling fallback keeps working, but transient drops no longer require the caller to manually resubscribe.Suggested Implementation
fetch(...).then(...).catch(...)stream-reading logic in a reconnect loop, reusingbackoffDelayMs()(or a similarly shaped helper) for delay calculation between attempts.onErrorwith a terminal error.() => controller.abort()) fully stops the reconnect loop — an in-flight backoff timer must be cleared, not just the current fetch aborted.onReconnecting?: (attempt: number, delayMs: number) => voidcallback so the admin UI (app/admin/*) can surface connection status if desired (optional — check if any consumer currently uses this method before wiring UI).fetchto simulate: a stream that closes mid-read, a stream that never opens (network error), and a stream that stays healthy — asserting reconnect attempts, backoff delays, and correct cleanup on unsubscribe.Acceptance Criteria
onError.RETRY_MAX_DELAY_MS.onErroris still invoked appropriately so existing/callers' polling-fallback behavior is preserved.npm run typecheck,npm run lint,npm testpass.Likely Affected Files/Directories
lib/api/live.tsdocs/admin-events-stream-contract.mdtest/(new test file, e.g.test/webhook-stream-reconnect.test.ts)