diff --git a/src/main/agent-hooks/server.test.ts b/src/main/agent-hooks/server.test.ts index c1870d35a7c..34ef9b68db1 100644 --- a/src/main/agent-hooks/server.test.ts +++ b/src/main/agent-hooks/server.test.ts @@ -1070,6 +1070,74 @@ describe('AgentHookServer listener replay', () => { } }) + it('does not let late Codex tool hooks with explicit prompt resurrect an inferred interrupt', () => { + vi.useFakeTimers() + vi.setSystemTime(1_000) + try { + const server = new AgentHookServer() + server.ingestRemote( + { + paneKey: PANE, + tabId: 'tab-1', + worktreeId: 'wt-1', + hasExplicitPrompt: true, + hookEventName: 'UserPromptSubmit', + payload: { + state: 'working', + prompt: 'Run sleep 30, then reply done.', + agentType: 'codex' + } + }, + 'conn-1' + ) + const baseline = server.getStatusSnapshot()[0] + + vi.setSystemTime(1_500) + expect( + server.inferInterrupt({ + paneKey: PANE, + baselineUpdatedAt: baseline.receivedAt, + baselineStateStartedAt: baseline.stateStartedAt, + baselinePrompt: 'Run sleep 30, then reply done.', + baselineAgentType: 'codex', + intent: 'plain-escape' + }) + ).toBe(true) + + vi.setSystemTime(6_000) + server.ingestRemote( + { + paneKey: PANE, + tabId: 'tab-1', + worktreeId: 'wt-1', + hasExplicitPrompt: true, + hookEventName: 'PostToolUse', + payload: { + state: 'working', + prompt: 'Run sleep 30, then reply done.', + agentType: 'codex', + toolName: 'Bash', + toolInput: 'sleep 30' + } + }, + 'conn-1' + ) + + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ + state: 'done', + prompt: 'Run sleep 30, then reply done.', + agentType: 'codex', + interrupted: true, + receivedAt: 1_500, + stateStartedAt: 1_500 + }) + ]) + } finally { + vi.useRealTimers() + } + }) + it('allows a new prompt after an inferred interrupt', () => { vi.useFakeTimers() vi.setSystemTime(1_000) diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index 4dc64d523de..8a10080168c 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -448,7 +448,7 @@ function isToolProgressWorkingAfterInterrupt(next: AgentHookEventPayload): boole if (next.payload.state !== 'working') { return false } - if (next.payload.agentType !== 'claude') { + if (next.payload.agentType !== 'claude' && next.payload.agentType !== 'codex') { return false } // Why: a same-prompt retry is another UserPromptSubmit, while late post-Ctrl+C progress arrives as tool lifecycle work. @@ -1260,6 +1260,9 @@ export class AgentHookServer { (effectivePayload.hasExplicitPrompt !== true && Date.now() - previous.receivedAt <= INTERRUPTED_DONE_LATE_WORKING_SUPPRESSION_MS)) ) { + if (effectivePayload.payload.agentType === 'codex') { + markCodexLeadTurnInterrupted(this.state, effectivePayload.paneKey) + } return previous } if ( diff --git a/src/main/pty/legacy-terminal-posix-tombstone.ts b/src/main/pty/legacy-terminal-posix-tombstone.ts index bf575d3bc8b..7fd4a677b17 100644 --- a/src/main/pty/legacy-terminal-posix-tombstone.ts +++ b/src/main/pty/legacy-terminal-posix-tombstone.ts @@ -4,7 +4,17 @@ const POSIX_TOMBSTONE = String.raw`#!/usr/bin/env bash set -u command_name="__ORCA_COMMAND__" -wrapper_dir="$(cd -- "$(dirname -- "${SHELL_DOLLAR}{BASH_SOURCE[0]}")" && pwd)" +# Why: parameter expansion, not the external dirname — if that were unresolvable the substitution +# would be empty and cd into it succeeds, silently making wrapper_dir the cwd so the wrapper +# fails to exclude itself from PATH. +wrapper_src="${SHELL_DOLLAR}{BASH_SOURCE[0]}" +case "$wrapper_src" in + # Why: with no slash the %/* strip yields the file name, not a directory, so self-exclusion + # would miss the wrapper's own dir and the lookup would resolve back to this script. + */*) wrapper_dir="$(cd -- "${SHELL_DOLLAR}{wrapper_src%/*}" 2>/dev/null && pwd)" ;; + *) wrapper_dir="$PWD" ;; +esac +[[ -n "$wrapper_dir" ]] || wrapper_dir="$PWD" legacy_wrapper_dir="${SHELL_DOLLAR}{ORCA_ATTRIBUTION_SHIM_DIR:-}" cleaned_path="${SHELL_DOLLAR}{PATH:-}" @@ -32,9 +42,9 @@ filter_path() { normalized="${SHELL_DOLLAR}{normalized%/}" done candidate="${SHELL_DOLLAR}{entry:-.}" - if [[ -z "$entry" ]]; then - # Why: an empty PATH element means the current directory, so keeping it would let a - # repository-local git/gh win the lookup below. + if [[ "$entry" != /* ]]; then + # Why: an empty or relative PATH element resolves against the current directory, so keeping + # it would let a repository-local git/gh win the lookup below. : elif [[ -n "$legacy_target" && "$normalized" == "$legacy_target" ]]; then : diff --git a/src/main/pty/legacy-terminal-shim-dir.test.ts b/src/main/pty/legacy-terminal-shim-dir.test.ts index 3bd489817a1..64c4436b2df 100644 --- a/src/main/pty/legacy-terminal-shim-dir.test.ts +++ b/src/main/pty/legacy-terminal-shim-dir.test.ts @@ -113,16 +113,58 @@ describe('legacy terminal shim neutralization', () => { expect(cmd).not.toContain('where.exe') expect(cmd).toContain('for %%P in ("%orca_clean_path:;=" "%") do call :orca_try_candidate') expect(cmd).toContain(`if exist "%%~fG\\${command}.exe"`) - // A candidate inside the wrapper directory must still be rejected. - expect(cmd).toContain('if /I not "%%~fG\\"=="%~dp0"') + // A candidate inside the wrapper directory must still be rejected, compared against the + // cached wrapper dir because %~dp0 is rebound inside a CALL. + expect(cmd).toContain('if /I not "%%~fG\\"=="%orca_wrapper_dir%"') + // Relative entries resolve against the cwd, so they must be rejected like empty ones. + // Why: the rooted-path test must not shell out — an external tool would itself be + // resolved from the cwd, reintroducing the hijack. + expect(cmd).not.toContain('findstr') + expect(cmd).toContain('if "%orca_candidate:~1,2%"==":\\" goto orca_candidate_rooted') + expect(cmd).toContain('if "%orca_candidate:~0,2%"=="\\\\" goto orca_candidate_rooted') + // Why: these two guards are the only thing stopping an empty element reaching the cwd on + // Windows; deleting either left every other assertion green. + expect(cmd).toContain('if "%~1"=="" exit /b') const powershell = readFileSync(join(win32Dir, `${command}-wrapper.ps1`), 'utf8') expect(powershell).not.toContain('Get-Command') expect(powershell).toContain("($env:PATH -split ';')") + // Why: the rooted check must reject drive-relative 'C:foo', which still resolves against + // the cwd — so the IsPathRooted call must be gone, replaced by an explicit prefix match. + expect(powershell).not.toContain('[IO.Path]::IsPathRooted(') + expect(powershell).toContain("-notmatch '^([A-Za-z]:") + expect(powershell).toContain('if (-not $dir) { continue }') expect(powershell).toContain('Test-Path -LiteralPath $candidate -PathType Leaf') } }) + it('guards both cmd PATH walks so an empty variable cannot break parsing', () => { + // Why: `%VAR:;=" "%` on an empty variable leaves an unbalanced quote that desynchronizes + // cmd parsing for the rest of the file, turning the not-found branch into + // `1>&2 was unexpected at this time.` Reproduced on Windows before this guard. + const userData = makeUserDataDir() + const win32Dir = join(userData, 'orca-terminal-attribution', 'win32') + mkdirSync(win32Dir, { recursive: true }) + + neutralizeLegacyTerminalShimDir(userData) + + for (const command of ['git', 'gh'] as const) { + const cmd = readFileSync(join(win32Dir, `${command}.cmd`), 'utf8') + expect(cmd).toContain('if not defined PATH goto :orca_path_walked') + expect(cmd).toContain('if not defined orca_clean_path goto :orca_candidates_walked') + for (const [guard, loop] of [ + ['if not defined PATH goto :orca_path_walked', 'for %%P in ("%PATH:;='], + [ + 'if not defined orca_clean_path goto :orca_candidates_walked', + 'for %%P in ("%orca_clean_path:;=' + ] + ] as const) { + expect(cmd.indexOf(guard)).toBeGreaterThan(-1) + expect(cmd.indexOf(guard)).toBeLessThan(cmd.indexOf(loop)) + } + } + }) + itOnPosix('does not let an empty PATH element resolve the command from the cwd', async () => { // Why (STA-4169): an empty PATH element means the current directory on POSIX. const userData = makeUserDataDir() @@ -141,6 +183,13 @@ describe('legacy terminal shim neutralization', () => { writeFileSync(join(hostile, 'git'), "#!/usr/bin/env bash\nprintf 'HOSTILE\\n'\n", { mode: 0o755 }) + // Reachable only through a relative PATH entry resolved against the hostile cwd. + mkdirSync(join(hostile, 'node_modules', '.bin'), { recursive: true }) + writeFileSync( + join(hostile, 'node_modules', '.bin', 'git'), + "#!/usr/bin/env bash\nprintf 'HOSTILE\\n'\n", + { mode: 0o755 } + ) const result = spawnSync(join(shimDir, 'git'), ['--version'], { cwd: hostile, @@ -152,6 +201,33 @@ describe('legacy terminal shim neutralization', () => { expect(result.stdout).toContain('REAL') expect(result.stdout).not.toContain('HOSTILE') + + // Why: every spelling that means "current directory" must lose — a leading/trailing empty + // element, and any relative entry (`.`, or a repo-local bin dir). + for (const cwdSpelling of [ + `:${realBin}:/usr/bin:/bin`, + `${realBin}:/usr/bin:/bin:`, + `.:${realBin}:/usr/bin:/bin`, + `..:${realBin}:/usr/bin:/bin`, + `node_modules/.bin:${realBin}:/usr/bin:/bin` + ]) { + const run = spawnSync(join(shimDir, 'git'), ['--version'], { + cwd: hostile, + env: { ...process.env, PATH: cwdSpelling }, + encoding: 'utf8' + }) + expect(run.stdout, `PATH=${cwdSpelling}`).toContain('REAL') + expect(run.stdout, `PATH=${cwdSpelling}`).not.toContain('HOSTILE') + } + + // Why: invoked with no slash in $0, `${BASH_SOURCE%/*}` yields the file name rather than a + // directory, so self-exclusion missed the shim dir and the lookup resolved back to itself. + const noSlash = spawnSync('bash', ['git', '--version'], { + cwd: shimDir, + env: { ...process.env, PATH: `${shimDir}:${realBin}:/usr/bin:/bin` }, + encoding: 'utf8' + }) + expect(noSlash.stdout).toContain('REAL') }) it('removes every Windows PATH occurrence of both captured wrapper directories', () => { diff --git a/src/main/pty/legacy-terminal-shim-dir.ts b/src/main/pty/legacy-terminal-shim-dir.ts index abeeb7a7263..0d380bcfd65 100644 --- a/src/main/pty/legacy-terminal-shim-dir.ts +++ b/src/main/pty/legacy-terminal-shim-dir.ts @@ -1,6 +1,10 @@ import { chmodSync, existsSync, mkdirSync, renameSync, rmSync, writeFileSync } from 'node:fs' import { join } from 'node:path' import { renderLegacyTerminalPosixTombstone } from './legacy-terminal-posix-tombstone' +import { + renderLegacyTerminalWindowsCmdTombstone, + renderLegacyTerminalWindowsPowerShellTombstone +} from './legacy-terminal-windows-tombstone' const LEGACY_TERMINAL_ATTRIBUTION_ENABLE_ENV_KEY = 'ORCA_ENABLE_GIT_ATTRIBUTION' const LEGACY_TERMINAL_ATTRIBUTION_BYPASS_ENV_KEY = 'ORCA_ATTRIBUTION_BYPASS' @@ -25,93 +29,6 @@ export const LEGACY_TERMINAL_SHIM_REMOTE_ENV_KEYS = [ LEGACY_TERMINAL_ATTRIBUTION_ENABLE_ENV_KEY ] as const -const WIN32_PASSTHROUGH_WRAPPER = String.raw`@echo off -setlocal -set "orca_real=%ORCA_REAL___ORCA_UPPER_COMMAND__%" -set "orca_wrapper_dir=%~dp0" -set "orca_legacy_wrapper_dir=%ORCA_ATTRIBUTION_SHIM_DIR%" -set "orca_clean_path=" -for %%P in ("%PATH:;=" "%") do call :orca_append_path "%%~P" -set "PATH=%orca_clean_path%" -set "ORCA_ENABLE_GIT_ATTRIBUTION=" -set "ORCA_GIT_COMMIT_TRAILER=" -set "ORCA_GH_PR_FOOTER=" -set "ORCA_GH_ISSUE_FOOTER=" -set "ORCA_ATTRIBUTION_SHIM_DIR=" -set "ORCA_ATTRIBUTION_BYPASS=" -set "ORCA_REAL_GIT=" -set "ORCA_REAL_GH=" -if defined orca_real for %%G in ("%orca_real%") do if /I "%%~dpG"=="%~dp0" set "orca_real=" -rem Why: clear a captured path that no longer exists, or the PATH walk below is skipped. -if defined orca_real if not exist "%orca_real%" set "orca_real=" -if defined orca_real goto run -rem Why: an unqualified Windows command lookup searches the current directory before PATH, so a -rem repository-local __ORCA_COMMAND__.exe would win. Walk the cleaned PATH ourselves instead. -for %%P in ("%orca_clean_path:;=" "%") do call :orca_try_candidate "%%~P" -if not defined orca_real ( - echo Orca compatibility wrapper could not locate __ORCA_COMMAND__ on PATH. 1>&2 - exit /b 127 -) -:run -"%orca_real%" %* -exit /b %ERRORLEVEL% - -:orca_try_candidate -if defined orca_real exit /b -if "%~1"=="" exit /b -for %%G in ("%~1") do if /I not "%%~fG\"=="%~dp0" if exist "%%~fG\__ORCA_COMMAND__.exe" set "orca_real=%%~fG\__ORCA_COMMAND__.exe" -exit /b - -:orca_append_path -for %%G in ("%~1") do set "orca_path_entry_dir=%%~fG\" -if /I "%orca_path_entry_dir%"=="%orca_wrapper_dir%" exit /b -if defined orca_legacy_wrapper_dir for %%G in ("%orca_legacy_wrapper_dir%") do if /I "%orca_path_entry_dir%"=="%%~fG\" exit /b -if defined orca_clean_path (set "orca_clean_path=%orca_clean_path%;%~1") else set "orca_clean_path=%~1" -exit /b -` - -const POWERSHELL_PASSTHROUGH_WRAPPER = String.raw`$ErrorActionPreference = 'Stop' -$commandName = '__ORCA_COMMAND__' -$realCommand = [Environment]::GetEnvironmentVariable('ORCA_REAL___ORCA_UPPER_COMMAND__') -$wrapperDir = Split-Path -Parent $MyInvocation.MyCommand.Path -$legacyWrapperDir = $env:ORCA_ATTRIBUTION_SHIM_DIR -$wrapperDirs = @($wrapperDir, $legacyWrapperDir) | Where-Object { $_ } | ForEach-Object { $_.TrimEnd('\') } -$env:PATH = (($env:PATH -split ';') | Where-Object { - $pathEntry = $_ - $pathEntry -and -not ($wrapperDirs | Where-Object { - [string]::Equals($_, $pathEntry.TrimEnd('\'), [StringComparison]::OrdinalIgnoreCase) - }) -}) -join ';' -'ORCA_ENABLE_GIT_ATTRIBUTION', 'ORCA_GIT_COMMIT_TRAILER', 'ORCA_GH_PR_FOOTER', 'ORCA_GH_ISSUE_FOOTER', 'ORCA_ATTRIBUTION_SHIM_DIR', 'ORCA_REAL_GIT', 'ORCA_REAL_GH', 'ORCA_ATTRIBUTION_BYPASS' | ForEach-Object { Remove-Item "Env:$_" -ErrorAction SilentlyContinue } -if ($realCommand) { - try { - $capturedDir = Split-Path -Parent ([IO.Path]::GetFullPath($realCommand)) - if ([string]::Equals($capturedDir.TrimEnd('\'), $wrapperDir.TrimEnd('\'), [StringComparison]::OrdinalIgnoreCase)) { - $realCommand = $null - } - } catch { - $realCommand = $null - } -} -if (-not $realCommand -or -not (Test-Path -LiteralPath $realCommand)) { - # Why: resolve only against the cleaned PATH directories. Ambient lookup could pick up a - # repository-local git.exe/gh.exe from the current directory. - $realCommand = $null - foreach ($dir in ($env:PATH -split ';')) { - if (-not $dir) { continue } - if ($wrapperDirs | Where-Object { [string]::Equals($_, $dir.TrimEnd('\'), [StringComparison]::OrdinalIgnoreCase) }) { continue } - $candidate = Join-Path $dir "$commandName.exe" - if (Test-Path -LiteralPath $candidate -PathType Leaf) { $realCommand = $candidate; break } - } -} -if (-not $realCommand) { - [Console]::Error.WriteLine("Orca compatibility wrapper could not locate $commandName on PATH.") - exit 127 -} -& $realCommand @args -exit $LASTEXITCODE -` - let neutralized = false let neutralizationRetryTimer: ReturnType | null = null let neutralizationRetryAttempt = 0 @@ -179,27 +96,20 @@ function writeNeutralWrappers(rootDir: string): void { mkdirSync(posixDir, { recursive: true }) mkdirSync(win32Dir, { recursive: true }) for (const command of ['git', 'gh'] as const) { - const upperCommand = command.toUpperCase() writeFileAtomically(join(posixDir, command), renderLegacyTerminalPosixTombstone(command), 0o755) writeFileAtomically( join(win32Dir, `${command}.cmd`), - renderWindowsWrapper(WIN32_PASSTHROUGH_WRAPPER, command, upperCommand), + renderLegacyTerminalWindowsCmdTombstone(command), 0o755 ) writeFileAtomically( join(win32Dir, `${command}-wrapper.ps1`), - renderWindowsWrapper(POWERSHELL_PASSTHROUGH_WRAPPER, command, upperCommand), + renderLegacyTerminalWindowsPowerShellTombstone(command), 0o755 ) } } -function renderWindowsWrapper(template: string, command: string, upperCommand: string): string { - return template - .replaceAll('__ORCA_UPPER_COMMAND__', upperCommand) - .replaceAll('__ORCA_COMMAND__', command) -} - function writeFileAtomically(filePath: string, contents: string, mode: number): void { const temporaryPath = `${filePath}.orca-neutralizing-${process.pid}` try { diff --git a/src/main/pty/legacy-terminal-windows-tombstone.ts b/src/main/pty/legacy-terminal-windows-tombstone.ts new file mode 100644 index 00000000000..4d5bce15044 --- /dev/null +++ b/src/main/pty/legacy-terminal-windows-tombstone.ts @@ -0,0 +1,133 @@ +// Why: kept beside the POSIX tombstone so the generated wrapper text for every platform lives +// in one place, and so the shim-dir module stays under the max-lines limit. + +const WIN32_PASSTHROUGH_WRAPPER = String.raw`@echo off +setlocal +set "orca_real=%ORCA_REAL___ORCA_UPPER_COMMAND__%" +set "orca_wrapper_dir=%~dp0" +set "orca_legacy_wrapper_dir=%ORCA_ATTRIBUTION_SHIM_DIR%" +set "orca_clean_path=" +rem Why: an empty PATH leaves the substitution below with an unbalanced quote, which +rem desynchronizes cmd parsing for the rest of the file. Skip the line entirely instead. +if not defined PATH goto :orca_path_walked +for %%P in ("%PATH:;=" "%") do call :orca_append_path "%%~P" +:orca_path_walked +set "PATH=%orca_clean_path%" +set "ORCA_ENABLE_GIT_ATTRIBUTION=" +set "ORCA_GIT_COMMIT_TRAILER=" +set "ORCA_GH_PR_FOOTER=" +set "ORCA_GH_ISSUE_FOOTER=" +set "ORCA_ATTRIBUTION_SHIM_DIR=" +set "ORCA_ATTRIBUTION_BYPASS=" +set "ORCA_REAL_GIT=" +set "ORCA_REAL_GH=" +if defined orca_real for %%G in ("%orca_real%") do if /I "%%~dpG"=="%~dp0" set "orca_real=" +rem Why: clear a captured path that no longer exists, or the PATH walk below is skipped. +if defined orca_real if not exist "%orca_real%" set "orca_real=" +if defined orca_real goto run +rem Why: an unqualified Windows command lookup searches the current directory before PATH, so a +rem repository-local __ORCA_COMMAND__.exe would win. Walk the cleaned PATH ourselves instead. +if not defined orca_clean_path goto :orca_candidates_walked +for %%P in ("%orca_clean_path:;=" "%") do call :orca_try_candidate "%%~P" +:orca_candidates_walked +if not defined orca_real ( + echo Orca compatibility wrapper could not locate __ORCA_COMMAND__ on PATH. 1>&2 + exit /b 127 +) +:run +"%orca_real%" %* +exit /b %ERRORLEVEL% + +:orca_try_candidate +if defined orca_real exit /b +if "%~1"=="" exit /b +rem Why: a relative entry resolves against the current directory, same exposure as an empty one. +rem Tested in pure batch on purpose: an external tool invoked here would itself be resolved from +rem the current directory, reintroducing the very hijack this guard exists to prevent. +set "orca_candidate=%~1" +if "%orca_candidate:~0,2%"=="\\" goto orca_candidate_rooted +if "%orca_candidate:~1,2%"==":\" goto orca_candidate_rooted +if "%orca_candidate:~1,2%"==":/" goto orca_candidate_rooted +exit /b +:orca_candidate_rooted +rem Why: %~dp0 is rebound to this label inside CALL, so compare against the cached wrapper dir. +for %%G in ("%~1") do ( + if /I not "%%~fG\"=="%orca_wrapper_dir%" ( + if exist "%%~fG\__ORCA_COMMAND__.exe" set "orca_real=%%~fG\__ORCA_COMMAND__.exe" + if not defined orca_real if exist "%%~fG\__ORCA_COMMAND__.cmd" set "orca_real=%%~fG\__ORCA_COMMAND__.cmd" + if not defined orca_real if exist "%%~fG\__ORCA_COMMAND__.bat" set "orca_real=%%~fG\__ORCA_COMMAND__.bat" + ) +) +exit /b + +:orca_append_path +for %%G in ("%~1") do set "orca_path_entry_dir=%%~fG\" +if /I "%orca_path_entry_dir%"=="%orca_wrapper_dir%" exit /b +if defined orca_legacy_wrapper_dir for %%G in ("%orca_legacy_wrapper_dir%") do if /I "%orca_path_entry_dir%"=="%%~fG\" exit /b +if defined orca_clean_path (set "orca_clean_path=%orca_clean_path%;%~1") else set "orca_clean_path=%~1" +exit /b +` + +const POWERSHELL_PASSTHROUGH_WRAPPER = String.raw`$ErrorActionPreference = 'Stop' +$commandName = '__ORCA_COMMAND__' +$realCommand = [Environment]::GetEnvironmentVariable('ORCA_REAL___ORCA_UPPER_COMMAND__') +$wrapperDir = Split-Path -Parent $MyInvocation.MyCommand.Path +$legacyWrapperDir = $env:ORCA_ATTRIBUTION_SHIM_DIR +$wrapperDirs = @($wrapperDir, $legacyWrapperDir) | Where-Object { $_ } | ForEach-Object { $_.TrimEnd('\') } +$env:PATH = (($env:PATH -split ';') | Where-Object { + $pathEntry = $_ + $pathEntry -and -not ($wrapperDirs | Where-Object { + [string]::Equals($_, $pathEntry.TrimEnd('\'), [StringComparison]::OrdinalIgnoreCase) + }) +}) -join ';' +'ORCA_ENABLE_GIT_ATTRIBUTION', 'ORCA_GIT_COMMIT_TRAILER', 'ORCA_GH_PR_FOOTER', 'ORCA_GH_ISSUE_FOOTER', 'ORCA_ATTRIBUTION_SHIM_DIR', 'ORCA_REAL_GIT', 'ORCA_REAL_GH', 'ORCA_ATTRIBUTION_BYPASS' | ForEach-Object { Remove-Item "Env:$_" -ErrorAction SilentlyContinue } +if ($realCommand) { + try { + $capturedDir = Split-Path -Parent ([IO.Path]::GetFullPath($realCommand)) + if ([string]::Equals($capturedDir.TrimEnd('\'), $wrapperDir.TrimEnd('\'), [StringComparison]::OrdinalIgnoreCase)) { + $realCommand = $null + } + } catch { + $realCommand = $null + } +} +if (-not $realCommand -or -not (Test-Path -LiteralPath $realCommand)) { + # Why: resolve only against the cleaned PATH directories. Ambient lookup could pick up a + # repository-local git.exe/gh.exe from the current directory. + $realCommand = $null + foreach ($dir in ($env:PATH -split ';')) { + if (-not $dir) { continue } + # Why: a relative entry resolves against the current directory, same exposure as an empty one. + # IsPathRooted is not enough: it accepts drive-relative 'C:foo', which resolves against the + # current directory on that drive. IsPathFullyQualified is absent on Windows PowerShell 5.1, + # so match the same prefixes the cmd wrapper accepts. + if ($dir -notmatch '^([A-Za-z]:[\\/]|\\\\)') { continue } + if ($wrapperDirs | Where-Object { [string]::Equals($_, $dir.TrimEnd('\'), [StringComparison]::OrdinalIgnoreCase) }) { continue } + foreach ($ext in @('.exe', '.cmd', '.bat')) { + $candidate = Join-Path $dir "$commandName$ext" + if (Test-Path -LiteralPath $candidate -PathType Leaf) { $realCommand = $candidate; break } + } + if ($realCommand) { break } + } +} +if (-not $realCommand) { + [Console]::Error.WriteLine("Orca compatibility wrapper could not locate $commandName on PATH.") + exit 127 +} +& $realCommand @args +exit $LASTEXITCODE +` + +function renderWindowsWrapper(template: string, command: string, upperCommand: string): string { + return template + .replaceAll('__ORCA_UPPER_COMMAND__', upperCommand) + .replaceAll('__ORCA_COMMAND__', command) +} + +export function renderLegacyTerminalWindowsCmdTombstone(command: 'git' | 'gh'): string { + return renderWindowsWrapper(WIN32_PASSTHROUGH_WRAPPER, command, command.toUpperCase()) +} + +export function renderLegacyTerminalWindowsPowerShellTombstone(command: 'git' | 'gh'): string { + return renderWindowsWrapper(POWERSHELL_PASSTHROUGH_WRAPPER, command, command.toUpperCase()) +} diff --git a/src/renderer/src/components/terminal-pane/agent-interrupt-inference.test.ts b/src/renderer/src/components/terminal-pane/agent-interrupt-inference.test.ts index 98cc941aadd..9bab0bd5005 100644 --- a/src/renderer/src/components/terminal-pane/agent-interrupt-inference.test.ts +++ b/src/renderer/src/components/terminal-pane/agent-interrupt-inference.test.ts @@ -72,34 +72,59 @@ describe('agent interrupt inference', () => { it.each([ ['plain-escape', 'gemini'], - ['ctrl-c', 'gemini'] - ] as const)( - 'emits a strict baseline request for %s from Gemini immediately', - (intent, agentType) => { - vi.useFakeTimers() - let entry: AgentStatusEntry | undefined = makeEntry({ agentType }) - const inferInterrupt = vi.fn() - const tracker = createAgentInterruptInference({ - paneKey: PANE_KEY, - getStatusEntry: () => entry, - inferInterrupt, - now: () => 1_100 - }) + ['ctrl-c', 'gemini'], + ['plain-escape', 'codex'] + ] as const)('emits a strict baseline request for %s from %s immediately', (intent, agentType) => { + vi.useFakeTimers() + let entry: AgentStatusEntry | undefined = makeEntry({ agentType }) + const inferInterrupt = vi.fn() + const tracker = createAgentInterruptInference({ + paneKey: PANE_KEY, + getStatusEntry: () => entry, + inferInterrupt, + now: () => 1_100 + }) - tracker.observeInputIntent(intent) + tracker.observeInputIntent(intent) - expect(inferInterrupt).toHaveBeenCalledWith({ - paneKey: PANE_KEY, - baselineUpdatedAt: 1_000, - baselineStateStartedAt: 900, - baselinePrompt: 'write tests', - baselineAgentType: agentType, - intent - }) - tracker.dispose() - entry = undefined - } - ) + expect(inferInterrupt).toHaveBeenCalledWith({ + paneKey: PANE_KEY, + baselineUpdatedAt: 1_000, + baselineStateStartedAt: 900, + baselinePrompt: 'write tests', + baselineAgentType: agentType, + intent + }) + tracker.dispose() + entry = undefined + }) + + it('records a Codex Escape before its immediate done hook replaces the working row', () => { + vi.useFakeTimers() + let entry: AgentStatusEntry | undefined = makeEntry({ agentType: 'codex' }) + const inferInterrupt = vi.fn() + const tracker = createAgentInterruptInference({ + paneKey: PANE_KEY, + getStatusEntry: () => entry, + inferInterrupt, + now: () => 1_100 + }) + + tracker.observeInputIntent('plain-escape') + entry = makeEntry({ state: 'done', updatedAt: 1_101, stateStartedAt: 1_101 }) + vi.advanceTimersByTime(500) + + expect(inferInterrupt).toHaveBeenCalledTimes(1) + expect(inferInterrupt).toHaveBeenCalledWith({ + paneKey: PANE_KEY, + baselineUpdatedAt: 1_000, + baselineStateStartedAt: 900, + baselinePrompt: 'write tests', + baselineAgentType: 'codex', + intent: 'plain-escape' + }) + tracker.dispose() + }) it('reports Escape while Claude is waiting on AskUserQuestion', () => { vi.useFakeTimers() @@ -344,7 +369,7 @@ describe('agent interrupt inference', () => { it('cancels when a newer hook update arrives during the settle window', () => { vi.useFakeTimers() const inferInterrupt = vi.fn() - let entry: AgentStatusEntry | undefined = makeEntry() + let entry: AgentStatusEntry | undefined = makeEntry({ agentType: 'custom-agent' }) const tracker = createAgentInterruptInference({ paneKey: PANE_KEY, getStatusEntry: () => entry, diff --git a/src/renderer/src/components/terminal-pane/agent-interrupt-inference.ts b/src/renderer/src/components/terminal-pane/agent-interrupt-inference.ts index b847badcdfc..8e2f4cecde3 100644 --- a/src/renderer/src/components/terminal-pane/agent-interrupt-inference.ts +++ b/src/renderer/src/components/terminal-pane/agent-interrupt-inference.ts @@ -15,7 +15,7 @@ export type AgentInterruptInference = { intent: AgentInterruptInputIntent, entry?: AgentStatusEntry | null, baselineSequence?: number - ): void + ): boolean | Promise | undefined flushPending(): boolean | Promise dispose(): void } @@ -50,7 +50,8 @@ function shouldFlushInterruptImmediately( ): boolean { return ( requiresDoubleEscapeForAgent(baseline.agentType, baseline.intent) || - baseline.agentType === 'gemini' + baseline.agentType === 'gemini' || + (baseline.agentType === 'codex' && baseline.intent === 'plain-escape') ) } @@ -261,12 +262,12 @@ export function createAgentInterruptInference({ } pendingBaseline = baseline if (shouldFlushInterruptImmediately(baseline)) { - // Why: these agents can emit their idle/done hook immediately after an - // accepted interrupt. Flush before that hook overwrites the working baseline. - void flushPending() - return + // Why: these interrupts can emit an idle/done hook before the settle timer, + // overwriting the working baseline and losing the interrupted outcome. + return flushPending() } pendingTimer = setTimer(flushPendingFromTimer, AGENT_INTERRUPT_SETTLE_MS) + return undefined }, flushPending, dispose() { diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 572d3e68b19..b4a683874ee 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -2015,10 +2015,10 @@ export function connectPanePty( // "question answered" signal no hook will ever deliver. questionAnsweredInference.observeSentTerminalInput(data) } - let pendingTerminalInputWrite: Promise | null = null + let pendingTerminalInputWrite: Promise | null = null let sequencedInterruptStatusBaseline: AgentStatusEntry | null | undefined let interruptStatusBaselineSequence = 0 - const setPendingTerminalInputWrite = (promise: Promise): void => { + const setPendingTerminalInputWrite = (promise: Promise): void => { pendingTerminalInputWrite = promise void promise.finally(() => { if (pendingTerminalInputWrite === promise) { @@ -2031,7 +2031,9 @@ export function connectPanePty( if (!pendingWrite) { return interruptInference.flushPending() } - return pendingWrite.then(() => interruptInference.flushPending()) + return pendingWrite.then((immediateResult) => { + return immediateResult ?? 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 @@ -4166,26 +4168,28 @@ export function connectPanePty( clearPendingTerminalInputIntent() const writePromise = transport .sendInputAccepted(data) - .then((accepted) => { + .then((accepted): boolean | Promise | null => { if (accepted) { // Why: rejected writes use transport recovery and must not arm a parser probe. markAcceptedTerminalInputSent() observeAcceptedShellCommandInput(data) observeAcceptedTerminalInput(data, acknowledgedIntent) - interruptInference.observeInputIntent( + const immediateResult = interruptInference.observeInputIntent( acknowledgedIntent, interruptStatusBaseline, capturedBaselineSequence ) observeTitleOnlyInterrupt() - } else { - // Why: Esc/Ctrl+C are the first keys users press on a frozen pane; - // an unbound-transport reject here must arm recovery too. - requestRecoveryForUndeliverableInput() + return immediateResult ?? null } + // Why: Esc/Ctrl+C are the first keys users press on a frozen pane; + // an unbound-transport reject here must arm recovery too. + requestRecoveryForUndeliverableInput() + return null }) .catch((err) => { console.warn('[agent-interrupt] acknowledged terminal input failed:', err) + return null }) setPendingTerminalInputWrite(writePromise) return diff --git a/src/renderer/src/lib/workspace-browser-tab-open.test.ts b/src/renderer/src/lib/workspace-browser-tab-open.test.ts index 936bed7bb2c..3e8b93a6726 100644 --- a/src/renderer/src/lib/workspace-browser-tab-open.test.ts +++ b/src/renderer/src/lib/workspace-browser-tab-open.test.ts @@ -131,6 +131,32 @@ describe('openWorkspaceBrowserTab', () => { expect(createBrowserTab).not.toHaveBeenCalled() }) + it('waits for host registration before reconciling an asserted runtime link', async () => { + const createBrowserTab = vi.fn() + mocks.state = { + ...ownerState(toRuntimeExecutionHostId('hub-a')), + ...browserCapableRuntime('hub-a'), + createBrowserTab, + defaultBrowserSessionProfileId: 'client-profile', + defaultBrowserSessionProfileIdByHostId: {} + } + await openWorkspaceBrowserTab({ + workspaceId: WORKSPACE_ID, + url: 'https://example.com/pinned', + intent: { kind: 'url' }, + expectedRuntimeEnvironmentId: 'hub-a' + }) + + expect(mocks.createRemote).toHaveBeenCalledWith( + expect.objectContaining({ + environmentId: 'hub-a', + waitForRegistration: true, + worktreeId: WORKSPACE_ID + }) + ) + expect(createBrowserTab).not.toHaveBeenCalled() + }) + it('fails closed when the workspace route swaps away from the pane runtime before opening', async () => { mocks.state = { ...ownerState(toRuntimeExecutionHostId('hub-a')), diff --git a/src/renderer/src/lib/workspace-browser-tab-open.ts b/src/renderer/src/lib/workspace-browser-tab-open.ts index cc6adbb5669..52416b75739 100644 --- a/src/renderer/src/lib/workspace-browser-tab-open.ts +++ b/src/renderer/src/lib/workspace-browser-tab-open.ts @@ -224,6 +224,8 @@ export async function openWorkspaceBrowserTab( environmentId, url: request.url, targetGroupId: request.targetGroupId, + // Owner-pinned links need the host tab published before client reconciliation. + ...(expectedEnvironmentId !== null ? { waitForRegistration: true } : {}), // Why: the tab is opened from this workspace's tab bar, so surface that // workspace — otherwise a background worktree looks like nothing happened. selectWorktree: true, diff --git a/src/renderer/src/runtime/web-runtime-session.ts b/src/renderer/src/runtime/web-runtime-session.ts index 34dcfae1d3a..164edbed4b7 100644 --- a/src/renderer/src/runtime/web-runtime-session.ts +++ b/src/renderer/src/runtime/web-runtime-session.ts @@ -463,6 +463,8 @@ export async function createWebRuntimeSessionBrowserTab(args: { clientTargetGroupId?: string clientTargetGroupCreated?: boolean focusOnCreate?: boolean + /** Wait until a renderer-backed host can publish the new page in its session snapshot. */ + waitForRegistration?: boolean selectWorktree?: boolean stagedTitle?: string stagedFocusAddressBar?: boolean @@ -549,7 +551,7 @@ export async function createWebRuntimeSessionBrowserTab(args: { // Why: place the new browser in the clicked split group so the host snapshot is authoritative for it (no left-snap). ...(args.targetGroupId ? { targetGroupId: args.targetGroupId } : {}), // Why: web clients need the local tab now; waiting for host webview registration makes the workspace appear to close. - waitForRegistration: false + waitForRegistration: args.waitForRegistration ?? false }, timeoutMs: 15_000 })) as RuntimeRpcResponse