fix(ui): render a running tool the same way as a settled one - #1961

Astro-Han · 2026-08-03T07:36:30Z

Summary

A running command rendered in a bespoke trow (Terminal icon, Chinese count summary, shimmer, settle fade) and switched to Astryx chrome the moment it settled. Three causes, all fixed here.

The status was wrong. materializeTools reads a tool_call with no tool_result as interrupted — replayed from JSONL alone it cannot tell "never finished" from "not finished yet". mergeLiveOverPersisted preferred that persisted guess over a live running, so every long command sat under an interrupted row for its whole run. Live wins outright now; the live side stamps interrupted itself via terminalizeLiveSteps on abort/error/complete, so the abort signal the override defended survives without it.

The renderer was forked. ToolTrow kept two implementations behind supportsAstryxToolCall: Astryx ChatToolCalls for ordinary tools, the bespoke trow for sandbox denials, interruptions, and permission waits. Astryx's four statuses cover all the product ones — interrupted and sandbox denials are failures, and they keep a visible word through the stats slot that a red icon alone cannot say. The expanded panel is unchanged; it moves to ToolCallDetail, exported because Astryx owns row expansion internally and the panel is what the product actually decides.

The status enum carried a ghost. waiting_permission was never produced by any code path — only the Storybook and e2e fixtures constructed it. The sandbox-boundary fixture that did shows why it was never needed: the permission ask lives in the sandbox_boundary_request event and SandboxBoundaryPrompt, with the composer's own 等待你确认权限… label. Once the fork above removed its last distinguishing behavior — the product-owned disclosure that force-expanded — it was identical to pending.

Net effect is subtraction: the trow renderer, its summary and motion helpers (whose only remaining consumers were their own unit tests), the ToolActivity card shell nothing rendered anymore, the toolVariants table, toolStatusLabel and the status copy only it read, and the CSS for all of it. The label passed to ChatToolCalls went too — Astryx has never read it.

Verification

Live app, disclosure-output fixture: a running Bash now draws as an Astryx row (spinner, name, intent, chevron), identical in kind to the settled row above it.

npm run build:test + full workspace suite (--serial): pass. @maka/ui 240, @maka/desktop 1392, @maka/headless 1305, maka-agent 467, @maka/runtime 2715, @maka/storage 1015.
npm run lint, npm run format:check, node scripts/check-dead-css.mjs --check, node scripts/check-story-annotations.mjs: clean.
npm run e2e (58 specs): pass, including disclosure-output.spec.ts, which already asserted the Astryx contract.
New regression tests: a still-running tool stays running across the persisted merge (packages/ui, apps/desktop), and every status renders through astryx-chat-tool-calls with no maka-tool-trow anywhere.

Pre-existing failure on main

Rebased onto faff12ade. Four builtin-tools path-containment tests fail there — mkdtemp returns /var/... while the session cwd resolves to /private/var/... on macOS, so resolveExistingInsideCwd rejects its own temp root. Reproduced on a clean origin/main worktree; this PR touches neither workspace-executor.ts nor builtin-tools.ts.

materializeTools reads a tool_call with no tool_result as `interrupted` — replayed from JSONL alone it cannot tell "never finished" from "not finished yet". mergeLiveOverPersisted preferred that persisted guess over a live `running`, so every long command rendered as an interrupted tool for its whole run and only snapped back once it exited. Live wins outright. The live side stamps `interrupted` itself via terminalizeLiveSteps on abort/error/complete, so the abort signal it was defending survives without the override.

ToolTrow kept two implementations behind supportsAstryxToolCall: Astryx ChatToolCalls for ordinary tools, and a bespoke trow (Terminal icon, Chinese count summary, TextShimmer, settle fade) for sandbox denials, interruptions, and permission waits. The two looked nothing alike, so a tool changed appearance as it crossed a status boundary. Astryx's four statuses cover all six product ones: `interrupted` and sandbox denials are failures, and they keep a visible word through the `stats` slot that a red icon alone cannot say. The expanded panel is unchanged — it moves to ToolCallDetail, exported because Astryx owns row expansion internally and the panel is what the product actually decides. Retires the trow renderer, its summary/motion helpers (whose only remaining consumers were their own unit tests), the ToolActivity card shell nothing rendered anymore, the toolVariants table, and the CSS for all of it. The `label` passed to ChatToolCalls went too: Astryx has never read it.

No code path ever produced it. Only the Storybook and e2e fixtures constructed it, and the sandbox-boundary fixture that did shows why it was never needed: the permission ask lives in the `sandbox_boundary_request` event and SandboxBoundaryPrompt, with the composer's own 等待你确认权限… label. The tool row was a second, silent copy of that state. The previous commit removed its last distinguishing behavior — the product-owned disclosure that force-expanded — leaving it identical to `pending`. Keeping a status that nothing emits and nothing renders differently is a placeholder for an imagined requirement; wiring permission presentation later can reintroduce one member. Also retires toolStatusLabel, which had no consumers left, and the status copy only it read.

A `tool_call` with no `tool_result` is the absence of evidence, not evidence of a terminal state. Both materializers read that gap as `interrupted`, which put a running turn and its own tool in direct contradiction — `materializeTurns` reported `{turn: running, tool: interrupted}` for the same session. The turn record is the authority and was already on hand: runtime appends `turn_state: 'running'` when the turn opens, and both `tool_call` and `turn_state` carry `turnId`. A running turn now means the call is in flight; only a turn that has itself ended makes the missing result mean the tool never finished. Sessions written before `turn_state` have no record and keep reading as `interrupted`. This is the root of the same bug the merge fix treated downstream. That one relied on a live projection being present, so it left the symptom reachable whenever live state is gone — a renderer reload, or re-attaching to a session running in the background. The live-over-persisted test that hand-built an already-interrupted projection was true on the spread alone; it now drives the real chain (tool_start → tool_output_delta → abort).

