Skip duplicate handoff exchange under React Strict Mode

[Tom-Achache]

Summary

The bootstrap effect in useManagedAuthSession calls exchangeHandoffCode(sessionId, handoffCode) on mount with no "already started" guard. Under React 18+ Strict Mode, effects run mount → cleanup → mount in dev:

1. First mount fires POST /auth/connections/:id/exchange — server consumes the handoff code and returns the JWT.
2. Cleanup sets cancelled = true, which stops React state updates but can't unsend the HTTP request.
3. Second mount re-fires the exchange with a now-consumed code → server rejects → setUIState("error") → onError({ message: "Failed to start session" }), even though auth would have worked fine on the first mount.

The component already uses this pattern (callbackFiredRef) to keep onSuccess/onError one-shot — this PR extends the same idea to the bootstrap exchange itself, keyed on (sessionId, handoffCode) so a genuine prop change still re-runs the effect.

Why a ref instead of server-side idempotency

Either fixes it; happy to do the alternative if you'd prefer:

• Client-side ref guard (this PR): ~3 lines, contained, no API changes.
• Server-side idempotency: cache the issued JWT against the handoff code for a short grace window (e.g., 30s) and return the same JWT on duplicate exchanges. Nice side benefit: also covers network retries and double-clicks.

Changes

• packages/managed-auth-react/src/session/useManagedAuthSession.ts: add exchangedKeyRef (mirrors callbackFiredRef), short-circuit the effect when the same (sessionId, handoffCode) pair has already started.
• Changeset (patch).

Verification

• bun run typecheck ✅
• bun run build ✅ (no behavior change in prod — Strict Mode double-invocation only fires in dev)

Context

We're integrators of @onkernel/managed-auth-react. Hit this when re-enabling Strict Mode in our Next.js app — every local Managed Auth test failed with "Failed to start session" on the second mount, even though the first exchange succeeded. Currently working around it by toggling reactStrictMode: false before testing, which is the kind of thing that's easy to commit by accident. Flagged with the team via Slack, who pointed us here.

---

Note

Medium Risk
Touches managed-auth session bootstrap logic; a mistake could prevent session initialization or leave polling running, though the change is narrowly scoped to guarding duplicate init in dev Strict Mode.

Overview
Prevents useManagedAuthSession from running exchangeHandoffCode twice for the same (sessionId, handoffCode) during React 18+ Strict Mode’s mount→cleanup→mount cycle, avoiding false "Failed to start session" errors.

Replaces the effect’s closure cancelled flag with a ref-tracked (key, active) guard so in-flight exchanges can still commit on Strict Mode remounts, while real unmounts stop polling and async state updates. Adds a patch changeset for @onkernel/managed-auth-react.

^{Reviewed by Cursor Bugbot for commit 26e1885. Bugbot is set up for automated code reviews on this repo. Configure here.}

[firetiger-agent]

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 managed auth client library (packages/managed-auth-react), not kernel API endpoints or Temporal workflows as specified in the filter.

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

[Tom-Achache]

Good catch @cursor — you're right, that's strictly worse than the original bug (visible error → invisible dead state). Pushed 90c173b to fix it.

The closure-local cancelled flag is gone. Staleness is now tracked via the identity of an object on exchangeRef:

const exchangeRef = useRef<{ key: string } | null>(null);

useEffect(() => {
  const exchangeKey = `${sessionId}::${handoffCode}`;
  if (exchangeRef.current?.key === exchangeKey) return;     // Strict Mode re-mount → skip
  const ref = { key: exchangeKey };                          // fresh identity per real exchange
  exchangeRef.current = ref;

  (async () => {
    const token = await exchangeHandoffCode(...);
    if (exchangeRef.current !== ref) return;                 // stale only if ref was *replaced*
    setJwt(token);
    ...
  })();
  return () => { stopPolling(); };                           // no longer flips cancelled
}, [sessionId, handoffCode]);

Walking the four cases:

Scenario	Behavior
Strict Mode mount → cleanup → mount	Second mount sees same key, returns early without touching exchangeRef. First mount's async resolves with exchangeRef.current === ref (same object) → setJwt fires ✅
Real prop change mid-exchange	Effect re-runs with new key, replaces exchangeRef.current with a fresh object. Old async resolves with exchangeRef.current !== ref → stale, setState dropped ✅
Component truly unmounts	useRef is destroyed with the instance. Any in-flight async resolves to setState on an unmounted component — a no-op warning-free in React 18+ ✅
Remount with same props (different instance)	Fresh useRef from a fresh component instance → ref starts null → fresh exchange ✅

Cleanup no longer needs to invalidate the in-flight call — staleness is detected by ref-identity, not by a flag.

Timing note for the Strict Mode case: the synthetic unmount/remount runs synchronously within the same React commit, before any microtasks. So await exchangeHandoffCode can't resolve between cleanup and remount, which would be the only window where the second mount's early-return could orphan the first mount's result.

[cursor]

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 90c173b. Configure here.}

[Tom-Achache]

Both real bugs — pushed 10ada60.

Added 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.

const exchangeKey = `${sessionId}::${handoffCode}`;
const cleanup = () => {
  if (exchangeRef.current?.key === exchangeKey) {
    exchangeRef.current.active = false;
  }
  stopPolling();
};

if (exchangeRef.current?.key === exchangeKey) {
  exchangeRef.current.active = true;   // Strict Mode remount of same exchange
  return cleanup;                       // ← key fix: cleanup preserved, not dropped
}

const ref = { key: exchangeKey, active: true };
exchangeRef.current = ref;

(async () => {
  const token = await exchangeHandoffCode(...);
  if (exchangeRef.current !== ref || !ref.active) return;   // unmount OR prop change
  setJwt(token);
  ...
})();

return cleanup;

Walking the cases:

Scenario	Trace
Strict Mode mount → cleanup → mount	M1: ref={key, active:true}. Cleanup1: active=false. M2: same key → active=true, return cleanup. Async resolves: ref===ref ∧ active=true → setJwt ✅
Real unmount during exchange	M1: ref={key, active:true}. Async starts. Unmount → cleanup: active=false. Async resolves: !active → skip setState; no startPolling ✅
Real unmount after exchange started polling	M1 commits → startPolling. Unmount → cleanup: active=false + stopPolling clears the interval ✅
Prop change (sessionId/handoffCode)	M1: ref1={key1, active:true}. Cleanup1: active=false. M2: new key → fresh ref2={key2, active:true}, async2 starts. Old async resolves: ref1 !== exchangeRef.current → skip ✅
Strict Mode + real unmount after	M1 → cleanup1 (active=false). M2 → active=true again, returns cleanup. Real unmount → cleanup2 runs (same closure): active=false, stopPolling ✅

stopPolling() is idempotent (null-checks before clearing) so the Strict Mode double-call from cleanup1 + cleanup2 is safe.

Both typecheck and build clean.

[masnwilliams]

fix itself looks correct and i've verified locally — strict-mode remount no longer double-exchanges, real unmount stops polling, prop-change still re-exchanges. happy to approve once the comment nits below are addressed.

1. drop the review-tool reference

in useManagedAuthSession.ts the bootstrap-effect comment includes:

The fix has three moving parts (Cursor #10 review iterations):

please remove the parenthetical. review-tooling/process references don't belong in published package source — they don't help a future reader and they leak context about how the PR was reviewed. the three numbered invariants underneath are the load-bearing part and stand on their own.

2. tighten the comment blocks

both comments (the exchangeRef declaration and the effect-body comment) are doing real work explaining a non-obvious invariant, so i don't want to lose them — but they're ~30% longer than they need to be. specifically:

• the exchangeRef block has a paragraph describing what key and active mean that largely restates what the names already convey. the why (strict mode mount→cleanup→mount, can't use closure flag) is what's worth keeping.
• the effect-body block's prose intro before the numbered list duplicates the ref-declaration block. one of the two should carry the strict-mode explanation; the other can just point at the invariants.

aim for: one short paragraph stating the problem + the three numbered invariants. no need to re-explain strict mode twice.

no other changes requested. ship it once these are in and i'll approve.

[Tom-Achache]

Thanks @masnwilliams — both addressed in 26e1885.

1. Dropped the (Cursor #10 review iterations) parenthetical.
2. Consolidated: the exchangeRef declaration is now a one-sentence field-naming comment that points at the effect for the why; the effect block carries the strict-mode rationale + the three numbered invariants once. Net −10 lines from the two blocks.

Typecheck and build clean. Ready for re-review whenever you have a minute.

[masnwilliams]

comment cleanup looks good — single source of truth on the strict-mode explanation and the review-tool reference is gone. fix verified locally. lgtm 🚢