feat: subscribe to managed auth state via SSE instead of polling#9
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e-export type Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Firetiger deploy monitoring skipped This PR didn't match the auto-monitor filter configured on your GitHub connection:
Reason: PR modifies client-side auth UI logic (useManagedAuthSession hook), not kernel API endpoints or Temporal workflows. To monitor this PR anyway, reply with |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
masnwilliams
left a comment
There was a problem hiding this comment.
nice cleanup — moving off polling is a real win, and the terminal-state guarding via terminalRef is well-threaded across connectStream / scheduleReconnect / resyncAndConnect / onClose. notes grouped by severity.
correctness
mergeStateEventdrops four event fields —useManagedAuthSession.ts:49-71never readsflow_type,post_login_url,live_view_url, orhosted_url. The server emits all of them (kernel/kernelcmd/api/api/managed_auth.go:1731,1775,1779+) and they're declared onManagedAuthStateEventData(types.ts:63, 73-75), but they never reach consumers. Either remove from the event type or extendManagedAuthResponseand merge them through.err.statusaccessed via untyped cast —useManagedAuthSession.ts:222-223.retrieveManagedAuththrowsManagedAuthApiError; useinstanceoffor type safety and to match theonErrorbranch on line 241.
robustness
- Reconnect loop has no upper bound —
useManagedAuthSession.ts:181-192. If the backend is down, this reconnects forever at the 15s cap with no signal to the user. The UI stays ondiscovering/awaiting_inputetc. while the stream silently flaps. Consider a max-attempts →errortransition, or expose areconnectingflag from the hook. onCloseonly fires on natural stream end —lib/api.ts:263. On non-OK response (203), missing body (210), fatal error (255), or unexpected reader exception (264), onlyonErrorfires;onClosenever runs. Worth documenting onManagedAuthStreamHandlers, or always callingonCloseafter terminalonError.- CRLF SSE frames silently dropped —
lib/api.ts:219accepts\r\n\r\nas a separator, butlib/api.ts:233splits by\nonly. CRLF input leaves a trailing\ron each line, soeventTypebecomes"managed_auth_state\r"and the comparison on line 238 fails. Server uses\ntoday (verified), so this is latent — but the parser advertises CRLF support it doesn't deliver. Either strip trailing\rper line, or only accept\n\nas separator.
minor / nits
lib/api.ts:255— hard-coded500status on stream-error frames. Other non-HTTP error paths in this file use0(lines 210, 268);0is more consistent.lib/api.ts:242-244— malformedmanaged_auth_stateJSON is silently swallowed. The fatal-error branch (252) surfaces parse failures viafatal=true; consider surfacing the state-event parse error as a non-fatalonErrorso consumers know a payload dropped.useManagedAuthSession.ts:154-155—if (!base) return;is unreachable in the current flow (init always setsstateRefbeforeconnectStream). Drop or convert to an assertion.
|
regarding your reviews, have incorporated everything except points 2 and 3 of Nit 2: JSON should not be malformed since we are using Nit 3: yes, technically true for now, but retaining the defense check in case code refactors over time. Harmless defense as of now. |
|
thanks for the quick turnaround — nearly everything addressed :bufo-blob-clap: addressed:
still open (both nits, non-blocking):
|
masnwilliams
left a comment
There was a problem hiding this comment.
ack on the two nits — both reasonable as-is. nice work :bufo-blob-clap:
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 87d13ae. Configure here.
| } | ||
| return; | ||
| } | ||
| connectStream(t); |
There was a problem hiding this comment.
Stale closure chain in reconnection self-reference
Medium Severity
resyncAndConnect calls connectStream(t) using the closure reference from the render when the outer connectStream was created. Since connectStream depends on options (a new object every render), fireSuccessOnce, and fireErrorOnce, the reconnection path perpetually reuses the stale versions of these from the original stream setup. Unlike the old polling code — where stopPolling/startPolling on each submit cycle picked up the latest pollOnce with fresh callbacks — the SSE stream's handlers are never refreshed, so onSuccess/onError changes are permanently ignored for terminal states reached via the stream or its reconnections.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 87d13ae. Configure here.
There was a problem hiding this comment.
theoretical edge case


Summary
Replaces the 2s polling loop in
useManagedAuthSessionwith a subscription to the existing/auth/connections/{id}/eventsSSE endpoint.Resolves KERNEL-1268. Also fixes the form flash-back race condition described in KERNEL-1256.
Changes
Work in progress — PR opened as a scaffold for incremental development.
Test plan
text/event-streamconnection instead of recurringGET /auth/connections/{id}pollsonSuccess/onErrorfire exactly once🤖 Generated with Claude Code
Note
Medium Risk
Replaces the session state update mechanism with a custom SSE stream parser plus reconnect logic, which can affect auth flow progression and error/terminal handling if the stream parsing or reconnection behavior is off.
Overview
Managed auth session updates now use server-sent events instead of 2s polling.
useManagedAuthSessionreplaces its interval-based refresh loop with a subscription to/auth/connections/{id}/events, adds exponential backoff reconnection (isReconnecting), and ensures terminal states stop the stream and fireonSuccess/onErroronly once.Adds a fetch-based SSE implementation (
streamManagedAuthEvents) that can sendAuthorizationheaders, parsesmanaged_auth_state/errorevents, and introducesManagedAuthStateEventDataplus new optional response fields (flow_type,post_login_url,live_view_url,hosted_url).Reviewed by Cursor Bugbot for commit 87d13ae. Bugbot is set up for automated code reviews on this repo. Configure here.