Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-strict-mode-double-exchange.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@onkernel/managed-auth-react": patch
---

Guard the bootstrap effect in `useManagedAuthSession` against React 18+ Strict Mode's mount → cleanup → mount double-invocation. Without the guard, the second mount re-fires `exchangeHandoffCode` with a now-consumed handoff code and the component lands in the error state ("Failed to start session") even when auth would have worked. Tracked per `(sessionId, handoffCode)` so a genuine prop change still triggers a fresh exchange.
37 changes: 32 additions & 5 deletions packages/managed-auth-react/src/session/useManagedAuthSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ export function useManagedAuthSession(
success: false,
error: false,
});
// Tracks the in-flight (or completed) bootstrap exchange so the second
// mount of a React 18+ Strict Mode mount → cleanup → mount cycle can
// adopt the result of the first mount's exchange instead of refiring it
// with a now-consumed handoff code. Keyed by ``{ key }`` (not the raw
// string) so the *identity* of the object identifies a single exchange:
// a real prop change replaces the object, naturally invalidating the
// previous async's setState calls via the staleness check below.
const exchangeRef = useRef<{ key: string } | null>(null);

const stopPolling = useCallback(() => {
if (pollDelayRef.current) {
Expand Down Expand Up @@ -167,18 +175,38 @@ export function useManagedAuthSession(
);

useEffect(() => {
let cancelled = false;
// Strict-Mode-safe one-shot init. Under React 18+ Strict Mode in dev,
// effects run mount → cleanup → mount. The handoff code is one-shot
// server-side, so the original code refired ``exchangeHandoffCode``
// on the second mount and landed in the error state.
//
// A closure-local ``cancelled`` flag doesn't work as a guard either:
// the cleanup would flip it to true and the first mount's in-flight
// exchange would skip its own ``setJwt`` on resolve, leaving the
// component silently stuck with jwt === null (Cursor #10 review).
//
// Instead, store an object on a ref. The second mount sees the same
// ``key`` and short-circuits without touching the ref — so the first
// mount's async resolves, the ref-identity check passes, and the JWT
// is committed. A real (sessionId, handoffCode) change replaces the
// ref with a new object; the previous async's staleness check then
// fails by identity and its setState calls are dropped cleanly.
const exchangeKey = `${sessionId}::${handoffCode}`;
if (exchangeRef.current?.key === exchangeKey) return;
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
const ref = { key: exchangeKey };
exchangeRef.current = ref;

(async () => {
try {
const token = await exchangeHandoffCode(
sessionId,
handoffCode,
options,
);
if (cancelled) return;
if (exchangeRef.current !== ref) return;
setJwt(token);
const initial = await retrieveManagedAuth(sessionId, token, options);
if (cancelled) return;
if (exchangeRef.current !== ref) return;
setState(initial);
const derived = deriveUIState(initial);
if (
Expand Down Expand Up @@ -213,7 +241,7 @@ export function useManagedAuthSession(
setUIState("prime");
}
} catch (err) {
if (cancelled) return;
if (exchangeRef.current !== ref) return;
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
const message =
err instanceof Error ? err.message : "Failed to start session";
setInitError(message);
Expand All @@ -225,7 +253,6 @@ export function useManagedAuthSession(
}
})();
return () => {
cancelled = true;
stopPolling();
};
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down