fix(ui): normalize tool activity presentation by Astro-Han · Pull Request #712 · apache/maka

Astro-Han · 2026-07-10T09:42:14Z

Summary

Collapse ordinary tool diagnostics by default while keeping permission and error states visible.
Preserve explicit user disclosure choices across normal tool lifecycle transitions.
Replace transport chunk counts with user-relevant diagnostic flags.
Carry an optional semantic activityKind from tool declarations through runtime events, persistence, replay, and live desktop state.

Why

Tool activity rows exposed raw commands and transport details such as STDOUT N, while lifecycle changes also controlled whether details were open. Long PowerShell commands made this especially visible on Windows, but the underlying presentation-state coupling was platform-independent.

Scope

Changed:

Added a pure tool activity presentation and disclosure policy.
Added the backward-compatible ToolActivityKind contract and legacy tool-name fallback.
Classified built-in read, search, edit, and command tools.
Added regression coverage across runtime, UI, replay, and desktop live events.

Not included:

Tool execution behavior or shell command generation.
OS-specific command rewriting.
Changes to legacy session storage requirements.

Verification

npm run -w @maka/runtime test — 1134 passed, 2 environment-dependent skipped
npm run -w @maka/ui test — 52 passed
npm run -w @maka/desktop test — 2335 passed
npm run typecheck — passed for all workspaces

User-facing impact

Ordinary tool activity stays compact, permission requests and failures remain prominent, and internal stdout/stderr chunk counts are no longer shown. The optional persisted field is backward-compatible; no migration, documentation, or changelog change is required.

Reviewer notes

The main review seams are packages/ui/src/tool-activity/presentation.ts and the activityKind projection path. Legacy rows and custom tools without a declared kind continue to use tool-name inference.

A Windows screenshot is not included because this workspace has no Windows renderer. The platform-independent disclosure and text behavior is covered by contract tests.

Checklist

Scope matches the PR title and excludes unrelated changes
Verification lists commands and results
User-facing impact, docs, changelog, migrations, and breaking changes are noted
Risk, rollback, and review focus are called out
UI screenshot absence is explained

Live stream and settled result previews used separate shells with diagnostic chrome. Share one Codex-like well for command and body, drop always-on copy/cwd/exit headers, and lock the quiet panel with presentation and desktop contracts.

Caption 11px made expanded reasoning look smaller than tool rows. Use the shared base 13px reading size for live and settled thinking.

Built-in read/search tools still persist generic JSON, so the quiet panel dumped args objects and stringified content. Format known Read/Grep/Glob/Edit/Write shapes as path/pattern + plain body text.

Any kind:json result or redacted-args fallback could still pretty-print escaped objects. Format every quiet-panel payload as plain headline + body text so Read/Grep and unknown tools never show brace dumps.

Key-only secrets slipped past value redaction in key:value formatting, and Write/Edit dropped the path when args and result headlines matched. Redact keys and full lines, and always keep a single invocation line.

Update redaction/result-preview contracts for quiet formatting and point the computed-style fixture at TOOL_OUTPUT_* instead of retired stream DOM.

Close Codex P1s: sensitive-key structural redaction, append unconsumed JSON diagnostics, honor terminal status/truncation flags, render shell_run instead of [shell_run], and fall back unknown activityKind.

Mask secrets embedded in property names, give shell_run its own single panel (no shared-well nest), skip the generic failure banner on cancelled terminals, and label timed_out distinctly.

Map cancelled terminal/shell_run to interrupted so cards and trows do not say 失败; keep pre-yield live chunks when shell_run lands empty; redact sensitive key payloads for =/: /space separators; bucket legacy StopBackgroundTask names as command.

Mask full multi-token sensitive key payloads; preserve live truncation and redaction when shell_run yields empty streams; share cancel→ interrupted status mapping in @maka/core for UI and runtime materializers; stop nesting rich previews inside the shared quiet well.

Trust call-level isError so successful shell_run cancelled observations stay completed; keep redacted/truncated notes when live chunks are empty.

Mask api/private key and bare auth payloads in quiet key labels. Keep live stream chunks until durable terminal/shell_run evidence has real streams.

Text handoff strips answer slots but retains outputChunks tools. Terminal liveTurn is evidence-only, so ChatView no longer treats it as in-flight.

