From 8b44d3b7b5f3262eb32644f4e7be7b365ed65eb4 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Wed, 11 Feb 2026 15:39:15 +0900 Subject: [PATCH] fix: add null guards for output.output in tool.execute.after hooks Three hooks crashed with TypeError when MCP tools returned results where output.output is undefined. Added type guards to all affected hooks: - comment-checker/hook.ts: guard before toLowerCase() - edit-error-recovery/hook.ts: guard before toLowerCase() - task-resume-info/hook.ts: guard before startsWith()/includes()/trimEnd() - Added test for undefined output.output in edit-error-recovery Fixes #1746 --- bun.lock | 28 +++++++++---------- .../comment-checker/hook.output-guard.test.ts | 24 ++++++++++++++++ src/hooks/comment-checker/hook.ts | 2 ++ src/hooks/edit-error-recovery/hook.ts | 1 + src/hooks/edit-error-recovery/index.test.ts | 11 ++++++++ src/hooks/task-resume-info/hook.test.ts | 17 +++++++++++ src/hooks/task-resume-info/hook.ts | 1 + 7 files changed, 70 insertions(+), 14 deletions(-) create mode 100644 src/hooks/comment-checker/hook.output-guard.test.ts create mode 100644 src/hooks/task-resume-info/hook.test.ts diff --git a/bun.lock b/bun.lock index 4a416c88d..713fc4f35 100644 --- a/bun.lock +++ b/bun.lock @@ -28,13 +28,13 @@ "typescript": "^5.7.3", }, "optionalDependencies": { - "oh-my-opencode-darwin-arm64": "3.3.1", - "oh-my-opencode-darwin-x64": "3.3.1", - "oh-my-opencode-linux-arm64": "3.3.1", - "oh-my-opencode-linux-arm64-musl": "3.3.1", - "oh-my-opencode-linux-x64": "3.3.1", - "oh-my-opencode-linux-x64-musl": "3.3.1", - "oh-my-opencode-windows-x64": "3.3.1", + "oh-my-opencode-darwin-arm64": "3.5.1", + "oh-my-opencode-darwin-x64": "3.5.1", + "oh-my-opencode-linux-arm64": "3.5.1", + "oh-my-opencode-linux-arm64-musl": "3.5.1", + "oh-my-opencode-linux-x64": "3.5.1", + "oh-my-opencode-linux-x64-musl": "3.5.1", + "oh-my-opencode-windows-x64": "3.5.1", }, }, }, @@ -226,19 +226,19 @@ "object-inspect": ["object-inspect@1.13.4", "", {}, "sha512-W67iLl4J2EXEGTbfeHCffrjDfitvLANg0UlX3wFUUSTx92KXRFegMHUVgSqE+wvhAbi4WqjGg9czysTV2Epbew=="], - "oh-my-opencode-darwin-arm64": ["oh-my-opencode-darwin-arm64@3.3.1", "", { "os": "darwin", "cpu": "arm64", "bin": { "oh-my-opencode": "bin/oh-my-opencode" } }, "sha512-R+o42Km6bsIaW6D3I8uu2HCF3BjIWqa/fg38W5y4hJEOw4mL0Q7uV4R+0vtrXRHo9crXTK9ag0fqVQUm+Y6iAQ=="], + "oh-my-opencode-darwin-arm64": ["oh-my-opencode-darwin-arm64@3.5.1", "", { "os": "darwin", "cpu": "arm64", "bin": { "oh-my-opencode": "bin/oh-my-opencode" } }, "sha512-oH+c/+Z/ULIK+8T1jQFpzISHsvQPyYJfA6bceiD9sgFy1OY1NjRh4a3sFk8cXy6uRVKpivWDFOfbVTcZ2kbKWA=="], - "oh-my-opencode-darwin-x64": ["oh-my-opencode-darwin-x64@3.3.1", "", { "os": "darwin", "cpu": "x64", "bin": { "oh-my-opencode": "bin/oh-my-opencode" } }, "sha512-7VTbpR1vH3OEkoJxBKtYuxFPX8M3IbJKoeHWME9iK6FpT11W1ASsjyuhvzB1jcxSeqF8ddMnjitlG5ub6h5EVw=="], + "oh-my-opencode-darwin-x64": ["oh-my-opencode-darwin-x64@3.5.1", "", { "os": "darwin", "cpu": "x64", "bin": { "oh-my-opencode": "bin/oh-my-opencode" } }, "sha512-wnBYQ9BZBLbzgSNIJZOIJS03zf+b4trAQeYmG+yCLn8y7FWXqw1KmjJ88/bbMXTuZ4RSMKWpXb1Afgdsred+DQ=="], - "oh-my-opencode-linux-arm64": ["oh-my-opencode-linux-arm64@3.3.1", "", { "os": "linux", "cpu": "arm64", "bin": { "oh-my-opencode": "bin/oh-my-opencode" } }, "sha512-BZ/r/CFlvbOxkdZZrRoT16xFOjibRZHuwQnaE4f0JvOzgK6/HWp3zJI1+2/aX/oK5GA6lZxNWRrJC/SKUi8LEg=="], + "oh-my-opencode-linux-arm64": ["oh-my-opencode-linux-arm64@3.5.1", "", { "os": "linux", "cpu": "arm64", "bin": { "oh-my-opencode": "bin/oh-my-opencode" } }, "sha512-19KNJex1LeU/S14IsJbumOvZa9O6F7X4BLIY7MfjtHtTk0dRFL+tbbXmlafecBMigEKlLdJ+HTW3TnQgp7Ih8A=="], - "oh-my-opencode-linux-arm64-musl": ["oh-my-opencode-linux-arm64-musl@3.3.1", "", { "os": "linux", "cpu": "arm64", "bin": { "oh-my-opencode": "bin/oh-my-opencode" } }, "sha512-U90Wruf21h+CJbtcrS7MeTAc/5VOF6RI+5jr7qj/cCxjXNJtjhyJdz/maehArjtgf304+lYCM/Mh1i+G2D3YFQ=="], + "oh-my-opencode-linux-arm64-musl": ["oh-my-opencode-linux-arm64-musl@3.5.1", "", { "os": "linux", "cpu": "arm64", "bin": { "oh-my-opencode": "bin/oh-my-opencode" } }, "sha512-mCCnym3nBTJP+xzK+AS4YPFQiT2sZWmjhOhOy7PjNY6Is4jkfT1C2e9ZrIU/2VoVLV6V5q7hQGh1jgleU+FxwQ=="], - "oh-my-opencode-linux-x64": ["oh-my-opencode-linux-x64@3.3.1", "", { "os": "linux", "cpu": "x64", "bin": { "oh-my-opencode": "bin/oh-my-opencode" } }, "sha512-sYzohSNdwsAhivbXcbhPdF1qqQi2CCI7FSgbmvvfBOMyZ8HAgqOFqYW2r3GPdmtywzkjOTvCzTG56FZwEjx15w=="], + "oh-my-opencode-linux-x64": ["oh-my-opencode-linux-x64@3.5.1", "", { "os": "linux", "cpu": "x64", "bin": { "oh-my-opencode": "bin/oh-my-opencode" } }, "sha512-sDYt4adNuwb+p1RzHb7IR9zvbAnYYgZofjPvceirBorffp63f+aypYFxjFpfmbT87o/Eb/Hgzm4sHliJtd1UmQ=="], - "oh-my-opencode-linux-x64-musl": ["oh-my-opencode-linux-x64-musl@3.3.1", "", { "os": "linux", "cpu": "x64", "bin": { "oh-my-opencode": "bin/oh-my-opencode" } }, "sha512-aG5pZ4eWS0YSGUicOnjMkUPrIqQV4poYF+d9SIvrfvlaMcK6WlQn7jXzgNCwJsfGn5lyhSmjshZBEU+v79Ua3w=="], + "oh-my-opencode-linux-x64-musl": ["oh-my-opencode-linux-x64-musl@3.5.1", "", { "os": "linux", "cpu": "x64", "bin": { "oh-my-opencode": "bin/oh-my-opencode" } }, "sha512-tz/0QSS5AKIiKj6cMom5VQSnEYpMIP/SRTaP5WYNOYhnUkXMwXEncQ7FIcj2vovMCXuqA9a8ujVY0zTs7TeALw=="], - "oh-my-opencode-windows-x64": ["oh-my-opencode-windows-x64@3.3.1", "", { "os": "win32", "cpu": "x64", "bin": { "oh-my-opencode": "bin/oh-my-opencode.exe" } }, "sha512-FGH7cnzBqNwjSkzCDglMsVttaq+MsykAxa7ehaFK+0dnBZArvllS3W13a3dGaANHMZzfK0vz8hNDUdVi7Z63cA=="], + "oh-my-opencode-windows-x64": ["oh-my-opencode-windows-x64@3.5.1", "", { "os": "win32", "cpu": "x64", "bin": { "oh-my-opencode": "bin/oh-my-opencode.exe" } }, "sha512-zfpRS6HIkSwE8btajJzSYxhqsE5kDkop896/XGS3LLIAAZt0RtCmT3C1plxVfI9oAABfgcaiveCxJ5f9AlKPcQ=="], "on-finished": ["on-finished@2.4.1", "", { "dependencies": { "ee-first": "1.1.1" } }, "sha512-oVlzkg3ENAhCk2zdv7IJwd/QUD4z2RxRwpkcGY8psCVcCYZNq4wYnVWALHM+brtuJjePWiYF/ClmuDr8Ch5+kg=="], diff --git a/src/hooks/comment-checker/hook.output-guard.test.ts b/src/hooks/comment-checker/hook.output-guard.test.ts new file mode 100644 index 000000000..498e87ab9 --- /dev/null +++ b/src/hooks/comment-checker/hook.output-guard.test.ts @@ -0,0 +1,24 @@ +import { describe, it, expect, mock } from "bun:test" + +mock.module("./cli-runner", () => ({ + initializeCommentCheckerCli: () => {}, + getCommentCheckerCliPathPromise: () => Promise.resolve("/tmp/fake-comment-checker"), + isCliPathUsable: () => true, + processWithCli: async () => {}, + processApplyPatchEditsWithCli: async () => {}, +})) + +const { createCommentCheckerHooks } = await import("./hook") + +describe("comment-checker output guard", () => { + //#given output.output is undefined + //#when tool.execute.after is called + //#then should return without throwing + it("should not throw when output.output is undefined", async () => { + const hooks = createCommentCheckerHooks() + const input = { tool: "Write", sessionID: "ses_test", callID: "call_test" } + const output = { title: "ok", output: undefined as unknown as string, metadata: {} } + + await expect(hooks["tool.execute.after"](input, output)).resolves.toBeUndefined() + }) +}) diff --git a/src/hooks/comment-checker/hook.ts b/src/hooks/comment-checker/hook.ts index 898937643..19e43e29d 100644 --- a/src/hooks/comment-checker/hook.ts +++ b/src/hooks/comment-checker/hook.ts @@ -89,6 +89,8 @@ export function createCommentCheckerHooks(config?: CommentCheckerConfig) { ): Promise => { debugLog("tool.execute.after:", { tool: input.tool, callID: input.callID }) + if (!output.output || typeof output.output !== "string") return + const toolLower = input.tool.toLowerCase() // Only skip if the output indicates a tool execution failure diff --git a/src/hooks/edit-error-recovery/hook.ts b/src/hooks/edit-error-recovery/hook.ts index 84ac9e9dc..4a7fe3384 100644 --- a/src/hooks/edit-error-recovery/hook.ts +++ b/src/hooks/edit-error-recovery/hook.ts @@ -43,6 +43,7 @@ export function createEditErrorRecoveryHook(_ctx: PluginInput) { output: { title: string; output: string; metadata: unknown } ) => { if (input.tool.toLowerCase() !== "edit") return + if (!output.output || typeof output.output !== "string") return const outputLower = output.output.toLowerCase() const hasEditError = EDIT_ERROR_PATTERNS.some((pattern) => diff --git a/src/hooks/edit-error-recovery/index.test.ts b/src/hooks/edit-error-recovery/index.test.ts index bafe9311e..9f82598b6 100644 --- a/src/hooks/edit-error-recovery/index.test.ts +++ b/src/hooks/edit-error-recovery/index.test.ts @@ -21,6 +21,17 @@ describe("createEditErrorRecoveryHook", () => { metadata: {}, }) + describe("#given output.output is undefined", () => { + //#when tool.execute.after is called + //#then should return without throwing + it("#then should not throw", async () => { + const input = createInput("Edit") + const output = { title: "Edit", output: undefined as unknown as string, metadata: {} } + + await expect(hook["tool.execute.after"](input, output)).resolves.toBeUndefined() + }) + }) + describe("#given Edit tool with oldString/newString same error", () => { describe("#when the error message is detected", () => { it("#then should append the recovery reminder", async () => { diff --git a/src/hooks/task-resume-info/hook.test.ts b/src/hooks/task-resume-info/hook.test.ts new file mode 100644 index 000000000..42776cfe6 --- /dev/null +++ b/src/hooks/task-resume-info/hook.test.ts @@ -0,0 +1,17 @@ +import { describe, it, expect } from "bun:test" +import { createTaskResumeInfoHook } from "./hook" + +describe("createTaskResumeInfoHook", () => { + describe("tool.execute.after", () => { + //#given output.output is undefined + //#when tool.execute.after is called + //#then should return without throwing + it("should not throw when output.output is undefined", async () => { + const hook = createTaskResumeInfoHook() + const input = { tool: "Task", sessionID: "ses_test", callID: "call_test" } + const output = { title: "Result", output: undefined as unknown as string, metadata: {} } + + await expect(hook["tool.execute.after"](input, output)).resolves.toBeUndefined() + }) + }) +}) diff --git a/src/hooks/task-resume-info/hook.ts b/src/hooks/task-resume-info/hook.ts index f5c0c5185..946f9439a 100644 --- a/src/hooks/task-resume-info/hook.ts +++ b/src/hooks/task-resume-info/hook.ts @@ -21,6 +21,7 @@ export function createTaskResumeInfoHook() { output: { title: string; output: string; metadata: unknown } ) => { if (!TARGET_TOOLS.includes(input.tool)) return + if (!output.output || typeof output.output !== "string") return if (output.output.startsWith("Error:") || output.output.startsWith("Failed")) return if (output.output.includes("\nto continue:")) return