-
Notifications
You must be signed in to change notification settings - Fork 0
ci(bump-callers): add detect-unreviewed-merge caller fleet (BE-6294) #117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+309
−14
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
9afca0a
ci(bump-callers): add detect-unreviewed-merge caller fleet (BE-6294)
mattmillerai 577b1c8
ci(bump-callers): harden the detect-unreviewed-merge entrypoint (BE-6…
mattmillerai 1d81fe9
ci(bump-callers): seed the detect-unreviewed-merge roster and fix the…
mattmillerai 72d5d79
ci(bump-callers): ship the detect-unreviewed-merge roster unseeded, n…
mattmillerai 9b93a60
ci(bump-callers): stop the header contradicting itself on seeding (BE…
mattmillerai a8ef9a8
ci(bump-callers): stop the ALLOW_EMPTY note contradicting the pre-see…
mattmillerai bdd99d9
Merge remote-tracking branch 'origin/main' into matt/be-6294-detect-u…
mattmillerai File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
270 changes: 270 additions & 0 deletions
270
.github/workflows/bump-detect-unreviewed-merge-callers.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,270 @@ | ||
| name: Bump detect-unreviewed-merge callers | ||
|
|
||
| # When detect-unreviewed-merge.yml 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. | ||
| # | ||
| # 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, auto-label 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 | ||
| # detect-unreviewed-merge.yml change must not spuriously bump the other fleets' | ||
| # callers, and vice versa.) | ||
| # | ||
| # Unlike cursor-review/groom/pr-size, this reusable loads NOTHING at run time — | ||
| # no `workflows_ref:` input, no prompt/script directory — so the workflow file | ||
| # itself is the whole surface a caller pins, and the path filter has a single | ||
| # entry. | ||
| # | ||
| # 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 `DETECT_UNREVIEWED_MERGE_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 the | ||
| # other fleets' variables. `bump-callers.sh` `::add-mask::`es every repo name | ||
| # before IT echoes anything. | ||
| # KNOWN GAP (shared by every bump-* entrypoint, 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. | ||
| # | ||
| # CONSEQUENCE — this fleet ships DELIBERATELY UNSEEDED. Every other seeded fleet | ||
| # already publishes its roster this way, so their names are out; this fleet's | ||
| # callers include two non-public repos that no existing roster contains, and | ||
| # seeding before the masking fix lands would publish them on the very first run. | ||
| # A public log entry cannot be unpublished; a red run can. So the variable stays | ||
| # unset until the masking fix lands, and the resulting hard-fail is the | ||
| # intentional nag (see ALLOW_EMPTY below) rather than an oversight. Seed it — | ||
| # `gh variable set` per the update flow below — as the immediate follow-on to | ||
| # that fix, NOT as a way to turn the red run green. | ||
| # | ||
| # ALLOW_EMPTY is set to "false" below — already `bump-callers.sh`'s default, but | ||
| # stated explicitly (as `bump-auto-label-callers.yml` does) so the property | ||
| # survives a future flip of that default. This fleet has a known, non-empty set | ||
| # of LIVE CALLERS, so an empty run is never "this fleet has no members" and must | ||
| # never be a silent no-op: that would leave every caller un-bumped with nobody | ||
| # noticing, exactly the drift this directory exists to prevent. | ||
| # | ||
| # READ THIS BEFORE ACTING ON A RED RUN. An empty roster right now is the EXPECTED | ||
| # PRE-SEED STATE described above, not a clobbered variable. Until the masking fix | ||
| # lands, the correct response to that red run is to leave it red — do NOT "fix" | ||
| # it by seeding the variable, which is precisely the irreversible disclosure this | ||
| # header exists to prevent. Once masking is in place, seeding is the follow-on | ||
| # and the run goes green on its own. (After that point, and only after it, an | ||
| # empty roster does mean the variable was clobbered and should be restored.) | ||
| # Either way the answer is never to flip ALLOW_EMPTY on. | ||
| # | ||
| # Update flow — adding/removing a caller needs NO public commit: | ||
| # gh variable set DETECT_UNREVIEWED_MERGE_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: | ||
| - .github/workflows/detect-unreviewed-merge.yml | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| # Serialize runs of this fleet. The bumper pushes to a STABLE branch | ||
| # (ci/bump-detect-unreviewed-merge) 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). | ||
| # Edge, shared by all seven bump-* entrypoints: "newest pending" is by ENQUEUE | ||
| # time, not by commit date, so manually re-running a superseded run while a | ||
| # newer push run is still pending evicts that newer run — the re-run then finds | ||
| # WATCHED changed and correctly no-ops, and nobody pins the newer SHA. It is | ||
| # self-healing (the next change to WATCHED bumps everything forward) and | ||
| # recoverable now (`workflow_dispatch` from main pins the current tip), so it is | ||
| # documented rather than guarded against here: the guard belongs in the shared | ||
| # staleness script, not forked into one of seven copies. | ||
| concurrency: | ||
| group: bump-detect-unreviewed-merge-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 | ||
|
mattmillerai marked this conversation as resolved.
|
||
| # 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@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| persist-credentials: false | ||
|
|
||
| - name: Generate Cloud Code Bot token | ||
| uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1.12.0 — pinned: see dependabot.yml ignore (v2+ breaks cross-repo `owner:` token scoping) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| 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 auto-label and groom fleets' entrypoints. | ||
| owner: Comfy-Org | ||
|
mattmillerai marked this conversation as resolved.
mattmillerai marked this conversation as resolved.
mattmillerai marked this conversation as resolved.
mattmillerai marked this conversation as resolved.
|
||
| 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: DETECT_UNREVIEWED_MERGE_CALLERS | ||
| TAG: detect-unreviewed-merge | ||
| WORKFLOW_FILE: detect-unreviewed-merge.yml | ||
|
mattmillerai marked this conversation as resolved.
|
||
| # Redundant with bump-callers.sh's own default, deliberately — see the | ||
| # header: this fleet has a known non-empty set of live callers, so an | ||
| # empty roster is never "this fleet has no members" and must fail | ||
| # loudly regardless of what that default becomes. RIGHT NOW that | ||
| # failure is EXPECTED — the roster ships unseeded until the masking | ||
| # fix lands (header, "READ THIS BEFORE ACTING ON A RED RUN"). Do NOT | ||
| # respond to it by seeding the variable; only after masking is in | ||
| # place does an empty roster mean it was clobbered. | ||
| 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.DETECT_UNREVIEWED_MERGE_CALLERS }} | ||
|
mattmillerai marked this conversation as resolved.
mattmillerai marked this conversation as resolved.
mattmillerai marked this conversation as resolved.
mattmillerai marked this conversation as resolved.
|
||
| 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. | ||
| # `--refs` plus an exact-refname match, not just the first line: git | ||
| # matches ref patterns at component boundaries, so a branch literally | ||
| # named `foo/refs/heads/main` also matches this pattern and could be | ||
| # the line a bare `%%\t*` parse consumes. | ||
| if ! ls_remote=$(git ls-remote --refs origin refs/heads/main); then | ||
| echo "::error::Could not look up the current main tip (git ls-remote failed)" | ||
| exit 1 | ||
| fi | ||
| main_tip=$(awk '$2 == "refs/heads/main" { print $1; exit }' <<<"$ls_remote") | ||
|
mattmillerai marked this conversation as resolved.
|
||
| if [[ -z "$main_tip" ]]; then | ||
| echo "::error::git ls-remote returned no SHA for refs/heads/main" | ||
| exit 1 | ||
| fi | ||
| WATCHED=.github/workflows/detect-unreviewed-merge.yml | ||
| if [[ "$main_tip" != "$GITHUB_SHA" ]]; then | ||
|
mattmillerai marked this conversation as resolved.
|
||
| # 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 | ||
| # Prove FETCH_HEAD resolves to a real commit BEFORE reading blobs | ||
| # out of it. `git rev-parse --verify --quiet` returns empty both for | ||
| # "that path is absent from this tree" and for "this revision could | ||
| # not be resolved at all" (a partial fetch, an unexpected FETCH_HEAD | ||
| # state). Without this guard the second case is indistinguishable | ||
| # from deletion and would exit 0 as "decommissioned" — the same | ||
| # "a lookup we couldn't perform is not evidence" anti-pattern the | ||
| # ls-remote guard above rejects, but silently no-opping the whole | ||
| # fleet. With it, an empty tip_blob genuinely means absent-from-tree. | ||
| if ! main_tip=$(git rev-parse --verify --quiet "FETCH_HEAD^{commit}"); then | ||
| echo "::error::Fetched main but FETCH_HEAD does not resolve to a commit — cannot compare $WATCHED" | ||
| exit 1 | ||
| fi | ||
| tip_blob=$(git rev-parse --verify --quiet "FETCH_HEAD:$WATCHED" || true) | ||
| # HEAD is this run's own checkout, so it must resolve. An empty | ||
| # here_blob means $WATCHED is absent at github.sha, which is the | ||
| # deletion-commit case the final guard handles — don't let it fall | ||
| # into the "changed since" branch below and be reported as a stale | ||
| # re-run, which would be a misleading log for a real decommission. | ||
| if ! here_blob=$(git rev-parse --verify --quiet "HEAD:$WATCHED"); then | ||
| echo "::warning::$WATCHED is absent at this run's own commit $GITHUB_SHA — treating as decommissioned and bumping nothing. If any caller still pins it, retire those callers." | ||
| exit 0 | ||
| fi | ||
| if [[ -z "$tip_blob" ]]; then | ||
|
mattmillerai marked this conversation as resolved.
|
||
| # ::warning:: not a bare echo: if the reusable was deleted while | ||
| # live callers still pin it, they all hard-fail at startup and a | ||
| # silently-green run here is the fleet's only chance to say so. | ||
| echo "::warning::$WATCHED no longer exists on main ($main_tip) — treating as decommissioned and bumping nothing. If any caller still pins it, retire those callers." | ||
| exit 0 | ||
| fi | ||
| if [[ "$tip_blob" != "$here_blob" ]]; then | ||
|
mattmillerai marked this conversation as resolved.
mattmillerai marked this conversation as resolved.
mattmillerai marked this conversation as resolved.
|
||
| 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 | ||
| # Pin callers to the VERIFIED TIP, not to this run's stale | ||
| # github.sha. We have just proved $WATCHED is byte-identical at | ||
| # both, so the tip is the same reusable content at a commit that is | ||
| # actually current — pinning the older SHA would hand every caller a | ||
| # non-tip commit (and, on a land-then-revert, re-pin them backwards). | ||
| # | ||
| # COUPLED TO THE PATH FILTER — this is only sound because the `paths:` | ||
| # trigger has exactly ONE entry, so a single-blob comparison covers | ||
| # everything a caller pins at this SHA. If you ever widen that filter | ||
| # (an asset directory, a `workflows_ref` surface, as cursor-review / | ||
| # groom / pr-size already have), this comparison silently | ||
| # under-verifies: callers would be pinned to a tip whose other | ||
| # relevant content was never compared. Widen the comparison to every | ||
| # filtered path in the same change, or drop this re-point. | ||
| echo "main moved to $main_tip since $GITHUB_SHA, but $WATCHED is unchanged — this run is still the only one for that change; pinning callers to $main_tip and proceeding" | ||
| # `export` explicitly: the env: binding already marks it exported, | ||
| # but the script is a separate process and this must not depend on | ||
| # that inherited attribute surviving a reassignment. | ||
| export NEW_SHA="$main_tip" | ||
|
mattmillerai marked this conversation as resolved.
|
||
| fi | ||
| # The push path filter also matches a commit that DELETES the | ||
| # reusable workflow; bumping callers to a SHA where it is gone would | ||
| # break every caller. Deletion means decommissioning — no-op. | ||
| # Test "$WATCHED", not a second copy of the literal path: two literals | ||
| # drift apart on a rename, and the stale one would name a file that | ||
| # never exists, making this test always true and the whole fleet a | ||
| # permanent silent no-op. | ||
| if [[ ! -f "$WATCHED" ]]; then | ||
| echo "::warning::$WATCHED absent at this SHA — treating as decommissioned and bumping nothing. If any caller still pins it, retire those callers." | ||
| exit 0 | ||
| fi | ||
| bash .github/bump-callers/bump-callers.sh | ||
|
mattmillerai marked this conversation as resolved.
mattmillerai marked this conversation as resolved.
mattmillerai marked this conversation as resolved.
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.