Skip to content

Commit 3ecfa9b

Browse files
committed
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 7f35cb8 commit 3ecfa9b

6 files changed

Lines changed: 95 additions & 70 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: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,6 @@ 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,
808807
}
@@ -1036,7 +1035,10 @@ export function attachSessionBridge(
10361035
setStatusFlash(shell, STALL_NOTICE_MESSAGE)
10371036
}
10381037

1039-
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))
10401042
}
10411043

10421044
setShellBridgeHooks(shell, {

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

Lines changed: 57 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,15 @@ describe("resolveTurnLabel closed-set guarantee", () => {
3939

4040
for (const currentToolName of leakingIdentifiers) {
4141
test(`"${currentToolName}" resolves to a member of the closed set`, () => {
42-
const label = resolveTurnLabel({
43-
isProcessing: true,
44-
status: "running",
45-
awaitingResponse: false,
46-
currentToolName,
47-
streamingType: "tool",
48-
})
42+
const label = resolveTurnLabel(
43+
{
44+
isProcessing: true,
45+
status: "running",
46+
currentToolName,
47+
streamingType: "tool",
48+
},
49+
false,
50+
)
4951
expect(label).not.toBe(currentToolName)
5052
expect(ACTIVITY_STATES).toContain(label!)
5153
})
@@ -56,7 +58,6 @@ describe("resolveTurnLabel closed-set guarantee", () => {
5658
{
5759
isProcessing: true,
5860
status: "running",
59-
awaitingResponse: false,
6061
currentToolName: "run_shell",
6162
streamingType: "tool",
6263
},
@@ -67,13 +68,15 @@ describe("resolveTurnLabel closed-set guarantee", () => {
6768
})
6869

6970
test("waiting on the operator is distinguishable from working", () => {
70-
const label = resolveTurnLabel({
71-
isProcessing: true,
72-
status: "blocked",
73-
awaitingResponse: false,
74-
currentToolName: "run_shell",
75-
streamingType: "tool",
76-
})
71+
const label = resolveTurnLabel(
72+
{
73+
isProcessing: true,
74+
status: "blocked",
75+
currentToolName: "run_shell",
76+
streamingType: "tool",
77+
},
78+
false,
79+
)
7780
expect(label).toBe("waiting")
7881
expect(label).not.toBe("working")
7982
expect(ACTIVITY_STATES).toContain(label!)
@@ -83,79 +86,81 @@ describe("resolveTurnLabel closed-set guarantee", () => {
8386
describe("resolveTurnLabel", () => {
8487
test("idle processing off yields no label", () => {
8588
expect(
86-
resolveTurnLabel({
87-
isProcessing: false,
88-
status: "idle",
89-
awaitingResponse: false,
90-
currentToolName: null,
91-
streamingType: null,
92-
}),
89+
resolveTurnLabel(
90+
{
91+
isProcessing: false,
92+
status: "idle",
93+
currentToolName: null,
94+
streamingType: null,
95+
},
96+
false,
97+
),
9398
).toBeUndefined()
9499
})
95100

96101
test("blocked gate shows a waiting-on-operator state", () => {
97102
expect(
98-
resolveTurnLabel({
99-
isProcessing: true,
100-
status: "blocked",
101-
awaitingResponse: false,
102-
currentToolName: "run_shell",
103-
streamingType: "tool",
104-
}),
103+
resolveTurnLabel(
104+
{
105+
isProcessing: true,
106+
status: "blocked",
107+
currentToolName: "run_shell",
108+
streamingType: "tool",
109+
},
110+
false,
111+
),
105112
).toBe("waiting")
106113
})
107114

108115
test("stopping beats tool phase", () => {
109116
expect(
110-
resolveTurnLabel({
111-
isProcessing: true,
112-
status: "stopping",
113-
awaitingResponse: false,
114-
currentToolName: "grep",
115-
streamingType: "tool",
116-
}),
117+
resolveTurnLabel(
118+
{
119+
isProcessing: true,
120+
status: "stopping",
121+
currentToolName: "grep",
122+
streamingType: "tool",
123+
},
124+
false,
125+
),
117126
).toBe("stopping")
118127
})
119128

120129
test("tool phase maps to its semantic activity, never the raw name", () => {
121130
expect(
122-
resolveTurnLabel({
123-
isProcessing: true,
124-
status: "running",
125-
awaitingResponse: true,
126-
currentToolName: "grep",
127-
streamingType: "tool",
128-
}),
131+
resolveTurnLabel(
132+
{
133+
isProcessing: true,
134+
status: "running",
135+
currentToolName: "grep",
136+
streamingType: "tool",
137+
},
138+
false,
139+
),
129140
).toBe("researching")
130141
})
131142

132143
test("thinking and text phases", () => {
133144
const base = {
134145
isProcessing: true,
135146
status: "running" as const,
136-
awaitingResponse: false,
137147
currentToolName: null,
138148
}
139149
expect(
140-
resolveTurnLabel({ ...base, streamingType: "thinking" }),
150+
resolveTurnLabel({ ...base, streamingType: "thinking" }, false),
141151
).toBe("thinking")
142152
expect(
143-
resolveTurnLabel({ ...base, streamingType: "text" }),
153+
resolveTurnLabel({ ...base, streamingType: "text" }, false),
144154
).toBe("working")
145155
expect(
146-
resolveTurnLabel({
147-
...base,
148-
awaitingResponse: true,
149-
streamingType: null,
150-
}),
156+
resolveTurnLabel({ ...base, streamingType: null }, false),
151157
).toBe("working")
152158
})
153159
})
154160

155161
describe("resolveRampPhase", () => {
156162
const base = {
157163
isProcessing: true,
158-
awaitingResponse: false,
159164
currentToolName: null,
160165
streamingType: null,
161166
}

src/tui-opentui/session-chrome.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ 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
2625
}
@@ -61,8 +60,12 @@ const TOOL_ACTIVITY_STATES: Readonly<Record<string, ActivityState>> = {
6160
write_file: "building",
6261
edit_file: "building",
6362
run_shell: "building",
63+
delete_file: "building",
6464
manage_tasks: "planning",
6565
task: "planning",
66+
advance_workflow: "planning",
67+
tool_search: "researching",
68+
search_agents: "researching",
6669
ask_operator: "waiting",
6770
submit_output: "working",
6871
}
@@ -77,14 +80,16 @@ function activityStateForTool(name: string | null): ActivityState {
7780
* unpunctuated — the ramp's color and motion carry the state, so the word only
7881
* has to name it. Returns undefined when idle so the phase segment disappears.
7982
*
80-
* `isStalled` is the caller's own `shouldNoticeStall`/`isStalledForDisplay`
81-
* result (see stall-watchdog.ts) — this function does not re-derive
82-
* staleness, it only ranks "stalled" against the other phases so the ticker
83-
* and the ramp never disagree about which runs look stuck.
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.
8489
*/
8590
export function resolveTurnLabel(
8691
input: TurnLabelInput,
87-
isStalled: boolean = false,
92+
isStalled: boolean,
8893
): ActivityState | undefined {
8994
if (!input.isProcessing) return undefined
9095
if (input.status === "blocked") return "waiting"

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. */

src/tui-opentui/turn-monitor.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,10 @@ describe("turn progress label", () => {
8080

8181
t.bridge.handle({
8282
type: "inference.tool_call.end",
83-
data: { name: "bash", callId: "c1" },
83+
data: { name: "mcp__glitchtip__resolve_issue", callId: "c1" },
8484
})
85-
// Unmapped tool identifiers fall back to the generic working state
86-
// rather than leaking the raw name.
85+
// Unmapped tool identifiers — including MCP tools — fall back to the
86+
// generic working state rather than leaking the raw name.
8787
expect(t.shell.lockupPhase).toBe("working")
8888

8989
t.bridge.handle({ type: "reactor.done", data: {} })

0 commit comments

Comments
 (0)