diff --git a/app/src/components/editor/AssistantPanel.tsx b/app/src/components/editor/AssistantPanel.tsx index 53b6afd..5c19858 100644 --- a/app/src/components/editor/AssistantPanel.tsx +++ b/app/src/components/editor/AssistantPanel.tsx @@ -102,10 +102,34 @@ export function AssistantPanel({ useEffect(() => () => abortRef.current?.abort(), []); + // Tracks the input of whichever tool call is currently "running", keyed by + // tool name, so a matching `tool_result` can compute pathsToTouch/ + // shouldMutate synchronously -- a ref, not React state. The previous + // version derived these from *inside* the `setTurns` state-updater + // callback and read them immediately after calling `setTurns()`; React + // does not guarantee that updater has actually run by then (especially + // here, since every event arrives from an async stream-reader loop, not + // a synchronous React event handler), so `onMutated()` was silently + // skipped on every real write -- confirmed live: a real write_file call + // succeeded (file landed on disk, Claude correctly reported success) but + // the Explorer never refreshed, because `shouldMutate` was still read as + // `false` at the point `onMutated()` was gated on it. + const runningToolInputRef = useRef>>(new Map()); + const applyEvent = useCallback( (event: AssistantEvent) => { + if (event.kind === "tool_call") runningToolInputRef.current.set(event.tool, event.input); + let pathsToTouch: string[] = []; let shouldMutate = false; + if (event.kind === "tool_result" && event.ok) { + const input = runningToolInputRef.current.get(event.tool); + if (input) { + pathsToTouch = pathsTouchedBy(event.tool, input); + shouldMutate = true; + } + } + if (event.kind === "tool_result") runningToolInputRef.current.delete(event.tool); setTurns((prev) => { const next = [...prev]; @@ -129,10 +153,6 @@ export function AssistantPanel({ const b = blocks[i]; if (b.kind === "tool" && b.tool === event.tool && b.status === "running") { blocks[i] = { ...b, status: event.ok ? "ok" : "error", summary: event.summary }; - if (event.ok) { - pathsToTouch = pathsTouchedBy(b.tool, b.input); - shouldMutate = true; - } break; } } diff --git a/app/tests/assistant.test.ts b/app/tests/assistant.test.ts index 36befdd..4723342 100644 --- a/app/tests/assistant.test.ts +++ b/app/tests/assistant.test.ts @@ -184,3 +184,35 @@ describe("assistant.ts sendMessage — CLI auth-failure text detection", () => { expect(src).toContain("This server's Claude subscription login isn't available right now"); }); }); + +// Regression guard for a real, live-reproduced bug: AssistantPanel.tsx's +// `applyEvent` used to mutate `pathsToTouch`/`shouldMutate` *inside* the +// `setTurns` state-updater callback, then read those same variables +// immediately after calling `setTurns()`. React does not guarantee an +// updater function has actually run by the time `setState()` returns +// (especially here, since every event arrives from an async stream-reader +// loop, not a synchronous React event handler) — so `onMutated()` was +// silently skipped on every real successful write. Reproduced live end- +// to-end (real browser, real SSE stream, a fake OpenAI-compatible server +// standing in for the LLM): a `write_file` call genuinely succeeded (the +// file landed on disk, the assistant correctly reported success), but the +// file explorer never refreshed to show it. No React Testing Library/jsdom +// harness exists in this project to drive AssistantPanel directly (see the +// permission-mode and CLI-auth-detection guards above for the same +// convention), so this locks in the fix at the source level: the mutation +// flags must be computed from a synchronous ref BEFORE `setTurns` is +// called, never read from variables a state updater was responsible for +// setting. +describe("AssistantPanel.tsx applyEvent — no setState-timing hazard on file-mutation detection", () => { + it("computes pathsToTouch/shouldMutate before calling setTurns, via a ref, not by reading variables a state updater sets", () => { + const src = readFileSync(path.join(__dirname, "..", "src", "components", "editor", "AssistantPanel.tsx"), "utf8"); + const setTurnsIdx = src.indexOf("setTurns((prev) => {\n const next = [...prev];\n const last = next[next.length - 1];\n if (!last || last.role !== \"assistant\")"); + const pathsIdx = src.indexOf("let pathsToTouch"); + expect(setTurnsIdx).toBeGreaterThan(-1); + expect(pathsIdx).toBeGreaterThan(-1); + // The mutation-flag computation must appear BEFORE the setTurns call in + // source order -- i.e. it no longer depends on that updater having run. + expect(pathsIdx).toBeLessThan(setTurnsIdx); + expect(src).toContain("runningToolInputRef"); + }); +});