Repository navigation
[groom] Four bump fleets watch their whole asset directory, tests and README included, so a test-only commit fans a churn bump PR across the fleet #214
Description
Groom auto-builder — Comfy-Org/github-workflows: this CONFIRMED finding could not be auto-built (patch modifies a CI-privileged path (per .github/groom/patch_policy.py — workflow/action defs, lockfiles, or build/test config that executes in pre-review CI) — a human must author these changes), so it is filed for a human. · run
Verified
Fleets whose push paths filter carries the exclusions: bump-area-label (35-36), bump-linear-ticket (41-42), bump-pr-risk (87-88), bump-pr-derisk (106-109), bump-public-repo-hygiene (46-47) - each excluding the tool tests tree and its README.md, with the documented reason that a test-only or docs-only commit would fan a pure-churn SHA-bump PR across the whole fleet. bump-pr-size excludes scripts/check-pr-size/*_test.go with a five-line rationale (that tree has no README). So the comparison set is five fleets excluding tests+README and one excluding tests only - the finder said six excluding exactly those paths, which is close but not exact.
Fleets that have an asset directory and NO exclusion:
- bump-agents-md-callers.yml:48-50 - .github/agents-md-integrity/**, which contains tests/ and README.md.
- bump-coderabbit-config-callers.yml:60-62 - .github/coderabbit-config/**, same.
- bump-groom-callers.yml:58-65 - .github/groom/**, whose tests/ holds 9 files.
- bump-cursor-review-callers.yml:56-72 - inconsistent inside one file: it excludes scripts/check-pr-size/*_test.go but pulls in .github/cursor-review/** (which contains tests/ and README.md) with no exclusion.
The remaining three fleets (auto-label, detect-unreviewed-merge, assign-reviewers) watch a workflow file only and are out of scope.
Impact (counterfactual)
A tests-only or README-only commit under those directories would start a bump run that mints the org-wide App token, force-resets the fleet stable ci/bump-* branch and opens a review round in every enrolled caller repo for a diff with no consumer-visible behaviour change.
Fix
Add the two negations to the four filters, and in the SAME change move those fleets preflight inputs across: preflight.sh header is explicit that when a filter carries exclude entries, the bare WATCHED_ASSETS tree-OID comparison reads a test-only commit as changed-since and produces a FALSE stale verdict that freezes the fleet - so these four need WATCHED_PATHSPECS mirroring the filter (exclusions included) plus WATCHED_EXEC, exactly as bump-pr-risk, bump-pr-size and bump-cursor-review already do for their excluded trees. .github/bump-callers/tests/test_paths_contract.sh enforces the mirror in both directions, so a half-done change fails there rather than silently.
Risk
Low ONLY IF the preflight inputs move at the same time; doing the filter alone is the documented fleet-freeze failure mode. That makes this more than add-two-lines-times-four - treat the pathspec migration as the main body of work.
Steelman
For groom the exclusion is least obviously right, because .github/groom mixes executed prose (the finder/verifier/builder briefs, which consumers DO run at the pinned SHA) with tests; but that argues for excluding tests/ and README.md specifically, which is exactly the proposal.
Activity
mattmillerai commentedon Sep 18, 2026
Triage: confirmed, and it fired on the tip of
maintoday. Leaving this open, tracked internally.Re-derived the census against
origin/main@cf7b8d9c2614ad9f8ee41acf245381ad76fcdc8b; the four-fleet defect set is unchanged and the body's line numbers are still exact.A live reproduction, not a counterfactual.
cf7b8d9("test(groom): prove name resolution is dead in the sandbox, non-vacuously", #300) — the current tip — touches exactly two files:.github/groom/README.mdand.github/groom/tests/sandbox-tests.sh. Pure tests+README.bump-groom-callers.yml:65watches.github/groom/**with no exclusion and the commit carries no skip trailer, so it fired a churn bump. The "Impact (counterfactual)" section can be promoted to an observation.Still-unfixed defect set: agents-md
:48-50, coderabbit-config:60-62, groom:58-65, cursor-review:56-82(which excludesscripts/check-pr-size/*_test.goat:82but pulls in.github/cursor-review/**unexcluded). Your comparison set is right too, including the note that pr-size excludes tests only because that tree has no README — confirmed, it holds onlygo.mod,main.go,main_test.go,size.go,size_test.go.The freeze mechanic is real and still documented at
preflight.sh:60-74and:1128-1152, andtest_paths_contract.shstill enforces the mirror in both directions (:278-281hard-fails on a!entry with noWATCHED_PATHSPECS;:294-300set-equivalence). One softening: because that test runs in CI, a filter-alone change is a red build, not a fleet outage. The sequencing advice stands; the severity is lower than "fleet-freeze".Two corrections to the fix prescription:
WATCHED_EXEC"exactly as bump-pr-risk, bump-pr-size and bump-cursor-review already do". pr-size and cursor-review do not setWATCHED_EXEC— they setWATCHED_PATHSPECSonly. Andtest_paths_contract.shnever mentionsWATCHED_EXECat all, so nothing enforces it. That is a separate defect (tracked separately, see [groom] Five bump fleets watch an asset DIRECTORY kept alive by tests/ and README.md with no WATCHED_EXEC, so deleting an executed script would still read as a healthy surface #248) and is not required by this fix..github/groom/tests/holds 10 files, not 9. Trivial, but the count is in the body.A trap worth knowing before anyone writes this.
test_paths_contract.sh:316-330intends to skip/**directory exclusions — its comment says "File globs only. A/**directory exclusion … cannot diverge this way". But the guard iscase "$negbase" in *'*'*) ;; *) continue ;; esacwithnegbase="**", and**matches*'*'*, so it is not skipped; it then runsfind . -mindepth 2 -name "**", which matches every ordinary filename..github/cursor-review/tests/containsfixtures/catalog-2026-08-10.txtat depth 2 — so adding!.github/cursor-review/tests/**would fail at:328with a misleading message. The five existing…/tests/**exclusions pass only because those trees happen to be flat, as do the other three here; only cursor-review trips it. This was established by reading the script and checking both bash primitives in isolation — the suite was not run, so confirming it is the first task of the tracked work rather than a claim.Also since filing: the
Skip-caller-bump:trailer landed (#235, hardened by #264) as a per-commit opt-out. It has never been used by any commit, includingcf7b8d9, so it does not close this — but it is a complementary remediation worth weighing.Reopen or comment if the census or the corrections miss something.
Created by Claude Code