diff --git a/config/scripts/computer-use-windows-horizontal-scroll.test.mjs b/config/scripts/computer-use-windows-horizontal-scroll.test.mjs new file mode 100644 index 00000000000..3d76908daac --- /dev/null +++ b/config/scripts/computer-use-windows-horizontal-scroll.test.mjs @@ -0,0 +1,48 @@ +import { readFileSync } from 'node:fs' +import { join, resolve } from 'node:path' +import { describe, expect, it } from 'vitest' + +const projectDir = resolve(import.meta.dirname, '../..') + +function source(path) { + return readFileSync(join(projectDir, path), 'utf8') +} + +function sourceBetween(contents, startMarker, endMarker) { + const start = contents.indexOf(startMarker) + const end = contents.indexOf(endMarker, start + startMarker.length) + if (start === -1 || end === -1) { + throw new Error(`Missing source boundary: ${startMarker} → ${endMarker}`) + } + return contents.slice(start, end) +} + +describe('Windows computer-use horizontal scroll', () => { + it('routes left and right through the horizontal wheel with native signs', () => { + const windows = source('native/computer-use-windows/runtime.ps1') + const mouseEvents = sourceBetween(windows, '$MouseEvents = @{', 'function Write-OrcaJson') + const scroll = sourceBetween(windows, ' "scroll" {', ' "drag" {') + const left = sourceBetween( + scroll, + '} elseif ($Operation.direction -eq "left") {', + '} elseif ($Operation.direction -eq "right") {' + ) + const right = sourceBetween( + scroll, + '} elseif ($Operation.direction -eq "right") {', + '} elseif ($Operation.direction -ne "up") {' + ) + + expect(mouseEvents).toContain('HorizontalWheel = 0x01000') + expect(scroll).toContain('$mouseEvent = $MouseEvents.Wheel') + expect(left).toContain('$mouseEvent = $MouseEvents.HorizontalWheel') + expect(left).toContain('$delta = -1 * $delta') + expect(right).toContain('$mouseEvent = $MouseEvents.HorizontalWheel') + expect(right).not.toContain('$delta = -1 * $delta') + expect(scroll).toContain( + '[OrcaDesktopWin32]::mouse_event($mouseEvent, 0, 0, $delta, [UIntPtr]::Zero)' + ) + expect(scroll).not.toContain('mouse_event($MouseEvents.Wheel') + expect(scroll).toContain('throw "unsupported scroll direction: $($Operation.direction)"') + }) +}) diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index d7e251d0d86..7ccdd5acf2d 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -14,6 +14,7 @@ "../src/main/agent-hooks/managed-agent-hook-registry.ts", "../src/main/agent-hooks/managed-hook-script-refresh.ts", "../src/main/agent-hooks/runtime-home-hook-command.ts", + "../src/main/agent-hooks/windows-powershell-hook-launcher.ts", "../src/main/amp/agent-status-plugin-source.ts", "../src/main/amp/hook-service.ts", "../src/main/amp/managed-plugin-install-status.ts", diff --git a/native/computer-use-windows/runtime.ps1 b/native/computer-use-windows/runtime.ps1 index 2aa581901e6..4b68525c7c6 100644 --- a/native/computer-use-windows/runtime.ps1 +++ b/native/computer-use-windows/runtime.ps1 @@ -179,6 +179,7 @@ $MouseEvents = @{ MiddleDown = 0x0020 MiddleUp = 0x0040 Wheel = 0x0800 + HorizontalWheel = 0x01000 } function Write-OrcaJson($Payload) { @@ -1235,16 +1236,22 @@ function Invoke-OrcaOperation($Operation) { } "scroll" { $delta = 120 * [int][Math]::Ceiling((Get-OrcaPositiveNumber $Operation.pages "pages")) - if ($Operation.direction -eq "down" -or $Operation.direction -eq "right") { + $mouseEvent = $MouseEvents.Wheel + if ($Operation.direction -eq "down") { $delta = -1 * $delta - } elseif ($Operation.direction -ne "up" -and $Operation.direction -ne "left") { + } elseif ($Operation.direction -eq "left") { + $mouseEvent = $MouseEvents.HorizontalWheel + $delta = -1 * $delta + } elseif ($Operation.direction -eq "right") { + $mouseEvent = $MouseEvents.HorizontalWheel + } elseif ($Operation.direction -ne "up") { throw "unsupported scroll direction: $($Operation.direction)" } $point = Get-OrcaElementScreenPoint $element if ($null -eq $point) { $point = Get-OrcaScreenPoint $Operation $windowFrame } [void][OrcaDesktopWin32]::SetForegroundWindow($handle) [void][OrcaDesktopWin32]::SetCursorPos([int]$point.x, [int]$point.y) - [OrcaDesktopWin32]::mouse_event($MouseEvents.Wheel, 0, 0, $delta, [UIntPtr]::Zero) + [OrcaDesktopWin32]::mouse_event($mouseEvent, 0, 0, $delta, [UIntPtr]::Zero) $action = [pscustomobject]@{ path = "synthetic"; actionName = "scroll"; fallbackReason = $null } } "drag" { diff --git a/src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts b/src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts index 14235654d37..5204c6b0b0b 100644 --- a/src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts +++ b/src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts @@ -43,6 +43,22 @@ describe('createHookStatusSessionTabsInvalidator', () => { expect(changed(working({}, payload))).toBe(true) }) + it('invalidates when the completion stamp is added, changed, or removed', () => { + const changed = createHookStatusSessionTabsInvalidator() + changed(working()) + + expect(changed(working({}, { turnCompletedAt: 100 }))).toBe(true) + expect(changed(working({}, { turnCompletedAt: 200 }))).toBe(true) + expect(changed(working())).toBe(true) + }) + + it('invalidates when the assistant body changes', () => { + const changed = createHookStatusSessionTabsInvalidator() + changed(working({}, { lastAssistantMessage: 'First answer' })) + + expect(changed(working({}, { lastAssistantMessage: 'Corrected answer' }))).toBe(true) + }) + it('ignores resume-identity rows, which the provider-session path owns', () => { const changed = createHookStatusSessionTabsInvalidator() diff --git a/src/main/agent-hooks/hook-status-session-tabs-invalidation.ts b/src/main/agent-hooks/hook-status-session-tabs-invalidation.ts index 59b259fa849..3aa90b530f4 100644 --- a/src/main/agent-hooks/hook-status-session-tabs-invalidation.ts +++ b/src/main/agent-hooks/hook-status-session-tabs-invalidation.ts @@ -28,7 +28,9 @@ export function createHookStatusSessionTabsInvalidator(): { (previous.agentType ?? null) !== (next.agentType ?? null) || (previous.toolName ?? null) !== (next.toolName ?? null) || (previous.interactivePrompt ?? null) !== (next.interactivePrompt ?? null) || - (previous.interrupted ?? false) !== (next.interrupted ?? false) + (previous.interrupted ?? false) !== (next.interrupted ?? false) || + (previous.turnCompletedAt ?? null) !== (next.turnCompletedAt ?? null) || + (previous.lastAssistantMessage ?? null) !== (next.lastAssistantMessage ?? null) ) } // Why: a cleared pane must re-arm, else the memo swallows the first event of the diff --git a/src/main/agent-hooks/installer-utils.test.ts b/src/main/agent-hooks/installer-utils.test.ts index 2013865aa96..f219125b78d 100644 --- a/src/main/agent-hooks/installer-utils.test.ts +++ b/src/main/agent-hooks/installer-utils.test.ts @@ -71,7 +71,10 @@ describe('readHooksJsonWithRaw', () => { for (const contents of [`\uFEFF\uFEFF${body}`, ` \uFEFF${body}`, `{\uFEFF"hooks": {}}`]) { writeFileSync(configPath, contents, 'utf-8') - expect(readHooksJsonWithRaw(configPath)).toEqual({ raw: contents, config: null }) + expect(readHooksJsonWithRaw(configPath)).toEqual({ + raw: contents, + config: null + }) } }) @@ -82,7 +85,10 @@ describe('readHooksJsonWithRaw', () => { it('keeps the raw bytes when the contents are not a JSON object', () => { writeFileSync(configPath, 'not json\n', 'utf-8') - expect(readHooksJsonWithRaw(configPath)).toEqual({ raw: 'not json\n', config: null }) + expect(readHooksJsonWithRaw(configPath)).toEqual({ + raw: 'not json\n', + config: null + }) }) }) @@ -95,7 +101,9 @@ describe('writeHooksJson', () => { writeHooksJson(configPath, { hooks: { Stop: [] } }) expect(lstatSync(configPath).isSymbolicLink()).toBe(true) - expect(JSON.parse(readFileSync(targetPath, 'utf-8'))).toEqual({ hooks: { Stop: [] } }) + expect(JSON.parse(readFileSync(targetPath, 'utf-8'))).toEqual({ + hooks: { Stop: [] } + }) }) it('does not replace a dangling hook config symlink', () => { @@ -182,9 +190,15 @@ describe('writeHooksJson', () => { }) it('updates the .bak file to the previous version on each write', () => { - const v1: HooksConfig = { hooks: { Stop: [{ hooks: [{ type: 'command', command: 'v1' }] }] } } - const v2: HooksConfig = { hooks: { Stop: [{ hooks: [{ type: 'command', command: 'v2' }] }] } } - const v3: HooksConfig = { hooks: { Stop: [{ hooks: [{ type: 'command', command: 'v3' }] }] } } + const v1: HooksConfig = { + hooks: { Stop: [{ hooks: [{ type: 'command', command: 'v1' }] }] } + } + const v2: HooksConfig = { + hooks: { Stop: [{ hooks: [{ type: 'command', command: 'v2' }] }] } + } + const v3: HooksConfig = { + hooks: { Stop: [{ hooks: [{ type: 'command', command: 'v3' }] }] } + } writeHooksJson(configPath, v1) writeHooksJson(configPath, v2) @@ -310,7 +324,10 @@ describe('removeManagedCommands', () => { [ { hooks: [ - { type: 'command', command: '/bin/sh "/path/agent-hooks/copilot-hook.sh"' }, + { + type: 'command', + command: '/bin/sh "/path/agent-hooks/copilot-hook.sh"' + }, { type: 'command', command: 'echo keep me' } ] } @@ -381,7 +398,14 @@ describe('hookDefinitionHasManagedCommand', () => { ).toBe(true) expect( hookDefinitionHasManagedCommand( - { hooks: [{ type: 'command', command: '/bin/sh "/path/agent-hooks/copilot-hook.sh"' }] }, + { + hooks: [ + { + type: 'command', + command: '/bin/sh "/path/agent-hooks/copilot-hook.sh"' + } + ] + }, match ) ).toBe(true) @@ -539,7 +563,7 @@ describe('wrapPosixHookCommand', () => { }) const qualifiedWindowsPowerShellCommand = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ function decodeWindowsHookCommand(command: string): string { const encodedCommand = command.match(/ -EncodedCommand (\S+)$/)?.[1] @@ -549,7 +573,8 @@ function decodeWindowsHookCommand(command: string): string { function expectedDecodedWindowsHookCommand(scriptPath: string): string { const quoted = `'${scriptPath.replaceAll("'", "''")}'` - return `if (Test-Path -LiteralPath ${quoted} -PathType Leaf) { & ${quoted}; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null; exit 0` + // Why: PowerShell progress CLIXML corrupts consumers that merge stderr into JSON stdout. + return `$ProgressPreference='SilentlyContinue'; if (Test-Path -LiteralPath ${quoted} -PathType Leaf) { & ${quoted}; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null; exit 0` } describe('wrapWindowsHookCommand', () => { @@ -725,11 +750,32 @@ describe('wrapRuntimeHomeHookCommand', () => { process.platform === 'win32' ? join(process.env.ProgramFiles ?? 'C:\\Program Files', 'Git', 'bin', 'bash.exe') : '/bin/sh' - const result = spawnSync(shell, ['-c', command], { input: Buffer.alloc(1_000_000, 'x') }) + const result = spawnSync(shell, ['-c', command], { + input: Buffer.alloc(1_000_000, 'x') + }) expect(result.error).toBeUndefined() expect(result.status).toBe(0) }) + + it('emits neutral JSON when a lifecycle script is missing', () => { + const shell = + process.platform === 'win32' + ? join(process.env.ProgramFiles ?? 'C:\\Program Files', 'Git', 'bin', 'bash.exe') + : '/bin/sh' + const result = spawnSync( + shell, + ['-c', wrapRuntimeHomeHookCommand('missing-orca-hook', { neutralJsonWhenMissing: true })], + { + env: { ...process.env, HOME: tmpDir.replaceAll('\\', '/') }, + input: Buffer.alloc(1_000_000, 'x') + } + ) + + expect(result.error).toBeUndefined() + expect(result.status, result.stderr.toString()).toBe(0) + expect(JSON.parse(result.stdout.toString().trim())).toEqual({}) + }) }) describe('buildWindowsAgentHookPostCommand', () => { diff --git a/src/main/agent-hooks/installer-utils.ts b/src/main/agent-hooks/installer-utils.ts index 8861d41f0cc..0bab6e22c2a 100644 --- a/src/main/agent-hooks/installer-utils.ts +++ b/src/main/agent-hooks/installer-utils.ts @@ -16,6 +16,7 @@ import { grantDirAcl, isPermissionError } from '../win32-utils' import { POSIX_HOOK_STDIN_DRAIN_COMMAND } from './hook-stdin-contract' import { resolveHooksJsonWritePath } from './hook-config-write-path' import { writeRollingFileBackup } from '../rolling-file-backup' +import { wrapWindowsPowerShellEncodedCommand } from './windows-powershell-hook-launcher' export type HookCommandConfig = { type: 'command' @@ -120,16 +121,14 @@ export function wrapPosixHookCommand(scriptPath: string, env: Record `$env:${key} = ${quotePowerShellString(value)}; `) .join('') const command = `${envPrefix}if (Test-Path -LiteralPath ${quoted} -PathType Leaf) { & ${quoted}; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null; exit 0` - const encodedCommand = Buffer.from(command, 'utf16le').toString('base64') - return `${getWindowsPowerShellExecutablePath()} -NoProfile -ExecutionPolicy Bypass -EncodedCommand ${encodedCommand}` + return wrapWindowsPowerShellEncodedCommand(command) } export const WINDOWS_CMD_SAFE_PATH = /^[A-Za-z0-9_.:\\~-]+$/ diff --git a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts index f805e6c25f1..3232c702dfd 100644 --- a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts +++ b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts @@ -69,6 +69,7 @@ vi.mock('os', async (importOriginal) => { import { AntigravityHookService } from '../antigravity/hook-service' import { ClaudeHookService } from '../claude/hook-service' +import { getRemoteManagedCommand } from '../claude/hook-settings' import { CodexHookService } from '../codex/hook-service' import { CommandCodeHookService } from '../command-code/hook-service' import { CopilotHookService } from '../copilot/hook-service' @@ -155,6 +156,7 @@ const LOCAL_INSTALLERS = [ type HookRun = { exitCode: number | null stdinErrors: NodeJS.ErrnoException[] + stderr: string stdout: string } @@ -164,8 +166,9 @@ function runHookProcess( env: NodeJS.ProcessEnv ): Promise { return new Promise((resolve, reject) => { - const child = spawn(executable, args, { env, stdio: ['pipe', 'pipe', 'ignore'] }) + const child = spawn(executable, args, { env, stdio: ['pipe', 'pipe', 'pipe'] }) const stdinErrors: NodeJS.ErrnoException[] = [] + let stderr = '' let stdout = '' const timeout = setTimeout(() => { child.kill('SIGKILL') @@ -178,10 +181,13 @@ function runHookProcess( child.stdout.on('data', (chunk: Buffer) => { stdout += chunk.toString() }) + child.stderr.on('data', (chunk: Buffer) => { + stderr += chunk.toString() + }) child.stdin.on('error', (error: NodeJS.ErrnoException) => stdinErrors.push(error)) child.on('close', (exitCode) => { clearTimeout(timeout) - resolve({ exitCode, stdinErrors, stdout }) + resolve({ exitCode, stdinErrors, stderr, stdout }) }) child.stdin.end(LARGE_PAYLOAD) }) @@ -401,9 +407,75 @@ describe('Windows managed hook stdin structure', () => { } } ) + + // Why: command-shape tests missed conhost discarding the JSON consumers observe (#14818). + it.skipIf(process.platform !== 'win32')( + 'emits parseable JSON on stdout from the registered Claude hook command, through cmd.exe and Git Bash', + async () => { + const home = mkdtempSync(join(tmpdir(), 'orca-hook-stdout-json-')) + homedirMock.mockReturnValue(home) + try { + expect(new ClaudeHookService().install().state).toBe('installed') + const settings = JSON.parse( + readFileSync(join(home, '.claude', 'settings.json'), 'utf8') + ) as { hooks: Record } + const entry = settings.hooks.PreToolUse[0].hooks[0] + expect(entry.args).toBeUndefined() + + // Why: MSYS rewrites switches and paths, so the command must survive both shells (#14815). + const gitBash = findGitBash() + const shells = [ + { name: 'cmd.exe', executable: 'cmd.exe', args: ['/d', '/c', entry.command] }, + { name: 'Git Bash', executable: gitBash, args: ['-c', entry.command] } + ] + // Why: cover guard exit, reached curl, and the launcher's missing-script fallback. + const environments = [ + { name: 'no Orca env', env: hookEnvironment({ USERPROFILE: home }) }, + { + name: 'Orca env with dead listener', + env: hookEnvironment({ + USERPROFILE: home, + ORCA_AGENT_HOOK_PORT: '59999', + ORCA_AGENT_HOOK_TOKEN: 'token', + ORCA_PANE_KEY: 'tab:leaf' + }) + }, + { + name: 'missing managed script', + env: hookEnvironment({ USERPROFILE: join(home, 'absent') }) + } + ] + for (const shell of shells) { + for (const environment of environments) { + const label = `${shell.name} / ${environment.name}` + const result = await runHookProcess(shell.executable, shell.args, environment.env) + expect(result.exitCode, `${label} exit code`).toBe(0) + expect(result.stderr, `${label} stderr`).toBe('') + expect(() => JSON.parse(result.stdout.trim()), `${label} stdout is JSON`).not.toThrow() + expect(JSON.parse(result.stdout.trim()), `${label} stdout`).toEqual({}) + } + } + } finally { + homedirMock.mockImplementation(() => process.env.HOME ?? tmpdir()) + rmSync(home, { recursive: true, force: true }) + } + }, + // Why: six shell launches can overrun the default while the suite competes for cores. + 60_000 + ) }) describe.skipIf(process.platform === 'win32')('managed hook stdin lifecycle', () => { + it('emits neutral JSON when the Claude lifecycle script is missing', async () => { + const command = getRemoteManagedCommand('/home/dev/.orca/agent-hooks/claude-hook.sh') + const result = await runPosixHook(command) + + expect(result.exitCode).toBe(0) + expect(result.stdinErrors).toHaveLength(0) + expect(result.stderr).toBe('') + expect(JSON.parse(result.stdout.trim())).toEqual({}) + }) + it('captures stdin before every possible whole-script success exit', async () => { const scripts = await generatePosixScripts() for (const [agent, script] of scripts) { diff --git a/src/main/agent-hooks/runtime-home-hook-command.ts b/src/main/agent-hooks/runtime-home-hook-command.ts index 3ae887fc054..6d594c14d19 100644 --- a/src/main/agent-hooks/runtime-home-hook-command.ts +++ b/src/main/agent-hooks/runtime-home-hook-command.ts @@ -1,22 +1,32 @@ import { POSIX_HOOK_STDIN_DRAIN_COMMAND } from './hook-stdin-contract' +import { + encodeWindowsPowerShellHookCommand, + WINDOWS_POWERSHELL_HOOK_SWITCHES +} from './windows-powershell-hook-launcher' const MANAGED_SCRIPT_BASE_NAME = /^[A-Za-z0-9_-]+$/ const WINDOWS_GIT_BASH_RUNTIME_HOME_UNSAFE = '*\\&*|*\\^*|*\\(*|*\\)*|*\\;*|*,*|*=*|*%*|*\\!*' -export function wrapRuntimeHomeHookCommand(scriptBaseName: string): string { +export function wrapRuntimeHomeHookCommand( + scriptBaseName: string, + options: { neutralJsonWhenMissing?: boolean } = {} +): string { if (!MANAGED_SCRIPT_BASE_NAME.test(scriptBaseName)) { throw new Error(`Invalid managed script base name: ${scriptBaseName}`) } const windowsScript = `"$HOME/.orca/agent-hooks/${scriptBaseName}.cmd"` const posixScript = `"$HOME/.orca/agent-hooks/${scriptBaseName}.sh"` const drain = POSIX_HOOK_STDIN_DRAIN_COMMAND + const missingScriptFallback = options.neutralJsonWhenMissing ? `${drain}; printf '{}\\n'` : drain const powershell = '"$SYSTEMROOT/System32/WindowsPowerShell/v1.0/powershell.exe"' - const powershellCommand = `$homePath = $env:HOME -replace '^/([A-Za-z])/', '$1:/'; $scriptPath = Join-Path $homePath '.orca\\agent-hooks\\${scriptBaseName}.cmd'; if (Test-Path -LiteralPath $scriptPath -PathType Leaf) { & $scriptPath; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null; exit 0` - const encodedCommand = Buffer.from(powershellCommand, 'utf16le').toString('base64') - const powershellInvocation = `${powershell} -NoProfile -ExecutionPolicy Bypass -EncodedCommand ${encodedCommand}` - const encodedWindowsBranch = `if [ -f ${powershell} ]; then ${powershellInvocation}; else ${drain}; fi` - const windowsBranch = `if [ -f ${windowsScript} ]; then case "$HOME" in ${WINDOWS_GIT_BASH_RUNTIME_HOME_UNSAFE}) ${encodedWindowsBranch} ;; *) ${windowsScript} ;; esac; else ${drain}; fi` - const posixBranch = `if [ -f ${posixScript} ] && [ -r ${posixScript} ] && [ -x ${posixScript} ]; then /bin/sh ${posixScript}; else ${drain}; fi` + const powershellFallback = options.neutralJsonWhenMissing ? "; Write-Output '{}'" : '' + const powershellCommand = `$homePath = $env:HOME -replace '^/([A-Za-z])/', '$1:/'; $scriptPath = Join-Path $homePath '.orca\\agent-hooks\\${scriptBaseName}.cmd'; if (Test-Path -LiteralPath $scriptPath -PathType Leaf) { & $scriptPath; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null${powershellFallback}; exit 0` + const encodedCommand = encodeWindowsPowerShellHookCommand(powershellCommand) + // Why: the Git Bash and native Windows launchers must suppress windows identically (#14815). + const powershellInvocation = `${powershell} ${WINDOWS_POWERSHELL_HOOK_SWITCHES} -EncodedCommand ${encodedCommand}` + const encodedWindowsBranch = `if [ -f ${powershell} ]; then ${powershellInvocation}; else ${missingScriptFallback}; fi` + const windowsBranch = `if [ -f ${windowsScript} ]; then case "$HOME" in ${WINDOWS_GIT_BASH_RUNTIME_HOME_UNSAFE}) ${encodedWindowsBranch} ;; *) ${windowsScript} ;; esac; else ${missingScriptFallback}; fi` + const posixBranch = `if [ -f ${posixScript} ] && [ -r ${posixScript} ] && [ -x ${posixScript} ]; then /bin/sh ${posixScript}; else ${missingScriptFallback}; fi` // Why: OSTYPE is shell-owned, so platform selection adds no process to every hook invocation. - return `if [ -z "\${HOME-}" ]; then ${drain}; else case "\${OSTYPE-}" in msys*|cygwin*|win32*) ${windowsBranch} ;; *) ${posixBranch} ;; esac; fi` + return `if [ -z "\${HOME-}" ]; then ${missingScriptFallback}; else case "\${OSTYPE-}" in msys*|cygwin*|win32*) ${windowsBranch} ;; *) ${posixBranch} ;; esac; fi` } diff --git a/src/main/agent-hooks/server-ingest-terminal-status.test.ts b/src/main/agent-hooks/server-ingest-terminal-status.test.ts index b6182529628..8661701e1dc 100644 --- a/src/main/agent-hooks/server-ingest-terminal-status.test.ts +++ b/src/main/agent-hooks/server-ingest-terminal-status.test.ts @@ -27,6 +27,97 @@ afterEach(() => { }) describe('AgentHookServer ingestTerminalStatus', () => { + it('preserves a hook turn stamp when an OSC repaint omits hook-only completion text', () => { + const server = new AgentHookServer() + const listener = vi.fn() + server.setListener(listener) + + server.ingestRemote( + { + paneKey: PANE, + payload: { state: 'working', prompt: 'review the PR', agentType: 'claude' } + }, + 'conn-1' + ) + listener.mockClear() + server.ingestRemote( + { + paneKey: PANE, + payload: { + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + lastAssistantMessage: 'Review complete.', + turnCompletedAt: 1_700_000_005_000 + } + }, + 'conn-1' + ) + server.ingestTerminalStatus({ + paneKey: PANE, + connectionId: 'conn-1', + payload: { state: 'working', prompt: 'review the PR', agentType: 'claude' } + }) + + expect(listener).toHaveBeenCalledOnce() + expect(server.getStatusSnapshot()[0]).toMatchObject({ + state: 'working', + lastAssistantMessage: 'Review complete.', + turnCompletedAt: 1_700_000_005_000 + }) + + server.ingestTerminalStatus({ + paneKey: PANE, + connectionId: 'conn-1', + payload: { state: 'done', prompt: 'review the PR', agentType: 'claude' } + }) + + expect(listener).toHaveBeenCalledTimes(2) + expect(listener).toHaveBeenLastCalledWith( + expect.objectContaining({ payload: expect.objectContaining({ state: 'done' }) }) + ) + }) + + it('accepts an identical-prompt hook boundary after a stamped turn', () => { + const server = new AgentHookServer() + const listener = vi.fn() + server.setListener(listener) + + server.ingestRemote( + { + paneKey: PANE, + hookEventName: 'Stop', + payload: { + state: 'working', + prompt: 'check status', + agentType: 'claude', + turnCompletedAt: 1_700_000_005_000 + } + }, + 'conn-1' + ) + server.ingestRemote( + { + paneKey: PANE, + hookEventName: 'UserPromptSubmit', + payload: { state: 'working', prompt: 'check status', agentType: 'claude' } + }, + 'conn-1' + ) + + expect(listener).toHaveBeenCalledTimes(2) + expect(listener).toHaveBeenLastCalledWith( + expect.objectContaining({ + payload: expect.objectContaining({ + state: 'working', + prompt: 'check status', + turnCompletedAt: undefined + }) + }) + ) + expect(server.getStatusSnapshot()[0]?.turnCompletedAt).toBeUndefined() + }) + // Why: the OSC 9999 payload cannot carry a provider session, so letting it overwrite the row // erased the session id from persisted rows and from headless `orca serve` — which serves these // rows straight to mobile — leaving Chat UI with no transcript to subscribe to (#10630). diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index f819414ad4b..48ff387fad4 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -416,7 +416,8 @@ function toAgentStatusIpcPayload(entry: EnrichedAgentHookEventPayload): AgentSta // Why: OSC never carries model/children; omit both so an equivalent OSC ping preserves the hook-cached identity graph. function equivalentParsedAgentStatusPayload( a: ParsedAgentStatusPayload, - b: ParsedAgentStatusPayload + b: ParsedAgentStatusPayload, + preserveActiveTurnStamp = false ): boolean { return ( a.state === b.state && @@ -425,11 +426,14 @@ function equivalentParsedAgentStatusPayload( a.toolName === b.toolName && a.toolInput === b.toolInput && a.interactivePrompt === b.interactivePrompt && - a.lastAssistantMessage === b.lastAssistantMessage && + (a.lastAssistantMessage === b.lastAssistantMessage || + (preserveActiveTurnStamp && b.lastAssistantMessage === undefined)) && a.interrupted === b.interrupted && // Why: a session-boundary done must never be deduped against a cached real done — // the flag has to reach receivers deterministically (STA-3386). - a.sessionBoundary === b.sessionBoundary + a.sessionBoundary === b.sessionBoundary && + (a.turnCompletedAt === b.turnCompletedAt || + (preserveActiveTurnStamp && b.turnCompletedAt === undefined)) ) } @@ -630,6 +634,7 @@ export class AgentHookServer { private assistantMessageRetryTimers = new Map>() private codexSubagentPollTimers = new Map>() private promptSentDedupeByPaneKey = new Map() + private activeHookTurnCompletedAtByPaneKey = new Map() private promptSentHashSalt = randomBytes(16).toString('hex') private closedAgentStatusTabIds = new Set() private closedAgentStatusPaneKeys = new Set() @@ -917,6 +922,9 @@ export class AgentHookServer { prompt: payload.prompt, agentType: payload.agentType, ...(restored.state === 'done' && restored.interrupted ? { interrupted: true } : {}), + ...(restored.turnCompletedAt !== undefined + ? { turnCompletedAt: restored.turnCompletedAt } + : {}), ...(payload.subagents ? { subagents: payload.subagents } : {}) } }) @@ -1138,6 +1146,10 @@ export class AgentHookServer { payload: AgentHookEventPayload, onAccepted?: () => void ): EnrichedAgentHookEventPayload { + if (payload.hookEventName === 'UserPromptSubmit') { + // Why: the prompt boundary is authoritative even when text is unchanged; its next OSC working row must not inherit the prior cron/background turn stamp. + this.activeHookTurnCompletedAtByPaneKey.delete(payload.paneKey) + } const previous = this.state.lastStatusByPaneKey.get(payload.paneKey) as | EnrichedAgentHookEventPayload | undefined @@ -1277,6 +1289,15 @@ export class AgentHookServer { } const cachedPayload = resolveCachedClaudeCompactOwnership(previous, effectivePayload) const enriched = this.attachStatusTiming(cachedPayload, now) + if ( + typeof enriched.payload.turnCompletedAt === 'number' && + Number.isFinite(enriched.payload.turnCompletedAt) + ) { + this.activeHookTurnCompletedAtByPaneKey.set( + enriched.paneKey, + enriched.payload.turnCompletedAt + ) + } // Why: an identity-matched event can still leave the aggregate backed only by another restored child; keep liveness reconciliation eligible. if (enriched.restoredUnconfirmed) { this.runtimeObservedStatusPaneKeys.delete(enriched.paneKey) @@ -1591,6 +1612,11 @@ export class AgentHookServer { if (this.runtimeObservedStatusPaneKeys.delete(previousOwnerPaneKey)) { this.runtimeObservedStatusPaneKeys.add(toPaneKey) } + const activeTurnCompletedAt = this.activeHookTurnCompletedAtByPaneKey.get(previousOwnerPaneKey) + if (activeTurnCompletedAt !== undefined) { + this.activeHookTurnCompletedAtByPaneKey.delete(previousOwnerPaneKey) + this.activeHookTurnCompletedAtByPaneKey.set(toPaneKey, activeTurnCompletedAt) + } const authorityObservation = this.currentAuthorityObservations.get(previousOwnerPaneKey) if (authorityObservation) { const owner = parsePaneKey(toPaneKey) @@ -1646,6 +1672,7 @@ export class AgentHookServer { this.clearAssistantMessageRetry(key) this.clearCodexSubagentPoll(key) clearPaneCacheState(this.state, key) + this.activeHookTurnCompletedAtByPaneKey.delete(key) this.runtimeObservedStatusPaneKeys.delete(key) this.currentAuthorityObservations.delete(key) this.promptSentDedupeByPaneKey.delete(key) @@ -1679,6 +1706,7 @@ export class AgentHookServer { } this.legacyPaneKeyAliases.delete(legacyPaneKey) clearPaneCacheState(this.state, legacyPaneKey) + this.activeHookTurnCompletedAtByPaneKey.delete(legacyPaneKey) this.currentAuthorityObservations.delete(legacyPaneKey) this.promptSentDedupeByPaneKey.delete(legacyPaneKey) if (shouldClearStablePaneKey && this.state.lastStatusByPaneKey.has(entry.stablePaneKey)) { @@ -1688,6 +1716,7 @@ export class AgentHookServer { if (shouldClearStablePaneKey) { // Why: hydrated rows live under the stable key; if this PTY dies before ptyPaneKey rebuilds, alias cleanup is the only evictor. clearPaneCacheState(this.state, entry.stablePaneKey) + this.activeHookTurnCompletedAtByPaneKey.delete(entry.stablePaneKey) this.runtimeObservedStatusPaneKeys.delete(entry.stablePaneKey) this.currentAuthorityObservations.delete(entry.stablePaneKey) this.promptSentDedupeByPaneKey.delete(entry.stablePaneKey) @@ -1829,12 +1858,15 @@ export class AgentHookServer { const previous = this.state.lastStatusByPaneKey.get(paneKey) as | EnrichedAgentHookEventPayload | undefined + const preserveActiveTurnStamp = + previous?.payload.turnCompletedAt !== undefined && + previous.payload.turnCompletedAt === this.activeHookTurnCompletedAtByPaneKey.get(paneKey) if ( !previous?.restoredUnconfirmed && previous?.connectionId === connectionId && previous.tabId === tabId && previous.worktreeId === worktreeId && - equivalentParsedAgentStatusPayload(previous.payload, event.payload) + equivalentParsedAgentStatusPayload(previous.payload, event.payload, preserveActiveTurnStamp) ) { return } @@ -2331,6 +2363,7 @@ export class AgentHookServer { return null } this.state.lastStatusByPaneKey.delete(resolvedPaneKey) + this.activeHookTurnCompletedAtByPaneKey.delete(resolvedPaneKey) if (!options?.preserveAuthority) { this.hydratedLaunchTokenHashByPaneKey.delete(resolvedPaneKey) this.persistedAuthorityCommitmentsByPaneKey.delete(resolvedPaneKey) @@ -2411,6 +2444,7 @@ export class AgentHookServer { this.clearAssistantMessageRetry(paneKey) this.clearCodexSubagentPoll(paneKey) clearPaneCacheState(this.state, paneKey) + this.activeHookTurnCompletedAtByPaneKey.delete(paneKey) this.runtimeObservedStatusPaneKeys.delete(paneKey) this.currentAuthorityObservations.delete(paneKey) this.promptSentDedupeByPaneKey.delete(paneKey) @@ -2432,6 +2466,7 @@ export class AgentHookServer { this.clearAssistantMessageRetry(resolvedPaneKey) this.clearCodexSubagentPoll(resolvedPaneKey) clearPaneCacheState(this.state, resolvedPaneKey) + this.activeHookTurnCompletedAtByPaneKey.delete(resolvedPaneKey) this.currentAuthorityObservations.delete(resolvedPaneKey) this.promptSentDedupeByPaneKey.delete(resolvedPaneKey) let clearedAlias = false @@ -2441,6 +2476,7 @@ export class AgentHookServer { paneKeys.add(legacyPaneKey) paneKeys.add(stablePaneKey.stablePaneKey) clearPaneCacheState(this.state, legacyPaneKey) + this.activeHookTurnCompletedAtByPaneKey.delete(legacyPaneKey) this.currentAuthorityObservations.delete(legacyPaneKey) this.promptSentDedupeByPaneKey.delete(legacyPaneKey) clearedAlias = true diff --git a/src/main/agent-hooks/windows-powershell-hook-launcher.ts b/src/main/agent-hooks/windows-powershell-hook-launcher.ts new file mode 100644 index 00000000000..e3974ef8ff1 --- /dev/null +++ b/src/main/agent-hooks/windows-powershell-hook-launcher.ts @@ -0,0 +1,27 @@ +// Why: centralizing the launcher keeps window suppression consistent across installers (#14815). + +// Why: an absolute forward-slash path avoids PATH hijacking and survives cmd.exe and Git Bash. +export function getWindowsSystem32Path(relativePath: string): string { + const systemRoot = process.env.SystemRoot || 'C:\\Windows' + return `${systemRoot.replaceAll('\\', '/')}/System32/${relativePath}` +} + +export function getWindowsPowerShellExecutablePath(): string { + return getWindowsSystem32Path('WindowsPowerShell/v1.0/powershell.exe') +} + +// Why: unlike conhost, hidden PowerShell relays hook output and exit status (#14818). +export const WINDOWS_POWERSHELL_HOOK_SWITCHES = + '-NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden' + +// Why: redirected PowerShell progress becomes CLIXML that can corrupt merged JSON output. +const HOOK_PROGRESS_SILENCER = "$ProgressPreference='SilentlyContinue'; " + +// Why: encoding shields paths and switches from cmd.exe and MSYS rewriting (#6078, #14815). +export function encodeWindowsPowerShellHookCommand(command: string): string { + return Buffer.from(`${HOOK_PROGRESS_SILENCER}${command}`, 'utf16le').toString('base64') +} + +export function wrapWindowsPowerShellEncodedCommand(command: string): string { + return `${getWindowsPowerShellExecutablePath()} ${WINDOWS_POWERSHELL_HOOK_SWITCHES} -EncodedCommand ${encodeWindowsPowerShellHookCommand(command)}` +} diff --git a/src/main/browser/anti-detection-permission-status.test.ts b/src/main/browser/anti-detection-permission-status.test.ts new file mode 100644 index 00000000000..d686e32c975 --- /dev/null +++ b/src/main/browser/anti-detection-permission-status.test.ts @@ -0,0 +1,210 @@ +import { runInNewContext } from 'node:vm' +import { describe, expect, it } from 'vitest' + +import { ANTI_DETECTION_SCRIPT } from './anti-detection' + +type PermissionQueryResult = EventTarget & { + state: string + onchange: EventListener | null + marker: string +} + +type PermissionStatusConstructor = { + new (): PermissionQueryResult + prototype: PermissionQueryResult +} + +type AntiDetectionContext = { + Notification: { + permission: string + requestPermission: (callback?: (permission: string) => void) => Promise + } + PermissionStatus: PermissionStatusConstructor + dispatchPermissionChange: (name: string) => void + navigator: { + permissions: { + query: (descriptor: { name: string }) => Promise + } + } +} + +function createContext(args: { + nativeNotificationPermission: string + requestedNotificationPermission: string + rejectedPermissions?: string[] +}): AntiDetectionContext & Record { + class PermissionStatus extends EventTarget { + #state = 'denied' + #onchange: EventListener | null = null + marker = 'real-status' + + get state(): string { + return this.#state + } + + get onchange(): EventListener | null { + return this.#onchange + } + + set onchange(listener: EventListener | null) { + if (this.#onchange) { + super.removeEventListener('change', this.#onchange) + } + this.#onchange = typeof listener === 'function' ? listener : null + if (this.#onchange) { + super.addEventListener('change', this.#onchange) + } + } + } + + const statuses = new Map() + const rejectedPermissions = new Set(args.rejectedPermissions) + + class Permissions { + query(descriptor: { name: string }): Promise { + if (rejectedPermissions.has(descriptor.name)) { + return Promise.reject(new Error('Unsupported permission')) + } + const status = new PermissionStatus() + const permissionStatuses = statuses.get(descriptor.name) ?? [] + permissionStatuses.push(status) + statuses.set(descriptor.name, permissionStatuses) + return Promise.resolve(status) + } + } + + const Notification = { + permission: args.nativeNotificationPermission, + requestPermission(callback?: (permission: string) => void): Promise { + callback?.(args.requestedNotificationPermission) + return Promise.resolve(args.requestedNotificationPermission) + } + } + Object.defineProperty(Notification, 'permission', { + configurable: true, + get: () => args.nativeNotificationPermission + }) + + return { + Date, + Event, + EventTarget, + Object, + Promise, + Set, + performance: { now: () => 0 }, + // Why: the script's Firefox gate reads navigator.userAgent, so a non-Firefox UA keeps these + // tests on the ordinary-page path where the PermissionStatus override applies. + window: { chrome: {} }, + navigator: { + userAgent: + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/150.0.0.0 Safari/537.36', + plugins: [], + languages: [], + permissions: new Permissions() + }, + Permissions, + PermissionStatus, + Notification, + dispatchPermissionChange(name: string): void { + for (const status of statuses.get(name) ?? []) { + status.dispatchEvent(new Event('change')) + } + } + } as AntiDetectionContext & Record +} + +describe('ANTI_DETECTION_SCRIPT — PermissionStatus', () => { + it('keeps an existing notification status current after permission changes', async () => { + const context = createContext({ + nativeNotificationPermission: 'denied', + requestedNotificationPermission: 'granted' + }) + + runInNewContext(ANTI_DETECTION_SCRIPT, context) + const status = await context.navigator.permissions.query({ name: 'notifications' }) + + expect(context.Notification.permission).toBe('default') + expect(status.state).toBe('prompt') + + await expect(context.Notification.requestPermission()).resolves.toBe('granted') + + expect(context.Notification.permission).toBe('granted') + expect(status.state).toBe('granted') + }) + + it('preserves native PermissionStatus identity and methods', async () => { + const context = createContext({ + nativeNotificationPermission: 'denied', + requestedNotificationPermission: 'granted' + }) + + runInNewContext(ANTI_DETECTION_SCRIPT, context) + const status = await context.navigator.permissions.query({ name: 'camera' }) + const expectedSource = Function.prototype.toString.call( + context.PermissionStatus.prototype.addEventListener + ) + + expect(status).toBeInstanceOf(context.PermissionStatus) + expect(status.state).toBe('prompt') + expect(status.constructor.name).toBe('PermissionStatus') + expect(status.marker).toBe('real-status') + expect(status.addEventListener.name).toBe('addEventListener') + expect(status.addEventListener).toBe(status.addEventListener) + expect(Function.prototype.toString.call(status.addEventListener)).toBe(expectedSource) + expect(expectedSource).toContain('addEventListener') + }) + + it('delivers change events through the returned status with the overridden state', async () => { + const context = createContext({ + nativeNotificationPermission: 'denied', + requestedNotificationPermission: 'granted' + }) + + runInNewContext(ANTI_DETECTION_SCRIPT, context) + const status = await context.navigator.permissions.query({ name: 'notifications' }) + const events: { receiver: EventTarget; target: EventTarget | null; state: string }[] = [] + const recordEvent = function (this: EventTarget, event: Event): void { + events.push({ + receiver: this, + target: event.target, + state: (event.target as PermissionQueryResult).state + }) + } + + status.addEventListener('change', recordEvent) + expect(() => { + status.onchange = function (this: EventTarget, event): void { + recordEvent.call(this, event) + } + }).not.toThrow() + + await context.Notification.requestPermission() + context.dispatchPermissionChange('notifications') + + expect(events).toHaveLength(2) + expect(events).toEqual([ + { receiver: status, target: status, state: 'granted' }, + { receiver: status, target: status, state: 'granted' } + ]) + }) + + // Why: 'camera' rather than 'storage-access' — #14685 narrowed the intercepted set to + // camera/microphone, so a name outside it falls through to the real query and never reaches + // the fallback at all. + it('uses a non-enumerable EventTarget fallback when the native query rejects', async () => { + const context = createContext({ + nativeNotificationPermission: 'denied', + requestedNotificationPermission: 'granted', + rejectedPermissions: ['camera'] + }) + + runInNewContext(ANTI_DETECTION_SCRIPT, context) + const status = await context.navigator.permissions.query({ name: 'camera' }) + + expect(status).toBeInstanceOf(EventTarget) + expect(status).not.toBeInstanceOf(context.PermissionStatus) + expect(status.state).toBe('prompt') + expect(Object.keys(status)).toEqual([]) + }) +}) diff --git a/src/main/browser/anti-detection.ts b/src/main/browser/anti-detection.ts index b955b200229..d725fbf965f 100644 --- a/src/main/browser/anti-detection.ts +++ b/src/main/browser/anti-detection.ts @@ -86,12 +86,43 @@ export const ANTI_DETECTION_SCRIPT = `(function() { 'camera', 'microphone' ]); const origQuery = Permissions.prototype.query; + // Why: sites must receive the genuine PermissionStatus so native events, brand checks and method + // identity survive. Shadow only state, and resolve it lazily so existing statuses stay current. + function withOverriddenState(realStatus, stateProvider) { + Object.defineProperty(realStatus, 'state', { + configurable: true, + get: stateProvider + }); + return realStatus; + } + // Why: some names the real implementation rejects outright; fall back to an EventTarget so + // listener registration still works instead of throwing. + function fallbackStatus(stateProvider) { + const status = new EventTarget(); + Object.defineProperties(status, { + state: { configurable: true, get: stateProvider }, + onchange: { configurable: true, value: null, writable: true } + }); + return status; + } + function queryWithState(permissions, desc, stateProvider) { + let real; + try { + real = origQuery.call(permissions, desc); + } catch { + return Promise.resolve(fallbackStatus(stateProvider)); + } + return Promise.resolve(real).then( + (status) => withOverriddenState(status, stateProvider), + () => fallbackStatus(stateProvider) + ); + } Permissions.prototype.query = function(desc) { if (desc.name === 'notifications') { - return Promise.resolve({ state: notificationPermissionState(), onchange: null }); + return queryWithState(this, desc, notificationPermissionState); } if (promptPerms.has(desc.name)) { - return Promise.resolve({ state: 'prompt', onchange: null }); + return queryWithState(this, desc, () => 'prompt'); } return origQuery.call(this, desc); }; diff --git a/src/main/claude/hook-service.test.ts b/src/main/claude/hook-service.test.ts index b3fa02debc5..6f8eafb46e3 100644 --- a/src/main/claude/hook-service.test.ts +++ b/src/main/claude/hook-service.test.ts @@ -34,14 +34,28 @@ function hasManagedCommand(hook: TestHook, matcher: (command: string | undefined } describe('getWindowsManagedLifecycleHook', () => { - it('resolves the managed script from the runtime Windows profile', () => { + it('resolves the managed script from the runtime Windows profile, as a single command string', () => { const scriptPath = 'C:\\Users\\%name%\\a^b&c\\.orca\\agent-hooks\\claude-hook.cmd' const hook = getWindowsManagedLifecycleHook(scriptPath) - expect(hook.args?.[0]).toBe('--headless') - expect(hook.args?.[1]).toMatch(/\\System32\\cmd\.exe$/i) - expect(hook.args?.at(-1)).toBe('%USERPROFILE%\\.orca\\agent-hooks\\claude-hook.cmd') - expect(hook.args).not.toContain(scriptPath) + expect(hook.args).toBeUndefined() + expect(hook.command).toMatch( + /\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden / + ) + expect(hook.command).not.toContain(scriptPath) + // Why: Git Bash/MSYS mangles backslash paths and slash-prefixed switches. + expect(hook.command.replace(/-EncodedCommand \S+$/, '')).not.toMatch(/\\| \/[a-zA-Z]+( |$)/) + + const encoded = hook.command.match(/-EncodedCommand (\S+)$/)?.[1] + const decoded = Buffer.from(encoded ?? '', 'base64').toString('utf16le') + expect(decoded).toContain('$env:USERPROFILE') + expect(decoded).toContain('.orca\\agent-hooks\\claude-hook.cmd') + }) + + it('is still recognized as managed by createManagedCommandMatcher (#14825)', () => { + const scriptPath = 'C:\\Users\\alice\\.orca\\agent-hooks\\claude-hook.cmd' + const hook = getWindowsManagedLifecycleHook(scriptPath) + expect(isClaudeManagedCommand(hook.command)).toBe(true) }) }) @@ -199,9 +213,17 @@ describe('ClaudeHookService.install', () => { expect(hasManagedCommand(legacy.hooks.StopFailure[0].hooks[0], isClaudeManagedCommand)).toBe( true ) - expect( - readFileSync(join(tmpHome, '.orca', 'agent-hooks', CLAUDE_SCRIPT_FILE_NAME), 'utf-8') - ).toContain('DEVIN_PROJECT_DIR') + const managedScript = readFileSync( + join(tmpHome, '.orca', 'agent-hooks', CLAUDE_SCRIPT_FILE_NAME), + 'utf-8' + ) + expect(managedScript).toContain('DEVIN_PROJECT_DIR') + // Why: guard and Devin-skip paths must still return neutral JSON (#14818). + expect(managedScript).toMatch( + process.platform === 'win32' + ? /^@echo off\r\nsetlocal\r\necho \{\}\r\n/ + : /^#!\/bin\/sh\nprintf "\{\}\\n"\n/ + ) } finally { vi.unstubAllEnvs() rmSync(tmpHome, { recursive: true, force: true }) @@ -255,7 +277,12 @@ describe('ClaudeHookService.install', () => { mkdirSync(join(tmpHome, '.claude'), { recursive: true }) writeFileSync( settingsPath, - JSON.stringify({ statusLine: { type: 'command', command: '/usr/local/bin/my-statusline' } }) + JSON.stringify({ + statusLine: { + type: 'command', + command: '/usr/local/bin/my-statusline' + } + }) ) expect(new ClaudeHookService().install().state).toBe('installed') @@ -339,7 +366,7 @@ describe('ClaudeHookService.install', () => { }) it.skipIf(process.platform !== 'win32')( - 'runs portable managed hooks through headless exec form', + 'runs portable managed hooks through a single headless command string', () => { const tmpHome = mkdtempSync(join(tmpdir(), 'orca claude home with spaces ')) vi.stubEnv('HOME', tmpHome) @@ -351,24 +378,20 @@ describe('ClaudeHookService.install', () => { readFileSync(join(tmpHome, '.claude', 'settings.json'), 'utf-8') ) as { hooks: Record } - const system32 = join(process.env.SystemRoot ?? 'C:\\Windows', 'System32') const scriptPath = join(tmpHome, '.orca', 'agent-hooks', CLAUDE_SCRIPT_FILE_NAME) - const runtimeScriptPath = join( - '%USERPROFILE%', - '.orca', - 'agent-hooks', - CLAUDE_SCRIPT_FILE_NAME - ) for (const eventName of ['UserPromptSubmit', 'Stop', 'StopFailure']) { const hook = settings.hooks[eventName]?.[0]?.hooks?.[0] - expect(hook).toEqual({ - type: 'command', - command: join(system32, 'conhost.exe'), - args: ['--headless', join(system32, 'cmd.exe'), '/d', '/c', runtimeScriptPath], - timeout: 10 - }) - expect(hook.args).not.toContain(scriptPath) + expect(hook?.args).toBeUndefined() + expect(hook?.command).toMatch( + /\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden / + ) + expect(hook?.command).not.toContain(scriptPath) + + const encoded = hook?.command.match(/-EncodedCommand (\S+)$/)?.[1] + const decoded = Buffer.from(encoded ?? '', 'base64').toString('utf16le') + expect(decoded).toContain('$env:USERPROFILE') + expect(decoded).toContain(`.orca\\agent-hooks\\${CLAUDE_SCRIPT_FILE_NAME}`) } } finally { vi.unstubAllEnvs() @@ -393,6 +416,8 @@ describe('ClaudeHookService.install', () => { expect(script).toContain('--data-urlencode "payload@-"') expect(script).toContain('/hook/claude') expect(script).not.toMatch(/Invoke-WebRequest/i) + // Why: guard and Devin-skip paths must still return neutral JSON (#14818). + expect(script.split('\r\n')[2]).toBe('echo {}') } finally { vi.unstubAllEnvs() rmSync(tmpHome, { recursive: true, force: true }) @@ -436,6 +461,10 @@ describe('ClaudeHookService.installRemote', () => { const script = fs.files.get('/home/dev/.orca/agent-hooks/claude-hook.sh') expect(script).toContain('#!/bin/sh') expect(script).toContain('DEVIN_PROJECT_DIR') + // Why: remote guard paths must still return neutral JSON (#14818). + expect(script!.indexOf('printf "{}\\n"')).toBe( + script!.indexOf('#!/bin/sh') + '#!/bin/sh\n'.length + ) // Why: payload is piped to curl via stdin (`payload@-`) so it never lands // on the curl command line (EDR oversized-command-line false positive), // matching the Windows curl.exe hook post. diff --git a/src/main/claude/hook-service.ts b/src/main/claude/hook-service.ts index 47269b1e17d..57270575c5c 100644 --- a/src/main/claude/hook-service.ts +++ b/src/main/claude/hook-service.ts @@ -65,6 +65,8 @@ function getManagedScript( return [ '@echo off', 'setlocal', + // Why: Claude-compatible permission hooks fail closed on empty stdout (#14818). + 'echo {}', // Why: refresh endpoint coordinates for PTYs surviving an Orca restart. 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', // Why (#11549): the env guards must outrank the Devin skip — the Devin skip parks in more.com, @@ -86,6 +88,8 @@ function getManagedScript( return [ '#!/bin/sh', + // Why: Claude-compatible permission hooks fail closed on empty stdout (#14818). + 'printf "{}\\n"', ...buildPosixHookPayloadCapture(), ...(options.skipWhenDevinImportsClaude ? [ diff --git a/src/main/claude/hook-settings.ts b/src/main/claude/hook-settings.ts index d39b8e27bcb..3447c62ffa5 100644 --- a/src/main/claude/hook-settings.ts +++ b/src/main/claude/hook-settings.ts @@ -6,7 +6,9 @@ import { getSharedManagedScriptPath, isPlainObject, MANAGED_HOOK_TIMEOUT_SECONDS, + quotePowerShellString, removeManagedCommands, + wrapWindowsPowerShellEncodedCommand, type HookCommandConfig, type HookDefinition, type HooksConfig @@ -16,38 +18,59 @@ import { wrapRuntimeHomeHookCommand } from '../agent-hooks/runtime-home-hook-com export type ClaudeCompatibleHookSettings = { configDirName: '.claude' | '.openclaude' scriptBaseName: 'claude-hook' | 'openclaude-hook' - supportsExecHookArgs: boolean + usesWindowsPowerShellLauncher: boolean } export const CLAUDE_HOOK_SETTINGS: ClaudeCompatibleHookSettings = { configDirName: '.claude', scriptBaseName: 'claude-hook', - supportsExecHookArgs: true + usesWindowsPowerShellLauncher: true } export const OPENCLAUDE_HOOK_SETTINGS: ClaudeCompatibleHookSettings = { configDirName: '.openclaude', scriptBaseName: 'openclaude-hook', - supportsExecHookArgs: false + usesWindowsPowerShellLauncher: false } export const CLAUDE_EVENTS = [ // Why: SessionStart is the only event a resumed/idle session emits before the // first prompt; without it the sidebar row can't exist until the user types (STA-3386). - { eventName: 'SessionStart', definition: { hooks: [{ type: 'command', command: '' }] } }, - { eventName: 'UserPromptSubmit', definition: { hooks: [{ type: 'command', command: '' }] } }, - { eventName: 'Stop', definition: { hooks: [{ type: 'command', command: '' }] } }, + { + eventName: 'SessionStart', + definition: { hooks: [{ type: 'command', command: '' }] } + }, + { + eventName: 'UserPromptSubmit', + definition: { hooks: [{ type: 'command', command: '' }] } + }, + { + eventName: 'Stop', + definition: { hooks: [{ type: 'command', command: '' }] } + }, // Why: OpenClaude skips normal Stop hooks after API/model errors and emits // StopFailure instead; without this hook Orca leaves the turn spinning. - { eventName: 'StopFailure', definition: { hooks: [{ type: 'command', command: '' }] } }, + { + eventName: 'StopFailure', + definition: { hooks: [{ type: 'command', command: '' }] } + }, // Why: subagent/teammate lifecycle feeds the sidebar's child rows and keeps // a pane 'working' while background children outlive the lead's turn. // TeammateIdle parks turn-based teammates without trusting their permanently // "running" background_tasks entry to gate the pane. // Older Claude builds ignore unregistered event names (StopFailure precedent). - { eventName: 'SubagentStart', definition: { hooks: [{ type: 'command', command: '' }] } }, - { eventName: 'SubagentStop', definition: { hooks: [{ type: 'command', command: '' }] } }, - { eventName: 'TeammateIdle', definition: { hooks: [{ type: 'command', command: '' }] } }, + { + eventName: 'SubagentStart', + definition: { hooks: [{ type: 'command', command: '' }] } + }, + { + eventName: 'SubagentStop', + definition: { hooks: [{ type: 'command', command: '' }] } + }, + { + eventName: 'TeammateIdle', + definition: { hooks: [{ type: 'command', command: '' }] } + }, // Why: PreToolUse gives the dashboard a live readout of the in-flight tool // (name + input preview) before it completes. { @@ -108,11 +131,15 @@ export function getRemoteConfigPath(remoteHome: string, settings = CLAUDE_HOOK_S return `${remoteHome.replace(/\/$/, '')}/${settings.configDirName}/settings.json` } -export function getManagedCommand(scriptPath: string): string { +export function getManagedCommand( + scriptPath: string, + options: { neutralJsonWhenMissing?: boolean } = {} +): string { const scriptFileName = basename(scriptPath) const extension = extname(scriptFileName) return wrapRuntimeHomeHookCommand( - extension ? scriptFileName.slice(0, -extension.length) : scriptFileName + extension ? scriptFileName.slice(0, -extension.length) : scriptFileName, + options ) } @@ -120,25 +147,25 @@ export function getManagedLifecycleHook( scriptPath: string, settings = CLAUDE_HOOK_SETTINGS ): HookCommandConfig { - if (process.platform !== 'win32' || !settings.supportsExecHookArgs) { - return buildManagedCommandHook(getManagedCommand(scriptPath)) + if (process.platform !== 'win32' || !settings.usesWindowsPowerShellLauncher) { + return buildManagedCommandHook(getManagedCommand(scriptPath, { neutralJsonWhenMissing: true })) } return getWindowsManagedLifecycleHook(scriptPath) } +// Why: some Claude-compatible consumers ignore `args`, so the invocation must be self-contained. export function getWindowsManagedLifecycleHook(scriptPath: string): HookCommandConfig { - const system32 = win32.join(process.env.SystemRoot ?? 'C:\\Windows', 'System32') - const runtimeScriptPath = win32.join( - '%USERPROFILE%', - '.orca', - 'agent-hooks', - win32.basename(scriptPath) - ) - // Why: Claude's Windows shell form opens Git Bash consoles; exec form hosts the client in a windowless console. + const scriptFileName = win32.basename(scriptPath) + // Why: runtime profile resolution keeps the managed entry portable across users (STA-3348). + const quotedRelativePath = quotePowerShellString(`.orca\\agent-hooks\\${scriptFileName}`) + // Why: compat consumers require neutral JSON even when the managed script is missing (#14818). + const innerCommand = + `$scriptPath = Join-Path $env:USERPROFILE ${quotedRelativePath}; ` + + 'if (Test-Path -LiteralPath $scriptPath -PathType Leaf) { & $scriptPath; exit $LASTEXITCODE }; ' + + "[Console]::In.ReadToEnd() | Out-Null; Write-Output '{}'; exit 0" return { type: 'command', - command: win32.join(system32, 'conhost.exe'), - args: ['--headless', win32.join(system32, 'cmd.exe'), '/d', '/c', runtimeScriptPath], + command: wrapWindowsPowerShellEncodedCommand(innerCommand), timeout: MANAGED_HOOK_TIMEOUT_SECONDS } } @@ -154,7 +181,7 @@ export function hasSameManagedHookInvocation( } export function getRemoteManagedCommand(scriptPath: string): string { - return getManagedCommand(scriptPath) + return getManagedCommand(scriptPath, { neutralJsonWhenMissing: true }) } export function applyManagedHooks( diff --git a/src/main/codex/hook-service-managed-install.test.ts b/src/main/codex/hook-service-managed-install.test.ts index 4f53c51513f..22f6bf698c5 100644 --- a/src/main/codex/hook-service-managed-install.test.ts +++ b/src/main/codex/hook-service-managed-install.test.ts @@ -30,7 +30,7 @@ vi.mock('os', async (importOriginal) => { import { CodexHookService } from './hook-service' const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ const homes = setupCodexHookHomes(homedirMock, getPathMock) @@ -235,24 +235,31 @@ describe('CodexHookService', () => { // would block the event loop and starve this handler, so the child is // spawned asynchronously while the server drains the request concurrently. let resolveReceived: (value: { headers: Record; body: string }) => void - const receivedPromise = new Promise<{ headers: Record; body: string }>( - (resolve) => { - resolveReceived = resolve - } - ) + const receivedPromise = new Promise<{ + headers: Record + body: string + }>((resolve) => { + resolveReceived = resolve + }) const server = createServer((req, res) => { const chunks: Buffer[] = [] req.on('data', (c: Buffer) => chunks.push(c)) req.on('end', () => { res.end('ok') - resolveReceived({ headers: req.headers, body: Buffer.concat(chunks).toString('utf-8') }) + resolveReceived({ + headers: req.headers, + body: Buffer.concat(chunks).toString('utf-8') + }) }) }) await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) const port = (server.address() as AddressInfo).port try { - const payload = JSON.stringify({ prompt: '你好世界', hook_event_name: 'UserPromptSubmit' }) + const payload = JSON.stringify({ + prompt: '你好世界', + hook_event_name: 'UserPromptSubmit' + }) // Why: this suite may run inside an Orca-launched terminal whose env // already carries ORCA_AGENT_HOOK_ENDPOINT/PORT/TOKEN. The managed // script sources that endpoint file, so leave it out or the hook posts diff --git a/src/main/command-code/hook-service.test.ts b/src/main/command-code/hook-service.test.ts index 6e294516324..ce59e20b056 100644 --- a/src/main/command-code/hook-service.test.ts +++ b/src/main/command-code/hook-service.test.ts @@ -21,7 +21,7 @@ vi.mock('os', async () => { import { CommandCodeHookService } from './hook-service' const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ describe('CommandCodeHookService', () => { let homeDir: string @@ -136,7 +136,10 @@ describe('CommandCodeHookService', () => { body += chunk }) req.on('end', () => { - requests.push({ body, token: req.headers['x-orca-agent-hook-token'] }) + requests.push({ + body, + token: req.headers['x-orca-agent-hook-token'] + }) res.statusCode = 204 res.end() }) diff --git a/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts b/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts index 7f47e794252..72c897088f5 100644 --- a/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts +++ b/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts @@ -17,6 +17,9 @@ import { import { ProcessGoneDedupe } from './process-gone-dedupe' import { recordProcessGoneCrash, type ProcessGoneCrashEvent } from './process-gone-recorder' +const noMinidump = async () => null +const attachDetails = async () => null + function event(overrides: Partial = {}): ProcessGoneCrashEvent { return { source: 'renderer', @@ -82,9 +85,32 @@ describe('recordProcessGoneCrash killed/1 ordering', () => { recordProcessGoneCrash({ record } as never, gpuKill, dedupe) recordProcessGoneCrash({ record } as never, networkServiceKill, dedupe) - recordProcessGoneCrash({ record } as never, rendererKill, dedupe) + recordProcessGoneCrash( + { record, attachDetails } as never, + rendererKill, + dedupe, + noMinidump + ) expect(record).toHaveBeenCalledOnce() + // Timing proximity is evidence, not authority to discard an ambiguous report. + expect(record).toHaveBeenCalledWith( + expect.objectContaining({ + breadcrumbs: expect.arrayContaining([ + expect.objectContaining({ + name: 'process_gone_suppressed', + data: expect.objectContaining({ source: 'child', processType: 'GPU' }) + }), + expect.objectContaining({ + name: 'process_gone_suppressed', + data: expect.objectContaining({ + source: 'child', + serviceName: 'network.mojom.NetworkService' + }) + }) + ]) + }) + ) }) it('suppresses the fleet sequence only after independent session-end intent', () => { diff --git a/src/main/crash-reporting/process-gone-recorder.test.ts b/src/main/crash-reporting/process-gone-recorder.test.ts index 8a0f77dd1e2..70e6ca93f04 100644 --- a/src/main/crash-reporting/process-gone-recorder.test.ts +++ b/src/main/crash-reporting/process-gone-recorder.test.ts @@ -436,10 +436,9 @@ describe('recordProcessGoneCrash', () => { } } - // Why child kills: the decode gate is source-agnostic, and a non-recoverable - // child persists synchronously on every branch of the crash-reporting stack - // (the renderer killed path defers behind a sibling-kill settle), so the - // platform stub is still in force when the gate reads process.platform. + // Why child kills: the decode gate is source-agnostic and reads + // process.platform synchronously at record time, so the platform stub is + // still in force when it runs. const nonRecoverableChildKill = (overrides: Partial): ProcessGoneCrashEvent => event({ source: 'child', diff --git a/src/main/cursor/hook-service.test.ts b/src/main/cursor/hook-service.test.ts index f861d4f2a66..b723d947db3 100644 --- a/src/main/cursor/hook-service.test.ts +++ b/src/main/cursor/hook-service.test.ts @@ -31,7 +31,7 @@ const CURSOR_EVENTS = [ const CURSOR_SCRIPT_FILE_NAME = process.platform === 'win32' ? 'cursor-hook.cmd' : 'cursor-hook.sh' const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ describe('CursorHookService', () => { let homeDir: string diff --git a/src/main/droid/hook-service.test.ts b/src/main/droid/hook-service.test.ts index ef31754a7e7..0a65facab21 100644 --- a/src/main/droid/hook-service.test.ts +++ b/src/main/droid/hook-service.test.ts @@ -25,7 +25,7 @@ vi.mock('os', async () => { import { DroidHookService } from './hook-service' const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ describe('DroidHookService', () => { let homeDir: string diff --git a/src/main/gemini/hook-service.test.ts b/src/main/gemini/hook-service.test.ts index 1a80c1568bf..fc5a302a7d1 100644 --- a/src/main/gemini/hook-service.test.ts +++ b/src/main/gemini/hook-service.test.ts @@ -26,7 +26,7 @@ vi.mock('os', async (importOriginal) => { import { GeminiHookService } from './hook-service' const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ describe('GeminiHookService', () => { let homeDir: string diff --git a/src/main/ghostty/mapper-extended.test.ts b/src/main/ghostty/mapper-extended.test.ts index 402e9253f21..189e5bf4217 100644 --- a/src/main/ghostty/mapper-extended.test.ts +++ b/src/main/ghostty/mapper-extended.test.ts @@ -90,9 +90,9 @@ describe('mapGhosttyToOrca — window-padding', () => { expect(result.unsupportedKeys).toEqual([]) }) - it('preserves odd-sum dual-value padding as a fractional average', () => { + it('rounds odd-sum dual-value padding to the nearest fitted pixel', () => { const result = mapGhosttyToOrca({ 'window-padding-x': '1,2' }) - expect(result.diff).toEqual({ terminalPaddingX: 1.5 }) + expect(result.diff).toEqual({ terminalPaddingX: 2 }) expect(result.unsupportedKeys).toEqual([]) }) diff --git a/src/main/ghostty/numeric-config-values.ts b/src/main/ghostty/numeric-config-values.ts index f8a7ce7b468..5ced69afa48 100644 --- a/src/main/ghostty/numeric-config-values.ts +++ b/src/main/ghostty/numeric-config-values.ts @@ -1,3 +1,5 @@ +import { normalizeTerminalPadding } from '../../shared/terminal-padding-settings' + // Why: `Number("1e10")` succeeds and passes `Number.isInteger`, so a Ghostty // config with `window-padding-x = 1e9` would sail through the mapper and land // an absurd value in the store. Restrict to plain decimal integers. @@ -11,8 +13,7 @@ export const parseStrictInt = (v: string): number | null => { } // Why: Ghostty accepts "top,bottom" / "left,right" pairs for window paddings, -// but Orca stores a single value per axis — average the pair so the total -// padding along the axis stays the same. +// but Orca stores an integer per axis — preserve the nearest total inset. export const parsePaddingValue = (v: string): number | null => { const parts = v.split(',') if (parts.length > 2) { @@ -26,5 +27,5 @@ export const parsePaddingValue = (v: string): number | null => { } nums.push(num) } - return nums.reduce((sum, num) => sum + num, 0) / nums.length + return normalizeTerminalPadding(nums.reduce((sum, num) => sum + num, 0) / nums.length) } diff --git a/src/main/grok/hook-service.test.ts b/src/main/grok/hook-service.test.ts index ba469506316..f08779aed02 100644 --- a/src/main/grok/hook-service.test.ts +++ b/src/main/grok/hook-service.test.ts @@ -23,7 +23,7 @@ import { POSIX_HOOK_STDIN_READER } from '../agent-hooks/hook-stdin-contract' const GROK_SCRIPT_FILE_NAME = process.platform === 'win32' ? 'grok-hook.cmd' : 'grok-hook.sh' const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ type WindowsGrokHookRun = { status: number | null @@ -118,7 +118,7 @@ describe('GrokHookService', () => { expect(script).toContain('set "ORCA_GROK_HOME="') expect(script).toContain('if not defined GROK_HOME goto :orca_grok_home_ready') expect(script).toContain('%GROK_HOME:~4096,1%') - expect(script).toContain('set "ORCA_GROK_HOME=%GROK_HOME%"') + expect(script).toContain('set "ORCA_GROK_HOME=%GROK_HOME:"=%"') expect(script).toContain('%ORCA_GROK_HOME:~4096,1%') expect(script).toContain(':orca_grok_home_ready') expect(script).toContain('if not defined ORCA_GROK_HOME goto :orca_grok_home_ready') @@ -269,7 +269,7 @@ describe('GrokHookService', () => { expect(script).toContain('%SystemRoot%\\System32\\curl.exe') // Why: windows-grok-hook-script.test.ts pins the GROK_HOME guard shape itself, // and does so on every platform rather than only on Windows runners. - expect(script).toContain('set "ORCA_GROK_HOME=%GROK_HOME%"') + expect(script).toContain('set "ORCA_GROK_HOME=%GROK_HOME:"=%"') expect(script).toContain('--data-urlencode "grokHome=%ORCA_GROK_HOME%"') } else { // Why: payload is piped to curl via stdin (`payload@-`) so it never lands @@ -343,7 +343,11 @@ describe('GrokHookService', () => { `${JSON.stringify( { hooks: { - Notification: [{ hooks: [{ type: 'command', command: '/usr/local/bin/user-hook' }] }] + Notification: [ + { + hooks: [{ type: 'command', command: '/usr/local/bin/user-hook' }] + } + ] } }, null, diff --git a/src/main/grok/windows-grok-hook-script.test.ts b/src/main/grok/windows-grok-hook-script.test.ts index a8aea9065f0..98da524e7c7 100644 --- a/src/main/grok/windows-grok-hook-script.test.ts +++ b/src/main/grok/windows-grok-hook-script.test.ts @@ -26,8 +26,9 @@ function findUnguardedSubstringReads(lines: readonly string[]): string[] { defined.add(guarded[1]) } // A value is provably non-empty only if something outside an expansion survives: - // `set "V=%OTHER%"` may undefine V, `set "V=%V%."` cannot. - for (const [, name, value] of line.matchAll(/set "(\w+)=([^"]*)"/g)) { + // `set "V=%OTHER%"` may undefine V, `set "V=%V%."` cannot. The lazy value stops at + // the quote that closes the `set`, so a substitution like `%V:"=%` stays one token. + for (const [, name, value] of line.matchAll(/set "(\w+)=(.*?)"(?=\s|$)/g)) { if (value.replaceAll(/%[^%]*%/g, '') === '') { defined.delete(name) } else { @@ -93,6 +94,7 @@ describe.skipIf(process.platform !== 'win32')('buildWindowsGrokHookScript (win32 grokHome: string | null paneKey: string | null worktreeId: string | null + payload: string | null } const PANE_KEY = 'tab!1:pane!2' @@ -149,7 +151,8 @@ describe.skipIf(process.platform !== 'win32')('buildWindowsGrokHookScript (win32 stderr, grokHome: posted?.get('grokHome') ?? null, paneKey: posted?.get('paneKey') ?? null, - worktreeId: posted?.get('worktreeId') ?? null + worktreeId: posted?.get('worktreeId') ?? null, + payload: posted?.get('payload') ?? null }) ) }) @@ -188,6 +191,36 @@ describe.skipIf(process.platform !== 'win32')('buildWindowsGrokHookScript (win32 expect(result.grokHome).toBe('C:\\Users\\test\\.grok\\.') }) + // Why (#14221): `setx GROK_HOME "C:\path\"` stores `C:\path"` — the CRT turns the + // `\"` into a literal quote. That quote unbalanced the trailing-backslash `if`, so + // cmd aborted the whole script before curl and every Grok hook event failed. + it('exits 0 and strips a trailing quote from GROK_HOME', async () => { + const result = await runHook('C:\\Users\\test\\.grok"') + + expect(result.stderr).not.toContain('unexpected at this time') + expect(result.status).toBe(0) + expect(result.grokHome).toBe('C:\\Users\\test\\.grok') + expect(result.payload).toBe('{"session_id":"s"}') + }) + + // Why: an embedded quote closed curl's `grokHome=` argument early, which swallowed + // the `^` continuation and dropped the `payload@-` line — a silent, exit-0 failure. + it('strips an embedded quote without truncating the curl arguments', async () => { + const result = await runHook('C:\\Users\\a"b\\.grok') + + expect(result.status).toBe(0) + expect(result.grokHome).toBe('C:\\Users\\ab\\.grok') + expect(result.payload).toBe('{"session_id":"s"}') + }) + + it('posts an empty grokHome when GROK_HOME is only a quote', async () => { + const result = await runHook('"') + + expect(result.status).toBe(0) + expect(result.grokHome).toBe('') + expect(result.payload).toBe('{"session_id":"s"}') + }) + it('drops a GROK_HOME past the envelope limit', async () => { const result = await runHook(`C:\\${'a'.repeat(4094)}`) diff --git a/src/main/grok/windows-grok-hook-script.ts b/src/main/grok/windows-grok-hook-script.ts index 5a6e81ee633..e68c8914eeb 100644 --- a/src/main/grok/windows-grok-hook-script.ts +++ b/src/main/grok/windows-grok-hook-script.ts @@ -21,6 +21,7 @@ const WINDOWS_GROK_HOOK_POST_COMMAND = buildWindowsAgentHookPostCommand('grok', * * - Guard substring ops behind `if defined` + goto (not a parenthesized block). * - Reject oversized values before copying them onto a cmd input line. + * - Strip `"` before the value reaches an `if` operand or the curl argument. */ export function buildWindowsGrokHookScript(): string { return [ @@ -35,7 +36,10 @@ export function buildWindowsGrokHookScript(): string { 'set "ORCA_GROK_HOME="', 'if not defined GROK_HOME goto :orca_grok_home_ready', `if not "%GROK_HOME:~${GROK_HOME_ENVELOPE_MAX_LENGTH},1%"=="" goto :orca_grok_home_ready`, - 'set "ORCA_GROK_HOME=%GROK_HOME%"', + // Why (#14221): `setx GROK_HOME "C:\path\"` stores a literal trailing quote, which + // unbalances the trailing-backslash `if` below (exit 255) and truncates the curl + // argument. `"` is illegal in a Windows path, so strip it rather than preserve it. + 'set "ORCA_GROK_HOME=%GROK_HOME:"=%"', 'if not defined ORCA_GROK_HOME goto :orca_grok_home_ready', 'if "%ORCA_GROK_HOME:~-1%"=="\\" set "ORCA_GROK_HOME=%ORCA_GROK_HOME%."', // Why: the trailing-backslash safety sentinel counts toward the relay envelope. diff --git a/src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts b/src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts index 7981366cadb..11b9fc97afd 100644 --- a/src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts +++ b/src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts @@ -130,6 +130,23 @@ describe('headless hook agent-status projection (#11761)', () => { ) }) + it('publishes a gated turn end as event metadata, not stored agent status', async () => { + const turnCompletedAt = Date.now() + const runtime = await createRuntimeWithHookRows([ + hookRow({ + state: 'working', + interactivePrompt: undefined, + turnCompletedAt + }) + ]) + + const result = await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`) + const tab = result.tabs[0] + + expect(tab).toMatchObject({ type: 'terminal', turnCompletedAt }) + expect(tab?.type === 'terminal' && tab.agentStatus).not.toHaveProperty('turnCompletedAt') + }) + it('publishes no hook transport identity to clients', async () => { const agentStatus = await projectAgentStatus([ hookRow({ launchToken: 'lt-secret', promptInteractionKey: 'turn-1' }) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 69b7c342438..179a05e2f2b 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -11806,7 +11806,9 @@ export class OrcaRuntimeService { (previous.payload.agentType ?? null) !== (payload.agentType ?? null) || (previous.payload.toolName ?? null) !== (payload.toolName ?? null) || (previous.payload.interactivePrompt ?? null) !== (payload.interactivePrompt ?? null) || - (previous.payload.interrupted ?? false) !== (payload.interrupted ?? false) + (previous.payload.interrupted ?? false) !== (payload.interrupted ?? false) || + (previous.payload.turnCompletedAt ?? null) !== (payload.turnCompletedAt ?? null) || + (previous.payload.lastAssistantMessage ?? null) !== (payload.lastAssistantMessage ?? null) ) } @@ -32428,6 +32430,31 @@ export class OrcaRuntimeService { : livePty ? this.issuePtyHandle(livePty) : null + const projectedAgentStatus = + agentStatus ?? + this.buildPtyMobileAgentStatus( + mobileStatusPty, + tab, + terminalHandle, + retainedAgentStatus, + getHookRowsForPane + ) + const projectedStatusEntry = projectedAgentStatus.agentStatus as + | (AgentStatusEntry & { turnCompletedAt?: number }) + | undefined + const { turnCompletedAt: projectedTurnCompletedAt, ...clientStatusFields } = + projectedStatusEntry ?? {} + const clientAgentStatus = projectedStatusEntry + ? { agentStatus: clientStatusFields as AgentStatusEntry } + : {} + const rawTurnCompletedAt = + hookAgentStatus?.live?.payload.turnCompletedAt ?? + this.getHookAgentRowForPane(getHookRowsForPane(paneKey)).live?.payload.turnCompletedAt ?? + projectedTurnCompletedAt + const turnCompletedAt = + typeof rawTurnCompletedAt === 'number' && Number.isFinite(rawTurnCompletedAt) + ? rawTurnCompletedAt + : undefined tabs.push({ type: 'terminal', id: tab.id, @@ -32437,14 +32464,8 @@ export class OrcaRuntimeService { ...(tab.ptyId ? { ptyId: tab.ptyId } : {}), ...(tab.terminalTheme ? { terminalTheme: tab.terminalTheme } : {}), ...(launchAgent ? { launchAgent } : {}), - ...(agentStatus ?? - this.buildPtyMobileAgentStatus( - mobileStatusPty, - tab, - terminalHandle, - retainedAgentStatus, - getHookRowsForPane - )), + ...clientAgentStatus, + ...(turnCompletedAt !== undefined ? { turnCompletedAt } : {}), ...(tab.parentLayout ? { parentLayout: tab.parentLayout } : {}), ...(tab.startupCwd ? { startupCwd: tab.startupCwd } : {}), ...(tab.color != null ? { color: tab.color } : {}), diff --git a/src/renderer/src/assets/terminal-container-geometry.test.ts b/src/renderer/src/assets/terminal-container-geometry.test.ts index db079c9095f..5e2bfcc0258 100644 --- a/src/renderer/src/assets/terminal-container-geometry.test.ts +++ b/src/renderer/src/assets/terminal-container-geometry.test.ts @@ -5,9 +5,7 @@ const terminalCss = fs.readFileSync(new URL('./terminal.css', import.meta.url), describe('terminal container geometry', () => { it('keeps the hidden link tooltip out of the fitted terminal height', () => { - expect(terminalCss).toMatch( - /\.xterm-container\s*{[^}]*height:\s*calc\(100% - var\(--pane-padding-y, 4px\)\);/s - ) + expect(terminalCss).toMatch(/\.xterm-container\s*{[^}]*height:\s*100%;/s) expect(terminalCss).toMatch( /\.pane\[data-has-title\] \.xterm-container\s*{[^}]*height:\s*calc\(100% - var\(--orca-pane-title-height\)\);/s ) @@ -15,4 +13,30 @@ describe('terminal container geometry', () => { /\.pane-link-tooltip\s*{[^}]*height:\s*var\(--orca-terminal-link-tooltip-height\);/s ) }) + + it('insets the xterm grid on both axes from pane padding', () => { + expect(terminalCss).toMatch( + /\.xterm-container \.xterm\s*{[^}]*padding-top:\s*var\(--pane-padding-y, 4px\);/s + ) + expect(terminalCss).toMatch( + /\.xterm-container \.xterm\s*{[^}]*padding-right:\s*var\(--pane-padding-x, 4px\);/s + ) + expect(terminalCss).toMatch( + /\.xterm-container \.xterm\s*{[^}]*padding-bottom:\s*var\(--pane-padding-y, 4px\);/s + ) + expect(terminalCss).toMatch( + /\.xterm-container \.xterm\s*{[^}]*padding-left:\s*var\(--pane-padding-x, 4px\);/s + ) + expect(terminalCss).toMatch(/\.xterm-container\s*{[^}]*width:\s*100%;/s) + expect(terminalCss).not.toMatch(/\.xterm-container\s*{[^}]*margin-left:/s) + }) + + it('matches live DOM and WebGL background layers across translucent padding bands', () => { + expect(terminalCss).toMatch( + /\.xterm-container \.xterm::before\s*{[^}]*border-color:\s*var\(--orca-terminal-live-background, transparent\);/s + ) + expect(terminalCss).toMatch( + /\.xterm-container\[data-terminal-renderer='webgl'\] \.xterm::after\s*{[^}]*border-color:\s*var\(--orca-terminal-live-background, transparent\);/s + ) + }) }) diff --git a/src/renderer/src/assets/terminal.css b/src/renderer/src/assets/terminal.css index ee5e0af01f5..66a8a18828a 100644 --- a/src/renderer/src/assets/terminal.css +++ b/src/renderer/src/assets/terminal.css @@ -30,6 +30,42 @@ width: 100%; } +.xterm-container .xterm { + box-sizing: border-box; + height: 100%; + width: 100%; + /* Why on .xterm, not the container: FitAddon subtracts this element's + padding when proposing cols/rows, and the themed background (including + opacity) fills the inset. Container margins only framed the start edges + and stacked a darker gutter at fractional opacity. */ + padding-top: var(--pane-padding-y, 4px); + padding-right: var(--pane-padding-x, 4px); + padding-bottom: var(--pane-padding-y, 4px); + padding-left: var(--pane-padding-x, 4px); +} + +/* xterm paints the live viewport background again under the grid. Mirror that + layer in the padding bands, including runtime OSC 11 color changes. */ +.xterm-container .xterm::before, +.xterm-container[data-terminal-renderer='webgl'] .xterm::after { + box-sizing: border-box; + position: absolute; + inset: 0; + content: ''; + pointer-events: none; + border-style: solid; + border-width: var(--pane-padding-y, 4px) var(--pane-padding-x, 4px); +} + +.xterm-container .xterm::before { + border-color: var(--orca-terminal-live-background, transparent); +} + +/* WebGL adds one source-over background layer beyond the DOM renderer. */ +.xterm-container[data-terminal-renderer='webgl'] .xterm::after { + border-color: var(--orca-terminal-live-background, transparent); +} + .pane-manager-root .xterm-viewport { /* Why: xterm/VS Code keep the scrollbar gutter stable. `auto` lets Linux DOM scrollbars appear/disappear, which can feed resize/reflow jitter. */ @@ -177,7 +213,7 @@ .pane[data-terminal-attention] .xterm-container::after { content: ''; position: absolute; - inset: 0 4px 4px 0; + inset: 0; z-index: 5; pointer-events: none; border-radius: 2px; @@ -498,16 +534,14 @@ border-color: var(--ring); } -/* Baseline xterm-container layout — padding is driven by CSS custom properties - so Ghostty-imported window-padding-x/y can be applied without inline styles. - The fallback 4px preserves the legacy default. */ +/* Container fills the pane so the .xterm background (and its padding) is the + terminal surface. Title offset stays here because the title overlay is a + sibling of the container, not of .xterm. */ .xterm-container { box-sizing: border-box; position: relative; - width: calc(100% - var(--pane-padding-x, 4px)); - height: calc(100% - var(--pane-padding-y, 4px)); - margin-top: var(--pane-padding-y, 4px); - margin-left: var(--pane-padding-x, 4px); + width: 100%; + height: 100%; } /* When a pane has a title, shift the terminal content down to make room. diff --git a/src/renderer/src/components/settings/TerminalWindowSection.padding.test.tsx b/src/renderer/src/components/settings/TerminalWindowSection.padding.test.tsx new file mode 100644 index 00000000000..5f54fb38a9e --- /dev/null +++ b/src/renderer/src/components/settings/TerminalWindowSection.padding.test.tsx @@ -0,0 +1,81 @@ +// @vitest-environment happy-dom + +import { act, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { GlobalSettings } from '../../../../shared/global-settings-types' +import { getDefaultSettings } from '../../../../shared/constants' +import { TerminalWindowSection } from './TerminalWindowSection' + +type NumberFieldProps = { + label: string + value: number + onChange: (value: number) => void +} + +const numberFields = vi.hoisted(() => new Map()) + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, defaultValue: string) => defaultValue +})) + +vi.mock('./SettingsFormControls', () => ({ + ColorField: function ColorField() { + return null + }, + NumberField: function NumberField(props: NumberFieldProps) { + numberFields.set(props.label, props) + return null + } +})) + +vi.mock('./SearchableSetting', () => ({ + SearchableSetting: function SearchableSetting({ children }: { children?: ReactNode }) { + return children ?? null + } +})) + +vi.mock('../ui/button', () => ({ Button: () => null })) +vi.mock('../ui/label', () => ({ Label: () => null })) +vi.mock('../ui/switch', () => ({ Switch: () => null })) + +describe('TerminalWindowSection padding', () => { + let container: HTMLDivElement + let root: Root + + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + numberFields.clear() + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + }) + + afterEach(() => { + act(() => root.unmount()) + document.body.replaceChildren() + }) + + it('displays and persists the same integer padding used by terminal fitting', () => { + const updateSettings = vi.fn() + const settings = { + ...getDefaultSettings('/tmp'), + terminalPaddingX: 1.5, + terminalPaddingY: 2.5 + } as GlobalSettings + + act(() => + root.render() + ) + + const horizontal = numberFields.get('Horizontal Padding') + const vertical = numberFields.get('Vertical Padding') + expect(horizontal?.value).toBe(2) + expect(vertical?.value).toBe(3) + + act(() => horizontal?.onChange(3.5)) + act(() => vertical?.onChange(4.5)) + expect(updateSettings).toHaveBeenNthCalledWith(1, { terminalPaddingX: 4 }) + expect(updateSettings).toHaveBeenNthCalledWith(2, { terminalPaddingY: 5 }) + }) +}) diff --git a/src/renderer/src/components/settings/TerminalWindowSection.tsx b/src/renderer/src/components/settings/TerminalWindowSection.tsx index 863d7fa11b0..5a09073cc27 100644 --- a/src/renderer/src/components/settings/TerminalWindowSection.tsx +++ b/src/renderer/src/components/settings/TerminalWindowSection.tsx @@ -1,6 +1,7 @@ import { useRef, useState } from 'react' import { RotateCw } from 'lucide-react' import type { GlobalSettings } from '../../../../shared/global-settings-types' +import { normalizeTerminalPadding } from '../../../../shared/terminal-padding-settings' import { Button } from '../ui/button' import { Label } from '../ui/label' import { Switch } from '../ui/switch' @@ -185,13 +186,15 @@ export function TerminalWindowSection({ 'Horizontal Padding' )} description="" - value={settings.terminalPaddingX ?? 4} + value={normalizeTerminalPadding(settings.terminalPaddingX ?? 4)} defaultValue={4} min={0} max={512} step={1} suffix="px" - onChange={(value) => updateSettings({ terminalPaddingX: Math.max(0, value) })} + onChange={(value) => + updateSettings({ terminalPaddingX: normalizeTerminalPadding(value) }) + } /> @@ -212,13 +215,15 @@ export function TerminalWindowSection({ 'Vertical Padding' )} description="" - value={settings.terminalPaddingY ?? 4} + value={normalizeTerminalPadding(settings.terminalPaddingY ?? 4)} defaultValue={4} min={0} max={512} step={1} suffix="px" - onChange={(value) => updateSettings({ terminalPaddingY: Math.max(0, value) })} + onChange={(value) => + updateSettings({ terminalPaddingY: normalizeTerminalPadding(value) }) + } /> diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-dispose-leak.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-dispose-leak.test.ts index 896ecc88548..ad1aab0ddcf 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-dispose-leak.test.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-dispose-leak.test.ts @@ -101,4 +101,76 @@ describe('agent completion coordinator identity map stays bounded (leak regressi coordinator.dispose() // isLive() still true -> remount, not teardown expect(getAgentCompletionCoordinatorIdentityCountForTest()).toBe(1) }) + + it('clears per-coordinator working boundaries on reset and dispose', () => { + const live = { value: true } + const coordinator = createAgentCompletionCoordinator( + makeOptions('tab-1:leaf-working-boundary', live) + ) + const working = { + state: 'working' as const, + prompt: '', + agentType: 'claude' as const, + stateStartedAt: 1_000 + } + + coordinator.observeHookStatus(working) + expect(getAgentCompletionCoordinatorIdentityCountForTest()).toBe(1) + coordinator.resetCompletionState() + expect(getAgentCompletionCoordinatorIdentityCountForTest()).toBe(0) + + coordinator.observeHookStatus(working) + expect(getAgentCompletionCoordinatorIdentityCountForTest()).toBe(1) + coordinator.dispose() + expect(getAgentCompletionCoordinatorIdentityCountForTest()).toBe(0) + }) + + it('retains stamped replay state when a mounted sibling disposes before remount', () => { + const paneKey = 'tab-1:leaf-production-remount' + const dispatchCompletion = vi.fn() + const hookLive = { value: true } + const paneLive = { value: true } + const hookCoordinator = createAgentCompletionCoordinator({ + ...makeOptions(paneKey, hookLive), + dispatchCompletion + }) + const paneCoordinator = createAgentCompletionCoordinator({ + ...makeOptions(paneKey, paneLive), + dispatchCompletion + }) + + hookCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_000 + }) + hookCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_000, + turnCompletedAt: 2_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + paneLive.value = false + paneCoordinator.dispose() + const remountedLive = { value: true } + const remounted = createAgentCompletionCoordinator({ + ...makeOptions(paneKey, remountedLive), + dispatchCompletion + }) + vi.advanceTimersByTime(2_000) + remounted.observeTitleWorking() + remounted.observeClassifiedTitleCompletion('Claude done') + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + hookLive.value = false + remountedLive.value = false + hookCoordinator.dispose() + remounted.dispose() + expect(getAgentCompletionCoordinatorIdentityCountForTest()).toBe(0) + }) }) diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-hook-title-precedence.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-hook-title-precedence.test.ts index 130d9e98edd..f28a3579a66 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-hook-title-precedence.test.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-hook-title-precedence.test.ts @@ -174,6 +174,17 @@ describe('agent completion coordinator', () => { await flushAsyncTicks() expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + result = processResult('codex') + vi.advanceTimersByTime(2_000) + await flushAsyncTicks() + result = processResult('zsh', false) + vi.advanceTimersByTime(750) + await flushAsyncTicks() + vi.advanceTimersByTime(750) + await flushAsyncTicks() + + expect(dispatchCompletion).toHaveBeenCalledTimes(2) }) it('keeps duplicate done-only hooks inside replay guard suppressed after process inspection', async () => { diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-boundary.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-boundary.test.ts new file mode 100644 index 00000000000..11d3cd0aac7 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-boundary.test.ts @@ -0,0 +1,409 @@ +import { describe, expect, it, vi } from 'vitest' +import { createAgentCompletionCoordinator } from './agent-completion-coordinator' +import { + HOOK_DONE_QUIET_MS, + useAgentCompletionCoordinatorLifecycle +} from './agent-completion-coordinator-test-harness' + +describe('agent completion coordinator stamped turn boundary', () => { + useAgentCompletionCoordinatorLifecycle() + + it('ignores a turn end time that cannot name a turn', () => { + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: Number.NaN + }) + + expect(dispatchCompletion).not.toHaveBeenCalled() + }) + + it('does not announce a gated Stop for a turn it never saw start', () => { + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + + expect(dispatchCompletion).not.toHaveBeenCalled() + }) + + it('does not announce the all-clear after first seeing a stamped turn tail', () => { + const dispatchCompletion = vi.fn() + const firstCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + firstCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + firstCoordinator.dispose() + + const remounted = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + remounted.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_055_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).not.toHaveBeenCalled() + }) + + it('announces a gated Stop immediately and treats the later all-clear as the same turn', () => { + const dispatchCompletion = vi.fn() + const dispatchHookLifecycle = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + dispatchHookLifecycle, + isLive: () => true + }) + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + lastAssistantMessage: 'Which cells need hand-verification?', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + expect(dispatchCompletion).toHaveBeenCalledWith( + 'claude', + expect.objectContaining({ + source: 'hook', + quietedHookDone: false, + agentStatus: expect.objectContaining({ + state: 'done', + lastAssistantMessage: 'Which cells need hand-verification?', + stateStartedAt: 1_700_000_005_000, + turnCompletedAt: 1_700_000_005_000 + }) + }) + ) + expect(dispatchHookLifecycle).toHaveBeenLastCalledWith( + expect.objectContaining({ state: 'working', turnCompletedAt: 1_700_000_005_000 }) + ) + + coordinator.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + lastAssistantMessage: 'Which cells need hand-verification?', + stateStartedAt: 1_700_000_055_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + expect(dispatchHookLifecycle).toHaveBeenLastCalledWith( + expect.objectContaining({ state: 'done', turnCompletedAt: 1_700_000_005_000 }) + ) + }) + + it.each(['before', 'after'] as const)( + 'does not let a sibling OSC all-clear arriving %s the host stamp duplicate it', + (allClearOrder) => { + const dispatchCompletion = vi.fn() + const localCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const hostCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 5_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 2_000 + }) + if (allClearOrder === 'before') { + localCoordinator.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 5_500 + }) + } + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + lastAssistantMessage: 'Review complete.', + stateStartedAt: 2_000, + turnCompletedAt: 2_500 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + if (allClearOrder === 'after') { + localCoordinator.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 5_500 + }) + } + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + } + ) + + it('does not consume a fresh turn as the all-clear for an earlier stamped tail', () => { + const dispatchCompletion = vi.fn() + const localCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const hostCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000, + turnCompletedAt: 2_500 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'second turn', + agentType: 'claude', + stateStartedAt: 6_000 + }) + localCoordinator.observeHookStatus({ + state: 'done', + prompt: 'second turn', + agentType: 'claude', + stateStartedAt: 6_500 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(2) + }) + + it('keeps sibling tail suppression after the host stamped all-clear arrives first', () => { + const dispatchCompletion = vi.fn() + const localCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const hostCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000, + turnCompletedAt: 2_500 + }) + hostCoordinator.observeHookStatus({ + state: 'done', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 3_000, + turnCompletedAt: 2_500 + }) + localCoordinator.observeHookStatus({ + state: 'done', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_500 + }) + localCoordinator.observeHookStatus({ + state: 'done', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_500 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + }) + + it('releases a stamped tail when its host reports a new same-state turn', () => { + const dispatchCompletion = vi.fn() + const localCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const hostCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000, + turnCompletedAt: 2_500 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'second turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'second turn', + agentType: 'claude', + stateStartedAt: 5_000 + }) + localCoordinator.observeHookStatus({ + state: 'done', + prompt: 'second turn', + agentType: 'claude', + stateStartedAt: 5_500 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-replay.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-replay.test.ts new file mode 100644 index 00000000000..8e4fe3b45df --- /dev/null +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-replay.test.ts @@ -0,0 +1,388 @@ +import { describe, expect, it, vi } from 'vitest' +import { createAgentCompletionCoordinator } from './agent-completion-coordinator' +import { + HOOK_DONE_QUIET_MS, + useAgentCompletionCoordinatorLifecycle +} from './agent-completion-coordinator-test-harness' + +describe('agent completion coordinator stamped turn replay', () => { + useAgentCompletionCoordinatorLifecycle() + + it.each(['pty-first', 'host-first'] as const)( + 'releases stamped fallback dedupe when the next turn arrives %s', + (workingOrder) => { + const dispatchCompletion = vi.fn() + const localCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + statusLane: 'pty', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const hostCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + statusLane: 'hook', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const observeHostNextTurn = () => + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'second turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000, + turnCompletedAt: 2_500 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + if (workingOrder === 'pty-first') { + localCoordinator.observeTitleWorking() + observeHostNextTurn() + } else { + observeHostNextTurn() + localCoordinator.observeTitleWorking() + } + localCoordinator.observeClassifiedTitleCompletion('Claude done') + + expect(dispatchCompletion).toHaveBeenCalledTimes(2) + } + ) + + it.each(['before', 'after'] as const)( + 'keeps sibling tail replay suppression when the pane remounts %s its all-clear', + (remountOrder) => { + const dispatchCompletion = vi.fn() + const createLocalCoordinator = () => + createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + statusLane: 'pty', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const localCoordinator = createLocalCoordinator() + const hostCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + statusLane: 'hook', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const localAllClear = { + state: 'done' as const, + prompt: 'first turn', + agentType: 'claude' as const, + stateStartedAt: 5_500 + } + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000, + turnCompletedAt: 2_500 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + if (remountOrder === 'after') { + localCoordinator.observeHookStatus(localAllClear) + } + localCoordinator.dispose() + const remounted = createLocalCoordinator() + remounted.observeHookStatus(localAllClear) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + } + ) + + it('does not replay a stamped turn or its all-clear after a working title blip', () => { + const dispatchCompletion = vi.fn() + const dispatchHookLifecycle = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + dispatchHookLifecycle, + isLive: () => true + }) + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + vi.advanceTimersByTime(2_000) + coordinator.observeTitleWorking() + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + coordinator.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_050_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + expect(dispatchHookLifecycle).toHaveBeenLastCalledWith( + expect.objectContaining({ state: 'done', turnCompletedAt: 1_700_000_005_000 }) + ) + }) + + it('does not double-fire after a live remount between the gated Stop and the all-clear', () => { + const dispatchCompletion = vi.fn() + const firstCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + firstCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + firstCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + firstCoordinator.dispose() + + const remounted = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + remounted.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_055_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + }) + + it('keeps stamped replay suppression through a working title and live remount', () => { + const dispatchCompletion = vi.fn() + const firstCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + firstCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + firstCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + vi.advanceTimersByTime(2_000) + firstCoordinator.observeTitleWorking() + firstCoordinator.dispose() + + const remounted = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + remounted.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_055_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + }) + + it('does not let a pane title duplicate another coordinator stamped completion', () => { + const dispatchCompletion = vi.fn() + const hookCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const paneCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + hookCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + hookCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + vi.advanceTimersByTime(2_000) + paneCoordinator.observeTitleWorking() + paneCoordinator.observeClassifiedTitleCompletion('Claude done') + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + hookCoordinator.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_055_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + paneCoordinator.observeTitleWorking() + paneCoordinator.observeClassifiedTitleCompletion('Claude done') + + expect(dispatchCompletion).toHaveBeenCalledTimes(2) + }) + + it('does not let a vetoed gated Stop swallow the later all-clear', () => { + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + coordinator.observeClassifiedTitleCompletion('Claude done') + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + coordinator.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_055_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts index a388ec17457..9614d0bbffc 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts @@ -5,6 +5,8 @@ import type { RuntimeTerminalProcessInspection } from '@/runtime/runtime-termina export type AgentCompletionStatusSnapshot = ParsedAgentStatusPayload & { stateStartedAt?: number + /** Renderer-local boundary used only to reject a delayed cross-host completion. */ + localStateStartedAt?: number } export type AgentCompletionDispatchMeta = { @@ -21,6 +23,7 @@ export type AgentAttentionDispatchMeta = { export type AgentCompletionCoordinatorOptions = { paneKey: string + statusLane?: 'hook' | 'pty' getPtyId: () => string | null getSettings: () => Pick | null | undefined inspectProcess: ( @@ -50,6 +53,7 @@ export type AgentCompletionCoordinator = { observeTitleWorking: () => void observeOutputActivity: () => void observeHookStatus: (payload: AgentCompletionStatusSnapshot) => void + seedHookStatus: (payload: AgentCompletionStatusSnapshot) => void startProcessTracking: () => void hasPendingHookDoneCompletion: () => boolean resetCompletionState: (options?: { requireFreshWorking?: boolean }) => void diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts index b3059ca6390..f0802bd123a 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts @@ -31,10 +31,29 @@ type LastCompletionIdentity = { source: CompletionIdentitySource identity: string agentIdentity: string | null + /** End time of the Claude lead turn already announced while the pane stayed `working` for background inventory. The later all-clear `done` repeats this value and must not raise a second banner. */ + lastTurnCompletedAtNotified?: number } // Why: worktree switches remount a pane while the PTY/hook stream stays live, so stale completion replays must outlive one coordinator. const lastCompletionIdentityByPaneKey = new Map() +// Why: a late-pair stamped tail must suppress its replayed all-clear even though no banner was eligible. +const handledTurnCompletedAtsByPaneKey = new Map() +// Why: title fallback stays blocked only until the stamped background tail reaches its all-clear. +const pendingStampedTailByPaneKey = new Map< + string, + { + turnCompletedAt: number + originLane: string + eligibleWorkingBoundaryByLane: Map + consumedIdentityByLane: Map + tailOpen: boolean + } +>() +// Why: a sibling all-clear belongs to the stamped tail only while its exact working interval is still current. +const workingBoundaryByPaneKey = new Map>() +const coordinatorCountByPaneKey = new Map() +let nextCoordinatorId = 1 const IDLE_POLL_INTERVAL_MS = 2_000 const ACTIVE_POLL_INTERVAL_MS = 750 @@ -51,6 +70,7 @@ const COMPLETION_REPLAY_GUARD_MS = 1_000 const HOOK_DONE_QUIET_MS = 1_500 // Why: under "Approve for me" Codex resumes almost immediately, so debounce the OS attention notification so a self-resolving pause raises no false banner (#8387). const CODEX_ATTENTION_QUIET_MS = 1_500 +const HANDLED_TURN_STAMP_LIMIT = 16 const POLL_TIER_INTERVAL_MS: Record = { active: ACTIVE_POLL_INTERVAL_MS, @@ -59,6 +79,10 @@ const POLL_TIER_INTERVAL_MS: Record = { 'no-evidence': NO_EVIDENCE_POLL_INTERVAL_MS } +function isFiniteTurnCompletedAt(value: number | undefined): value is number { + return typeof value === 'number' && Number.isFinite(value) +} + function isCompletionHookState(state: ParsedAgentStatusPayload['state']): boolean { // Why: only a genuine 'done' ends a turn; 'waiting'/'blocked' go through isAttentionHookState. return state === 'done' @@ -72,6 +96,15 @@ function isAttentionHookState(state: ParsedAgentStatusPayload['state']): boolean export function createAgentCompletionCoordinator( options: AgentCompletionCoordinatorOptions ): AgentCompletionCoordinator { + const coordinatorId = nextCoordinatorId++ + const coordinatorLane = options.statusLane ?? `coordinator:${coordinatorId}` + let inheritedWorkingBoundary = pendingStampedTailByPaneKey + .get(options.paneKey) + ?.eligibleWorkingBoundaryByLane.get(coordinatorLane) + coordinatorCountByPaneKey.set( + options.paneKey, + (coordinatorCountByPaneKey.get(options.paneKey) ?? 0) + 1 + ) let disposed = false let agentIdentityEstablished = false let hasAgentRunEvidence = false @@ -169,15 +202,113 @@ export function createAgentCompletionCoordinator( return `${source}:${currentTurn}:${processSession}` } + function completionIdentityFor( + state: string, + agentType: string | undefined, + timestamp: number + ): string { + return [state, agentType ?? '', String(Math.trunc(timestamp))].join(':') + } + function hookCompletionIdentity(payload: AgentCompletionStatusSnapshot): string | null { - if (typeof payload.stateStartedAt !== 'number' || !Number.isFinite(payload.stateStartedAt)) { + // Why: `stateStartedAt` is pinned while the reported state does not change. A Claude pane held at `working` by background inventory would otherwise give every turn in the run the same identity. + const timestamp = isFiniteTurnCompletedAt(payload.turnCompletedAt) + ? payload.turnCompletedAt + : payload.stateStartedAt + if (typeof timestamp !== 'number' || !Number.isFinite(timestamp)) { return null } - return [ - payload.state, - payload.agentType ?? '', - String(Math.trunc(payload.stateStartedAt)) - ].join(':') + return completionIdentityFor(payload.state, payload.agentType, timestamp) + } + + function turnCompletedAtAlreadyHandled(turnCompletedAt: number): boolean { + return ( + handledTurnCompletedAtsByPaneKey.get(options.paneKey)?.includes(turnCompletedAt) === true || + lastCompletionIdentityByPaneKey.get(options.paneKey)?.lastTurnCompletedAtNotified === + turnCompletedAt + ) + } + + function rememberHandledTurnCompletedAt(turnCompletedAt: number): void { + const handled = handledTurnCompletedAtsByPaneKey.get(options.paneKey) ?? [] + if (handled.includes(turnCompletedAt)) { + return + } + handled.push(turnCompletedAt) + if (handled.length > HANDLED_TURN_STAMP_LIMIT) { + handled.shift() + } + handledTurnCompletedAtsByPaneKey.set(options.paneKey, handled) + } + + function recordWorkingBoundary(stateStartedAt: number | undefined): void { + let boundaries = workingBoundaryByPaneKey.get(options.paneKey) + if (!isFiniteTurnCompletedAt(stateStartedAt)) { + boundaries?.delete(coordinatorLane) + if (boundaries?.size === 0) { + workingBoundaryByPaneKey.delete(options.paneKey) + } + return + } + if (!boundaries) { + boundaries = new Map() + workingBoundaryByPaneKey.set(options.paneKey, boundaries) + } + inheritedWorkingBoundary = undefined + boundaries.set(coordinatorLane, stateStartedAt) + } + + function clearWorkingBoundary(): void { + inheritedWorkingBoundary = undefined + const boundaries = workingBoundaryByPaneKey.get(options.paneKey) + boundaries?.delete(coordinatorLane) + if (boundaries?.size === 0) { + workingBoundaryByPaneKey.delete(options.paneKey) + } + } + + function consumePendingStampedTailForAgent( + agentIdentity: string | null, + completionIdentity: string | null + ): boolean { + const pendingStampedTail = pendingStampedTailByPaneKey.get(options.paneKey) + const stampedCompletion = lastCompletionIdentityByPaneKey.get(options.paneKey) + const currentWorkingBoundary = workingBoundaryByPaneKey + .get(options.paneKey) + ?.get(coordinatorLane) + const eligibleWorkingBoundary = + pendingStampedTail?.eligibleWorkingBoundaryByLane.get(coordinatorLane) + if ( + pendingStampedTail === undefined || + stampedCompletion?.lastTurnCompletedAtNotified !== pendingStampedTail.turnCompletedAt || + agentIdentity === null || + stampedCompletion.agentIdentity !== agentIdentity || + eligibleWorkingBoundary === undefined || + (currentWorkingBoundary !== eligibleWorkingBoundary && + inheritedWorkingBoundary !== eligibleWorkingBoundary) + ) { + return false + } + pendingStampedTail.eligibleWorkingBoundaryByLane.delete(coordinatorLane) + inheritedWorkingBoundary = undefined + if (completionIdentity) { + pendingStampedTail.consumedIdentityByLane.set(coordinatorLane, completionIdentity) + } + pendingStampedTail.tailOpen = pendingStampedTail.eligibleWorkingBoundaryByLane.size > 0 + return true + } + + function consumeStampedTailForCurrentCoordinator(turnCompletedAt: number): void { + const pendingStampedTail = pendingStampedTailByPaneKey.get(options.paneKey) + if (pendingStampedTail?.turnCompletedAt !== turnCompletedAt) { + return + } + pendingStampedTail.eligibleWorkingBoundaryByLane.delete(coordinatorLane) + pendingStampedTail.tailOpen = pendingStampedTail.eligibleWorkingBoundaryByLane.size > 0 + } + + function hasUnconsumedStampedTail(): boolean { + return pendingStampedTailByPaneKey.get(options.paneKey)?.tailOpen === true } function hookCompletionAgentIdentity(payload: AgentCompletionStatusSnapshot): string | null { @@ -247,12 +378,27 @@ export function createAgentCompletionCoordinator( if (!completionIdentity) { return false } + if ( + completionIdentity.source === 'hook' && + pendingStampedTailByPaneKey + .get(options.paneKey) + ?.consumedIdentityByLane.get(coordinatorLane) === completionIdentity.identity + ) { + return true + } const previous = lastCompletionIdentityByPaneKey.get(options.paneKey) if (!previous) { return false } if (previous.source === completionIdentity.source) { - return previous.identity === completionIdentity.identity + return ( + previous.identity === completionIdentity.identity || + (completionIdentity.source === 'hook' && + consumePendingStampedTailForAgent( + completionIdentity.agentIdentity, + completionIdentity.identity + )) + ) } return ( previous.agentIdentity !== null && @@ -269,24 +415,26 @@ export function createAgentCompletionCoordinator( terminalIdleConfirmed?: boolean agentStatus?: AgentCompletionStatusSnapshot completionIdentity?: LastCompletionIdentity | null + /** Announce only. The pane is still genuinely `working` (Claude background inventory), so the synthetic `done` must not run pane lifecycle. */ + notifyWithoutLifecycle?: boolean } = {} - ): void { + ): boolean { if (source !== 'hook' && pendingHookDoneTimer !== null) { - return + return false } if (requiresFreshWorking || lastCompletedTurn === currentTurn) { - return + return false } if (!options.isLive() || !hasAgentRunEvidence) { - return + return false } const now = Date.now() const token = completionToken(source) if (token === lastCompletionToken && now - lastCompletionAt < COMPLETION_REPLAY_GUARD_MS) { - return + return false } if (completionIdentityAlreadyNotified(optionsOverride.completionIdentity)) { - return + return false } lastCompletionToken = token lastCompletionAt = now @@ -297,8 +445,19 @@ export function createAgentCompletionCoordinator( clearPendingCodexAttention() if (optionsOverride.completionIdentity) { lastCompletionIdentityByPaneKey.set(options.paneKey, optionsOverride.completionIdentity) + if (optionsOverride.completionIdentity.lastTurnCompletedAtNotified !== undefined) { + rememberHandledTurnCompletedAt( + optionsOverride.completionIdentity.lastTurnCompletedAtNotified + ) + } else { + pendingStampedTailByPaneKey.delete(options.paneKey) + } } - if (source === 'hook' && optionsOverride.agentStatus) { + if ( + source === 'hook' && + optionsOverride.agentStatus && + optionsOverride.notifyWithoutLifecycle !== true + ) { options.dispatchHookLifecycle?.(optionsOverride.agentStatus) } if (optionsOverride.quietedHookDone === true || source === 'process-exit') { @@ -309,9 +468,17 @@ export function createAgentCompletionCoordinator( ...(optionsOverride.terminalIdleConfirmed === true ? { terminalIdleConfirmed: true } : {}), ...(optionsOverride.agentStatus ? { agentStatus: optionsOverride.agentStatus } : {}) }) + } else if (optionsOverride.notifyWithoutLifecycle === true && optionsOverride.agentStatus) { + // Why: the pane is still `working`; the synthetic done must carry its own snapshot or the notification would read the pinned working row. + options.dispatchCompletion(title, { + source, + quietedHookDone: false, + agentStatus: optionsOverride.agentStatus + }) } else { options.dispatchCompletion(title) } + return true } function dispatchAttentionNotification(payload: AgentCompletionStatusSnapshot): void { @@ -449,6 +616,17 @@ export function createAgentCompletionCoordinator( function handleRecognizedProcess(process: RecognizedAgentProcess): void { pendingProcessExitAgent = null + const replayIdentity = lastCompletionIdentityByPaneKey.get(options.paneKey) + if ( + !lastForegroundAgent && + processSession > 0 && + !hasUnconsumedStampedTail() && + replayIdentity?.source === 'hook' && + replayIdentity.agentIdentity === process.agent + ) { + // Why: the prior observed process exited and this is a new same-agent process session, so its fallback completion cannot replay the old hook turn. + lastCompletionIdentityByPaneKey.delete(options.paneKey) + } if (lastForegroundAgent?.agent !== process.agent) { if (lastForegroundAgent && hasAgentRunEvidence) { if ( @@ -509,7 +687,8 @@ export function createAgentCompletionCoordinator( const exited = lastForegroundAgent pendingProcessExitAgent = null if (options.shouldSuppressConfirmedProcessExitCompletion?.(exited) !== true) { - dispatchCompletion('process-exit', exited.processName, { + const replayIdentityBeforeExit = lastCompletionIdentityByPaneKey.get(options.paneKey) + const committed = dispatchCompletion('process-exit', exited.processName, { terminalIdleConfirmed: true, completionIdentity: { source: 'process-exit', @@ -517,6 +696,15 @@ export function createAgentCompletionCoordinator( agentIdentity: exited.agent } }) + if ( + !committed && + !hasUnconsumedStampedTail() && + replayIdentityBeforeExit?.source === 'hook' && + replayIdentityBeforeExit.agentIdentity === exited.agent + ) { + // Why: this confirmed exit consumed the hook turn's process backstop; a later process instance must be allowed to complete independently. + lastCompletionIdentityByPaneKey.delete(options.paneKey) + } } lastForegroundAgent = null clearAgentRunEvidence() @@ -697,7 +885,9 @@ export function createAgentCompletionCoordinator( clearPendingCodexAttention() workingStatusObserved = true requiresFreshWorking = false - lastCompletionIdentityByPaneKey.delete(options.paneKey) + if (!hasUnconsumedStampedTail()) { + lastCompletionIdentityByPaneKey.delete(options.paneKey) + } currentTurn += 1 dropPendingTitle() return true @@ -792,6 +982,67 @@ export function createAgentCompletionCoordinator( establishAgentEvidence() } if (payload.state === 'working') { + const turnCompletedAt = isFiniteTurnCompletedAt(payload.turnCompletedAt) + ? payload.turnCompletedAt + : undefined + if (turnCompletedAt !== undefined) { + const alreadyHandled = turnCompletedAtAlreadyHandled(turnCompletedAt) + if (!alreadyHandled) { + const eligibleWorkingBoundaryByLane = new Map( + workingBoundaryByPaneKey.get(options.paneKey) + ) + eligibleWorkingBoundaryByLane.delete(coordinatorLane) + pendingStampedTailByPaneKey.set(options.paneKey, { + turnCompletedAt, + originLane: coordinatorLane, + eligibleWorkingBoundaryByLane, + consumedIdentityByLane: new Map(), + tailOpen: true + }) + } + // Why: Claude's lead Stop already ended the turn; the pane stays `working` only for background inventory. Announce now, keep lifecycle on the reported working row. + if ( + workingStatusObserved && + !alreadyHandled && + lastCompletionIdentity?.lastTurnCompletedAtNotified !== turnCompletedAt + ) { + const completionSnapshot: AgentCompletionStatusSnapshot = { + ...payload, + state: 'done', + stateStartedAt: turnCompletedAt, + turnCompletedAt + } + const completionIdentity: LastCompletionIdentity = { + source: 'hook', + identity: completionIdentityFor('done', payload.agentType, turnCompletedAt), + agentIdentity: hookCompletionAgentIdentity(payload), + lastTurnCompletedAtNotified: turnCompletedAt + } + const committed = dispatchCompletion('hook', payload.agentType ?? options.paneKey, { + notifyWithoutLifecycle: true, + agentStatus: completionSnapshot, + completionIdentity + }) + if (committed) { + lastCompletionIdentity = completionIdentity + } + } else if (!workingStatusObserved) { + rememberHandledTurnCompletedAt(turnCompletedAt) + } + options.dispatchHookLifecycle?.(payload) + return + } + const pendingStampedTail = pendingStampedTailByPaneKey.get(options.paneKey) + if (pendingStampedTail?.originLane === coordinatorLane) { + pendingStampedTailByPaneKey.delete(options.paneKey) + if ( + lastCompletionIdentityByPaneKey.get(options.paneKey)?.lastTurnCompletedAtNotified === + pendingStampedTail.turnCompletedAt + ) { + lastCompletionIdentityByPaneKey.delete(options.paneKey) + } + } + recordWorkingBoundary(payload.stateStartedAt) clearPendingHookDone() // Why: resumed work cancels the debounced attention so a self-resolving pause never notifies. clearPendingCodexAttention() @@ -824,6 +1075,35 @@ export function createAgentCompletionCoordinator( establishAgentEvidence() } const hookIdentity = hookCompletionIdentity(payload) + const turnCompletedAt = isFiniteTurnCompletedAt(payload.turnCompletedAt) + ? payload.turnCompletedAt + : undefined + if ( + turnCompletedAt === undefined && + consumePendingStampedTailForAgent(hookCompletionAgentIdentity(payload), hookIdentity) + ) { + // Why: paired host hooks carry the stamp while the sibling OSC coordinator sees only the matching all-clear. + lastCompletionIdentity = hookIdentity + ? { + source: 'hook', + identity: hookIdentity, + agentIdentity: hookCompletionAgentIdentity(payload) + } + : null + options.dispatchHookLifecycle?.(payload) + return + } + if ( + turnCompletedAt !== undefined && + (turnCompletedAtAlreadyHandled(turnCompletedAt) || + lastCompletionIdentity?.lastTurnCompletedAtNotified === turnCompletedAt || + lastCompletedTurn === currentTurn) + ) { + // Why: this `done` is the background all-clear of a turn already announced (or completed from another source). Keep pane lifecycle; do not raise a second banner. + consumeStampedTailForCurrentCoordinator(turnCompletedAt) + options.dispatchHookLifecycle?.(payload) + return + } if ( hookIdentity && lastCompletionIdentity?.source === 'hook' && @@ -869,6 +1149,14 @@ export function createAgentCompletionCoordinator( } } + function seedHookStatus(payload: AgentCompletionStatusSnapshot): void { + const { turnCompletedAt, ...unstampedPayload } = payload + if (isFiniteTurnCompletedAt(turnCompletedAt)) { + rememberHandledTurnCompletedAt(turnCompletedAt) + } + observeHookStatus(unstampedPayload) + } + function markTitleCompletionNotified(title: string): void { lastCompletionIdentity = { source: 'title', @@ -903,17 +1191,31 @@ export function createAgentCompletionCoordinator( lastForegroundAgent = null requiresFreshWorking = options.requireFreshWorking ?? false inspectionGeneration += 1 + clearWorkingBoundary() } function dispose(): void { + if (disposed) { + return + } disposed = true clearPollTimer() clearPendingHookDone() clearPendingCodexAttention() dropPendingTitle() - // Why: module-scoped identity survives a live-stream remount; evict only on genuine teardown (isLive() false) so it can't leak per closed pane. - if (!options.isLive()) { + clearWorkingBoundary() + const remainingCoordinators = (coordinatorCountByPaneKey.get(options.paneKey) ?? 1) - 1 + if (remainingCoordinators > 0) { + coordinatorCountByPaneKey.set(options.paneKey, remainingCoordinators) + } else { + coordinatorCountByPaneKey.delete(options.paneKey) + } + // Why: one pane can have hook and mounted coordinators; a remount disposal must not erase replay state still owned by its live sibling. + if (remainingCoordinators <= 0 && !options.isLive()) { lastCompletionIdentityByPaneKey.delete(options.paneKey) + handledTurnCompletedAtsByPaneKey.delete(options.paneKey) + pendingStampedTailByPaneKey.delete(options.paneKey) + workingBoundaryByPaneKey.delete(options.paneKey) } } @@ -923,6 +1225,7 @@ export function createAgentCompletionCoordinator( observeTitleWorking, observeOutputActivity, observeHookStatus, + seedHookStatus, startProcessTracking, hasPendingHookDoneCompletion, resetCompletionState, @@ -932,8 +1235,18 @@ export function createAgentCompletionCoordinator( export function resetAgentCompletionCoordinatorIdentitiesForTest(): void { lastCompletionIdentityByPaneKey.clear() + handledTurnCompletedAtsByPaneKey.clear() + pendingStampedTailByPaneKey.clear() + workingBoundaryByPaneKey.clear() + coordinatorCountByPaneKey.clear() + nextCoordinatorId = 1 } export function getAgentCompletionCoordinatorIdentityCountForTest(): number { - return lastCompletionIdentityByPaneKey.size + return new Set([ + ...lastCompletionIdentityByPaneKey.keys(), + ...handledTurnCompletedAtsByPaneKey.keys(), + ...pendingStampedTailByPaneKey.keys(), + ...workingBoundaryByPaneKey.keys() + ]).size } diff --git a/src/renderer/src/components/terminal-pane/agent-completion-snapshot-staleness.ts b/src/renderer/src/components/terminal-pane/agent-completion-snapshot-staleness.ts index f93542cf196..7b973c4a0be 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-snapshot-staleness.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-snapshot-staleness.ts @@ -8,16 +8,20 @@ export function isSupersededAgentCompletionSnapshot( if (!storedAgentStatus || !snapshot) { return false } - if (typeof snapshot.stateStartedAt !== 'number') { + const comparableStateStartedAt = snapshot.localStateStartedAt ?? snapshot.stateStartedAt + if (typeof comparableStateStartedAt !== 'number') { return storedAgentStatus.state !== snapshot.state } // Why: hook completion notifications are delayed by a quiet window; by the // time they fire, the same pane may already belong to a newer agent turn. - if (storedAgentStatus.stateStartedAt > snapshot.stateStartedAt) { + if (storedAgentStatus.stateStartedAt > comparableStateStartedAt) { return true } + const hasStampedTurn = + typeof snapshot.turnCompletedAt === 'number' && Number.isFinite(snapshot.turnCompletedAt) return ( - storedAgentStatus.stateStartedAt === snapshot.stateStartedAt && - storedAgentStatus.state !== snapshot.state + storedAgentStatus.stateStartedAt === comparableStateStartedAt && + storedAgentStatus.state !== snapshot.state && + !hasStampedTurn ) } diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 93ab49fb5a9..9dc930fb25f 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -2426,6 +2426,7 @@ export function connectPanePty( const agentCompletionCoordinator = createAgentCompletionCoordinator({ paneKey: cacheKey, + statusLane: 'pty', getPtyId: () => transport.getPtyId(), getSettings: () => useAppStore.getState().settings, inspectProcess: inspectRuntimeTerminalProcess, diff --git a/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts b/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts index 6f7bec8592b..8998b6a43af 100644 --- a/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts @@ -150,6 +150,7 @@ describe('applyTerminalAppearance theme assignment', () => { getBoundingClientRect: () => ({ width: measurable ? 800 : 0, height: measurable ? 600 : 0 }) }, fitAddon: { + fit: vi.fn(), proposeDimensions: () => (measurable ? { cols: 80, rows: 24 } : undefined) } } as unknown as ManagedPane @@ -360,6 +361,74 @@ describe('applyTerminalAppearance theme assignment', () => { // Latest wins, exactly one write: intermediate hidden values never touch xterm. expect(writes).toEqual([21]) }) + + it('stamps settings-UI padding defaults when padding is unset', () => { + const pane = makePane(1) + const manager = makeManager([pane]) + applyTerminalAppearance( + manager, + getDefaultSettings('/tmp'), + true, + new Map(), + new Map(), + 'false', + new Map(), + new Map() + ) + expect(manager.setPaneStyleOptions).toHaveBeenCalledWith( + expect.objectContaining({ paddingX: 4, paddingY: 4 }) + ) + }) + + it('rounds imported half-pixel padding before fitting', () => { + const pane = makePane(1) + const manager = makeManager([pane]) + const settings = { + ...getDefaultSettings('/tmp'), + terminalPaddingX: 1.5, + terminalPaddingY: 2.5 + } + + applyTerminalAppearance( + manager, + settings, + true, + new Map(), + new Map(), + 'false', + new Map(), + new Map() + ) + + expect(manager.setPaneStyleOptions).toHaveBeenCalledWith( + expect.objectContaining({ paddingX: 2, paddingY: 3 }) + ) + }) + + it('stamps padding before fitting the pane', () => { + const pane = makePane(1) + pane.fitAddon.proposeDimensions = () => ({ cols: 79, rows: 23 }) + const manager = makeManager([pane]) + + applyTerminalAppearance( + manager, + getDefaultSettings('/tmp'), + true, + new Map(), + new Map(), + 'false', + new Map(), + new Map() + ) + + const setPaneStyleOptions = vi.mocked(manager.setPaneStyleOptions) + const fit = vi.mocked(pane.fitAddon.fit) + expect(setPaneStyleOptions).toHaveBeenCalledOnce() + expect(fit).toHaveBeenCalledOnce() + expect(setPaneStyleOptions.mock.invocationCallOrder[0]!).toBeLessThan( + fit.mock.invocationCallOrder[0]! + ) + }) }) describe('publishTerminalViewAttributesAtAppStart', () => { diff --git a/src/renderer/src/components/terminal-pane/terminal-appearance.ts b/src/renderer/src/components/terminal-pane/terminal-appearance.ts index bb61a50fbbc..f520d98baba 100644 --- a/src/renderer/src/components/terminal-pane/terminal-appearance.ts +++ b/src/renderer/src/components/terminal-pane/terminal-appearance.ts @@ -3,6 +3,7 @@ import type { PaneManager } from '@/lib/pane-manager/pane-manager' import type { GlobalSettings } from '../../../../shared/global-settings-types' import { resolveTerminalFontWeights } from '../../../../shared/terminal-fonts' import { resolveTerminalLigaturesEnabled } from '../../../../shared/terminal-ligatures' +import { normalizeTerminalPadding } from '../../../../shared/terminal-padding-settings' import { getBuiltinTheme, resolvePaneStyleOptions, @@ -158,6 +159,23 @@ export function applyTerminalAppearance( settings.terminalLigatures, settings.terminalFontFamily ) + // FitAddon parses each CSS padding as an integer, so normalize imported half-pixels before styling and fitting. + const paddingX = normalizeTerminalPadding(settings.terminalPaddingX ?? 4) + const paddingY = normalizeTerminalPadding(settings.terminalPaddingY ?? 4) + + // Why before the pane loop: FitAddon subtracts live .xterm padding, so the + // CSS vars must be stamped before safeFit or a padding shrink leaves a stale grid. + manager.setPaneStyleOptions({ + splitBackground: paneBackground, + paneBackground, + inactivePaneOpacity: paneStyles.inactivePaneOpacity, + activePaneOpacity: paneStyles.activePaneOpacity, + opacityTransitionMs: paneStyles.opacityTransitionMs, + dividerThicknessPx: paneStyles.dividerThicknessPx, + focusFollowsMouse: paneStyles.focusFollowsMouse, + paddingX, + paddingY + }) for (const pane of manager.getPanes()) { // Why value-gated: writing options.theme rebuilds the palette, discarding TUI OSC 4/10/11/12 mutations; skip on no-op change. @@ -227,16 +245,4 @@ export function applyTerminalAppearance( safeFit(pane) } } - - manager.setPaneStyleOptions({ - splitBackground: paneBackground, - paneBackground, - inactivePaneOpacity: paneStyles.inactivePaneOpacity, - activePaneOpacity: paneStyles.activePaneOpacity, - opacityTransitionMs: paneStyles.opacityTransitionMs, - dividerThicknessPx: paneStyles.dividerThicknessPx, - focusFollowsMouse: paneStyles.focusFollowsMouse, - paddingX: settings.terminalPaddingX, - paddingY: settings.terminalPaddingY - }) } diff --git a/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts b/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts index 0fa4e74dd88..1267b986827 100644 --- a/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts +++ b/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts @@ -190,6 +190,99 @@ describe('dispatchTerminalNotification', () => { expect(mockState.markTerminalPaneUnread).toHaveBeenCalledWith(paneKey) }) + it('builds the notification id from a completion snapshot, not the pinned working row', () => { + const pinnedWorkingStartedAt = Date.now() - 60_000 + // Why: the stored row must name the event's agent, or it is dropped for identity mismatch + // and the assertion would hold whichever side of the `??` wins. + mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, { + state: 'working', + stateStartedAt: pinnedWorkingStartedAt, + agentType: 'claude', + terminalTitle: 'claude' + }) + const turnCompletedAt = Date.now() + + dispatchTerminalNotification('wt-primary', { + source: 'agent-task-complete', + terminalTitle: 'claude', + paneKey, + agentStatusSnapshot: { + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: turnCompletedAt + } + }) + + expect(window.api.notifications.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + notificationId: buildAgentNotificationId({ + worktreeId: 'wt-primary', + paneKey, + stateStartedAt: turnCompletedAt + }) + }) + ) + }) + + it.each([ + { clientStateStartedAt: 5_000, hostTurnCompletedAt: 2_000 }, + { clientStateStartedAt: 2_000, hostTurnCompletedAt: 5_000 } + ])( + 'accepts a host-stamped completion across client/host clock skew %#', + ({ clientStateStartedAt, hostTurnCompletedAt }) => { + mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, { + state: 'working', + stateStartedAt: clientStateStartedAt, + agentType: 'claude', + terminalTitle: 'claude' + }) + + dispatchTerminalNotification('wt-primary', { + source: 'agent-task-complete', + terminalTitle: 'claude', + paneKey, + agentStatusSnapshot: { + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: hostTurnCompletedAt, + localStateStartedAt: clientStateStartedAt, + turnCompletedAt: hostTurnCompletedAt + } + }) + + expect(window.api.notifications.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ source: 'agent-task-complete' }) + ) + } + ) + + it('drops a host-stamped completion after a newer client turn starts', () => { + mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, { + state: 'working', + stateStartedAt: 6_000, + agentType: 'claude', + terminalTitle: 'claude' + }) + + dispatchTerminalNotification('wt-primary', { + source: 'agent-task-complete', + terminalTitle: 'claude', + paneKey, + agentStatusSnapshot: { + state: 'done', + prompt: 'previous turn', + agentType: 'claude', + stateStartedAt: 2_000, + localStateStartedAt: 5_000, + turnCompletedAt: 2_000 + } + }) + + expect(window.api.notifications.dispatch).not.toHaveBeenCalled() + }) + it('uses a live pane key when inactive worktree tab membership is not hydrated', () => { mockState.tabsByWorktree = {} diff --git a/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts b/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts index 20a0ddadf89..29b4ace2c1f 100644 --- a/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts +++ b/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts @@ -121,7 +121,7 @@ export function dispatchTerminalNotification( return } const agentNotificationStateStartedAt = - freshStoredAgentStatus?.stateStartedAt ?? eventAgentStatusSnapshot?.stateStartedAt + eventAgentStatusSnapshot?.stateStartedAt ?? freshStoredAgentStatus?.stateStartedAt // Why: main-process hook IPC can update inactive/unmounted worktrees before // the renderer's live-PTY map catches up. A fresh accepted hook snapshot is // authoritative for agent completion; title/BEL-only paths still need PTY liveness. diff --git a/src/renderer/src/hooks/agent-hook-completion-background-turn-notifications.test.ts b/src/renderer/src/hooks/agent-hook-completion-background-turn-notifications.test.ts new file mode 100644 index 00000000000..8583c3bb0de --- /dev/null +++ b/src/renderer/src/hooks/agent-hook-completion-background-turn-notifications.test.ts @@ -0,0 +1,270 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ParsedAgentStatusPayload } from '../../../shared/agent-status-types' +import { createHookListenerState, normalizeHookPayload } from '../../../shared/agent-hook-listener' +import { makePaneKey } from '../../../shared/stable-pane-id' + +const dispatchTerminalNotification = vi.fn() +const dispatchAgentHookTerminalLifecycle = vi.fn() + +const HOOK_DONE_QUIET_MS = 1_500 +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +const PANE_KEY = makePaneKey('tab-1', LEAF_ID) +const WORKTREE_ID = 'wt-1' + +type MockStoreState = { + settings: { + experimentalTerminalAttention?: boolean + notifications: { + enabled: boolean + agentTaskComplete: boolean + } + } + ptyIdsByTabId: Record + suppressedPtyExitIds: Record + tabsByWorktree: Record + terminalLayoutsByTabId: Record + agentLaunchConfigByPaneKey: Record + agentStatusByPaneKey: Record + getAgentLaunchConfigForStatusEntry: () => undefined + getAgentLaunchConfigForStatusMetadata: () => undefined +} + +let mockStoreState: MockStoreState + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockStoreState + } +})) + +vi.mock('@/components/terminal-pane/use-notification-dispatch', () => ({ + dispatchTerminalNotification +})) + +vi.mock('@/components/terminal-pane/agent-hook-terminal-lifecycle', () => ({ + dispatchAgentHookTerminalLifecycle +})) + +type HookRow = ParsedAgentStatusPayload & { stateStartedAt: number } + +function pinStateStartedAt( + previous: HookRow | undefined, + payload: ParsedAgentStatusPayload, + now: number +): HookRow { + return { + ...payload, + stateStartedAt: previous && previous.state === payload.state ? previous.stateStartedAt : now + } +} + +describe('Claude background-turn completion notifications', () => { + beforeEach(() => { + vi.resetModules() + vi.useFakeTimers() + vi.setSystemTime(1_700_000_000_000) + dispatchTerminalNotification.mockClear() + dispatchAgentHookTerminalLifecycle.mockClear() + mockStoreState = { + settings: { + experimentalTerminalAttention: false, + notifications: { + enabled: true, + agentTaskComplete: true + } + }, + ptyIdsByTabId: { + 'tab-1': ['pty-1'] + }, + suppressedPtyExitIds: {}, + tabsByWorktree: { + [WORKTREE_ID]: [{ id: 'tab-1', ptyId: 'pty-1' }] + }, + terminalLayoutsByTabId: {}, + agentLaunchConfigByPaneKey: {}, + agentStatusByPaneKey: {}, + getAgentLaunchConfigForStatusEntry: () => undefined, + getAgentLaunchConfigForStatusMetadata: () => undefined + } + }) + + afterEach(() => { + vi.useRealTimers() + }) + + async function play(events: { at: number; payload: Record }[]): Promise<{ + banners: { at: number; body: string | undefined; stateStartedAt: number | undefined }[] + rows: HookRow[] + }> { + const { observeAgentHookCompletionForNotification } = + await import('./agent-hook-completion-notifications') + const listener = createHookListenerState() + const rows: HookRow[] = [] + let previous: HookRow | undefined + const banners: { + at: number + body: string | undefined + stateStartedAt: number | undefined + }[] = [] + + dispatchTerminalNotification.mockImplementation((_worktreeId, event) => { + banners.push({ + at: Date.now(), + body: event.agentStatusSnapshot?.lastAssistantMessage, + stateStartedAt: event.agentStatusSnapshot?.stateStartedAt + }) + }) + + for (const event of events) { + vi.setSystemTime(event.at) + const normalized = normalizeHookPayload( + listener, + 'claude', + { paneKey: PANE_KEY, payload: event.payload }, + 'production' + ) + if (!normalized) { + continue + } + const row = pinStateStartedAt(previous, normalized.payload, event.at) + previous = row + rows.push(row) + observeAgentHookCompletionForNotification({ + paneKey: PANE_KEY, + worktreeId: WORKTREE_ID, + payload: row + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + } + + return { banners, rows } + } + + it('announces a subagent turn at Stop with that turn’s text, not when the child later drains', async () => { + const { banners, rows } = await play([ + { + at: 1_700_000_000_000, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'review the PR' } + }, + { + at: 1_700_000_001_000, + payload: { + hook_event_name: 'SubagentStart', + agent_id: 'a1', + agent_type: 'general-purpose' + } + }, + { + at: 1_700_000_006_000, + payload: { + hook_event_name: 'Stop', + last_assistant_message: 'Which cells need hand-verification?', + background_tasks: [{ id: 'a1', type: 'subagent', status: 'running' }] + } + }, + { + at: 1_700_000_055_000, + payload: { hook_event_name: 'SubagentStop', agent_id: 'a1' } + } + ]) + + const stop = rows.find((row) => row.lastAssistantMessage?.includes('hand-verification')) + expect(stop?.state).toBe('working') + expect(stop?.turnCompletedAt).toBe(1_700_000_006_000) + expect(banners).toHaveLength(1) + expect(banners[0]?.at).toBe(1_700_000_006_000) + expect(banners[0]?.body).toBe('Which cells need hand-verification?') + expect(banners[0]?.stateStartedAt).toBe(1_700_000_006_000) + }) + + it('notifies each background-shell turn with a distinct notification identity', async () => { + const { banners } = await play([ + { + at: 1_700_000_000_000, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'start the server' } + }, + { + at: 1_700_000_001_000, + payload: { + hook_event_name: 'Stop', + last_assistant_message: 'Dev server is up.', + background_tasks: [{ id: 'shell-1', type: 'shell', status: 'running' }] + } + }, + { + at: 1_700_000_010_000, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'add a route' } + }, + { + at: 1_700_000_011_000, + payload: { + hook_event_name: 'Stop', + last_assistant_message: 'Route added.', + background_tasks: [{ id: 'shell-1', type: 'shell', status: 'running' }] + } + } + ]) + + expect(banners).toHaveLength(2) + expect(banners[0]).toMatchObject({ + at: 1_700_000_001_000, + body: 'Dev server is up.', + stateStartedAt: 1_700_000_001_000 + }) + expect(banners[1]).toMatchObject({ + at: 1_700_000_011_000, + body: 'Route added.', + stateStartedAt: 1_700_000_011_000 + }) + expect(banners[0]?.stateStartedAt).not.toBe(banners[1]?.stateStartedAt) + }) + + it('notifies every turn under a persistent session cron with distinct ids', async () => { + const cron = [{ id: 'cron-1' }] + const { banners } = await play([ + { + at: 1_700_000_000_000, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'first' } + }, + { + at: 1_700_000_001_000, + payload: { + hook_event_name: 'Stop', + last_assistant_message: 'First done.', + session_crons: cron + } + }, + { + at: 1_700_000_002_000, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'second' } + }, + { + at: 1_700_000_003_000, + payload: { + hook_event_name: 'Stop', + last_assistant_message: 'Second done.', + session_crons: cron + } + }, + { + at: 1_700_000_004_000, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'third' } + }, + { + at: 1_700_000_005_000, + payload: { + hook_event_name: 'Stop', + last_assistant_message: 'Third done.', + session_crons: cron + } + } + ]) + + expect(banners.map((banner) => banner.body)).toEqual([ + 'First done.', + 'Second done.', + 'Third done.' + ]) + expect(new Set(banners.map((banner) => banner.stateStartedAt)).size).toBe(3) + }) +}) diff --git a/src/renderer/src/hooks/agent-hook-completion-fresh-working.test.ts b/src/renderer/src/hooks/agent-hook-completion-fresh-working.test.ts new file mode 100644 index 00000000000..006a848fdab --- /dev/null +++ b/src/renderer/src/hooks/agent-hook-completion-fresh-working.test.ts @@ -0,0 +1,122 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ParsedAgentStatusPayload } from '../../../shared/agent-status-types' +import { makePaneKey } from '../../../shared/stable-pane-id' + +const dispatchTerminalNotification = vi.fn() +const dispatchAgentHookTerminalLifecycle = vi.fn() + +const PANE_KEY = makePaneKey('tab-1', '11111111-1111-4111-8111-111111111111') +const WORKTREE_ID = 'wt-1' + +type MockStoreState = { + settings: { + experimentalTerminalAttention: boolean + notifications: { enabled: boolean; agentTaskComplete: boolean } + } + ptyIdsByTabId: Record + suppressedPtyExitIds: Record + tabsByWorktree: Record + terminalLayoutsByTabId: Record + agentLaunchConfigByPaneKey: Record + agentStatusByPaneKey: Record + getAgentLaunchConfigForStatusEntry: () => undefined + getAgentLaunchConfigForStatusMetadata: () => undefined +} + +let mockStoreState: MockStoreState + +vi.mock('@/store', () => ({ + useAppStore: { getState: () => mockStoreState } +})) + +vi.mock('@/components/terminal-pane/use-notification-dispatch', () => ({ + dispatchTerminalNotification +})) + +vi.mock('@/components/terminal-pane/agent-hook-terminal-lifecycle', () => ({ + dispatchAgentHookTerminalLifecycle +})) + +function working(turnCompletedAt?: number): ParsedAgentStatusPayload { + return { + state: 'working', + prompt: 'implement notifications', + agentType: 'claude', + lastAssistantMessage: turnCompletedAt === undefined ? undefined : 'Turn complete.', + turnCompletedAt + } +} + +describe('agent hook completion fresh-working gate', () => { + beforeEach(() => { + vi.resetModules() + vi.useFakeTimers() + dispatchTerminalNotification.mockClear() + dispatchAgentHookTerminalLifecycle.mockClear() + mockStoreState = { + settings: { + experimentalTerminalAttention: false, + notifications: { enabled: true, agentTaskComplete: false } + }, + ptyIdsByTabId: { 'tab-1': ['pty-1'] }, + suppressedPtyExitIds: {}, + tabsByWorktree: { [WORKTREE_ID]: [{ id: 'tab-1', ptyId: 'pty-1' }] }, + terminalLayoutsByTabId: {}, + agentLaunchConfigByPaneKey: {}, + agentStatusByPaneKey: {}, + getAgentLaunchConfigForStatusEntry: () => undefined, + getAgentLaunchConfigForStatusMetadata: () => undefined + } + }) + + afterEach(() => vi.useRealTimers()) + + it('stays gated through a stamped working completion after re-enable', async () => { + const { + observeAgentHookCompletionForNotification, + syncAgentHookCompletionNotificationSettings + } = await import('./agent-hook-completion-notifications') + + syncAgentHookCompletionNotificationSettings() + observeAgentHookCompletionForNotification({ + paneKey: PANE_KEY, + worktreeId: WORKTREE_ID, + payload: { ...working(), stateStartedAt: 1_000 } + }) + + mockStoreState.settings.notifications.agentTaskComplete = true + syncAgentHookCompletionNotificationSettings() + observeAgentHookCompletionForNotification({ + paneKey: PANE_KEY, + worktreeId: WORKTREE_ID, + payload: working(1_500) + }) + + expect(dispatchTerminalNotification).not.toHaveBeenCalled() + + observeAgentHookCompletionForNotification({ + paneKey: PANE_KEY, + worktreeId: WORKTREE_ID, + payload: { ...working(), stateStartedAt: 2_000 } + }) + observeAgentHookCompletionForNotification({ + paneKey: PANE_KEY, + worktreeId: WORKTREE_ID, + payload: { ...working(2_500), stateStartedAt: 2_000 } + }) + + expect(dispatchTerminalNotification).toHaveBeenCalledTimes(1) + expect(dispatchTerminalNotification).toHaveBeenCalledWith( + WORKTREE_ID, + expect.objectContaining({ + source: 'agent-task-complete', + paneKey: PANE_KEY, + agentStatusSnapshot: expect.objectContaining({ + state: 'done', + turnCompletedAt: 2_500, + lastAssistantMessage: 'Turn complete.' + }) + }) + ) + }) +}) diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications-import.test.ts b/src/renderer/src/hooks/agent-hook-completion-notifications-import.test.ts new file mode 100644 index 00000000000..390439d894c --- /dev/null +++ b/src/renderer/src/hooks/agent-hook-completion-notifications-import.test.ts @@ -0,0 +1,17 @@ +import { expect, it, vi } from 'vitest' + +vi.mock('@/store', () => ({ + useAppStore: vi.fn() +})) + +vi.mock('@/components/terminal-pane/use-notification-dispatch', () => ({ + dispatchTerminalNotification: vi.fn() +})) + +vi.mock('@/components/terminal-pane/agent-hook-terminal-lifecycle', () => ({ + dispatchAgentHookTerminalLifecycle: vi.fn() +})) + +it('does not read the Zustand store while importing completion notifications', async () => { + await expect(import('./agent-hook-completion-notifications')).resolves.toBeDefined() +}) diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts b/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts index 4803731fb70..63c33c7f26e 100644 --- a/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts +++ b/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts @@ -167,9 +167,7 @@ describe('agent hook completion notifications', () => { } }) - afterEach(() => { - vi.useRealTimers() - }) + afterEach(() => vi.useRealTimers()) it('requires fresh working after notifications start disabled and later re-enable', async () => { mockStoreState.settings.notifications.agentTaskComplete = false @@ -178,6 +176,7 @@ describe('agent hook completion notifications', () => { syncAgentHookCompletionNotificationSettings } = await import('./agent-hook-completion-notifications') + syncAgentHookCompletionNotificationSettings() mockStoreState.settings.notifications.agentTaskComplete = true syncAgentHookCompletionNotificationSettings() diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications.ts b/src/renderer/src/hooks/agent-hook-completion-notifications.ts index 125c0994514..7d226331eed 100644 --- a/src/renderer/src/hooks/agent-hook-completion-notifications.ts +++ b/src/renderer/src/hooks/agent-hook-completion-notifications.ts @@ -11,6 +11,7 @@ import { collectLeafIdsInOrder } from '@/components/terminal-pane/layout-seriali import { createCodexAutoApprovalHookCompletionSuppressor } from '@/components/terminal-pane/codex-auto-approval-notification-suppression' import { dispatchAgentHookTerminalLifecycle } from '@/components/terminal-pane/agent-hook-terminal-lifecycle' import { + isAgentHookCompletionTrackingEnabled, shouldSyncAgentHookCompletionForStoreUpdate, type AgentHookCompletionStoreSnapshot } from './agent-hook-completion-store-sync' @@ -33,8 +34,8 @@ type PaneCoordinatorLivenessSnapshot = Pick< const coordinatorsByPaneKey = new Map() const paneKeysRequiringFreshWorking = new Set() -let wasAgentTaskCompleteTrackingEnabled = isAgentTaskCompleteTrackingEnabled() -let requireFreshWorkingForNewTrackingCoordinators = !wasAgentTaskCompleteTrackingEnabled +let wasAgentTaskCompleteTrackingEnabled: boolean | undefined +let requireFreshWorkingForNewTrackingCoordinators = false let lastPrunedLivenessSnapshot: PaneCoordinatorLivenessSnapshot | null = null function disposeCoordinatorForPaneKey(paneKey: string): void { @@ -111,9 +112,12 @@ function isAgentTaskCompleteTrackingEnabled(): boolean { return isAgentTaskCompleteNotificationEnabled() || isTerminalAttentionEnabled() } -export function syncAgentHookCompletionNotificationSettings(): boolean { - pruneClosedPaneCoordinators() - const enabled = isAgentTaskCompleteTrackingEnabled() +function syncAgentTaskCompleteTrackingEnabled(enabled: boolean): void { + if (wasAgentTaskCompleteTrackingEnabled === undefined) { + wasAgentTaskCompleteTrackingEnabled = enabled + requireFreshWorkingForNewTrackingCoordinators = !enabled + return + } if (enabled !== wasAgentTaskCompleteTrackingEnabled) { requireFreshWorkingForNewTrackingCoordinators = true for (const paneKey of coordinatorsByPaneKey.keys()) { @@ -121,6 +125,12 @@ export function syncAgentHookCompletionNotificationSettings(): boolean { } } wasAgentTaskCompleteTrackingEnabled = enabled +} + +export function syncAgentHookCompletionNotificationSettings(): boolean { + pruneClosedPaneCoordinators() + const enabled = isAgentTaskCompleteTrackingEnabled() + syncAgentTaskCompleteTrackingEnabled(enabled) return enabled } @@ -133,6 +143,9 @@ export function syncAgentHookCompletionNotificationsForStoreUpdate( if (!shouldSyncAgentHookCompletionForStoreUpdate(current, previous)) { return false } + if (wasAgentTaskCompleteTrackingEnabled === undefined) { + syncAgentTaskCompleteTrackingEnabled(isAgentHookCompletionTrackingEnabled(previous)) + } syncAgentHookCompletionNotificationSettings() return true } @@ -231,6 +244,7 @@ function paneCanReceiveHookCompletion(paneKey: string, tabIndex?: TabIndex): boo function createCoordinator(paneKey: string, worktreeId: string): AgentCompletionCoordinator { return createAgentCompletionCoordinator({ paneKey, + statusLane: 'hook', getPtyId: () => getPtyIdForPaneKey(paneKey), getSettings: () => useAppStore.getState().settings, inspectProcess: async (): Promise => ({ @@ -272,18 +286,28 @@ function createCoordinator(paneKey: string, worktreeId: string): AgentCompletion export function observeAgentHookCompletionForNotification({ paneKey, worktreeId, - payload + payload, + seedOnly }: { paneKey: string worktreeId: string payload: AgentCompletionStatusSnapshot + seedOnly?: boolean }): void { - pruneClosedPaneCoordinators() - if (!paneCanReceiveHookCompletion(paneKey)) { - return + // Why: replay seeds already passed indexed snapshot ownership; re-resolving every row makes startup batches quadratic. + if (seedOnly !== true) { + pruneClosedPaneCoordinators() + if (!paneCanReceiveHookCompletion(paneKey)) { + return + } } - const trackingEnabled = syncAgentHookCompletionNotificationSettings() + const trackingEnabled = isAgentTaskCompleteTrackingEnabled() + if (seedOnly === true) { + syncAgentTaskCompleteTrackingEnabled(trackingEnabled) + } else { + syncAgentHookCompletionNotificationSettings() + } let entry = coordinatorsByPaneKey.get(paneKey) if (!entry || entry.worktreeId !== worktreeId) { @@ -299,10 +323,14 @@ export function observeAgentHookCompletionForNotification({ } // Why: notification preferences may suppress alerts, but accepted hooks must // still release pane-owned cursor/cache effects after the quiet window. - if (payload.state === 'working' && trackingEnabled) { + if (payload.state === 'working' && payload.turnCompletedAt === undefined && trackingEnabled) { paneKeysRequiringFreshWorking.delete(paneKey) } - entry.coordinator.observeHookStatus(payload) + if (seedOnly === true) { + entry.coordinator.seedHookStatus(payload) + } else { + entry.coordinator.observeHookStatus(payload) + } } export function resetAgentHookCompletionNotificationCoordinators(): void { diff --git a/src/renderer/src/hooks/agent-hook-completion-store-sync.ts b/src/renderer/src/hooks/agent-hook-completion-store-sync.ts index e900c5b0c5d..fc47a20205a 100644 --- a/src/renderer/src/hooks/agent-hook-completion-store-sync.ts +++ b/src/renderer/src/hooks/agent-hook-completion-store-sync.ts @@ -24,7 +24,9 @@ export type AgentHookCompletionStoreSnapshot = { type TabVisit = () => void -function isTrackingEnabled(state: AgentHookCompletionStoreSnapshot): boolean { +export function isAgentHookCompletionTrackingEnabled( + state: AgentHookCompletionStoreSnapshot +): boolean { const notifications = state.settings?.notifications const notificationEnabled = notifications?.enabled !== false && notifications?.agentTaskComplete !== false @@ -80,7 +82,9 @@ function shouldSync( previous: AgentHookCompletionStoreSnapshot, visitTab?: TabVisit ): boolean { - if (isTrackingEnabled(current) !== isTrackingEnabled(previous)) { + if ( + isAgentHookCompletionTrackingEnabled(current) !== isAgentHookCompletionTrackingEnabled(previous) + ) { return true } if ( diff --git a/src/renderer/src/hooks/ipc-events-agent-status-store-test-fixtures.ts b/src/renderer/src/hooks/ipc-events-agent-status-store-test-fixtures.ts index 4a05afbdd11..609b3fb562c 100644 --- a/src/renderer/src/hooks/ipc-events-agent-status-store-test-fixtures.ts +++ b/src/renderer/src/hooks/ipc-events-agent-status-store-test-fixtures.ts @@ -31,6 +31,7 @@ export type AgentStatusSetData = { lastAssistantMessage?: string interrupted?: boolean sessionBoundary?: boolean + turnCompletedAt?: number terminalHandle?: string launchToken?: string providerSession?: { key: 'session_id'; id: string } diff --git a/src/renderer/src/hooks/useIpcEvents-agent-status-turn-completion.test.ts b/src/renderer/src/hooks/useIpcEvents-agent-status-turn-completion.test.ts new file mode 100644 index 00000000000..0a9cb1a6c30 --- /dev/null +++ b/src/renderer/src/hooks/useIpcEvents-agent-status-turn-completion.test.ts @@ -0,0 +1,92 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + buildStoreState, + FUTURE_PANE_KEY, + type AgentStatusSetData, + type StoreLike +} from './ipc-events-agent-status-store-test-fixtures' +import { + buildWindowApi, + stubAuxiliaryModules, + stubReactSyncEffect +} from './ipc-events-agent-status-window-test-fixtures' + +describe('useIpcEvents agent status turn completion', () => { + beforeEach(() => { + vi.resetModules() + vi.unstubAllGlobals() + }) + + it('forwards turnCompletedAt through the agent-status IPC whitelist', async () => { + const setAgentStatus = vi.fn() + const observeAgentHookCompletionForNotification = vi.fn() + const onSetListenerRef: { current: ((data: AgentStatusSetData) => void) | null } = { + current: null + } + const storeState: StoreLike = buildStoreState({ + setAgentStatus, + workspaceSessionReady: true, + settings: { terminalFontSize: 13, notifications: { enabled: true, agentTaskComplete: true } }, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'Claude' }] + } + }) + + stubReactSyncEffect() + vi.doMock('../store', () => ({ + useAppStore: { + subscribe: vi.fn(() => () => {}), + getState: () => storeState + } + })) + vi.doMock('./agent-hook-completion-notifications', () => ({ + observeAgentHookCompletionForNotification, + resetAgentHookCompletionNotificationCoordinators: vi.fn(), + syncAgentHookCompletionNotificationSettings: vi.fn(), + syncAgentHookCompletionNotificationsForStoreUpdate: vi.fn() + })) + stubAuxiliaryModules() + vi.stubGlobal( + 'window', + buildWindowApi({ + onSet: (cb) => { + onSetListenerRef.current = cb + return () => {} + } + }) + ) + + const { useIpcEvents } = await import('./useIpcEvents') + useIpcEvents() + await Promise.resolve() + + if (typeof onSetListenerRef.current !== 'function') { + throw new Error('Expected agentStatus.onSet listener to be registered') + } + onSetListenerRef.current({ + paneKey: FUTURE_PANE_KEY, + tabId: 'tab-future', + worktreeId: 'wt-1', + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + lastAssistantMessage: 'Which cells need hand-verification?', + turnCompletedAt: 1_700_000_005_000, + receivedAt: 1_700_000_005_000, + stateStartedAt: 1_700_000_000_000 + }) + + expect(observeAgentHookCompletionForNotification).toHaveBeenCalledWith( + expect.objectContaining({ + paneKey: FUTURE_PANE_KEY, + worktreeId: 'wt-1', + payload: expect.objectContaining({ + state: 'working', + agentType: 'claude', + turnCompletedAt: 1_700_000_005_000, + lastAssistantMessage: 'Which cells need hand-verification?' + }) + }) + ) + }) +}) diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 9817519819a..55d9cd062eb 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -3203,6 +3203,7 @@ export function useIpcEvents(): void { lastAssistantMessage: data.lastAssistantMessage, interrupted: data.interrupted, sessionBoundary: data.sessionBoundary, + turnCompletedAt: data.turnCompletedAt, // Why: same trap as interactivePrompt — this rebuild is a field whitelist, so subagent child rows vanish if omitted. subagents: data.subagents }) @@ -3406,7 +3407,7 @@ export function useIpcEvents(): void { : undefined } const applyPostCommitNotification = (): void => { - if (options?.replay !== true && statusWorktreeId) { + if (statusWorktreeId && (options?.replay !== true || resolvedPayload.state === 'working')) { // Why: local Codex/Claude hooks arrive via this main-process IPC path, not the PTY OSC fallback, so task-complete notifications must observe accepted hook state here too. const notificationPayload = typeof data.stateStartedAt === 'number' @@ -3415,7 +3416,8 @@ export function useIpcEvents(): void { observeAgentHookCompletionForNotification({ paneKey, worktreeId: statusWorktreeId, - payload: notificationPayload + payload: notificationPayload, + ...(options?.replay === true ? { seedOnly: true } : {}) }) } } diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 3060a5b34c0..adc869d8eb4 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -825,7 +825,9 @@ "linearDescription": "Orca에서 Linear가 작동하는 방식, 설정 체크리스트, 에이전트 스킬, 프롬프트 예시입니다.", "pluginsTitle": "플러그인", "pluginsDescription": "실험적인 Orca 플러그인을 설치하고 관리합니다.", - "tasksDescription": "제공업체를 연결하고 Linear 스킬을 설치한 다음 작업에 표시할 항목을 선택하세요." + "tasksDescription": "제공업체를 연결하고 Linear 스킬을 설치한 다음 작업에 표시할 항목을 선택하세요.", + "automationsTitle": "자동화", + "automationsDescription": "에이전트 작업을 예약하고 사이드바에 자동화 표시 여부를 선택합니다." }, "useAppMenuPaste": { "pasteTooLarge": "붙여넣기 내용이 너무 큽니다." @@ -10144,6 +10146,22 @@ "saveFailed": "가시성 기본값을 저장할 수 없습니다.", "updateServer": "소스 기본값을 구성하려면 이 서버를 업데이트하세요.", "updateServerDefaults": "표시 기본값을 구성하려면 이 서버를 업데이트하세요." + }, + "automations": { + "title": "자동화", + "description": "에이전트 작업을 예약하고 사이드바에 자동화 표시 여부를 선택합니다.", + "showButton": "자동화 버튼 표시", + "showButtonDescription": "사이드바에 자동화 바로 가기를 표시합니다.", + "howItWorksTitle": "자동화 작동 방식", + "howItWorksDescription": "에이전트 작업을 한 번만 예약하면 Orca가 각 실행을 생성하고 결과를 한곳에 모아 관리합니다.", + "defineStepTitle": "작업 설명", + "defineStepDescription": "프로젝트, 에이전트, 프롬프트 및 일정을 선택합니다.", + "runStepTitle": "Orca가 각 실행 시작", + "runStepDescription": "예약 시간이 되면 선택된 에이전트에게 새로운 작업 공간이 할당됩니다.", + "reviewStepTitle": "결과 검토", + "reviewStepDescription": "최근 실행을 검토하고 필요할 때 언제든지 작업을 계속할 수 있습니다.", + "openAutomations": "자동화 열기", + "openAutomationsDescription": "일정을 생성하고 최근 실행을 확인합니다." } }, "right": { diff --git a/src/renderer/src/lib/pane-manager/pane-background-compositing.test.ts b/src/renderer/src/lib/pane-manager/pane-background-compositing.test.ts new file mode 100644 index 00000000000..c7540a68c1d --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-background-compositing.test.ts @@ -0,0 +1,57 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ManagedPaneInternal } from './pane-manager-types' +import { + disposePaneTerminalBackgroundObserver, + observePaneTerminalBackground +} from './pane-background-compositing' + +const observers: { callback: MutationCallback; disconnect: ReturnType }[] = [] + +beforeEach(() => { + observers.length = 0 + vi.stubGlobal( + 'MutationObserver', + class { + readonly disconnect = vi.fn() + + constructor(readonly callback: MutationCallback) { + observers.push(this) + } + + observe(): void {} + } + ) +}) + +afterEach(() => { + vi.unstubAllGlobals() +}) + +describe('terminal padding background compositing', () => { + it('tracks live xterm background changes and releases the observer', () => { + const terminalElement = { style: { backgroundColor: 'rgba(0, 0, 0, 0.5)' } } + const setProperty = vi.fn() + const pane = { + terminal: { element: terminalElement }, + xtermContainer: { + style: { setProperty } + } + } as unknown as ManagedPaneInternal + + observePaneTerminalBackground(pane) + expect(setProperty).toHaveBeenCalledWith( + '--orca-terminal-live-background', + 'rgba(0, 0, 0, 0.5)' + ) + + terminalElement.style.backgroundColor = 'rgb(255, 0, 0)' + observers[0]?.callback([], observers[0] as unknown as MutationObserver) + expect(setProperty).toHaveBeenLastCalledWith( + '--orca-terminal-live-background', + 'rgb(255, 0, 0)' + ) + + disposePaneTerminalBackgroundObserver(pane) + expect(observers[0]?.disconnect).toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-background-compositing.ts b/src/renderer/src/lib/pane-manager/pane-background-compositing.ts new file mode 100644 index 00000000000..754315ae751 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-background-compositing.ts @@ -0,0 +1,30 @@ +import type { ManagedPaneInternal } from './pane-manager-types' + +const observers = new WeakMap() + +export function observePaneTerminalBackground(pane: ManagedPaneInternal): void { + disposePaneTerminalBackgroundObserver(pane) + const terminalElement = pane.terminal.element + if (!terminalElement) { + return + } + + let previous = '' + const sync = () => { + const background = terminalElement.style.backgroundColor + if (!background || background === previous) { + return + } + previous = background + pane.xtermContainer.style.setProperty('--orca-terminal-live-background', background) + } + const observer = new MutationObserver(sync) + observer.observe(terminalElement, { attributes: true, attributeFilter: ['style'] }) + observers.set(pane, observer) + sync() +} + +export function disposePaneTerminalBackgroundObserver(pane: ManagedPaneInternal): void { + observers.get(pane)?.disconnect() + observers.delete(pane) +} diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts index 9cb442a2073..89544daafca 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts @@ -497,6 +497,7 @@ describe('openTerminal — addon and provider wiring', () => { addEventListener: vi.fn(), removeEventListener: vi.fn(), querySelector: vi.fn(() => null), + style: { backgroundColor: '' }, classList: { contains: vi.fn(() => false) } } as unknown as HTMLElement vi.stubGlobal( diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts index 3c89a6ec723..079925d1826 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts @@ -20,6 +20,10 @@ import { rebuildAttachedWebgl } from './pane-webgl-reattach' import { configureLazyArabicShapingJoiner } from './terminal-arabic-shaping-joiner' import { TerminalLigaturesAddon } from './terminal-ligatures-addon' import { installTerminalImeCandidateAnchor } from './terminal-ime-candidate-anchor' +import { + disposePaneTerminalBackgroundObserver, + observePaneTerminalBackground +} from './pane-background-compositing' // --------------------------------------------------------------------------- // Pane creation, terminal open/close, addon management @@ -44,6 +48,7 @@ export function openTerminal(pane: ManagedPaneInternal): void { // Open terminal into DOM terminal.open(xtermContainer) + observePaneTerminalBackground(pane) // Why: terminal.element sits under the padded xterm container. Pane-level // placement keeps the hover URL on the true bottom-left window corner. container.appendChild(linkTooltip) @@ -175,6 +180,7 @@ export function disposePane( pane.pendingInitialFitRafId = null } cancelPendingWebglRefresh(pane) + disposePaneTerminalBackgroundObserver(pane) detachPaneFitResizeObserver(pane) if (pane.panePointerDownHandler) { pane.container.removeEventListener('pointerdown', pane.panePointerDownHandler) diff --git a/src/renderer/src/lib/pane-manager/pane-webgl-renderer.test.ts b/src/renderer/src/lib/pane-manager/pane-webgl-renderer.test.ts index b3990f20400..ddaf330a48d 100644 --- a/src/renderer/src/lib/pane-manager/pane-webgl-renderer.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-webgl-renderer.test.ts @@ -4,6 +4,7 @@ import type { ManagedPaneInternal } from './pane-manager-types' import { attachWebgl, clearTerminalWebglAttachBackoff, + disposeWebgl, resetTerminalWebglSuggestion, resetWebglTextureAtlas } from './pane-webgl-renderer' @@ -11,7 +12,7 @@ import { notifyPaneFitSucceeded } from './pane-fit-webgl-attach-signal' import { safeFit } from './pane-fit' import { disposePane } from './pane-lifecycle' -function createPane(options: { loadAddon?: () => void } = {}): ManagedPaneInternal { +function createPane(options: { loadAddon?: (addon: unknown) => void } = {}): ManagedPaneInternal { const leafId = '22222222-2222-4222-8222-222222222222' as never return { id: 1, @@ -24,7 +25,7 @@ function createPane(options: { loadAddon?: () => void } = {}): ManagedPaneIntern loadAddon: vi.fn(options.loadAddon) } as never, container: {} as never, - xtermContainer: {} as never, + xtermContainer: { dataset: {} } as never, linkTooltip: {} as never, terminalGpuAcceleration: 'on', gpuRenderingEnabled: true, @@ -95,6 +96,69 @@ describe('terminal WebGL addon lifecycle', () => { expect(pane.webglAddon).toBeNull() }) + it('tracks the active renderer for WebGL padding compositing', () => { + const pane = createPane() + + attachWebgl(pane) + expect(pane.xtermContainer.dataset.terminalRenderer).toBe('webgl') + + disposeWebgl(pane) + expect(pane.xtermContainer.dataset.terminalRenderer).toBe('dom') + }) + + it('uses source-over alpha blending for colored translucent backgrounds', () => { + const blendFuncSeparate = vi.fn() + const gl = { + SRC_ALPHA: 1, + ONE_MINUS_SRC_ALPHA: 2, + ONE: 3, + blendFuncSeparate + } + const pane = createPane({ + loadAddon: (addon) => { + ;(addon as { _renderer: { _gl: typeof gl } })._renderer = { _gl: gl } + } + }) + + attachWebgl(pane) + + expect(blendFuncSeparate).toHaveBeenCalledWith(1, 2, 3, 2) + }) + + it('restores source-over alpha blending after WebGL context restoration', () => { + const blendFuncSeparate = vi.fn() + const canvas = new EventTarget() + const gl = { + SRC_ALPHA: 1, + ONE_MINUS_SRC_ALPHA: 2, + ONE: 3, + blendFuncSeparate + } + const pane = createPane({ + loadAddon: (addon) => { + canvas.addEventListener('webglcontextrestored', () => blendFuncSeparate(4, 5, 6, 7)) + ;(addon as { _renderer: { _canvas: EventTarget; _gl: typeof gl } })._renderer = { + _canvas: canvas, + _gl: gl + } + } + }) + + attachWebgl(pane) + blendFuncSeparate.mockClear() + canvas.dispatchEvent(new Event('webglcontextrestored')) + + expect(blendFuncSeparate).toHaveBeenCalledTimes(2) + expect(blendFuncSeparate).toHaveBeenNthCalledWith(1, 4, 5, 6, 7) + expect(blendFuncSeparate).toHaveBeenLastCalledWith(1, 2, 3, 2) + + disposeWebgl(pane) + blendFuncSeparate.mockClear() + canvas.dispatchEvent(new Event('webglcontextrestored')) + expect(blendFuncSeparate).toHaveBeenCalledOnce() + expect(blendFuncSeparate).toHaveBeenCalledWith(4, 5, 6, 7) + }) + it('disposes the previous addon before attaching a replacement', () => { const pane = createPane() attachWebgl(pane) diff --git a/src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts b/src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts index f162052cebe..488bf32c94c 100644 --- a/src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts +++ b/src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts @@ -24,6 +24,10 @@ let suggestedRendererType: 'dom' | undefined type ReleasableWebglContext = { getExtension(name: 'WEBGL_lose_context'): WEBGL_lose_context | null isContextLost?: () => boolean + blendFuncSeparate?: (srcRgb: number, dstRgb: number, srcAlpha: number, dstAlpha: number) => void + readonly SRC_ALPHA: number + readonly ONE_MINUS_SRC_ALPHA: number + readonly ONE: number } type XtermWebglAddonInternals = { @@ -33,6 +37,11 @@ type XtermWebglAddonInternals = { } } +const webglContextRestoreHandlers = new WeakMap< + WebglAddon, + { canvas: HTMLCanvasElement; handler: EventListener } +>() + export function resetTerminalWebglSuggestion(): void { // Why: toggling GPU settings should let "auto" retry WebGL after an earlier // attach failure suggested DOM rendering for this app session. Per-pane @@ -89,6 +98,7 @@ export function disposeWebgl( options?: { refreshDimensions?: boolean } ): void { cancelPendingWebglRefresh(pane) + markPaneTerminalRenderer(pane, 'dom') if (!pane.webglAddon) { return } @@ -118,6 +128,7 @@ export function disposeWebgl( } function releaseXtermWebglContext(webglAddon: ManagedPaneInternal['webglAddon']): void { + removeWebglContextRestoreHandler(webglAddon) try { // Why: xterm removes the canvas on dispose, but Windows/ANGLE can keep the // driver context alive long enough for rapid terminal activation to hit @@ -259,7 +270,10 @@ export function attachWebgl(pane: ManagedPaneInternal): void { disposeWebgl(pane, { refreshDimensions: true }) }) pane.terminal.loadAddon(addon) + normalizeWebglAlphaBlending(addon) + installWebglContextRestoreHandler(addon) pane.webglAddon = addon + markPaneTerminalRenderer(pane, 'webgl') refreshTerminalAfterWebglAttach(pane) } catch (err) { if (pane.terminalGpuAcceleration === 'auto') { @@ -278,3 +292,37 @@ export function attachWebgl(pane: ManagedPaneInternal): void { pane.webglAddon = null } } + +function normalizeWebglAlphaBlending(webglAddon: WebglAddon): void { + const gl = (webglAddon as unknown as XtermWebglAddonInternals)._renderer?._gl + // xterm's shared blendFunc squares destination alpha. Standard source-over keeps colored translucent backgrounds pixel-identical to CSS. + gl?.blendFuncSeparate?.(gl.SRC_ALPHA, gl.ONE_MINUS_SRC_ALPHA, gl.ONE, gl.ONE_MINUS_SRC_ALPHA) +} + +function installWebglContextRestoreHandler(webglAddon: WebglAddon): void { + const canvas = (webglAddon as unknown as XtermWebglAddonInternals)._renderer?._canvas + if (!canvas) { + return + } + const handler = () => normalizeWebglAlphaBlending(webglAddon) + canvas.addEventListener('webglcontextrestored', handler) + webglContextRestoreHandlers.set(webglAddon, { canvas, handler }) +} + +function removeWebglContextRestoreHandler(webglAddon: ManagedPaneInternal['webglAddon']): void { + if (!webglAddon) { + return + } + const registered = webglContextRestoreHandlers.get(webglAddon) + if (!registered) { + return + } + registered.canvas.removeEventListener('webglcontextrestored', registered.handler) + webglContextRestoreHandlers.delete(webglAddon) +} + +function markPaneTerminalRenderer(pane: ManagedPaneInternal, renderer: 'dom' | 'webgl'): void { + if (pane.xtermContainer.dataset) { + pane.xtermContainer.dataset.terminalRenderer = renderer + } +} diff --git a/src/renderer/src/lib/pane-manager/terminal-webgl-hidden-retention.test.ts b/src/renderer/src/lib/pane-manager/terminal-webgl-hidden-retention.test.ts index cfe3fba4e37..4b749c50e75 100644 --- a/src/renderer/src/lib/pane-manager/terminal-webgl-hidden-retention.test.ts +++ b/src/renderer/src/lib/pane-manager/terminal-webgl-hidden-retention.test.ts @@ -10,6 +10,7 @@ import { function createPane(withAddon = true): ManagedPaneInternal { return { + xtermContainer: { dataset: {} }, terminal: { blur: vi.fn() }, webglAddon: withAddon ? ({ dispose: vi.fn() } as unknown as ManagedPaneInternal['webglAddon']) diff --git a/src/renderer/src/runtime/web-runtime-session.ts b/src/renderer/src/runtime/web-runtime-session.ts index 0f4bae03f6a..f7c65b84c29 100644 --- a/src/renderer/src/runtime/web-runtime-session.ts +++ b/src/renderer/src/runtime/web-runtime-session.ts @@ -853,7 +853,7 @@ export async function refreshWebRuntimeSessionTabsSnapshot( // Why: eager refreshes can resolve after the user switched worktrees; update tabs without stealing focus. const patch = applyFreshWebSessionTabsSnapshot(state, snapshot, environmentId) return patch === state ? state : patch - }) + }, snapshot) } catch (error) { if (options.errorMode === 'throw') { throw error diff --git a/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts b/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts new file mode 100644 index 00000000000..124625efd17 --- /dev/null +++ b/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts @@ -0,0 +1,325 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-types' +import { makePaneKey } from '../../../shared/stable-pane-id' +import { toWebTerminalSurfaceTabId } from '../../../shared/terminal-surface-id' + +const mocks = vi.hoisted(() => ({ + observeAgentHookCompletionForNotification: vi.fn() +})) + +vi.mock('@/hooks/agent-hook-completion-notifications', () => ({ + observeAgentHookCompletionForNotification: mocks.observeAgentHookCompletionForNotification +})) + +import { useAppStore } from '@/store' +import { + createAgentCompletionCoordinator, + resetAgentCompletionCoordinatorIdentitiesForTest +} from '@/components/terminal-pane/agent-completion-coordinator' +import { dispatchTerminalNotification } from '@/components/terminal-pane/use-notification-dispatch' +import { + markRendererOwnedAgentStatusWrite, + registerRendererOwnedAgentStatusPane, + resetRendererOwnedAgentStatusPanesForTests +} from '@/components/terminal-pane/renderer-owned-agent-status-registry' +import { + applyWebSessionTabsSnapshot, + applyWebSessionTabsStorePatch, + resetWebSessionTabsSnapshotFreshnessForTests +} from './web-session-tabs-sync' + +const ENVIRONMENT_ID = 'web-env-1' +const WORKTREE_ID = 'repo::/worktree' +const HOST_TAB_ID = 'host-tab-1' +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +const NOW = 1_700_000_000_000 +const initialState = useAppStore.getInitialState() + +function makeAgentSnapshot( + snapshotVersion: number, + updatedAt: number, + turnCompletedAt?: number, + state: 'working' | 'done' = 'working' +): RuntimeMobileSessionTabsResult { + return { + worktree: WORKTREE_ID, + publicationEpoch: 'epoch-1', + snapshotVersion, + activeGroupId: 'host-group-1', + activeTabId: HOST_TAB_ID, + activeTabType: 'terminal', + tabs: [ + { + type: 'terminal', + id: `${HOST_TAB_ID}::${LEAF_ID}`, + title: 'Claude working', + parentTabId: HOST_TAB_ID, + leafId: LEAF_ID, + isActive: true, + status: 'ready', + terminal: 'terminal-1', + ...(turnCompletedAt !== undefined ? { turnCompletedAt } : {}), + agentStatus: { + state, + prompt: 'review the PR', + updatedAt, + stateStartedAt: NOW, + agentType: 'claude', + paneKey: makePaneKey(HOST_TAB_ID, LEAF_ID), + tabId: HOST_TAB_ID, + worktreeId: WORKTREE_ID, + stateHistory: [] + } + } + ] + } +} + +function applySnapshot(snapshot: RuntimeMobileSessionTabsResult, live: boolean): void { + applyWebSessionTabsStorePatch( + (state) => applyWebSessionTabsSnapshot(state, snapshot, ENVIRONMENT_ID, NOW), + snapshot, + live + ) +} + +describe('paired session-tab agent completion notifications', () => { + beforeEach(() => { + vi.useFakeTimers() + mocks.observeAgentHookCompletionForNotification.mockReset() + resetAgentCompletionCoordinatorIdentitiesForTest() + resetWebSessionTabsSnapshotFreshnessForTests() + resetRendererOwnedAgentStatusPanesForTests() + resetAgentCompletionCoordinatorIdentitiesForTest() + useAppStore.setState(initialState, true) + }) + + afterEach(() => { + useAppStore.setState(initialState, true) + resetWebSessionTabsSnapshotFreshnessForTests() + resetRendererOwnedAgentStatusPanesForTests() + vi.unstubAllGlobals() + vi.useRealTimers() + }) + + it('forwards accepted working snapshots and live statuses', () => { + applySnapshot(makeAgentSnapshot(1, NOW), false) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(1) + + applySnapshot(makeAgentSnapshot(2, NOW + 1_000), true) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(2) + + const turnCompletedAt = NOW + 2_000 + applySnapshot(makeAgentSnapshot(3, NOW + 2_000, turnCompletedAt), true) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenLastCalledWith({ + paneKey: makePaneKey(toWebTerminalSurfaceTabId(HOST_TAB_ID), LEAF_ID), + worktreeId: WORKTREE_ID, + payload: expect.objectContaining({ + state: 'working', + stateStartedAt: NOW, + turnCompletedAt + }) + }) + + applySnapshot(makeAgentSnapshot(4, NOW + 3_000, turnCompletedAt + 1_000), false) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(4) + }) + + it('announces one live stamped completion after a late-pair working seed', () => { + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: makePaneKey(toWebTerminalSurfaceTabId(HOST_TAB_ID), LEAF_ID), + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + mocks.observeAgentHookCompletionForNotification.mockImplementation(({ payload, seedOnly }) => + seedOnly ? coordinator.seedHookStatus(payload) : coordinator.observeHookStatus(payload) + ) + + applySnapshot(makeAgentSnapshot(1, NOW), false) + applySnapshot(makeAgentSnapshot(2, NOW + 1_000, NOW + 1_000), true) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + }) + + it('announces a host-stamped turn while client OSC status owns the pane', () => { + const paneKey = makePaneKey(toWebTerminalSurfaceTabId(HOST_TAB_ID), LEAF_ID) + registerRendererOwnedAgentStatusPane(paneKey, ENVIRONMENT_ID) + markRendererOwnedAgentStatusWrite(paneKey) + useAppStore.setState({ + agentStatusByPaneKey: { + [paneKey]: { + state: 'working', + prompt: 'review the PR', + updatedAt: NOW, + stateStartedAt: NOW, + agentType: 'claude', + paneKey, + tabId: toWebTerminalSurfaceTabId(HOST_TAB_ID), + worktreeId: WORKTREE_ID, + stateHistory: [] + } + } + }) + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey, + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + mocks.observeAgentHookCompletionForNotification.mockImplementation(({ payload, seedOnly }) => + seedOnly ? coordinator.seedHookStatus(payload) : coordinator.observeHookStatus(payload) + ) + + applySnapshot(makeAgentSnapshot(1, NOW - 2_000), true) + const turnCompletedAt = NOW + 20_000 + applySnapshot(makeAgentSnapshot(2, NOW - 1_000, turnCompletedAt), true) + applySnapshot(makeAgentSnapshot(3, NOW, turnCompletedAt, 'done'), true) + + expect(useAppStore.getState().agentStatusByPaneKey[paneKey]?.state).toBe('working') + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(3) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + payload: expect.objectContaining({ + localStateStartedAt: NOW, + turnCompletedAt + }) + }) + ) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + expect(dispatchCompletion).toHaveBeenCalledWith( + 'claude', + expect.objectContaining({ + source: 'hook', + quietedHookDone: false, + agentStatus: expect.objectContaining({ + state: 'done', + prompt: 'review the PR', + turnCompletedAt + }) + }) + ) + }) + + it('keeps a delayed host stamp bound to the prior client turn', () => { + const paneKey = makePaneKey(toWebTerminalSurfaceTabId(HOST_TAB_ID), LEAF_ID) + registerRendererOwnedAgentStatusPane(paneKey, ENVIRONMENT_ID) + markRendererOwnedAgentStatusWrite(paneKey) + const setClientTurn = (stateStartedAt: number, prompt: string): void => { + useAppStore.setState({ + agentStatusByPaneKey: { + [paneKey]: { + state: 'working', + prompt, + updatedAt: NOW, + stateStartedAt, + agentType: 'claude', + paneKey, + tabId: toWebTerminalSurfaceTabId(HOST_TAB_ID), + worktreeId: WORKTREE_ID, + stateHistory: [] + } + } + }) + } + + setClientTurn(5_000, 'first turn') + applySnapshot(makeAgentSnapshot(1, NOW - 2_000), true) + setClientTurn(6_000, 'second turn') + applySnapshot(makeAgentSnapshot(2, NOW - 1_500), true) + const turnCompletedAt = NOW + 20_000 + applySnapshot(makeAgentSnapshot(3, NOW - 1_000, turnCompletedAt), true) + + const delayedPayload = + mocks.observeAgentHookCompletionForNotification.mock.calls.at(-1)?.[0]?.payload + expect(delayedPayload).toMatchObject({ + localStateStartedAt: 5_000, + turnCompletedAt + }) + + const notificationDispatch = vi.fn() + vi.stubGlobal('window', { api: { notifications: { dispatch: notificationDispatch } } }) + dispatchTerminalNotification(WORKTREE_ID, { + source: 'agent-task-complete', + terminalTitle: 'Claude working', + paneKey, + agentCompletionSource: 'hook', + agentStatusSnapshot: delayedPayload + }) + + expect(notificationDispatch).not.toHaveBeenCalled() + }) + + it('suppresses a stamped reconnect tail and its live all-clear', () => { + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: makePaneKey(toWebTerminalSurfaceTabId(HOST_TAB_ID), LEAF_ID), + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + mocks.observeAgentHookCompletionForNotification.mockImplementation(({ payload, seedOnly }) => + seedOnly ? coordinator.seedHookStatus(payload) : coordinator.observeHookStatus(payload) + ) + + applySnapshot(makeAgentSnapshot(1, NOW, NOW), false) + applySnapshot(makeAgentSnapshot(2, NOW + 1_000, NOW, 'done'), true) + + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(2) + expect(dispatchCompletion).not.toHaveBeenCalled() + }) + + it('seeds an older restarted-host stamp without replaying its completion', () => { + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: makePaneKey(toWebTerminalSurfaceTabId(HOST_TAB_ID), LEAF_ID), + statusLane: 'hook', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + mocks.observeAgentHookCompletionForNotification.mockImplementation(({ payload, seedOnly }) => + seedOnly ? coordinator.seedHookStatus(payload) : coordinator.observeHookStatus(payload) + ) + const firstTurnCompletedAt = NOW + 100_000 + const secondTurnCompletedAt = NOW + 200_000 + + applySnapshot(makeAgentSnapshot(1, NOW), false) + applySnapshot(makeAgentSnapshot(2, NOW + 1_000, firstTurnCompletedAt), true) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + applySnapshot(makeAgentSnapshot(3, NOW + 2_000), true) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(3) + applySnapshot(makeAgentSnapshot(4, NOW + 3_000, secondTurnCompletedAt), true) + expect(dispatchCompletion).toHaveBeenCalledTimes(2) + applySnapshot(makeAgentSnapshot(5, NOW + 4_000), true) + + applySnapshot( + { + ...makeAgentSnapshot(1, NOW + 5_000, firstTurnCompletedAt), + publicationEpoch: 'epoch-2' + }, + false + ) + applySnapshot( + { + ...makeAgentSnapshot(2, NOW + 6_000, firstTurnCompletedAt, 'done'), + publicationEpoch: 'epoch-2' + }, + true + ) + vi.advanceTimersByTime(1_500) + + expect(dispatchCompletion).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/renderer/src/runtime/web-session-tabs-sync.test.ts b/src/renderer/src/runtime/web-session-tabs-sync.test.ts index 372c4eb4d5e..e6ddab72906 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.test.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.test.ts @@ -35,6 +35,9 @@ vi.mock('../store', () => ({ } })) +vi.mock('@/hooks/agent-hook-completion-notifications', () => ({ + observeAgentHookCompletionForNotification: vi.fn() +})) describe('applyWebSessionTabsSnapshot', () => { beforeEach(resetWebSessionTabsSyncTestState) diff --git a/src/renderer/src/runtime/web-session-tabs-sync.ts b/src/renderer/src/runtime/web-session-tabs-sync.ts index 2a5eb362dfb..4daf9d82399 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.ts @@ -6,9 +6,11 @@ import type { RuntimeRpcResponse } from '../../../shared/runtime-rpc-envelope' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' import { AGENT_STATUS_STALE_AFTER_MS, + pickParsedAgentStatusPayload, type AgentStatusEntry } from '../../../shared/agent-status-types' import { agentEntryCompletionAt } from '../../../shared/agent-completion-time' +import { normalizeTurnCompletedAtField } from '../../../shared/agent-status-field-normalization' import { agentProviderSessionsEqual } from '../../../shared/agent-session-resume' import type { RuntimeMobileSessionTabsResult, @@ -96,6 +98,7 @@ import { resetWebSessionBrowserPlacementsForTests } from './web-session-browser-placement' import { suppressE2eWebRuntimeBrowserSnapshot } from './web-runtime-browser-creation-e2e-fault' +import { observeAgentHookCompletionForNotification } from '@/hooks/agent-hook-completion-notifications' import { buildWebSessionExistingTabIndex, type WebSessionExistingTabIndex @@ -158,6 +161,17 @@ const hostSessionTabMappingKeysByEnvironmentAndWorktree = new Map< string, Map> >() +// Why: a delayed host stamp belongs to the client boundary seen with its preceding ordered frame. +const hostWorkingClientBoundaryByPaneKey = new Map< + string, + { + hostStateStartedAt: number + hostPrompt: string + clientStateStartedAt: number + stamped: boolean + } +>() +const HOST_WORKING_CLIENT_BOUNDARY_LIMIT = 512 let receivedSessionTabsFrameSequence = 0 type TerminalSurface = RuntimeMobileSessionTerminalClientTab @@ -644,6 +658,7 @@ export function resetWebSessionTabsSnapshotFreshnessForTests(): void { lastHostTerminalTabCountByWorktree.clear() hostSessionTabIdByLocalKey.clear() hostSessionTabMappingKeysByEnvironmentAndWorktree.clear() + hostWorkingClientBoundaryByPaneKey.clear() resetWebSessionBrowserPlacementsForTests() } @@ -3407,18 +3422,138 @@ function applyWebSessionTabsSnapshotOperations( } export function applyWebSessionTabsStorePatch( - buildPatch: (state: AppState) => WebSessionTabsSyncState | Partial + buildPatch: (state: AppState) => WebSessionTabsSyncState | Partial, + agentStatusSnapshots?: RuntimeMobileSessionTabsResult | readonly RuntimeMobileSessionTabsResult[], + allowCompletionNotification = false ): void { let mirroredAgentStatusChanged = false + const acceptedNotificationStatuses: { + paneKey: string + worktreeId: string + seedOnly?: true + payload: ReturnType & { + stateStartedAt: number + localStateStartedAt?: number + } + }[] = [] useAppStore.setState((state) => { const patch = buildPatch(state) mirroredAgentStatusChanged = patch !== state && Object.hasOwn(patch, 'agentStatusByPaneKey') + if (agentStatusSnapshots) { + const nextAgentStatuses = patch.agentStatusByPaneKey ?? state.agentStatusByPaneKey + const snapshots = Array.isArray(agentStatusSnapshots) + ? agentStatusSnapshots + : [agentStatusSnapshots] + for (const snapshot of snapshots) { + for (const surface of snapshot.tabs) { + if (surface.type !== 'terminal') { + continue + } + const remapped = remapHostAgentStatus(surface) + const accepted = remapped ? nextAgentStatuses[remapped.paneKey] : undefined + const turnCompletedAt = remapped + ? normalizeTurnCompletedAtField(surface.turnCompletedAt, remapped.state) + : undefined + if (remapped?.state === 'done' && turnCompletedAt === undefined) { + hostWorkingClientBoundaryByPaneKey.delete(remapped.paneKey) + } + // Why: client OSC owns display state, but only the host hook stream carries background-turn stamps. + const clientOwnedNotification = Boolean( + remapped && + isClientAuthoritativeAgentStatusPane(remapped.paneKey) && + (remapped.state === 'working' || turnCompletedAt !== undefined) + ) + if ( + !remapped || + (!clientOwnedNotification && + (!accepted || + accepted === state.agentStatusByPaneKey[remapped.paneKey] || + !agentStatusEntryEqual(accepted, remapped))) + ) { + continue + } + const notificationStatus = clientOwnedNotification ? remapped : accepted + if (!notificationStatus) { + continue + } + const currentClientStateStartedAt = clientOwnedNotification + ? state.agentStatusByPaneKey[notificationStatus.paneKey]?.stateStartedAt + : undefined + let localStateStartedAt = currentClientStateStartedAt + if ( + clientOwnedNotification && + notificationStatus.state === 'working' && + currentClientStateStartedAt !== undefined + ) { + if (turnCompletedAt === undefined) { + const retainedBoundary = hostWorkingClientBoundaryByPaneKey.get( + notificationStatus.paneKey + ) + if ( + !retainedBoundary || + retainedBoundary.hostStateStartedAt !== notificationStatus.stateStartedAt || + retainedBoundary.hostPrompt !== notificationStatus.prompt || + retainedBoundary.stamped + ) { + hostWorkingClientBoundaryByPaneKey.delete(notificationStatus.paneKey) + hostWorkingClientBoundaryByPaneKey.set(notificationStatus.paneKey, { + hostStateStartedAt: notificationStatus.stateStartedAt, + hostPrompt: notificationStatus.prompt, + clientStateStartedAt: currentClientStateStartedAt, + stamped: false + }) + if (hostWorkingClientBoundaryByPaneKey.size > HOST_WORKING_CLIENT_BOUNDARY_LIMIT) { + const oldestPaneKey = hostWorkingClientBoundaryByPaneKey.keys().next().value + if (oldestPaneKey !== undefined) { + hostWorkingClientBoundaryByPaneKey.delete(oldestPaneKey) + } + } + } + } else { + const retainedBoundary = hostWorkingClientBoundaryByPaneKey.get( + notificationStatus.paneKey + ) + if ( + retainedBoundary?.hostStateStartedAt === notificationStatus.stateStartedAt && + retainedBoundary.hostPrompt === notificationStatus.prompt + ) { + localStateStartedAt = retainedBoundary.clientStateStartedAt + retainedBoundary.stamped = true + } + } + } + if (!allowCompletionNotification && notificationStatus.state !== 'working') { + continue + } + acceptedNotificationStatuses.push({ + paneKey: notificationStatus.paneKey, + worktreeId: notificationStatus.worktreeId ?? snapshot.worktree, + ...(!allowCompletionNotification ? { seedOnly: true as const } : {}), + payload: { + ...pickParsedAgentStatusPayload({ + ...notificationStatus, + ...(turnCompletedAt !== undefined ? { turnCompletedAt } : {}) + }), + stateStartedAt: notificationStatus.stateStartedAt, + ...(clientOwnedNotification + ? { + localStateStartedAt + } + : {}) + } + }) + } + } + } return patch }) // Why: paired-web snapshots bypass setAgentStatus, so arm the stale-boundary timer explicitly like local hook events do. if (mirroredAgentStatusChanged) { useAppStore.getState().scheduleAgentStatusFreshness() } + for (const status of acceptedNotificationStatuses) { + observeAgentHookCompletionForNotification(status) + } } function loadInitialWebSessionTabs( @@ -3485,8 +3620,9 @@ function loadInitialWebSessionTabs( receivedFrames[index]! ) ) - applyWebSessionTabsStorePatch((state) => - applyFreshWebSessionTabsSnapshots(state, applicable, environmentId) + applyWebSessionTabsStorePatch( + (state) => applyFreshWebSessionTabsSnapshots(state, applicable, environmentId), + applicable ) } finally { for (const finishRecovery of finishRecoveries) { @@ -3793,8 +3929,9 @@ export function useWebSessionTabsSync(): void { batch.deferredRepairWorktrees.delete(worktreeId) } if (operations.length > 0) { - applyWebSessionTabsStorePatch((state) => - applyWebSessionTabsSnapshotOperations(state, operations) + applyWebSessionTabsStorePatch( + (state) => applyWebSessionTabsSnapshotOperations(state, operations), + operations.map(({ snapshot }) => snapshot) ) } finishVisibilityResumeBatchIfIdle(batch) @@ -4103,8 +4240,10 @@ export function useWebSessionTabsSync(): void { : [] ) if (freshSnapshots.length > 0) { - applyWebSessionTabsStorePatch((state) => - applyWebSessionTabsSnapshots(state, freshSnapshots, environmentId) + applyWebSessionTabsStorePatch( + (state) => + applyWebSessionTabsSnapshots(state, freshSnapshots, environmentId), + freshSnapshots ) } const freshSnapshotSet = new Set(freshSnapshots) @@ -4173,8 +4312,10 @@ export function useWebSessionTabsSync(): void { acceptReplayedWebSessionTabsSnapshot(environmentId, recovered.worktree) } if (shouldApplyWebSessionTabsSnapshot(recovered, environmentId)) { - applyWebSessionTabsStorePatch((state) => - applyWebSessionTabsSnapshot(state, recovered, environmentId) + applyWebSessionTabsStorePatch( + (state) => applyWebSessionTabsSnapshot(state, recovered, environmentId), + recovered, + event.type === 'updated' && !replayed ) recordVisibilityResumeSnapshot(environmentId, recovered, receivedFrame) } @@ -4296,8 +4437,11 @@ export function useWebSessionTabsSync(): void { skipWakeRespawn: shouldSkipWebRuntimeWakeTerminalRespawn(activeWorktreeId) }) if (fresh) { - applyWebSessionTabsStorePatch((state) => - applyWebSessionTabsSnapshot(state, recovered, environmentId) + const replayed = isRuntimeSubscriptionReplayResponse(response) + applyWebSessionTabsStorePatch( + (state) => applyWebSessionTabsSnapshot(state, recovered, environmentId), + recovered, + event.type === 'updated' && !replayed ) recordVisibilityResumeSnapshotRef.current(environmentId, recovered, receivedFrame) } diff --git a/src/shared/agent-hook-listener-claude-subagents.test.ts b/src/shared/agent-hook-listener-claude-subagents.test.ts index 15f0ac57252..5278247cf8d 100644 --- a/src/shared/agent-hook-listener-claude-subagents.test.ts +++ b/src/shared/agent-hook-listener-claude-subagents.test.ts @@ -37,6 +37,64 @@ describe('shared agent-hook-listener', () => { const stop = claudeEvent({ hook_event_name: 'Stop', background_tasks: [] }) expect(stop?.payload.state).toBe('done') expect(stop?.payload.subagents).toBeUndefined() + expect(stop?.payload.turnCompletedAt).toBeUndefined() + }) + + it('stamps turnCompletedAt on a gated lead Stop and repeats it on the all-clear', () => { + vi.useFakeTimers() + try { + vi.setSystemTime(1_700_000_005_000) + claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'review the PR' }) + claudeEvent({ + hook_event_name: 'SubagentStart', + agent_id: 'a1', + agent_type: 'general-purpose' + }) + const stop = claudeEvent({ + hook_event_name: 'Stop', + last_assistant_message: 'Which cells need hand-verification?', + background_tasks: [ + { + id: 'a1', + type: 'subagent', + status: 'running', + description: 'Review loop', + agent_type: 'general-purpose' + } + ] + }) + expect(stop?.payload.state).toBe('working') + expect(stop?.payload.turnCompletedAt).toBe(1_700_000_005_000) + expect(stop?.payload.lastAssistantMessage).toBe('Which cells need hand-verification?') + + vi.setSystemTime(1_700_000_055_000) + const drained = claudeEvent({ hook_event_name: 'SubagentStop', agent_id: 'a1' }) + expect(drained?.payload.state).toBe('done') + expect(drained?.payload.turnCompletedAt).toBe(1_700_000_005_000) + + claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'next turn' }) + const nextStop = claudeEvent({ hook_event_name: 'Stop', background_tasks: [] }) + expect(nextStop?.payload.state).toBe('done') + expect(nextStop?.payload.turnCompletedAt).toBeUndefined() + } finally { + vi.useRealTimers() + } + }) + + it('does not stamp an interrupted Stop even while a child still gates the pane', () => { + claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'review the PR' }) + claudeEvent({ + hook_event_name: 'SubagentStart', + agent_id: 'a1', + agent_type: 'general-purpose' + }) + const stop = claudeEvent({ + hook_event_name: 'Stop', + is_interrupt: true, + background_tasks: [{ id: 'a1', type: 'subagent', status: 'running' }] + }) + expect(stop?.payload.state).toBe('working') + expect(stop?.payload.turnCompletedAt).toBeUndefined() }) it.each([ @@ -631,8 +689,14 @@ describe('shared agent-hook-listener', () => { // Why: the lead already finished; the answer resumes the child, so the // emitted state is gated up to working only while that child still runs. - expect(clearClaudeAnsweredQuestionWait(state, PANE_KEY)).toEqual({ state: 'working' }) - expect(state.claudeLeadStateByPaneKey.get(PANE_KEY)).toEqual({ state: 'done' }) + expect(clearClaudeAnsweredQuestionWait(state, PANE_KEY)).toEqual({ + state: 'working', + turnCompletedAt: expect.any(Number) + }) + expect(state.claudeLeadStateByPaneKey.get(PANE_KEY)).toEqual({ + state: 'done', + turnCompletedAt: expect.any(Number) + }) const drained = claudeEvent({ hook_event_name: 'SubagentStop', agent_id: 'a1' }) expect(drained?.payload.state).toBe('done') diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 82c9bfc9fec..ec9a947b94c 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -156,8 +156,10 @@ export type ClaudeLeadTurnState = { waitingAgentId?: string /** Tool call that owns the wait; late completions from parallel sibling tools must not dismiss its card. */ waitingToolUseId?: string + /** End time of the lead turn closed while background inventory kept the pane `working`. Repeated on the later all-clear `done`. */ + turnCompletedAt?: number /** Lead state a child-induced wait displaced, restored when the wait clears; can't invent 'working' since the done-gate only downgrades done→working, never back. */ - stateBeforeWait?: Pick + stateBeforeWait?: Pick } type CodexLeadTurnState = { @@ -2753,7 +2755,7 @@ function clearClaudePendingWaitForAgent( export function clearClaudeAnsweredQuestionWait( state: HookListenerState, paneKey: string -): Pick { +): Pick { const lead = state.claudeLeadStateByPaneKey.get(paneKey) const restored = lead?.state === 'waiting' @@ -2768,7 +2770,15 @@ export function clearClaudeAnsweredQuestionWait( : {} ) const effectiveState = resolveClaudePaneState(state, paneKey, restored) - return effectiveState === restored.state ? restored : { state: effectiveState } + return effectiveState === restored.state + ? restored + : { + state: effectiveState, + ...(restored.interrupted ? { interrupted: true as const } : {}), + ...(restored.turnCompletedAt !== undefined + ? { turnCompletedAt: restored.turnCompletedAt } + : {}) + } } /** Re-emit the cached lead state without touching its tool/prompt caches; child churn and parallel completions must not dismiss live cards. */ @@ -2802,7 +2812,9 @@ function buildClaudeCachedLeadStatusPayload( interrupted: lead?.interrupted }), updateToolSnapshot: false, - interrupted: lead?.interrupted + interrupted: lead?.interrupted, + // Why: draining the last background child is this turn's all-clear; the stamp lets a consumer pair it with the announcement already sent. + turnCompletedAt: lead?.turnCompletedAt }) } @@ -2969,7 +2981,8 @@ function normalizeClaudeEvent( return buildClaudeStatusPayload(state, eventName, promptText, paneKey, hookPayload, { stateName: resolveClaudePaneState(state, paneKey, restored), updateToolSnapshot: true, - interrupted: restored.interrupted + interrupted: restored.interrupted, + turnCompletedAt: restored.turnCompletedAt }) } @@ -3001,17 +3014,14 @@ function normalizeClaudeEvent( ? previousLead.stateBeforeWait : { state: previousLead.state, - ...(previousLead.interrupted ? { interrupted: true as const } : {}) + ...(previousLead.interrupted ? { interrupted: true as const } : {}), + // Why: a child's permission pause displaces an already-finished lead; keep the end time so the later drain is still that turn's tail. + ...(previousLead.turnCompletedAt !== undefined + ? { turnCompletedAt: previousLead.turnCompletedAt } + : {}) } : undefined const waitingToolUseId = eventToolUseId ?? previousLead?.waitingToolUseId - state.claudeLeadStateByPaneKey.set(paneKey, { - state: reportedStateName, - ...(interrupted ? { interrupted } : {}), - ...(isWaitingInducing && eventAgentId ? { waitingAgentId: eventAgentId } : {}), - ...(isAskUserQuestionWait && waitingToolUseId !== undefined ? { waitingToolUseId } : {}), - ...(stateBeforeWait ? { stateBeforeWait } : {}) - }) if (interrupted && eventAgentId === undefined) { state.claudeRunningNonAgentTaskPaneKeys.delete(paneKey) @@ -3022,6 +3032,25 @@ function normalizeClaudeEvent( state: reportedStateName, interrupted }) + // Why: the lead already ended — the pane stays `working` only because background inventory is still registered. `stateStartedAt` is pinned for that whole run, so this end time is the per-turn identity and the later all-clear's pair key. + const turnCompletedAt = + eventAgentId === undefined && + isTurnBoundary && + reportedStateName === 'done' && + effectiveState === 'working' && + interrupted !== true + ? Date.now() + : undefined + + state.claudeLeadStateByPaneKey.set(paneKey, { + state: reportedStateName, + ...(interrupted ? { interrupted } : {}), + ...(isWaitingInducing && eventAgentId ? { waitingAgentId: eventAgentId } : {}), + ...(isAskUserQuestionWait && waitingToolUseId !== undefined ? { waitingToolUseId } : {}), + ...(stateBeforeWait ? { stateBeforeWait } : {}), + ...(turnCompletedAt !== undefined ? { turnCompletedAt } : {}) + }) + const effectiveRoster = state.claudeSubagentRosterByPaneKey.get(paneKey) if ( isTurnBoundary && @@ -3039,7 +3068,8 @@ function normalizeClaudeEvent( return buildClaudeStatusPayload(state, eventName, promptText, paneKey, hookPayload, { stateName: effectiveState, updateToolSnapshot: true, - interrupted + interrupted, + turnCompletedAt }) } @@ -3054,6 +3084,7 @@ function buildClaudeStatusPayload( updateToolSnapshot: boolean interrupted?: boolean sessionBoundary?: boolean + turnCompletedAt?: number } ): ParsedAgentStatusPayload | null { // Why: child-driven refreshes are roster bookkeeping, not lead tool activity; read the cached snapshot without merging so they can't clear a live AskUserQuestion card or clobber the tool preview. @@ -3078,6 +3109,7 @@ function buildClaudeStatusPayload( lastAssistantMessage: snapshot.lastAssistantMessage, interrupted: options.interrupted, sessionBoundary: options.sessionBoundary, + turnCompletedAt: options.turnCompletedAt, subagents: claudeRosterToSnapshots(state.claudeSubagentRosterByPaneKey.get(paneKey)) }) } diff --git a/src/shared/agent-status-field-normalization.ts b/src/shared/agent-status-field-normalization.ts index a56d4c412b4..98b830a7375 100644 --- a/src/shared/agent-status-field-normalization.ts +++ b/src/shared/agent-status-field-normalization.ts @@ -211,3 +211,11 @@ export function normalizeOptionalMultilineField( const normalized = normalizeMultilineField(value, maxLength) return normalized.length > 0 ? normalized : undefined } + +/** Keep a lead turn's end time only on the gated `working` row and that turn's later all-clear `done`. */ +export function normalizeTurnCompletedAtField(value: unknown, state: string): number | undefined { + if (typeof value !== 'number' || !Number.isFinite(value)) { + return undefined + } + return state === 'working' || state === 'done' ? value : undefined +} diff --git a/src/shared/agent-status-types.test.ts b/src/shared/agent-status-types.test.ts index 1dea3dc70fe..f82932f3aeb 100644 --- a/src/shared/agent-status-types.test.ts +++ b/src/shared/agent-status-types.test.ts @@ -4,6 +4,7 @@ import { isFreshNonDoneAgentStatus, parseAgentStatusPayload, normalizeAgentStatusPayload, + pickParsedAgentStatusPayload, AGENT_STATUS_JSON_STRUCTURE_LIMITS, AGENT_STATUS_MAX_FIELD_LENGTH, AGENT_STATUS_MAX_SUBAGENTS, @@ -435,6 +436,41 @@ Fix dispatch fallback preview for normalized status prompts` ).toBeUndefined() }) + it('keeps turnCompletedAt on the gated working row and its all-clear done, nowhere else', () => { + for (const state of ['working', 'done'] as const) { + expect( + parseAgentStatusPayload(`{"state":"${state}","turnCompletedAt":1767225601000}`)! + .turnCompletedAt + ).toBe(1767225601000) + } + for (const state of ['blocked', 'waiting'] as const) { + expect( + parseAgentStatusPayload(`{"state":"${state}","turnCompletedAt":1767225601000}`)! + .turnCompletedAt + ).toBeUndefined() + } + for (const raw of ['"1767225601000"', 'null', 'true']) { + expect( + parseAgentStatusPayload(`{"state":"done","turnCompletedAt":${raw}}`)!.turnCompletedAt + ).toBeUndefined() + } + for (const value of [Number.NaN, Number.POSITIVE_INFINITY]) { + expect( + normalizeAgentStatusPayload({ state: 'done', turnCompletedAt: value })!.turnCompletedAt + ).toBeUndefined() + } + }) + + it('carries turnCompletedAt through the client-visible payload projection', () => { + expect( + pickParsedAgentStatusPayload({ + state: 'working', + prompt: 'run the build', + turnCompletedAt: 1767225601000 + }).turnCompletedAt + ).toBe(1767225601000) + }) + it('requires strict boolean true for interrupted (rejects truthy non-boolean)', () => { // Why: parser uses `=== true`, so truthy string/number sentinels don't count. expect( diff --git a/src/shared/agent-status-types.ts b/src/shared/agent-status-types.ts index 80ce1a4f823..c1e3db6e606 100644 --- a/src/shared/agent-status-types.ts +++ b/src/shared/agent-status-types.ts @@ -7,7 +7,8 @@ import { normalizeInteractivePromptField, normalizeOptionalField, normalizeOptionalMultilineField, - normalizePromptField + normalizePromptField, + normalizeTurnCompletedAtField } from './agent-status-field-normalization' import { assertJsonTextStructureWithinLimits } from './json-text-structure-limit' @@ -184,6 +185,10 @@ export type AgentStatusPayload = { * completions (notifications, automation runs, unread badges, finished timestamps) * must ignore it. Only meaningful on `done`. */ sessionBoundary?: boolean + /** Wall-clock ms when the lead turn ended while Claude background inventory kept the pane `working`. + * `stateStartedAt` stays pinned for that whole working run, so this is the per-turn identity. + * Present on the gated `working` row and that turn's later all-clear `done`. Event-only — not stored on AgentStatusEntry. */ + turnCompletedAt?: number /** Live in-process children of the reporting session. See AgentStatusEntry. */ subagents?: AgentSubagentSnapshot[] } @@ -217,6 +222,7 @@ export function pickParsedAgentStatusPayload( : {}), ...(row.interrupted !== undefined ? { interrupted: row.interrupted } : {}), ...(row.sessionBoundary !== undefined ? { sessionBoundary: row.sessionBoundary } : {}), + ...(row.turnCompletedAt !== undefined ? { turnCompletedAt: row.turnCompletedAt } : {}), ...(row.subagents !== undefined ? { subagents: row.subagents } : {}) } } @@ -417,6 +423,7 @@ function normalizeAgentStatusObject(parsed: unknown): ParsedAgentStatusPayload | // Why: only meaningful on `done`; coerce to undefined elsewhere so it can't leak stale truth across transitions. interrupted: obj.interrupted === true && state === 'done' ? true : undefined, sessionBoundary: obj.sessionBoundary === true && state === 'done' ? true : undefined, + turnCompletedAt: normalizeTurnCompletedAtField(obj.turnCompletedAt, state), subagents: normalizeSubagentsField(obj.subagents) } } diff --git a/src/shared/runtime-types.ts b/src/shared/runtime-types.ts index d6b257ca85f..69c8c972e36 100644 --- a/src/shared/runtime-types.ts +++ b/src/shared/runtime-types.ts @@ -181,6 +181,8 @@ export type RuntimeMobileSessionTerminalTab = { ptyId?: string | null terminalTheme?: RuntimeMobileTerminalTheme agentStatus?: AgentStatusEntry | null + /** Event-only lead-turn end time for paired clients; never persisted in AgentStatusEntry. */ + turnCompletedAt?: number launchAgent?: TuiAgent startupCwd?: string parentLayout?: TerminalLayoutSnapshot diff --git a/src/shared/terminal-padding-settings.test.ts b/src/shared/terminal-padding-settings.test.ts new file mode 100644 index 00000000000..ee7ac432f46 --- /dev/null +++ b/src/shared/terminal-padding-settings.test.ts @@ -0,0 +1,14 @@ +import { describe, expect, it } from 'vitest' +import { normalizeTerminalPadding } from './terminal-padding-settings' + +describe('normalizeTerminalPadding', () => { + it('rounds fractional padding to the nearest fitted pixel', () => { + expect(normalizeTerminalPadding(1.5)).toBe(2) + expect(normalizeTerminalPadding(2.49)).toBe(2) + }) + + it('keeps padding inside the settings range', () => { + expect(normalizeTerminalPadding(-1)).toBe(0) + expect(normalizeTerminalPadding(513)).toBe(512) + }) +}) diff --git a/src/shared/terminal-padding-settings.ts b/src/shared/terminal-padding-settings.ts new file mode 100644 index 00000000000..1f18a46973f --- /dev/null +++ b/src/shared/terminal-padding-settings.ts @@ -0,0 +1,6 @@ +const MIN_TERMINAL_PADDING = 0 +const MAX_TERMINAL_PADDING = 512 + +export function normalizeTerminalPadding(value: number): number { + return Math.min(MAX_TERMINAL_PADDING, Math.max(MIN_TERMINAL_PADDING, Math.round(value))) +}