diff --git a/.github/cursor-review/README.md b/.github/cursor-review/README.md index 15e8922..6fab887 100644 --- a/.github/cursor-review/README.md +++ b/.github/cursor-review/README.md @@ -35,7 +35,7 @@ PR gets the `cursor-review` label │ Google ▢ ▢ │ │ Moonshot ▢ ▢ │ │ │ - │ each cell: build prompt → run cursor-agent → extract-findings.py │ + │ each cell: cursor-agent records findings through a stdio MCP tool │ └───────────────────────────────┬───────────────────────────────────────┘ │ 8 findings artifacts ▼ @@ -71,9 +71,12 @@ Each model runs **two review types**: [`prompt-edge-case.md`](prompt-edge-case.md). A single **judge** model ([`prompt-judge.md`](prompt-judge.md)) then adjudicates -all 8 cells' findings into the final review. If a cell fails (checkout, agent, -extraction), it still shows up in the panel summary tagged `error` rather than -silently vanishing — the review tells you what didn't run. +all 8 cells' findings and submits the final review through the same slim stdio +MCP server. Model prose is never parsed for results: tool schemas validate the +records before writing them, which removes formatting drift, markdown fences, +truncated JSON, and reformat retries from the result path. If a cell fails +(checkout, agent, or tool submission), it still shows up in the panel summary +tagged `error` rather than silently vanishing. ## What's in this directory @@ -82,7 +85,7 @@ silently vanishing — the review tells you what didn't run. | [`prompt-adversarial.md`](prompt-adversarial.md) | Prompt for the security/reliability review pass. | | [`prompt-edge-case.md`](prompt-edge-case.md) | Prompt for the correctness/logic review pass. | | [`prompt-judge.md`](prompt-judge.md) | Prompt the judge model uses to consolidate panel findings into one review. | -| [`extract-findings.py`](extract-findings.py) | Parses a cell's raw `cursor-agent` output into a normalized findings record. Always emits structured JSON — even on empty output or parse failure — so the consolidate step has uniform input. | +| [`review-output-mcp.py`](review-output-mcp.py) | Dependency-free stdio MCP server. Reviewers record individual findings and finish; the judge submits the final findings array. It validates and atomically writes the normalized records consumed by later jobs. | | [`post-review.py`](post-review.py) | Reads the judge's consolidated findings and posts **one** PR review with line-anchored inline comments and severity badges. | | [`gate-unresolved.py`](gate-unresolved.py) | The opt-in blocking gate (`blocking: true`). Queries the PR's review threads and exits non-zero while any cursor-review finding thread is unresolved. | | [`slack-notify.sh`](slack-notify.sh) | Sends the start/complete Slack DMs to the triggerer (no-ops without a token). | diff --git a/.github/cursor-review/extract-findings.py b/.github/cursor-review/extract-findings.py deleted file mode 100644 index 5cdf2ca..0000000 --- a/.github/cursor-review/extract-findings.py +++ /dev/null @@ -1,193 +0,0 @@ -#!/usr/bin/env python3 -"""Parse raw cursor-agent output into a normalized findings record. - -Used by per-cell matrix steps AND by the judge/consolidate step. Each caller -converts the model's raw stdout into a JSON file the next step can ingest. The -output is always structured — even on parse failures or empty output — so the -downstream step has a uniform input. - -Output shape: - { - "model": str, - "review_type": str, - "status": "ok" | "empty" | "error" | "parse_error", - "findings": [{"file": str, "line": int, "side": "RIGHT", "body": str}, ...], - "error": str # only when status != "ok" - } -""" - -import argparse -import json -import re - - -def _try_load(snippet: str): - """json.loads `snippet`, returning the value only if it's a list or dict. - - A bare number/string/bool is never a findings payload, so reject it — this - keeps the brace-scan below from "succeeding" on a stray scalar. - """ - try: - value = json.loads(snippet) - except (json.JSONDecodeError, ValueError): - return None - return value if isinstance(value, (list, dict)) else None - - -def _iter_json_candidates(text: str): - """Yield each top-level balanced {...} / [...] region embedded in `text`. - - String- and escape-aware: braces or brackets inside JSON string literals - don't throw off the nesting count, so prose like `... the findings […] are` - surrounding a real array doesn't corrupt the match the way a naive - first-`[`/last-`]` slice does. Regions are yielded in document order; the - caller parses each and keeps the last that is findings-shaped. - """ - openers = {"{", "["} - closers = {"}", "]"} - i, n = 0, len(text) - while i < n: - if text[i] not in openers: - i += 1 - continue - depth = 0 - in_str = False - escape = False - j = i - while j < n: - c = text[j] - if in_str: - if escape: - escape = False - elif c == "\\": - escape = True - elif c == '"': - in_str = False - elif c == '"': - in_str = True - elif c in openers: - depth += 1 - elif c in closers: - depth -= 1 - if depth == 0: - yield text[i : j + 1] - break - j += 1 - # Resume scanning after this region (or after an unbalanced opener). - i = j + 1 - - -def parse_json_findings(raw_text: str): - """Extract the findings JSON value from raw model output. - - Tolerates surrounding prose and markdown fences. Returns the parsed value - (a findings array, or a `{"findings": [...]}` wrapper), or None if no - findings-shaped JSON could be located. - - Crucially this scans for a *findings-shaped* region, not merely the first - thing that parses as JSON, and prefers the LAST such region. The judge - (esp. on verification-heavy diffs, BE-3160) opens with prose that quotes - individual finding OBJECTS or scalar lists inline while reasoning, then - emits the real array LAST. Taking the first parseable region there yields - an un-coercible object (→ spurious parse_error) or a bogus scalar list, - while the genuine findings array sits further down. Layered so a clean - response still takes the fast path: - - 1. The whole output is the findings JSON. - 2. A fenced ```json (or bare ```) block holds it — last valid block wins. - 3. A balanced {...}/[...] region embedded in prose — last valid wins. - """ - text = raw_text.strip() - - # Fast path: the whole response is the findings payload. - whole = _try_load(text) - if coerce_findings_list(whole) is not None: - return whole - - # Fenced blocks: prose/verification precedes the answer, so the last - # findings-shaped fence is the real one. - best = None - for match in re.finditer(r"```(?:json)?\s*\n(.*?)```", text, re.DOTALL): - parsed = _try_load(match.group(1).strip()) - if coerce_findings_list(parsed) is not None: - best = parsed - if best is not None: - return best - - # Bare balanced regions embedded in prose: keep the LAST findings-shaped - # one so an inline finding object / scalar list quoted mid-reasoning never - # shadows the real array that follows it. - for candidate in _iter_json_candidates(text): - parsed = _try_load(candidate) - if coerce_findings_list(parsed) is not None: - best = parsed - return best - - -def coerce_findings_list(parsed): - """Reduce a parsed JSON value to the findings list, or None if it isn't one. - - A findings list is a JSON array of finding OBJECTS (an empty array is - allowed — "no findings"), or an object wrapping such an array under a - findings-like key. The panel cells and judge are asked for a bare JSON - array, but a model intermittently wraps it as `{"findings": [...]}` (or a - near-synonym key); unwrap those so a well-formed-but-wrapped response - parses instead of being discarded as a parse_error. - - Requiring the elements to be objects is what lets the extractor above skip - a scalar list the judge quotes in prose (e.g. `["contains", "startswith"]` - while narrating jq builtins) and keep scanning for the real findings array. - """ - if isinstance(parsed, list): - return parsed if all(isinstance(item, dict) for item in parsed) else None - if isinstance(parsed, dict): - for key in ("findings", "results", "items", "reviews"): - value = parsed.get(key) - if isinstance(value, list) and all(isinstance(item, dict) for item in value): - return value - return None - - -def main(): - parser = argparse.ArgumentParser() - parser.add_argument("--raw", required=True, help="Path to raw cursor-agent output") - parser.add_argument("--out", required=True, help="Path to write the findings JSON file") - parser.add_argument("--model", required=True) - parser.add_argument("--review-type", required=True) - args = parser.parse_args() - - record = {"model": args.model, "review_type": args.review_type} - - try: - with open(args.raw, encoding="utf-8") as f: - raw = f.read() - except (OSError, UnicodeDecodeError) as e: - record.update(status="error", error=f"Could not read raw output: {e}", findings=[]) - with open(args.out, "w", encoding="utf-8") as f: - json.dump(record, f) - return - - if not raw.strip(): - record.update(status="empty", error="Cursor agent produced empty output.", findings=[]) - with open(args.out, "w", encoding="utf-8") as f: - json.dump(record, f) - return - - findings = coerce_findings_list(parse_json_findings(raw)) - - if findings is None: - # Truncate raw so artifacts stay small even on chatty parse failures. - record.update( - status="parse_error", - error=f"Could not parse JSON findings from output. First 500 chars:\n{raw[:500]}", - findings=[], - ) - else: - record.update(status="ok", findings=findings) - - with open(args.out, "w", encoding="utf-8") as f: - json.dump(record, f) - - -if __name__ == "__main__": - main() diff --git a/.github/cursor-review/post-review.py b/.github/cursor-review/post-review.py index 9a83bfe..a08b1ae 100644 --- a/.github/cursor-review/post-review.py +++ b/.github/cursor-review/post-review.py @@ -12,7 +12,7 @@ ... ], "panel": [ - {"model": str, "review_type": str, "status": "ok"|"empty"|"error"|"parse_error"}, + {"model": str, "review_type": str, "status": "ok"|"error"}, ... ] } @@ -28,7 +28,7 @@ import sys # Severity scale, ordered most → least urgent. Drives sort order, the inline -# comment prefix, and the summary table. The judge is instructed to emit one +# comment prefix, and the summary table. The judge tool accepts one # of these strings per finding (see prompt-judge.md); anything missing or # unrecognized falls back to DEFAULT_SEVERITY so a malformed value can never # drop a finding — it just lands in the middle bucket. diff --git a/.github/cursor-review/prompt-adversarial.md b/.github/cursor-review/prompt-adversarial.md index 2b0a4e3..0875649 100644 --- a/.github/cursor-review/prompt-adversarial.md +++ b/.github/cursor-review/prompt-adversarial.md @@ -19,25 +19,22 @@ Do NOT flag: - Performance micro-optimizations unless they create a DoS vector - Issues in test files unless the test itself is masking a real bug -Review the following diff and report every finding. You MUST respond with ONLY a JSON -array — no prose, no markdown fences, no explanation outside the array. - -Each element must be an object with exactly these keys: -- "file": string — the file path relative to the repo root -- "line": integer — the line number in the NEW side of the diff where the issue exists -- "side": "RIGHT" — always RIGHT since findings are on the new code -- "severity": string — one of "critical", "high", "medium", "low", "nit" +Review the following diff and record every finding with the +`cursor_review_record_finding` tool. Call it once per distinct issue using: +- `file`: the file path relative to the repo root +- `line`: the line number in the NEW side of the diff where the issue exists +- `side`: `RIGHT` since findings are on the new code +- `severity`: one of `critical`, `high`, `medium`, `low`, `nit` ("critical" = exploitable hole / data loss / crash on a normal path; "high" = real bug on a plausible input; "medium" = bug on an edge path; "low" = minor security/reliability concern; "nit" = very low-impact security/reliability concern) -- "body": string — a concise description of the issue (1-3 sentences) +- `body`: a concise description of the issue (1-3 sentences) -If you find no issues, return an empty array: [] +After recording all findings, call `cursor_review_finish` exactly once. Call it +even if you found no issues. Do not put findings in your final response: only +tool calls are collected. -Example response: -[ - {"file": "internal/api/handler.go", "line": 42, "side": "RIGHT", "severity": "critical", "body": "User-supplied `filename` is passed to `os.Open` without path-traversal validation. An attacker can read arbitrary files with `../../etc/passwd`."}, - {"file": "internal/worker/upload.go", "line": 118, "side": "RIGHT", "severity": "high", "body": "The goroutine captures `ctx` from the outer scope but the parent function returns and cancels the context before the upload completes, causing silent data loss."} -] +Example `cursor_review_record_finding` arguments: +`{"file":"internal/api/handler.go","line":42,"side":"RIGHT","severity":"critical","body":"User-supplied filename is passed to os.Open without path-traversal validation."}` === BEGIN DIFF === diff --git a/.github/cursor-review/prompt-edge-case.md b/.github/cursor-review/prompt-edge-case.md index a506868..a2e9625 100644 --- a/.github/cursor-review/prompt-edge-case.md +++ b/.github/cursor-review/prompt-edge-case.md @@ -21,25 +21,22 @@ Do NOT flag: - Performance optimizations unless they cause correctness issues - Issues in test files unless the test itself is masking a real bug -Review the following diff and report every finding. You MUST respond with ONLY a JSON -array — no prose, no markdown fences, no explanation outside the array. - -Each element must be an object with exactly these keys: -- "file": string — the file path relative to the repo root -- "line": integer — the line number in the NEW side of the diff where the issue exists -- "side": "RIGHT" — always RIGHT since findings are on the new code -- "severity": string — one of "critical", "high", "medium", "low", "nit" +Review the following diff and record every finding with the +`cursor_review_record_finding` tool. Call it once per distinct issue using: +- `file`: the file path relative to the repo root +- `line`: the line number in the NEW side of the diff where the issue exists +- `side`: `RIGHT` since findings are on the new code +- `severity`: one of `critical`, `high`, `medium`, `low`, `nit` ("critical" = data loss / crash on a normal path; "high" = real bug on a plausible input; "medium" = bug on an edge path; "low" = minor correctness issue; "nit" = very low-impact correctness/clarity issue) -- "body": string — a concise description of the issue (1-3 sentences) +- `body`: a concise description of the issue (1-3 sentences) -If you find no issues, return an empty array: [] +After recording all findings, call `cursor_review_finish` exactly once. Call it +even if you found no issues. Do not put findings in your final response: only +tool calls are collected. -Example response: -[ - {"file": "pkg/retry/retrier.go", "line": 55, "side": "RIGHT", "severity": "high", "body": "When `maxRetries` is 0 the loop body never executes, so the function returns `nil` instead of running the operation once. The guard should be `i <= maxRetries`."}, - {"file": "internal/router/router.go", "line": 203, "side": "RIGHT", "severity": "medium", "body": "The `default` branch of the select sends on `errCh` without checking if the channel is full. If two goroutines hit this path simultaneously the second send blocks forever, leaking the goroutine."} -] +Example `cursor_review_record_finding` arguments: +`{"file":"pkg/retry/retrier.go","line":55,"side":"RIGHT","severity":"high","body":"When maxRetries is 0 the loop never executes, so the operation is not attempted."}` === BEGIN DIFF === diff --git a/.github/cursor-review/prompt-judge.md b/.github/cursor-review/prompt-judge.md index d7c9b15..1a78a01 100644 --- a/.github/cursor-review/prompt-judge.md +++ b/.github/cursor-review/prompt-judge.md @@ -1,13 +1,8 @@ -RESPOND WITH ONLY A JSON ARRAY. Your entire response must be a single JSON -array of finding objects (or `[]` if none) — no prose, no preamble, no -explanation, no markdown code fences before or after it. Do not narrate your -reasoning. The exact element schema is specified at the end of this prompt. - You have NO shell, filesystem, or web/search tools in this environment. Do not attempt to use them and do not narrate attempts to (e.g. "shell execution isn't available here", "let me confirm via documentation", "verification changes my adjudication"). Adjudicate solely from the panel findings and diff provided -below, and emit ONLY the JSON array — any prose preamble breaks the contract. +below, then submit the result through the final-review tool. You are a senior software engineer adjudicating findings from a panel of AI code reviewers. The panel ran a 4-lab × 2-review-type matrix (8 cells total): @@ -34,17 +29,17 @@ Selection guidance: - Cap the final list at 10 findings. Below 10 is fine if there genuinely aren't more. -Output: a JSON array, no prose, no markdown fences. Each element is an -object with exactly: -- "file": string — repo-relative path -- "line": integer — a line number that appears on the RIGHT (new) side of +Submit the result exactly once with the `cursor_review_submit_final` tool. Its +`findings` argument contains each kept finding using: +- `file`: repo-relative path +- `line`: a line number that appears on the RIGHT (new) side of one of the diff hunks below. Lines that aren't in any hunk cannot be anchored as inline comments — GitHub will reject them. If a finding's natural anchor isn't shown in the diff, RETARGET it to the nearest RIGHT-side line that IS in a hunk, or DROP the finding. -- "side": "RIGHT" — always -- "severity": string — exactly one of "critical", "high", "medium", "low", - "nit". Use this rubric: +- `side`: `RIGHT` — always +- `severity`: exactly one of `critical`, `high`, `medium`, `low`, `nit`. + Use this rubric: - "critical": exploitable security hole, data loss/corruption, or a crash on a normal path. Ship-blocker. - "high": a real bug that will misbehave on a plausible input, or a serious @@ -53,11 +48,13 @@ object with exactly: a blocker. - "low": minor correctness or robustness issue with limited impact. - "nit": style, naming, or polish — optional to address. -- "body": string — concise (1-3 sentences). Do NOT prefix the body with a +- `body`: concise (1-3 sentences). Do NOT prefix the body with a severity word or emoji; the severity field drives the rendered badge. END with attribution like `_Raised by 3 of 8 reviewers (gpt-5.3-codex-xhigh adversarial, claude-opus-4-8-thinking-xhigh edge-case, gemini-3.1-pro adversarial)._` -Order the array most-severe first. If no findings rise to the bar, return []. +Order findings most-severe first. If no findings rise to the bar, submit an +empty `findings` array. Do not put the result in your final response: only the +tool call is collected. === BEGIN PANEL FINDINGS === diff --git a/.github/cursor-review/review-output-mcp.py b/.github/cursor-review/review-output-mcp.py new file mode 100644 index 0000000..6184c1a --- /dev/null +++ b/.github/cursor-review/review-output-mcp.py @@ -0,0 +1,271 @@ +#!/usr/bin/env python3 +"""Minimal stdio MCP server for durable Cursor Review output.""" + +import argparse +import json +import os +import posixpath +import sys +import tempfile + +SEVERITIES = ("critical", "high", "medium", "low", "nit") +PROTOCOL_VERSION = "2025-06-18" +SUPPORTED_PROTOCOL_VERSIONS = { + "2024-11-05", + "2025-03-26", + "2025-06-18", + "2025-11-25", +} + +FINDING_SCHEMA = { + "type": "object", + "additionalProperties": False, + "required": ["file", "line", "side", "severity", "body"], + "properties": { + "file": {"type": "string", "minLength": 1, "maxLength": 1024}, + "line": {"type": "integer", "minimum": 1}, + "side": {"type": "string", "enum": ["RIGHT"]}, + "severity": {"type": "string", "enum": list(SEVERITIES)}, + "body": {"type": "string", "minLength": 1, "maxLength": 20000}, + }, +} + + +def write_record(path, record): + directory = os.path.dirname(os.path.abspath(path)) + os.makedirs(directory, exist_ok=True) + fd, temporary_path = tempfile.mkstemp(dir=directory, prefix=".review-output-") + try: + with os.fdopen(fd, "w", encoding="utf-8") as output: + json.dump(record, output) + os.replace(temporary_path, path) + except BaseException: + try: + os.unlink(temporary_path) + except FileNotFoundError: + pass + raise + + +def initial_record(args): + record = { + "status": "error", + "error": "Cursor agent did not submit structured review output.", + "findings": [], + } + if args.model: + record["model"] = args.model + if args.review_type: + record["review_type"] = args.review_type + return record + + +def read_record(args): + try: + with open(args.out, encoding="utf-8") as source: + return json.load(source) + except (OSError, json.JSONDecodeError): + return initial_record(args) + + +def validate_finding(value): + if not isinstance(value, dict): + raise ValueError("finding must be an object") + expected = {"file", "line", "side", "severity", "body"} + if set(value) != expected: + raise ValueError(f"finding keys must be exactly {sorted(expected)}") + + path = value["file"] + if not isinstance(path, str) or not path or len(path) > 1024: + raise ValueError("file must be a non-empty string of at most 1024 characters") + if path.startswith("/") or "\\" in path or "\x00" in path: + raise ValueError("file must be a repo-relative POSIX path") + normalized = posixpath.normpath(path) + if normalized in (".", "..") or normalized.startswith("../"): + raise ValueError("file must stay inside the repository") + + line = value["line"] + if isinstance(line, bool) or not isinstance(line, int) or line < 1: + raise ValueError("line must be a positive integer") + if value["side"] != "RIGHT": + raise ValueError("side must be RIGHT") + if value["severity"] not in SEVERITIES: + raise ValueError(f"severity must be one of {', '.join(SEVERITIES)}") + body = value["body"] + if not isinstance(body, str) or not body or len(body) > 20000: + raise ValueError("body must be a non-empty string of at most 20000 characters") + + return {**value, "file": normalized} + + +def tools_for(mode): + if mode == "reviewer": + return [ + { + "name": "cursor_review_record_finding", + "description": ( + "Record one code-review finding. Call once per distinct issue. " + "Only findings recorded with this tool count." + ), + "inputSchema": FINDING_SCHEMA, + }, + { + "name": "cursor_review_finish", + "description": ( + "Finish this review after recording every finding. Call exactly once, " + "including when there are no findings." + ), + "inputSchema": { + "type": "object", + "additionalProperties": False, + "properties": {}, + }, + }, + ] + return [ + { + "name": "cursor_review_submit_final", + "description": ( + "Submit the final adjudicated review. Call exactly once with every finding " + "that should be posted, or an empty findings array. This tool is the only " + "channel used for the final result." + ), + "inputSchema": { + "type": "object", + "additionalProperties": False, + "required": ["findings"], + "properties": { + "findings": { + "type": "array", + "maxItems": 10, + "items": FINDING_SCHEMA, + } + }, + }, + } + ] + + +def call_tool(args, name, arguments): + if not isinstance(arguments, dict): + raise ValueError("arguments must be an object") + + if args.mode == "reviewer" and name == "cursor_review_record_finding": + finding = validate_finding(arguments) + record = read_record(args) + if record.get("status") == "ok": + raise ValueError("review is already finished") + findings = record.get("findings") + if not isinstance(findings, list): + findings = [] + record.update( + status="error", + error="Cursor agent recorded findings but did not finish the review.", + findings=[*findings, finding], + ) + write_record(args.out, record) + return f"Recorded {finding['severity']} finding on {finding['file']}:{finding['line']}." + + if args.mode == "reviewer" and name == "cursor_review_finish": + if arguments: + raise ValueError("cursor_review_finish takes no arguments") + record = read_record(args) + if record.get("status") == "ok": + raise ValueError("review is already finished") + findings = record.get("findings") + record.update(status="ok", findings=findings if isinstance(findings, list) else []) + record.pop("error", None) + write_record(args.out, record) + return f"Finished review with {len(record['findings'])} finding(s)." + + if args.mode == "judge" and name == "cursor_review_submit_final": + if read_record(args).get("status") == "ok": + raise ValueError("final review is already submitted") + if set(arguments) != {"findings"} or not isinstance(arguments["findings"], list): + raise ValueError("findings must be an array and the only argument") + if len(arguments["findings"]) > 10: + raise ValueError("final review is capped at 10 findings") + findings = [validate_finding(finding) for finding in arguments["findings"]] + record = initial_record(args) + record.update(status="ok", findings=findings) + record.pop("error", None) + write_record(args.out, record) + return f"Submitted final review with {len(findings)} finding(s)." + + raise ValueError(f"unknown tool for {args.mode} mode: {name}") + + +def result(request_id, value): + return {"jsonrpc": "2.0", "id": request_id, "result": value} + + +def error(request_id, code, message): + return {"jsonrpc": "2.0", "id": request_id, "error": {"code": code, "message": message}} + + +def handle(args, message): + request_id = message.get("id") + method = message.get("method") + if method == "initialize": + params = message.get("params") or {} + requested = params.get("protocolVersion") if isinstance(params, dict) else None + return result(request_id, { + "protocolVersion": ( + requested if requested in SUPPORTED_PROTOCOL_VERSIONS else PROTOCOL_VERSION + ), + "capabilities": {"tools": {"listChanged": False}}, + "serverInfo": {"name": "cursor-review-output", "version": "1.0.0"}, + }) + if method == "ping": + return result(request_id, {}) + if method == "tools/list": + return result(request_id, {"tools": tools_for(args.mode)}) + if method == "tools/call": + params = message.get("params") or {} + try: + if not isinstance(params, dict): + raise ValueError("params must be an object") + text = call_tool(args, params.get("name"), params.get("arguments", {})) + return result(request_id, { + "content": [{"type": "text", "text": text}], + "isError": False, + }) + except (KeyError, TypeError, ValueError) as exc: + return result(request_id, { + "content": [{"type": "text", "text": str(exc)}], + "isError": True, + }) + if method and method.startswith("notifications/"): + return None + return error(request_id, -32601, f"unknown method: {method}") + + +def serve(args): + for line in sys.stdin: + try: + message = json.loads(line) + if not isinstance(message, dict): + raise ValueError("request must be an object") + response = handle(args, message) + except (json.JSONDecodeError, ValueError) as exc: + response = error(None, -32700, str(exc)) + if response is not None: + print(json.dumps(response, separators=(",", ":")), flush=True) + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--mode", choices=("reviewer", "judge"), required=True) + parser.add_argument("--out", required=True) + parser.add_argument("--model") + parser.add_argument("--review-type") + parser.add_argument("--init", action="store_true") + args = parser.parse_args() + if args.init: + write_record(args.out, initial_record(args)) + return + serve(args) + + +if __name__ == "__main__": + main() diff --git a/.github/cursor-review/tests/test_extract_findings.py b/.github/cursor-review/tests/test_extract_findings.py deleted file mode 100644 index 17e4a56..0000000 --- a/.github/cursor-review/tests/test_extract_findings.py +++ /dev/null @@ -1,197 +0,0 @@ -#!/usr/bin/env python3 -"""Regression tests for extract-findings.py JSON recovery. - -The judge/consolidate step intermittently returns valid findings JSON wrapped -in a sentence or two of prose. Discarding the whole run on that (the BE-1916 -parse_error class — hit on ComfyUI #487 and Alex's PR) is the bug these tests -guard against: prose-wrapped JSON must be recovered, not thrown away. - -Run: python3 .github/cursor-review/tests/test_extract_findings.py -""" - -import importlib.util -import json -import os -import tempfile -import unittest - -# extract-findings.py has a hyphen, so import it by path rather than `import`. -_MODULE_PATH = os.path.join(os.path.dirname(__file__), "..", "extract-findings.py") -_spec = importlib.util.spec_from_file_location("extract_findings", _MODULE_PATH) -ef = importlib.util.module_from_spec(_spec) -_spec.loader.exec_module(ef) - - -# A real-finding payload reused across cases. -FINDINGS = [ - { - "file": "internal/api/handler.go", - "line": 42, - "side": "RIGHT", - "severity": "high", - "body": "User-supplied filename reaches os.Open without traversal checks.", - }, - { - "file": "internal/worker/upload.go", - "line": 118, - "side": "RIGHT", - "severity": "medium", - "body": "Context is cancelled before the upload completes [see hunk], losing data.", - }, -] - - -def _findings(raw): - """Mirror main()'s parse → coerce pipeline.""" - return ef.coerce_findings_list(ef.parse_json_findings(raw)) - - -class ParseJsonFindingsTest(unittest.TestCase): - def test_bare_array(self): - self.assertEqual(_findings(json.dumps(FINDINGS)), FINDINGS) - - def test_empty_array(self): - self.assertEqual(_findings("[]"), []) - - def test_prose_wrapped_array_487(self): - # The #487 failure shape: a prose verdict, THEN the JSON array. The - # prose even contains a bracket pair, which is what broke the old naive - # first-`[`/last-`]` slice. - raw = ( - "Based on my review of the actual code, I can adjudicate the two " - "findings [both raised by multiple reviewers] as follows. Here is " - "the consolidated result:\n\n" + json.dumps(FINDINGS) + "\n\n" - "These are the only items that rise to the bar." - ) - self.assertEqual(_findings(raw), FINDINGS) - - def test_fenced_json_block(self): - raw = ( - "Sure — here are the findings:\n\n```json\n" - + json.dumps(FINDINGS) - + "\n```\nLet me know if you need more." - ) - self.assertEqual(_findings(raw), FINDINGS) - - def test_fenced_block_no_lang(self): - raw = "Result:\n\n```\n" + json.dumps(FINDINGS) + "\n```\n" - self.assertEqual(_findings(raw), FINDINGS) - - def test_object_wrapped_findings(self): - raw = "Here you go:\n" + json.dumps({"findings": FINDINGS}) - self.assertEqual(_findings(raw), FINDINGS) - - def test_brackets_inside_string_literals(self): - # Brackets inside a body string must not confuse the balanced scanner. - payload = [{"file": "a.py", "line": 1, "side": "RIGHT", "body": "uses arr[i] and m['k']"}] - raw = "Verdict below.\n" + json.dumps(payload) - self.assertEqual(_findings(raw), payload) - - def test_genuinely_malformed_returns_none(self): - self.assertIsNone(_findings("I reviewed the code and found nothing actionable.")) - - def test_truncated_json_returns_none(self): - # An unterminated array can't be recovered — must fail, not half-parse. - self.assertIsNone(_findings('[{"file": "a.py", "line": 1, "body": "oops"')) - - def test_scalar_is_not_findings(self): - # A bare number is valid JSON but never a findings payload. - self.assertIsNone(_findings("42")) - - def test_object_without_findings_key(self): - self.assertIsNone(_findings('{"summary": "looks good", "count": 0}')) - - def test_scalar_list_is_not_findings(self): - # A list of scalars is valid JSON but never a findings payload — it - # must not be mistaken for one (the PR-146 jq-builtins prose shape). - self.assertIsNone(_findings('["contains", "startswith", "ascii_downcase"]')) - - def test_verification_prose_with_inline_object_then_array_be3160(self): - # PR-145 shape: verification prose that quotes a single finding OBJECT - # inline, THEN the real array. The old first-parseable-region logic - # returned the un-coercible inline object → spurious parse_error. - raw = ( - "Verification changes my adjudication significantly. Three panel " - 'findings turn out to be misreads. For instance {"file": "b.go", ' - '"line": 3, "severity": "low"} does not hold on inspection.\n\n' - "Final consolidated findings:\n" + json.dumps(FINDINGS) - ) - self.assertEqual(_findings(raw), FINDINGS) - - def test_tool_narration_prose_with_scalar_list_then_array_be3160(self): - # PR-146 shape: tool-attempt narration containing an inline scalar list, - # THEN the real array. The old logic returned the scalar list as bogus - # findings; extraction must recover the genuine array instead. - raw = ( - "Shell execution isn't available here. Let me confirm the jq builtin " - 'set via documentation. The relevant keys are ["contains", ' - '"startswith", "ascii_downcase"].\n\n' + json.dumps(FINDINGS) - ) - self.assertEqual(_findings(raw), FINDINGS) - - def test_last_findings_array_wins(self): - # When multiple findings-shaped arrays appear, the LAST (the real answer - # after prose) wins over an earlier draft the judge second-guessed. - earlier = [{"file": "old.go", "line": 1, "side": "RIGHT", "body": "superseded"}] - raw = ( - "My first pass produced:\n" + json.dumps(earlier) + "\n\nOn " - "reflection that was wrong. The final answer is:\n" + json.dumps(FINDINGS) - ) - self.assertEqual(_findings(raw), FINDINGS) - - -class MainEndToEndTest(unittest.TestCase): - """Drive main() the way the workflow does, asserting the status field.""" - - def _run(self, raw_text): - with tempfile.TemporaryDirectory() as d: - raw_path = os.path.join(d, "raw.txt") - out_path = os.path.join(d, "out.json") - with open(raw_path, "w", encoding="utf-8") as f: - f.write(raw_text) - import sys - - argv = sys.argv - sys.argv = [ - "extract-findings.py", - "--raw", raw_path, - "--out", out_path, - "--model", "judge-model", - "--review-type", "judge", - ] - try: - ef.main() - finally: - sys.argv = argv - with open(out_path, encoding="utf-8") as f: - return json.load(f) - - def test_prose_wrapped_parses_ok(self): - raw = "After review, my verdict is:\n\n" + json.dumps(FINDINGS) - record = self._run(raw) - self.assertEqual(record["status"], "ok") - self.assertEqual(record["findings"], FINDINGS) - - def test_verification_prose_then_array_is_ok_be3160(self): - # Acceptance #1: a judge output containing valid JSON after prose must - # consolidate (status ok), not fail as parse_error. - raw = ( - "Verification changes my adjudication significantly. Three panel " - "findings turn out to be misreads.\n\n" + json.dumps(FINDINGS) - ) - record = self._run(raw) - self.assertEqual(record["status"], "ok") - self.assertEqual(record["findings"], FINDINGS) - - def test_malformed_is_parse_error(self): - record = self._run("I could not find anything worth flagging.") - self.assertEqual(record["status"], "parse_error") - self.assertEqual(record["findings"], []) - - def test_empty_is_empty(self): - record = self._run(" \n ") - self.assertEqual(record["status"], "empty") - - -if __name__ == "__main__": - unittest.main() diff --git a/.github/cursor-review/tests/test_review_output_mcp.py b/.github/cursor-review/tests/test_review_output_mcp.py new file mode 100644 index 0000000..f0774ff --- /dev/null +++ b/.github/cursor-review/tests/test_review_output_mcp.py @@ -0,0 +1,156 @@ +#!/usr/bin/env python3 +"""Contract tests for the Cursor Review stdio MCP output tools.""" + +import importlib.util +import json +import os +import subprocess +import sys +import tempfile +import unittest + +MODULE_PATH = os.path.join(os.path.dirname(__file__), "..", "review-output-mcp.py") +SPEC = importlib.util.spec_from_file_location("review_output_mcp", MODULE_PATH) +MCP = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(MCP) + +FINDING = { + "file": "internal/api/handler.go", + "line": 42, + "side": "RIGHT", + "severity": "high", + "body": "User-supplied filename reaches os.Open without traversal checks.", +} + + +class ValidationTest(unittest.TestCase): + def test_finding_is_normalized(self): + finding = {**FINDING, "file": "internal/api/../api/handler.go"} + self.assertEqual(MCP.validate_finding(finding)["file"], FINDING["file"]) + + def test_rejects_path_traversal(self): + with self.assertRaisesRegex(ValueError, "inside the repository"): + MCP.validate_finding({**FINDING, "file": "../secret"}) + + def test_rejects_unknown_fields(self): + with self.assertRaisesRegex(ValueError, "keys must be exactly"): + MCP.validate_finding({**FINDING, "confidence": 0.9}) + + +class StdioServerTest(unittest.TestCase): + def run_server(self, mode, messages, include_initial=False): + with tempfile.TemporaryDirectory() as directory: + output_path = os.path.join(directory, "findings.json") + subprocess.run( + [sys.executable, MODULE_PATH, "--mode", mode, "--out", output_path, + "--model", "test-model", "--review-type", mode, "--init"], + check=True, + ) + with open(output_path, encoding="utf-8") as source: + initial_record = json.load(source) + process = subprocess.run( + [sys.executable, MODULE_PATH, "--mode", mode, "--out", output_path, + "--model", "test-model", "--review-type", mode], + input="".join(json.dumps(message) + "\n" for message in messages), + text=True, + capture_output=True, + check=True, + ) + with open(output_path, encoding="utf-8") as source: + record = json.load(source) + responses = [json.loads(line) for line in process.stdout.splitlines()] + if include_initial: + return record, responses, initial_record + return record, responses + + def test_reviewer_records_findings_and_finishes(self): + record, responses = self.run_server("reviewer", [ + {"jsonrpc": "2.0", "id": 1, "method": "initialize", "params": {}}, + {"jsonrpc": "2.0", "id": 2, "method": "tools/call", "params": { + "name": "cursor_review_record_finding", "arguments": FINDING, + }}, + {"jsonrpc": "2.0", "id": 3, "method": "tools/call", "params": { + "name": "cursor_review_finish", "arguments": {}, + }}, + ]) + self.assertEqual(record["status"], "ok") + self.assertEqual(record["findings"], [FINDING]) + self.assertEqual([response["id"] for response in responses], [1, 2, 3]) + + def test_reviewer_must_finish_empty_review(self): + record, _ = self.run_server("reviewer", []) + self.assertEqual(record["status"], "error") + self.assertEqual(record["findings"], []) + + def test_recorded_finding_is_incomplete_until_finish(self): + record, _ = self.run_server("reviewer", [ + {"jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": { + "name": "cursor_review_record_finding", "arguments": FINDING, + }}, + ]) + self.assertEqual(record["status"], "error") + self.assertEqual(record["findings"], [FINDING]) + + def test_cannot_overwrite_finished_review(self): + record, responses = self.run_server("reviewer", [ + {"jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": { + "name": "cursor_review_finish", "arguments": {}, + }}, + {"jsonrpc": "2.0", "id": 2, "method": "tools/call", "params": { + "name": "cursor_review_record_finding", "arguments": FINDING, + }}, + ]) + self.assertEqual(record["findings"], []) + self.assertTrue(responses[1]["result"]["isError"]) + + def test_judge_submits_empty_final_review(self): + record, _ = self.run_server("judge", [ + {"jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": { + "name": "cursor_review_submit_final", "arguments": {"findings": []}, + }}, + ]) + self.assertEqual(record["status"], "ok") + self.assertEqual(record["findings"], []) + + def test_invalid_tool_input_does_not_replace_output(self): + record, responses, initial_record = self.run_server("judge", [ + {"jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": { + "name": "cursor_review_submit_final", + "arguments": {"findings": [FINDING, {**FINDING, "line": 0}]}, + }}, + ], include_initial=True) + self.assertEqual(record, initial_record) + self.assertTrue(responses[0]["result"]["isError"]) + + def test_rejects_falsy_non_object_tool_arguments(self): + record, responses = self.run_server("reviewer", [ + {"jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": { + "name": "cursor_review_finish", "arguments": [], + }}, + ]) + self.assertEqual(record["status"], "error") + self.assertTrue(responses[0]["result"]["isError"]) + + def test_negotiates_only_supported_protocol(self): + _, responses = self.run_server("reviewer", [ + {"jsonrpc": "2.0", "id": 1, "method": "initialize", "params": { + "protocolVersion": "2099-01-01", + }}, + ]) + self.assertEqual( + responses[0]["result"]["protocolVersion"], MCP.PROTOCOL_VERSION + ) + + def test_negotiates_current_protocol(self): + _, responses = self.run_server("reviewer", [ + {"jsonrpc": "2.0", "id": 1, "method": "initialize", "params": { + "protocolVersion": "2025-11-25", + }}, + ]) + self.assertEqual( + responses[0]["result"]["protocolVersion"], "2025-11-25" + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/workflows/cursor-review.yml b/.github/workflows/cursor-review.yml index 1c182c4..db3243b 100644 --- a/.github/workflows/cursor-review.yml +++ b/.github/workflows/cursor-review.yml @@ -5,7 +5,7 @@ name: Cursor Review (reusable) # judge model consolidates the findings into ONE review posted on the PR, # with per-finding severity badges (critical/high/medium/low/nit). # -# Single source of truth: the prompts and the extract/post/notify scripts +# Single source of truth: the prompts and the output/post/notify scripts # live in THIS repo under .github/cursor-review/. Consumer repos add only a # thin caller — they don't carry copies of the review logic, so there's no # drift to keep in sync. @@ -290,8 +290,8 @@ jobs: - name: Seed default findings artifact # Pre-seed a minimal artifact tagged status=error so this cell # ALWAYS shows up in the panel summary, even if a later step - # (checkout, Cursor install, prompt build, extract) fails. The - # `Extract findings` step overwrites this on success. Without + # (checkout, Cursor install, prompt build, tool submission) fails. The + # MCP output tool overwrites this on success. Without # this, an early-failing cell silently disappears from the panel # and the consolidated review undercounts the matrix. env: @@ -354,6 +354,57 @@ jobs: # would just ship malicious bits as a "new version" we'd then bump to. run: cursor-agent --version + - name: Configure structured review output + env: + MODEL: ${{ matrix.model }} + REVIEW_TYPE: ${{ matrix.review_type }} + run: | + python3 "$CURSOR_REVIEW_ASSETS/review-output-mcp.py" \ + --mode reviewer \ + --out /tmp/findings-out/findings.json \ + --model "$MODEL" \ + --review-type "$REVIEW_TYPE" \ + --init + + rm -rf -- .cursor + mkdir -m 700 .cursor + python3 - <<'PY' + import json, os, sys, tempfile + + if os.path.islink(".cursor"): + raise SystemExit("Refusing to write Cursor config through a symlinked .cursor directory") + + config = { + "mcpServers": { + "cursor-review-output": { + "type": "stdio", + "command": sys.executable, + "args": [ + os.path.join(os.environ["CURSOR_REVIEW_ASSETS"], "review-output-mcp.py"), + "--mode", "reviewer", + "--out", "/tmp/findings-out/findings.json", + "--model", os.environ["MODEL"], + "--review-type", os.environ["REVIEW_TYPE"], + ], + } + } + } + cli_config = { + "permissions": { + "allow": [ + "Mcp(cursor-review-output:cursor_review_record_finding)", + "Mcp(cursor-review-output:cursor_review_finish)", + ], + "deny": [], + } + } + for filename, value in (("mcp.json", config), ("cli.json", cli_config)): + fd, temporary = tempfile.mkstemp(dir=".cursor", prefix=f".{filename}-") + with os.fdopen(fd, "w", encoding="utf-8") as output: + json.dump(value, output) + os.replace(temporary, os.path.join(".cursor", filename)) + PY + - name: Build prompt env: REVIEW_TYPE: ${{ matrix.review_type }} @@ -379,6 +430,7 @@ jobs: cursor-agent \ --print \ --trust \ + --approve-mcps \ --model "$MODEL" \ < /tmp/prompt.txt \ > /tmp/review-raw.txt 2>/tmp/review-stderr.txt || true @@ -389,20 +441,10 @@ jobs: echo "=== Stderr ===" cat /tmp/review-stderr.txt - - name: Extract findings + - name: Show structured findings if: always() - env: - MODEL: ${{ matrix.model }} - REVIEW_TYPE: ${{ matrix.review_type }} run: | - mkdir -p /tmp/findings-out - python3 "$CURSOR_REVIEW_ASSETS/extract-findings.py" \ - --raw /tmp/review-raw.txt \ - --out /tmp/findings-out/findings.json \ - --model "$MODEL" \ - --review-type "$REVIEW_TYPE" - - echo "=== Extracted findings ===" + echo "=== Structured findings ===" cat /tmp/findings-out/findings.json echo "" @@ -461,8 +503,59 @@ jobs: path: /tmp/panel pattern: findings-* + - name: Configure structured final review + run: | + python3 "$CURSOR_REVIEW_ASSETS/review-output-mcp.py" \ + --mode judge \ + --out /tmp/judge-findings.json \ + --model "$JUDGE_MODEL" \ + --review-type judge \ + --init + + rm -rf -- .cursor + mkdir -m 700 .cursor + python3 - <<'PY' + import json, os, sys, tempfile + + if os.path.islink(".cursor"): + raise SystemExit("Refusing to write Cursor config through a symlinked .cursor directory") + + config = { + "mcpServers": { + "cursor-review-output": { + "type": "stdio", + "command": sys.executable, + "args": [ + os.path.join(os.environ["CURSOR_REVIEW_ASSETS"], "review-output-mcp.py"), + "--mode", "judge", + "--out", "/tmp/judge-findings.json", + "--model", os.environ["JUDGE_MODEL"], + "--review-type", "judge", + ], + } + } + } + cli_config = { + "permissions": { + "allow": [ + "Mcp(cursor-review-output:cursor_review_submit_final)", + ], + "deny": [], + } + } + for filename, value in (("mcp.json", config), ("cli.json", cli_config)): + fd, temporary = tempfile.mkstemp(dir=".cursor", prefix=f".{filename}-") + with os.fdopen(fd, "w", encoding="utf-8") as output: + json.dump(value, output) + os.replace(temporary, os.path.join(".cursor", filename)) + PY + - name: Aggregate panel findings id: aggregate + env: + PANEL_MODELS: >- + ["gpt-5.3-codex-xhigh", "claude-opus-4-8-thinking-xhigh", + "gemini-3.1-pro", "kimi-k2.5"] run: | # Each artifact directory contains one findings.json. Combine into # a single panel.json array preserving cell metadata. @@ -474,6 +567,24 @@ jobs: with open(path, encoding="utf-8") as f: panel.append(json.load(f)) + expected = { + (model, review_type) + for model in json.loads(os.environ["PANEL_MODELS"]) + for review_type in ("adversarial", "edge-case") + } + present = { + (cell.get("model"), cell.get("review_type")) + for cell in panel + } + for model, review_type in sorted(expected - present): + panel.append({ + "model": model, + "review_type": review_type, + "status": "error", + "error": "Cell findings artifact was not uploaded.", + "findings": [], + }) + with open("/tmp/panel.json", "w", encoding="utf-8") as f: json.dump(panel, f, indent=2) @@ -508,16 +619,13 @@ jobs: env: CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }} run: | - run_judge() { - cursor-agent \ - --print \ - --trust \ - --model "$JUDGE_MODEL" \ - < "$1" \ - > /tmp/judge-raw.txt 2>/tmp/judge-stderr.txt || true - } - - run_judge /tmp/judge-prompt.txt + cursor-agent \ + --print \ + --trust \ + --approve-mcps \ + --model "$JUDGE_MODEL" \ + < /tmp/judge-prompt.txt \ + > /tmp/judge-raw.txt 2>/tmp/judge-stderr.txt || true echo "=== Judge raw output (first 1000 chars) ===" head -c 1000 /tmp/judge-raw.txt @@ -525,68 +633,34 @@ jobs: echo "=== Judge stderr ===" cat /tmp/judge-stderr.txt - # LLMs intermittently wrap the findings array in prose. extract-findings.py - # recovers most of that (fenced block / embedded brace-match), but if it - # still can't parse, retry the judge ONCE with a terse strict-JSON - # reminder appended — a single retry clears the large majority of these - # transient parse failures. The retry overwrites /tmp/judge-raw.txt, so - # the parse step below reads whichever attempt the judge last produced. - python3 "$CURSOR_REVIEW_ASSETS/extract-findings.py" \ - --raw /tmp/judge-raw.txt \ - --out /tmp/judge-probe.json \ - --model "$JUDGE_MODEL" \ - --review-type judge - PROBE_STATUS=$(python3 -c "import json; print(json.load(open('/tmp/judge-probe.json')).get('status','?'))") - - if [ "$PROBE_STATUS" != "ok" ]; then - echo "Judge output did not parse (status=$PROBE_STATUS) — retrying once with a strict-JSON reminder." - { - cat /tmp/judge-prompt.txt - echo "" - echo "=== REMINDER ===" - echo "Your previous response could not be parsed as JSON. Return ONLY the JSON array of findings — no prose, no explanation, no markdown code fences. Your entire response must be a single JSON array (return [] if there are no findings). You have no shell, filesystem, or web tools here, so do not narrate attempts to use them or your verification reasoning; emit only the array." - } > /tmp/judge-prompt-retry.txt - run_judge /tmp/judge-prompt-retry.txt - echo "=== Judge retry raw output (first 1000 chars) ===" - head -c 1000 /tmp/judge-raw.txt - echo "" - echo "=== Judge retry stderr ===" - cat /tmp/judge-stderr.txt - fi + echo "=== Structured final review ===" + cat /tmp/judge-findings.json + echo "" - name: Build consolidated findings file id: consolidated env: OK_COUNT: ${{ steps.aggregate.outputs.ok_count }} run: | - # If at least one cell produced findings, parse the judge output - # into a findings file. Otherwise short-circuit — post-review.py - # will render a "no contributors" review with the panel summary. + # If no cells completed, short-circuit — post-review.py will render + # a "no contributors" review with the panel summary. Otherwise the + # judge's MCP tool has already written /tmp/judge-findings.json. if [ "$OK_COUNT" = "0" ]; then - echo "No panel cells contributed findings — skipping judge parse." + echo "No panel cells contributed findings — skipping judge." echo '{"status": "ok", "findings": []}' > /tmp/judge-findings.json - else - python3 "$CURSOR_REVIEW_ASSETS/extract-findings.py" \ - --raw /tmp/judge-raw.txt \ - --out /tmp/judge-findings.json \ - --model "$JUDGE_MODEL" \ - --review-type judge fi - # If the judge errored or its output couldn't be parsed, surface - # that to the next step so we post an honest error review rather - # than silently claiming "no findings" when the panel did produce - # content. + # If the judge did not call its final-review tool, surface that to + # the next step rather than silently claiming "no findings". JUDGE_STATUS=$(python3 -c "import json,sys; print(json.load(open('/tmp/judge-findings.json')).get('status','?'))") echo "Judge status: $JUDGE_STATUS" echo "judge_status=$JUDGE_STATUS" >> "$GITHUB_OUTPUT" # Combine judge findings with panel metadata (model, review_type, # status only — drop full per-cell findings to keep the post payload - # small) for the post-review script. If the judge FAILED (still no - # parseable verdict after the retry), fall back to the raw per-cell - # findings so the run surfaces what the panel found instead of a bare - # "Review failed". + # small) for the post-review script. If the judge did not submit, + # fall back to the raw per-cell findings so the run still surfaces + # what the panel found. python3 - <<'PY' import json, os @@ -697,7 +771,7 @@ jobs: --repo "$REPO" \ --commit-sha "$HEAD_SHA" \ --triggered-by "$TRIGGERED_BY" \ - --notice "⚠️ The judge step could not produce a parseable verdict after a retry (status=${JUDGE_STATUS}). The findings below are the raw, un-adjudicated panel output — they may contain duplicates or false positives the judge would normally filter." + --notice "⚠️ The judge step did not submit a final review through its structured-output tool (status=${JUDGE_STATUS}). The findings below are the raw, un-adjudicated panel output — they may contain duplicates or false positives the judge would normally filter." else JUDGE_ERROR=$(python3 -c "import json; print(json.load(open('/tmp/judge-findings.json')).get('error','(no error message)'))") python3 "$CURSOR_REVIEW_ASSETS/post-review.py" \ diff --git a/.github/workflows/test-cursor-review-scripts.yml b/.github/workflows/test-cursor-review-scripts.yml index fde5e6e..3714f00 100644 --- a/.github/workflows/test-cursor-review-scripts.yml +++ b/.github/workflows/test-cursor-review-scripts.yml @@ -1,9 +1,9 @@ name: Test cursor-review scripts -# Runs the Python unit tests for the cursor-review helper scripts (the -# extract-findings.py JSON parser in particular). These scripts drive the -# review panel + judge consolidation, so a parser regression silently breaks -# every consumer repo's review — cheap to guard with a unit run on change. +# Runs the Python unit tests for the cursor-review helper scripts, especially +# the stdio MCP output contract used by reviewers and the judge. These scripts +# drive panel consolidation for every consumer repo, so they are cheap to +# guard with a unit run on change. on: pull_request: