From c241eaf526ecf12ab936143d6b7494474bb6dbee Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Thu, 7 May 2026 11:38:51 -0700 Subject: [PATCH] fix(agent-hooks): no-op stale managed hook entries instead of exiting 127 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A managed hook entry in ~/.claude/settings.json (and the codex/gemini/cursor equivalents) embeds the absolute script path under that Orca install's userData. When the script is missing — the user wiped userData, switched dev↔prod, the previous Orca instance overwrote the path with one whose script no longer exists, etc. — `/bin/sh ""` returns 127 and Claude reports `PreToolUse hook (failed) error: hook exited with code 127` on every tool call. Wrap the POSIX launcher in `[ -x "" ] && /bin/sh "" || true` so a missing/non-executable script becomes a silent exit 0. Failures inside the script itself still propagate. The existing managed-command matcher is substring-based on `agent-hooks/`, so the next install sweeps unguarded entries from older builds and replaces them with the guarded form automatically. Co-authored-by: Orca --- src/main/agent-hooks/installer-utils.test.ts | 34 +++++++++++++++++++- src/main/agent-hooks/installer-utils.ts | 12 +++++++ src/main/claude/hook-service.ts | 18 +++++++---- src/main/codex/hook-service.ts | 3 +- src/main/cursor/hook-service.ts | 3 +- src/main/gemini/hook-service.ts | 3 +- 6 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/main/agent-hooks/installer-utils.test.ts b/src/main/agent-hooks/installer-utils.test.ts index 906bceec451..05125b861f2 100644 --- a/src/main/agent-hooks/installer-utils.test.ts +++ b/src/main/agent-hooks/installer-utils.test.ts @@ -10,7 +10,12 @@ import { } from 'fs' import { tmpdir } from 'os' import { join } from 'path' -import { createManagedCommandMatcher, writeHooksJson, type HooksConfig } from './installer-utils' +import { + createManagedCommandMatcher, + wrapPosixHookCommand, + writeHooksJson, + type HooksConfig +} from './installer-utils' let tmpDir: string let configPath: string @@ -141,4 +146,31 @@ describe('createManagedCommandMatcher', () => { it('does not match hooks for a different agent', () => { expect(match('/bin/sh "/path/agent-hooks/gemini-hook.sh"')).toBe(false) }) + + it('matches the guarded launcher form so wrapped commands sweep correctly', () => { + // Why: wrapPosixHookCommand wraps the launcher in `[ -x ... ] && ... || true` + // so a stale entry no-ops instead of returning exit 127. The sweep on + // install() must still recognize the guarded form as managed, otherwise + // repeated installs would accumulate one guarded + one unguarded entry. + expect( + match( + '[ -x "/Users/alice/Library/Application Support/Orca/agent-hooks/claude-hook.sh" ] && /bin/sh "/Users/alice/Library/Application Support/Orca/agent-hooks/claude-hook.sh" || true' + ) + ).toBe(true) + }) +}) + +describe('wrapPosixHookCommand', () => { + it('produces a guarded command that no-ops when the script is missing', () => { + const cmd = wrapPosixHookCommand('/does/not/exist.sh') + expect(cmd).toBe('[ -x "/does/not/exist.sh" ] && /bin/sh "/does/not/exist.sh" || true') + }) + + it('preserves spaces in the script path (Library/Application Support case)', () => { + // Why: Electron's userData on macOS lives under "Application Support" with + // a space. The guard must keep the path quoted so `[ -x ]` and `/bin/sh` + // each see one argument. + const cmd = wrapPosixHookCommand('/Users/a/Library/Application Support/Orca/agent-hooks/x.sh') + expect(cmd).toContain('"/Users/a/Library/Application Support/Orca/agent-hooks/x.sh"') + }) }) diff --git a/src/main/agent-hooks/installer-utils.ts b/src/main/agent-hooks/installer-utils.ts index eb1d028ebff..3df19d3d3e0 100644 --- a/src/main/agent-hooks/installer-utils.ts +++ b/src/main/agent-hooks/installer-utils.ts @@ -65,6 +65,18 @@ export function createManagedCommandMatcher( } } +// Why: a stale managed hook entry (left over after the user wiped userData, +// switched dev↔prod installs, or had a partial install fail) used to fire +// `/bin/sh ""` on every tool call, which exits 127 and surfaces +// as `PreToolUse hook (failed) error: hook exited with code 127` in the agent +// transcript. Wrapping the launcher in `[ -x ... ] && ... || true` makes a +// missing/non-executable script a silent no-op so a broken install never +// poisons the user's session. Failures inside the script itself are +// unaffected — only the missing-script case short-circuits. +export function wrapPosixHookCommand(scriptPath: string): string { + return `[ -x "${scriptPath}" ] && /bin/sh "${scriptPath}" || true` +} + export function removeManagedCommands( definitions: HookDefinition[], isManagedCommand: (command: string | undefined) => boolean diff --git a/src/main/claude/hook-service.ts b/src/main/claude/hook-service.ts index a78445ec19f..d8b85e3ad1f 100644 --- a/src/main/claude/hook-service.ts +++ b/src/main/claude/hook-service.ts @@ -6,6 +6,7 @@ import { createManagedCommandMatcher, readHooksJson, removeManagedCommands, + wrapPosixHookCommand, writeHooksJson, writeManagedScript, type HookDefinition @@ -48,13 +49,16 @@ function getManagedScriptPath(): string { } function getManagedCommand(scriptPath: string): string { - // Why: on Windows, Claude Code runs hooks through Git Bash (`/usr/bin/bash`). - // A path with single backslashes (e.g. `C:\Users\…\claude-hook.cmd`) is - // interpreted by bash as a string with escape sequences, so `\U`, `\A`, etc. - // collapse and the launcher fails with `command not found`. Emit forward - // slashes — Windows accepts them in path arguments and bash leaves them - // intact, so the same JSON value works through every shell layer. - return process.platform === 'win32' ? scriptPath.replaceAll('\\', '/') : `/bin/sh "${scriptPath}"` + if (process.platform === 'win32') { + // Why: on Windows, Claude Code runs hooks through Git Bash (`/usr/bin/bash`). + // A path with single backslashes (e.g. `C:\Users\…\claude-hook.cmd`) is + // interpreted by bash as a string with escape sequences, so `\U`, `\A`, etc. + // collapse and the launcher fails with `command not found`. Emit forward + // slashes — Windows accepts them in path arguments and bash leaves them + // intact, so the same JSON value works through every shell layer. + return scriptPath.replaceAll('\\', '/') + } + return wrapPosixHookCommand(scriptPath) } function getManagedScript(): string { diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index d57555c06d5..c7ed872f32f 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -6,6 +6,7 @@ import { createManagedCommandMatcher, readHooksJson, removeManagedCommands, + wrapPosixHookCommand, writeHooksJson, writeManagedScript, type HookDefinition @@ -39,7 +40,7 @@ function getManagedScriptPath(): string { } function getManagedCommand(scriptPath: string): string { - return process.platform === 'win32' ? scriptPath : `/bin/sh "${scriptPath}"` + return process.platform === 'win32' ? scriptPath : wrapPosixHookCommand(scriptPath) } function getManagedScript(): string { diff --git a/src/main/cursor/hook-service.ts b/src/main/cursor/hook-service.ts index ed7d479c798..9d45681d5ad 100644 --- a/src/main/cursor/hook-service.ts +++ b/src/main/cursor/hook-service.ts @@ -6,6 +6,7 @@ import { createManagedCommandMatcher, readHooksJson, removeManagedCommands, + wrapPosixHookCommand, writeHooksJson, writeManagedScript, type HookDefinition @@ -51,7 +52,7 @@ function getManagedScriptPath(): string { } function getManagedCommand(scriptPath: string): string { - return process.platform === 'win32' ? scriptPath : `/bin/sh "${scriptPath}"` + return process.platform === 'win32' ? scriptPath : wrapPosixHookCommand(scriptPath) } function getManagedScript(): string { diff --git a/src/main/gemini/hook-service.ts b/src/main/gemini/hook-service.ts index 4239460bd4c..40e4fb5d2fe 100644 --- a/src/main/gemini/hook-service.ts +++ b/src/main/gemini/hook-service.ts @@ -6,6 +6,7 @@ import { createManagedCommandMatcher, readHooksJson, removeManagedCommands, + wrapPosixHookCommand, writeHooksJson, writeManagedScript, type HookDefinition @@ -37,7 +38,7 @@ function getManagedScriptPath(): string { } function getManagedCommand(scriptPath: string): string { - return process.platform === 'win32' ? scriptPath : `/bin/sh "${scriptPath}"` + return process.platform === 'win32' ? scriptPath : wrapPosixHookCommand(scriptPath) } function getManagedScript(): string {