fix: guard output.output in tool after-hooks for MCP tools (#1720)
MCP tool responses can have undefined output.output, causing TypeError crashes in tool.execute.after hooks. Changes: - comment-checker/hook.ts: guard output.output with ?? '' before toLowerCase() - edit-error-recovery/hook.ts: guard output.output with ?? '' before toLowerCase() - task-resume-info/hook.ts: extract output.output ?? '' into outputText before all string operations - Added tests for undefined output.output in edit-error-recovery and task-resume-info
This commit is contained in:
@@ -92,7 +92,7 @@ export function createCommentCheckerHooks(config?: CommentCheckerConfig) {
|
||||
const toolLower = input.tool.toLowerCase()
|
||||
|
||||
// Only skip if the output indicates a tool execution failure
|
||||
const outputLower = output.output.toLowerCase()
|
||||
const outputLower = (output.output ?? "").toLowerCase()
|
||||
const isToolFailure =
|
||||
outputLower.includes("error:") ||
|
||||
outputLower.includes("failed to") ||
|
||||
|
||||
@@ -44,7 +44,7 @@ export function createEditErrorRecoveryHook(_ctx: PluginInput) {
|
||||
) => {
|
||||
if (input.tool.toLowerCase() !== "edit") return
|
||||
|
||||
const outputLower = output.output.toLowerCase()
|
||||
const outputLower = (output.output ?? "").toLowerCase()
|
||||
const hasEditError = EDIT_ERROR_PATTERNS.some((pattern) =>
|
||||
outputLower.includes(pattern.toLowerCase())
|
||||
)
|
||||
|
||||
@@ -102,6 +102,23 @@ describe("createEditErrorRecoveryHook", () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given MCP tool with undefined output.output", () => {
|
||||
describe("#when output.output is undefined", () => {
|
||||
it("#then should not crash", async () => {
|
||||
const input = createInput("Edit")
|
||||
const output = {
|
||||
title: "Edit",
|
||||
output: undefined as unknown as string,
|
||||
metadata: {},
|
||||
}
|
||||
|
||||
await hook["tool.execute.after"](input, output)
|
||||
|
||||
expect(output.output).toBeUndefined()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given case insensitive tool name", () => {
|
||||
describe("#when tool is 'edit' lowercase", () => {
|
||||
it("#then should still detect and append reminder", async () => {
|
||||
|
||||
@@ -21,14 +21,15 @@ export function createTaskResumeInfoHook() {
|
||||
output: { title: string; output: string; metadata: unknown }
|
||||
) => {
|
||||
if (!TARGET_TOOLS.includes(input.tool)) return
|
||||
if (output.output.startsWith("Error:") || output.output.startsWith("Failed")) return
|
||||
if (output.output.includes("\nto continue:")) return
|
||||
const outputText = output.output ?? ""
|
||||
if (outputText.startsWith("Error:") || outputText.startsWith("Failed")) return
|
||||
if (outputText.includes("\nto continue:")) return
|
||||
|
||||
const sessionId = extractSessionId(output.output)
|
||||
const sessionId = extractSessionId(outputText)
|
||||
if (!sessionId) return
|
||||
|
||||
output.output =
|
||||
output.output.trimEnd() +
|
||||
outputText.trimEnd() +
|
||||
`\n\nto continue: task(session_id="${sessionId}", prompt="...")`
|
||||
}
|
||||
|
||||
|
||||
101
src/hooks/task-resume-info/index.test.ts
Normal file
101
src/hooks/task-resume-info/index.test.ts
Normal file
@@ -0,0 +1,101 @@
|
||||
import { describe, it, expect } from "bun:test"
|
||||
import { createTaskResumeInfoHook } from "./index"
|
||||
|
||||
describe("createTaskResumeInfoHook", () => {
|
||||
const hook = createTaskResumeInfoHook()
|
||||
const afterHook = hook["tool.execute.after"]
|
||||
|
||||
const createInput = (tool: string) => ({
|
||||
tool,
|
||||
sessionID: "test-session",
|
||||
callID: "test-call-id",
|
||||
})
|
||||
|
||||
describe("#given MCP tool with undefined output.output", () => {
|
||||
describe("#when tool.execute.after is called", () => {
|
||||
it("#then should not crash", async () => {
|
||||
const input = createInput("task")
|
||||
const output = {
|
||||
title: "delegate_task",
|
||||
output: undefined as unknown as string,
|
||||
metadata: {},
|
||||
}
|
||||
|
||||
await afterHook(input, output)
|
||||
|
||||
expect(output.output).toBeUndefined()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given non-target tool", () => {
|
||||
describe("#when tool is not in TARGET_TOOLS", () => {
|
||||
it("#then should not modify output", async () => {
|
||||
const input = createInput("Read")
|
||||
const output = {
|
||||
title: "Read",
|
||||
output: "some output",
|
||||
metadata: {},
|
||||
}
|
||||
|
||||
await afterHook(input, output)
|
||||
|
||||
expect(output.output).toBe("some output")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given target tool with session ID in output", () => {
|
||||
describe("#when output contains a session ID", () => {
|
||||
it("#then should append resume info", async () => {
|
||||
const input = createInput("call_omo_agent")
|
||||
const output = {
|
||||
title: "delegate_task",
|
||||
output: "Task completed.\nSession ID: ses_abc123",
|
||||
metadata: {},
|
||||
}
|
||||
|
||||
await afterHook(input, output)
|
||||
|
||||
expect(output.output).toContain("to continue:")
|
||||
expect(output.output).toContain("ses_abc123")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given target tool with error output", () => {
|
||||
describe("#when output starts with Error:", () => {
|
||||
it("#then should not modify output", async () => {
|
||||
const input = createInput("task")
|
||||
const output = {
|
||||
title: "task",
|
||||
output: "Error: something went wrong",
|
||||
metadata: {},
|
||||
}
|
||||
|
||||
await afterHook(input, output)
|
||||
|
||||
expect(output.output).toBe("Error: something went wrong")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given target tool with already-continued output", () => {
|
||||
describe("#when output already contains continuation info", () => {
|
||||
it("#then should not add duplicate", async () => {
|
||||
const input = createInput("task")
|
||||
const output = {
|
||||
title: "task",
|
||||
output:
|
||||
'Done.\nSession ID: ses_abc123\nto continue: task(session_id="ses_abc123", prompt="...")',
|
||||
metadata: {},
|
||||
}
|
||||
|
||||
await afterHook(input, output)
|
||||
|
||||
const matches = output.output.match(/to continue:/g)
|
||||
expect(matches?.length).toBe(1)
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user