Skip to content

Commit 55b9273

Browse files
pm(check-expected-skips): derive the expected-skip roster from the workflows' job-level if: gates (#26648)
<!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi9QNR2GRCdq6fNoXi6jwg2Xb)_ Fixes #26439 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:`check-expected-skips` 不再手抄「允许 skip 的 job 名单」,改为直接读各 workflow 里每个 job 的 `if:` 推出名单;手抄的 16 行全部删除。 - 为什么改:手抄名单一漏就红(#26111 就是一次),新增或改名一个带门控的 job 都得来这里补一行;本 PR 实测 main 上已经又漏了两个(`Plan Test Core shards`、`Delete the reapable branches`)。 - 风险与回滚:只动一个 `scripts/pm` 脚本,不发布、不改 workflow;判错的方向是「多红」不是「放过」——不认识的写法一律拒收并点名。回滚即 revert 本 PR。 - 先看:`scripts/pm/check-expected-skips.mjs` 的 `gateTerm` / `jobGate`(认哪几种写法)、`filterTermProblem`(filter 契约三条),再看自测里 ADD / RENAME / UN-GATE 几条。 - 两分钟验证:`node scripts/pm/check-expected-skips.mjs --roster`,预期首行写 `20 expected-skip name(s), every one derived … none listed by hand`,且无 ⚠️ refused 行。 - 需要你做的:无,席位落地。 ## What changes `scripts/pm/check-expected-skips.mjs` only. - **The hand roster is gone.** `EXPECTED_SKIPS` (16 rows), `GATE_KINDS`, `gateSpellings`, the reason constants and `auditRoster` are deleted. The roster is now derived from the tree on every run: `rosterFromTree` = `deriveGatedSkips` (new) + `deriveNonPrEventSkips` (existing shape), off one shared `scanWorkflowJobs` scan. - **`deriveGatedSkips`** reads each job's job-level `if:`, splits it at top-level `&&` (outside parentheses and literals), and admits the job when at least one conjunct is a recognised gate term: - `filter-output`: `needs.filter.outputs.OUTPUT != 'false'`, or a parenthesised `||` whose every alternative is one; - `pr-event`: `github.event_name != 'pull_request'` or `!= 'merge_group'` (the per-PR events; the full-battery jobs); - `event-action`: `github.event.action != 'ACTION'` (a labeled / unlabeled / edited run of the same head); - `label`: `contains(github.event.pull_request.labels.*.name, 'LABEL')` (opt-in) or its `!` negation (opt-out). - The name is the job's `name:` verbatim (raw matrix template included), so the hard-coded `/3`, `/8`, `/16` shard sizes disappear with the list: GitHub reports a pre-expansion skip under the raw `name:`, never a size read from `matrix:`. - **Refusals (the skip stays exit 4, and `--roster` names it):** a `filter` term whose output lacks the `|| 'true'` widening, is not declared by the `filter` job, or sits on a job without `needs: filter`; an `if:` that reads `needs.filter.outputs.` in any spelling the recogniser does not read; a name two jobs carry; a name that cannot be told. A job with no gate term at all is never admitted. - Each derived row's reason is built from the terms the job spells (queue-widened vs queue-read outputs, full battery, opt-in / opt-out label, unread conjunct count, raw template). - `judgeCheckRuns` and `renderRoster` now take the roster explicitly (no hand-list default); `--roster` prints `gate: job-gate filter core|crosspkg + pr-event pull_request|merge_group`-style lines. - The header's "what the roster answers" section is rewritten for the derivation at the same line count, so the `self-test-reads` declaration stays at `:137` (pinned by `dispatch-gates.self-test.mjs`) — no edit outside this file. ## Why The card: the roster was a hand copy of ci.yml's `if:` gates and went red on every new or renamed gated job until someone added a row (#26111: `Build UI Deps`). Measured on `origin/main` `7c34e5dc2`, the class had already recurred twice more on main: | skipped check-run | job | main's roster | this PR | |:---|:---|:---|:---| | `Plan Test Core shards` | `ci.yml` › `test-plan` (gated on `core` \|\| `crosspkg`, added with the shard planner) | exit 4 | exit 0 | | `Delete the reapable branches` | `merged-branch-reaper.yml` › `reap` (`!= 'pull_request'`, the workflow runs on PRs touching itself) | exit 4 | exit 0 | ## Tests The self-test is rewritten in this file (no other test file moves): 173 → 206 cases. - **Recogniser** cases: top-level splitting, each term kind, and the near-misses refused (`== 'true'`, a filter term inside a top-level `||`, `(a) || (b)`, `!= 'push'`, `== 'pull_request'`, double-quoted labels, status functions, two `${{ }}` blocks, unbalanced parentheses). - **The measured heads (#18315, #20748) are judged against a frozen fixture tree**, not this checkout's job names, so a gated job renamed in a live workflow moves no case here. That also lets `MEASURED_18315` go back to verbatim (`Test Core (… /6)`, the re-spelling to `/8` is undone). - **Acceptance fixtures on the fixture tree:** ADD (the #26111 shape: a new gated job is expected on its first skip, nothing listed; the tree without it reads ⛔), RENAME (new name expected, old name ⛔), DELETE, UN-GATE (`if:` removed, `if: always()`, an ungated job added: each ⛔ — a required job silently not run still reds), RE-GATE, the three filter-contract refusals, the unread-spelling refusal, shared / untellable names, an unreadable workflow refused once. - **Live tree:** structural only — nothing refused, ci.yml's filter contract read (at least one filter-gated row), every admitted row a real job with a string `if:`, no bare status-function `if:` admitted, the never-skipping sentinels out, the joined roster frozen and duplicate-free. The CLI cases build their payloads from the live roster. ## Acceptance notes - **Hypothesis 1 (the card): holds, with one correction.** Every one of the 16 hand rows falls out of the rule — measured by `--roster` before/after on `7c34e5dc2`: the 18 names before (16 listed + 2 derived) are all present after, plus the two live misses above, 20 in all, 0 refused. The correction: shard sizes are not read from `matrix:` blocks; the skipped check-run carries the raw `name:`, and ci.yml's `test` matrix is `fromJSON(needs.test-plan.outputs.shards …)`, which has no static size to read. - **Hypothesis 2: holds.** The gate's real question survives: `TypeScript Type Check` skipped still reds on both trees (below), and the self-test keeps the un-gated cases. - **`reap` is now expected.** Its `if:` conjoins `github.event_name != 'pull_request'`, so its skip on a PR head is by construction; the non-PR-event shape still refuses it (that case is kept). - **Boundary kept:** the derivation is a recogniser of closed spellings, never an evaluator; it answers the NAME question only, as before. - The file's self-test carries the verdict handshake but not the battery-name floor described in AGENTS.md § Writing a `--self-test`; unchanged here (not this card), noted only. - `node scripts/measure-self-test-floor.mjs --self-test` exits 1 on this head naming `scripts/pm/check-prior-rulings.mjs` and `scripts/pm/dispatch-gates.mjs` (handshake spelling census); neither file is touched here and the script is not a wired check, so it reads the same on `main`. ## Verification All readings on head `98a3dc80`. <details><summary>Acceptance proof on disk: add + rename a gated ci.yml job (one-off, scratch worktrees)</summary> In two scratch worktrees (main `7c34e5dc2` and this head), ci.yml was edited: `build-docs` renamed to `Build Docs Site`, and a new job `Brand New Gated Job` added with `if: ${{ !cancelled() && needs.filter.outputs.core != 'false' }}`. Then `--self-test` and `--check-runs-json` on one-skip payloads: | | main | this PR | |:---|:---|:---| | `--self-test` | exit 1 — `Build Docs: ci.yml job build-docs is named "Build Docs Site" — the check-run name moved; correct the row` | exit 0, 206 cases | | `Brand New Gated Job` skipped | exit 4 | exit 0 | | `Build Docs Site` skipped | exit 4 | exit 0 | | `Build Docs` (the old name) skipped | exit 0 (false OK) | exit 4 | No file under `scripts/pm` was edited for the PR-side column. </details> <details><summary>The check still reds on the defect it was written for</summary> One-skip payloads, `--check-runs-json`, current tree: | skipped | main | this PR | |:---|:---|:---| | `TypeScript Type Check` (no gate) | exit 4 | exit 4 | | `Plan Test Core shards` | exit 4 | exit 0 | | `Delete the reapable branches` | exit 4 | exit 0 | | `Build UI Deps` (the #26111 job) | exit 0 | exit 0 | </details> <details><summary>Local gates (dispatch-gates --commands, 31 families)</summary> - `pnpm check:pm-expected-skips` → `✓ check-expected-skips self-test: 206 cases pass` - the 30 other derived commands: exit 0 each (list in the card report) - `pnpm check:pm-dispatch-gates` → exit 0, `✓ dispatch-gates self-test (fast tier): 1935 cases pass` - `dispatch-gates --ran` → `31 derived famil(ies) accounted for — 31 run, 0 NOT-MEASURED (a DERIVED zero)` - `pnpm check:ci-filter-parity` exit 0, `pnpm check:required-contexts` exit 0 — neither reads this file (`git grep` for its name in both scripts: 0 hits), so their output is unchanged by construction - `eslint --no-inline-config scripts/pm/check-expected-skips.mjs` exit 0; `check:nul-bytes` exit 0 </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (not touched) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. (eslint on the file locally; repo-wide in CI) - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset — nothing publishes (`scripts/pm/**` only), `skip-changeset`; both declaration lines are filled in above. - [x] 6. `packages/spec` touched: n/a. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English (the 维护者速读 block is the required Chinese summary). - [x] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ADHkmTBKk6ks5yi6zYDatg --- _Generated by [Claude Code](https://claude.ai/code/session_01ADHkmTBKk6ks5yi6zYDatg)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent b49b752 commit 55b9273

1 file changed

Lines changed: 654 additions & 490 deletions

File tree

0 commit comments

Comments
 (0)