ci(bump-callers): add preflight.sh — one tested staleness/decommission guard (BE-6475)

.github/bump-callers/README.md

@@ -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
@@ -104,6 +108,149 @@ 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), a lookup it could not
+perform (failed `ls-remote`, failed fetch, unresolvable `FETCH_HEAD`, a
+`rev-parse` that *failed* rather than reporting absence), or an answer that
+contradicts history (the **direction guard** below): none of those is evidence of
+staleness, so it fails loudly rather than silently no-opping the fleet.
+
+**The direction guard.** When main has moved, the fetched tip must *descend from*
+this run's commit before anything is compared against it or pinned to it. If main
+moved BACKWARDS — a force-push, a revert-reset, or a stale replica answering the
+tip lookup — both outcomes are silently wrong: with the watched content differing
+at the older commit the run reads as a stale re-run and exits green ("the newer
+commit has its own run" — about an *older* commit), freezing every caller behind a
+run that will never come; with the content identical, the re-point pins the whole
+fleet BACKWARDS. So `git merge-base --is-ancestor` gates both, and a failure is an
+`::error::`, not a skip.
+
+It is measured **twice**, because descending from this run's commit is the weaker
+half. `ls-remote` and the fetch are two round trips, and a rewind landing in that
+window — or a stale replica answering the second one — can hand back a commit that
+is older than the tip just reported yet still *ahead* of this run, which sails
+through a HEAD-only check and gets compared and pinned anyway. So the tip
+`ls-remote` reported must be an ancestor of the fetched one before that move is
+logged as an advance, and the fetched one must be a descendant of this run's
+commit before anything is read out of it.
+
+Both need real history to answer: against a `--depth=1` graft `--is-ancestor`
+returns false even for a legitimate forward move, so the fetch adds `--unshallow`
+when the checkout is shallow (which `actions/checkout` makes it) — `--unshallow`
+errors out on an already-complete clone, hence a probe rather than an
+unconditional flag. The probe is `git rev-parse --is-shallow-repository`, not a
+stat of `$(git rev-parse --git-dir)/shallow`: inside a **linked worktree** that
+marker lives in the common git dir while `--git-dir` names the per-worktree one,
+so the hand-rolled form false-negatives exactly there and silently skips the
+deepening the guard is supposed to rest on.
+
+**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.
+
+They must not be **wider** than the filter either, which is the direction an
+excluding fleet gets wrong. A commit touching only an excluded path (pr-risk's
+`scripts/pr-risk/tests`, its `README.md`) starts no run of its own, but it does
+change the tree OID of an over-broad `WATCHED_ASSETS` — so this run reports "the
+watched surface changed since", skips green as a stale re-run, and waits on a
+later run that will never exist. An exclusion is a reason to narrow the inputs, or
+to leave that fleet on its own guard; never to point `WATCHED_ASSETS` at the whole
+directory.
+
+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` carried two checks this
+> script did not, and **BE-6670 decided both** rather than leaving the swap to
+> choose:
+>
+> - Its **is-ancestor check is adopted here, for every fleet** (BE-6675) — see
+>   the direction guard above. It was never pr-risk-specific.
+> - Its **`git rev-list` "did a later *commit* touch a watched path" test is
+>   deliberately not ported.** It exists because pr-risk has no re-point, so a
+>   land-then-revert (net content change of zero) makes that fleet call this run
+>   the only one for the change and pin callers *backwards* to `github.sha`. The
+>   re-point already answers that case by pinning the verified tip — which on a
+>   land-then-revert is the revert commit, i.e. forward. Adding rev-list on top
+>   would only skip a run whose content the tip still needs pinned. That settles
+>   land-then-revert; it is not a claim that comparing objects expresses
+>   everything a rev-list *pathspec* can. Swapping an excluding fleet across means
+>   narrowing its inputs to what its filter really watches (above), not adding
+>   rev-list back.
+
 ## 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