Skip to content

check-required-contexts asserts no paths: on the pull_request trigger but nothing about types: — a dropped default activity type is the same permanent-pending failure, unguarded #8304

Description

@os-zhuang

Filing unassigned; domain:* routing and grading are the triage seat's. Found while implementing #8012 (PR #8302), which is what makes the gap live rather than theoretical.

The gap

scripts/check-required-contexts.mjs assertion 7 guards the pull_request: trigger of every required-context workflow against paths: / paths-ignore:, with this rationale in its own header:

a path-filtered trigger produces NO check run on a PR that misses the glob — not a skip, an absence — which is permanent pending

A restrictive types: list produces exactly the same absence, and nothing checks it. Naming any types: REPLACES GitHub's default [opened, synchronize, reopened]. A workflow whose trigger reads types: [synchronize] publishes no check run when a PR is opened, so the required context sits permanently pending — the identical wedge, through a different key on the same trigger. grep -n types scripts/check-required-contexts.mjs returns nothing: the pin has no opinion on the field.

Why it is live now, not hypothetical

Until PR #8302 no required-context workflow used types: at all — measured across the registry:

workflow pull_request types
lint.yml (default)
ci.yml (default)
adr-merge-approval.yml [opened, synchronize, reopened, auto_merge_enabled, auto_merge_disabled] ← added by #8302

#8012 needed auto_merge_enabled / auto_merge_disabled to observe auto-merge arming, so adr-merge-approval.yml now carries an explicit list in which the three defaults are restated by hand. That restatement is load-bearing and machine-unverified: an editor who later prunes the list to "just the auto-merge ones it needs" silently stops the gate reporting on newly opened PRs, and the symptom is a permanently pending required context — the #5617 shape the whole script exists to prevent.

PR #8302's trigger block carries a comment saying exactly this, but a comment is the enforcement level this script was written to replace.

Suggested shape (not a ruling)

Add to assertion 7: if the pull_request: trigger names types:, the list must be a superset of [opened, synchronize, reopened]. That is checkable entirely repo-side from data already parsed, needs no settings access, and the script's self-test idiom already has the fixture machinery for it (fixture('paths: on ci.yml', ...) mutates a workflow and asserts the problem text).

Worth deciding at the same time whether ready_for_review belongs in the required floor. A draft PR already gets opened/synchronize runs so the context exists, which is why its absence is not a defect today — but it is the kind of thing that should be ruled once and pinned, not re-reasoned per workflow.

Not claimed

No behaviour is broken on main right now: the list in #8302 is correct as written. This is a missing guard on a newly created exposure, filed so it is not discovered the next time a required context goes quietly pending.

Backlink: PR #8302, issue #8012.


Generated by Claude Code

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions