diff --git a/plugins/codex/scripts/stop-review-gate-hook.mjs b/plugins/codex/scripts/stop-review-gate-hook.mjs index 2346bdcf4..7460f6a8d 100644 --- a/plugins/codex/scripts/stop-review-gate-hook.mjs +++ b/plugins/codex/scripts/stop-review-gate-hook.mjs @@ -17,6 +17,8 @@ const STOP_REVIEW_TIMEOUT_MS = 15 * 60 * 1000; const SCRIPT_DIR = path.dirname(fileURLToPath(import.meta.url)); const ROOT_DIR = path.resolve(SCRIPT_DIR, ".."); const STOP_REVIEW_TASK_MARKER = "Run a stop-gate review of the previous Claude turn."; +const STOP_GATE_MANUAL_OR_BYPASS_HINT = + "Run /codex:review --wait manually or bypass the gate by running /codex:setup --disable-review-gate."; function readHookInput() { const raw = fs.readFileSync(0, "utf8").trim(); @@ -66,13 +68,12 @@ function buildSetupNote(cwd) { return `Codex is not set up for the review gate.${detail} Run /codex:setup.`; } -function parseStopReviewOutput(rawOutput) { +export function parseStopReviewOutput(rawOutput) { const text = String(rawOutput ?? "").trim(); if (!text) { return { ok: false, - reason: - "The stop-time Codex review task returned no final output. Run /codex:review --wait manually or bypass the gate." + reason: `The stop-time Codex review task returned no final output. ${STOP_GATE_MANUAL_OR_BYPASS_HINT}` }; } @@ -90,8 +91,7 @@ function parseStopReviewOutput(rawOutput) { return { ok: false, - reason: - "The stop-time Codex review task returned an unexpected answer. Run /codex:review --wait manually or bypass the gate." + reason: `The stop-time Codex review task returned an unexpected answer. ${STOP_GATE_MANUAL_OR_BYPASS_HINT}` }; } @@ -112,8 +112,7 @@ function runStopReview(cwd, input = {}) { if (result.error?.code === "ETIMEDOUT") { return { ok: false, - reason: - "The stop-time Codex review task timed out after 15 minutes. Run /codex:review --wait manually or bypass the gate." + reason: `The stop-time Codex review task timed out after 15 minutes. ${STOP_GATE_MANUAL_OR_BYPASS_HINT}` }; } @@ -122,8 +121,8 @@ function runStopReview(cwd, input = {}) { return { ok: false, reason: detail - ? `The stop-time Codex review task failed: ${detail}` - : "The stop-time Codex review task failed. Run /codex:review --wait manually or bypass the gate." + ? `The stop-time Codex review task failed: ${detail} ${STOP_GATE_MANUAL_OR_BYPASS_HINT}` + : `The stop-time Codex review task failed. ${STOP_GATE_MANUAL_OR_BYPASS_HINT}` }; } @@ -133,8 +132,7 @@ function runStopReview(cwd, input = {}) { } catch { return { ok: false, - reason: - "The stop-time Codex review task returned invalid JSON. Run /codex:review --wait manually or bypass the gate." + reason: `The stop-time Codex review task returned invalid JSON. ${STOP_GATE_MANUAL_OR_BYPASS_HINT}` }; } } @@ -175,10 +173,34 @@ function main() { logNote(runningTaskNote); } -try { - main(); -} catch (error) { - const message = error instanceof Error ? error.message : String(error); - process.stderr.write(`${message}\n`); - process.exitCode = 1; +export function resolveCanonicalPath(filePath) { + try { + return fs.realpathSync.native(filePath); + } catch { + try { + return fs.realpathSync(filePath); + } catch { + return path.resolve(filePath); + } + } +} + +export function isCliEntry(argv = process.argv, moduleUrl = import.meta.url) { + const invoked = argv[1]; + if (!invoked) { + return false; + } + return ( + resolveCanonicalPath(invoked) === resolveCanonicalPath(fileURLToPath(moduleUrl)) + ); +} + +if (isCliEntry()) { + try { + main(); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + process.stderr.write(`${message}\n`); + process.exitCode = 1; + } } diff --git a/tests/stop-review-gate-hook.test.mjs b/tests/stop-review-gate-hook.test.mjs new file mode 100644 index 000000000..11adee443 --- /dev/null +++ b/tests/stop-review-gate-hook.test.mjs @@ -0,0 +1,89 @@ +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +import { + isCliEntry, + parseStopReviewOutput, + resolveCanonicalPath +} from "../plugins/codex/scripts/stop-review-gate-hook.mjs"; + +const HOOK_MODULE_URL = new URL( + "../plugins/codex/scripts/stop-review-gate-hook.mjs", + import.meta.url +).href; +const HOOK_MODULE_PATH = fileURLToPath(HOOK_MODULE_URL); + +const ESCAPE_COMMAND = "/codex:setup --disable-review-gate"; + +test("resolveCanonicalPath follows symlinks to the same target", () => { + const linkDir = fs.mkdtempSync(path.join(os.tmpdir(), "stop-hook-canonical-")); + const linkPath = path.join(linkDir, "stop-review-gate-hook.mjs"); + + try { + fs.symlinkSync(HOOK_MODULE_PATH, linkPath); + assert.equal(resolveCanonicalPath(linkPath), resolveCanonicalPath(HOOK_MODULE_PATH)); + } catch (error) { + if (error?.code === "EPERM" || error?.code === "ENOENT") { + return; + } + throw error; + } finally { + fs.rmSync(linkDir, { recursive: true, force: true }); + } +}); + +test("isCliEntry treats a symlinked argv path as the hook entry", () => { + const linkDir = fs.mkdtempSync(path.join(os.tmpdir(), "stop-hook-entry-")); + const linkPath = path.join(linkDir, "stop-review-gate-hook.mjs"); + + try { + fs.symlinkSync(HOOK_MODULE_PATH, linkPath); + assert.equal(isCliEntry(["node", linkPath], HOOK_MODULE_URL), true); + assert.equal(isCliEntry(["node", HOOK_MODULE_PATH], HOOK_MODULE_URL), true); + assert.equal(isCliEntry(["node", pathToFileURL(linkPath).href], HOOK_MODULE_URL), false); + } catch (error) { + if (error?.code === "EPERM" || error?.code === "ENOENT") { + return; + } + throw error; + } finally { + fs.rmSync(linkDir, { recursive: true, force: true }); + } +}); + +test("isCliEntry returns false for imports and unrelated scripts", () => { + assert.equal(isCliEntry(["node"], HOOK_MODULE_URL), false); + assert.equal( + isCliEntry(["node", fileURLToPath(import.meta.url)], HOOK_MODULE_URL), + false + ); +}); + +test("parseStopReviewOutput includes escape command for empty output", () => { + const result = parseStopReviewOutput(""); + assert.equal(result.ok, false); + assert.match(result.reason, new RegExp(ESCAPE_COMMAND.replace("/", "\\/"))); +}); + +test("parseStopReviewOutput includes escape command for unexpected answer", () => { + const result = parseStopReviewOutput("maybe later"); + assert.equal(result.ok, false); + assert.match(result.reason, new RegExp(ESCAPE_COMMAND.replace("/", "\\/"))); +}); + +test("parseStopReviewOutput allows clean stop review", () => { + const result = parseStopReviewOutput("ALLOW: No blocking issues found."); + assert.equal(result.ok, true); + assert.equal(result.reason, null); +}); + +test("parseStopReviewOutput omits escape command for Codex BLOCK findings", () => { + const result = parseStopReviewOutput("BLOCK: Missing empty-state guard"); + assert.equal(result.ok, false); + assert.match(result.reason, /still need fixes/i); + assert.doesNotMatch(result.reason, new RegExp(ESCAPE_COMMAND.replace("/", "\\/"))); +});