You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[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
Groom auto-builder — Comfy-Org/github-workflows: this CONFIRMED finding could not be auto-built (patch modifies a CI-privileged or dataset-of-record path (per .github/groom/patch_policy.py — workflow/action defs, lockfiles, build/test config that executes in pre-review CI, graded eval cases under a suites/**/cases/ tree, or a symlink into such a tree) — a human must author these changes), so it is filed for a human. · run
Verified
preflight.sh:854-861 documents the failure mode in its own words: a per-file fleet's directory can OUTLIVE the scripts inside it - scripts/pr-risk still exists once tests/ and README.md are all that is left - so a directory probe would report the surface healthy and bump every caller onto a SHA where the graders it executes are gone. WATCHED_EXEC (documented at preflight.sh:129-138, implemented at :862-892) exists for exactly that, and a miss takes the ::warning:: decommission path instead of bumping.
Fleets that DO set WATCHED_EXEC: area-label, linear-ticket, pr-risk, pr-derisk and public-repo-hygiene (bump-public-repo-hygiene-callers.yml:91-93). The finder said four and missed public-repo-hygiene - correct the count to five.
Fleets whose WATCHED_ASSETS names a DIRECTORY and that set no WATCHED_EXEC (verified by reading each preflight step):
bump-cursor-review-callers.yml:145-147 - .github/cursor-review AND scripts/check-pr-size
bump-groom-callers.yml:117 - .github/groom (15 entries, of which README.md and tests/ are not executed)
bump-pr-size-callers.yml:125 - scripts/check-pr-size, whose own paths: filter excludes *_test.go, so the files that would keep the directory alive are ones the fleet deliberately does not watch
Each of those directories would retain README.md and/or tests/ after every executed file was deleted, so the tree-existence probe would still resolve. Deleting e.g. .github/cursor-review/post-review.py or .github/groom/ledger.py could therefore let the fleet re-pin every enrolled caller to that SHA, and each consumer's reusable would hard-fail at run time with no signal from this side. Inferred from the code paths above; no incident is claimed.
The three fleets with no asset directory at all (auto-label, detect-unreviewed-merge, assign-reviewers) say so in their own comments and are correctly exempt.
Fix
Add WATCHED_EXEC to the five entrypoints, listing the files a pinned caller actually executes - the same audit bump-pr-risk-callers.yml:160-183 already walked, which found its inherited list was short of what a caller runs. Plain repo-relative FILE paths only; preflight.sh rejects a directory there.
Close it structurally: preflight.sh already refuses a set-but-empty WATCHED_EXEC, so extend it to REQUIRE WATCHED_EXEC whenever a WATCHED_ASSETS entry resolves to a directory, mirroring the existing WATCHED_ASSETS / WATCHED_PATHSPECS coverage assertion at :118-126. Land step 1 before step 2, or every affected fleet reds at once.
Risk
Low-medium. A too-broad WATCHED_EXEC makes a legitimate rename read as a decommission and no-ops the fleet behind a ::warning:: - loud and recoverable, unlike the silent bump it replaces. Not security-adjacent: no credential or permission decision changes.
Triage: confirmed, still exactly five fleets. Leaving this open, tracked internally.
Re-derived the census independently against origin/main @ cf7b8d9c2614ad9f8ee41acf245381ad76fcdc8b rather than re-reading the body, and it lands on the same answer. Note the body's line numbers are now stale by +5 — 45c51bc (#264) inserted a GH_TOKEN comment plus a variable into every entrypoint's preflight env:.
Fleets with a directory-valued WATCHED_ASSETS and noWATCHED_EXEC, current lines:
fleet
WATCHED_ASSETS
bump-agents-md-callers.yml
:106
bump-coderabbit-config-callers.yml
:118
bump-cursor-review-callers.yml
:150-152
bump-groom-callers.yml
:122
bump-pr-size-callers.yml
:130
The five that do set it: area-label :76-80, linear-ticket :82-85, pr-risk :183-191, pr-derisk :217-226, public-repo-hygiene :96-98 (the verifier's correction to the finder holds — that last one was the one the finder missed). The three workflow-file-only fleets are correctly exempt, and 5 + 5 + 3 = 13 matches the full file count.
The mechanism checks out end to end: preflight.sh:906-909 documents the failure mode in its own words, WATCHED_EXEC is documented at :129-138 and implemented at :411-438 / :900-945 / :1196-1218, and the local directory probe at :1187-1195 uses -e rather than -d. When the deleting commit is the run's own commit, the tree-OID comparison and the pathspec diff both see no change and both presence probes pass, so the run emits proceed=true.
Fix step (b) has not landed — nothing in preflight.sh cross-references asset_dirs against watched_exec. Step (a) has not either; no commit since 2026-08-29 touches this.
One correction to the body. "Kept alive by tests/ and README.md" is accurate for four of the five, but not for scripts/check-pr-size, which holds only go.mod, main.go, main_test.go, size.go, size_test.go — no tests/ dir, no README. Same failure mode, different survivors: deleting main.go/size.go/go.mod leaves the *_test.go files holding the directory open, and those are exactly the files that fleet's own filter excludes. Your parenthetical about the *_test.go exclusion already had this right; only the summary phrasing needs adjusting.
A constraint for whoever writes step (b), recorded here so it is not rediscovered the hard way: preflight.sh:1181-1186 deliberately uses -e and not -d because a fleet may legitimately watch a single FILE. So the requirement has to be conditional on the entry actually resolving to a directory — mirroring the existing [[ -d "$f" ]] idiom at :433 — never unconditional.
Also worth folding into the same change: .github/bump-callers/README.md:190 says WATCHED_EXEC is needed "Only by pr-risk and pr-derisk today", which is already wrong for three fleets and would be wrong about ten after this lands. The same claim repeats around :401-427.
Tracked internally; the issue stays open until the fix lands. Reopen or comment if the census misses a fleet.
Activity
mattmillerai commentedon Sep 18, 2026
Triage: confirmed, still exactly five fleets. Leaving this open, tracked internally.
Re-derived the census independently against
origin/main@cf7b8d9c2614ad9f8ee41acf245381ad76fcdc8brather than re-reading the body, and it lands on the same answer. Note the body's line numbers are now stale by +5 —45c51bc(#264) inserted aGH_TOKENcomment plus a variable into every entrypoint's preflightenv:.Fleets with a directory-valued
WATCHED_ASSETSand noWATCHED_EXEC, current lines:WATCHED_ASSETSbump-agents-md-callers.yml:106bump-coderabbit-config-callers.yml:118bump-cursor-review-callers.yml:150-152bump-groom-callers.yml:122bump-pr-size-callers.yml:130The five that do set it: area-label
:76-80, linear-ticket:82-85, pr-risk:183-191, pr-derisk:217-226, public-repo-hygiene:96-98(the verifier's correction to the finder holds — that last one was the one the finder missed). The three workflow-file-only fleets are correctly exempt, and 5 + 5 + 3 = 13 matches the full file count.The mechanism checks out end to end:
preflight.sh:906-909documents the failure mode in its own words,WATCHED_EXECis documented at:129-138and implemented at:411-438/:900-945/:1196-1218, and the local directory probe at:1187-1195uses-erather than-d. When the deleting commit is the run's own commit, the tree-OID comparison and the pathspec diff both see no change and both presence probes pass, so the run emitsproceed=true.Fix step (b) has not landed — nothing in
preflight.shcross-referencesasset_dirsagainstwatched_exec. Step (a) has not either; no commit since 2026-08-29 touches this.One correction to the body. "Kept alive by
tests/andREADME.md" is accurate for four of the five, but not forscripts/check-pr-size, which holds onlygo.mod,main.go,main_test.go,size.go,size_test.go— notests/dir, no README. Same failure mode, different survivors: deletingmain.go/size.go/go.modleaves the*_test.gofiles holding the directory open, and those are exactly the files that fleet's own filter excludes. Your parenthetical about the*_test.goexclusion already had this right; only the summary phrasing needs adjusting.A constraint for whoever writes step (b), recorded here so it is not rediscovered the hard way:
preflight.sh:1181-1186deliberately uses-eand not-dbecause a fleet may legitimately watch a single FILE. So the requirement has to be conditional on the entry actually resolving to a directory — mirroring the existing[[ -d "$f" ]]idiom at:433— never unconditional.Also worth folding into the same change:
.github/bump-callers/README.md:190saysWATCHED_EXECis needed "Only bypr-riskandpr-derisktoday", which is already wrong for three fleets and would be wrong about ten after this lands. The same claim repeats around:401-427.Tracked internally; the issue stays open until the fix lands. Reopen or comment if the census misses a fleet.
Created by Claude Code