diff --git a/.github/cursor-review/README.md b/.github/cursor-review/README.md index 7ee26ae..f067a8e 100644 --- a/.github/cursor-review/README.md +++ b/.github/cursor-review/README.md @@ -84,13 +84,13 @@ silently vanishing — the review tells you what didn't run. | [`prompt-judge.md`](prompt-judge.md) | Prompt the judge model uses to consolidate panel findings into one review. | | [`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. | +| [`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). | ## Adopt it in your repo The review logic lives here; your repo adds only a **thin caller**. Pin `uses:` -to a full commit SHA (see the [top-level README](../../README.md#usage) for the +to a full commit SHA (see the [top-level README](../../README.md#pinning) for the why and the versioning policy). **1. Add the caller workflow** at `.github/workflows/ci-cursor-review.yml`: @@ -143,45 +143,30 @@ an app token is required). The opt-in roster lives in the caller's `vars.CURSOR_REVIEW_OPTED_IN_LOGINS`. See that workflow's header for the full example and the `vars.APP_ID` / `CLOUD_CODE_BOT_PRIVATE_KEY` requirements. -### Optional: make the review blocking (merge gate) +### The blocking gate is currently NOT available (regressed) -By default the review is **advisory** — it posts findings as PR review threads, -but an unresolved (red) review never blocks merge. Opt into a blocking gate by -passing `blocking: true`: +> **Do not pass `blocking: true`.** `cursor-review.yml` declares no such input +> today, and GitHub rejects an undeclared input at startup — a caller that passes +> it gets a zero-job `startup_failure` with no logs to explain it. -```yaml -jobs: - cursor-review: - uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@ # v1 - with: - blocking: true - secrets: - CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }} -``` +The review is **advisory**: it posts findings as PR review threads, and an +unresolved (red) review never blocks merge. -With `blocking: true`, a final **Blocking gate** job queries the PR's review -threads and **fails the check while any cursor-review finding thread is -unresolved**. Resolve the thread(s) — or push a fix and re-trigger the review — -and the gate goes green. It is idempotent: resolving threads and re-running the -check turns it green without a fresh panel. - -> **This workflow cannot set branch protection.** A red check is visible but -> does not block merge on its own. To actually gate merges you must **also mark -> the `Blocking gate` check as a required status check** in the calling repo: -> *Settings → Branches → Branch protection rule* (or *Rulesets*) → **Require -> status checks to pass** → add **`Blocking gate`** (it appears once the -> workflow has run at least once with `blocking: true`). The check name is -> prefixed with your caller job id, e.g. `cursor-review / Blocking gate`. - -Notes: - -- `blocking: false` (the default, or simply not passing the input) is **exactly - today's behavior** — no caller changes until it opts in. -- A thread is recognized as a cursor-review thread by its originating review's - body marker, so the gate works whether the review posts as - `github-actions[bot]` or under a dedicated `bot_app_id`. -- Outdated threads (their code changed since the finding was posted) don't - block — a re-review re-posts anything still wrong as a fresh thread. +An opt-in gate did exist. A `blocking:` input and a fail-closed **Blocking gate** +job — which queried the PR's review threads and failed while any cursor-review +finding thread was unresolved — shipped in +[#16](https://github.com/Comfy-Org/github-workflows/pull/16) (BE-1891). Both were +removed from `cursor-review.yml` in +[#31](https://github.com/Comfy-Org/github-workflows/pull/31), a change whose +stated purpose was fixing judge-findings extraction; the deletion looks +unintentional. The gate's implementation +([`gate-unresolved.py`](gate-unresolved.py)) is still here, orphaned and unwired. + +Restoring it is tracked separately. Note that marking `… / Consolidate panel` +required is **not** a substitute: GitHub counts a skipped required check as +passing, and that job skips whenever no review runs (no trigger label, dedupe hit, +diff over `diff_size_cap`, fork PR), so the check would go green in precisely the +cases a gate is meant to catch. ## Configuration knobs @@ -196,7 +181,10 @@ descriptions live in the [workflow header](../workflows/cursor-review.yml). | `diff_excludes` | lockfiles, `node_modules`, `dist`, `vendor`, minified/generated files | Pathspecs excluded from both the size count and the reviewed diff. | | `workflows_ref` | `main` | Ref this directory's prompts/scripts are loaded from. Pin to your `uses:` SHA. | | `bot_app_id` | `''` | Optional GitHub App ID; when set (with `BOT_APP_PRIVATE_KEY`), the review posts under that App's identity instead of `github-actions[bot]`. | -| `blocking` | `false` | Opt-in merge gate. `true` fails the **Blocking gate** check while any cursor-review finding thread is unresolved. See [Make the review blocking](#optional-make-the-review-blocking-merge-gate). | +| `run_without_label` | `false` | Run on plain PR events instead of requiring the trigger label. Also requires widening the caller's `types:` — see [the setup guide](../../docs/callers/cursor-review.md). | + +There is **no `blocking` input** — see [the regression note +above](#the-blocking-gate-is-currently-not-available-regressed). ### Escape hatches diff --git a/.github/workflows/detect-unreviewed-merge.yml b/.github/workflows/detect-unreviewed-merge.yml index 51b7923..feb5fd4 100644 --- a/.github/workflows/detect-unreviewed-merge.yml +++ b/.github/workflows/detect-unreviewed-merge.yml @@ -13,13 +13,21 @@ name: Detect Unreviewed Merge (reusable) # concurrency: # group: detect-unreviewed-merge-${{ github.sha }} # cancel-in-progress: false +# permissions: +# contents: read +# pull-requests: read # jobs: # detect: -# uses: Comfy-Org/unreviewed-merges/.github/workflows/detector.yml@v1 +# # This repo, pinned by full commit SHA. The path below is the live one; +# # an older example pointed at Comfy-Org/unreviewed-merges/.../detector.yml, +# # which does not exist and does not resolve. +# uses: Comfy-Org/github-workflows/.github/workflows/detect-unreviewed-merge.yml@ # v1 # with: # approval-mode: latest-per-reviewer # or 'any-approval' for private repos # secrets: # UNREVIEWED_MERGES_TOKEN: ${{ secrets.UNREVIEWED_MERGES_TOKEN }} +# +# Setup guide: docs/callers/detect-unreviewed-merge.md on: workflow_call: diff --git a/.gitignore b/.gitignore index 476c505..b0e7f74 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,13 @@ __pycache__/ *.py[cod] *.pyc + +# Go build output. pr-size.yml compiles this from source into RUNNER_TEMP on +# every run, so a committed binary is dead weight — and a local `go build` here +# produces a host-arch executable that cannot run on the ubuntu-latest runners. +/scripts/check-pr-size/check-pr-size +/scripts/check-pr-size/check-pr-size.exe + +# Stray local logs +err.log +*.err.log diff --git a/AGENTS.md b/AGENTS.md index b27b481..4675c89 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,12 +62,16 @@ tests — run the matching command above for whatever you touched. - `README.md` — the public workflow catalog: per-workflow purpose, the SHA-pin usage pattern, and the versioning policy. Keep its table in sync when you add a workflow. +- `docs/callers/` — one setup guide per reusable workflow: a complete, + copy-pasteable caller (including `on:` and the exact permission grant), + required secrets/vars, and per-workflow footguns. Add a page when you add a + workflow; the README table links to it. ## Reusable workflow catalog (what each does) - `cursor-review.yml` — label-triggered multi-model PR review (4-lab × 2-type - panel → judge → one PR review with severity badges). Advisory by default; - `blocking: true` gates on unresolved findings. + panel → judge → one PR review with severity badges). Advisory: it posts a + review, it does not gate. - `cursor-review-auto-label.yml` — translates PR assignment/open into the review label (via an app token, since a `GITHUB_TOKEN`-applied label won't fire runs). - `groom.yml` — scheduled/dispatch org-wide code-cleanup sweep: read-only finder @@ -95,7 +99,7 @@ tests — run the matching command above for whatever you touched. workflow files, commit messages, and PR text. - **Pin everything by full commit SHA**, with a trailing `# v1` comment — both the `uses:` in callers and every third-party action here. Bare `@v1` fails the - pin-validation (`pinact`, `zizmor`) that consumer CI runs. See README "Usage". + pin-validation (`pinact`, `zizmor`) that consumer CI runs. See README "Pinning". - **Scripts are the single source of truth**, loaded at run time from a pinned ref of THIS repo — never from the caller's checkout. That's what makes the reviewer/checker tamper-proof: a PR can't rewrite the logic judging it. The @@ -108,9 +112,18 @@ tests — run the matching command above for whatever you touched. callers). Do not fork the script — a forked copy is how other shared org machinery has drifted. - **New reusable workflow?** `on: workflow_call` + a header comment documenting - inputs/secrets/triggers + a caller-pattern example, then update the README - table (README "Adding a new reusable workflow"). Move the floating major tag + inputs/secrets/triggers + a caller-pattern example, then a + `docs/callers/.md` setup guide and a row in the README table (see + CONTRIBUTING.md "Adding a new reusable workflow"). Move the floating major tag after merge. +- **Document only inputs that exist.** GitHub rejects an unknown input at startup, + so a phantom input in the docs is a broken caller for whoever copies it. Check + `on.workflow_call.inputs` before documenting a knob. The `cursor-review` + `blocking:` input is the worked example: it shipped in #16, was deleted from the + workflow by #31 (a judge-extraction fix) while its docs and + `gate-unresolved.py` were left behind, and the stale docs outlived it here, in + the README, and in `.github/cursor-review/README.md`. **Deleting an input is a + docs change too** — grep the repo for its name in the same commit. - **Versioning:** semver-style major tags (`v1`, `v2`). Breaking changes bump the major; backwards-compatible changes move the existing tag in place (`git tag -f v1 && git push -f origin v1`). This tag force-move is the @@ -126,6 +139,9 @@ tests — run the matching command above for whatever you touched. ## Deeper docs - [`README.md`](README.md) — public catalog, SHA-pin usage, versioning. +- [`docs/callers/`](docs/callers/) — per-workflow setup guides (copy-pasteable callers). +- [`CONTRIBUTING.md`](CONTRIBUTING.md) — tests to run, breaking-change rules, enrollment. +- [`SECURITY.md`](SECURITY.md) — disclosure process + the agent credential boundary. - [`.github/cursor-review/README.md`](.github/cursor-review/README.md) — review panel internals + adoption. - [`.github/agents-md-integrity/README.md`](.github/agents-md-integrity/README.md) — the checker + its knobs. - [`.github/bump-callers/README.md`](.github/bump-callers/README.md) — the shared bumper + its fleets. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..7b6f04e --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,101 @@ +# Contributing + +Thanks for helping out. One thing to understand before you start: + +> **Every change here is live-fire.** Other repositories call these workflows at +> pinned SHAs, and moving the `v1` tag pushes your change to every caller pinned +> to the tag on their next run. There is no staging environment. A workflow that +> is merely *wrong* fails someone else's CI; one that is wrong about +> **permissions** fails it at startup with no logs to explain why. + +## Before you open a PR + +Run the tests for whatever you touched. They are the same commands CI runs: + +```bash +# groom (ledger + dedup) +python3 -m unittest discover -s .github/groom/tests -p 'test_*.py' -v + +# cursor-review (finding extraction, bot identity) +python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py' -v + +# AGENTS.md standard checker +python3 -m unittest discover -s .github/agents-md-integrity/tests -p 'test_*.py' -v + +# caller-bump machinery +shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh +bash .github/bump-callers/tests/test_bump_callers.sh + +# PR-size classifier +cd scripts/check-pr-size && go vet ./... && go test ./... +``` + +Do **not** commit build outputs. `scripts/check-pr-size/` is compiled from source +by `pr-size.yml` at run time; a committed binary is dead weight (and the wrong +architecture for the `ubuntu-latest` runners). + +## Changing a reusable workflow + +Ask two questions in order: + +**1. Does this change what a caller must send or grant?** Adding a required +input, adding a required secret, or making a nested job request a *new* +permission are all **breaking**, even though nothing in the caller's YAML +changed. GitHub validates the permission grant at **startup**, so a caller that +was fine yesterday fails with an opaque "workflow file issue" and zero jobs. If +the answer is yes, bump the major tag (`v1` → `v2`) and let callers opt in. + +**2. Does it change assets loaded at run time?** Several workflows fetch prompts, +briefs, or checker scripts from `workflows_ref` while running. A caller that +pins `uses:` to a SHA but leaves `workflows_ref: main` will mix your new assets +with its old workflow. When you change assets and workflow together, say so in +the PR body so callers bump both. + +## Adding a new reusable workflow + +1. Add `.github/workflows/.yml` with `on: workflow_call:`. + Document every input and secret inline. +2. Declare **minimum permissions per job**, not at the workflow level. Callers + must grant the union of what your nested jobs request — keep that union small + and state it in the header comment. +3. Add a setup guide at `docs/callers/.md` following the shape + of the existing ones: a **complete, copy-pasteable** caller (including `on:`), + the exact permission grant, required vs optional secrets and `vars`, and any + footguns. A guide that omits `on:` or the permission grant is not a guide. +4. Add a one-line row to the [README](README.md#workflows) table linking to it. +5. If the workflow should be adopted broadly, add a `vars._CALLERS` roster + and a `bump--callers.yml` job so pins get bumped automatically. See + [.github/bump-callers/](.github/bump-callers/). +6. Add a `test-.yml` if it ships scripts. + +## Enrolling a repository as a caller + +Two steps. Missing the second is the common mistake: + +1. Add the caller workflow to the consumer repo (see + [`docs/callers/`](docs/callers/)). +2. **Add the repo to the matching `vars._CALLERS` roster** on this repo. + That roster is what `bump-*-callers.yml` reads to keep pins current. A repo + absent from it keeps its original SHA forever, drifts behind the reusable, and + eventually breaks when the two stop being compatible. + +## Security-sensitive areas + +The AI workflows (`cursor-review.yml`, `groom.yml`) aim to split model execution +from credential use: the agent jobs run with `contents: read` and mint no GitHub +token, emitting a patch or findings that a **separate** job applies as a GitHub +App. Preserving that boundary matters more than convenience — do not give an agent +job a write token to save a step. + +The split is clean for `groom.yml` and for `cursor-review.yml`'s 8 panel cells. +It is **not** clean for cursor-review's judge: `Consolidate panel` runs the judge +model and posts the review in one `pull-requests: write` job. Treat that as a known +rough edge to work *toward* the boundary, not as licence to widen it elsewhere. See +[SECURITY.md](SECURITY.md). + +## Review + +Reviewers and assignees are routed automatically from +[`.github/reviewers.yml`](.github/reviewers.yml) by path. If you are changing a +bucket's globs, read the comments at the top of that file first — it explains why +the buckets are the durable part and the names are not. diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..7ceb934 --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 Comfy Org + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md index 17d83d0..ee1acc0 100644 --- a/README.md +++ b/README.md @@ -1,56 +1,126 @@ # Comfy-Org reusable GitHub Actions workflows -Shared, versioned [reusable workflows](https://docs.github.com/en/actions/using-workflows/reusing-workflows) for use across Comfy-Org repositories. +Shared, versioned [reusable +workflows](https://docs.github.com/en/actions/using-workflows/reusing-workflows) +for use across Comfy-Org repositories. -This repo is **public** so any repo — public or private, inside or outside the org — can call these workflows with no extra GitHub Actions settings. +This repo is **public**, so any repo — public or private, inside or outside the +org — can call these workflows with no extra GitHub Actions settings. + +**→ [Setup guides for every workflow](docs/callers/)** — each gives you a +complete, copy-pasteable caller. ## Workflows -| Workflow | Purpose | -|---|---| -| [`detect-unreviewed-merge.yml`](.github/workflows/detect-unreviewed-merge.yml) | SOC 2 compliance — detects PRs merged without prior approval and opens a tracking issue in [`Comfy-Org/unreviewed-merges`](https://github.com/Comfy-Org/unreviewed-merges). | -| [`cursor-review.yml`](.github/workflows/cursor-review.yml) | Label-triggered multi-model code review. A 4-lab × 2-review-type cursor-agent panel runs adversarial + edge-case passes, a judge model consolidates them into one PR review with per-finding severity badges, and the triggerer gets Slack start/complete DMs. Advisory by default; opt in with `blocking: true` to fail a (required-status-check) gate while findings stay unresolved. Prompts and scripts live in [`.github/cursor-review/`](.github/cursor-review) — the single source of truth, so consumer repos carry only a thin caller. Self-hostable via `runs_on` (JSON, default `ubuntu-latest`) and panel models overridable via `models` (JSON array) for accounts lacking a default provider. Requires `CURSOR_API_KEY` (+ optional `SLACK_BOT_TOKEN`). | -| [`cursor-review-auto-label.yml`](.github/workflows/cursor-review-auto-label.yml) | Companion to `cursor-review.yml`. On PR assignment, applies the review label for an opted-in reviewer (via the CLOUD_CODE_BOT app token, so the label actually triggers the review). The opt-in roster lives in the caller's `vars.CURSOR_REVIEW_OPTED_IN_LOGINS` — no roster is baked into the workflow. Requires `vars.APP_ID` + `CLOUD_CODE_BOT_PRIVATE_KEY`. | -| [`assign-reviewers.yml`](.github/workflows/assign-reviewers.yml) | Auto-requests expertise-aware, load-balanced PR reviewers with new-folk randomization. Matches changed paths against a caller-repo `.github/reviewers.yml` (path-glob → reviewers, plus a `default_pool`), drops the author + `vars.REVIEWER_EXCLUDE`, ranks candidates by open review load (steering off anyone at/over `vars.REVIEWER_LOAD_CAP`), and may swap a slot for a `vars.REVIEWER_GROWTH_POOL` member. Requests go through the CLOUD_CODE_BOT app token so they work on fork PRs. Requires `vars.APP_ID` + `CLOUD_CODE_BOT_PRIVATE_KEY`. | -| [`assign-prs-to-author.yml`](.github/workflows/assign-prs-to-author.yml) | Housekeeping — assigns every open PR with no assignees to its author (bot-authored PRs skipped by default). Run on a schedule from a thin caller; useful when a team tracks PR ownership via assignees. The calling job needs `pull-requests: write` and `issues: write`. | -| [`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. | -| [`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. | +| Workflow | Purpose | Setup | +|---|---|---| +| [`groom.yml`](.github/workflows/groom.yml) | Scheduled whole-repo code-cleanup sweep. A read-only finder agent proposes refactors, an independent verifier re-checks each, and survivors are deduped against a durable ledger and filed as `groom` issues. Runs on a daily base cron gated by a `GROOM_INTERVAL_DAYS` runtime cadence (default 7). Opt-in builder turns top findings into review-gated PRs. | [groom.md](docs/callers/groom.md) | +| [`cursor-review.yml`](.github/workflows/cursor-review.yml) | Label-triggered multi-model code review. A 4-lab × 2-review-type panel runs adversarial + edge-case passes; a judge model consolidates them into one PR review with per-finding severity badges, and the triggerer gets Slack DMs. | [cursor-review.md](docs/callers/cursor-review.md) | +| [`cursor-review-auto-label.yml`](.github/workflows/cursor-review-auto-label.yml) | Companion to the above. Applies the review label for an opted-in reviewer using an App token, so the label actually fires the review. | [cursor-review-auto-label.md](docs/callers/cursor-review-auto-label.md) | +| [`pr-size.yml`](.github/workflows/pr-size.yml) | PR-size cap. Fails (or, in `mode: warn`, only reports) when a PR's net **non-generated** diff exceeds `max_lines`, excluding lockfiles and generated code. Bypass label for legitimately large changes. | [pr-size.md](docs/callers/pr-size.md) | +| [`agents-md-integrity.yml`](.github/workflows/agents-md-integrity.yml) | Enforces the org `AGENTS.md` standard: it exists, stays under a line ceiling, `CLAUDE.md` is a thin shim, no legacy `.cursorrules`. Wires in as a required status check. | [agents-md-integrity.md](docs/callers/agents-md-integrity.md) | +| [`assign-reviewers.yml`](.github/workflows/assign-reviewers.yml) | Expertise-aware, load-balanced PR routing from a per-repo `.github/reviewers.yml`, with new-folk randomization. Writes the **assignee** field. | [assign-reviewers.md](docs/callers/assign-reviewers.md) | +| [`assign-prs-to-author.yml`](.github/workflows/assign-prs-to-author.yml) | Housekeeping. Assigns every open PR with no assignees to its author. Run on a schedule. | [assign-prs-to-author.md](docs/callers/assign-prs-to-author.md) | +| [`stale.yml`](.github/workflows/stale.yml) | Stale-PR sweeper (`actions/stale`) plus a Slack digest that names the repo on every PR line, so multi-repo batches in one channel stay readable. | [stale.md](docs/callers/stale.md) | +| [`detect-unreviewed-merge.yml`](.github/workflows/detect-unreviewed-merge.yml) | SOC 2 compliance. Detects PRs merged without prior approval and opens a tracking issue in [`Comfy-Org/unreviewed-merges`](https://github.com/Comfy-Org/unreviewed-merges). | [detect-unreviewed-merge.md](docs/callers/detect-unreviewed-merge.md) | + +Per-workflow inputs and secrets are also documented in each workflow file's +header comment; the setup guides above are the maintained, copy-pasteable version. -## Usage +## Quick start -Reference a workflow by full path and pin to a **full commit SHA** (with the version as a trailing comment). Also set explicit minimum permissions on the calling job so the default permissive token scope isn't granted: +A caller is a **complete workflow file** in your repo at +`.github/workflows/.yml`. It needs its own `on:` trigger — the reusable +workflow does not supply one — and its calling job must grant the permissions the +reusable's nested jobs declare: ```yaml -permissions: - contents: read - pull-requests: read +name: PR Size Cap + +on: + pull_request: + types: [opened, synchronize, reopened, labeled, unlabeled] jobs: - my-job: - uses: Comfy-Org/github-workflows/.github/workflows/.yml@ # v1 + size: + permissions: + contents: read + uses: Comfy-Org/github-workflows/.github/workflows/pr-size.yml@ with: - : - secrets: - : ${{ secrets. }} + workflows_ref: + max_lines: 1000 +``` + +Get the current SHA: + +```bash +gh api repos/Comfy-Org/github-workflows/commits/main --jq .sha +``` + +Then follow the [setup guide](docs/callers/) for whichever workflow you are +adopting — the exact permission grant, required secrets, and per-workflow +footguns are there. + +> **Permissions fail at startup.** A nested job cannot request more +> `GITHUB_TOKEN` scope than the calling job grants, and GitHub validates that +> *before any job runs* — regardless of `if:` guards, and regardless of a GitHub +> App token doing the real writes. Get it wrong and you get an opaque "workflow +> file issue" with **zero jobs and no logs**, not a failing step. Grant exactly +> what the setup guide lists. + +## Pinning + +Pin `uses:` to a **full commit SHA**, with the version as a trailing comment: + +```yaml +uses: Comfy-Org/github-workflows/.github/workflows/pr-size.yml@8c4ff3e… # v1 ``` -The SHA-pin format satisfies pin-validation tooling (`pinact`, `zizmor`, etc.) and gives auditors immutable supply-chain evidence. Dependabot/Renovate can auto-bump the SHA when the upstream tag moves. +The SHA-pin format satisfies pin-validation tooling (`pinact`, `zizmor`) and gives +auditors immutable supply-chain evidence. Dependabot/Renovate can auto-bump it. + +A bare `@v1` tag is technically allowed but **will fail** in repos that run pin +validation in CI — and because `v1` is force-pushed for compatible changes, it +silently changes what you execute. + +**Keep `workflows_ref` equal to your `uses:` SHA.** `groom`, `cursor-review`, +`pr-size`, and `agents-md-integrity` load assets — agent briefs, review prompts, +checker scripts — from `workflows_ref` at run time. It defaults to `main`, so a +SHA-pinned caller that leaves it unset runs an old workflow against today's +assets. -A bare `@v1` tag is technically allowed but **will fail** in repos that run pin-validation in CI (e.g. `cloud`, `ComfyUI_frontend`). +## Staying current -Per-workflow inputs, required secrets, and triggers are documented in each workflow file's header comment. +Enrolling a repo is **two steps**: + +1. Merge the caller workflow into your repo. +2. Add the repo to the matching roster variable here — `GROOM_CALLERS`, + `CURSOR_REVIEW_CALLERS`, `PR_SIZE_CALLERS`, `AGENTS_MD_CALLERS`, or + `ASSIGN_REVIEWERS_CALLERS`. + +The `bump-*-callers.yml` workflows read those rosters to open pin-bump PRs when a +reusable moves. **A repo absent from the roster keeps its original SHA forever**, +drifts behind, and eventually breaks when the two stop being compatible. Details +in [docs/callers/README.md](docs/callers/README.md#staying-current). ## Versioning -Workflows in this repo use **semver-style major-version tags** (`v1`, `v2`, …). +Major-version tags (`v1`, `v2`, …). + +- **Breaking changes** bump the major; callers opt in. Adding a required input, + adding a required secret, or making a nested job request a new permission are + all breaking — even though nothing in the caller's YAML changed. +- **Backwards-compatible changes** move the existing major tag in place + (`git tag -f v1 && git push -f origin v1`). Callers pinned to the tag pick + it up on their next run; SHA-pinned callers opt in by bumping. + +## Contributing + +See [CONTRIBUTING.md](CONTRIBUTING.md) — how to add a new reusable workflow, which +tests to run, and why every change here is live-fire. -- Breaking changes bump the major (`v1` → `v2`); callers opt in. -- Backwards-compatible changes update the existing major tag in place (`git tag -f v1 && git push -f origin v1`) — callers pinned to the tag pick up the update on the next run; callers pinned to a SHA opt in by bumping the SHA. +Security issues: **do not open a public issue.** See [SECURITY.md](SECURITY.md). -## Adding a new reusable workflow +## License -1. Add the workflow file under `.github/workflows/.yml` with `on: workflow_call:` and a header comment documenting inputs/secrets. -2. Update the table in this README. -3. Move the floating `v1` tag (or cut a new major) once the change is reviewed and merged. +[MIT](LICENSE). diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..238d1e7 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,66 @@ +# Security policy + +## Reporting a vulnerability + +**Do not open a public issue for a security problem.** + +Report privately via GitHub's [private vulnerability +reporting](https://github.com/Comfy-Org/github-workflows/security/advisories/new) +on this repository, or email **security@comfy.org**. + +Please include the workflow file and, where you can, the ref (tag or commit SHA) +you observed the problem on — several of these workflows behave differently +across refs, and the ref narrows the blast radius fast. + +We aim to acknowledge within 3 business days. + +## What is in scope + +This repo contains reusable GitHub Actions workflows that other repositories call +and grant credentials to. In scope: + +- **Privilege escalation** — a workflow obtaining more `GITHUB_TOKEN` scope than + the calling job granted, or using a scope it does not need. +- **Secret exfiltration** — any path by which `ANTHROPIC_API_KEY`, + `CURSOR_API_KEY`, `SLACK_BOT_TOKEN`, `UNREVIEWED_MERGES_TOKEN`, or a GitHub App + private key could reach a log, artifact, comment, or third party. +- **Untrusted input reaching a privileged context** — PR titles, branch names, + commit messages, or fork-authored content flowing into a shell without + quoting, or into a job that holds write scope. +- **`pull_request_target` / fork-PR issues** — anything that lets a fork PR run + with the base repo's secrets. +- **Agent-boundary breaks** — where a model step is meant to run with **no write + credentials**, emitting patches or findings that a *separate* job applies, a way + to make that step hold a write token — or to influence the privileged job from + inside the unprivileged one — is a vulnerability. The separation is real for + groom and for cursor-review's 8 panel cells (`contents: read` only). It is + **not** absolute for cursor-review's judge: the `Consolidate panel` job runs the + judge model and posts the review in the same `pull-requests: write` job, so + treat findings about that boundary as in scope rather than known-and-accepted. +- **Supply chain** — an unpinned or mutable third-party action reference, or a + way to make a caller resolve assets from a ref other than the one it pinned. + +## What is not in scope + +- Findings that require an existing org admin or repo write access. +- The AI workflows producing a wrong, low-quality, or duplicate review comment, + finding, or issue. That is a quality bug — open a normal issue. +- Spend caused by a workflow running more often than intended, absent a security + boundary being crossed. Also a normal issue. +- Missing branch protection or required status checks on a *consumer* repo. Raise + that with the consumer repo's owners. + +## Notes for consumers + +- **Pin by full commit SHA**, not by tag. The `v1` tag is a *moving* pointer that + we force-push for backwards-compatible changes; a SHA is immutable. See + [Pinning](README.md#pinning). +- **Keep `workflows_ref` equal to your `uses:` SHA.** Several workflows load + prompts, briefs, or checker scripts from `workflows_ref` at runtime. If it + drifts from the pinned workflow — or is left at the default `main` — you are + executing assets from a ref you did not review. +- **Grant the documented minimum permissions**, not `write-all`. Each workflow's + required grant is listed in [`docs/callers/`](docs/callers/). +- These workflows are MIT licensed and provided as-is. If you call them from + outside Comfy-Org, you are trusting this repo's contents at whatever ref you + pin; review it as you would any dependency. diff --git a/docs/callers/README.md b/docs/callers/README.md new file mode 100644 index 0000000..151f7da --- /dev/null +++ b/docs/callers/README.md @@ -0,0 +1,234 @@ +# Setting up a caller + +Each reusable workflow in this repo is wired in the same way: you add a small +**caller** workflow to your own repository that hands off to the shared one. The +per-workflow pages give you a complete, copy-pasteable caller: + +| Workflow | Setup guide | What you must provide | +|---|---|---| +| `groom.yml` | [groom.md](groom.md) | `ANTHROPIC_API_KEY` | +| `cursor-review.yml` | [cursor-review.md](cursor-review.md) | `CURSOR_API_KEY` | +| `cursor-review-auto-label.yml` | [cursor-review-auto-label.md](cursor-review-auto-label.md) | `vars.APP_ID` + App key | +| `pr-size.yml` | [pr-size.md](pr-size.md) | nothing (bot comment optional) | +| `agents-md-integrity.yml` | [agents-md-integrity.md](agents-md-integrity.md) | nothing | +| `assign-reviewers.yml` | [assign-reviewers.md](assign-reviewers.md) | `vars.APP_ID` + App key + `.github/reviewers.yml` | +| `assign-prs-to-author.yml` | [assign-prs-to-author.md](assign-prs-to-author.md) | nothing | +| `stale.yml` | [stale.md](stale.md) | `SLACK_BOT_TOKEN` (optional) | +| `detect-unreviewed-merge.yml` | [detect-unreviewed-merge.md](detect-unreviewed-merge.md) | `UNREVIEWED_MERGES_TOKEN` | + +Everything below applies to all of them. Read it once. + +--- + +## The shape of a caller + +A caller is a **complete workflow file** in your repo at +`.github/workflows/.yml`. It needs its own `on:` trigger — the reusable +workflow does not supply one, so a caller without `on:` never runs: + +```yaml +name: Groom + +on: + schedule: + - cron: '17 9 * * 1' # Mondays 09:17 UTC + workflow_dispatch: + +jobs: + groom: + permissions: # see "Permissions" below — this is not optional + contents: read + issues: write + pull-requests: read + actions: read # groom's runtime cadence gate reads run history + uses: Comfy-Org/github-workflows/.github/workflows/groom.yml@ + with: + cadence: 7 + interval_days: 7 + workflows_ref: # see "Pinning" — do not leave this at main + secrets: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} +``` + +Three parts people leave out, in order of how much pain they cause: + +1. `permissions:` on the calling **job** +2. `on:` at the top +3. the `vars.*_CALLERS` roster entry (see [Staying current](#staying-current)) + +--- + +## Permissions — the one that fails at startup + +> A nested job in a reusable workflow **cannot request more `GITHUB_TOKEN` scope +> than the calling job grants.** GitHub validates this at **startup**, before any +> job runs — regardless of `if:` guards, and regardless of a GitHub App token +> doing the real writes. + +When the grant is short, the run does not fail *a step*. It fails with an opaque +"workflow file issue", **zero jobs, and no logs**. There is nothing to read, +which is what makes it expensive to debug. This has already bitten this repo's +own groom pilot once. + +So: grant exactly the union listed on the workflow's setup page. More than the +minimum is a needless privilege; less is a startup failure. + +```yaml +jobs: + my-job: + permissions: # on the JOB, not the workflow + contents: read + issues: write +``` + +A caller-level `permissions:` block also works, but per-job is clearer once a +caller grows a second job. + +--- + +## Pinning + +Pin `uses:` to a **full 40-character commit SHA**, with the human-readable +version as a trailing comment: + +```yaml +uses: Comfy-Org/github-workflows/.github/workflows/pr-size.yml@8c4ff3e… # v1 +``` + +Get the current SHA: + +```bash +gh api repos/Comfy-Org/github-workflows/commits/main --jq .sha +``` + +**Why not `@v1`?** The `v1` tag is a *moving* pointer — we force-push it for +backwards-compatible changes, so `@v1` silently changes what you execute. A SHA +is immutable and gives auditors real supply-chain evidence. Repos that run pin +validation in CI (`pinact`, `zizmor`) **will fail** a bare tag. + +### `workflows_ref` must match your pin + +Several workflows (`groom`, `cursor-review`, `pr-size`, `agents-md-integrity`) +load assets — agent briefs, review prompts, checker scripts — from a ref **at run +time**, controlled by the `workflows_ref` input. It defaults to `main`. + +That default means a caller pinned to a SHA runs **old workflow + newest +assets**. Usually harmless, occasionally not, and always confusing. Set it +explicitly to the same SHA: + +```yaml +uses: Comfy-Org/github-workflows/.github/workflows/groom.yml@07154fb… +with: + workflows_ref: 07154fb… # keep in lock-step with the uses: SHA above +``` + +--- + +## Concurrency + +Check whether the reusable already declares a `concurrency` group. Today only +`groom.yml` does (`groom-${{ github.repository }}`). + +**If it does, do not declare the same group in your caller.** The caller holds +the group while waiting for its `uses:` job, which is waiting to acquire the same +group with `cancel-in-progress: false`. Neither yields. The run hangs until +timeout. Let the reusable own serialization and stay out of the group. + +If the reusable declares none, a caller-level group is fine and often useful +(`stale.yml`'s header shows exactly that). + +--- + +## Staying current + +Enrolling a repo is **two steps**, and the second is the one people miss. + +1. Merge the caller workflow into your repo. +2. Add the repo to the matching roster variable on **this** repo: + + | Workflow | Roster variable | + |---|---| + | `groom.yml` | `GROOM_CALLERS` | + | `cursor-review.yml` | `CURSOR_REVIEW_CALLERS` | + | `pr-size.yml` | `PR_SIZE_CALLERS` | + | `agents-md-integrity.yml` | `AGENTS_MD_CALLERS` | + | `assign-reviewers.yml` | `ASSIGN_REVIEWERS_CALLERS` | + + ```bash + gh api repos/Comfy-Org/github-workflows/actions/variables/GROOM_CALLERS --jq .value + ``` + + Each entry is `{"repo": "...", "file": ".github/workflows/.yml", "label": ""}`. + +The `bump-*-callers.yml` workflows read those rosters to open pin-bump PRs when +the reusable moves. **A repo absent from the roster keeps its original SHA +forever** — it drifts behind, and eventually breaks when the two are no longer +compatible. That is not hypothetical: this repo's own `ci-groom.yml` is not in +`GROOM_CALLERS`, and its pin has not moved since the day it was written. + +--- + +## Verifying it works + +Before trusting a schedule, prove the wiring. How you trigger it depends on the +caller's `on:` block — there is no universal recipe. + +```bash +# 1. Does the caller parse and resolve at all? (all callers) +gh workflow list --repo / + +# 2. Read the result of its most recent run +gh run list --repo / --workflow .yml --limit 1 +``` + +**Scheduled / dispatchable callers** — `groom`, `stale`, `assign-prs-to-author`. +These declare `workflow_dispatch`, so you can fire them by hand: + +```bash +gh workflow run .yml --repo / +``` + +`groom` and `stale` are the two that take a **`dry_run`** input: a full audit that +files/labels nothing and prints what it *would* do. Use it before the first live +run. It only reaches the reusable if your caller plumbs a `workflow_dispatch` input +through to `with: dry_run:` — [groom.md](groom.md) and [stale.md](stale.md) both +show that wiring. + +```bash +gh workflow run groom.yml --repo / -f dry_run=true +``` + +**PR-triggered callers** — `cursor-review`, `cursor-review-auto-label`, `pr-size`, +`agents-md-integrity`, `assign-reviewers`. As shown in their setup guides these +declare **no `workflow_dispatch` and no `dry_run`**, so `gh workflow run` errors out +instead of starting a run. Verify by opening a throwaway PR — and for +`cursor-review`, applying the review label. + +Adding `workflow_dispatch` to the caller is only worth it for +`agents-md-integrity`, which has no event dependency at all: it checks out the repo +and validates the files, so a manual dispatch is a genuine smoke test. For the +others it buys nothing — `cursor-review`, `cursor-review-auto-label` and `pr-size` +read `github.event.pull_request`, which a dispatch does not populate, and +`assign-reviewers` logs `Not a pull_request event — nothing to do` and exits. + +**Push-triggered caller** — `detect-unreviewed-merge` runs on push to the default +branch. Verify it by landing any commit there, then reading the run. + +`conclusion: startup_failure` with no job logs is **always** worth checking the +permission grant first. + +## Secrets + +Comfy-Org provides `ANTHROPIC_API_KEY` and `CURSOR_API_KEY` at the **org** level, +so enrolled repos generally need no per-repo secret. Confirm rather than assume: + +```bash +gh secret list --repo / # repo-level +gh variable list --repo / +``` + +**Omitting** a required `secrets:` mapping fails at startup — loudly, with zero +jobs. But **mapping a secret that is not set** passes an empty string through, and +that failure is quieter: `cursor-review` degrades to a panel where every cell +produces nothing rather than failing red. So confirm the secret exists, not just +that the mapping is present. diff --git a/docs/callers/agents-md-integrity.md b/docs/callers/agents-md-integrity.md new file mode 100644 index 0000000..d9d0aaa --- /dev/null +++ b/docs/callers/agents-md-integrity.md @@ -0,0 +1,88 @@ +# `agents-md-integrity.yml` — enforce the Comfy `AGENTS.md` standard + +Read [the shared caller contract](README.md) first. + +## What it does + +Checks that your repo follows the org's agent-instructions standard: + +- a top-level `AGENTS.md` **exists** and stays under a hard line ceiling + (`max_lines`, default 200; warns over `warn_lines`, default 150) +- a root `CLAUDE.md` **exists** and is a thin `@AGENTS.md` shim rather than a + divergent copy. With the default `require_shim: true`, a repo that has + `AGENTS.md` and *no* `CLAUDE.md` **fails** — Claude Code reads only `CLAUDE.md` + and does not fall back, so a missing shim means invisible instructions. Set + `require_shim: false` to only validate a `CLAUDE.md` that is already there. +- no legacy `.cursorrules` (gated by `forbid_cursorrules`) +- every nested monorepo `AGENTS.md` has a sibling `@AGENTS.md` shim and is under + the ceiling (gated by `check_nested`) +- `AGENTS.md` has a CODEOWNERS DRI (`require_codeowners`, warn-only by default) + +Fails with a non-zero exit and GitHub annotations, so it wires in cleanly as a +required status check. The checker lives in +[`.github/agents-md-integrity/`](../../.github/agents-md-integrity). + +## Prerequisites + +None. No secrets. + +## Caller + +`.github/workflows/agents-md-integrity.yml`: + +```yaml +name: AGENTS.md Integrity + +on: + pull_request: + push: + branches: [main] # or [master] — your default branch + +jobs: + check: + permissions: + contents: read + uses: Comfy-Org/github-workflows/.github/workflows/agents-md-integrity.yml@ + with: + workflows_ref: +``` + +Then add your repo to `vars.AGENTS_MD_CALLERS`. + +## Required permissions + +```yaml +contents: read +``` + +## Inputs + +| Input | Default | Notes | +|---|---|---| +| `max_lines` | `200` | Hard ceiling. Over this fails. | +| `warn_lines` | `150` | Warns without failing. | +| `forbid_cursorrules` | `true` | Fail on a legacy `.cursorrules`. | +| `check_nested` | `true` | Also check nested monorepo `AGENTS.md` files. | +| `require_shim` | `true` | A root `CLAUDE.md` shim must **exist** (and import `@AGENTS.md`). `false` still rejects a divergent `CLAUDE.md`, but tolerates its absence. | +| `require_codeowners` | `false` | Require a CODEOWNERS DRI for `AGENTS.md`. | +| `agents_file` | `AGENTS.md` | Override the filename. | +| `workflows_ref` | `main` | **Set to your `uses:` SHA** — the checker script loads from this ref. | + +## The `CLAUDE.md` shim + +Claude Code reads only `CLAUDE.md`, so the standard keeps content in `AGENTS.md` +and makes `CLAUDE.md` a two-line pointer. This repo's own is the reference: + +```markdown + +@AGENTS.md +``` + +## Gotchas + +**Adopt on `pull_request` before adding it as a required check.** An existing repo +with a 400-line `AGENTS.md` fails immediately; you want that visible on a PR, not +blocking the queue. + +**`check_nested: true` on a large monorepo** can surface a lot at once. Land the +top-level fix first, then switch it on. diff --git a/docs/callers/assign-prs-to-author.md b/docs/callers/assign-prs-to-author.md new file mode 100644 index 0000000..df77efc --- /dev/null +++ b/docs/callers/assign-prs-to-author.md @@ -0,0 +1,64 @@ +# `assign-prs-to-author.yml` — assign unassigned PRs to their authors + +Read [the shared caller contract](README.md) first. + +## What it does + +Housekeeping. Sweeps every open PR with no assignees and assigns it to its +author. Bot-authored PRs are skipped by default. + +Useful when a team tracks PR ownership through the assignee field — Comfy-Org +does — because a PR with no assignee is invisible to that view. + +## Prerequisites + +None. No secrets; it uses the caller's `GITHUB_TOKEN`. + +## Caller + +`.github/workflows/assign-prs-to-author.yml`: + +```yaml +name: Housekeeping - Assign PRs to Author + +on: + schedule: + - cron: '0 2 * * *' # daily at 02:00 UTC + workflow_dispatch: + +jobs: + assign: + permissions: + pull-requests: write + issues: write # assignees are set through the issues API + uses: Comfy-Org/github-workflows/.github/workflows/assign-prs-to-author.yml@ +``` + +## Required permissions + +```yaml +pull-requests: write +issues: write +``` + +**Both.** GitHub sets assignees through the *issues* API even for pull requests, +so `pull-requests: write` alone is not enough — and because the grant is validated +at startup, getting it wrong gives you a zero-job `startup_failure` with no logs. + +## Inputs + +| Input | Default | Notes | +|---|---|---| +| `skip-bots` | `true` | Skip PRs opened by bot accounts (Dependabot, Renovate, app tokens). Set `false` to assign those to their bot author too. | + +Note the hyphen — it is `skip-bots`, not `skip_bots`. + +## Gotchas + +**Run it on a schedule, never on `pull_request`.** It is a sweep over all open +PRs, not a per-PR reaction. Wiring it to PR events makes it do the same full scan +on every event. + +**It only touches PRs with *no* assignee.** An existing assignee is never +replaced, so it is safe to run alongside +[`assign-reviewers.yml`](assign-reviewers.md). diff --git a/docs/callers/assign-reviewers.md b/docs/callers/assign-reviewers.md new file mode 100644 index 0000000..2efd108 --- /dev/null +++ b/docs/callers/assign-reviewers.md @@ -0,0 +1,139 @@ +# `assign-reviewers.yml` — expertise-aware, load-balanced routing + +Read [the shared caller contract](README.md) first. + +## What it does + +Matches a PR's changed paths against a **caller-repo** `.github/reviewers.yml` +(path-glob → reviewers, plus a `default_pool`), drops the author and anyone in +`vars.REVIEWER_EXCLUDE`, ranks the remaining candidates by current open load +(steering off anyone at or over `vars.REVIEWER_LOAD_CAP`), may swap one slot for a +`vars.REVIEWER_GROWTH_POOL` member for new-folk randomization, and assigns the top +`num_reviewers`. + +> **Despite the name, it writes the ASSIGNEE field, not reviewer requests.** +> Comfy-Org routes and alerts people via assignees, so an entry under +> `reviewers:` means "gets assigned". + +The assignee write goes through the CLOUD_CODE_BOT App token rather than +`GITHUB_TOKEN`, so the calling job needs only `contents: read`. **This does not +buy fork support** — see the fork gotcha below. + +## Prerequisites + +| | | +|---|---| +| `vars.APP_ID` | **Required.** CLOUD_CODE_BOT app id. | +| `secrets.CLOUD_CODE_BOT_PRIVATE_KEY` | **Required.** | +| `.github/reviewers.yml` in **your** repo | **Required.** The expertise map. | +| `vars.REVIEWER_GROWTH_POOL` | Optional. Logins for new-folk randomization. | +| `vars.REVIEWER_LOAD_CAP` | Optional. Max open reviews before steering off. | +| `vars.REVIEWER_EXCLUDE` | Optional. Logins to hard-exclude. | + +## Caller + +`.github/workflows/assign-reviewers.yml`: + +```yaml +name: Assign Reviewers + +on: + pull_request: + types: [opened, ready_for_review] + +jobs: + assign: + # Same-repo, human-authored PRs only. On a fork PR — and on a Dependabot PR, + # which is same-repo but reads the Dependabot secret store — the secret below + # arrives empty, so the App-token step hard-fails and the PR carries a red X + # for a routing decision that could never have been made. See the gotcha. + if: >- + github.event.pull_request.head.repo.full_name == github.repository + && github.actor != 'dependabot[bot]' + permissions: + contents: read + uses: Comfy-Org/github-workflows/.github/workflows/assign-reviewers.yml@ + with: + num_reviewers: 2 + secrets: + CLOUD_CODE_BOT_PRIVATE_KEY: ${{ secrets.CLOUD_CODE_BOT_PRIVATE_KEY }} +``` + +Then add your repo to `vars.ASSIGN_REVIEWERS_CALLERS`. + +## Required permissions + +```yaml +contents: read +``` + +The assignee write goes through the App token. + +## Inputs + +| Input | Default | Notes | +|---|---|---| +| `reviewer_config_path` | `.github/reviewers.yml` | Where your expertise map lives. | +| `num_reviewers` | `2` | How many people to assign. | +| `skip_label` | `skip-auto-assign` | Present on a PR ⇒ skip routing. | + +## Your `reviewers.yml` + +```yaml +default_pool: # fallback when no rule matches — see the warning below + - octocat + - hubot + +rules: + - paths: + - services/api/** + - proto/** + reviewers: [alice, bob] + + - paths: + - infra/** + reviewers: [carol] +``` + +[This repo's own `reviewers.yml`](../../.github/reviewers.yml) is a worked example +with commentary on how the buckets were seeded. + +## Gotchas + +**Dependabot PRs need the same skip as forks, for a different reason.** They are +same-repo, so the fork guard alone lets them through — but Dependabot-triggered +runs read the *Dependabot* secret store rather than Actions secrets, so +`CLOUD_CODE_BOT_PRIVATE_KEY` is still empty. The reusable *does* skip bot authors +(`pr.user.type === 'Bot'`), but that check runs inside the job, **after** the +App-token mint has already failed — so the red X lands before the skip is reached. +Hence `&& github.actor != 'dependabot[bot]'` in the guard above. + +**Fork PRs are not routed, and the caller must skip them explicitly.** The +`pull_request` event withholds repository secrets from fork-originated runs, so +`CLOUD_CODE_BOT_PRIVATE_KEY` arrives empty and the App-token mint hard-fails — a +red check and no routing either way. Hence the `if:` guard in the caller above; a +skipped job reports as neutral instead. Routing forks would mean +`pull_request_target`, which runs privileged against untrusted head code while +this workflow reads `.github/reviewers.yml` from the head SHA — that combination +turns the expertise map into a real escalation path, so it is not offered. +[`ci-assign-reviewers.yml`](../../.github/workflows/ci-assign-reviewers.yml) in +this repo is the worked example. + +**A matched bucket does not fall back to `default_pool`.** `default_pool` is +consulted only when *no* rule matched the changed paths. The author is dropped +**after** that choice, so a rule whose reviewer list is just the PR author matches, +yields a one-person candidate set, loses that person to the author-drop, and ends +at `No eligible candidates after exclusions — nothing to assign`. It never reaches +`default_pool`. Write each bucket with at least one member who is not the usual +author of changes in those paths. + +**`default_pool` must never be one person** — and must not be whoever opens most +PRs in the repo. When it *is* used (no rule matched), the same author-drop applies, +so a single-member pool that happens to be the author assigns nobody. + +**Globs are the durable part; names are not.** People change teams. Write bucket +globs deliberately and expect the roster inside them to churn. + +**Seeding from commit authorship is usually wrong.** On a repo where most commits +have one author, authorship carries no routing signal — seed from who has +**approved** merged PRs in that area instead. diff --git a/docs/callers/cursor-review-auto-label.md b/docs/callers/cursor-review-auto-label.md new file mode 100644 index 0000000..2dc22b2 --- /dev/null +++ b/docs/callers/cursor-review-auto-label.md @@ -0,0 +1,100 @@ +# `cursor-review-auto-label.yml` — apply the review label for opted-in reviewers + +Read [the shared caller contract](README.md) first. + +## What it does + +Companion to [`cursor-review.yml`](cursor-review.md). When a person becomes +responsible for a PR, this applies the review label that `cursor-review.yml` +triggers on — keeping the "fire on label" contract intact instead of bolting a +second trigger onto the review itself. + +The opt-in roster lives in **your** repo as +`vars.CURSOR_REVIEW_OPTED_IN_LOGINS` (whitespace-separated GitHub logins). No +roster is baked into the workflow. + +## Prerequisites + +| | | +|---|---| +| `vars.APP_ID` | **Required.** The CLOUD_CODE_BOT app id. | +| `secrets.CLOUD_CODE_BOT_PRIVATE_KEY` | **Required.** | +| `vars.CURSOR_REVIEW_OPTED_IN_LOGINS` | The opt-in roster. Empty means nobody gets auto-labeled. | + +## Caller + +`.github/workflows/cursor-review-auto-label.yml`: + +```yaml +name: Cursor Review Auto-Label + +on: + pull_request: + # `assigned` is the core behavior. Add `opened` / `ready_for_review` to also + # label at PR creation time. + types: [assigned, opened, ready_for_review] + +jobs: + auto-label: + # Same-repo, human-authored PRs only. Both fork PRs and Dependabot PRs run + # without Actions secrets, so the App-token mint fails on either. See the + # gotcha. + if: >- + github.event.pull_request.head.repo.full_name == github.repository + && github.actor != 'dependabot[bot]' + permissions: + contents: read + uses: Comfy-Org/github-workflows/.github/workflows/cursor-review-auto-label.yml@ + secrets: + CLOUD_CODE_BOT_PRIVATE_KEY: ${{ secrets.CLOUD_CODE_BOT_PRIVATE_KEY }} +``` + +## Required permissions + +```yaml +contents: read +``` + +Only that. The label write goes through the App token, not `GITHUB_TOKEN`. + +## Inputs + +| Input | Default | Notes | +|---|---|---| +| `review_label` | `cursor-review` | Must match `cursor-review.yml`'s `review_label`. | +| `skip_label` | `skip-cursor-review` | Present on a PR ⇒ never auto-label it. | +| `runs_on` | `'"ubuntu-latest"'` | JSON. Set for self-hosted runners. | + +## Why the App token is mandatory + +A label applied with the default `GITHUB_TOKEN` **does not trigger workflow +runs** — GitHub suppresses events raised by that token to prevent recursion. The +label would appear on the PR and no review would start, with nothing in any log +to explain it. Hence `vars.APP_ID` and the App private key are required rather +than optional here. + +## Gotchas + +**Dependabot PRs fail the same way, and the fork guard alone misses them.** +Dependabot branches live in the base repo, so they pass the cross-repo test, but +Dependabot-triggered runs read the *Dependabot* secret store rather than Actions +secrets — the private key is empty and the mint fails. This workflow has no +bot-author skip of its own, so the caller-level `&& github.actor != +'dependabot[bot]'` above is what keeps dependency PRs green. + +**Guard the job against fork PRs yourself.** `pull_request` withholds secrets from +fork-originated runs, so `CLOUD_CODE_BOT_PRIVATE_KEY` arrives empty and the +App-token mint hard-fails — e.g. the moment a maintainer assigns a reviewer to an +external contribution, that PR picks up a red check. The reusable does **not** +carry a fork guard of its own, so add the `if:` shown in the caller above. Nothing +is lost: [`cursor-review.yml`](cursor-review.md) skips fork PRs anyway, so the +label would have had nothing to trigger. + +**Which moments fire is the caller's choice.** This workflow reacts to whatever +`pull_request` event you pass through; it does not pick triggers for you. + +**Keep `review_label` in sync** between this workflow and `cursor-review.yml`. A +mismatch labels PRs that nothing is listening for. + +**An auto-applied label is not proof a review ran.** Check the run, not the +label. diff --git a/docs/callers/cursor-review.md b/docs/callers/cursor-review.md new file mode 100644 index 0000000..1704240 --- /dev/null +++ b/docs/callers/cursor-review.md @@ -0,0 +1,176 @@ +# `cursor-review.yml` — label-triggered multi-model code review + +Read [the shared caller contract](README.md) first. + +## What it does + +A 4-lab × 2-review-type `cursor-agent` panel runs adversarial and edge-case +passes over the PR diff. A judge model consolidates them into **one** PR review +with per-finding severity badges. The person who applied the label gets Slack +start/complete DMs. + +**Advisory only, and there is currently no supported way to make it blocking.** +The panel posts the review and succeeds regardless of what it found; no input +fails the run on findings. + +Do **not** try to build a gate by marking `… / Consolidate panel` a required +check. GitHub counts a *skipped* required check as passing, and that job is +`if:`-gated on five conditions — it skips when the trigger label is absent, when a +review already exists for the head SHA (the dedupe below), when the diff is over +`diff_size_cap`, on fork PRs, and when the panel itself is skipped. So the check +goes green in exactly the cases where no review ran, which is the opposite of a +gate. + +A real opt-in gate did exist — a `blocking:` input and a fail-closed **Blocking +gate** job — but both were dropped from `cursor-review.yml` in +[#31](https://github.com/Comfy-Org/github-workflows/pull/31), which was otherwise +a judge-extraction fix. Its script (`.github/cursor-review/gate-unresolved.py`) +is still in the tree, orphaned. Restoring it is tracked separately; until then, +treat this review as advisory and gate on human approval. + +Prompts and scripts live in [`.github/cursor-review/`](../../.github/cursor-review) +— the single source of truth, so your repo carries only a thin caller. + +## Prerequisites + +| | | +|---|---| +| `secrets.CURSOR_API_KEY` | **Required.** Org-level in Comfy-Org. | +| `secrets.SLACK_BOT_TOKEN` | Optional. Without it the review still posts; only the DMs are skipped. | +| `vars.REVIEW_BOT_APP_ID` + `secrets.BOT_APP_PRIVATE_KEY` | Optional. Posts the review as your App instead of `github-actions[bot]`. | +| A review label | Default `cursor-review`. Create it in your repo. | + +## Caller + +`.github/workflows/cursor-review.yml`: + +```yaml +name: Cursor Review + +on: + pull_request: + # Label-gated mode. If you set `run_without_label: true` below, this list + # must also carry [opened, reopened, ready_for_review, synchronize] — see + # the gotcha at the bottom. + types: [labeled, unlabeled] + +concurrency: + # cursor-review declares no group of its own, so a caller-level group is safe + # and worth having — it stops label-toggling from stacking panels. + # NOTE: label.name is part of the key only because this caller is label-only. + # Drop it if you widen `types:` — see the run_without_label gotcha. + group: cursor-review-pr-${{ github.event.pull_request.number }}-${{ github.event.label.name }} + cancel-in-progress: true + +jobs: + review: + permissions: + contents: read + pull-requests: write + uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@ + with: + workflows_ref: + bot_app_id: ${{ vars.REVIEW_BOT_APP_ID }} + secrets: + CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }} + SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} + BOT_APP_PRIVATE_KEY: ${{ secrets.BOT_APP_PRIVATE_KEY }} +``` + +Then add your repo to `vars.CURSOR_REVIEW_CALLERS`. + +## Required permissions + +```yaml +contents: read +pull-requests: write # posting the consolidated review +``` + +## Inputs + +| Input | Default | Notes | +|---|---|---| +| `judge_model` | `claude-opus-4-8-thinking-max` | Consolidates the panel into one review. | +| `diff_size_cap` | `5000` | Skip review above this diff size. | +| `review_label` | `cursor-review` | The label that triggers a run. | +| `diff_excludes` | lockfiles, `node_modules`, `.claude`, `dist`, `vendor`, `*.generated.*`, `*.min.js` | Paths kept out of **both** the size-budget count and the reviewed diff. Passing your own value **replaces** the default list, so re-state the entries you still want. | +| `workflows_ref` | `main` | **Set to your `uses:` SHA** — prompts load from this ref at run time. | +| `bot_app_id` | `''` | Post as your App. | +| `run_without_label` | `false` | Run on every PR rather than waiting for the label. **Also requires widening your caller's `types:`** — see the gotcha. | + +## Gotchas + +**The label must be applied by a GitHub App token, not `GITHUB_TOKEN`.** Events +raised by `GITHUB_TOKEN` do not trigger workflow runs, so a label applied by +another workflow using the default token silently fails to start a review. That +is exactly what [`cursor-review-auto-label.yml`](cursor-review-auto-label.md) +exists to handle. + +**Applying the label does not guarantee a run.** If the event was swallowed, +remove the label, confirm it is gone, then re-add it. + +**One review per commit — re-labeling alone will not re-review.** The gate skips +the panel if a non-dismissed consolidated review already exists for the PR's HEAD +SHA, so a remove-and-re-add on unchanged content no-ops by design. To get a fresh +review of the same commit, **dismiss the existing review first**, then apply the +label. + +Pushing a commit clears the dedupe (new head SHA), but with the label-gated caller +above it does **not** by itself start a run — `types: [labeled, unlabeled]` omits +`synchronize`, so a push delivers no event at all. After pushing you still toggle +the label. Add `synchronize` to `types:` if you want every push re-reviewed (and +see the spend warning below). + +**Dependabot PRs are not covered by the fork skip.** Dependabot's branches live in +the base repo, so the gate's cross-repo check treats them as ordinary PRs — but +Dependabot-triggered runs read the *Dependabot* secret store, not Actions secrets. +`CURSOR_API_KEY` therefore arrives empty and the token is read-only, so under +`run_without_label: true` (or a `synchronize` trigger) every dependency PR burns +the matrix and fails red. Keep those PRs out with a caller-level guard: + +```yaml + if: github.actor != 'dependabot[bot]' +``` + +**Fork PRs are skipped, deliberately.** `pull_request` withholds secrets from +fork-originated runs, so `CURSOR_API_KEY` would be empty (every panel cell +produces nothing) and `GITHUB_TOKEN` read-only (posting the review 403s). The gate +detects the cross-repo head and skips cleanly rather than burning the matrix and +failing red on every external contribution. Do not "fix" this with +`pull_request_target` — that runs privileged against untrusted head code. + +**`run_without_label: true` needs a wider trigger, or it never fires.** The input +only tells the reusable's gate to accept `opened` / `reopened` / +`ready_for_review` / `synchronize`; it cannot add those events to *your* `on:` +block. A caller left on `types: [labeled, unlabeled]` therefore still runs only +when a label is toggled — ordinary PRs are silently never reviewed, with nothing +in any log to explain it. Set both together: + +```yaml +on: + pull_request: + types: [opened, reopened, ready_for_review, synchronize, labeled, unlabeled] + +concurrency: + # Drop `label.name` from the key — see below. + group: cursor-review-pr-${{ github.event.pull_request.number }} + cancel-in-progress: true +# ... + with: + run_without_label: true +``` + +**Drop `github.event.label.name` from the concurrency group when you widen +`types:`.** That expression is empty on `opened`/`synchronize`/`reopened`, so +label events and plain PR events resolve to *different* groups and cannot cancel +each other. A push racing a label toggle then runs two 8-cell panels +concurrently — and because the head-SHA dedupe is evaluated before either posts, +both pass it and you get two panels and two reviews. Keying on the PR number +alone keeps every trigger in one group. + +Keep `labeled`/`unlabeled` in the list even in label-free mode: the label path +stays live alongside it, which is how you force a re-review on an unchanged commit +(dismiss the existing review, then apply the label — see the dedupe gotcha below). + +**`run_without_label: true` reviews every PR.** On a busy repo that is a large +step up in spend. Start label-gated. diff --git a/docs/callers/detect-unreviewed-merge.md b/docs/callers/detect-unreviewed-merge.md new file mode 100644 index 0000000..2b76864 --- /dev/null +++ b/docs/callers/detect-unreviewed-merge.md @@ -0,0 +1,89 @@ +# `detect-unreviewed-merge.yml` — SOC 2 unreviewed-merge detection + +Read [the shared caller contract](README.md) first. + +## What it does + +Detects a PR merged **without prior approval** and opens a tracking issue in +[`Comfy-Org/unreviewed-merges`](https://github.com/Comfy-Org/unreviewed-merges). +This is SOC 2 compliance evidence: the control is "changes are reviewed", and this +is the detective control that catches exceptions. + +It reports; it does not block. Blocking is branch protection's job. + +## Prerequisites + +| | | +|---|---| +| `secrets.UNREVIEWED_MERGES_TOKEN` | **Required.** Fine-grained PAT with `issues: write` on `Comfy-Org/unreviewed-merges`. | + +The token needs write on the *tracking* repo, not on yours — findings are +centralized so auditors read one place. + +## Caller + +`.github/workflows/detect-unreviewed-merge.yml`: + +```yaml +name: Detect Unreviewed Merge + +on: + push: + branches: [main] # or [master] — your default branch + +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@ + with: + approval-mode: latest-per-reviewer # 'any-approval' for private repos + secrets: + UNREVIEWED_MERGES_TOKEN: ${{ secrets.UNREVIEWED_MERGES_TOKEN }} +``` + +It triggers on **push to the default branch**, not on a `pull_request` event — +the merge commit is the thing being audited, so the check runs after the merge +lands. `concurrency` is keyed by `github.sha` for the same reason. + +## Required permissions + +```yaml +contents: read +pull-requests: read +``` + +Read-only on your repo. The issue write happens on the tracking repo via the PAT. + +## Inputs + +| Input | Default | Notes | +|---|---|---| +| `approval-mode` | `latest-per-reviewer` | Which historical approvals count. Pick deliberately — see below. | + +### Choosing `approval-mode` + +- **`latest-per-reviewer`** — for OSS repos that have *"dismiss stale reviews on + new commits"* enabled. A dismissed approval does **not** count, matching what + branch protection actually enforced. +- **`any-approval`** — for private repos **without** stale-dismissal. Any + historical `APPROVED` counts. + +Getting this backwards produces audit noise in one direction or false confidence +in the other. Check the repo's branch-protection settings, then pick. + +## Gotchas + +**Use this repo's path, not the old one.** Some older examples reference +`Comfy-Org/unreviewed-merges/.github/workflows/detector.yml` — that path **does +not exist** and will not resolve. The workflow lives here, in +`Comfy-Org/github-workflows`, which is what the ~11 live callers use. + +**There is no `*_CALLERS` roster for this one yet**, so pin bumps are manual. +Check periodically that your pin is not far behind `main`. diff --git a/docs/callers/groom.md b/docs/callers/groom.md new file mode 100644 index 0000000..7597048 --- /dev/null +++ b/docs/callers/groom.md @@ -0,0 +1,188 @@ +# `groom.yml` — scheduled code-cleanup sweep + +Read [the shared caller contract](README.md) first — pinning, permissions, and the +roster step apply here and are not repeated below. + +## What it does + +A **read-only FINDER** agent scans a clean default-branch checkout — the whole +repo, not a diff — for high-value refactors. An **independent VERIFIER** agent in +a fresh session re-checks each finding as CONFIRM / DOWNGRADE / REJECT and +attaches a stable dedup signature. Survivors are deduped against a durable +GitHub-issue-state ledger and filed as `groom`-labeled issues. Security-adjacent +findings get `groom-security` instead — investigate, don't auto-implement. + +Default mode is **finds-only: no commits, no PRs, and it never merges.** + +The model steps hold **no write credentials.** The `audit` jobs are +`contents: read`; filing happens in a separate job as a GitHub App you nominate. +Preserve that split — see [SECURITY.md](../../SECURITY.md). + +## Prerequisites + +| | | +|---|---| +| `secrets.ANTHROPIC_API_KEY` | **Required.** The finder and verifier bill through it. Available org-wide in Comfy-Org. | +| `vars.APP_ID` + `secrets.CLOUD_CODE_BOT_PRIVATE_KEY` | Optional. Files issues as cloud-code-bot instead of `github-actions[bot]`, so groom's output is a distinct, queryable actor. | + +## Minimal caller — finds-only + +Put this at `.github/workflows/groom.yml` in your repo: + +```yaml +name: Groom + +on: + schedule: + # Frequent BASE cron (daily). Effective cadence is the runtime `interval_days` + # gate below, not this cron — GitHub Actions cron is static in the file, so a + # daily tick + a runtime gate is how you get a tunable cadence with no + # workflow-file edit. Pick a NON-round minute and stagger it against other + # repos — top-of-hour is the most congested slot on GitHub's scheduler. + - cron: '17 9 * * *' + workflow_dispatch: # bypasses the interval gate (not the volume gate) + inputs: + dry_run: + description: Run the full audit but do NOT open issues — print what it would file. + type: boolean + default: false + +# NOTE: deliberately no caller-level `concurrency:` — see "Footguns" below. + +jobs: + groom: + permissions: + contents: read + issues: write + pull-requests: read + actions: read # the interval gate reads run history for the last real run + uses: Comfy-Org/github-workflows/.github/workflows/groom.yml@ + with: + workflows_ref: + bot_app_id: ${{ vars.APP_ID }} + # Cadence knob + the matching volume-gate window (BE-4004). Wire both to + # one repo Actions variable so they can't drift: retune weekly -> + # every-3-days -> daily by editing `GROOM_INTERVAL_DAYS`, no workflow edit. + interval_days: ${{ vars.GROOM_INTERVAL_DAYS || '7' }} + cadence: ${{ vars.GROOM_INTERVAL_DAYS || '7' }} + # `github.event.inputs` is null on a schedule event, so '' != 'true' -> false. + # Scheduled runs always file live; only a manual dispatch can dry-run. + dry_run: ${{ github.event.inputs.dry_run == 'true' }} + secrets: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + BOT_APP_PRIVATE_KEY: ${{ secrets.CLOUD_CODE_BOT_PRIVATE_KEY }} +``` + +Then add your repo to `vars.GROOM_CALLERS` — see +[Staying current](README.md#staying-current). + +## Required permissions + +```yaml +contents: read +issues: write # the `file` job +pull-requests: read # the `build_select` job +actions: read # the interval gate (BE-4004) — reads this workflow's run + # history to find the last real groom run +``` + +Grant all four **even when `bot_app_id` is set** and the App token does the +actual writing. GitHub validates the grant at startup against what the nested +jobs *declare*, not against what they end up using. A short grant produces a +zero-job `startup_failure` with no logs — omitting `actions: read` specifically +rejects the run with "requesting 'actions: read', but is only allowed +'actions: none'" rather than degrading to a fail-open daily run. + +## Inputs + +All are optional. The ones that matter: + +| Input | Default | Why you'd change it | +|---|---|---| +| `interval_days` | `7` | Effective run cadence (BE-4004): on the daily base cron, a tick within this many days of the last REAL run early-exits before the finder. Wire it to a `GROOM_INTERVAL_DAYS` repo Actions variable so cadence is a variable edit, not a workflow-file change. `workflow_dispatch` bypasses this gate (not `volume_gate`). `0` disables the throttle. | +| `cadence` | `7` | Volume-gate window in days — set to match `interval_days` (wire both to the same variable) so the merge-activity check lines up with how often a real run can happen. Feeds the volume gate below. | +| `volume_gate` | `true` | Skips the (expensive) audit when nothing merged in `cadence` days. **This is the cost control — leave it on** for scheduled runs. | +| `dry_run` | `false` | Full audit + dedup, files nothing, prints what it would file. Use before the first live run. | +| `max_findings` | `12` | Cap issues per run. Lower it on a first pilot. | +| `model` | `claude-opus-5` | The finder/verifier model. | +| `themes` | `duplication, inconsistent patterns, missing abstractions, complexity hotspots, dead code` | Steer the finder at particular kinds of cleanup. The default mirrors the finder brief's own five dimensions, so it is a no-op; **narrow** it (e.g. `duplication, dead code`) to focus a repo. Security/auth-adjacent findings are filed regardless of theme. | +| `scope_label` / `scope_desc` | `whole-repo` | Cosmetic labels for the scope in issue bodies. | +| `workflows_ref` | `main` | **Set to your `uses:` SHA.** Briefs load from this ref at run time. | +| `bot_app_id` | `''` | File as your App rather than `github-actions[bot]`. | +| `builder` | `false` | Opt into PR-writing — see below. | +| `max_prs` | `'5'` | Only with `builder: true`. Typed **string**, deliberately. | +| `pr_size_limit` | `400` | Only with `builder: true`. Caps a built PR's diff. | + +## Opt-in auto-builder + +With `builder: true`, the top `max_prs` CONFIRMED, non-security findings become +**review-gated PRs** — full CI plus cursor-review, **never auto-merged** — +instead of issues. Security findings still file as `groom-security` issues; the +builder skips them. + +The security boundary holds: a credential-free `build` job emits only a patch +artifact, and a separate `build_pr` job opens the PR as the bot. Requires +`bot_app_id`. The ledger tracks PR state (open/merged/closed) so a built finding +is not re-proposed. + +`max_prs` is a **string, not a number**, so a caller can forward its own +`workflow_dispatch` input straight through without a `fromJSON()` cast, and an +operator can raise the ceiling for one manual run: + +```yaml + inputs: + max_prs: + description: How many PRs this run may open. + type: string + default: '1' +# ... + with: + builder: true + max_prs: ${{ github.event.inputs.max_prs || '1' }} +``` + +Parsing and clamping happen once inside the reusable: empty → default, +non-numeric → 0 PRs plus a warning. Never a failed run. + +Start at `max_prs: 1`. The current large-repo builder pilot runs at exactly that. + +## Footguns + +**Never declare `concurrency: groom-…` in your caller.** `groom.yml` already +declares `concurrency: groom-${{ github.repository }}` with +`cancel-in-progress: false` — the TOCTOU guard for its read-then-file ledger. +Duplicating that group **deadlocks the run**: your caller holds the group while +its `uses:` job waits for the same group, and neither yields until timeout. + +**Keep `workflows_ref` in lock-step with `uses:`.** The finder and verifier briefs +and the dedup ledger load from `workflows_ref` while running. Left at the default +`main`, a SHA-pinned caller runs an old workflow against today's briefs. + +**Bump the pin and `workflows_ref` together.** The two must agree, so a run always +loads the briefs matching the workflow that files them. + +## Before you trust the schedule + +```bash +gh workflow run groom.yml --repo / -f dry_run=true +gh run list --repo / --workflow groom.yml --limit 1 +``` + +A dry run exercises the full audit and dedup and prints what it *would* file to +the job summary. On a large repo it also proves the finder completes inside the +timeout before you find that out on a Monday morning. + +Note that `volume_gate` can gate a dry run down to nothing on a quiet week. If +you want the dry run to always do the full audit, bypass the gate for dispatches +only — live scheduled runs keep it on so a quiescent repo still skips the spend: + +```yaml + volume_gate: ${{ github.event.inputs.dry_run != 'true' }} +``` + +## Cost + +Each ungated run is a whole-repo Opus finder pass plus an independent verifier +pass. `volume_gate: true` means a week with nothing merged costs nothing. That +gate plus a weekly cron is what makes broad enrollment affordable — a repo that +went quiet stops billing on its own. diff --git a/docs/callers/pr-size.md b/docs/callers/pr-size.md new file mode 100644 index 0000000..aca64f0 --- /dev/null +++ b/docs/callers/pr-size.md @@ -0,0 +1,92 @@ +# `pr-size.yml` — PR-size cap + +Read [the shared caller contract](README.md) first. + +## What it does + +Fails a PR whose net diff exceeds `max_lines` **non-generated** changed lines, to +keep diffs reviewable. In `mode: warn` it reports without failing. + +Excluded from the count: dependency lockfiles, `linguist-generated` files (read +from the **base ref**, so a PR cannot exempt itself by editing +`.gitattributes`), Go generated-code markers, and anything you add via +`extra_lockfiles` / `extra_generated_globs`. + +The counting logic and its tests live in +[`scripts/check-pr-size/`](../../scripts/check-pr-size) and are compiled from +source at run time. + +## Prerequisites + +None required. Optionally `vars.APP_ID` + `secrets.BOT_APP_PRIVATE_KEY` for the +explanatory sticky comment. + +## Caller + +`.github/workflows/pr-size.yml`: + +```yaml +name: PR Size Cap + +on: + pull_request: + branches: [main] # or [master] — YOUR default branch; see the gotcha + # labeled/unlabeled are beyond the default set, so toggling the bypass label + # re-runs the check immediately instead of needing a push. + types: [opened, synchronize, reopened, labeled, unlabeled] + +jobs: + size: + permissions: + contents: read + uses: Comfy-Org/github-workflows/.github/workflows/pr-size.yml@ + with: + workflows_ref: + max_lines: 1000 + bot_app_id: ${{ vars.APP_ID }} + secrets: + BOT_APP_PRIVATE_KEY: ${{ secrets.BOT_APP_PRIVATE_KEY }} +``` + +Then add your repo to `vars.PR_SIZE_CALLERS`. + +## Required permissions + +```yaml +contents: read +``` + +## Inputs + +| Input | Default | Notes | +|---|---|---| +| `max_lines` | `1000` | Net non-generated changed lines allowed. | +| `mode` | `enforce` | `warn` reports without failing — the right first step on an existing repo. | +| `bypass_label` | `oversized-ok` | Waves through a legitimately large change. | +| `extra_lockfiles` | `''` | Additional lockfiles to exclude. | +| `extra_generated_globs` | `''` | Additional generated-path globs to exclude. | +| `comment` | `true` | Sticky bot comment explaining an overage. | +| `bot_app_id` | `''` | Without it, degrades to status + step summary. | +| `workflows_ref` | `main` | **Set to your `uses:` SHA** — the tool is built from this ref. | + +## Gotchas + +**`branches:` must name your actual default branch.** The filter matches the PR's +**base**, so a repo whose default is `master` (or `develop`, or a release train) +gets *no* runs from the snippet above — the check silently never fires, no status +appears, and nothing signals the misconfiguration. Adapt the value, or drop the +`branches:` filter to size every PR regardless of base. + +**Start in `mode: warn`.** Turning on enforcement over an existing PR queue fails +open PRs that were fine when opened. Warn for a cycle, read the numbers, then +enforce. + +**Without `bot_app_id` the bypass route is not discoverable.** The failing status +alone does not mention `oversized-ok`; the sticky comment is what tells an author +the escape hatch exists. Supply the App or expect confused authors. + +**Go workspaces:** a consumer with a root `go.work` needs `GOWORK=off` for the +tool build, since `go build` otherwise discovers the consumer's workspace. + +**Do not commit the compiled binary.** `pr-size.yml` builds it from source into +`RUNNER_TEMP` on every run. diff --git a/docs/callers/stale.md b/docs/callers/stale.md new file mode 100644 index 0000000..fec752d --- /dev/null +++ b/docs/callers/stale.md @@ -0,0 +1,94 @@ +# `stale.yml` — stale-PR sweeper + Slack digest + +Read [the shared caller contract](README.md) first. + +## What it does + +Runs [`actions/stale`](https://github.com/actions/stale) over PRs, then posts a +Slack digest of what it touched. PRs inactive for `days_before_pr_stale` get the +`stale` label; still-inactive PRs are closed after +`days_before_pr_close` more days. + +The digest names the source repo in the header **and on every PR line** +(`your-org/your-repo#123`), so batches from different repos posted to the same +channel stay unambiguous line by line. + +Issues are untouched — this is PR-only. + +## Prerequisites + +`secrets.SLACK_BOT_TOKEN` is optional. Without it the sweep still runs and only +the Slack heads-up is skipped. + +## Caller + +`.github/workflows/stale.yml`: + +```yaml +name: Stale PRs + +on: + schedule: + - cron: '0 3 * * *' + workflow_dispatch: + inputs: + dry-run: + description: Preview without applying + type: boolean + default: false + +permissions: + pull-requests: write + issues: write # required by actions/stale even for PR-only runs + +concurrency: + # stale.yml declares no group of its own, so a caller-level group is safe. + group: stale + cancel-in-progress: false + +jobs: + stale: + uses: Comfy-Org/github-workflows/.github/workflows/stale.yml@ + with: + slack_channel: # see the gotcha below + dry_run: ${{ inputs.dry-run || false }} + secrets: + SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} +``` + +## Required permissions + +```yaml +pull-requests: write +issues: write +``` + +`issues: write` is required by `actions/stale` itself even on a PR-only run. + +## Inputs + +| Input | Default | Notes | +|---|---|---| +| `slack_channel` | an internal Comfy-Org channel id | **Set this explicitly.** See below. | +| `days_before_pr_stale` | `14` | Inactivity before labeling. | +| `days_before_pr_close` | `14` | Further inactivity before closing. | +| `stale_pr_label` | `stale` | The label applied. | +| `exempt_pr_labels` | `pinned,security,work-in-progress,wip,dependencies,do-not-merge` | Never swept. Passing your own value **replaces** this list — re-state the entries you still want. | +| `stale_pr_message` | *(see workflow)* | Comment posted when labeling. | +| `close_pr_message` | *(see workflow)* | Comment posted when closing. | +| `dry_run` | `false` | Report without labeling or closing. | + +## Gotchas + +**Always set `slack_channel` explicitly.** The default is Comfy-Org's internal +channel id. A caller outside that workspace inherits it, tries to post somewhere +it has no business posting, and gets a confusing Slack API error rather than a +clear misconfiguration. + +**Dry-run first, on a repo with a backlog.** `dry_run: true` shows you the set it +would label and close. On a repo with old open PRs, the first live run can close a +lot at once — and closing someone's six-month-old PR without warning is a people +problem, not a CI problem. + +**`exempt_pr_labels` is your safety valve.** Add whatever your repo uses for +long-lived work before the first live run, not after. diff --git a/scripts/check-pr-size/check-pr-size b/scripts/check-pr-size/check-pr-size deleted file mode 100755 index 998fb92..0000000 Binary files a/scripts/check-pr-size/check-pr-size and /dev/null differ