diff --git a/.github/cursor-review/README.md b/.github/cursor-review/README.md index 2120606..ff74b46 100644 --- a/.github/cursor-review/README.md +++ b/.github/cursor-review/README.md @@ -87,6 +87,7 @@ silently vanishing — the review tells you what didn't run. | [`gate-unresolved.py`](gate-unresolved.py) | **Orphaned — nothing calls this today.** Implemented the opt-in blocking gate: queries the PR's review threads and exits non-zero while any cursor-review finding thread is unresolved. The job that ran it was dropped from `cursor-review.yml` in #31; see [the regression note](#the-blocking-gate-is-currently-not-available-regressed). | | [`slack-notify.sh`](slack-notify.sh) | Sends the start/complete Slack DMs to the triggerer (no-ops without a token). | | [`install-cursor-cli.sh`](install-cursor-cli.sh) | Installs the Cursor agent CLI from the versioned, sha256-pinned release artifact — not `curl cursor.com/install \| bash`. Used by all three CLI-using jobs; the pin (`CURSOR_CLI_VERSION` / `CURSOR_CLI_SHA256`) lives in `cursor-review.yml`'s top-level `env:`. | +| [`build-ledger.py`](build-ledger.py) | Builds the **prior-review ledger** — what earlier rounds raised on this PR and how the author answered — and splices it into the panel/judge prompts. Also the prompt splicer, so the no-ledger path is byte-identical to the pre-ledger prompt. | ## Adopt it in your repo diff --git a/.github/cursor-review/build-ledger.py b/.github/cursor-review/build-ledger.py new file mode 100644 index 0000000..ae25c3c --- /dev/null +++ b/.github/cursor-review/build-ledger.py @@ -0,0 +1,794 @@ +#!/usr/bin/env python3 +"""Build the prior-review ledger — what earlier rounds already raised and how the +author answered — and splice it into the panel/judge prompts. + +Why this exists +--------------- +The review workflow is stateless between runs: every round re-derives the PR +from the full diff with no memory, so a finding that was refuted, declined, or +deliberately deferred in round 1 comes straight back in round 2 and 3, and the +author hand-writes the same rebuttal again. A string/embedding dedupe cannot fix +that — the judge rewrites each finding's prose every round, so a true repeat +pair scores *lower* on text similarity than two unrelated findings on the same +file. The prior context has to enter the prompt, so that is what this does. + +Two subcommands: + +``build`` + Fetch the PR's prior *consolidated* reviews, their per-finding thread roots, + the reply chains hanging off them, and each thread's ``isResolved`` / + ``isOutdated`` flags. Emit ``ledger.json`` (structured) plus ``ledger.md`` + and ``ledger-judge.md`` (the delimited, explicitly-untrusted blocks the panel + and the judge read). + +``splice`` + Insert a rendered block into a prompt immediately before its trailing + ``=== BEGIN … ===`` marker. With no block to insert the output is + **byte-identical** to the prompt file — that is the first-round + no-regression property, and ``test_build_ledger.py`` pins it. + +What a ledger entry deliberately does NOT carry +----------------------------------------------- +There is **no derived ``verdict`` / ``disposition`` field.** Real author replies +open with "Not taking this one", "Refuted — the premise doesn't hold", "Real, +but DEFERRED", "Half fixed, half accepted", "Already addressed (dup…)" — no +regex or keyword match survives that corpus, and a wrong label is worse than no +label because the judge would trust it. Entries carry the verbatim prose plus +the *checkable structural* flags (resolved / outdated / reply count / whether the +replier is the PR author) and let the judge read the rest. + +Prompt injection +---------------- +The ledger imports PR comment text into a reviewer prompt on a workflow whose +``consolidate`` job holds ``pull-requests: write``, so it is a new untrusted +channel. The workflow's ``gate`` job already skips fork PRs, so the surface is +same-repo PRs plus bot-authored text (Dependabot, cloud-code-bot) — and, on a +public repo, anything any reader can post to the PR. Four controls, in order of +how much they carry: + +1. **Provenance.** A prior round is only trusted from a Bot author + (``_consolidated_reviews``). The marker alone is public, so on a public PR + anyone could otherwise forge a round and write straight into this prompt. +2. **Fence integrity.** Every imported string is defanged so it cannot forge + the block's own delimiters (``_defang_fences``). The delimiters are what + make the block DATA; without this the rest is decoration. +3. **Labelling.** The block is delimited and labelled DATA, NOT INSTRUCTIONS, + and text trying to steer the review is to be disregarded AND reported. +4. **Steering.** A prior reply justifies dropping a finding only when it gives + a *checkable technical reason*, and only a reply from the PR author or a + maintainer counts as an answer at all. +""" + +import argparse +import importlib.util +import json +import os +import re +import subprocess +import sys + +# Size caps. Anything they drop is stated inside the text the model reads, so a +# partial ledger can never be mistaken for a complete one. +MAX_ROUNDS = 3 +MAX_BODY_CHARS = 600 +MAX_LEDGER_BYTES = 40 * 1024 +# Per-entry reply cap. Without it one hot thread can serialize past the byte cap +# on its own, and the byte cap would then have to drop the whole ledger to get +# under. The most RECENT replies are kept — they are the author's current +# position — and dropping any is disclosed on the entry itself. +MAX_REPLIES_PER_ENTRY = 8 +TRUNCATION_MARKER = " …[truncated]" + +# Max re-raises the judge may emit per review. Enforced deterministically in +# post-review.py; stated here because the judge prompt block quotes it. +REPEAT_CAP = 2 + + +def _load_gate_unresolved(): + """Import gate-unresolved.py by path (its name has a hyphen). + + Reused, not re-implemented: the ledger takes ``CONSOLIDATED_MARKER`` (the + canonical "is this our review?" discriminator, also used by the workflow's + dup-check) and the paging ``reviewThreads`` query from that one module. + """ + path = os.path.join(os.path.dirname(os.path.abspath(__file__)), "gate-unresolved.py") + spec = importlib.util.spec_from_file_location("gate_unresolved", path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +gate_unresolved = _load_gate_unresolved() +CONSOLIDATED_MARKER = gate_unresolved.CONSOLIDATED_MARKER + +# post-review.py renders every inline finding as " **