From 0f5a79673911b89895468836fca0a4e695027fea Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 31 May 2026 05:41:23 +0900 Subject: [PATCH] fix(codex): harden lsp post tool hook --- .../plugin/components/lsp/src/cli.ts | 10 +++--- .../components/lsp/src/codex-hook-cli.ts | 33 +++++++++++++++++++ .../plugin/components/lsp/src/codex-hook.ts | 28 ++-------------- .../lsp/test/codex-hook-cli.test.ts | 28 ++++++++++++++++ .../components/lsp/test/package-smoke.test.ts | 12 ++++--- 5 files changed, 75 insertions(+), 36 deletions(-) create mode 100644 packages/omo-codex/plugin/components/lsp/src/codex-hook-cli.ts create mode 100644 packages/omo-codex/plugin/components/lsp/test/codex-hook-cli.test.ts diff --git a/packages/omo-codex/plugin/components/lsp/src/cli.ts b/packages/omo-codex/plugin/components/lsp/src/cli.ts index ad7bdd9d6..72c020a5f 100644 --- a/packages/omo-codex/plugin/components/lsp/src/cli.ts +++ b/packages/omo-codex/plugin/components/lsp/src/cli.ts @@ -1,12 +1,12 @@ #!/usr/bin/env node import { spawn } from "node:child_process"; -import { dirname, resolve } from "node:path"; +import { createRequire } from "node:module"; import { argv, execPath, stderr } from "node:process"; -import { fileURLToPath } from "node:url"; -import { runPostToolUseHookCli } from "./codex-hook.js"; +import { runPostToolUseHookCli } from "./codex-hook-cli.js"; -const PACKAGE_LSP_MCP_CLI = "../../../../../lsp-tools-mcp/dist/cli.js"; +const require = createRequire(import.meta.url); +const PACKAGE_LSP_MCP_CLI = "@code-yeongyu/lsp-tools-mcp/dist/cli.js"; async function main(): Promise { const [command = "mcp", subcommand = ""] = argv.slice(2); @@ -31,7 +31,7 @@ main().catch((error: unknown) => { }); async function runPackageLspMcpCli(): Promise { - const cliPath = resolve(dirname(fileURLToPath(import.meta.url)), PACKAGE_LSP_MCP_CLI); + const cliPath = require.resolve(PACKAGE_LSP_MCP_CLI); const child = spawn(execPath, [cliPath, "mcp"], { stdio: "inherit" }); await new Promise((resolve, reject) => { child.once("error", reject); diff --git a/packages/omo-codex/plugin/components/lsp/src/codex-hook-cli.ts b/packages/omo-codex/plugin/components/lsp/src/codex-hook-cli.ts new file mode 100644 index 000000000..c23a12179 --- /dev/null +++ b/packages/omo-codex/plugin/components/lsp/src/codex-hook-cli.ts @@ -0,0 +1,33 @@ +import { stdin as processStdin } from "node:process"; + +import { disposeDefaultLspManager } from "@code-yeongyu/lsp-tools-mcp/dist/lsp/manager.js"; + +import { isRecord, runLspPostToolUseHook } from "./codex-hook.js"; + +export async function runPostToolUseHookCli(stdin: NodeJS.ReadStream = processStdin): Promise { + try { + const raw = await readStdin(stdin); + if (!raw.trim()) return; + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (error) { + if (error instanceof SyntaxError) return; + throw error; + } + const input = isRecord(parsed) ? parsed : {}; + const output = await runLspPostToolUseHook(input); + if (output) process.stdout.write(output); + } finally { + await disposeDefaultLspManager(); + } +} + +async function readStdin(stdin: NodeJS.ReadStream): Promise { + stdin.setEncoding("utf8"); + let raw = ""; + for await (const chunk of stdin) { + raw += chunk; + } + return raw; +} diff --git a/packages/omo-codex/plugin/components/lsp/src/codex-hook.ts b/packages/omo-codex/plugin/components/lsp/src/codex-hook.ts index 8bd565354..913d7fec2 100644 --- a/packages/omo-codex/plugin/components/lsp/src/codex-hook.ts +++ b/packages/omo-codex/plugin/components/lsp/src/codex-hook.ts @@ -1,8 +1,6 @@ import { readFileSync } from "node:fs"; -import { stdin as processStdin } from "node:process"; -import { disposeDefaultLspManager } from "../../../../../lsp-tools-mcp/dist/lsp/manager.js"; -import { executeLspDiagnostics } from "../../../../../lsp-tools-mcp/dist/tools.js"; +import { executeLspDiagnostics } from "@code-yeongyu/lsp-tools-mcp/dist/tools.js"; export type DiagnosticsRunner = (filePath: string) => Promise; @@ -191,19 +189,6 @@ export function extractMutatedFilePaths(input: CodexPostToolUseInput): string[] return [...paths]; } -export async function runPostToolUseHookCli(stdin: NodeJS.ReadStream = processStdin): Promise { - try { - const raw = await readStdin(stdin); - if (!raw.trim()) return; - const parsed: unknown = JSON.parse(raw); - const input = isRecord(parsed) ? parsed : {}; - const output = await runLspPostToolUseHook(input); - if (output) process.stdout.write(output); - } finally { - await disposeDefaultLspManager(); - } -} - function isMutationTool(value: unknown): boolean { if (typeof value !== "string") return false; return MUTATION_TOOL_NAMES.has(value.toLowerCase()); @@ -271,15 +256,6 @@ function addPatchFiles(paths: Set, value: unknown): void { } } -function isRecord(value: unknown): value is Record { +export function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } - -async function readStdin(stdin: NodeJS.ReadStream): Promise { - stdin.setEncoding("utf8"); - let raw = ""; - for await (const chunk of stdin) { - raw += chunk; - } - return raw; -} diff --git a/packages/omo-codex/plugin/components/lsp/test/codex-hook-cli.test.ts b/packages/omo-codex/plugin/components/lsp/test/codex-hook-cli.test.ts new file mode 100644 index 000000000..7d8ff7ad0 --- /dev/null +++ b/packages/omo-codex/plugin/components/lsp/test/codex-hook-cli.test.ts @@ -0,0 +1,28 @@ +import { spawnSync } from "node:child_process"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +import { describe, expect, it } from "vitest"; + +describe("codex PostToolUse hook CLI", () => { + it("#given malformed post-tool-use stdin #when hook CLI runs #then it no-ops without stderr", () => { + // given + const input = "break;\n"; + + // when + const result = runBuiltHookCli(input); + + // then + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(result.stdout).toBe(""); + }); +}); + +function runBuiltHookCli(input: string): ReturnType { + const cliPath = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../dist/cli.js"); + return spawnSync(process.execPath, [cliPath, "hook", "post-tool-use"], { + input, + encoding: "utf8", + }); +} diff --git a/packages/omo-codex/plugin/components/lsp/test/package-smoke.test.ts b/packages/omo-codex/plugin/components/lsp/test/package-smoke.test.ts index af99201c2..1c50fe3b4 100644 --- a/packages/omo-codex/plugin/components/lsp/test/package-smoke.test.ts +++ b/packages/omo-codex/plugin/components/lsp/test/package-smoke.test.ts @@ -56,6 +56,7 @@ describe("plugin package metadata", () => { const hooksJson = readHooksJson("hooks/hooks.json"); const mcpJson = readMcpJson(".mcp.json"); const cliSource = readFileSync("src/cli.ts", "utf8"); + const codexHookCliSource = readFileSync("src/codex-hook-cli.ts", "utf8"); const codexHookSource = readFileSync("src/codex-hook.ts", "utf8"); const sourceFiles = readdirSync("src"); @@ -79,11 +80,12 @@ describe("plugin package metadata", () => { expect(lspServer?.command).toBe("node"); expect(lspServer?.args).toEqual(["../../../../lsp-tools-mcp/dist/cli.js", "mcp"]); expect(cliSource).not.toContain("./lazy-lsp-mcp.js"); - expect(cliSource).not.toContain("@code-yeongyu/lsp-tools-mcp"); - expect(cliSource).toContain("../../../../../lsp-tools-mcp/dist/cli.js"); - expect(codexHookSource).not.toContain("@code-yeongyu/lsp-tools-mcp"); - expect(codexHookSource).toContain("../../../../../lsp-tools-mcp/dist/lsp/manager.js"); - expect(codexHookSource).toContain("../../../../../lsp-tools-mcp/dist/tools.js"); + expect(cliSource).toContain("@code-yeongyu/lsp-tools-mcp/dist/cli.js"); + expect(cliSource).not.toContain("../../../../../lsp-tools-mcp/dist/cli.js"); + expect(codexHookCliSource).toContain("@code-yeongyu/lsp-tools-mcp/dist/lsp/manager.js"); + expect(codexHookSource).toContain("@code-yeongyu/lsp-tools-mcp/dist/tools.js"); + expect(codexHookCliSource).not.toContain("../../../../../lsp-tools-mcp/dist/lsp/manager.js"); + expect(codexHookSource).not.toContain("../../../../../lsp-tools-mcp/dist/tools.js"); expect(sourceFiles.filter((name) => name.startsWith("lazy-mcp") || name === "lazy-lsp-mcp.ts")).toEqual([]); });