Skip to content

Commit 74f27bc

Browse files
Tom-Achacheclaude
andauthored
Skip duplicate handoff exchange under React Strict Mode (#10)
* Skip duplicate handoff exchange under React Strict Mode The bootstrap effect in useManagedAuthSession calls exchangeHandoffCode on mount with no "already started" guard. Under React 18+ Strict Mode, effects run mount → cleanup → mount in dev: the first mount consumes the handoff code server-side, and the second mount re-fires the exchange with an already-consumed code. The component lands in the error state and fires onError("Failed to start session"), even though auth would have worked fine. The existing ``cancelled`` flag only stops React state updates; it can't unsend the HTTP request that consumed the code on the first mount. Add a useRef guard keyed on (sessionId, handoffCode) — same pattern as the existing callbackFiredRef — so the second mount short-circuits while a genuine prop change still re-runs the effect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Adopt in-flight exchange via ref identity instead of cancelled flag Cursor flagged the original guard: under Strict Mode's mount → cleanup → mount cycle, the cleanup flipped the closure-local ``cancelled`` to true while the second mount short-circuited at the new ref guard. The first mount's in-flight exchange would then resolve, read ``cancelled === true``, and skip its own ``setJwt`` call — leaving the component silently stuck with jwt === null after the server had already consumed the handoff code. That's strictly worse than the original bug (visible error → invisible dead state). Replace the closure-local flag with a ref-identity check. Each distinct (sessionId, handoffCode) gets a fresh ``{ key }`` object on ``exchangeRef``; the in-flight async reads the ref and bails iff the ref has been *replaced* (genuine prop change). A Strict Mode re-mount sees the same key and returns early without touching the ref — so when the first mount's async resolves, ``exchangeRef.current === ref`` still holds and the JWT is committed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Track active flag on exchangeRef + always return cleanup Cursor flagged two correct bugs in 90c173b: 1. The short-circuit ``return;`` on the Strict Mode second mount returned undefined, replacing the first mount's cleanup — so when the component actually unmounted later, ``stopPolling`` never ran and any started interval/timeout leaked. 2. Without the closure-local ``cancelled`` flag, a real unmount no longer signaled the in-flight async to stop. The ref-identity check only caught a *prop change*, not unmount, so the async kept firing ``setState``/``startPolling`` against a dead component. Both fix together by adding ``active: boolean`` to the ref object. Cleanup flips it false; the matching-key remount flips it back true. The async checks both ``exchangeRef.current !== ref`` (stale identity) and ``!ref.active`` (real unmount) before each ``setState``. And the short-circuit path now returns the same cleanup function so React preserves it across the synthetic unmount/remount. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Tighten Strict Mode comments per review - Drop the parenthetical pointing at the review tool — process refs don't belong in published source. - Collapse the exchangeRef declaration block to one short sentence naming the fields; defer the invariants to the effect comment so the strict-mode rationale lives in exactly one place. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 52ebb75 commit 74f27bc

2 files changed

Lines changed: 49 additions & 8 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@onkernel/managed-auth-react": patch
3+
---
4+
5+
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.

packages/managed-auth-react/src/session/useManagedAuthSession.ts

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,11 @@ export function useManagedAuthSession(
8686
success: false,
8787
error: false,
8888
});
89+
// Tracks the in-flight bootstrap exchange. ``key`` identifies which
90+
// (sessionId, handoffCode) pair it belongs to; ``active`` is false
91+
// between cleanup and the matching-key remount. See the effect below
92+
// for the invariants these fields enforce.
93+
const exchangeRef = useRef<{ key: string; active: boolean } | null>(null);
8994

9095
const stopPolling = useCallback(() => {
9196
if (pollDelayRef.current) {
@@ -167,18 +172,52 @@ export function useManagedAuthSession(
167172
);
168173

169174
useEffect(() => {
170-
let cancelled = false;
175+
// Strict-Mode-safe one-shot init. Under React 18+ Strict Mode in dev,
176+
// effects run mount → cleanup → mount; the handoff code is one-shot
177+
// server-side, so a naive remount refires the exchange against an
178+
// already-consumed code. Three invariants make this safe:
179+
//
180+
// 1. Guard the exchange by ref identity, not a closure-local
181+
// ``cancelled`` flag — a closure flag set by the synthetic
182+
// cleanup would orphan the first mount's in-flight result.
183+
// 2. Track an ``active`` flag on the ref so the async can
184+
// distinguish a real unmount (active stays false) from a
185+
// Strict Mode unmount/remount (active flips false → true
186+
// synchronously before the async resolves).
187+
// 3. Always return the cleanup, even on the short-circuit path —
188+
// React only keeps the most recent effect's cleanup, so a bare
189+
// ``return`` from the second mount would orphan ``stopPolling``
190+
// and leak the interval at real unmount.
191+
const exchangeKey = `${sessionId}::${handoffCode}`;
192+
const cleanup = () => {
193+
if (exchangeRef.current?.key === exchangeKey) {
194+
exchangeRef.current.active = false;
195+
}
196+
stopPolling();
197+
};
198+
199+
if (exchangeRef.current?.key === exchangeKey) {
200+
// Strict Mode remount of the same exchange: cleanup just flipped
201+
// active=false; flip it back so the in-flight async can commit.
202+
// Return the cleanup so a later *real* unmount still stops polling.
203+
exchangeRef.current.active = true;
204+
return cleanup;
205+
}
206+
207+
const ref = { key: exchangeKey, active: true };
208+
exchangeRef.current = ref;
209+
171210
(async () => {
172211
try {
173212
const token = await exchangeHandoffCode(
174213
sessionId,
175214
handoffCode,
176215
options,
177216
);
178-
if (cancelled) return;
217+
if (exchangeRef.current !== ref || !ref.active) return;
179218
setJwt(token);
180219
const initial = await retrieveManagedAuth(sessionId, token, options);
181-
if (cancelled) return;
220+
if (exchangeRef.current !== ref || !ref.active) return;
182221
setState(initial);
183222
const derived = deriveUIState(initial);
184223
if (
@@ -213,7 +252,7 @@ export function useManagedAuthSession(
213252
setUIState("prime");
214253
}
215254
} catch (err) {
216-
if (cancelled) return;
255+
if (exchangeRef.current !== ref || !ref.active) return;
217256
const message =
218257
err instanceof Error ? err.message : "Failed to start session";
219258
setInitError(message);
@@ -224,10 +263,7 @@ export function useManagedAuthSession(
224263
}
225264
}
226265
})();
227-
return () => {
228-
cancelled = true;
229-
stopPolling();
230-
};
266+
return cleanup;
231267
// eslint-disable-next-line react-hooks/exhaustive-deps
232268
}, [sessionId, handoffCode]);
233269

0 commit comments

Comments
 (0)