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:

workflowpull_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

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions