From 3b925bd3d64e1bfb8c4f752231fdd0c7e3e09cf8 Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Fri, 3 Jul 2026 14:06:17 -0700 Subject: [PATCH] Suppress stale title identity on proven shell foreground; extend wrapper retry ladder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live verification on PR #7059 found two gaps: a TUI that dies leaving a stuck agent title kept painting the tab through the title-fallback layer (shell-foreground proof now suppresses title-derived identity and the local completed-hook fallback), and the daemon's wrapper→agent ancestry resolution can take >1.5s for real node-wrapped CLIs (the read ladder gets a second bounded re-read at 3.5s). Co-authored-by: Orca --- .../pane-foreground-agent-tracker.test.ts | 26 ++++++++++++++-- .../pane-foreground-agent-tracker.ts | 18 ++++++----- .../lib/use-tab-agent-process-signals.test.ts | 30 +++++++++++++++++++ src/renderer/src/lib/use-tab-agent.ts | 18 +++++++---- 4 files changed, 76 insertions(+), 16 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/pane-foreground-agent-tracker.test.ts b/src/renderer/src/components/terminal-pane/pane-foreground-agent-tracker.test.ts index 160d76d3839..d83a7816d2e 100644 --- a/src/renderer/src/components/terminal-pane/pane-foreground-agent-tracker.test.ts +++ b/src/renderer/src/components/terminal-pane/pane-foreground-agent-tracker.test.ts @@ -6,6 +6,7 @@ import type { PaneForegroundAgentEntry } from '@/store/slices/pane-foreground-ag const COMMAND_SETTLE_MS = 350 const WRAPPER_RESOLVE_RETRY_MS = 1200 +const SECOND_WRAPPER_RETRY_MS = 3500 describe('createPaneForegroundAgentTracker', () => { const readForegroundProcess = vi.fn<(ptyId: string) => Promise>() @@ -51,8 +52,13 @@ describe('createPaneForegroundAgentTracker', () => { expect(publish).toHaveBeenLastCalledWith({ agent: 'claude', shellForeground: false }) }) - it('re-reads once when the first read still sees an interpreter wrapper', async () => { - readForegroundProcess.mockResolvedValueOnce('node').mockResolvedValueOnce('claude') + it('re-reads on a bounded ladder while the read still sees an interpreter wrapper', async () => { + // Why: daemon shell/helper→agent ancestry resolution has been observed to + // take >1.5s for real node-wrapped CLIs, so the ladder gets two re-reads. + readForegroundProcess + .mockResolvedValueOnce('node') + .mockResolvedValueOnce('node') + .mockResolvedValueOnce('claude') const tracker = makeTracker() tracker.onCommandStarted() @@ -62,9 +68,25 @@ describe('createPaneForegroundAgentTracker', () => { await flushSettleRead(WRAPPER_RESOLVE_RETRY_MS) expect(readForegroundProcess).toHaveBeenCalledTimes(2) + + await flushSettleRead(SECOND_WRAPPER_RETRY_MS) + expect(readForegroundProcess).toHaveBeenCalledTimes(3) expect(publish).toHaveBeenLastCalledWith({ agent: 'claude', shellForeground: false }) }) + it('stops after the ladder and publishes no identity for a persistent unknown process', async () => { + readForegroundProcess.mockResolvedValue('some-unknown-tool') + const tracker = makeTracker() + + tracker.onCommandStarted() + await flushSettleRead( + COMMAND_SETTLE_MS + WRAPPER_RESOLVE_RETRY_MS + SECOND_WRAPPER_RETRY_MS + 10_000 + ) + + expect(readForegroundProcess).toHaveBeenCalledTimes(3) + expect(publish).toHaveBeenLastCalledWith({ agent: null, shellForeground: false }) + }) + it('publishes shell foreground when the read lands after a fast command already exited', async () => { readForegroundProcess.mockResolvedValue('zsh') const tracker = makeTracker() diff --git a/src/renderer/src/components/terminal-pane/pane-foreground-agent-tracker.ts b/src/renderer/src/components/terminal-pane/pane-foreground-agent-tracker.ts index dd5e99a85ac..d8e9abc54dd 100644 --- a/src/renderer/src/components/terminal-pane/pane-foreground-agent-tracker.ts +++ b/src/renderer/src/components/terminal-pane/pane-foreground-agent-tracker.ts @@ -4,9 +4,10 @@ import type { PaneForegroundAgentEntry } from '@/store/slices/pane-foreground-ag // Why: the read must land after the shell has exec'd the command; and when it // still sees a node/python wrapper, the daemon resolves that ancestry -// asynchronously, so give its cache one bounded re-read — not a retry ladder. +// asynchronously — observed to take >1.5s for real node-wrapped CLIs — so +// give its cache two bounded re-reads, not an open-ended retry loop. const COMMAND_SETTLE_MS = 350 -const WRAPPER_RESOLVE_RETRY_MS = 1200 +const WRAPPER_RESOLVE_RETRY_DELAYS_MS = [1200, 3500] as const type PaneForegroundAgentTrackerDeps = { getPtyId: () => string | null @@ -45,15 +46,15 @@ export function createPaneForegroundAgentTracker(deps: PaneForegroundAgentTracke } } - const scheduleRead = (delayMs: number, allowWrapperRetry: boolean): void => { + const scheduleRead = (delayMs: number, retryIndex: number): void => { const generation = readGeneration readTimer = window.setTimeout(() => { readTimer = null - void readForeground(generation, allowWrapperRetry) + void readForeground(generation, retryIndex) }, delayMs) } - async function readForeground(generation: number, allowWrapperRetry: boolean): Promise { + async function readForeground(generation: number, retryIndex: number): Promise { const ptyId = trackablePtyId() if (disposed || generation !== readGeneration || !ptyId) { return @@ -71,8 +72,9 @@ export function createPaneForegroundAgentTracker(deps: PaneForegroundAgentTracke deps.publish({ agent: null, shellForeground: true }) return } - if (allowWrapperRetry && processName) { - scheduleRead(WRAPPER_RESOLVE_RETRY_MS, false) + const retryDelay = WRAPPER_RESOLVE_RETRY_DELAYS_MS[retryIndex] + if (retryDelay !== undefined && processName) { + scheduleRead(retryDelay, retryIndex + 1) return } deps.publish({ agent: null, shellForeground: false }) @@ -87,7 +89,7 @@ export function createPaneForegroundAgentTracker(deps: PaneForegroundAgentTracke // Why: the foreground left the prompt the moment C fired — stale // shell-foreground evidence must not clear the command that just started. deps.publish({ agent: null, shellForeground: false }) - scheduleRead(COMMAND_SETTLE_MS, true) + scheduleRead(COMMAND_SETTLE_MS, 0) }, onCommandFinished() { cancelPendingRead() diff --git a/src/renderer/src/lib/use-tab-agent-process-signals.test.ts b/src/renderer/src/lib/use-tab-agent-process-signals.test.ts index 91db1b3e564..9033e985eba 100644 --- a/src/renderer/src/lib/use-tab-agent-process-signals.test.ts +++ b/src/renderer/src/lib/use-tab-agent-process-signals.test.ts @@ -80,6 +80,36 @@ describe('resolveTabAgentFromSignals process identity', () => { ).toBeNull() }) + it('suppresses stuck-title identity once shell foreground is proven on a manual pane', () => { + // Why: pre-probe-removal, the foreground read cleared icons for TUIs that + // died with a stuck title; shell-foreground evidence restores that. + expect( + resolveTabAgentFromSignals({ + hasObservedAgentSignal: true, + isRemote: false, + title: '✳ Claude Code', + hookAgent: null, + processAgent: null, + processShellForeground: true, + launchAgent: undefined + }) + ).toBeNull() + }) + + it('keeps remote title identity even when a shell-foreground flag is passed', () => { + expect( + resolveTabAgentFromSignals({ + hasObservedAgentSignal: true, + isRemote: true, + title: '✳ Claude Code', + hookAgent: null, + processAgent: null, + processShellForeground: true, + launchAgent: undefined + }) + ).toBe('claude') + }) + it('keeps launch identity while the recognized process is still in the foreground', () => { expect( resolveTabAgentFromSignals({ diff --git a/src/renderer/src/lib/use-tab-agent.ts b/src/renderer/src/lib/use-tab-agent.ts index 684a09482ed..cffa5a42d98 100644 --- a/src/renderer/src/lib/use-tab-agent.ts +++ b/src/renderer/src/lib/use-tab-agent.ts @@ -73,22 +73,28 @@ export function resolveTabAgentFromSignals(args: { // A live title that explicitly names a *different* agent, once the pane has // shown any activity, overrides that stale launch identity so the tab icon // tracks what is actually running (codex launch reused for claude, etc.). + // Why: OSC 133;D proved this local pane's foreground is back at the shell, + // so any title-derived identity is stale by definition — a TUI that died + // with a stuck title must not keep painting the tab through the title layer. + const processProvesShell = !args.isRemote && args.processShellForeground === true const titleOverridesLaunch = launchAgent !== null && explicitTitleAgent !== null && explicitTitleAgent !== launchAgent && args.hasObservedAgentSignal - const titleAgent = titleOverridesLaunch - ? explicitTitleAgent - : launchAgent - ? null - : explicitTitleAgent + const titleAgent = processProvesShell + ? null + : titleOverridesLaunch + ? explicitTitleAgent + : launchAgent + ? null + : explicitTitleAgent const hasCompletedHook = (args.focusedCompletedHookAgent ?? null) !== null const noAgentTitle = titleShowsNoAgent(args.title, args.defaultTitle) // Why: remote pane titles can lag their runtime, so keep the last completed // hook identity instead of flashing unknown when the title reads as a shell. const completedHookAgent = - !args.isRemote && noAgentTitle && hasCompletedHook + !args.isRemote && (noAgentTitle || processProvesShell) && hasCompletedHook ? null : (args.focusedCompletedHookAgent ?? args.siblingCompletedHookAgent ?? null) const focusedHookAgent = args.hookAgent ?? null