diff --git a/docs/TUI.md b/docs/TUI.md index 0a8ecb48c..2583f46a4 100644 --- a/docs/TUI.md +++ b/docs/TUI.md @@ -52,9 +52,14 @@ branch at its right (`AppShell.promptTopRule` / `promptBottomRule`, `src/tui-opentui/shell.ts`). Both rules cost zero transcript rows because they ride the prompt box's own border. -While a turn is live the lockup slot swaps the wordmark for the phase word — -`thinking`, `streaming 12 tok`, the running tool's name — led by a single -density cell (`rampPulse`, `src/tui-opentui/ramp.ts`). The cell, not the word, +While a turn is live the lockup slot swaps the wordmark for a semantic +activity word — never the raw tool, MCP server, or plugin identifier that is +actually executing. `resolveTurnLabel` (`src/tui-opentui/session-chrome.ts`) +maps execution onto the closed set `ACTIVITY_STATES` exported from that +module (`thinking`, `planning`, `researching`, `building`, `working`, +`waiting`, `stalled`, `stopping`); that export is the source of truth for +what the slot can say, not this list. It is led by a single density cell +(`rampPulse`, `src/tui-opentui/ramp.ts`). The cell, not the word, is what says whether the session is healthy, and it carries four states: | State | Cell | Reads as | diff --git a/src/tui-opentui/runtime-bridge.ts b/src/tui-opentui/runtime-bridge.ts index a461ba7a4..35433db3b 100644 --- a/src/tui-opentui/runtime-bridge.ts +++ b/src/tui-opentui/runtime-bridge.ts @@ -802,12 +802,10 @@ export function attachSessionBridge( const input = { isProcessing: turn.isProcessing, status: turn.status, - awaitingResponse: turn.awaitingResponse, currentToolName: turn.currentToolName, streamingType: turn.streamingType, - streamTokenCount: turn.streamTokenCount, } - const label = resolveTurnLabel(input) + const label = resolveTurnLabel(input, isStalled) if (label === undefined) { // The bottom-left status slot rides the same re-entry as the landing // mark, so it crossfades between phases without a timer of its own. @@ -1037,7 +1035,10 @@ export function attachSessionBridge( setStatusFlash(shell, STALL_NOTICE_MESSAGE) } - paintPhaseAt(nowMs, level !== "quiet") + // Same "is this stalled at all" question `paintPhase` asks above — call + // the one definition (`isStalledForDisplay`) rather than re-deriving it + // from `stallLevel`'s result, so the two call sites can never disagree. + paintPhaseAt(nowMs, isStalledForDisplay(stallArgs)) } setShellBridgeHooks(shell, { diff --git a/src/tui-opentui/session-chrome.test.ts b/src/tui-opentui/session-chrome.test.ts index 12f67eed9..58fcbc203 100644 --- a/src/tui-opentui/session-chrome.test.ts +++ b/src/tui-opentui/session-chrome.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test" import { + ACTIVITY_STATES, classifyAgentSendFailure, classifySendFailureMessage, resolveRampPhase, @@ -9,94 +10,157 @@ import { shouldSettleUiAfterSendFailure, } from "./session-chrome.js" +// The load-bearing guarantee: whatever tool identifier, MCP server name, or +// plugin name the runtime hands us, the rendered ticker string must land in +// the small closed set of human activity states — never the raw identifier. +// A previously-unmapped tool (or one this test doesn't enumerate) must still +// fall back into the set rather than leaking through verbatim. +describe("resolveTurnLabel closed-set guarantee", () => { + const leakingIdentifiers = [ + "run_shell", + "grep", + "read_file", + "write_file", + "edit_file", + "search_files", + "list_dir", + "web_search", + "web_fetch", + "manage_tasks", + "task", + "submit_output", + "ask_operator", + "mcp__glitchtip__authenticate", + "mcp__railway__deploy", + "some_未knownしplugin_tool", + "a-plugin-defined-tool-name", + "totally_unmapped_future_tool", + ] + + for (const currentToolName of leakingIdentifiers) { + test(`"${currentToolName}" resolves to a member of the closed set`, () => { + const label = resolveTurnLabel( + { + isProcessing: true, + status: "running", + currentToolName, + streamingType: "tool", + }, + false, + ) + expect(label).not.toBe(currentToolName) + expect(ACTIVITY_STATES).toContain(label!) + }) + } + + test("a stalled turn renders a distinct stalled state", () => { + const label = resolveTurnLabel( + { + isProcessing: true, + status: "running", + currentToolName: "run_shell", + streamingType: "tool", + }, + true, + ) + expect(label).toBe("stalled") + expect(ACTIVITY_STATES).toContain(label!) + }) + + test("waiting on the operator is distinguishable from working", () => { + const label = resolveTurnLabel( + { + isProcessing: true, + status: "blocked", + currentToolName: "run_shell", + streamingType: "tool", + }, + false, + ) + expect(label).toBe("waiting") + expect(label).not.toBe("working") + expect(ACTIVITY_STATES).toContain(label!) + }) +}) + describe("resolveTurnLabel", () => { test("idle processing off yields no label", () => { expect( - resolveTurnLabel({ - isProcessing: false, - status: "idle", - awaitingResponse: false, - currentToolName: null, - streamingType: null, - }), + resolveTurnLabel( + { + isProcessing: false, + status: "idle", + currentToolName: null, + streamingType: null, + }, + false, + ), ).toBeUndefined() }) - test("blocked gate shows approval wait", () => { + test("blocked gate shows a waiting-on-operator state", () => { expect( - resolveTurnLabel({ - isProcessing: true, - status: "blocked", - awaitingResponse: false, - currentToolName: "run_shell", - streamingType: "tool", - }), - ).toBe("blocked") + resolveTurnLabel( + { + isProcessing: true, + status: "blocked", + currentToolName: "run_shell", + streamingType: "tool", + }, + false, + ), + ).toBe("waiting") }) test("stopping beats tool phase", () => { expect( - resolveTurnLabel({ - isProcessing: true, - status: "stopping", - awaitingResponse: false, - currentToolName: "grep", - streamingType: "tool", - }), + resolveTurnLabel( + { + isProcessing: true, + status: "stopping", + currentToolName: "grep", + streamingType: "tool", + }, + false, + ), ).toBe("stopping") }) - test("tool phase beats generic working", () => { + test("tool phase maps to its semantic activity, never the raw name", () => { expect( - resolveTurnLabel({ - isProcessing: true, - status: "running", - awaitingResponse: true, - currentToolName: "grep", - streamingType: "tool", - }), - ).toBe("grep") + resolveTurnLabel( + { + isProcessing: true, + status: "running", + currentToolName: "grep", + streamingType: "tool", + }, + false, + ), + ).toBe("researching") }) test("thinking and text phases", () => { const base = { isProcessing: true, status: "running" as const, - awaitingResponse: false, currentToolName: null, } expect( - resolveTurnLabel({ ...base, streamingType: "thinking" }), + resolveTurnLabel({ ...base, streamingType: "thinking" }, false), ).toBe("thinking") expect( - resolveTurnLabel({ ...base, streamingType: "text", streamTokenCount: 7 }), - ).toBe("streaming 7 tok") - expect( - resolveTurnLabel({ - ...base, - awaitingResponse: true, - streamingType: null, - }), + resolveTurnLabel({ ...base, streamingType: "text" }, false), ).toBe("working") - }) - - test("text phase with no count yet reads zero", () => { expect( - resolveTurnLabel({ - isProcessing: true, - status: "running", - awaitingResponse: false, - currentToolName: null, - streamingType: "text", - }), - ).toBe("streaming 0 tok") + resolveTurnLabel({ ...base, streamingType: null }, false), + ).toBe("working") }) }) describe("resolveRampPhase", () => { const base = { isProcessing: true, - awaitingResponse: false, currentToolName: null, streamingType: null, } diff --git a/src/tui-opentui/session-chrome.ts b/src/tui-opentui/session-chrome.ts index 943dd506e..deb51e3d1 100644 --- a/src/tui-opentui/session-chrome.ts +++ b/src/tui-opentui/session-chrome.ts @@ -20,11 +20,59 @@ export type TurnStatus = export type TurnLabelInput = { readonly isProcessing: boolean readonly status: TurnStatus - readonly awaitingResponse: boolean readonly currentToolName: string | null readonly streamingType: "text" | "thinking" | "tool" | null - /** Text deltas seen so far this turn; read only while `streamingType` is `text`. */ - readonly streamTokenCount?: number +} + +/** + * Closed set the status ticker is allowed to render. Every path through + * `resolveTurnLabel` returns one of these — never a tool identifier, MCP + * server name, or plugin name. This is what the leak-prevention test checks + * membership against, so it must stay the single source of truth for "what + * can appear in the ticker." + */ +export const ACTIVITY_STATES = [ + "thinking", + "planning", + "researching", + "building", + "working", + "waiting", + "stalled", + "stopping", +] as const + +export type ActivityState = (typeof ACTIVITY_STATES)[number] + +/** + * Execution → activity-state mapping, kept in this one place with an + * explicit fallback so a newly added tool (built-in, MCP, or plugin) renders + * a generic "working" state instead of leaking its identifier — no ticker + * change is required to add a tool correctly. + */ +const TOOL_ACTIVITY_STATES: Readonly> = { + read_file: "researching", + search_files: "researching", + grep: "researching", + list_dir: "researching", + web_search: "researching", + web_fetch: "researching", + write_file: "building", + edit_file: "building", + run_shell: "building", + delete_file: "building", + manage_tasks: "planning", + task: "planning", + advance_workflow: "planning", + tool_search: "researching", + search_agents: "researching", + ask_operator: "waiting", + submit_output: "working", +} + +function activityStateForTool(name: string | null): ActivityState { + if (name === null) return "working" + return TOOL_ACTIVITY_STATES[name] ?? "working" } /** @@ -32,22 +80,25 @@ export type TurnLabelInput = { * unpunctuated — the ramp's color and motion carry the state, so the word only * has to name it. Returns undefined when idle so the phase segment disappears. * - * Text streaming carries a live count (`streaming 7 tok`) rather than the - * bare word: it is the one phase with something to count, and the count is - * what tells the operator the slot is not stalled. + * `isStalled` is the caller's own `isStalledForDisplay` result (see + * stall-watchdog.ts) — this function does not re-derive staleness, it only + * ranks "stalled" against the other phases so the ticker and the ramp never + * disagree about which runs look stuck. Required, not defaulted: a caller + * that forgets to pass it is exactly the bug this state exists to prevent — + * a wedged run silently painted as ordinary work. */ -export function resolveTurnLabel(input: TurnLabelInput): string | undefined { +export function resolveTurnLabel( + input: TurnLabelInput, + isStalled: boolean, +): ActivityState | undefined { if (!input.isProcessing) return undefined - if (input.status === "blocked") return "blocked" + if (input.status === "blocked") return "waiting" if (input.status === "stopping" || input.status === "stopped") { return "stopping" } - if (input.currentToolName !== null) return input.currentToolName - if (input.streamingType === "tool") return "tool" + if (isStalled) return "stalled" + if (input.currentToolName !== null) return activityStateForTool(input.currentToolName) if (input.streamingType === "thinking") return "thinking" - if (input.streamingType === "text") { - return `streaming ${String(input.streamTokenCount ?? 0)} tok` - } return "working" } diff --git a/src/tui-opentui/shell.ts b/src/tui-opentui/shell.ts index 47efdee0d..ba0ae4228 100644 --- a/src/tui-opentui/shell.ts +++ b/src/tui-opentui/shell.ts @@ -69,6 +69,7 @@ import { type LockupInput, } from "./lockup.js" import type { RampPhase, StallAge } from "./ramp.js" +import type { ActivityState } from "./session-chrome.js" import { BORDER, composeCostContextMeter, @@ -641,8 +642,12 @@ export type AppShell = { */ lockupNowMs: number lockupAnimating: boolean - /** Live phase word the slot shows, or null for the idle wordmark. */ - lockupPhase: string | null + /** + * Live activity state the slot shows, or null for the idle wordmark. + * Typed to the closed set (not `string`) so a raw tool/MCP/plugin + * identifier reaching this field is a compile error, not just a test one. + */ + lockupPhase: ActivityState | null /** Clock reading when `lockupPhase` last changed — the fade's origin. */ lockupChangedMs: number /** Density ramp phase for the same turn — drives the slot's pulse cell and tint. */ @@ -876,8 +881,11 @@ function syncLandingSuggestions(shell: AppShell): void { export type LockupFrame = { readonly nowMs: number readonly animating: boolean - /** Live phase word, or null for the idle wordmark. */ - readonly phase: string | null + /** + * Live activity state, or null for the idle wordmark. Typed to the closed + * set so the caller cannot hand this a raw tool identifier. + */ + readonly phase: ActivityState | null /** The turn's ramp phase, or null when idle. */ readonly rampPhase: RampPhase | null /** How long the turn has been stalled, or null when it is not stalled. */ diff --git a/src/tui-opentui/turn-monitor.test.ts b/src/tui-opentui/turn-monitor.test.ts index 0ac5b2cd3..7ef061c1f 100644 --- a/src/tui-opentui/turn-monitor.test.ts +++ b/src/tui-opentui/turn-monitor.test.ts @@ -73,16 +73,18 @@ describe("turn progress label", () => { expect(t.shell.lockupPhase).toBe("thinking") t.bridge.handle({ type: "inference.text.delta", data: { token: "hi" } }) - expect(t.shell.lockupPhase).toBe("streaming 1 tok") + expect(t.shell.lockupPhase).toBe("working") t.bridge.handle({ type: "inference.text.delta", data: { token: " there" } }) - expect(t.shell.lockupPhase).toBe("streaming 2 tok") + expect(t.shell.lockupPhase).toBe("working") t.bridge.handle({ type: "inference.tool_call.end", - data: { name: "bash", callId: "c1" }, + data: { name: "mcp__glitchtip__resolve_issue", callId: "c1" }, }) - expect(t.shell.lockupPhase).toBe("bash") + // Unmapped tool identifiers — including MCP tools — fall back to the + // generic working state rather than leaking the raw name. + expect(t.shell.lockupPhase).toBe("working") t.bridge.handle({ type: "reactor.done", data: {} }) expect(t.shell.lockupPhase).toBeNull() @@ -223,7 +225,7 @@ describe("turn progress label", () => { }) }) - test("an open permission overlay freezes the ramp and reads blocked", async () => { + test("an open permission overlay freezes the ramp and reads waiting", async () => { await withTestRenderer(async (h) => { const t: Harness = await setup(h) try { @@ -231,7 +233,7 @@ describe("turn progress label", () => { t.shell.overlayKind = "permissions" t.bridge.gateOpened() t.tick() - expect(t.shell.lockupPhase).toBe("blocked") + expect(t.shell.lockupPhase).toBe("waiting") // Frozen is the signal: the ramp must not move while a human is asked. const frozen = t.shell.lockupPhase