Skip to content

Commit 2d91477

Browse files
Render a semantic activity state in the status ticker, not raw tool names (#419)
* Add closed-set test for the status ticker's activity state The ticker currently renders whatever raw tool identifier is executing. This test pins the fix: the rendered label must always be a member of a small closed set of activity states, never a tool, MCP server, or plugin name, and stalled/waiting-on-operator must render distinctly. * Render a semantic activity state in the status ticker, not raw tool names The ticker rendered whatever tool identifier was currently executing — internal plumbing vocabulary leaking into a product surface, and redundant with the transcript, which already shows the tool call. Replace it with a closed set of human activity states (thinking, planning, researching, building, working, waiting, stalled, stopping). The execution-to-state mapping lives in one place with an explicit fallback to 'working', so an unmapped tool, MCP server, or plugin name can never reach the ticker and adding a tool needs no ticker change. Stalled reuses the existing stall-watchdog signal rather than a second notion of stuck, and waiting on operator approval is now its own state distinct from active work. * Make the ticker's closed set compiler-enforced and fix review findings - AppShell.lockupPhase and LockupFrame.phase in shell.ts are now typed ActivityState | null instead of string | null, so a raw tool identifier reaching the ticker is a type error at the setLockupFrame boundary, not only a test failure. lockup.ts's LockupInput.phase stays a generic string deliberately — its own tests exercise arbitrary CJK/astral text to check width math unrelated to the activity vocabulary, and the leak boundary is already closed one layer up. - isStalled on resolveTurnLabel is now required, matching resolveRampPhase; a caller that forgets it is the exact bug this state exists to prevent. - runtime-bridge.ts's second stall check site now calls isStalledForDisplay instead of re-deriving 'not quiet' from stallLevel's result, so the two call sites share one definition of stalled. - Dropped unread TurnLabelInput.awaitingResponse. - docs/TUI.md corrected: it described the phase slot as showing 'the running tool's name', which this change makes false; it now points at ACTIVITY_STATES as the source of truth. - Added delete_file/advance_workflow/tool_search/search_agents to the tool-to-state table. - Test fallback case swapped from the fictional 'bash' tool to a real MCP identifier, since 'bash' cannot occur at runtime.
1 parent 8addf44 commit 2d91477

6 files changed

Lines changed: 215 additions & 84 deletions

File tree

docs/TUI.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,14 @@ branch at its right (`AppShell.promptTopRule` / `promptBottomRule`,
5252
`src/tui-opentui/shell.ts`). Both rules cost zero transcript rows because they
5353
ride the prompt box's own border.
5454

55-
While a turn is live the lockup slot swaps the wordmark for the phase word —
56-
`thinking`, `streaming 12 tok`, the running tool's name — led by a single
57-
density cell (`rampPulse`, `src/tui-opentui/ramp.ts`). The cell, not the word,
55+
While a turn is live the lockup slot swaps the wordmark for a semantic
56+
activity word — never the raw tool, MCP server, or plugin identifier that is
57+
actually executing. `resolveTurnLabel` (`src/tui-opentui/session-chrome.ts`)
58+
maps execution onto the closed set `ACTIVITY_STATES` exported from that
59+
module (`thinking`, `planning`, `researching`, `building`, `working`,
60+
`waiting`, `stalled`, `stopping`); that export is the source of truth for
61+
what the slot can say, not this list. It is led by a single density cell
62+
(`rampPulse`, `src/tui-opentui/ramp.ts`). The cell, not the word,
5863
is what says whether the session is healthy, and it carries four states:
5964

6065
| State | Cell | Reads as |

src/tui-opentui/runtime-bridge.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -802,12 +802,10 @@ export function attachSessionBridge(
802802
const input = {
803803
isProcessing: turn.isProcessing,
804804
status: turn.status,
805-
awaitingResponse: turn.awaitingResponse,
806805
currentToolName: turn.currentToolName,
807806
streamingType: turn.streamingType,
808-
streamTokenCount: turn.streamTokenCount,
809807
}
810-
const label = resolveTurnLabel(input)
808+
const label = resolveTurnLabel(input, isStalled)
811809
if (label === undefined) {
812810
// The bottom-left status slot rides the same re-entry as the landing
813811
// mark, so it crossfades between phases without a timer of its own.
@@ -1037,7 +1035,10 @@ export function attachSessionBridge(
10371035
setStatusFlash(shell, STALL_NOTICE_MESSAGE)
10381036
}
10391037

1040-
paintPhaseAt(nowMs, level !== "quiet")
1038+
// Same "is this stalled at all" question `paintPhase` asks above — call
1039+
// the one definition (`isStalledForDisplay`) rather than re-deriving it
1040+
// from `stallLevel`'s result, so the two call sites can never disagree.
1041+
paintPhaseAt(nowMs, isStalledForDisplay(stallArgs))
10411042
}
10421043

10431044
setShellBridgeHooks(shell, {

src/tui-opentui/session-chrome.test.ts

Lines changed: 118 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, test } from "bun:test"
22

33
import {
4+
ACTIVITY_STATES,
45
classifyAgentSendFailure,
56
classifySendFailureMessage,
67
resolveRampPhase,
@@ -9,94 +10,157 @@ import {
910
shouldSettleUiAfterSendFailure,
1011
} from "./session-chrome.js"
1112

13+
// The load-bearing guarantee: whatever tool identifier, MCP server name, or
14+
// plugin name the runtime hands us, the rendered ticker string must land in
15+
// the small closed set of human activity states — never the raw identifier.
16+
// A previously-unmapped tool (or one this test doesn't enumerate) must still
17+
// fall back into the set rather than leaking through verbatim.
18+
describe("resolveTurnLabel closed-set guarantee", () => {
19+
const leakingIdentifiers = [
20+
"run_shell",
21+
"grep",
22+
"read_file",
23+
"write_file",
24+
"edit_file",
25+
"search_files",
26+
"list_dir",
27+
"web_search",
28+
"web_fetch",
29+
"manage_tasks",
30+
"task",
31+
"submit_output",
32+
"ask_operator",
33+
"mcp__glitchtip__authenticate",
34+
"mcp__railway__deploy",
35+
"some_未knownしplugin_tool",
36+
"a-plugin-defined-tool-name",
37+
"totally_unmapped_future_tool",
38+
]
39+
40+
for (const currentToolName of leakingIdentifiers) {
41+
test(`"${currentToolName}" resolves to a member of the closed set`, () => {
42+
const label = resolveTurnLabel(
43+
{
44+
isProcessing: true,
45+
status: "running",
46+
currentToolName,
47+
streamingType: "tool",
48+
},
49+
false,
50+
)
51+
expect(label).not.toBe(currentToolName)
52+
expect(ACTIVITY_STATES).toContain(label!)
53+
})
54+
}
55+
56+
test("a stalled turn renders a distinct stalled state", () => {
57+
const label = resolveTurnLabel(
58+
{
59+
isProcessing: true,
60+
status: "running",
61+
currentToolName: "run_shell",
62+
streamingType: "tool",
63+
},
64+
true,
65+
)
66+
expect(label).toBe("stalled")
67+
expect(ACTIVITY_STATES).toContain(label!)
68+
})
69+
70+
test("waiting on the operator is distinguishable from working", () => {
71+
const label = resolveTurnLabel(
72+
{
73+
isProcessing: true,
74+
status: "blocked",
75+
currentToolName: "run_shell",
76+
streamingType: "tool",
77+
},
78+
false,
79+
)
80+
expect(label).toBe("waiting")
81+
expect(label).not.toBe("working")
82+
expect(ACTIVITY_STATES).toContain(label!)
83+
})
84+
})
85+
1286
describe("resolveTurnLabel", () => {
1387
test("idle processing off yields no label", () => {
1488
expect(
15-
resolveTurnLabel({
16-
isProcessing: false,
17-
status: "idle",
18-
awaitingResponse: false,
19-
currentToolName: null,
20-
streamingType: null,
21-
}),
89+
resolveTurnLabel(
90+
{
91+
isProcessing: false,
92+
status: "idle",
93+
currentToolName: null,
94+
streamingType: null,
95+
},
96+
false,
97+
),
2298
).toBeUndefined()
2399
})
24100

25-
test("blocked gate shows approval wait", () => {
101+
test("blocked gate shows a waiting-on-operator state", () => {
26102
expect(
27-
resolveTurnLabel({
28-
isProcessing: true,
29-
status: "blocked",
30-
awaitingResponse: false,
31-
currentToolName: "run_shell",
32-
streamingType: "tool",
33-
}),
34-
).toBe("blocked")
103+
resolveTurnLabel(
104+
{
105+
isProcessing: true,
106+
status: "blocked",
107+
currentToolName: "run_shell",
108+
streamingType: "tool",
109+
},
110+
false,
111+
),
112+
).toBe("waiting")
35113
})
36114

37115
test("stopping beats tool phase", () => {
38116
expect(
39-
resolveTurnLabel({
40-
isProcessing: true,
41-
status: "stopping",
42-
awaitingResponse: false,
43-
currentToolName: "grep",
44-
streamingType: "tool",
45-
}),
117+
resolveTurnLabel(
118+
{
119+
isProcessing: true,
120+
status: "stopping",
121+
currentToolName: "grep",
122+
streamingType: "tool",
123+
},
124+
false,
125+
),
46126
).toBe("stopping")
47127
})
48128

49-
test("tool phase beats generic working", () => {
129+
test("tool phase maps to its semantic activity, never the raw name", () => {
50130
expect(
51-
resolveTurnLabel({
52-
isProcessing: true,
53-
status: "running",
54-
awaitingResponse: true,
55-
currentToolName: "grep",
56-
streamingType: "tool",
57-
}),
58-
).toBe("grep")
131+
resolveTurnLabel(
132+
{
133+
isProcessing: true,
134+
status: "running",
135+
currentToolName: "grep",
136+
streamingType: "tool",
137+
},
138+
false,
139+
),
140+
).toBe("researching")
59141
})
60142

61143
test("thinking and text phases", () => {
62144
const base = {
63145
isProcessing: true,
64146
status: "running" as const,
65-
awaitingResponse: false,
66147
currentToolName: null,
67148
}
68149
expect(
69-
resolveTurnLabel({ ...base, streamingType: "thinking" }),
150+
resolveTurnLabel({ ...base, streamingType: "thinking" }, false),
70151
).toBe("thinking")
71152
expect(
72-
resolveTurnLabel({ ...base, streamingType: "text", streamTokenCount: 7 }),
73-
).toBe("streaming 7 tok")
74-
expect(
75-
resolveTurnLabel({
76-
...base,
77-
awaitingResponse: true,
78-
streamingType: null,
79-
}),
153+
resolveTurnLabel({ ...base, streamingType: "text" }, false),
80154
).toBe("working")
81-
})
82-
83-
test("text phase with no count yet reads zero", () => {
84155
expect(
85-
resolveTurnLabel({
86-
isProcessing: true,
87-
status: "running",
88-
awaitingResponse: false,
89-
currentToolName: null,
90-
streamingType: "text",
91-
}),
92-
).toBe("streaming 0 tok")
156+
resolveTurnLabel({ ...base, streamingType: null }, false),
157+
).toBe("working")
93158
})
94159
})
95160

96161
describe("resolveRampPhase", () => {
97162
const base = {
98163
isProcessing: true,
99-
awaitingResponse: false,
100164
currentToolName: null,
101165
streamingType: null,
102166
}

src/tui-opentui/session-chrome.ts

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,34 +20,85 @@ export type TurnStatus =
2020
export type TurnLabelInput = {
2121
readonly isProcessing: boolean
2222
readonly status: TurnStatus
23-
readonly awaitingResponse: boolean
2423
readonly currentToolName: string | null
2524
readonly streamingType: "text" | "thinking" | "tool" | null
26-
/** Text deltas seen so far this turn; read only while `streamingType` is `text`. */
27-
readonly streamTokenCount?: number
25+
}
26+
27+
/**
28+
* Closed set the status ticker is allowed to render. Every path through
29+
* `resolveTurnLabel` returns one of these — never a tool identifier, MCP
30+
* server name, or plugin name. This is what the leak-prevention test checks
31+
* membership against, so it must stay the single source of truth for "what
32+
* can appear in the ticker."
33+
*/
34+
export const ACTIVITY_STATES = [
35+
"thinking",
36+
"planning",
37+
"researching",
38+
"building",
39+
"working",
40+
"waiting",
41+
"stalled",
42+
"stopping",
43+
] as const
44+
45+
export type ActivityState = (typeof ACTIVITY_STATES)[number]
46+
47+
/**
48+
* Execution → activity-state mapping, kept in this one place with an
49+
* explicit fallback so a newly added tool (built-in, MCP, or plugin) renders
50+
* a generic "working" state instead of leaking its identifier — no ticker
51+
* change is required to add a tool correctly.
52+
*/
53+
const TOOL_ACTIVITY_STATES: Readonly<Record<string, ActivityState>> = {
54+
read_file: "researching",
55+
search_files: "researching",
56+
grep: "researching",
57+
list_dir: "researching",
58+
web_search: "researching",
59+
web_fetch: "researching",
60+
write_file: "building",
61+
edit_file: "building",
62+
run_shell: "building",
63+
delete_file: "building",
64+
manage_tasks: "planning",
65+
task: "planning",
66+
advance_workflow: "planning",
67+
tool_search: "researching",
68+
search_agents: "researching",
69+
ask_operator: "waiting",
70+
submit_output: "working",
71+
}
72+
73+
function activityStateForTool(name: string | null): ActivityState {
74+
if (name === null) return "working"
75+
return TOOL_ACTIVITY_STATES[name] ?? "working"
2876
}
2977

3078
/**
3179
* Single session-phase label accompanying the density ramp. Lowercase and
3280
* unpunctuated — the ramp's color and motion carry the state, so the word only
3381
* has to name it. Returns undefined when idle so the phase segment disappears.
3482
*
35-
* Text streaming carries a live count (`streaming 7 tok`) rather than the
36-
* bare word: it is the one phase with something to count, and the count is
37-
* what tells the operator the slot is not stalled.
83+
* `isStalled` is the caller's own `isStalledForDisplay` result (see
84+
* stall-watchdog.ts) — this function does not re-derive staleness, it only
85+
* ranks "stalled" against the other phases so the ticker and the ramp never
86+
* disagree about which runs look stuck. Required, not defaulted: a caller
87+
* that forgets to pass it is exactly the bug this state exists to prevent —
88+
* a wedged run silently painted as ordinary work.
3889
*/
39-
export function resolveTurnLabel(input: TurnLabelInput): string | undefined {
90+
export function resolveTurnLabel(
91+
input: TurnLabelInput,
92+
isStalled: boolean,
93+
): ActivityState | undefined {
4094
if (!input.isProcessing) return undefined
41-
if (input.status === "blocked") return "blocked"
95+
if (input.status === "blocked") return "waiting"
4296
if (input.status === "stopping" || input.status === "stopped") {
4397
return "stopping"
4498
}
45-
if (input.currentToolName !== null) return input.currentToolName
46-
if (input.streamingType === "tool") return "tool"
99+
if (isStalled) return "stalled"
100+
if (input.currentToolName !== null) return activityStateForTool(input.currentToolName)
47101
if (input.streamingType === "thinking") return "thinking"
48-
if (input.streamingType === "text") {
49-
return `streaming ${String(input.streamTokenCount ?? 0)} tok`
50-
}
51102
return "working"
52103
}
53104

src/tui-opentui/shell.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ import {
6969
type LockupInput,
7070
} from "./lockup.js"
7171
import type { RampPhase, StallAge } from "./ramp.js"
72+
import type { ActivityState } from "./session-chrome.js"
7273
import {
7374
BORDER,
7475
composeCostContextMeter,
@@ -641,8 +642,12 @@ export type AppShell = {
641642
*/
642643
lockupNowMs: number
643644
lockupAnimating: boolean
644-
/** Live phase word the slot shows, or null for the idle wordmark. */
645-
lockupPhase: string | null
645+
/**
646+
* Live activity state the slot shows, or null for the idle wordmark.
647+
* Typed to the closed set (not `string`) so a raw tool/MCP/plugin
648+
* identifier reaching this field is a compile error, not just a test one.
649+
*/
650+
lockupPhase: ActivityState | null
646651
/** Clock reading when `lockupPhase` last changed — the fade's origin. */
647652
lockupChangedMs: number
648653
/** Density ramp phase for the same turn — drives the slot's pulse cell and tint. */
@@ -876,8 +881,11 @@ function syncLandingSuggestions(shell: AppShell): void {
876881
export type LockupFrame = {
877882
readonly nowMs: number
878883
readonly animating: boolean
879-
/** Live phase word, or null for the idle wordmark. */
880-
readonly phase: string | null
884+
/**
885+
* Live activity state, or null for the idle wordmark. Typed to the closed
886+
* set so the caller cannot hand this a raw tool identifier.
887+
*/
888+
readonly phase: ActivityState | null
881889
/** The turn's ramp phase, or null when idle. */
882890
readonly rampPhase: RampPhase | null
883891
/** How long the turn has been stalled, or null when it is not stalled. */

0 commit comments

Comments
 (0)