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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 98 additions & 1 deletion .github/bump-callers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ forward automatically instead of silently drifting commits behind.
@SHORT" diff) and, if a bump PR is already open, refreshes its title/body to
the new SHA rather than opening another. A fresh PR is opened only when none
is open (first bump, or the prior one merged/closed since the last run).
- **`tests/`** — a `bash` functional suite (stubs `gh`, no network), run by
- **`preflight.sh`** — the staleness/decommission guard that runs *before* the
bump script (see [Preflight](#preflight) below). Also one file on purpose: it
was an inline copy in every entrypoint, and the copies drifted.
- **`tests/`** — `bash` functional suites (stubs `gh` / builds throwaway local
repos; no network), run by
[`test-bump-callers.yml`](../workflows/test-bump-callers.yml) plus shellcheck.

## The fleets
Expand Down Expand Up @@ -104,6 +108,99 @@ otherwise they are left as found and the run logs a warning. Inert for every
caller today (all call exactly one reusable); it exists so a caller that starts
calling two cannot be corrupted.

## Preflight

Before an entrypoint may bump anything it has to answer two questions: is this
run **stale** (has a later commit already touched the watched surface, so *that*
commit has its own run?), and has the watched surface been **decommissioned**
(deleted, so pinning callers to this SHA would break every one of them)?

`preflight.sh` is that guard. It used to be an inline copy in each
`bump-*-callers.yml`, and the copies drifted — several skipped on a bare tip
mismatch, which throws away the only run for a change and freezes every caller,
and the one that compared content forgot to re-point the pin at the verified tip.
The extracted script deliberately adopts the hardened semantics: exact-refname
tip parse (a branch literally named `foo/refs/heads/main` matches the ls-remote
pattern at component boundaries and must not be consumed), `FETCH_HEAD`
verification before any object is read out of it, deletion tested through the
`$WATCHED` **variable** rather than a second copy of the literal path, and the
re-point that pins callers to the verified tip instead of a stale `github.sha`.

| Input (env) | |
|---|---|
| `WATCHED` | **required** — repo-relative path of the watched reusable workflow (e.g. `.github/workflows/groom.yml`) |
| `WATCHED_ASSETS` | optional — the watched asset directory (e.g. `.github/groom`). Empty/unset means the fleet is single-path |
| `NEW_SHA` | the candidate SHA, normally `github.sha` |
| `GITHUB_SHA`, `GITHUB_OUTPUT` | provided by Actions |

Both watched paths are **literal paths, not the globs from the `paths:` filter** —
`.github/groom`, never `.github/groom/**` and never a trailing slash. A glob
resolves to nothing (`[[ -d '.github/groom/**' ]]` is false,
`git rev-parse 'HEAD:.github/groom/**'` is empty), so it would make every
comparison verify nothing and the fleet a permanent silent no-op. The script
rejects that shape up front rather than reporting it as a decommission, and it
likewise rejects a `NEW_SHA` that is not a full 40-character lowercase SHA (it is
emitted verbatim into `$GITHUB_OUTPUT`, so a newline in it injects output lines)
and a `HEAD` that is not `GITHUB_SHA` (a `ref:` override in the consuming
checkout would have it compare main against itself).

| Output (step output) | |
|---|---|
| `proceed` | `true` → run `bump-callers.sh`; `false` → stale or decommissioned, do nothing |
| `new_sha` | the SHA to pin — `NEW_SHA`, or the verified main tip when the run was re-pointed forward |

Both outputs are written on **every** exit-0 path. The script exits non-zero only
for an input it cannot trust (the shape checks above) or a lookup it could not
perform (failed `ls-remote`, failed fetch, unresolvable `FETCH_HEAD`, a
`rev-parse` that *failed* rather than reporting absence): neither is evidence of
staleness, so it fails loudly rather than silently no-opping the fleet.

**A multi-path fleet must pass `WATCHED_ASSETS`.** The re-point is only sound
because every entry in the fleet's `paths:` trigger is covered by the comparison
— for `agents-md-integrity` (`.github/agents-md-integrity/**`), `cursor-review`
(`.github/cursor-review/**`), `groom` (`.github/groom/**`) and `pr-size`
(`scripts/check-pr-size/**`) that includes the asset directory the reusable loads
its prompts/scripts/briefs from at run time. (`pr-risk` is multi-path too, but its
filter also carries `:(exclude)` entries that one `WATCHED_ASSETS` string cannot
express — see the note below.) Compare `WATCHED` alone on one of those and a
commit touching only the assets reads as "unchanged", so callers get pinned to a
tip whose other relevant content was never verified. Read the entrypoint's
`paths:` rather than trusting this list, and if you widen a fleet's path filter,
widen these inputs in the same change.

Consumption is two steps — the guard, then the bump gated on its output:

```yaml
- name: Preflight (staleness / decommission guard)
id: preflight
env:
WATCHED: .github/workflows/groom.yml
WATCHED_ASSETS: .github/groom # omit for a single-path fleet
NEW_SHA: ${{ github.sha }}
run: bash .github/bump-callers/preflight.sh

- name: Bump SHA in caller repos
if: steps.preflight.outputs.proceed == 'true'
env:
GH_TOKEN: ${{ steps.token.outputs.token }}
NEW_SHA: ${{ steps.preflight.outputs.new_sha }}
# …VAR_NAME / TAG / WORKFLOW_FILE / CALLERS_JSON as before
run: bash .github/bump-callers/bump-callers.sh
```

`new_sha` is a **step output**, not a `$GITHUB_ENV` export, and the consuming
step reads it through its own `env:` binding. That is deliberate: a step-level
`env: NEW_SHA:` takes precedence over the job environment, so a `$GITHUB_ENV`
write would be silently overridden by the very binding it is meant to correct.

> **The entrypoints still carry their inline copies.** Swapping them over to this
> script is a separate change. `bump-pr-risk-callers.yml` needs a decision rather
> than a swap: its copy has hardening this one does not implement — a
> `git rev-list` "did a later *commit* touch a watched path" test (rather than a
> net-content comparison) and an is-ancestor check that refuses to pin an
> orphaned commit — so folding it in, or keeping that fleet on its own guard, has
> to be chosen deliberately, not by deleting the checks.

## How the pin rewrite is scoped (and why it asserts afterwards)

The rewrite targets the **pin token**, not "any 40-hex on a line that mentions
Expand Down
Loading