From f9869ada6d8ff1e7af4396d6eebd5190850f0873 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 3 Nov 2026 14:02:33 +0700 Subject: [PATCH] fix(chat): a finished turn ends without a Stop hook; Stop no longer flickers Claude Code skips the Stop hook in roughly one turn in fifteen (only the prompt-suggestion helper's SubagentStop arrives), so the main agent stayed "working" until the idle Notification a minute later and the chat kept showing Stop on a finished answer. The transcript's turn_duration entry is now an end of turn too: one newer than the session's last prompt, read while main still works with no tool open, lands it in Waiting like Stop. The chat's Stop button now drops to Send only after every busy signal has been quiet for 2.5 s (Stop pressed still drops at once), so a spinner line that comes and goes while the CLI redraws no longer flips it back and forth, and only the newest "working" read is applied. --- client/src/components/run/PaneChat.tsx | 39 ++++++- .../run/__tests__/PaneChat.test.tsx | 38 +++++++ .../__tests__/turn-end-without-stop.test.js | 106 ++++++++++++++++++ server/routes/hooks.js | 32 ++++++ 4 files changed, 209 insertions(+), 6 deletions(-) create mode 100644 server/__tests__/turn-end-without-stop.test.js diff --git a/client/src/components/run/PaneChat.tsx b/client/src/components/run/PaneChat.tsx index 65ab7cb8..eb07c95a 100644 --- a/client/src/components/run/PaneChat.tsx +++ b/client/src/components/run/PaneChat.tsx @@ -124,6 +124,11 @@ const isSettable = (m: unknown): m is PermissionMode => MODES.some(([v]) => v == const OUTPUT_FETCH_MS = 1000; /** A tool with no known busy line counts as at work this long after its pane last drew. */ const OUTPUT_BUSY_MS = 2000; +/** How long Stop stays up after every busy signal dropped: the screen's busy + * line comes and goes while the CLI redraws, and without a hold the button + * flickered between Stop and Send mid-turn. An explicit end (Stop pressed) + * is not held. */ +const BUSY_HOLD_MS = 2500; const SIDE_KEY = "agenthub.chat.panelWidth"; const clampSide = (f: number) => Math.round(Math.min(0.7, Math.max(0.2, f)) * 100) / 100; const outlineKey = (sessionId: string) => `agenthub.chat.panel.${sessionId}`; @@ -140,6 +145,23 @@ const storedSide = () => { * older; every Claude model since (Sonnet/Opus 4.6+, the 5 family) has 1M * (LiteLLM's list, 2026-10). Null for another vendor's model. */ // ponytail: a fixed rule, not a table; read LiteLLM's max_input_tokens if a new 200K model ships. +/** `value`, except that a drop to false shows only once it has lasted + * `holdMs`; a change of `release` (Stop pressed) lets it drop at once. */ +export function useHeldTrue(value: boolean, holdMs: number, release: unknown): boolean { + const [held, setHeld] = useState(value); + const released = useRef(release); + useEffect(() => { + if (value) return setHeld(true); + if (released.current !== release) { + released.current = release; + return setHeld(false); + } + const timer = window.setTimeout(() => setHeld(false), holdMs); + return () => window.clearTimeout(timer); + }, [value, holdMs, release]); + return value || held; +} + export function contextWindow(model: string | null): number | null { // A Codex/omp chat reports its own window (ContextMeter's `window`). if (!model?.startsWith("claude-")) return null; @@ -622,13 +644,18 @@ export const PaneChat = memo(function PaneChat({ const [waiting, setWaiting] = useState(false); // Stop presses, for the conversation's live line (issue #74). const [stops, setStops] = useState(0); - const readWorking = useCallback( - () => + // Reads overlap (every throttled transcript change starts one); only the + // newest may land, or a slow older answer put back a state already left. + const readSeq = useRef(0); + const readWorking = useCallback(() => { + const seq = ++readSeq.current; + return ( api.agents .list({ session_id: sessionId, status: "working" }) // With session_id the server answers every agent of the session, its // status filter ignored: check the status here. .then((r) => { + if (seq !== readSeq.current) return; const main = r.agents.filter((a) => a.type === "main"); // A fresh start, resume or `/clear` leaves main "working" at an // empty prompt (SessionStart's flag) until the first prompt. @@ -637,9 +664,9 @@ export const PaneChat = memo(function PaneChat({ ); setWaiting(main.some((a) => agentAwaitingReason(a) === "notification")); }) - .catch(() => {}), - [sessionId] - ); + .catch(() => {}) + ); + }, [sessionId]); useEffect(() => { void readWorking(); }, [readWorking]); @@ -714,7 +741,7 @@ export const PaneChat = memo(function PaneChat({ }, 700); return () => window.clearInterval(timer); }, [runId, onScreen]); - const busy = working || screenBusy; + const busy = useHeldTrue(working || screenBusy, BUSY_HOLD_MS, stops); const asking = waiting || menu !== null; // Gone until the pane redraws: no second press on a stale menu. const choose = (index: number) => { diff --git a/client/src/components/run/__tests__/PaneChat.test.tsx b/client/src/components/run/__tests__/PaneChat.test.tsx index 9237939c..a325f40f 100644 --- a/client/src/components/run/__tests__/PaneChat.test.tsx +++ b/client/src/components/run/__tests__/PaneChat.test.tsx @@ -215,6 +215,10 @@ describe("PaneChat — Stop", () => { await act(async () => { await vi.advanceTimersByTimeAsync(5000); }); + // Send comes back once nothing has said busy for BUSY_HOLD_MS. + await act(async () => { + await vi.advanceTimersByTimeAsync(2600); + }); await waitFor(() => expect(screen.queryByTestId("pane-chat-stop")).toBeNull()); } finally { vi.useRealTimers(); @@ -233,6 +237,10 @@ describe("PaneChat — Stop", () => { await act(async () => { await vi.advanceTimersByTimeAsync(800); }); + // Send comes back once nothing has said busy for BUSY_HOLD_MS. + await act(async () => { + await vi.advanceTimersByTimeAsync(2600); + }); await waitFor(() => expect(screen.queryByTestId("pane-chat-stop")).toBeNull()); } finally { vi.useRealTimers(); @@ -240,6 +248,32 @@ describe("PaneChat — Stop", () => { } }); + it("does not flicker to Send when the busy line drops out for a moment", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + vi.mocked(readRunBusy).mockReturnValue(true); + try { + renderChat(); + expect(await screen.findByTestId("pane-chat-stop")).toBeInTheDocument(); + // A redraw: the spinner line is gone for a poll or two. + vi.mocked(readRunBusy).mockReturnValue(false); + await act(async () => { + await vi.advanceTimersByTimeAsync(1500); + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(50); + }); + expect(screen.getByTestId("pane-chat-stop")).toBeInTheDocument(); + vi.mocked(readRunBusy).mockReturnValue(true); + await act(async () => { + await vi.advanceTimersByTimeAsync(800); + }); + expect(screen.getByTestId("pane-chat-stop")).toBeInTheDocument(); + } finally { + vi.useRealTimers(); + vi.mocked(readRunBusy).mockReturnValue(false); + } + }); + it("counts a tool with no known busy line as working while its pane draws", async () => { vi.useFakeTimers({ shouldAdvanceTime: true }); vi.mocked(readRunBusy).mockReturnValue(null); @@ -261,6 +295,10 @@ describe("PaneChat — Stop", () => { await act(async () => { await vi.advanceTimersByTimeAsync(2800); }); + // Send comes back once nothing has said busy for BUSY_HOLD_MS. + await act(async () => { + await vi.advanceTimersByTimeAsync(2600); + }); await waitFor(() => expect(screen.queryByTestId("pane-chat-stop")).toBeNull()); } finally { vi.useRealTimers(); diff --git a/server/__tests__/turn-end-without-stop.test.js b/server/__tests__/turn-end-without-stop.test.js new file mode 100644 index 00000000..8b7e0dc3 --- /dev/null +++ b/server/__tests__/turn-end-without-stop.test.js @@ -0,0 +1,106 @@ +/** + * @file Regression test: a turn the transcript says has ended lands the main + * agent in Waiting even when Claude Code never sends the Stop hook (it skips + * it in some turns — only the prompt-suggestion helper's SubagentStop + * arrives). Before, the agent stayed `working` until the idle Notification a + * minute later and the chat showed Stop on a finished answer. A turn end OLDER + * than the last prompt, or one read on new work (UserPromptSubmit), must not. + * @author Nguyễn Ngọc Trí Vĩ + */ + +const { describe, it, before, after } = require("node:test"); +const assert = require("node:assert/strict"); +const path = require("path"); +const fs = require("fs"); +const os = require("os"); +const http = require("http"); + +const STAMP = `turn-end-${Date.now()}-${process.pid}`; +const TMP = path.join(os.tmpdir(), STAMP); +process.env.DASHBOARD_DB_PATH = path.join(TMP, "dashboard.db"); +process.env.CLAUDE_HOME = path.join(TMP, "home"); +process.env.DASHBOARD_DATA_DIR = path.join(TMP, "data"); + +const { createApp, startServer } = require("../index"); +const { stmts } = require("../db"); + +let server; +let BASE; + +function post(body) { + return new Promise((resolve, reject) => { + const url = new URL("/api/hooks/event", BASE); + const payload = JSON.stringify(body); + const r = http.request( + { + hostname: url.hostname, + port: url.port, + path: url.pathname, + method: "POST", + headers: { "Content-Type": "application/json", "Content-Length": Buffer.byteLength(payload) }, + }, + (res) => { + res.resume(); + res.on("end", () => resolve(res.statusCode)); + } + ); + r.on("error", reject); + r.write(payload); + r.end(); + }); +} + +const line = (o) => JSON.stringify(o) + "\n"; +const user = (text) => ({ type: "user", message: { role: "user", content: text } }); +const reply = (id) => ({ + type: "assistant", + message: { id, model: "claude-sonnet-5", content: [{ type: "text", text: "ok" }], usage: { input_tokens: 1, output_tokens: 1 } }, +}); +const turnEnd = (ts) => ({ type: "system", subtype: "turn_duration", durationMs: 900, timestamp: ts }); + +before(async () => { + fs.mkdirSync(TMP, { recursive: true }); + server = await startServer(createApp(), 0); + BASE = `http://127.0.0.1:${server.address().port}`; +}); + +after(() => { + server?.close(); + fs.rmSync(TMP, { recursive: true, force: true }); +}); + +describe("a finished turn without a Stop hook", () => { + it("lands in Waiting on the next hook that reads the turn's end", async () => { + const cwd = path.join(TMP, "work"); + fs.mkdirSync(cwd, { recursive: true }); + const sid = "12121212-3434-5656-7878-909090909090"; + const tpath = path.join(TMP, `${sid}.jsonl`); + const event = (extra = {}) => ({ session_id: sid, cwd, transcript_path: tpath, ...extra }); + const main = () => stmts.getAgent.get(`${sid}-main`); + + fs.writeFileSync(tpath, line(user("hello"))); + assert.equal(await post({ hook_type: "UserPromptSubmit", data: event({ prompt: "hello" }) }), 200); + assert.equal(main().status, "working"); + + // The turn ends; Claude Code writes turn_duration but sends no Stop — + // only a helper agent's SubagentStop arrives. + fs.appendFileSync(tpath, line(reply("m1")) + line(turnEnd(new Date().toISOString()))); + assert.equal( + await post({ hook_type: "SubagentStop", data: event({ agent_id: "a1", agent_type: "" }) }), + 200 + ); + assert.equal(main().status, "waiting"); + assert.equal(stmts.getSession.get(sid).awaiting_reason, "stop"); + + // A new prompt: that turn's old end must not end the new one. + await new Promise((r) => setTimeout(r, 5)); + fs.appendFileSync(tpath, line(user("again"))); + assert.equal(await post({ hook_type: "UserPromptSubmit", data: event({ prompt: "again" }) }), 200); + assert.equal(main().status, "working"); + assert.equal( + await post({ hook_type: "SubagentStop", data: event({ agent_id: "a2", agent_type: "" }) }), + 200 + ); + assert.equal(main().status, "working", "no turn end newer than the prompt yet"); + }); +}); diff --git a/server/routes/hooks.js b/server/routes/hooks.js index f06b80e4..5a313b84 100644 --- a/server/routes/hooks.js +++ b/server/routes/hooks.js @@ -294,6 +294,10 @@ const apiErrorSummariesStmt = db.prepare( const turnDurationStampsStmt = db.prepare( "SELECT created_at FROM events WHERE session_id = ? AND event_type = 'TurnDuration'" ); +// When the session's current turn began: the newest UserPromptSubmit. +const lastPromptAtStmt = db.prepare( + "SELECT MAX(created_at) AS at FROM events WHERE session_id = ? AND event_type = 'UserPromptSubmit'" +); // What this process already handled of each session's transcript parse. // `transcriptCache.extract` returns the SAME result object while the file is @@ -1202,6 +1206,7 @@ const processEventTx = db.transaction((hookType, data, tool = "claude", peer = n ? null : tailAfter(result.turnDurations, mark?.lastTurn, (a, b) => a === b); if (freshTurns && freshTurns.length) { + endTurnFromTranscript(sessionId, mainAgentId, hookType, freshTurns); // `turnDurations` is every turn in the transcript (up to 1000), on // every hook: dedupe against ONE read of the recorded timestamps (a // covering-index scan) instead of a query per turn — 23 ms per hook on @@ -1327,6 +1332,33 @@ const processEventTx = db.transaction((hookType, data, tool = "claude", peer = n * that transaction COMMITTED — in order, in the same tick. A frame sent from * inside it described a row a later throw could still roll back. */ +/** + * A turn the transcript says has ended is over, Stop hook or not. Claude Code + * writes a `turn_duration` entry when a turn finishes, but in roughly one turn + * in fifteen it never sends the Stop hook (seen on 2.1.31x: only the + * prompt-suggestion helper's SubagentStop arrives), and the main agent then + * stayed `working` until the idle Notification a minute later — the chat + * showed Stop on a finished answer. So a turn end NEWER than the session's + * last prompt, read while the main agent is still working with no tool open, + * lands it in Waiting exactly like Stop does. A UserPromptSubmit/PreToolUse is + * new work and never ends a turn: its own read may still carry the previous + * turn's entry. + */ +function endTurnFromTranscript(sessionId, mainAgentId, hookType, turns) { + if (!mainAgentId || hookType === "UserPromptSubmit" || hookType === "PreToolUse") return; + const main = stmts.getAgent.get(mainAgentId); + if (!main || main.status !== "working" || main.current_tool) return; + const prompt = lastPromptAtStmt.get(sessionId)?.at; + const end = turns.reduce((m, td) => (td.timestamp && td.timestamp > m ? td.timestamp : m), ""); + if (!end || (prompt && end <= prompt)) return; + const now = new Date().toISOString(); + stmts.updateAgent.run(null, "waiting", null, null, null, null, mainAgentId); + stmts.setSessionAwaitingInput.run(now, "stop", sessionId); + stmts.setAgentAwaitingInput.run(now, "stop", mainAgentId); + broadcast("session_updated", stmts.getSession.get(sessionId)); + broadcast("agent_updated", stmts.getAgent.get(mainAgentId)); +} + function processEvent(hookType, data, tool, peer = null) { const outer = pendingMarks; pendingMarks = [];