Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions .github/workflows/ci-cursor-review.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
name: CI - Cursor Review

# Thin caller enrolling THIS repo in its own reusable cursor-review workflow.
# The review logic (panel matrix, judge consolidation, prompts, scripts) lives
# in cursor-review.yml in this same repo, but the caller still pins it by full
# commit SHA like every external consumer: a local `uses: ./...` path would run
# the PR's OWN copy of the reviewer, letting a PR weaken the panel judging it.
# Pinning to a merged main SHA keeps the reviewer tamper-proof; the bump-callers
# workflow keeps the pin fresh (this repo is enrolled in CURSOR_REVIEW_CALLERS).
#
# Label-triggered for now (apply `cursor-review` to a PR to fire the panel;
# `skip-cursor-review` vetoes) — the reusable's gate at this pin only honors
# label events. Once the pin advances past the commit adding the reusable's
# `run_without_label` input, the auto-bump PR for this file should also add
# `run_without_label: true` under `with:` and widen the trigger types to
# [opened, reopened, ready_for_review, labeled, unlabeled] — this repo has no

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium — The recommended future trigger list [opened, reopened, ready_for_review, labeled, unlabeled] omits synchronize, yet the reusable's run_without_label gate and the run_without_label input description both explicitly support synchronize. Because specifying types overrides GitHub's defaults, an auto-bump PR that applies this guidance would never re-review new commits pushed to an open PR, leaving the latest code unanalyzed. Raised by 3 of 8 reviewers (gpt-5.6-sol-max adversarial, gemini-3.1-pro edge-case, gpt-5.6-sol-max edge-case).

# labeling automation, and every non-draft PR here is worth a review (its
# files become other repos' required checks).

on:
pull_request:
types: [labeled, unlabeled]

