Skip to content

Fix skip-prime discovery state recovery - #33

Merged
aylee855 merged 6 commits into
mainfrom
hypeship/fix-skip-prime-refresh
Sep 16, 2026
Merged

aylee855 merged 6 commits into
mainfrom
hypeship/fix-skip-prime-refresh

Conversation

@aylee855

@aylee855 aylee855 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Preserve already-ready login fields in the React SDK and recover from missing or overlapping state updates without waiting for the event stream to disconnect.

  • Keep the initial API-derived state when appearance.layout.skipPrimeStep=true.
  • Include Keep a ready login form when the prime step is dismissed #34's existing continue-path fix and regression test for skipPrimeStep=false; Keep a ready login form when the prime step is dismissed #34 itself is unchanged by this branch.
  • Move discovery, reconnect, and stale-submission snapshot recovery into one session transport controller.
  • Coalesce state requests. If a live event overlaps a snapshot, keep the displayed state and schedule another read with backoff, even if discovery has ended. A local arrival counter is not treated as an authoritative server revision.
  • Invalidate and abort obsolete snapshots on submission, session change, terminal state, and unmount. Ignore callbacks from replaced streams.

Expected behavior

skipPrimeStep=true skips the initial consent/start screen. false shows that screen until the user clicks continue. Both paths preserve readiness already fetched by the SDK. If the initial snapshot still says discovering and readiness occurs before the stream connects, the silent-discovery refresh recovers it.

Setting skipPrimeStep=false avoids the original automatic-start reset, but is not a universal workaround: the separate continue-path reset is fixed by #34.

Recovery and scope

  • One shared snapshot reconciliation operation, with one active state request per transport. Reconnect can restore the event stream while that request is pending.
  • Quiet discovery refreshes after 15 seconds; transient failures and ordering conflicts retry with backoff from 1 second up to 15 seconds. There is no routine polling while input is ready, but an unresolved conflict still requires reconciliation.
  • Submissions cancel pending recovery; stale-interaction responses use the same reconciliation path. Terminal handling stops timers and requests.
  • Reconnect and snapshot retries have separate backoff counters. A live event resets reconnect backoff without cancelling convergence after conflicting snapshots.
  • If a stale-interaction refresh temporarily fails, restore the UI from the latest known state so the rejection is visible while recovery continues; preserve terminal states.
  • Shared hook across all domains and both start modes. No dependencies, public component API, backend, schema, or domain-skill changes.
  • Existing consumers must adopt a published package containing the fix; merging alone does not update their installations.

Affected released code

Prior package inspection found the initial-state overwrite in 0.1.0, 0.2.0, 0.3.0, 0.4.0, 0.4.1, 0.5.0, 0.5.1, 0.5.2, and 0.5.3. The first three use polling, which normally corrects the reset; the prolonged event-stream stall is established as a code risk for the checked 0.4.x and 0.5.x releases. This is not evidence of observed impact in every version.

Validation

  • Added a deterministic reproduction of snapshot SUCCESS overlapping live AWAITING_INPUT; it failed in both start modes before this rewrite.
  • bun test: 81 passed, 0 failed at 5954737, including combined Keep a ready login form when the prime step is dismissed #34 coverage, newer and older overlapping snapshots, repeated conflicts, transient recovery failures, coalesced reconnects, active submissions, session changes, expiry, and cleanup.
  • Reproduced both follow-up bugs before fixing them: a recovered stream still inherited inflated reconnect backoff, and a transient stale-submission refresh left the submitting UI visible. Both now pass. Added guards for continued backoff when the stream never recovers and preservation of terminal success during stale recovery.
  • bun run build, bun run typecheck, bun run format:check, and git diff --check: passed.
  • Deslop and self-review completed; the shared operation replaces duplicated ordering and expiry logic.
  • Two fresh live login tests with manual OTP entry passed at 4812aa5 on September 16, 2026, exercising the rewritten transport in both skipPrimeStep=true and false modes. In both cases the backend was already awaiting OTP input before opening the hosted UI. The automatic path showed the field; the manual path showed Continue and then allowed OTP submission. Both reached UI and backend success without a manual refresh.
  • Backend OTP-acceptance-to-success timings were approximately 16.3 seconds and 9.0 seconds, respectively; these are not click-to-screen measurements or a comparative performance benchmark.
  • The follow-up fixes at 5954737 have automated regression coverage and full validation above; the live OTP tests were not repeated after those two fixes.

