Skip to content

fix(runtime-host,ui,cli): name live tool calls on compact and collapsed rows - #3376

Open
me2seeks wants to merge 3 commits into
apache:mainfrom
me2seeks:fix/tool-compact-row-target
Open

fix(runtime-host,ui,cli): name live tool calls on compact and collapsed rows#3376
me2seeks wants to merge 3 commits into
apache:mainfrom
me2seeks:fix/tool-compact-row-target

Conversation

@me2seeks

@me2seeks me2seeks commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Closes #3336. Closes #3338.

Problem

Collapsed tool rows on Desktop and compact rows on the TUI rendered name-only● Bash, ● Task Create — with no hint of what the call does. Two distinct causes, one shared root:

  1. Shared root (live window): Runtime Host live tool_start frames omit args entirely (lean-channel decision from feat(cli): add Runtime Host-backed TUI sessions #2308), and durable args arrive only with the turn-end transcript reconcile. So during the one window a user actually watches, neither surface can name the call.
  2. Desktop-specific: even after args arrived (and on history replay), the collapsed row/group-header target read only item.intent, which describeToolIntent produces solely for ExploreAgent — every other tool rendered ● Name forever.

What changes

Layer Change
@maka/core formatToolInvocationLine gains per-tool lines for task_create (first subject + count), task_update (subject / id → status), GoalSet (condition), AskUserQuestion (first question + count). New projectToolArgsPreview(toolName, args) builds a bounded, redacted, whitelist-shaped args subset for the live wire (never file bodies or option payloads; sensitive keys dropped structurally; every string through redactSecrets; per-string 240 chars, whole preview 2 KB).
runtime-host wire Live tool_start frames carry optional intent (pass-through, 512 B) and argsPreview (≤8 KB). The strict decoder accepts and bounds both. RUNTIME_HOST_COMPATIBILITY_EPOCH bumped 59 → 60: older clients reject unknown keys on this event and would tear the connection down.
Desktop (@maka/ui) Collapsed row + collapsed group header target: intent ?? firstLine(formatToolInvocationLine(args ?? argsPreview)), hard-capped at 120 chars. Works live (preview), after settle (full args), and on history replay (persisted args) — history needs no wire change.
TUI (packages/cli) Compact rows consume argsPreview while live (turn-end reconcile still replaces it with durable full args). The dim (no output) placeholder now appears only when the row cannot name the call● Bash $ git add -A no longer carries the disclaimer. Empty args objects no longer render as input: {} noise.

formatToolInvocationLine stays client-side, so each surface formats in its own locale; the host ships data, not text.

Verification

  • Clean affected/full workspace build passed on current origin/main (a956b1ae0) after applying the repository dependency patches.
  • 268/268 focused tests passed across Core preview projection, Runtime Host protocol/coordinator/projector, CLI transcript, UI collapsed-row presentation, and Desktop redaction contract.
  • Biome passed for all 19 changed files; git diff --check is clean.
  • git range-diff confirms the rebase preserves the reviewed three-commit semantics; the intentional protocol change is the fresh epoch 60 ledger entry above main epochs 57–59.

Notes / follow-ups

AI use

  • Generative tooling made a substantive contribution
  • No generative tool made a substantive contribution

Tool(s) and scope: Maka (AI coding agent) authored the implementation and tests; the diff was human-reviewed before push. Generated-by: Maka trailers are present on the branch commits.

@me2seeks
me2seeks force-pushed the fix/tool-compact-row-target branch from c8d63d8 to 61391a6 Compare August 22, 2026 13:43
@Astro-Han

Copy link
Copy Markdown
Contributor

Heads up — this is currently conflicting with main, so I can't review or merge it as-is. Your CI is fully green, so it really is just the base that's behind.

One thing worth knowing: #3397 landed on 2026-08-22 and added ASF license headers across ~2685 files, so the rebase will touch more than you'd expect, and any file you add now needs a header (npm run write:asf-headers).

Ping me once it's rebased and I'll pick it up.

@Astro-Han

Copy link
Copy Markdown
Contributor

Independent review of 61391a6eb498539dc20a69e8bbf638c1ebfbf009. Three [P2]s, all reproduced against this head. Posting them here rather than inline: each defect's decisive line sits in unchanged context, so GitHub cannot anchor a review comment to it.

[P2] A Bash row that was redacted while live shows the original value again once the turn settles

packages/cli/src/pi-transcript-tools.ts:727-743 — the Bash case returns early:

return `$ ${firstRealLine ?? command.split('\n')[0]!.trim()}`;   // before the shared formatter

Two things combine. projectToolArgsPreview redacts only the live copy — and then pi-transcript.ts:385-388 does entry.input = structuredClone(durable.input), replacing it with the full, unredacted durable args. Meanwhile this Bash branch returns before reaching the shared formatToolInvocationLine, so the shared redactor never sees the text.

Reproduced with a placeholder secret: the live row renders Authorization: Bearer <redacted>; after reconcileToolsWithStoredMessages runs over that same row, it renders the placeholder's original value.

Why this is worse than "not redacted at all": the row was already redacted in front of the user, and then un-redacted itself. Someone who watched it go by has no reason to look again, and anyone reading the scrollback later sees the raw value with nothing indicating it was ever meant to be hidden. A redactor that reverses itself breaks a promise it already made.

Existing tests cover the live preview only — nothing exercises the live → durable reconcile, which is why the round trip passes.

Minimal fix: keep the "first non-comment command line" presentation (it is genuinely the useful part), but route the final text through the shared redactor instead of returning early, plus a regression asserting a placeholder secret does not reappear across reconcile.

[P2] A synthesized empty args = {} hides the live WriteStdin preview

packages/ui/src/live-turn-projection.ts:329-341 with packages/core/src/tool-activity-args.ts:163-179. projectToolActivityArgs('WriteStdin', undefined) is the only path returning {}. Given a Host WriteStdin tool_start with no full args but a populated argsPreview (inputPreview={text:"ls -la\n",bytes:7,truncated:false}, size={cols:80,rows:24}), the projected item carries both a complete argsPreview and args = {}.

The wrapper picks with args ?? argsPreview. {} is non-nullish, so it wins, and the row collapses to Background terminal interaction — the typed input and the 80x24 geometry both vanish, in precisely the case this PR exists to improve.

Why the tests miss it: the UI tests construct items with args: undefined directly and the core tests format the preview directly — neither goes through the real applyLiveTurnEvent, which is the only place that empty object is manufactured. Both sides test around it.

Fix: do not synthesize args when the event has no full args, and add a WriteStdin regression that runs through live projection.

[P2][design] The new live-preview allowlist omits current built-in structured tools

packages/core/src/tool-quiet-preview.ts:318-361,429-490. projectToolArgsPreview('deep_research_start', {objective:'…', scope_level:'standard'}) returns undefined, so the live row shows the bare tool name — while the durable formatter, given the same args, prints objective: … and scope_level: standard. deep_research_record_step keeps only status and drops the objective.

The durable generic formatter reads arbitrary structured fields; the new preview authority permits 18 scalar keys, and neither objective nor scope_level is among them. The tests enumerate the headline-shaped tools that were picked, without cross-checking the live preview against the current built-in schema.

This is not a hypothetical MCP tooldeep_research_start is a reachable built-in today (packages/runtime/src/deep-research-tools.ts:124-148). Fix by admitting a bounded/redacted objective (and stating the policy for the other current structured tools), with a built-in-coverage test.

Where the entropy actually is

The file count is not the problem — 12 production and 7 test files, 352 production additions against 437 test. Propagation across core, wire, and two clients is a real contract boundary.

The problem is that "the invocation name" now has two authorities in each of two places: one shared core formatter plus 8 tool-specific branches in the CLI (Bash / Read / WriteStdin / Write / Edit / Grep / Glob / maka_computer), and separately the ARGS_PREVIEW_* allowlist versus the durable generic formatter's wider field access. Findings 1 and 3 are not two unrelated bugs — they are the observable drift of those two pairs.

Convergence worth aiming for: the shared redacted invocation formatter is the single semantic authority, surface-specific code decorates only, and preview coverage has a checkable contract against the current tool catalog rather than a hand-maintained parallel field table.

Candidates raised and withdrawn

  • ExploreAgent / legacy agent_swarm objective — the current call name is ExploreAgent and describeToolIntent already supplies a bounded, redacted intent. Withdrawn.
  • Arbitrary MCP {owner,repo} unnamed while live — behavior confirmed, but withdrawn as a separate finding since deep_research_start already demonstrates the same design gap via an in-repo path.
  • Read / StopBackgroundTask live polls still reading event.args — pre-existing folding, not a regression within this PR's naming charter. Withdrawn.
  • Removing (no output) once a target exists — an explicit product choice; the status dot and expanded card still carry the outcome. Withdrawn.
  • The decoder validating only byte bounds, not exact preview shape — the Host is the production authority; no trust-boundary bypass established. Withdrawn.

Gate state

  • CONFLICTING / DIRTY. Against origin/main=2e5fe9c5, merge-tree gives 2 content conflicts: packages/runtime-host/src/protocol/index.ts and packages/ui/src/tool-activity.tsx.
  • check-runs on this head: 0. gh run list --commit: 0. statusCheckRollup: empty. There is an older maintainer comment stating CI was green at the time — that is a historical description, not a binding gate on this head, and I am not treating it as one.
  • All three review endpoints read separately: 1 issue comment (a rebase request, no findings), 0 reviews, 0 inline comments. Nothing to deduplicate against.
  • Local on this exact head: clean tree, npm ci fine, Desktop dependency build fine, CLI + eval build fine, 8 focused test files 202/202 green. Three targeted probes reproduce all three findings above stably.

Blind line — provisional judgment sealed before the endpoints were read. Reviewed 2026-08-23 13:35 UTC. No overall verdict offered.

@me2seeks
me2seeks force-pushed the fix/tool-compact-row-target branch from 61391a6 to c05b843 Compare August 23, 2026 05:58
@me2seeks

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (2e5fe9c55). Three conflict resolutions:

  1. Compatibility epoch (protocol/index.ts): main had advanced to 39; this PR's wire change (optional intent/argsPreview on live tool_start frames) is now epoch 40 with both comment lists preserved.
  2. ui/tool-activity.tsx: merged main's inferredTarget parameter (accumulated Computer-Use target) with this PR's args-derived naming — precedence is runtime intent → caller-inferred target → first invocation line. collapsedToolTarget gains an optional preferred value; group-header callers are unchanged.

Verified on the new head: full root build, runtime-host 1092/1092 (incl. handshake-compatibility), CLI 385/385, biome clean. UI suite is 218/219 — the one failure (composer-plus-menu › a loading catalog holds the row still and marks the held state) reproduces identically on a clean checkout of current main, so it is pre-existing and unrelated to this branch.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at exact head c05b843952f1808417560c0a6e72b27c70c6c5df.

No P0–P2. The mechanism here is well chosen and the security-sensitive part is built the right way round.

The problem is real: live tool_start frames from the Runtime Host omit full args, so a compact row could only print ● Bash with nothing to say what ran until the durable transcript arrived at turn end. Rather than widen the wire to carry full args — a Write call would drag an entire file across — this adds a bounded, redacted argsPreview and has the display path read args ?? argsPreview, so durable replay keeps priority and never sees the preview.

Three details worth calling out as correct, because each is a place this kind of change usually goes wrong:

  1. projectToolArgsPreview uses an allowlist, not a denylist. ARGS_PREVIEW_SCALAR_KEYS enumerates the keys formatToolInvocationLine can actually render; file contents, option payloads and provider blobs have no path into the preview because they are simply not on the list. A denylist here would have needed updating every time a tool grew a new field.
  2. Redaction happens before truncation. boundPreviewString calls redactSecrets(value) and then slices to 240 chars. The other order is the classic bug — slicing first can leave a partial secret that the redactor no longer recognizes as one, and the truncated remainder ships.
  3. The bound is enforced at the decode seam, not just the encode seam. SESSION_TOOL_ARGS_PREVIEW_MAX_BYTES is checked in decodeSessionToolEvent alongside an explicit assertAllowedKeys, so a malformed or oversized preview is rejected on arrival rather than trusted because the sender was supposed to bound it.

The CLI-side change is the modest half and reads well: event.args ?? event.argsPreview in both the shell-poll and the general tool_start branch, plus suppressing the no output placeholder once the row can name the call. ● Bash $ git add -A (no output) really is worse than ● Bash $ git add -A.

[P3]'input' is the one broadly-named key on the scalar allowlist. For today's tools it resolves to something short, but unlike command / path / pattern it does not name a shape, so a future tool with an input field holding a payload would pass the allowlist and be bounded only by the 240-char cap and the redactor. Not a live defect; just the entry most likely to age badly. A comment noting which tools it exists for would keep the next reader from widening it further.

Not mergeable as-is: this head conflicts with current main in packages/runtime-host/src/protocol/index.ts (I confirmed by rebase, not just by the API flag). A rebase is needed before this can land; the conflict is in the export surface rather than in the new logic, so I don't expect it to disturb the review conclusions above.

Verification: exact-head test is completed/success. I did not run the Desktop or Playwright suites and am not claiming them.

@me2seeks
me2seeks force-pushed the fix/tool-compact-row-target branch 2 times, most recently from 7452d8a to 0b99e89 Compare August 23, 2026 13:30

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at exact head 27d506e07a6b89b80aa7ad5d5ca467bc5e21c6ba. No P0-P2 on the code. One [P3] inline about the rebase (the epoch number this branch claims is now taken on main). Exact-head test is terminal green (ran 2026-08-23 16:38-16:53).

Context: this head moved after the 2026-08-23 review at c05b8439 - a rebase onto a newer base plus 27d506e07, which resolves conflict markers the rebase left in protocol/index.ts. I verified that resolution directly: the final file has no markers, keeps both epoch comments, and the PR-authored deltas in all 19 files are semantically identical to the reviewed head (the only textual differences in e.g. pi-transcript.ts are base changes from main, not this PR). The 08-23 review's conclusions carry; the still-open [P3] from it ('input' as the one broadly-named scalar allowlist key, worth a comment naming the tools it exists for) was not addressed by the rebase and remains open - I did not re-raise it.

Independent checks done on this head:

  • Redaction order: boundPreviewString runs redactSecrets before the 240-char slice, in both projectToolArgsPreview and the new formatToolInvocationLine branches - the truncation cannot ship a partial secret the redactor no longer recognizes.
  • Bounds enforced three times: core caps each string (240) and the whole preview (2,048, dropping lowest-priority fields with the highest-priority present field always surviving); the host re-checks 8 KB before emitting (projectArgsPreviewForWire); the decoder re-checks at the seam (requireEncodedByteLimit) plus assertAllowedKeys. A formatter change cannot silently bloat frames.
  • Full args never cross the live wire: the coordinator test proves a 100 KB content field produces a preview of {command} only, and 'args' in event === false.
  • The 2 KB budget-drop loop terminates (droppable list shrinks each iteration) and protects the top-priority present field.
  • Naming authority stays client-side, which answers the cross-layer question: the host ships data (intent pass-through, argsPreview projected by the same @maka/core code the clients use), never text; both Desktop and TUI render through the shared formatToolInvocationLine. The surfaces can still differ by design - Desktop uses the UI locale and a 120-char first-line cap, the TUI formats in en - but they cannot each compute a different name from the same data, because there is one formatter and one data source per window (preview live, full args after reconcile, persisted args on replay). args ?? argsPreview ordering means durable args always win over the preview once they exist.

Gate: exact-head test green; the branch is CONFLICTING against main, which blocks merge but not review. Not mergeable as-is - see the inline for what the rebase must do.

Comment thread packages/runtime-host/src/protocol/index.ts Outdated
me2seeks added a commit to me2seeks/maka-agent that referenced this pull request Aug 23, 2026
…ed rows

Rebase of apache#3376 onto current main as one clean change: live tool_start
frames may carry optional intent / argsPreview keys so compact and
collapsed rows can name the call before durable args arrive. The strict
decoder's allowed-key union retains main's shellRunRef alongside them,
and correlated hidden-shell polls keep publishing only their
correlation ref. Compatibility epoch advances to 45.

Generated-by: maka
@me2seeks
me2seeks force-pushed the fix/tool-compact-row-target branch from 27d506e to d0be632 Compare August 23, 2026 22:35
@me2seeks

Copy link
Copy Markdown
Contributor Author

Rebased as a single clean change onto main (c79e9eb4): epoch is now 45 with the rewritten history entry, and decodeSessionToolEvent's allowed-key list is the three-way union (intent, argsPreview, plus main's shellRunRef). One deliberate reconciliation: correlated hidden-shell polls keep publishing only their correlation ref (main's #3569 minimal-frame contract), so argsPreview synthesis skips exactly those frames — every other live tool start still names itself for compact rows. Epoch guard passes against the new base; runtime-host 1127 green.

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review of d0be632d7ef9aade5f23358cc1cdb902e2fa0a70.

GO. No P0–P2. 1×P3 on leftover epoch changelog text. MERGEABLE / BLOCKED. Not approving: no terminal-green test on this head (CI action_required). Current main test is green.

What this solves

Live tool_start frames were name-only because the lean channel omits args. Bounded, redacted argsPreview plus optional intent, consumed as intent ?? formatToolInvocationLine(args ?? argsPreview), is the minimal live-window fix. Hidden-shell poll frames still ship shellRunRef without a preview — correct.

Conflict resolution (the two hard checks)

  • tool_start allowed-keys on this head is the three-way union: shellRunRef + intent + argsPreview (session-continuity.ts decoder). Independently confirmed; I am not re-filing the old P3.
  • Epoch bump is required: older Clients use a strict allowed-key list and tear down on unknown keys. A compatible-change declaration would be wrong.
  • After rebase onto current main, read RUNTIME_HOST_COMPATIBILITY_EPOCH there and take a number greater than that — do not treat this head's integer as reserved.

See inline for the changelog leftover.

Comment thread packages/runtime-host/src/protocol/index.ts Outdated

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at d0be632d. One [P2] and one [P3], both inline. Not approving on this head.

What holds: tool-call identity survives the change. The live projection is keyed by toolUseId, terminal reconcile and durable hydration line up, and the compact/replay name fallback preserves the mapping — so a call does not change identity or lose its counterpart across compaction, which was the main risk here.

中文

d0be632d 上审,一条 [P2] 加一条 [P3],均为行内;本 head 不 approve。

成立的部分:工具调用的身份在本次改动下是保住的——实时投影以 toolUseId 为键,终态 reconcile 与持久化 hydration 对得上,压缩/重放的名称回退也保留了对应关系,因此调用不会在压缩前后换身份或丢失对应项,而这正是这个改动最主要的风险面。

Comment thread packages/core/src/tool-quiet-preview.ts
Comment thread packages/cli/src/pi-transcript.ts Outdated
me2seeks added a commit to me2seeks/maka-agent that referenced this pull request Aug 25, 2026
…ed rows

Rebase of apache#3376 onto current main as one clean change: live tool_start
frames may carry optional intent / argsPreview keys so compact and
collapsed rows can name the call before durable args arrive. The strict
decoder's allowed-key union retains main's shellRunRef alongside them,
and correlated hidden-shell polls keep publishing only their
correlation ref. Compatibility epoch advances to 45.

Generated-by: maka
@me2seeks
me2seeks force-pushed the fix/tool-compact-row-target branch from d0be632 to ac77480 Compare August 25, 2026 02:26

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found blocking issues.

[P2] TUI drops intent for Explore

pi-transcript.ts:573-588 only collects displayName + args, not the new intent. The preview lacks intent, so live display remains ● Explore (running) without hydration fix.

[P2] Desktop WriteStdin preview hidden by empty object

live-turn-projection.ts:355-373 projects undefined args to {}, so builtin-preview always sees {} instead of wire argsPreview with Enter/80x24.

[P3] Compatibility epoch ledger duplicated

protocol/index.ts history is inaccurate and duplicated; needs single accurate ledger.

Checks on ac77480df5 are test: failure + format:check failure — not green.

简体中文存在两项展示阻断与一项兼容记录问题。

@github-actions github-actions Bot added the effort/L Under 1000 readable lines label Aug 27, 2026
@Astro-Han

Copy link
Copy Markdown
Contributor

This is a synthesis of the independent blind review by @Ox-Qronos at exact head 9e2f378b110909b17b9bb2ee0d27dd77747416a6 (base 38f0a275c, delta from ac77480d). I verified the drift and the exact-head CI myself; the file:line findings below are from Ox's sealed delta review.

What I checked myself:

  • Read the bounded diff ac77480d → 9e2f378b and confirmed the two new PR-owned commits: 747f7c3e6 (display + epoch ledger fix) and 09734579d (list-subject redaction).
  • Checked exact-head CI: test run 32992286767 SUCCESS, windows_recovery run 32992287189 SUCCESS.
  • Checked merge state: mergeStateStatus=CONFLICTINGmain has advanced 38f0a275c → 4cc781f31 since the last merge.

Findings from Ox's review:

Code — GO — prior 3 findings are all closed and the two new commits are sound.

  • Closed P2 — TUI dropping Explore intent: now closed. Durable detail fills entry.intent at pi-transcript.ts:456, events and calls propagate it (:660/:961), render cache key includes intent (:1417), and live-turn-projection also preserves it (:357).
  • Closed P2 — Desktop WriteStdin preview swallowed by empty object: now closed. live-turn-projection.ts:362-366 keeps wire argsPreview, builtin-preview.ts:47 uses args ?? argsPreview, and projectToolArgsPreview first passes the canonical activity projection so WriteStdin's inputPreview shape is caught by the whitelist.
  • Closed P3 — Epoch ledger duplication/inaccuracy: now closed. protocol/index.ts is now a single ledger (duplicate 30 removed, missing 40 added, head now epoch 54 with an accurate description of intent/argsPreview).
  • New commit 747f7c3 (display + ledger): as above, correctly landed.
  • New commit 0973457 (list redaction): previewListSubjects for tasks/questions — sensitive keys are rejected column-wise, subject text is sanitized via safeKeyLabel (masks sensitive payload then redactSecrets), entries that become empty after sanitization are dropped (prevents empty-label injection). 15 new tests ('password=short-secret'→'<redacted>' etc.) pass. Fail-closed. Local core 15/15 green; red/green against merge-base shows the new tests fail without the patch and pass with it.

Gate — NO-GO — CONFLICTING after main advanced. The PR now conflicts with main on one file: packages/runtime-host/src/protocol/index.ts — epoch constant (PR is 54, main at 4cc781f31 is 55), so a mechanical rebase (bump the PR's epoch to 55) resolves it. merge-tree shows no other conflicts. Hosted checks remain green, but the PR cannot be merged until rebased to 4cc781f31.

Verification: git diff --check PASS; changed-file checks PASS; head unchanged; OPEN/MERGEABLE → CONFLICTING. No Docker cells.

What I did not re-do: a full first-pass derivation — this was a delta rebind plus two new-commit reads; ui layer tool-activity-presentation local render tests fail locally due to a dual-React-instance environment issue (invalid hook call) that is not PR-attributable and is covered by the green hosted test.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-reviewed exact head 9e2f378b110909b17b9bb2ee0d27dd77747416a6. One of the three earlier issues is fixed, but two P2 issues remain.

  1. packages/cli/src/pi-transcript.ts still replaces the redacted live Bash preview with the durable full arguments during hydration, and the Bash-specific branch in packages/cli/src/pi-transcript-tools.ts returns the command before the shared redactor runs. A production-path probe showed the same compact row changing from Authorization: Bearer <redacted> while live to the original token after durable hydration. Please route the final Bash line through the shared redactor and add a live-to-durable regression that proves the token never reappears.

  2. deep_research_start is still unnamed during its live phase. projectToolArgsPreview() returns no preview for its supported objective and scope_level fields, while the durable formatter later displays both. This is a current built-in Runtime tool, not a hypothetical extension. Please give it a bounded, redacted naming projection—or derive the preview from an explicit tool naming contract—and add a live-to-durable convergence test.

The former Desktop WriteStdin issue is closed: the projection now chooses event.args ?? event.argsPreview before canonicalization, and the production-seam test preserves both the input preview and terminal size.

There is also a non-blocking cleanup issue: the generic live-preview allowlist includes an unowned scalar input field, so an arbitrary MCP tool can place up to 240 characters of user-authored body text on the lean live wire. It is bounded and credential-redacted, but removing that generic key or requiring tool-specific ownership would keep the contract narrower.

Separately, this head is CONFLICTING/DIRTY against current main, with a conflict in packages/runtime-host/src/protocol/index.ts. The branch uses compatibility epoch 54 while main is at 55; the rebase must preserve the existing entries and assign this incompatible wire change a fresh epoch (56 with the current ledger). The exact-head test and windows_recovery checks succeeded, and 286 focused tests passed locally, but a new head still requires fresh review.


Posted by an automated review agent operated by @WAWQAQ. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.

简体中文

本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。

me2seeks added a commit to me2seeks/maka-agent that referenced this pull request Aug 28, 2026
…ed rows

Rebase of apache#3376 onto current main as one clean change: live tool_start
frames may carry optional intent / argsPreview keys so compact and
collapsed rows can name the call before durable args arrive. The strict
decoder's allowed-key union retains main's shellRunRef alongside them,
and correlated hidden-shell polls keep publishing only their
correlation ref. Compatibility epoch advances to 45.

Generated-by: maka
@me2seeks
me2seeks force-pushed the fix/tool-compact-row-target branch from 9e2f378 to 0ba90c5 Compare August 28, 2026 02:20
@jackwener
jackwener requested a review from Astro-Han August 28, 2026 04:15

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-reviewed exact head 0ba90c552ac06e0ae22520c50a3c7fda3573692c. The code is ready from this review: no P0–P2 findings remain. One non-blocking documentation cleanup remains.

The three issues from the previous review are closed:

  • The CLI now keeps Bash arguments redacted after the live row is hydrated from the durable transcript. Both the compact input and multiline shell_run.cmd paths pass through the shared redactor, and the new live-to-durable regression proves a bearer token does not reappear.
  • deep_research_start now has a bounded live name derived only from objective and scope_level; artifact_content does not enter the preview frame. The live and durable presentation now converge.
  • Generic input is no longer admitted for arbitrary tools. Tests through the Runtime Host production projection show that raw input, forged inputPreview, and terminal size shapes do not enter a third-party tool's live frame; those shapes remain available only to WriteStdin.

The preview contract remains bounded and redacted: individual display strings are capped at 240 characters, the projected object at 2 KiB, and the transport frame at 8 KiB. I also checked the broader tasks and questions fields; they carry the same bounded display-headline semantics as subject, title, and query, so I found no separate exposure issue there.

[P3] Please refresh the PR description before merge. It still says the compatibility epoch moves 29 → 30, while this head correctly moves current main from 56 → 57. The verification counts in the description are also from an older revision. This does not affect the implementation, but the merge record should describe the code that will land.

Verification on this head: Core 692/692, Runtime Host 1310/1310 (9 environment skips), UI 237/237, CLI transcript 97/97, and Desktop redaction contract 3/3; affected builds and diff checks pass. Hosted test and windows_recovery are both completed successfully. The head merges cleanly with current main (10470e620304a7887a46e896feaa508f6165e9f4), producing tree 69d4621f76aa5057ce7524f24373c8442c568d83. All four review threads are resolved.


Posted by an automated review agent operated by @WAWQAQ. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.

简体中文

本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。

…ed rows

Rebase of apache#3376 onto current main as one clean change: live tool_start
frames may carry optional intent / argsPreview keys so compact and
collapsed rows can name the call before durable args arrive. The strict
decoder's allowed-key union retains main's shellRunRef alongside them,
and correlated hidden-shell polls keep publishing only their
correlation ref. Compatibility epoch advances to 45.

Generated-by: maka
- pi-transcript: split joined push line (Biome)
- tool-quiet-preview: redact list subjects via isSensitiveKey/redactSecrets
- protocol: restore missing 40 and drop duplicate 30

Generated-by: maka
@me2seeks
me2seeks force-pushed the fix/tool-compact-row-target branch from 0ba90c5 to 76fc5a1 Compare August 28, 2026 08:18
@me2seeks

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and force-pushed head 76fc5a10f. The semantic range is unchanged; the protocol ledger now preserves main epochs 57–59 and assigns this strict tool_start wire expansion epoch 60. Clean workspace build passed, focused affected tests are 268/268, and Biome/diff checks are clean. The PR description now reflects the current epoch and validation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/L Under 1000 readable lines

Projects

None yet

3 participants