fix(agent-hooks): silence exit-127 spam from stale managed hook entries (#1536)

Co-authored-by: Orca <help@stably.ai>

src/main/agent-hooks/installer-utils.test.ts

@@ -6,11 +6,18 @@ import {
   readFileSync,
   readdirSync,
   rmSync,
-  writeFileSync
+  writeFileSync,
+  chmodSync
 } from 'fs'
 import { tmpdir } from 'os'
 import { join } from 'path'
-import { createManagedCommandMatcher, writeHooksJson, type HooksConfig } from './installer-utils'
+import { spawnSync } from 'child_process'
+import {
+  createManagedCommandMatcher,
+  wrapPosixHookCommand,
+  writeHooksJson,
+  type HooksConfig
+} from './installer-utils'
 
 let tmpDir: string
 let configPath: string
@@ -141,4 +148,66 @@ 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 `if [ -x ... ]; then ...; fi`
+    // 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(
+        'if [ -x "/Users/alice/Library/Application Support/Orca/agent-hooks/claude-hook.sh" ]; then /bin/sh "/Users/alice/Library/Application Support/Orca/agent-hooks/claude-hook.sh"; fi'
+      )
+    ).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("if [ -x '/does/not/exist.sh' ]; then /bin/sh '/does/not/exist.sh'; fi")
+  })
+
+  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'")
+  })
+
+  it('escapes embedded single quotes so the wrapped command stays well-formed', () => {
+    // Why: POSIX single-quote escape renders ' as '\''. Verify a path with an
+    // embedded quote does not break out of the quoting and instead reaches
+    // /bin/sh as a single argument.
+    const cmd = wrapPosixHookCommand("/path/with'quote/x.sh")
+    expect(cmd).toBe(
+      "if [ -x '/path/with'\\''quote/x.sh' ]; then /bin/sh '/path/with'\\''quote/x.sh'; fi"
+    )
+  })
+
+  it.skipIf(process.platform === 'win32')(
+    'returns exit code 0 when the script does not exist (no-op)',
+    () => {
+      const cmd = wrapPosixHookCommand('/does/not/exist.sh')
+      const result = spawnSync('/bin/sh', ['-c', cmd])
+      expect(result.status).toBe(0)
+    }
+  )
+
+  // Why: commit 4d618795 explicitly switched from `&& ... || true` (which
+  // swallowed non-zero exits) to `if ... then ... fi` (which preserves the
+  // script's exit code). This test guards against a future regression that
+  // re-introduces the swallowing form.
+  it.skipIf(process.platform === 'win32')(
+    'propagates the script exit code when the script runs and fails',
+    () => {
+      const scriptPath = join(tmpDir, 'fails.sh')
+      writeFileSync(scriptPath, '#!/bin/sh\nexit 7\n', 'utf-8')
+      chmodSync(scriptPath, 0o755)
+      const cmd = wrapPosixHookCommand(scriptPath)
+      const result = spawnSync('/bin/sh', ['-c', cmd])
+      expect(result.status).toBe(7)
+    }
+  )
 })

src/main/agent-hooks/installer-utils.ts

@@ -65,6 +65,22 @@ 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 "<missing path>"` 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 `if [ -x ... ]; then ...; fi` 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 {
+  // Why: POSIX single-quote escape so $, `, ", and \ in scriptPath are taken
+  // literally — avoids a shell-injection footgun if a future caller passes an
+  // arbitrary path.
+  const quoted = `'${scriptPath.replaceAll("'", "'\\''")}'`
+  return `if [ -x ${quoted} ]; then /bin/sh ${quoted}; fi`
+}
+
 export function removeManagedCommands(
   definitions: HookDefinition[],
   isManagedCommand: (command: string | undefined) => boolean

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 {

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 {

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 {

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 {