Release status

Open for review; not merged or published. bun run lint remains unsupported because no workspace package defines a lint script.


Note

Medium Risk
Changes client-side auth session state machine and timing around SSE, snapshots, and submissions; mistakes could cause stuck spinners, wrong UI, or missed expiry, but scope is limited to the React SDK with heavy test coverage.

Overview
Refactors managed-auth session sync into a new createSessionTransport layer that owns the event stream, timed discovery snapshots, reconnect backoff, and GET reconciliation—with retrieveManagedAuth now accepting an AbortSignal so stale reads can be cancelled.

useManagedAuthSession delegates streaming/recovery to that transport instead of inline reconnect logic. startFlow sets UI from the latest server state (so a form already at AWAITING_INPUT stays visible after the prime/consent step) and no longer forces discovering. Stale-interaction submits go through transport.resync(); submissions abort in-flight snapshot work so UI is not rolled back mid-submit.

Discovery still silently re-fetches after 15s while discovering; overlapping live SSE events vs snapshots trigger another read with backoff instead of applying an older GET over newer UI. Extensive discovery/recovery tests were added.

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

@vercel

vercel Bot commented Sep 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
managed-auth-react-demo Ready Ready Preview Sep 16, 2026 1:10am UTC

@aylee855
aylee855 marked this pull request as ready for review September 15, 2026 23:18
@ehfeng
ehfeng removed their request for review September 15, 2026 23:52
@ehfeng

ehfeng commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

I reviewed, seems good, but ultimately not my call. removed myself from reviewer

startFlow assumed discovery was still running and forced the UI into a
loading state. When the session had already reached AWAITING_INPUT while
the consent step was on screen, that hid a ready form behind a spinner
the backend had no further transition to clear. Derive the UI state from
the session state we already hold.

@masnwilliams masnwilliams left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the root-cause fix is directionally right and the regression coverage is strong, but two deterministic state-ordering bugs remain. both targeted reproductions fail against this head. the new recovery flow also needs one transport-level reconciliation path rather than another nested timer/request state machine.

validation: 57 existing tests pass; build, typecheck, formatting, and diff checks pass.

Comment thread packages/managed-auth-react/src/session/useManagedAuthSession.ts Outdated
Comment thread packages/managed-auth-react/src/session/useManagedAuthSession.ts
Comment thread packages/managed-auth-react/src/session/useManagedAuthSession.ts Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 using default effort and found 2 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 4812aa5. Configure here.

Comment thread packages/managed-auth-react/src/session/transport.ts
Comment thread packages/managed-auth-react/src/session/useManagedAuthSession.ts

@masnwilliams masnwilliams left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the latest rewrite addresses all three original blockers: manual start preserves ready state, conflicting remote updates converge, and transport recovery now has one canonical controller. i resolved those threads.

one deterministic reconnect-backoff bug remains: a pending snapshot can reopen a failed replacement stream immediately, bypassing the scheduled delay. the existing 81 tests and all build/type/format checks pass; the targeted reproduction for this overlap fails.

Comment thread packages/managed-auth-react/src/session/transport.ts

@masnwilliams masnwilliams left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

re-reviewed latest head. the original blockers are fixed: ready state survives both start modes, overlapping updates converge through the shared transport, and recovery ownership is cleanly extracted. the reconnect/snapshot overlap noted in my follow-up can skip one delay, but backoff state is preserved and the behavior is bounded, so I am treating it as non-blocking. 81 tests plus build, typecheck, formatting, and diff checks pass.

@aylee855
aylee855 merged commit c91e582 into main Sep 16, 2026
5 checks passed
@aylee855
aylee855 deleted the hypeship/fix-skip-prime-refresh branch September 16, 2026 02:08
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.

3 participants