mirror of
https://github.com/stablyai/orca.git
synced 2026-10-12 16:03:03 +00:00
fix(agent-hooks): no-op stale managed hook entries instead of exiting 127
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 "<missing>"` returns 127 and Claude reports `PreToolUse hook (failed) error: hook exited with code 127` on every tool call. Wrap the POSIX launcher in `[ -x "<path>" ] && /bin/sh "<path>" || 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/<file>`, so the next install sweeps unguarded entries from older builds and replaces them with the guarded form automatically. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -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"')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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 "<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 `[ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user