Merge pull request #240 from mvanhorn/osc/85-fix-pretooluse-hook-concurrency
fix(hooks): replace sys.exit(2) with non-fatal tool blocking in PreToolUse
This commit is contained in:
@@ -202,7 +202,15 @@ function renderHookHandlers(
|
||||
const wrapped = options.requireError
|
||||
? ` if (input?.error) {\n${statements.map((line) => ` ${line}`).join("\n")}\n }`
|
||||
: rendered
|
||||
|
||||
// Wrap tool.execute.before handlers in try-catch to prevent a failing hook
|
||||
// from crashing parallel tool call batches (causes API 400 errors).
|
||||
// See: https://github.com/EveryInc/compound-engineering-plugin/issues/85
|
||||
const isPreToolUse = event === "tool.execute.before"
|
||||
const note = options.note ? ` // ${options.note}\n` : ""
|
||||
if (isPreToolUse) {
|
||||
return ` "${event}": async (input) => {\n${note} try {\n ${wrapped}\n } catch (err) {\n console.error("[hook] ${event} error (non-fatal):", err)\n }\n }`
|
||||
}
|
||||
return ` "${event}": async (input) => {\n${note}${wrapped}\n }`
|
||||
}
|
||||
|
||||
|
||||
@@ -132,6 +132,18 @@ describe("convertClaudeToOpenCode", () => {
|
||||
expect(hookFile!.content).toContain("// timeout: 30s")
|
||||
expect(hookFile!.content).toContain("// Prompt hook for Write|Edit")
|
||||
expect(hookFile!.content).toContain("// Agent hook for Write|Edit: security-sentinel")
|
||||
|
||||
// PreToolUse (tool.execute.before) handlers are wrapped in try-catch
|
||||
// to prevent hook failures from crashing parallel tool call batches (#85)
|
||||
const beforeIdx = hookFile!.content.indexOf('"tool.execute.before"')
|
||||
const afterIdx = hookFile!.content.indexOf('"tool.execute.after"')
|
||||
const beforeBlock = hookFile!.content.slice(beforeIdx, afterIdx)
|
||||
expect(beforeBlock).toContain("try {")
|
||||
expect(beforeBlock).toContain("} catch (err) {")
|
||||
|
||||
// PostToolUse (tool.execute.after) handlers are NOT wrapped in try-catch
|
||||
const afterBlock = hookFile!.content.slice(afterIdx, hookFile!.content.indexOf('"session.created"'))
|
||||
expect(afterBlock).not.toContain("try {")
|
||||
})
|
||||
|
||||
test("converts MCP servers", async () => {
|
||||
|
||||
Reference in New Issue
Block a user