Skip to content

ci(guards): a filter gating a guard is checked for drift but not for coverage — 2 of 6 canonical filters have no guard, and 39 inline ones are unenumerated #17386

Open

Description

@mrveiss

What

A guard gated by a path filter is only as good as the filter. The repo knows this — repo_tests/python_filter_covers_its_guards_test.py exists to catch "a required check gated by a filter that omits the inputs it guards", and .github/filters/python-paths.yml carries a dozen comments recording individual instances of it.

That guard reads one filter. _FILTER = _REPO_ROOT / ".github" / "filters" / "python-paths.yml". There are six.

Measured coverage — which guards read which canonical filter:

filter guards reading it
python-paths.yml 6
code-quality-paths.yml 4
backend-python-paths.yml 3
frontend-paths.yml 2
docker-paths.yml 0
docs-only-paths.yml 0

Plus 39 workflows carrying inline path filters, which are outside the canonical-file scheme entirely.

The live instance that prompted this

#17378 added a shell file-size ratchet pinning ten grandfathered .sh files. Its --audit-ceilings step runs in code-quality, gated on a filter that carried '**/*.py' generically but only three specific shell prefixes. Measured against the real patterns, 4 of the 10 files it pins were uncovered:

UNCOVERED  install.sh                                                          (1265)
UNCOVERED  scripts/pr-preflight.sh                                             (818)
UNCOVERED  autobot-slm-backend/ansible/deploy.sh                               (718)
UNCOVERED  autobot-infrastructure/autobot-slm-backend/scripts/bootstrap-slm.sh (652)

Why it was invisible is the part that generalises. The well-behaved case self-triggers: lowering a ceiling touches repo_tests/**, which IS in the filter, so a correct change runs the audit. The misbehaving case — shrink a file, forget the ceiling — touches only the .sh, and skips. The gate ran for everyone who did not need it and skipped everyone who did. The PR's own CI was green while proving nothing about the scenario the gate exists for, because its diff happened to touch repo_tests/**.

Already fixed in that PR. Filed here because the shape is general and the guard against it is not.

The distinction that matters

Two different things are checked today, and only one of them is this:

  • Drift between a canonical filter and its workflow copy — covered by check_workflow_path_filters.py.
  • Whether a filter's patterns cover the domain the gated guard MEASURES — covered for python-paths.yml, and not otherwise.

The second is the one that lets a gate pass vacuously.

A second unguarded case, not yet a defect

docs-only-paths.yml is 7 patterns deciding whether enforce-precommit's repo-wide sweep runs — measured at 1008s of a 1142s job (#15299). The workflow is carefully built: no job-level filter, on purpose, so the required context always reports. But if those 7 patterns ever classify as "docs-only" a change the sweep needed to see, the sweep silently does not run. Nothing compares the classification to the sweep's domain. Not observed failing; recorded because it is the same shape one step from the same outcome.

Acceptance criteria

  • For each canonical filter that gates a guard, an assertion that the filter's patterns cover the set the guard measures. Generalising python_filter_covers_its_guards_test.py to take the filter as a parameter is the obvious route; whether the other five can express their domains the same way is the open question, and "this one cannot be expressed mechanically, here is why" is a legitimate answer for any of them.
  • The check is proved by mutation on a path it reaches: remove a pattern the guard depends on and confirm the new assertion reddens by name. A parametrised check proved on python-paths.yml alone proves only that filter — the same trap as the gate this issue is about.
  • docker-paths.yml and docs-only-paths.yml either gain coverage or gain a recorded reason they need none.
  • The 39 inline workflow filters are enumerated and each is either brought under a canonical file or recorded as deliberately inline. A count is enough for the ones that are fine; the point is that the set is known rather than assumed.

Not in scope

Changing what any filter matches. This is about knowing whether a filter covers its guard, not about widening filters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions