From 01ef6dc2161b73a473e0df73f5a62ef8e1853c25 Mon Sep 17 00:00:00 2001 From: kevin9327 Date: Sat, 22 Aug 2026 07:23:21 +0900 Subject: [PATCH 1/2] Keep a New chat from being undone by the check it raced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `useBotThread` verifies a remembered thread on mount before handing it out, a network round-trip during which the chat renders. `startNew` — the New chat button — runs from an event handler, and its own mint is not held behind that check: `checkKnown` is a read rather than a mint, so the `mintingRef` guard that serialises the two mints is clear while it runs. So the two race. Press New chat while the mount-time check is still in flight, and if the mint resolves first, the check's resolution then runs `setThreadId(existing)` and puts the old thread back: the person asked for a new conversation, watched it appear, and landed in the previous one, with nothing on screen to say why. The `current` flag that resolution already reads only covers the agent changing or the component unmounting, not this. `startNew` now sets a ref the mount-time resolution reads, so a check about the thread the person just left cannot overwrite the one they just asked for. It is reset at the top of the effect, so a new agent resolves from scratch. This is the same shape as the `current` and `mountedRef` guards beside it, and like them it lives in the hook's async coordination — which is why `threadToUse`, the pure decision it wraps, is where the test is and stays green. Co-Authored-By: Claude Opus 4.8 --- app/src/lib/copilot/bot-thread.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/app/src/lib/copilot/bot-thread.ts b/app/src/lib/copilot/bot-thread.ts index 271a3b10..467a4266 100644 --- a/app/src/lib/copilot/bot-thread.ts +++ b/app/src/lib/copilot/bot-thread.ts @@ -144,10 +144,16 @@ export function useBotThread(agentId: string): BotThread { // Shared between the effect's own resolution mint and `startNew`'s mint so the two can never // race each other into two POST /mint calls fighting over the same localStorage slot. const mintingRef = useRef(false); + // Set once `startNew` has taken over, so the mount-time check that is still in flight does not + // then overwrite the fresh thread with the remembered one. `checkKnown` is a read, not a mint, so + // `mintingRef` is clear while it runs and would not have stopped `startNew` starting during it. + const startedNewRef = useRef(false); useEffect(() => { let current = true; mountedRef.current = true; + // A new agent resolves from scratch, so any earlier `startNew` no longer speaks for this one. + startedNewRef.current = false; setThreadId(undefined); setHistory("ready"); @@ -172,7 +178,10 @@ export function useBotThread(agentId: string): BotThread { adoptFresh(); } else { void checkKnown(existing).then((outcome) => { - if (!current) return; + // `startedNewRef` as well as `current`: the agent may have changed (which `current` catches), + // or the person may have pressed New chat while this check was in flight, and a check about + // the thread they just left must not put it back. + if (!current || startedNewRef.current) return; const decision = threadToUse({ remembered: existing, known: outcome.known, @@ -195,6 +204,8 @@ export function useBotThread(agentId: string): BotThread { const startNew = useCallback(() => { if (mintingRef.current) return; mintingRef.current = true; + // From here the mount-time check must not touch the thread: the person has asked for a new one. + startedNewRef.current = true; void mint().then((minted) => { mintingRef.current = false; if (!mountedRef.current) return; From 0b3614fd9eac09e2fb6cc38c0d95c14514309772 Mon Sep 17 00:00:00 2001 From: David McKay Date: Mon, 24 Aug 2026 09:40:48 -0700 Subject: [PATCH 2/2] Latch the new-chat guard only once a thread exists Setting it at the press means a mint that fails leaves the mount-time check disarmed with nothing having replaced the thread, and the chat pane renders only when there is one: an empty screen, no message, no way back but a reload. Moving it after the guard costs nothing, since assignment and check both run to completion on one thread. --- app/src/lib/copilot/bot-thread.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/app/src/lib/copilot/bot-thread.ts b/app/src/lib/copilot/bot-thread.ts index 467a4266..37ec4ace 100644 --- a/app/src/lib/copilot/bot-thread.ts +++ b/app/src/lib/copilot/bot-thread.ts @@ -204,8 +204,6 @@ export function useBotThread(agentId: string): BotThread { const startNew = useCallback(() => { if (mintingRef.current) return; mintingRef.current = true; - // From here the mount-time check must not touch the thread: the person has asked for a new one. - startedNewRef.current = true; void mint().then((minted) => { mintingRef.current = false; if (!mountedRef.current) return; @@ -213,6 +211,20 @@ export function useBotThread(agentId: string): BotThread { // end up worse off — mid-conversation and suddenly unable to send — than before they pressed // it. if (!minted) return; + /* + * Latched here rather than at the press, and the difference is a blank screen. + * + * Set on the press, a mint that then fails leaves the mount-time check disarmed with nothing + * having replaced the thread: `bot.tsx` renders the chat only when there is one, so the person + * is left looking at an empty pane with no message and no way back but a reload. Set here, a + * failed mint disarms nothing and the remembered thread is still restored. + * + * This is not a weaker guard. Assignment and the check below both run to completion on one + * thread, so a `checkKnown` that resolves after this sees it, and one that resolves before it + * has already restored a thread that this line is about to replace — which is what the person + * asked for. + */ + startedNewRef.current = true; remember(agentId, minted); setThreadId(minted); setHistory("ready");