diff --git a/.github/bump-callers/README.md b/.github/bump-callers/README.md index d3544c3..64f341a 100644 --- a/.github/bump-callers/README.md +++ b/.github/bump-callers/README.md @@ -28,6 +28,24 @@ forward automatically instead of silently drifting commits behind. | [`bump-pr-size-callers.yml`](../workflows/bump-pr-size-callers.yml) | `pr-size.yml` or `scripts/check-pr-size/**` | `PR_SIZE_CALLERS` | empty `[]` (grows as callers land) | | [`bump-assign-reviewers-callers.yml`](../workflows/bump-assign-reviewers-callers.yml) | `assign-reviewers.yml` | `ASSIGN_REVIEWERS_CALLERS` | empty `[]` (grows as callers land) | | [`bump-groom-callers.yml`](../workflows/bump-groom-callers.yml) | `groom.yml` or `groom/**` | `GROOM_CALLERS` | empty `[]` (grows as callers land) | +| [`bump-auto-label-callers.yml`](../workflows/bump-auto-label-callers.yml) | `cursor-review-auto-label.yml` | `AUTO_LABEL_CALLERS` | non-empty (hard-fails if empty) | + +### Reusables with no fleet — deliberate, not an oversight + +| Reusable | Callers | Why no fleet | +|---|---|---| +| `stale.yml` | 0 | Nothing to bump. Add a fleet when the first caller lands. | +| `assign-prs-to-author.yml` | 0 | Same. | +| `detect-unreviewed-merge.yml` | ~12 | **A real gap.** Its pins are bumped by hand. Deferred deliberately, not missed. | + +A reusable that has callers but no fleet is the trap this whole directory exists +to prevent: the pins simply never move, so consumers drift behind indefinitely +and only find out when the caller and the reusable stop being compatible. That is +not hypothetical — the groom fleet omitted *its own* caller (`ci-groom.yml`) from +`GROOM_CALLERS`, the pin sat unchanged from the day it was written, and the caller +ended up failing at startup against a reusable it had drifted away from. Before +adding a caller anywhere, check that its fleet exists **and** that the repo is in +the variable; the second half is the one people skip. They stay as thin entrypoints rather than one matrix because their triggers differ: a `cursor-review.yml` change must not spuriously bump agents-md or @@ -36,7 +54,11 @@ flow, the trailing-newline fix, the single-line PR body) lives once in `bump-callers.sh`. Registering a new fleet is: add a thin entrypoint (copy an existing one, swap the path filter + `VAR_NAME`/`TAG`/`WORKFLOW_FILE`/ `ALLOW_EMPTY`), seed its variable, and add a row to this table + the paths in -`test-bump-callers.yml`. +`test-bump-callers.yml`. **Then `workflow_dispatch` the new entrypoint once.** +Landing a fleet does not touch the reusable it watches, so its own merge matches +no path filter and fires no run — callers that were already stale when the fleet +was created stay stale until the reusable next changes. Every entrypoint carries +`workflow_dispatch` for exactly this. The **groom** fleet is the one that most needs this: a groom caller pins the reusable **twice** — the `uses:` SHA *and* the `workflows_ref:` input that loads @@ -57,6 +79,14 @@ repo-level Actions **variable** (config, not a credential) as a JSON array of `{"repo","file","label"}` objects (`label` optional). `bump-callers.sh` `::add-mask::`es every repo name out of the run logs before echoing it. +> **Known gap.** Each entrypoint hands the roster to the script through the +> step's `env:`, and Actions prints a step's env block *before* the step runs — +> so the raw roster appears in the (public) log ahead of any masking. Closing it +> means fetching the variable at run time (`gh variable get`) and masking it +> before first use, which needs a token permission the fleets do not mint today. +> It is fleet-wide; no single entrypoint can fix it. Until then, assume the +> roster is public. + Adding/removing a caller needs **no public commit** — edit the variable: ```bash diff --git a/.github/workflows/bump-auto-label-callers.yml b/.github/workflows/bump-auto-label-callers.yml new file mode 100644 index 0000000..40bf8be --- /dev/null +++ b/.github/workflows/bump-auto-label-callers.yml @@ -0,0 +1,200 @@ +name: Bump cursor-review-auto-label callers + +# When the cursor-review-auto-label reusable workflow is updated on main, open a +# SHA-bump PR in every repo that pins a caller against it. PRs are opened by +# Cloud Code Bot so they are easy to filter and merge. +# +# WHY THIS FLEET EXISTS (BE-4682): auto-label had callers but no bumper. Its +# pins therefore never moved — every consumer stayed frozen on whatever SHA it +# was first wired to, drifting behind the reusable indefinitely. That is not a +# theoretical rot: the sibling groom fleet left its own caller +# (ci-groom.yml) off GROOM_CALLERS, the pin never bumped, and the caller +# eventually failed at startup against a reusable it was no longer compatible +# with. A reusable with callers and no bumper is the same trap with a longer +# fuse. +# +# This is a thin entrypoint over the shared bumper at +# .github/bump-callers/bump-callers.sh — the same script drives the +# cursor-review, agents-md-integrity, pr-size, assign-reviewers AND groom +# caller fleets. Keeping ONE implementation is deliberate: a forked copy is how +# other shared machinery in the org has drifted. The entrypoints differ only in +# their path-filter trigger and the parameters passed below. (They stay separate +# rather than a single matrix because their triggers differ — a +# cursor-review-auto-label.yml change must not spuriously bump the other fleets' +# callers, and vice versa.) +# +# NOTE ON THE PAIRING: auto-label is the companion to cursor-review.yml, but it +# is a SEPARATE fleet on purpose. The two reusables are pinned independently by +# consumers (many repos run cursor-review without auto-label), so folding them +# together would bump callers of one on a change to the other. +# +# The caller list is NOT hardcoded here. This repo is PUBLIC (workflow file and +# Actions run logs are both publicly viewable), and most callers are private, so +# their names must never appear in this file or its logs. The list lives in the +# repo-level Actions variable `AUTO_LABEL_CALLERS` (config, not a credential — a +# variable, not a secret, since secrets are write-only via the API) as a JSON +# array of {"repo","file","label"} objects, same shape as CURSOR_REVIEW_CALLERS. +# `bump-callers.sh` `::add-mask::`es every repo name before IT echoes anything. +# KNOWN GAP (shared by all six entrypoints, tracked separately): passing the +# roster through this step's `env:` means Actions prints the raw value in the +# step's env dump before the script — and its masking — ever runs. Closing it +# means reading the variable at run time (`gh variable get`) and masking it +# first, which needs a token permission this fleet does not mint today; it is a +# fleet-wide change, not one this entrypoint can make alone. Until then, treat +# the roster as visible in the public run log. +# Unlike the fleets seeded empty, this one is seeded NON-EMPTY (the +# callers already existed before the bumper did), so ALLOW_EMPTY is false below: +# an empty list here means the variable was clobbered, not that the fleet has no +# members, and failing loudly beats silently bumping nothing. +# +# Update flow — adding/removing a caller needs NO public commit: +# gh variable set AUTO_LABEL_CALLERS --repo Comfy-Org/github-workflows \ +# --body "$(jq -c . callers.json)" +# Keep the canonical callers.json in a PRIVATE infra/ops repo so variable edits +# have a reviewed source of truth; the org audit log records each edit. (The +# specific home repo is intentionally not named here — this file is public.) + +on: + workflow_dispatch: {} # allow on-demand runs (e.g. to re-bump callers) + push: + branches: [main] + paths: + # cursor-review-auto-label.yml is entirely self-contained — it applies the + # review label inline via actions/github-script and loads no external + # prompt/script asset dir (unlike cursor-review/** or + # scripts/check-pr-size/**), and it exposes no `workflows_ref` input. So + # the workflow file itself is the only path that can change its behavior. + - .github/workflows/cursor-review-auto-label.yml + +permissions: + contents: read + +# Serialize runs of this fleet. The bumper pushes to a STABLE branch +# (ci/bump-auto-label) shared across runs, so two overlapping runs (a rapid +# second main push, or a push racing a manual re-run) would force-reset that +# branch and race the PR update — an older run finishing last could leave the +# committed diff pinned to a stale SHA. cancel-in-progress: false lets the +# running bump finish; GitHub keeps only the newest pending run, so the latest +# SHA always wins (BE-3882). +concurrency: + group: bump-auto-label-callers + cancel-in-progress: false + +jobs: + bump: + # Guard against a workflow_dispatch run from a non-main ref: github.sha is + # the tip of whatever ref was selected, and the bumper force-resets a stable + # shared branch — a manual run from an older ref would pin every caller to a + # stale SHA (and run this job's script, with the App token minted below, + # from an unreviewed commit). The push trigger is already main-only, so this + # only ever skips stray manual runs. + if: github.ref == 'refs/heads/main' + runs-on: ubuntu-latest + # The job is a bounded loop of `git`/`gh` network calls; nothing here should + # take minutes. Without this a hung call would hold a runner for the default + # 6 hours (and, via the serializing concurrency group above, block every + # later bump behind it). + timeout-minutes: 20 + steps: + - name: Checkout + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false + + - name: Generate Cloud Code Bot token + uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1.12.0 + id: token + with: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.CLOUD_CODE_BOT_PRIVATE_KEY }} + # `owner:` with no `repositories:` is deliberate and cannot be narrowed: + # the caller list is a runtime variable (see the header), so the repos + # this token must reach are not knowable at authoring time — and naming + # them here would leak private caller names into a public file. + # + # The PERMISSIONS can be narrowed without naming anything, so they are: + # unset, the minted token carries every permission the app holds on every + # repo it is installed on, far past what this bumper needs. It commits + # the rewritten caller file via the Git Data API (contents) and opens or + # updates the bump PR (pull-requests); `gh pr create --label` stamps the + # caller's label through the issues API (issues) — same trio, and the + # same reasoning, as the groom fleet's entrypoint. + owner: Comfy-Org + permission-contents: write + permission-pull-requests: write + permission-issues: write + + - name: Bump SHA in caller repos + env: + GH_TOKEN: ${{ steps.token.outputs.token }} + NEW_SHA: ${{ github.sha }} + VAR_NAME: AUTO_LABEL_CALLERS + TAG: auto-label + WORKFLOW_FILE: cursor-review-auto-label.yml + # Seeded NON-EMPTY (the callers predate this fleet), so an empty list + # means the variable was clobbered — fail rather than no-op silently. + ALLOW_EMPTY: "false" + # JSON array of {"repo","file","label"} — see the header comment for the + # update flow. Kept in a variable (not the file) so private caller names + # never land in this public repo. NOTE: this env binding is itself the + # known log-exposure gap documented in the header — Actions dumps the + # step env before the script's `::add-mask::` runs. + CALLERS_JSON: ${{ vars.AUTO_LABEL_CALLERS }} + run: | + # The main-only ref guard above cannot catch a manual RE-RUN of an + # older main run: github.ref is refs/heads/main but github.sha is + # that run's original (now stale) commit, and the bumper would + # force-repin every caller to it. So establish the current main tip + # first; the guard below decides whether this run is genuinely stale. + # Don't pipe into `cut`: the pipeline would report cut's status, so a + # failed ls-remote (network blip, remote hiccup) yields an EMPTY + # main_tip that then compares unequal to github.sha and silently + # no-ops the whole fleet bump as if the run were stale. A lookup we + # couldn't perform is not evidence of staleness — fail loudly. + if ! ls_remote=$(git ls-remote origin refs/heads/main); then + echo "::error::Could not look up the current main tip (git ls-remote failed)" + exit 1 + fi + main_tip=${ls_remote%%$'\t'*} + if [[ -z "$main_tip" ]]; then + echo "::error::git ls-remote returned no SHA for refs/heads/main" + exit 1 + fi + WATCHED=.github/workflows/cursor-review-auto-label.yml + if [[ "$main_tip" != "$GITHUB_SHA" ]]; then + # main has moved on. That alone does NOT make this run stale: the + # push trigger is path-filtered to WATCHED, so an unrelated commit + # landing in the seconds between this run's trigger and this check + # starts NO run of its own. Skipping on a bare SHA mismatch would + # discard the only run for this change and leave every caller + # frozen — the exact pin-drift this fleet exists to prevent. + # What actually distinguishes a stale re-run is that WATCHED has + # CHANGED since: then a later commit did touch the path and does + # have its own run, which will pin the newer content. + if ! git fetch --depth=1 origin main; then + echo "::error::Could not fetch the current main tip to compare $WATCHED" + exit 1 + fi + main_tip=$(git rev-parse FETCH_HEAD) + tip_blob=$(git rev-parse --verify --quiet "FETCH_HEAD:$WATCHED" || true) + here_blob=$(git rev-parse --verify --quiet "HEAD:$WATCHED" || true) + if [[ -z "$tip_blob" ]]; then + echo "$WATCHED no longer exists on main ($main_tip) — decommissioned; nothing to bump" + exit 0 + fi + if [[ "$tip_blob" != "$here_blob" ]]; then + echo "github.sha $GITHUB_SHA is behind main ($main_tip) and $WATCHED changed since — stale run/re-run; the newer commit has its own run. Nothing to bump" + exit 0 + fi + echo "main moved to $main_tip since $GITHUB_SHA, but $WATCHED is unchanged — this run is still the only one for that change; proceeding" + fi + # The push path filter also matches a commit that DELETES the + # reusable workflow; bumping callers to a SHA where the file is gone + # would break every caller. Deletion means decommissioning — no-op. + # (This fleet has no run-time asset dir — the yml is self-contained — + # so unlike the sibling entrypoints only the workflow file is checked.) + if [[ ! -f .github/workflows/cursor-review-auto-label.yml ]]; then + echo "cursor-review-auto-label.yml absent at this SHA — decommissioned; nothing to bump" + exit 0 + fi + bash .github/bump-callers/bump-callers.sh diff --git a/.github/workflows/test-bump-callers.yml b/.github/workflows/test-bump-callers.yml index 6b63502..beb7086 100644 --- a/.github/workflows/test-bump-callers.yml +++ b/.github/workflows/test-bump-callers.yml @@ -2,8 +2,9 @@ name: Test bump-callers script # Runs the functional tests + shellcheck for the shared caller-bump script # (.github/bump-callers/bump-callers.sh). That one script drives the SHA-bump -# fan-out for the cursor-review, agents-md-integrity, pr-size, assign-reviewers -# AND groom caller fleets, so a regression here silently breaks every consumer +# fan-out for the cursor-review, cursor-review-auto-label, agents-md-integrity, +# pr-size, assign-reviewers AND groom caller fleets, so a regression here +# silently breaks every consumer # repo's pin auto-bump (or, worse, leaks a private caller name into this public # repo's run logs) — cheap to guard with a unit run on change. Mirrors # test-cursor-review-scripts.yml / test-agents-md-integrity.yml. @@ -17,6 +18,7 @@ on: - '.github/workflows/bump-pr-size-callers.yml' - '.github/workflows/bump-assign-reviewers-callers.yml' - '.github/workflows/bump-groom-callers.yml' + - '.github/workflows/bump-auto-label-callers.yml' - '.github/workflows/test-bump-callers.yml' push: branches: [main] @@ -27,6 +29,7 @@ on: - '.github/workflows/bump-pr-size-callers.yml' - '.github/workflows/bump-assign-reviewers-callers.yml' - '.github/workflows/bump-groom-callers.yml' + - '.github/workflows/bump-auto-label-callers.yml' - '.github/workflows/test-bump-callers.yml' permissions: diff --git a/AGENTS.md b/AGENTS.md index 42dc18d..3ad6636 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -78,18 +78,23 @@ tests — run the matching command above for whatever you touched. - `assign-reviewers.yml` — expertise-aware, load-balanced reviewer requests. - `assign-prs-to-author.yml` — assigns unassigned open PRs to their author. - `detect-unreviewed-merge.yml` — SOC 2: flags PRs merged without approval. -- `bump-cursor-review-callers.yml` / `bump-agents-md-callers.yml` / - `bump-pr-size-callers.yml` / `bump-assign-reviewers-callers.yml` / - `bump-groom-callers.yml` — thin entrypoints over `bump-callers.sh` that fan SHA - bumps out to consumers. A groom caller pins TWICE (`uses:` + `workflows_ref:`); - the shared rewrite moves both, so never hand-bump one alone. +- `bump-cursor-review-callers.yml` / `bump-auto-label-callers.yml` / + `bump-agents-md-callers.yml` / `bump-pr-size-callers.yml` / + `bump-assign-reviewers-callers.yml` / `bump-groom-callers.yml` — thin + entrypoints over `bump-callers.sh` that fan SHA bumps out to consumers. A groom + caller pins TWICE (`uses:` + `workflows_ref:`); the shared rewrite moves both, + so never hand-bump one alone. `stale.yml` and `assign-prs-to-author.yml` have + no fleet because they have no callers; `detect-unreviewed-merge.yml` has ~12 + callers and no fleet — a known, deferred gap, so its pins move by hand. ## Conventions & gotchas - **Public repo — never leak private caller names.** Consumer repo lists live in - repo **variables** (`CURSOR_REVIEW_CALLERS`, `AGENTS_MD_CALLERS`), never - hardcoded in a workflow file or printed to run logs (logs are public). The - bumper masks names it processes. Keep private repo paths/detail out of + repo **variables** — one per fleet (`CURSOR_REVIEW_CALLERS`, + `AUTO_LABEL_CALLERS`, `AGENTS_MD_CALLERS`, `PR_SIZE_CALLERS`, + `ASSIGN_REVIEWERS_CALLERS`, `GROOM_CALLERS`; the bump-callers README table is + canonical) — never hardcoded in a workflow file or printed to run logs (logs + are public). The bumper masks names it processes. Keep private repo paths/detail out of 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 @@ -105,6 +110,12 @@ tests — run the matching command above for whatever you touched. `cursor-review.yml` change doesn't spuriously bump agents-md or pr-size callers). Do not fork the script — a forked copy is how other shared org machinery has drifted. +- **Enrolling a caller is TWO steps.** Merge the caller, *and* add the repo to + its `vars.*_CALLERS` roster. Skipping the second is the most repeated mistake + here: the pin then never moves, the caller drifts behind the reusable, and it + fails at startup much later with no obvious cause. This repo did it to its own + `ci-groom.yml`. When auditing, compare the roster against reality in both + directions — a roster entry whose caller file does not exist is equally broken. - **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