diff --git a/AGENTS.md b/AGENTS.md index b083465..4b2e0ed 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,9 +46,11 @@ Plus a **Constraints** block that forbids: touching files outside the list, rena ## Where things live -- `plugins/cursor/scripts/.mjs` — command entrypoints (10; `adversarial-review` has no script of its own — it reuses `review.mjs --adversarial`). `session-hook.mjs` is the one non-command script: the SessionStart/SessionEnd hook entrypoint. -- `plugins/cursor/hooks/hooks.json` — Claude Code hook registration (session lifecycle). -- `plugins/cursor/scripts/lib/*.mjs` — shared helpers (run, id, args, paths, jobs, kill, parse, cursor, git, invoked, plan, hints, md). +- `plugins/cursor/scripts/.mjs` — command entrypoints (10; `adversarial-review` has no script of its own — it reuses `review.mjs --adversarial`). Two non-command scripts: `session-hook.mjs` (SessionStart/SessionEnd) and `stop-review-gate-hook.mjs` (Stop). +- `plugins/cursor/hooks/hooks.json` — Claude Code hook registration (session lifecycle + optional stop review gate). +- `plugins/cursor/prompts/*.md` — prompt templates (`{{UPPER_SNAKE}}` placeholders, loaded via `lib/prompts.mjs`). Edit prompts here, not as JS string arrays. +- `plugins/cursor/schemas/review-output.schema.json` — the structured-review contract; validated hand-rolled in `lib/review-output.mjs` (zero-deps — do not add a schema-validator package). +- `plugins/cursor/scripts/lib/*.mjs` — shared helpers (run, id, args, paths, jobs, kill, parse, cursor, git, invoked, plan, hints, md, prompts, config, review-output). - `plugins/cursor/commands/*.md` — slash command wrappers. - `plugins/cursor/agents/cursor-runner.md` — the handoff subagent prompt. - `plugins/cursor/skills/composer-prompting/` — Cursor prompt-shaping guidance the `cursor-runner` subagent references via its `skills:` frontmatter. `SKILL.md` is the always-loaded spine; the detailed material lives in `references/*.md` (prompt anatomy, model selection, anti-patterns), loaded on demand. diff --git a/CHANGELOG.md b/CHANGELOG.md index 45cf59b..ad26644 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Stop review gate** (opt-in, per repo) — `/cursor:setup --enable-review-gate`. Before Claude Code ends a turn, the new `Stop` hook (`scripts/stop-review-gate-hook.mjs`) has a Cursor model review that turn's edits against `prompts/stop-review-gate.md`; the run must answer `ALLOW: …` or `BLOCK: …` on its first line, and a `BLOCK` keeps the turn going with the reason surfaced to Claude. Ported from `openai/codex-plugin-cc`'s stop-time review gate, with two hardening additions: `stop_hook_active` short-circuits so a block can never loop forever, and a missing/logged-out Cursor CLI degrades to a stderr note instead of blocking the session. Per-repo toggle persisted by the new `lib/config.mjs` (`/config/.json`). +- **Structured review output** — review prompts now ask the model to append a fenced ```json verdict block (contract: `schemas/review-output.schema.json` — verdict, summary, findings with severity/file/line, next steps). When it parses (hand-rolled validation in `lib/review-output.mjs`, zero-deps), the data is stored on the job record as `review` and the raw JSON fence is stripped from the human-facing summary; `/cursor:result --json` exposes it. Reviews from models that ignore the instruction stay unstructured — parsing never fails a review. +- **Prompt templates externalized** — the review prompt moved from a JS string array in `review.mjs` to `prompts/review.md` (`{{UPPER_SNAKE}}` placeholders, new `lib/prompts.mjs` loader/interpolator, blank-line collapse for empty optional blocks). Prompts are now reviewable as prose in PRs. - **Session lifecycle hooks** (`hooks/hooks.json` + `scripts/session-hook.mjs`), modelled on `openai/codex-plugin-cc`. **SessionStart** exports the Claude session id (and `CLAUDE_PLUGIN_DATA`) into the session env via `CLAUDE_ENV_FILE`, so every job records which session started it (`sessionId` on the job record, stamped in `createJob`). **SessionEnd** cancels the session's still-running jobs — a closed Claude session no longer leaves detached workers and `cursor-agent` running unattended. Jobs from other sessions or without a session stamp are never touched. - **`/cursor:status` scopes its default view to the current session.** The no-arg table now shows this session's jobs plus unattributed ones, with a hint line when rows are hidden; `--all` lifts both the session scope and the 10-row cap. New `lib/jobs.mjs#filterJobsForSession`. - **`--json` on `/cursor:status`, `/cursor:result`, `/cursor:setup`.** Status and result emit the raw job record(s); setup emits the full doctor report (`checks[].ok`, `allOk`). Meant for scripting and future hooks that branch on job state instead of parsing Markdown. diff --git a/README.md b/README.md index bc53a70..f4c9cc0 100644 --- a/README.md +++ b/README.md @@ -316,13 +316,23 @@ Shortcut for `/cursor:delegate --resume `. Without a task, sends an emp Shells out to `cursor-agent ls` and lists Cursor's own chat sessions for this repo. If that call times out or returns empty, the plugin falls back to its local job registry. -### `/cursor:setup [--doctor] [--print-models] [--install] [--json]` +### `/cursor:setup [--doctor] [--print-models] [--install] [--json] [--enable-review-gate|--disable-review-gate]` -Runs a quick health-check. `--doctor` produces extended diagnostics (Node version, PATH, `CURSOR_API_KEY` presence masked, jobs dir writability, cursor-agent version). `--print-models` shells out to `cursor-agent --list-models`. `--install` prints the install command but **does not run it** — you must copy-paste it yourself. `--json` emits the full doctor report as JSON (`checks[].ok`, `allOk`) for scripting. +Runs a quick health-check. `--doctor` produces extended diagnostics (Node version, PATH, `CURSOR_API_KEY` presence masked, jobs dir writability, cursor-agent version). `--print-models` shells out to `cursor-agent --list-models`. `--install` prints the install command but **does not run it** — you must copy-paste it yourself. `--json` emits the full doctor report as JSON (`checks[].ok`, `allOk`) for scripting. `--enable-review-gate` / `--disable-review-gate` toggle the per-repo [stop review gate](#stop-review-gate-opt-in). + +## Stop review gate (opt-in) + +``` +/cursor:setup --enable-review-gate # per repo; --disable-review-gate turns it off +``` + +When enabled, a `Stop` hook has a Cursor model review each turn's edits **before Claude Code is allowed to end the turn**. The reviewer answers `ALLOW: …` or `BLOCK: …`; a block feeds the reason back to Claude, which keeps working until the issue is fixed. Turns without code changes are allowed through immediately, a missing or logged-out Cursor CLI degrades to a note instead of blocking, and the gate never fires twice in a row for the same stop (no loops). Off by default. + +Review runs also emit a machine-readable verdict: the review prompt asks for a trailing JSON block (`schemas/review-output.schema.json` — verdict, findings with severity/file/line, next steps). When present, it is stored on the job record and available via `/cursor:result --json`; the human-facing summary stays pure Markdown. ## Session lifecycle -The plugin registers two Claude Code hooks (`hooks/hooks.json`): +The plugin registers Claude Code hooks (`hooks/hooks.json`): - **SessionStart** exports the Claude session id into the session's environment, so every job created from that session is stamped with it. `/cursor:status` uses the stamp to scope its default view to *your* jobs — parallel Claude sessions in the same repo stop seeing each other's runs (use `--all` for everything). - **SessionEnd** cancels the session's still-running jobs. Background delegates run as detached workers, so without this a closed Claude session would leave `cursor-agent` running unattended. Jobs from other sessions — or jobs with no session stamp — are left alone. diff --git a/plugins/cursor/.prettierignore b/plugins/cursor/.prettierignore index ff9d47c..bd02fe9 100644 --- a/plugins/cursor/.prettierignore +++ b/plugins/cursor/.prettierignore @@ -4,3 +4,4 @@ coverage package-lock.json *.ndjson .cursor-plugin-cc +prompts/ diff --git a/plugins/cursor/commands/setup.md b/plugins/cursor/commands/setup.md index 4bfe1d2..e448f1f 100644 --- a/plugins/cursor/commands/setup.md +++ b/plugins/cursor/commands/setup.md @@ -1,9 +1,11 @@ --- -description: Health-check Cursor CLI, list models, or guide installation. -argument-hint: '[--doctor] [--print-models] [--install] [--json]' +description: Health-check Cursor CLI, list models, guide installation, or toggle the stop-time review gate. +argument-hint: '[--doctor] [--print-models] [--install] [--json] [--enable-review-gate|--disable-review-gate]' allowed-tools: Bash(node:*) --- !`node "${CLAUDE_PLUGIN_ROOT}/scripts/setup.mjs" -- "$ARGUMENTS"` Present the check results as-is. If any check failed, tell the user concretely what to do (install cursor-agent, run `cursor-agent login`, run `npm install` inside the plugin). Never attempt to run the installer yourself. + +`--enable-review-gate` / `--disable-review-gate` toggle the per-repo stop-time review gate: when enabled, a Cursor model reviews each turn's edits before Claude Code is allowed to stop, and a `BLOCK` verdict keeps the turn going. Relay the confirmation message verbatim. diff --git a/plugins/cursor/hooks/hooks.json b/plugins/cursor/hooks/hooks.json index 684d30a..a92e014 100644 --- a/plugins/cursor/hooks/hooks.json +++ b/plugins/cursor/hooks/hooks.json @@ -1,6 +1,17 @@ { - "description": "Session lifecycle for Cursor jobs: stamp jobs with the owning Claude session, cancel the session's running jobs on exit.", + "description": "Session lifecycle for Cursor jobs (stamp + cleanup) and the optional stop-time review gate.", "hooks": { + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/stop-review-gate-hook.mjs\"", + "timeout": 900 + } + ] + } + ], "SessionStart": [ { "hooks": [ diff --git a/plugins/cursor/prompts/review.md b/plugins/cursor/prompts/review.md new file mode 100644 index 0000000..6da3a57 --- /dev/null +++ b/plugins/cursor/prompts/review.md @@ -0,0 +1,43 @@ +{{ROLE}} + +**Review target:** {{REVIEW_TARGET}} + +{{FOCUS_BLOCK}} + +Review ONLY the changes below — not the entire codebase. + +{{BODY}} + +--- + +**How to respond:** + +- Group findings by severity: **Blocking**, **Should-fix**, **Nits**. +- For each finding: `path:line` — what is wrong, why it matters, and a concrete fix (described, not applied). +- Flag correctness bugs, security holes, missing error handling, broken or missing tests, and deviations from the repo conventions (read `AGENTS.md` / `.cursor/rules` / `CLAUDE.md` if present). +{{ADVERSARIAL_GUIDANCE}} +- End with a one-line verdict on its own line: **APPROVE**, **APPROVE WITH NITS**, or **REQUEST CHANGES**. +- After the verdict, append a fenced ```json code block that restates the review as machine-readable data (no new content), shaped exactly like this: + +```json +{ + "verdict": "approve | approve-with-nits | request-changes", + "summary": "one-sentence overall assessment", + "findings": [ + { + "severity": "blocking | should-fix | nit", + "title": "short finding title", + "file": "path/relative/to/repo", + "line": 42, + "body": "what is wrong and why it matters", + "recommendation": "the concrete fix, described" + } + ], + "next_steps": ["optional follow-up actions"] +} +``` + +**Hard constraints:** + +- This is a **READ-ONLY review.** Do NOT modify, create, or delete any files. Do NOT run commands that change state. Do NOT stage or commit anything. If you spot a bug, describe the fix — never apply it. +- If the diff above was truncated, you MAY read specific files for context (read-only), but never edit them. diff --git a/plugins/cursor/prompts/stop-review-gate.md b/plugins/cursor/prompts/stop-review-gate.md new file mode 100644 index 0000000..548e154 --- /dev/null +++ b/plugins/cursor/prompts/stop-review-gate.md @@ -0,0 +1,40 @@ + +Run a stop-gate review of the previous Claude turn. +Only review the work from the previous Claude turn. +Only review it if Claude actually made code changes in that turn. +Pure status, setup, or reporting output does not count as reviewable work. +For example, the output of /cursor:setup or /cursor:status does not count. +Only direct edits made in that specific turn count. +If the previous Claude turn was only a status update, a summary, a setup/login check, a review result, or output from a command that did not itself make direct edits in that turn, return ALLOW immediately and do no further work. +Challenge whether that specific work and its design choices should ship. + +{{CLAUDE_RESPONSE_BLOCK}} + + + +Return a compact final answer. +Your first line must be exactly one of: +- ALLOW: +- BLOCK: +Do not put anything before that first line. + + + +Use ALLOW if the previous turn did not make code changes or if you do not see a blocking issue. +Use ALLOW immediately, without extra investigation, if the previous turn was not an edit-producing turn. +Use BLOCK only if the previous turn made code changes and you found something that still needs to be fixed before stopping. + + + +Ground every blocking claim in the repository context or tool outputs you inspected during this run. +Do not treat the previous Claude response as proof that code changes happened; verify that from the repository state before you block. +Do not block based on older edits from earlier turns when the immediately previous turn did not itself make direct edits. + + + +If the previous turn did make code changes, check for second-order failures, empty-state behavior, retries, stale state, rollback risk, and design tradeoffs before you finalize. + + + +This is a READ-ONLY check. Do NOT modify, create, or delete any files. Do NOT run commands that change state. Do NOT stage or commit anything. + diff --git a/plugins/cursor/schemas/review-output.schema.json b/plugins/cursor/schemas/review-output.schema.json new file mode 100644 index 0000000..c38b20d --- /dev/null +++ b/plugins/cursor/schemas/review-output.schema.json @@ -0,0 +1,58 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "Cursor review structured output", + "description": "The fenced ```json block a /cursor:review run appends after its Markdown findings. Parsed by scripts/lib/review-output.mjs and stored on the job record as `review`.", + "type": "object", + "additionalProperties": false, + "required": ["verdict", "summary", "findings"], + "properties": { + "verdict": { + "type": "string", + "enum": ["approve", "approve-with-nits", "request-changes"] + }, + "summary": { + "type": "string", + "minLength": 1 + }, + "findings": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "required": ["severity", "title", "file", "body"], + "properties": { + "severity": { + "type": "string", + "enum": ["blocking", "should-fix", "nit"] + }, + "title": { + "type": "string", + "minLength": 1 + }, + "file": { + "type": "string", + "minLength": 1 + }, + "line": { + "type": ["integer", "null"], + "minimum": 1 + }, + "body": { + "type": "string", + "minLength": 1 + }, + "recommendation": { + "type": "string" + } + } + } + }, + "next_steps": { + "type": "array", + "items": { + "type": "string", + "minLength": 1 + } + } + } +} diff --git a/plugins/cursor/scripts/lib/config.mjs b/plugins/cursor/scripts/lib/config.mjs new file mode 100644 index 0000000..ba0f5cb --- /dev/null +++ b/plugins/cursor/scripts/lib/config.mjs @@ -0,0 +1,42 @@ +// Per-repo plugin config, persisted at `/config/.json` +// (deliberately outside jobs// so config never mixes with job +// records). Currently the only key is the stop-review-gate toggle. + +import { readFileSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { ensureDir, pluginHome, repoHash } from './paths.mjs'; + +const DEFAULTS = Object.freeze({ stopReviewGate: false }); + +/** + * @param {string} repoPath + * @returns {string} + */ +export function configPath(repoPath) { + return join(pluginHome(), 'config', `${repoHash(repoPath)}.json`); +} + +/** + * @param {string} repoPath + * @returns {{stopReviewGate: boolean}} + */ +export function getConfig(repoPath) { + try { + return { ...DEFAULTS, ...JSON.parse(readFileSync(configPath(repoPath), 'utf8')) }; + } catch { + return { ...DEFAULTS }; + } +} + +/** + * @param {string} repoPath + * @param {string} key + * @param {unknown} value + * @returns {{stopReviewGate: boolean}} + */ +export function setConfigValue(repoPath, key, value) { + ensureDir(join(pluginHome(), 'config')); + const next = { ...getConfig(repoPath), [key]: value }; + writeFileSync(configPath(repoPath), JSON.stringify(next, null, 2) + '\n', 'utf8'); + return next; +} diff --git a/plugins/cursor/scripts/lib/jobs.mjs b/plugins/cursor/scripts/lib/jobs.mjs index 6edaab4..bbfdcaf 100644 --- a/plugins/cursor/scripts/lib/jobs.mjs +++ b/plugins/cursor/scripts/lib/jobs.mjs @@ -39,6 +39,7 @@ export const SESSION_ID_ENV = 'CURSOR_PLUGIN_CC_SESSION_ID'; * @property {boolean=} background * @property {boolean=} cloud * @property {string=} sessionId + * @property {import('./review-output.mjs').ReviewOutput=} review */ /** diff --git a/plugins/cursor/scripts/lib/prompts.mjs b/plugins/cursor/scripts/lib/prompts.mjs new file mode 100644 index 0000000..1a6969a --- /dev/null +++ b/plugins/cursor/scripts/lib/prompts.mjs @@ -0,0 +1,31 @@ +// Prompt templates live in `/prompts/*.md` so they can be +// reviewed and diffed as prose instead of JS string arrays. Placeholders use +// `{{UPPER_SNAKE}}`; unknown keys interpolate to the empty string. + +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +// lib/ → scripts/ → plugin root +const PLUGIN_ROOT = dirname(dirname(dirname(fileURLToPath(import.meta.url)))); + +/** + * @param {string} name Template basename without extension (e.g. "review"). + * @returns {string} + */ +export function loadPromptTemplate(name) { + return readFileSync(join(PLUGIN_ROOT, 'prompts', `${name}.md`), 'utf8'); +} + +/** + * @param {string} template + * @param {Record} variables + * @returns {string} + */ +export function interpolateTemplate(template, variables) { + const filled = template.replace(/\{\{([A-Z_]+)\}\}/g, (_, key) => + Object.prototype.hasOwnProperty.call(variables, key) ? String(variables[key] ?? '') : '', + ); + // Optional blocks interpolate to '' and would leave runs of blank lines. + return filled.replace(/\n{3,}/g, '\n\n'); +} diff --git a/plugins/cursor/scripts/lib/review-output.mjs b/plugins/cursor/scripts/lib/review-output.mjs new file mode 100644 index 0000000..e87359a --- /dev/null +++ b/plugins/cursor/scripts/lib/review-output.mjs @@ -0,0 +1,104 @@ +// Parse the structured ```json block a review run appends after its Markdown +// findings (contract: schemas/review-output.schema.json). Hand-rolled +// structural validation — no schema-validator dependency (zero-deps rule). +// Reviews from models that ignore the instruction simply stay unstructured; +// nothing here is allowed to fail a review. + +export const VERDICTS = ['approve', 'approve-with-nits', 'request-changes']; +export const SEVERITIES = ['blocking', 'should-fix', 'nit']; + +const FENCE_RE = /```json\s*\n([\s\S]*?)```/g; + +/** + * Last fenced ```json block in the text (the verdict block is instructed to + * come last; earlier blocks may be quoted diff content). + * + * @param {string} text + * @returns {string|null} + */ +export function extractJsonBlock(text) { + let last = null; + for (const m of String(text ?? '').matchAll(FENCE_RE)) last = m[1]; + return last; +} + +/** + * Strip the trailing structured block from a review summary for display — + * the data lives on the job record; the JSON fence is noise for a human. + * + * @param {string} text + * @returns {string} + */ +export function stripJsonBlock(text) { + const s = String(text ?? ''); + const matches = [...s.matchAll(FENCE_RE)]; + const last = matches[matches.length - 1]; + if (!last) return s; + return (s.slice(0, last.index) + s.slice(last.index + last[0].length)).trim(); +} + +/** + * @param {unknown} f + * @returns {boolean} + */ +function isValidFinding(f) { + if (!f || typeof f !== 'object' || Array.isArray(f)) return false; + const o = /** @type {Record} */ (f); + if (!SEVERITIES.includes(String(o.severity))) return false; + if (typeof o.title !== 'string' || !o.title.trim()) return false; + if (typeof o.file !== 'string' || !o.file.trim()) return false; + if (typeof o.body !== 'string' || !o.body.trim()) return false; + if (o.line != null && (!Number.isInteger(o.line) || Number(o.line) < 1)) return false; + if (o.recommendation != null && typeof o.recommendation !== 'string') return false; + return true; +} + +/** + * @typedef {Object} ReviewOutput + * @property {'approve'|'approve-with-nits'|'request-changes'} verdict + * @property {string} summary + * @property {Array<{severity: string, title: string, file: string, line?: number|null, body: string, recommendation?: string}>} findings + * @property {string[]} next_steps + */ + +/** + * @param {string} text Full review summary text. + * @returns {{ok: true, data: ReviewOutput} | {ok: false, error: string}} + */ +export function parseReviewOutput(text) { + const block = extractJsonBlock(text); + if (!block) return { ok: false, error: 'no fenced json block found' }; + let parsed; + try { + parsed = JSON.parse(block); + } catch (err) { + return { + ok: false, + error: `invalid JSON: ${err instanceof Error ? err.message : String(err)}`, + }; + } + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + return { ok: false, error: 'top level is not an object' }; + } + if (!VERDICTS.includes(String(parsed.verdict))) { + return { ok: false, error: `unknown verdict: ${String(parsed.verdict)}` }; + } + if (typeof parsed.summary !== 'string' || !parsed.summary.trim()) { + return { ok: false, error: 'missing summary' }; + } + if (!Array.isArray(parsed.findings) || !parsed.findings.every(isValidFinding)) { + return { ok: false, error: 'findings missing or malformed' }; + } + const nextSteps = Array.isArray(parsed.next_steps) + ? parsed.next_steps.filter((s) => typeof s === 'string' && s.trim()) + : []; + return { + ok: true, + data: { + verdict: parsed.verdict, + summary: parsed.summary, + findings: parsed.findings, + next_steps: nextSteps, + }, + }; +} diff --git a/plugins/cursor/scripts/review.mjs b/plugins/cursor/scripts/review.mjs index b850f8f..97cf63d 100644 --- a/plugins/cursor/scripts/review.mjs +++ b/plugins/cursor/scripts/review.mjs @@ -14,6 +14,8 @@ import { } from './lib/jobs.mjs'; import { ensureDir, jobsDir, logsDir } from './lib/paths.mjs'; import { extractChatId, summariseEvents } from './lib/parse.mjs'; +import { interpolateTemplate, loadPromptTemplate } from './lib/prompts.mjs'; +import { parseReviewOutput, stripJsonBlock } from './lib/review-output.mjs'; const BOOLEAN_FLAGS = ['background', 'wait', 'adversarial', 'git-check', 'help']; @@ -40,37 +42,18 @@ function parseFlags(argv) { } export function buildReviewPrompt({ label, body, focus, adversarial }) { - const role = adversarial - ? 'You are a skeptical senior engineer running an ADVERSARIAL review. Challenge whether the chosen implementation and design are the right approach — question assumptions, tradeoffs, and failure modes, not only the implementation defects.' - : 'You are a senior code reviewer.'; - const lines = [role, '', `**Review target:** ${label}`]; - if (focus) lines.push('', `**Reviewer focus (prioritise this):** ${focus}`); - lines.push( - '', - 'Review ONLY the changes below — not the entire codebase.', - '', - body, - '---', - '', - '**How to respond:**', - '- Group findings by severity: **Blocking**, **Should-fix**, **Nits**.', - '- For each finding: `path:line` — what is wrong, why it matters, and a concrete fix (described, not applied).', - '- Flag correctness bugs, security holes, missing error handling, broken or missing tests, and deviations from the repo conventions (read `AGENTS.md` / `.cursor/rules` / `CLAUDE.md` if present).', - ); - if (adversarial) { - lines.push( - '- Challenge the design: is this the right approach? What assumptions does it rest on? Where does it break under real-world load, concurrency, or edge cases?', - '- Would a different approach be simpler, safer, or cheaper to maintain? If so, say what and why — but stay grounded in the diff, do not invent requirements.', - ); - } - lines.push( - '- End with a one-line verdict on its own line: **APPROVE**, **APPROVE WITH NITS**, or **REQUEST CHANGES**.', - '', - '**Hard constraints:**', - '- This is a **READ-ONLY review.** Do NOT modify, create, or delete any files. Do NOT run commands that change state. Do NOT stage or commit anything. If you spot a bug, describe the fix — never apply it.', - '- If the diff above was truncated, you MAY read specific files for context (read-only), but never edit them.', - ); - return lines.join('\n'); + const template = loadPromptTemplate('review'); + return interpolateTemplate(template, { + ROLE: adversarial + ? 'You are a skeptical senior engineer running an ADVERSARIAL review. Challenge whether the chosen implementation and design are the right approach — question assumptions, tradeoffs, and failure modes, not only the implementation defects.' + : 'You are a senior code reviewer.', + REVIEW_TARGET: label, + FOCUS_BLOCK: focus ? `**Reviewer focus (prioritise this):** ${focus}` : '', + BODY: body, + ADVERSARIAL_GUIDANCE: adversarial + ? '- Challenge the design: is this the right approach? What assumptions does it rest on? Where does it break under real-world load, concurrency, or edge cases?\n- Would a different approach be simpler, safer, or cheaper to maintain? If so, say what and why — but stay grounded in the diff, do not invent requirements.' + : '', + }); } function renderResult(out, { jobId, status, summary, chatId, warnings }) { @@ -127,18 +110,24 @@ async function runReview({ flags, context, jobId, root, onEvent }) { const warnings = postFlightWarnings(summary); const status = result.exitCode === 0 && summary.success && warnings.length === 0 ? 'done' : 'failed'; + // The prompt asks the reviewer to append a fenced ```json verdict block + // (schemas/review-output.schema.json). When it parses, store the data on + // the record and hide the raw fence from the human-facing summary. + const structured = parseReviewOutput(summary.summary ?? ''); + const displaySummary = structured.ok ? stripJsonBlock(summary.summary ?? '') : summary.summary; updateJob(root, jobId, { status, exitCode: result.exitCode, finishedAt: new Date().toISOString(), summary: warnings.length > 0 - ? `${summary.summary}\n\n[plugin post-flight]\n${warnings.join('\n\n')}` - : summary.summary, + ? `${displaySummary}\n\n[plugin post-flight]\n${warnings.join('\n\n')}` + : displaySummary, filesTouched: summary.filesTouched, + ...(structured.ok ? { review: structured.data } : {}), ...(chatId ? { cursorChatId: chatId } : {}), }); - return { result, summary, chatId, warnings, status }; + return { result, summary: { ...summary, summary: displaySummary }, chatId, warnings, status }; } async function foreground(flags, context, jobId, root) { diff --git a/plugins/cursor/scripts/setup.mjs b/plugins/cursor/scripts/setup.mjs index 128f883..71e2681 100644 --- a/plugins/cursor/scripts/setup.mjs +++ b/plugins/cursor/scripts/setup.mjs @@ -3,7 +3,9 @@ import { accessSync, constants as fsConstants, existsSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { parseCommandArgv } from './lib/args.mjs'; +import { getConfig, setConfigValue } from './lib/config.mjs'; import { authStatus, listConfiguredMcps, listModels, resolveBin } from './lib/cursor.mjs'; +import { repoRoot } from './lib/git.mjs'; import { ensureDir, jobsDir, pluginHome } from './lib/paths.mjs'; import { run } from './lib/run.mjs'; @@ -80,6 +82,17 @@ async function gatherDoctor() { { ok: true, detail: apiKey ? `set (${maskKey(apiKey)})` : 'not set (using local session)' }, ]); + const root = await repoRoot(process.cwd()); + checks.push([ + 'stop review gate', + { + ok: true, + detail: getConfig(root).stopReviewGate + ? 'enabled for this repo (disable with `--disable-review-gate`)' + : 'disabled (enable with `--enable-review-gate`)', + }, + ]); + const mcps = bin ? await listConfiguredMcps() : []; // The CURSOR_API_KEY check is already `ok:true` whether or not the key is @@ -89,6 +102,26 @@ async function gatherDoctor() { return { bin, checks, mcps, allOk }; } +/** + * @param {boolean} enable + * @returns {Promise} + */ +async function toggleReviewGate(enable) { + const root = await repoRoot(process.cwd()); + setConfigValue(root, 'stopReviewGate', enable); + if (enable) { + process.stdout.write( + 'Stop review gate **enabled** for this repository.\n\n' + + 'Before Claude Code ends a turn, a Cursor model reviews the work from that turn; ' + + 'a `BLOCK: …` verdict keeps the session going until the issue is fixed. ' + + 'Disable anytime with `/cursor:setup --disable-review-gate`.\n', + ); + } else { + process.stdout.write('Stop review gate **disabled** for this repository.\n'); + } + return 0; +} + async function doctor(asJson = false) { const { bin, checks, mcps, allOk } = await gatherDoctor(); @@ -193,7 +226,16 @@ async function baseCheck() { * @returns {Promise} */ export async function main(rawArgv) { - const { flags } = parseCommandArgv(rawArgv, ['doctor', 'print-models', 'install', 'json']); + const { flags } = parseCommandArgv(rawArgv, [ + 'doctor', + 'print-models', + 'install', + 'json', + 'enable-review-gate', + 'disable-review-gate', + ]); + if (flags['enable-review-gate'] || flags['enableReviewGate']) return toggleReviewGate(true); + if (flags['disable-review-gate'] || flags['disableReviewGate']) return toggleReviewGate(false); // --json always emits the full structured doctor report — hooks and scripts // branch on `checks[].ok` / `allOk` instead of parsing Markdown. if (flags['json']) return doctor(true); diff --git a/plugins/cursor/scripts/stop-review-gate-hook.mjs b/plugins/cursor/scripts/stop-review-gate-hook.mjs new file mode 100644 index 0000000..1602bdd --- /dev/null +++ b/plugins/cursor/scripts/stop-review-gate-hook.mjs @@ -0,0 +1,176 @@ +#!/usr/bin/env node +// Claude Code Stop hook: optional review gate, off by default. +// +// When enabled (`/cursor:setup --enable-review-gate`), a Cursor model reviews +// the previous Claude turn before the session is allowed to stop. The run +// must answer `ALLOW: …` or `BLOCK: …` on its first line +// (prompts/stop-review-gate.md); a BLOCK is surfaced to Claude Code as +// `{"decision": "block", "reason": …}` so the turn continues with fixes. +// +// The gate never runs when Cursor is missing/unauthenticated (it degrades to +// a stderr note), and `stop_hook_active` short-circuits to ALLOW so a block +// can never loop forever. + +import { readFileSync } from 'node:fs'; +import { getConfig } from './lib/config.mjs'; +import { authStatus, resolveBin, resolveModel, runHeadless } from './lib/cursor.mjs'; +import { repoRoot } from './lib/git.mjs'; +import { id as newId } from './lib/id.mjs'; +import { SESSION_ID_ENV, listJobs, rawLogPath } from './lib/jobs.mjs'; +import { ensureDir, logsDir } from './lib/paths.mjs'; +import { summariseEvents } from './lib/parse.mjs'; +import { interpolateTemplate, loadPromptTemplate } from './lib/prompts.mjs'; +import { invokedAsScript as __isScript } from './lib/invoked.mjs'; + +const GATE_TIMEOUT_SEC = 600; + +/** @returns {Record} */ +function readHookInput() { + try { + const raw = readFileSync(0, 'utf8').trim(); + return raw ? JSON.parse(raw) : {}; + } catch { + return {}; + } +} + +/** + * First-line ALLOW/BLOCK contract, tolerant of markdown wrapping + * (`**ALLOW: fine**` etc.). + * + * @param {string} text + * @returns {{ok: boolean, reason: string|null}} + */ +export function parseGateOutput(text) { + const firstLine = ( + String(text ?? '') + .trim() + .split(/\r?\n/, 1)[0] ?? '' + ) + .replace(/^[#>*_`\s-]+/, '') + .trim(); + if (/^ALLOW\b/i.test(firstLine)) return { ok: true, reason: null }; + if (/^BLOCK\b/i.test(firstLine)) { + const reason = firstLine.replace(/^BLOCK[:\s]*/i, '').trim() || String(text).trim(); + return { + ok: false, + reason: `Cursor stop-gate review found issues that still need fixes before stopping: ${reason}`, + }; + } + return { + ok: false, + reason: + 'The stop-gate Cursor review returned an unexpected answer. Run `/cursor:review --wait` manually or disable the gate with `/cursor:setup --disable-review-gate`.', + }; +} + +/** + * @param {Record} input + * @param {string} root + * @returns {Promise<{ok: boolean, reason: string|null}>} + */ +async function runGateReview(input, root) { + const template = loadPromptTemplate('stop-review-gate'); + const last = String(input.last_assistant_message ?? '').trim(); + const prompt = interpolateTemplate(template, { + CLAUDE_RESPONSE_BLOCK: last ? `Previous Claude response:\n${last}` : '', + }); + ensureDir(logsDir(root)); + const result = await runHeadless({ + prompt, + model: resolveModel(undefined), + cloud: false, + force: true, + timeoutSec: GATE_TIMEOUT_SEC, + logPath: rawLogPath(root, `gate-${newId(8)}`), + cwd: root, + }); + if (result.killed) { + return { + ok: false, + reason: `The stop-gate Cursor review timed out after ${GATE_TIMEOUT_SEC / 60} minutes. Run \`/cursor:review --wait\` manually or disable the gate with \`/cursor:setup --disable-review-gate\`.`, + }; + } + const summary = summariseEvents(result.events); + if (result.exitCode !== 0 || !summary.success) { + return { + ok: false, + reason: `The stop-gate Cursor review failed (exit ${result.exitCode}). Run \`/cursor:review --wait\` manually or disable the gate with \`/cursor:setup --disable-review-gate\`.`, + }; + } + return parseGateOutput(summary.summary ?? ''); +} + +/** + * @param {Record} input + * @param {{out?: (s: string) => void, err?: (s: string) => void}} [io] + * @returns {Promise} + */ +export async function handleStop(input, io = {}) { + const out = io.out ?? ((s) => process.stdout.write(s)); + const err = io.err ?? ((s) => process.stderr.write(s)); + + // Claude Code sets stop_hook_active when the turn is already continuing + // because a stop hook blocked — never block again, or the gate would loop. + if (input.stop_hook_active) return 0; + + const cwd = typeof input.cwd === 'string' && input.cwd ? input.cwd : process.cwd(); + const root = await repoRoot(cwd); + + const sessionId = + (typeof input.session_id === 'string' && input.session_id) || process.env[SESSION_ID_ENV]; + const running = listJobs(root).filter( + (j) => j.status === 'running' && (!sessionId || !j.sessionId || j.sessionId === sessionId), + ); + const runningNote = + running.length > 0 + ? `Cursor job(s) still running: ${running.map((j) => `\`${j.id}\``).join(', ')} — check \`/cursor:status\`, cancel with \`/cursor:cancel \` if unwanted.` + : null; + + if (!getConfig(root).stopReviewGate) { + if (runningNote) err(runningNote + '\n'); + return 0; + } + + // Preflight: a misconfigured Cursor must degrade to a note, not a block. + try { + await resolveBin(); + } catch { + err('Stop review gate is enabled but cursor-agent is not installed. Run /cursor:setup.\n'); + if (runningNote) err(runningNote + '\n'); + return 0; + } + const auth = await authStatus(); + if (!auth.loggedIn) { + err('Stop review gate is enabled but Cursor CLI is not logged in. Run `cursor-agent login`.\n'); + if (runningNote) err(runningNote + '\n'); + return 0; + } + + const review = await runGateReview(input, root); + if (!review.ok) { + out( + JSON.stringify({ + decision: 'block', + reason: runningNote ? `${runningNote} ${review.reason}` : review.reason, + }) + '\n', + ); + return 0; + } + if (runningNote) err(runningNote + '\n'); + return 0; +} + +const invokedAsScript = __isScript(import.meta.url); + +if (invokedAsScript) { + handleStop(readHookInput()) + .then((code) => process.exit(code)) + .catch((err) => { + process.stderr.write( + `stop-review-gate failed: ${err instanceof Error ? err.message : String(err)}\n`, + ); + // Hook infrastructure failures must never trap the user in a session. + process.exit(0); + }); +} diff --git a/plugins/cursor/tests/config.test.mjs b/plugins/cursor/tests/config.test.mjs new file mode 100644 index 0000000..ac15fb5 --- /dev/null +++ b/plugins/cursor/tests/config.test.mjs @@ -0,0 +1,38 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { configPath, getConfig, setConfigValue } from '../scripts/lib/config.mjs'; +import { makeTempHome } from './helpers.mjs'; + +describe('config', () => { + let tmp; + const prevHome = process.env.CURSOR_PLUGIN_CC_HOME; + const repoA = '/tmp/repo-a'; + const repoB = '/tmp/repo-b'; + + beforeEach(() => { + tmp = makeTempHome(); + process.env.CURSOR_PLUGIN_CC_HOME = tmp.dir; + }); + + afterEach(() => { + if (prevHome === undefined) delete process.env.CURSOR_PLUGIN_CC_HOME; + else process.env.CURSOR_PLUGIN_CC_HOME = prevHome; + tmp.cleanup(); + }); + + it('defaults to stopReviewGate: false', () => { + expect(getConfig(repoA)).toEqual({ stopReviewGate: false }); + }); + + it('persists a toggled value per repo', () => { + setConfigValue(repoA, 'stopReviewGate', true); + expect(getConfig(repoA).stopReviewGate).toBe(true); + expect(getConfig(repoB).stopReviewGate).toBe(false); + setConfigValue(repoA, 'stopReviewGate', false); + expect(getConfig(repoA).stopReviewGate).toBe(false); + }); + + it('keeps config outside the jobs dir', () => { + expect(configPath(repoA)).toContain('/config/'); + expect(configPath(repoA)).not.toContain('/jobs/'); + }); +}); diff --git a/plugins/cursor/tests/fixtures/cursor-agent-stub.mjs b/plugins/cursor/tests/fixtures/cursor-agent-stub.mjs index fafee76..312b9cb 100755 --- a/plugins/cursor/tests/fixtures/cursor-agent-stub.mjs +++ b/plugins/cursor/tests/fixtures/cursor-agent-stub.mjs @@ -3,6 +3,17 @@ // the CURSOR_AGENT_STUB_FIXTURE env var, then exits. import { readFileSync } from 'node:fs'; +// `cursor-agent status` — auth check used by authStatus(). Controlled by +// CURSOR_AGENT_STUB_AUTH: 'out' simulates a logged-out CLI. +if (process.argv[2] === 'status') { + if (process.env.CURSOR_AGENT_STUB_AUTH === 'out') { + process.stdout.write('Not logged in\n'); + process.exit(1); + } + process.stdout.write('Logged in as test-stub\n'); + process.exit(0); +} + const fixture = process.env.CURSOR_AGENT_STUB_FIXTURE; if (!fixture) { process.stderr.write('stub: CURSOR_AGENT_STUB_FIXTURE not set\n'); diff --git a/plugins/cursor/tests/fixtures/cursor-events/gate-allow.ndjson b/plugins/cursor/tests/fixtures/cursor-events/gate-allow.ndjson new file mode 100644 index 0000000..e8748ce --- /dev/null +++ b/plugins/cursor/tests/fixtures/cursor-events/gate-allow.ndjson @@ -0,0 +1,2 @@ +{"type":"system","subtype":"init","session_id":"chat_gate_001","cwd":"/tmp/repo"} +{"type":"result","subtype":"success","chat_id":"chat_gate_001","result":"ALLOW: previous turn made no code changes."} diff --git a/plugins/cursor/tests/fixtures/cursor-events/gate-block.ndjson b/plugins/cursor/tests/fixtures/cursor-events/gate-block.ndjson new file mode 100644 index 0000000..291d12e --- /dev/null +++ b/plugins/cursor/tests/fixtures/cursor-events/gate-block.ndjson @@ -0,0 +1,2 @@ +{"type":"system","subtype":"init","session_id":"chat_gate_002","cwd":"/tmp/repo"} +{"type":"result","subtype":"success","chat_id":"chat_gate_002","result":"BLOCK: delegate.mjs timeout path has no test and the error branch swallows the exit code."} diff --git a/plugins/cursor/tests/gate.test.mjs b/plugins/cursor/tests/gate.test.mjs new file mode 100644 index 0000000..f1d07d6 --- /dev/null +++ b/plugins/cursor/tests/gate.test.mjs @@ -0,0 +1,99 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { setConfigValue } from '../scripts/lib/config.mjs'; +import { handleStop, parseGateOutput } from '../scripts/stop-review-gate-hook.mjs'; +import { STUB_BIN, makeTempHome } from './helpers.mjs'; + +const GATE_ALLOW_FIXTURE = new URL('./fixtures/cursor-events/gate-allow.ndjson', import.meta.url) + .pathname; +const GATE_BLOCK_FIXTURE = new URL('./fixtures/cursor-events/gate-block.ndjson', import.meta.url) + .pathname; + +describe('stop review gate', () => { + let tmp; + let stdout; + let stderr; + const io = { + out: (s) => { + stdout += s; + }, + err: (s) => { + stderr += s; + }, + }; + const prevHome = process.env.CURSOR_PLUGIN_CC_HOME; + const prevBin = process.env.CURSOR_AGENT_BIN; + const prevFix = process.env.CURSOR_AGENT_STUB_FIXTURE; + const prevAuth = process.env.CURSOR_AGENT_STUB_AUTH; + + beforeEach(() => { + tmp = makeTempHome(); + stdout = ''; + stderr = ''; + process.env.CURSOR_PLUGIN_CC_HOME = tmp.dir; + process.env.CURSOR_AGENT_BIN = STUB_BIN; + process.env.CURSOR_AGENT_STUB_FIXTURE = GATE_ALLOW_FIXTURE; + delete process.env.CURSOR_AGENT_STUB_AUTH; + }); + + afterEach(() => { + if (prevHome === undefined) delete process.env.CURSOR_PLUGIN_CC_HOME; + else process.env.CURSOR_PLUGIN_CC_HOME = prevHome; + if (prevBin === undefined) delete process.env.CURSOR_AGENT_BIN; + else process.env.CURSOR_AGENT_BIN = prevBin; + if (prevFix === undefined) delete process.env.CURSOR_AGENT_STUB_FIXTURE; + else process.env.CURSOR_AGENT_STUB_FIXTURE = prevFix; + if (prevAuth === undefined) delete process.env.CURSOR_AGENT_STUB_AUTH; + else process.env.CURSOR_AGENT_STUB_AUTH = prevAuth; + tmp.cleanup(); + }); + + describe('parseGateOutput', () => { + it('accepts ALLOW and BLOCK first lines, with markdown tolerance', () => { + expect(parseGateOutput('ALLOW: fine').ok).toBe(true); + expect(parseGateOutput('**ALLOW: fine**').ok).toBe(true); + const block = parseGateOutput('BLOCK: missing test\nmore detail'); + expect(block.ok).toBe(false); + expect(block.reason).toContain('missing test'); + }); + + it('treats anything else as unexpected (fails closed)', () => { + const res = parseGateOutput('I reviewed the code and it looks good.'); + expect(res.ok).toBe(false); + expect(res.reason).toContain('unexpected answer'); + }); + }); + + it('does nothing when the gate is disabled', async () => { + await handleStop({ cwd: tmp.dir }, io); + expect(stdout).toBe(''); + }); + + it('short-circuits when stop_hook_active is set (no loop)', async () => { + setConfigValue(tmp.dir, 'stopReviewGate', true); + await handleStop({ cwd: tmp.dir, stop_hook_active: true }, io); + expect(stdout).toBe(''); + }); + + it('allows silently on an ALLOW verdict', async () => { + setConfigValue(tmp.dir, 'stopReviewGate', true); + await handleStop({ cwd: tmp.dir, last_assistant_message: 'edited foo.ts' }, io); + expect(stdout).toBe(''); + }); + + it('emits a block decision on a BLOCK verdict', async () => { + setConfigValue(tmp.dir, 'stopReviewGate', true); + process.env.CURSOR_AGENT_STUB_FIXTURE = GATE_BLOCK_FIXTURE; + await handleStop({ cwd: tmp.dir, last_assistant_message: 'edited foo.ts' }, io); + const decision = JSON.parse(stdout); + expect(decision.decision).toBe('block'); + expect(decision.reason).toContain('timeout path has no test'); + }); + + it('degrades to a stderr note when Cursor CLI is logged out', async () => { + setConfigValue(tmp.dir, 'stopReviewGate', true); + process.env.CURSOR_AGENT_STUB_AUTH = 'out'; + await handleStop({ cwd: tmp.dir }, io); + expect(stdout).toBe(''); + expect(stderr).toContain('cursor-agent login'); + }); +}); diff --git a/plugins/cursor/tests/prompts.test.mjs b/plugins/cursor/tests/prompts.test.mjs new file mode 100644 index 0000000..1dfea8f --- /dev/null +++ b/plugins/cursor/tests/prompts.test.mjs @@ -0,0 +1,24 @@ +import { describe, expect, it } from 'vitest'; +import { interpolateTemplate, loadPromptTemplate } from '../scripts/lib/prompts.mjs'; + +describe('prompts', () => { + it('interpolates known keys and blanks unknown ones', () => { + const out = interpolateTemplate('a {{FOO}} b {{MISSING}} c', { FOO: 'x' }); + expect(out).toBe('a x b c'); + }); + + it('collapses blank-line runs left by empty optional blocks', () => { + const out = interpolateTemplate('line1\n\n{{OPT}}\n\nline2', { OPT: '' }); + expect(out).toBe('line1\n\nline2'); + }); + + it('loads the shipped templates', () => { + const review = loadPromptTemplate('review'); + expect(review).toContain('{{REVIEW_TARGET}}'); + expect(review).toContain('READ-ONLY review'); + const gate = loadPromptTemplate('stop-review-gate'); + expect(gate).toContain('{{CLAUDE_RESPONSE_BLOCK}}'); + expect(gate).toContain('ALLOW:'); + expect(gate).toContain('BLOCK:'); + }); +}); diff --git a/plugins/cursor/tests/review-output.test.mjs b/plugins/cursor/tests/review-output.test.mjs new file mode 100644 index 0000000..59746a6 --- /dev/null +++ b/plugins/cursor/tests/review-output.test.mjs @@ -0,0 +1,86 @@ +import { describe, expect, it } from 'vitest'; +import { + extractJsonBlock, + parseReviewOutput, + stripJsonBlock, +} from '../scripts/lib/review-output.mjs'; + +const GOOD = { + verdict: 'approve-with-nits', + summary: 'Solid change, one naming nit.', + findings: [ + { + severity: 'nit', + title: 'Magic number', + file: 'src/foo.ts', + line: 12, + body: '42 should be a named constant.', + recommendation: 'Extract MAX_RETRIES.', + }, + ], + next_steps: ['Rename the constant.'], +}; + +function withBlock(obj) { + return `## Findings\n\nsome markdown\n\nAPPROVE WITH NITS\n\n\`\`\`json\n${JSON.stringify(obj, null, 2)}\n\`\`\`\n`; +} + +describe('review-output', () => { + it('parses a valid trailing json block', () => { + const res = parseReviewOutput(withBlock(GOOD)); + expect(res.ok).toBe(true); + if (res.ok) { + expect(res.data.verdict).toBe('approve-with-nits'); + expect(res.data.findings).toHaveLength(1); + expect(res.data.next_steps).toEqual(['Rename the constant.']); + } + }); + + it('uses the LAST json block when several are present', () => { + const text = '```json\n{"quoted": "diff content"}\n```\n' + withBlock(GOOD); + const res = parseReviewOutput(text); + expect(res.ok).toBe(true); + expect(extractJsonBlock(text)).toContain('approve-with-nits'); + }); + + it('rejects an unknown verdict', () => { + const res = parseReviewOutput(withBlock({ ...GOOD, verdict: 'ship-it' })); + expect(res.ok).toBe(false); + }); + + it('rejects malformed findings', () => { + const res = parseReviewOutput( + withBlock({ ...GOOD, findings: [{ severity: 'blocking', title: 'x' }] }), + ); + expect(res.ok).toBe(false); + }); + + it('accepts null/absent line and missing next_steps', () => { + const res = parseReviewOutput( + withBlock({ + verdict: 'approve', + summary: 'ok', + findings: [{ severity: 'nit', title: 't', file: 'f', body: 'b', line: null }], + }), + ); + expect(res.ok).toBe(true); + if (res.ok) expect(res.data.next_steps).toEqual([]); + }); + + it('reports missing block and broken JSON distinctly', () => { + expect(parseReviewOutput('plain markdown only').ok).toBe(false); + const broken = 'text\n```json\n{not json\n```\n'; + const res = parseReviewOutput(broken); + expect(res.ok).toBe(false); + if (!res.ok) expect(res.error).toContain('invalid JSON'); + }); + + it('stripJsonBlock removes only the trailing block', () => { + const text = withBlock(GOOD); + const stripped = stripJsonBlock(text); + expect(stripped).toContain('APPROVE WITH NITS'); + expect(stripped).not.toContain('```json'); + // No block → unchanged. + expect(stripJsonBlock('no block here')).toBe('no block here'); + }); +});