mirror of
https://github.com/stablyai/orca.git
synced 2026-10-12 16:03:03 +00:00
fix(agent-hooks): preserve script exit code in wrapPosixHookCommand
The previous `[ -x ... ] && /bin/sh ... || true` form silently swallowed non-zero exits from the script itself, contradicting the doc comment. Switch to `if [ -x ... ]; then /bin/sh ...; fi` so only the missing-script case becomes a no-op and real script failures propagate. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -148,13 +148,13 @@ describe('createManagedCommandMatcher', () => {
|
||||
})
|
||||
|
||||
it('matches the guarded launcher form so wrapped commands sweep correctly', () => {
|
||||
// Why: wrapPosixHookCommand wraps the launcher in `[ -x ... ] && ... || true`
|
||||
// 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(
|
||||
'[ -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'
|
||||
'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)
|
||||
})
|
||||
@@ -163,7 +163,7 @@ describe('createManagedCommandMatcher', () => {
|
||||
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')
|
||||
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)', () => {
|
||||
|
||||
@@ -69,12 +69,12 @@ export function createManagedCommandMatcher(
|
||||
// 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
|
||||
// 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 {
|
||||
return `[ -x "${scriptPath}" ] && /bin/sh "${scriptPath}" || true`
|
||||
return `if [ -x "${scriptPath}" ]; then /bin/sh "${scriptPath}"; fi`
|
||||
}
|
||||
|
||||
export function removeManagedCommands(
|
||||
|
||||
Reference in New Issue
Block a user