Skip to content

[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

Description

@cloud-code-bot

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-agents-md-callers.yml:101 - .github/agents-md-integrity (holds check_agents_md.py, README.md, tests/)
  • bump-coderabbit-config-callers.yml:113 - .github/coderabbit-config (check_coderabbit_config.py, schema_drift.py, schema.v2.json, requirements.txt, README.md, tests/)
  • 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

  1. 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.
  2. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    groomRefactor / tech-debt cleanup candidate filed by the groom sweep (finds-only)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions