From eebcd8ac88bfac60f4095cf157e81798920c13a4 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Mon, 22 Jun 2026 11:13:50 -0700 Subject: [PATCH] Skip cursor-review on fork PRs; degrade to job summary on read-only token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fork PRs can't run this review: the pull_request event withholds secrets (CURSOR_API_KEY is empty, so every panel cell produces empty output) and grants a read-only GITHUB_TOKEN (posting the consolidated review returns HTTP 403 "Resource not accessible by integration"). The review can neither analyze nor post, yet it still ran the 8-cell matrix + judge and failed the check red on every external contribution (e.g. Comfy-Desktop#973). Two complementary fixes: 1. Gate: skip when head.repo.full_name != github.repository (the cross-repo signal). No wasted matrix, no red X — the check is simply skipped, the standard outcome for a workflow that needs secrets a fork PR can't have. 2. post-review.py: when a review POST fails with a read-only-token 403, render the review into the job step summary and exit 0 instead of failing. Defense-in-depth for same-repo runs that can still hit a read-only token (read-only default workflow permissions, token-downgrading events). Genuine non-permission failures (e.g. HTTP 422) still exit 1 — no silent swallow. Co-Authored-By: Claude Opus 4.8 --- .github/cursor-review/post-review.py | 117 ++++++++++++++++++++++----- .github/workflows/cursor-review.yml | 15 ++++ 2 files changed, 112 insertions(+), 20 deletions(-) diff --git a/.github/cursor-review/post-review.py b/.github/cursor-review/post-review.py index 48c6510..118a953 100644 --- a/.github/cursor-review/post-review.py +++ b/.github/cursor-review/post-review.py @@ -23,6 +23,7 @@ import argparse import json +import os import subprocess import sys @@ -109,6 +110,78 @@ def gh_post_review(repo: str, pr_number: str, payload: str) -> subprocess.Comple ) +def is_read_only_token_error(result: subprocess.CompletedProcess) -> bool: + """True when the POST failed because the token can't write to the PR. + + The gate skips fork PRs (which always hit this), but a read-only token can + still occur on same-repo runs — org/repo default workflow permissions set + to read-only, or events that downgrade the token. GitHub answers those with + HTTP 403 'Resource not accessible by integration'. That's an environment + constraint, not a review failure, so callers degrade to the job summary + rather than failing the check red. + """ + blob = result.stderr or "" + return "Resource not accessible by integration" in blob or "HTTP 403" in blob + + +def write_step_summary(markdown: str) -> None: + """Render the review into the Actions run summary as a no-write fallback. + + Used when the PR can't be written to (read-only token): the content is + still delivered — in the run's Summary tab — instead of being lost. + """ + note = ( + "> ℹ️ This review could not be posted on the PR because the run's " + "`GITHUB_TOKEN` is read-only (e.g. read-only default workflow " + "permissions). Posting it here instead.\n\n" + ) + path = os.environ.get("GITHUB_STEP_SUMMARY") + if not path: + # No summary file (e.g. a local run) — fall back to stdout so the + # content isn't silently dropped. + print(note + markdown) + return + with open(path, "a", encoding="utf-8") as f: + f.write(note + markdown + "\n") + + +def post_or_degrade(repo, pr_number, payload, summary_markdown, context) -> bool: + """POST a review; degrade to the step summary on a read-only token. + + Returns True when the review was delivered — either posted on the PR, or + (when the token is read-only) written to the job step summary. Returns + False only on a genuine POST failure the caller should handle itself + (e.g. retry without inline anchors). + """ + result = gh_post_review(repo, pr_number, payload) + if result.returncode == 0: + return True + if is_read_only_token_error(result): + print( + f"{context}: token is read-only — writing the review to the job " + "summary instead of the PR.", + file=sys.stderr, + ) + write_step_summary(summary_markdown) + return True + print(f"{context} POST failed: {result.stderr}", file=sys.stderr) + return False + + +def render_findings_markdown(review_body: str, comments: list[dict]) -> str: + """Flatten the review body + inline comments into one markdown blob. + + Inline review comments don't render in a step summary, so list them + underneath the body when degrading to the summary or a body-only review. + """ + md = review_body + if comments: + md += "\n\n---\n\n" + for c in comments: + md += f"**`{c['path']}:{c['line']}`** — {c['body']}\n\n" + return md + + def build_panel_summary(panel: list[dict]) -> str: if not panel: return "" @@ -170,19 +243,14 @@ def normalize_comments(findings: list[dict]) -> list[dict]: def post_error_review(repo, pr_number, commit_sha, header, error_message): safe = neutralize_mentions(error_message) + body_text = ( + f"{header}\n\n⚠️ **Review failed**\n\n```\n{safe}\n```\n\n" + "Re-trigger by removing and re-adding the `cursor-review` label." + ) payload = json.dumps( - { - "body": ( - f"{header}\n\n⚠️ **Review failed**\n\n```\n{safe}\n```\n\n" - "Re-trigger by removing and re-adding the `cursor-review` label." - ), - "event": "COMMENT", - "commit_id": commit_sha, - } + {"body": body_text, "event": "COMMENT", "commit_id": commit_sha} ) - result = gh_post_review(repo, pr_number, payload) - if result.returncode != 0: - print(f"Error-review POST failed: {result.stderr}", file=sys.stderr) + if not post_or_degrade(repo, pr_number, payload, body_text, "Error review"): raise SystemExit(1) @@ -241,9 +309,9 @@ def main(): payload = json.dumps( {"body": body_text, "event": "COMMENT", "commit_id": args.commit_sha} ) - result = gh_post_review(args.repo, args.pr_number, payload) - if result.returncode != 0: - print(f"No-findings review POST failed: {result.stderr}", file=sys.stderr) + if not post_or_degrade( + args.repo, args.pr_number, payload, body_text, "No-findings review" + ): raise SystemExit(1) return @@ -271,13 +339,22 @@ def main(): result = gh_post_review(args.repo, args.pr_number, payload) if result.returncode != 0: + # A read-only token rejects any write, so the inline-less fallback below + # would fail the same way — degrade straight to the job summary instead. + if is_read_only_token_error(result): + print( + "Review: token is read-only — writing the review to the job " + "summary instead of the PR.", + file=sys.stderr, + ) + write_step_summary(render_findings_markdown(review_body, comments)) + return + print(f"Review POST failed: {result.stderr}", file=sys.stderr) # Fallback: same body without inline anchors. Typical cause is line # numbers that fall outside the diff context — often the model picked # a line near the change but not on the change. - fallback_body = review_body + "\n\n---\n\n" - for c in comments: - fallback_body += f"**`{c['path']}:{c['line']}`** — {c['body']}\n\n" + fallback_body = render_findings_markdown(review_body, comments) fallback_body += "\n_(Inline comments could not be anchored to the diff; listed above instead.)_" fallback_payload = json.dumps( @@ -287,9 +364,9 @@ def main(): "commit_id": args.commit_sha, } ) - fallback_result = gh_post_review(args.repo, args.pr_number, fallback_payload) - if fallback_result.returncode != 0: - print(f"Fallback review POST also failed: {fallback_result.stderr}", file=sys.stderr) + if not post_or_degrade( + args.repo, args.pr_number, fallback_payload, fallback_body, "Fallback review" + ): raise SystemExit(1) diff --git a/.github/workflows/cursor-review.yml b/.github/workflows/cursor-review.yml index 98c4deb..810421e 100644 --- a/.github/workflows/cursor-review.yml +++ b/.github/workflows/cursor-review.yml @@ -127,7 +127,22 @@ jobs: LABEL_NAME: ${{ github.event.label.name }} PR_LABELS: ${{ toJSON(github.event.pull_request.labels.*.name) }} GH_EVENT_ACTION: ${{ github.event.action }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + BASE_REPO: ${{ github.repository }} run: | + # PRs from forks can't run this review. The pull_request event grants + # fork PRs no access to secrets — so CURSOR_API_KEY is empty and every + # panel cell produces empty output — and a read-only GITHUB_TOKEN, so + # posting the consolidated review returns HTTP 403. The review can + # neither analyze nor post, so skip cleanly instead of burning the + # 8-cell matrix + judge and failing red on every external contribution. + # (head.repo.full_name != github.repository is the cross-repo signal.) + if [ -n "$HEAD_REPO" ] && [ "$HEAD_REPO" != "$BASE_REPO" ]; then + echo "PR is from a fork ($HEAD_REPO != $BASE_REPO) — review needs secrets and write access a fork PR can't have. Skipping." + echo "should_run=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + # skip-cursor-review wins even if the trigger label is also present. if echo "$PR_LABELS" | jq -e 'index("skip-cursor-review")' > /dev/null; then echo "skip-cursor-review label present — skipping."