From 7028c1f40a523e8899d2a27072bb802bc97f4e7b Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Thu, 21 May 2026 01:17:04 +0900 Subject: [PATCH] refactor(packages): extract comment-checker-core package --- .../package-layering-refactor/learnings.md | 19 ++ bun.lock | 7 + package.json | 6 +- packages/comment-checker-core/index.d.ts | 1 + packages/comment-checker-core/package.json | 18 ++ .../src/apply-patch-edits.ts | 164 +++++++++++++++ packages/comment-checker-core/src/index.ts | 30 +++ packages/comment-checker-core/src/runner.ts | 119 +++++++++++ packages/comment-checker-core/src/types.ts | 114 ++++++++++ packages/comment-checker-core/tsconfig.json | 14 ++ .../comment-checker/apply-patch-edits.ts | 197 ++---------------- src/hooks/comment-checker/cli.ts | 174 +++------------- src/hooks/comment-checker/hook.ts | 2 +- src/hooks/comment-checker/types.ts | 52 ++--- 14 files changed, 562 insertions(+), 355 deletions(-) create mode 100644 packages/comment-checker-core/index.d.ts create mode 100644 packages/comment-checker-core/package.json create mode 100644 packages/comment-checker-core/src/apply-patch-edits.ts create mode 100644 packages/comment-checker-core/src/index.ts create mode 100644 packages/comment-checker-core/src/runner.ts create mode 100644 packages/comment-checker-core/src/types.ts create mode 100644 packages/comment-checker-core/tsconfig.json diff --git a/.omo/notepads/package-layering-refactor/learnings.md b/.omo/notepads/package-layering-refactor/learnings.md index 704cfa256..717921004 100644 --- a/.omo/notepads/package-layering-refactor/learnings.md +++ b/.omo/notepads/package-layering-refactor/learnings.md @@ -34,3 +34,22 @@ - Core should own `buildSgArgs()` + `runSg()` orchestration and error mapping. - OMO-specific binary resolution stays adapter-side (`getAstGrepPath` in `cli-binary-path-resolution.ts`). - OMO-specific process spawn stays adapter-side (`bun-spawn-shim.ts`), injected via core deps (`spawnProcess`). + +## [2026-05-21T00:00:00Z] Task 8 (worktree) +- Created `packages/comment-checker-core/` with `package.json`, `tsconfig.json`, `index.d.ts`, and `src/` barrel. +- Moved pure apply-patch parser + metadata parsing into `packages/comment-checker-core/src/apply-patch-edits.ts`. +- Moved shared comment-checker types into `packages/comment-checker-core/src/types.ts`. +- Added injectable pure runner in `packages/comment-checker-core/src/runner.ts`: + - `resolveCommentCheckerBinary()` + - `runCommentChecker()` with injected `spawn`, `existsSync`, and timer functions. +- Kept OMO-specific adapter pieces in place (`hook.ts`, `pending-calls.ts`, `initialization-gate.ts`, `downloader.ts`). +- Added per-file shims at original locations: + - `src/hooks/comment-checker/apply-patch-edits.ts` + - `src/hooks/comment-checker/types.ts` +- Updated `src/hooks/comment-checker/cli.ts` to keep Bun spawn glue locally while delegating pure runner + resolver to core package. +- Updated `src/hooks/comment-checker/hook.ts` to import `extractApplyPatchEdits` from `@oh-my-opencode/comment-checker-core`. +- Updated root workspace wiring (`package.json` workspaces, devDependency, typecheck:packages) and ran `bun install`. +- Verification: + - `bun run typecheck` exit 0 + - `bun test` 7312/1/2/7315 (baseline-matching drift) + - `bun run build` exit 0 diff --git a/bun.lock b/bun.lock index 965a4269a..8ecdd20a2 100644 --- a/bun.lock +++ b/bun.lock @@ -26,6 +26,7 @@ "devDependencies": { "@oh-my-opencode/ast-grep-core": "workspace:*", "@oh-my-opencode/ast-grep-mcp": "workspace:*", + "@oh-my-opencode/comment-checker-core": "workspace:*", "@oh-my-opencode/rules-core": "workspace:*", "@oh-my-opencode/utils": "workspace:*", "@types/js-yaml": "^4.0.9", @@ -70,6 +71,10 @@ "bun-types": "1.3.12", }, }, + "packages/comment-checker-core": { + "name": "@oh-my-opencode/comment-checker-core", + "version": "0.1.0", + }, "packages/rules-core": { "name": "@oh-my-opencode/rules-core", "version": "0.1.0", @@ -161,6 +166,8 @@ "@oh-my-opencode/ast-grep-mcp": ["@oh-my-opencode/ast-grep-mcp@workspace:packages/ast-grep-mcp"], + "@oh-my-opencode/comment-checker-core": ["@oh-my-opencode/comment-checker-core@workspace:packages/comment-checker-core"], + "@oh-my-opencode/rules-core": ["@oh-my-opencode/rules-core@workspace:packages/rules-core"], "@oh-my-opencode/utils": ["@oh-my-opencode/utils@workspace:packages/utils"], diff --git a/package.json b/package.json index 96d5d593b..d4ca6eb4e 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,8 @@ "packages/rules-core", "packages/ast-grep-core", "packages/ast-grep-mcp", - "packages/utils" + "packages/utils", + "packages/comment-checker-core" ], "bin": { "oh-my-opencode": "bin/oh-my-opencode.js", @@ -44,7 +45,7 @@ "prepublishOnly": "bun run clean && bun run build:lsp-tools-mcp && bun run build", "test:model-capabilities": "bun test src/shared/model-capability-aliases.test.ts src/shared/model-capability-guardrails.test.ts src/shared/model-capabilities.test.ts src/cli/doctor/checks/model-resolution.test.ts --bail", "typecheck": "tsgo --noEmit && bun run typecheck:packages", - "typecheck:packages": "tsgo --noEmit -p packages/rules-core/tsconfig.json && tsgo --noEmit -p packages/ast-grep-core/tsconfig.json && tsgo --noEmit -p packages/ast-grep-mcp/tsconfig.json && tsgo --noEmit -p packages/utils/tsconfig.json", + "typecheck:packages": "tsgo --noEmit -p packages/rules-core/tsconfig.json && tsgo --noEmit -p packages/ast-grep-core/tsconfig.json && tsgo --noEmit -p packages/ast-grep-mcp/tsconfig.json && tsgo --noEmit -p packages/utils/tsconfig.json && tsgo --noEmit -p packages/comment-checker-core/tsconfig.json", "typecheck:script": "tsgo --noEmit -p script/tsconfig.json", "test": "bun test", "build:ast-grep-mcp": "bun run --cwd packages/ast-grep-mcp build" @@ -90,6 +91,7 @@ "devDependencies": { "@oh-my-opencode/ast-grep-core": "workspace:*", "@oh-my-opencode/ast-grep-mcp": "workspace:*", + "@oh-my-opencode/comment-checker-core": "workspace:*", "@oh-my-opencode/rules-core": "workspace:*", "@oh-my-opencode/utils": "workspace:*", "@typescript/native-preview": "7.0.0-dev.20260518.1", diff --git a/packages/comment-checker-core/index.d.ts b/packages/comment-checker-core/index.d.ts new file mode 100644 index 000000000..3b86b5228 --- /dev/null +++ b/packages/comment-checker-core/index.d.ts @@ -0,0 +1 @@ +export * from "./src/index" diff --git a/packages/comment-checker-core/package.json b/packages/comment-checker-core/package.json new file mode 100644 index 000000000..7d064e132 --- /dev/null +++ b/packages/comment-checker-core/package.json @@ -0,0 +1,18 @@ +{ + "name": "@oh-my-opencode/comment-checker-core", + "version": "0.1.0", + "type": "module", + "private": true, + "description": "Pure TypeScript comment-checker parsing and runner core for oh-my-opencode.", + "exports": { + ".": { + "types": "./index.d.ts", + "import": "./src/index.ts" + } + }, + "types": "./index.d.ts", + "scripts": { + "typecheck": "tsgo --noEmit -p tsconfig.json", + "test": "bun test src/*.test.ts" + } +} diff --git a/packages/comment-checker-core/src/apply-patch-edits.ts b/packages/comment-checker-core/src/apply-patch-edits.ts new file mode 100644 index 000000000..7b02b26f8 --- /dev/null +++ b/packages/comment-checker-core/src/apply-patch-edits.ts @@ -0,0 +1,164 @@ +import type { ApplyPatchAccumulator, ApplyPatchFileMetadata, CheckerEdit } from "./types" + +export function extractApplyPatchEdits( + details: unknown, + args?: Record, +): CheckerEdit[] { + const metadataEdits = getApplyPatchMetadataFiles(details) + .filter((file) => file.type?.toLowerCase() !== "delete") + .map((file) => ({ + filePath: file.movePath ?? file.filePath, + before: file.before, + after: file.after, + })) + + if (metadataEdits.length > 0) return metadataEdits + + const patch = args === undefined ? undefined : getString(args, ["patchText", "input", "patch", "command"]) + if (patch === undefined) return [] + + return parseApplyPatchRequests(patch) +} + +export function getApplyPatchMetadataFiles(details: unknown): ApplyPatchFileMetadata[] { + if (!isRecord(details)) return [] + + const direct = readApplyPatchMetadataFiles(details["files"]) + if (direct.length > 0) return direct + + const resultDetails = details["result"] + const result = isRecord(resultDetails) ? readApplyPatchMetadataFiles(resultDetails["files"]) : [] + if (result.length > 0) return result + + const metadataDetails = details["metadata"] + return isRecord(metadataDetails) ? readApplyPatchMetadataFiles(metadataDetails["files"]) : [] +} + +export function readApplyPatchMetadataFiles(value: unknown): ApplyPatchFileMetadata[] { + if (!Array.isArray(value)) return [] + + const files: ApplyPatchFileMetadata[] = [] + for (const item of value) { + if (!isRecord(item)) continue + + const filePath = getString(item, ["filePath", "file_path", "path"]) + const movePath = getString(item, ["movePath", "move_path"]) + const before = getString(item, ["before", "old", "oldString", "old_string"]) + const after = getString(item, ["after", "new", "newString", "new_string"]) + const type = getString(item, ["type", "operation"]) + + if (filePath === undefined || before === undefined || after === undefined) continue + + files.push({ + filePath, + before, + after, + ...(movePath === undefined ? {} : { movePath }), + ...(type === undefined ? {} : { type }), + }) + } + + return files +} + +export function parseApplyPatchRequests(patch: string): CheckerEdit[] { + const edits: CheckerEdit[] = [] + let current: ApplyPatchAccumulator | undefined + + const flush = (): void => { + if (current === undefined) return + + if (current.operation === "add") { + const after = joinPatchLines(current.newLines) + if (after.length > 0) { + edits.push({ filePath: current.filePath, before: "", after }) + } + } + + if (current.operation === "update") { + const after = joinPatchLines(current.newLines) + if (after.length > 0) { + edits.push({ + filePath: current.movePath ?? current.filePath, + before: joinPatchLines(current.oldLines), + after, + }) + } + } + + current = undefined + } + + for (const line of patch.split(/\r?\n/)) { + if (line === "*** Begin Patch" || line === "*** End Patch") continue + + if (line.startsWith("*** Add File: ")) { + flush() + current = makeAccumulator("add", line.slice("*** Add File: ".length).trim()) + continue + } + + if (line.startsWith("*** Update File: ")) { + flush() + current = makeAccumulator("update", line.slice("*** Update File: ".length).trim()) + continue + } + + if (line.startsWith("*** Delete File: ")) { + flush() + current = makeAccumulator("delete", line.slice("*** Delete File: ".length).trim()) + continue + } + + if (line.startsWith("*** Move to: ")) { + if (current?.operation === "update") { + current.movePath = line.slice("*** Move to: ".length).trim() + } + continue + } + + if (current === undefined || line.startsWith("@@")) continue + + if (current.operation === "add") { + if (line.startsWith("+")) current.newLines.push(line.slice(1)) + continue + } + + if (current.operation === "update") { + if (line.startsWith("-")) current.oldLines.push(line.slice(1)) + if (line.startsWith("+")) current.newLines.push(line.slice(1)) + } + } + + flush() + return edits +} + +export function makeAccumulator( + operation: ApplyPatchAccumulator["operation"], + filePath: string, +): ApplyPatchAccumulator { + return { + operation, + filePath, + oldLines: [], + newLines: [], + } +} + +export function getString(input: Record, keys: readonly string[]): string | undefined { + for (const key of keys) { + const value = input[key] + if (typeof value === "string") return value + } + + return undefined +} + +export function joinPatchLines(lines: readonly string[]): string { + return lines.length === 0 ? "" : `${lines.join("\n")}\n` +} + +export function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null +} diff --git a/packages/comment-checker-core/src/index.ts b/packages/comment-checker-core/src/index.ts new file mode 100644 index 000000000..2718504ba --- /dev/null +++ b/packages/comment-checker-core/src/index.ts @@ -0,0 +1,30 @@ +export { + extractApplyPatchEdits, + getApplyPatchMetadataFiles, + getString, + isRecord, + joinPatchLines, + makeAccumulator, + parseApplyPatchRequests, + readApplyPatchMetadataFiles, +} from "./apply-patch-edits" +export { resolveCommentCheckerBinary, runCommentChecker } from "./runner" +export type { + ApplyPatchAccumulator, + ApplyPatchFileMetadata, + CheckResult, + CheckerEdit, + CommentFilter, + CommentInfo, + CommentType, + FileComments, + FilterResult, + HookInput, + PendingCall, + ResolveCommentCheckerBinaryInput, + RunCommentCheckerInput, + RunCommentCheckerOptions, + SpawnFn, + SpawnProcess, + SpawnSignal, +} from "./types" diff --git a/packages/comment-checker-core/src/runner.ts b/packages/comment-checker-core/src/runner.ts new file mode 100644 index 000000000..31adda13c --- /dev/null +++ b/packages/comment-checker-core/src/runner.ts @@ -0,0 +1,119 @@ +import { createRequire } from "node:module" +import { dirname, join } from "node:path" + +import type { + CheckResult, + ResolveCommentCheckerBinaryInput, + RunCommentCheckerInput, + RunCommentCheckerOptions, + SpawnProcess, + SpawnSignal, +} from "./types" + +const EMPTY_RESULT: CheckResult = { hasComments: false, message: "" } + +function killProcessSafely(process: SpawnProcess, signal: SpawnSignal): void { + try { + process.kill(signal) + } catch (error) { + if (!(error instanceof Error)) { + throw error + } + } +} + +export function resolveCommentCheckerBinary(input: ResolveCommentCheckerBinaryInput): string | null { + const packageName = input.packageName ?? "@code-yeongyu/comment-checker" + + if (input.cachedBinaryPath !== null && input.existsSync(input.cachedBinaryPath)) { + return input.cachedBinaryPath + } + + if (input.importMetaUrl === undefined) { + return null + } + + try { + const require = createRequire(input.importMetaUrl) + const packageJsonPath = require.resolve(`${packageName}/package.json`) + const binaryPath = join(dirname(packageJsonPath), "bin", input.binaryName) + return input.existsSync(binaryPath) ? binaryPath : null + } catch (error) { + if (error instanceof Error) { + return null + } + throw error + } +} + +export async function runCommentChecker( + input: RunCommentCheckerInput, + options: RunCommentCheckerOptions, +): Promise { + if (input.binaryPath === null || !options.existsSync(input.binaryPath)) { + return EMPTY_RESULT + } + + const args = [input.binaryPath, "check"] + if (input.customPrompt !== undefined) { + args.push("--prompt", input.customPrompt) + } + + const timeoutMs = options.timeoutMs ?? 30_000 + const killGraceMs = options.killGraceMs ?? 1_000 + const setTimer = options.setTimeoutFn ?? setTimeout + const clearTimer = options.clearTimeoutFn ?? clearTimeout + + const process = options.spawn(args) + process.stdin.write(JSON.stringify(input.hookInput)) + process.stdin.end() + + let timeoutId: ReturnType | null = null + let graceId: ReturnType | null = null + + const timeoutPromise = new Promise<"timeout">((resolve) => { + timeoutId = setTimer(() => { + killProcessSafely(process, "SIGTERM") + + graceId = setTimer(() => { + killProcessSafely(process, "SIGKILL") + }, killGraceMs) + + resolve("timeout") + }, timeoutMs) + }) + + try { + const stdoutPromise = new Response(process.stdout).text() + const stderrPromise = new Response(process.stderr).text() + const exitCodePromise = process.exited + const completed = Promise.all([stdoutPromise, stderrPromise, exitCodePromise] as const) + const race = await Promise.race([completed, timeoutPromise] as const) + + if (race === "timeout") { + return EMPTY_RESULT + } + + const [_stdout, stderr, exitCode] = race + if (exitCode === 0) { + return EMPTY_RESULT + } + if (exitCode === 2) { + return { hasComments: true, message: stderr } + } + + return EMPTY_RESULT + } catch (error) { + if (error instanceof Error) { + return EMPTY_RESULT + } + throw error + } finally { + if (timeoutId !== null) { + clearTimer(timeoutId) + } + if (graceId !== null) { + clearTimer(graceId) + } + } +} diff --git a/packages/comment-checker-core/src/types.ts b/packages/comment-checker-core/src/types.ts new file mode 100644 index 000000000..d7497b9ce --- /dev/null +++ b/packages/comment-checker-core/src/types.ts @@ -0,0 +1,114 @@ +export type CheckerEdit = { + readonly filePath: string + readonly before: string + readonly after: string +} + +export type ApplyPatchFileMetadata = { + readonly filePath: string + readonly movePath?: string + readonly before: string + readonly after: string + readonly type?: string +} + +export type ApplyPatchAccumulator = { + operation: "add" | "update" | "delete" + filePath: string + movePath?: string + oldLines: string[] + newLines: string[] +} + +export type CommentType = "line" | "block" | "docstring" + +export interface CommentInfo { + readonly text: string + readonly lineNumber: number + readonly filePath: string + readonly commentType: CommentType + readonly isDocstring: boolean + readonly metadata?: Record +} + +export interface PendingCall { + readonly filePath: string + readonly content?: string + readonly oldString?: string + readonly newString?: string + readonly edits?: readonly { old_string: string; new_string: string }[] + readonly tool: "write" | "edit" | "multiedit" + readonly sessionID: string + readonly timestamp: number +} + +export interface FileComments { + readonly filePath: string + readonly comments: readonly CommentInfo[] +} + +export interface FilterResult { + readonly shouldSkip: boolean + readonly reason?: string +} + +export type CommentFilter = (comment: CommentInfo) => FilterResult + +export interface HookInput { + readonly session_id: string + readonly tool_name: string + readonly transcript_path: string + readonly cwd: string + readonly hook_event_name: string + readonly tool_input: { + readonly file_path?: string + readonly content?: string + readonly old_string?: string + readonly new_string?: string + readonly edits?: readonly { old_string: string; new_string: string }[] + } + readonly tool_response?: unknown +} + +export interface CheckResult { + readonly hasComments: boolean + readonly message: string +} + +export type SpawnSignal = "SIGTERM" | "SIGKILL" + +export type SpawnProcess = { + readonly stdin: { + write(input: string): void + end(): void + } + readonly stdout: ReadableStream + readonly stderr: ReadableStream + readonly exited: Promise + kill(signal: SpawnSignal): void +} + +export type SpawnFn = (args: readonly string[]) => SpawnProcess + +export interface ResolveCommentCheckerBinaryInput { + readonly binaryName: string + readonly cachedBinaryPath: string | null + readonly existsSync: (path: string) => boolean + readonly importMetaUrl?: string + readonly packageName?: string +} + +export interface RunCommentCheckerInput { + readonly hookInput: HookInput + readonly binaryPath: string | null + readonly customPrompt?: string +} + +export interface RunCommentCheckerOptions { + readonly spawn: SpawnFn + readonly existsSync: (path: string) => boolean + readonly timeoutMs?: number + readonly killGraceMs?: number + readonly setTimeoutFn?: typeof setTimeout + readonly clearTimeoutFn?: typeof clearTimeout +} diff --git a/packages/comment-checker-core/tsconfig.json b/packages/comment-checker-core/tsconfig.json new file mode 100644 index 000000000..7a69d1222 --- /dev/null +++ b/packages/comment-checker-core/tsconfig.json @@ -0,0 +1,14 @@ +{ + "compilerOptions": { + "target": "ESNext", + "module": "ESNext", + "moduleResolution": "bundler", + "strict": true, + "esModuleInterop": true, + "skipLibCheck": true, + "forceConsistentCasingInFileNames": true, + "lib": ["ESNext"], + "types": ["bun-types"] + }, + "include": ["src/**/*"] +} diff --git a/src/hooks/comment-checker/apply-patch-edits.ts b/src/hooks/comment-checker/apply-patch-edits.ts index 5d9daab8a..9f4c69858 100644 --- a/src/hooks/comment-checker/apply-patch-edits.ts +++ b/src/hooks/comment-checker/apply-patch-edits.ts @@ -1,180 +1,21 @@ -import type { ApplyPatchEdit } from "./cli-runner" +import { + extractApplyPatchEdits, + getApplyPatchMetadataFiles, + getString, + isRecord, + joinPatchLines, + makeAccumulator, + parseApplyPatchRequests, + readApplyPatchMetadataFiles, +} from "@oh-my-opencode/comment-checker-core" -type ApplyPatchFileMetadata = { - readonly filePath: string - readonly movePath?: string - readonly before: string - readonly after: string - readonly type?: string -} - -type ApplyPatchAccumulator = { - operation: "add" | "update" | "delete" - filePath: string - movePath?: string - oldLines: string[] - newLines: string[] -} - -export function extractApplyPatchEdits( - details: unknown, - args?: Record, -): ApplyPatchEdit[] { - const metadataEdits = getApplyPatchMetadataFiles(details) - .filter((file) => file.type?.toLowerCase() !== "delete") - .map((file) => ({ - filePath: file.movePath ?? file.filePath, - before: file.before, - after: file.after, - })) - - if (metadataEdits.length > 0) return metadataEdits - - const patch = args === undefined ? undefined : getString(args, ["patchText", "input", "patch", "command"]) - if (patch === undefined) return [] - - return parseApplyPatchEdits(patch) -} - -function getApplyPatchMetadataFiles(details: unknown): ApplyPatchFileMetadata[] { - if (!isRecord(details)) return [] - - const direct = readApplyPatchMetadataFiles(details["files"]) - if (direct.length > 0) return direct - - const resultDetails = details["result"] - const result = isRecord(resultDetails) ? readApplyPatchMetadataFiles(resultDetails["files"]) : [] - if (result.length > 0) return result - - const metadataDetails = details["metadata"] - return isRecord(metadataDetails) ? readApplyPatchMetadataFiles(metadataDetails["files"]) : [] -} - -function readApplyPatchMetadataFiles(value: unknown): ApplyPatchFileMetadata[] { - if (!Array.isArray(value)) return [] - - const files: ApplyPatchFileMetadata[] = [] - for (const item of value) { - if (!isRecord(item)) continue - - const filePath = getString(item, ["filePath", "file_path", "path"]) - const movePath = getString(item, ["movePath", "move_path"]) - const before = getString(item, ["before", "old", "oldString", "old_string"]) - const after = getString(item, ["after", "new", "newString", "new_string"]) - const type = getString(item, ["type", "operation"]) - - if (filePath === undefined || before === undefined || after === undefined) continue - - files.push({ - filePath, - before, - after, - ...(movePath === undefined ? {} : { movePath }), - ...(type === undefined ? {} : { type }), - }) - } - - return files -} - -function parseApplyPatchEdits(patch: string): ApplyPatchEdit[] { - const edits: ApplyPatchEdit[] = [] - let current: ApplyPatchAccumulator | undefined - - const flush = (): void => { - if (current === undefined) return - - if (current.operation === "add") { - const after = joinPatchLines(current.newLines) - if (after.length > 0) { - edits.push({ filePath: current.filePath, before: "", after }) - } - } - - if (current.operation === "update") { - const after = joinPatchLines(current.newLines) - if (after.length > 0) { - edits.push({ - filePath: current.movePath ?? current.filePath, - before: joinPatchLines(current.oldLines), - after, - }) - } - } - - current = undefined - } - - for (const line of patch.split(/\r?\n/)) { - if (line === "*** Begin Patch" || line === "*** End Patch") continue - - if (line.startsWith("*** Add File: ")) { - flush() - current = makeAccumulator("add", line.slice("*** Add File: ".length).trim()) - continue - } - - if (line.startsWith("*** Update File: ")) { - flush() - current = makeAccumulator("update", line.slice("*** Update File: ".length).trim()) - continue - } - - if (line.startsWith("*** Delete File: ")) { - flush() - current = makeAccumulator("delete", line.slice("*** Delete File: ".length).trim()) - continue - } - - if (line.startsWith("*** Move to: ")) { - if (current?.operation === "update") { - current.movePath = line.slice("*** Move to: ".length).trim() - } - continue - } - - if (current === undefined || line.startsWith("@@")) continue - - if (current.operation === "add") { - if (line.startsWith("+")) current.newLines.push(line.slice(1)) - continue - } - - if (current.operation === "update") { - if (line.startsWith("-")) current.oldLines.push(line.slice(1)) - if (line.startsWith("+")) current.newLines.push(line.slice(1)) - } - } - - flush() - return edits -} - -function makeAccumulator( - operation: ApplyPatchAccumulator["operation"], - filePath: string, -): ApplyPatchAccumulator { - return { - operation, - filePath, - oldLines: [], - newLines: [], - } -} - -function getString(input: Record, keys: readonly string[]): string | undefined { - for (const key of keys) { - const value = input[key] - if (typeof value === "string") return value - } - - return undefined -} - -function joinPatchLines(lines: readonly string[]): string { - return lines.length === 0 ? "" : `${lines.join("\n")}\n` -} - -function isRecord(value: unknown): value is Record { - return typeof value === "object" && value !== null +export { + extractApplyPatchEdits, + getApplyPatchMetadataFiles, + getString, + isRecord, + joinPatchLines, + makeAccumulator, + parseApplyPatchRequests, + readApplyPatchMetadataFiles, } diff --git a/src/hooks/comment-checker/cli.ts b/src/hooks/comment-checker/cli.ts index 14a128d49..25596fd9f 100644 --- a/src/hooks/comment-checker/cli.ts +++ b/src/hooks/comment-checker/cli.ts @@ -1,9 +1,14 @@ import { spawn } from "../../shared/bun-spawn-shim" -import { createRequire } from "module" -import { dirname, join } from "path" +import { join } from "path" import { existsSync } from "fs" import * as fs from "fs" import { tmpdir } from "os" +import { + resolveCommentCheckerBinary, + runCommentChecker as runCommentCheckerCore, + type CheckResult, + type HookInput, +} from "@oh-my-opencode/comment-checker-core" import { getCachedBinaryPath, ensureCommentCheckerBinary } from "./downloader" const DEBUG = process.env.COMMENT_CHECKER_DEBUG === "1" @@ -21,33 +26,15 @@ function getBinaryName(): string { } function findCommentCheckerPathSync(): string | null { - const binaryName = getBinaryName() - - // Check cached binary first (safest path - no module resolution needed) - const cachedPath = getCachedBinaryPath() - if (cachedPath) { - debugLog("found binary in cache:", cachedPath) - return cachedPath - } - - // Guard against undefined import.meta.url (can happen on Windows during plugin loading) - if (!import.meta.url) { - debugLog("import.meta.url is undefined, skipping package resolution") - return null - } - - try { - const require = createRequire(import.meta.url) - const cliPkgPath = require.resolve("@code-yeongyu/comment-checker/package.json") - const cliDir = dirname(cliPkgPath) - const binaryPath = join(cliDir, "bin", binaryName) - - if (existsSync(binaryPath)) { - debugLog("found binary in main package:", binaryPath) - return binaryPath - } - } catch (err) { - debugLog("main package not installed or resolution failed:", err) + const resolvedPath = resolveCommentCheckerBinary({ + binaryName: getBinaryName(), + cachedBinaryPath: getCachedBinaryPath(), + existsSync, + importMetaUrl: import.meta.url, + }) + if (resolvedPath !== null) { + debugLog("resolved binary path:", resolvedPath) + return resolvedPath } debugLog("no binary found in known locations") @@ -121,26 +108,7 @@ export function startBackgroundInit(): void { } } -export interface HookInput { - session_id: string - tool_name: string - transcript_path: string - cwd: string - hook_event_name: string - tool_input: { - file_path?: string - content?: string - old_string?: string - new_string?: string - edits?: Array<{ old_string: string; new_string: string }> - } - tool_response?: unknown -} - -export interface CheckResult { - hasComments: boolean - message: string -} +export type { HookInput, CheckResult } /** * Run comment-checker CLI with given input. @@ -150,104 +118,28 @@ export interface CheckResult { */ export async function runCommentChecker(input: HookInput, cliPath?: string, customPrompt?: string): Promise { const binaryPath = cliPath ?? resolvedCliPath ?? getCommentCheckerPathSync() - + if (!binaryPath) { debugLog("comment-checker binary not found") return { hasComments: false, message: "" } } - if (!existsSync(binaryPath)) { - debugLog("comment-checker binary does not exist:", binaryPath) - return { hasComments: false, message: "" } - } - - const jsonInput = JSON.stringify(input) - debugLog("running comment-checker with input:", jsonInput.substring(0, 200)) - - let didTimeout = false - try { - const args = [binaryPath, "check"] - if (customPrompt) { - args.push("--prompt", customPrompt) - } - - const proc = spawn(args, { - stdin: "pipe", - stdout: "pipe", - stderr: "pipe", - }) - - let timeoutId: ReturnType | null = null - const timeoutPromise = new Promise<"timeout">(resolve => { - timeoutId = setTimeout(async () => { - didTimeout = true - debugLog("comment-checker timed out after 30s; sending SIGTERM") - try { - proc.kill("SIGTERM") - } catch (err) { - debugLog("failed to SIGTERM:", err) - } - const graceTimer = setTimeout(() => { - try { - proc.kill("SIGKILL") - debugLog("sent SIGKILL after grace period") - } catch { - } - }, 1000) - try { - await proc.exited - } catch { - } - clearTimeout(graceTimer) - resolve("timeout") - }, 30_000) - }) - - try { - // Write JSON to stdin - proc.stdin.write(jsonInput) - proc.stdin.end() - - const stdoutPromise = new Response(proc.stdout).text() - const stderrPromise = new Response(proc.stderr).text() - const exitCodePromise = proc.exited - - const raceResult = await Promise.race([ - Promise.all([stdoutPromise, stderrPromise, exitCodePromise] as const), - timeoutPromise, - ]) - - if (raceResult === "timeout") { - return { hasComments: false, message: "" } - } - - const [stdout, stderr, exitCode] = raceResult - - debugLog("exit code:", exitCode, "stdout length:", stdout.length, "stderr length:", stderr.length) - - if (exitCode === 0) { - return { hasComments: false, message: "" } - } - - if (exitCode === 2) { - // Comments detected - message is in stderr - return { hasComments: true, message: stderr } - } - - // Error case - debugLog("unexpected exit code:", exitCode, "stderr:", stderr) - return { hasComments: false, message: "" } - } finally { - if (timeoutId !== null) { - clearTimeout(timeoutId) - } - } - } catch (err) { - if (didTimeout) { - return { hasComments: false, message: "" } - } - debugLog("failed to run comment-checker:", err) + const result = await runCommentCheckerCore( + { hookInput: input, binaryPath, customPrompt }, + { + existsSync, + spawn: (args: readonly string[]) => + spawn([...args], { + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + }), + }, + ) + return result + } catch (error) { + debugLog("failed to run comment-checker:", error) return { hasComments: false, message: "" } } } diff --git a/src/hooks/comment-checker/hook.ts b/src/hooks/comment-checker/hook.ts index ed1e36225..38a88fb7a 100644 --- a/src/hooks/comment-checker/hook.ts +++ b/src/hooks/comment-checker/hook.ts @@ -8,7 +8,7 @@ import { processWithCli, processApplyPatchEditsWithCli, } from "./cli-runner" -import { extractApplyPatchEdits } from "./apply-patch-edits" +import { extractApplyPatchEdits } from "@oh-my-opencode/comment-checker-core" import { registerPendingCall, startPendingCallCleanup, diff --git a/src/hooks/comment-checker/types.ts b/src/hooks/comment-checker/types.ts index 51d96de4c..5beb6bbb9 100644 --- a/src/hooks/comment-checker/types.ts +++ b/src/hooks/comment-checker/types.ts @@ -1,33 +1,19 @@ -export type CommentType = "line" | "block" | "docstring" - -export interface CommentInfo { - text: string - lineNumber: number - filePath: string - commentType: CommentType - isDocstring: boolean - metadata?: Record -} - -export interface PendingCall { - filePath: string - content?: string - oldString?: string - newString?: string - edits?: Array<{ old_string: string; new_string: string }> - tool: "write" | "edit" | "multiedit" - sessionID: string - timestamp: number -} - -export interface FileComments { - filePath: string - comments: CommentInfo[] -} - -export interface FilterResult { - shouldSkip: boolean - reason?: string -} - -export type CommentFilter = (comment: CommentInfo) => FilterResult +export type { + ApplyPatchAccumulator, + ApplyPatchFileMetadata, + CheckResult, + CheckerEdit, + CommentFilter, + CommentInfo, + CommentType, + FileComments, + FilterResult, + HookInput, + PendingCall, + ResolveCommentCheckerBinaryInput, + RunCommentCheckerInput, + RunCommentCheckerOptions, + SpawnFn, + SpawnProcess, + SpawnSignal, +} from "@oh-my-opencode/comment-checker-core"