diff --git a/.github/cursor-review/__pycache__/extract-findings.cpython-314.pyc b/.github/cursor-review/__pycache__/extract-findings.cpython-314.pyc new file mode 100644 index 0000000..d108bd4 Binary files /dev/null and b/.github/cursor-review/__pycache__/extract-findings.cpython-314.pyc differ diff --git a/.github/cursor-review/__pycache__/post-review.cpython-314.pyc b/.github/cursor-review/__pycache__/post-review.cpython-314.pyc new file mode 100644 index 0000000..b89654f Binary files /dev/null and b/.github/cursor-review/__pycache__/post-review.cpython-314.pyc differ diff --git a/.github/cursor-review/extract-findings.py b/.github/cursor-review/extract-findings.py new file mode 100644 index 0000000..8c774eb --- /dev/null +++ b/.github/cursor-review/extract-findings.py @@ -0,0 +1,101 @@ +#!/usr/bin/env python3 +"""Parse raw cursor-agent output into a normalized findings record. + +Used by per-cell matrix steps. Each cell calls this to convert the model's +raw stdout into a JSON file the consolidate step can ingest. The output is +always structured — even on parse failures or empty output — so the +consolidate 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 parse_json_findings(raw_text: str): + """Extract a JSON array from raw model output, tolerating surrounding prose. + + Returns the parsed value, or None if no JSON array could be located. + """ + text = raw_text.strip() + try: + return json.loads(text) + except json.JSONDecodeError: + pass + + fenced = re.search(r"```(?:json)?\s*\n(.*?)```", text, re.DOTALL) + if fenced: + try: + return json.loads(fenced.group(1).strip()) + except json.JSONDecodeError: + pass + + start = text.find("[") + end = text.rfind("]") + if start != -1 and end != -1 and end > start: + try: + return json.loads(text[start : end + 1]) + except json.JSONDecodeError: + pass + + 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 = 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=[], + ) + elif not isinstance(findings, list): + record.update( + status="parse_error", + error=f"Output parsed but is not an array (got {type(findings).__name__}).", + 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 new file mode 100644 index 0000000..48c6510 --- /dev/null +++ b/.github/cursor-review/post-review.py @@ -0,0 +1,297 @@ +#!/usr/bin/env python3 +"""Post a single consolidated cursor-review to a GitHub PR. + +The consolidate step produces one findings file (output of the judge call, +augmented with panel metadata). This script reads that file and posts ONE +PR review with line-anchored inline comments. + +Findings file shape: + { + "findings": [ + {"file": str, "line": int, "side": "RIGHT", "severity": str, "body": str}, + ... + ], + "panel": [ + {"model": str, "review_type": str, "status": "ok"|"empty"|"error"|"parse_error"}, + ... + ] + } + +Falls back to a body-only review (no inline anchors) if GitHub rejects the +inline payload — typical cause is line numbers that don't match the diff. +""" + +import argparse +import json +import subprocess +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 +# 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. +SEVERITY_ORDER = ["critical", "high", "medium", "low", "nit"] +SEVERITY_EMOJI = { + "critical": "🔴", + "high": "🟠", + "medium": "🟡", + "low": "🟢", + "nit": "⚪", +} +SEVERITY_LABEL = { + "critical": "Critical", + "high": "High", + "medium": "Medium", + "low": "Low", + "nit": "Nit", +} +DEFAULT_SEVERITY = "medium" + + +def normalize_severity(value) -> str: + """Coerce a model-supplied severity into one of SEVERITY_ORDER. + + Tolerant by design: unknown, missing, or non-string values become + DEFAULT_SEVERITY rather than dropping the finding. + """ + if not isinstance(value, str): + return DEFAULT_SEVERITY + candidate = value.strip().lower() + return candidate if candidate in SEVERITY_EMOJI else DEFAULT_SEVERITY + + +def severity_rank(severity: str) -> int: + try: + return SEVERITY_ORDER.index(severity) + except ValueError: + return len(SEVERITY_ORDER) + + +def build_severity_summary(enriched: list[dict]) -> str: + """Render a CodeRabbit-style severity breakdown table, highest first. + + Only severities that actually occur get a row, so a PR with three nits + doesn't carry four empty rows of ceremony. + """ + counts: dict[str, int] = {} + for item in enriched: + counts[item["severity"]] = counts.get(item["severity"], 0) + 1 + rows = [ + f"| {SEVERITY_EMOJI[sev]} {SEVERITY_LABEL[sev]} | {counts[sev]} |" + for sev in SEVERITY_ORDER + if counts.get(sev) + ] + if not rows: + return "" + return "| Severity | Count |\n| --- | --- |\n" + "\n".join(rows) + + +def neutralize_mentions(text: str) -> str: + """Insert ZWSP after each `@` so model output can't trigger GitHub mentions.""" + return str(text).replace("@", "@\u200B") + + +def gh_post_review(repo: str, pr_number: str, payload: str) -> subprocess.CompletedProcess: + return subprocess.run( + [ + "gh", + "api", + "--method", + "POST", + f"/repos/{repo}/pulls/{pr_number}/reviews", + "--input", + "-", + ], + input=payload, + text=True, + capture_output=True, + ) + + +def build_panel_summary(panel: list[dict]) -> str: + if not panel: + return "" + ok = sum(1 for c in panel if c.get("status") == "ok") + failed = [c for c in panel if c.get("status") != "ok"] + parts = [f"_Panel: {ok}/{len(panel)} reviewers contributed findings._"] + if failed: + names = ", ".join( + f"{c.get('model','?')}:{c.get('review_type','?')} ({c.get('status','?')})" + for c in failed + ) + parts.append(f"_Reviewers that did not contribute: {names}_") + return "\n\n".join(parts) + + +def normalize_comments(findings: list[dict]) -> list[dict]: + """Build sorted, severity-tagged inline comments from raw judge findings. + + Returns a list of {"severity": str, "comment": dict} entries sorted most + → least urgent. The nested `comment` is the GitHub review-comment payload + (path/line/side/body) with the severity badge prefixed into the body; + severity is kept alongside (not inside) so the summary table can count it + without leaking an unknown key into the GitHub API request. + """ + enriched = [] + for finding in findings: + if not isinstance(finding, dict): + print(f"Skipping non-dict finding: {finding!r}", file=sys.stderr) + continue + path = finding.get("file", "") + line = finding.get("line") + body = finding.get("body", "") + if not path or not line or not body: + continue + try: + line_int = int(line) + except (TypeError, ValueError): + print(f"Skipping non-integer line {line!r} for {path}", file=sys.stderr) + continue + if line_int <= 0: + print(f"Skipping non-positive line {line_int} for {path}", file=sys.stderr) + continue + severity = normalize_severity(finding.get("severity")) + badge = f"{SEVERITY_EMOJI[severity]} **{SEVERITY_LABEL[severity]}** — " + enriched.append( + { + "severity": severity, + "comment": { + "path": path, + "line": line_int, + "side": "RIGHT", + "body": badge + neutralize_mentions(body), + }, + } + ) + enriched.sort(key=lambda item: severity_rank(item["severity"])) + return enriched + + +def post_error_review(repo, pr_number, commit_sha, header, error_message): + safe = neutralize_mentions(error_message) + payload = json.dumps( + { + "body": ( + f"{header}\n\n⚠️ **Review failed**\n\n```\n{safe}\n```\n\n" + "Re-trigger by removing and re-adding the `cursor-review` label." + ), + "event": "COMMENT", + "commit_id": commit_sha, + } + ) + result = gh_post_review(repo, pr_number, payload) + if result.returncode != 0: + print(f"Error-review POST failed: {result.stderr}", file=sys.stderr) + raise SystemExit(1) + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--findings", required=True, help="Path to consolidated findings JSON") + parser.add_argument("--pr-number", required=True) + parser.add_argument("--repo", required=True) + parser.add_argument("--commit-sha", required=True) + parser.add_argument("--triggered-by", default=None) + parser.add_argument("--error-message", default=None, help="If set, post an error review with this message") + args = parser.parse_args() + + attribution = f"\n\n_Triggered by @{args.triggered_by}._" if args.triggered_by else "" + header = f"## 🔍 Cursor Review — Consolidated panel{attribution}" + + if args.error_message: + post_error_review(args.repo, args.pr_number, args.commit_sha, header, args.error_message) + return + + try: + with open(args.findings, encoding="utf-8") as f: + data = json.load(f) + except (OSError, json.JSONDecodeError) as e: + post_error_review( + args.repo, + args.pr_number, + args.commit_sha, + header, + f"Could not load findings file: {e}", + ) + return + + findings = data.get("findings", []) or [] + panel = data.get("panel", []) or [] + panel_summary = build_panel_summary(panel) + + if not findings: + # Distinguish two cases that both produce zero findings: + # 1. Panel ran, judge picked nothing → genuinely no high-signal issues. + # 2. Every panel cell errored → judge was skipped, no judging happened. + # Headlining (1) and (2) the same way ("No high-signal findings") is + # misleading on (2), so check the panel metadata explicitly. + all_failed = bool(panel) and all(c.get("status") != "ok" for c in panel) + if all_failed: + body_text = ( + f"{header}\n\n⚠️ **Panel did not produce any findings.**\n\n" + "Every reviewer in the matrix failed to contribute — see the " + "panel summary for which cells errored, and the run logs for " + "the underlying cause." + ) + else: + body_text = f"{header}\n\n✅ No high-signal findings." + if panel_summary: + body_text += f"\n\n{panel_summary}" + payload = json.dumps( + {"body": body_text, "event": "COMMENT", "commit_id": args.commit_sha} + ) + result = gh_post_review(args.repo, args.pr_number, payload) + if result.returncode != 0: + print(f"No-findings review POST failed: {result.stderr}", file=sys.stderr) + raise SystemExit(1) + return + + enriched = normalize_comments(findings) + comments = [item["comment"] for item in enriched] + + review_body = f"{header}\n\nFound **{len(comments)}** finding(s)." + severity_summary = build_severity_summary(enriched) + if severity_summary: + review_body += f"\n\n{severity_summary}" + if panel_summary: + review_body += f"\n\n{panel_summary}" + if not comments and findings: + review_body += "\n\n_(All findings had invalid file/line references and were dropped.)_" + + payload = json.dumps( + { + "body": review_body, + "event": "COMMENT", + "commit_id": args.commit_sha, + "comments": comments, + } + ) + + result = gh_post_review(args.repo, args.pr_number, payload) + + if result.returncode != 0: + print(f"Review POST failed: {result.stderr}", file=sys.stderr) + # Fallback: same body without inline anchors. Typical cause is line + # numbers that fall outside the diff context — often the model picked + # a line near the change but not on the change. + fallback_body = review_body + "\n\n---\n\n" + for c in comments: + fallback_body += f"**`{c['path']}:{c['line']}`** — {c['body']}\n\n" + fallback_body += "\n_(Inline comments could not be anchored to the diff; listed above instead.)_" + + fallback_payload = json.dumps( + { + "body": fallback_body, + "event": "COMMENT", + "commit_id": args.commit_sha, + } + ) + fallback_result = gh_post_review(args.repo, args.pr_number, fallback_payload) + if fallback_result.returncode != 0: + print(f"Fallback review POST also failed: {fallback_result.stderr}", file=sys.stderr) + raise SystemExit(1) + + +if __name__ == "__main__": + main() diff --git a/.github/cursor-review/prompt-adversarial.md b/.github/cursor-review/prompt-adversarial.md new file mode 100644 index 0000000..b815806 --- /dev/null +++ b/.github/cursor-review/prompt-adversarial.md @@ -0,0 +1,43 @@ +You are a senior security and reliability engineer performing an adversarial code review. +Your goal is to find bugs, security vulnerabilities, race conditions, data leaks, +injection vectors, denial-of-service risks, and any other defects that a malicious or +careless actor could exploit or trigger. + +Focus on: +- Input validation gaps (path traversal, injection, overflow) +- Authentication / authorization bypasses +- Race conditions and TOCTOU issues +- Resource exhaustion (unbounded allocations, missing timeouts) +- Error handling that leaks internal state +- Unsafe concurrency patterns (missing locks, deadlocks) +- Secrets or credentials exposed in logs or responses +- Incorrect or missing access control checks + +Do NOT flag: +- Style preferences or naming conventions +- Missing documentation or comments +- 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" + ("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) + +If you find no issues, return an empty array: [] + +Example response: +[ + {"file": "services/ingest/handler/prompt.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": "services/inference/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."} +] + +=== BEGIN DIFF === diff --git a/.github/cursor-review/prompt-edge-case.md b/.github/cursor-review/prompt-edge-case.md new file mode 100644 index 0000000..7cc2e52 --- /dev/null +++ b/.github/cursor-review/prompt-edge-case.md @@ -0,0 +1,45 @@ +You are a senior software engineer performing an edge-case review. +Your goal is to find logic errors, off-by-one mistakes, unhandled edge cases, +incorrect assumptions, missing nil/null checks, broken error propagation, and +subtle behavioral bugs that only surface under unusual but valid inputs. + +Focus on: +- Nil/null pointer dereferences and missing nil checks +- Off-by-one errors in loops, slices, and pagination +- Unhandled error returns (especially in Go where errors are values) +- Incorrect type assertions or unsafe casts +- Boundary conditions (empty collections, zero values, max int, unicode) +- Broken error wrapping that loses context or sentinel identity +- Goroutine leaks or missing cleanup in defer chains +- Incorrect assumptions about map ordering, slice capacity, or channel behavior +- Silent data loss from ignored return values +- State corruption from partial failure in multi-step operations + +Do NOT flag: +- Style preferences or naming conventions +- Missing documentation or comments +- 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" + ("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) + +If you find no issues, return an empty array: [] + +Example response: +[ + {"file": "common/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": "services/dispatcher/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."} +] + +=== BEGIN DIFF === diff --git a/.github/cursor-review/prompt-judge.md b/.github/cursor-review/prompt-judge.md new file mode 100644 index 0000000..5ec5b8b --- /dev/null +++ b/.github/cursor-review/prompt-judge.md @@ -0,0 +1,52 @@ +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): +- Labs: OpenAI, Anthropic, Google, Moonshot +- Review types: adversarial (security/abuse) and edge-case (correctness/logic) + +Your goal: from the panel's findings, surface the actionable ones — real +bugs and risks the author should fix or address before merging. Drop noise, +false positives, and duplicates. You MAY keep genuinely useful low-priority +items (minor nits) but classify them honestly via the severity field below; +do not inflate a nit into a bug or bury a real bug as a nit. + +Selection guidance: +- A finding raised by multiple reviewers, especially across labs or across + review types, is a strong signal. Consensus is NOT required, though — a + single sharp finding from one reviewer can make the cut if it is clearly + a real bug. +- DROP findings that misread the code or rely on assumptions outside the + diff. +- DROP near-duplicates: when two findings describe the same issue, keep the + clearest one and merge the attribution into its body. +- PREFER specificity. Rewrite a finding's body when you can make it more + actionable. +- 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 + 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: + - "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 + risk that should be fixed before merge. + - "medium": a bug or risk on an edge/uncommon path; should be fixed but not + 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 + 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-7-thinking-xhigh edge-case, gemini-3.1-pro adversarial)._` + +Order the array most-severe first. If no findings rise to the bar, return []. + +=== BEGIN PANEL FINDINGS === diff --git a/.github/cursor-review/slack-notify.sh b/.github/cursor-review/slack-notify.sh new file mode 100755 index 0000000..de2c732 --- /dev/null +++ b/.github/cursor-review/slack-notify.sh @@ -0,0 +1,224 @@ +#!/bin/bash + +# Generic Slack notification script for GitHub Actions +# Usage: ./slack-notify.sh <message> [details_file] [channel] +# +# Delivery modes (checked in order): +# DM mode: set SLACK_BOT_TOKEN + DM_GITHUB_USER +# Resolves {DM_GITHUB_USER}@comfy.org to a Slack user ID and sends a DM. +# Falls back silently (exit 0) if the user can't be resolved. +# Webhook mode: set SLACK_WEBHOOK_URL +# Posts to the channel the webhook is configured for. + +set -e + +STATUS="${1:-failure}" +TITLE="${2:-GitHub Action Notification}" +MESSAGE="${3:-No details provided}" +DETAILS_FILE="${4:-}" +CHANNEL="${5:-#prod-alerts}" +WEBHOOK_URL="${SLACK_WEBHOOK_URL}" + +# Determine color based on status +if [ "$STATUS" = "success" ]; then + COLOR="good" +elif [ "$STATUS" = "warning" ]; then + COLOR="warning" +else + COLOR="danger" +fi + +# Read additional details if file provided +ADDITIONAL_DETAILS="" +if [ -n "$DETAILS_FILE" ] && [ -f "$DETAILS_FILE" ]; then + ADDITIONAL_DETAILS=$(head -c 3000 "$DETAILS_FILE") +fi + +# ── DM mode ──────────────────────────────────────────────────────────────────── +if [ -n "${SLACK_BOT_TOKEN:-}" ] && [ -n "${DM_GITHUB_USER:-}" ]; then + # GitHub username → comfy.org email prefix overrides for anyone whose + # GitHub handle doesn't match their email prefix. Keys are lowercase for + # case-insensitive lookup (GitHub usernames are case-insensitive). + declare -A GITHUB_EMAIL_MAP + GITHUB_EMAIL_MAP[millermedia]=mattmiller + GITHUB_EMAIL_MAP[huntcsg]=hunter + GITHUB_EMAIL_MAP[skishore23]=kishore + GITHUB_EMAIL_MAP[robinjhuang]=robin + GITHUB_EMAIL_MAP[luke-mino-altherr]=luke + GITHUB_EMAIL_MAP[fengsi]=si + GITHUB_EMAIL_MAP[deepanjanroy]=dproy + GITHUB_EMAIL_MAP[deepme987]=deep + GITHUB_EMAIL_MAP[synap5e]=simonpinfold + GITHUB_EMAIL_MAP[purzbeats]=purz + + NORMALIZED_USER="${DM_GITHUB_USER,,}" + EMAIL_PREFIX="${GITHUB_EMAIL_MAP[$NORMALIZED_USER]:-$NORMALIZED_USER}" + echo "DM mode: resolving ${EMAIL_PREFIX}@comfy.org (GitHub: ${DM_GITHUB_USER})" + + EMAIL="${EMAIL_PREFIX}@comfy.org" + # --data-urlencode safely encodes the email (guards against [ ] in bot logins + # being treated as curl URL glob syntax). Drop -f so HTTP errors return JSON + # instead of aborting under set -e; we inspect .ok ourselves. + if ! LOOKUP=$(curl -sSL \ + -H "Authorization: Bearer $SLACK_BOT_TOKEN" \ + --get --data-urlencode "email=${EMAIL}" \ + "https://slack.com/api/users.lookupByEmail"); then + echo "Slack lookup transport failed for ${EMAIL} — skipping DM" + exit 0 + fi + + SLACK_USER_ID=$(echo "$LOOKUP" | jq -r 'if .ok and (.user.id != null) then .user.id else "" end') + + if [ -z "$SLACK_USER_ID" ]; then + SLACK_ERROR=$(echo "$LOOKUP" | jq -r '.error // "unknown"') + if [ "$SLACK_ERROR" = "users_not_found" ]; then + echo "Slack user not found for $EMAIL — skipping DM" + exit 0 + else + echo "⚠️ Slack lookup failed for $EMAIL ($SLACK_ERROR) — check token/scopes" + exit 1 + fi + fi + + echo "Resolved ${DM_GITHUB_USER} → $SLACK_USER_ID" + + RUN_URL="${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" + + DM_TEXT="$MESSAGE" + if [ -n "$ADDITIONAL_DETAILS" ]; then + DM_TEXT="${MESSAGE} +\`\`\`${ADDITIONAL_DETAILS}\`\`\`" + fi + + PAYLOAD=$(jq -n \ + --arg channel "$SLACK_USER_ID" \ + --arg color "$COLOR" \ + --arg title "$TITLE" \ + --arg text "$DM_TEXT" \ + --arg repo "${GITHUB_REPOSITORY:-}" \ + --arg run_url "$RUN_URL" \ + '{ + channel: $channel, + attachments: [{ + color: $color, + title: $title, + text: $text, + mrkdwn_in: ["text"], + fields: [ + { title: "Repository", value: $repo, short: true }, + { title: "Run", value: ("<" + $run_url + "|View Details>"), short: true } + ], + footer: "GitHub Actions" + }] + }') + + if ! RESPONSE=$(curl -sSL -X POST \ + -H "Authorization: Bearer $SLACK_BOT_TOKEN" \ + -H "Content-Type: application/json" \ + --data "$PAYLOAD" \ + https://slack.com/api/chat.postMessage); then + echo "❌ chat.postMessage transport failed — skipping" + exit 0 + fi + + if echo "$RESPONSE" | jq -e '.ok' > /dev/null; then + echo "✅ DM sent to $SLACK_USER_ID" + exit 0 + else + echo "❌ Failed to send DM: $(echo "$RESPONSE" | jq -r '.error // "unknown"')" + exit 0 + fi +fi + +# ── Webhook mode ─────────────────────────────────────────────────────────────── +if [ -z "$WEBHOOK_URL" ]; then + echo "❌ No delivery method configured: set SLACK_BOT_TOKEN+DM_GITHUB_USER or SLACK_WEBHOOK_URL" + exit 1 +fi + +# Optional top-level mention (webhook mode only). +MENTION_TEXT="" +if [ -n "${MENTION_USER_ID:-}" ]; then + MENTION_TEXT=$(jq -nr --arg u "$MENTION_USER_ID" '"<@" + $u + ">"') +fi + +RUN_URL="${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" + +# Build the full Slack payload via jq so every interpolated value (PR titles +# containing quotes, branches with special chars, ADDITIONAL_DETAILS captured +# from arbitrary command output) is properly JSON-escaped. The previous +# heredoc-with-${var} approach generated invalid JSON whenever a value +# contained `"` or `\`, which then tripped the `jq empty` validator below +# and silently dropped notifications. +SLACK_MESSAGE=$(jq -n \ + --arg channel "$CHANNEL" \ + --arg mention_text "$MENTION_TEXT" \ + --arg color "$COLOR" \ + --arg title "$TITLE" \ + --arg message "$MESSAGE" \ + --arg repository "${GITHUB_REPOSITORY:-}" \ + --arg workflow "${GITHUB_WORKFLOW:-}" \ + --arg run_url "$RUN_URL" \ + --arg event_name "${GITHUB_EVENT_NAME:-}" \ + --arg ref_name "${GITHUB_REF_NAME:-}" \ + --arg actor "${GITHUB_ACTOR:-}" \ + --arg details "$ADDITIONAL_DETAILS" \ + --argjson ts "$(date +%s)" \ + '{ + channel: $channel, + attachments: ( + [{ + color: $color, + title: $title, + text: $message, + fields: [ + { title: "Repository", value: $repository, short: true }, + { title: "Workflow", value: $workflow, short: true }, + { title: "Run", value: ("<" + $run_url + "|View Details>"), short: true }, + { title: "Trigger", value: $event_name, short: true }, + { title: "Branch", value: $ref_name, short: true }, + { title: "Actor", value: $actor, short: true } + ], + footer: "GitHub Actions", + ts: $ts + }] + + (if ($details | length) > 0 then [{ + color: $color, + title: "Details", + text: ("```" + $details + "```"), + mrkdwn_in: ["text"] + }] else [] end) + ) + } + + (if ($mention_text | length) > 0 then { text: $mention_text } else {} end)') + +# Validate JSON before sending +echo "🔍 Validating JSON payload..." +if ! echo "$SLACK_MESSAGE" | jq empty 2>/dev/null; then + echo "❌ Invalid JSON payload generated" + echo "Payload:" + echo "$SLACK_MESSAGE" + exit 1 +fi + +# Send to Slack and capture response +echo "📤 Sending notification to Slack..." +RESPONSE=$(curl -X POST -H 'Content-type: application/json' \ + --data "$SLACK_MESSAGE" \ + "$WEBHOOK_URL" \ + --silent --show-error --write-out "\n%{http_code}") + +# Extract HTTP status code and body +HTTP_CODE=$(echo "$RESPONSE" | tail -n1) +BODY=$(echo "$RESPONSE" | head -n-1) + +# Check if Slack accepted the message +if [ "$BODY" = "ok" ] && [ "$HTTP_CODE" = "200" ]; then + echo "✅ Slack notification sent successfully" + exit 0 +else + echo "❌ Slack notification failed" + echo "HTTP Code: $HTTP_CODE" + echo "Response: $BODY" + exit 1 +fi diff --git a/.github/workflows/cursor-review.yml b/.github/workflows/cursor-review.yml new file mode 100644 index 0000000..7c5c6e7 --- /dev/null +++ b/.github/workflows/cursor-review.yml @@ -0,0 +1,700 @@ +name: Cursor Review (reusable) + +# Reusable, label-triggered multi-model code review. A 4-lab × 2-review-type +# panel (8 cursor-agent cells) runs adversarial + edge-case passes, then a +# 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 +# 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. +# +# Caller pattern (place in source repo at .github/workflows/ci-cursor-review.yml): +# +# name: CI - Cursor Review +# on: +# pull_request: +# types: [labeled, unlabeled] +# permissions: +# contents: read +# pull-requests: write +# concurrency: +# group: cursor-review-pr-${{ github.event.pull_request.number }}-${{ github.event.label.name }} +# cancel-in-progress: true +# jobs: +# cursor-review: +# uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@main +# with: +# # Repo-specific pathspecs excluded from the size cap and the diff. +# diff_excludes: >- +# :!**/package-lock.json +# :!data/object_info.json.gz +# # Pin the assets ref to the same ref you pin `uses:` to for +# # reproducibility (defaults to main). +# workflows_ref: main +# secrets: +# CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }} +# SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} + +on: + workflow_call: + inputs: + judge_model: + description: >- + Single judge model that adjudicates panel findings into one + consolidated review. Picked to match the highest reasoning tier in + the panel. + type: string + required: false + default: claude-opus-4-7-thinking-xhigh + diff_size_cap: + description: >- + Max changed lines (added + removed, after diff_excludes). PRs over + the cap are skipped — too large for a useful single-pass review. + type: number + required: false + default: 5000 + review_label: + description: Label whose addition triggers the review. + type: string + required: false + default: cursor-review + diff_excludes: + description: >- + Whitespace-separated git pathspecs excluded from BOTH the size-budget + count and the diff fed to Cursor. Pass your repo's generated/vendored + paths (lockfiles, snapshots, fixtures) so they don't blow the cap or + waste review signal. Folded scalar collapses newlines to spaces so the + value word-splits into pathspec args. Paths with a leading underscore + need the long-form `:(exclude)` syntax — the short-form `:!` parser + reads `_` as pathspec magic. + type: string + required: false + default: >- + :!**/package-lock.json + :!**/yarn.lock + :!**/pnpm-lock.yaml + :!**/go.sum + :!**/node_modules/** + :!**/.claude/** + :!**/dist/** + :!**/vendor/** + :!**/*.generated.* + :!**/*.min.js + :!**/*.min.css + workflows_ref: + description: >- + Ref of Comfy-Org/github-workflows to load the prompts and scripts + from. Pin this to the same ref you pin `uses:` to for reproducibility. + type: string + required: false + default: main + secrets: + CURSOR_API_KEY: + description: Cursor API key for cursor-agent (the panel + judge models bill through it). + required: true + SLACK_BOT_TOKEN: + description: Slack bot token for the start/complete DM notifications. Optional — DMs are skipped if absent. + required: false + +# DIFF_SIZE_CAP / REVIEW_LABEL / JUDGE_MODEL / DIFF_EXCLUDES are mapped from +# `inputs` here so the run steps below read them verbatim from the original +# (private-repo) workflow without per-step plumbing. `inputs` is available to +# workflow-level env in a called workflow; `secrets` is not (those stay in jobs). +env: + DIFF_SIZE_CAP: ${{ inputs.diff_size_cap }} + REVIEW_LABEL: ${{ inputs.review_label }} + JUDGE_MODEL: ${{ inputs.judge_model }} + DIFF_EXCLUDES: ${{ inputs.diff_excludes }} + # Where the assets checkout lands (this repo, .github/cursor-review/*). + CURSOR_REVIEW_ASSETS: ${{ github.workspace }}/_cursor_review_assets/.github/cursor-review + +jobs: + gate: + name: Gate + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + outputs: + should_run: ${{ steps.check.outputs.should_run }} + already_reviewed: ${{ steps.dup.outputs.already_reviewed }} + steps: + - name: Check trigger label + id: check + env: + LABEL_NAME: ${{ github.event.label.name }} + PR_LABELS: ${{ toJSON(github.event.pull_request.labels.*.name) }} + GH_EVENT_ACTION: ${{ github.event.action }} + run: | + # skip-cursor-review wins even if the trigger label is also present. + if echo "$PR_LABELS" | jq -e 'index("skip-cursor-review")' > /dev/null; then + echo "skip-cursor-review label present — skipping." + echo "should_run=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + # Only adding the trigger label fires the matrix. Removing it cancels + # any in-progress run via the concurrency group and then no-ops here. + if [ "$LABEL_NAME" = "$REVIEW_LABEL" ] && [ "$GH_EVENT_ACTION" = "labeled" ]; then + echo "$REVIEW_LABEL label added — running." + echo "should_run=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + + # Removing skip-cursor-review while the trigger label is present + # unblocks a previously skipped review — treat it as a fresh trigger. + if [ "$LABEL_NAME" = "skip-cursor-review" ] && [ "$GH_EVENT_ACTION" = "unlabeled" ]; then + if echo "$PR_LABELS" | jq -e --arg l "$REVIEW_LABEL" 'index($l)' > /dev/null; then + echo "skip-cursor-review removed while $REVIEW_LABEL is present — running." + echo "should_run=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + fi + + echo "Label '$LABEL_NAME' is not the trigger label — skipping." + echo "should_run=false" >> "$GITHUB_OUTPUT" + + - name: Check for prior review on this commit + id: dup + if: steps.check.outputs.should_run == 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + # Idempotency: skip the matrix if a Cursor Review consolidated + # review already exists for this exact HEAD SHA. Protects against + # label remove+re-add (or any other event-replay path) from + # re-running the panel on unchanged content. + # + # Escape hatches: + # - Push commits → new HEAD SHA, check finds nothing, runs. + # - Dismiss the existing review → DISMISSED state filtered out + # below, so re-applying the label fires a fresh run. + # + # API failure falls through to running, not skipping — better to + # double-review than silently no-op. + EXISTING=$(gh api "repos/$REPO/pulls/$PR_NUMBER/reviews" 2>/dev/null \ + | jq --arg sha "$HEAD_SHA" '[.[] | select(.commit_id == $sha and .state != "DISMISSED" and (.body | startswith("## 🔍 Cursor Review — Consolidated panel")))] | length' \ + 2>/dev/null || echo 0) + + if [ "$EXISTING" -gt 0 ]; then + echo "Already reviewed at $HEAD_SHA ($EXISTING existing review(s)) — skipping matrix." + echo "already_reviewed=true" >> "$GITHUB_OUTPUT" + else + echo "No prior review for $HEAD_SHA — proceeding." + echo "already_reviewed=false" >> "$GITHUB_OUTPUT" + fi + + diff-size: + name: Diff size check + needs: gate + if: needs.gate.outputs.should_run == 'true' && needs.gate.outputs.already_reviewed != 'true' + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + within_cap: ${{ steps.count.outputs.within_cap }} + steps: + - name: Checkout PR repo + uses: actions/checkout@v6 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Count changed lines + id: count + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + # numstat reports "-\t-" for binary files; treat those as 0 lines so + # binary-only PRs can't bypass the size cap by producing no insertion + # tokens for grep to count. + # $DIFF_EXCLUDES is intentionally unquoted: bash word-splits it into + # separate pathspec args. Pathspecs all start with ":!", so pathname + # expansion finds no matches and leaves them literal. + # shellcheck disable=SC2086 + TOTAL=$(git diff --numstat "$BASE_SHA...$HEAD_SHA" -- . $DIFF_EXCLUDES \ + | awk '{ if ($1 != "-") added += $1; if ($2 != "-") removed += $2 } END { print (added + removed) + 0 }') + echo "Changed lines: $TOTAL (cap: $DIFF_SIZE_CAP)" + + if [ "$TOTAL" -gt "$DIFF_SIZE_CAP" ]; then + echo "Diff exceeds cap — skipping review." + echo "within_cap=false" >> "$GITHUB_OUTPUT" + else + echo "within_cap=true" >> "$GITHUB_OUTPUT" + fi + + review: + # Each cell produces a findings artifact. No cell posts a review — + # consolidate adjudicates the panel and posts ONE review at the end. + name: '${{ matrix.review_type }} (${{ matrix.model }})' + needs: [gate, diff-size] + if: needs.gate.outputs.should_run == 'true' && needs.gate.outputs.already_reviewed != 'true' && needs.diff-size.outputs.within_cap == 'true' + runs-on: ubuntu-latest + timeout-minutes: 30 + permissions: + contents: read + strategy: + fail-fast: false + matrix: + # Pinned to the highest reasoning tier available for each lab. Gemini + # and Kimi only ship a single tier in Cursor's catalog; OpenAI Codex + # and Claude expose xhigh variants we use here. + model: + - gpt-5.3-codex-xhigh + - claude-opus-4-7-thinking-xhigh + - gemini-3.1-pro + - kimi-k2.5 + review_type: [adversarial, edge-case] + steps: + - 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 + # this, an early-failing cell silently disappears from the panel + # and the consolidated review undercounts the matrix. + env: + MODEL: ${{ matrix.model }} + REVIEW_TYPE: ${{ matrix.review_type }} + run: | + mkdir -p /tmp/findings-out + python3 - <<'PY' + import json, os + record = { + "model": os.environ["MODEL"], + "review_type": os.environ["REVIEW_TYPE"], + "status": "error", + "error": "Cell did not reach the extraction step.", + "findings": [], + } + with open("/tmp/findings-out/findings.json", "w", encoding="utf-8") as f: + json.dump(record, f) + PY + + - name: Checkout PR repo + uses: actions/checkout@v6 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Load cursor-review assets + # Trusted prompts/scripts come from THIS workflow's repo (public, + # pinned via workflows_ref) — never from the PR checkout, so a + # malicious PR can't rewrite the prompt to exfiltrate the diff or + # smuggle instructions to the model. + uses: actions/checkout@v6 + with: + repository: Comfy-Org/github-workflows + ref: ${{ inputs.workflows_ref }} + path: _cursor_review_assets + persist-credentials: false + + - name: Generate diff + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + # See diff_excludes input for the rationale and pathspec list. + # shellcheck disable=SC2086 + git diff "$BASE_SHA...$HEAD_SHA" -- . $DIFF_EXCLUDES > /tmp/pr-diff.patch + + echo "Diff size: $(wc -l < /tmp/pr-diff.patch) lines" + + - name: Install Cursor agent CLI + run: | + curl https://cursor.com/install -fsSL | bash + echo "$HOME/.cursor/bin" >> "$GITHUB_PATH" + + - name: Log Cursor agent version + # The install script is unpinned (curl | bash from cursor.com), so we + # log the installed version into the run for forensics rather than + # gating on a hard-coded baseline. A version equality check produced + # daily toil without adding integrity protection — a CDN compromise + # would just ship malicious bits as a "new version" we'd then bump to. + run: cursor-agent --version + + - name: Build prompt + env: + REVIEW_TYPE: ${{ matrix.review_type }} + run: | + { + cat "$CURSOR_REVIEW_ASSETS/prompt-${REVIEW_TYPE}.md" + cat /tmp/pr-diff.patch + echo "" + echo "=== END DIFF ===" + } > /tmp/prompt.txt + + echo "Prompt size: $(wc -c < /tmp/prompt.txt) bytes" + + - name: Run cursor review + env: + CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }} + MODEL: ${{ matrix.model }} + run: | + # --sandbox is intentionally omitted. The hosted runner is already + # an ephemeral VM, so the agent's AppArmor sandbox doesn't add + # protection — and as of cursor-agent 2026.05.05 it fails to start + # on ubuntu-latest, producing empty output. + cursor-agent \ + --print \ + --trust \ + --model "$MODEL" \ + < /tmp/prompt.txt \ + > /tmp/review-raw.txt 2>/tmp/review-stderr.txt || true + + echo "=== Raw output (first 500 chars) ===" + head -c 500 /tmp/review-raw.txt + echo "" + echo "=== Stderr ===" + cat /tmp/review-stderr.txt + + - name: Extract 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 ===" + cat /tmp/findings-out/findings.json + echo "" + + - name: Upload findings artifact + if: always() + uses: actions/upload-artifact@v7 + with: + name: findings-${{ matrix.review_type }}-${{ matrix.model }} + path: /tmp/findings-out/findings.json + if-no-files-found: error + retention-days: 7 + + consolidate: + # Adjudicates the 8 cell artifacts into one consolidated review. + # Calls cursor-agent one more time as the judge, with prompt-judge.md. + name: Consolidate panel + needs: [gate, diff-size, review] + if: always() && needs.gate.outputs.should_run == 'true' && needs.gate.outputs.already_reviewed != 'true' && needs.diff-size.outputs.within_cap == 'true' && needs.review.result != 'skipped' + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: read + pull-requests: write + steps: + - name: Checkout PR repo + uses: actions/checkout@v6 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Load cursor-review assets + uses: actions/checkout@v6 + with: + repository: Comfy-Org/github-workflows + ref: ${{ inputs.workflows_ref }} + path: _cursor_review_assets + persist-credentials: false + + - name: Generate diff + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + # See diff_excludes input for the rationale and pathspec list. + # shellcheck disable=SC2086 + git diff "$BASE_SHA...$HEAD_SHA" -- . $DIFF_EXCLUDES > /tmp/pr-diff.patch + + - name: Install Cursor agent CLI + run: | + curl https://cursor.com/install -fsSL | bash + echo "$HOME/.cursor/bin" >> "$GITHUB_PATH" + + - name: Download panel findings + uses: actions/download-artifact@v8 + with: + path: /tmp/panel + pattern: findings-* + + - name: Aggregate panel findings + id: aggregate + run: | + # Each artifact directory contains one findings.json. Combine into + # a single panel.json array preserving cell metadata. + python3 - <<'PY' + import json, os, glob + + panel = [] + for path in sorted(glob.glob("/tmp/panel/findings-*/findings.json")): + with open(path, encoding="utf-8") as f: + panel.append(json.load(f)) + + with open("/tmp/panel.json", "w", encoding="utf-8") as f: + json.dump(panel, f, indent=2) + + ok = sum(1 for c in panel if c.get("status") == "ok") + print(f"Panel: {ok}/{len(panel)} cells contributed findings.") + + # Emit metadata for the next step to gate on. + with open(os.environ["GITHUB_OUTPUT"], "a") as g: + g.write(f"ok_count={ok}\n") + g.write(f"total={len(panel)}\n") + PY + + - name: Build judge prompt + if: steps.aggregate.outputs.ok_count != '0' + run: | + { + cat "$CURSOR_REVIEW_ASSETS/prompt-judge.md" + cat /tmp/panel.json + echo "" + echo "=== END PANEL FINDINGS ===" + echo "" + echo "=== BEGIN DIFF ===" + cat /tmp/pr-diff.patch + echo "" + echo "=== END DIFF ===" + } > /tmp/judge-prompt.txt + + echo "Judge prompt size: $(wc -c < /tmp/judge-prompt.txt) bytes" + + - name: Run judge + if: steps.aggregate.outputs.ok_count != '0' + env: + CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }} + run: | + cursor-agent \ + --print \ + --trust \ + --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 + echo "" + echo "=== Judge stderr ===" + cat /tmp/judge-stderr.txt + + - 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 [ "$OK_COUNT" = "0" ]; then + echo "No panel cells contributed findings — skipping judge parse." + 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. + 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. + python3 - <<'PY' + import json + with open("/tmp/panel.json", encoding="utf-8") as f: + panel = json.load(f) + panel_meta = [ + {"model": c.get("model"), "review_type": c.get("review_type"), "status": c.get("status")} + for c in panel + ] + with open("/tmp/judge-findings.json", encoding="utf-8") as f: + judge = json.load(f) + findings = judge.get("findings") or [] + consolidated = {"findings": findings, "panel": panel_meta} + with open("/tmp/consolidated.json", "w", encoding="utf-8") as f: + json.dump(consolidated, f) + print(f"Consolidated: {len(findings)} judge finding(s), {len(panel_meta)} panel cells.") + PY + + - name: Resolve triggerer + id: triggerer + env: + GH_ACTOR: ${{ github.actor }} + PR_ASSIGNEES: ${{ toJSON(github.event.pull_request.assignees.*.login) }} + run: | + # github.actor on pull_request.labeled is the user who applied the + # label — but when the auto-labeler workflow applies it via a bot + # App token, github.actor becomes "<app>[bot]" and the human who + # actually triggered the chain (by assigning themselves) is lost. + # Recover them by falling back to the PR's first assignee whenever + # the actor is a bot. + if [[ "$GH_ACTOR" == *"[bot]" ]]; then + FALLBACK=$(echo "$PR_ASSIGNEES" | jq -r '.[0] // empty') + if [ -n "$FALLBACK" ]; then + echo "Actor $GH_ACTOR is a bot — using assignee '$FALLBACK' for attribution." + echo "triggered_by=$FALLBACK" >> "$GITHUB_OUTPUT" + else + echo "Actor $GH_ACTOR is a bot and no assignee found — keeping bot attribution." + echo "triggered_by=$GH_ACTOR" >> "$GITHUB_OUTPUT" + fi + else + echo "triggered_by=$GH_ACTOR" >> "$GITHUB_OUTPUT" + fi + + - name: Post review + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + TRIGGERED_BY: ${{ steps.triggerer.outputs.triggered_by }} + JUDGE_STATUS: ${{ steps.consolidated.outputs.judge_status }} + run: | + # If the judge call itself failed but the panel had content, post + # an error review so we don't silently misreport. The "ok with + # zero findings" path stays normal — that's the legitimate + # "no high-signal findings" case. + if [ "$JUDGE_STATUS" != "ok" ]; then + 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" \ + --findings /tmp/consolidated.json \ + --pr-number "$PR_NUMBER" \ + --repo "$REPO" \ + --commit-sha "$HEAD_SHA" \ + --triggered-by "$TRIGGERED_BY" \ + --error-message "Judge call failed (status=${JUDGE_STATUS}): ${JUDGE_ERROR}" + else + python3 "$CURSOR_REVIEW_ASSETS/post-review.py" \ + --findings /tmp/consolidated.json \ + --pr-number "$PR_NUMBER" \ + --repo "$REPO" \ + --commit-sha "$HEAD_SHA" \ + --triggered-by "$TRIGGERED_BY" + fi + + notify-start: + name: Notify start + needs: [gate, diff-size] + if: needs.gate.outputs.should_run == 'true' && needs.gate.outputs.already_reviewed != 'true' && needs.diff-size.outputs.within_cap == 'true' + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Load cursor-review assets + uses: actions/checkout@v6 + with: + repository: Comfy-Org/github-workflows + ref: ${{ inputs.workflows_ref }} + path: _cursor_review_assets + persist-credentials: false + + - name: Resolve triggerer + id: triggerer + env: + GH_ACTOR: ${{ github.actor }} + PR_ASSIGNEES: ${{ toJSON(github.event.pull_request.assignees.*.login) }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + run: | + if [[ "$GH_ACTOR" == *"[bot]" ]]; then + ASSIGNEE=$(echo "$PR_ASSIGNEES" | jq -r '.[0] // empty') + # Prefer PR author for DM routing — they opened the work and most want + # the review status. Falls back to assignee, then empty (skips DM) if + # the author is also a bot (e.g. Dependabot PRs). + RESOLVED="${PR_AUTHOR:-${ASSIGNEE:-}}" + if [[ "$RESOLVED" == *"[bot]" ]]; then RESOLVED=""; fi + echo "triggered_by=$RESOLVED" >> "$GITHUB_OUTPUT" + else + echo "triggered_by=$GH_ACTOR" >> "$GITHUB_OUTPUT" + fi + + - name: DM triggerer — review starting + continue-on-error: true + env: + SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} + DM_GITHUB_USER: ${{ steps.triggerer.outputs.triggered_by }} + PR_TITLE: ${{ github.event.pull_request.title }} + PR_URL: ${{ github.event.pull_request.html_url }} + run: | + chmod +x "$CURSOR_REVIEW_ASSETS/slack-notify.sh" + MSG="*${PR_TITLE}*"$'\n'"${PR_URL}"$'\n'"8-cell panel running, then a judge consolidates." + "$CURSOR_REVIEW_ASSETS/slack-notify.sh" \ + "warning" \ + "Cursor review starting" \ + "$MSG" + + notify-complete: + name: Notify complete + needs: [gate, diff-size, review, consolidate] + if: always() && needs.gate.outputs.should_run == 'true' && needs.gate.outputs.already_reviewed != 'true' && needs.diff-size.outputs.within_cap == 'true' && needs.review.result != 'skipped' + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Load cursor-review assets + uses: actions/checkout@v6 + with: + repository: Comfy-Org/github-workflows + ref: ${{ inputs.workflows_ref }} + path: _cursor_review_assets + persist-credentials: false + + - name: Resolve triggerer + id: triggerer + env: + GH_ACTOR: ${{ github.actor }} + PR_ASSIGNEES: ${{ toJSON(github.event.pull_request.assignees.*.login) }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + run: | + if [[ "$GH_ACTOR" == *"[bot]" ]]; then + ASSIGNEE=$(echo "$PR_ASSIGNEES" | jq -r '.[0] // empty') + # Prefer PR author for DM routing — they opened the work and most want + # the review status. Falls back to assignee, then empty (skips DM) if + # the author is also a bot (e.g. Dependabot PRs). + RESOLVED="${PR_AUTHOR:-${ASSIGNEE:-}}" + if [[ "$RESOLVED" == *"[bot]" ]]; then RESOLVED=""; fi + echo "triggered_by=$RESOLVED" >> "$GITHUB_OUTPUT" + else + echo "triggered_by=$GH_ACTOR" >> "$GITHUB_OUTPUT" + fi + + - name: DM triggerer — review complete + continue-on-error: true + env: + SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} + DM_GITHUB_USER: ${{ steps.triggerer.outputs.triggered_by }} + PR_TITLE: ${{ github.event.pull_request.title }} + PR_URL: ${{ github.event.pull_request.html_url }} + CONSOLIDATE_RESULT: ${{ needs.consolidate.result }} + run: | + chmod +x "$CURSOR_REVIEW_ASSETS/slack-notify.sh" + if [ "$CONSOLIDATE_RESULT" = "success" ]; then + STATUS="success" + TITLE="Cursor review complete" + DETAIL="One consolidated review is on the PR." + else + STATUS="warning" + TITLE="Cursor review failed to post" + DETAIL="Consolidate step did not succeed — see the run for details." + fi + MSG="*${PR_TITLE}*"$'\n'"${PR_URL}"$'\n'"${DETAIL}" + "$CURSOR_REVIEW_ASSETS/slack-notify.sh" \ + "$STATUS" \ + "$TITLE" \ + "$MSG" diff --git a/README.md b/README.md index a064bc9..99d7873 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,7 @@ This repo is **public** so any repo — public or private, inside or outside the | Workflow | Purpose | |---|---| | [`detect-unreviewed-merge.yml`](.github/workflows/detect-unreviewed-merge.yml) | SOC 2 compliance — detects PRs merged without prior approval and opens a tracking issue in [`Comfy-Org/unreviewed-merges`](https://github.com/Comfy-Org/unreviewed-merges). | +| [`cursor-review.yml`](.github/workflows/cursor-review.yml) | Label-triggered multi-model code review. A 4-lab × 2-review-type cursor-agent panel runs adversarial + edge-case passes, a judge model consolidates them into one PR review with per-finding severity badges, and the triggerer gets Slack start/complete DMs. Prompts and scripts live in [`.github/cursor-review/`](.github/cursor-review) — the single source of truth, so consumer repos carry only a thin caller. Requires `CURSOR_API_KEY` (+ optional `SLACK_BOT_TOKEN`). | ## Usage