From b39a7ed4c24042480ca01ad5613a67dbf2fe0542 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 14 Aug 2026 19:25:04 -0700 Subject: [PATCH] fix(sidebar): stop the agent row's tool step from flickering on every hook (#11075) --- .../dashboard/DashboardAgentRow.tsx | 8 +- ...mpact-agent-row.tool-step-flicker.test.tsx | 122 +++++++++++ .../worktree-card-compact-agent-row.tsx | 27 ++- .../hooks/use-settled-agent-tool-step.test.ts | 189 ++++++++++++++++++ .../src/hooks/use-settled-agent-tool-step.ts | 56 ++++++ 5 files changed, 389 insertions(+), 13 deletions(-) create mode 100644 src/renderer/src/components/sidebar/worktree-card-compact-agent-row.tool-step-flicker.test.tsx create mode 100644 src/renderer/src/hooks/use-settled-agent-tool-step.test.ts create mode 100644 src/renderer/src/hooks/use-settled-agent-tool-step.ts diff --git a/src/renderer/src/components/dashboard/DashboardAgentRow.tsx b/src/renderer/src/components/dashboard/DashboardAgentRow.tsx index 460c251b98b..64a899029de 100644 --- a/src/renderer/src/components/dashboard/DashboardAgentRow.tsx +++ b/src/renderer/src/components/dashboard/DashboardAgentRow.tsx @@ -13,6 +13,7 @@ import type { DashboardAgentRow as DashboardAgentRowData } from './useDashboardD import { getAgentRowPrimaryText } from '@/lib/agent-row-primary-text' import { useAgentRowConversationName } from './use-agent-row-conversation-name' import { lastEnteredDoneAt } from './agent-finished-timestamp' +import { useSettledAgentToolStep } from '@/hooks/use-settled-agent-tool-step' // Why: narrow the dashboard's rollup states to shared dot states, defaulting unknowns to 'idle' so a row never crashes. function asDotState(state: AgentStatusState | 'idle'): AgentDotState { @@ -146,8 +147,11 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({ const model = agent.entry.model?.trim() ?? '' // Why: gate tool fields on 'working' — a stale tool line on a done row reads as still-running. const isWorking = agent.state === 'working' - const toolName = isWorking ? (agent.entry.toolName?.trim() ?? '') : '' - const toolInput = isWorking ? (agent.entry.toolInput?.trim() ?? '') : '' + // Why: hooks rewrite the pair several times a second; hold it for a readable dwell (#11075). + const { toolName, toolInput } = useSettledAgentToolStep( + isWorking ? (agent.entry.toolName?.trim() ?? '') : '', + isWorking ? (agent.entry.toolInput?.trim() ?? '') : '' + ) const lastAssistantMessage = agent.entry.lastAssistantMessage?.trim() ?? '' const isInterrupted = agent.entry.interrupted === true const lineage = agent.lineage diff --git a/src/renderer/src/components/sidebar/worktree-card-compact-agent-row.tool-step-flicker.test.tsx b/src/renderer/src/components/sidebar/worktree-card-compact-agent-row.tool-step-flicker.test.tsx new file mode 100644 index 00000000000..c1c2850fc3a --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-card-compact-agent-row.tool-step-flicker.test.tsx @@ -0,0 +1,122 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { DashboardAgentRow as DashboardAgentRowData } from '@/components/dashboard/useDashboardData' +import { AGENT_TOOL_STEP_DWELL_MS } from '@/hooks/use-settled-agent-tool-step' +import { CompactAgentRow } from './worktree-card-compact-agent-row' + +globalThis.IS_REACT_ACT_ENVIRONMENT = true + +const PANE_KEY = 'tab-1:leaf-1' + +function cursorAgentRow(toolName: string, toolInput: string): DashboardAgentRowData { + return { + paneKey: PANE_KEY, + tab: { id: 'tab-1', worktreeId: 'wt-1' }, + agentType: 'cursor', + rowSource: 'live', + state: 'working', + startedAt: 1000, + entry: { + prompt: 'Refactor the intake flow', + state: 'working', + paneKey: PANE_KEY, + updatedAt: 1000, + stateStartedAt: 1000, + stateHistory: [], + agentType: 'cursor', + toolName, + toolInput + } + } as unknown as DashboardAgentRowData +} + +let container: HTMLDivElement +let root: Root + +function renderRow(agent: DashboardAgentRowData): void { + act(() => { + root.render( + + ) + }) +} + +function rowText(): string { + return container.querySelector('.compact-agent-row')?.textContent?.trim() ?? '' +} + +beforeEach(() => { + vi.useFakeTimers() + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(() => { + act(() => { + root.unmount() + }) + container.remove() + vi.useRealTimers() +}) + +describe('compact sidebar agent row tool-step flicker (#11075)', () => { + it('does not repaint the row on every hook while a cursor turn runs', () => { + renderRow(cursorAgentRow('Read', 'src/a.ts')) + const settled = rowText() + expect(settled).toContain('Read: src/a.ts') + + // A cursor turn emits pre/post tool hooks several times a second. Every one + // of these rewrote the row's single truncated line before the fix. + const burst: [string, string][] = [ + ['Grep', 'TODO'], + ['Edit', 'src/b.ts'], + ['Bash', 'pnpm test'], + ['Read', 'src/c.ts'] + ] + for (const [toolName, toolInput] of burst) { + act(() => { + vi.advanceTimersByTime(60) + }) + renderRow(cursorAgentRow(toolName, toolInput)) + expect(rowText()).toBe(settled) + } + + act(() => { + vi.advanceTimersByTime(AGENT_TOOL_STEP_DWELL_MS) + }) + // The newest step lands; 'Grep: TODO', 'Edit: src/b.ts', and 'Bash: pnpm test' + // were never painted at all. + expect(rowText()).toContain('Read: src/c.ts') + }) + + it('keeps the prompt visible the whole time', () => { + renderRow(cursorAgentRow('Read', 'src/a.ts')) + for (const [toolName, toolInput] of [ + ['Grep', 'TODO'], + ['Edit', 'src/b.ts'] + ] as [string, string][]) { + act(() => { + vi.advanceTimersByTime(60) + }) + renderRow(cursorAgentRow(toolName, toolInput)) + expect(rowText()).toContain('Refactor the intake flow') + } + }) + + it('settles on the newest tool step once the burst stops', () => { + renderRow(cursorAgentRow('Read', 'src/a.ts')) + act(() => { + vi.advanceTimersByTime(40) + }) + renderRow(cursorAgentRow('Bash', 'pnpm build')) + act(() => { + vi.advanceTimersByTime(AGENT_TOOL_STEP_DWELL_MS) + }) + + expect(rowText()).toContain('Bash: pnpm build') + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-card-compact-agent-row.tsx b/src/renderer/src/components/sidebar/worktree-card-compact-agent-row.tsx index ac60427fea6..1a49415b8d0 100644 --- a/src/renderer/src/components/sidebar/worktree-card-compact-agent-row.tsx +++ b/src/renderer/src/components/sidebar/worktree-card-compact-agent-row.tsx @@ -10,6 +10,7 @@ import { translate } from '@/i18n/i18n' import { getAgentRowPrimaryText } from '@/lib/agent-row-primary-text' import { useAgentRowConversationName } from '@/components/dashboard/use-agent-row-conversation-name' import { lastEnteredDoneAt } from '@/components/dashboard/agent-finished-timestamp' +import { useSettledAgentToolStep, type AgentToolStep } from '@/hooks/use-settled-agent-tool-step' import CacheTimer, { usePromptCacheCountdownForPane } from './CacheTimer' function formatShortTimeAgo(ts: number, now: number): string { @@ -36,19 +37,15 @@ function getCompactAgentPrimary( return prompt || agentStateLabel(getAgentDotState(agent)) } -function getCompactAgentSecondary(agent: DashboardAgentRowData): string { +function getCompactAgentSecondary(agent: DashboardAgentRowData, toolStep: AgentToolStep): string { if (agent.entry.interrupted === true) { return 'Interrupted by user' } - if (agent.state === 'working') { - const toolName = agent.entry.toolName?.trim() ?? '' - const toolInput = agent.entry.toolInput?.trim() ?? '' - if (toolName && toolInput) { - return `${toolName}: ${toolInput}` - } - if (toolName) { - return toolName - } + if (toolStep.toolName && toolStep.toolInput) { + return `${toolStep.toolName}: ${toolStep.toolInput}` + } + if (toolStep.toolName) { + return toolStep.toolName } const lastAssistantMessage = agent.entry.lastAssistantMessage?.trim() if (lastAssistantMessage) { @@ -123,7 +120,15 @@ export const CompactAgentRow = React.memo(function CompactAgentRow({ const conversationName = useAgentRowConversationName(agent) const primary = getCompactAgentPrimary(agent, conversationName) const isLineageChild = agent.lineage?.depth === 1 - const secondary = getCompactAgentSecondary(agent) + // Why: gate on 'working' — a stale tool step on a done row reads as still-running. + const isWorking = agent.state === 'working' + // Why: the tool step shares this row's one truncated line with the prompt, so a + // per-hook rewrite reads as the identity itself flickering (#11075). + const toolStep = useSettledAgentToolStep( + isWorking ? (agent.entry.toolName?.trim() ?? '') : '', + isWorking ? (agent.entry.toolInput?.trim() ?? '') : '' + ) + const secondary = getCompactAgentSecondary(agent, toolStep) const model = agent.entry.model?.trim() ?? '' const shortTime = getCompactAgentTime(agent, now) const cacheTimer = usePromptCacheCountdownForPane(agent.paneKey, cacheTimerActive) diff --git a/src/renderer/src/hooks/use-settled-agent-tool-step.test.ts b/src/renderer/src/hooks/use-settled-agent-tool-step.test.ts new file mode 100644 index 00000000000..362002b864d --- /dev/null +++ b/src/renderer/src/hooks/use-settled-agent-tool-step.test.ts @@ -0,0 +1,189 @@ +// @vitest-environment happy-dom + +import { act, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { AGENT_TOOL_STEP_DWELL_MS, useSettledAgentToolStep } from './use-settled-agent-tool-step' + +type StepProps = { name: string; input: string } + +function renderStep(initialProps: StepProps) { + return renderHook(({ name, input }: StepProps) => useSettledAgentToolStep(name, input), { + initialProps + }) +} + +function passDwell(): void { + act(() => { + vi.advanceTimersByTime(AGENT_TOOL_STEP_DWELL_MS) + }) +} + +beforeEach(() => { + vi.useFakeTimers() +}) + +afterEach(() => { + vi.useRealTimers() +}) + +describe('useSettledAgentToolStep', () => { + it('shows the mounted step without waiting for a dwell', () => { + const { result } = renderStep({ name: 'Read', input: 'src/main.ts' }) + + expect(result.current).toEqual({ toolName: 'Read', toolInput: 'src/main.ts' }) + }) + + it('holds the mounted step for one dwell before the first rewrite lands', () => { + const { result, rerender } = renderStep({ name: '', input: '' }) + + act(() => { + rerender({ name: 'Read', input: 'src/main.ts' }) + }) + expect(result.current).toEqual({ toolName: '', toolInput: '' }) + + passDwell() + expect(result.current).toEqual({ toolName: 'Read', toolInput: 'src/main.ts' }) + }) + + it('coalesces a burst of hook rewrites to the newest step', () => { + const { result, rerender } = renderStep({ name: 'Read', input: 'a.ts' }) + passDwell() + + act(() => { + rerender({ name: 'Read', input: 'b.ts' }) + }) + expect(result.current).toEqual({ toolName: 'Read', toolInput: 'b.ts' }) + + for (const [name, input] of [ + ['Grep', 'TODO'], + ['Edit', 'c.ts'], + ['Bash', 'pnpm test'] + ] as [string, string][]) { + act(() => { + vi.advanceTimersByTime(80) + rerender({ name, input }) + }) + // Three rewrites inside the dwell paint nothing. + expect(result.current).toEqual({ toolName: 'Read', toolInput: 'b.ts' }) + } + + passDwell() + // Only the newest lands — the two intermediates are never painted. + expect(result.current).toEqual({ toolName: 'Bash', toolInput: 'pnpm test' }) + }) + + it('always lands the final step once the burst stops', () => { + const { result, rerender } = renderStep({ name: 'Read', input: 'a.ts' }) + passDwell() + + act(() => { + rerender({ name: 'Read', input: 'b.ts' }) + }) + act(() => { + vi.advanceTimersByTime(50) + rerender({ name: 'Bash', input: 'pnpm build' }) + }) + passDwell() + + expect(result.current).toEqual({ toolName: 'Bash', toolInput: 'pnpm build' }) + }) + + it('never pairs one step name with another step input', () => { + const seen: string[] = [] + const { rerender } = renderHook( + ({ name, input }: StepProps) => { + const step = useSettledAgentToolStep(name, input) + seen.push(`${step.toolName}|${step.toolInput}`) + return step + }, + { initialProps: { name: 'Read', input: 'a.ts' } } + ) + passDwell() + + act(() => { + rerender({ name: 'Bash', input: 'pnpm test' }) + }) + act(() => { + vi.advanceTimersByTime(40) + rerender({ name: 'Grep', input: 'needle' }) + }) + passDwell() + + const validPairs = new Set(['Read|a.ts', 'Bash|pnpm test', 'Grep|needle']) + expect(seen.every((pair) => validPairs.has(pair))).toBe(true) + }) + + it('coalesces a transient clear away when a new step arrives inside the dwell', () => { + // Why: cursor postToolUseFailure clears the pair for one event; painting that + // blank frame is what swings the row between unrelated text (#11075). + const { result, rerender } = renderStep({ name: 'Read', input: 'a.ts' }) + passDwell() + + act(() => { + rerender({ name: 'Bash', input: 'pnpm test' }) + }) + act(() => { + vi.advanceTimersByTime(30) + rerender({ name: '', input: '' }) + }) + act(() => { + vi.advanceTimersByTime(30) + rerender({ name: 'Edit', input: 'fix.ts' }) + }) + expect(result.current).toEqual({ toolName: 'Bash', toolInput: 'pnpm test' }) + + passDwell() + expect(result.current).toEqual({ toolName: 'Edit', toolInput: 'fix.ts' }) + }) + + it('lands a clear that outlives the dwell so a finished row drops its tool step', () => { + const { result, rerender } = renderStep({ name: 'Read', input: 'a.ts' }) + passDwell() + + act(() => { + rerender({ name: 'Bash', input: 'pnpm test' }) + }) + act(() => { + vi.advanceTimersByTime(30) + rerender({ name: '', input: '' }) + }) + passDwell() + + expect(result.current).toEqual({ toolName: '', toolInput: '' }) + }) + + it('keeps repainting on schedule instead of restarting the dwell on every rewrite', () => { + const { result, rerender } = renderStep({ name: 'Read', input: 'a.ts' }) + passDwell() + + // A hook every 50ms for well over one dwell must still land a fresh step. + for (let elapsed = 0; elapsed <= AGENT_TOOL_STEP_DWELL_MS * 2; elapsed += 50) { + act(() => { + vi.advanceTimersByTime(50) + rerender({ name: 'Bash', input: `step-${elapsed}` }) + }) + } + + expect(result.current.toolName).toBe('Bash') + expect(result.current.toolInput).toMatch(/^step-\d+$/) + }) + + it('clears its pending dwell on unmount', () => { + const clearTimeoutSpy = vi.spyOn(globalThis, 'clearTimeout') + const { rerender, unmount } = renderStep({ name: 'Read', input: 'a.ts' }) + passDwell() + + act(() => { + rerender({ name: 'Bash', input: 'pnpm test' }) + }) + act(() => { + vi.advanceTimersByTime(20) + rerender({ name: 'Grep', input: 'needle' }) + }) + unmount() + + expect(clearTimeoutSpy).toHaveBeenCalled() + expect(() => vi.advanceTimersByTime(AGENT_TOOL_STEP_DWELL_MS)).not.toThrow() + clearTimeoutSpy.mockRestore() + }) +}) diff --git a/src/renderer/src/hooks/use-settled-agent-tool-step.ts b/src/renderer/src/hooks/use-settled-agent-tool-step.ts new file mode 100644 index 00000000000..722fd53a09c --- /dev/null +++ b/src/renderer/src/hooks/use-settled-agent-tool-step.ts @@ -0,0 +1,56 @@ +import { useEffect, useRef, useState } from 'react' + +export type AgentToolStep = { + toolName: string + toolInput: string +} + +/** + * Minimum time a tool step stays on screen before the next one may replace it. + * STYLEGUIDE.md: feedback that lives under ~100ms "reads as a glitch"; agents + * emit several hooks per second, so the row needs a readable floor. + */ +export const AGENT_TOOL_STEP_DWELL_MS = 800 + +/** + * Holds an agent's `toolName`/`toolInput` pair steady for a minimum dwell so the + * row reads instead of flickering (#11075). Steps arriving inside the dwell are + * coalesced to the newest one, so the pair is always atomic and the last value + * always lands. Hook events themselves are untouched — this is display only, and + * every other consumer still sees each event. + */ +export function useSettledAgentToolStep( + toolName: string, + toolInput: string, + dwellMs: number = AGENT_TOOL_STEP_DWELL_MS +): AgentToolStep { + const [settled, setSettled] = useState(() => ({ toolName, toolInput })) + const paintedAtRef = useRef(null) + + useEffect(() => { + // Why: mount already painted a step, so its dwell starts here — otherwise the + // first rewrite after mount lands instantly and the row still flickers once. + paintedAtRef.current ??= Date.now() + if (toolName === settled.toolName && toolInput === settled.toolInput) { + return + } + const paint = (): void => { + paintedAtRef.current = Date.now() + setSettled({ toolName, toolInput }) + } + // Why: an ABSOLUTE deadline off the last paint. Rescheduling on the next hook + // therefore re-aims at the same instant instead of restarting the dwell, which + // is what lets this effect own its own cleanup without starving the row. + const remainingMs = paintedAtRef.current + dwellMs - Date.now() + if (remainingMs <= 0) { + paint() + return + } + const timer = setTimeout(paint, remainingMs) + return () => { + clearTimeout(timer) + } + }, [toolName, toolInput, dwellMs, settled]) + + return settled +}