feat(comment-checker): run checks for apply_patch edits
This commit is contained in:
@@ -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<void> {
|
||||
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))
|
||||
}
|
||||
|
||||
83
src/hooks/comment-checker/hook.apply-patch.test.ts
Normal file
83
src/hooks/comment-checker/hook.apply-patch.test.ts
Normal file
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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<void> => {
|
||||
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) {
|
||||
|
||||
Reference in New Issue
Block a user