From ec5a53446507a57296f2f770ba1bb49c6f8dd00e Mon Sep 17 00:00:00 2001 From: Ramnique Singh <30795890+ramnique@users.noreply.github.com> Date: Thu, 2 Jul 2026 11:31:24 +0530 Subject: [PATCH] fix(x/renderer): StrictMode-safe feed lifecycle; drop obsolete runs:fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chat stuck at 'Thinking…' with a completed turn on disk: the stores subscribed to the sessions:events feed in their constructors while cleanup was effect-managed, so React StrictMode's dev-mode mount -> cleanup -> mount cycle tore down the one subscription and never restored it. The renderer then loaded a mid-turn snapshot and was deaf to every event after (the backend turn completed fine). Feed attachment is now an effect-managed, idempotent connect() on both stores; hooks return its disconnect as the effect cleanup. New tests pin the bug: a StrictMode-style connect -> cleanup -> connect store test, and the hook tests now render under a StrictMode wrapper. Also removed the composer's runs:fetch model-freeze effect (the ENOENT noise): sessions carry model/permission per message, so the picker stays live for existing chats. Co-Authored-By: Claude Fable 5 --- .../components/chat-input-with-mentions.tsx | 13 ++----- .../src/hooks/useSessionChat.test.tsx | 13 +++++-- .../apps/renderer/src/hooks/useSessionChat.ts | 2 +- apps/x/apps/renderer/src/hooks/useSessions.ts | 3 +- .../src/lib/session-chat/store.test.ts | 37 +++++++++++++++++-- .../renderer/src/lib/session-chat/store.ts | 35 +++++++++++++----- 6 files changed, 74 insertions(+), 29 deletions(-) diff --git a/apps/x/apps/renderer/src/components/chat-input-with-mentions.tsx b/apps/x/apps/renderer/src/components/chat-input-with-mentions.tsx index 182ed20a..4ed34844 100644 --- a/apps/x/apps/renderer/src/components/chat-input-with-mentions.tsx +++ b/apps/x/apps/renderer/src/components/chat-input-with-mentions.tsx @@ -342,22 +342,15 @@ function ChatInputInner({ } }) - // When a run exists, freeze the dropdown to the run's resolved model+provider. + // Sessions runtime: model and permission mode are per-message turn config, + // so nothing is frozen for an existing chat — the picker stays live. useEffect(() => { if (!runId) { setLockedModel(null) setPermissionMode('auto') return } - let cancelled = false - window.ipc.invoke('runs:fetch', { runId }).then((run) => { - if (cancelled) return - if (run.provider && run.model) { - setLockedModel({ provider: run.provider, model: run.model }) - } - setPermissionMode(run.permissionMode ?? 'manual') - }).catch(() => { /* legacy run or fetch failure — leave unlocked */ }) - return () => { cancelled = true } + setLockedModel(null) }, [runId]) useEffect(() => { diff --git a/apps/x/apps/renderer/src/hooks/useSessionChat.test.tsx b/apps/x/apps/renderer/src/hooks/useSessionChat.test.tsx index 30caf6a9..3ec452f2 100644 --- a/apps/x/apps/renderer/src/hooks/useSessionChat.test.tsx +++ b/apps/x/apps/renderer/src/hooks/useSessionChat.test.tsx @@ -1,3 +1,4 @@ +import { StrictMode } from 'react' import { act, renderHook, waitFor } from '@testing-library/react' import { describe, expect, it } from 'vitest' import type { SessionBusEvent } from '@x/shared/src/sessions.js' @@ -69,7 +70,7 @@ function makeDeps() { describe('useSessionChat', () => { it('seeds from the session, follows live events, and routes actions', async () => { const { deps, calls, emit } = makeDeps() - const { result } = renderHook(() => useSessionChat(S1, deps)) + const { result } = renderHook(() => useSessionChat(S1, deps), { wrapper: StrictMode }) await waitFor(() => { expect(result.current.latestTurnId).toBe('turn-1') @@ -109,10 +110,14 @@ describe('useSessionChat', () => { ]) }) - it('unsubscribes from the feed on unmount', async () => { + it('unsubscribes from the feed on unmount (StrictMode double-mounts included)', async () => { const { deps, getUnsubscribed } = makeDeps() - const { unmount } = renderHook(() => useSessionChat(S1, deps)) + const { unmount } = renderHook(() => useSessionChat(S1, deps), { + wrapper: StrictMode, + }) unmount() - expect(getUnsubscribed()).toBe(1) + // StrictMode's simulated cleanup plus the real unmount: every subscribe + // was matched by an unsubscribe. + expect(getUnsubscribed()).toBe(2) }) }) diff --git a/apps/x/apps/renderer/src/hooks/useSessionChat.ts b/apps/x/apps/renderer/src/hooks/useSessionChat.ts index c598330c..6f8ee5ce 100644 --- a/apps/x/apps/renderer/src/hooks/useSessionChat.ts +++ b/apps/x/apps/renderer/src/hooks/useSessionChat.ts @@ -16,7 +16,7 @@ export function useSessionChat( deps: SessionChatStoreDeps = defaultDeps, ) { const [store] = useState(() => new SessionChatStore(deps)) - useEffect(() => () => store.dispose(), [store]) + useEffect(() => store.connect(), [store]) useEffect(() => { void store.setSession(sessionId) }, [store, sessionId]) diff --git a/apps/x/apps/renderer/src/hooks/useSessions.ts b/apps/x/apps/renderer/src/hooks/useSessions.ts index 872c6a6c..9c4a7213 100644 --- a/apps/x/apps/renderer/src/hooks/useSessions.ts +++ b/apps/x/apps/renderer/src/hooks/useSessions.ts @@ -13,8 +13,9 @@ const defaultDeps: SessionChatStoreDeps = { export function useSessions(deps: SessionChatStoreDeps = defaultDeps) { const [store] = useState(() => new SessionListStore(deps)) useEffect(() => { + const disconnect = store.connect() void store.load() - return () => store.dispose() + return disconnect }, [store]) const snapshot = useSyncExternalStore(store.subscribe, store.getSnapshot) const client = deps.client diff --git a/apps/x/apps/renderer/src/lib/session-chat/store.test.ts b/apps/x/apps/renderer/src/lib/session-chat/store.test.ts index c8b95bcf..96a23abb 100644 --- a/apps/x/apps/renderer/src/lib/session-chat/store.test.ts +++ b/apps/x/apps/renderer/src/lib/session-chat/store.test.ts @@ -86,20 +86,26 @@ class FakeClient implements SessionsClient { function makeStore() { const client = new FakeClient() let emit: SessionFeedListener = () => undefined + let subscribed = 0 let unsubscribed = 0 const store = new SessionChatStore({ client, subscribeFeed: (listener) => { + subscribed += 1 emit = listener return () => { unsubscribed += 1 + emit = () => undefined } }, }) + const disconnect = store.connect() return { client, store, + disconnect, emit: (event: SessionBusEvent) => emit(event), + getSubscribed: () => subscribed, getUnsubscribed: () => unsubscribed, } } @@ -245,12 +251,36 @@ describe('SessionChatStore', () => { expect(store.getSnapshot().chatState?.conversation).toEqual([]) }) - it('surfaces load errors and disposes its feed subscription', async () => { - const { store, getUnsubscribed } = makeStore() + it('surfaces load errors and disconnects its feed subscription', async () => { + const { store, disconnect, getUnsubscribed } = makeStore() await store.setSession('missing-session') expect(store.getSnapshot().error).toMatch(/session not found/) - store.dispose() + disconnect() expect(getUnsubscribed()).toBe(1) + void store + }) + + it('survives a StrictMode-style connect -> cleanup -> connect cycle', async () => { + // React StrictMode runs every effect's mount/cleanup/mount cycle in dev; + // the feed must re-attach or the store goes permanently deaf (the bug + // this test pins: a constructor-made subscription torn down by the first + // cleanup was never restored, leaving the chat stuck at "Thinking…"). + const { client, store, disconnect, emit, getSubscribed } = makeStore() + client.sessions.set(S1, sessionState(S1, [])) + disconnect() + const disconnect2 = store.connect() + expect(getSubscribed()).toBe(2) + + await store.setSession(S1) + emit(turnEvent(S1, 'turn-1', created('turn-1', S1, user('go')))) + emit(turnEvent(S1, 'turn-1', requested('turn-1', 0, [user('go')]))) + emit(turnEvent(S1, 'turn-1', completed('turn-1', 0, assistantText('done')))) + emit(turnEvent(S1, 'turn-1', turnCompleted('turn-1'))) + expect(store.getSnapshot().chatState?.isProcessing).toBe(false) + expect( + store.getSnapshot().chatState?.conversation.filter(isChatMessage).map((m) => m.content), + ).toEqual(['go', 'done']) + disconnect2() }) }) @@ -267,6 +297,7 @@ describe('SessionListStore', () => { return () => undefined }, }) + store.connect() await store.load() expect(store.getSnapshot().sessions.map((s) => s.sessionId).sort()).toEqual(['a', 'b']) diff --git a/apps/x/apps/renderer/src/lib/session-chat/store.ts b/apps/x/apps/renderer/src/lib/session-chat/store.ts index 8dc3b1f7..42408d4c 100644 --- a/apps/x/apps/renderer/src/lib/session-chat/store.ts +++ b/apps/x/apps/renderer/src/lib/session-chat/store.ts @@ -39,7 +39,8 @@ export interface SessionChatStoreDeps { // hook is a thin useSyncExternalStore subscription over it. export class SessionChatStore { private readonly client: SessionsClient - private readonly unsubscribeFeed: () => void + private readonly subscribeFeed: (listener: SessionFeedListener) => () => void + private feedDisconnect: (() => void) | null = null private readonly listeners = new Set<() => void>() private sessionId: string | null = null @@ -63,12 +64,20 @@ export class SessionChatStore { constructor(deps: SessionChatStoreDeps) { this.client = deps.client - this.unsubscribeFeed = deps.subscribeFeed(this.onFeedEvent) + this.subscribeFeed = deps.subscribeFeed } - dispose(): void { - this.unsubscribeFeed() - this.listeners.clear() + // Feed attachment is effect-managed and idempotent so React StrictMode's + // mount -> cleanup -> mount cycle re-attaches cleanly (a constructor-made + // subscription would be torn down by the first cleanup and never restored). + connect(): () => void { + if (!this.feedDisconnect) { + this.feedDisconnect = this.subscribeFeed(this.onFeedEvent) + } + return () => { + this.feedDisconnect?.() + this.feedDisconnect = null + } } subscribe = (onChange: () => void): (() => void) => { @@ -241,7 +250,8 @@ export interface SessionListSnapshot { export class SessionListStore { private readonly client: SessionsClient - private readonly unsubscribeFeed: () => void + private readonly subscribeFeed: (listener: SessionFeedListener) => () => void + private feedDisconnect: (() => void) | null = null private readonly listeners = new Set<() => void>() private entries = new Map() private loading = true @@ -249,12 +259,17 @@ export class SessionListStore { constructor(deps: SessionChatStoreDeps) { this.client = deps.client - this.unsubscribeFeed = deps.subscribeFeed(this.onFeedEvent) + this.subscribeFeed = deps.subscribeFeed } - dispose(): void { - this.unsubscribeFeed() - this.listeners.clear() + connect(): () => void { + if (!this.feedDisconnect) { + this.feedDisconnect = this.subscribeFeed(this.onFeedEvent) + } + return () => { + this.feedDisconnect?.() + this.feedDisconnect = null + } } subscribe = (onChange: () => void): (() => void) => {