From 02ba70a847b60e59fa5cef79896415eea17a3ec0 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sun, 16 Aug 2026 20:48:26 -0700 Subject: [PATCH] fix(agent-hooks): make the Windows managed hook survive Claude-hooks-compat consumers (#14825) * fix(agent-hooks): make the Windows managed hook survive Claude-hooks-compat consumers `~/.claude/settings.json` is not read only by Claude Code. Third-party Claude-hooks-compat layers (cursor-agent, Devin) import the same file and reimplement hook execution, so Orca's entry has to survive consumers that support strictly less than the documented schema. Three separate defects came from assuming otherwise. 1. The entry depended on `args`, which a compat consumer ignores. `args` is valid Claude Code syntax, but cursor-agent spawns `command` alone -- so `conhost.exe` ran bare, which opens an interactive console that never closes. Hook payloads were typed into those stranded shells (#14815). The entry is now one self-contained `command` string that depends on nothing optional. 2. `conhost.exe --headless` never relayed anything. It implements the ConPTY server protocol, not a generic no-window wrapper: it does not wait for the hosted process and relays neither exit code nor stdout. Measured directly -- `conhost --headless cmd /c "echo X& exit /b 42"` yields empty stdout and no exit code, while the replacement returns both and waits. So every hook was fire-and-forget, and whatever it printed was discarded. Replaced with `-WindowStyle Hidden`, which suppresses the window and keeps wait/exit-code/stdout intact. 3. The hook never wrote anything to stdout. Guards exited silently and curl's output went to nul. Claude Code documents empty stdout as "no decision", but cursor-agent treats PreToolUse as a permission gate, fails to parse empty stdout as JSON, and blocks the tool call -- so every shell command in every cursor-agent session on Windows failed (#14818). The script now writes `{}` first, on both the Windows and POSIX branches, which is documented to be identical to writing nothing for real Claude Code. Gemini and Antigravity already did this. Defects 2 and 3 are causally linked: `{}` cannot reach any consumer while conhost is swallowing stdout, so neither fix works without the other. Also fixed while establishing the contract: - The launcher's own missing-script fallback returned empty stdout, reproducing #14818 whenever `~/.orca` was cleaned or an install was half-finished. It now emits `{}` too. - PowerShell serializes progress records to stderr as CLIXML when stderr is redirected; a consumer merging stderr into stdout would see those bytes before the JSON. Every encoded payload now silences progress. - `runtime-home-hook-command.ts` built its own launcher without window suppression -- exactly the drift #14815 asks to prevent. All launcher construction now goes through `windows-powershell-hook-launcher.ts`, so the switch list cannot be present in one installer and missing in another. - Renamed `usesWindowsHeadlessHook` to `usesWindowsPowerShellLauncher`; nothing is headless anymore, and the flag selects a launcher. Testing: the new regression test asserts the effect a consumer observes -- it runs the exact `command` string from settings.json through both cmd.exe and Git Bash, across the guard-exit, reached-curl, and missing-script paths, and parses stdout. Verified it fails when `conhost --headless` is reintroduced. The previous tests all asserted installer intent, which is why they passed through all three defects. * fix(agent-hooks): close hook launcher review gaps --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- config/tsconfig.cli.json | 1 + src/main/agent-hooks/installer-utils.test.ts | 68 +++++++++++++--- src/main/agent-hooks/installer-utils.ts | 16 ++-- .../managed-hook-stdin-lifecycle.test.ts | 76 +++++++++++++++++- .../agent-hooks/runtime-home-hook-command.ts | 26 +++++-- .../windows-powershell-hook-launcher.ts | 27 +++++++ src/main/claude/hook-service.test.ts | 77 +++++++++++++------ src/main/claude/hook-service.ts | 4 + src/main/claude/hook-settings.ts | 77 +++++++++++++------ .../hook-service-managed-install.test.ts | 23 ++++-- src/main/command-code/hook-service.test.ts | 7 +- src/main/cursor/hook-service.test.ts | 2 +- src/main/droid/hook-service.test.ts | 2 +- src/main/gemini/hook-service.test.ts | 2 +- src/main/grok/hook-service.test.ts | 8 +- 15 files changed, 322 insertions(+), 94 deletions(-) create mode 100644 src/main/agent-hooks/windows-powershell-hook-launcher.ts 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/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/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/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/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/grok/hook-service.test.ts b/src/main/grok/hook-service.test.ts index 9a58aabaf24..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 @@ -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,