Skip to content

feat: subscribe to managed auth state via SSE instead of polling#3

Open
masnwilliams wants to merge 5 commits intomainfrom
hypeship/sse-auth-events
Open

feat: subscribe to managed auth state via SSE instead of polling#3
masnwilliams wants to merge 5 commits intomainfrom
hypeship/sse-auth-events

Conversation

@masnwilliams
Copy link
Copy Markdown
Contributor

@masnwilliams masnwilliams commented May 4, 2026

Summary

  • Swap the 2s polling loop in useManagedAuthSession for a subscription to the existing /auth/connections/{id}/events SSE endpoint.
  • Add streamManagedAuthEvents to the api client. Uses fetch + ReadableStream (not EventSource) so the Authorization: Bearer header is honored — EventSource can't send custom headers.
  • Drop the POST_SUBMIT_DELAY_MS polling-resume hack. The server only emits on real status/step changes, so an optimistic submitting UI sticks until the workflow actually advances; no more snap-back to awaiting_input.
  • Keep the initial GET /auth/connections/{id} so we still have id / domain / profile_name for callbacks (the SSE event payload doesn't carry them).
  • Reconnect with exponential backoff on transient stream errors; treat 401 / 410 as session expired.

Test plan

  • Local dev: start a managed auth flow, confirm DevTools shows a single text/event-stream connection instead of recurring GET /auth/connections/{id} polls.
  • Submit credentials and confirm the UI does not flash back to the form before transitioning to submitting / next step.
  • Kill the network tab mid-flow and confirm the stream reconnects.
  • Reach a terminal state (success / failure / expired) and confirm the stream closes cleanly and onSuccess / onError fire exactly once.

Note

Medium Risk
Replaces a simple polling loop with a streaming SSE client plus reconnect/backoff and terminal-state handling, which could affect auth flow reliability and edge-case error behavior.

Overview
Moves managed auth session state updates from 2s polling (GET /auth/connections/{id}) to an authenticated SSE stream (GET /auth/connections/{id}/events), removing the post-submit polling-delay hack that could briefly revert the UI to awaiting_input.

Adds streamManagedAuthEvents (manual SSE parsing over fetch/ReadableStream) plus a new ManagedAuthStateEventData event type, and updates the session hook to merge SSE deltas, fire onSuccess/onError exactly once, and reconnect with exponential backoff (resyncing via retrieveManagedAuth after disconnects and treating 401/410 as expired).

Reviewed by Cursor Bugbot for commit c20d90b. Bugbot is set up for automated code reviews on this repo. Configure here.

Replace 2s polling against /auth/connections/{id} with the existing
/auth/connections/{id}/events SSE endpoint. The server only emits on
status/step change, so submissions no longer race against a stale
poll snapping the UI back to awaiting_input.

Adds streamManagedAuthEvents to the api client (fetch + ReadableStream
SSE reader so the Authorization header is honored). The hook keeps the
initial GET to populate id/domain/profile_name, then merges each state
event into the response and re-derives the UI state. Drops the
post-submit polling delay hack — the optimistic submitting UI sticks
until the server emits a real change.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@masnwilliams masnwilliams marked this pull request as ready for review May 4, 2026 00:55
@firetiger-agent
Copy link
Copy Markdown

Firetiger deploy monitoring skipped

This PR didn't match the auto-monitor filter configured on your GitHub connection:

Any PR that changes the kernel API. Monitor changes to API endpoints (packages/api/cmd/api/) and Temporal workflows (packages/api/lib/temporal) in the kernel repo

Reason: PR modifies the API client and auth endpoints but doesn't change core kernel API endpoints (packages/api/cmd/api/) or Temporal workflows (packages/api/lib/temporal); appears to be a client-side SSE subscription improvement.

To monitor this PR anyway, reply with @firetiger monitor this.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment thread packages/managed-auth-react/src/lib/api.ts
The previous error-event branch fired handlers.onError but kept reading
from the stream. The hook treats onError as terminal — it nulls the
disconnect ref and schedules a reconnect — so the original stream was
left running alongside the new one, both feeding state events into the
hook and racing on disconnectRef on close.

Abort the controller and return after dispatching the error so the
stream contract matches the hook's expectations: onError fires at most
once and is mutually exclusive with onClose.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@masnwilliams masnwilliams requested a review from dcruzeneil2 May 5, 2026 01:59
Comment thread packages/managed-auth-react/src/lib/api.ts Outdated
Group all SDK-mirrored protocol types in lib/types.ts so the drift
surface against @onkernel/sdk is audited in one place. api.ts re-exports
the type to keep the existing import path stable.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@masnwilliams masnwilliams requested a review from dcruzeneil2 May 5, 2026 23:13
Comment thread packages/managed-auth-react/src/session/useManagedAuthSession.ts
Comment thread packages/managed-auth-react/src/session/useManagedAuthSession.ts
Comment thread packages/managed-auth-react/src/lib/api.ts
- ManagedAuthApiError gains a fatal flag. Server-emitted SSE error
  events now mark themselves fatal; the hook treats fatal errors as
  terminal instead of looping reconnect forever.
- On reconnect after a stream drop, refetch the current session via
  GET /auth/connections/{id} before resubscribing, so we don't miss
  state changes that happened during the disconnect window.
- SSE message-separator + line-split now handle \r\n and \r in
  addition to \n, per the spec.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 4 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c20d90b. Configure here.

}
return;
}
connectStream(t);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reconnect can outlive cleanup

Medium Severity

The resyncAndConnect function lacks a cancellation mechanism. If the component unmounts or the session changes during its retrieveManagedAuth request, the stale response can update state and open a new SSE stream for an outdated session.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c20d90b. Configure here.

current === "success" || current === "expired" || current === "error"
? current
: "awaiting_input",
isTerminal(current) ? current : "awaiting_input",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Submit expiry stays active

Medium Severity

submit treats every submission failure as a form error. A 401 or 410 from submit* no longer triggers a resync, so an expired session can remain on awaiting_input and skip onError unless SSE later reports expiry.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c20d90b. Configure here.

website_error: ev.website_error ?? null,
error_message: ev.error_message ?? null,
error_code: ev.error_code ?? null,
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SSO provider state stays stale

Low Severity

mergeStateEvent never updates or clears sso_provider, even though the UI reads it for SSO-specific form copy. After an SSO transition, SSE updates can leave the form using the old provider value from the initial GET.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c20d90b. Configure here.

stateRef.current = fresh;
setState(fresh);
const derived = deriveUIState(fresh);
setUIState(derived);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resync can undo submitting state

Medium Severity

resyncAndConnect always applies the GET result to uiState. If the stream reconnects after a submit but before the backend advances, a stale AWAITING_INPUT response can replace the optimistic submitting state and reintroduce the form snap-back.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c20d90b. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants