From 6b6c48da0bdb721094206c216f9d70e3b8f6914f Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Mon, 27 Jul 2026 19:17:21 -0700 Subject: [PATCH 1/3] feat(cursor-review): weekly catalog-drift check for the panel pins (BE-4819) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-PR preflight fails loud when a PINNED model id is delisted, but only at review time, on somebody's PR, after the pin has already gone bad — and by design it never notices a NEWER model shipping. The only guard against that second case was a `last checked ` comment above the pins, which went stale inside 13 days: both catalog swaps found by the BE-4817 audit landed in that window and were caught by hand, not by CI. Add a scheduled (Mon 06:17 UTC) + dispatchable check that reads the pins OUT of cursor-review.yml — panel heredoc plus the judge_model input default, never a duplicated list — diffs them against `cursor-agent models`, and reports delisted pins, unpinned same-lab catalog ids (a review-me list, not a recommendation; NO-ZDR markers surfaced verbatim), and a `last checked` date older than 30 days through one sticky issue that is updated in place and closed automatically on a clean run. Comparison + rendering live in .github/cursor-review/catalog-drift.py with unit tests, including one that runs the extractors against the real cursor-review.yml so a refactor there can't silently blind the check; cursor-review.yml joins that test job's path filter for the same reason. --- .github/cursor-review/README.md | 19 + .github/cursor-review/catalog-drift.py | 491 ++++++++++++++++++ .../cursor-review/tests/test_catalog_drift.py | 360 +++++++++++++ .../workflows/cursor-review-catalog-drift.yml | 152 ++++++ .../workflows/test-cursor-review-scripts.yml | 9 + AGENTS.md | 5 +- README.md | 1 + 7 files changed, 1036 insertions(+), 1 deletion(-) create mode 100644 .github/cursor-review/catalog-drift.py create mode 100644 .github/cursor-review/tests/test_catalog_drift.py create mode 100644 .github/workflows/cursor-review-catalog-drift.yml diff --git a/.github/cursor-review/README.md b/.github/cursor-review/README.md index 7ee26ae..2794e03 100644 --- a/.github/cursor-review/README.md +++ b/.github/cursor-review/README.md @@ -75,6 +75,24 @@ 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. +These pins live in exactly one place — the `Define panel models` step of +`cursor-review.yml`'s preflight job (the `judge_model` input default is the +fifth pin) — and the preflight validates them against Cursor's live catalog on +every run, so a delisted pin fails loudly instead of silently running the panel +a lab short. + +Drift the preflight *can't* see — a **newer** model shipping — is now +machine-checked weekly by +[`cursor-review-catalog-drift.yml`](../workflows/cursor-review-catalog-drift.yml), +which diffs the pins against `cursor-agent models` and files one sticky +`[cursor-review catalog drift]` issue listing delisted pins, unpinned same-lab +catalog ids, and an audit date older than 30 days. It reports; it never bumps a +pin — picking the newest highest-reasoning **ZDR-eligible** model stays a human +call (Cursor marks only NO-ZDR models inline, e.g. Fable's `(NO ZDR)` suffix). +The `last checked ` comment above the pins remains the **human**-audit +record: refresh it when you re-audit, and the weekly check will tell you when +it has gone stale. + ## What's in this directory | File | Role | @@ -85,6 +103,7 @@ silently vanishing — the review tells you what didn't run. | [`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. | | [`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. | +| [`catalog-drift.py`](catalog-drift.py) | Backs the weekly catalog-drift check. Extracts the pins from `cursor-review.yml`, diffs them against raw `cursor-agent models` output, and renders the sticky issue title + body (delisted pins, unpinned same-lab ids, stale audit date). Reports only — it never edits a pin. | | [`slack-notify.sh`](slack-notify.sh) | Sends the start/complete Slack DMs to the triggerer (no-ops without a token). | ## Adopt it in your repo diff --git a/.github/cursor-review/catalog-drift.py b/.github/cursor-review/catalog-drift.py new file mode 100644 index 0000000..78c2569 --- /dev/null +++ b/.github/cursor-review/catalog-drift.py @@ -0,0 +1,491 @@ +#!/usr/bin/env python3 +"""Detect drift between the cursor-review panel pins and Cursor's live catalog. + +The per-PR preflight in `cursor-review.yml` already fails loud when a *pinned* +model id disappears — but only at review time, on somebody's PR, after the pin +has already gone bad. And by design it never notices a *newer* model shipping. +Until BE-4819 the only guard against that second case was a `last checked +` comment above the pins, which went stale inside 13 days: both catalog +swaps found by the BE-4817 audit (Opus 5 on 2026-07-24, Kimi K3 on ~2026-07-26) +landed in that window and were caught by hand, not by CI. + +This script is the machine half of that audit. Given the workflow file and the +raw output of `cursor-agent models`, it reports three kinds of drift: + + * **delisted pin** — a pinned id (panel or judge) is absent from the live + catalog. Urgent: consumer PRs will start failing preflight. + * **unpinned same-lab ids** — catalog ids from a lab the panel already pins, + which the panel does *not* pin. A REVIEW-ME list, never an auto-recommendation: + picking "newest highest-reasoning ZDR-eligible" needs human judgment, and ZDR + especially — Cursor only marks NO-ZDR inline (e.g. a `(NO ZDR)` suffix), so any + such marker on the line is surfaced verbatim rather than interpreted. + * **stale audit date** — the `last checked YYYY-MM-DD` comment is older than + `--stale-days` (or missing entirely). + +Pins are read out of `cursor-review.yml` itself rather than duplicated here, so +this checker stays zero-maintenance when the pins change. + +Usage (see cursor-review-catalog-drift.yml): + + python3 catalog-drift.py \ + --workflow .github/workflows/cursor-review.yml \ + --catalog /tmp/catalog.txt \ + --title-out /tmp/drift-title.txt \ + --body-out /tmp/drift-body.md \ + --json-out /tmp/drift-report.json \ + --run-url "$RUN_URL" + +Exit code is 0 for both "drift" and "no drift" — findings are reported through +the sticky issue, not the run status. A non-zero exit means the checker itself +could not run (pins unreadable), which is a real defect in the checker or in the +workflow it parses. The caller decides separately whether a delisted pin should +also redden the run (it does; see the workflow's final step). +""" + +import argparse +import datetime +import json +import os +import re +import sys + +STICKY_TITLE_PREFIX = "[cursor-review catalog drift]" +DEFAULT_STALE_DAYS = 30 +# GitHub caps an issue body at 65536 chars; leave room for the report above the +# raw-catalog fold rather than losing the whole body to a 422. +MAX_CATALOG_CHARS = 40000 +MAX_BODY_CHARS = 60000 + +_HEREDOC_START = re.compile(r"cat\s*>\s*/tmp/models\.json\s*<<\s*'?JSON'?") +_HEREDOC_END = re.compile(r"^\s*JSON\s*$") +_QUOTED = re.compile(r'"([^"]+)"') +_JUDGE_KEY = re.compile(r"^(\s*)judge_model\s*:\s*$") +_DEFAULT_KEY = re.compile(r"^\s*default\s*:\s*(.+?)\s*$") +_LAST_CHECKED = re.compile(r"last checked\s+(\d{4}-\d{2}-\d{2})", re.IGNORECASE) +# A catalog id is lowercase alphanumeric with `.`/`-`/`_` separators, and always +# carries at least one `-` or `.` — that separator requirement is what keeps +# prose lines ("Available models:") out of the parsed id list. +_ID_TOKEN = re.compile(r"^[a-z0-9][a-z0-9._-]*$") +_BULLET = re.compile(r"^[-*>•\s]+") + + +class ExtractionError(Exception): + """The pins could not be read out of the workflow file.""" + + +# -------------------------------------------------------------------------- +# Extraction — read the pins out of cursor-review.yml (never duplicate them) +# -------------------------------------------------------------------------- + + +def extract_panel_models(workflow_text): + """Return the panel model ids from the 'Define panel models' heredoc.""" + lines = workflow_text.splitlines() + start = None + for i, line in enumerate(lines): + if _HEREDOC_START.search(line): + start = i + break + if start is None: + raise ExtractionError( + "could not find the `cat > /tmp/models.json <<'JSON'` heredoc in the " + "preflight 'Define panel models' step" + ) + + body = [] + terminated = False + for line in lines[start + 1 :]: + if _HEREDOC_END.match(line): + terminated = True + break + body.append(line) + if not terminated: + raise ExtractionError("the /tmp/models.json heredoc is not terminated by a `JSON` line") + + raw = "\n".join(body) + try: + parsed = json.loads(raw) + except json.JSONDecodeError: + # Tolerate a heredoc that stops being strict JSON (a trailing comma, an + # inline comment): fall back to the quoted tokens, same as the ticket's + # `grep -oE '"[^"]+"'` sketch. + parsed = _QUOTED.findall(raw) + if not isinstance(parsed, list) or not all(isinstance(m, str) for m in parsed): + raise ExtractionError("the /tmp/models.json heredoc is not a JSON array of strings") + models = [m.strip() for m in parsed if m.strip()] + if not models: + raise ExtractionError("the /tmp/models.json heredoc contains no model ids") + return models + + +def extract_judge_model(workflow_text): + """Return the `judge_model` workflow input's default value.""" + lines = workflow_text.splitlines() + for i, line in enumerate(lines): + key = _JUDGE_KEY.match(line) + if not key: + continue + indent = len(key.group(1)) + for follow in lines[i + 1 :]: + if not follow.strip(): + continue + if len(follow) - len(follow.lstrip()) <= indent: + break # left the judge_model block without seeing a default + default = _DEFAULT_KEY.match(follow) + if default: + # Trim an inline `# comment`, then quotes. A folded/literal + # scalar (`default: >-`) or any value with embedded whitespace + # is not a bare model id — raise rather than "check" a pin like + # `>-`, which would report as delisted on every single run. + value = re.split(r"\s+#", default.group(1).strip(), maxsplit=1)[0].strip() + value = value.strip("\"'") + if value and not value[0] in ">|" and not re.search(r"\s", value): + return value + raise ExtractionError( + f"the `judge_model` input's default is not a bare model id: {value!r}" + ) + break + raise ExtractionError("could not find the `judge_model` input's `default:` value") + + +def extract_last_checked(workflow_text): + """Return the `last checked YYYY-MM-DD` audit date, or None if absent.""" + match = _LAST_CHECKED.search(workflow_text) + if not match: + return None + try: + return datetime.date.fromisoformat(match.group(1)) + except ValueError: + return None + + +# -------------------------------------------------------------------------- +# Catalog parsing + comparison +# -------------------------------------------------------------------------- + + +def catalog_entries(catalog_text): + """Parse `cursor-agent models` output into ordered (id, note) pairs. + + `note` is whatever else the line carries — Cursor marks non-ZDR models + inline (e.g. a `(NO ZDR)` suffix), and that marker is exactly what a human + needs to see before promoting a model into a panel that reviews private + diffs, so it is passed through verbatim rather than parsed. + """ + seen = {} + order = [] + for raw in catalog_text.splitlines(): + line = _BULLET.sub("", raw.strip()) + if not line: + continue + parts = line.split(None, 1) + model_id = parts[0] + if not _ID_TOKEN.match(model_id) or not re.search(r"[-.]", model_id): + continue + note = parts[1].strip() if len(parts) > 1 else "" + if model_id in seen: + if note and not seen[model_id]: + seen[model_id] = note + continue + seen[model_id] = note + order.append(model_id) + return [(m, seen[m]) for m in order] + + +def present(model_id, catalog_text): + """Whole-token containment check — the same regex the preflight uses.""" + pattern = r"(? `gpt` (preflight's split).""" + return re.split(r"[-.]", model_id, maxsplit=1)[0].lower() + + +def analyze(panel_models, judge_model, catalog_text, last_checked, today, stale_days): + """Compare the pins against the catalog and return the drift report.""" + entries = catalog_entries(catalog_text) + pinned = list(panel_models) + if judge_model not in pinned: + pinned.append(judge_model) + + delisted = [] + for model_id in pinned: + if present(model_id, catalog_text): + continue + lab = lab_of(model_id) + roles = [] + if model_id in panel_models: + roles.append("panel") + if model_id == judge_model: + roles.append("judge") + delisted.append( + { + "id": model_id, + "roles": roles, + "lab": lab, + "same_lab_available": [m for m, _ in entries if lab_of(m) == lab], + } + ) + + # Labs are derived from the pins themselves (not a hardcoded list) so a pin + # bump to a newly-branded lab keeps this checker zero-maintenance. With + # today's pins this resolves to exactly gpt- / claude- / gemini- / kimi-. + labs = [] + for model_id in pinned: + lab = lab_of(model_id) + if lab not in labs: + labs.append(lab) + pinned_set = set(pinned) + unpinned = [] + for lab in labs: + candidates = [ + {"id": m, "note": note} for m, note in entries if lab_of(m) == lab and m not in pinned_set + ] + if candidates: + unpinned.append( + { + "lab": lab, + "pinned": [m for m in pinned if lab_of(m) == lab], + "candidates": candidates, + } + ) + + audit = { + "last_checked": last_checked.isoformat() if last_checked else None, + "age_days": (today - last_checked).days if last_checked else None, + "stale_days": stale_days, + } + audit["stale"] = last_checked is None or audit["age_days"] > stale_days + + return { + "pins": {"panel": list(panel_models), "judge": judge_model}, + "catalog_ids": [m for m, _ in entries], + "delisted": delisted, + "unpinned": unpinned, + "audit": audit, + "urgent": bool(delisted), + "has_findings": bool(delisted or unpinned or audit["stale"]), + } + + +# -------------------------------------------------------------------------- +# Rendering +# -------------------------------------------------------------------------- + + +def summary_line(report): + """One-line description of the findings — used as the sticky issue title.""" + bits = [] + delisted = report["delisted"] + if delisted: + bits.append(f"{len(delisted)} delisted pin{'s' if len(delisted) != 1 else ''}") + count = sum(len(g["candidates"]) for g in report["unpinned"]) + if count: + bits.append(f"{count} unpinned same-lab id{'s' if count != 1 else ''}") + audit = report["audit"] + if audit["stale"]: + if audit["last_checked"] is None: + bits.append("no audit date") + else: + bits.append(f"audit date {audit['age_days']}d old") + return ", ".join(bits) if bits else "no drift" + + +def issue_title(report): + return f"{STICKY_TITLE_PREFIX} {summary_line(report)}" + + +def _fenced(text): + """Fence `text` with enough backticks to survive fences inside it.""" + longest = max((len(m) for m in re.findall(r"`+", text)), default=0) + fence = "`" * max(3, longest + 1) + return f"{fence}text\n{text}\n{fence}" + + +def render_body(report, catalog_text, run_url=None, checked_at=None): + """Render the sticky issue body (also used as the run's step summary).""" + pins = report["pins"] + audit = report["audit"] + out = [] + + if report["has_findings"]: + out.append( + "Drift detected between the cursor-review panel pins and Cursor's live model " + "catalog. Pins are read from `.github/workflows/cursor-review.yml` (the preflight " + "`Define panel models` step + the `judge_model` input default) — update them there, " + "not here." + ) + else: + out.append( + "No drift: every pinned model id is present in Cursor's live catalog, no unpinned " + "same-lab ids are available, and the audit date is current." + ) + + meta = [] + if checked_at: + meta.append(f"Checked **{checked_at}**") + if run_url: + meta.append(f"[run]({run_url})") + meta.append("panel pins: " + ", ".join(f"`{m}`" for m in pins["panel"])) + meta.append(f"judge pin: `{pins['judge']}`") + out.append(" · ".join(meta)) + + if report["delisted"]: + out.append( + f"## 🚨 Delisted pin{'s' if len(report['delisted']) != 1 else ''} — fix first\n\n" + "These pinned ids are **no longer in the catalog**. Every consumer PR that triggers " + "cursor-review will fail the preflight job until the pin is updated." + ) + for item in report["delisted"]: + roles = "/".join(item["roles"]) or "pin" + same_lab = ( + ", ".join(f"`{m}`" for m in item["same_lab_available"]) + if item["same_lab_available"] + else "_(no same-lab id in the catalog)_" + ) + out.append(f"- `{item['id']}` ({roles}) — available for lab `{item['lab']}`: {same_lab}") + + if report["unpinned"]: + out.append( + "## Unpinned same-lab catalog ids — review me\n\n" + "Catalog ids from labs the panel already pins that are **not** pinned today. This is a " + "**review-me list, not a recommendation**: picking the newest highest-reasoning " + "*ZDR-eligible* model is a human call. Cursor only marks NO-ZDR models inline, so any " + "marker on the catalog line is reproduced verbatim below — an id with no marker is " + "**not** thereby confirmed ZDR-eligible; check the catalog before promoting one." + ) + for group in report["unpinned"]: + pinned_now = ", ".join(f"`{m}`" for m in group["pinned"]) or "_none_" + out.append(f"**`{group['lab']}`** (pinned: {pinned_now})") + out.append( + "\n".join( + f"- `{c['id']}`" + (f" — `{c['note']}`" if c["note"] else "") + for c in group["candidates"] + ) + ) + + if audit["stale"]: + if audit["last_checked"] is None: + out.append( + "## Stale audit date\n\n" + "No `last checked YYYY-MM-DD` comment was found above the panel pins in " + "`cursor-review.yml`. That comment is the human-audit record — restore it when " + "you next review the pins." + ) + else: + out.append( + f"## Stale audit date\n\n" + f"The pins were last human-audited on **{audit['last_checked']}** " + f"({audit['age_days']} days ago, threshold {audit['stale_days']}). Re-audit the " + f"pins and refresh the `last checked` comment in `cursor-review.yml`." + ) + + catalog = catalog_text.rstrip("\n") + if len(catalog) > MAX_CATALOG_CHARS: + catalog = catalog[:MAX_CATALOG_CHARS] + "\n… truncated — see the workflow run log for the full catalog." + out.append( + "
\nRaw cursor-agent models output\n\n" + + _fenced(catalog) + + "\n
" + ) + out.append( + "_Filed by the weekly `cursor-review-catalog-drift` check. This issue is sticky — it is " + "updated in place each run and closed automatically once a run finds no drift._" + ) + body = "\n\n".join(out) + "\n" + if len(body) > MAX_BODY_CHARS: + # Last-resort clamp: GitHub rejects an oversized body outright (422), and + # a failed issue write would lose the whole report. A truncated report + # still names the delisted pins, which lead the body. + body = body[:MAX_BODY_CHARS] + "\n\n_… report truncated — see the workflow run log._\n" + return body + + +# -------------------------------------------------------------------------- +# CLI +# -------------------------------------------------------------------------- + + +def _write(path, text): + if not path: + return + with open(path, "w", encoding="utf-8") as handle: + handle.write(text) + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("--workflow", required=True, help="Path to cursor-review.yml.") + parser.add_argument("--catalog", required=True, help="Path to raw `cursor-agent models` output.") + parser.add_argument("--stale-days", type=int, default=DEFAULT_STALE_DAYS) + parser.add_argument("--now", default=None, help="ISO-8601 timestamp to treat as now (tests).") + parser.add_argument("--run-url", default=None) + parser.add_argument("--title-out", default=None) + parser.add_argument("--body-out", default=None) + parser.add_argument("--json-out", default=None) + args = parser.parse_args(argv) + + with open(args.workflow, encoding="utf-8") as handle: + workflow_text = handle.read() + with open(args.catalog, encoding="utf-8") as handle: + catalog_text = handle.read() + + if args.now: + now = datetime.datetime.fromisoformat(args.now) + else: + now = datetime.datetime.now(datetime.timezone.utc) + + try: + panel_models = extract_panel_models(workflow_text) + judge_model = extract_judge_model(workflow_text) + except ExtractionError as exc: + # Better red than silent: if the pins can't be read, the checker cannot + # say anything true about drift, and a green run would be a lie. + print(f"::error::Could not read the cursor-review pins from {args.workflow}: {exc}") + return 1 + + if not catalog_text.strip(): + print("::error::The Cursor catalog output is empty — nothing to compare the pins against.") + return 1 + if not catalog_entries(catalog_text): + # Non-empty but no id parsed = a garbled catalog (an error page, a + # format change), not a catalog where every pin happens to be delisted. + # Fail loudly instead of crying wolf about four delisted pins. + print( + "::error::No model ids could be parsed out of the Cursor catalog output — " + "the format may have changed. Raw output:" + ) + print(catalog_text) + return 1 + + report = analyze( + panel_models, + judge_model, + catalog_text, + extract_last_checked(workflow_text), + now.date(), + args.stale_days, + ) + body = render_body(report, catalog_text, args.run_url, now.strftime("%Y-%m-%d %H:%M UTC")) + + _write(args.title_out, issue_title(report) + "\n") + _write(args.body_out, body) + _write(args.json_out, json.dumps(report, indent=2) + "\n") + + github_output = os.environ.get("GITHUB_OUTPUT") + if github_output: + with open(github_output, "a", encoding="utf-8") as handle: + handle.write(f"has_findings={str(report['has_findings']).lower()}\n") + handle.write(f"urgent={str(report['urgent']).lower()}\n") + + print(f"Panel pins: {', '.join(panel_models)}") + print(f"Judge pin: {judge_model}") + print(f"Catalog ids parsed: {len(report['catalog_ids'])}") + print(f"Drift: {summary_line(report)}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/cursor-review/tests/test_catalog_drift.py b/.github/cursor-review/tests/test_catalog_drift.py new file mode 100644 index 0000000..98767d2 --- /dev/null +++ b/.github/cursor-review/tests/test_catalog_drift.py @@ -0,0 +1,360 @@ +#!/usr/bin/env python3 +"""Regression tests for catalog-drift.py. + +The weekly drift check (BE-4819) is only worth having if it is right about three +things, so these tests pin exactly those: + + * **extraction** — the pins are read OUT of cursor-review.yml (panel heredoc + + `judge_model` default). A duplicated list would rot; a silently-failed + extraction would report "no drift" forever, so extraction failure must be a + loud non-zero exit, and the extractors are also run against the REAL + workflow file in this repo so a refactor there can't quietly blind the check. + * **comparison** — delisted pins use the preflight's whole-token match (so + `kimi-k2.7` does not "match" inside `kimi-k2.75`), unpinned same-lab ids are + grouped by the labs the pins actually use, and the audit date goes stale at + the threshold, not before. + * **reporting** — NO-ZDR markers survive into the body verbatim, the raw + catalog is folded into a
block, and a clean run says so (that is + what closes the sticky issue). + +Run: python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py' +""" + +import datetime +import importlib.util +import io +import json +import os +import tempfile +import unittest +from contextlib import redirect_stdout + +# catalog-drift.py has a hyphen, so import it by path rather than `import`. +_MODULE_PATH = os.path.join(os.path.dirname(__file__), "..", "catalog-drift.py") +_spec = importlib.util.spec_from_file_location("catalog_drift", _MODULE_PATH) +cd = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(cd) + +_REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "..")) +_REAL_WORKFLOW = os.path.join(_REPO_ROOT, ".github", "workflows", "cursor-review.yml") + +WORKFLOW = """\ +on: + workflow_call: + inputs: + judge_model: + description: >- + Single judge model. + type: string + required: false + default: claude-opus-4-8-thinking-max + diff_size_cap: + type: number + default: 5000 + +jobs: + preflight: + steps: + - name: Define panel models + id: models + # Pinned to each lab's newest highest-reasoning tier in Cursor's + # catalog (last checked 2026-07-14). + run: | + cat > /tmp/models.json <<'JSON' + [ + "gpt-5.6-sol-max", + "claude-opus-4-8-thinking-max", + "gemini-3.1-pro", + "kimi-k2.7-code" + ] + JSON + echo "models=$(jq -c . /tmp/models.json)" >> "$GITHUB_OUTPUT" +""" + +CATALOG = """\ +gpt-5.6-sol-max +gpt-5.6-sol +claude-opus-4-8-thinking-max +gemini-3.1-pro +kimi-k2.7-code +fable-5-max (NO ZDR) +""" + +PANEL = ["gpt-5.6-sol-max", "claude-opus-4-8-thinking-max", "gemini-3.1-pro", "kimi-k2.7-code"] +JUDGE = "claude-opus-4-8-thinking-max" +TODAY = datetime.date(2026, 7, 27) + + +def analyze(catalog=CATALOG, panel=None, judge=JUDGE, last_checked=datetime.date(2026, 7, 14), today=TODAY): + return cd.analyze( + list(PANEL) if panel is None else panel, judge, catalog, last_checked, today, cd.DEFAULT_STALE_DAYS + ) + + +class ExtractionTest(unittest.TestCase): + def test_extracts_panel_models_from_the_heredoc(self): + self.assertEqual(cd.extract_panel_models(WORKFLOW), PANEL) + + def test_extracts_judge_model_default(self): + self.assertEqual(cd.extract_judge_model(WORKFLOW), JUDGE) + + def test_judge_extraction_ignores_a_later_inputs_default(self): + # `diff_size_cap`'s default must never be mistaken for judge_model's. + self.assertNotEqual(cd.extract_judge_model(WORKFLOW), "5000") + + def test_extracts_last_checked_date(self): + self.assertEqual(cd.extract_last_checked(WORKFLOW), datetime.date(2026, 7, 14)) + + def test_missing_last_checked_is_none_not_an_error(self): + self.assertIsNone(cd.extract_last_checked("no audit comment here")) + + def test_heredoc_with_a_trailing_comma_falls_back_to_quoted_tokens(self): + text = WORKFLOW.replace('"kimi-k2.7-code"', '"kimi-k2.7-code",') + self.assertEqual(cd.extract_panel_models(text), PANEL) + + def test_missing_heredoc_raises(self): + with self.assertRaises(cd.ExtractionError): + cd.extract_panel_models("jobs:\n preflight:\n steps: []\n") + + def test_unterminated_heredoc_raises(self): + text = WORKFLOW.replace(" JSON\n", "") + with self.assertRaises(cd.ExtractionError): + cd.extract_panel_models(text) + + def test_judge_default_with_an_inline_comment_is_trimmed(self): + text = WORKFLOW.replace( + "default: claude-opus-4-8-thinking-max", + "default: claude-opus-4-8-thinking-max # top ZDR-eligible tier", + ) + self.assertEqual(cd.extract_judge_model(text), JUDGE) + + def test_folded_judge_default_raises_instead_of_pinning_a_scalar_marker(self): + # `>-` would sail through as a "pin" and report delisted every run. + text = WORKFLOW.replace( + " default: claude-opus-4-8-thinking-max\n", + " default: >-\n claude-opus-4-8-thinking-max\n", + ) + with self.assertRaises(cd.ExtractionError): + cd.extract_judge_model(text) + + def test_missing_judge_default_raises(self): + text = WORKFLOW.replace(" default: claude-opus-4-8-thinking-max\n", "") + with self.assertRaises(cd.ExtractionError): + cd.extract_judge_model(text) + + def test_extraction_works_against_the_real_cursor_review_workflow(self): + # The whole design rests on reading the pins out of the live workflow + # file; if a refactor there breaks these anchors, fail HERE (in a cheap + # unit run) rather than silently reporting "no drift" every Monday. + with open(_REAL_WORKFLOW, encoding="utf-8") as handle: + real = handle.read() + panel = cd.extract_panel_models(real) + self.assertTrue(panel, "no panel models extracted from the real workflow") + self.assertTrue(all(m and " " not in m for m in panel), panel) + judge = cd.extract_judge_model(real) + self.assertTrue(judge and " " not in judge, judge) + self.assertIsNotNone( + cd.extract_last_checked(real), + "the real workflow lost its `last checked YYYY-MM-DD` audit comment", + ) + + +class CatalogParsingTest(unittest.TestCase): + def test_parses_ids_and_notes(self): + entries = dict(cd.catalog_entries(CATALOG)) + self.assertIn("gpt-5.6-sol", entries) + self.assertEqual(entries["fable-5-max"], "(NO ZDR)") + + def test_skips_prose_and_bullets(self): + entries = cd.catalog_entries("Available models:\n\n - gpt-5.6-sol-max\n * kimi-k2.7-code\n") + self.assertEqual([m for m, _ in entries], ["gpt-5.6-sol-max", "kimi-k2.7-code"]) + + def test_dedupes_repeated_ids(self): + entries = cd.catalog_entries("gpt-5.6-sol\ngpt-5.6-sol (NO ZDR)\n") + self.assertEqual(entries, [("gpt-5.6-sol", "(NO ZDR)")]) + + def test_present_matches_whole_tokens_only(self): + self.assertTrue(cd.present("kimi-k2.7", "kimi-k2.7\n")) + self.assertFalse(cd.present("kimi-k2.7", "kimi-k2.75-code\n")) + self.assertFalse(cd.present("gpt-5.6-sol", "gpt-5.6-sol-max\n")) + + def test_lab_of_splits_on_the_first_separator(self): + self.assertEqual(cd.lab_of("gpt-5.6-sol-max"), "gpt") + self.assertEqual(cd.lab_of("kimi-k2.7-code"), "kimi") + + +class AnalyzeTest(unittest.TestCase): + def test_clean_catalog_with_no_extras_has_no_findings(self): + catalog = "\n".join(PANEL) + "\n" + report = analyze(catalog=catalog) + self.assertFalse(report["has_findings"]) + self.assertFalse(report["urgent"]) + self.assertEqual(cd.summary_line(report), "no drift") + + def test_delisted_pin_is_urgent_and_names_same_lab_alternatives(self): + catalog = CATALOG.replace("kimi-k2.7-code\n", "kimi-k3-code\n") + report = analyze(catalog=catalog) + self.assertTrue(report["urgent"]) + delisted = report["delisted"] + self.assertEqual([d["id"] for d in delisted], ["kimi-k2.7-code"]) + self.assertEqual(delisted[0]["roles"], ["panel"]) + self.assertEqual(delisted[0]["same_lab_available"], ["kimi-k3-code"]) + + def test_delisted_judge_pin_is_reported_with_its_role(self): + catalog = "gpt-5.6-sol-max\ngemini-3.1-pro\nkimi-k2.7-code\n" + report = analyze(catalog=catalog) + self.assertEqual([d["id"] for d in report["delisted"]], [JUDGE]) + # The judge default is also a panel pin here, so both roles show. + self.assertEqual(report["delisted"][0]["roles"], ["panel", "judge"]) + + def test_judge_pin_outside_the_panel_is_still_checked(self): + report = analyze(judge="claude-judge-only") + self.assertEqual([d["id"] for d in report["delisted"]], ["claude-judge-only"]) + self.assertEqual(report["delisted"][0]["roles"], ["judge"]) + + def test_unpinned_same_lab_ids_are_grouped_and_listed(self): + report = analyze() + groups = {g["lab"]: g for g in report["unpinned"]} + self.assertEqual([c["id"] for c in groups["gpt"]["candidates"]], ["gpt-5.6-sol"]) + self.assertTrue(report["has_findings"]) + self.assertFalse(report["urgent"]) + + def test_ids_from_unpinned_labs_are_not_listed_as_candidates(self): + # `fable-*` is a lab the panel does not pin — it belongs in the raw + # catalog fold, not in the review-me list. + report = analyze() + self.assertNotIn("fable", [g["lab"] for g in report["unpinned"]]) + + def test_a_newly_shipped_same_lab_model_shows_up(self): + report = analyze(catalog=CATALOG + "claude-opus-5-thinking-max\n") + claude = [g for g in report["unpinned"] if g["lab"] == "claude"][0] + self.assertEqual([c["id"] for c in claude["candidates"]], ["claude-opus-5-thinking-max"]) + + def test_audit_date_is_stale_only_past_the_threshold(self): + catalog = "\n".join(PANEL) + "\n" + fresh = analyze(catalog=catalog, last_checked=TODAY - datetime.timedelta(days=30)) + self.assertFalse(fresh["audit"]["stale"]) + self.assertFalse(fresh["has_findings"]) + stale = analyze(catalog=catalog, last_checked=TODAY - datetime.timedelta(days=31)) + self.assertTrue(stale["audit"]["stale"]) + self.assertTrue(stale["has_findings"]) + self.assertFalse(stale["urgent"]) + + def test_missing_audit_date_counts_as_stale(self): + report = analyze(catalog="\n".join(PANEL) + "\n", last_checked=None) + self.assertTrue(report["audit"]["stale"]) + self.assertIn("no audit date", cd.summary_line(report)) + + +class RenderTest(unittest.TestCase): + def test_title_carries_the_sticky_prefix(self): + title = cd.issue_title(analyze()) + self.assertTrue(title.startswith(cd.STICKY_TITLE_PREFIX), title) + + def test_body_reports_delisted_pins_prominently(self): + catalog = CATALOG.replace("kimi-k2.7-code\n", "kimi-k3-code\n") + body = cd.render_body(analyze(catalog=catalog), catalog, "https://example/run") + self.assertIn("Delisted pin", body) + self.assertIn("`kimi-k2.7-code`", body) + self.assertIn("`kimi-k3-code`", body) + + def test_body_surfaces_no_zdr_markers_verbatim(self): + catalog = CATALOG + "gpt-5.7-preview (NO ZDR)\n" + body = cd.render_body(analyze(catalog=catalog), catalog) + self.assertIn("gpt-5.7-preview", body) + self.assertIn("(NO ZDR)", body) + self.assertIn("review-me list, not a recommendation", body) + + def test_body_folds_the_raw_catalog_into_details(self): + body = cd.render_body(analyze(), CATALOG) + self.assertIn("
", body) + self.assertIn("fable-5-max (NO ZDR)", body) + + def test_body_fence_survives_backticks_in_the_catalog(self): + catalog = CATALOG + "```\n" + body = cd.render_body(analyze(catalog=catalog), catalog) + self.assertIn("````text", body) + + def test_oversized_catalog_is_truncated(self): + catalog = CATALOG + ("gpt-filler-x\n" * 6000) + body = cd.render_body(analyze(catalog=catalog), catalog) + self.assertIn("truncated", body) + self.assertLess(len(body), 65536) + + def test_clean_body_says_no_drift(self): + catalog = "\n".join(PANEL) + "\n" + body = cd.render_body(analyze(catalog=catalog), catalog) + self.assertIn("No drift", body) + + +class MainTest(unittest.TestCase): + def _run(self, workflow_text, catalog_text, extra=None): + tmp = tempfile.mkdtemp() + wf = os.path.join(tmp, "cursor-review.yml") + cat = os.path.join(tmp, "catalog.txt") + with open(wf, "w", encoding="utf-8") as handle: + handle.write(workflow_text) + with open(cat, "w", encoding="utf-8") as handle: + handle.write(catalog_text) + argv = [ + "--workflow", wf, + "--catalog", cat, + "--now", "2026-07-27T06:17:00", + "--title-out", os.path.join(tmp, "title.txt"), + "--body-out", os.path.join(tmp, "body.md"), + "--json-out", os.path.join(tmp, "report.json"), + ] + (extra or []) + buf = io.StringIO() + with redirect_stdout(buf): + code = cd.main(argv) + return code, tmp, buf.getvalue() + + def test_end_to_end_reports_findings_and_writes_outputs(self): + code, tmp, _ = self._run(WORKFLOW, CATALOG) + self.assertEqual(code, 0) + with open(os.path.join(tmp, "report.json"), encoding="utf-8") as handle: + report = json.load(handle) + self.assertTrue(report["has_findings"]) + self.assertFalse(report["urgent"]) + with open(os.path.join(tmp, "title.txt"), encoding="utf-8") as handle: + self.assertTrue(handle.read().startswith(cd.STICKY_TITLE_PREFIX)) + self.assertTrue(os.path.getsize(os.path.join(tmp, "body.md")) > 0) + + def test_unreadable_pins_exit_non_zero_instead_of_reporting_clean(self): + code, _, out = self._run("nothing useful here\n", CATALOG) + self.assertEqual(code, 1) + self.assertIn("::error::", out) + + def test_empty_catalog_exits_non_zero(self): + code, _, out = self._run(WORKFLOW, "\n") + self.assertEqual(code, 1) + self.assertIn("::error::", out) + + def test_unparseable_catalog_exits_non_zero_instead_of_crying_wolf(self): + # `cursor-agent models` exits 1 (and prints nothing to stdout) when it + # can't authenticate, so the workflow's own guard catches that case — + # this is the belt to that suspenders: a catalog that IS non-empty but + # yields no ids must not be reported as "every pin is delisted". + code, _, out = self._run(WORKFLOW, "Error: Authentication required.\n") + self.assertEqual(code, 1) + self.assertIn("::error::", out) + self.assertNotIn("delisted", out.lower()) + + def test_github_output_receives_the_flags(self): + tmp = tempfile.mkdtemp() + out_file = os.path.join(tmp, "gh_output") + os.environ["GITHUB_OUTPUT"] = out_file + try: + catalog = CATALOG.replace("kimi-k2.7-code\n", "kimi-k3-code\n") + code, _, _ = self._run(WORKFLOW, catalog) + finally: + del os.environ["GITHUB_OUTPUT"] + self.assertEqual(code, 0) + with open(out_file, encoding="utf-8") as handle: + written = handle.read() + self.assertIn("has_findings=true", written) + self.assertIn("urgent=true", written) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/workflows/cursor-review-catalog-drift.yml b/.github/workflows/cursor-review-catalog-drift.yml new file mode 100644 index 0000000..a43e7f1 --- /dev/null +++ b/.github/workflows/cursor-review-catalog-drift.yml @@ -0,0 +1,152 @@ +name: Cursor Review — catalog drift check + +# Weekly guard against the cursor-review panel pins going stale silently +# (BE-4819). The per-PR preflight in cursor-review.yml already fails loud when a +# PINNED id is delisted — but only at review time, on somebody's PR, after the +# pin has already gone bad; and by design it never notices a NEWER model +# shipping. The only guard against that second case was a `last checked ` +# comment above the pins, which went stale inside 13 days (the BE-4817 audit +# found two swaps — Opus 5 on 2026-07-24, Kimi K3 on ~2026-07-26 — that landed +# in that window and were caught by hand, not by CI). +# +# This run reads the pins OUT OF cursor-review.yml (never a duplicated list, so +# it stays zero-maintenance across pin bumps), diffs them against +# `cursor-agent models`, and reports three things through ONE sticky issue +# (title prefix `[cursor-review catalog drift]`, updated in place, closed +# automatically when a run finds nothing): +# +# * a delisted pin — urgent, consumer PRs are about to go red at preflight; +# * unpinned same-lab catalog ids — a REVIEW-ME list, not a recommendation +# (newest highest-reasoning ZDR-eligible is a human call); +# * a `last checked` audit date older than 30 days. +# +# The `last checked` comment stays the human-audit record — this check does not +# write it, it just tells you when it has gone stale. +# +# Not reusable on purpose: it reads THIS repo's cursor-review.yml, which is the +# single source of truth for every consumer's pins. + +on: + schedule: + # Mondays 06:17 UTC — off the hour, so it doesn't queue behind the + # top-of-hour scheduling spike. + - cron: '17 6 * * 1' + workflow_dispatch: + +permissions: + contents: read + issues: write + +# One drift run at a time: two overlapping runs would race on the sticky issue +# (both reading "no open issue" and filing a duplicate). Queue instead of +# cancelling so a manual dispatch during a scheduled run still reports. +concurrency: + group: cursor-review-catalog-drift + cancel-in-progress: false + +jobs: + drift: + name: Compare pins against the live catalog + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Checkout + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + persist-credentials: false + + - name: Install Cursor agent CLI + env: + CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }} + run: | + set -euo pipefail + # Unlike the per-PR preflight there is no fork case here — a scheduled + # run in this repo either has the org secret or the check is broken. + # Fail rather than "skip cleanly", which would be a green run that + # checked nothing (the exact silent-staleness this workflow exists to + # end). + if [ -z "${CURSOR_API_KEY:-}" ]; then + echo "::error::CURSOR_API_KEY is not available to this workflow — the drift check cannot read the live catalog. Grant the org secret to this repo (it already backs ci-cursor-review.yml)." + exit 1 + fi + curl https://cursor.com/install -fsSL | bash + echo "$HOME/.cursor/bin" >> "$GITHUB_PATH" + + - name: Fetch the live Cursor catalog + env: + CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }} + run: | + set -euo pipefail + # Same "better red than silent" stance as the preflight: an auth or + # network failure here is a hard fail, not an assumed-clean run. + if ! cursor-agent models > /tmp/catalog.txt 2>/tmp/catalog-stderr.txt; then + echo "::error::'cursor-agent models' failed — cannot check the panel pins against the live catalog." + cat /tmp/catalog-stderr.txt + exit 1 + fi + echo "=== Cursor catalog ===" + cat /tmp/catalog.txt + + - name: Compare pins against the catalog + id: drift + env: + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -euo pipefail + python3 .github/cursor-review/catalog-drift.py \ + --workflow .github/workflows/cursor-review.yml \ + --catalog /tmp/catalog.txt \ + --run-url "$RUN_URL" \ + --title-out /tmp/drift-title.txt \ + --body-out /tmp/drift-body.md \ + --json-out /tmp/drift-report.json + cat /tmp/drift-body.md >> "$GITHUB_STEP_SUMMARY" + + - name: Open, update, or close the sticky drift issue + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + HAS_FINDINGS: ${{ steps.drift.outputs.has_findings }} + run: | + set -euo pipefail + PREFIX='[cursor-review catalog drift]' + + # Find-by-title (the sticky key), filtered on an EXACT prefix rather + # than `--search`, which is fuzzy AND eventually consistent — a search + # that hasn't indexed the issue we filed would file a duplicate. The + # generous limit is what keeps that promise as this repo accumulates + # open issues (groom files here too): the sticky ages out of a + # newest-first page long before it stops mattering. + numbers=$(gh issue list -R "$REPO" --state open --limit 500 --json number,title \ + | jq -r --arg p "$PREFIX" 'map(select(.title | startswith($p))) | .[].number') + existing=$(printf '%s\n' "$numbers" | head -n 1) + extra=$(printf '%s\n' "$numbers" | tail -n +2 | tr '\n' ' ' | sed 's/ *$//') + if [ -n "$extra" ]; then + echo "::warning::More than one open sticky drift issue found; updating #${existing} and leaving these for a human to close: ${extra}" + fi + + if [ "$HAS_FINDINGS" = "true" ]; then + title=$(cat /tmp/drift-title.txt) + if [ -n "$existing" ]; then + gh issue edit "$existing" -R "$REPO" --title "$title" --body-file /tmp/drift-body.md + echo "Updated sticky drift issue #${existing}." + else + gh issue create -R "$REPO" --title "$title" --body-file /tmp/drift-body.md + fi + elif [ -n "$existing" ]; then + # Clean run: close the sticky issue so it never lingers past the + # drift it reported. A later run opens a fresh one if drift returns. + gh issue close "$existing" -R "$REPO" --comment "$(cat /tmp/drift-body.md)" + echo "Closed sticky drift issue #${existing} — no drift." + else + echo "No drift and no open sticky issue — nothing to do." + fi + + - name: Fail the run on a delisted pin + # The sticky issue carries every finding; only the urgent one reddens + # the run, so the weekly check stays green while a review-me list waits + # for a human but shouts when consumer PRs are about to fail preflight. + if: steps.drift.outputs.urgent == 'true' + run: | + echo "::error::A pinned cursor-review model id is missing from Cursor's live catalog — consumer PRs will fail the review preflight until the pin is updated. See the '[cursor-review catalog drift]' issue." + exit 1 diff --git a/.github/workflows/test-cursor-review-scripts.yml b/.github/workflows/test-cursor-review-scripts.yml index fde5e6e..5b49805 100644 --- a/.github/workflows/test-cursor-review-scripts.yml +++ b/.github/workflows/test-cursor-review-scripts.yml @@ -4,16 +4,25 @@ name: Test cursor-review scripts # 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. +# +# cursor-review.yml itself is in the path filter because catalog-drift.py reads +# the model pins OUT of it (BE-4819) — the test suite asserts those anchors +# still parse, and that assertion is only worth anything if it runs when the +# file it parses changes. on: pull_request: paths: - '.github/cursor-review/**' + - '.github/workflows/cursor-review.yml' + - '.github/workflows/cursor-review-catalog-drift.yml' - '.github/workflows/test-cursor-review-scripts.yml' push: branches: [main] paths: - '.github/cursor-review/**' + - '.github/workflows/cursor-review.yml' + - '.github/workflows/cursor-review-catalog-drift.yml' - '.github/workflows/test-cursor-review-scripts.yml' permissions: diff --git a/AGENTS.md b/AGENTS.md index b27b481..573a95c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,7 +43,10 @@ tests — run the matching command above for whatever you touched. repo's own CI callers (`ci-*.yml`) and the `test-*.yml` script tests. - `.github/cursor-review/` — prompts + scripts behind `cursor-review.yml` (the multi-model review panel + judge). Single source of truth; loaded at run - time, never copied into consumers. Tests in `tests/`. + time, never copied into consumers. Also holds `catalog-drift.py`, the + comparison logic behind the weekly `cursor-review-catalog-drift.yml` check + (BE-4819) — it reads the pins *out of* `cursor-review.yml`, so never + duplicate the model list. Tests in `tests/`. - `.github/agents-md-integrity/` — `check_agents_md.py`, the checker behind `agents-md-integrity.yml` (enforces this AGENTS.md standard). Tests in `tests/`. - `.github/groom/` — briefs + building blocks behind the reusable **groom** diff --git a/README.md b/README.md index 17d83d0..f0f9a91 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,7 @@ This repo is **public** so any repo — public or private, inside or outside the | [`pr-size.yml`](.github/workflows/pr-size.yml) | PR-size cap — fails (or, in `mode: warn`, only reports) when a PR's net diff exceeds `max_lines` non-generated changed lines, keeping diffs reviewable. Excludes dependency lockfiles, `linguist-generated` files (read from the base ref, so a PR can't exempt itself), Go generated-code markers, and per-repo `extra_lockfiles` / `extra_generated_globs`. A `bypass_label` (default `oversized-ok`) waves through a legitimately large change; a sticky bot comment explains overages when `bot_app_id` + `BOT_APP_PRIVATE_KEY` are supplied (degrades to status + step summary without them). Counting logic + tests live in [`scripts/check-pr-size/`](scripts/check-pr-size). | | [`stale.yml`](.github/workflows/stale.yml) | Stale-PR sweeper (`actions/stale`) plus a Slack digest of what it touched. PRs inactive for N days are labeled `stale`; still-inactive PRs are closed. The digest header names the source repo so batches from different repos posted to the same channel are unambiguous. Thresholds, messages, exempt labels, and the Slack channel are inputs; the caller owns the schedule + dry-run toggle. The calling job needs `pull-requests: write` and `issues: write`. Optional `SLACK_BOT_TOKEN`. | | [`groom.yml`](.github/workflows/groom.yml) | Scheduled/dispatch org-wide **code-cleanup sweep** (finds only — no commits, no PRs, never merges). A read-only FINDER agent scans a clean default-branch checkout (whole-repo, not a diff) for high-value refactors; an INDEPENDENT VERIFIER agent (fresh session) re-checks each as CONFIRM/DOWNGRADE/REJECT with a stable dedup signature; survivors are deduped against a durable GitHub-issue-state ledger and filed as `groom`-labeled GitHub issues (security-adjacent ones get `groom-security` — investigate, don't auto-implement). Mirrors the cursor-review topology: briefs + ledger live in [`.github/groom/`](.github/groom) as the single source of truth. The finder/verifier/builder agent jobs invoke the Claude CLI directly and mint no GitHub token, so they need nothing beyond `contents: read`; filing runs in a separate job as the bot you configure via `bot_app_id` (Comfy: cloud-code-bot). `dry_run` reports what it would file without opening issues. Runs on a **daily base cron** with a runtime cadence gate: set repo Actions variable `GROOM_INTERVAL_DAYS` (default 7 = weekly) to retune how often a real run happens — weekly → every-3-days → daily — with no workflow-file edit; a tick within the interval no-ops before the finder (`workflow_dispatch` bypasses the interval gate, but the volume gate — when the caller leaves it on — still applies). The calling job must grant `contents: read` + `issues: write` + `pull-requests: read` + `actions: read` — the first three are declared by the `file` / `build_select` jobs (needed even with `bot_app_id` set), and the interval gate needs `actions: read` (reads run history for the last real run); GitHub rejects a shorter grant at startup. Requires `ANTHROPIC_API_KEY` (+ `BOT_APP_PRIVATE_KEY` when `bot_app_id` is set). **Opt-in auto-builder** (`builder: true`, BE-4003): the top `max_prs` (default 5) CONFIRMED, non-security findings become **review-gated PRs** (full CI + cursor-review, **never auto-merged**) instead of issues; a credential-free `build` job emits only a patch artifact and a separate `build_pr` job opens the PR as the bot, preserving the security boundary. The ledger's PR-state (open/merged/closed) stops a built finding being re-proposed. Requires `bot_app_id`. `max_prs` is typed **`string`**, not `number`, so a caller can forward its own `workflow_dispatch` input straight through (`max_prs: ${{ github.event.inputs.max_prs \|\| '1' }}`) and let an operator raise the ceiling for one manual run — no `fromJSON()` cast in the caller, and the parse/clamp (empty → default, non-numeric → 0 PRs + warning, never a failed run) happens once inside the reusable. | +| [`cursor-review-catalog-drift.yml`](.github/workflows/cursor-review-catalog-drift.yml) | **Internal to this repo — not `workflow_call`able.** Weekly (Mon 06:17 UTC) + `workflow_dispatch` drift check between the `cursor-review.yml` model pins and Cursor's live `cursor-agent models` catalog. Reads the pins *out of* `cursor-review.yml` (panel heredoc + `judge_model` default) rather than duplicating them, then reports: a **delisted pin** (urgent — consumer PRs are about to fail the review preflight; this also fails the run), **unpinned same-lab catalog ids** (a review-me list, never an auto-recommendation — "newest highest-reasoning ZDR-eligible" is a human call, and NO-ZDR markers are surfaced verbatim), and a **`last checked` audit date** older than 30 days. Findings land in one sticky issue (`[cursor-review catalog drift]`, updated in place, closed automatically on a clean run) with the raw catalog folded in. Comparison logic + tests live in [`.github/cursor-review/catalog-drift.py`](.github/cursor-review/catalog-drift.py). Requires `CURSOR_API_KEY`. | | [`agents-md-integrity.yml`](.github/workflows/agents-md-integrity.yml) | Enforces the Comfy `AGENTS.md` standard on the caller repo: a top-level `AGENTS.md` must exist and stay under a hard line ceiling (`max_lines`, default 200; warns over `warn_lines`, default 150), a `CLAUDE.md` (if present) must be a thin `@AGENTS.md` shim rather than a divergent copy, no legacy `.cursorrules` (gated `forbid_cursorrules`), every nested monorepo `AGENTS.md` needs a sibling `@AGENTS.md` shim and to be under the ceiling (gated `check_nested`), and `AGENTS.md` should have a CODEOWNERS DRI (`require_codeowners`, warn-only by default). Fails with a non-zero exit + GitHub annotations so it wires in as a required status check. The checker lives in [`.github/agents-md-integrity/`](.github/agents-md-integrity) (pin `workflows_ref` to the same ref as `uses:`); no secrets required. | ## Usage From bc8dead722c234906f26ad00b672b9f7ff6daa43 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Mon, 27 Jul 2026 19:32:16 -0700 Subject: [PATCH 2/3] fix(cursor-review): least-privilege the drift check + paginate the sticky lookup (BE-4819) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the CodeRabbit review on #87. * Split the sticky-issue write out of the job that runs Cursor's installer. `curl https://cursor.com/install | bash` is a mutable third-party artifact Cursor ships no checksummed/versioned build of, and it ran in the same job that held `issues: write` — so every later `gh` call was downstream of code Cursor can change at will. The `drift` job now holds only `contents: read` and hands its rendered report to a new credential-free-of-installers `sticky` job via an artifact; `issues: write` exists in exactly one job that runs no third-party code. Same boundary groom.yml draws around its agent jobs, and it drops the workflow-level `permissions:` block that granted the write scope to every job (zizmor's excess-permissions finding). * Paginate the sticky-issue lookup. `gh issue list --limit 500` silently truncates, and a sticky that falls off the end of that page gets filed a second time. Walk every open issue with `gh api --paginate`, filtering out PRs (`/issues` returns them; `gh issue list` did not). * `value[0] not in ">|"` instead of `not value[0] in ">|"` (Ruff E713). No behavior change to the drift report itself; the 78 script tests still pass. --- .github/cursor-review/catalog-drift.py | 2 +- .../workflows/cursor-review-catalog-drift.yml | 67 ++++++++++++++++--- 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/.github/cursor-review/catalog-drift.py b/.github/cursor-review/catalog-drift.py index 78c2569..e913d75 100644 --- a/.github/cursor-review/catalog-drift.py +++ b/.github/cursor-review/catalog-drift.py @@ -139,7 +139,7 @@ def extract_judge_model(workflow_text): # `>-`, which would report as delisted on every single run. value = re.split(r"\s+#", default.group(1).strip(), maxsplit=1)[0].strip() value = value.strip("\"'") - if value and not value[0] in ">|" and not re.search(r"\s", value): + if value and value[0] not in ">|" and not re.search(r"\s", value): return value raise ExtractionError( f"the `judge_model` input's default is not a bare model id: {value!r}" diff --git a/.github/workflows/cursor-review-catalog-drift.yml b/.github/workflows/cursor-review-catalog-drift.yml index a43e7f1..fb68548 100644 --- a/.github/workflows/cursor-review-catalog-drift.yml +++ b/.github/workflows/cursor-review-catalog-drift.yml @@ -33,9 +33,9 @@ on: - cron: '17 6 * * 1' workflow_dispatch: -permissions: - contents: read - issues: write +# Permissions are declared PER JOB, not here: a workflow-level `issues: write` +# would hand the write scope to the job that runs Cursor's installer too (see +# the `drift` job comment). # One drift run at a time: two overlapping runs would race on the sticky issue # (both reading "no open issue" and filing a duplicate). Queue instead of @@ -49,6 +49,20 @@ jobs: name: Compare pins against the live catalog runs-on: ubuntu-latest timeout-minutes: 15 + # NO write scope, deliberately — the same boundary groom.yml draws around + # its agent jobs. This job pipes a MUTABLE third-party installer + # (`curl https://cursor.com/install | bash`) into the shell and puts + # `~/.cursor/bin` on PATH, so every later `gh`/`python3` call in it is + # already downstream of code Cursor can change at will. Cursor publishes no + # versioned, checksummed artifact to pin, so the mitigation is to give this + # job nothing worth stealing a write with: it emits the rendered report as + # an artifact, and the credentialed `sticky` job below (which runs no + # third-party code) is the only place `issues: write` exists. + permissions: + contents: read + outputs: + has_findings: ${{ steps.drift.outputs.has_findings }} + urgent: ${{ steps.drift.outputs.urgent }} steps: - name: Checkout uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 @@ -102,23 +116,54 @@ jobs: --json-out /tmp/drift-report.json cat /tmp/drift-body.md >> "$GITHUB_STEP_SUMMARY" + - name: Upload the rendered drift report + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: cursor-review-catalog-drift + path: | + /tmp/drift-title.txt + /tmp/drift-body.md + /tmp/drift-report.json + if-no-files-found: error + retention-days: 7 + + sticky: + name: Open, update, or close the sticky drift issue + needs: drift + runs-on: ubuntu-latest + timeout-minutes: 10 + # The credentialed sink: it only reads the artifact `drift` rendered and + # talks to the issues API. No checkout, no installer, no `cursor-agent`. + permissions: + contents: read + issues: write + steps: + - name: Download the rendered drift report + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + with: + name: cursor-review-catalog-drift + path: /tmp + - name: Open, update, or close the sticky drift issue env: GH_TOKEN: ${{ github.token }} REPO: ${{ github.repository }} - HAS_FINDINGS: ${{ steps.drift.outputs.has_findings }} + HAS_FINDINGS: ${{ needs.drift.outputs.has_findings }} run: | set -euo pipefail PREFIX='[cursor-review catalog drift]' # Find-by-title (the sticky key), filtered on an EXACT prefix rather # than `--search`, which is fuzzy AND eventually consistent — a search - # that hasn't indexed the issue we filed would file a duplicate. The - # generous limit is what keeps that promise as this repo accumulates - # open issues (groom files here too): the sticky ages out of a - # newest-first page long before it stops mattering. - numbers=$(gh issue list -R "$REPO" --state open --limit 500 --json number,title \ - | jq -r --arg p "$PREFIX" 'map(select(.title | startswith($p))) | .[].number') + # that hasn't indexed the issue we filed would file a duplicate. Walk + # EVERY open issue (`--paginate`, not a `--limit` page) so the sticky + # can't fall off the end of the result set and get filed twice as this + # repo accumulates open issues (groom files here too). `/issues` + # returns PRs as well, hence the `.pull_request == null` filter that + # `gh issue list` applied for us. + numbers=$(gh api "repos/${REPO}/issues?state=open&per_page=100" --paginate \ + --jq '.[] | select(.pull_request == null) | {number, title}' \ + | jq -rs --arg p "$PREFIX" 'map(select(.title | startswith($p))) | .[].number') existing=$(printf '%s\n' "$numbers" | head -n 1) extra=$(printf '%s\n' "$numbers" | tail -n +2 | tr '\n' ' ' | sed 's/ *$//') if [ -n "$extra" ]; then @@ -146,7 +191,7 @@ jobs: # The sticky issue carries every finding; only the urgent one reddens # the run, so the weekly check stays green while a review-me list waits # for a human but shouts when consumer PRs are about to fail preflight. - if: steps.drift.outputs.urgent == 'true' + if: needs.drift.outputs.urgent == 'true' run: | echo "::error::A pinned cursor-review model id is missing from Cursor's live catalog — consumer PRs will fail the review preflight until the pin is updated. See the '[cursor-review catalog drift]' issue." exit 1 From 7bf109ba6e727bd460ec2d3dc01915c345f4bfae Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Mon, 27 Jul 2026 19:56:48 -0700 Subject: [PATCH 3/3] fix(cursor-review): close the drift check's silent-miss paths from the review panel (BE-4819) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the cursor-review panel findings on #87. * HIGH — `present()` scanned the RAW catalog text with a whole-token regex, so a delisted pin still mentioned anywhere in the output was treated as present and never reported. That is not an edge case: a delist normally ships WITH a note naming the id it replaces ("kimi-k3-code (replaces kimi-k2.7-code)"), so the urgent path would have stayed silent in exactly the scenario it exists for, while consumer PRs went red at preflight. Now compares against the PARSED catalog ids, the same source `unpinned` already used. The trade-off is deliberate: a garbled catalog over-reports (loud) instead of under-reporting. * HIGH — the installer step no longer receives `CURSOR_API_KEY`. The presence check moved to its own step, so the org secret is not in scope while an unpinned remote script runs. (Pinning the installer itself spans all four call sites in this repo — proposed as a follow-up.) * MEDIUM — a new finding class: a pinned model whose catalog line now carries a NO-ZDR marker. Nothing breaks, so nothing flagged it, and private review diffs kept flowing to a model that may retain them. This is the one marker the checker interprets rather than reproduces, and only for a pin already in service; it reddens the run like a delisted pin. * MEDIUM — the quoted-token fallback (and the strict-JSON path) now reject any entry that is not a bare model id, instead of adopting a quoted comment or a "TODO pick a fifth" placeholder as a pin. A guessed pin set means false delisted alarms every Monday, or a real pin dropped and never monitored. * MEDIUM — the raw catalog is never echoed unfenced. `cursor-agent` output is semi-trusted and this repo is public, so a line beginning `::` would be executed by the runner (`::add-mask::`, `::stop-commands::`). The shell echo is wrapped in a `::stop-commands::` fence; the script's unparseable-catalog dump prefixes every line with `| `. * MEDIUM — the sticky key is now label + title prefix, not the public title alone, so the bot cannot adopt and rewrite an issue anyone opened with that title. (Pagination landed in the previous commit.) * LOW — `last checked` is read only within 40 lines above the panel heredoc, so an unrelated match elsewhere can't shadow the real audit date; and a future-dated typo now counts as stale instead of reading as fresh and suppressing the alert until 30 days past that future date. * LOW — merge catalog notes on a repeated id instead of keeping the first, so a later `(NO ZDR)` on a duplicate line is not discarded. * NIT — catalog notes render in a length-sized, padded inline code span. They are unconstrained third-party text reproduced in a bot-authored issue in a PUBLIC repo; a bare single-backtick span let a note break out and inject markdown or an `@mention` that notifies real people. Also requires a model id to contain a letter, so a numbered catalog format parses as nothing (routing to the diagnostic "format may have changed" exit) rather than as the ids ['1.', '2.'] with every real pin reading as delisted. The `lab_of` prefix heuristic dropping a lab's rebranded family, and prose words admitted as catalog ids, are left as-is and proposed as a follow-up — both distort the review-me list, neither can produce a false green. 94 tests (was 78); actionlint + shellcheck clean. --- .github/cursor-review/README.md | 18 +- .github/cursor-review/catalog-drift.py | 226 +++++++++++++++--- .../cursor-review/tests/test_catalog_drift.py | 141 ++++++++++- .../workflows/cursor-review-catalog-drift.yml | 81 +++++-- README.md | 2 +- 5 files changed, 398 insertions(+), 70 deletions(-) diff --git a/.github/cursor-review/README.md b/.github/cursor-review/README.md index 2794e03..e32e7a7 100644 --- a/.github/cursor-review/README.md +++ b/.github/cursor-review/README.md @@ -81,14 +81,18 @@ fifth pin) — and the preflight validates them against Cursor's live catalog on every run, so a delisted pin fails loudly instead of silently running the panel a lab short. -Drift the preflight *can't* see — a **newer** model shipping — is now -machine-checked weekly by +Drift the preflight *can't* see — a **newer** model shipping, or a pin quietly +reclassified non-ZDR — is machine-checked weekly by [`cursor-review-catalog-drift.yml`](../workflows/cursor-review-catalog-drift.yml), which diffs the pins against `cursor-agent models` and files one sticky -`[cursor-review catalog drift]` issue listing delisted pins, unpinned same-lab -catalog ids, and an audit date older than 30 days. It reports; it never bumps a -pin — picking the newest highest-reasoning **ZDR-eligible** model stays a human -call (Cursor marks only NO-ZDR models inline, e.g. Fable's `(NO ZDR)` suffix). +`[cursor-review catalog drift]` issue listing delisted pins, **pins whose catalog +line now says NO-ZDR**, unpinned same-lab catalog ids, and an audit date older +than 30 days. It reports; it never bumps a pin — picking the newest +highest-reasoning **ZDR-eligible** model stays a human call (Cursor marks only +NO-ZDR models inline, e.g. Fable's `(NO ZDR)` suffix). That marker is the one +thing the checker interprets rather than merely reproducing, and only on a pin +already in service: cursor-review sends private diffs to every pinned model, so +a pin losing ZDR eligibility reddens the run exactly like a delisted one. The `last checked ` comment above the pins remains the **human**-audit record: refresh it when you re-audit, and the weekly check will tell you when it has gone stale. @@ -103,7 +107,7 @@ it has gone stale. | [`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. | | [`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. | -| [`catalog-drift.py`](catalog-drift.py) | Backs the weekly catalog-drift check. Extracts the pins from `cursor-review.yml`, diffs them against raw `cursor-agent models` output, and renders the sticky issue title + body (delisted pins, unpinned same-lab ids, stale audit date). Reports only — it never edits a pin. | +| [`catalog-drift.py`](catalog-drift.py) | Backs the weekly catalog-drift check. Extracts the pins from `cursor-review.yml`, diffs them against raw `cursor-agent models` output, and renders the sticky issue title + body (delisted pins, pins marked NO-ZDR, unpinned same-lab ids, stale audit date). Reports only — it never edits a pin. | | [`slack-notify.sh`](slack-notify.sh) | Sends the start/complete Slack DMs to the triggerer (no-ops without a token). | ## Adopt it in your repo diff --git a/.github/cursor-review/catalog-drift.py b/.github/cursor-review/catalog-drift.py index e913d75..278a2ca 100644 --- a/.github/cursor-review/catalog-drift.py +++ b/.github/cursor-review/catalog-drift.py @@ -10,10 +10,15 @@ landed in that window and were caught by hand, not by CI. This script is the machine half of that audit. Given the workflow file and the -raw output of `cursor-agent models`, it reports three kinds of drift: +raw output of `cursor-agent models`, it reports four kinds of drift: * **delisted pin** — a pinned id (panel or judge) is absent from the live catalog. Urgent: consumer PRs will start failing preflight. + * **pin marked NO-ZDR** — a pinned id is still listed, but its catalog line now + carries a NO-ZDR marker. Also urgent, for the opposite reason: nothing + breaks, and private review diffs quietly keep flowing to a model that may + retain them. This is the one marker the script interprets rather than merely + reproducing (see `_NO_ZDR`). * **unpinned same-lab ids** — catalog ids from a lab the panel already pins, which the panel does *not* pin. A REVIEW-ME list, never an auto-recommendation: picking "newest highest-reasoning ZDR-eligible" needs human judgment, and ZDR @@ -38,8 +43,8 @@ Exit code is 0 for both "drift" and "no drift" — findings are reported through the sticky issue, not the run status. A non-zero exit means the checker itself could not run (pins unreadable), which is a real defect in the checker or in the -workflow it parses. The caller decides separately whether a delisted pin should -also redden the run (it does; see the workflow's final step). +workflow it parses. The caller decides separately whether an urgent finding +should also redden the run (it does; see the workflow's final step). """ import argparse @@ -67,25 +72,58 @@ # prose lines ("Available models:") out of the parsed id list. _ID_TOKEN = re.compile(r"^[a-z0-9][a-z0-9._-]*$") _BULLET = re.compile(r"^[-*>•\s]+") +# The ONE catalog marker worth interpreting rather than merely reproducing: a +# PINNED model reclassified non-ZDR means private review diffs are flowing to a +# model that may retain them. Notes are otherwise passed through verbatim (see +# `catalog_entries`) precisely because promotion is a human call — but a +# confidentiality regression on a pin already in service is not a "review me". +_NO_ZDR = re.compile(r"no[\s_-]*zdr", re.IGNORECASE) +# How far above the `Define panel models` heredoc the `last checked` audit +# comment is allowed to sit. Anchoring the search here (rather than scanning the +# whole file) stops an unrelated `last checked` elsewhere in cursor-review.yml +# from shadowing the real pin-adjacent date; the real-workflow test guards the +# window from being too tight. +LAST_CHECKED_WINDOW = 40 class ExtractionError(Exception): """The pins could not be read out of the workflow file.""" +def _is_model_id(token): + """True for a bare catalog/model id — see `_ID_TOKEN`. + + The letter requirement keeps a numbered-list marker (`1.` in a hypothetical + `1. gpt-5.6-sol-max` catalog line) from parsing as an id. That matters more + than it looks: `present()` trusts this parse, so a catalog of ['1.', '2.'] + would look perfectly valid while reporting every real pin as delisted. + Parsing nothing instead routes it to `main`'s diagnostic "no ids could be + parsed — the format may have changed" hard exit. + """ + return ( + bool(_ID_TOKEN.match(token)) + and re.search(r"[-.]", token) is not None + and re.search(r"[a-z]", token) is not None + ) + + # -------------------------------------------------------------------------- # Extraction — read the pins out of cursor-review.yml (never duplicate them) # -------------------------------------------------------------------------- +def _heredoc_start(lines): + """Index of the `cat > /tmp/models.json <<'JSON'` line, or None.""" + for i, line in enumerate(lines): + if _HEREDOC_START.search(line): + return i + return None + + def extract_panel_models(workflow_text): """Return the panel model ids from the 'Define panel models' heredoc.""" lines = workflow_text.splitlines() - start = None - for i, line in enumerate(lines): - if _HEREDOC_START.search(line): - start = i - break + start = _heredoc_start(lines) if start is None: raise ExtractionError( "could not find the `cat > /tmp/models.json <<'JSON'` heredoc in the " @@ -115,6 +153,21 @@ def extract_panel_models(workflow_text): models = [m.strip() for m in parsed if m.strip()] if not models: raise ExtractionError("the /tmp/models.json heredoc contains no model ids") + # EVERY entry must look like a bare model id. A *guessed* pin set is worse + # than a red run: the quoted-token fallback would otherwise adopt a quoted + # comment, and even a strictly-valid JSON array can carry a placeholder + # ("TODO pick a fifth"). Either way the checker would report a bogus pin as + # delisted every Monday, or silently monitor the wrong set — the exact + # silent staleness this checker exists to end. + bad = [m for m in models if not _is_model_id(m)] + if bad: + raise ExtractionError( + f"the /tmp/models.json heredoc yielded entries that are not bare model ids: {bad!r} " + "(expected a single lowercase token containing a `-` or `.`, e.g. " + "`gpt-5.6-sol-max`). If a lab has shipped a separator-less id, relax `_is_model_id` " + "here AND in the catalog parser together — they must agree, or the pin will read as " + "delisted every run." + ) return models @@ -149,8 +202,19 @@ def extract_judge_model(workflow_text): def extract_last_checked(workflow_text): - """Return the `last checked YYYY-MM-DD` audit date, or None if absent.""" - match = _LAST_CHECKED.search(workflow_text) + """Return the pins' `last checked YYYY-MM-DD` audit date, or None if absent. + + Searched only in the `LAST_CHECKED_WINDOW` lines immediately above the panel + heredoc, so an unrelated `last checked` elsewhere in cursor-review.yml can't + shadow the real pin-adjacent date. None means "report it stale", which is the + safe direction: a missing audit record is itself a finding. + """ + lines = workflow_text.splitlines() + start = _heredoc_start(lines) + window = ( + lines[max(0, start - LAST_CHECKED_WINDOW) : start + 1] if start is not None else lines + ) + match = _LAST_CHECKED.search("\n".join(window)) if not match: return None try: @@ -180,22 +244,40 @@ def catalog_entries(catalog_text): continue parts = line.split(None, 1) model_id = parts[0] - if not _ID_TOKEN.match(model_id) or not re.search(r"[-.]", model_id): + if not _is_model_id(model_id): continue note = parts[1].strip() if len(parts) > 1 else "" if model_id in seen: - if note and not seen[model_id]: - seen[model_id] = note + # MERGE rather than keep-first: if the catalog lists an id twice and + # only the second line carries `(NO ZDR)`, keeping the first note + # would discard the one marker a human must see. + if note and note not in _split_notes(seen[model_id]): + seen[model_id] = f"{seen[model_id]} / {note}" if seen[model_id] else note continue seen[model_id] = note order.append(model_id) return [(m, seen[m]) for m in order] -def present(model_id, catalog_text): - """Whole-token containment check — the same regex the preflight uses.""" - pattern = r"(? stale_days + # A NEGATIVE age is a future-dated typo, not a fresh audit — without this it + # reads as current and suppresses the stale alert until `stale_days` past + # that future date. + audit["stale"] = ( + last_checked is None or audit["age_days"] > stale_days or audit["age_days"] < 0 + ) + audit["future_dated"] = audit["age_days"] is not None and audit["age_days"] < 0 return { "pins": {"panel": list(panel_models), "judge": judge_model}, "catalog_ids": [m for m, _ in entries], "delisted": delisted, + "zdr_risk": zdr_risk, "unpinned": unpinned, "audit": audit, - "urgent": bool(delisted), - "has_findings": bool(delisted or unpinned or audit["stale"]), + # `urgent` is what reddens the weekly run: a pin the preflight is about + # to reject, or a pin that is no longer ZDR-eligible. + "urgent": bool(delisted or zdr_risk), + "has_findings": bool(delisted or zdr_risk or unpinned or audit["stale"]), } @@ -281,6 +384,9 @@ def summary_line(report): delisted = report["delisted"] if delisted: bits.append(f"{len(delisted)} delisted pin{'s' if len(delisted) != 1 else ''}") + zdr = report.get("zdr_risk") or [] + if zdr: + bits.append(f"{len(zdr)} pin{'s' if len(zdr) != 1 else ''} marked NO-ZDR") count = sum(len(g["candidates"]) for g in report["unpinned"]) if count: bits.append(f"{count} unpinned same-lab id{'s' if count != 1 else ''}") @@ -304,6 +410,22 @@ def _fenced(text): return f"{fence}text\n{text}\n{fence}" +def _inline_code(text): + """Render `text` as an inline code span that its own backticks can't escape. + + Catalog notes are unconstrained third-party free text reproduced verbatim in + a bot-authored issue in a PUBLIC repo. A bare single-backtick span would let + a note carrying a backtick break out and inject markdown — or an `@mention` + that notifies real people — so the delimiter is sized to the note (per + CommonMark) and padded when the note starts or ends with a backtick. + """ + flat = re.sub(r"\s+", " ", text).strip() + longest = max((len(m) for m in re.findall(r"`+", flat)), default=0) + delim = "`" * (longest + 1) + pad = " " if flat.startswith("`") or flat.endswith("`") else "" + return f"{delim}{pad}{flat}{pad}{delim}" + + def render_body(report, catalog_text, run_url=None, checked_at=None): """Render the sticky issue body (also used as the run's step summary).""" pins = report["pins"] @@ -347,6 +469,18 @@ def render_body(report, catalog_text, run_url=None, checked_at=None): ) out.append(f"- `{item['id']}` ({roles}) — available for lab `{item['lab']}`: {same_lab}") + if report.get("zdr_risk"): + out.append( + "## 🚨 Pinned model marked NO-ZDR — fix first\n\n" + "These ids are still in the catalog but its line now carries a NO-ZDR marker. " + "cursor-review sends **private diffs** to every pinned model, so a pin that is no " + "longer zero-data-retention eligible is a confidentiality regression: confirm against " + "the catalog and repin to a ZDR-eligible tier." + ) + for item in report["zdr_risk"]: + roles = "/".join(item["roles"]) or "pin" + out.append(f"- `{item['id']}` ({roles}) — catalog note: {_inline_code(item['note'])}") + if report["unpinned"]: out.append( "## Unpinned same-lab catalog ids — review me\n\n" @@ -361,7 +495,7 @@ def render_body(report, catalog_text, run_url=None, checked_at=None): out.append(f"**`{group['lab']}`** (pinned: {pinned_now})") out.append( "\n".join( - f"- `{c['id']}`" + (f" — `{c['note']}`" if c["note"] else "") + f"- `{c['id']}`" + (f" — {_inline_code(c['note'])}" if c["note"] else "") for c in group["candidates"] ) ) @@ -374,6 +508,15 @@ def render_body(report, catalog_text, run_url=None, checked_at=None): "`cursor-review.yml`. That comment is the human-audit record — restore it when " "you next review the pins." ) + elif audit.get("future_dated"): + out.append( + f"## Stale audit date — future-dated\n\n" + f"The `last checked` comment in `cursor-review.yml` reads " + f"**{audit['last_checked']}**, which is in the future ({-audit['age_days']} days " + f"from now) — almost certainly a typo. It is treated as stale rather than fresh, " + f"since a future date would otherwise suppress this alert. Fix the date and " + f"re-audit the pins." + ) else: out.append( f"## Stale audit date\n\n" @@ -408,6 +551,17 @@ def render_body(report, catalog_text, run_url=None, checked_at=None): # -------------------------------------------------------------------------- +def _quoted_for_log(text): + """Prefix every line so none can be read as a GitHub workflow command. + + `cursor-agent` output is semi-trusted third-party text, and this runs in a + PUBLIC repo's Actions log. A line beginning `::` would be executed by the + runner (`::add-mask::`, `::stop-commands::`, `::error::`), so the catalog is + never echoed raw. + """ + return "\n".join("| " + line for line in text.splitlines()) + + def _write(path, text): if not path: return @@ -455,9 +609,9 @@ def main(argv=None): # Fail loudly instead of crying wolf about four delisted pins. print( "::error::No model ids could be parsed out of the Cursor catalog output — " - "the format may have changed. Raw output:" + "the format may have changed. Raw output (each line prefixed `| `):" ) - print(catalog_text) + print(_quoted_for_log(catalog_text)) return 1 report = analyze( diff --git a/.github/cursor-review/tests/test_catalog_drift.py b/.github/cursor-review/tests/test_catalog_drift.py index 98767d2..2b2bc53 100644 --- a/.github/cursor-review/tests/test_catalog_drift.py +++ b/.github/cursor-review/tests/test_catalog_drift.py @@ -108,10 +108,57 @@ def test_extracts_last_checked_date(self): def test_missing_last_checked_is_none_not_an_error(self): self.assertIsNone(cd.extract_last_checked("no audit comment here")) + def test_last_checked_is_read_near_the_pins_not_the_first_match_anywhere(self): + # An unrelated `last checked` elsewhere in the file must not shadow the + # real pin-adjacent audit date. + preamble = "# changelog: last checked 2020-01-01\n" + "#\n" * (cd.LAST_CHECKED_WINDOW + 5) + self.assertEqual(cd.extract_last_checked(preamble + WORKFLOW), datetime.date(2026, 7, 14)) + + def test_last_checked_far_from_the_pins_is_not_adopted(self): + text = WORKFLOW.replace( + " # catalog (last checked 2026-07-14).", + "\n".join(["# stray: last checked 2020-01-01"] + [""] * cd.LAST_CHECKED_WINDOW), + ) + self.assertIsNone(cd.extract_last_checked(text)) + def test_heredoc_with_a_trailing_comma_falls_back_to_quoted_tokens(self): text = WORKFLOW.replace('"kimi-k2.7-code"', '"kimi-k2.7-code",') self.assertEqual(cd.extract_panel_models(text), PANEL) + def test_quoted_token_fallback_rejects_a_quoted_non_id(self): + # The fallback must not adopt a quoted comment as a "pin": a guessed pin + # set means false delisted alarms every Monday, or a real pin silently + # dropped and then never monitored. + text = WORKFLOW.replace( + ' "kimi-k2.7-code"\n', + ' "kimi-k2.7-code", # see "the Kimi tier" in the catalog\n', + ) + with self.assertRaises(cd.ExtractionError): + cd.extract_panel_models(text) + + def test_pin_and_catalog_id_shape_rules_agree(self): + # The pin validator and the catalog parser MUST accept the same shapes: + # a pin the catalog parser would never emit reads as delisted every run. + # Both currently require a `-`/`.`, so a separator-less id (`o3`) is + # rejected at extraction — a loud checker-defect signal, not a false + # claim about the catalog. If that rule is ever relaxed, relax it in + # both places; this test is the tripwire. + for token in ["o3", "sonnet", "gpt5"]: + self.assertFalse(cd._is_model_id(token), token) + self.assertEqual(cd.catalog_entries(token + "\n"), []) + for token in ["gpt-5.6-sol-max", "kimi-k2.7-code", "gemini-3.1-pro"]: + self.assertTrue(cd._is_model_id(token), token) + self.assertEqual([m for m, _ in cd.catalog_entries(token + "\n")], [token]) + + def test_valid_json_with_a_placeholder_entry_raises(self): + # Strict JSON is not enough — a placeholder would be "checked" as a pin. + text = WORKFLOW.replace( + ' "kimi-k2.7-code"\n', + ' "kimi-k2.7-code",\n "TODO pick a fifth"\n', + ) + with self.assertRaises(cd.ExtractionError): + cd.extract_panel_models(text) + def test_missing_heredoc_raises(self): with self.assertRaises(cd.ExtractionError): cd.extract_panel_models("jobs:\n preflight:\n steps: []\n") @@ -165,6 +212,14 @@ def test_parses_ids_and_notes(self): self.assertIn("gpt-5.6-sol", entries) self.assertEqual(entries["fable-5-max"], "(NO ZDR)") + def test_a_numbered_list_marker_is_not_mistaken_for_an_id(self): + # `1.` satisfies "lowercase token with a separator", so without the + # letter requirement a numbered catalog format would parse as the ids + # ['1.', '2.'] — a catalog that looks valid while every real pin reads + # as delisted. Parsing nothing is the better failure: `main` turns it + # into the diagnostic "the format may have changed" hard exit. + self.assertEqual(cd.catalog_entries("1. gpt-5.6-sol-max\n2. kimi-k2.7-code\n"), []) + def test_skips_prose_and_bullets(self): entries = cd.catalog_entries("Available models:\n\n - gpt-5.6-sol-max\n * kimi-k2.7-code\n") self.assertEqual([m for m, _ in entries], ["gpt-5.6-sol-max", "kimi-k2.7-code"]) @@ -173,10 +228,21 @@ def test_dedupes_repeated_ids(self): entries = cd.catalog_entries("gpt-5.6-sol\ngpt-5.6-sol (NO ZDR)\n") self.assertEqual(entries, [("gpt-5.6-sol", "(NO ZDR)")]) - def test_present_matches_whole_tokens_only(self): - self.assertTrue(cd.present("kimi-k2.7", "kimi-k2.7\n")) - self.assertFalse(cd.present("kimi-k2.7", "kimi-k2.75-code\n")) - self.assertFalse(cd.present("gpt-5.6-sol", "gpt-5.6-sol-max\n")) + def test_a_later_no_zdr_note_on_a_repeated_id_is_not_discarded(self): + # Keep-first would drop the one marker a human must see. + entries = dict(cd.catalog_entries("gpt-5.6-sol 200k ctx\ngpt-5.6-sol (NO ZDR)\n")) + self.assertIn("(NO ZDR)", entries["gpt-5.6-sol"]) + self.assertIn("200k ctx", entries["gpt-5.6-sol"]) + + def test_repeated_identical_notes_are_not_duplicated(self): + entries = dict(cd.catalog_entries("gpt-5.6-sol (NO ZDR)\ngpt-5.6-sol (NO ZDR)\n")) + self.assertEqual(entries["gpt-5.6-sol"], "(NO ZDR)") + + def test_present_matches_parsed_ids_exactly(self): + ids = {m for m, _ in cd.catalog_entries(CATALOG)} + self.assertTrue(cd.present("kimi-k2.7-code", ids)) + self.assertFalse(cd.present("kimi-k2.7", ids)) + self.assertFalse(cd.present("gpt-5.6-sol-max-plus", ids)) def test_lab_of_splits_on_the_first_separator(self): self.assertEqual(cd.lab_of("gpt-5.6-sol-max"), "gpt") @@ -207,6 +273,36 @@ def test_delisted_judge_pin_is_reported_with_its_role(self): # The judge default is also a panel pin here, so both roles show. self.assertEqual(report["delisted"][0]["roles"], ["panel", "judge"]) + def test_delisted_pin_mentioned_only_in_a_successor_note_is_still_reported(self): + # The regression the raw-text scan had: a delist normally ships WITH a + # note naming the id it replaces, so scanning the text found the dead pin + # and reported "no drift" while consumer PRs went red at preflight. + catalog = CATALOG.replace( + "kimi-k2.7-code\n", "kimi-k3-code (replaces kimi-k2.7-code)\n" + ) + report = analyze(catalog=catalog) + self.assertTrue(report["urgent"]) + self.assertEqual([d["id"] for d in report["delisted"]], ["kimi-k2.7-code"]) + + def test_a_pinned_model_marked_no_zdr_is_urgent(self): + catalog = CATALOG.replace("gemini-3.1-pro\n", "gemini-3.1-pro (NO ZDR)\n") + report = analyze(catalog=catalog) + self.assertEqual([z["id"] for z in report["zdr_risk"]], ["gemini-3.1-pro"]) + self.assertEqual(report["zdr_risk"][0]["roles"], ["panel"]) + self.assertFalse(report["delisted"]) + self.assertTrue(report["urgent"]) + self.assertIn("NO-ZDR", cd.summary_line(report)) + + def test_an_unmarked_pin_is_not_a_zdr_risk(self): + catalog = "\n".join(PANEL) + "\n" + self.assertEqual(analyze(catalog=catalog)["zdr_risk"], []) + + def test_a_no_zdr_marker_on_an_unpinned_id_is_not_urgent(self): + # `fable-5-max (NO ZDR)` is in CATALOG but not pinned — it belongs in the + # raw fold, and must not redden the weekly run. + self.assertEqual(analyze()["zdr_risk"], []) + self.assertFalse(analyze()["urgent"]) + def test_judge_pin_outside_the_panel_is_still_checked(self): report = analyze(judge="claude-judge-only") self.assertEqual([d["id"] for d in report["delisted"]], ["claude-judge-only"]) @@ -240,6 +336,18 @@ def test_audit_date_is_stale_only_past_the_threshold(self): self.assertTrue(stale["has_findings"]) self.assertFalse(stale["urgent"]) + def test_a_future_dated_audit_comment_counts_as_stale_not_fresh(self): + # A typo'd future date yields a negative age, which read as "fresh" and + # suppressed the alert until ~stale_days past that future date. + report = analyze( + catalog="\n".join(PANEL) + "\n", last_checked=TODAY + datetime.timedelta(days=400) + ) + self.assertTrue(report["audit"]["stale"]) + self.assertTrue(report["audit"]["future_dated"]) + self.assertTrue(report["has_findings"]) + body = cd.render_body(report, "\n".join(PANEL) + "\n") + self.assertIn("future", body.lower()) + def test_missing_audit_date_counts_as_stale(self): report = analyze(catalog="\n".join(PANEL) + "\n", last_checked=None) self.assertTrue(report["audit"]["stale"]) @@ -281,6 +389,23 @@ def test_oversized_catalog_is_truncated(self): self.assertIn("truncated", body) self.assertLess(len(body), 65536) + def test_a_backtick_in_a_catalog_note_cannot_break_out_of_its_code_span(self): + # Notes are unconstrained third-party text reproduced in a bot-authored + # issue in a PUBLIC repo; a bare single-backtick span would let one inject + # markdown or an @mention that notifies real people. + catalog = CATALOG + "gpt-5.7-preview `@everyone` see docs\n" + body = cd.render_body(analyze(catalog=catalog), catalog) + # Two-backtick delimiter, padded because the note itself starts with one. + self.assertIn("`` `@everyone` see docs ``", body) + self.assertNotIn("- `gpt-5.7-preview` — `` `@everyone`` see", body) + + def test_a_pinned_no_zdr_marker_is_called_out_in_the_body(self): + catalog = CATALOG.replace("gemini-3.1-pro\n", "gemini-3.1-pro (NO ZDR)\n") + body = cd.render_body(analyze(catalog=catalog), catalog) + self.assertIn("NO-ZDR", body) + self.assertIn("`gemini-3.1-pro`", body) + self.assertIn("confidentiality regression", body) + def test_clean_body_says_no_drift(self): catalog = "\n".join(PANEL) + "\n" body = cd.render_body(analyze(catalog=catalog), catalog) @@ -340,6 +465,14 @@ def test_unparseable_catalog_exits_non_zero_instead_of_crying_wolf(self): self.assertIn("::error::", out) self.assertNotIn("delisted", out.lower()) + def test_an_unparseable_catalog_cannot_smuggle_a_workflow_command_into_the_log(self): + # `cursor-agent` output is semi-trusted and this repo is public: a line + # beginning `::` echoed raw would be EXECUTED by the runner. + code, _, out = self._run(WORKFLOW, "Error page\n::add-mask::hunter2\n") + self.assertEqual(code, 1) + self.assertNotIn("\n::add-mask::", out) + self.assertIn("| ::add-mask::hunter2", out) + def test_github_output_receives_the_flags(self): tmp = tempfile.mkdtemp() out_file = os.path.join(tmp, "gh_output") diff --git a/.github/workflows/cursor-review-catalog-drift.yml b/.github/workflows/cursor-review-catalog-drift.yml index fb68548..028c905 100644 --- a/.github/workflows/cursor-review-catalog-drift.yml +++ b/.github/workflows/cursor-review-catalog-drift.yml @@ -16,9 +16,12 @@ name: Cursor Review — catalog drift check # automatically when a run finds nothing): # # * a delisted pin — urgent, consumer PRs are about to go red at preflight; +# * a pin whose catalog line now says NO-ZDR — also urgent, and the quieter +# failure: nothing breaks, private review diffs just keep going to a model +# that may retain them; # * unpinned same-lab catalog ids — a REVIEW-ME list, not a recommendation # (newest highest-reasoning ZDR-eligible is a human call); -# * a `last checked` audit date older than 30 days. +# * a `last checked` audit date older than 30 days (or future-dated). # # The `last checked` comment stays the human-audit record — this check does not # write it, it just tells you when it has gone stale. @@ -69,20 +72,30 @@ jobs: with: persist-credentials: false - - name: Install Cursor agent CLI + - name: Check the Cursor credential is present + # Separate from the install step so the org secret is NOT in scope while + # a remote installer script runs (see the next step). Unlike the per-PR + # preflight there is no fork case here — a scheduled run in this repo + # either has the org secret or the check is broken. Fail rather than + # "skip cleanly", which would be a green run that checked nothing (the + # exact silent staleness this workflow exists to end). env: CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }} run: | set -euo pipefail - # Unlike the per-PR preflight there is no fork case here — a scheduled - # run in this repo either has the org secret or the check is broken. - # Fail rather than "skip cleanly", which would be a green run that - # checked nothing (the exact silent-staleness this workflow exists to - # end). if [ -z "${CURSOR_API_KEY:-}" ]; then echo "::error::CURSOR_API_KEY is not available to this workflow — the drift check cannot read the live catalog. Grant the org secret to this repo (it already backs ci-cursor-review.yml)." exit 1 fi + echo "CURSOR_API_KEY is present." + + - name: Install Cursor agent CLI + # NO `env:` on purpose — the installer needs no credential, so it does + # not get one. It is still an unpinned remote script piped into bash + # (Cursor publishes no checksummed release artifact); the containing job + # holds no write scope for that reason. + run: | + set -euo pipefail curl https://cursor.com/install -fsSL | bash echo "$HOME/.cursor/bin" >> "$GITHUB_PATH" @@ -91,15 +104,25 @@ jobs: CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }} run: | set -euo pipefail + # `cursor-agent` output is semi-trusted third-party text and this repo + # is public, so it is never echoed raw: a line beginning `::` would be + # executed by the runner as a workflow command (`::add-mask::`, + # `::stop-commands::`). Fence the echo with an unguessable token so the + # runner treats the whole block as plain text. + stop_token="catalog-$(openssl rand -hex 16)" # Same "better red than silent" stance as the preflight: an auth or # network failure here is a hard fail, not an assumed-clean run. if ! cursor-agent models > /tmp/catalog.txt 2>/tmp/catalog-stderr.txt; then echo "::error::'cursor-agent models' failed — cannot check the panel pins against the live catalog." + echo "::stop-commands::${stop_token}" cat /tmp/catalog-stderr.txt + echo "::${stop_token}::" exit 1 fi echo "=== Cursor catalog ===" + echo "::stop-commands::${stop_token}" cat /tmp/catalog.txt + echo "::${stop_token}::" - name: Compare pins against the catalog id: drift @@ -152,16 +175,27 @@ jobs: run: | set -euo pipefail PREFIX='[cursor-review catalog drift]' + LABEL='cursor-review-catalog-drift' - # Find-by-title (the sticky key), filtered on an EXACT prefix rather - # than `--search`, which is fuzzy AND eventually consistent — a search - # that hasn't indexed the issue we filed would file a duplicate. Walk - # EVERY open issue (`--paginate`, not a `--limit` page) so the sticky - # can't fall off the end of the result set and get filed twice as this - # repo accumulates open issues (groom files here too). `/issues` - # returns PRs as well, hence the `.pull_request == null` filter that - # `gh issue list` applied for us. - numbers=$(gh api "repos/${REPO}/issues?state=open&per_page=100" --paginate \ + # The sticky key is LABEL + exact title prefix, and neither half is + # optional: + # * the label, because a public title prefix alone is squattable — + # anyone can open "[cursor-review catalog drift] …" and the bot + # would adopt and rewrite their issue. Applying a label needs write + # access, so label+prefix means only a trusted actor can be adopted. + # * `--paginate` over EVERY matching open issue, not a `--limit` + # page: a sticky that fell off the end of a bounded newest-first + # page would be filed a second time, and a clean run could no + # longer close the original (groom files issues in this repo too). + # Not `--search`, which is fuzzy AND eventually consistent — a search + # that hasn't indexed the issue we just filed would file a duplicate. + # `/issues` returns PRs as well, hence the `.pull_request == null` + # filter that `gh issue list` applied for us. + if ! gh label create "$LABEL" -R "$REPO" --color BFD4F2 \ + --description 'Sticky report from the weekly cursor-review catalog-drift check'; then + echo "Label ${LABEL} already exists (or could not be created); continuing." + fi + numbers=$(gh api "repos/${REPO}/issues?state=open&labels=${LABEL}&per_page=100" --paginate \ --jq '.[] | select(.pull_request == null) | {number, title}' \ | jq -rs --arg p "$PREFIX" 'map(select(.title | startswith($p))) | .[].number') existing=$(printf '%s\n' "$numbers" | head -n 1) @@ -176,7 +210,8 @@ jobs: gh issue edit "$existing" -R "$REPO" --title "$title" --body-file /tmp/drift-body.md echo "Updated sticky drift issue #${existing}." else - gh issue create -R "$REPO" --title "$title" --body-file /tmp/drift-body.md + gh issue create -R "$REPO" --title "$title" --label "$LABEL" \ + --body-file /tmp/drift-body.md fi elif [ -n "$existing" ]; then # Clean run: close the sticky issue so it never lingers past the @@ -187,11 +222,13 @@ jobs: echo "No drift and no open sticky issue — nothing to do." fi - - name: Fail the run on a delisted pin - # The sticky issue carries every finding; only the urgent one reddens - # the run, so the weekly check stays green while a review-me list waits - # for a human but shouts when consumer PRs are about to fail preflight. + - name: Fail the run on an urgent finding + # The sticky issue carries every finding; only the urgent ones redden the + # run, so the weekly check stays green while a review-me list waits for a + # human but shouts when a pin is actively broken — either delisted + # (consumer PRs are about to fail preflight) or marked NO-ZDR (private + # diffs are flowing to a model that may retain them). if: needs.drift.outputs.urgent == 'true' run: | - echo "::error::A pinned cursor-review model id is missing from Cursor's live catalog — consumer PRs will fail the review preflight until the pin is updated. See the '[cursor-review catalog drift]' issue." + echo "::error::A pinned cursor-review model is either missing from Cursor's live catalog (consumer PRs will fail the review preflight) or now marked NO-ZDR (private diffs are going to a non-ZDR model). See the '[cursor-review catalog drift]' issue." exit 1 diff --git a/README.md b/README.md index f0f9a91..c8165d8 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ This repo is **public** so any repo — public or private, inside or outside the | [`pr-size.yml`](.github/workflows/pr-size.yml) | PR-size cap — fails (or, in `mode: warn`, only reports) when a PR's net diff exceeds `max_lines` non-generated changed lines, keeping diffs reviewable. Excludes dependency lockfiles, `linguist-generated` files (read from the base ref, so a PR can't exempt itself), Go generated-code markers, and per-repo `extra_lockfiles` / `extra_generated_globs`. A `bypass_label` (default `oversized-ok`) waves through a legitimately large change; a sticky bot comment explains overages when `bot_app_id` + `BOT_APP_PRIVATE_KEY` are supplied (degrades to status + step summary without them). Counting logic + tests live in [`scripts/check-pr-size/`](scripts/check-pr-size). | | [`stale.yml`](.github/workflows/stale.yml) | Stale-PR sweeper (`actions/stale`) plus a Slack digest of what it touched. PRs inactive for N days are labeled `stale`; still-inactive PRs are closed. The digest header names the source repo so batches from different repos posted to the same channel are unambiguous. Thresholds, messages, exempt labels, and the Slack channel are inputs; the caller owns the schedule + dry-run toggle. The calling job needs `pull-requests: write` and `issues: write`. Optional `SLACK_BOT_TOKEN`. | | [`groom.yml`](.github/workflows/groom.yml) | Scheduled/dispatch org-wide **code-cleanup sweep** (finds only — no commits, no PRs, never merges). A read-only FINDER agent scans a clean default-branch checkout (whole-repo, not a diff) for high-value refactors; an INDEPENDENT VERIFIER agent (fresh session) re-checks each as CONFIRM/DOWNGRADE/REJECT with a stable dedup signature; survivors are deduped against a durable GitHub-issue-state ledger and filed as `groom`-labeled GitHub issues (security-adjacent ones get `groom-security` — investigate, don't auto-implement). Mirrors the cursor-review topology: briefs + ledger live in [`.github/groom/`](.github/groom) as the single source of truth. The finder/verifier/builder agent jobs invoke the Claude CLI directly and mint no GitHub token, so they need nothing beyond `contents: read`; filing runs in a separate job as the bot you configure via `bot_app_id` (Comfy: cloud-code-bot). `dry_run` reports what it would file without opening issues. Runs on a **daily base cron** with a runtime cadence gate: set repo Actions variable `GROOM_INTERVAL_DAYS` (default 7 = weekly) to retune how often a real run happens — weekly → every-3-days → daily — with no workflow-file edit; a tick within the interval no-ops before the finder (`workflow_dispatch` bypasses the interval gate, but the volume gate — when the caller leaves it on — still applies). The calling job must grant `contents: read` + `issues: write` + `pull-requests: read` + `actions: read` — the first three are declared by the `file` / `build_select` jobs (needed even with `bot_app_id` set), and the interval gate needs `actions: read` (reads run history for the last real run); GitHub rejects a shorter grant at startup. Requires `ANTHROPIC_API_KEY` (+ `BOT_APP_PRIVATE_KEY` when `bot_app_id` is set). **Opt-in auto-builder** (`builder: true`, BE-4003): the top `max_prs` (default 5) CONFIRMED, non-security findings become **review-gated PRs** (full CI + cursor-review, **never auto-merged**) instead of issues; a credential-free `build` job emits only a patch artifact and a separate `build_pr` job opens the PR as the bot, preserving the security boundary. The ledger's PR-state (open/merged/closed) stops a built finding being re-proposed. Requires `bot_app_id`. `max_prs` is typed **`string`**, not `number`, so a caller can forward its own `workflow_dispatch` input straight through (`max_prs: ${{ github.event.inputs.max_prs \|\| '1' }}`) and let an operator raise the ceiling for one manual run — no `fromJSON()` cast in the caller, and the parse/clamp (empty → default, non-numeric → 0 PRs + warning, never a failed run) happens once inside the reusable. | -| [`cursor-review-catalog-drift.yml`](.github/workflows/cursor-review-catalog-drift.yml) | **Internal to this repo — not `workflow_call`able.** Weekly (Mon 06:17 UTC) + `workflow_dispatch` drift check between the `cursor-review.yml` model pins and Cursor's live `cursor-agent models` catalog. Reads the pins *out of* `cursor-review.yml` (panel heredoc + `judge_model` default) rather than duplicating them, then reports: a **delisted pin** (urgent — consumer PRs are about to fail the review preflight; this also fails the run), **unpinned same-lab catalog ids** (a review-me list, never an auto-recommendation — "newest highest-reasoning ZDR-eligible" is a human call, and NO-ZDR markers are surfaced verbatim), and a **`last checked` audit date** older than 30 days. Findings land in one sticky issue (`[cursor-review catalog drift]`, updated in place, closed automatically on a clean run) with the raw catalog folded in. Comparison logic + tests live in [`.github/cursor-review/catalog-drift.py`](.github/cursor-review/catalog-drift.py). Requires `CURSOR_API_KEY`. | +| [`cursor-review-catalog-drift.yml`](.github/workflows/cursor-review-catalog-drift.yml) | **Internal to this repo — not `workflow_call`able.** Weekly (Mon 06:17 UTC) + `workflow_dispatch` drift check between the `cursor-review.yml` model pins and Cursor's live `cursor-agent models` catalog. Reads the pins *out of* `cursor-review.yml` (panel heredoc + `judge_model` default) rather than duplicating them, then reports: a **delisted pin** (urgent — consumer PRs are about to fail the review preflight; this also fails the run), a **pin whose catalog line now says NO-ZDR** (also urgent, and the quieter failure — nothing breaks, private review diffs just keep flowing to a model that may retain them), **unpinned same-lab catalog ids** (a review-me list, never an auto-recommendation — "newest highest-reasoning ZDR-eligible" is a human call, and NO-ZDR markers are surfaced verbatim), and a **`last checked` audit date** older than 30 days (or future-dated). Findings land in one sticky issue (`[cursor-review catalog drift]`, label `cursor-review-catalog-drift`, updated in place, closed automatically on a clean run) with the raw catalog folded in. Least privilege by construction: the job that pipes Cursor's installer into bash holds only `contents: read` and hands its rendered report to a separate `issues: write` job via an artifact. Comparison logic + tests live in [`.github/cursor-review/catalog-drift.py`](.github/cursor-review/catalog-drift.py). Requires `CURSOR_API_KEY`. | | [`agents-md-integrity.yml`](.github/workflows/agents-md-integrity.yml) | Enforces the Comfy `AGENTS.md` standard on the caller repo: a top-level `AGENTS.md` must exist and stay under a hard line ceiling (`max_lines`, default 200; warns over `warn_lines`, default 150), a `CLAUDE.md` (if present) must be a thin `@AGENTS.md` shim rather than a divergent copy, no legacy `.cursorrules` (gated `forbid_cursorrules`), every nested monorepo `AGENTS.md` needs a sibling `@AGENTS.md` shim and to be under the ceiling (gated `check_nested`), and `AGENTS.md` should have a CODEOWNERS DRI (`require_codeowners`, warn-only by default). Fails with a non-zero exit + GitHub annotations so it wires in as a required status check. The checker lives in [`.github/agents-md-integrity/`](.github/agents-md-integrity) (pin `workflows_ref` to the same ref as `uses:`); no secrets required. | ## Usage