Merge pull request #1907 from BowTiedSwan/fix/json-retry-loop
feat(hooks): add json-error-recovery hook to prevent infinite retry loops
This commit is contained in:
commit
f4eef9f534
@ -33,6 +33,7 @@ export const HookNameSchema = z.enum([
|
|||||||
"claude-code-hooks",
|
"claude-code-hooks",
|
||||||
"auto-slash-command",
|
"auto-slash-command",
|
||||||
"edit-error-recovery",
|
"edit-error-recovery",
|
||||||
|
"json-error-recovery",
|
||||||
"delegate-task-retry",
|
"delegate-task-retry",
|
||||||
"prometheus-md-only",
|
"prometheus-md-only",
|
||||||
"sisyphus-junior-notepad",
|
"sisyphus-junior-notepad",
|
||||||
|
|||||||
@ -29,6 +29,7 @@ export { createCategorySkillReminderHook } from "./category-skill-reminder";
|
|||||||
export { createRalphLoopHook, type RalphLoopHook } from "./ralph-loop";
|
export { createRalphLoopHook, type RalphLoopHook } from "./ralph-loop";
|
||||||
export { createAutoSlashCommandHook } from "./auto-slash-command";
|
export { createAutoSlashCommandHook } from "./auto-slash-command";
|
||||||
export { createEditErrorRecoveryHook } from "./edit-error-recovery";
|
export { createEditErrorRecoveryHook } from "./edit-error-recovery";
|
||||||
|
export { createJsonErrorRecoveryHook } from "./json-error-recovery";
|
||||||
export { createPrometheusMdOnlyHook } from "./prometheus-md-only";
|
export { createPrometheusMdOnlyHook } from "./prometheus-md-only";
|
||||||
export { createSisyphusJuniorNotepadHook } from "./sisyphus-junior-notepad";
|
export { createSisyphusJuniorNotepadHook } from "./sisyphus-junior-notepad";
|
||||||
export { createTaskResumeInfoHook } from "./task-resume-info";
|
export { createTaskResumeInfoHook } from "./task-resume-info";
|
||||||
|
|||||||
58
src/hooks/json-error-recovery/hook.ts
Normal file
58
src/hooks/json-error-recovery/hook.ts
Normal file
@ -0,0 +1,58 @@
|
|||||||
|
import type { PluginInput } from "@opencode-ai/plugin"
|
||||||
|
|
||||||
|
export const JSON_ERROR_TOOL_EXCLUDE_LIST = [
|
||||||
|
"bash",
|
||||||
|
"read",
|
||||||
|
"glob",
|
||||||
|
"grep",
|
||||||
|
"webfetch",
|
||||||
|
"look_at",
|
||||||
|
"grep_app_searchgithub",
|
||||||
|
"websearch_web_search_exa",
|
||||||
|
] as const
|
||||||
|
|
||||||
|
export const JSON_ERROR_PATTERNS = [
|
||||||
|
/json parse error/i,
|
||||||
|
/failed to parse json/i,
|
||||||
|
/invalid json/i,
|
||||||
|
/malformed json/i,
|
||||||
|
/unexpected end of json input/i,
|
||||||
|
/syntaxerror:\s*unexpected token.*json/i,
|
||||||
|
/json[^\n]*expected '\}'/i,
|
||||||
|
/json[^\n]*unexpected eof/i,
|
||||||
|
] as const
|
||||||
|
|
||||||
|
const JSON_ERROR_REMINDER_MARKER = "[JSON PARSE ERROR - IMMEDIATE ACTION REQUIRED]"
|
||||||
|
const JSON_ERROR_EXCLUDED_TOOLS = new Set<string>(JSON_ERROR_TOOL_EXCLUDE_LIST)
|
||||||
|
|
||||||
|
export const JSON_ERROR_REMINDER = `
|
||||||
|
[JSON PARSE ERROR - IMMEDIATE ACTION REQUIRED]
|
||||||
|
|
||||||
|
You sent invalid JSON arguments. The system could not parse your tool call.
|
||||||
|
STOP and do this NOW:
|
||||||
|
|
||||||
|
1. LOOK at the error message above to see what was expected vs what you sent.
|
||||||
|
2. CORRECT your JSON syntax (missing braces, unescaped quotes, trailing commas, etc).
|
||||||
|
3. RETRY the tool call with valid JSON.
|
||||||
|
|
||||||
|
DO NOT repeat the exact same invalid call.
|
||||||
|
`
|
||||||
|
|
||||||
|
export function createJsonErrorRecoveryHook(_ctx: PluginInput) {
|
||||||
|
return {
|
||||||
|
"tool.execute.after": async (
|
||||||
|
input: { tool: string; sessionID: string; callID: string },
|
||||||
|
output: { title: string; output: string; metadata: unknown }
|
||||||
|
) => {
|
||||||
|
if (JSON_ERROR_EXCLUDED_TOOLS.has(input.tool.toLowerCase())) return
|
||||||
|
if (typeof output.output !== "string") return
|
||||||
|
if (output.output.includes(JSON_ERROR_REMINDER_MARKER)) return
|
||||||
|
|
||||||
|
const hasJsonError = JSON_ERROR_PATTERNS.some((pattern) => pattern.test(output.output))
|
||||||
|
|
||||||
|
if (hasJsonError) {
|
||||||
|
output.output += `\n${JSON_ERROR_REMINDER}`
|
||||||
|
}
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
196
src/hooks/json-error-recovery/index.test.ts
Normal file
196
src/hooks/json-error-recovery/index.test.ts
Normal file
@ -0,0 +1,196 @@
|
|||||||
|
import { beforeEach, describe, expect, it } from "bun:test"
|
||||||
|
import type { PluginInput } from "@opencode-ai/plugin"
|
||||||
|
|
||||||
|
import {
|
||||||
|
createJsonErrorRecoveryHook,
|
||||||
|
JSON_ERROR_PATTERNS,
|
||||||
|
JSON_ERROR_REMINDER,
|
||||||
|
JSON_ERROR_TOOL_EXCLUDE_LIST,
|
||||||
|
} from "./index"
|
||||||
|
|
||||||
|
describe("createJsonErrorRecoveryHook", () => {
|
||||||
|
let hook: ReturnType<typeof createJsonErrorRecoveryHook>
|
||||||
|
|
||||||
|
type ToolExecuteAfterHandler = NonNullable<
|
||||||
|
ReturnType<typeof createJsonErrorRecoveryHook>["tool.execute.after"]
|
||||||
|
>
|
||||||
|
type ToolExecuteAfterInput = Parameters<ToolExecuteAfterHandler>[0]
|
||||||
|
type ToolExecuteAfterOutput = Parameters<ToolExecuteAfterHandler>[1]
|
||||||
|
|
||||||
|
const createMockPluginInput = (): PluginInput => {
|
||||||
|
return {
|
||||||
|
client: {} as PluginInput["client"],
|
||||||
|
directory: "/tmp/test",
|
||||||
|
} as PluginInput
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
hook = createJsonErrorRecoveryHook(createMockPluginInput())
|
||||||
|
})
|
||||||
|
|
||||||
|
describe("tool.execute.after", () => {
|
||||||
|
const createInput = (tool = "Edit"): ToolExecuteAfterInput => ({
|
||||||
|
tool,
|
||||||
|
sessionID: "test-session",
|
||||||
|
callID: "test-call-id",
|
||||||
|
})
|
||||||
|
|
||||||
|
const createOutput = (outputText: string): ToolExecuteAfterOutput => ({
|
||||||
|
title: "Tool Error",
|
||||||
|
output: outputText,
|
||||||
|
metadata: {},
|
||||||
|
})
|
||||||
|
|
||||||
|
const createUnknownOutput = (value: unknown): { title: string; output: unknown; metadata: Record<string, unknown> } => ({
|
||||||
|
title: "Tool Error",
|
||||||
|
output: value,
|
||||||
|
metadata: {},
|
||||||
|
})
|
||||||
|
|
||||||
|
it("appends reminder when output includes JSON parse error", async () => {
|
||||||
|
// given
|
||||||
|
const input = createInput()
|
||||||
|
const output = createOutput("JSON parse error: expected '}' in JSON body")
|
||||||
|
|
||||||
|
// when
|
||||||
|
await hook["tool.execute.after"](input, output)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(output.output).toContain(JSON_ERROR_REMINDER)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("appends reminder when output includes SyntaxError", async () => {
|
||||||
|
// given
|
||||||
|
const input = createInput()
|
||||||
|
const output = createOutput("SyntaxError: Unexpected token in JSON at position 10")
|
||||||
|
|
||||||
|
// when
|
||||||
|
await hook["tool.execute.after"](input, output)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(output.output).toContain(JSON_ERROR_REMINDER)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("does not append reminder for normal output", async () => {
|
||||||
|
// given
|
||||||
|
const input = createInput()
|
||||||
|
const output = createOutput("Task completed successfully")
|
||||||
|
|
||||||
|
// when
|
||||||
|
await hook["tool.execute.after"](input, output)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(output.output).toBe("Task completed successfully")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("does not append reminder for empty output", async () => {
|
||||||
|
// given
|
||||||
|
const input = createInput()
|
||||||
|
const output = createOutput("")
|
||||||
|
|
||||||
|
// when
|
||||||
|
await hook["tool.execute.after"](input, output)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(output.output).toBe("")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("does not append reminder for false positive non-JSON text", async () => {
|
||||||
|
// given
|
||||||
|
const input = createInput()
|
||||||
|
const output = createOutput("Template failed: expected '}' before newline")
|
||||||
|
|
||||||
|
// when
|
||||||
|
await hook["tool.execute.after"](input, output)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(output.output).toBe("Template failed: expected '}' before newline")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("does not append reminder for excluded tools", async () => {
|
||||||
|
// given
|
||||||
|
const input = createInput("Read")
|
||||||
|
const output = createOutput("JSON parse error: unexpected end of JSON input")
|
||||||
|
|
||||||
|
// when
|
||||||
|
await hook["tool.execute.after"](input, output)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(output.output).toBe("JSON parse error: unexpected end of JSON input")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("does not append reminder when reminder already exists", async () => {
|
||||||
|
// given
|
||||||
|
const input = createInput()
|
||||||
|
const output = createOutput(`JSON parse error: invalid JSON\n${JSON_ERROR_REMINDER}`)
|
||||||
|
|
||||||
|
// when
|
||||||
|
await hook["tool.execute.after"](input, output)
|
||||||
|
|
||||||
|
// then
|
||||||
|
const reminderCount = output.output.split("[JSON PARSE ERROR - IMMEDIATE ACTION REQUIRED]").length - 1
|
||||||
|
expect(reminderCount).toBe(1)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("does not append duplicate reminder on repeated execution", async () => {
|
||||||
|
// given
|
||||||
|
const input = createInput()
|
||||||
|
const output = createOutput("JSON parse error: invalid JSON arguments")
|
||||||
|
|
||||||
|
// when
|
||||||
|
await hook["tool.execute.after"](input, output)
|
||||||
|
await hook["tool.execute.after"](input, output)
|
||||||
|
|
||||||
|
// then
|
||||||
|
const reminderCount = output.output.split("[JSON PARSE ERROR - IMMEDIATE ACTION REQUIRED]").length - 1
|
||||||
|
expect(reminderCount).toBe(1)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("ignores non-string output values", async () => {
|
||||||
|
// given
|
||||||
|
const input = createInput()
|
||||||
|
const values: unknown[] = [42, null, undefined, { error: "invalid json" }]
|
||||||
|
|
||||||
|
// when
|
||||||
|
for (const value of values) {
|
||||||
|
const output = createUnknownOutput(value)
|
||||||
|
await hook["tool.execute.after"](input, output as ToolExecuteAfterOutput)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(output.output).toBe(value)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe("JSON_ERROR_PATTERNS", () => {
|
||||||
|
it("contains known parse error patterns", () => {
|
||||||
|
// given
|
||||||
|
const output = "JSON parse error: unexpected end of JSON input"
|
||||||
|
|
||||||
|
// when
|
||||||
|
const isMatched = JSON_ERROR_PATTERNS.some((pattern) => pattern.test(output))
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(isMatched).toBe(true)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe("JSON_ERROR_TOOL_EXCLUDE_LIST", () => {
|
||||||
|
it("contains content-heavy tools that should be excluded", () => {
|
||||||
|
// given
|
||||||
|
const expectedExcludedTools: Array<(typeof JSON_ERROR_TOOL_EXCLUDE_LIST)[number]> = [
|
||||||
|
"read",
|
||||||
|
"bash",
|
||||||
|
"webfetch",
|
||||||
|
]
|
||||||
|
|
||||||
|
// when
|
||||||
|
const allExpectedToolsIncluded = expectedExcludedTools.every((toolName) =>
|
||||||
|
JSON_ERROR_TOOL_EXCLUDE_LIST.includes(toolName)
|
||||||
|
)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(allExpectedToolsIncluded).toBe(true)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
6
src/hooks/json-error-recovery/index.ts
Normal file
6
src/hooks/json-error-recovery/index.ts
Normal file
@ -0,0 +1,6 @@
|
|||||||
|
export {
|
||||||
|
createJsonErrorRecoveryHook,
|
||||||
|
JSON_ERROR_TOOL_EXCLUDE_LIST,
|
||||||
|
JSON_ERROR_PATTERNS,
|
||||||
|
JSON_ERROR_REMINDER,
|
||||||
|
} from "./hook"
|
||||||
@ -14,6 +14,7 @@ import {
|
|||||||
createInteractiveBashSessionHook,
|
createInteractiveBashSessionHook,
|
||||||
createRalphLoopHook,
|
createRalphLoopHook,
|
||||||
createEditErrorRecoveryHook,
|
createEditErrorRecoveryHook,
|
||||||
|
createJsonErrorRecoveryHook,
|
||||||
createDelegateTaskRetryHook,
|
createDelegateTaskRetryHook,
|
||||||
createTaskResumeInfoHook,
|
createTaskResumeInfoHook,
|
||||||
createStartWorkHook,
|
createStartWorkHook,
|
||||||
@ -44,6 +45,7 @@ export type SessionHooks = {
|
|||||||
interactiveBashSession: ReturnType<typeof createInteractiveBashSessionHook> | null
|
interactiveBashSession: ReturnType<typeof createInteractiveBashSessionHook> | null
|
||||||
ralphLoop: ReturnType<typeof createRalphLoopHook> | null
|
ralphLoop: ReturnType<typeof createRalphLoopHook> | null
|
||||||
editErrorRecovery: ReturnType<typeof createEditErrorRecoveryHook> | null
|
editErrorRecovery: ReturnType<typeof createEditErrorRecoveryHook> | null
|
||||||
|
jsonErrorRecovery: ReturnType<typeof createJsonErrorRecoveryHook> | null
|
||||||
delegateTaskRetry: ReturnType<typeof createDelegateTaskRetryHook> | null
|
delegateTaskRetry: ReturnType<typeof createDelegateTaskRetryHook> | null
|
||||||
startWork: ReturnType<typeof createStartWorkHook> | null
|
startWork: ReturnType<typeof createStartWorkHook> | null
|
||||||
prometheusMdOnly: ReturnType<typeof createPrometheusMdOnlyHook> | null
|
prometheusMdOnly: ReturnType<typeof createPrometheusMdOnlyHook> | null
|
||||||
@ -134,6 +136,10 @@ export function createSessionHooks(args: {
|
|||||||
? safeHook("edit-error-recovery", () => createEditErrorRecoveryHook(ctx))
|
? safeHook("edit-error-recovery", () => createEditErrorRecoveryHook(ctx))
|
||||||
: null
|
: null
|
||||||
|
|
||||||
|
const jsonErrorRecovery = isHookEnabled("json-error-recovery")
|
||||||
|
? safeHook("json-error-recovery", () => createJsonErrorRecoveryHook(ctx))
|
||||||
|
: null
|
||||||
|
|
||||||
const delegateTaskRetry = isHookEnabled("delegate-task-retry")
|
const delegateTaskRetry = isHookEnabled("delegate-task-retry")
|
||||||
? safeHook("delegate-task-retry", () => createDelegateTaskRetryHook(ctx))
|
? safeHook("delegate-task-retry", () => createDelegateTaskRetryHook(ctx))
|
||||||
: null
|
: null
|
||||||
@ -170,6 +176,7 @@ export function createSessionHooks(args: {
|
|||||||
interactiveBashSession,
|
interactiveBashSession,
|
||||||
ralphLoop,
|
ralphLoop,
|
||||||
editErrorRecovery,
|
editErrorRecovery,
|
||||||
|
jsonErrorRecovery,
|
||||||
delegateTaskRetry,
|
delegateTaskRetry,
|
||||||
startWork,
|
startWork,
|
||||||
prometheusMdOnly,
|
prometheusMdOnly,
|
||||||
|
|||||||
@ -40,6 +40,7 @@ export function createToolExecuteAfterHandler(args: {
|
|||||||
await hooks.categorySkillReminder?.["tool.execute.after"]?.(input, output)
|
await hooks.categorySkillReminder?.["tool.execute.after"]?.(input, output)
|
||||||
await hooks.interactiveBashSession?.["tool.execute.after"]?.(input, output)
|
await hooks.interactiveBashSession?.["tool.execute.after"]?.(input, output)
|
||||||
await hooks.editErrorRecovery?.["tool.execute.after"]?.(input, output)
|
await hooks.editErrorRecovery?.["tool.execute.after"]?.(input, output)
|
||||||
|
await hooks.jsonErrorRecovery?.["tool.execute.after"]?.(input, output)
|
||||||
await hooks.delegateTaskRetry?.["tool.execute.after"]?.(input, output)
|
await hooks.delegateTaskRetry?.["tool.execute.after"]?.(input, output)
|
||||||
await hooks.atlasHook?.["tool.execute.after"]?.(input, output)
|
await hooks.atlasHook?.["tool.execute.after"]?.(input, output)
|
||||||
await hooks.taskResumeInfo?.["tool.execute.after"]?.(input, output)
|
await hooks.taskResumeInfo?.["tool.execute.after"]?.(input, output)
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user