[PR #170] [MERGED] ci(workflow-pins): fail closed on a job-level `uses:` this scanner cannot read #174

New issue

Closed

opened 2026-10-31 22:05:19 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 22:05:19 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-typescript-sdk/pull/170
Author: @mattmillerai
Created: 9/21/2026
Status: ✅ Merged
Merged: 10/9/2026
Merged by: @mattmillerai

Base: matt/be-16299-check-workflow-pins-regression-suite ← Head: matt/be-16336-check-workflow-pins-fail-closed

📝 Commits (10+)

04eba98 ci(cursor-review): read the watched pathspecs from upstream, and stop reading faults as verdicts
b541197 ci(cursor-review): read a vanished bump workflow as broken, not as a fault
d80dbd4 ci(cursor-review): stop the watchdog reading faults, decoys and magic as verdicts
278ec24 Merge remote-tracking branch 'origin/matt/be-15270-resync-cursor-review-pins' into matt/be-16336-check-workflow-pins-fail-closed
fcfbab4 ci(workflow-pins): fail closed on a job-level uses: this scanner cannot read
42365c0 ci(cursor-review): close the round-5 holes in the pin-freshness watchdog
5596838 Merge remote-tracking branch 'origin/matt/be-15270-resync-cursor-review-pins' into matt/be-16336-check-workflow-pins-fail-closed
ecfcadd ci(workflow-pins): match the reusable's owner/repo case-insensitively
bb3c999 ci(cursor-review): close the panel's round-9 fail-open holes
31d999b Merge remote-tracking branch 'origin/matt/be-15270-resync-cursor-review-pins' into matt/be-16336-check-workflow-pins-fail-closed

📊 Changes

4 files changed (+2901 additions, -206 deletions)

View changed files

📝 .github/dependabot.yml (+44 -6)
📝 .github/workflows/ci.yml (+1938 -120)
📝 scripts/check-workflow-pins.mjs (+716 -75)
📝 scripts/check-workflow-pins.test.ts (+203 -5)

📄 Description

STACKED — merging lands on matt/be-16299-check-workflow-pins-regression-suite (owned by @mattmillerai, PR #169), NOT main. #169 is itself stacked on #153. GitHub retargets this PR as each foundation merges; do not read the checks below as "safe to merge into main".

One consequence a reviewer should know: ci.yml triggers on pull_request: branches: [main], so while this PR is stacked the repo's own gates — test, workflow-pins, public-repo-hygiene, build-check — do not run on it at all. That is pre-existing behaviour for every stacked PR here (it applies to #169 and #153 too), not something this change introduced. They were all run locally; see Verified below, and they will run for real once GitHub retargets this PR to main.

ELI-5

scripts/check-workflow-pins.mjs reads workflow YAML one line at a time, so it only recognises a reusable-workflow call written as one plain line. Write the same call a different but perfectly legal way — folded (uses: >-), with the value pushed onto the next line, or with the whole job spelled as a flow mapping (review: { uses: ... }) — and the scanner saw nothing there at all. Put one of those next to a normal caller and the lint happily reported "all pins agree" over a genuinely split pin, while --print-pin handed the daily freshness watchdog the other job's SHA as if it were the only pin in the repo. Now the scanner tells those three apart from "no caller here": it reports the file and line, says which spelling it choked on, tells you to write uses: …@<sha> on one line, and exits 1 in both modes.

What changed

scripts/check-workflow-pins.mjs, text-level only — no yaml import, because the two CI jobs that run this script deliberately have no pnpm install.

USES_KEY_RE matches a uses KEY with any value, including none on the line. The gap between it and USES_RE (which matches only the values the scanner can read) is exactly the set of lines that used to vanish.
unreadableUsesReason classifies that gap: a quote that never closes (multi-line quoted scalar), or anything else still naming the org's reusables (owner/path/ref did not parse). The other two shapes, a block-scalar header and an empty value with the value on the next line, are now caught earlier by #153's own USES_BLOCK_SCALAR_RE / USES_EMPTY_VALUE_RE checks. Those read the folded value and fail in both modes with #153's message. This PR originally classified them here too (folded/literal block scalar, value on a continuation line); those arms were removed at f462d5c because they could no longer fire. It returns null — a silent skip, as today — for a single-line value naming some other owner (./.github/workflows/local.yml, octo/repo/.github/workflows/x.yml@v1): those pin once and have no workflows_ref to disagree with.
FLOW_JOB_RE catches a job whose whole body is a flow mapping, positioned by its owning key being a ROOT jobs: (asked structurally off #153's owningKeyChain, the same way #153's isJobLevelUses asks it) rather than by hardcoding two-space indent.
findCallers now returns { callers, unreadable }. Both modes read unreadable and both report it before their own checks — --print-pin writing nothing whatsoever to stdout, since ci.yml captures that mode with $(...) and any stray byte would become the "pin" it feeds to git.
The zero-caller guard's message no longer offers "a folded uses: >-, a flow mapping" as one half of an either/or; those are now reported by name, so reaching that guard really does mean no job-level uses: exists.

scripts/check-workflow-pins.test.ts gains describe("spellings the scanner cannot read fail closed"), now eight cases, each asserting the lint mode and --print-pin together. Every positive fixture places the unreadable job beside a plain, correctly-pinned sibling, because that sibling is what made the old silence survivable; without it the zero-caller guard would have caught the file and the case would prove nothing.

Two design calls worth a reviewer's eye

A flow-mapping job is reported even if it calls nothing. A flow mapping nests to any depth on one line, so "does this contain a uses:" is precisely the question a line-anchored scanner cannot answer — guessing would reinstate the silent skip. No workflow here is written that way, and the remedy is a reformat.

--print-pin refuses when the unreadable job is in a different file than the reusable it was asked about. Deliberate: an unreadable job anywhere could be a second caller of that same reusable, which is exactly the ambiguity --print-pin already refuses to resolve.

Falsification of the "cannot read" claim

This diff adds a deny path, so the premise was checked empirically rather than asserted, on the two paths that could make it false:

No built-in YAML parser exists to use instead. On Node v26.4.0 (newer than the 22/24 CI runs): require('node:module').builtinModules matches nothing for /yaml/i; import('node:yaml') → ERR_UNKNOWN_BUILTIN_MODULE; node:util exposes only parseEnv and parseArgs.
The yaml devDependency is genuinely unreachable in the CI shape. import('yaml') from a tree with no node_modules → ERR_MODULE_NOT_FOUND. yaml is in devDependencies only, and both workflow-pins (ci.yml:231) and cursor-review-pin-freshness run checkout + setup-node + node scripts/… with no install step — unchanged by this PR.

Note the outcome is not a capability the author loses: the error names the spelling to write instead, and the one-line form is already what every caller in the repo uses.

Corpus sweep — including the half not fixed

Swept all 5 files under .github/workflows: 22 uses: lines, 21 of them step-level and 1 job-level (the real caller, readable, still the only thing the lint reports). Zero occurrences today of any shape this guard does not cover — 0 flow-mapping jobs, 0 alias-valued uses: *ref, 0 multi-document files, 0 merge-key anchors. So the guard changes nothing about the current tree and the remaining gaps are hypothetical rather than latent.

Review round 6 — cursor-review panel

The panel raised 10 findings. Only one of them is in this PR's own commit (fcfbab4, which touches scripts/check-workflow-pins.mjs and its test and nothing else); the other nine are in .github/workflows/ci.yml and in commentShaCandidates, both of which reach this diff only because the branch merges #153's branch.

Fixed here (1): the reusable's owner/repo was compared case-sensitively, so comfy-org/github-workflows/... — which GitHub resolves to the very same repo and calls successfully — matched neither USES_RE nor unreadableUsesReason, and was skipped in silence. Reproduced against the pre-change script on a two-job fixture pinned to two different SHAs: 1 caller(s) checked, all pins agree, exit 0, with --print-pin returning the other job's SHA. Fixed in ecfcadd with a regression test.
Fixed by re-merging #153's tip (5): the GIT_NO_LAZY_FETCH existence check, the block-scalar-header comment, the :^ exclusion synonym and the dash-column block indent were all fixed by #153's 42365c0. #153 has since landed bb3c999 ("close the panel's round-9 fail-open holes"), re-merged here in 31d999b, which also fixes the fifth — commentShaCandidates now discriminates dates by LENGTH (!/^[0-9]{8}$/) on both branches instead of dropping every all-digit token, so a 7-digit abbreviation is checked like any other and the ~1-in-27 silent skip of assertion (3) is closed. Those five threads are resolved.
Left for #153 (4), three since fixed there: one Medium (the age clamp does not bound the mute — a future-dated commit yields state=grace daily) and three Low/Nit (the unprobed wildcard pathspec, the per-line-anchored pin_out guard, and always() where !cancelled() is meant). All four are in the cursor-review-pin-freshness job in ci.yml — #153's code — and all four were re-checked against bb3c999 and still stand; that commit fixed a different nine. #153 is open and merges first, so patching ci.yml from here would only collide with its next round. Those four threads are deliberately left unresolved and are itemised on #153 (comment 5757140139). #153 has since fixed three of them: 56a738b stops a future-dated commit past the skew tolerance from counting as grace, the pin_out guard is a whole-value =~ match, and the steps use !cancelled(). f462d5c merges that tip, so those three threads are resolved. The unprobed-wildcard thread is still open because the code is unchanged at 56a738b.

Residual

#153 has since widened USES_RE to match the owner/repo case-insensitively, which supersedes this PR's choice to keep it case-sensitive. The concern that choice rested on (the lint exiting 0 on a split pair across two spellings) is answered on #153's side by its spelling lint (4), which names the non-canonical caller and exits 1. --print-pin sees two callers and refuses. The letter-case test now asserts both. unreadableUsesReason still matches case-insensitively for a lowercase owner/repo that USES_RE cannot parse.
Alias-valued uses: is still a silent skip. uses: *some_anchor reads as a single-line non-org value, so unreadableUsesReason returns null and it is skipped — yet the alias could resolve to an org reusable with a pin the lint never checks. Resolving anchors means tracking &anchor definitions across the file, which is a second parser's worth of work and outside this ticket's one guard. 0 occurrences in the tree today. Worth its own ticket if anchors ever appear in these workflows.
A jobs: block written wholly as a flow mapping is still a silent skip. FLOW_JOB_RE requires the line's owning key to be the column-0 jobs:; jobs: { review: { uses: … } } has no owning key, so nothing reports it. Same fix family as the alias gap. 0 occurrences today.
Multi-document YAML and merge keys (<<: *base) are unhandled by the scanner generally — not introduced here, not fixed here. 0 occurrences today.
The upstream investigation that produced this plan was not readable from this environment, so its evidence was re-derived rather than trusted: the three silent-skip shapes were reproduced against the pre-change script in a throwaway fixture tree (all three skipped, lint printed all pins agree, --print-pin returned the sibling SHA) before any code was written.
The repo's own CI gates never ran against this PR in GitHub Actions, because ci.yml is scoped to pull_request: branches: [main] and this PR's base is a stacked branch. Every one of them was run locally instead and is listed under Verified; the first CI-side run of test / workflow-pins / public-repo-hygiene / build-check against this diff happens after retargeting. Nothing here can substitute for that.
Verified by mutation at f462d5c: disabling the flow-job check reddens 1 of 34 tests. Disabling the unreadable key path reddens the 2 cases added at that merge. Before those cases were added it reddened nothing, because the letter-case case no longer reaches that path. Earlier mutations, also not committed — nothing in CI re-runs it. With both unreadable.push sites commented out, exactly the 4 original positive cases go red and the negative case plus all 26 pre-existing cases stay green; with unreadableUsesReason's return null flipped to a reason, the negative case goes red (as does the pre-existing other-org/shared case). The case-insensitivity case added in review round 6 was mutation-checked the same way: reverting the toLowerCase() reddens that case and only that case, 1 of 32.

Provenance

Authored by: agent-work loop
Verified: at f462d5c (merge of #153's tip 56a738b, with the findCallers conflict resolved): pnpm vitest run scripts/check-workflow-pins.test.ts: 34 passed, 0 failed. pnpm test: 654 passed, 0 failed across 27 files. pnpm lint, pnpm format:check, pnpm typecheck, pnpm check:spec-drift, node scripts/check-public-repo-hygiene.mjs: all clean. node scripts/check-workflow-pins.mjs on the real tree: exit 0, 1 caller(s) checked, all pins agree. --print-pin cursor-review.yml: bare 40-hex SHA, exit 0. Two mutation runs as described under Residual. .github/ is byte-identical to 56a738b. These are LOCAL results: the repo's own gates still do not run in GitHub Actions against this PR while it is stacked, for the reason given at the top.
Deviations: none on the ticket's plan. Two mechanical adjustments: the branch is based on #169 as specified but #169's branch was 3 commits behind #153's tip (which is where isJobLevelUses, owningKey and blockScalarMask actually live), so #153's branch is merged in — ci.yml is byte-identical to #153's tip (re-verified at 31d999b against bb3c999; git diff against that ref is empty) and carries no new pnpm install. The unreadable-value check is written in the if (!USES_RE.exec(...)) branch rather than as a separate pass before it; same set of lines, one regex evaluation instead of two. Review round 6 added the owner/repo case-insensitivity fix and re-merged #153's tip to pick up its round-5 watchdog fixes; a later pass re-merged bb3c999 to pick up its round-9 fixes, which is the only change in 31d999b. No other scope change. f462d5c re-merged #153 at 56a738b, which conflicted in findCallers. #153 had replaced the owningKey(lines, …) walk-back with owningKeyChain, and it had added its own block-scalar and continuation-line uses: checks. This PR's call sites were moved to the chain API. The two unreadableUsesReason arms those checks superseded were removed, and the affected tests now assert #153's messages. Two cases were added so the unreadable path stays covered.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-typescript-sdk/pull/170 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/21/2026 **Status:** ✅ Merged **Merged:** 10/9/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `matt/be-16299-check-workflow-pins-regression-suite` ← **Head:** `matt/be-16336-check-workflow-pins-fail-closed` --- ### 📝 Commits (10+) - [`04eba98`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/04eba989ccfe3fb101886ac093a5fbbfeea34cbd) ci(cursor-review): read the watched pathspecs from upstream, and stop reading faults as verdicts - [`b541197`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/b541197d6b0fc205cbb79f1edb74f95c3c8858ef) ci(cursor-review): read a vanished bump workflow as broken, not as a fault - [`d80dbd4`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/d80dbd4408920323c9f21d10b44b9270055d703f) ci(cursor-review): stop the watchdog reading faults, decoys and magic as verdicts - [`278ec24`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/278ec24b64b4edeeb53246aa4b687d2b94d5b1b4) Merge remote-tracking branch 'origin/matt/be-15270-resync-cursor-review-pins' into matt/be-16336-check-workflow-pins-fail-closed - [`fcfbab4`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/fcfbab471e898da1787e3b9ca688bd681ad5b755) ci(workflow-pins): fail closed on a job-level uses: this scanner cannot read - [`42365c0`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/42365c01b22629af587153e3f7adeea901d11a9f) ci(cursor-review): close the round-5 holes in the pin-freshness watchdog - [`5596838`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/55968385ae120b30b1e3c0b8c614c4aab6a7caf2) Merge remote-tracking branch 'origin/matt/be-15270-resync-cursor-review-pins' into matt/be-16336-check-workflow-pins-fail-closed - [`ecfcadd`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/ecfcadda29de93875245fd5944fecd512546dd0b) ci(workflow-pins): match the reusable's owner/repo case-insensitively - [`bb3c999`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/bb3c999e3d03d573fa5400a74d14b066ea5051e1) ci(cursor-review): close the panel's round-9 fail-open holes - [`31d999b`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/31d999b09c0742c8e66e8bc245e98ce475d81249) Merge remote-tracking branch 'origin/matt/be-15270-resync-cursor-review-pins' into matt/be-16336-check-workflow-pins-fail-closed ### 📊 Changes **4 files changed** (+2901 additions, -206 deletions) <details> <summary>View changed files</summary> 📝 `.github/dependabot.yml` (+44 -6) 📝 `.github/workflows/ci.yml` (+1938 -120) 📝 `scripts/check-workflow-pins.mjs` (+716 -75) 📝 `scripts/check-workflow-pins.test.ts` (+203 -5) </details> ### 📄 Description **STACKED — merging lands on `matt/be-16299-check-workflow-pins-regression-suite` (owned by @mattmillerai, PR #169), NOT `main`.** #169 is itself stacked on #153. GitHub retargets this PR as each foundation merges; do not read the checks below as "safe to merge into main". One consequence a reviewer should know: `ci.yml` triggers on `pull_request: branches: [main]`, so while this PR is stacked the repo's own gates — `test`, `workflow-pins`, `public-repo-hygiene`, `build-check` — do not run on it at all. That is pre-existing behaviour for every stacked PR here (it applies to #169 and #153 too), not something this change introduced. They were all run locally; see **Verified** below, and they will run for real once GitHub retargets this PR to `main`. ## ELI-5 `scripts/check-workflow-pins.mjs` reads workflow YAML one line at a time, so it only recognises a reusable-workflow call written as one plain line. Write the same call a different but perfectly legal way — folded (`uses: >-`), with the value pushed onto the next line, or with the whole job spelled as a flow mapping (`review: { uses: ... }`) — and the scanner saw *nothing there at all*. Put one of those next to a normal caller and the lint happily reported "all pins agree" over a genuinely split pin, while `--print-pin` handed the daily freshness watchdog the *other* job's SHA as if it were the only pin in the repo. Now the scanner tells those three apart from "no caller here": it reports the file and line, says which spelling it choked on, tells you to write `uses: …@<sha>` on one line, and exits 1 in both modes. ## What changed `scripts/check-workflow-pins.mjs`, text-level only — no `yaml` import, because the two CI jobs that run this script deliberately have no `pnpm install`. - `USES_KEY_RE` matches a `uses` KEY with *any* value, including none on the line. The gap between it and `USES_RE` (which matches only the values the scanner can read) is exactly the set of lines that used to vanish. - `unreadableUsesReason` classifies that gap: a quote that never closes (`multi-line quoted scalar`), or anything else still naming the org's reusables (`owner/path/ref did not parse`). The other two shapes, a block-scalar header and an empty value with the value on the next line, are now caught earlier by #153's own `USES_BLOCK_SCALAR_RE` / `USES_EMPTY_VALUE_RE` checks. Those read the folded value and fail in both modes with #153's message. This PR originally classified them here too (`folded/literal block scalar`, `value on a continuation line`); those arms were removed at `f462d5c` because they could no longer fire. It returns `null` — a silent skip, as today — for a single-line value naming some other owner (`./.github/workflows/local.yml`, `octo/repo/.github/workflows/x.yml@v1`): those pin once and have no `workflows_ref` to disagree with. - `FLOW_JOB_RE` catches a job whose whole body is a flow mapping, positioned by its owning key being a ROOT `jobs:` (asked structurally off #153's `owningKeyChain`, the same way #153's `isJobLevelUses` asks it) rather than by hardcoding two-space indent. - `findCallers` now returns `{ callers, unreadable }`. Both modes read `unreadable` and both report it **before** their own checks — `--print-pin` writing nothing whatsoever to stdout, since ci.yml captures that mode with `$(...)` and any stray byte would become the "pin" it feeds to git. - The zero-caller guard's message no longer offers "a folded `uses: >-`, a flow mapping" as one half of an either/or; those are now reported by name, so reaching that guard really does mean no job-level `uses:` exists. `scripts/check-workflow-pins.test.ts` gains `describe("spellings the scanner cannot read fail closed")`, now eight cases, each asserting the lint mode and `--print-pin` together. Every positive fixture places the unreadable job **beside a plain, correctly-pinned sibling**, because that sibling is what made the old silence survivable; without it the zero-caller guard would have caught the file and the case would prove nothing. ## Two design calls worth a reviewer's eye **A flow-mapping job is reported even if it calls nothing.** A flow mapping nests to any depth on one line, so "does this contain a `uses:`" is precisely the question a line-anchored scanner cannot answer — guessing would reinstate the silent skip. No workflow here is written that way, and the remedy is a reformat. **`--print-pin` refuses when the unreadable job is in a different file than the reusable it was asked about.** Deliberate: an unreadable job anywhere could be a second caller of that same reusable, which is exactly the ambiguity `--print-pin` already refuses to resolve. ## Falsification of the "cannot read" claim This diff adds a deny path, so the premise was checked empirically rather than asserted, on the two paths that could make it false: - **No built-in YAML parser exists to use instead.** On Node v26.4.0 (newer than the 22/24 CI runs): `require('node:module').builtinModules` matches nothing for `/yaml/i`; `import('node:yaml')` → `ERR_UNKNOWN_BUILTIN_MODULE`; `node:util` exposes only `parseEnv` and `parseArgs`. - **The `yaml` devDependency is genuinely unreachable in the CI shape.** `import('yaml')` from a tree with no `node_modules` → `ERR_MODULE_NOT_FOUND`. `yaml` is in `devDependencies` only, and both `workflow-pins` (ci.yml:231) and `cursor-review-pin-freshness` run checkout + setup-node + `node scripts/…` with no install step — unchanged by this PR. Note the outcome is not a capability the author loses: the error names the spelling to write instead, and the one-line form is already what every caller in the repo uses. ## Corpus sweep — including the half not fixed Swept all 5 files under `.github/workflows`: 22 `uses:` lines, 21 of them step-level and 1 job-level (the real caller, readable, still the only thing the lint reports). **Zero** occurrences today of any shape this guard does *not* cover — 0 flow-mapping jobs, 0 alias-valued `uses: *ref`, 0 multi-document files, 0 merge-key anchors. So the guard changes nothing about the current tree and the remaining gaps are hypothetical rather than latent. ## Review round 6 — cursor-review panel The panel raised **10** findings. Only **one** of them is in this PR's own commit (`fcfbab4`, which touches `scripts/check-workflow-pins.mjs` and its test and nothing else); the other nine are in `.github/workflows/ci.yml` and in `commentShaCandidates`, both of which reach this diff only because the branch merges #153's branch. - **Fixed here (1):** the reusable's owner/repo was compared **case-sensitively**, so `comfy-org/github-workflows/...` — which GitHub resolves to the very same repo and calls successfully — matched neither `USES_RE` nor `unreadableUsesReason`, and was skipped in silence. Reproduced against the pre-change script on a two-job fixture pinned to two different SHAs: `1 caller(s) checked, all pins agree`, exit 0, with `--print-pin` returning the other job's SHA. Fixed in `ecfcadd` with a regression test. - **Fixed by re-merging #153's tip (5):** the `GIT_NO_LAZY_FETCH` existence check, the block-scalar-header comment, the `:^` exclusion synonym and the dash-column block indent were all fixed by #153's `42365c0`. #153 has since landed `bb3c999` ("close the panel's round-9 fail-open holes"), re-merged here in `31d999b`, which also fixes the fifth — `commentShaCandidates` now discriminates dates by LENGTH (`!/^[0-9]{8}$/`) on both branches instead of dropping every all-digit token, so a 7-digit abbreviation is checked like any other and the ~1-in-27 silent skip of assertion (3) is closed. Those five threads are resolved. - **Left for #153 (4), three since fixed there:** one Medium (the `age` clamp does not bound the mute — a future-dated commit yields `state=grace` daily) and three Low/Nit (the unprobed wildcard pathspec, the per-line-anchored `pin_out` guard, and `always()` where `!cancelled()` is meant). All four are in the `cursor-review-pin-freshness` job in `ci.yml` — #153's code — and all four were **re-checked against `bb3c999` and still stand**; that commit fixed a different nine. #153 is open and merges first, so patching `ci.yml` from here would only collide with its next round. **Those four threads are deliberately left unresolved** and are itemised on #153 (comment `5757140139`). #153 has since fixed three of them: `56a738b` stops a future-dated commit past the skew tolerance from counting as grace, the `pin_out` guard is a whole-value `=~` match, and the steps use `!cancelled()`. `f462d5c` merges that tip, so those three threads are resolved. The unprobed-wildcard thread is still open because the code is unchanged at `56a738b`. ## Residual - **#153 has since widened `USES_RE` to match the owner/repo case-insensitively**, which supersedes this PR's choice to keep it case-sensitive. The concern that choice rested on (the lint exiting 0 on a split pair across two spellings) is answered on #153's side by its spelling lint (4), which names the non-canonical caller and exits 1. `--print-pin` sees two callers and refuses. The letter-case test now asserts both. `unreadableUsesReason` still matches case-insensitively for a lowercase owner/repo that `USES_RE` cannot parse. - **Alias-valued `uses:` is still a silent skip.** `uses: *some_anchor` reads as a single-line non-org value, so `unreadableUsesReason` returns `null` and it is skipped — yet the alias could resolve to an org reusable with a pin the lint never checks. Resolving anchors means tracking `&anchor` definitions across the file, which is a second parser's worth of work and outside this ticket's one guard. 0 occurrences in the tree today. Worth its own ticket if anchors ever appear in these workflows. - **A `jobs:` block written wholly as a flow mapping is still a silent skip.** `FLOW_JOB_RE` requires the line's owning key to be the column-0 `jobs:`; `jobs: { review: { uses: … } }` has no owning key, so nothing reports it. Same fix family as the alias gap. 0 occurrences today. - **Multi-document YAML and merge keys (`<<: *base`) are unhandled** by the scanner generally — not introduced here, not fixed here. 0 occurrences today. - **The upstream investigation that produced this plan was not readable from this environment**, so its evidence was re-derived rather than trusted: the three silent-skip shapes were reproduced against the pre-change script in a throwaway fixture tree (all three skipped, lint printed `all pins agree`, `--print-pin` returned the sibling SHA) before any code was written. - **The repo's own CI gates never ran against this PR in GitHub Actions**, because `ci.yml` is scoped to `pull_request: branches: [main]` and this PR's base is a stacked branch. Every one of them was run locally instead and is listed under **Verified**; the first CI-side run of `test` / `workflow-pins` / `public-repo-hygiene` / `build-check` against this diff happens after retargeting. Nothing here can substitute for that. - **Verified by mutation at `f462d5c`:** disabling the flow-job check reddens 1 of 34 tests. Disabling the `unreadable` key path reddens the 2 cases added at that merge. Before those cases were added it reddened nothing, because the letter-case case no longer reaches that path. **Earlier mutations, also not committed** — nothing in CI re-runs it. With both `unreadable.push` sites commented out, exactly the 4 original positive cases go red and the negative case plus all 26 pre-existing cases stay green; with `unreadableUsesReason`'s `return null` flipped to a reason, the negative case goes red (as does the pre-existing `other-org/shared` case). The case-insensitivity case added in review round 6 was mutation-checked the same way: reverting the `toLowerCase()` reddens that case and only that case, 1 of 32. ## Provenance - **Authored by:** agent-work loop - **Verified:** at `f462d5c` (merge of #153's tip `56a738b`, with the `findCallers` conflict resolved): `pnpm vitest run scripts/check-workflow-pins.test.ts`: 34 passed, 0 failed. `pnpm test`: 654 passed, 0 failed across 27 files. `pnpm lint`, `pnpm format:check`, `pnpm typecheck`, `pnpm check:spec-drift`, `node scripts/check-public-repo-hygiene.mjs`: all clean. `node scripts/check-workflow-pins.mjs` on the real tree: exit 0, `1 caller(s) checked, all pins agree`. `--print-pin cursor-review.yml`: bare 40-hex SHA, exit 0. Two mutation runs as described under Residual. `.github/` is byte-identical to `56a738b`. These are LOCAL results: the repo's own gates still do not run in GitHub Actions against this PR while it is stacked, for the reason given at the top. - **Deviations:** none on the ticket's plan. Two mechanical adjustments: the branch is based on #169 as specified but #169's branch was 3 commits behind #153's tip (which is where `isJobLevelUses`, `owningKey` and `blockScalarMask` actually live), so #153's branch is merged in — `ci.yml` is byte-identical to #153's tip (re-verified at `31d999b` against `bb3c999`; `git diff` against that ref is empty) and carries no new `pnpm install`. The unreadable-value check is written in the `if (!USES_RE.exec(...))` branch rather than as a separate pass before it; same set of lines, one regex evaluation instead of two. Review round 6 added the owner/repo case-insensitivity fix and re-merged #153's tip to pick up its round-5 watchdog fixes; a later pass re-merged `bb3c999` to pick up its round-9 fixes, which is the only change in `31d999b`. No other scope change. `f462d5c` re-merged #153 at `56a738b`, which conflicted in `findCallers`. #153 had replaced the `owningKey(lines, …)` walk-back with `owningKeyChain`, and it had added its own block-scalar and continuation-line `uses:` checks. This PR's call sites were moved to the chain API. The two `unreadableUsesReason` arms those checks superseded were removed, and the affected tests now assert #153's messages. Two cases were added so the `unreadable` path stays covered. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 22:05:19 +00:00