diff --git a/src/hooks/comment-checker/cli-runner.ts b/src/hooks/comment-checker/cli-runner.ts index 8721836e..b546125f 100644 --- a/src/hooks/comment-checker/cli-runner.ts +++ b/src/hooks/comment-checker/cli-runner.ts @@ -58,6 +58,45 @@ export async function processWithCli( } } +export interface ApplyPatchEdit { + filePath: string + before: string + after: string +} + +export async function processApplyPatchEditsWithCli( + sessionID: string, + edits: ApplyPatchEdit[], + output: { output: string }, + cliPath: string, + customPrompt: string | undefined, + debugLog: (...args: unknown[]) => void, +): Promise { + debugLog("processing apply_patch edits:", edits.length) + + for (const edit of edits) { + const hookInput: HookInput = { + session_id: sessionID, + tool_name: "Edit", + transcript_path: "", + cwd: process.cwd(), + hook_event_name: "PostToolUse", + tool_input: { + file_path: edit.filePath, + old_string: edit.before, + new_string: edit.after, + }, + } + + const result = await runCommentChecker(hookInput, cliPath, customPrompt) + + if (result.hasComments && result.message) { + debugLog("CLI detected comments for apply_patch file:", edit.filePath) + output.output += `\n\n${result.message}` + } + } +} + export function isCliPathUsable(cliPath: string | null): cliPath is string { return Boolean(cliPath && existsSync(cliPath)) } diff --git a/src/hooks/comment-checker/hook.apply-patch.test.ts b/src/hooks/comment-checker/hook.apply-patch.test.ts new file mode 100644 index 00000000..ec1b4cd8 --- /dev/null +++ b/src/hooks/comment-checker/hook.apply-patch.test.ts @@ -0,0 +1,83 @@ +import { describe, it, expect, mock, beforeEach } from "bun:test" + +const processApplyPatchEditsWithCli = mock(async () => {}) + +mock.module("./cli-runner", () => ({ + initializeCommentCheckerCli: () => {}, + getCommentCheckerCliPathPromise: () => Promise.resolve("/tmp/fake-comment-checker"), + isCliPathUsable: () => true, + processWithCli: async () => {}, + processApplyPatchEditsWithCli, +})) + +const { createCommentCheckerHooks } = await import("./hook") + +describe("comment-checker apply_patch integration", () => { + beforeEach(() => { + processApplyPatchEditsWithCli.mockClear() + }) + + it("runs comment checker using apply_patch metadata.files", async () => { + // given + const hooks = createCommentCheckerHooks() + + const input = { tool: "apply_patch", sessionID: "ses_test", callID: "call_test" } + const output = { + title: "ok", + output: "Success. Updated the following files:\nM src/a.ts", + metadata: { + files: [ + { + filePath: "/repo/src/a.ts", + before: "const a = 1\n", + after: "// comment\nconst a = 1\n", + type: "update", + }, + { + filePath: "/repo/src/old.ts", + movePath: "/repo/src/new.ts", + before: "const b = 1\n", + after: "// moved comment\nconst b = 1\n", + type: "move", + }, + { + filePath: "/repo/src/delete.ts", + before: "// deleted\n", + after: "", + type: "delete", + }, + ], + }, + } + + // when + await hooks["tool.execute.after"](input, output) + + // then + expect(processApplyPatchEditsWithCli).toHaveBeenCalledTimes(1) + expect(processApplyPatchEditsWithCli).toHaveBeenCalledWith( + "ses_test", + [ + { filePath: "/repo/src/a.ts", before: "const a = 1\n", after: "// comment\nconst a = 1\n" }, + { filePath: "/repo/src/new.ts", before: "const b = 1\n", after: "// moved comment\nconst b = 1\n" }, + ], + expect.any(Object), + "/tmp/fake-comment-checker", + undefined, + expect.any(Function), + ) + }) + + it("skips when apply_patch metadata.files is missing", async () => { + // given + const hooks = createCommentCheckerHooks() + const input = { tool: "apply_patch", sessionID: "ses_test", callID: "call_test" } + const output = { title: "ok", output: "ok", metadata: {} } + + // when + await hooks["tool.execute.after"](input, output) + + // then + expect(processApplyPatchEditsWithCli).toHaveBeenCalledTimes(0) + }) +}) diff --git a/src/hooks/comment-checker/hook.ts b/src/hooks/comment-checker/hook.ts index 79d147b3..89893764 100644 --- a/src/hooks/comment-checker/hook.ts +++ b/src/hooks/comment-checker/hook.ts @@ -1,7 +1,15 @@ import type { PendingCall } from "./types" import type { CommentCheckerConfig } from "../../config/schema" -import { initializeCommentCheckerCli, getCommentCheckerCliPathPromise, isCliPathUsable, processWithCli } from "./cli-runner" +import z from "zod" + +import { + initializeCommentCheckerCli, + getCommentCheckerCliPathPromise, + isCliPathUsable, + processWithCli, + processApplyPatchEditsWithCli, +} from "./cli-runner" import { registerPendingCall, startPendingCallCleanup, takePendingCall } from "./pending-calls" import * as fs from "fs" @@ -81,13 +89,7 @@ export function createCommentCheckerHooks(config?: CommentCheckerConfig) { ): Promise => { debugLog("tool.execute.after:", { tool: input.tool, callID: input.callID }) - const pendingCall = takePendingCall(input.callID) - if (!pendingCall) { - debugLog("no pendingCall found for:", input.callID) - return - } - - debugLog("processing pendingCall:", pendingCall) + const toolLower = input.tool.toLowerCase() // Only skip if the output indicates a tool execution failure const outputLower = output.output.toLowerCase() @@ -102,17 +104,75 @@ export function createCommentCheckerHooks(config?: CommentCheckerConfig) { return } - try { - // Wait for CLI path resolution - const cliPath = await getCommentCheckerCliPathPromise() + const ApplyPatchMetadataSchema = z.object({ + files: z.array( + z.object({ + filePath: z.string(), + movePath: z.string().optional(), + before: z.string(), + after: z.string(), + type: z.string().optional(), + }), + ), + }) + if (toolLower === "apply_patch") { + const parsed = ApplyPatchMetadataSchema.safeParse(output.metadata) + if (!parsed.success) { + debugLog("apply_patch metadata schema mismatch, skipping") + return + } + + const edits = parsed.data.files + .filter((f) => f.type !== "delete") + .map((f) => ({ + filePath: f.movePath ?? f.filePath, + before: f.before, + after: f.after, + })) + + if (edits.length === 0) { + debugLog("apply_patch had no editable files, skipping") + return + } + + try { + const cliPath = await getCommentCheckerCliPathPromise() + if (!isCliPathUsable(cliPath)) { + debugLog("CLI not available, skipping comment check") + return + } + + debugLog("using CLI for apply_patch:", cliPath) + await processApplyPatchEditsWithCli( + input.sessionID, + edits, + output, + cliPath, + config?.custom_prompt, + debugLog, + ) + } catch (err) { + debugLog("apply_patch comment check failed:", err) + } + return + } + + const pendingCall = takePendingCall(input.callID) + if (!pendingCall) { + debugLog("no pendingCall found for:", input.callID) + return + } + + debugLog("processing pendingCall:", pendingCall) + + try { + const cliPath = await getCommentCheckerCliPathPromise() if (!isCliPathUsable(cliPath)) { - // CLI not available - silently skip comment checking debugLog("CLI not available, skipping comment check") return } - // CLI mode only debugLog("using CLI:", cliPath) await processWithCli(input, pendingCall, output, cliPath, config?.custom_prompt, debugLog) } catch (err) {