From b47474b036b2115217f2ef616e7c9ff7445d35f9 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 5 Jul 2026 23:06:40 -0700 Subject: [PATCH] Fix terminal tab icon detection for foreground agents (#7476) * Fix terminal tab icon detection for foreground agents - Register with bash-preexec's preexec_functions array to prevent its DEBUG trap re-arming from silencing Orca's command-start signals. - Serve the last-resolved identity past its cache TTL (stale-while- revalidate) while the active foreground process is a wrapper. - Reschedule confirming reads on duplicate OSC 133;D sequences to prevent nested shells from prematurely clearing tab identities. - Trigger self-limiting foreground sampling on visible PTY binding, Enter keystrokes, and pane focus changes. * Fix terminal shell integration and DEBUG trap chaining in bash Chain external DEBUG traps (e.g., starship, bash-preexec) dynamically in a prompt epilogue rather than using static hooks. This ensures our own DEBUG trap survives re-arming by third-party frameworks and reliably emits OSC 133 sequences. Additionally: - Print the shell-ready marker inside the precmd hook to avoid modifying and displacing hooks at the end of PROMPT_COMMAND. - Skip redundant foreground state checks in the PTY connection when a shell prompt is already active and no background agent is expected. * Fix terminal shell integration and DEBUG trap chaining in bash Chain external DEBUG traps (e.g., starship, bash-preexec) dynamically in a prompt epilogue rather than using static hooks. This ensures our own DEBUG trap survives re-arming by third-party frameworks and reliably emits OSC 133 sequences. Additionally: - Print the shell-ready marker inside the precmd hook to avoid modifying and displacing hooks at the end of PROMPT_COMMAND. - Skip redundant foreground state checks in the PTY connection when a shell prompt is already active and no background agent is expected. * Remove debug artifacts from codex icon investigation Drop temporary diff, handoff notes, and screenshot evidence that were accidentally committed during debugging. * Mirror upstream bash-preexec so preexec dispatch test reflects real command The naive $BASH_COMMAND imitation captured Orca's chained __orca_osc133_epilogue instead of the user command, failing on CI. Read the command from history like real bash-preexec does. Co-authored-by: Orca * Fix promisify overload typecheck error in windowsHide wrapper test The wrapper's static type (...args: unknown[]) => unknown makes util.promisify resolve to its zero-arg callback overload, so calling the promisified function with args tripped TS2554 under typecheck. Type the promisified result to the variadic custom-symbol impl the wrapper copies at runtime. Co-authored-by: Orca --------- Co-authored-by: Orca --- src/main/daemon/pty-subprocess.test.ts | 75 ++++++ src/main/daemon/pty-subprocess.ts | 22 ++ src/main/daemon/shell-ready.test.ts | 87 ++++++- src/main/daemon/shell-ready.ts | 68 ++--- .../pane-foreground-agent-tracker.test.ts | 221 +++++++++++++++- .../pane-foreground-agent-tracker.ts | 136 ++++++++-- .../terminal-pane/pty-connection.test.ts | 243 +++++++++++++++++- .../terminal-pane/pty-connection.ts | 121 ++++++++- .../use-terminal-pane-lifecycle.ts | 7 + 9 files changed, 921 insertions(+), 59 deletions(-) diff --git a/src/main/daemon/pty-subprocess.test.ts b/src/main/daemon/pty-subprocess.test.ts index fccd92e9d..ad8f14cb0 100644 --- a/src/main/daemon/pty-subprocess.test.ts +++ b/src/main/daemon/pty-subprocess.test.ts @@ -367,6 +367,81 @@ describe('createPtySubprocess', () => { } }) + it('serves the resolved agent identity past the cache TTL while a wrapper holds the foreground', async () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-06-16T12:00:00.000Z')) + const proc = mockPtyProcess() + proc.process = 'node' + spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'darwin' }) + resolveAgentForegroundProcessMock.mockResolvedValue('grok') + + try { + const handle = createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24 + }) + + expect(handle.getForegroundProcess()).toBe('node') + await Promise.resolve() + await Promise.resolve() + expect(handle.getForegroundProcess()).toBe('grok') + + // Why: renderer reads poll slower than the 1s cache TTL — an expired + // cache must keep answering with the resolved identity, not the wrapper. + vi.advanceTimersByTime(1_500) + expect(handle.getForegroundProcess()).toBe('grok') + } finally { + vi.useRealTimers() + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } + }) + + it('clears an expired identity when the wrapper tree no longer resolves to an agent', async () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-06-16T12:00:00.000Z')) + const proc = mockPtyProcess() + proc.process = 'node' + spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'darwin' }) + resolveAgentForegroundProcessMock.mockResolvedValueOnce('grok').mockResolvedValue('node') + + try { + const handle = createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24 + }) + + expect(handle.getForegroundProcess()).toBe('node') + await Promise.resolve() + await Promise.resolve() + expect(handle.getForegroundProcess()).toBe('grok') + // Flush the first refresh's finally so the next read can revalidate. + await Promise.resolve() + await Promise.resolve() + + // An unrelated wrapper (e.g. npm) now owns the pane: the stale-served + // identity is revalidated and dropped once the refresh finds no agent. + vi.advanceTimersByTime(1_500) + expect(handle.getForegroundProcess()).toBe('grok') + await Promise.resolve() + await Promise.resolve() + await Promise.resolve() + expect(handle.getForegroundProcess()).toBe('node') + } finally { + vi.useRealTimers() + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } + }) + it('serves daemon Windows wrapper agent foreground from an async cache', async () => { const proc = mockPtyProcess() proc.process = 'node.exe' diff --git a/src/main/daemon/pty-subprocess.ts b/src/main/daemon/pty-subprocess.ts index b9800941b..a4fdfce15 100644 --- a/src/main/daemon/pty-subprocess.ts +++ b/src/main/daemon/pty-subprocess.ts @@ -941,6 +941,16 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl ) { cachedAgentForeground = null startupAgentForeground = null + } else if ( + cachedAgentForeground !== null && + Date.now() - cachedAgentForeground.refreshedAt > FOREGROUND_AGENT_CACHE_TTL_MS && + currentFallbackProcess !== null && + isAgentForegroundWrapperProcess(currentFallbackProcess) + ) { + // Why: the wrapper's tree no longer resolves to an agent — an expired + // identity must not transfer to an unrelated wrapper (e.g. npm right + // after an agent exit). Fresh identities survive one-off scan hiccups. + cachedAgentForeground = null } return } @@ -998,6 +1008,18 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl ) { return cachedAgentForeground.processName } + // Why: a wrapper foreground (node/python) can never identify itself, and + // readers poll slower than the cache TTL — returning the raw wrapper here + // would hide the resolved identity forever. Serve the last resolved agent + // while the scheduled refresh revalidates; exit truth is safe because an + // exited agent's foreground falls back to the shell, not a wrapper. + if ( + cachedAgentForeground && + fallbackProcess !== null && + isAgentForegroundWrapperProcess(fallbackProcess) + ) { + return cachedAgentForeground.processName + } const activeStartupAgentForeground = getActiveStartupAgentForeground(now) if (fallbackProcess && isShellProcess(fallbackProcess) && activeStartupAgentForeground) { return activeStartupAgentForeground.processName diff --git a/src/main/daemon/shell-ready.test.ts b/src/main/daemon/shell-ready.test.ts index 860e3f27a..af3ced5ad 100644 --- a/src/main/daemon/shell-ready.test.ts +++ b/src/main/daemon/shell-ready.test.ts @@ -515,11 +515,16 @@ describePosix('daemon shell-ready launch config', () => { expect(bashRc).toContain('printf "\\033]133;D;%s\\007"') expect(bashRc).toContain('printf "\\033]133;C\\007"') + // precmd is prepended (captures $? first) and the epilogue is appended last, + // so a framework that must be last in PROMPT_COMMAND stays between them. expect(bashRc).toContain( - 'PROMPT_COMMAND="__orca_osc133_precmd${PROMPT_COMMAND:+;${PROMPT_COMMAND}}"' + 'PROMPT_COMMAND="__orca_osc133_precmd${PROMPT_COMMAND:+;${PROMPT_COMMAND}};__orca_osc133_epilogue"' ) - expect(bashRc.indexOf("trap '__orca_osc133_preexec' DEBUG")).toBeGreaterThan( - bashRc.indexOf('if [[ "${ORCA_SHELL_READY_MARKER:-0}" == "1" ]]; then') + // The final DEBUG arming runs after PROMPT_COMMAND setup so the rcfile's own + // commands are not mistaken for a foreground command (lastIndexOf skips the + // identical re-arm inside __orca_osc133_epilogue). + expect(bashRc.lastIndexOf("trap '__orca_osc133_preexec' DEBUG")).toBeGreaterThan( + bashRc.indexOf('PROMPT_COMMAND="__orca_osc133_precmd') ) expect(zshrc).toContain('printf "\\033]133;D;%s\\007"') expect(zshrc).toContain('printf "\\033]133;C\\007"') @@ -556,6 +561,82 @@ describePosix('daemon shell-ready launch config', () => { } ) + itWithBash( + 'still emits 133;C when bash-preexec re-arms the DEBUG trap at first prompt', + async () => { + const { getDaemonBashShellReadyRcfileContent } = await importFreshShellReady() + // Minimal bash-preexec imitation (iTerm2/starship setups): re-arms its own + // DEBUG trap from PROMPT_COMMAND at the first prompt — silencing Orca's + // trap — and dispatches preexec_functions with the command as $1. + writeFileSync( + join(userDataPath, '.bash_profile'), + [ + 'preexec_functions=()', + '__bp_preexec_invoke_exec() {', + ' [[ -n "${__bp_interactive_mode:-}" ]] || return', + ' __bp_interactive_mode=""', + ' local f', + ' for f in "${preexec_functions[@]}"; do "$f" "$BASH_COMMAND"; done', + '}', + "__bp_arm() { __bp_interactive_mode=1; trap '__bp_preexec_invoke_exec' DEBUG; }", + 'PROMPT_COMMAND="${PROMPT_COMMAND:+$PROMPT_COMMAND;}__bp_arm"' + ].join('\n') + ) + + const output = runInteractiveBashRcfile(getDaemonBashShellReadyRcfileContent(), userDataPath) + + expectBashOsc133Lifecycle(output) + } + ) + + itWithBash( + 'dispatches a non-empty preexec_functions against the real command, not Orca hooks', + async () => { + const { getDaemonBashShellReadyRcfileContent } = await importFreshShellReady() + // Why: Orca's epilogue captures bash-preexec's re-armed DEBUG trap and + // chains it. A real preexec callback must fire against the user's command — + // not __orca_osc133_epilogue. Mirror upstream bash-preexec faithfully: it + // enables `functrace` (so Orca's `trap -p DEBUG` capture sees its trap), + // defers that install to the first prompt via PROMPT_COMMAND, and reads the + // command from `history` (so DEBUG fires on prompt hooks never dispatch a + // phantom). The naive `$BASH_COMMAND` imitation does none of these. + writeFileSync( + join(userDataPath, '.bash_profile'), + [ + 'preexec_functions=(__user_preexec)', + '__user_preexec() { printf \'USER_PREEXEC:%s\\n\' "$1"; }', + '__bp_inside=0', + '__bp_last_hist=""', + '__bp_preexec_invoke_exec() {', + ' (( __bp_inside > 0 )) && return', + ' [[ -n "${__bp_interactive_mode:-}" ]] || return', + ' local __bp_inside=1', + ' local this_command', + ' this_command="$(builtin history 1)"', + ' this_command="${this_command#"${this_command%%[![:space:]]*}"}"', + ' this_command="${this_command#* }"', + ' this_command="${this_command#"${this_command%%[![:space:]]*}"}"', + ' [[ -n "$this_command" && "$this_command" != "$__bp_last_hist" ]] || return', + ' __bp_last_hist="$this_command"', + ' __bp_interactive_mode=""', + ' local f', + ' for f in "${preexec_functions[@]}"; do "$f" "$this_command"; done', + '}', + "__bp_arm() { set -o functrace; __bp_interactive_mode=1; trap '__bp_preexec_invoke_exec' DEBUG; }", + 'PROMPT_COMMAND="${PROMPT_COMMAND:+$PROMPT_COMMAND;}__bp_arm"' + ].join('\n') + ) + + const output = runInteractiveBashRcfile(getDaemonBashShellReadyRcfileContent(), userDataPath) + + expectBashOsc133Lifecycle(output) + expect(output).toContain('USER_PREEXEC:true') + expect(output).toContain('USER_PREEXEC:false') + expect(output).not.toContain('USER_PREEXEC:__orca_osc133') + expect(output).not.toContain('USER_PREEXEC:__bp_') + } + ) + itWithBash('normalizes array PROMPT_COMMAND hooks so bash 3.2 still runs cleanup', async () => { const { getDaemonBashShellReadyRcfileContent } = await importFreshShellReady() writeFileSync( diff --git a/src/main/daemon/shell-ready.ts b/src/main/daemon/shell-ready.ts index ff60fa63a..909a75ddf 100644 --- a/src/main/daemon/shell-ready.ts +++ b/src/main/daemon/shell-ready.ts @@ -135,9 +135,10 @@ __orca_osc133_precmd() { unset __orca_in_command fi printf "\\033]133;A\\007" -} -__orca_osc133_prompt_done() { - unset __orca_in_prompt_command + # Why: emit the shell-ready marker here (not a trailing PROMPT_COMMAND entry) + # so a framework that must be last in PROMPT_COMMAND — bash-preexec — is not + # displaced by one of Orca's own hooks. + [[ "\${ORCA_SHELL_READY_MARKER:-0}" == "1" ]] && printf "${SHELL_READY_MARKER}" } __orca_run_user_debug_trap() { if [[ -n "\${__orca_user_debug_trap:-}" ]]; then @@ -146,17 +147,43 @@ __orca_run_user_debug_trap() { } __orca_osc133_preexec() { __orca_run_user_debug_trap + # Why: a framework (bash-preexec/starship) may replace our DEBUG trap at the + # first prompt; __orca_osc133_epilogue re-takes it each prompt and stores the + # framework's trap here, so the framework's own preexec still runs while our + # command-start C survives its re-arm. + if [[ -n "\${__orca_chained_debug_trap:-}" ]]; then + eval "$__orca_chained_debug_trap" || true + fi [[ -z "\${__orca_in_prompt_command:-}" ]] || return - # Why: bash DEBUG fires for every simple command, including PROMPT_COMMAND - # bodies. Skip our own prompt-time helpers so they don't mark the shell as - # "in command" before the prompt has even drawn. + # Why: a chained trap can invoke us more than once for a single command, so + # emit C only on the first fire (the __orca_in_command gate), and never for a + # prompt-time hook — ours or bash-preexec's __bp_* helpers. + [[ -z "\${__orca_in_command:-}" ]] || return case "$BASH_COMMAND" in - *__orca_osc133_precmd*|*__orca_osc133_prompt_done*|*__orca_prompt_mark*) return ;; + *__orca_osc133_*|*__bp_*) return ;; esac printf "\\033]133;C\\007" __orca_in_command=1 } -# Why: prepend so we capture $? before the user's PROMPT_COMMAND chain mutates it. +# Why: runs LAST every prompt — closes the prompt window (so command starts emit +# C) and re-arms our single DEBUG trap. A framework that replaced DEBUG at the +# first prompt is captured and chained rather than discarded, so it keeps working +# while its re-arm can no longer silence Orca's command-start signal. +__orca_osc133_epilogue() { + unset __orca_in_prompt_command + local __orca_spec="$(trap -p DEBUG)" + case "$__orca_spec" in + "" | *__orca_osc133_preexec* ) __orca_chained_debug_trap="" ;; + * ) + __orca_spec="\${__orca_spec#trap -- }" + __orca_spec="\${__orca_spec% DEBUG}" + eval "__orca_chained_debug_trap=$__orca_spec" + ;; + esac + trap '__orca_osc133_preexec' DEBUG +} +# Why: normalize an array PROMPT_COMMAND (bash 5.1+) to a string so prepend/append +# below is uniform, and capture $? in precmd before the user's chain mutates it. __orca_normalize_prompt_command() { local __orca_joined="" __orca_prompt_part if [[ "$(declare -p PROMPT_COMMAND 2>/dev/null)" == "declare -a"* ]]; then @@ -171,27 +198,8 @@ __orca_normalize_prompt_command() { PROMPT_COMMAND="$__orca_joined" fi } -__orca_prepend_prompt_command() { - __orca_normalize_prompt_command - PROMPT_COMMAND="__orca_osc133_precmd\${PROMPT_COMMAND:+;\${PROMPT_COMMAND}}" -} -__orca_append_prompt_command() { - local command="$1" - __orca_normalize_prompt_command - if [[ -n "\${PROMPT_COMMAND:-}" ]]; then - PROMPT_COMMAND="\${PROMPT_COMMAND};$command" - else - PROMPT_COMMAND="$command" - fi -} -__orca_prepend_prompt_command -if [[ "\${ORCA_SHELL_READY_MARKER:-0}" == "1" ]]; then - __orca_prompt_mark() { - printf "${SHELL_READY_MARKER}" - } - __orca_append_prompt_command "__orca_prompt_mark" -fi -__orca_append_prompt_command "__orca_osc133_prompt_done" +__orca_normalize_prompt_command +PROMPT_COMMAND="__orca_osc133_precmd\${PROMPT_COMMAND:+;\${PROMPT_COMMAND}};__orca_osc133_epilogue" __orca_debug_trap_spec="$(trap -p DEBUG)" if [[ -n "$__orca_debug_trap_spec" ]]; then __orca_debug_trap_command="\${__orca_debug_trap_spec#trap -- }" @@ -199,7 +207,7 @@ if [[ -n "$__orca_debug_trap_spec" ]]; then eval "__orca_user_debug_trap=$__orca_debug_trap_command" fi unset __orca_debug_trap_spec __orca_debug_trap_command -unset -f __orca_normalize_prompt_command __orca_prepend_prompt_command __orca_append_prompt_command +unset -f __orca_normalize_prompt_command # Why: arm DEBUG after wrapper setup; otherwise bash treats our own rcfile # commands as a foreground command and emits a fake C/D before the first prompt. trap '__orca_osc133_preexec' DEBUG 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 5a2e8c258..ab5aaa03b 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 @@ -5,12 +5,14 @@ import { createPaneForegroundAgentTracker } from './pane-foreground-agent-tracke import type { PaneForegroundAgentEntry } from '@/store/slices/pane-foreground-agent' const COMMAND_SETTLE_MS = 350 +const VISIBLE_PTY_SETTLE_MS = 350 const WRAPPER_RESOLVE_RETRY_MS = 1200 const SECOND_WRAPPER_RETRY_MS = 3500 describe('createPaneForegroundAgentTracker', () => { const readForegroundProcess = vi.fn<(ptyId: string) => Promise>() const publish = vi.fn<(entry: PaneForegroundAgentEntry) => void>() + const onConfirmedShellForeground = vi.fn<() => void>() let ptyId: string | null = 'pty-1' function makeTracker(): ReturnType { @@ -18,7 +20,8 @@ describe('createPaneForegroundAgentTracker', () => { getPtyId: () => ptyId, isTrackablePtyId: (id) => !id.startsWith('remote:') && !id.startsWith('ssh:'), readForegroundProcess, - publish + publish, + onConfirmedShellForeground }) } @@ -30,6 +33,7 @@ describe('createPaneForegroundAgentTracker', () => { vi.useFakeTimers() readForegroundProcess.mockReset() publish.mockReset() + onConfirmedShellForeground.mockReset() ptyId = 'pty-1' }) @@ -52,6 +56,72 @@ describe('createPaneForegroundAgentTracker', () => { expect(publish).toHaveBeenLastCalledWith({ agent: 'claude', shellForeground: false }) }) + it('reads the foreground for a visible PTY so restored running Codex panes regain identity', async () => { + readForegroundProcess.mockResolvedValue('codex') + const tracker = makeTracker() + + tracker.onVisiblePtyBound() + expect(readForegroundProcess).not.toHaveBeenCalled() + + await flushSettleRead(VISIBLE_PTY_SETTLE_MS) + + expect(readForegroundProcess).toHaveBeenCalledExactlyOnceWith('pty-1') + expect(publish).toHaveBeenLastCalledWith({ agent: 'codex', shellForeground: false }) + }) + + it('does not retry or publish visible PTY reads for an idle shell foreground', async () => { + readForegroundProcess.mockResolvedValue('zsh') + const tracker = makeTracker() + + tracker.onVisiblePtyBound() + await flushSettleRead( + VISIBLE_PTY_SETTLE_MS + WRAPPER_RESOLVE_RETRY_MS + SECOND_WRAPPER_RETRY_MS + 10_000 + ) + + expect(readForegroundProcess).toHaveBeenCalledExactlyOnceWith('pty-1') + expect(publish).not.toHaveBeenCalled() + }) + + it('lets command-start sampling supersede a pending visible PTY read', async () => { + readForegroundProcess.mockResolvedValue('codex') + const tracker = makeTracker() + + tracker.onVisiblePtyBound() + tracker.onCommandStarted() + await flushSettleRead(VISIBLE_PTY_SETTLE_MS) + + expect(readForegroundProcess).toHaveBeenCalledExactlyOnceWith('pty-1') + expect(publish).toHaveBeenLastCalledWith({ agent: 'codex', shellForeground: false }) + }) + + it('does not let visible PTY sampling downgrade pending command-start sampling', async () => { + readForegroundProcess.mockResolvedValueOnce('bash').mockResolvedValueOnce('codex') + const tracker = makeTracker() + + tracker.onCommandStarted() + tracker.onVisiblePtyBound() + await flushSettleRead(COMMAND_SETTLE_MS) + expect(readForegroundProcess).toHaveBeenCalledTimes(1) + + await flushSettleRead(WRAPPER_RESOLVE_RETRY_MS) + expect(readForegroundProcess).toHaveBeenCalledTimes(2) + expect(publish).toHaveBeenLastCalledWith({ agent: 'codex', shellForeground: false }) + }) + + it('retries visible PTY reads only while a foreground wrapper may resolve to an agent', async () => { + readForegroundProcess.mockResolvedValueOnce('node').mockResolvedValueOnce('codex') + const tracker = makeTracker() + + tracker.onVisiblePtyBound() + await flushSettleRead(VISIBLE_PTY_SETTLE_MS) + expect(readForegroundProcess).toHaveBeenCalledTimes(1) + expect(publish).not.toHaveBeenCalled() + + await flushSettleRead(WRAPPER_RESOLVE_RETRY_MS) + expect(readForegroundProcess).toHaveBeenCalledTimes(2) + expect(publish).toHaveBeenLastCalledWith({ agent: 'codex', shellForeground: false }) + }) + 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. @@ -124,16 +194,159 @@ describe('createPaneForegroundAgentTracker', () => { expect(readForegroundProcess).not.toHaveBeenCalled() }) - it('cancels a pending read when the command finishes first', async () => { + it('confirms a rapid command start->finish instead of trusting the D', async () => { + // Why: a leaked nested-shell 133;C->133;D pair (or a fast real command) + // cancels the command-start read; confirm the foreground so a still-running + // manual agent with no launchAgent/hook keeps its identity. readForegroundProcess.mockResolvedValue('claude') const tracker = makeTracker() tracker.onCommandStarted() tracker.onCommandFinished() - await flushSettleRead(COMMAND_SETTLE_MS + WRAPPER_RESOLVE_RETRY_MS) + await flushSettleRead(COMMAND_SETTLE_MS) + + expect(readForegroundProcess).toHaveBeenCalledExactlyOnceWith('pty-1') + expect(publish).toHaveBeenLastCalledWith({ agent: 'claude', shellForeground: false }) + expect(publish).not.toHaveBeenCalledWith({ agent: null, shellForeground: true }) + }) + + it('confirms duplicate 133;D pairs instead of fast-pathing past the pending confirmation', async () => { + // Why: user shell integrations (iTerm/VS Code) double up Orca's OSC 133, so + // every D arrives twice ~50ms apart. The second D cancels the first D's + // confirming read; it must reschedule the confirmation, not trust the D. + readForegroundProcess.mockResolvedValue('grok') + const tracker = makeTracker() + + tracker.onCommandStarted() + tracker.onCommandStarted() + tracker.onCommandFinished() + await flushSettleRead(50) + tracker.onCommandFinished() + await flushSettleRead(COMMAND_SETTLE_MS) + + expect(readForegroundProcess).toHaveBeenCalledExactlyOnceWith('pty-1') + expect(publish).toHaveBeenLastCalledWith({ agent: 'grok', shellForeground: false }) + expect(publish).not.toHaveBeenCalledWith({ agent: null, shellForeground: true }) + }) + + it('still marks shell without a read for a duplicate D pair on an idle pane', async () => { + const tracker = makeTracker() + + tracker.onCommandFinished() + await flushSettleRead(50) + tracker.onCommandFinished() + await flushSettleRead(COMMAND_SETTLE_MS) + + // Why: with no command read in flight the first D takes the no-RPC path, + // so its duplicate has nothing to re-confirm and stays RPC-free too. + expect(readForegroundProcess).not.toHaveBeenCalled() + expect(publish).toHaveBeenCalledTimes(2) + expect(publish).toHaveBeenLastCalledWith({ agent: null, shellForeground: true }) + }) + + it('marks shell on a rapid command start->finish when the foreground is a shell', async () => { + readForegroundProcess.mockResolvedValue('zsh') + const tracker = makeTracker() + + tracker.onCommandStarted() + tracker.onCommandFinished() + await flushSettleRead(COMMAND_SETTLE_MS) + + expect(readForegroundProcess).toHaveBeenCalledExactlyOnceWith('pty-1') + expect(publish).toHaveBeenLastCalledWith({ agent: null, shellForeground: true }) + }) + + it('confirms the foreground before clearing a pane an agent has owned', async () => { + readForegroundProcess.mockResolvedValue('codex') + const tracker = makeTracker() + + tracker.onCommandStarted() + await flushSettleRead(COMMAND_SETTLE_MS) + expect(publish).toHaveBeenLastCalledWith({ agent: 'codex', shellForeground: false }) + + publish.mockClear() + readForegroundProcess.mockClear() + tracker.onCommandFinished() + // Why: a leaked nested-shell 133;D must not clear Codex before the read. + expect(publish).not.toHaveBeenCalled() + + await flushSettleRead(COMMAND_SETTLE_MS) + expect(readForegroundProcess).toHaveBeenCalledExactlyOnceWith('pty-1') + expect(publish).toHaveBeenLastCalledWith({ agent: 'codex', shellForeground: false }) + expect(publish).not.toHaveBeenCalledWith({ agent: null, shellForeground: true }) + }) + + it('marks shell foreground when the confirming read shows the agent is gone', async () => { + readForegroundProcess.mockResolvedValueOnce('codex').mockResolvedValueOnce('zsh') + const tracker = makeTracker() + + tracker.onCommandStarted() + await flushSettleRead(COMMAND_SETTLE_MS) + + publish.mockClear() + tracker.onCommandFinished() + await flushSettleRead(COMMAND_SETTLE_MS) + + expect(readForegroundProcess).toHaveBeenLastCalledWith('pty-1') + expect(publish).toHaveBeenLastCalledWith({ agent: null, shellForeground: true }) + }) + + it('signals confirmed shell foreground only when the read proves the agent exited', async () => { + // Reads: command-start=codex, first finish=codex (still running), second finish=zsh (exited). + readForegroundProcess + .mockResolvedValueOnce('codex') + .mockResolvedValueOnce('codex') + .mockResolvedValueOnce('zsh') + const tracker = makeTracker() + + tracker.onCommandStarted() + await flushSettleRead(COMMAND_SETTLE_MS) + + // A leaked nested-shell 133;D while Codex still owns the foreground: no signal. + tracker.onCommandFinished() + await flushSettleRead(COMMAND_SETTLE_MS) + expect(onConfirmedShellForeground).not.toHaveBeenCalled() + + // Genuine exit -> read sees a shell -> signal fires exactly once. + tracker.onCommandFinished() + await flushSettleRead(COMMAND_SETTLE_MS) + expect(onConfirmedShellForeground).toHaveBeenCalledTimes(1) + }) + + it('returns to the no-RPC finished path after the agent is confirmed gone', async () => { + readForegroundProcess.mockResolvedValueOnce('codex').mockResolvedValueOnce('zsh') + const tracker = makeTracker() + + tracker.onCommandStarted() + await flushSettleRead(COMMAND_SETTLE_MS) + tracker.onCommandFinished() + await flushSettleRead(COMMAND_SETTLE_MS) + + publish.mockClear() + readForegroundProcess.mockClear() + tracker.onCommandFinished() expect(readForegroundProcess).not.toHaveBeenCalled() - expect(publish).toHaveBeenLastCalledWith({ agent: null, shellForeground: true }) + expect(publish).toHaveBeenCalledExactlyOnceWith({ agent: null, shellForeground: true }) + }) + + it('confirms a command finished for a launch-known agent pane before any read', async () => { + readForegroundProcess.mockResolvedValue('codex') + const tracker = createPaneForegroundAgentTracker({ + getPtyId: () => ptyId, + isTrackablePtyId: (id) => !id.startsWith('remote:') && !id.startsWith('ssh:'), + readForegroundProcess, + publish, + hasKnownAgentIdentity: () => true + }) + + tracker.onCommandFinished() + // Why: launchAgent/hook identity means the 133;D is confirmed, not trusted. + expect(publish).not.toHaveBeenCalled() + + await flushSettleRead(COMMAND_SETTLE_MS) + expect(readForegroundProcess).toHaveBeenCalledExactlyOnceWith('pty-1') + expect(publish).toHaveBeenLastCalledWith({ agent: 'codex', shellForeground: false }) }) it('never reads or publishes for remote or ssh panes', async () => { 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 1af2f78fd..8e9d37327 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 @@ -1,4 +1,7 @@ -import { recognizeAgentProcess } from '../../../../shared/agent-process-recognition' +import { + isAgentForegroundWrapperProcess, + recognizeAgentProcess +} from '../../../../shared/agent-process-recognition' import type { PaneForegroundAgentEntry } from '@/store/slices/pane-foreground-agent' // Why: the read must land after the shell has exec'd the command; and when it @@ -6,7 +9,9 @@ import type { PaneForegroundAgentEntry } from '@/store/slices/pane-foreground-ag // 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 VISIBLE_PTY_SETTLE_MS = 350 const WRAPPER_RESOLVE_RETRY_DELAYS_MS = [1200, 3500] as const +type ForegroundReadReason = 'command' | 'visible-pty' | 'command-finished' type PaneForegroundAgentTrackerDeps = { getPtyId: () => string | null @@ -15,22 +20,39 @@ type PaneForegroundAgentTrackerDeps = { isTrackablePtyId: (ptyId: string) => boolean readForegroundProcess: (ptyId: string) => Promise publish: (entry: PaneForegroundAgentEntry) => void + /** True when the pane is otherwise known to run an agent (launchAgent, live + * hook status). Lets a restored agent pane confirm — rather than trust — a + * 133;D before any command-start read has recorded its own evidence. */ + hasKnownAgentIdentity?: () => boolean + /** Fired when a confirming read proves the foreground genuinely returned to a + * shell (agent exited). Lets callers clear a stale agent-named tab title that + * the shell never repaints. */ + onConfirmedShellForeground?: () => void } /** * Publishes process-table identity for a pane at OSC 133 command boundaries: * one foreground read when a command starts (that is when the foreground - * changes), and a no-RPC shell-foreground mark when it finishes — 133;D is - * the ONLY source of shell-foreground proof; reads never produce it. + * changes), and a shell-foreground mark when it finishes. A 133;D is normally + * the shell-foreground proof; for a pane an agent has owned it is confirmed by a + * foreground read first, because a full-screen agent's nested command shells + * leak their own 133;D onto the main PTY. */ export function createPaneForegroundAgentTracker(deps: PaneForegroundAgentTrackerDeps): { + onVisiblePtyBound: () => void onCommandStarted: () => void onCommandFinished: () => void dispose: () => void } { let disposed = false - let readTimer: number | null = null + let readTimer: ReturnType | null = null + let scheduledReadReason: ForegroundReadReason | null = null + let activeReadReason: ForegroundReadReason | null = null let readGeneration = 0 + // Why: a full-screen agent (Codex, etc.) runs nested command shells whose own + // OSC 133;D leaks onto the main PTY. For a pane an agent has owned, that D is + // not proof the prompt returned, so confirm the foreground before clearing. + let hasForegroundAgentEvidence = false const trackablePtyId = (): string | null => { const ptyId = deps.getPtyId() @@ -40,30 +62,53 @@ export function createPaneForegroundAgentTracker(deps: PaneForegroundAgentTracke const cancelPendingRead = (): void => { readGeneration += 1 if (readTimer !== null) { - window.clearTimeout(readTimer) + clearTimeout(readTimer) readTimer = null } + scheduledReadReason = null + activeReadReason = null } - const scheduleRead = (delayMs: number, retryIndex: number): void => { + const scheduleRead = ( + delayMs: number, + retryIndex: number, + reason: ForegroundReadReason + ): void => { const generation = readGeneration - readTimer = window.setTimeout(() => { + scheduledReadReason = reason + readTimer = setTimeout(() => { readTimer = null - void readForeground(generation, retryIndex) + scheduledReadReason = null + activeReadReason = reason + void readForeground(generation, retryIndex, reason).finally(() => { + if (generation === readGeneration && activeReadReason === reason) { + activeReadReason = null + } + }) }, delayMs) } - async function readForeground(generation: number, retryIndex: number): Promise { + async function readForeground( + generation: number, + retryIndex: number, + reason: ForegroundReadReason + ): Promise { const ptyId = trackablePtyId() if (disposed || generation !== readGeneration || !ptyId) { return } - const processName = await deps.readForegroundProcess(ptyId).catch(() => null) + let processName: string | null = null + try { + processName = await deps.readForegroundProcess(ptyId) + } catch { + processName = null + } if (disposed || generation !== readGeneration) { return } const recognized = recognizeAgentProcess(processName) if (recognized) { + hasForegroundAgentEvidence = true deps.publish({ agent: recognized.agent, shellForeground: false }) return } @@ -72,14 +117,50 @@ export function createPaneForegroundAgentTracker(deps: PaneForegroundAgentTracke // a nested one (sh/bash without integration); marking shell-foreground // would suppress live title identity. Only 133;D proves the prompt. const retryDelay = WRAPPER_RESOLVE_RETRY_DELAYS_MS[retryIndex] - if (retryDelay !== undefined && processName) { - scheduleRead(retryDelay, retryIndex + 1) + const shouldRetry = + retryDelay !== undefined && + processName && + (reason === 'command' || isAgentForegroundWrapperProcess(processName)) + if (shouldRetry) { + scheduleRead(retryDelay, retryIndex + 1, reason) return } - deps.publish({ agent: null, shellForeground: false }) + if (reason === 'command') { + deps.publish({ agent: null, shellForeground: false }) + return + } + if (reason === 'command-finished') { + // Why: the 133;D fired AND the foreground shows no agent — together that is + // real prompt proof, so the agent truly exited. Reset the evidence so the + // pane's ordinary shell commands go back to the no-RPC finished path. + hasForegroundAgentEvidence = false + deps.publish({ agent: null, shellForeground: true }) + // Why: confirmed exit — let callers clear a stale agent title the shell + // won't repaint (a plain `codex`/`grok` leaves its OSC title behind). + deps.onConfirmedShellForeground?.() + } } return { + onVisiblePtyBound() { + // Why: command-start and command-finished reads own the exit decision; + // visibility recovery is lower-authority and must never cancel them. + if ( + scheduledReadReason === 'command' || + activeReadReason === 'command' || + scheduledReadReason === 'command-finished' || + activeReadReason === 'command-finished' + ) { + return + } + cancelPendingRead() + if (!trackablePtyId()) { + return + } + // Why: restored/manual agent panes can become visible while Codex is + // already foreground, so no OSC 133 command-start event will seed the tab icon. + scheduleRead(VISIBLE_PTY_SETTLE_MS, 0, 'visible-pty') + }, onCommandStarted() { cancelPendingRead() if (!trackablePtyId()) { @@ -88,14 +169,39 @@ 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, 0) + scheduleRead(COMMAND_SETTLE_MS, 0, 'command') }, onCommandFinished() { + // Why: a rapid 133;C→133;D pair cancels the command-start read before it + // can identify the foreground — that pair is exactly a leaked nested-shell + // command under a full-screen agent (or a fast real shell command), so on a + // no-identity pane confirm it rather than trusting the D as a prompt return. + // A pending confirming read counts too: user shell integrations double up + // Orca's OSC 133, and the duplicate D must re-confirm, not fast-path past + // the in-flight confirmation it just cancelled. + const commandReadWasPending = + scheduledReadReason === 'command' || + activeReadReason === 'command' || + scheduledReadReason === 'command-finished' || + activeReadReason === 'command-finished' cancelPendingRead() if (!trackablePtyId()) { return } - deps.publish({ agent: null, shellForeground: true }) + // Why: trust the 133;D and mark shell without an RPC only when nothing hints + // at an agent — no prior agent evidence, no launch/hook identity, and no + // command-start read racing this finish. + if ( + !hasForegroundAgentEvidence && + deps.hasKnownAgentIdentity?.() !== true && + !commandReadWasPending + ) { + deps.publish({ agent: null, shellForeground: true }) + return + } + // Why: confirm the foreground before clearing — if the agent still owns it, + // the read republishes its identity; only a genuine shell result clears it. + scheduleRead(COMMAND_SETTLE_MS, 0, 'command-finished') }, dispose() { disposed = true diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index ae4e0bbeb..fb69c5e78 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -21,6 +21,7 @@ import { beginAgentStartupDeliveryAttempt, resetAgentStartupDelayedDeliveryForTests } from '@/lib/agent-startup-delayed-delivery' +import type { PaneForegroundAgentEntry } from '@/store/slices/pane-foreground-agent' // Repro command: // pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/pty-connection.test.ts -t "OpenTUI-style small ANSI redraw" @@ -169,6 +170,7 @@ type StoreState = { consumePendingSnapshot: ReturnType runtimePaneTitlesByTabId: Record> agentStatusByPaneKey: Record + paneForegroundAgentByPaneKey: Record sleepingAgentSessionsByPaneKey: Record agentLaunchConfigByPaneKey: Record getAgentLaunchConfigForStatusEntry: ReturnType @@ -738,6 +740,7 @@ describe('connectPanePty', () => { consumePendingSnapshot: vi.fn(() => null), runtimePaneTitlesByTabId: {}, agentStatusByPaneKey: {}, + paneForegroundAgentByPaneKey: {}, sleepingAgentSessionsByPaneKey: {}, agentLaunchConfigByPaneKey: {}, getAgentLaunchConfigForStatusEntry: vi.fn((entry: { paneKey: string }) => { @@ -772,8 +775,12 @@ describe('connectPanePty', () => { ), removeAgentStatus: vi.fn(), dropAgentStatus: vi.fn(), - setPaneForegroundAgent: vi.fn(), - clearPaneForegroundAgent: vi.fn(), + setPaneForegroundAgent: vi.fn((paneKey: string, entry: PaneForegroundAgentEntry) => { + mockStoreState.paneForegroundAgentByPaneKey[paneKey] = entry + }), + clearPaneForegroundAgent: vi.fn((paneKey: string) => { + delete mockStoreState.paneForegroundAgentByPaneKey[paneKey] + }), markTerminalTabUnread: vi.fn(), markTerminalPaneUnread: vi.fn(), markAgentCompletionPaneUnread: vi.fn() @@ -14262,6 +14269,238 @@ describe('connectPanePty', () => { }) }) + describe('visible foreground agent sampling (perf)', () => { + const VISIBLE_PTY_SETTLE_MS = 350 + + // Why: every connectPanePty binding in this file shares the tab-1/LEAF_1 pane + // key, and an undisposed reattach binding elsewhere can resolve a foreground + // read into this test's store slice. Give each sampling case its own tabId so + // no other test's publish can pollute the pane identity it asserts on. + async function connectRestoredPaneForForegroundSampling( + args: { + ptyId?: string + tabId?: string + isVisibleRef?: { current: boolean } + } = {} + ): Promise<{ + binding: { noteVisibilityResume: () => void } + deps: ReturnType + transport: MockTransport + cacheKey: string + }> { + const { connectPanePty } = await import('./pty-connection') + const ptyId = args.ptyId ?? 'tab-pty' + const tabId = args.tabId ?? `tab-${ptyId}` + const transport = createMockTransport(ptyId) + transport.connect.mockImplementation(async ({ sessionId }: { sessionId?: string }) => { + return sessionId ? { id: sessionId } : null + }) + transportFactoryQueue.push(transport) + const deps = createDeps({ + tabId, + restoredLeafId: LEAF_1, + restoredPtyIdByLeafId: { [LEAF_1]: ptyId }, + ...(args.isVisibleRef ? { isVisibleRef: args.isVisibleRef } : {}) + }) + const binding = connectPanePty( + createPane(1) as never, + createManager(1) as never, + deps as never + ) as unknown as { noteVisibilityResume: () => void } + await vi.advanceTimersByTimeAsync(20) + await flushAsyncTicks(20) + return { binding, deps, transport, cacheKey: makePaneKey(tabId, LEAF_1) } + } + + async function advanceVisibleForegroundRead(): Promise { + await vi.advanceTimersByTimeAsync(VISIBLE_PTY_SETTLE_MS) + await flushAsyncTicks() + } + + function foregroundReadCallsFor(ptyId: string): unknown[][] { + return vi + .mocked(window.api.pty.getForegroundProcess) + .mock.calls.filter(([calledPtyId]) => calledPtyId === ptyId) + } + + it('does not inspect foreground process for a fresh visible spawn', async () => { + vi.useFakeTimers() + const { connectPanePty } = await import('./pty-connection') + const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) + getForegroundProcess.mockResolvedValue('codex') + const ptyId = 'pty-fresh-visible-no-sample' + transportFactoryQueue.push(createMockTransport(ptyId)) + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId: null }] }, + ptyIdsByTabId: { 'tab-1': [] }, + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf', leafId: LEAF_1 }, + activeLeafId: LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: {} + } + } + } as StoreState + + connectPanePty(createPane(1) as never, createManager(1) as never, createDeps() as never) + await vi.advanceTimersByTimeAsync(20) + await flushAsyncTicks(20) + const spawnHandler = createdTransportOptions[0]?.onPtySpawn as + | ((ptyId: string) => void) + | undefined + spawnHandler?.(ptyId) + await advanceVisibleForegroundRead() + + expect(foregroundReadCallsFor(ptyId)).toHaveLength(0) + }) + + it('samples exactly one visible restored PTY with no stronger identity signal', async () => { + vi.useFakeTimers() + const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) + getForegroundProcess.mockResolvedValue('codex') + const ptyId = 'pty-restored-visible-sample' + + const { cacheKey } = await connectRestoredPaneForForegroundSampling({ ptyId }) + expect(foregroundReadCallsFor(ptyId)).toHaveLength(0) + + await advanceVisibleForegroundRead() + + expect(foregroundReadCallsFor(ptyId)).toEqual([[ptyId]]) + expect(mockStoreState.setPaneForegroundAgent).toHaveBeenCalledWith(cacheKey, { + agent: 'codex', + shellForeground: false + }) + }) + + it('does not sample hidden restored PTYs', async () => { + vi.useFakeTimers() + const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) + getForegroundProcess.mockResolvedValue('codex') + const ptyId = 'pty-hidden-restored-no-sample' + + await connectRestoredPaneForForegroundSampling({ + ptyId, + isVisibleRef: { current: false } + }) + await advanceVisibleForegroundRead() + + expect(foregroundReadCallsFor(ptyId)).toHaveLength(0) + }) + + it('samples once when an identityless hidden pane resumes visible', async () => { + vi.useFakeTimers() + const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) + getForegroundProcess.mockResolvedValue('codex') + const isVisibleRef = { current: false } + const ptyId = 'pty-hidden-then-visible-sample' + const { binding } = await connectRestoredPaneForForegroundSampling({ ptyId, isVisibleRef }) + await advanceVisibleForegroundRead() + expect(foregroundReadCallsFor(ptyId)).toHaveLength(0) + + isVisibleRef.current = true + binding.noteVisibilityResume() + await advanceVisibleForegroundRead() + + expect(foregroundReadCallsFor(ptyId)).toEqual([[ptyId]]) + }) + + it('does not sample when launch metadata already supplies tab identity', async () => { + vi.useFakeTimers() + const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) + getForegroundProcess.mockResolvedValue('codex') + const ptyId = 'pty-launch-identity-no-sample' + const tabId = `tab-${ptyId}` + mockStoreState.tabsByWorktree = { + 'wt-1': [{ id: tabId, ptyId, launchAgent: 'codex' }] + } + + await connectRestoredPaneForForegroundSampling({ ptyId, tabId }) + await advanceVisibleForegroundRead() + + expect(foregroundReadCallsFor(ptyId)).toHaveLength(0) + }) + + it('does not sample when a live hook row already supplies pane identity', async () => { + vi.useFakeTimers() + const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) + getForegroundProcess.mockResolvedValue('codex') + const ptyId = 'pty-hook-identity-no-sample' + const tabId = `tab-${ptyId}` + mockStoreState.agentStatusByPaneKey[makePaneKey(tabId, LEAF_1)] = { + state: 'working', + agentType: 'codex' + } + + await connectRestoredPaneForForegroundSampling({ ptyId, tabId }) + await advanceVisibleForegroundRead() + + expect(foregroundReadCallsFor(ptyId)).toHaveLength(0) + }) + + it('does not sample when process identity is already known', async () => { + vi.useFakeTimers() + const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) + getForegroundProcess.mockResolvedValue('codex') + const ptyId = 'pty-process-identity-no-sample' + const tabId = `tab-${ptyId}` + mockStoreState.paneForegroundAgentByPaneKey[makePaneKey(tabId, LEAF_1)] = { + agent: 'codex', + shellForeground: false + } + + await connectRestoredPaneForForegroundSampling({ ptyId, tabId }) + await advanceVisibleForegroundRead() + + expect(foregroundReadCallsFor(ptyId)).toHaveLength(0) + }) + + it('does not re-sample once 133;D proved the pane is at a shell prompt', async () => { + vi.useFakeTimers() + const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) + getForegroundProcess.mockResolvedValue('codex') + const ptyId = 'pty-shell-foreground-no-sample' + const tabId = `tab-${ptyId}` + mockStoreState.paneForegroundAgentByPaneKey[makePaneKey(tabId, LEAF_1)] = { + agent: null, + shellForeground: true + } + + await connectRestoredPaneForForegroundSampling({ ptyId, tabId }) + await advanceVisibleForegroundRead() + + expect(foregroundReadCallsFor(ptyId)).toHaveLength(0) + }) + + it('re-samples a shell-marked pane a launch agent still owns', async () => { + // Why: a reattach or a full-screen agent's leaked nested-shell 133;D leaves + // shellForeground on a launchAgent pane, suppressing its icon. The launch + // metadata means an agent is expected, so re-read to recover its identity. + vi.useFakeTimers() + const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) + getForegroundProcess.mockResolvedValue('codex') + const ptyId = 'pty-shell-foreground-launch-agent-sample' + const tabId = `tab-${ptyId}` + mockStoreState.tabsByWorktree = { + 'wt-1': [{ id: tabId, ptyId, launchAgent: 'codex' }] + } + mockStoreState.paneForegroundAgentByPaneKey[makePaneKey(tabId, LEAF_1)] = { + agent: null, + shellForeground: true + } + + const { cacheKey } = await connectRestoredPaneForForegroundSampling({ ptyId, tabId }) + await advanceVisibleForegroundRead() + + expect(foregroundReadCallsFor(ptyId)).toEqual([[ptyId]]) + expect(mockStoreState.setPaneForegroundAgent).toHaveBeenCalledWith(cacheKey, { + agent: 'codex', + shellForeground: false + }) + }) + }) + describe('terminal input liveness IPC gating (perf)', () => { // Why (perf regression guard): listSessions() is a renderer→main→daemon // round-trip over every live session. Terminal input must never start that diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 8d009829b..453cc81e7 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -5,6 +5,7 @@ import type { IBuffer, IDisposable } from '@xterm/xterm' import { resolveCursorAgentImeAnchor } from '@/lib/pane-manager/terminal-ime-anchor' import { detectAgentStatusFromTitle, + agentTypeToIconAgent, isGeminiTerminalTitle, isClaudeAgent } from '@/lib/agent-status' @@ -165,6 +166,7 @@ import { normalizeCompatibleAgentTitleForOwner, resolveCompatibleAgentTypeForOwner } from '../../../../shared/agent-title-owner' +import { resolveExplicitTerminalTitleAgentType } from '../../../../shared/terminal-title-agent-type' import { isExpectedAgentProcess, recognizeAgentProcessFromCommandLine @@ -669,6 +671,10 @@ let inactiveForegroundImmediateBudgetWindowStart = 0 type PanePtyBinding = IDisposable & { syncProcessTracking: () => void noteVisibilityResume: () => void + /** Re-sample process identity when the pane gains intra-tab focus: the tab + * icon follows the active leaf, and a shell-marked entry on a still-running + * agent pane has no OSC boundary left to correct it. */ + sampleForegroundAgentOnFocus: () => void reconcileIfSessionDead: (liveSessionIds: Set, snapshotRequestedAt?: number) => void reconcileIfSessionMissing: (hasPty: HasPty, livenessRequestedAt?: number) => void } @@ -1762,12 +1768,86 @@ export function connectPanePty( } return pendingWrite.then(() => interruptInference.flushPending()) } + // Why: the 133;D confirmation guard and the visible-pane resampler both key off + // "does this pane expect an agent"; derive each signal once so the two callers + // can't drift and silently reintroduce the icon bug this fix closes. + const paneHasLiveHookAgentIcon = (state: ReturnType): boolean => { + const entry = state.agentStatusByPaneKey[cacheKey] + return entry?.state !== 'done' && Boolean(agentTypeToIconAgent(entry?.agentType)) + } + const paneExpectsLaunchAgent = (state: ReturnType): boolean => { + const tab = (state.tabsByWorktree[deps.worktreeId] ?? []).find( + (candidate) => candidate.id === deps.tabId + ) + return Boolean( + tab?.launchAgent ?? paneStartup?.launchAgent ?? paneStartup?.initialAgentStatus?.agent + ) + } + // Why: a launched/hook-known agent pane must confirm — not trust — a 133;D so a + // full-screen agent's leaked nested-shell 133;D can't clear its tab identity, + // even on a restore where no command-start read has recorded evidence yet. + const paneHasKnownAgentIdentity = (): boolean => { + const state = useAppStore.getState() + return paneHasLiveHookAgentIcon(state) || paneExpectsLaunchAgent(state) + } + // Why: a plain `codex`/`grok` sets its OSC title and the shell never repaints + // it on exit, so a confirmed return-to-shell must clear a title that still + // names an agent — otherwise the tab reads "grok" over a bare prompt. Only + // reset an agent-named title; user/shell-set titles are left untouched. + const clearStaleAgentTabTitleOnConfirmedShell = (): void => { + const state = useAppStore.getState() + const currentTitle = state.runtimePaneTitlesByTabId?.[deps.tabId]?.[pane.id] + const tab = (state.tabsByWorktree[deps.worktreeId] ?? []).find( + (entry) => entry.id === deps.tabId + ) + const title = currentTitle ?? tab?.title + if (!title || resolveExplicitTerminalTitleAgentType(title) === null) { + return + } + const neutralTitle = neutralTerminalTitle() + deps.setRuntimePaneTitle(deps.tabId, pane.id, neutralTitle) + if (manager.getActivePane()?.id === pane.id) { + deps.updateTabTitle(deps.tabId, neutralTitle) + } + } const paneForegroundAgentTracker = createPaneForegroundAgentTracker({ getPtyId: () => transport.getPtyId(), isTrackablePtyId: (id) => !isRemoteRuntimePtyId(id) && parseAppSshPtyId(id) === null, readForegroundProcess: (id) => window.api.pty.getForegroundProcess(id), - publish: (entry) => useAppStore.getState().setPaneForegroundAgent(cacheKey, entry) + publish: (entry) => useAppStore.getState().setPaneForegroundAgent(cacheKey, entry), + hasKnownAgentIdentity: paneHasKnownAgentIdentity, + onConfirmedShellForeground: clearStaleAgentTabTitleOnConfirmedShell }) + const sampleVisiblePaneForegroundAgent = (): void => { + if (!deps.isVisibleRef.current) { + return + } + const state = useAppStore.getState() + const foreground = state.paneForegroundAgentByPaneKey[cacheKey] + // Why: live process identity already paints the icon — nothing to read. + if (foreground?.agent) { + return + } + if (paneHasLiveHookAgentIcon(state)) { + return + } + const expectsAgent = paneExpectsLaunchAgent(state) + // Why: with no shell mark yet, launchAgent bootstrap already paints the icon, + // so a read is pointless. Otherwise probe the foreground: a shell mark (from a + // reattach or a full-screen agent's leaked nested-shell 133;D) can hide a + // still-running agent the launch/hook identity says is expected, which no + // later 133;C will reseed. A genuinely idle shell just returns a shell name + // and publishes nothing, so the probe is self-limiting. + if (!foreground?.shellForeground && expectsAgent) { + return + } + // Why: a 133;D already proved this pane is back at a shell prompt; with no + // agent expectation there is nothing to recover, so trust it and don't re-read. + if (foreground?.shellForeground && !expectsAgent) { + return + } + paneForegroundAgentTracker.onVisiblePtyBound() + } const commandLifecycle = createTerminalCommandLifecycle({ onCommandStarted: () => paneForegroundAgentTracker.onCommandStarted(), onCommandFinished: () => { @@ -1840,6 +1920,14 @@ export function connectPanePty( ) { return } + // Why: user shell frameworks (bash-preexec/iTerm2) can replace Orca's + // OSC 133;C hook, so a manually launched agent produces no command-start + // signal at all. Enter at a shell-foreground prompt is the user-side + // equivalent; the sample is gated to panes with no live agent identity + // and publishes nothing for an idle shell. + if (event.key === 'Enter' && !event.metaKey && !event.ctrlKey && !event.altKey) { + sampleVisiblePaneForegroundAgent() + } deps.clearTerminalTabUnread(deps.tabId) deps.clearTerminalPaneUnread(cacheKey) deps.clearWorktreeUnread(deps.worktreeId) @@ -2236,7 +2324,11 @@ export function connectPanePty( } const bindActivePanePty = ( ptyId: string, - options: { seedInitialAgentStatus?: boolean; updateTabPtyId?: 'always' | 'if-missing' } = {} + options: { + seedInitialAgentStatus?: boolean + updateTabPtyId?: 'always' | 'if-missing' + sampleVisibleForegroundAgent?: boolean + } = {} ): void => { if (activePanePtyBinding && activePanePtyBinding !== ptyId) { reportPanePtyVisibility(activePanePtyBinding, false) @@ -2259,6 +2351,11 @@ export function connectPanePty( // frame-level sync often runs before that async result arrives. scheduleRuntimeGraphSync() agentCompletionCoordinator.startProcessTracking() + // Why: fresh spawns receive future OSC command-start events; only adopted or + // restored PTYs may already be inside Codex with no new foreground signal. + if (options.sampleVisibleForegroundAgent === true) { + sampleVisiblePaneForegroundAgent() + } } const onPtySpawn = (ptyId: string): void => { @@ -3768,7 +3865,10 @@ export function connectPanePty( ) { // Why: daemon createOrAttach can turn an apparent fresh spawn into // a reattach; the transport skips onPtySpawn there to preserve recency. - bindActivePanePty(resolvedPtyId, { updateTabPtyId: 'if-missing' }) + bindActivePanePty(resolvedPtyId, { + updateTabPtyId: 'if-missing', + sampleVisibleForegroundAgent: true + }) } if (resolvedPtyId) { reconcilePtySizeAfterSpawn(resolvedPtyId, cols, rows) @@ -5334,6 +5434,7 @@ export function connectPanePty( deps.syncPanePtyLayoutBinding(pane.id, ptyId) deps.updateTabPtyId(deps.tabId, ptyId) agentCompletionCoordinator.startProcessTracking() + sampleVisiblePaneForegroundAgent() // Why: mobile terminal streaming needs the exact screen state from // xterm.js. The shared helper installs both the SerializeAddon-backed @@ -5895,7 +5996,10 @@ export function connectPanePty( onError: reportError } }) - bindActivePanePty(attachPtyId, { updateTabPtyId: 'if-missing' }) + bindActivePanePty(attachPtyId, { + updateTabPtyId: 'if-missing', + sampleVisibleForegroundAgent: true + }) if (attachPtyId === eagerLivePtyId) { registerPaneSerializerFor(attachPtyId) } @@ -5949,7 +6053,10 @@ export function connectPanePty( }) // Why: this path reuses a PTY spawned by an earlier mount, so no // later spawn event will bind this remounted pane's DOM/container. - bindActivePanePty(spawnedPtyId, { updateTabPtyId: 'if-missing' }) + bindActivePanePty(spawnedPtyId, { + updateTabPtyId: 'if-missing', + sampleVisibleForegroundAgent: true + }) }) .catch((err) => { reportError(err instanceof Error ? err.message : String(err)) @@ -6059,6 +6166,10 @@ export function connectPanePty( noteVisibilityResume() { ptySizeReassertion.request({ fit: false }) consumeHibernatedAgentWake() + sampleVisiblePaneForegroundAgent() + }, + sampleForegroundAgentOnFocus() { + sampleVisiblePaneForegroundAgent() }, reconcileIfSessionDead, reconcileIfSessionMissing, diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index e40614115..4baa9944e 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -1282,6 +1282,13 @@ export function useTerminalPaneLifecycle({ persistLayoutSnapshot() } reportActiveRendererPtyForPane(paneTransportsRef.current, pane.id) + // Why: the tab icon resolves from the active leaf's process identity; + // focusing a shell-marked pane whose agent is still running must + // re-sample it, since no further OSC boundary will. + const focusedBinding = panePtyBindings.get(pane.id) as + | (IDisposable & { sampleForegroundAgentOnFocus?: () => void }) + | undefined + focusedBinding?.sampleForegroundAgentOnFocus?.() // Why: when the user switches focus between split panes, update the // tab title to the newly active pane's last-known title so the tab // label reflects the focused agent — not a stale title from the