From 3832f20fc498bc7cfa81865bb51c07efd07c4ec0 Mon Sep 17 00:00:00 2001 From: Maria Carter Date: Sun, 2 Aug 2026 16:56:18 +0200 Subject: [PATCH 1/3] feat: close the audit's reference and content blind spots, ship lint.yml in the kit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pin audit reported the fleet uniform through the whole v1.9.0 gap, because comparing deployed pin lines to the latest tag cannot see three things. Two of them are now checked. tools/fleet-pin-audit.sh gains: - A REFERENCE check — every uses: pin in templates/github/*.yml must equal the latest tag's SHA. Checks 2 and 3 measure the fleet against templates/, so a stale reference makes both of them lie. That is the v1.9.0 failure exactly: the wave repinned the fleet to a54c91e while the kit's own stubs still said 80c35fe, and an audit that only compared deployed pins to the latest tag called the fleet uniform throughout. - A CONTENT check — the whole waved file against its templates/github/ source, not just the pin line. This closes the other two holes: a file with no uses: line at all (an unconverted 190-line copy of what is now a 66-line stub) is no longer invisible, and DRIVER_AGENTS_REF — a raw SHA in an env: block that no bot can bump — is now compared like any other line. Only SHOPIFY_STORE_NAME is normalized away, since it is the one difference a correctly-waved repo is supposed to have. DriverDigital/workflows itself is skipped: its reusables share basenames with the stubs that call them. - A non-zero exit on any drift, so a wave can gate on it. Verified against the live fleet: 21 repo@branch pairs, 108 pin rows all at 90f0d066, 127 files byte-identical to templates/, zero drift. That run also shows the v1.11.0 wave has in fact landed, which the README still said it had not — corrected. Still unchecked, and now stated as such in both docs: the tripwire parity between templates/github/claude.yml and driver-agents' canonical instructions. The audit proves the fleet matches templates/; only the by-hand release step proves templates/ matches canonical. Also adds templates/github/lint.yml — actionlint and shellcheck over the installing repo's own .github/workflows/. This repo's own lint.yml is a superset and stays where it is; the kit version drops the three steps that reference templates/. Installed into driver-agents and driver-agents-app alongside pr-first-review.yml, which the kit README now documents as a supported partial install. --- README.md | 52 +++++++++++++------ docs/fleet-operations.md | 50 +++++++++++++------ templates/github/README.md | 32 +++++++++--- templates/github/lint.yml | 57 +++++++++++++++++++++ tools/fleet-pin-audit.sh | 100 +++++++++++++++++++++++++++++++------ 5 files changed, 240 insertions(+), 51 deletions(-) create mode 100644 templates/github/lint.yml diff --git a/README.md b/README.md index cb90628..36c7a82 100644 --- a/README.md +++ b/README.md @@ -42,8 +42,9 @@ one wave. **Decided 2026-08-02: shipped as `v1.11.0` rather than folded into the v1.10.0 wave.** Folding it in would have put `claude.yml` content on 18 branches that exists in no tag, and `tools/fleet-pin-audit.sh` -compares only stub pin lines against the latest tag — never `templates/` content — so it would have -reported the fleet uniform and green over the gap. Executed as: merge → tag `v1.11.0` at that merge +compared only stub pin lines against the latest tag — never `templates/` content — so it would have +reported the fleet uniform and green over the gap. (That blind spot is now closed: the audit also +checks `templates/` against the latest tag and every waved file against `templates/`.) Executed as: merge → tag `v1.11.0` at that merge commit → repin all six stubs to `v1.11.0` → **one** wave. That is what `v1.7.0` and `v1.8.0` each did. **Note on the pin sequence:** `v1.9.0` (`a54c91e`, the store-secret rename) never got its kit repin @@ -109,8 +110,9 @@ Waved to all 21 repin targets on 2026-08-02; fleet uniform, 108 pins, zero stale green on the new names, old-name secrets deleted. - **Reusables unchanged.** Note this release **never got its kit repin commit** — `templates/` sat at `v1.8.0`'s SHA while the deployed fleet was waved to `a54c91e`, leaving the fleet a release *ahead* - of the kit templates until `v1.11.0` closed it. That gap is invisible to - `tools/fleet-pin-audit.sh`; see [`docs/fleet-operations.md`](docs/fleet-operations.md). + of the kit templates until `v1.11.0` closed it. That gap was invisible to + `tools/fleet-pin-audit.sh` at the time; its reference check now catches exactly this shape — see + [`docs/fleet-operations.md`](docs/fleet-operations.md). ### `v1.8.0` (2026-07-31, kit-only) @@ -168,8 +170,11 @@ Waved to all 21 repin targets on 2026-08-02; fleet uniform, 108 pins, zero stale prefixes, NFC-normalize, **collapse whitespace**, and diff against the blockquote portion of `claude.yml`'s `--append-system-prompt`. The whitespace collapse is mandatory — the kit flattens canonical's paragraph break to a single space (forced by the no-newline constraint), so a strict - byte compare reports a false failure. Nothing else re-checks this: `fleet-pin-audit.sh` greps only - stub pin lines, and `DRIVER_AGENTS_REF` is a raw SHA in an env var that no bot can bump. + byte compare reports a false failure. **Nothing else re-checks this.** `fleet-pin-audit.sh` proves + the fleet matches `templates/github/claude.yml` — it cannot prove that file's blockquote still + matches canonical at the new pin, and `DRIVER_AGENTS_REF` is a raw SHA in an env var that no bot + can bump. The audit catches a *fleet* that fell behind `templates/`; only this step catches + `templates/` falling behind driver-agents. 2. Repin every caller stub in `templates/github/` to that tag's SHA and commit. Until this lands, the kit's stubs still point at the PREVIOUS tag's reusables. - **If the release ADDS a reusable**, its stub lands *in this step*, not in the PR that added the @@ -178,7 +183,8 @@ Waved to all 21 repin targets on 2026-08-02; fleet uniform, 108 pins, zero stale `bonsai-status-sync.yml`'s stub landed at `v1.11.0`. `lint.yml` fails the build on any stub left carrying a placeholder pin, so this step cannot be silently skipped. 3. Only then re-copy `templates/github/` into consumer repos (`tools/fleet-pin-audit.sh --stale` - to confirm the fleet converged afterwards). + to confirm the fleet converged afterwards — it now checks waved file **content** against + `templates/`, not just the pin line, and exits non-zero on any drift, so a wave can gate on it). - **When a full workflow becomes a stub** (as `bonsai-status-sync.yml` did — this applies to the v1.11.0 wave specifically), the wave diff contains a `templates/github/` path AND a `.github/workflows/` path with the SAME basename. The @@ -220,15 +226,24 @@ produces a silent `startup_failure` — no check run, no notification). **The onboarding kit lives here: `templates/github/`** (moved from `driver-bonsai-mcp` 2026-07-15). It carries the six caller stubs above plus `claude.yml` (the implementer, still a full per-repo workflow), -`shopify-tool-smoke.yml` (store repos only) and `pull_request_template.md`. +`shopify-tool-smoke.yml` (store repos only), `lint.yml` (actionlint over the installing repo's own +workflows) and `pull_request_template.md`. + +**Not every repo takes the whole kit.** A repo that is not on the Bonsai → PR pipeline can install +`pr-first-review.yml` + `lint.yml` alone and skip the rest as inert weight — +[`driver-agents`](https://github.com/DriverDigital/driver-agents) and +[`driver-agents-app`](https://github.com/DriverDigital/driver-agents-app) run exactly that subset. +The trade-off is written up in `templates/github/README.md` under *Partial install*. **`bonsai-status-sync.yml` finished converting at `v1.11.0`.** The reusable landed 2026-08-02 and its stub landed in this tag's repin commit, so the kit now installs a 66-line stub instead of the old 190-line copy — see *Release + repin order* above and [`docs/reusable-conversion-scope.md`](docs/reusable-conversion-scope.md). The two-step was deliberate and matches how `dependabot-keep-current` was added: a new reusable's stub cannot be pinned until the tag containing that reusable exists, so the reusable lands first and the stub follows in -the repin commit. `lint.yml` fails the build on any stub still carrying a placeholder pin. **The fleet has not -been waved yet** — consumer repos still run the 190-line copy until the v1.11.0 wave. +the repin commit. `lint.yml` fails the build on any stub still carrying a placeholder pin. **The v1.11.0 wave +has landed** — `tools/fleet-pin-audit.sh` reads clean across all 21 repo@branch pairs (108 pin rows at +`90f0d066`, 127 files byte-identical to `templates/` after store-handle normalization, verified 2026-08-02), +so every consumer repo now runs the 66-line stub. **`claude.yml` stays a per-repo copy** — that half of the conversion is tabled pending the OIDC spike (whether Claude App token minting survives inside a cross-repo reusable), so it remains the kit's main drift surface @@ -236,10 +251,19 @@ and the reason re-copies still need care. Two files in `.github/workflows/` are **this repo's own CI**, not products — they are `workflow_call`-free and never ship to the fleet: `lint.yml` (actionlint + shellcheck over the reusables *and* the kit, so a -broken workflow can't reach consumer repos — it reports red on the PR, but **pin `actionlint` as a -required check** if you want it to actually block a merge) and `dependabot-auto-merge.yml` (auto-merges -this repo's own `github-owned` Dependabot bumps; the `claude-code-action` group is deliberately excluded, -so those land by hand). +broken workflow can't reach consumer repos) and `dependabot-auto-merge.yml` (auto-merges this repo's own +`github-owned` Dependabot bumps; the `claude-code-action` group is deliberately excluded, so those land by +hand). + +**`actionlint` is a required status check on `main`** (set 2026-08-02) — before that, `lint.yml` could +report red without being able to block. Note the name collision: this repo's own `lint.yml` and the kit's +`templates/github/lint.yml` are **different files**. The kit one runs actionlint over the installing repo's +`.github/workflows/` and nothing else; this one additionally lints `templates/github/`, gates on placeholder +pins, and asserts `claude.yml`'s system prompt still tokenizes. Both use the job id `actionlint`, so the +required-check context string is the same either way. `enforce_admins` stays **`false`** here, deliberately: +it is `false` fleet-wide because direct-push repin waves depend on it, and while no wave has ever pushed +directly to *this* repo, diverging from the fleet default would make the one exception the thing to +remember. The 1-approval review rule is what actually gates merges. ## The three identities diff --git a/docs/fleet-operations.md b/docs/fleet-operations.md index 5b274cd..81a864a 100644 --- a/docs/fleet-operations.md +++ b/docs/fleet-operations.md @@ -95,22 +95,40 @@ Dependabot drift — but survey, don't assume. --- -## What the pin audit cannot see - -`tools/fleet-pin-audit.sh` greps only -`DriverDigital/workflows/.github/workflows/@` and compares the SHA to the latest tag. -Three consequences: - -- **A file with no `uses:` line is invisible.** An unconverted 190-line copy has none, so the audit - cannot tell a repo that was skipped by a stub conversion from one that never carried the file. -- **Content is never compared.** `DRIVER_AGENTS_REF` is a raw SHA in an `env:` block, and the - system-prompt text is just text. A fleet running kit content from no tag reports clean. -- **The reference itself can drift.** The audit compares against the latest *tag*, never against - `templates/`. When those disagree the audit reports uniform while real drift sits in the source of - truth — which is exactly how the v1.9.0 gap went unnoticed for a day. - -This is why the release order requires the tag to contain what gets waved, and why a -`DRIVER_AGENTS_REF` bump must re-run the canonical parity check by hand. +## What the pin audit checks — and the one thing it still cannot see + +`tools/fleet-pin-audit.sh` used to grep only +`DriverDigital/workflows/.github/workflows/@` and compare that SHA to the latest tag, +which left three holes. Two of them let the v1.9.0 gap read green for a day. All three are now +checked; the script runs them in this order and exits non-zero if any fires: + +1. **Reference** — every `uses:` pin in `templates/github/*.yml` equals the latest tag's SHA. + Checks 2 and 3 measure the fleet *against* `templates/`, so a stale reference makes both of them + lie. This is the v1.9.0 failure exactly: the wave repinned the fleet to `a54c91e` while the kit's + own stubs still said `80c35fe`, and an audit that only ever compared deployed pins to the latest + tag called the fleet uniform throughout. When this fires, nothing below it means anything — + fix step 2 of the README's release order first. +2. **Pins** — each deployed caller stub's `uses:` SHA vs that tag. The original check, unchanged. +3. **Content** — the whole waved file vs its `templates/github/` source, byte for byte. This is + what closes the other two holes: a file with **no `uses:` line at all** (an unconverted 190-line + copy of what is now a 66-line stub) is no longer invisible, and `DRIVER_AGENTS_REF` — a raw SHA + in an `env:` block that no bot can bump — is now compared like any other line. + +Two things worth knowing about check 3: + +- **Only `SHOPIFY_STORE_NAME` is normalized away.** It is the one difference a correctly-waved repo + is *supposed* to have. Everything else that differs is reported, third-party action pins included: + a repo whose Dependabot bumped `actions/checkout` past the kit's pin is drift worth seeing, and it + means the kit is behind, not that the repo is wrong. +- **`DriverDigital/workflows` itself is skipped.** Its `.github/workflows/` holds the *reusables*, + which share basenames with the stubs that call them — `pr-first-review.yml` is a ~200-line + reusable there and a 25-line stub in the kit — so comparing it against `templates/` would report + six phantom drifts. + +**Still unchecked: the tripwire parity between `templates/` and canonical.** The audit proves the +fleet matches `templates/github/claude.yml`; it cannot prove that file's `--append-system-prompt` +blockquote still matches driver-agents `docs/agent-instructions-shopify.md` at the pinned +`DRIVER_AGENTS_REF`. That comparison is by hand, at release time — step 1 of the release order. --- diff --git a/templates/github/README.md b/templates/github/README.md index 0773763..0fe7f8e 100644 --- a/templates/github/README.md +++ b/templates/github/README.md @@ -12,6 +12,7 @@ lifecycle. | `claude.yml` | `.github/workflows/claude.yml` | The implementer — claude-code-action reads an `@claude`'d issue, creates a **development-linked branch** from it, writes code, and opens a **real PR** from that branch; it addresses revisions when `@claude`'d on the PR (standalone comment, review, or inline comment). | | `pull_request_template.md` | `.github/pull_request_template.md` | Prompts human PRs to **link the Bonsai issue** (`Closes #N`) so the sync can resolve the task. AI PRs link automatically via the issue's development branch. | | `shopify-tool-smoke.yml` | `.github/workflows/` — **STORE REPOS ONLY** | Manual (`workflow_dispatch`) diagnostic for the Shopify admin tool: secrets → `driver-agents` clone at the pin → token mint → Admin API, read-only. Fails **loudly** where `claude.yml` degrades — that's the point. Skip it in repos with no store. | +| `lint.yml` | `.github/workflows/lint.yml` | actionlint + shellcheck over the installing repo's own `.github/workflows/`. Guards the one CI failure with no signal: a YAML or shell error surfaces as a `startup_failure` — no check run, no notification — which on the PR page is indistinguishable from checks that have not started. Check-run context is the job id, **`actionlint`**. Not the same file as this repo's own `.github/workflows/lint.yml`, which runs a superset and never ships. | ### Caller stubs (thin — they call this repo's reusables at a pinned SHA) @@ -105,22 +106,41 @@ Ready to Deploy → Delivered / Deployed / Completed. The workflows never set th cp templates/github/dependabot-validate.yml .github/workflows/ cp templates/github/dependabot-report.yml .github/workflows/ cp templates/github/dependabot-keep-current.yml .github/workflows/ + cp templates/github/lint.yml .github/workflows/ cp templates/github/pull_request_template.md .github/pull_request_template.md ``` **Re-copying into a repo that already has the kit?** Preserve that repo's own Dependabot action pins — re-copy the workflow bodies, but don't clobber pins Dependabot has since bumped there. + **And check for an existing `.github/workflows/lint.yml`** — a repo that hand-rolled its own would + be silently clobbered by the kit's, which is the only kit filename likely to already exist. + + **Partial install (`pr-first-review.yml` + `lint.yml` only).** For a repo that is *not* on the + Bonsai → PR pipeline — no orchestrator issues, no ticketed rail — these two are the useful subset + and the rest are inert weight. This is what `driver-agents` and `driver-agents-app` run. Know what + you are giving up: `pr-first-review.yml` deliberately excludes `dependabot[bot]` authors and + self-skips on any PR whose linked issue carries a Bonsai uuid, so with nothing else installed a + Dependabot PR or a ticketed PR gets **zero** automated review rather than a different one. Add the + Dependabot pair if and when that repo turns Dependabot on. 5. **Board strings are no longer edited here.** As of v1.11.0 `bonsai-status-sync.yml` is a caller stub; the exact Bonsai status strings live only in the central reusable (`DriverDigital/workflows/.github/workflows/bonsai-status-sync.yml`). A board rename is therefore a kit release + fleet repin, not a local edit — changing the strings in one repo does nothing. A miss still fails the workflow loudly with `STATUS_NOT_FOUND` rather than flipping silently. 6. **Pin the required check.** Run a test PR (one human, one Dependabot), then pin the **exact - check context GitHub reports** — for a reusable-workflow job that is - ` / `, expected **`validate / validate`**. Copy the literal - string from the first run's checks list; the workflow display name is not part of it. Also add a - rule requiring a **human** approver (e.g. CODEOWNERS) so no bot signal satisfies the merge gate. - `dependabot-validate` always runs and branches internally — never `if:`-skip it, because a - *skipped* required check counts as not-passed and would block every human PR. + check context GitHub reports**. Copy the literal string from the first run's checks list; the + workflow display **name** is never part of it. Two shapes: + - A **reusable-workflow** job reports ` / ` — for the full kit + that is **`validate / validate`** (`dependabot-validate`), and `pr-first-review` reports + **`review / review`**. + - A **local** job reports its bare job id — `lint.yml` reports **`actionlint`**. + + On a partial install with no `dependabot-validate`, `actionlint` is the one check that runs on + every PR unconditionally, so it is the right thing to require. Do **not** require + `review / review` on its own: `pr-first-review` skips drafts, forks, bot authors and ticketed + PRs by design, and a *skipped* required check counts as not-passed — it would block exactly the + PRs it is meant to leave alone. (`dependabot-validate` avoids this by always running and + branching internally, which is why it must never be `if:`-skipped.) Also add a rule requiring a + **human** approver (e.g. CODEOWNERS) so no bot signal satisfies the merge gate. ## Multi-branch repos (e.g. Palmers — independent release branches) diff --git a/templates/github/lint.yml b/templates/github/lint.yml new file mode 100644 index 0000000..d22a3f7 --- /dev/null +++ b/templates/github/lint.yml @@ -0,0 +1,57 @@ +name: Lint workflows + +# KIT FILE — install into a repo's .github/workflows/ unchanged. +# +# actionlint over the installing repo's own .github/workflows/, plus shellcheck over every `run:` +# block (shellcheck is preinstalled on ubuntu-latest). It exists because a broken workflow is the +# one CI failure with NO signal: a YAML or shell error surfaces as a `startup_failure` — no check +# run, no notification — which on the PR page is indistinguishable from checks that simply have +# not started yet. Every other rail in this kit assumes its own workflow file parses. +# +# This is NOT the reusables' own lint job. DriverDigital/workflows runs a superset in its own +# .github/workflows/lint.yml (it additionally lints templates/github/ and asserts kit invariants); +# that file stays there and is never installed anywhere. +# +# Job id `actionlint` is the check-run context — pin THAT literal string as the required check. + +on: + pull_request: + # Multi-branch repos (Palmers-style, one release branch per country store) should add their own + # release branches here — this list is not inferred. + push: + branches: [main] + +permissions: + contents: read + +concurrency: + group: lint-${{ github.ref }} + cancel-in-progress: true + +jobs: + actionlint: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + # Pinned release + checksum instead of the upstream `rhysd/actionlint` action: the org + # enforces a selected-actions allowlist, so a dependency-free download is one less thing to + # get approved, and the checksum keeps it honest if a release asset is ever replaced. + # Bump VERSION and SHA256 TOGETHER — Dependabot does not manage this pair. + - name: Install actionlint + env: + VERSION: 1.7.12 + SHA256: 8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8 + run: | + set -euo pipefail + curl -fsSL --max-time 120 -o actionlint.tar.gz \ + "https://github.com/rhysd/actionlint/releases/download/v${VERSION}/actionlint_${VERSION}_linux_amd64.tar.gz" + echo "${SHA256} actionlint.tar.gz" | sha256sum -c - + tar -xzf actionlint.tar.gz actionlint + + # With no paths actionlint globs .github/workflows/ itself. + - name: Lint .github/workflows/ + run: | + set -euo pipefail + ./actionlint -color diff --git a/tools/fleet-pin-audit.sh b/tools/fleet-pin-audit.sh index ca97f6a..78b3c92 100755 --- a/tools/fleet-pin-audit.sh +++ b/tools/fleet-pin-audit.sh @@ -1,38 +1,87 @@ #!/usr/bin/env bash -# fleet-pin-audit.sh — one-command drift detector for the central-workflow pins. +# fleet-pin-audit.sh — one-command drift detector for the central-workflow kit. # -# Scans every non-archived DriverDigital repo's .github/workflows/ (default -# branch, plus every main* branch of Palmers — the kit is installed per country -# branch there) for `uses: DriverDigital/workflows/...@SHA` caller-stub pins, -# prints one line per pin, and summarizes pins by SHA against the latest tag. +# Three checks, because a pin line alone never proved the fleet was current: # -# Dependabot does NOT bump these reusable-workflow pins in practice (verified -# 2026-07-16: zero such PRs fleet-wide, even in repos with a github-actions -# block) — repins happen as manual waves, and this script is how drift gets -# seen between waves. Needs: gh (authenticated), org read access. +# 1. REFERENCE — every `uses:` pin in templates/github/*.yml equals the latest tag's SHA. Checks +# 2 and 3 measure the fleet against `templates/`, so a stale reference makes both of them lie. +# That is exactly how the v1.9.0 gap survived a day: the wave repinned the fleet to `a54c91e` +# while the kit's own stubs still said `80c35fe`, and an audit that compared deployed pins to +# the latest *tag* — never to `templates/` — called the fleet uniform the whole time. +# 2. PINS — every deployed caller stub's `uses: DriverDigital/workflows/...@SHA` vs that tag. +# 3. CONTENT — the whole waved file vs its templates/github/ source. The pin is one line of it: +# `DRIVER_AGENTS_REF` is a raw SHA in an `env:` block, the implementer's system prompt is just +# text, and an unconverted 190-line copy of a workflow that is now a 66-line stub has no +# `uses:` line at all — so a pin grep sees none of them. Only state the kit documents as +# per-repo is normalized away (`SHOPIFY_STORE_NAME`); anything else that differs is drift. +# +# Scans every non-archived DriverDigital repo's .github/workflows/ (default branch, plus every +# main* branch of Palmers — the kit is installed per country branch there). +# +# Dependabot does NOT bump these reusable-workflow pins in practice (verified 2026-07-16: zero such +# PRs fleet-wide, even in repos with a github-actions block) — repins happen as manual waves, and +# this script is how drift gets seen between waves. Needs: gh (authenticated), org read access. # # Usage: tools/fleet-pin-audit.sh # full report -# tools/fleet-pin-audit.sh --stale # only pins not at the latest tag +# tools/fleet-pin-audit.sh --stale # only what has drifted +# +# Exits non-zero when anything has drifted, so a wave can gate on it. set -u ORG="${ORG:-DriverDigital}" +KIT="$(cd "$(dirname "$0")/../templates/github" && pwd)" + LATEST="$(gh api "repos/$ORG/workflows/tags" --jq '.[0] | "\(.name) \(.commit.sha)"')" LATEST_TAG="${LATEST%% *}"; LATEST_SHA="${LATEST#* }"; LATEST_SHA8="${LATEST_SHA:0:8}" +# The one difference a correctly-waved repo is SUPPOSED to have: its own store handle, which the +# kit ships empty. Everything else — third-party action pins included — is reported. A consumer +# repo whose Dependabot bumped `actions/checkout` past the kit's pin is drift worth seeing: it +# means the kit is behind, not that the repo is wrong. +kit_normalize() { sed 's/^\( *SHOPIFY_STORE_NAME:\).*/\1 /'; } + +# 1. REFERENCE — templates/ against the latest tag. +reference="$( + for t in "$KIT"/*.yml; do + grep -o "$ORG/workflows/\.github/workflows/[^@]*@[0-9a-f]\{40\}" "$t" | sed 's/.*@//' \ + | while read -r sha; do + [ "$sha" = "$LATEST_SHA" ] \ + || echo "templates/github/$(basename "$t") pins @${sha:0:8} — $LATEST_TAG is $LATEST_SHA8" + done + done +)" + scan_ref() { # repo ref - local repo="$1" ref="$2" files f raw + local repo="$1" ref="$2" files f raw local_kit deployed files="$(gh api "repos/$ORG/$repo/contents/.github/workflows?ref=$ref" --jq '.[].name' 2>/dev/null)" || return 0 for f in $files; do raw="$(gh api "repos/$ORG/$repo/contents/.github/workflows/$f?ref=$ref" -H 'Accept: application/vnd.github.raw' 2>/dev/null)" || continue + + # 2. PINS printf '%s' "$raw" | grep -o "$ORG/workflows/.github/workflows/[^@]*@[0-9a-f]*" \ | sed "s|$ORG/workflows/.github/workflows/||; s|@\([0-9a-f]\{8\}\)[0-9a-f]*|@\1|" \ - | while read -r line; do echo "$repo@$ref $f $line"; done + | while read -r line; do echo "PIN $repo@$ref $f $line"; done + + # 3. CONTENT — only for files the kit actually ships. + [ -f "$KIT/$f" ] || continue + deployed="$(printf '%s' "$raw" | kit_normalize)" + local_kit="$(kit_normalize < "$KIT/$f")" + if [ "$deployed" = "$local_kit" ]; then + echo "CONTENT $repo@$ref $f ok" + else + echo "CONTENT $repo@$ref $f DRIFT $(command diff \ + <(printf '%s\n' "$deployed") <(printf '%s\n' "$local_kit") | grep -c '^[<>]') lines differ" + fi done } report="$( gh repo list "$ORG" --limit 100 --no-archived --json name,defaultBranchRef \ --jq '.[] | "\(.name) \(.defaultBranchRef.name)"' | while read -r repo def; do + # Skip the kit repo itself: its .github/workflows/ holds the REUSABLES, which share basenames + # with the stubs that call them (pr-first-review.yml is a 200-line reusable here and a 25-line + # stub in the kit), so a content compare against templates/ would report six phantom drifts. + [ "$repo" = "workflows" ] && continue scan_ref "$repo" "$def" if [ "$repo" = "Palmers" ]; then gh api "repos/$ORG/Palmers/branches?per_page=100" --jq '.[].name' 2>/dev/null \ @@ -41,11 +90,32 @@ report="$( done | sort )" +pins="$(printf '%s\n' "$report" | grep '^PIN ' | sed 's/^PIN //')" +stale="$(printf '%s\n' "$pins" | grep -v "@$LATEST_SHA8")" +content="$(printf '%s\n' "$report" | grep '^CONTENT ' | sed 's/^CONTENT //')" +drift="$(printf '%s\n' "$content" | grep ' DRIFT ')" + if [ "${1:-}" = "--stale" ]; then - printf '%s\n' "$report" | grep -v "@$LATEST_SHA8" || echo "(no stale pins — fleet uniform at $LATEST_TAG)" + [ -n "$reference" ] && { echo "reference drift (templates/ is not at $LATEST_TAG):"; printf '%s\n' "$reference"; echo; } + [ -n "$stale" ] && { echo "stale pins:"; printf '%s\n' "$stale"; echo; } + [ -n "$drift" ] && { echo "content drift:"; printf '%s\n' "$drift"; echo; } + [ -n "$reference$stale$drift" ] || echo "(converged — templates/, pins, and waved content all at $LATEST_TAG)" else - printf '%s\n' "$report" + printf '%s\n' "$pins" + echo + printf '%s\n' "$content" fi + echo +if [ -n "$reference" ]; then + echo "REFERENCE DRIFT — templates/ pins disagree with $LATEST_TAG, so every line above is measured" + echo "against a stale baseline. Fix step 2 of the README's release order before trusting this run:" + printf ' %s\n' "$reference" +else + echo "reference: templates/github/ pins all at $LATEST_TAG ($LATEST_SHA8)" +fi echo "latest: $LATEST_TAG ($LATEST_SHA8) — pins by SHA:" -printf '%s\n' "$report" | sed 's/.*@//' | sort | uniq -c | sort -rn +printf '%s\n' "$pins" | sed 's/.*@//' | sort | uniq -c | sort -rn +echo "content: $(printf '%s\n' "$content" | grep -c ' ok$') match templates/, $(printf '%s\n' "$content" | grep -c ' DRIFT ') drifted" + +[ -z "$reference$stale$drift" ] From d0f0c6e8d8d18e83fc48cb96015af706f3637d8d Mon Sep 17 00:00:00 2001 From: Maria Carter Date: Sun, 2 Aug 2026 16:59:55 +0200 Subject: [PATCH 2/3] docs: record actionlint as a required check on main, and the enforce_admins call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Set via the narrow required_status_checks sub-resource, not a whole-object PUT — that endpoint replaces everything and silently drops any field left out of the body, the 1-approval rule included. Diffing the full protection object before and after confirms only contexts/checks moved. enforce_admins stays false, which means an admin can still merge past a red actionlint. Requiring the check makes it binding for everyone else and puts a red X in front of an admin who previously had nothing to override. Flipping the flag was considered and rejected for fleet uniformity; the reasoning is written down rather than left implicit. --- README.md | 9 +++++---- docs/fleet-operations.md | 31 ++++++++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 36c7a82..c04df5c 100644 --- a/README.md +++ b/README.md @@ -260,10 +260,11 @@ report red without being able to block. Note the name collision: this repo's own `templates/github/lint.yml` are **different files**. The kit one runs actionlint over the installing repo's `.github/workflows/` and nothing else; this one additionally lints `templates/github/`, gates on placeholder pins, and asserts `claude.yml`'s system prompt still tokenizes. Both use the job id `actionlint`, so the -required-check context string is the same either way. `enforce_admins` stays **`false`** here, deliberately: -it is `false` fleet-wide because direct-push repin waves depend on it, and while no wave has ever pushed -directly to *this* repo, diverging from the fleet default would make the one exception the thing to -remember. The 1-approval review rule is what actually gates merges. +required-check context string is the same either way. `enforce_admins` stays **`false`** here, deliberately +— which means an admin can still merge past a red `actionlint`. Requiring the check makes it binding for +everyone else and puts a red X in front of an admin who previously had nothing to override; that was worth +having on its own. Flipping the flag was considered and rejected for uniformity — reasoning in +[`docs/fleet-operations.md`](docs/fleet-operations.md#branch-protection). ## The three identities diff --git a/docs/fleet-operations.md b/docs/fleet-operations.md index 81a864a..19e6474 100644 --- a/docs/fleet-operations.md +++ b/docs/fleet-operations.md @@ -174,6 +174,31 @@ branches have **no protection at all** — `studio-sulzer@main` and `Team-Laird@ protection endpoint). Every other kit branch is protected. The kit's onboarding steps assume a human-approver rule exists, so on those two a bot signal alone could satisfy a merge. -On this repo, `main` has `required_status_checks` with `strict: true` but empty `contexts` — so -`lint.yml` reports red without being able to block. The context string to add is **`actionlint`** -(the job id at `.github/workflows/lint.yml:28`; the workflow-level `name:` is not part of it). +On this repo, `main` requires **`actionlint`** (set 2026-08-02; before that `required_status_checks` +had `strict: true` but empty `contexts`, so `lint.yml` could report red without being able to block). +The context is the **job id** at `.github/workflows/lint.yml:28` — the workflow-level `name:` is not +part of it. Applied through the narrow sub-resource, never a whole-object `PUT`: + +```bash +gh api -X PATCH repos/DriverDigital/workflows/branches/main/protection/required_status_checks \ + -f 'contexts[]=actionlint' +``` + +`PUT /branches/{branch}/protection` **replaces** the entire protection object, so any field left out +of the body is silently deleted — the 1-approval review rule included. The `PATCH` above touches +`required_status_checks` and nothing else; diffing the full object before and after confirmed only +`contexts`/`checks` moved. GitHub bound the context to the Actions app (`app_id: 15368`) on its own, +which is the stricter outcome: only a check run from Actions can satisfy it. + +**`enforce_admins` stays `false` here — deliberately, and know what that buys.** With it `false`, an +admin can merge past *everything*: a red `actionlint`, no approval, `strict` or not. So requiring the +check does not make it unbypassable for Maria — it makes it unbypassable for everyone else, and it +puts a red X in front of an admin who would otherwise have had nothing to override. That is the +actual value, and it was worth having either way. + +Flipping it to `true` was considered and rejected. The fleet-wide `false` exists because direct-push +repin waves depend on it, and that reasoning genuinely does *not* apply to this repo: no wave has ever +pushed here, and every commit on `main` is a PR merge. The reason to leave it alone anyway is +uniformity — one repo with a different admin rule is a thing to remember at exactly the wrong moment, +and the failure mode it would prevent (an admin knowingly merging red) is not one that has happened. +Revisit if it ever does. From 474f2f2d45020d742c11b754e3838633756a1d63 Mon Sep 17 00:00:00 2001 From: Maria Carter Date: Sun, 2 Aug 2026 17:23:11 +0200 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20address=20the=20review=20=E2=80=94?= =?UTF-8?q?=20audit=20guards,=20SC2015,=20and=20four=20wrong=20doc=20claim?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review over the branch surfaced ten findings that survived refutation. All are fixed here. tools/fleet-pin-audit.sh: - The content check could not see EOF-only drift. Both sides went through command substitution, which strips ALL trailing newlines, so a file differing only at the end compared equal and reported ok — the exact class of invisible drift this check was added to catch. Raw content now goes straight to a temp file, never a variable. - Having fixed that, nine files started reporting drift for a missing final newline: the three stub-rails-only pairs were waved without one and are otherwise identical. Nine permanent red rows is how a detector stops being read, so trailing blank lines and the final newline are now normalized DELIBERATELY, alongside the store handle, and both are documented as the only two normalizations. Internal blank lines are still compared. - A failed `gh repo list` produced an empty report, which fell through to "(converged)" and exit 0 — a drift detector reporting clean because it saw nothing at all. Enumeration now happens outside the report subshell so it can abort the run, and a second guard fails if zero pins are found fleet-wide, since every kit pair carries at least one. - Raised the repo-list limit past the silent truncation cliff. - An apostrophe in a comment inside the $( ) broke the parse: bash opens a quote on one even in a comment, and reports the error at EOF rather than the line. Noted in place. templates/github/lint.yml keeps SHELLCHECK_OPTS=--exclude=SC2015. actionlint treats an info-level shellcheck finding as a hard failure and this job is meant to be a required check, so dropping the exclusion would let ordinary defensive shell block every PR. Upstream removed the rule in shellcheck 0.11, so it is a false positive by its author's judgement, not a suppressed bug. Four doc claims were wrong and are corrected: - "a skipped required check counts as not-passed" is backwards. GitHub accepts a check run whose conclusion is skipped. The real reason not to require `review / review` is its trigger list omitting `synchronize`, so a follow-up commit produces no check run for that head SHA and a MISSING required context blocks forever. Both places that taught the wrong model now teach this one. - "every commit on main is a PR merge" is false. Six commits have no associated PR, including a54c91e, the v1.9.0 release, pushed yesterday. That is a stronger reason to leave enforce_admins false than the uniformity argument it replaces: enforce_admins true would have blocked every one of them. - "six phantom drifts" is now seven — this change adds lint.yml as a shared basename between the reusables and the kit. - "Every other kit branch is protected" understated the exposure. Surveyed all 21 pairs: 8 Palmers branches require zero approving reviews, so 10 pairs lack a human-approver rule, not 2. The README no longer says the two agent repos run the partial install — the PRs are open, not merged, and they should land only after this one ships templates/github/lint.yml. --- README.md | 21 +++++++---- docs/fleet-operations.md | 52 ++++++++++++++++++------- templates/github/README.md | 21 ++++++++--- templates/github/lint.yml | 13 ++++++- tools/fleet-pin-audit.sh | 77 +++++++++++++++++++++++++++++--------- 5 files changed, 138 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index c04df5c..4599b66 100644 --- a/README.md +++ b/README.md @@ -230,10 +230,12 @@ carries the six caller stubs above plus `claude.yml` (the implementer, still a f workflows) and `pull_request_template.md`. **Not every repo takes the whole kit.** A repo that is not on the Bonsai → PR pipeline can install -`pr-first-review.yml` + `lint.yml` alone and skip the rest as inert weight — -[`driver-agents`](https://github.com/DriverDigital/driver-agents) and -[`driver-agents-app`](https://github.com/DriverDigital/driver-agents-app) run exactly that subset. -The trade-off is written up in `templates/github/README.md` under *Partial install*. +`pr-first-review.yml` + `lint.yml` alone and skip the rest as inert weight. That subset is proposed for +[`driver-agents`](https://github.com/DriverDigital/driver-agents/pull/6) and +[`driver-agents-app`](https://github.com/DriverDigital/driver-agents-app/pull/2) — **both PRs are open, +not merged**, and they should land only *after* this repo ships `templates/github/lint.yml`, since until +then the file they install has no upstream source to be re-copied from. The trade-off is written up in +`templates/github/README.md` under *Partial install*. **`bonsai-status-sync.yml` finished converting at `v1.11.0`.** The reusable landed 2026-08-02 and its stub landed in this tag's repin commit, so the kit now installs a 66-line stub instead of the old 190-line copy — @@ -263,7 +265,8 @@ pins, and asserts `claude.yml`'s system prompt still tokenizes. Both use the job required-check context string is the same either way. `enforce_admins` stays **`false`** here, deliberately — which means an admin can still merge past a red `actionlint`. Requiring the check makes it binding for everyone else and puts a red X in front of an admin who previously had nothing to override; that was worth -having on its own. Flipping the flag was considered and rejected for uniformity — reasoning in +having on its own. Flipping the flag would break this repo's own release habit — six commits on `main`, +`v1.9.0`'s included, were pushed directly with no PR. Detail in [`docs/fleet-operations.md`](docs/fleet-operations.md#branch-protection). ## The three identities @@ -324,8 +327,12 @@ task. The `config/reviewers.json` copy in **this** repo is reference only — no ## First-run / required-check -The `dependabot-validate` job **always runs and branches internally** (non-Dependabot PRs no-op green) — a -*skipped* required check counts as not-passed and would block every human PR, so it must never be `if:`-skipped. +The `dependabot-validate` job **always runs and branches internally** (non-Dependabot PRs no-op green), and +must never be `if:`-skipped. The mechanism is worth stating precisely, because the intuitive version is +wrong: GitHub *does* accept a check run whose conclusion is `skipped`. The problem is that `if:`-skipping +the **caller** job means the reusable never starts, so the nested `validate / validate` context is never +created at all — and a required context with **no check run** for the head commit blocks forever. Reason +about whether a check run exists for the head SHA, not about the word "skipped". After the first run on a test PR: pin the **exact required-check context GitHub reports** — for a reusable-workflow job it is ` / `, expected **`validate / validate`** (the workflow display name is NOT part of the context; copy the literal string from the first run's checks list). diff --git a/docs/fleet-operations.md b/docs/fleet-operations.md index 19e6474..8c6924a 100644 --- a/docs/fleet-operations.md +++ b/docs/fleet-operations.md @@ -109,21 +109,26 @@ checked; the script runs them in this order and exits non-zero if any fires: tag called the fleet uniform throughout. When this fires, nothing below it means anything — fix step 2 of the README's release order first. 2. **Pins** — each deployed caller stub's `uses:` SHA vs that tag. The original check, unchanged. -3. **Content** — the whole waved file vs its `templates/github/` source, byte for byte. This is +3. **Content** — the whole waved file vs its `templates/github/` source, line for line. This is what closes the other two holes: a file with **no `uses:` line at all** (an unconverted 190-line copy of what is now a 66-line stub) is no longer invisible, and `DRIVER_AGENTS_REF` — a raw SHA in an `env:` block that no bot can bump — is now compared like any other line. Two things worth knowing about check 3: -- **Only `SHOPIFY_STORE_NAME` is normalized away.** It is the one difference a correctly-waved repo - is *supposed* to have. Everything else that differs is reported, third-party action pins included: - a repo whose Dependabot bumped `actions/checkout` past the kit's pin is drift worth seeing, and it - means the kit is behind, not that the repo is wrong. +- **Exactly two things are normalized away.** First, `SHOPIFY_STORE_NAME` — the one difference a + correctly-waved repo is *supposed* to have. Second, trailing blank lines and the final newline: + the three stub-rails-only pairs (`Team-Laird@develop`, `The-Gathery@develop`, + `driver-bonsai-mcp@main`) were waved without a final newline and are otherwise identical, and nine + permanently-red rows for a byte nobody can act on is how a detector stops being read. Internal + blank lines *are* compared. Everything else that differs is reported, third-party action pins + included: a repo whose Dependabot bumped `actions/checkout` past the kit's pin is drift worth + seeing, and it means the kit is behind, not that the repo is wrong. - **`DriverDigital/workflows` itself is skipped.** Its `.github/workflows/` holds the *reusables*, which share basenames with the stubs that call them — `pr-first-review.yml` is a ~200-line reusable there and a 25-line stub in the kit — so comparing it against `templates/` would report - six phantom drifts. + seven phantom drifts — the six stubs plus `lint.yml`, whose kit copy is a trimmed version of + this repo's own CI file of the same name. **Still unchecked: the tripwire parity between `templates/` and canonical.** The audit proves the fleet matches `templates/github/claude.yml`; it cannot prove that file's `--append-system-prompt` @@ -171,8 +176,18 @@ the installed stub — the wave covers it anyway. `enforce_admins` is `false` fleet-wide, which is what makes direct-push waves work. Two live kit branches have **no protection at all** — `studio-sulzer@main` and `Team-Laird@develop` (404 on the -protection endpoint). Every other kit branch is protected. The kit's onboarding steps assume a -human-approver rule exists, so on those two a bot signal alone could satisfy a merge. +protection endpoint). Every other kit branch has a protection object — but **having one is not the +same as requiring a human**, and the gap is wider than those two. Surveyed across all 21 pairs +2026-08-02: + +| Pairs | `required_approving_review_count` | | +|---|---|---| +| 11 | `1` | Avara, Driver-Digital-Website, Kissy-Kissy, LaPointe, LittleMe, The-Gathery, client-workspaces, driver-bonsai-mcp, foundrae-blackridge, plugins, vite-plugin-shopify-clean | +| **8** | **`0`** | **every Palmers branch** — `main`, `-au`, `-ca`, `-in`, `-ma`, `-me`, `-sa`, `-uk` | +| **2** | **no protection at all** | **`studio-sulzer@main`, `Team-Laird@develop`** | + +The kit's onboarding steps assume a human-approver rule exists. On **10** of the 21 pairs it does +not, so a bot signal alone could satisfy a merge — not the 2 this section used to name. On this repo, `main` requires **`actionlint`** (set 2026-08-02; before that `required_status_checks` had `strict: true` but empty `contexts`, so `lint.yml` could report red without being able to block). @@ -196,9 +211,18 @@ check does not make it unbypassable for Maria — it makes it unbypassable for e puts a red X in front of an admin who would otherwise have had nothing to override. That is the actual value, and it was worth having either way. -Flipping it to `true` was considered and rejected. The fleet-wide `false` exists because direct-push -repin waves depend on it, and that reasoning genuinely does *not* apply to this repo: no wave has ever -pushed here, and every commit on `main` is a PR merge. The reason to leave it alone anyway is -uniformity — one repo with a different admin rule is a thing to remember at exactly the wrong moment, -and the failure mode it would prevent (an admin knowingly merging red) is not one that has happened. -Revisit if it ever does. +Flipping it to `true` was considered and rejected, and the reason is stronger than uniformity: **this +repo is itself pushed to directly.** No repin *wave* targets it, but six commits on `main` have no +associated PR — including `a54c91e`, the v1.9.0 release commit, pushed 2026-08-01: + +``` +a54c91e feat(kit): rename store app secrets to DRIVER_ENGINEERING_APP_* (v1.9.0) +5bee8c4 Onboarding kit moves here from driver-bonsai-mcp (repo-split Stage 0) +34826c9 fix: dependabot-validate falls back to npm install when no lockfile — v1.5.4 +c960ec1 fix: allowed_bots claude[bot] so round-1 actually reviews — v1.5.2 +69bd828 fix: drop the gh-based author re-check — v1.5.1 +499a51e feat: ticketed-review reusable (Phase 3 ticketed-rail auto-review loop) — v1.5.0 +``` + +`enforce_admins: true` would have blocked every one of those. Releases here have repeatedly gone out +as direct pushes, so the flag stays `false` until that stops being true. diff --git a/templates/github/README.md b/templates/github/README.md index 0fe7f8e..a1db64f 100644 --- a/templates/github/README.md +++ b/templates/github/README.md @@ -135,12 +135,21 @@ Ready to Deploy → Delivered / Deployed / Completed. The workflows never set th - A **local** job reports its bare job id — `lint.yml` reports **`actionlint`**. On a partial install with no `dependabot-validate`, `actionlint` is the one check that runs on - every PR unconditionally, so it is the right thing to require. Do **not** require - `review / review` on its own: `pr-first-review` skips drafts, forks, bot authors and ticketed - PRs by design, and a *skipped* required check counts as not-passed — it would block exactly the - PRs it is meant to leave alone. (`dependabot-validate` avoids this by always running and - branching internally, which is why it must never be `if:`-skipped.) Also add a rule requiring a - **human** approver (e.g. CODEOWNERS) so no bot signal satisfies the merge gate. + every PR unconditionally, so it is the right thing to require — but **run it once and let it go + green before you pin it**. `lint.yml` lints every workflow the repo already has, not just the + ones this kit ships, so a repo with its own hand-written workflows can have findings to fix on + day one. (`actionlint` fails on *info*-level shellcheck findings too. The kit already excludes + `SC2015`, which the runner's older shellcheck still reports and upstream has since dropped; add + further exclusions to `SHELLCHECK_OPTS` in that repo's copy if it needs them.) + + Do **not** require `review / review`. The reason is its **trigger list**, not its `if:` guards: + `pr-first-review.yml` fires on `opened`, `ready_for_review` and `reopened` — deliberately *not* + `synchronize`. Push a follow-up commit and no run is created for that head SHA at all, so the + required context is **missing** rather than skipped, and a missing required check blocks the merge + indefinitely. (A job that *runs* and reports `skipped` is a different case, and GitHub does accept + that as satisfying a required check — so reason from whether a check run exists for the head + commit, not from the word "skipped".) Also add a rule requiring a **human** approver (e.g. + CODEOWNERS) so no bot signal satisfies the merge gate. ## Multi-branch repos (e.g. Palmers — independent release branches) diff --git a/templates/github/lint.yml b/templates/github/lint.yml index d22a3f7..9f7ca06 100644 --- a/templates/github/lint.yml +++ b/templates/github/lint.yml @@ -50,8 +50,19 @@ jobs: echo "${SHA256} actionlint.tar.gz" | sha256sum -c - tar -xzf actionlint.tar.gz actionlint - # With no paths actionlint globs .github/workflows/ itself. + # With no paths actionlint globs .github/workflows/ itself — every workflow the installing + # repo has, not just the ones this kit ships. - name: Lint .github/workflows/ + env: + # SC2015 ("A && B || C is not if-then-else") is the ONLY exclusion, and it is a false + # positive on the shape it fires for here: `cond && action || true`, where the trailing C + # is a literal `true` — a no-op that exists so `set -e` doesn't abort when cond is false. + # C running "unexpectedly" is precisely the intent. Upstream agrees: shellcheck 0.11 no + # longer reports this shape at all, so the finding only appears on the older shellcheck + # preinstalled on the runner. Kept because actionlint treats an INFO-level shellcheck + # finding as a hard failure, and this job is meant to be a required check — without the + # exclusion, ordinary defensive shell in a repo's own workflows blocks every PR. + SHELLCHECK_OPTS: --exclude=SC2015 run: | set -euo pipefail ./actionlint -color diff --git a/tools/fleet-pin-audit.sh b/tools/fleet-pin-audit.sh index 78b3c92..c5b1c18 100755 --- a/tools/fleet-pin-audit.sh +++ b/tools/fleet-pin-audit.sh @@ -12,8 +12,9 @@ # 3. CONTENT — the whole waved file vs its templates/github/ source. The pin is one line of it: # `DRIVER_AGENTS_REF` is a raw SHA in an `env:` block, the implementer's system prompt is just # text, and an unconverted 190-line copy of a workflow that is now a 66-line stub has no -# `uses:` line at all — so a pin grep sees none of them. Only state the kit documents as -# per-repo is normalized away (`SHOPIFY_STORE_NAME`); anything else that differs is drift. +# `uses:` line at all — so a pin grep sees none of them. Exactly two things are normalized +# away — the per-repo store handle and trailing blank lines (see `kit_normalize` for why); +# anything else that differs is drift. # # Scans every non-archived DriverDigital repo's .github/workflows/ (default branch, plus every # main* branch of Palmers — the kit is installed per country branch there). @@ -34,11 +35,22 @@ KIT="$(cd "$(dirname "$0")/../templates/github" && pwd)" LATEST="$(gh api "repos/$ORG/workflows/tags" --jq '.[0] | "\(.name) \(.commit.sha)"')" LATEST_TAG="${LATEST%% *}"; LATEST_SHA="${LATEST#* }"; LATEST_SHA8="${LATEST_SHA:0:8}" -# The one difference a correctly-waved repo is SUPPOSED to have: its own store handle, which the -# kit ships empty. Everything else — third-party action pins included — is reported. A consumer -# repo whose Dependabot bumped `actions/checkout` past the kit's pin is drift worth seeing: it -# means the kit is behind, not that the repo is wrong. -kit_normalize() { sed 's/^\( *SHOPIFY_STORE_NAME:\).*/\1 /'; } +# EXACTLY TWO normalizations, both deliberate. Everything else that differs is reported — third-party +# action pins included: a consumer repo whose Dependabot bumped `actions/checkout` past the kit's pin +# is drift worth seeing, since it means the kit is behind, not that the repo is wrong. +# +# 1. SHOPIFY_STORE_NAME — the one difference a correctly-waved repo is SUPPOSED to have. The kit +# ships it empty; Avara carries "avara". Anchored to a line that STARTS with the key, so the ten +# other mentions per file (comments, shell) still compare normally. +# 2. Trailing blank lines and the final newline. Three stub-rails-only pairs (Team-Laird@develop, +# The-Gathery@develop, driver-bonsai-mcp@main) were waved without a final newline and are +# otherwise byte-identical. That is not drift anyone can act on, and a detector that reports +# nine permanent red rows is a detector nobody reads. Internal blank lines ARE still compared — +# awk buffers blanks and only emits them once a non-blank line follows. +kit_normalize() { + sed 's/^\( *SHOPIFY_STORE_NAME:\).*/\1 /' \ + | awk '/^[[:space:]]*$/ { blank++; next } { while (blank-- > 0) print ""; blank = 0; print }' +} # 1. REFERENCE — templates/ against the latest tag. reference="$( @@ -51,36 +63,55 @@ reference="$( done )" +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT + scan_ref() { # repo ref - local repo="$1" ref="$2" files f raw local_kit deployed + local repo="$1" ref="$2" files f files="$(gh api "repos/$ORG/$repo/contents/.github/workflows?ref=$ref" --jq '.[].name' 2>/dev/null)" || return 0 for f in $files; do - raw="$(gh api "repos/$ORG/$repo/contents/.github/workflows/$f?ref=$ref" -H 'Accept: application/vnd.github.raw' 2>/dev/null)" || continue + # Straight to a file, never a variable: `$(...)` strips ALL trailing newlines, so a deployed + # file differing from the kit only in trailing blank lines would compare equal and report `ok`. + # A drift detector may not have a shape of drift it cannot see. + gh api "repos/$ORG/$repo/contents/.github/workflows/$f?ref=$ref" \ + -H 'Accept: application/vnd.github.raw' > "$TMP/raw" 2>/dev/null || continue # 2. PINS - printf '%s' "$raw" | grep -o "$ORG/workflows/.github/workflows/[^@]*@[0-9a-f]*" \ + grep -o "$ORG/workflows/.github/workflows/[^@]*@[0-9a-f]*" "$TMP/raw" \ | sed "s|$ORG/workflows/.github/workflows/||; s|@\([0-9a-f]\{8\}\)[0-9a-f]*|@\1|" \ | while read -r line; do echo "PIN $repo@$ref $f $line"; done # 3. CONTENT — only for files the kit actually ships. [ -f "$KIT/$f" ] || continue - deployed="$(printf '%s' "$raw" | kit_normalize)" - local_kit="$(kit_normalize < "$KIT/$f")" - if [ "$deployed" = "$local_kit" ]; then + kit_normalize < "$TMP/raw" > "$TMP/deployed" + kit_normalize < "$KIT/$f" > "$TMP/kit" + if command diff -q "$TMP/deployed" "$TMP/kit" >/dev/null 2>&1; then echo "CONTENT $repo@$ref $f ok" else - echo "CONTENT $repo@$ref $f DRIFT $(command diff \ - <(printf '%s\n' "$deployed") <(printf '%s\n' "$local_kit") | grep -c '^[<>]') lines differ" + echo "CONTENT $repo@$ref $f DRIFT $(command diff "$TMP/deployed" "$TMP/kit" \ + | grep -c '^[<>]') lines differ" fi done } +# Enumerate the fleet OUTSIDE the report subshell — a failure here has to be able to kill the run. +# `--limit 200` against ~58 non-archived repos today; the old 100 was a silent truncation cliff. +repos="$(gh repo list "$ORG" --limit 200 --no-archived --json name,defaultBranchRef \ + --jq '.[] | "\(.name) \(.defaultBranchRef.name)"')" || repos="" +if [ -z "$repos" ]; then + echo "FATAL: could not enumerate $ORG repos (gh failed, or auth/network is down)." >&2 + echo "This run proves NOTHING. An empty scan is not a clean fleet — do not read it as one." >&2 + exit 2 +fi + report="$( - gh repo list "$ORG" --limit 100 --no-archived --json name,defaultBranchRef \ - --jq '.[] | "\(.name) \(.defaultBranchRef.name)"' | while read -r repo def; do + printf '%s\n' "$repos" | while read -r repo def; do # Skip the kit repo itself: its .github/workflows/ holds the REUSABLES, which share basenames # with the stubs that call them (pr-first-review.yml is a 200-line reusable here and a 25-line - # stub in the kit), so a content compare against templates/ would report six phantom drifts. + # stub in the kit), so a content compare against templates/ would report seven phantom drifts: + # the six stubs, plus lint.yml, whose kit copy is a trimmed version of the CI file of the same + # name here. NB: no apostrophes in comments inside this $( ) — bash opens a quote on one even + # in a comment, and the parse error it produces points at EOF, not at the line. [ "$repo" = "workflows" ] && continue scan_ref "$repo" "$def" if [ "$repo" = "Palmers" ]; then @@ -95,6 +126,16 @@ stale="$(printf '%s\n' "$pins" | grep -v "@$LATEST_SHA8")" content="$(printf '%s\n' "$report" | grep '^CONTENT ' | sed 's/^CONTENT //')" drift="$(printf '%s\n' "$content" | grep ' DRIFT ')" +# Every kit repo@branch pair carries at least one caller stub, so zero pins fleet-wide means the +# scan read nothing — rate limiting, a revoked token, an org rename. Without this the run falls +# straight through to "(converged)" and exit 0, which is the exact failure this whole tool exists +# to stop: a clean report that proves nothing. +if [ -z "$pins" ]; then + echo "FATAL: zero caller-stub pins found across $(printf '%s\n' "$repos" | grep -c .) repos." >&2 + echo "The fleet always carries some, so the scan failed to read — this is not a clean fleet." >&2 + exit 2 +fi + if [ "${1:-}" = "--stale" ]; then [ -n "$reference" ] && { echo "reference drift (templates/ is not at $LATEST_TAG):"; printf '%s\n' "$reference"; echo; } [ -n "$stale" ] && { echo "stale pins:"; printf '%s\n' "$stale"; echo; }