Delayed processing/continuing flags can lag one frame past complete; do not re-lock the footer when liveTurn is already terminal evidence-only.

…irement) The synced comments still described chat call sites and the tool stream as consumers, but #712 retired streamVariants/LiveIndicator from the tool body — they have no production consumer now (chat-stream-cascade-contract pins it). State that in the comments, and drop the contradictory 'LiveIndicator exported only on cross-package consumer' line so the promotion rule is stated once (cross-package consumer or explicit public-API need) and points to README.

…owner The barrel promotion rule is volatile when duplicated into inline source comments (consumer lists drift as symbols retire — e.g. streamVariants/ LiveIndicator in #712). Reverting the index.ts/chat.tsx comment edits keeps this PR docs-only and makes packages/ui/README.md the single source of truth (the README now says so explicitly). Cleaning up the stale source comments / dead symbols is a separate change.

Round 14 review: 'revert + README says comments may lag' still left the stale 'second consumer' / streamVariants/LiveIndicator consumer lists in the source comments, so the conflict source survived. Real root correction: the inline comments no longer re-derive the promotion rule or track consumers (that list drifts as symbols retire, e.g. #712) — they keep their local implementation intent and point at packages/ui/README.md for the rule. README stays the single owner; dead-symbol cleanup stays a separate change.

Round 15 found the prior root correction only covered 2 of the consumer/ promotion comment blocks; markerVariants/TextShimmer/toolVariants/previewVariants still re-derived the rule or tracked consumers, and streamVariants/LiveIndicator still assumed a call site that #712 removed. Exhaustively replace every such block: keep local implementation intent + a short pointer to the README, drop all consumer counts and promotion derivations. Also drop the README's 'inline comments may lag' line (no longer needed once the comments don't re-derive).

…irement) The synced comments still described chat call sites and the tool stream as consumers, but #712 retired streamVariants/LiveIndicator from the tool body — they have no production consumer now (chat-stream-cascade-contract pins it). State that in the comments, and drop the contradictory 'LiveIndicator exported only on cross-package consumer' line so the promotion rule is stated once (cross-package consumer or explicit public-API need) and points to README.

…owner The barrel promotion rule is volatile when duplicated into inline source comments (consumer lists drift as symbols retire — e.g. streamVariants/ LiveIndicator in #712). Reverting the index.ts/chat.tsx comment edits keeps this PR docs-only and makes packages/ui/README.md the single source of truth (the README now says so explicitly). Cleaning up the stale source comments / dead symbols is a separate change.

Round 14 review: 'revert + README says comments may lag' still left the stale 'second consumer' / streamVariants/LiveIndicator consumer lists in the source comments, so the conflict source survived. Real root correction: the inline comments no longer re-derive the promotion rule or track consumers (that list drifts as symbols retire, e.g. #712) — they keep their local implementation intent and point at packages/ui/README.md for the rule. README stays the single owner; dead-symbol cleanup stays a separate change.

Round 15 found the prior root correction only covered 2 of the consumer/ promotion comment blocks; markerVariants/TextShimmer/toolVariants/previewVariants still re-derived the rule or tracked consumers, and streamVariants/LiveIndicator still assumed a call site that #712 removed. Exhaustively replace every such block: keep local implementation intent + a short pointer to the README, drop all consumer counts and promotion derivations. Also drop the README's 'inline comments may lag' line (no longer needed once the comments don't re-derive).

16 rounds of review showed cleaning inline source comments is a bottomless local patch: chat.tsx has many historical consumer/call-site mentions (incl. #712-retired streamVariants dead-code notes), and each fix surfaced an adjacent one. Root correction per receiving-code-review: keep this PR docs-only (3 READMEs), revert the index.ts/chat.tsx comment edits to main, and make packages/ui/README.md the single owner of the barrel promotion rule. Source-comment cleanup and dead- symbol removal are separate changes.

@theme

#725) * docs(ui): add architecture README for @maka/ui Target-oriented README for the shared UI package: four export surfaces (primitives / ui.tsx / top-level features / components.tsx), the off-barrel convention, the data-slot hook rule with its exceptions, where new code goes, and the ui.tsx→primitives convergence direction. Transitional surfaces are marked with direction + end state, not TODOs. * docs(desktop): add architecture READMEs for the desktop app and renderer Target-oriented READMEs for the Electron app shell and its renderer: the main/preload/renderer split, the main naming convention, the three-pattern IPC contract and the registerIpc() registration step, the actual main.ts startup order, the renderer AppShell + app-shell-<scope>-<action> split, the styles/tokens layout (with the --foreground-N wash-vs-text split), and the primitive-first authoring rule. Direction + end state only, no TODOs. * docs(ui): add architecture README for @maka/ui * docs(desktop): add architecture READMEs for the desktop app and renderer * docs(frontend): fix round-3 review findings in READMEs - ui: drop the broken @maka/ui/icons ProviderLogo example (it lives in the renderer); note icons re-exports Lucide symbols; use markerVariants as a real off-barrel example instead of the zero-consumer LiveIndicator; clarify 'runtime consumer' (preload imports types). - renderer: describe AppShell slices as app-shell-* one-concern modules (not a strict two-segment rule several existing slices violate); make the reference-shell.css breadcrumb point at the file's own header. - desktop: fix startup order (window created early, background startup concurrent, handlers before renderer entry that prefetches pre-mount); route main→renderer push through safeSendToRenderer (raw webContents.send throws on destroyed windows); add src/global.d.ts to the new-IPC steps. * docs(frontend): fix round-4 review findings in READMEs - desktop: window is created hidden, revealed after first AppShell paint (notifyRendererReady gate), not 'preload skeleton shows within ms'; drop the false 'background mutations always push via channels, UI converges lazily' invariant (interrupted-session recovery doesn't emit). - ui: scope the off-barrel 'don't re-export' rule to single in-package consumers with no cross-package consumer (previewVariants is re-exported for exactly that cross-package reason), resolving the contradiction with the promotion rule. * docs(frontend): fix round-5 review findings in READMEs - desktop/renderer: window reveal has a fallback timer, and main.tsx's onboarding prefetch can time out to a fail-soft loading state — stop claiming 'only after first paint' / 'never sees skeleton' / 'no loading flash' as absolute paths. - renderer: maka-tokens.css is the main token source, but a few @theme Tailwind-bridge values (e.g. --shadow-maka-panel) live in styles.css and are contract-pinned there — document the exception instead of claiming a single source. - ui: clarify 'model-provider brand logos' (renderer settings/provider-*); bot-provider logos are in @maka/ui's bot-brand-logo. * docs(frontend): fix round-6 review findings in READMEs - renderer: document the contract-pinned index.html inline .maka-preload skeleton (hardcoded colors, no CSS vars — maka-tokens.css hasn't loaded yet) as the narrow exception to 'styles.css is the only CSS entry'. - ui: resolve the barrel-rule contradiction — new feature components re-export from components.tsx, but only reach index.ts when they have a second or cross-package consumer (primitives are always re-exported). * docs(ui): fix the feature-component barrel rule (index.ts re-exports components.tsx) index.ts does 'export * from ./components.js', so re-exporting a feature component from components.tsx already puts it on the package barrel — there is no separate 'add to index.ts later' stage. Rule now: relative import while single in-package consumer; re-export from components.tsx (barrel follows automatically) once a second or cross-package consumer appears. * docs(renderer): fix the @theme bridge and local-var rule The @theme Tailwind bridge is split: most aliases (color/typography/spacing/ radius) live in maka-tokens.css, a few values (e.g. --shadow-maka-panel) in styles.css — both contract-pinned, so check which file owns a value before moving it. /* local: ... */ is the rule for new component-local vars; existing ones don't all carry it yet. * docs(renderer): describe the @theme bridge accurately (overlaps, not split) Both maka-tokens.css and styles.css carry an @theme inline block, and their color aliases overlap (--color-background/accent/muted appear in both); styles.css also carries the typography/line-height/font-weight/tracking/ spacing/radius/shadow bridges. Each value's home is contract-pinned (spacing/letter-spacing/foreground-tier contracts), so stop describing it as a clean split and point to the owning contract instead. * docs(frontend): fix round-10 review findings - ui: barrel promotion is cross-package consumer or explicit public-API need, not 'second in-package consumer' (attachment-file-card has two in-package consumers but stays off-barrel); remove the markerVariants example that implied otherwise. - renderer: maka-tokens.css tail is a large recipe section (base/utilities/ recipes/animations), not 'a few fallbacks'; the @theme bridge overlap is concrete (--color-muted maps to --foreground-5 in styles.css but --muted in maka-tokens.css); only some bridge values are contract-pinned (spacing/letter-spacing/foreground-tier), overlapping color aliases are not — don't claim 'each' is pinned. * docs(frontend): fix round-11 review findings - renderer: stop claiming overlapping color aliases have no contract pin — some do (--color-control in styles.css via design-system-governance-406; --color-muted-foreground in maka-tokens.css via foreground-tier); only some (e.g. --color-background/accent/muted) are unpinned. - desktop: narrow the safe-send claim — the contract test scans a fixed file list for direct mainWindow.webContents.send forms; new *-ipc-main.ts files aren't auto-covered, so route through the guard in every new file. - ui: sync the index.ts and chat.tsx LiveIndicator comments to the README barrel rule (cross-package consumer or explicit public-API need, not a second in-package consumer; attachment-file-card precedent), so the README is the single source of the promotion rule. * docs(ui): correct the LiveIndicator/streamVariants comments (#712 retirement) The synced comments still described chat call sites and the tool stream as consumers, but #712 retired streamVariants/LiveIndicator from the tool body — they have no production consumer now (chat-stream-cascade-contract pins it). State that in the comments, and drop the contradictory 'LiveIndicator exported only on cross-package consumer' line so the promotion rule is stated once (cross-package consumer or explicit public-API need) and points to README. * docs(ui): revert the source-comment edits; README is the barrel-rule owner The barrel promotion rule is volatile when duplicated into inline source comments (consumer lists drift as symbols retire — e.g. streamVariants/ LiveIndicator in #712). Reverting the index.ts/chat.tsx comment edits keeps this PR docs-only and makes packages/ui/README.md the single source of truth (the README now says so explicitly). Cleaning up the stale source comments / dead symbols is a separate change. * docs(ui): point inline barrel comments at README (true root correction) Round 14 review: 'revert + README says comments may lag' still left the stale 'second consumer' / streamVariants/LiveIndicator consumer lists in the source comments, so the conflict source survived. Real root correction: the inline comments no longer re-derive the promotion rule or track consumers (that list drifts as symbols retire, e.g. #712) — they keep their local implementation intent and point at packages/ui/README.md for the rule. README stays the single owner; dead-symbol cleanup stays a separate change. * docs(ui): exhaustively point all chat.tsx barrel comments at README Round 15 found the prior root correction only covered 2 of the consumer/ promotion comment blocks; markerVariants/TextShimmer/toolVariants/previewVariants still re-derived the rule or tracked consumers, and streamVariants/LiveIndicator still assumed a call site that #712 removed. Exhaustively replace every such block: keep local implementation intent + a short pointer to the README, drop all consumer counts and promotion derivations. Also drop the README's 'inline comments may lag' line (no longer needed once the comments don't re-derive). * docs(ui): revert all source-comment edits — PR is docs-only (3 READMEs) 16 rounds of review showed cleaning inline source comments is a bottomless local patch: chat.tsx has many historical consumer/call-site mentions (incl. #712-retired streamVariants dead-code notes), and each fix surfaced an adjacent one. Root correction per receiving-code-review: keep this PR docs-only (3 READMEs), revert the index.ts/chat.tsx comment edits to main, and make packages/ui/README.md the single owner of the barrel promotion rule. Source-comment cleanup and dead- symbol removal are separate changes.

…pulse (#736) #712 retired the tool live-output stream onto the quiet ToolTrow panel, leaving streamVariants (shell cva), LiveIndicator (live dot), and @Keyframes maka-pulse with zero production consumers. Remove all three and update the tests/contracts that referenced them: flip the chat-stream-cascade contract from "present" pins to a "stays removed" guard, drop the chat-primitives tests + imports, the maka-pulse keyframe + its frame contract, the 406 allowlist entry, the story demo, and the obsolete barrel enumeration + maka-pulse_ banned guard. Stale prose cross-references are deferred to a follow-up comment-cleanup PR. Refs #712.

…script header The header of scripts/check-chat-marker-computed-style.mjs still claimed the harness diffs the retired PR3 stream shell (panel / header / flags + pills / body / chunk, with data-live="true") and "proves the tool live-output stream shell migration (#332 / PR3)". Since #712 / #736 retired that shell, the fixture only constructs the quiet tool-output panel (panel / command / body, same production classes on both sides). Rewrite the two stale header passages to state the current coverage: the marker-migration baseline + the quiet-panel layout-invariant check, with the PR3 stream shell marked as retired history. Comments only; no code changed. The chat-stream-cascade-contract fixture-alignment assertions still pass (they match code identifiers, not header prose). Found by Codex review of #742.

* docs: drop stale stream-substrate cross-references from comments #736 removed streamVariants, LiveIndicator, and @Keyframes maka-pulse. Strip the prose that still named those symbols as current examples, peers, or retirement destinations, leaving the #736 "stays removed" guard and the accurate retirement narratives intact. Comments only — no code or CSS declarations changed. - packages/ui/src/primitives/chat.tsx: TextShimmer / toolVariants / previewVariants docs no longer cite LiveIndicator / maka-pulse / streamVariants as live peers. - apps/desktop/src/renderer/maka-tokens.css: maka-text-shimmer keyframe doc cites maka-tool-pulse (still in use) instead of the removed maka-pulse. - scripts/check-chat-marker-computed-style.mjs: header drops the stale LiveIndicator / maka-pulse "one element not diffed" sentence; the PR3b section's maka-tool-pulse running-dot note still stands. - design-system-governance-406-contract.test.ts: animation-name extraction example uses maka-tool-pulse (in use) instead of maka-pulse. - chat-stream-cascade-contract.test.ts: drop the dead "onto the shared maka-pulse" destination from the per-feature breath comment. Verified: chat-stream-cascade-contract + chat-tool-card-cascade-contract + design-system-governance-406 contract tests (15/15) and @maka/ui + @maka/desktop (main/renderer/storybook) typechecks all green. * docs: correct retired stream-shell coverage claims in computed-style script header The header of scripts/check-chat-marker-computed-style.mjs still claimed the harness diffs the retired PR3 stream shell (panel / header / flags + pills / body / chunk, with data-live="true") and "proves the tool live-output stream shell migration (#332 / PR3)". Since #712 / #736 retired that shell, the fixture only constructs the quiet tool-output panel (panel / command / body, same production classes on both sides). Rewrite the two stale header passages to state the current coverage: the marker-migration baseline + the quiet-panel layout-invariant check, with the PR3 stream shell marked as retired history. Comments only; no code changed. The chat-stream-cascade-contract fixture-alignment assertions still pass (they match code identifiers, not header prose). Found by Codex review of #742.

Astro-Han · 2026-07-10T09:42:14Z

Summary

Collapse ordinary tool diagnostics by default while keeping permission and error states visible.
Preserve explicit user disclosure choices across normal tool lifecycle transitions.
Replace transport chunk counts with user-relevant diagnostic flags.
Carry an optional semantic activityKind from tool declarations through runtime events, persistence, replay, and live desktop state.

Why

Tool activity rows exposed raw commands and transport details such as STDOUT N, while lifecycle changes also controlled whether details were open. Long PowerShell commands made this especially visible on Windows, but the underlying presentation-state coupling was platform-independent.

Scope

Changed:

Added a pure tool activity presentation and disclosure policy.
Added the backward-compatible ToolActivityKind contract and legacy tool-name fallback.
Classified built-in read, search, edit, and command tools.
Added regression coverage across runtime, UI, replay, and desktop live events.

Not included:

Tool execution behavior or shell command generation.
OS-specific command rewriting.
Changes to legacy session storage requirements.

Verification

npm run -w @maka/runtime test — 1134 passed, 2 environment-dependent skipped
npm run -w @maka/ui test — 52 passed
npm run -w @maka/desktop test — 2335 passed
npm run typecheck — passed for all workspaces

User-facing impact

Ordinary tool activity stays compact, permission requests and failures remain prominent, and internal stdout/stderr chunk counts are no longer shown. The optional persisted field is backward-compatible; no migration, documentation, or changelog change is required.

Reviewer notes

The main review seams are packages/ui/src/tool-activity/presentation.ts and the activityKind projection path. Legacy rows and custom tools without a declared kind continue to use tool-name inference.

A Windows screenshot is not included because this workspace has no Windows renderer. The platform-independent disclosure and text behavior is covered by contract tests.

Checklist

Scope matches the PR title and excludes unrelated changes
Verification lists commands and results
User-facing impact, docs, changelog, migrations, and breaking changes are noted
Risk, rollback, and review focus are called out
UI screenshot absence is explained