[PR #324] [MERGED] ci(bump-callers): exclude tests/ and README.md from the agents-md, coderabbit-config, groom and cursor-review fleets #323

New issue

Closed

opened 2026-10-26 22:38:29 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:38:29 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/324
Author: @mattmillerai
Created: 9/19/2026
Status: ✅ Merged
Merged: 9/20/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-15267-bump-exclude-tests-readme

📝 Commits (3)

eac8756 ci(bump-callers): exclude tests/ and README.md from the agents-md, coderabbit-config, groom and cursor-review fleets
94bb741 docs(bump-callers): fix the coderabbit-config branch name and de-list the excluding fleets
ce3cb99 ci(bump-callers): exclude catalog-drift.py and schema_drift.py from the cursor-review and coderabbit-config fleet triggers (#327)

📊 Changes

6 files changed (+167 additions, -16 deletions)

View changed files

📝 .github/bump-callers/README.md (+27 -10)
📝 .github/bump-callers/tests/test_paths_contract.sh (+48 -0)
📝 .github/workflows/bump-agents-md-callers.yml (+18 -0)
📝 .github/workflows/bump-coderabbit-config-callers.yml (+26 -0)
📝 .github/workflows/bump-cursor-review-callers.yml (+30 -6)
📝 .github/workflows/bump-groom-callers.yml (+18 -0)

📄 Description

ELI-5

Some robots watch a folder and, whenever anything in it changes, they run
around to every project that uses that folder and file a "please update your
bookmark" pull request. Trouble is, they also did this when someone only
edited the folder's tests/ or its README.md — files nobody's project
actually runs — so everyone got a pointless PR to review. Five of these
robots had already been taught to ignore those two things; this change
teaches the last four (agents-md, coderabbit-config, groom, cursor-review)
the same lesson, using the exact same spelling.

There's a second, subtler trap: if you tell the robot "ignore tests/" in
its trigger but forget to tell its staleness check the same thing, the
check keeps seeing the folder as "changed" and the robot freezes forever.
So every exclusion is mirrored into WATCHED_PATHSPECS, and a contract test
(test_paths_contract.sh) fails the build if the two ever disagree.

What changed

For bump-agents-md-callers.yml, bump-coderabbit-config-callers.yml,
bump-groom-callers.yml and bump-cursor-review-callers.yml:

Added '!<dir>/tests/**' and '!<dir>/README.md' to the push: paths:
filter, with a short comment modelled on the clean fleets.
Mirrored those exclusions into WATCHED_PATHSPECS as
:(exclude)<dir>/tests (no /**) and :(exclude)<dir>/README.md, carrying
the canonical two-line "MIRRORS the paths: filter above … test_paths_contract.sh
fails the build otherwise" comment. The three flat fleets get a new block;
cursor-review's existing block (which already excluded check-pr-size/*_test.go)
gains the two new lines, and its inline note is widened so it no longer credits
only the *_test.go exclusion.
WATCHED_ASSETS is left as-is — it still drives the decommission probe and
the coverage assertion. WATCHED_EXEC (added to these fleets by a prior PR)
is untouched; no WATCHED_EXEC was added here (that is a separate,
out-of-scope change).

Docs (.github/bump-callers/README.md): the fleet-table "Triggers on" rows for
the four fleets now say "(minus its tests/ and README.md, which no caller
executes)"; the stale WATCHED_PATHSPECS "Used by" list is replaced with "every
fleet whose paths: filter carries a ! exclusion"; and the parenthetical that
singled out pr-risk as the only excluding fleet is generalised (my change makes
three more fleets excluding fleets, so leaving it would be newly inaccurate).

Prerequisite / neighbour status (both already merged)

Prerequisite — the contract-test glob-flatness guard fix (the /**
directory-exclusion skip) is merged on main (#308). Without it the
cursor-review hunk would fail at test_paths_contract.sh with a misleading
"** in a SUBDIRECTORY" message, because .github/cursor-review/tests/ has a
fixtures/ subdirectory. Confirmed passing here.
Neighbour — the WATCHED_EXEC change to five fleets is merged (#302). It
edits the same preflight env: blocks, so line numbers drifted from the
ticket's diff (written against cf7b8d9); the new WATCHED_PATHSPECS blocks
were woven in above the existing WATCHED_EXEC, matching pr-risk's env ordering.

Verification

test_paths_contract.sh → 151 passed, 0 failed (higher than the ticket's
107 because suites were added since; all four target fleets report
"WATCHED_PATHSPECS mirrors the paths: filter, exclusions included", and each
/** and README.md exclusion is recognised as EQUIVALENT — the prerequisite
fix at work).
test_preflight.sh → 445 passed, 0 failed; test_bump_callers.sh →
437 passed, 0 failed (both exactly as the ticket predicted).
Repo-wide lints green: check_workflow_pins.py, the org-repo-literal lint,
and check_agents_md.py (two pre-existing, unrelated warnings). All four YAMLs
parse.
Real-object staleness proof against the live repro cf7b8d9 (tests+README
only under .github/groom/):
- bare comparison git diff --quiet cf7b8d9^ cf7b8d9 -- .github/workflows/groom.yml .github/groom → exit 1 (reads as changed → would freeze the fleet);
- with ':(exclude).github/groom/tests' ':(exclude).github/groom/README.md' appended → exit 0 (correctly reads as unchanged).
Safety grep over the four asset dirs: no consumer-executed reusable/script
reads tests/ or README.md at run time — every hit is a test-file string, a
code comment (key-broker.mjs), or a docstring (catalog-drift.py, which is
deliberately absent from WATCHED_EXEC).
Merging this fires no bump fleet: bump-*-callers.yml and
.github/bump-callers/** are in no fleet's paths: filter (only CI's
test-bump-callers.yml runs).

Residual

Nothing is left unfixed for this ticket's stated scope — all four fleets carry
the exclusions and mirrored pathspecs, docs are in sync, and the prerequisite
and neighbour both landed. Deliberately out of scope, per the ticket:

WATCHED_EXEC for the five fleets (a separate change, already merged as #302).
The ALLOW_EMPTY / roster-description doc fixes the source spike found to
misdescribe non-empty rosters (README.md roster column; the ALLOW_EMPTY
comments in bump-groom-callers.yml and bump-agents-md-callers.yml) — a
separate docs fix, untouched here.
bump-pr-size-callers.yml (already correct; not in scope).

Raised in review and deferred to a tracked follow-up (not dropped):

The same "no pinned caller executes it" rule also applies to four CI-only
scripts still inside the watched globs — catalog-drift.py and
wire-bot-identity.py (cursor-review), schema_drift.py (coderabbit-config)
and key-broker.mjs (groom), each already documented as DELIBERATELY ABSENT
from its fleet's WATCHED_EXEC. Excluding individually-named scripts needs a
per-file call this PR's scope does not make (key-broker.mjs may simply be
dead code), so it is filed separately.
The pre-existing scripts/check-pr-size/*_test.go exclusion matches
differently on the two sides (git's default pathspec * crosses /, Actions'
does not). Unreachable today — that directory is a flat, single Go package —
and its pathspec lines are byte-identical to bump-pr-size-callers.yml by
design, so aligning them is a pr-size-fleet change too. Left as-is, explained
on the thread.

Unexercised artifacts (named for honesty): the source Linear tickets and the
GitHub issue that motivated this work could not be opened from the sandbox;
their conclusions were verified instead through the merged prerequisite/neighbour
PRs (#302, #308) and the real-object proof above. The historical churn PR the
repro produced was referenced but not re-run (it is evidence of the bug, not an
artifact to exercise).

Review round

Follow-up commit 94bb741 (comment/docs only — no paths:, pathspec or
behavior change) addresses the two review findings that landed on text this PR
itself adds:

bump-coderabbit-config-callers.yml's new comment named the stable branch
ci/bump-coderabbit-config; bump-callers.sh builds ci/bump-${TAG} and this
fleet's TAG is coderabbit-config-validate, so it contradicted the
concurrency comment in the same file. Corrected, with the tag/directory
mismatch spelled out inline (this is the only fleet where they differ).
The README parenthetical enumerating the excluding fleets was already
incomplete — area-label, linear-ticket and public-repo-hygiene carry !
exclusions and set WATCHED_PATHSPECS too — and its "since this change"
wording would go stale on merge. Replaced with the generalized phrasing this
PR already adopted for the WATCHED_PATHSPECS input row.

All four review threads answered and resolved.

Provenance

Authored by: agent-work loop
Verified: test_paths_contract.sh (151/0), test_preflight.sh (445/0),
test_bump_callers.sh (437/0), check_workflow_pins.py, org-repo-literal lint,
check_agents_md.py, YAML parse of all four workflows, and the real-object
git diff staleness proof against cf7b8d9. Re-run unchanged after
94bb741: test_paths_contract.sh (151/0), test_preflight.sh (445/0),
test_bump_callers.sh (437/0), shellcheck -x over the bump-callers
scripts and tests, check_workflow_pins.py, the org-repo-literal lint and
check_agents_md.py (same two pre-existing, unrelated warnings).
Deviations: none from the ticket plan, beyond weaving the new
WATCHED_PATHSPECS blocks around the already-merged WATCHED_EXEC (line
numbers in the ticket's diff predate #302) and one honesty edit to a README
parenthetical that my change would otherwise have made inaccurate — which
review then found was still under-inclusive, corrected in 94bb741 by
dropping the fleet enumeration entirely.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/324 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/19/2026 **Status:** ✅ Merged **Merged:** 9/20/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-15267-bump-exclude-tests-readme` --- ### 📝 Commits (3) - [`eac8756`](https://github.com/Comfy-Org/github-workflows/commit/eac8756f9e70ae5ab1073dd6509eace4f09850f7) ci(bump-callers): exclude tests/ and README.md from the agents-md, coderabbit-config, groom and cursor-review fleets - [`94bb741`](https://github.com/Comfy-Org/github-workflows/commit/94bb741ed94fa61f76d6ad73e59aa41bbafb4938) docs(bump-callers): fix the coderabbit-config branch name and de-list the excluding fleets - [`ce3cb99`](https://github.com/Comfy-Org/github-workflows/commit/ce3cb9935d89bc2ebee1b0453ae8e4be7f0d6bb0) ci(bump-callers): exclude catalog-drift.py and schema_drift.py from the cursor-review and coderabbit-config fleet triggers (#327) ### 📊 Changes **6 files changed** (+167 additions, -16 deletions) <details> <summary>View changed files</summary> 📝 `.github/bump-callers/README.md` (+27 -10) 📝 `.github/bump-callers/tests/test_paths_contract.sh` (+48 -0) 📝 `.github/workflows/bump-agents-md-callers.yml` (+18 -0) 📝 `.github/workflows/bump-coderabbit-config-callers.yml` (+26 -0) 📝 `.github/workflows/bump-cursor-review-callers.yml` (+30 -6) 📝 `.github/workflows/bump-groom-callers.yml` (+18 -0) </details> ### 📄 Description ## ELI-5 Some robots watch a folder and, whenever *anything* in it changes, they run around to every project that uses that folder and file a "please update your bookmark" pull request. Trouble is, they also did this when someone only edited the folder's `tests/` or its `README.md` — files nobody's project actually runs — so everyone got a pointless PR to review. Five of these robots had already been taught to ignore those two things; this change teaches the last four (agents-md, coderabbit-config, groom, cursor-review) the same lesson, using the exact same spelling. There's a second, subtler trap: if you tell the robot "ignore `tests/`" in its trigger but forget to tell its *staleness check* the same thing, the check keeps seeing the folder as "changed" and the robot freezes forever. So every exclusion is mirrored into `WATCHED_PATHSPECS`, and a contract test (`test_paths_contract.sh`) fails the build if the two ever disagree. ## What changed For `bump-agents-md-callers.yml`, `bump-coderabbit-config-callers.yml`, `bump-groom-callers.yml` and `bump-cursor-review-callers.yml`: - Added `'!<dir>/tests/**'` and `'!<dir>/README.md'` to the `push:` `paths:` filter, with a short comment modelled on the clean fleets. - Mirrored those exclusions into `WATCHED_PATHSPECS` as `:(exclude)<dir>/tests` (no `/**`) and `:(exclude)<dir>/README.md`, carrying the canonical two-line "MIRRORS the `paths:` filter above … `test_paths_contract.sh` fails the build otherwise" comment. The three flat fleets get a new block; cursor-review's existing block (which already excluded `check-pr-size/*_test.go`) gains the two new lines, and its inline note is widened so it no longer credits only the `*_test.go` exclusion. - `WATCHED_ASSETS` is left as-is — it still drives the decommission probe and the coverage assertion. `WATCHED_EXEC` (added to these fleets by a prior PR) is untouched; **no `WATCHED_EXEC` was added here** (that is a separate, out-of-scope change). Docs (`.github/bump-callers/README.md`): the fleet-table "Triggers on" rows for the four fleets now say "(minus its `tests/` and `README.md`, which no caller executes)"; the stale `WATCHED_PATHSPECS` "Used by" list is replaced with "every fleet whose `paths:` filter carries a `!` exclusion"; and the parenthetical that singled out `pr-risk` as the only excluding fleet is generalised (my change makes three more fleets excluding fleets, so leaving it would be newly inaccurate). ## Prerequisite / neighbour status (both already merged) - **Prerequisite** — the contract-test glob-flatness guard fix (the `/**` directory-exclusion skip) is merged on `main` (#308). Without it the cursor-review hunk would fail at `test_paths_contract.sh` with a misleading "`**` in a SUBDIRECTORY" message, because `.github/cursor-review/tests/` has a `fixtures/` subdirectory. Confirmed passing here. - **Neighbour** — the `WATCHED_EXEC` change to five fleets is merged (#302). It edits the same preflight `env:` blocks, so line numbers drifted from the ticket's diff (written against `cf7b8d9`); the new `WATCHED_PATHSPECS` blocks were woven in above the existing `WATCHED_EXEC`, matching pr-risk's env ordering. ## Verification - `test_paths_contract.sh` → **151 passed, 0 failed** (higher than the ticket's 107 because suites were added since; all four target fleets report "WATCHED_PATHSPECS mirrors the `paths:` filter, exclusions included", and each `/**` and `README.md` exclusion is recognised as EQUIVALENT — the prerequisite fix at work). - `test_preflight.sh` → **445 passed, 0 failed**; `test_bump_callers.sh` → **437 passed, 0 failed** (both exactly as the ticket predicted). - Repo-wide lints green: `check_workflow_pins.py`, the org-repo-literal lint, and `check_agents_md.py` (two pre-existing, unrelated warnings). All four YAMLs parse. - **Real-object staleness proof** against the live repro `cf7b8d9` (tests+README only under `.github/groom/`): - bare comparison `git diff --quiet cf7b8d9^ cf7b8d9 -- .github/workflows/groom.yml .github/groom` → exit **1** (reads as changed → would freeze the fleet); - with `':(exclude).github/groom/tests' ':(exclude).github/groom/README.md'` appended → exit **0** (correctly reads as unchanged). - **Safety grep** over the four asset dirs: no consumer-executed reusable/script reads `tests/` or `README.md` at run time — every hit is a test-file string, a code comment (`key-broker.mjs`), or a docstring (`catalog-drift.py`, which is deliberately absent from `WATCHED_EXEC`). - Merging this fires **no** bump fleet: `bump-*-callers.yml` and `.github/bump-callers/**` are in no fleet's `paths:` filter (only CI's `test-bump-callers.yml` runs). ## Residual Nothing is left unfixed for this ticket's stated scope — all four fleets carry the exclusions and mirrored pathspecs, docs are in sync, and the prerequisite and neighbour both landed. Deliberately out of scope, per the ticket: - `WATCHED_EXEC` for the five fleets (a separate change, already merged as #302). - The `ALLOW_EMPTY` / roster-description doc fixes the source spike found to misdescribe non-empty rosters (`README.md` roster column; the `ALLOW_EMPTY` comments in `bump-groom-callers.yml` and `bump-agents-md-callers.yml`) — a separate docs fix, untouched here. - `bump-pr-size-callers.yml` (already correct; not in scope). Raised in review and deferred to a tracked follow-up (not dropped): - The same "no pinned caller executes it" rule also applies to four CI-only scripts still inside the watched globs — `catalog-drift.py` and `wire-bot-identity.py` (cursor-review), `schema_drift.py` (coderabbit-config) and `key-broker.mjs` (groom), each already documented as DELIBERATELY ABSENT from its fleet's `WATCHED_EXEC`. Excluding individually-named scripts needs a per-file call this PR's scope does not make (`key-broker.mjs` may simply be dead code), so it is filed separately. - The pre-existing `scripts/check-pr-size/*_test.go` exclusion matches differently on the two sides (git's default pathspec `*` crosses `/`, Actions' does not). Unreachable today — that directory is a flat, single Go package — and its pathspec lines are byte-identical to `bump-pr-size-callers.yml` by design, so aligning them is a pr-size-fleet change too. Left as-is, explained on the thread. Unexercised artifacts (named for honesty): the source Linear tickets and the GitHub issue that motivated this work could not be opened from the sandbox; their conclusions were verified instead through the merged prerequisite/neighbour PRs (#302, #308) and the real-object proof above. The historical churn PR the repro produced was referenced but not re-run (it is evidence of the bug, not an artifact to exercise). ## Review round Follow-up commit `94bb741` (comment/docs only — no `paths:`, pathspec or behavior change) addresses the two review findings that landed on text this PR itself adds: - `bump-coderabbit-config-callers.yml`'s new comment named the stable branch `ci/bump-coderabbit-config`; `bump-callers.sh` builds `ci/bump-${TAG}` and this fleet's `TAG` is `coderabbit-config-validate`, so it contradicted the concurrency comment in the same file. Corrected, with the tag/directory mismatch spelled out inline (this is the only fleet where they differ). - The README parenthetical enumerating the excluding fleets was already incomplete — `area-label`, `linear-ticket` and `public-repo-hygiene` carry `!` exclusions and set `WATCHED_PATHSPECS` too — and its "since this change" wording would go stale on merge. Replaced with the generalized phrasing this PR already adopted for the `WATCHED_PATHSPECS` input row. All four review threads answered and resolved. ## Provenance - **Authored by:** agent-work loop - **Verified:** `test_paths_contract.sh` (151/0), `test_preflight.sh` (445/0), `test_bump_callers.sh` (437/0), `check_workflow_pins.py`, org-repo-literal lint, `check_agents_md.py`, YAML parse of all four workflows, and the real-object `git diff` staleness proof against `cf7b8d9`. Re-run unchanged after `94bb741`: `test_paths_contract.sh` (151/0), `test_preflight.sh` (445/0), `test_bump_callers.sh` (437/0), `shellcheck -x` over the bump-callers scripts and tests, `check_workflow_pins.py`, the org-repo-literal lint and `check_agents_md.py` (same two pre-existing, unrelated warnings). - **Deviations:** none from the ticket plan, beyond weaving the new `WATCHED_PATHSPECS` blocks around the already-merged `WATCHED_EXEC` (line numbers in the ticket's diff predate #302) and one honesty edit to a README parenthetical that my change would otherwise have made inaccurate — which review then found was still under-inclusive, corrected in `94bb741` by dropping the fleet enumeration entirely. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:38:29 +00:00