Astryx's collapsed group header projects only the last call, so an interrupted or sandbox-blocked row behind a trailing success showed a green check and a wrench count for a run that went wrong. The bespoke group this replaced carried that signal in its summary line. Astryx has no group-level slot to put it back — `label` is destructured and dropped, and the collapsed header renders neither `stats` nor any aggregate — so the group opens itself when a row still runs or carries an outcome word, which are exactly the cases the header misreports. Also drop the two imports and the `close` copy pair left behind when their components went, and stop routing `interrupted` through both `errorMessage` and `stats`, which made a screen reader say it twice.

A settled group whose earlier row failed stays collapsed, and the collapsed header keeps projecting the last call alone. Overriding that would put a rule only this product knows next to Astryx's own chrome — the bespoke group summary this branch just removed. The outcome is one click away in the group; accepting that density is what a single visual language costs. The story kept for the grouped surface now documents the trade-off instead of asserting an override.

… live Only the active session is subscribed and events do not replay, so a turn that ends while the user looks elsewhere leaves its projection frozen mid-run. reconcileTerminalLiveTurn hands a tool off only once it is interrupted or has a result, so a frozen `running` never clears itself: a long command aborted in the background kept spinning until a full renderer reload. Live still wins by default — it leads the persisted refresh — but a turn that recorded its own end is the counter-evidence, and no tool inside it is still running. Only the status is taken back; live output chunks stay live, since persisted JSONL does not store them. This needs the evidence to be real, and `status` alone is not: for sessions written before `turn_state`, deriveTurnRecords falls back to inferLegacyTurnStatus, so `completed` there is a guess about old data rather than a record. TurnViewModel now carries `recordedStatus`, present only when a turn_state message set it, and reconciliation reads that. Also correct three claims this branch had made about the system: deriveTurnRecords always yields a record (the legacy fallback simply never returns `running`, which is why the outcome held); Astryx ignores defaultIsExpanded for a lone call, so that comment only describes real groups; and asserting markup lacks `maka-tool-trow` was true by construction once the class was gone.

`tool_start` opens a call as `pending` and it only reaches `running` once output arrives, so a tool that never streams stays pending for its whole life. The group-expansion check looked only for `running`, while the reconciliation code in materialize.ts already defined in-flight as `pending || running` — the same concept spelled two ways, and the shorter spelling was wrong. It shows up with parallel calls, where the last one can settle first: Astryx's collapsed header projects that last call, so the group reported a green check while a sibling was still working. Sharing the in-flight predicate fixes the case and removes the second spelling. Also correct the errorMessage comment: Astryx reads that text out from an expanded row's status icon, but its collapsed group header carries neither errorMessage nor stats.

…nance Two facts were being reconstructed by their consumers instead of stated by their owner. `ToolActivityStatus`: core already owned the settled half (`SettledToolActivityStatus`), while the full union was spelled longhand in three packages — which is why dropping `waiting_permission` earlier in this branch took three edits. Core now owns both halves and the in-flight predicate, so "is this call still in flight" is one function rather than four inline copies drifting apart. One had already drifted: the tool-group expansion check tested only `running`. `TurnRecord.statusSource`: `deriveTurnRecords` knows whether a status was read from a `turn_state` message or reconstructed by `inferLegacyTurnStatus`, and discarded that. Reconciliation needs it — an inferred `completed` is a reading of old data, not evidence a turn ended — so the previous commit rebuilt it in the UI by rescanning every message. Core states it instead, deleting that scan, the second `deriveTurnRecords` call, and the UI-only `recordedStatus` field. The predicate belongs in core rather than `@maka/ui` for a concrete reason: `session-event-health.ts` importing a *value* from `@maka/ui` pulled the whole UI package into an esbuild-bundled main-process test and broke it on `Dynamic require of "react"`. Also drop a comment pointing at the `maka-tool-pulse` keyframe this branch deleted, and a merge condition that could never be false.

The rule for a `tool_call` with no `tool_result` — read it against its turn — was spelled twice, once in each materializer, with near-identical prose. The same PR had just moved the status union into core because it should be spelled once; the rule deserved the same seam. It now lives beside `toolResultActivityStatus`, which already maps evidence to status, so the TUI transcript and the desktop renderer cannot drift apart. `isInFlightToolStatus` was still hand-inlined in `ToolCallDetail`, two lines below the import of the predicate itself, and it is what selects the live stream and drives the output autoscroll. It now calls the predicate, which is also a type guard so `InFlightToolActivityStatus` has a consumer. `TextShimmer`'s `delayed` prop lost its only callers when the bespoke rows went; drop the prop, the `data-delayed` attribute, its CSS rule and its doc paragraph. Rename the comments that still called an Astryx group a trow.

Astro-Han force-pushed the fix/tool-activity-astryx-convergence branch from 0205515 to fd771e4 Compare August 3, 2026 07:59

Astro-Han added 11 commits August 3, 2026 18:14

test(ui): lock the tool row to one renderer across every status

3a6c57e

Astro-Han force-pushed the fix/tool-activity-astryx-convergence branch from 804b23b to f054b6c Compare August 3, 2026 10:19

Astro-Han marked this pull request as ready for review August 3, 2026 10:28

Astro-Han merged commit 2da7a81 into main Aug 3, 2026
11 checks passed

Navigation Menu

Uh oh!

fix(ui): render a running tool the same way as a settled one - #1961