# Cancel any earlier in-flight review run for the same PR when cursor-review is
# removed or re-applied. Scoping by label name ensures that removing an
# unrelated label does not kill a running cursor review.
concurrency:
group: cursor-review-pr-${{ github.event.pull_request.number }}-${{ github.event.label.name }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium — The concurrency group embeds github.event.label.name, so a run started by the cursor-review label (group ...-cursor-review) and the labeled event adding skip-cursor-review (group ...-skip-cursor-review) fall into different groups. With cancel-in-progress: true the veto label therefore cannot cancel an already-running review, which keeps consuming secrets and posts anyway. In run_without_label mode it is worse: non-label events like synchronize produce an empty label segment (...-) that won't serialize against label events either. Raised by 3 of 8 reviewers (gpt-5.6-sol-max adversarial, gemini-3.1-pro edge-case, gpt-5.6-sol-max edge-case).

cancel-in-progress: true

jobs:
cursor-review:
permissions:
contents: read
pull-requests: write
uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@fe6b08e278c27ceacf2651cf441d0752ca2e78ee # main (fe6b08e)
with:
# Keep the assets ref in lock-step with the `uses:` ref above.
workflows_ref: fe6b08e278c27ceacf2651cf441d0752ca2e78ee
# No vendored/generated content in this repo; defensive excludes only.
diff_excludes: >-
:!**/*-lock.json
:!**/dist/**
secrets:
CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium — CURSOR_API_KEY is handed to a cursor-agent --trust process that ingests attacker-controlled same-repository PR content; the fork check only blocks forks, not same-repo contributors. A contributor who can push a branch (and, in label mode, apply the trigger label) could use prompt injection or a malicious workspace config to execute commands and exfiltrate the key — a risk run_without_label broadens by auto-triggering on any same-repo PR. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).

SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }}
32 changes: 32 additions & 0 deletions .github/workflows/ci-detect-unreviewed-merge.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Caller enrolling THIS repo in its own reusable unreviewed-merge detector.
# Named ci-detect-unreviewed-merge.yml because the canonical caller path
# (detect-unreviewed-merge.yml) is occupied here by the reusable itself.
# Otherwise a verbatim copy of the template in
# Comfy-Org/unreviewed-merges/detector/detect-unreviewed-merge.yml.
#
# PREREQUISITE (operator): provision `secrets.UNREVIEWED_MERGES_TOKEN` in this
# repo's settings (see the unreviewed-merges repo README). Until it exists,
# this job fails loudly on pushes to main — deliberately, so enrollment is
# never silently inert.

name: Detect Unreviewed Merge

on:
push:
branches: [main]

concurrency:
group: detect-unreviewed-merge-${{ github.sha }}
cancel-in-progress: false

permissions:
contents: read
pull-requests: read

jobs:
detect:
uses: Comfy-Org/github-workflows/.github/workflows/detect-unreviewed-merge.yml@4d9cb6b87f953bb7cd69954280e1465fb9bd2040 # v1
with:
approval-mode: latest-per-reviewer
secrets:
UNREVIEWED_MERGES_TOKEN: ${{ secrets.UNREVIEWED_MERGES_TOKEN }}
25 changes: 25 additions & 0 deletions .github/workflows/cursor-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,16 @@ on:
type: string
required: false
default: ''
run_without_label:
description: >-
Run on plain pull_request events (opened / reopened / ready_for_review
/ synchronize) instead of requiring the trigger label — for repos with
no labeling automation. The fork skip, the skip-cursor-review label,
and the same-commit dedupe all still apply. Default false
(label-triggered, unchanged for existing callers).
type: boolean
required: false
default: false
secrets:
CURSOR_API_KEY:
description: Cursor API key for cursor-agent (the panel + judge models bill through it).
Expand Down Expand Up @@ -149,6 +159,7 @@ jobs:
GH_EVENT_ACTION: ${{ github.event.action }}
HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }}
BASE_REPO: ${{ github.repository }}
RUN_WITHOUT_LABEL: ${{ inputs.run_without_label }}
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
Expand All @@ -170,6 +181,20 @@ jobs:
exit 0
fi

# Label-free mode (run_without_label input): callers in repos with no
# labeling automation opt in to running on plain PR events. Sits
# BELOW the fork skip and skip-cursor-review checks — those still
# veto — and the same-commit dedupe below still prevents re-reviews.
if [ "$RUN_WITHOUT_LABEL" = "true" ]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium — Same-repo Dependabot PRs pass the fork check, but GitHub withholds Actions secrets and downgrades GITHUB_TOKEN for Dependabot events. When a caller enables run_without_label, this branch sets should_run=true for Dependabot opened/synchronize events, so the panel runs with an empty CURSOR_API_KEY and the review job fails on every Dependabot update — blocking merges if it is a required check. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low — run_without_label lets multiple activity types fire on one HEAD SHA (e.g. opened then synchronize, or open-as-draft then ready_for_review); the same-commit dedupe is a separate, non-atomic step, so two near-simultaneous events can both pass it (neither has posted yet) and emit duplicate reviews. The reusable defines no concurrency of its own and the example caller group keyed on github.event.label.name is empty for these non-label events, so rapid synchronize pushes can also queue unbounded expensive panels — run_without_label callers need explicit concurrency guidance. Raised by 2 of 8 reviewers (claude-opus-4-8-thinking-max adversarial, claude-opus-4-8-thinking-max edge-case).

case "$GH_EVENT_ACTION" in

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low — In run_without_label mode this case handles only opened/reopened/ready_for_review/synchronize, so an unlabeled event (e.g. removing skip-cursor-review to unblock a PR) falls through to the legacy check that requires labeled + the trigger label and no-ops. Since the recommended trigger types include unlabeled, removing the veto label silently fails to re-trigger a review until another qualifying event occurs. Raised by 3 of 8 reviewers (gpt-5.6-sol-max adversarial, gemini-3.1-pro edge-case, gpt-5.6-sol-max edge-case).

opened|reopened|ready_for_review|synchronize)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium — In run_without_label mode the gate matches opened/synchronize (plus reopened/ready_for_review) with no github.event.pull_request.draft check, and GitHub fires opened/synchronize for draft PRs. Enrolled repos will run the full 8-cell panel + judge on every draft and every push to a draft, burning paid CURSOR_API_KEY quota and contradicting the caller comment's stated 'every non-draft PR' intent. Add a draft guard (or force callers to trigger only on ready_for_review). Raised by 5 of 8 reviewers (claude-opus-4-8-thinking-max adversarial, gemini-3.1-pro adversarial, gpt-5.6-sol-max edge-case, claude-opus-4-8-thinking-max edge-case, gemini-3.1-pro edge-case).

echo "run_without_label mode — running on '$GH_EVENT_ACTION'."
echo "should_run=true" >> "$GITHUB_OUTPUT"
exit 0
;;
esac
fi

# Only adding the trigger label fires the matrix. Removing it cancels
# any in-progress run via the concurrency group and then no-ops here.
if [ "$LABEL_NAME" = "$REVIEW_LABEL" ] && [ "$GH_EVENT_ACTION" = "labeled" ]; then
Expand Down
Loading