From e3c5bc02a94bb3fa2e4ea69f0873b9ecfbe76bcf Mon Sep 17 00:00:00 2001 From: Orca Worker Date: Sat, 5 Sep 2026 14:37:56 -0700 Subject: [PATCH] fix(hooks): register the Claude hook script directly on Windows (#18875) The Windows Claude Code lifecycle hook was registered as `powershell.exe -NoProfile -EncodedCommand <...>` whose entire decoded payload was a `Test-Path` and a call to `~/.orca/agent-hooks/claude-hook.cmd`. Every hook event paid a full PowerShell start-up to reach a script that exits at its first `ORCA_PANE_KEY` guard, so sessions outside Orca paid it to do nothing. Register the script path itself instead, with `|| echo {}` for the neutral-JSON-when-missing contract (#14818). Measured on Windows 11, invoked as Claude Code invokes it (`printf payload | bash -c -l ""`): idle (n=12) baseline 177ms | before 471ms | after 213ms 10-way conc (n=40) -- | before 656ms | after 296ms p95 under load -- | before 696ms | after 337ms It also drops an interpreter from the chain the hook's timeout kill must tear down. Killing the hook does not kill its PowerShell grandchild, which still holds the stdout handle the agent reads to EOF -- measured, EOF arrived 352ms AFTER the kill, when the orphan exited by itself. msys2 creates children suspended and resumes them after, so a kill landing in that window strands one that never exits and EOF never comes; that is the reported frozen session. The encoded launcher stays as the fallback for profile paths the shells cannot carry bare (space, `%`, `^`, `&`, non-ASCII) and for hosts where Git Bash is not resolvable, because PowerShell 5.1 rejects `||`. Every other agent's hook is untouched, as is the remote/SSH path. Not adopted from the report: `cmd.exe /d /c ` (MSYS rewrites the `/c` under Git Bash -- measured, the invocation fails), and raising the 10s timeout (the orphan survives the kill regardless; the fast path puts the hook 30x under the budget so the kill effectively stops firing). --- docs/reference/windows-edr-posture.md | 24 ++- .../windows-direct-cmd-hook-command.test.ts | 137 ++++++++++++++++ .../windows-direct-cmd-hook-command.ts | 38 +++++ .../windows-hook-payload-delivery.test.ts | 6 + .../windows-powershell-hook-launcher.ts | 5 + src/main/claude/hook-service.test.ts | 152 ++++++++++++++++-- src/main/claude/hook-settings.ts | 35 +++- 7 files changed, 377 insertions(+), 20 deletions(-) create mode 100644 src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts create mode 100644 src/main/agent-hooks/windows-direct-cmd-hook-command.ts diff --git a/docs/reference/windows-edr-posture.md b/docs/reference/windows-edr-posture.md index 06eb2d5ff9b..d4d66fa7265 100644 --- a/docs/reference/windows-edr-posture.md +++ b/docs/reference/windows-edr-posture.md @@ -166,7 +166,8 @@ What remains is `-EncodedCommand` without the bypass: the PTY bootstraps `src/main/providers/windows-shell-args.ts`), the hook wrappers (`src/main/agent-hooks/windows-powershell-hook-launcher.ts` and its callers `src/main/agent-hooks/runtime-home-hook-command.ts`, -`src/main/agent-hooks/installer-utils.ts`, `src/main/claude/hook-settings.ts`), +`src/main/agent-hooks/installer-utils.ts`, and `src/main/claude/hook-settings.ts` +— that last one only as a *fallback* since #18875, see below), `src/main/runtime/windows-default-route-interfaces.ts`, `src/main/runtime/orchestration/setup-completion-signal.ts`, `src/shared/hermes-startup-query.ts`, and the four ex-bypass sites above. @@ -242,6 +243,27 @@ breadth: every interpreter hop between Orca and the thing the user asked for add a scored edge, which is why the shipped doctrine of #15520 and #15595 is to *shorten the interpreter chain* rather than to hide a window. +#18875 is a worked example of that doctrine. The Claude Code lifecycle hook was +registered as `powershell.exe -NoProfile -EncodedCommand <...>` whose entire +decoded payload was a `Test-Path` and a call to `~/.orca/agent-hooks/claude-hook.cmd`. +It now registers the script path itself (` || echo {}`), so `bash -> +powershell -> cmd -> curl` became `bash -> cmd -> curl` and one +`powershell.exe -EncodedCommand` per hook event — a first-class Defender alert +title — leaves the tree. The reporting box fired ~6 900 of them in five days, +70% from Claude sessions that were not running under Orca at all and whose hook +exits at its first `ORCA_PANE_KEY` guard. + +What is measured is latency and the hop count, nothing else: median 471 ms -> +213 ms per event idle, and 656 ms -> 296 ms (p95 696 ms -> 337 ms) under 10-way +concurrency, invoked as Claude Code invokes it. **No EDR verdict on either tree +was measured**, so claim the removed `-EncodedCommand` spelling and the shorter +chain, not a score. `cmd.exe` remains in the tree, spelled by MSYS's own `.cmd` +spawn rather than by us — the doc's one "unavoidable for `.cmd`/`.bat`" case, +carrying an absolute path and two literal tokens, with no caret escaping, no +encoding and no free text. The encoded launcher is still the shape for profile +paths the shells cannot carry bare (a space, `%`, `^`, `&`, non-ASCII) and for +hosts where Git Bash is not resolvable, because PowerShell 5.1 rejects `||`. + ### Computer use: screen capture, synthetic input, runtime-compiled MSIL `native/computer-use-windows/runtime.ps1` is a large PowerShell script. diff --git a/src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts b/src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts new file mode 100644 index 00000000000..a6582d5161b --- /dev/null +++ b/src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts @@ -0,0 +1,137 @@ +// Why (#18875): the registered Windows Claude hook is now the script path itself, so this file +// pins the two things that make that safe — the shape carries nothing MSYS or cmd.exe rewrites, +// and it still answers with neutral JSON when the script is gone. The live legs run the string +// through BOTH hosts Claude Code can pick, because the shape has to parse in either. +import { describe, expect, it } from 'vitest' +import { execFileSync } from 'node:child_process' +import { existsSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { WINDOWS_CMD_SAFE_PATH } from './installer-utils' +import { wrapWindowsDirectCmdHookCommand } from './windows-direct-cmd-hook-command' +import { findGitBash } from './windows-git-bash-path.test-fixture' + +const SAFE_PATH = 'C:\\Users\\alice\\.orca\\agent-hooks\\claude-hook.cmd' + +describe('wrapWindowsDirectCmdHookCommand', () => { + it('emits the script path with forward slashes and a neutral-JSON fallback', () => { + expect(wrapWindowsDirectCmdHookCommand(SAFE_PATH)).toBe( + 'C:/Users/alice/.orca/agent-hooks/claude-hook.cmd || echo {}' + ) + }) + + it('spells nothing either shell would rewrite or reinterpret', () => { + const command = wrapWindowsDirectCmdHookCommand(SAFE_PATH)! + + // Why: MSYS rewrites `/c`-shaped tokens into drive paths — a literal `cmd.exe /d /c ` + // does not survive Git Bash (measured), which is why no interpreter is spelled at all. + expect(command).not.toMatch(/ \/[a-zA-Z]+( |$)/) + expect(command).not.toMatch(/\\/) + expect(command).not.toMatch(/["']/) + expect(command).not.toMatch(/powershell|cmd\.exe|conhost/i) + // Why: `2>nul` writes a literal file named `nul` into the cwd under MSYS (measured), and no + // stderr sink parses in both hosts. The missing-script line is left on stderr deliberately. + expect(command).not.toContain('2>') + }) + + it('declines any path the shells cannot carry bare', () => { + for (const path of [ + 'C:\\Users\\Bob Smith\\.orca\\agent-hooks\\claude-hook.cmd', + 'C:\\Users\\%name%\\.orca\\agent-hooks\\claude-hook.cmd', + 'C:\\Users\\a^b\\.orca\\agent-hooks\\claude-hook.cmd', + 'C:\\Users\\a&b\\.orca\\agent-hooks\\claude-hook.cmd', + 'C:\\Users\\a(b)\\.orca\\agent-hooks\\claude-hook.cmd', + 'C:\\Users\\rené\\.orca\\agent-hooks\\claude-hook.cmd', + '/home/alice/.orca/agent-hooks/claude-hook.sh' + ]) { + expect(wrapWindowsDirectCmdHookCommand(path), path).toBeNull() + } + }) +}) + +describe.skipIf(process.platform !== 'win32')('direct hook command, run by both hook hosts', () => { + // Why: the fixture throws when Git Bash is absent, and that is a skip here, not a failure — + // a box without it never gets this command shape in the first place. + const gitBash = ((): string | null => { + try { + return findGitBash() + } catch { + return null + } + })() + + function runInCmd(command: string, cwd: string): { stdout: string; status: number } { + return runCapture('cmd.exe', ['/d', '/c', command], cwd) + } + + function runInBash(command: string, cwd: string): { stdout: string; status: number } { + return runCapture(gitBash!, ['-c', command], cwd) + } + + function runCapture(file: string, args: string[], cwd: string) { + try { + const stdout = execFileSync(file, args, { + cwd, + input: '{"hook_event_name":"PreToolUse"}', + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'] + }) + return { stdout, status: 0 } + } catch (error) { + const failure = error as { stdout?: string; status?: number } + return { stdout: failure.stdout ?? '', status: failure.status ?? 1 } + } + } + + // Why: a runner whose TEMP sits under a profile with a space is the encoded-launcher case, + // so these legs skip rather than assert a contract that shape never claimed. + const tempIsCmdSafe = WINDOWS_CMD_SAFE_PATH.test(join(tmpdir(), 'orca-direct-hook-x', 'x.cmd')) + const canRunLive = Boolean(gitBash) && tempIsCmdSafe + + function withTempDir(run: (dir: string, scriptPath: string, command: string) => void): void { + const dir = mkdtempSync(join(tmpdir(), 'orca-direct-hook-')) + try { + const scriptPath = join(dir, 'claude-hook.cmd') + const command = wrapWindowsDirectCmdHookCommand(scriptPath) + expect(command, 'precondition: temp path must be cmd-safe').not.toBeNull() + run(dir, scriptPath, command!) + } finally { + rmSync(dir, { recursive: true, force: true }) + } + } + + it.skipIf(!canRunLive)('answers {} and exit 0 in both hosts when the script exists', () => { + withTempDir((dir, scriptPath, command) => { + writeFileSync(scriptPath, '@echo off\r\necho {}\r\nexit /b 0\r\n', 'utf8') + for (const result of [runInCmd(command, dir), runInBash(command, dir)]) { + expect(result.stdout.trim()).toBe('{}') + expect(result.status).toBe(0) + } + }) + }) + + it.skipIf(!canRunLive)( + 'still answers {} and exit 0 in both hosts when the script is gone', + () => { + // Why: compat consumers require neutral JSON even with no managed script (#14818). The + // encoded launcher did this with a Test-Path; `|| echo {}` does it with no interpreter. + withTempDir((dir, scriptPath, command) => { + expect(existsSync(scriptPath)).toBe(false) + for (const result of [runInCmd(command, dir), runInBash(command, dir)]) { + expect(result.stdout.trim()).toBe('{}') + expect(result.status).toBe(0) + } + }) + } + ) + + it.skipIf(!canRunLive)('leaves no stray `nul` file behind in the working directory', () => { + // Why this is worth a test: adding `2>nul` to silence the missing-script line looks like + // tidy-up, but under MSYS it creates a real file named `nul` in the cwd — which is the + // user's repo. Measured on Windows 11. Keep stderr unredirected. + withTempDir((dir, _scriptPath, command) => { + runInBash(command, dir) + expect(readdirSync(dir)).not.toContain('nul') + }) + }) +}) diff --git a/src/main/agent-hooks/windows-direct-cmd-hook-command.ts b/src/main/agent-hooks/windows-direct-cmd-hook-command.ts new file mode 100644 index 00000000000..4de55e0f90d --- /dev/null +++ b/src/main/agent-hooks/windows-direct-cmd-hook-command.ts @@ -0,0 +1,38 @@ +import { WINDOWS_CMD_SAFE_PATH } from './installer-utils' + +/** + * Shortest launcher for a managed Windows `.cmd` hook: the script path itself (#18875). + * + * The registered hook used to be `powershell.exe -NoProfile -EncodedCommand <...>` whose + * whole payload was a `Test-Path` and a call. Measured on Windows 11, invoked as Claude + * Code invokes it (`printf payload | bash -c -l ""`), median over 12 runs: + * `bash -l -c true` 177ms, the encoded launcher 471ms, this shape 213ms. Under 10-way + * concurrency (n=40) the encoded launcher ran 656ms median / 696ms p95 against 296ms / + * 337ms here. PowerShell start-up was the whole difference, and it was paid before the + * `.cmd` could reach its `ORCA_PANE_KEY` guard — so sessions outside Orca, 70% of the + * reporter's 6949 fires, paid it to do nothing. + * + * It also removes an interpreter from the chain the hook's timeout kill has to tear down. + * Killing the hook does not kill its PowerShell grandchild, which still holds the stdout + * handle the agent reads to EOF: measured, EOF arrived 352ms *after* the kill, when the + * orphan exited on its own. An orphan that never exits (msys2 creates children suspended + * and resumes them after, so a kill landing in that window strands one) never yields EOF + * and the agent waits forever. + * + * Returns null when the caller must keep the encoded launcher instead. + */ +export function wrapWindowsDirectCmdHookCommand(scriptPath: string): string | null { + if (!WINDOWS_CMD_SAFE_PATH.test(scriptPath)) { + return null + } + // Why: forward slashes are the one separator both hosts read — bash eats `\` as an escape, + // and cmd.exe accepts `/` inside a path. Nothing here is a switch, so MSYS rewrites nothing + // (a literal `cmd.exe /d /c ` does not survive: measured, MSYS ate the `/c`). + const invocation = scriptPath.replaceAll('\\', '/') + // Why: neutral JSON when the script is missing (#14818) without an interpreter to Test-Path + // with. Valid in bash and cmd.exe alike; PowerShell 5.1 rejects `||`, which is what gates + // this shape on Git Bash being resolvable. The missing-script line stays on stderr + // unredirected: `2>nul` writes a literal `nul` file into the user's repo under MSYS, and + // no other sink parses in both hosts (measured). + return `${invocation} || echo {}` +} diff --git a/src/main/agent-hooks/windows-hook-payload-delivery.test.ts b/src/main/agent-hooks/windows-hook-payload-delivery.test.ts index 22103b178ff..e7390b5effa 100644 --- a/src/main/agent-hooks/windows-hook-payload-delivery.test.ts +++ b/src/main/agent-hooks/windows-hook-payload-delivery.test.ts @@ -32,6 +32,7 @@ vi.mock('os', async (importOriginal) => { }) import { ClaudeHookService } from '../claude/hook-service' +import { WINDOWS_CMD_SAFE_PATH } from './installer-utils' import { getConfigPath, getWindowsManagedLifecycleHook } from '../claude/hook-settings' import { findGitBash } from './windows-git-bash-path.test-fixture' @@ -166,6 +167,11 @@ describe.skipIf(process.platform !== 'win32')('Windows managed hook payload deli // Why: assert nothing about the launcher's shape here — this test's whole value is // that it fails for any launcher that loses the payload, named conhost or not. const registeredCommand = settings.hooks.PreToolUse[0].hooks[0].command + // ...with one exception: a cmd-safe profile must reach the script with no interpreter in + // front of it, or #18875's per-event PowerShell start-up has quietly come back. + if (WINDOWS_CMD_SAFE_PATH.test(join(home, '.orca', 'agent-hooks', 'claude-hook.cmd'))) { + expect(registeredCommand).not.toMatch(/powershell|-EncodedCommand/i) + } const listener = await startHookListener() server = listener.server diff --git a/src/main/agent-hooks/windows-powershell-hook-launcher.ts b/src/main/agent-hooks/windows-powershell-hook-launcher.ts index b9a9f6dd208..2cdb8c0f3fa 100644 --- a/src/main/agent-hooks/windows-powershell-hook-launcher.ts +++ b/src/main/agent-hooks/windows-powershell-hook-launcher.ts @@ -39,6 +39,11 @@ export function getWindowsPowerShellExecutablePath(): string { * Do not restore the flag to fix a console report. That trades every hook on an * AV host for a flicker. The answer is to shorten the interpreter chain — the * shipped doctrine of #15520 and #15595 — or a launcher that owns no console. + * + * #18875 took that answer for the Claude lifecycle hook, which now registers the + * managed `.cmd` path directly (`windows-direct-cmd-hook-command.ts`) and reaches + * this launcher only when the profile path is not cmd-safe or Git Bash is not + * resolvable. Every other caller still comes through here on every event. */ export const WINDOWS_POWERSHELL_HOOK_SWITCHES = '-NoProfile' diff --git a/src/main/claude/hook-service.test.ts b/src/main/claude/hook-service.test.ts index e2937015bea..cbc02aa3ac4 100644 --- a/src/main/claude/hook-service.test.ts +++ b/src/main/claude/hook-service.test.ts @@ -7,6 +7,7 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync import { tmpdir } from 'node:os' import { join } from 'node:path' import { vi, describe, expect, it } from 'vitest' +import type * as GitBashModule from '../git-bash' vi.mock('electron', () => ({ app: { @@ -14,11 +15,23 @@ vi.mock('electron', () => ({ } })) +// Why: the installed hook shape depends on whether Git Bash is resolvable on the host, so the +// install assertions below have to state which host they describe rather than inherit the box's. +const { gitBashAvailableMock } = vi.hoisted(() => ({ gitBashAvailableMock: { value: true } })) +vi.mock('../git-bash', async (importOriginal) => ({ + ...(await importOriginal()), + isGitBashAvailable: () => gitBashAvailableMock.value +})) + import type { SFTPWrapper } from 'ssh2' -import { createManagedCommandMatcher } from '../agent-hooks/installer-utils' +import { createManagedCommandMatcher, WINDOWS_CMD_SAFE_PATH } from '../agent-hooks/installer-utils' import { WINDOWS_HOOK_STDIN_DRAIN_LABEL } from '../agent-hooks/hook-stdin-contract' import { ClaudeHookService } from './hook-service' -import { getWindowsManagedLifecycleHook, OPENCLAUDE_HOOK_SETTINGS } from './hook-settings' +import { + CLAUDE_EVENTS, + getWindowsManagedLifecycleHook, + OPENCLAUDE_HOOK_SETTINGS +} from './hook-settings' const CLAUDE_SCRIPT_FILE_NAME = process.platform === 'win32' ? 'claude-hook.cmd' : 'claude-hook.sh' const STATUSLINE_SCRIPT_FILE_NAME = @@ -35,14 +48,29 @@ function hasManagedCommand(hook: TestHook, matcher: (command: string | undefined } describe('getWindowsManagedLifecycleHook', () => { - 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) + const SAFE_SCRIPT_PATH = 'C:\\Users\\alice\\.orca\\agent-hooks\\claude-hook.cmd' + const UNSAFE_SCRIPT_PATH = 'C:\\Users\\%name%\\a^b&c\\.orca\\agent-hooks\\claude-hook.cmd' + + it('registers the script itself, with no interpreter in front of it (#18875)', () => { + // Why this is the whole point: the encoded launcher spent a PowerShell start-up per hook + // event (471ms vs 201ms measured) before the .cmd could reach its ORCA_PANE_KEY guard, and + // its orphan outlived the hook's timeout kill still holding the stdout the agent reads. + const hook = getWindowsManagedLifecycleHook(SAFE_SCRIPT_PATH, { gitBashAvailable: true }) + + expect(hook.args).toBeUndefined() + expect(hook.command).toBe('C:/Users/alice/.orca/agent-hooks/claude-hook.cmd || echo {}') + expect(hook.command).not.toMatch(/powershell|-EncodedCommand|conhost/i) + // Why: Git Bash/MSYS mangles backslash paths and rewrites slash-prefixed switches. + expect(hook.command).not.toMatch(/\\/) + expect(hook.command).not.toMatch(/ \/[a-zA-Z]+( |$)/) + }) + + it('falls back to the encoded launcher when the profile path is not cmd-safe', () => { + const hook = getWindowsManagedLifecycleHook(UNSAFE_SCRIPT_PATH, { gitBashAvailable: true }) expect(hook.args).toBeUndefined() expect(hook.command).toMatch(/\/powershell\.exe -NoProfile -EncodedCommand /) - expect(hook.command).not.toContain(scriptPath) - // Why: Git Bash/MSYS mangles backslash paths and slash-prefixed switches. + expect(hook.command).not.toContain(UNSAFE_SCRIPT_PATH) expect(hook.command.replace(/-EncodedCommand \S+$/, '')).not.toMatch(/\\| \/[a-zA-Z]+( |$)/) const encoded = hook.command.match(/-EncodedCommand (\S+)$/)?.[1] @@ -51,10 +79,21 @@ describe('getWindowsManagedLifecycleHook', () => { expect(decoded).toContain('.orca\\agent-hooks\\claude-hook.cmd') }) + it('falls back to the encoded launcher when Git Bash is not resolvable', () => { + // Why: without Git Bash, Claude Code hosts the hook in PowerShell, and PowerShell 5.1 + // rejects `||` as a statement separator (measured) — every event would be a parse error. + const hook = getWindowsManagedLifecycleHook(SAFE_SCRIPT_PATH, { gitBashAvailable: false }) + + expect(hook.command).toMatch(/\/powershell\.exe -NoProfile -EncodedCommand /) + }) + 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) + for (const hook of [ + getWindowsManagedLifecycleHook(SAFE_SCRIPT_PATH, { gitBashAvailable: true }), + getWindowsManagedLifecycleHook(SAFE_SCRIPT_PATH, { gitBashAvailable: false }) + ]) { + expect(isClaudeManagedCommand(hook.command)).toBe(true) + } }) }) @@ -200,7 +239,13 @@ describe('ClaudeHookService.install', () => { const managedHook = legacyHooks.find((hook: TestHook) => hasManagedCommand(hook, isClaudeManagedCommand) ) - expect(JSON.stringify(managedHook)).not.toContain(tmpHome.replaceAll('\\', '/')) + // Why: POSIX resolves the profile at runtime (`${HOME-}`, STA-3348). Windows cannot — + // no single token expands in both Git Bash and cmd.exe — so it registers the absolute + // path, as Codex/Grok/Devin/Antigravity already do (#18875). A moved profile is caught + // by getStatus's exact match and rewritten, and `|| echo {}` keeps a stale entry neutral. + if (process.platform !== 'win32') { + expect(JSON.stringify(managedHook)).not.toContain(tmpHome.replaceAll('\\', '/')) + } expect( legacyHooks.some((hook: TestHook) => hasManagedCommand(hook, isClaudeManagedCommand)) ).toBe(true) @@ -365,7 +410,7 @@ describe('ClaudeHookService.install', () => { }) it.skipIf(process.platform !== 'win32')( - 'runs portable managed hooks through a single headless command string', + 'pins the encoded-launcher fallback for a profile path the shells cannot carry bare', () => { const tmpHome = mkdtempSync(join(tmpdir(), 'orca claude home with spaces ')) vi.stubEnv('HOME', tmpHome) @@ -397,6 +442,89 @@ describe('ClaudeHookService.install', () => { } ) + it.skipIf(process.platform !== 'win32')( + 'installs the bare script path on every event when the profile path is cmd-safe (#18875)', + () => { + const tmpHome = mkdtempSync(join(tmpdir(), 'orca-claude-direct-')) + vi.stubEnv('HOME', tmpHome) + vi.stubEnv('USERPROFILE', tmpHome) + const scriptPath = join(tmpHome, '.orca', 'agent-hooks', CLAUDE_SCRIPT_FILE_NAME) + // Why: a runner whose tmpdir carries a space (a profile-scoped TEMP) belongs to the + // fallback case above, not this one; skip rather than assert the wrong contract. + if (!WINDOWS_CMD_SAFE_PATH.test(scriptPath)) { + vi.unstubAllEnvs() + rmSync(tmpHome, { recursive: true, force: true }) + return + } + try { + expect(new ClaudeHookService().install().state).toBe('installed') + + const settings = JSON.parse( + readFileSync(join(tmpHome, '.claude', 'settings.json'), 'utf-8') + ) as { hooks: Record } + + const expected = `${scriptPath.replaceAll('\\', '/')} || echo {}` + for (const { eventName } of CLAUDE_EVENTS) { + const hook = settings.hooks[eventName]?.[0]?.hooks?.[0] + expect(hook?.args, eventName).toBeUndefined() + expect(hook?.command, eventName).toBe(expected) + } + // Why: the whole point of #18875 — no interpreter is started to reach the script. + expect(JSON.stringify(settings.hooks)).not.toMatch(/powershell|EncodedCommand/i) + expect(new ClaudeHookService().getStatus().state).toBe('installed') + } finally { + vi.unstubAllEnvs() + rmSync(tmpHome, { recursive: true, force: true }) + } + } + ) + + it.skipIf(process.platform !== 'win32')( + 'sweeps a previously installed encoded launcher on reinstall, keeping user hooks', + () => { + const tmpHome = mkdtempSync(join(tmpdir(), 'orca-claude-migrate-')) + vi.stubEnv('HOME', tmpHome) + vi.stubEnv('USERPROFILE', tmpHome) + const scriptPath = join(tmpHome, '.orca', 'agent-hooks', CLAUDE_SCRIPT_FILE_NAME) + if (!WINDOWS_CMD_SAFE_PATH.test(scriptPath)) { + vi.unstubAllEnvs() + rmSync(tmpHome, { recursive: true, force: true }) + return + } + try { + const settingsPath = join(tmpHome, '.claude', 'settings.json') + mkdirSync(join(tmpHome, '.claude'), { recursive: true }) + const stale = getWindowsManagedLifecycleHook(scriptPath, { gitBashAvailable: false }) + writeFileSync( + settingsPath, + JSON.stringify({ + hooks: { + Stop: [{ hooks: [stale] }], + PreToolUse: [{ matcher: '*', hooks: [stale] }], + UserPromptSubmit: [{ hooks: [{ type: 'command', command: 'echo mine' }] }] + } + }), + 'utf-8' + ) + + expect(new ClaudeHookService().install().state).toBe('installed') + + const settings = JSON.parse(readFileSync(settingsPath, 'utf-8')) as { + hooks: Record + } + expect(JSON.stringify(settings.hooks)).not.toContain('-EncodedCommand') + expect( + settings.hooks.UserPromptSubmit.some((definition) => + definition.hooks.some((hook) => hook.command === 'echo mine') + ) + ).toBe(true) + } finally { + vi.unstubAllEnvs() + rmSync(tmpHome, { recursive: true, force: true }) + } + } + ) + it.skipIf(process.platform !== 'win32')( 'posts from the managed .cmd via curl.exe, not a second PowerShell', () => { diff --git a/src/main/claude/hook-settings.ts b/src/main/claude/hook-settings.ts index c6cf3a9b53c..2d1cff0330a 100644 --- a/src/main/claude/hook-settings.ts +++ b/src/main/claude/hook-settings.ts @@ -14,23 +14,25 @@ import { type HooksConfig } from '../agent-hooks/installer-utils' import { wrapRuntimeHomeHookCommand } from '../agent-hooks/runtime-home-hook-command' +import { wrapWindowsDirectCmdHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' +import { isGitBashAvailable } from '../git-bash' export type ClaudeCompatibleHookSettings = { configDirName: '.claude' | '.openclaude' scriptBaseName: 'claude-hook' | 'openclaude-hook' - usesWindowsPowerShellLauncher: boolean + usesWindowsCompatLauncher: boolean } export const CLAUDE_HOOK_SETTINGS: ClaudeCompatibleHookSettings = { configDirName: '.claude', scriptBaseName: 'claude-hook', - usesWindowsPowerShellLauncher: true + usesWindowsCompatLauncher: true } export const OPENCLAUDE_HOOK_SETTINGS: ClaudeCompatibleHookSettings = { configDirName: '.openclaude', scriptBaseName: 'openclaude-hook', - usesWindowsPowerShellLauncher: false + usesWindowsCompatLauncher: false } export const CLAUDE_EVENTS = [ @@ -153,16 +155,35 @@ export function getManagedCommand( export function getManagedLifecycleHook( scriptPath: string, - settings = CLAUDE_HOOK_SETTINGS + settings = CLAUDE_HOOK_SETTINGS, + options: WindowsManagedLifecycleHookOptions = {} ): HookCommandConfig { - if (process.platform !== 'win32' || !settings.usesWindowsPowerShellLauncher) { + if (process.platform !== 'win32' || !settings.usesWindowsCompatLauncher) { return buildManagedCommandHook(getManagedCommand(scriptPath, { neutralJsonWhenMissing: true })) } - return getWindowsManagedLifecycleHook(scriptPath) + return getWindowsManagedLifecycleHook(scriptPath, options) } +export type WindowsManagedLifecycleHookOptions = { gitBashAvailable?: boolean } + // Why: some Claude-compatible consumers ignore `args`, so the invocation must be self-contained. -export function getWindowsManagedLifecycleHook(scriptPath: string): HookCommandConfig { +export function getWindowsManagedLifecycleHook( + scriptPath: string, + options: WindowsManagedLifecycleHookOptions = {} +): HookCommandConfig { + // Why (#18875): the encoded launcher spent a PowerShell start-up per hook event — 471ms + // against 213ms for the script alone — before the `.cmd` could reach its ORCA_PANE_KEY + // guard, and left a stdout-holding orphan behind when the hook timeout killed it. Take + // the direct path whenever the host that runs it can parse `||`: Claude Code hosts hooks + // in Git Bash, falling back to PowerShell only when Git Bash is absent, and PowerShell + // 5.1 rejects `||`. + const directCommand = + (options.gitBashAvailable ?? isGitBashAvailable()) + ? wrapWindowsDirectCmdHookCommand(scriptPath) + : null + if (directCommand) { + return { type: 'command', command: directCommand, timeout: MANAGED_HOOK_TIMEOUT_SECONDS } + } 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}`)