From 1f5d39b58e9bdac94edaa5a035b04460637950a0 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 17 Aug 2026 16:28:13 -0700 Subject: [PATCH] fix(native-chat): keep the working dots up while a command runs (#15188) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trailing "…" disappeared the moment Codex started running shell commands — the part of a turn that actually takes time. One turn showed dots while thinking, then went still for the length of a minute-long command, with only the tab spinner and the Stop button left to say anything was happening. The rule suppressed on `role === 'assistant'`. On the PTY transport a command row arrives as a synthetic `command:` marker and hit the show branch; on the structured transport the same work is a journal tool-call item projected as `role: 'assistant'` with tool blocks, so it hit the suppress branch instead. The module's header claimed the rule was "pure so the two transports can't drift" — the logic was pure, the inputs drifted, and isCommandMarkerId was dead code here. Suppression is now about the row's CONTENT rather than its role: a row carrying only tool-call/tool-result blocks is progress, not the turn's answer, so it keeps the dots. Prose still suppresses them, which is what stopped the placeholder from reflowing the list under streamed text. The existing tests only ever built `command:`-prefixed rows, which exist on the PTY path alone — that is why they were blind to the shipping transport. The new cases build rows through the real structured projection instead. Verified: 5778 tests across renderer native-chat and shared, web typecheck, oxlint clean. Mutation: dropping the tool-activity branch turns the new running-command assertion red. --- .../native-chat-typing-indicator.test.ts | 54 +++++++++++++++++++ .../native-chat-typing-indicator.ts | 31 +++++++++-- 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/src/renderer/src/components/native-chat/native-chat-typing-indicator.test.ts b/src/renderer/src/components/native-chat/native-chat-typing-indicator.test.ts index 65af547b7c0..db572e57364 100644 --- a/src/renderer/src/components/native-chat/native-chat-typing-indicator.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-typing-indicator.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it } from 'vitest' import { NATIVE_CHAT_STREAMING_ID } from '../../../../shared/native-chat-streaming' import type { NativeChatMessage } from '../../../../shared/native-chat-types' +import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' +import { projectStructuredItemsToNativeChat } from '../../../../shared/structured-agent-session-projection' import { shouldShowNativeChatTypingIndicator } from './native-chat-typing-indicator' function message(id: string, role: NativeChatMessage['role'], text = id): NativeChatMessage { @@ -82,3 +84,55 @@ describe('shouldShowNativeChatTypingIndicator', () => { expect(shouldShowNativeChatTypingIndicator({ messages: [], isWorking: true })).toBe(true) }) }) + +// These build rows through the REAL structured projection instead of hand-made +// `command:` marker ids. The hand-made ids only exist on the PTY transport, so +// tests using them were blind to how the shipping transport actually looks. +describe('with rows projected from the structured journal', () => { + function toolCallItem(sequence: number): AgentJournalRenderItem { + return { + itemId: `codex:thread-1:turn-1:${sequence}`, + revision: 1, + sequence, + observedAt: 1_800_000_000_000, + body: { + kind: 'tool-call', + name: 'shell', + state: 'running', + input: { command: 'sed -n 1,240p README.md' } + } + } as AgentJournalRenderItem + } + + function assistantTextItem(sequence: number): AgentJournalRenderItem { + return { + itemId: `codex:thread-1:turn-1:${sequence}`, + revision: 1, + sequence, + observedAt: 1_800_000_000_000, + body: { + kind: 'message', + role: 'assistant', + blocks: [{ type: 'text', text: "I'm checking PR 14696's metadata." }] + } + } as AgentJournalRenderItem + } + + it('keeps showing while a running command is the newest row', () => { + // The screenshot case: prose landed, then codex started running shell commands + // and the chat body went still for the length of the command. + const messages = projectStructuredItemsToNativeChat([assistantTextItem(1), toolCallItem(2)]) + expect(messages.at(-1)?.role).toBe('assistant') + expect(shouldShowNativeChatTypingIndicator({ messages, isWorking: true })).toBe(true) + }) + + it('still hides once prose is the newest row', () => { + const messages = projectStructuredItemsToNativeChat([toolCallItem(1), assistantTextItem(2)]) + expect(shouldShowNativeChatTypingIndicator({ messages, isWorking: true })).toBe(false) + }) + + it('stays hidden when the turn is not working, command row or not', () => { + const messages = projectStructuredItemsToNativeChat([toolCallItem(1)]) + expect(shouldShowNativeChatTypingIndicator({ messages, isWorking: false })).toBe(false) + }) +}) diff --git a/src/renderer/src/components/native-chat/native-chat-typing-indicator.ts b/src/renderer/src/components/native-chat/native-chat-typing-indicator.ts index db3f5fb696f..095e01983c3 100644 --- a/src/renderer/src/components/native-chat/native-chat-typing-indicator.ts +++ b/src/renderer/src/components/native-chat/native-chat-typing-indicator.ts @@ -1,13 +1,29 @@ -// When the trailing "…" row is allowed to render. The rule is pure so the two -// transports can't drift: the PTY path grows a synthetic streaming bubble while -// the structured path appends real journal items, but both mean the same thing — -// the turn's own assistant row is on screen, so a second placeholder below it is -// just an extra row that reflows the list when it later disappears. +// When the trailing "…" row is allowed to render. +// +// The rule suppresses the dots once the turn's own assistant ANSWER is on screen, +// because a placeholder below streamed text reflows the list when it disappears. +// It must not suppress on a row that only reports tool work: a shell command can +// run for a minute with nothing else arriving, and that is precisely when the +// user needs to see that the turn is still alive. +// +// Both transports have to agree, and matching on `role` alone does not get there: +// the PTY path emits synthetic `command:` marker rows, while the structured path +// projects a journal tool-call item as `role: 'assistant'` with tool blocks. Same +// meaning, different shape — so the predicate is about the row's CONTENT. import type { NativeChatMessage } from '../../../../shared/native-chat-types' import { NATIVE_CHAT_STREAMING_ID } from '../../../../shared/native-chat-streaming' import { isCommandMarkerId } from './native-chat-pending' +/** A row carrying only tool activity — no prose. It is progress, not an answer. */ +function isToolActivityOnlyRow(message: NativeChatMessage): boolean { + const blocks = message.blocks + if (!blocks || blocks.length === 0) { + return false + } + return blocks.every((block) => block.type === 'tool-call' || block.type === 'tool-result') +} + export function shouldShowNativeChatTypingIndicator(args: { messages: readonly NativeChatMessage[] isWorking: boolean @@ -23,6 +39,11 @@ export function shouldShowNativeChatTypingIndicator(args: { if (!message || message.role === 'user' || isCommandMarkerId(message.id)) { return true } + // Tool work is the strongest reason to KEEP the dots, so it decides here + // rather than falling through to the assistant-role check below. + if (isToolActivityOnlyRow(message)) { + return true + } // Status/system rows interleave mid-turn; they neither suppress nor unsuppress, // otherwise the dots would flicker back on between assistant chunks. if (message.role === 'assistant' || message.id === NATIVE_CHAT_STREAMING_ID) {