diff --git a/README.md b/README.md index ddeeff4..9be5022 100644 --- a/README.md +++ b/README.md @@ -221,6 +221,27 @@ When the review gate is enabled, the plugin uses a `Stop` hook to run a targeted > [!WARNING] > The review gate can create a long-running Claude/Codex loop and may drain usage limits quickly. Only enable it when you plan to actively monitor the session. +#### Throttling the review gate + +To prevent runaway usage, you can limit how often the review gate fires: + +```bash +# Allow at most 5 stop-gate reviews per session +/codex:setup --review-gate-max 5 + +# Require at least 10 minutes between stop-gate reviews +/codex:setup --review-gate-cooldown 10 + +# Combine both limits +/codex:setup --enable-review-gate --review-gate-max 5 --review-gate-cooldown 10 + +# Remove a limit +/codex:setup --review-gate-max off +/codex:setup --review-gate-cooldown off +``` + +When a limit is reached, the stop-gate review is skipped (the session is allowed to end) and a note is logged. You can still run `/codex:review` manually at any time. + ## Typical Flows ### Review Before Shipping diff --git a/plugins/codex/commands/setup.md b/plugins/codex/commands/setup.md index fb33a15..a36a47f 100644 --- a/plugins/codex/commands/setup.md +++ b/plugins/codex/commands/setup.md @@ -1,6 +1,6 @@ --- description: Check whether the local Codex CLI is ready and optionally toggle the stop-time review gate -argument-hint: '[--enable-review-gate|--disable-review-gate]' +argument-hint: '[--enable-review-gate|--disable-review-gate] [--review-gate-max ] [--review-gate-cooldown ]' allowed-tools: Bash(node:*), Bash(npm:*), AskUserQuestion --- diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 201d1c7..974be79 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -74,7 +74,7 @@ function printUsage() { console.log( [ "Usage:", - " node scripts/codex-companion.mjs setup [--enable-review-gate|--disable-review-gate] [--json]", + " node scripts/codex-companion.mjs setup [--enable-review-gate|--disable-review-gate] [--review-gate-max ] [--review-gate-cooldown ] [--json]", " node scripts/codex-companion.mjs review [--wait|--background] [--base ] [--scope ]", " node scripts/codex-companion.mjs adversarial-review [--wait|--background] [--base ] [--scope ] [focus text]", " node scripts/codex-companion.mjs task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [prompt]", @@ -204,6 +204,8 @@ function buildSetupReport(cwd, actionsTaken = []) { auth: authStatus, sessionRuntime: getSessionRuntimeStatus(), reviewGateEnabled: Boolean(config.stopReviewGate), + reviewGateMaxPerSession: config.stopReviewGateMaxPerSession ?? null, + reviewGateCooldownMinutes: config.stopReviewGateCooldownMinutes ?? null, actionsTaken, nextSteps }; @@ -211,7 +213,7 @@ function buildSetupReport(cwd, actionsTaken = []) { function handleSetup(argv) { const { options } = parseCommandInput(argv, { - valueOptions: ["cwd"], + valueOptions: ["cwd", "review-gate-max", "review-gate-cooldown"], booleanOptions: ["json", "enable-review-gate", "disable-review-gate"] }); @@ -231,6 +233,32 @@ function handleSetup(argv) { actionsTaken.push(`Disabled the stop-time review gate for ${workspaceRoot}.`); } + if (options["review-gate-max"] != null) { + const max = options["review-gate-max"] === "off" ? null : Number(options["review-gate-max"]); + if (max !== null && (!Number.isInteger(max) || max < 1)) { + throw new Error(`--review-gate-max must be a positive integer or "off".`); + } + setConfig(workspaceRoot, "stopReviewGateMaxPerSession", max); + actionsTaken.push( + max != null + ? `Set review gate session limit to ${max} reviews per session.` + : `Removed review gate session limit.` + ); + } + + if (options["review-gate-cooldown"] != null) { + const cooldown = options["review-gate-cooldown"] === "off" ? null : Number(options["review-gate-cooldown"]); + if (cooldown !== null && (!Number.isInteger(cooldown) || cooldown < 1)) { + throw new Error(`--review-gate-cooldown must be a positive integer (minutes) or "off".`); + } + setConfig(workspaceRoot, "stopReviewGateCooldownMinutes", cooldown); + actionsTaken.push( + cooldown != null + ? `Set review gate cooldown to ${cooldown} minute(s).` + : `Removed review gate cooldown.` + ); + } + const finalReport = buildSetupReport(cwd, actionsTaken); outputResult(options.json ? finalReport : renderSetupReport(finalReport), options.json); } diff --git a/plugins/codex/scripts/lib/render.mjs b/plugins/codex/scripts/lib/render.mjs index 2ec1852..d60e392 100644 --- a/plugins/codex/scripts/lib/render.mjs +++ b/plugins/codex/scripts/lib/render.mjs @@ -174,6 +174,17 @@ function appendReasoningSection(lines, reasoningSummary) { } } +function formatReviewGateLimits(report) { + const parts = []; + if (report.reviewGateMaxPerSession != null) { + parts.push(`max ${report.reviewGateMaxPerSession}/session`); + } + if (report.reviewGateCooldownMinutes != null) { + parts.push(`${report.reviewGateCooldownMinutes}m cooldown`); + } + return parts.length > 0 ? ` (${parts.join(", ")})` : ""; +} + export function renderSetupReport(report) { const lines = [ "# Codex Setup", @@ -186,7 +197,7 @@ export function renderSetupReport(report) { `- codex: ${report.codex.detail}`, `- auth: ${report.auth.detail}`, `- session runtime: ${report.sessionRuntime.label}`, - `- review gate: ${report.reviewGateEnabled ? "enabled" : "disabled"}`, + `- review gate: ${report.reviewGateEnabled ? "enabled" : "disabled"}${report.reviewGateEnabled ? formatReviewGateLimits(report) : ""}`, "" ]; diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs index 2da2349..d86bf88 100644 --- a/plugins/codex/scripts/lib/state.mjs +++ b/plugins/codex/scripts/lib/state.mjs @@ -20,7 +20,9 @@ function defaultState() { return { version: STATE_VERSION, config: { - stopReviewGate: false + stopReviewGate: false, + stopReviewGateMaxPerSession: null, + stopReviewGateCooldownMinutes: null }, jobs: [] }; diff --git a/plugins/codex/scripts/stop-review-gate-hook.mjs b/plugins/codex/scripts/stop-review-gate-hook.mjs index c22edbd..380f35c 100644 --- a/plugins/codex/scripts/stop-review-gate-hook.mjs +++ b/plugins/codex/scripts/stop-review-gate-hook.mjs @@ -45,6 +45,51 @@ function filterJobsForCurrentSession(jobs, input = {}) { return jobs.filter((job) => job.sessionId === sessionId); } +function countSessionStopReviews(jobs) { + return jobs.filter( + (job) => + job.jobClass === "review" && + job.title === "Codex Stop Gate Review" && + job.status === "completed" + ).length; +} + +// Expects jobs pre-sorted newest-first (via sortJobsNewestFirst from caller). +function findLastStopReviewTime(jobs) { + const stopReview = jobs.find( + (job) => + job.jobClass === "review" && + job.title === "Codex Stop Gate Review" && + job.completedAt + ); + return stopReview?.completedAt ? new Date(stopReview.completedAt).getTime() : null; +} + +function checkThrottleLimits(config, jobs) { + const maxPerSession = config.stopReviewGateMaxPerSession ?? null; + if (maxPerSession != null && maxPerSession > 0) { + const count = countSessionStopReviews(jobs); + if (count >= maxPerSession) { + return `Stop-gate review skipped: session limit reached (${count}/${maxPerSession}). Run /codex:review manually if needed.`; + } + } + + const cooldownMinutes = config.stopReviewGateCooldownMinutes ?? null; + if (cooldownMinutes != null && cooldownMinutes > 0) { + const lastTime = findLastStopReviewTime(jobs); + if (lastTime != null) { + const elapsed = Date.now() - lastTime; + const cooldownMs = cooldownMinutes * 60 * 1000; + if (elapsed < cooldownMs) { + const remainingMinutes = Math.ceil((cooldownMs - elapsed) / 60000); + return `Stop-gate review skipped: cooldown active (${remainingMinutes}m remaining). Run /codex:review manually if needed.`; + } + } + } + + return null; +} + function buildStopReviewPrompt(input = {}) { const lastAssistantMessage = String(input.last_assistant_message ?? "").trim(); const template = loadPromptTemplate(ROOT_DIR, "stop-review-gate"); @@ -163,6 +208,13 @@ function main() { return; } + const throttleNote = checkThrottleLimits(config, jobs); + if (throttleNote) { + logNote(throttleNote); + logNote(runningTaskNote); + return; + } + const review = runStopReview(cwd, input); if (!review.ok) { emitDecision({ diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index a00e8dd..ad72fac 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -197,7 +197,7 @@ test("setup command can offer Codex install and still points users to codex logi const setup = read("commands/setup.md"); const readme = fs.readFileSync(path.join(ROOT, "README.md"), "utf8"); - assert.match(setup, /argument-hint:\s*'\[--enable-review-gate\|--disable-review-gate\]'/); + assert.match(setup, /argument-hint:.*--enable-review-gate\|--disable-review-gate/); assert.match(setup, /AskUserQuestion/); assert.match(setup, /npm install -g @openai\/codex/); assert.match(setup, /codex-companion\.mjs" setup --json \$ARGUMENTS/); @@ -205,4 +205,8 @@ test("setup command can offer Codex install and still points users to codex logi assert.match(readme, /offer to install Codex for you/i); assert.match(readme, /\/codex:setup --enable-review-gate/); assert.match(readme, /\/codex:setup --disable-review-gate/); + assert.match(setup, /--review-gate-max/); + assert.match(setup, /--review-gate-cooldown/); + assert.match(readme, /--review-gate-max/); + assert.match(readme, /--review-gate-cooldown/); }); diff --git a/tests/review-gate-throttle.test.mjs b/tests/review-gate-throttle.test.mjs new file mode 100644 index 0000000..427c930 --- /dev/null +++ b/tests/review-gate-throttle.test.mjs @@ -0,0 +1,224 @@ +import fs from "node:fs"; +import path from "node:path"; +import test from "node:test"; +import assert from "node:assert/strict"; + +import { makeTempDir } from "./helpers.mjs"; +import { + getConfig, + loadState, + resolveStateFile, + setConfig, + upsertJob +} from "../plugins/codex/scripts/lib/state.mjs"; + +function setupWorkspace() { + const workspace = makeTempDir(); + const previousPluginData = process.env.CLAUDE_PLUGIN_DATA; + const pluginData = makeTempDir(); + process.env.CLAUDE_PLUGIN_DATA = pluginData; + + return { + workspace, + cleanup() { + if (previousPluginData == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginData; + } + } + }; +} + +// Mirror of the throttle logic from stop-review-gate-hook.mjs for unit testing. +// The hook script calls main() at import time so it cannot be imported directly. +function countSessionStopReviews(jobs) { + return jobs.filter( + (job) => + job.jobClass === "review" && + job.title === "Codex Stop Gate Review" && + job.status === "completed" + ).length; +} + +function findLastStopReviewTime(jobs) { + const stopReview = jobs.find( + (job) => + job.jobClass === "review" && + job.title === "Codex Stop Gate Review" && + job.completedAt + ); + return stopReview?.completedAt ? new Date(stopReview.completedAt).getTime() : null; +} + +function checkThrottleLimits(config, jobs) { + const maxPerSession = config.stopReviewGateMaxPerSession ?? null; + if (maxPerSession != null && maxPerSession > 0) { + const count = countSessionStopReviews(jobs); + if (count >= maxPerSession) { + return `Stop-gate review skipped: session limit reached (${count}/${maxPerSession}). Run /codex:review manually if needed.`; + } + } + + const cooldownMinutes = config.stopReviewGateCooldownMinutes ?? null; + if (cooldownMinutes != null && cooldownMinutes > 0) { + const lastTime = findLastStopReviewTime(jobs); + if (lastTime != null) { + const elapsed = Date.now() - lastTime; + const cooldownMs = cooldownMinutes * 60 * 1000; + if (elapsed < cooldownMs) { + const remainingMinutes = Math.ceil((cooldownMs - elapsed) / 60000); + return `Stop-gate review skipped: cooldown active (${remainingMinutes}m remaining). Run /codex:review manually if needed.`; + } + } + } + + return null; +} + +// --- Config storage tests --- + +test("setConfig stores stopReviewGateMaxPerSession", () => { + const { workspace, cleanup } = setupWorkspace(); + try { + setConfig(workspace, "stopReviewGateMaxPerSession", 5); + const config = getConfig(workspace); + assert.equal(config.stopReviewGateMaxPerSession, 5); + } finally { + cleanup(); + } +}); + +test("setConfig stores stopReviewGateCooldownMinutes", () => { + const { workspace, cleanup } = setupWorkspace(); + try { + setConfig(workspace, "stopReviewGateCooldownMinutes", 10); + const config = getConfig(workspace); + assert.equal(config.stopReviewGateCooldownMinutes, 10); + } finally { + cleanup(); + } +}); + +test("setConfig clears throttle config with null", () => { + const { workspace, cleanup } = setupWorkspace(); + try { + setConfig(workspace, "stopReviewGateMaxPerSession", 5); + setConfig(workspace, "stopReviewGateMaxPerSession", null); + const config = getConfig(workspace); + assert.equal(config.stopReviewGateMaxPerSession, null); + } finally { + cleanup(); + } +}); + +test("default config has null throttle values", () => { + const { workspace, cleanup } = setupWorkspace(); + try { + const config = getConfig(workspace); + assert.equal(config.stopReviewGateMaxPerSession, null); + assert.equal(config.stopReviewGateCooldownMinutes, null); + assert.equal(config.stopReviewGate, false); + } finally { + cleanup(); + } +}); + +test("loadState merges throttle defaults into existing state without throttle keys", () => { + const { workspace, cleanup } = setupWorkspace(); + try { + const stateFile = resolveStateFile(workspace); + fs.mkdirSync(path.dirname(stateFile), { recursive: true }); + fs.writeFileSync( + stateFile, + JSON.stringify({ + version: 1, + config: { stopReviewGate: true }, + jobs: [] + }), + "utf8" + ); + + const state = loadState(workspace); + assert.equal(state.config.stopReviewGate, true); + assert.equal(state.config.stopReviewGateMaxPerSession, null); + assert.equal(state.config.stopReviewGateCooldownMinutes, null); + } finally { + cleanup(); + } +}); + +// --- Throttle logic tests --- + +test("checkThrottleLimits returns null when no limits are configured", () => { + const config = { stopReviewGateMaxPerSession: null, stopReviewGateCooldownMinutes: null }; + const result = checkThrottleLimits(config, []); + assert.equal(result, null); +}); + +test("checkThrottleLimits returns null when under session limit", () => { + const config = { stopReviewGateMaxPerSession: 3, stopReviewGateCooldownMinutes: null }; + const jobs = [ + { jobClass: "review", title: "Codex Stop Gate Review", status: "completed" }, + { jobClass: "review", title: "Codex Stop Gate Review", status: "completed" } + ]; + const result = checkThrottleLimits(config, jobs); + assert.equal(result, null); +}); + +test("checkThrottleLimits blocks when session limit reached", () => { + const config = { stopReviewGateMaxPerSession: 2, stopReviewGateCooldownMinutes: null }; + const jobs = [ + { jobClass: "review", title: "Codex Stop Gate Review", status: "completed" }, + { jobClass: "review", title: "Codex Stop Gate Review", status: "completed" } + ]; + const result = checkThrottleLimits(config, jobs); + assert.match(result, /session limit reached \(2\/2\)/); +}); + +test("checkThrottleLimits does not count running or queued jobs toward limit", () => { + const config = { stopReviewGateMaxPerSession: 2, stopReviewGateCooldownMinutes: null }; + const jobs = [ + { jobClass: "review", title: "Codex Stop Gate Review", status: "completed" }, + { jobClass: "review", title: "Codex Stop Gate Review", status: "running" }, + { jobClass: "review", title: "Codex Stop Gate Review", status: "queued" } + ]; + const result = checkThrottleLimits(config, jobs); + assert.equal(result, null); +}); + +test("checkThrottleLimits ignores non-stop-gate review jobs", () => { + const config = { stopReviewGateMaxPerSession: 1, stopReviewGateCooldownMinutes: null }; + const jobs = [ + { jobClass: "review", title: "Codex Review", status: "completed" }, + { jobClass: "review", title: "Codex Adversarial Review", status: "completed" } + ]; + const result = checkThrottleLimits(config, jobs); + assert.equal(result, null); +}); + +test("checkThrottleLimits blocks when cooldown is active", () => { + const config = { stopReviewGateMaxPerSession: null, stopReviewGateCooldownMinutes: 10 }; + const recentTime = new Date(Date.now() - 3 * 60 * 1000).toISOString(); // 3 minutes ago + const jobs = [ + { jobClass: "review", title: "Codex Stop Gate Review", status: "completed", completedAt: recentTime } + ]; + const result = checkThrottleLimits(config, jobs); + assert.match(result, /cooldown active/); +}); + +test("checkThrottleLimits allows when cooldown has elapsed", () => { + const config = { stopReviewGateMaxPerSession: null, stopReviewGateCooldownMinutes: 5 }; + const oldTime = new Date(Date.now() - 10 * 60 * 1000).toISOString(); // 10 minutes ago + const jobs = [ + { jobClass: "review", title: "Codex Stop Gate Review", status: "completed", completedAt: oldTime } + ]; + const result = checkThrottleLimits(config, jobs); + assert.equal(result, null); +}); + +test("checkThrottleLimits allows when no previous stop-gate review exists for cooldown", () => { + const config = { stopReviewGateMaxPerSession: null, stopReviewGateCooldownMinutes: 10 }; + const result = checkThrottleLimits(config, []); + assert.equal(result, null); +});