Repository navigation
fix(native-chat): stop rendering tool output as the agent's streaming reply - #17782
Conversation
… reply A tool result could appear in native chat as a raw, un-collapsed "assistant" bubble that never went away for the rest of the turn — on mobile it showed up as a wall of a source file's contents, prefixed by "Exit code 1". Providers publish a tool's stdout/error as `lastAssistantMessage` so status cards and dashboard rows can preview what the agent just did. Native chat reuses that same field as its live streaming bubble, so the preview rendered as prose. For Claude the preview is *only ever* tool output mid-turn: claude-tool-fields writes real prose exclusively at Stop, so the bubble could never contain an actual streaming reply. It also could not be retired. The bubble hides once a transcript assistant block leads with the streamed text, and tool output never lands in one — so the only remaining exit was the turn ending, which is why a long tool-heavy turn pinned it on screen. Carry provenance instead of changing what the status surfaces show: mark the writes that come from a tool result/error, keep the flag in lockstep with the value it describes through the listener merge, and have both native-chat streaming paths ignore a flagged preview. Status cards, dashboard rows and automation capture are untouched. The wire field is optional, so an older host that never sends it keeps today's behavior rather than silently suppressing previews.
Greptile SummaryThe PR adds explicit provenance for agent-status messages sourced from tool output and uses it to prevent desktop and mobile native chat from rendering those previews as assistant replies.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/shared/agent-hook-listener/prompt-fields.ts | Keeps message provenance aligned with inherited, replaced, and explicitly cleared assistant-message values. |
| src/shared/agent-status-types.ts | Adds an optional normalized provenance field whose absent and false forms remain wire-compatible. |
| src/shared/native-chat-streaming.ts | Prevents flagged tool-output previews from becoming desktop native-chat streaming messages. |
| mobile/src/session/mobile-native-chat-streaming-gate.ts | Filters flagged mobile previews as no observation while preserving the existing stream-segment gate. |
| mobile/src/session/use-mobile-native-chat-controller.ts | Applies provenance filtering before throttling the mobile streaming preview. |
| src/renderer/src/runtime/web-session-tabs-sync.ts | Preserves provenance through mirrored status reconciliation while keeping message and flag ownership consistent. |
| src/renderer/src/runtime/sync-runtime-graph.ts | Includes provenance in mobile status change detection so flag-only transitions are synchronized. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
P[Provider hook event] --> N[Normalize agent status]
N --> T{Tool output?}
T -- Yes --> F[Set provenance flag]
T -- No --> A[Assistant prose]
F --> S[Status synchronization]
A --> S
S --> C[Status cards retain preview]
S --> G{Native-chat gate}
G -- Flagged --> H[Hide streaming bubble]
G -- Prose --> R[Render streaming reply]
Reviews (2): Last reviewed commit: "test(native-chat): cover restored tool p..." | Re-trigger Greptile
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (11)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe change adds Merge Risk: ⚪ Minimal · up to The change prevents tool output from appearing as a persistent assistant reply while preserving existing status previews; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description clearly explains the bug, root cause, fix, compatibility impact, and verification. However, it omits required template sections and information, including ELI5, Why, Linked Issue with a populated issue reference, Visual Proof or an explicit N/A justification, Testing checklist, AI Disclosure, Review, Agent skill upstream boundary, Notes, and the final Checklist. Resolution Add all missing template sections. Provide the required linked issue, add before/after visual evidence or write exactly
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The new lastAssistantMessageIsToolOutput flag never reaches the renderer store, so the fix is currently a no-op end-to-end on the live path.
Reviewed changes
- Provenance flag — new optional
lastAssistantMessageIsToolOutputonToolSnapshot,AgentStatusEntry, andAgentStatusPayload, normalized soabsent | false → undefined(wire-compatible, only ever means "known tool output"). - Provider writers — 8
*-tool-fields.tsfiles flag the 11 sites that write a tool result/error intolastAssistantMessage; all*-events.ts/status-build files pass the snapshot flag through to the payload. - Lockstep resolution —
resolveToolStateinherits the flag only when the message itself is inherited, and clears both onclearLastAssistantMessage. - Consumers — desktop
deriveNativeChatStreamingTexthard-gates onpreviewIsToolOutput; mobile addsmobileNativeChatStreamPreviewtreating a flagged preview as "no observation". - Tests — normalization + desktop derivation + mobile gate unit tests, plus an ablation claim in the PR body.
⚠️ The flag is dropped before it reaches agentStatusByPaneKey
Both consumers read the flag off AgentStatusEntry: desktop via useAppStore((s) => s.agentStatusByPaneKey[paneKey]?.lastAssistantMessageIsToolOutput) (NativeChatResolvedView.tsx), and mobile via activeSessionTab?.agentStatus (an AgentStatusEntry). But the diff adds the flag to the payload vocabulary and never writes it into the renderer store's entries. Two unmodified sites drop it:
src/renderer/src/hooks/ipc-events/normalize-agent-status-event.ts:10-24rebuilds the payload field-by-field and omitslastAssistantMessageIsToolOutput, so the renderer ingress discards it even though main'sagentStatus:setevent carries it via the...payloadspread insrc/main/index.ts.src/renderer/src/store/slices/agent-status.ts(setAgentStatus, entry literal ~2325-2399) copieslastAssistantMessage: payload.lastAssistantMessagebut neverlastAssistantMessageIsToolOutput.
Consequently hookPreviewIsToolOutput is always undefined, deriveNativeChatStreamingText never sees previewIsToolOutput: true, and desktop native chat keeps rendering tool stdout as the streaming reply — the exact symptom this PR reports. The same drop applies to mobile whenever its session-tab agentStatus originates from the desktop host's agentStatusByPaneKey (the normal paired case); only headless serve's buildPtyMobileAgentStatus spreads the raw hook payload (liveRow.payload) and would carry it. The new tests cannot catch this because they exercise the pure functions and the payload normalization directly, never the store/ingress plumbing.
Technical details
# Propagate lastAssistantMessageIsToolOutput into AgentStatusEntry
## Affected sites
- src/renderer/src/hooks/ipc-events/normalize-agent-status-event.ts:10-24 — field-by-field rebuild omits the flag; add `lastAssistantMessageIsToolOutput: data.lastAssistantMessageIsToolOutput`.
- src/renderer/src/store/slices/agent-status.ts:2357 — entry construction copies `lastAssistantMessage` but not the flag; add `lastAssistantMessageIsToolOutput: payload.lastAssistantMessageIsToolOutput`.
- Also audit `doneRetentionFieldsChanged` (~line 2458) and any other entry-field comparisons if the flag should participate in retention change detection.
## Required outcome
- A tool result/error written to `lastAssistantMessage` must land in `agentStatusByPaneKey[paneKey].lastAssistantMessageIsToolOutput === true` so both native-chat streaming paths suppress the bubble.
- A later prose `Stop` must produce `undefined` (or `false`), matching the `absent | false → undefined` normalization contract.
## Suggested approach
- Add a store-level test: feed an `AgentStatusIpcPayload` carrying the flag through `normalizeAgentStatusEvent` → `setAgentStatus`, and assert the entry carries it; mirror a test for the snapshot restore path.
## Open questions for the human
- Mobile in the desktop-hosted case: confirm the session-tab `agentStatus` path (renderer store entry → runtime graph sync → `session.tabs`) preserves the flag after the store fix; otherwise mobile needs the same treatment on that projection.DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
|
Review status: land after fixes. The review found and fixed in-scope provenance propagation gaps that made the original desktop fix ineffective or could reintroduce stale suppression:
Verification:
Electron QA: an Orca-orchestrated Grok worker attempted the required Electron/CDP validation, but the dev Electron launch/CDP target remained unavailable/hung after bounded retries. No screenshot evidence was produced, so visual QA remains blocked and is not being claimed as passing. No SSH or mobile-emulator validation was run because this diff is desktop/shared status plumbing rather than an SSH/mobile UI change. |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Renderer ingress propagation —
normalizeAgentStatusEventnow passeslastAssistantMessageIsToolOutputthrough, andsetAgentStatuswrites it onto the entry (plusdoneRetentionFieldsChanged), closing the gap where the flag never reachedagentStatusByPaneKey. - Mobile projection + mirrored host path —
serializeRuntimeMobileAgentStatusEntry,buildMirroredAgentStatusPatch, andagentStatusEntryEqualall carry the flag, so both mobile and the paired-client mirror keep provenance in lockstep with the message. - Claude roster-state restoration — the three restore sites (
seedClaudeLeadTurnFromPersistedStatus,clearClaudePendingWaitForAgent,clearClaudeAnsweredQuestionWait) preserve the flag when restoring a stashed lead preview. - Regression tests — new tests exercise normalization → store ingress, sync-graph projection, web-tabs mirror, and subagent roster restoration; all pass (79 tests across the 5 changed test files).
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
Attachments
|
|
Visual QA unblocked after targeted process cleanup.
No source changes were made during this QA cleanup. |
|
Manual verification completed on the PR tip (c0367ce).
|
… reply (stablyai#17782) * fix(native-chat): stop rendering tool output as the agent's streaming reply A tool result could appear in native chat as a raw, un-collapsed "assistant" bubble that never went away for the rest of the turn — on mobile it showed up as a wall of a source file's contents, prefixed by "Exit code 1". Providers publish a tool's stdout/error as `lastAssistantMessage` so status cards and dashboard rows can preview what the agent just did. Native chat reuses that same field as its live streaming bubble, so the preview rendered as prose. For Claude the preview is *only ever* tool output mid-turn: claude-tool-fields writes real prose exclusively at Stop, so the bubble could never contain an actual streaming reply. It also could not be retired. The bubble hides once a transcript assistant block leads with the streamed text, and tool output never lands in one — so the only remaining exit was the turn ending, which is why a long tool-heavy turn pinned it on screen. Carry provenance instead of changing what the status surfaces show: mark the writes that come from a tool result/error, keep the flag in lockstep with the value it describes through the listener merge, and have both native-chat streaming paths ignore a flagged preview. Status cards, dashboard rows and automation capture are untouched. The wire field is optional, so an older host that never sends it keeps today's behavior rather than silently suppressing previews. * fix(native-chat): preserve tool output provenance through renderer sync * fix(native-chat): retain preview provenance in Claude roster state * test(native-chat): cover restored tool preview provenance --------- Co-authored-by: Merge Sim <sim@local>
What was broken
A tool result could render in native chat as a raw, un-collapsed "assistant" bubble that never went away for the rest of the turn. Reported from mobile: a wall of a source file's contents, prefixed by
Exit code 1, sitting in the chat while the agent kept working.Root cause
Two independent things combine.
1. The preview is tool output. Providers publish a tool's stdout/error into
lastAssistantMessageso status cards, dashboard rows and notifications can preview what the agent just did — deliberate, and pinned byserver-claude-normalization.test.ts(PostToolUseBash →lastAssistantMessage: 'tests passed'). Native chat reuses that same field as its live streaming bubble, so the preview renders as prose.For Claude this is not an edge case.
claude-tool-fields.tsis the only writer of the field, and it writes real prose exclusively atStop:PostToolUsePostToolUseFailureStopSo mid-turn the bubble can only ever contain tool output — it is structurally incapable of showing a streaming reply.
2. It cannot be retired. The bubble hides only once a transcript assistant block leads with the streamed text:
assistantTailTextreads assistant text blocks; tool output never lands in one, socaughtUpcan never fire. The only remaining exit is the turn ending — which is why a long tool-heavy turn pins it on screen for minutes.The fix
Carry provenance rather than change what the status surfaces show.
lastAssistantMessageIsToolOutput) across all 8 providers that do this — 11 sites.resolveToolState, so a later prose turn does not inherit a previous tool result's flag.deriveNativeChatStreamingText(desktop) and a newmobileNativeChatStreamPreview(mobile).Status cards, dashboard rows, activity threads, notifications and automation capture are untouched — they still get the preview.
Mobile treats a flagged preview as no observation rather than empty text, so it also stays out of the gate's
prevTextand the next real reply is not mistaken for a continuation of a tool result.Wire compatibility
The field is optional and normalizes
absent | false → undefined, so it only ever means "known tool output". An older host that never sends it keeps today's behavior instead of silently suppressing previews. Perdocs/reference/remote-wire-compatibility.md, a new optional field is safe.Verification
pnpm tcand mobiletsc --noEmit: clean. Verified non-blind with a positive control — a bogus field name errors asTS2339 ... on type 'AgentStatusEntry', so the new field is really being checked.src/shared/agent-hook-listener,src/main/agent-hooks,src/shared/agent-status-types,src/renderer/.../native-chat: 184 files / 1955 tests pass.previewIsToolOutputclause turns the new gate test red (1 failed / 20 passed). Both restored and re-verified green.Note for review
src/shared/native-chat-streaming.tssays desktop and mobile both use it "so the show/hide rule can't drift between platforms" — but mobile actually has its ownmobile-native-chat-streaming-gate.tswith a different rule. That drift is pre-existing and left alone here; both paths are fixed independently.