Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions app/src/components/editor/AssistantPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<Map<string, Record<string, unknown>>>(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];
Expand All @@ -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;
}
}
Expand Down
32 changes: 32 additions & 0 deletions app/tests/assistant.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});
Loading