[PR #153] ci(cursor-review): re-sync both pins to 425c154 and stop Dependabot splitting them #155

New issue

Open

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

bsakaguchi commented 2026-10-31 22:05:06 +00:00

Owner

Copy link

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-typescript-sdk/pull/153
Author: @mattmillerai
Created: 9/18/2026
Status: 🔄 Open

Base: main ← Head: matt/be-15270-resync-cursor-review-pins

---

📝 Commits (10+)

• 1f8b4b9 ci(cursor-review): re-sync both pins to 425c154 and stop Dependabot splitting them
• a07ed16 ci(workflow-pins): assert the two cursor-review pins agree, in CI
• 38e14c9 ci(cursor-review): daily watchdog for a stale github-workflows pin (#163)
• 41fd901 ci(workflow-pins): read workflows_ref only as a with: input, and ignore fork bump PRs
• 6a1d0fc ci(cursor-review): say what actually happened when the watchdog closes its issue
• 6b02db7 ci(workflow-pins): read workflows_ref wherever with: sits in the job
• 786762c ci(cursor-review): bound the watchdog's mutes and fail its drift closed
• 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

📊 Changes

7 files changed (+5123 additions, -14 deletions)

View changed files

📝 .github/dependabot.yml (+56 -0)
📝 .github/workflows/ci-cursor-review.yml (+2 -2)
📝 .github/workflows/ci.yml (+3139 -12)
📝 package.json (+1 -0)
➕ scripts/__fixtures__/ci-cursor-review.83c0c87.yml (+75 -0)
➕ scripts/check-workflow-pins.mjs (+1071 -0)
➕ scripts/check-workflow-pins.test.ts (+779 -0)

📄 Description

ELI-5

Our Cursor review workflow points at an upstream repo twice — once for "which workflow file to run" and once for "which prompts and scripts that workflow should load." Those two pointers had drifted 31 commits apart, so every review this repo ran used one version's workflow driving another version's scripts. This points both at the same commit, and tells Dependabot to stop touching them — Dependabot only knows how to rewrite the first pointer, which is what pulled them apart in the first place.

What changed

.github/workflows/ci-cursor-review.yml — both pins now read 425c154ce5049324ee23ad10e106baeed4cfa31b, the commit the rest of the org's cursor-review callers were moved to on 2026-09-12. The uses: line's trailing comment, which Dependabot left stale at (ffcc3f5) while rewriting the SHA out from under it, is corrected to (425c154).

.github/dependabot.yml — the github-actions entry gains an ignore block naming the upstream reusables by their per-file path (Comfy-Org/github-workflows/.github/workflows/*). It carried a bare owner/repo entry alongside it until round 16 dropped that as dead — see below. No other Dependabot setting is touched.

The review rounds below added the other half — the part that notices if the pair splits again, whoever splits it:

scripts/check-workflow-pins.mjs (new) + its workflow-pins job in .github/workflows/ci.yml and a check:workflow-pins script in package.json — a text-level lint asserting, per caller, that uses: is a full 40-hex SHA, that workflows_ref: equals it, and that the trailing # … (short) comment agrees. It also serves --print-pin, which the watchdog below consumes.

cursor-review-pin-freshness in .github/workflows/ci.yml (new) — a schedule-only watchdog for the failure this PR's own history demonstrates: nobody bumps the pin at all. It reads the pin via --print-pin, reads upstream's own WATCHED_PATHSPECS list, and reports current / grace / delivered / stale / broken, with two sticky issues (the pin is stale; the watchdog cannot answer) that open, reopen and close themselves.

Why

The caller pins the upstream reusable twice: the uses: SHA selects the workflow definition, and the workflows_ref: input selects the prompts/scripts loaded at run time. The file's own comment already says to keep them matching. On main they did not: uses: sat on ebfd9e4 while workflows_ref: sat on ffcc3f5, 31 commits and 7 differing cursor-review assets apart.

Dependabot is the cause and would be the cause again. It understands uses: only. Verified against this repo's own history — the two bumps that produced the split, #135 and #141, each touched exactly one file and #135's patch is the whole story:

-    uses: …/cursor-review.yml@ffcc3f5…  # github-workflows main (ffcc3f5)
+    uses: …/cursor-review.yml@ce203b3…  # github-workflows main (ffcc3f5)

workflows_ref: untouched, and the explanatory comment left pointing at the old value.

The ignore entry is load-bearing, and this repo supplies its own proof rather than borrowing the sibling repo's: #135 and #141 are titled ci: bump Comfy-Org/github-workflows/.github/workflows/cursor-review.yml from … to …. Dependabot names the dependency by its full path, and a dependency-name without a wildcard is an exact match — so a bare Comfy-Org/github-workflows entry would have matched nothing and been inert. This PR originally carried one beside the globbed form as belt-and-braces; round 16 dropped it as dead, with the reasoning recorded both in the file and in that round's note below. The globbed per-file form is the one that actually covers these callers, and it is scoped to .github/workflows/* rather than a bare /* so it does not also silently freeze step-level composite actions from the same repo. Deliberately not collapsed to the single Comfy-Org/github-workflows*, which Dependabot expands to an unanchored .* and which would therefore also swallow any future sibling repo whose name merely starts with this one.

Riskiest line, and why it is safe

uses: moves backwards in upstream history (ebfd9e4, 2026-09-14 → 425c154, 2026-09-12). Three checks, all read-only against the upstream repo:

• The only commit between the two touches a single file — upstream's own caller at .github/workflows/ci-cursor-review.yml. Zero cursor-review assets and no change to the reusable cursor-review.yml itself. Everything this caller actually executes is byte-identical at both SHAs.
• The reusable's workflow_call interface is identical at both: same 14 inputs, same workflows_ref as the only required one, same three secrets with CURSOR_API_KEY the only required one. Neither input this caller passes (extra_generated_globs, workflows_ref) nor either secret it forwards has moved, and no newly-required input appears.
• After the change the workflow ref and the assets ref compare to an empty diff, which is the invariant the file's comment asks for and the property the upstream equality guard will check.

Verification

• python3 -c "import yaml; yaml.safe_load(...)" parses both changed YAML files.
• grep -oE '[0-9a-f]{40}' .github/workflows/ci-cursor-review.yml | sort -u → exactly 425c154ce5049324ee23ad10e106baeed4cfa31b.
• grep -c 'dependency-name: "Comfy-Org/github-workflows' → 1 — the single globbed per-file entry. It read 2 until round 16 dropped the bare owner/repo entry as dead.
• Full local gate green: pnpm lint, pnpm format:check, pnpm typecheck, pnpm check:spec-drift, node scripts/check-public-repo-hygiene.mjs, pnpm test (26 files, 620 tests passed).
• Upstream reads: all three SHAs resolve; 425c154 carries .github/workflows/cursor-review.yml; the interface and commit-range comparisons above.

Applying the cursor-review label to this PR exercises the changed caller end to end.

Judgment calls

• The source request's step-2 verification says "the PR diff touches only .github/dependabot.yml", which contradicts its own step 1 (re-sync both pins in the workflow file) and its own closing verification (which greps the workflow file for the SHA). Read as a stale clause carried over from a Dependabot-only change; this PR does both steps in one diff as the request's own headline instruction asks. The intended reading of that clause — no other Dependabot setting changed — is satisfied.
• grep -n 'Comfy-Org/github-workflows' .github/dependabot.yml returns 7 lines, not 1. Six of them are the ignore block's explanatory comment; the entry is exactly one. grep -c 'dependency-name: "Comfy-Org/github-workflows' is the check that means what was intended. (Before round 16 this read 4 lines against 2 entries.)
• The comment block started as the sibling repo's, supplied verbatim, but the review rounds have since rewritten it — correcting its claim that dependency-name matching is case-sensitive, narrowing the glob to .github/workflows/*, and recording why the bare entry was dropped. The tradeoff below is consequently now stated in-file as well as here: an ignore with no versions:/update-types: qualifier suppresses Dependabot security updates for these reusables as well as version updates. Accepted for the same reason it is accepted upstream — these are first-party org repos pinned to reviewed SHAs, not registry packages, so an advisory-driven bump is not the update path for them.
• No full-autonomy label, which the source request does not carry.

Residual

This repo now has no automated bumper for these pins at all. The ignore correctly stops Dependabot from splitting the pair, but the upstream fleet that moves both pins together only bumps repos on its CURSOR_REVIEW_CALLERS roster, and this repo is not on it. Falsified empirically rather than assumed: the full commit history of .github/workflows/ci-cursor-review.yml across all authors is three commits — the hand-authored enrolment in #111, then #135 and #141, both dependabot[bot]. No fleet bump PR has ever reached this repo. So after this merges the pins are correct and locked, and they will stay at 425c154 until either the roster entry is added or someone bumps them by hand. The follow-up is an operator action, outside what a repo PR can do: add {"repo":"Comfy-Org/comfy-typescript-sdk","file":".github/workflows/ci-cursor-review.yml"} to the CURSOR_REVIEW_CALLERS secret in the upstream workflows repo. Until then, whoever next needs an upstream cursor-review fix here must move both pins together by hand — the uses: SHA, the workflows_ref: input, and the trailing # github-workflows main (<short>) comment, which Dependabot has twice left stale.

Unexercised artifacts. The originating investigation ticket and its findings comment — the source of the "4 of 72 callers" measurement and of the upstream job.workflow_sha equality-guard epic — are on an issue tracker this session has no access to, so that corpus figure is carried forward unverified. What was verified here is the local slice: this repo contains exactly 1 caller of the upstream reusables (the other three workflows use pnpm/action-setup and contributor-assistant/github-action, untouched by the new ignore), it was split, and it is now consistent — 1 of 1 in-repo, 0 remaining. The other 3 of the 72 org-wide callers said to be split live in other repositories and are out of scope for this PR; nothing here fixes or measures them. The upstream equality guard itself is not yet in force at 425c154, so this change is verified against the invariant it will check rather than against a red build it turns green.

Review round — the panel's two findings (a07ed16)

Both were Low, both raised by 2 of 6 reviewers. One is fixed in code, one is deferred with evidence.

Fixed: nothing enforced the "keep workflows_ref matching" invariant. The panel's framing is what made this worth doing — the lines this PR replaces are themselves the proof that a comment is not a guard, since the pair sat split for eight days across two merged Dependabot PRs with nothing going red. scripts/check-workflow-pins.mjs is the missing half, wired in as a workflow-pins CI job. For every Comfy-Org/github-workflows caller it asserts three things about one upstream commit: uses: is a full 40-hex SHA (not a movable tag or branch), workflows_ref equals it exactly, and the trailing # … (short) comment agrees with it. A missing workflows_ref on a reusable that requires one is an error too — an omitted workflow_call input arrives as '', and actions/checkout with ref: '' silently takes the default branch, which is the same hole from the other side.

Verified against the real history, not only a synthetic fixture: run on 83c0c87's ci-cursor-review.yml it reports both halves of the split this PR fixes — the stale (ffcc3f5) comment and the ffcc3f5b… input against an ebfd9e44… uses: — and exits 1; on this branch it exits 0. Four more cases checked: a branch-pinned uses:, a deleted workflows_ref, a stale-comment-only drift, and quoted-scalar spellings of both pins in sync (passes) and split (fails, so quoting is not a bypass).

Text-level parsing is deliberate: the job runs from a bare checkout + setup-node with no pnpm install, the same shape as public-repo-hygiene, so it cannot import the yaml devDependency — the same constraint the upstream pin lint works under. The one structural limit (a workflows_ref: written inside a block scalar) is recorded in the script header and fails loud rather than silently passing. Upstream's own check_workflow_pins.py does not cover this case: it is explicitly an internal lint for the github-workflows repo and checks the reusable's side, not the caller's pin pair.

Deferred with evidence: the ignore entries suppress security updates, and the replacement bumper may not be wired up. Two halves, handled differently.

The missing versions:/update-types: qualifier is intentional and is now documented in dependabot.yml itself rather than only in this body. The reasoning is sharper than "accepted upstream": the problem is not which versions Dependabot picks, it is that it cannot move this dependency correctly at all, so a security bump would split the pair exactly like a routine one. Narrowing the ignore would buy a broken pin rather than a patched one.

The enrollment half is the live risk, and it is the Residual below. It is not confirmable in-band — the roster is an Actions secret with no gh secret get — so it is filed as a follow-up for someone with access to the private callers.json, rather than guessed at here. Worth separating from the guard above: check-workflow-pins.mjs catches a split pin and makes no network call, so it cannot catch a stale but internally consistent one. That is a different failure and needs the schedule-driven watchdog the follow-up proposes.

Review round 2 — CodeRabbit (41fd901)

Two findings, both Major, both valid, both fixed in code. No deferrals.

The pin lint read workflows_ref from the wrong place. jobBodyAfter returned every line in the calling job at or below the uses: indent, and WORKFLOWS_REF_RE accepted any indentation — so a workflows_ref: written as a sibling of uses: satisfied the pin even though GitHub never passes it to the reusable. Verified rather than assumed: SchemaStore's reusableWorkflowCallJob is additionalProperties: false over exactly {cache-mode, concurrency, if, name, needs, permissions, secrets, strategy, uses, with}, so such a key is not an input at all. That is the one direction this lint must never fail in — passing a caller whose real pin is missing or split.

jobBodyAfter is replaced by workflowsRefInput, which tracks the with: block and accepts only a direct child of it. That closes three shapes beyond the one reported: a workflows_ref under secrets:, one inside a block scalar, and one under a nested mapping beneath another input. The previous round's "documented limit" about block scalars is therefore retired; what remains is a flow-mapping with: { ... }, which now reads as absent and so fails loudly as a missing input on a reusable in REQUIRES_WORKFLOWS_REF rather than passing silently. Recorded in the script header.

Checked with a 14-case fixture matrix, which covers the rewrite's regression risk as well as the new behaviour: the four newly-rejected shapes fail, and every case the previous round claimed still holds — split pins, stale trailing comment, branch-pinned uses:, deleted workflows_ref, quoted scalars in sync and split — plus with: carrying a trailing comment and a flush-left comment inside the block. Run against 83c0c87's real ci-cursor-review.yml it still reports both halves of the historical split and exits 1.

The watchdog's "already delivered" probe could be muted by a fork. gh pr list --repo "$GH_REPO" --state open --head "$BUMP_BRANCH" filters by branch name; --repo selects the base repository, not the head one. Any fork PR on a branch called ci/bump-cursor-review would have read as a delivered bump and suppressed the stale-pin issue indefinitely — the precise three-week invisibility this watchdog exists to end. Cross-repository PRs are now filtered out, with the --repo/--head asymmetry spelled out at the call site since it is not self-evident. The jq was checked against the live API, not by eye: --json number,isCrossRepository is accepted and the expression returns 153 for this branch, empty for a fork-only or empty result set.

Unrelated red check, for whoever merges. sdk-parity (python surface freshness) is failing — and is failing identically on main (run for 4491e4b), with no commit in this PR touching anything it reads. The upstream comfy-python-sdk added a public RouterRunResult class to src/comfy_sdk/models.py, which the extractor hard-fails on by design rather than silently skipping. It is not a required check on main (the required set is Test (node 22), Test (node 24), build-check (publish dry run), public-repo-hygiene — all green here) and deciding whether that class belongs in the parity surface is a separate change to a separate script, so it is deliberately not bundled into a CI-pins PR.

Self-review round (6a1d0fc)

One finding, mine rather than a reviewer's, in code this PR adds.

The stale-pin issue closed with a sentence that is usually false. The close step is gated success() && state != 'delivered', which matches current and grace, but its comment read "the pin is current again" unconditionally. In grace the pin is still behind — the oldest commit it has not picked up is merely inside the 3-day window.

That is the ordinary path, not an edge case. A bump lands in grace rather than current almost every time, because upstream keeps committing while the bump PR sits in review: at the 0.39-day median gap between consumer-visible commits that this job's own comment measures, main has usually moved again before the PR merges. So the common outcome was the one carrying the wrong sentence, and whoever read the closing comment would believe a pin four commits behind is level with main.

Closing on grace is kept, and is right: this issue's premise is that nothing is moving the pin, and a bump landing inside the window falsifies exactly that. Only the wording is per-state now — the grace branch reports how far behind the pin actually is, names the oldest unpicked commit, and says the issue reopens if the pin freezes past the window again. BEHIND_COUNT/OLDEST_SHA are unset on the current branch (assess exits before writing them) and are read only on the grace branch; env: defines the names regardless, so set -u holds either way.

Verified by running the step's real run: block — extracted from the parsed YAML rather than retyped — against a stubbed gh in all three reachable cases: current with an open issue, grace with an open issue, and no open issue at all (a no-op). rc=0 each, backticks intact, no set -u failure on the empty variables.

One comment-only addition. check-workflow-pins.mjs documents a second "reads as input absent" limit beside the flow-mapping one: a with: block written above its job's uses: is not seen, because the scan starts at the uses: line. No behaviour change. (Superseded in round 3 below — CodeRabbit flagged this same limit and it turned out to be worth fixing rather than documenting, so the limit and this note are both gone as of 6b02db7.)

The unrelated red check now has a fix in flight. sdk-parity (python surface freshness) is still failing here and still failing identically on main (the upstream Python SDK's new RouterRunResult class; nothing on this branch touches what it reads). It is not a required check — the required set is Test (node 22), Test (node 24), build-check (publish dry run), public-repo-hygiene, all green here. #167 is open against it, which is where that fix belongs rather than bundled into a CI-pins PR.

Review round 3 — CodeRabbit (6b02db7)

One finding, Minor, valid, fixed in code. No deferrals.

The pin lint could not read a with: block written above its job's uses:. workflowsRefInput scanned only the lines after uses:, so that caller read as passing no pin at all — lint would have rejected a correctly pinned caller, and --print-pin would have fail()ed and taken the daily cursor-review-pin-freshness watchdog down with it.

This is the limit the self-review round above had just finished documenting, which is the interesting part: that round judged it acceptable because it reads as "input absent" and so fails loud rather than passing a split pin silently. The direction-of-error reasoning was right and the conclusion was still wrong — being loudly wrong about a spelling GitHub accepts is a false positive on a valid caller, and YAML block mappings are unordered, so key order carries no meaning to disagree with. The header had weighed this against carrying a YAML parser into a job that runs from a bare checkout with no pnpm install; the actual fix is a four-line backward walk, so that tradeoff was never the one on the table.

workflowsRefInput now starts just after the job's <job id>: key, found by walking back from uses: to the nearest shallower non-blank, non-comment line. The with:-tracking state machine is untouched — this only widens the window it reads — so every shape round 2 newly rejected is still rejected.

Checked against what the backward walk must not do as well as what it must, since leakage is the regression this introduces: with: before uses: passes and reports a split correctly when the pins disagree; the existing ordering still passes; and the invalid shapes still fail — a workflows_ref sibling of uses:, one nested deeper than the with: children, one belonging to a preceding job (the walk stops at the job's own key, so it cannot leak across jobs), and a decoy inside a block scalar above uses:. Real repo unchanged: lint exits 0 and --print-pin still prints 425c154. Key order is now recorded in the header as explicitly not a limit; the flow-mapping with: { ... } limit stands.

Review round 4 — cursor-review panel (786762c)

Ten findings on 6b02db7: one High, three Medium, five Low, one Nit. Nine are fixed in code, one half-declined with a reason, one self-identified follow-up filed. No deferrals of a reviewer's finding.

Nine of the ten share one shape, which is worth naming because it is the shape this PR is about: a check that degrades to silently healthy. The original bug was two pins drifting apart for eight days with nothing going red. Round 1 added a lint and a watchdog to end that, and round 4's panel found that the watchdog and the lint each had their own ways of reporting "fine" when they in fact knew nothing.

High — delivered suspended the watchdog with no upper bound. An open same-repo PR on ci/bump-cursor-review made the job exit 0 every day, forever, however frozen the pin was — the same silent freeze, now with an alibi. It gets a second clock of its own (BUMP_PR_GRACE_SECONDS, 7 days, deliberately separate from the 3-day pin grace because what is being waited on is a human merge, not a fleet delivery). Past it the verdict escalates to stale, which files the sticky issue and names the stuck PR as the thing to chase, in the ::error::, the issue body and the top of its checklist. That also closes the reviewers' second point: a pin that first went stale while such a PR was open previously left a ::notice:: on a scheduled run as its only record.

Medium — the watched-pathspec list drifted OPEN. The list mirrors upstream's WATCHED_PATHSPECS by hand, kept in sync by a comment. If upstream renamed a path, git log over it matched nothing, behind came back empty, and the job wrote state=current, reported "Pin is current" and closed its own issue every day regardless. The list is now one watched array used for both the existence check and the git log pathspec — so those two cannot disagree — and every entry must still resolve in the clone (ls-tree, trees only, no blob fetch) or the job fails closed. Deliberately not read out of /tmp/gw: parsing a file this repo does not own would trade a drift that now fails closed for a parse that could fail open.

Medium — merge-base --is-ancestor conflated a verdict with an outage. Exit 1 (not an ancestor) and exit 2+ (unreadable object, origin/main renamed, partial-clone fetch failure) were both reported as "Re-pin both pins", with 2>/dev/null discarding the only text that told them apart. Only exit 1 is a verdict now; a hard error reports stderr verbatim and writes no state, so no issue step fires — preserving the rule the file already stated, that a fault establishing nothing about the pin must not file an issue claiming something about it.

Medium — the sticky-issue lookups truncated. All of them paged gh issue list --state open --limit 100 and matched by title, so past 100 open issues the stale path would file a fresh duplicate daily and the close paths would silently stop closing. Lookup is now by a cursor-review-pin label (created idempotently before each use, so the first run on a fresh repo need not be the one that remembered), with the exact-title match kept on top because two titles now share that label.

Low — the close comment promised a reopen that never happened. Fixed by making the code match the promise rather than the reverse: the stale path now looks for a closed issue and reopens it, so one thread carries the history, which is what the sticky-issue design is for. The title changed to ...is stale and nothing is moving it, since the escalation above means a stale verdict can now coexist with an open bump PR. Verified no issue exists under either the old or the new title, so nothing is orphaned.

Low — the grace clock counted branch time, not landing time. %cI without --first-parent let a long-lived branch merged today contribute a weeks-old "oldest unpicked commit", blowing the window on day one and filing an issue saying nothing is moving the pin when the fleet had had one day. --first-parent costs nothing on the common path: a merge commit is still reported when it changes a watched path, and on a squash- or rebase-merging upstream the range is identical. Only the date moves, to the one that answers the question being asked.

Low — REQUIRES_WORKFLOWS_REF was an opt-in allowlist, so it failed open. Inverted to an opt-out NO_WORKFLOWS_REF (empty today). The reviewer's sharpest point was that the allowlist also swallowed the parser's documented limits — the flow-mapping with: one reads as "input absent" — so the opt-in default turned every parser gap into a silent pass on exactly the callers nobody had remembered to list. This supersedes the round-1 wording above, which described the requirement as per-reusable.

Low — the trailing-comment check flagged dates. SHORT_SHA_RE treated every 7-40 char hex word as a claimed SHA, so # github-workflows main (425c154) — bumped 20260919 reddened CI over 20260919. It now reads the documented parenthesised token when one is present, and otherwise falls back to whole-word candidates with all-digit runs dropped — the fallback so that a comment written without parentheses is still checked rather than silently skipped, which would have traded a false positive for a silent miss.

Low — the scanner had no structural awareness. Half fixed, half declined.

Fixed: a uses: line inside a run: | registered as a real caller. Not hypothetical here — the watchdog in ci.yml writes issue bodies about workflow pins inside run: |, so a future edit adding a line starting uses: would have produced a phantom caller, made --print-pin see two matches, and taken the daily cursor-review-pin-freshness job red over a pin problem that does not exist. Block scalars are skipped, keyed off the key's column rather than the line's so - run: | does not swallow its sibling shell:; a quoted "uses": key is recognised.

Declined: matching a step's - uses:. jobs.<id>.uses is the only place GitHub accepts a reusable-workflow call; a - uses: under steps: is a composite action, which pins once and has no workflows_ref to disagree with. Matching it would pull composite-action steps into the lint and — with the allowlist inverted to fail closed in this same round — make every one of them error demanding an input that cannot exist. The mapping-key restriction is what selects reusable calls, and that is now stated in the header rather than left implicit.

Nit — job hardening. workflow-pins gets permissions: contents: read and timeout-minutes: 5; the watchdog gets timeout-minutes: 10, which matters more there because its token can write issues.

Follow-up filed, not deferred from a reviewer. Three consecutive rounds have now fixed a parser defect in check-workflow-pins.mjs, each verified with an ad-hoc fixture matrix built in a temp directory and thrown away — so round N+1 re-verified nothing from round N by machine. A committed regression suite is proposed as its own ticket: it means deciding how vitest (currently src/**/*.test.ts only) should reach a scripts/*.mjs CLI, which is a repo-wide layout choice and does not belong in a pins PR.

Review round 5 — cursor-review panel (04eba98)

Ten findings: nine fixed, one recorded, one suggestion half-declined. Two were verified as live regressions against 786762c rather than taken on description, and one finding's premise was confirmed while its predicted consequence was not.

Medium — the watched-pathspec hand-copy had already drifted, and could not catch drift by addition. Three reviewers asked for the list to be read from upstream instead of mirrored. It now is: git show origin/main:.github/workflows/bump-cursor-review-callers.yml (blobs are lazily fetchable in the --filter=blob:none clone), parsed out of the WATCHED_PATHSPECS: | block scalar by indent. The case turned out to be stronger than reported — the shipped copy carried one of upstream's four :(exclude) entries. Measured over real upstream history from 425c154, the shipped list counts 8 consumer-visible commits where upstream's list counts 7; the extra is 03bae3a, a tests-only commit under .github/cursor-review/tests, precisely what upstream's filter exists to exclude. So the mirror was already manufacturing false staleness on top of being blind to additions. Fails closed on an unparseable list and on an all-:(exclude) result; the decommission probe is kept, over positive entries only.

Medium — a pin naming no commit at all was reported as a transient fault. The likeliest genuinely-wrong pin (typo, force-pushed away, fork-only) surfaces as merge-base --is-ancestor exit 128, which landed in the branch that deliberately writes no state — so a permanent breakage reddened the daily run with nothing sticky. git cat-file -e "$pin^{commit}" now routes it to broken. The exit-1 reason no longer names "an unknown commit", a cause it could not reach. The same treatment went to --print-pin failing, which previously killed the step before any state= was written — so an edit that hid the caller silenced the lint and the watchdog together.

Medium — the watchdog-blocked issue could never close after a stale verdict. stale exits 1, so a success() gate skipped the close in exactly the state the job recovers into. Now always() && state != '' && state != 'broken'.

Medium — zero callers passed green. Now an error: this repo does call the reusables, so an empty scan means the lint checked nothing.

Low — a step's uses: was matched as if it were a job's. Confirmed as a live regression. USES_RE only excluded - uses:; the - name: / uses: form written throughout this repo is a plain mapping key and matched — so with NO_WORKFLOWS_REF inverted to fail closed in round 4, a composite-action step hard-failed the lint demanding an input a composite action cannot accept. Against 786762c that fixture exits 1; it now exits 0. Selection is by position: a direct child of a <job id>: under the top-level jobs:.

Low — the all-digit filter did not cover the parenthesised branch. Confirmed as a live regression. # main (425c154) -- bumped (20260919) exits 1 against 786762c over the date; it now passes. Declined the other half — honouring only the first parenthesised token — because it trades this loud false positive for a silent miss of a stale token written after a fresh one, and the file's stated rule is that a silent skip is the direction it must not fail in.

Low — ls-tree faults read as a drift verdict. Output and status are captured separately; non-zero is now an unattributed hard error writing no verdict.

Low — printf | head -n 1 and SIGPIPE. Mechanism reproduces, consequence does not. On bash 5.2.21 that pipeline does return 141 under pipefail once $behind passes ~5000 lines. But set -e does not act on a failing command substitution in a plain assignment, so the shipped block runs to completion with correct newest/oldest/behind_count — verified at 20000 commits (1.34 MB). This was a latent spurious exit status, not the watchdog outage described. Changed anyway to parameter expansion, since it is simpler and stops a correct backlog carrying a 141; the code comment records the measured behaviour rather than the predicted abort.

Nit — the blocked path had no reopen fallback. Mirrored from the stale path: open → comment, closed → reopen + comment, neither → create.

Low — scheduled workflows are disabled after 60 days of repo inactivity. Recorded in the job comment as the reviewer suggested as the minimum, not otherwise fixed: the remedy is an org-level dispatcher outside this repo, which is an ops decision rather than blind-shippable work, and the trigger condition is far from current activity levels. Deliberately not filed as a follow-up ticket for that reason.

Review round 6 — CodeRabbit (b541197)

One finding, fixed. Confirmed against current code rather than taken on description.

Major — a vanished upstream bump workflow was classified as a transient fault. When git show "origin/main:$bump_yaml" failed, every cause took the no-state fault path. The issue steps are gated if: failure() && steps.assess.outputs.state == 'broken' (and == 'stale') with no generic failure handler, so an upstream rename or deletion — permanent, and never self-clearing — would have burned the scheduled job red every day without ever opening the sticky issue that says why. It was also inconsistent with the neighbouring "the WATCHED_PATHSPECS key is gone" case, which already routes to broken: the same permanent breakage, caught one step later.

ls-tree now probes the path before the branch decides. The three cases separate cleanly because the clone is --filter=blob:none, so trees are already local and ls-tree can still answer after git show has failed on the lazy blob fetch. Verified empirically against a scratch clone rather than assumed:

case	ls-tree rc	output	routed to
path present	0	non-empty	proceeds
absent from readable tree	0	empty	state=broken → sticky issue
unreadable ref	128	fatal: Not a valid object name	fault, no state (unchanged)

The third row is the one worth stating: the pre-existing transient-fault discipline is preserved, so this only reclassifies the deterministic case and adds no new way to suppress a verdict.

Two deliberate deviations from the suggested diff: the code comment points at the ls-tree probe below rather than "above", since the per-pathspec probe — where the separate status/output capture discipline is spelled out — sits after this block; and the reason string says "renamed or removed" and "Re-point the extractor", a rename being the likelier cause with the same operator action either way.

Review round 7 — cursor-review panel (d80dbd4)

Nine findings — three Medium, six Low — all fixed. Every one is a way the watchdog could report a verdict it had not established, all in the fail-open direction. Two reproduced before being fixed, and two measurements changed the shape of the fix.

Medium — git log ran with no captured status. The one git call in the step that never checked its exit code, fed pathspecs this repo does not control. Confirmed in isolation: with a malformed pathspec git log exits 128, set -e does not abort a plain var="$(cmd)" assignment, and behind comes back empty — byte-identical to "nothing landed", so state=current was written and the close step fired on a frozen pin. Status is now captured; a pathspec git refuses to parse is permanent, so it files broken rather than reddening the schedule daily with no issue.

Medium — three captures folded stderr into the value. ls_out and probe_out used emptiness as the "path is gone upstream" signal while 2>&1 merged stderr into it, so chatter on a successful ls-tree read as "still present"; upstream_src was parsed as YAML with git's diagnostics spliced in, which could truncate the watched list mid-block. All three now route stderr to a file, matching the --print-pin capture.

Medium — cat-file -e "$pin^{commit}" 2>/dev/null conflated fault with verdict. Measuring it changed the fix: peeling an absent name is itself fatal (128), not exit 1, so the peeling form cannot be distinguished from a genuine repository fault by exit code at all. Existence is now asked without ^{commit} — exit 1 + empty stderr = genuinely absent, 0 = present, anything else = a fault writing no verdict — with the commit-type assertion kept as a separate cat-file -t. Verified both ways: absent pin -> state=broken; removed object store -> no state, where the old line filed a public issue telling maintainers to re-pin a correct SHA.

Low — the WATCHED_PATHSPECS extractor read the wrong block. It matched the first textual occurrence with no block-scalar awareness. This one reproduced immediately: given an upstream fixture with the key quoted inside an earlier run: | body, the old extractor read the decoy list and failed the job claiming the decoy's path was missing from upstream. The scan is now block-scalar aware, is finished once the real block ends (so a later copy cannot reopen it — the done-never-reset case), and drops #-prefixed lines.

Low — :* skipped positive pathspecs. :(glob), :(icase), :(top), :(literal) and :/ are all positive, so an upstream rename of one degraded silently-healthy. Measurement again shaped the fix: ls-tree rejects :(glob), :(icase) and :(exclude) outright (exit 128), so probing entries verbatim would have turned every magic entry into a daily fault. Only exclusions are skipped now; positive entries have the prefix stripped before probing, wildcard entries count toward the guard without being probed, and git log still receives every entry verbatim. The probed -eq 0 guard became positive -eq 0 and no longer misdescribes the entries.

Low — the stale-pin close step used success(). That is the job's cumulative status, so a transient gh failure in the always() step above it left a stale-pin issue open on a run where the pin was demonstrably moving. It now names current and grace explicitly — under always() the old != 'delivered' would also have matched stale and broken, closing the stale-pin issue on a stale verdict.

Low — a future-dated commit made age negative and so trivially inside the grace window, an unbounded mute. Clamped to 0 with a ::warning::.

Low — owningKey walked back over raw lines while the forward scan tracked block scalars. Applied, with a correction to the finding: the literal case is not reachable through a valid workflow — a flush-left line inside run: | terminates the block scalar in YAML itself, so the file fails to parse and Actions rejects it. On that fixture old and new behave identically, so this was not a live silent skip. Kept as structural hardening; a valid two-caller fixture reports 2 callers on both old and new.

Review round 8 — cursor-review panel (42365c0)

Eight findings — four Medium, four Low — all fixed, all in the cursor-review-pin-freshness job. Two reproduced against the pre-fix program before being fixed, one claim about an external repo was verified before being acted on, and two findings were applied with a stated correction, one of which is the difference between fixing the bug and not.

Medium — the existence check misread the one pin it exists to catch. The blobless clone sets remote.origin.promisor, so a missing object makes git spawn a lazy git fetch that inherits stderr; a SHA in no upstream ref therefore arrived as exit 1 with non-empty stderr and the empty-stderr discriminator routed it to the no-verdict fault branch. Measured on git 2.43 against a real --filter=blob:none clone of github-workflows. The suggested fix was applied with a correction: GIT_NO_LAZY_FETCH=1 alone is not sufficient, because stderr is still non-empty on that path (warning: lazy fetching disabled; some objects may not be available), so keeping the empty-stderr test as a secondary signal would have left the bug exactly where it was. Disabling the fetch is what makes the exit code mean only what it says, so the code is now the whole discriminator — 1 is a verdict, any other non-zero is a fault. $pin is proven 40-hex beforehand, so the 128 a malformed name raises is only ever a real repository fault.

Medium — the awk extractor had drifted from its JS sibling in two ways, both reproducible. YAML permits a comment after a block-scalar header, so run: | # … opened no block and its body was scanned as structure — a quoted WATCHED_PATHSPECS: | decoy inside it won (old: decoy/should-not-win; new: the real list). Separately the block was keyed off the dash column, so for - run: | every sibling key was swallowed as content and a WATCHED_PATHSPECS: under a step-level env: was never seen, producing a permanent daily broken blaming upstream for a rename that never happened (old: empty; new: the real list). Both arms now use a key_col() helper that spans the - marker, which is the port of the JS sibling's group 1, and the generic opener tolerates the trailing comment.

Medium — the backlog counted commits the fleet had declined to bump for. Verified the claim against Comfy-Org/github-workflows rather than taking it on trust: .github/bump-callers/preflight.sh gates on a Skip-caller-bump: true trailer, every bump-*-callers.yml references that gate, and upstream's AGENTS.md tells authors to use it for comment/docs-only edits inside a watched surface. Counting those as owed drove the job to stale and filed a "nothing is moving it" issue while the fleet behaved exactly as designed. The walk now reads the trailer in the same git log and drops those commits, retaining any earlier untrailered owed commit. Read through git's own %(trailers:…) rather than a second copy of upstream's jq — upstream's comment says that regex is its emulation of git interpret-trailers --parse and that a second copy is the drift its directory exists to prevent. Spot-checked: a trailer in the trailing block is dropped, the same token quoted in prose is correctly not a declaration, false is kept.

Low — :^ exclusions were counted as positives. ^ is git's documented synonym for !, so a :^ entry fell to the unrecognised-short-magic arm, was counted toward the positive -eq 0 fail-closed guard and never probed — letting an all-exclusion list through with no real positive. Short magic also combines, so :/!p and :/^p are now matched as exclusions before the :/ arm, which would otherwise have probed a literal !p/^p.

Low — a degenerate probe was handed to ls-tree. Applied with a correction: only a probe still carrying magic (an unterminated :(glob, where ${path#*)} returns the entry unchanged) is a broken verdict. An empty probe is deliberately not, because :/ and :(top) are the documented spellings for the repository root — a positive pathspec that always resolves — and reporting broken there would file a false public issue against a correct upstream list. It counts toward the fail-closed guard and skips the probe, exactly as a wildcard entry does.

Low — delivered assumed delivery instead of proving it. The fleet reuses one branch across bumps, so a PR opened for an older SHA muted the entire watchdog for its full 7-day window. The PR's own caller pin is now read at its headRefOid, through the same --print-pin parser (against a scratch root, so it cannot disagree with workflow-pins about which line is the pin), and tested by ancestry rather than equality — the fleet pins to upstream's main tip, which legitimately sits ahead of any watched-surface commit, and demanding equality would call a good bump undelivered. Round 9 corrects what that ancestry is measured against: it was newest_sha, recomputed from upstream's tip on every run, which made delivered effectively unreachable (see round 9 below); it is now oldest_sha, the same commit the grace window is timed from. Any failure to read the pin leaves delivery unproven, which reads as undelivered — the watchdog stays loud rather than muting on an unreadable head — but round 9 also splits unprovable from disproven, so the two no longer share a verdict string.

Low — two captured diagnostics were interpolated unflattened. exists_err and ancestry_err were the only ones not passed through tr '\n' ' '. The findings compound: exists_err is the multi-line one precisely because of the promisor fetch above, and it relays server-controlled remote: lines, so an unflattened ::-leading line would be parsed as a workflow command rather than as text.

Review round 10 — cursor-review panel (fc0ccc3, b69ee16)

Nine findings — one Medium, seven Low, one Nit — all accepted and all fixed; none declined and none deferred to a follow-up ticket. Two of them close residues this description had previously recorded as below the ticket bar, and the panel was right about both.

Medium — the future-date clamp did not bound what it claimed to. Round 7 clamped a negative age to 0 and called that "the most generous reading that is still bounded by the grace window". It is not bounded at all: age is recomputed from the same fixed oldest_date on every run, so an oldest-unpicked commit dated a year ahead clamps to 0 on every daily run for that whole year — state=grace written each time, the stale-pin issue closed each time, the pin frozen throughout. That is precisely the unbounded silencing the clamp was added to remove. A future date now gates the grace early-exit through future_dated and is judged on the evidence below like any other stale pin; an open bump PR still mutes it, on its own separate 7-day clock. Checked at +1y (out of grace), -1h (grace) and -10d (out of grace). Ordinary upstream clock skew is seconds to minutes against a three-day window, so this cannot fire on skew.

Low — both 40-hex re-assertions anchored per LINE, not against the value. grep -Eq '^[0-9a-f]{40}$' passes a multi-line value whose second line is 40 hex. pin would then carry newlines into echo "pin=$pin" >>"$GITHUB_OUTPUT", injecting extra key=value records — a forged state= among them — into the very outputs every issue step is gated on. Both pin_out and head_pin now use [[ … =~ … ]] on the whole value. Confirmed against the pre-fix test: oops\n<40hex> and <40hex>\nstate=grace were both ACCEPTed before and are REJECTed now.

Low — upstream's pathspec list was echoed as executable workflow commands. WATCHED_PATHSPECS entries are read verbatim from a repo this one does not control, and the runner strips leading whitespace before matching a workflow command, so an entry spelled ::stop-commands::<token> or ::error::… was executed rather than logged — enough to suppress or forge this job's own verdict annotations. The same ::-at-line-start hazard the captured-stderr flattening already guarded against. A leading ::, with or without leading blanks, is now escaped to the runner's own percent-escape for a colon; a mid-line :: and a :(icase) prefix still print readably.

Low — stripping long-form pathspec magic discarded its matching semantics. :(icase)Foo/Bar was handed to ls-tree as a bare path and probed case-sensitively — the one thing that entry exists to relax — and :(attr:…) as a plain path. Either reads as missing and files a permanent daily broken sticky issue blaming upstream for a rename that never happened. Only top and literal leave the remainder naming the same fixed path (top is root-relative, which ls-tree <ref> -- <path> already is; literal only disables wildcards, which the wildcard skip drops anyway), so those are still probed. Every other keyword is counted toward the fail-closed positive guard and skipped with a log line, exactly as wildcard entries already are — the gap is visible rather than silent. An unterminated :(glob is deliberately still a broken verdict, which is how the classifier tells terminated from unterminated.

Low — fixed, predictable scratch paths. The clone target and all nine stderr capture files used hardcoded /tmp/… names while the sibling scratch root in the same step already used mktemp -d. Stated plainly: every runs-on here is ubuntu-latest, so on a fresh ephemeral VM neither half of the finding is reachable today — there is no leftover clone to trip over and nobody to pre-plant a symlink. It was worth changing anyway because the protection rested on a runner property nothing in the file states or enforces. All of them now live under one gwtmp="$(mktemp -d)" with a cleanup trap; the rewrite was scoped to the assess step, so build-check's unrelated /tmp/pack-check paths are untouched.

Low — two invisible-caller shapes the repo-wide guard cannot backstop. The zero-caller guard fires only when the whole repository yields nothing, so one caller in an unreadable shape is dropped in silence for as long as any other caller keeps the count non-zero — free to carry a branch ref or a split pin. Both shapes are now detected directly. A job-level uses: opening a block scalar is matched as a shape, its folded value read off the lines the mask marks as content, and rejected when it names this org's reusable (an unrelated uses: >- is read and left alone). And jobs.indent === 0 is gone: a YAML block mapping may legally begin at any consistent column and Actions accepts an indented root mapping, so jobs: being a root key is now asked structurally — nothing shallower owns it. This closes the owningKey residue round 7 recorded as unreachable.

Low — the owner/repo was interpolated case-sensitively. GitHub resolves owner and repository names without regard to case, so comfy-org/github-workflows/… is a working caller that the scan could not see. Only the owner and repository are relaxed, one character class per letter; the workflow path after them is a git path and stays case-sensitive, as do the ref comparisons.

Low — the Dependabot ignore is repo-wide, the freshness alarm is not. The total ignore is right for the reason its comment gives, and a second org reusable would be just as unmovable by Dependabot — so this was not fixed by narrowing it. The real gap is the asymmetry: the only staleness alarm is hardcoded to --print-pin cursor-review.yml, and the lint proves nothing beyond "these two pins agree with each other", which two frozen pins satisfy perfectly. The coupling is now enforced rather than assumed — WATCHDOG_COVERED (today: cursor-review.yml) fails the lint for any caller outside it, pointing at both the ignore block and the watchdog job. Adding a second org reusable is a red build until the watchdog is extended, rather than a silent freeze found three weeks later.

Nit — the comment SHA patterns accepted only lowercase hex. Hex commit IDs are case-insensitive, so # github-workflows main (DEADBEE) plainly names a commit but yielded no candidate, and assertion (3) was skipped without a word. Both patterns accept A-F; normalisation happens at the comparison site rather than in commentShaCandidates, so the error message still quotes the comment as written.

Review round 11 — cursor-review panel on #170 (a618f80)

The last of the five findings #170's panel raised against this PR's code. Four were already closed — the future-date clamp and the per-line 40-hex anchor by fc0ccc3, the commentShaCandidates date filter by the round-9 work that made that test a LENGTH one rather than a "contains no a-f" one. This round takes the fifth; the remaining one is declined, with the reasoning below and in Provenance rather than in a ticket.

Nit — both sticky-issue CLOSE steps fired on a cancelled run. always() is true for a CANCELLED run, not merely a successful or failed one, and a cancellation is the one outcome that is not a verdict. The gating was asymmetric in the fail-open direction: the two OPEN steps use failure() and the bump-PR note uses success(), all three false on cancellation, so nothing could ever open an issue on a cancelled run — but either close could fire on one. The window is real rather than theoretical, because this job runs under cancel-in-progress and a 10-minute timeout-minutes: a run killed after assess has written its state output but before these steps execute still carries a populated steps.assess.outputs.state, and under always() that closed the watchdog-blocked issue — or, on current/grace, the stale-pin issue — on the strength of a walk that was cut short. !cancelled() keeps the reasoning both comments already gave: success and failure both still pass it, so a stale verdict still closes the watchdog-blocked issue, which was exactly why success() was rejected there in round 7. It drops only the cancelled case.

Low — a wildcard pathspec counts toward positive but is never probed. Declined, not deferred. Upstream's WATCHED_PATHSPECS today carries exactly one wildcard entry, :(exclude)scripts/check-pr-size/*_test.go. That is an exclusion, so it never counts toward positive at all; the three positive entries are literal paths and every one of them is probed. The gap is therefore not reachable under the current upstream list. The suggested probe would not close it either: git log -1 origin/main -- "$path" answers "was anything matching this ever touched in reachable history", which stays true forever after a rename — so it would report a renamed tree as present, failing open in precisely the direction the probe exists to close. The skip is already counted toward the fail-closed guard and already logged, so the gap is visible rather than silent, and that is where it is left.

Review round 12 — cursor-review panel (0710de8, 8541d6f)

The panel's rounds 10 and 11 left 16 unresolved threads, which dedupe to 12 distinct findings — every round-11 thread re-raises a round-10 one, several marked "never answered". Ten are fixed here, two are deferred to tracked follow-ups, and one is applied while explicitly declining the mechanism it was reported under. Each claim was reproduced before being acted on.

Medium — the Skip-caller-bump trailer was read case-insensitively (fail-open). %(trailers:key=…,valueonly) matches the trailer KEY case-insensitively; verified against git 2.43, where SKIP-CALLER-BUMP: true, skip-caller-bump: true and Skip-caller-bump: true all come back as true. Upstream disagrees: .github/bump-callers/preflight.sh matches ^[Ss]kip-[Cc]aller-[Bb]ump:[[:space:]]*true[[:space:]]*$, a first-letter-only alternation, so the all-caps spelling is not honoured there and still owes a bump. A commit trailered that way vanished from behind, state=current was written over a frozen pin, and the job closed its own stale-pin issue. valueonly is what hid the spelling, so it is dropped and the whole trailer line is matched against upstream's own regex.

Medium — the bump-PR probe constrained only the head side. --head says nothing about where a PR is going, so one opened from ci/bump-cursor-review onto a release or throwaway branch set state=delivered and muted the watchdog for seven days without being able to move the caller on the default branch. Now filtered on both sides, with DEFAULT_BRANCH read from the event payload and asserted non-empty.

Medium — a vanished origin/main was classified as transient. An upstream default-branch rename is permanent and also breaks the git show, ls-tree and git log calls below, so the old no-verdict path would redden the schedule daily while filing nothing. It is now split out as a broken verdict with a sticky issue. Superseded in round 13: that branch only fired when origin/main stopped resolving, which a default-branch move need not cause. The ref is now resolved from refs/remotes/origin/HEAD, and this sub-branch is removed as unreachable.

Medium — the grace close asserted movement it never established. age comes from the oldest currently-owed commit, which drops out if upstream narrows WATCHED_PATHSPECS — re-read live every run — with the pin untouched. The false assertion is removed: the comment now states the backlog fact it has, says it is not proof the pin moved, and quotes the current pin to compare. Proving movement needs the pin recorded at filing time, which is deferred (below).

Low — the future-date gate had no skew tolerance. The comment argued "seconds to minutes against a three-day window" but the test was age < 0, so the window absorbed nothing at that edge and one second of forward skew went straight to stale. Now gated on a 15-minute SKEW_TOLERANCE_SECONDS, and future_dated becomes an output so the annotation and issue body stop saying "landed 0 days ago" while asserting the commit sat past the grace window.

Low — a non-breaking space smuggled a workflow command past the :: neutralisation. The runner trims with .NET TrimStart() (which strips U+00A0, U+202F, U+2028); glibc's POSIX [[:space:]] in a UTF-8 locale does not. Measured with an explicit \xc2\xa0 and the runner's trim simulated: the old expressions reduce the line to ::stop-commands::tok, which executes; with a non-whitespace - prefix it reduces to - \xa0::stop-commands::tok, which opens nothing. This corrects a claim round 10 made — see Deviations.

Low + Nit — three invisible-caller shapes in the lint. A plain multi-line uses: (no |/>, value simply indented beneath) matched neither USES_RE nor the block-scalar guard, and blockScalarMask does not mark its continuation either; a --- document marker above an indented root mapping was returned by owningKey as the owner of jobs:, dropping every caller in the file; and a quoted "with": key read as absent, reporting a caller that does pass workflows_ref as passing none. All three now handled, each confirmed broken against the pre-change script.

Low — the lint accepted any owner/repo case while Dependabot's ignore is literal. comfy-org/github-workflows ran fine and slipped past both ignore: entries, so Dependabot would rewrite its uses: and leave workflows_ref: behind. Taking the second option the panel offered, lint (4) now rejects a non-canonical spelling — the caller is still matched case-insensitively, since an unseen caller is the one failure this lint must not have — which makes the two canonical entries a complete defence rather than a partial one. Corrected in round 13: the premise was wrong. Dependabot matches ignore.dependency-name with Dependabot::Config::UpdateConfig.wildcard_match?, which lowercases both the pattern and the candidate, so comfy-org/github-workflows was already covered by the canonical-case entries and nothing slipped past. Lint (4) is kept as hygiene — one spelling across the caller, the ignore entries and --print-pin — and the false justification is removed from the lint comment, its error message and the dependabot.yml comment block.

Applied, but the reported mechanism is declined. Two reviewers reported printf '%s' "$log_err" | grep -q as a SIGPIPE/141 fault under pipefail. pipefail is indeed in force, but the SIGPIPE does not occur: tested on bash 5 + GNU grep with a 10 MB value and the match at offset 0, the pipeline still exits 0 and the broken branch is taken. In practice log_err is a flattened git pathspec error of a few hundred bytes against a 64 KiB buffer. Rewritten as a here-string anyway, so the broken verdict does not depend on grep buffering — but it was not a live fault, and that is recorded so the next round does not re-raise it as one.

Deferred to follow-ups (both agent-ok, both resolved on their threads with the reasoning): proving the pin moved before closing the stale-pin issue on grace (needs cross-run state — a pin marker in the issue body — plus body parsing on the close path), and checking the bump PR's uses: pin rather than only workflows_ref, since --print-pin returns the input only, so a split-pin bump PR currently proves delivery (needs a new verdict cause threaded through two message sites). Both land in a schedule-only job with no PR CI coverage, on a PR already at round 11.

Review round 13 — cursor-review panel (9d84ec9)

All nine remaining threads are fixed; none deferred. Each claim was reproduced before being acted on.

Medium — the Skip-caller-bump gate failed open on a spelling git normalises away. The test ran against %(trailers)' reconstruction of the trailer, which emits the trimmed token plus a hardcoded ": " — and git's separator search accepts whitespace before the colon. Confirmed on git 2.43: a commit trailered Skip-caller-bump : true arrives here as Skip-caller-bump: true and was skipped, while upstream's anchored regex rejects the raw line and still owes a bump — the same fail-open direction, and the same consequence, as the round-12 key-case bug. The panel's suggested fix, scanning %B per line, is declined: upstream requires the line in its trailing trailer block and says why — a body-wide search reads a merely quoted token as a declaration, so a commit documenting this feature would suppress its own bump. The test is now an AND across three passes. Git's trailer parse decides where a declaration may live; a raw %B check on the message's trailing paragraph decides whether it is spelled as upstream matches it; the owed list is rebuilt over the original input so the newest-first order the arithmetic reads positionally survives. A failed git show leaves a candidate unconfirmed — i.e. owed, the closed direction. Fixture-verified over six commit shapes: Skip-caller-bump : true → now owed, canonical → skipped, quoted-in-prose-only → owed, true (docs only) → owed, order preserved. The companion \r claim is not reachable — git strips trailing whitespace from a trailer value, so both gates already agreed; the class is switched to upstream's [[:space:]] regardless so the two spellings cannot drift.

Medium — the upstream default branch was hardcoded to origin/main. GitHub permits moving the default while keeping main, so origin/main continuing to resolve is not evidence it is the branch consumers see: every check would follow a frozen ref, and the job could write state=current and close its own stale-pin issue while consumer-visible commits piled up on the real default. It is now resolved once from refs/remotes/origin/HEAD — confirmed present on the exact --filter=blob:none --no-checkout clone this job makes — and threaded through the ancestry test, the WATCHED_PATHSPECS read, the ls-tree probes, the backlog walk and the issue prose. An unresolvable default is a broken verdict, not a guess at main.

Low — git clone was the one upstream-facing call whose status was not captured. A deleted, private or moved-without-redirect repository therefore failed identically every day on the no-verdict path: red in silence, nothing filed. Unreachable is now a broken verdict; DNS, TLS, 5xx and partial-clone faults stay on the transient path, which remains right for them.

Low — the stale-pin issue's step 1 contradicted its own diagnosis. "If one is named above, it has already delivered both pins and only the merge is owed" was emitted for every verdict naming a PR, including BUMP_DELIVERS=no, whose cause paragraph two lines up says a fresh bump is owed, and unknown, where delivery could not be verified at all. Step 1 is now built per verdict; all four renderings were checked against the parsed workflow.

Low — WORKFLOWS_REF_RE matched only the bare key while USES_RE and the with: test both tolerate a quoted one. "workflows_ref": <sha> — valid YAML that GitHub accepts and that does pass the input — therefore read as absent, failing the lint and --print-pin and turning into a broken verdict blaming a correct caller. Confirmed against the pre-fix script, which rejected such a caller with "passes no workflows_ref input".

Low — the dependabot.yml ignore was broader than the problem. A bare /* also swallowed step-level composite actions from the same repo, which carry a single pin with no workflows_ref: partner and which nothing else watches — isJobLevelUses skips step-level uses: by design and the watchdog only covers cursor-review.yml. Narrowed to .github/workflows/*, where reusable workflows must live, so no coverage of the pin pair is lost. Latent today (one caller, job-level), so fixed in place and documented rather than filed as a follow-up.

Low — the canonical-case justification was false in both files. See the corrected round-12 entry above.

Nit — the document-marker skip compared exactly, so --- # doc above an indented root mapping was returned by owningKey as the owner of jobs:, dropping every caller in the file with no output. Matched as a prefix now (/^(---|\.\.\.)(\s|$)/, which also covers --- !!map); the silent skip was reproduced against the pre-fix script.

Nit — the grace-close comment's paragraph break was a "" element inside printf '%s ', which emits one extra space rather than a blank line, running the backlog fact and the "this is not proof the pin moved" caveat together as one paragraph. Built as two paragraphs joined with %s\n; rendered output checked.

Review round 14 — cursor-review panel (65907ab)

All five remaining threads are fixed; none deferred. Each claim was reproduced before being acted on.

High — PASS 2 could never confirm a candidate, so the whole Skip-caller-bump filter was inert. --format=%B contains a %, so git reads it as tformat: and appends a terminator newline after a message body that already ends in one. awk therefore always saw a final empty record: the backward scan matched it on its first iteration, set start = n, the forward loop never executed, and every candidate fell through to exit 1. Reproduced on git 2.43 before changing anything — a correctly-trailered commit renders as four records ending in [], and the pre-fix program returned exit 1 for every shape tried. The direction is fail-closed but still wrong: each trailered commit was logged with the "upstream's own gate does not match literally" notice and counted as owing a bump, so a surface the fleet had deliberately declined to bump for would age past the grace window and file the "nothing is moving this pin" issue — the exact false alert this pass exists to prevent. Trailing blank records now come off before the block scan, which is also what the trailer-block rule means. Re-verified over ten commit shapes with the awk program extracted from the shipped YAML rather than retyped, so the test runs what ships; critically the negatives a naive "scan all of %B" fix would have broken still hold — a token that is only quoted mid-body, Skip-caller-bump : true (which upstream's anchored regex rejects), and a trailer sitting in an earlier paragraph than the trailing block are all still counted as owed.

Low — the broken reason for a missing or non-commit pin still hardcoded main. $upstream_branch is resolved from origin/HEAD and in scope, and the sibling ancestry reason immediately below already interpolated it, so this was the odd one out. Under the very default-branch rename round 13 added handling for, the sticky issue — the durable record a human acts on — told the reader to re-pin onto a branch that is no longer upstream's default, and a pin placed there fails the next run's ancestry check. This reason and the three ::error:: annotations for ancestry, the bump-workflow read and the path probes now interpolate; grep -n 'github-workflows main' is empty across the file. Also taken, though the panel did not list it: the stale-pin issue body's hand-bump step told the reader to write a trailing # github-workflows main (short) comment — the same instruction-goes-stale defect, with $UPSTREAM_BRANCH already in that step's env:, and check-workflow-pins.mjs validates only the parenthesised short SHA, not the branch word.

Low — an empty DEFAULT_BRANCH exited without writing any state=. The issue steps are gated failure() && steps.assess.outputs.state == 'broken' rather than on failure() alone, so nothing was filed; confirmed against the parsed workflow. Whether the scheduled event payload carries repository.default_branch is a fixed property of the event rather than a blip, so if it is ever empty this reddens the daily run forever in silence — which is precisely why the clone, origin/HEAD, pathspec and git log branches all write state=broken plus a broken_reason for their permanent faults. This one now does the same. The check sits inside the same assess step, so the output wiring is the one the issue step already reads.

Low — the clone-failure classifier's bare not found subsumed the explicit alternative and matched those two words anywhere in git's stderr — an HTTP reason phrase such as 404 Not Found, or an object-level message — while authentication failed and access denied come as easily from a transient token blip as from a permanent loss of access. Each of those wrote state=broken and filed a sticky issue asserting as fact that the repository had been deleted, made private or moved: fault reported as verdict, the same conflation the rest of the step removes, pointing the other way. Anchored on what GitHub really emits, established by driving a live clone of a nonexistent repo rather than from memory (remote: Repository not found. followed by fatal: repository '<url>' not found). The condition was then driven over real strings: the two genuine spellings and terminal prompts disabled classify permanent, while 404 Not Found, Could not resolve host, Authentication failed, object not found and HTTP 503 all fall to the transient no-verdict path. The permanent private-repo case is still caught — GitHub answers an unauthenticated clone of a private repo with Repository not found, not 403.

Low — the unknown bump-PR branch quoted $NEWEST_SHA, a stricter bar than the job's own test. Delivery is measured by merge-base --is-ancestor "$oldest_sha" "$head_pin", and the comment above that test rejects newest_sha at length because upstream's tip is recomputed every run, making delivery a moving target. Quoting $NEWEST_SHA in the remediation reintroduced that moving target in prose, so a reader who found the PR pinned to anything older would re-dispatch or close a PR this job would itself have counted as delivering. It now names $OLDEST_SHA ("if its pin already carries the oldest owed commit"), matching the no branch directly above.

Review round 15 — cursor-review panel (ca9b91f)

All four remaining threads are fixed; none deferred. Each claim was reproduced before being acted on.

Medium — the $GITHUB_OUTPUT flattening was newline-scoped, so a bare CR still split a record. The runner parses that file with .NET line reading, which ends a line on a lone \r as well as on \n, but every *_err capture flattened with tr '\n' ' '. A CR anywhere in another program's stderr therefore split broken_reason=... in two: the tail either forged a second key=value — a forged state= among them, the very injection the whole-value pin_out assertion was written to prevent — or carried no = at all, which makes the runner reject the whole file and drop every steps.assess.outputs.* the issue steps are gated on. Treated as systemic rather than local: the reviewer flagged the clone capture and noted pin_err had the same gap, and in fact all eight *_err captures in the job shared it; all eight now translate both terminators. The carrier was confirmed real rather than theoretical — git's sideband remote: progress emits a bare CR, verified by od -c on a captured stream, which now flattens to a single record.

Low — the same CR gap in the upstream-supplied watched entries. Both defences on that list are newline-scoped — sed anchors ^ per \n-delimited line, and the awk extractor trims only leading/trailing whitespace — so an interior CR survived into watched and slipped past the %3A%3A escape and the non-whitespace - marker, while the runner's own line splitter put the command at column 0. Reproduced with the reviewer's entry: foo<CR>::stop-commands::<token> rendered as - foo^M::stop-commands::TOKEN, and after piping through tr -d '\r' renders as - foo::stop-commands::TOKEN — one line, fully behind the marker. A plain leading :: entry still escapes to %3A%3Aevil, so the marker reasoning recorded in round 12 is undisturbed. Stripping was display-only at this round; round 16 supersedes that — the strip moved to extraction and now covers the pathspecs handed to ls-tree and git log too, for the reasons recorded there.

Low — USES_RE backtracked quadratically on a non-matching line. The path and ref were split by two nested lazy quantifiers over overlapping sets, so uses: Comfy-Org/github-workflows/ followed by a run of @ and a trailing quote gave the outer group one start position per @ and forced the inner group to grow to the quote from each. Measured before changing anything, and it is genuinely quadratic — 0.7 ms / 2.2 ms / 8.8 ms at 400 / 800 / 1600 @, ~4x per doubling. workflow-pins runs on pull_request against the merge commit, so a fork PR could have burned the job to its 5-minute timeout. Excluding @ from the path segment leaves exactly one split point per line, which is linear (flat 0.0 ms across the same range). Verified behaviour-preserving rather than merely faster: 13 realistic uses: lines — quoted and unquoted key and value, trailing comment, the #oops non-comment, mixed-case owner, the - uses: step form, and a multi-@ ref a@b@c — produce identical capture groups under both patterns, and refs containing @ still parse, since only the path group was narrowed.

Low — a dedented comment ended the continuation fold, dropping the caller in silence. In the fold for an indicator-less multi-line uses:, the dedent break was tested before the startsWith("#") skip, so a full-line comment at or below the key column terminated the fold, folded stayed empty and OWNER_REPO_RE never matched. Confirmed as a live silent-skip, not an ordering wart: a fixture with a column-0 comment mid-scalar parses under a real YAML parser to ...cursor-review.yml@main — a branch ref, exactly what this lint exists to catch — and the pre-fix script exited 0 reporting "1 caller(s) checked, all pins agree", with the repo-wide zero-caller guard held quiet by the other caller. After moving the skip above the break the same fixture exits 1. The repo's own workflows still lint clean, so no false positive was introduced.

Review round 16 — cursor-review panel (a8c02ef)

All five remaining threads are fixed; none deferred. Each claim was reproduced before being acted on.

Medium — broken_reason still embedded a CR-carrying upstream entry, re-opening the record split round 12 closed. Round 15 folded CR for display, but $path (the unparseable-magic branch) and ${missing# } (the vanished-path branch) go into $GITHUB_OUTPUT, where the fold never reached them. Fixed at the root the panel asked for rather than site-by-site: CR is now deleted from every entry once, in a loop straight after the mapfile, so no consumer ever sees one. That closes the forged-state= last-write-wins path (a state= in the record tail overriding the state=broken written a line earlier, firing the close step instead of the broken-issue step), the malformed-record path (a tail with no =, which makes the runner reject the file and drop every steps.assess.outputs.* the issue steps are gated on), and the Low below, in one change.

This deliberately changes scope: the strip is no longer display-only, so ls-tree and git log receive stripped entries. A CR is never legitimate inside a pathspec, and if upstream ever ships one the stripped path simply stops resolving — landing on the missing branch as a loud broken verdict naming the entry, which is the fail-closed direction this probe is built around. The now-redundant tr -d '\r' was removed from the display pipeline and its comment repointed. Verified the loop folds an interior CR back onto one line (foo<CR>::stop-commands::tok → foo::stop-commands::tok, fully behind the - marker) while leaving ordinary entries byte-identical.

Low — the same gap in the bare echos. not probed:, ::error::could not list ($probe) and both ::error::$reason echoes lacked the fold and the marker. Covered by the one-shot sanitisation above, which is why it was done at extraction instead of at four more call sites.

Low — gh pr list was the last upstream-facing call left to bare set -e. It exited writing no state= at all, and the issue steps are gated on the verdict rather than on failure(), so a permanent loss of pull-requests: read would redden the scheduled run identically every day while filing nothing. Its status and stderr are now captured and classified like the clone, origin/HEAD, pathspec, git log and DEFAULT_BRANCH branches.

Implementing it turned up a trap worth recording, because keying on the status alone would have made the fix worse than the bug: rate limiting also answers HTTP 403, primary and secondary alike. That arm would then swear the permission had been revoked every time this scheduled job hit a busy hour — a fault reported as a verdict, the same conflation pointing the other way. Rate-limit shapes are therefore tested first and fall through to the no-verdict path; the permanent arm is anchored on HTTP 401, bad credentials, resource not accessible by integration, scope messages and the missing-GH_TOKEN message. Exercised against the real gh strings for all eight cases: primary rate limit, secondary rate limit, integration-permission, bad credentials, missing GH_TOKEN, a scope demand, HTTP 502, DNS failure and timeout — each lands on the intended path.

Low — a duplicate workflows_ref: could pass the very check that exists to catch a split pin. workflowsRefInput returned the FIRST match, while the value GitHub actually passes is whichever its own YAML parser keeps — so a caller carrying two, the first agreeing with uses: and the second stale, linted clean. The scan now records a second occurrence and reports it as its own error (5), raised before the pair comparison since that comparison can only read one of the two. --print-pin fails on it as well, for the reason it already refuses two callers — it returns one pin and cannot choose — rather than handing the freshness watchdog a SHA that may not be in force. Verified both modes fire on a fixture carrying two workflows_ref: entries, and that the single-entry repo still lints clean and prints 425c154….

Low — the bare Comfy-Org/github-workflows Dependabot ignore entry was dead, and condemned by its neighbour's own reasoning. By the block's stated premise — Dependabot names a reusable by its FULL path — the only uses: a bare exact match can name is a repo-root composite action, which carries a single pin with no workflows_ref: partner: precisely the shape the glob beside it was narrowed to stop swallowing, on the grounds that ignoring it freezes it silently. Verified this repo has no such root-action uses: today, so the entry was dead as written; dropped, and the lead paragraph that claimed "Both entries are needed" rewritten. The tradeoff is recorded in the file: if the premise is ever wrong, the bump lands as a split pin and check-workflow-pins.mjs fails CI on the Dependabot PR — loud, on a PR nobody has merged, which beats a silent freeze nothing reports at all.

Review round 17 — cursor-review panel (673888f)

All four remaining threads are fixed; none deferred. Each claim was reproduced before being acted on.

Low (4 reviewers, re-raised twice as never-answered) — the pin lint's owning-key walks were quadratic. isJobLevelUses asks "which key owns this line" three times per caller and workflowsRefInput a fourth, and each was a fresh backward scan to a shallower line; round 16 also stopped workflowsRefInput returning at the first workflows_ref: (it has to see a second to report the duplicate), so it rescanned the whole job body per uses:. A file with N sibling job-level uses: Comfy-Org/github-workflows/... lines under one job key was therefore O(N²) with a trim()/trimStart() allocation per iteration — and workflow-pins runs on pull_request against the merge commit, so a fork PR could add such a .github/workflows/*.yml and push the job toward its 5-minute timeout: the same reachability that got the quadratic USES_RE backtracking fixed in round 15, by the same route. The owning-key chain is now built in one forward pass per file — the "previous smaller indent" stack, kept as a linked list — so a lookup is O(nesting depth), which YAML bounds at a handful of levels however long the file is; and the workflows_ref answer is memoised per job body, since it is a property of the job rather than of the uses: line that asked. Measured before changing anything and again after, on a single job with N siblings: 0.086 / 0.174 / 0.491 / 1.889 / 7.480 s at N = 500 / 1000 / 2000 / 4000 / 8000 — ~4× per doubling, genuinely quadratic — against a flat 0.052 / 0.054 / 0.065 / 0.082 / 0.105 s after. Verified behaviour-preserving rather than merely faster, which mattered more here than the speed: old and new produce byte-identical output and exit codes in both lint and --print-pin modes, over this repo and over six fixtures covering every structural case the earlier rounds fixed — with: above uses:, an indented root mapping, a --- # doc marker above one, a step-level composite action (still ignored), a flush-left run: | body quoting a uses: (still not read as structure), and a duplicate workflows_ref: (still error 5).

Low (2 reviewers) — stripping CR from the watched pathspecs reintroduced a fail-open. Round 16 moved the strip to extraction and argued it was safe because a mangled path stops resolving and lands on the missing branch as a loud broken verdict. That only holds for entries actually probed, and the probe continues before ls-tree for wildcards, for magic ls-tree cannot reproduce, and for an empty remainder — and upstream's list is glob- and :(exclude)-heavy, so those unprobed shapes are the common ones. Such an entry went straight to git log, matched nothing, and silently shrank the backlog toward state=current on a pin that is in fact frozen: the exact fail-open this job exists to prevent, reintroduced by the sanitiser. For entries that were probed the report was wrong in the other direction — the missing sticky issue quoted the stripped path and told a human upstream had renamed a path it never wrote. A CR is now its own broken verdict, decided once at extraction. That is a stronger form of the fix the panel asked for: keeping the raw entry for reporting while classifying CR separately still leaves every consumer needing its own CR reasoning, whereas rejecting the list outright needs none — a CR is never legitimate inside a pathspec, so an entry carrying one means this job can no longer trust the list it is reading, which is broken by the same standard as a list it cannot parse at all. The entry is reported with each CR rendered as the literal text <CR>, so the raw entry stays visible to whoever reads the sticky issue while the value embedded in $GITHUB_OUTPUT is guaranteed single-line and cannot split broken_reason= into a forged state= record or one carrying no =. The CRLF regression risk was checked against the real extractor — pulled out of the parsed YAML rather than retyped — because getting it wrong would turn a working job red every day: a fully CRLF upstream file yields byte-identical entries to an LF one and the check stays quiet, since awk's sub(/[[:space:]]+$/, "", entry) has already trimmed the trailing CR and only an interior CR can trigger. The reviewer's interior-CR case, a :(glob) entry that continues before ls-tree and so was precisely the silently-passed shape, now fails closed and reports as `:(glob)prompts/**<CR>::stop-commands::TOKEN` — one record, fully behind the - marker.

Low — the gh pr list permanent arm had no not-found shape. Round 16 anchored it on 401/403 spellings, but a repository the token can no longer see — made private, renamed, or access withdrawn — is not reported that way: gh answers Could not resolve to a Repository with the name '<owner>/<repo>' through GraphQL, and REST paths answer HTTP 404. Permanent and deterministic, yet matching neither alternation, so it fell through to the no-verdict exit 1 and reddened the scheduled run identically every day while filing nothing — the same fault/verdict conflation that arm was added to remove, arriving from the other end. Both shapes are added, along with Could not resolve to a User for a vanished owner, and the reason text no longer asserts permission loss as the only cause. Exercised over 13 real gh/GitHub strings, each landing on the intended arm: the three not-found shapes classify permanent; the four pre-existing permanent shapes are unchanged; primary rate limit, secondary rate limit and abuse detection still classify transient and are still tested first, which stays load-bearing because GitHub answers those with 403 as well; and HTTP 502, HTTP 503, DNS failure and a client timeout still fall to the no-verdict path.

Nit (3 reviewers, re-raised as never-answered) — the durable issue bodies hardcoded the window lengths. "3-day grace window" in three places and "past 7 days" in the delivered-note, while the enforced values are GRACE_SECONDS and BUMP_PR_GRACE_SECONDS on the assess step's env:. Tuning either knob left the sticky issue — the part that outlives the run — asserting a window the verdict was not reached under. assess now publishes grace_days and bump_pr_grace_days, derived from those same variables exactly as the grace-branch log line already did, and written before any exit so every path that can reach an issue step has them; the three steps read them off steps.assess.outputs and interpolate. Derived rather than copied into each issue step's env: deliberately: a second copy of the constant is the same staleness bug one level up, just harder to spot. Rendered at the current setting — output unchanged apart from past 7 days → past 7 day(s), which matches the $BUMP_AGE_DAYS day(s) already in that sentence — and at a hypothetical 5d/10d retune, where all three messages track the knob. The two remaining 3-day mentions are explanatory code comments about the mechanism, not emitted text, and are left as prose.

Review round 18 — cursor-review panel (dd45504)

Six threads: one Medium, four Low, one Nit. Five fixed here, the Medium already fixed in a stacked PR, none deferred.

Already fixed above this branch: --print-pin alone cannot prove a bump PR delivers (Medium). The panel is right — --print-pin returns only workflows_ref and by design never asserts it equals the PR's own uses: SHA, so a bump PR that moved just the input (precisely the split this repo shipped between #135 and #153) passed the oldest_sha ancestry test, set bump_delivers=yes and muted the watchdog for the full BUMP_PR_GRACE_SECONDS while merging it could not advance the workflow-definition pin. That is exactly what #172 fixes: it runs the script's default LINT mode over the same scratch root, and materialises every workflow file so the lint sees the file set the merged tree would. The fix is deliberately not duplicated here — #172 stacks above this branch and rewrites these same lines, so a second copy would only conflict with it.

Fixed: the sticky issues were identified by a human-editable field. Each mode now carries its own label — cursor-review-pin-stale and cursor-review-pin-blocked — and every lookup filters on that label alone. The lookups previously ANDed the shared cursor-review-pin label with exact title equality, because one shared label could not tell the two modes apart; so a maintainer who validly retitled the open issue, even just to add context, made every subsequent lookup miss — the next stale run filed a second issue and the renamed one was never closed on recovery. This is also what the block comment above these steps already claimed ("at most one issue ever carries each label"), which a single shared label could not deliver. The shared label stays on both as the grouping label a human browses by; it is no longer what finds them. Two smaller consequences fall out: the open/update paths take the lowest-numbered open match (the original thread, the one carrying the history) rather than whichever gh listed first, and the close paths close every open match, so a duplicate that somehow exists is reconciled instead of left falsely open — which the panel asked for on the same thread.

Fixed: a broken -> stale transition could leave no alert open at all. The watchdog-blocked close now also requires the stale-pin issue step (id: stale_issue) to have succeeded when the verdict is stale. The gate stays !cancelled() rather than success() on purpose — a stale verdict exits 1 and must still close the blocked alert, which is the whole reason success() was rejected — but because of that, a transient gh error in the step filing the replacement alert used to close the blocked issue anyway, leaving the run red with no sticky issue open in either mode until the next daily run re-filed one. On every other state that step is skipped, not failed, so the new clause is scoped to stale rather than asserting success unconditionally.

Fixed: only CR was rejected inside WATCHED_PATHSPECS, not other control bytes. The awk extractor trims just leading/trailing [[:space:]], so an entry carrying an interior ESC, BEL, VT or DEL survived the CR-only check — and the consequence is identical for the unprobed shapes (wildcard, unreproducible magic, empty remainder), which are the common ones in upstream's glob- and :(exclude)-heavy list: the entry skips ls-tree, goes straight to git log, matches nothing and shrinks the backlog toward state=current on a frozen pin. It was also echoed raw into the run log and embedded into broken_reason. The case now tests the [[:cntrl:]] class, and control bytes are rendered as <CR>, <TAB> or <0xNN> so the sticky issue still shows which byte upstream wrote while broken_reason stays single-line. Fails closed by construction: a locale whose [[:cntrl:]] covers more than C0+DEL can only reject more, never less.

Fixed: the published day windows were integer-divided. $((s / 86400)) truncates, so grace_days/bump_pr_grace_days only matched the enforced thresholds while both seconds constants stayed exact multiples of 86400 — at a GRACE_SECONDS of 129600 the durable issue asserted a "1-day grace window" for a verdict reached under 1.5 days, and anything under a day published a flat 0. That is a smaller version of the divergence these outputs were added to remove. Both values are text-only (nothing compares them numerically; all the arithmetic runs on the seconds constants), so they are now formatted with a trimmed fractional rendering. Chosen over the thread's other option — asserting whole days — because a mis-set knob should not take the watchdog itself offline.

Fixed (Nit): the missing space in "carriage return insideentry". Now part of the widened control-character message, which spaces before ${ctl_entries# } the way the parallel missing reason already did.

Review round 19 — cursor-review panel (56a738b)

Six threads: three Low, three Nit. Four fixed, two declined with reasoning, none deferred. The functional change is three lines; everything else is comment accuracy, which is most of what this round was about.

Fixed (Low, 2 reviewers): the reopen path revived the duplicate, not the sticky thread. The two --state closed lookups took max_by(.number) while every open/update lookup took min_by(.number), so the two sides disagreed about which issue is canonical — and the block comment above them already declares the lowest-numbered match "the original sticky thread, the one carrying the history". A mode that had ever accumulated a duplicate therefore revived the newest closed match on its next cycle and orphaned the thread the rest of the block treats as canonical. Both reopen lookups now use min_by, so picking a single issue and picking the oldest are the same decision on every path, and the rule is stated once in the block comment and referenced at both reopen sites. The same thread's second half — that the update path touches one issue while the close paths close all of them — is deliberate and now written down as intended rather than changed: commenting on one thread while a stale duplicate sits open is cosmetic, whereas leaving a duplicate open after recovery is a live false alert, so the close side is the one that must be exhaustive.

Fixed (Nit): the grace log line reintroduced the truncating division. It still derived its window with $((GRACE_SECONDS / 86400)) while the durable grace_days output goes through fmt_days, so at a non-whole-day knob the run log and the sticky issue disagreed about the same number. It now calls fmt_days "$GRACE_SECONDS". At the shipped GRACE_SECONDS=259200 both renderings are 3-day, so this is a no-op today and a fix for the mis-set case only.

Fixed (Nit): the [[:cntrl:]] comment overclaimed. It asserted the class test "cannot false-positive on a legitimate non-ASCII pathspec". Under a UTF-8 locale glibc's iswcntrl is also true for the C1 range U+0080–U+009F, which are legal bytes in a filename, so it can. The comment now states the bound exactly instead of denying it, and names the trade: a spurious broken verdict — the watchdog asking a human to look — rather than a missed stale pin, which is the direction this block is built to fail in. Every other code point above U+007F is genuinely unaffected, so that much of the old claim survives, narrowed to what is true. The rendering is documented too: <0xNN> carries the code point, not the UTF-8 bytes upstream wrote, so U+0085 renders <0x85> and not <0xC2><0x85>.

Fixed (Nit): the close loops promised more than the lookup can see. The comments said they close EVERY open match, but gh issue list --limit 100 returns only the 100 newest-created. The claim is softened to "every open match the lookup returns", with the bound and its consequence stated inline at both loops. Not looped to exhaustion: reaching truncation needs 100+ simultaneously-open issues under a single machine-written mode label, which is a far louder problem than the duplicate accumulation this loop reconciles, and the extra paging is failure surface on a recovery path that must not itself break.

Declined (Low): NUL is invisible to the control-character guard. Correct on the mechanism — $(git show ...) drops NUL before mapfile builds watched, so the case structurally cannot see it — but the byte cannot reach the guard to be caught. NUL is outside YAML's c-printable production, so an upstream bump-cursor-review-callers.yml carrying one is not a loadable YAML document: upstream's own Actions runner would fail to parse its workflow before this watchdog ever clones the repo. Hitting this needs a file simultaneously live enough to be the pin-bump workflow and malformed enough not to load. Rather than add a blob-to-disk probe and its own error-classification arm to the most-reviewed fault-handling path in this job — where every new arm is a fail-open risk of its own — the reasoning is recorded in the code beside the guard, so the next reader gets the boundary instead of re-deriving it. Not reachable under current code, so no follow-up was filed.

Declined (Low): the close loops act on a human-writable label. The finding turns on the grouping label and the closing label being the same; they are not. The close loops filter on the mode labels alone (cursor-review-pin-blocked, cursor-review-pin-stale), while the label that "invites grouping" is the shared cursor-review-pin, which nothing closes on — so the invited case, a maintainer grouping a related issue under the shared label, is untouched. Triggering this needs someone to apply a mode label, created by this workflow with the description "cursor-review pin freshness watchdog: cannot run", which reads as machine state rather than a topic tag. Both suggested mitigations also invert the failure direction: a body marker is absent from every issue already filed, so the close loops would silently skip them and leave standing exactly the false "the pin is not being watched" alerts this step exists to clear; an author filter has the same shape plus a version-dependent spelling for an App actor, and a filter that silently matches nothing closes nothing. The current blast radius is bounded and reversible — a mislabelled issue is closed with a comment naming the run, reopenable in one click — which is preferable to a filter whose failure mode is a permanently-open false alarm nobody notices. Not filed as a follow-up because the mitigation is not obviously net-positive and needs a design decision rather than blind implementation.

Provenance

• Authored by: agent-work loop
• Verified: Round 19 (56a738b) takes four of the panel's six open findings, declines two with reasoning on the threads, and defers none. The functional change is three lines; the remainder is comment accuracy. Each fix was checked against the behaviour it claims. The day-window rendering was measured before and after with the real fmt_days body: at the shipped 259200 both the old integer division and fmt_days render 3, so the log line is unchanged in production; the divergence appears only at a mis-set knob (129600 → 1 old vs 1.5 new, 43200 → 0 vs 0.5). The fmt_days call site was confirmed in scope: fmt_days is defined at the top of the same run: block that contains the grace log line, which is one step spanning both, so the call cannot resolve to an undefined function. The min_by change was confirmed exhaustive — all five single-issue lookups in the job now read min_by(.number).number // empty, with no max_by left in the file. Full local gate on 56a738b: pnpm lint, pnpm format:check, pnpm typecheck, pnpm test (26 files, 620 tests, 0 failed), pnpm check:spec-drift and pnpm check:workflow-pins (1 caller, pins agree) all pass; node scripts/check-public-repo-hygiene.mjs is clean; .github/workflows/ci.yml parses, and all 21 run: blocks in it — extracted from the parsed YAML rather than retyped, with none skipped — pass bash -n. CI status on 56a738b, read once after the push: Test (node 22), Test (node 24), workflow-pins, public-repo-hygiene, build-check, Block do-not-merge, CodeRabbit, cla-assistant and both Socket checks all pass. Checks are not fully settled: the cursor-review panel had not been dispatched on this head when this was written, so its next verdict is outstanding and nothing here claims it. The one failure is sdk-parity (python surface freshness), which is pre-existing, unrelated and not caused by this PR — it was reproduced this round on a pristine origin/main worktree with the identical ExtractionError: … unrecognized public class(es) RouterRunResult, main's own scheduled runs fail it (run 35573997014 on 4491e4b), it is tracked in #156 with a fix open in #167, and this PR changes no line of scripts/python-surface.mjs or scripts/sync-python-surface.mjs. Re-read on a later shepherd pass over the same head 56a738b (no new commits): the local gate was re-run green in a clean worktree — npm test (26 files, 620 tests, 0 failed), npm run lint, npm run typecheck, npm run format:check, and npm run check:workflow-pins (1 caller, pins agree). sdk-parity is still the only red check and still reproduces byte-identically on this branch's own tree; its fix in #167 is now APPROVED and MERGEABLE, so that check is expected to go green here once #167 lands, with no change required on this branch. origin/main merges into this branch cleanly (branch is 24 ahead, 2 behind), no merge_group run has ever been attempted for this PR, and all 118 review threads are resolved with 0 unresolved.
• Deviations: Round 19 (56a738b) changes only .github/workflows/ci.yml, entirely inside the cursor-review-pin-freshness job this PR adds; no TS/JS surface, no published API, no other workflow, scripts/check-workflow-pins.mjs and .github/dependabot.yml both untouched this round. Three things go beyond the literal threads. (1) The min_by/max_by thread also asked for one duplicate policy on both sides; that half is deliberately not taken — the close paths stay exhaustive while the open/update paths touch a single issue, because a stale duplicate left open after recovery is a live false alert whereas a duplicate carrying a stale body until recovery is cosmetic. The asymmetry is now documented as intended rather than left to be re-derived. (2) The NUL finding is answered with an in-code note instead of a probe: NUL is outside YAML's c-printable, so a file carrying one is not loadable by upstream's own Actions runner and cannot reach this job. Not reachable under current code, so no follow-up was filed. (3) The human-writable-label finding is declined rather than mitigated, because both suggested mitigations (a body marker, an author filter) fail open — silently skipping the already-filed issues they would need to close — which is the worse direction for a watchdog; it is recorded on the thread as needing a design decision rather than filed blind. Two notes for whoever lands the stack: #172 still owns the --print-pin delivery Medium and is not duplicated here, and #173 edits these same close steps and will need a rebase onto this head — ordinary stack churn, and nothing here was force-pushed. Nothing was deferred and no follow-up was filed this round. A later shepherd pass over this same head changed no code and pushed no commit; it corrected four stale factual claims in this description that round 16 had left behind when it dropped the bare Dependabot ignore entry — the "Both ignore entries are load-bearing" lead, the grep -c → 2 verification line, the grep -n "4 lines, not 2" judgment call, and the "comment block is used verbatim as supplied" judgment call — all four of which the file now contradicts. No claim of merge-readiness is made here; review approval is still outstanding.

Summary by CodeRabbit

• Chores
  • Updated automated dependency management for reusable workflow references.
  • Synchronized code review workflows with their pinned reusable workflow version.
  • Added validation to ensure workflow references use full, matching revisions.
  • Added scheduled monitoring for outdated workflow references, including grace-period handling and status tracking.
  • Added a command for manually checking workflow pin consistency.
  • No application functionality or user-facing behavior changed.

---

_{🔄 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/153 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/18/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `matt/be-15270-resync-cursor-review-pins` --- ### 📝 Commits (10+) - [`1f8b4b9`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/1f8b4b931abf0fd71580535cc383d2b8fea8501e) ci(cursor-review): re-sync both pins to 425c154 and stop Dependabot splitting them - [`a07ed16`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/a07ed16725874360f6550ec157907de705e32f66) ci(workflow-pins): assert the two cursor-review pins agree, in CI - [`38e14c9`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/38e14c993008cb56a572cb7f1663cfd4e21ef663) ci(cursor-review): daily watchdog for a stale github-workflows pin (#163) - [`41fd901`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/41fd901f9bcaaa76997a5215799ecc79a0d00079) ci(workflow-pins): read workflows_ref only as a with: input, and ignore fork bump PRs - [`6a1d0fc`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/6a1d0fcdf54d53fcbae948a2c1164a5183a6e98b) ci(cursor-review): say what actually happened when the watchdog closes its issue - [`6b02db7`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/6b02db7f631017db63122cc33596fe42f928217d) ci(workflow-pins): read workflows_ref wherever with: sits in the job - [`786762c`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/786762cc75a62e4ce4477156e446fe21895700c9) ci(cursor-review): bound the watchdog's mutes and fail its drift closed - [`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 ### 📊 Changes **7 files changed** (+5123 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `.github/dependabot.yml` (+56 -0) 📝 `.github/workflows/ci-cursor-review.yml` (+2 -2) 📝 `.github/workflows/ci.yml` (+3139 -12) 📝 `package.json` (+1 -0) ➕ `scripts/__fixtures__/ci-cursor-review.83c0c87.yml` (+75 -0) ➕ `scripts/check-workflow-pins.mjs` (+1071 -0) ➕ `scripts/check-workflow-pins.test.ts` (+779 -0) </details> ### 📄 Description ## ELI-5 Our Cursor review workflow points at an upstream repo twice — once for "which workflow file to run" and once for "which prompts and scripts that workflow should load." Those two pointers had drifted 31 commits apart, so every review this repo ran used one version's workflow driving another version's scripts. This points both at the same commit, and tells Dependabot to stop touching them — Dependabot only knows how to rewrite the first pointer, which is what pulled them apart in the first place. ## What changed **`.github/workflows/ci-cursor-review.yml`** — both pins now read `425c154ce5049324ee23ad10e106baeed4cfa31b`, the commit the rest of the org's cursor-review callers were moved to on 2026-09-12. The `uses:` line's trailing comment, which Dependabot left stale at `(ffcc3f5)` while rewriting the SHA out from under it, is corrected to `(425c154)`. **`.github/dependabot.yml`** — the `github-actions` entry gains an `ignore` block naming the upstream reusables by their per-file path (`Comfy-Org/github-workflows/.github/workflows/*`). It carried a bare `owner/repo` entry alongside it until round 16 dropped that as dead — see below. No other Dependabot setting is touched. The review rounds below added the other half — the part that notices if the pair splits again, whoever splits it: **`scripts/check-workflow-pins.mjs`** (new) + its `workflow-pins` job in `.github/workflows/ci.yml` and a `check:workflow-pins` script in `package.json` — a text-level lint asserting, per caller, that `uses:` is a full 40-hex SHA, that `workflows_ref:` equals it, and that the trailing `# … (short)` comment agrees. It also serves `--print-pin`, which the watchdog below consumes. **`cursor-review-pin-freshness` in `.github/workflows/ci.yml`** (new) — a schedule-only watchdog for the failure this PR's own history demonstrates: nobody bumps the pin at all. It reads the pin via `--print-pin`, reads upstream's own `WATCHED_PATHSPECS` list, and reports `current` / `grace` / `delivered` / `stale` / `broken`, with two sticky issues (the pin is stale; the watchdog cannot answer) that open, reopen and close themselves. ## Why The caller pins the upstream reusable **twice**: the `uses:` SHA selects the workflow definition, and the `workflows_ref:` input selects the prompts/scripts loaded at run time. The file's own comment already says to keep them matching. On `main` they did not: `uses:` sat on `ebfd9e4` while `workflows_ref:` sat on `ffcc3f5`, 31 commits and 7 differing cursor-review assets apart. Dependabot is the cause and would be the cause again. It understands `uses:` only. Verified against this repo's own history — the two bumps that produced the split, #135 and #141, each touched exactly one file and #135's patch is the whole story: ``` - uses: …/cursor-review.yml@ffcc3f5… # github-workflows main (ffcc3f5) + uses: …/cursor-review.yml@ce203b3… # github-workflows main (ffcc3f5) ``` `workflows_ref:` untouched, and the explanatory comment left pointing at the old value. **The `ignore` entry is load-bearing**, and this repo supplies its own proof rather than borrowing the sibling repo's: #135 and #141 are titled `ci: bump Comfy-Org/github-workflows/.github/workflows/cursor-review.yml from … to …`. Dependabot names the dependency by its **full path**, and a `dependency-name` without a wildcard is an exact match — so a bare `Comfy-Org/github-workflows` entry would have matched nothing and been inert. This PR originally carried one beside the globbed form as belt-and-braces; round 16 dropped it as dead, with the reasoning recorded both in the file and in that round's note below. The globbed per-file form is the one that actually covers these callers, and it is scoped to `.github/workflows/*` rather than a bare `/*` so it does not also silently freeze step-level composite actions from the same repo. Deliberately not collapsed to the single `Comfy-Org/github-workflows*`, which Dependabot expands to an unanchored `.*` and which would therefore also swallow any future sibling repo whose name merely starts with this one. ## Riskiest line, and why it is safe `uses:` moves **backwards** in upstream history (`ebfd9e4`, 2026-09-14 → `425c154`, 2026-09-12). Three checks, all read-only against the upstream repo: - The only commit between the two touches a single file — upstream's own caller at `.github/workflows/ci-cursor-review.yml`. **Zero** cursor-review assets and no change to the reusable `cursor-review.yml` itself. Everything this caller actually executes is byte-identical at both SHAs. - The reusable's `workflow_call` interface is identical at both: same 14 inputs, same `workflows_ref` as the only required one, same three secrets with `CURSOR_API_KEY` the only required one. Neither input this caller passes (`extra_generated_globs`, `workflows_ref`) nor either secret it forwards has moved, and no newly-required input appears. - After the change the workflow ref and the assets ref compare to an empty diff, which is the invariant the file's comment asks for and the property the upstream equality guard will check. ## Verification - `python3 -c "import yaml; yaml.safe_load(...)"` parses both changed YAML files. - `grep -oE '[0-9a-f]{40}' .github/workflows/ci-cursor-review.yml | sort -u` → exactly `425c154ce5049324ee23ad10e106baeed4cfa31b`. - `grep -c 'dependency-name: "Comfy-Org/github-workflows'` → `1` — the single globbed per-file entry. It read `2` until round 16 dropped the bare `owner/repo` entry as dead. - Full local gate green: `pnpm lint`, `pnpm format:check`, `pnpm typecheck`, `pnpm check:spec-drift`, `node scripts/check-public-repo-hygiene.mjs`, `pnpm test` (26 files, 620 tests passed). - Upstream reads: all three SHAs resolve; `425c154` carries `.github/workflows/cursor-review.yml`; the interface and commit-range comparisons above. Applying the `cursor-review` label to this PR exercises the changed caller end to end. ## Judgment calls - **The source request's step-2 verification says "the PR diff touches only `.github/dependabot.yml`", which contradicts its own step 1** (re-sync both pins in the workflow file) and its own closing verification (which greps the workflow file for the SHA). Read as a stale clause carried over from a Dependabot-only change; this PR does both steps in one diff as the request's own headline instruction asks. The intended reading of that clause — no *other* Dependabot setting changed — is satisfied. - **`grep -n 'Comfy-Org/github-workflows' .github/dependabot.yml` returns 7 lines, not 1.** Six of them are the `ignore` block's explanatory comment; the *entry* is exactly one. `grep -c 'dependency-name: "Comfy-Org/github-workflows'` is the check that means what was intended. (Before round 16 this read 4 lines against 2 entries.) - **The comment block started as the sibling repo's, supplied verbatim, but the review rounds have since rewritten it** — correcting its claim that `dependency-name` matching is case-sensitive, narrowing the glob to `.github/workflows/*`, and recording why the bare entry was dropped. The tradeoff below is consequently now stated in-file as well as here: an `ignore` with no `versions:`/`update-types:` qualifier suppresses Dependabot **security** updates for these reusables as well as version updates. Accepted for the same reason it is accepted upstream — these are first-party org repos pinned to reviewed SHAs, not registry packages, so an advisory-driven bump is not the update path for them. - **No `full-autonomy` label**, which the source request does not carry. ## Residual **This repo now has no automated bumper for these pins at all.** The `ignore` correctly stops Dependabot from splitting the pair, but the upstream fleet that moves both pins together only bumps repos on its `CURSOR_REVIEW_CALLERS` roster, and this repo is not on it. Falsified empirically rather than assumed: the full commit history of `.github/workflows/ci-cursor-review.yml` across all authors is three commits — the hand-authored enrolment in #111, then #135 and #141, both `dependabot[bot]`. No fleet bump PR has ever reached this repo. So after this merges the pins are correct and locked, and they will stay at `425c154` until either the roster entry is added or someone bumps them by hand. The follow-up is an operator action, outside what a repo PR can do: add `{"repo":"Comfy-Org/comfy-typescript-sdk","file":".github/workflows/ci-cursor-review.yml"}` to the `CURSOR_REVIEW_CALLERS` secret in the upstream workflows repo. Until then, whoever next needs an upstream cursor-review fix here must move **both** pins together by hand — the `uses:` SHA, the `workflows_ref:` input, and the trailing `# github-workflows main (<short>)` comment, which Dependabot has twice left stale. **Unexercised artifacts.** The originating investigation ticket and its findings comment — the source of the "4 of 72 callers" measurement and of the upstream `job.workflow_sha` equality-guard epic — are on an issue tracker this session has no access to, so that corpus figure is carried forward unverified. What was verified here is the local slice: this repo contains exactly **1** caller of the upstream reusables (the other three workflows use `pnpm/action-setup` and `contributor-assistant/github-action`, untouched by the new `ignore`), it was split, and it is now consistent — **1 of 1 in-repo, 0 remaining**. The other 3 of the 72 org-wide callers said to be split live in other repositories and are out of scope for this PR; nothing here fixes or measures them. The upstream equality guard itself is not yet in force at `425c154`, so this change is verified against the invariant it will check rather than against a red build it turns green. ## Review round — the panel's two findings (a07ed16) Both were Low, both raised by 2 of 6 reviewers. One is fixed in code, one is deferred with evidence. **Fixed: nothing enforced the "keep `workflows_ref` matching" invariant.** The panel's framing is what made this worth doing — the lines this PR replaces are themselves the proof that a comment is not a guard, since the pair sat split for eight days across two merged Dependabot PRs with nothing going red. `scripts/check-workflow-pins.mjs` is the missing half, wired in as a `workflow-pins` CI job. For every `Comfy-Org/github-workflows` caller it asserts three things about one upstream commit: `uses:` is a full 40-hex SHA (not a movable tag or branch), `workflows_ref` equals it exactly, and the trailing `# … (short)` comment agrees with it. A missing `workflows_ref` on a reusable that requires one is an error too — an omitted `workflow_call` input arrives as `''`, and `actions/checkout` with `ref: ''` silently takes the default branch, which is the same hole from the other side. Verified against the real history, not only a synthetic fixture: run on `83c0c87`'s `ci-cursor-review.yml` it reports **both** halves of the split this PR fixes — the stale `(ffcc3f5)` comment and the `ffcc3f5b…` input against an `ebfd9e44…` `uses:` — and exits 1; on this branch it exits 0. Four more cases checked: a branch-pinned `uses:`, a deleted `workflows_ref`, a stale-comment-only drift, and quoted-scalar spellings of both pins in sync (passes) and split (fails, so quoting is not a bypass). Text-level parsing is deliberate: the job runs from a bare checkout + `setup-node` with no `pnpm install`, the same shape as `public-repo-hygiene`, so it cannot import the `yaml` devDependency — the same constraint the upstream pin lint works under. The one structural limit (a `workflows_ref:` written inside a block scalar) is recorded in the script header and fails loud rather than silently passing. Upstream's own `check_workflow_pins.py` does not cover this case: it is explicitly an internal lint for the `github-workflows` repo and checks the *reusable's* side, not the *caller's* pin pair. **Deferred with evidence: the `ignore` entries suppress security updates, and the replacement bumper may not be wired up.** Two halves, handled differently. The missing `versions:`/`update-types:` qualifier is intentional and is now documented in `dependabot.yml` itself rather than only in this body. The reasoning is sharper than "accepted upstream": the problem is not *which* versions Dependabot picks, it is that it cannot move this dependency correctly at all, so a security bump would split the pair exactly like a routine one. Narrowing the ignore would buy a broken pin rather than a patched one. The enrollment half is the live risk, and it is the Residual below. It is not confirmable in-band — the roster is an Actions *secret* with no `gh secret get` — so it is **filed as a follow-up** for someone with access to the private `callers.json`, rather than guessed at here. Worth separating from the guard above: `check-workflow-pins.mjs` catches a *split* pin and makes no network call, so it cannot catch a *stale but internally consistent* one. That is a different failure and needs the schedule-driven watchdog the follow-up proposes. ## Review round 2 — CodeRabbit (41fd901) Two findings, both Major, both valid, both fixed in code. No deferrals. **The pin lint read `workflows_ref` from the wrong place.** `jobBodyAfter` returned every line in the calling job at or below the `uses:` indent, and `WORKFLOWS_REF_RE` accepted any indentation — so a `workflows_ref:` written as a *sibling* of `uses:` satisfied the pin even though GitHub never passes it to the reusable. Verified rather than assumed: SchemaStore's `reusableWorkflowCallJob` is `additionalProperties: false` over exactly `{cache-mode, concurrency, if, name, needs, permissions, secrets, strategy, uses, with}`, so such a key is not an input at all. That is the one direction this lint must never fail in — passing a caller whose real pin is missing or split. `jobBodyAfter` is replaced by `workflowsRefInput`, which tracks the `with:` block and accepts only a **direct child** of it. That closes three shapes beyond the one reported: a `workflows_ref` under `secrets:`, one inside a block scalar, and one under a nested mapping beneath another input. The previous round's "documented limit" about block scalars is therefore retired; what remains is a flow-mapping `with: { ... }`, which now reads as *absent* and so fails loudly as a missing input on a reusable in `REQUIRES_WORKFLOWS_REF` rather than passing silently. Recorded in the script header. Checked with a 14-case fixture matrix, which covers the rewrite's regression risk as well as the new behaviour: the four newly-rejected shapes fail, and every case the previous round claimed still holds — split pins, stale trailing comment, branch-pinned `uses:`, deleted `workflows_ref`, quoted scalars in sync and split — plus `with:` carrying a trailing comment and a flush-left comment inside the block. Run against `83c0c87`'s real `ci-cursor-review.yml` it still reports **both** halves of the historical split and exits 1. **The watchdog's "already delivered" probe could be muted by a fork.** `gh pr list --repo "$GH_REPO" --state open --head "$BUMP_BRANCH"` filters by branch *name*; `--repo` selects the base repository, not the head one. Any fork PR on a branch called `ci/bump-cursor-review` would have read as a delivered bump and suppressed the stale-pin issue indefinitely — the precise three-week invisibility this watchdog exists to end. Cross-repository PRs are now filtered out, with the `--repo`/`--head` asymmetry spelled out at the call site since it is not self-evident. The jq was checked against the live API, not by eye: `--json number,isCrossRepository` is accepted and the expression returns `153` for this branch, empty for a fork-only or empty result set. **Unrelated red check, for whoever merges.** `sdk-parity (python surface freshness)` is failing — and is failing identically on `main` (run for `4491e4b`), with no commit in this PR touching anything it reads. The upstream `comfy-python-sdk` added a public `RouterRunResult` class to `src/comfy_sdk/models.py`, which the extractor hard-fails on by design rather than silently skipping. It is not a required check on `main` (the required set is `Test (node 22)`, `Test (node 24)`, `build-check (publish dry run)`, `public-repo-hygiene` — all green here) and deciding whether that class belongs in the parity surface is a separate change to a separate script, so it is deliberately not bundled into a CI-pins PR. ## Self-review round (6a1d0fc) One finding, mine rather than a reviewer's, in code this PR adds. **The stale-pin issue closed with a sentence that is usually false.** The close step is gated `success() && state != 'delivered'`, which matches `current` **and** `grace`, but its comment read "the pin is current again" unconditionally. In `grace` the pin is still behind — the oldest commit it has not picked up is merely inside the 3-day window. That is the ordinary path, not an edge case. A bump lands in `grace` rather than `current` almost every time, because upstream keeps committing while the bump PR sits in review: at the 0.39-day median gap between consumer-visible commits that this job's own comment measures, `main` has usually moved again before the PR merges. So the **common** outcome was the one carrying the wrong sentence, and whoever read the closing comment would believe a pin four commits behind is level with `main`. Closing on `grace` is kept, and is right: this issue's premise is that **nothing** is moving the pin, and a bump landing inside the window falsifies exactly that. Only the wording is per-state now — the `grace` branch reports how far behind the pin actually is, names the oldest unpicked commit, and says the issue reopens if the pin freezes past the window again. `BEHIND_COUNT`/`OLDEST_SHA` are unset on the `current` branch (`assess` exits before writing them) and are read only on the `grace` branch; `env:` defines the names regardless, so `set -u` holds either way. Verified by running the step's real `run:` block — extracted from the parsed YAML rather than retyped — against a stubbed `gh` in all three reachable cases: `current` with an open issue, `grace` with an open issue, and no open issue at all (a no-op). `rc=0` each, backticks intact, no `set -u` failure on the empty variables. **One comment-only addition.** `check-workflow-pins.mjs` documents a second "reads as input absent" limit beside the flow-mapping one: a `with:` block written **above** its job's `uses:` is not seen, because the scan starts at the `uses:` line. No behaviour change. *(Superseded in round 3 below — CodeRabbit flagged this same limit and it turned out to be worth fixing rather than documenting, so the limit and this note are both gone as of `6b02db7`.)* **The unrelated red check now has a fix in flight.** `sdk-parity (python surface freshness)` is still failing here and still failing identically on `main` (the upstream Python SDK's new `RouterRunResult` class; nothing on this branch touches what it reads). It is not a required check — the required set is `Test (node 22)`, `Test (node 24)`, `build-check (publish dry run)`, `public-repo-hygiene`, all green here. #167 is open against it, which is where that fix belongs rather than bundled into a CI-pins PR. ## Review round 3 — CodeRabbit (6b02db7) One finding, Minor, valid, fixed in code. No deferrals. **The pin lint could not read a `with:` block written above its job's `uses:`.** `workflowsRefInput` scanned only the lines after `uses:`, so that caller read as passing no pin at all — `lint` would have rejected a correctly pinned caller, and `--print-pin` would have `fail()`ed and taken the daily `cursor-review-pin-freshness` watchdog down with it. This is the limit the self-review round above had just finished *documenting*, which is the interesting part: that round judged it acceptable because it reads as "input absent" and so fails loud rather than passing a split pin silently. The direction-of-error reasoning was right and the conclusion was still wrong — being loudly wrong about a spelling GitHub accepts is a false positive on a valid caller, and YAML block mappings are unordered, so key order carries no meaning to disagree with. The header had weighed this against carrying a YAML parser into a job that runs from a bare checkout with no `pnpm install`; the actual fix is a four-line backward walk, so that tradeoff was never the one on the table. `workflowsRefInput` now starts just after the job's `<job id>:` key, found by walking back from `uses:` to the nearest shallower non-blank, non-comment line. The `with:`-tracking state machine is untouched — this only widens the window it reads — so every shape round 2 newly rejected is still rejected. Checked against what the backward walk must **not** do as well as what it must, since leakage is the regression this introduces: `with:` before `uses:` passes and reports a *split* correctly when the pins disagree; the existing ordering still passes; and the invalid shapes still fail — a `workflows_ref` sibling of `uses:`, one nested deeper than the `with:` children, one belonging to a **preceding job** (the walk stops at the job's own key, so it cannot leak across jobs), and a decoy inside a block scalar above `uses:`. Real repo unchanged: lint exits 0 and `--print-pin` still prints `425c154`. Key order is now recorded in the header as explicitly *not* a limit; the flow-mapping `with: { ... }` limit stands. ## Review round 4 — cursor-review panel (786762c) Ten findings on `6b02db7`: one High, three Medium, five Low, one Nit. Nine are fixed in code, one half-declined with a reason, one self-identified follow-up filed. No deferrals of a reviewer's finding. Nine of the ten share one shape, which is worth naming because it is the shape this PR is about: a check that **degrades to silently healthy**. The original bug was two pins drifting apart for eight days with nothing going red. Round 1 added a lint and a watchdog to end that, and round 4's panel found that the watchdog and the lint each had their own ways of reporting "fine" when they in fact knew nothing. **High — `delivered` suspended the watchdog with no upper bound.** An open same-repo PR on `ci/bump-cursor-review` made the job exit 0 every day, forever, however frozen the pin was — the same silent freeze, now with an alibi. It gets a second clock of its own (`BUMP_PR_GRACE_SECONDS`, 7 days, deliberately separate from the 3-day pin grace because what is being waited on is a human merge, not a fleet delivery). Past it the verdict escalates to `stale`, which files the sticky issue and names the stuck PR as the thing to chase, in the `::error::`, the issue body and the top of its checklist. That also closes the reviewers' second point: a pin that first went stale while such a PR was open previously left a `::notice::` on a scheduled run as its only record. **Medium — the watched-pathspec list drifted OPEN.** The list mirrors upstream's `WATCHED_PATHSPECS` by hand, kept in sync by a comment. If upstream renamed a path, `git log` over it matched nothing, `behind` came back empty, and the job wrote `state=current`, reported "Pin is current" and **closed its own issue** every day regardless. The list is now one `watched` array used for both the existence check and the `git log` pathspec — so those two cannot disagree — and every entry must still resolve in the clone (`ls-tree`, trees only, no blob fetch) or the job fails closed. Deliberately not read out of `/tmp/gw`: parsing a file this repo does not own would trade a drift that now fails closed for a parse that could fail open. **Medium — `merge-base --is-ancestor` conflated a verdict with an outage.** Exit 1 (not an ancestor) and exit 2+ (unreadable object, `origin/main` renamed, partial-clone fetch failure) were both reported as "Re-pin both pins", with `2>/dev/null` discarding the only text that told them apart. Only exit 1 is a verdict now; a hard error reports `stderr` verbatim and writes **no** `state`, so no issue step fires — preserving the rule the file already stated, that a fault establishing nothing about the pin must not file an issue claiming something about it. **Medium — the sticky-issue lookups truncated.** All of them paged `gh issue list --state open --limit 100` and matched by title, so past 100 open issues the stale path would file a fresh duplicate daily and the close paths would silently stop closing. Lookup is now by a `cursor-review-pin` label (created idempotently before each use, so the first run on a fresh repo need not be the one that remembered), with the exact-title match kept on top because two titles now share that label. **Low — the close comment promised a reopen that never happened.** Fixed by making the code match the promise rather than the reverse: the stale path now looks for a closed issue and reopens it, so one thread carries the history, which is what the sticky-issue design is for. The title changed to `...is stale and nothing is moving it`, since the escalation above means a stale verdict can now coexist with an open bump PR. Verified no issue exists under either the old or the new title, so nothing is orphaned. **Low — the grace clock counted branch time, not landing time.** `%cI` without `--first-parent` let a long-lived branch merged today contribute a weeks-old "oldest unpicked commit", blowing the window on day one and filing an issue saying nothing is moving the pin when the fleet had had one day. `--first-parent` costs nothing on the common path: a merge commit is still reported when it changes a watched path, and on a squash- or rebase-merging upstream the range is identical. Only the date moves, to the one that answers the question being asked. **Low — `REQUIRES_WORKFLOWS_REF` was an opt-in allowlist, so it failed open.** Inverted to an opt-out `NO_WORKFLOWS_REF` (empty today). The reviewer's sharpest point was that the allowlist also swallowed the parser's documented limits — the flow-mapping `with:` one reads as "input absent" — so the opt-in default turned every parser gap into a silent pass on exactly the callers nobody had remembered to list. *This supersedes the round-1 wording above, which described the requirement as per-reusable.* **Low — the trailing-comment check flagged dates.** `SHORT_SHA_RE` treated every 7-40 char hex word as a claimed SHA, so `# github-workflows main (425c154) — bumped 20260919` reddened CI over `20260919`. It now reads the documented parenthesised token when one is present, and otherwise falls back to whole-word candidates with all-digit runs dropped — the fallback so that a comment written without parentheses is still checked rather than silently skipped, which would have traded a false positive for a silent miss. **Low — the scanner had no structural awareness.** Half fixed, half declined. *Fixed:* a `uses:` line inside a `run: |` registered as a real caller. Not hypothetical here — the watchdog in `ci.yml` writes issue bodies **about workflow pins** inside `run: |`, so a future edit adding a line starting `uses:` would have produced a phantom caller, made `--print-pin` see two matches, and taken the daily `cursor-review-pin-freshness` job red over a pin problem that does not exist. Block scalars are skipped, keyed off the *key's* column rather than the line's so `- run: |` does not swallow its sibling `shell:`; a quoted `"uses":` key is recognised. *Declined:* matching a step's `- uses:`. `jobs.<id>.uses` is the only place GitHub accepts a reusable-workflow call; a `- uses:` under `steps:` is a composite **action**, which pins once and has no `workflows_ref` to disagree with. Matching it would pull composite-action steps into the lint and — with the allowlist inverted to fail closed in this same round — make every one of them error demanding an input that cannot exist. The mapping-key restriction is what selects reusable calls, and that is now stated in the header rather than left implicit. **Nit — job hardening.** `workflow-pins` gets `permissions: contents: read` and `timeout-minutes: 5`; the watchdog gets `timeout-minutes: 10`, which matters more there because its token can write issues. **Follow-up filed, not deferred from a reviewer.** Three consecutive rounds have now fixed a parser defect in `check-workflow-pins.mjs`, each verified with an ad-hoc fixture matrix built in a temp directory and thrown away — so round N+1 re-verified nothing from round N by machine. A committed regression suite is proposed as its own ticket: it means deciding how vitest (currently `src/**/*.test.ts` only) should reach a `scripts/*.mjs` CLI, which is a repo-wide layout choice and does not belong in a pins PR. ## Review round 5 — cursor-review panel (04eba98) Ten findings: nine fixed, one recorded, one suggestion half-declined. Two were verified as **live regressions against `786762c`** rather than taken on description, and one finding's premise was confirmed while its predicted consequence was not. **Medium — the watched-pathspec hand-copy had already drifted, and could not catch drift by addition.** Three reviewers asked for the list to be read from upstream instead of mirrored. It now is: `git show origin/main:.github/workflows/bump-cursor-review-callers.yml` (blobs are lazily fetchable in the `--filter=blob:none` clone), parsed out of the `WATCHED_PATHSPECS: |` block scalar by indent. The case turned out to be stronger than reported — the shipped copy carried **one** of upstream's **four** `:(exclude)` entries. Measured over real upstream history from `425c154`, the shipped list counts **8** consumer-visible commits where upstream's list counts **7**; the extra is `03bae3a`, a tests-only commit under `.github/cursor-review/tests`, precisely what upstream's filter exists to exclude. So the mirror was already manufacturing false staleness on top of being blind to additions. Fails closed on an unparseable list and on an all-`:(exclude)` result; the decommission probe is kept, over positive entries only. **Medium — a pin naming no commit at all was reported as a transient fault.** The likeliest genuinely-wrong pin (typo, force-pushed away, fork-only) surfaces as `merge-base --is-ancestor` exit 128, which landed in the branch that deliberately writes no `state` — so a permanent breakage reddened the daily run with nothing sticky. `git cat-file -e "$pin^{commit}"` now routes it to `broken`. The exit-1 reason no longer names "an unknown commit", a cause it could not reach. The same treatment went to `--print-pin` failing, which previously killed the step before any `state=` was written — so an edit that hid the caller silenced the lint and the watchdog together. **Medium — the watchdog-blocked issue could never close after a stale verdict.** `stale` exits 1, so a `success()` gate skipped the close in exactly the state the job recovers into. Now `always() && state != '' && state != 'broken'`. **Medium — zero callers passed green.** Now an error: this repo does call the reusables, so an empty scan means the lint checked nothing. **Low — a step's `uses:` was matched as if it were a job's.** *Confirmed as a live regression.* `USES_RE` only excluded `- uses:`; the `- name:` / `uses:` form written throughout this repo is a plain mapping key and matched — so with `NO_WORKFLOWS_REF` inverted to fail closed in round 4, a composite-action step hard-failed the lint demanding an input a composite action cannot accept. Against `786762c` that fixture exits 1; it now exits 0. Selection is by position: a direct child of a `<job id>:` under the top-level `jobs:`. **Low — the all-digit filter did not cover the parenthesised branch.** *Confirmed as a live regression.* `# main (425c154) -- bumped (20260919)` exits 1 against `786762c` over the date; it now passes. **Declined** the other half — honouring only the first parenthesised token — because it trades this loud false positive for a silent miss of a stale token written after a fresh one, and the file's stated rule is that a silent skip is the direction it must not fail in. **Low — `ls-tree` faults read as a drift verdict.** Output and status are captured separately; non-zero is now an unattributed hard error writing no verdict. **Low — `printf | head -n 1` and SIGPIPE.** *Mechanism reproduces, consequence does not.* On bash 5.2.21 that pipeline does return **141** under `pipefail` once `$behind` passes ~5000 lines. But `set -e` does not act on a failing command substitution in a plain assignment, so the shipped block runs to completion with correct `newest`/`oldest`/`behind_count` — verified at 20000 commits (1.34 MB). This was a latent spurious exit status, not the watchdog outage described. Changed anyway to parameter expansion, since it is simpler and stops a correct backlog carrying a 141; the code comment records the measured behaviour rather than the predicted abort. **Nit — the blocked path had no reopen fallback.** Mirrored from the stale path: open → comment, closed → reopen + comment, neither → create. **Low — scheduled workflows are disabled after 60 days of repo inactivity.** Recorded in the job comment as the reviewer suggested as the minimum, not otherwise fixed: the remedy is an org-level dispatcher outside this repo, which is an ops decision rather than blind-shippable work, and the trigger condition is far from current activity levels. Deliberately not filed as a follow-up ticket for that reason. ## Review round 6 — CodeRabbit (b541197) One finding, fixed. Confirmed against current code rather than taken on description. **Major — a vanished upstream bump workflow was classified as a transient fault.** When `git show "origin/main:$bump_yaml"` failed, every cause took the no-`state` fault path. The issue steps are gated `if: failure() && steps.assess.outputs.state == 'broken'` (and `== 'stale'`) with no generic failure handler, so an upstream rename or deletion — permanent, and never self-clearing — would have burned the scheduled job red every day without ever opening the sticky issue that says why. It was also inconsistent with the neighbouring "the `WATCHED_PATHSPECS` key is gone" case, which already routes to `broken`: the same permanent breakage, caught one step later. `ls-tree` now probes the path before the branch decides. The three cases separate cleanly because the clone is `--filter=blob:none`, so trees are already local and `ls-tree` can still answer after `git show` has failed on the lazy blob fetch. Verified empirically against a scratch clone rather than assumed: | case | `ls-tree` rc | output | routed to | | --- | --- | --- | --- | | path present | 0 | non-empty | proceeds | | absent from readable tree | 0 | empty | `state=broken` → sticky issue | | unreadable ref | 128 | `fatal: Not a valid object name` | fault, no `state` (unchanged) | The third row is the one worth stating: the pre-existing transient-fault discipline is preserved, so this only reclassifies the deterministic case and adds no new way to suppress a verdict. Two deliberate deviations from the suggested diff: the code comment points at the `ls-tree` probe **below** rather than "above", since the per-pathspec probe — where the separate status/output capture discipline is spelled out — sits after this block; and the reason string says "renamed or removed" and "Re-point the extractor", a rename being the likelier cause with the same operator action either way. ## Review round 7 — cursor-review panel (d80dbd4) Nine findings — three Medium, six Low — all fixed. Every one is a way the watchdog could report a verdict it had not established, all in the fail-**open** direction. Two reproduced before being fixed, and two measurements changed the shape of the fix. **Medium — `git log` ran with no captured status.** The one git call in the step that never checked its exit code, fed pathspecs this repo does not control. Confirmed in isolation: with a malformed pathspec `git log` exits **128**, `set -e` does **not** abort a plain `var="$(cmd)"` assignment, and `behind` comes back empty — byte-identical to "nothing landed", so `state=current` was written and the close step fired on a frozen pin. Status is now captured; a pathspec git refuses to parse is permanent, so it files `broken` rather than reddening the schedule daily with no issue. **Medium — three captures folded stderr into the value.** `ls_out` and `probe_out` used emptiness as the "path is gone upstream" signal while `2>&1` merged stderr into it, so chatter on a *successful* `ls-tree` read as "still present"; `upstream_src` was parsed as YAML with git's diagnostics spliced in, which could truncate the watched list mid-block. All three now route stderr to a file, matching the `--print-pin` capture. **Medium — `cat-file -e "$pin^{commit}" 2>/dev/null` conflated fault with verdict.** Measuring it changed the fix: peeling an absent name is itself **fatal (128)**, not exit 1, so the peeling form cannot be distinguished from a genuine repository fault by exit code at all. Existence is now asked without `^{commit}` — exit 1 + empty stderr = genuinely absent, 0 = present, anything else = a fault writing no verdict — with the commit-type assertion kept as a separate `cat-file -t`. Verified both ways: absent pin -> `state=broken`; removed object store -> **no `state`**, where the old line filed a public issue telling maintainers to re-pin a correct SHA. **Low — the `WATCHED_PATHSPECS` extractor read the wrong block.** It matched the first *textual* occurrence with no block-scalar awareness. This one reproduced immediately: given an upstream fixture with the key quoted inside an earlier `run: |` body, the old extractor read the **decoy** list and failed the job claiming the decoy's path was missing from upstream. The scan is now block-scalar aware, is `finished` once the real block ends (so a later copy cannot reopen it — the `done`-never-reset case), and drops `#`-prefixed lines. **Low — `:*` skipped positive pathspecs.** `:(glob)`, `:(icase)`, `:(top)`, `:(literal)` and `:/` are all positive, so an upstream rename of one degraded silently-healthy. Measurement again shaped the fix: `ls-tree` **rejects** `:(glob)`, `:(icase)` and `:(exclude)` outright (exit 128), so probing entries verbatim would have turned every magic entry into a daily fault. Only exclusions are skipped now; positive entries have the prefix stripped before probing, wildcard entries count toward the guard without being probed, and `git log` still receives every entry verbatim. The `probed -eq 0` guard became `positive -eq 0` and no longer misdescribes the entries. **Low — the stale-pin close step used `success()`.** That is the job's cumulative status, so a transient `gh` failure in the `always()` step above it left a stale-pin issue open on a run where the pin was demonstrably moving. It now names `current` and `grace` explicitly — under `always()` the old `!= 'delivered'` would also have matched `stale` and `broken`, closing the stale-pin issue on a stale verdict. **Low — a future-dated commit made `age` negative** and so trivially inside the grace window, an unbounded mute. Clamped to 0 with a `::warning::`. **Low — `owningKey` walked back over raw lines** while the forward scan tracked block scalars. Applied, with a correction to the finding: the literal case is **not reachable through a valid workflow** — a flush-left line inside `run: |` terminates the block scalar in YAML itself, so the file fails to parse and Actions rejects it. On that fixture old and new behave identically, so this was not a live silent skip. Kept as structural hardening; a valid two-caller fixture reports 2 callers on both old and new. ## Review round 8 — cursor-review panel (42365c0) Eight findings — four Medium, four Low — all fixed, all in the `cursor-review-pin-freshness` job. Two reproduced against the pre-fix program before being fixed, one claim about an external repo was verified before being acted on, and **two findings were applied with a stated correction**, one of which is the difference between fixing the bug and not. **Medium — the existence check misread the one pin it exists to catch.** The blobless clone sets `remote.origin.promisor`, so a missing object makes git spawn a lazy `git fetch` that inherits stderr; a SHA in no upstream ref therefore arrived as exit 1 with *non-empty* stderr and the empty-stderr discriminator routed it to the no-verdict fault branch. Measured on git 2.43 against a real `--filter=blob:none` clone of `github-workflows`. The suggested fix was applied **with a correction**: `GIT_NO_LAZY_FETCH=1` alone is not sufficient, because stderr is still non-empty on that path (`warning: lazy fetching disabled; some objects may not be available`), so keeping the empty-stderr test as a secondary signal would have left the bug exactly where it was. Disabling the fetch is what makes the exit code mean only what it says, so the code is now the whole discriminator — 1 is a verdict, any other non-zero is a fault. `$pin` is proven 40-hex beforehand, so the 128 a malformed name raises is only ever a real repository fault. **Medium — the awk extractor had drifted from its JS sibling in two ways, both reproducible.** YAML permits a comment after a block-scalar header, so `run: | # …` opened no block and its body was scanned as *structure* — a quoted `WATCHED_PATHSPECS: |` decoy inside it won (old: `decoy/should-not-win`; new: the real list). Separately the block was keyed off the **dash** column, so for `- run: |` every sibling key was swallowed as content and a `WATCHED_PATHSPECS:` under a step-level `env:` was never seen, producing a permanent daily `broken` blaming upstream for a rename that never happened (old: empty; new: the real list). Both arms now use a `key_col()` helper that spans the `- ` marker, which is the port of the JS sibling's group 1, and the generic opener tolerates the trailing comment. **Medium — the backlog counted commits the fleet had declined to bump for.** Verified the claim against `Comfy-Org/github-workflows` rather than taking it on trust: `.github/bump-callers/preflight.sh` gates on a `Skip-caller-bump: true` trailer, every `bump-*-callers.yml` references that gate, and upstream's AGENTS.md tells authors to use it for comment/docs-only edits inside a watched surface. Counting those as owed drove the job to `stale` and filed a "nothing is moving it" issue while the fleet behaved exactly as designed. The walk now reads the trailer in the same `git log` and drops those commits, retaining any earlier untrailered owed commit. Read through git's **own** `%(trailers:…)` rather than a second copy of upstream's jq — upstream's comment says that regex is its emulation of `git interpret-trailers --parse` and that a second copy is the drift its directory exists to prevent. Spot-checked: a trailer in the trailing block is dropped, the same token *quoted in prose* is correctly not a declaration, `false` is kept. **Low — `:^` exclusions were counted as positives.** `^` is git's documented synonym for `!`, so a `:^` entry fell to the unrecognised-short-magic arm, was counted toward the `positive -eq 0` fail-closed guard and never probed — letting an all-exclusion list through with no real positive. Short magic also combines, so `:/!p` and `:/^p` are now matched as exclusions before the `:/` arm, which would otherwise have probed a literal `!p`/`^p`. **Low — a degenerate `probe` was handed to `ls-tree`.** Applied **with a correction**: only a probe *still carrying magic* (an unterminated `:(glob`, where `${path#*)}` returns the entry unchanged) is a `broken` verdict. An **empty** probe is deliberately not, because `:/` and `:(top)` are the documented spellings for the repository root — a positive pathspec that always resolves — and reporting `broken` there would file a false public issue against a correct upstream list. It counts toward the fail-closed guard and skips the probe, exactly as a wildcard entry does. **Low — `delivered` assumed delivery instead of proving it.** The fleet reuses one branch across bumps, so a PR opened for an older SHA muted the entire watchdog for its full 7-day window. The PR's own caller pin is now read at its `headRefOid`, through the same `--print-pin` parser (against a scratch root, so it cannot disagree with `workflow-pins` about which line is the pin), and tested by **ancestry** rather than equality — the fleet pins to upstream's main *tip*, which legitimately sits ahead of any watched-surface commit, and demanding equality would call a good bump undelivered. **Round 9 corrects what that ancestry is measured against:** it was `newest_sha`, recomputed from upstream's tip on every run, which made `delivered` effectively unreachable (see round 9 below); it is now `oldest_sha`, the same commit the grace window is timed from. Any failure to read the pin leaves delivery unproven, which reads as **undelivered** — the watchdog stays loud rather than muting on an unreadable head — but round 9 also splits *unprovable* from *disproven*, so the two no longer share a verdict string. **Low — two captured diagnostics were interpolated unflattened.** `exists_err` and `ancestry_err` were the only ones not passed through `tr '\n' ' '`. The findings compound: `exists_err` is the multi-line one precisely because of the promisor fetch above, and it relays server-controlled `remote:` lines, so an unflattened `::`-leading line would be parsed as a workflow command rather than as text. ## Review round 10 — cursor-review panel (fc0ccc3, b69ee16) Nine findings — one Medium, seven Low, one Nit — all accepted and all fixed; none declined and none deferred to a follow-up ticket. Two of them close residues this description had previously recorded as below the ticket bar, and the panel was right about both. **Medium — the future-date clamp did not bound what it claimed to.** Round 7 clamped a negative `age` to 0 and called that "the most generous reading that is still bounded by the grace window". It is not bounded at all: `age` is recomputed from the same *fixed* `oldest_date` on every run, so an oldest-unpicked commit dated a year ahead clamps to 0 on **every** daily run for that whole year — `state=grace` written each time, the stale-pin issue closed each time, the pin frozen throughout. That is precisely the unbounded silencing the clamp was added to remove. A future date now gates the grace early-exit through `future_dated` and is judged on the evidence below like any other stale pin; an open bump PR still mutes it, on its own separate 7-day clock. Checked at +1y (out of grace), -1h (grace) and -10d (out of grace). Ordinary upstream clock skew is seconds to minutes against a three-day window, so this cannot fire on skew. **Low — both 40-hex re-assertions anchored per LINE, not against the value.** `grep -Eq '^[0-9a-f]{40}$'` passes a multi-line value whose *second* line is 40 hex. `pin` would then carry newlines into `echo "pin=$pin" >>"$GITHUB_OUTPUT"`, injecting extra `key=value` records — a forged `state=` among them — into the very outputs every issue step is gated on. Both `pin_out` and `head_pin` now use `[[ … =~ … ]]` on the whole value. Confirmed against the pre-fix test: `oops\n<40hex>` and `<40hex>\nstate=grace` were both ACCEPTed before and are REJECTed now. **Low — upstream's pathspec list was echoed as executable workflow commands.** `WATCHED_PATHSPECS` entries are read verbatim from a repo this one does not control, and the runner strips leading whitespace before matching a workflow command, so an entry spelled `::stop-commands::<token>` or `::error::…` was *executed* rather than logged — enough to suppress or forge this job's own verdict annotations. The same `::`-at-line-start hazard the captured-stderr flattening already guarded against. A leading `::`, with or without leading blanks, is now escaped to the runner's own percent-escape for a colon; a mid-line `::` and a `:(icase)` prefix still print readably. **Low — stripping long-form pathspec magic discarded its matching semantics.** `:(icase)Foo/Bar` was handed to `ls-tree` as a bare path and probed case-**sensitively** — the one thing that entry exists to relax — and `:(attr:…)` as a plain path. Either reads as missing and files a permanent daily `broken` sticky issue blaming upstream for a rename that never happened. Only `top` and `literal` leave the remainder naming the same fixed path (`top` is root-relative, which `ls-tree <ref> -- <path>` already is; `literal` only disables wildcards, which the wildcard skip drops anyway), so those are still probed. Every other keyword is counted toward the fail-closed `positive` guard and skipped with a log line, exactly as wildcard entries already are — the gap is visible rather than silent. An unterminated `:(glob` is deliberately still a `broken` verdict, which is how the classifier tells terminated from unterminated. **Low — fixed, predictable scratch paths.** The clone target and all nine stderr capture files used hardcoded `/tmp/…` names while the sibling scratch root in the same step already used `mktemp -d`. Stated plainly: every `runs-on` here is `ubuntu-latest`, so on a fresh ephemeral VM neither half of the finding is reachable today — there is no leftover clone to trip over and nobody to pre-plant a symlink. It was worth changing anyway because the protection rested on a runner property nothing in the file states or enforces. All of them now live under one `gwtmp="$(mktemp -d)"` with a cleanup trap; the rewrite was scoped to the `assess` step, so `build-check`'s unrelated `/tmp/pack-check` paths are untouched. **Low — two invisible-caller shapes the repo-wide guard cannot backstop.** The zero-caller guard fires only when the *whole* repository yields nothing, so one caller in an unreadable shape is dropped in silence for as long as any other caller keeps the count non-zero — free to carry a branch ref or a split pin. Both shapes are now detected directly. A job-level `uses:` opening a **block scalar** is matched as a shape, its folded value read off the lines the mask marks as content, and rejected when it names this org's reusable (an unrelated `uses: >-` is read and left alone). And `jobs.indent === 0` is gone: a YAML block mapping may legally begin at any consistent column and Actions accepts an **indented root mapping**, so `jobs:` being a root key is now asked structurally — nothing shallower owns it. This closes the `owningKey` residue round 7 recorded as unreachable. **Low — the owner/repo was interpolated case-sensitively.** GitHub resolves owner and repository names without regard to case, so `comfy-org/github-workflows/…` is a working caller that the scan could not see. Only the owner and repository are relaxed, one character class per letter; the workflow path after them is a git path and stays case-sensitive, as do the ref comparisons. **Low — the Dependabot ignore is repo-wide, the freshness alarm is not.** The total ignore is right for the reason its comment gives, and a second org reusable would be just as unmovable by Dependabot — so this was not fixed by narrowing it. The real gap is the asymmetry: the only staleness alarm is hardcoded to `--print-pin cursor-review.yml`, and the lint proves nothing beyond "these two pins agree with each other", which two *frozen* pins satisfy perfectly. The coupling is now enforced rather than assumed — `WATCHDOG_COVERED` (today: `cursor-review.yml`) fails the lint for any caller outside it, pointing at both the ignore block and the watchdog job. Adding a second org reusable is a red build until the watchdog is extended, rather than a silent freeze found three weeks later. **Nit — the comment SHA patterns accepted only lowercase hex.** Hex commit IDs are case-insensitive, so `# github-workflows main (DEADBEE)` plainly names a commit but yielded no candidate, and assertion (3) was skipped without a word. Both patterns accept `A-F`; normalisation happens at the comparison site rather than in `commentShaCandidates`, so the error message still quotes the comment as written. ## Review round 11 — cursor-review panel on #170 (a618f80) The last of the five findings #170's panel raised against **this PR's** code. Four were already closed — the future-date clamp and the per-line 40-hex anchor by `fc0ccc3`, the `commentShaCandidates` date filter by the round-9 work that made that test a LENGTH one rather than a "contains no a-f" one. This round takes the fifth; the remaining one is declined, with the reasoning below and in Provenance rather than in a ticket. **Nit — both sticky-issue CLOSE steps fired on a cancelled run.** `always()` is true for a CANCELLED run, not merely a successful or failed one, and a cancellation is the one outcome that is not a verdict. The gating was asymmetric in the fail-open direction: the two OPEN steps use `failure()` and the bump-PR note uses `success()`, all three false on cancellation, so nothing could ever *open* an issue on a cancelled run — but either *close* could fire on one. The window is real rather than theoretical, because this job runs under `cancel-in-progress` and a 10-minute `timeout-minutes`: a run killed after `assess` has written its `state` output but before these steps execute still carries a populated `steps.assess.outputs.state`, and under `always()` that closed the watchdog-blocked issue — or, on `current`/`grace`, the stale-pin issue — on the strength of a walk that was cut short. `!cancelled()` keeps the reasoning both comments already gave: success and failure both still pass it, so a `stale` verdict still closes the watchdog-blocked issue, which was exactly why `success()` was rejected there in round 7. It drops only the cancelled case. **Low — a wildcard pathspec counts toward `positive` but is never probed. Declined, not deferred.** Upstream's `WATCHED_PATHSPECS` today carries exactly one wildcard entry, `:(exclude)scripts/check-pr-size/*_test.go`. That is an *exclusion*, so it never counts toward `positive` at all; the three positive entries are literal paths and every one of them is probed. The gap is therefore not reachable under the current upstream list. The suggested probe would not close it either: `git log -1 origin/main -- "$path"` answers "was anything matching this ever touched in reachable history", which stays true forever *after* a rename — so it would report a renamed tree as present, failing open in precisely the direction the probe exists to close. The skip is already counted toward the fail-closed guard and already logged, so the gap is visible rather than silent, and that is where it is left. ## Review round 12 — cursor-review panel (0710de8, 8541d6f) The panel's rounds 10 and 11 left **16 unresolved threads, which dedupe to 12 distinct findings** — every round-11 thread re-raises a round-10 one, several marked "never answered". Ten are fixed here, two are deferred to tracked follow-ups, and one is applied while explicitly declining the mechanism it was reported under. Each claim was reproduced before being acted on. **Medium — the `Skip-caller-bump` trailer was read case-insensitively (fail-open).** `%(trailers:key=…,valueonly)` matches the trailer KEY case-insensitively; verified against git 2.43, where `SKIP-CALLER-BUMP: true`, `skip-caller-bump: true` and `Skip-caller-bump: true` all come back as `true`. Upstream disagrees: `.github/bump-callers/preflight.sh` matches `^[Ss]kip-[Cc]aller-[Bb]ump:[[:space:]]*true[[:space:]]*$`, a first-letter-only alternation, so the all-caps spelling is not honoured there and still owes a bump. A commit trailered that way vanished from `behind`, `state=current` was written over a frozen pin, and the job closed its own stale-pin issue. `valueonly` is what hid the spelling, so it is dropped and the whole trailer line is matched against upstream's own regex. **Medium — the bump-PR probe constrained only the head side.** `--head` says nothing about where a PR is going, so one opened from `ci/bump-cursor-review` onto a release or throwaway branch set `state=delivered` and muted the watchdog for seven days without being able to move the caller on the default branch. Now filtered on both sides, with `DEFAULT_BRANCH` read from the event payload and asserted non-empty. **Medium — a vanished `origin/main` was classified as transient.** An upstream default-branch rename is permanent and also breaks the `git show`, `ls-tree` and `git log` calls below, so the old no-verdict path would redden the schedule daily while filing nothing. It is now split out as a `broken` verdict with a sticky issue. **Superseded in round 13:** that branch only fired when `origin/main` stopped resolving, which a default-branch *move* need not cause. The ref is now resolved from `refs/remotes/origin/HEAD`, and this sub-branch is removed as unreachable. **Medium — the `grace` close asserted movement it never established.** `age` comes from the oldest *currently-owed* commit, which drops out if upstream narrows `WATCHED_PATHSPECS` — re-read live every run — with the pin untouched. The false assertion is removed: the comment now states the backlog fact it has, says it is not proof the pin moved, and quotes the current pin to compare. Proving movement needs the pin recorded at filing time, which is **deferred** (below). **Low — the future-date gate had no skew tolerance.** The comment argued "seconds to minutes against a three-day window" but the test was `age < 0`, so the window absorbed nothing at that edge and one second of forward skew went straight to `stale`. Now gated on a 15-minute `SKEW_TOLERANCE_SECONDS`, and `future_dated` becomes an output so the annotation and issue body stop saying "landed 0 days ago" while asserting the commit sat past the grace window. **Low — a non-breaking space smuggled a workflow command past the `::` neutralisation.** The runner trims with .NET `TrimStart()` (which strips U+00A0, U+202F, U+2028); glibc's POSIX `[[:space:]]` in a UTF-8 locale does not. Measured with an explicit `\xc2\xa0` and the runner's trim simulated: the old expressions reduce the line to `::stop-commands::tok`, **which executes**; with a non-whitespace `- ` prefix it reduces to `- \xa0::stop-commands::tok`, which opens nothing. This corrects a claim round 10 made — see Deviations. **Low + Nit — three invisible-caller shapes in the lint.** A plain multi-line `uses:` (no `|`/`>`, value simply indented beneath) matched neither `USES_RE` nor the block-scalar guard, and `blockScalarMask` does not mark its continuation either; a `---` document marker above an indented root mapping was returned by `owningKey` as the owner of `jobs:`, dropping every caller in the file; and a quoted `"with":` key read as absent, reporting a caller that does pass `workflows_ref` as passing none. All three now handled, each confirmed broken against the pre-change script. **Low — the lint accepted any owner/repo case while Dependabot's ignore is literal.** `comfy-org/github-workflows` ran fine and slipped past both `ignore:` entries, so Dependabot would rewrite its `uses:` and leave `workflows_ref:` behind. Taking the second option the panel offered, lint (4) now rejects a non-canonical spelling — the caller is still *matched* case-insensitively, since an unseen caller is the one failure this lint must not have — which makes the two canonical entries a complete defence rather than a partial one. **Corrected in round 13:** the premise was wrong. Dependabot matches `ignore.dependency-name` with `Dependabot::Config::UpdateConfig.wildcard_match?`, which lowercases *both* the pattern and the candidate, so `comfy-org/github-workflows` was already covered by the canonical-case entries and nothing slipped past. Lint (4) is kept as hygiene — one spelling across the caller, the ignore entries and `--print-pin` — and the false justification is removed from the lint comment, its error message and the `dependabot.yml` comment block. **Applied, but the reported mechanism is declined.** Two reviewers reported `printf '%s' "$log_err" | grep -q` as a SIGPIPE/141 fault under `pipefail`. `pipefail` is indeed in force, but the SIGPIPE does not occur: tested on bash 5 + GNU grep with a 10 MB value and the match at **offset 0**, the pipeline still exits 0 and the `broken` branch is taken. In practice `log_err` is a flattened git pathspec error of a few hundred bytes against a 64 KiB buffer. Rewritten as a here-string anyway, so the `broken` verdict does not depend on grep buffering — but it was not a live fault, and that is recorded so the next round does not re-raise it as one. **Deferred to follow-ups (both `agent-ok`, both resolved on their threads with the reasoning):** proving the pin moved before closing the stale-pin issue on `grace` (needs cross-run state — a pin marker in the issue body — plus body parsing on the close path), and checking the bump PR's `uses:` pin rather than only `workflows_ref`, since `--print-pin` returns the input only, so a split-pin bump PR currently proves delivery (needs a new verdict cause threaded through two message sites). Both land in a `schedule`-only job with no PR CI coverage, on a PR already at round 11. ## Review round 13 — cursor-review panel (9d84ec9) All nine remaining threads are fixed; none deferred. Each claim was reproduced before being acted on. **Medium — the `Skip-caller-bump` gate failed open on a spelling git normalises away.** The test ran against `%(trailers)`' *reconstruction* of the trailer, which emits the trimmed token plus a hardcoded `": "` — and git's separator search accepts whitespace before the colon. Confirmed on git 2.43: a commit trailered `Skip-caller-bump : true` arrives here as `Skip-caller-bump: true` and was skipped, while upstream's anchored regex rejects the raw line and still owes a bump — the same fail-open direction, and the same consequence, as the round-12 key-case bug. The panel's suggested fix, scanning `%B` per line, is **declined**: upstream requires the line *in its trailing trailer block* and says why — a body-wide search reads a merely *quoted* token as a declaration, so a commit documenting this feature would suppress its own bump. The test is now an AND across three passes. Git's trailer parse decides **where** a declaration may live; a raw `%B` check on the message's trailing paragraph decides whether it is **spelled** as upstream matches it; the owed list is rebuilt over the original input so the newest-first order the arithmetic reads positionally survives. A failed `git show` leaves a candidate unconfirmed — i.e. owed, the closed direction. Fixture-verified over six commit shapes: `Skip-caller-bump : true` → now owed, canonical → skipped, quoted-in-prose-only → owed, `true (docs only)` → owed, order preserved. The companion `\r` claim is **not reachable** — git strips trailing whitespace from a trailer value, so both gates already agreed; the class is switched to upstream's `[[:space:]]` regardless so the two spellings cannot drift. **Medium — the upstream default branch was hardcoded to `origin/main`.** GitHub permits moving the default while *keeping* `main`, so `origin/main` continuing to resolve is not evidence it is the branch consumers see: every check would follow a frozen ref, and the job could write `state=current` and close its own stale-pin issue while consumer-visible commits piled up on the real default. It is now resolved once from `refs/remotes/origin/HEAD` — confirmed present on the exact `--filter=blob:none --no-checkout` clone this job makes — and threaded through the ancestry test, the `WATCHED_PATHSPECS` read, the `ls-tree` probes, the backlog walk and the issue prose. An unresolvable default is a `broken` verdict, not a guess at `main`. **Low — `git clone` was the one upstream-facing call whose status was not captured.** A deleted, private or moved-without-redirect repository therefore failed identically every day on the no-verdict path: red in silence, nothing filed. Unreachable is now a `broken` verdict; DNS, TLS, 5xx and partial-clone faults stay on the transient path, which remains right for them. **Low — the stale-pin issue's step 1 contradicted its own diagnosis.** "If one is named above, it has already delivered both pins and only the merge is owed" was emitted for every verdict naming a PR, including `BUMP_DELIVERS=no`, whose cause paragraph two lines up says a fresh bump is owed, and `unknown`, where delivery could not be verified at all. Step 1 is now built per verdict; all four renderings were checked against the parsed workflow. **Low — `WORKFLOWS_REF_RE` matched only the bare key** while `USES_RE` and the `with:` test both tolerate a quoted one. `"workflows_ref": <sha>` — valid YAML that GitHub accepts and that does pass the input — therefore read as *absent*, failing the lint and `--print-pin` and turning into a `broken` verdict blaming a correct caller. Confirmed against the pre-fix script, which rejected such a caller with "passes no `workflows_ref` input". **Low — the `dependabot.yml` ignore was broader than the problem.** A bare `/*` also swallowed step-level composite actions from the same repo, which carry a single pin with no `workflows_ref:` partner and which nothing else watches — `isJobLevelUses` skips step-level `uses:` by design and the watchdog only covers `cursor-review.yml`. Narrowed to `.github/workflows/*`, where reusable workflows must live, so no coverage of the pin pair is lost. Latent today (one caller, job-level), so fixed in place and documented rather than filed as a follow-up. **Low — the canonical-case justification was false in both files.** See the corrected round-12 entry above. **Nit — the document-marker skip compared exactly,** so `--- # doc` above an indented root mapping was returned by `owningKey` as the owner of `jobs:`, dropping every caller in the file with no output. Matched as a prefix now (`/^(---|\.\.\.)(\s|$)/`, which also covers `--- !!map`); the silent skip was reproduced against the pre-fix script. **Nit — the grace-close comment's paragraph break was a `""` element inside `printf '%s '`,** which emits one extra space rather than a blank line, running the backlog fact and the "this is not proof the pin moved" caveat together as one paragraph. Built as two paragraphs joined with `%s\n`; rendered output checked. ## Review round 14 — cursor-review panel (65907ab) All five remaining threads are fixed; none deferred. Each claim was reproduced before being acted on. **High — PASS 2 could never confirm a candidate, so the whole `Skip-caller-bump` filter was inert.** `--format=%B` contains a `%`, so git reads it as `tformat:` and appends a *terminator* newline after a message body that already ends in one. awk therefore always saw a final empty record: the backward scan matched it on its first iteration, set `start = n`, the forward loop never executed, and every candidate fell through to `exit 1`. Reproduced on git 2.43 before changing anything — a correctly-trailered commit renders as four records ending in `[]`, and the pre-fix program returned exit 1 for *every* shape tried. The direction is fail-*closed* but still wrong: each trailered commit was logged with the "upstream's own gate does not match literally" notice and counted as owing a bump, so a surface the fleet had deliberately declined to bump for would age past the grace window and file the "nothing is moving this pin" issue — the exact false alert this pass exists to prevent. Trailing blank records now come off before the block scan, which is also what the trailer-block rule means. Re-verified over ten commit shapes with the awk program extracted from the shipped YAML rather than retyped, so the test runs what ships; critically the negatives a naive "scan all of `%B`" fix would have broken still hold — a token that is only *quoted* mid-body, `Skip-caller-bump : true` (which upstream's anchored regex rejects), and a trailer sitting in an earlier paragraph than the trailing block are all still counted as owed. **Low — the `broken` reason for a missing or non-commit pin still hardcoded `main`.** `$upstream_branch` is resolved from `origin/HEAD` and in scope, and the sibling ancestry reason immediately below already interpolated it, so this was the odd one out. Under the very default-branch rename round 13 added handling for, the sticky issue — the durable record a human acts on — told the reader to re-pin onto a branch that is no longer upstream's default, and a pin placed there fails the next run's ancestry check. This reason and the three `::error::` annotations for ancestry, the bump-workflow read and the path probes now interpolate; `grep -n 'github-workflows main'` is empty across the file. Also taken, though the panel did not list it: the stale-pin issue body's hand-bump step told the reader to write a trailing `# github-workflows main (short)` comment — the same instruction-goes-stale defect, with `$UPSTREAM_BRANCH` already in that step's `env:`, and `check-workflow-pins.mjs` validates only the parenthesised short SHA, not the branch word. **Low — an empty `DEFAULT_BRANCH` exited without writing any `state=`.** The issue steps are gated `failure() && steps.assess.outputs.state == 'broken'` rather than on `failure()` alone, so nothing was filed; confirmed against the parsed workflow. Whether the scheduled event payload carries `repository.default_branch` is a fixed property of the event rather than a blip, so if it is ever empty this reddens the daily run forever in silence — which is precisely why the clone, `origin/HEAD`, pathspec and `git log` branches all write `state=broken` plus a `broken_reason` for their permanent faults. This one now does the same. The check sits inside the same `assess` step, so the output wiring is the one the issue step already reads. **Low — the clone-failure classifier's bare `not found` subsumed the explicit alternative** and matched those two words anywhere in git's stderr — an HTTP reason phrase such as `404 Not Found`, or an object-level message — while `authentication failed` and `access denied` come as easily from a transient token blip as from a permanent loss of access. Each of those wrote `state=broken` and filed a sticky issue asserting *as fact* that the repository had been deleted, made private or moved: fault reported as verdict, the same conflation the rest of the step removes, pointing the other way. Anchored on what GitHub really emits, established by driving a live clone of a nonexistent repo rather than from memory (`remote: Repository not found.` followed by `fatal: repository '<url>' not found`). The condition was then driven over real strings: the two genuine spellings and `terminal prompts disabled` classify permanent, while `404 Not Found`, `Could not resolve host`, `Authentication failed`, `object not found` and `HTTP 503` all fall to the transient no-verdict path. The permanent private-repo case is still caught — GitHub answers an unauthenticated clone of a private repo with `Repository not found`, not 403. **Low — the `unknown` bump-PR branch quoted `$NEWEST_SHA`, a stricter bar than the job's own test.** Delivery is measured by `merge-base --is-ancestor "$oldest_sha" "$head_pin"`, and the comment above that test rejects `newest_sha` at length because upstream's tip is recomputed every run, making delivery a moving target. Quoting `$NEWEST_SHA` in the remediation reintroduced that moving target in prose, so a reader who found the PR pinned to anything older would re-dispatch or close a PR this job would itself have counted as delivering. It now names `$OLDEST_SHA` ("if its pin already carries the oldest owed commit"), matching the `no` branch directly above. ## Review round 15 — cursor-review panel (ca9b91f) All four remaining threads are fixed; none deferred. Each claim was reproduced before being acted on. **Medium — the `$GITHUB_OUTPUT` flattening was newline-scoped, so a bare CR still split a record.** The runner parses that file with .NET line reading, which ends a line on a lone `\r` as well as on `\n`, but every `*_err` capture flattened with `tr '\n' ' '`. A CR anywhere in another program's stderr therefore split `broken_reason=...` in two: the tail either forged a second `key=value` — a forged `state=` among them, the very injection the whole-value `pin_out` assertion was written to prevent — or carried no `=` at all, which makes the runner reject the whole file and drop every `steps.assess.outputs.*` the issue steps are gated on. Treated as systemic rather than local: the reviewer flagged the clone capture and noted `pin_err` had the same gap, and in fact **all eight** `*_err` captures in the job shared it; all eight now translate both terminators. The carrier was confirmed real rather than theoretical — git's sideband `remote:` progress emits a bare CR, verified by `od -c` on a captured stream, which now flattens to a single record. **Low — the same CR gap in the upstream-supplied `watched` entries.** Both defences on that list are newline-scoped — `sed` anchors `^` per `\n`-delimited line, and the awk extractor trims only leading/trailing whitespace — so an interior CR survived into `watched` and slipped past the `%3A%3A` escape *and* the non-whitespace ` - ` marker, while the runner's own line splitter put the command at column 0. Reproduced with the reviewer's entry: `foo<CR>::stop-commands::<token>` rendered as ` - foo^M::stop-commands::TOKEN`, and after piping through `tr -d '\r'` renders as ` - foo::stop-commands::TOKEN` — one line, fully behind the marker. A plain leading `::` entry still escapes to `%3A%3Aevil`, so the marker reasoning recorded in round 12 is undisturbed. Stripping was display-only at this round; **round 16 supersedes that** — the strip moved to extraction and now covers the pathspecs handed to `ls-tree` and `git log` too, for the reasons recorded there. **Low — `USES_RE` backtracked quadratically on a non-matching line.** The path and ref were split by two nested lazy quantifiers over overlapping sets, so `uses: Comfy-Org/github-workflows/` followed by a run of `@` and a trailing quote gave the outer group one start position per `@` and forced the inner group to grow to the quote from each. Measured before changing anything, and it is genuinely quadratic — 0.7 ms / 2.2 ms / 8.8 ms at 400 / 800 / 1600 `@`, ~4x per doubling. `workflow-pins` runs on `pull_request` against the merge commit, so a fork PR could have burned the job to its 5-minute timeout. Excluding `@` from the path segment leaves exactly one split point per line, which is linear (flat 0.0 ms across the same range). Verified behaviour-preserving rather than merely faster: 13 realistic `uses:` lines — quoted and unquoted key and value, trailing comment, the `#oops` non-comment, mixed-case owner, the `- uses:` step form, and a multi-`@` ref `a@b@c` — produce identical capture groups under both patterns, and refs containing `@` still parse, since only the path group was narrowed. **Low — a dedented comment ended the continuation fold, dropping the caller in silence.** In the fold for an indicator-less multi-line `uses:`, the dedent `break` was tested before the `startsWith("#")` skip, so a full-line comment at or below the key column terminated the fold, `folded` stayed empty and `OWNER_REPO_RE` never matched. Confirmed as a live silent-skip, not an ordering wart: a fixture with a column-0 comment mid-scalar parses under a real YAML parser to `...cursor-review.yml@main` — a **branch ref**, exactly what this lint exists to catch — and the pre-fix script exited 0 reporting "1 caller(s) checked, all pins agree", with the repo-wide zero-caller guard held quiet by the other caller. After moving the skip above the break the same fixture exits 1. The repo's own workflows still lint clean, so no false positive was introduced. ## Review round 16 — cursor-review panel (a8c02ef) All five remaining threads are fixed; none deferred. Each claim was reproduced before being acted on. **Medium — `broken_reason` still embedded a CR-carrying upstream entry, re-opening the record split round 12 closed.** Round 15 folded CR for *display*, but `$path` (the unparseable-magic branch) and `${missing# }` (the vanished-path branch) go into `$GITHUB_OUTPUT`, where the fold never reached them. Fixed at the root the panel asked for rather than site-by-site: CR is now deleted from every entry **once, in a loop straight after the `mapfile`**, so no consumer ever sees one. That closes the forged-`state=` last-write-wins path (a `state=` in the record tail overriding the `state=broken` written a line earlier, firing the close step instead of the broken-issue step), the malformed-record path (a tail with no `=`, which makes the runner reject the file and drop every `steps.assess.outputs.*` the issue steps are gated on), **and** the Low below, in one change. This deliberately changes scope: the strip is no longer display-only, so `ls-tree` and `git log` receive stripped entries. A CR is never legitimate inside a pathspec, and if upstream ever ships one the stripped path simply stops resolving — landing on the `missing` branch as a loud `broken` verdict naming the entry, which is the fail-closed direction this probe is built around. The now-redundant `tr -d '\r'` was removed from the display pipeline and its comment repointed. Verified the loop folds an interior CR back onto one line (`foo<CR>::stop-commands::tok` → `foo::stop-commands::tok`, fully behind the ` - ` marker) while leaving ordinary entries byte-identical. **Low — the same gap in the bare `echo`s.** `not probed:`, `::error::could not list` (`$probe`) and both `::error::$reason` echoes lacked the fold and the marker. Covered by the one-shot sanitisation above, which is why it was done at extraction instead of at four more call sites. **Low — `gh pr list` was the last upstream-facing call left to bare `set -e`.** It exited writing no `state=` at all, and the issue steps are gated on the verdict rather than on `failure()`, so a *permanent* loss of `pull-requests: read` would redden the scheduled run identically every day while filing nothing. Its status and stderr are now captured and classified like the clone, `origin/HEAD`, pathspec, `git log` and `DEFAULT_BRANCH` branches. Implementing it turned up a trap worth recording, because keying on the status alone would have made the fix worse than the bug: **rate limiting also answers HTTP 403**, primary and secondary alike. That arm would then swear the permission had been revoked every time this scheduled job hit a busy hour — a fault reported as a verdict, the same conflation pointing the other way. Rate-limit shapes are therefore tested **first** and fall through to the no-verdict path; the permanent arm is anchored on `HTTP 401`, `bad credentials`, `resource not accessible by integration`, scope messages and the missing-`GH_TOKEN` message. Exercised against the real `gh` strings for all eight cases: primary rate limit, secondary rate limit, integration-permission, bad credentials, missing `GH_TOKEN`, a scope demand, HTTP 502, DNS failure and timeout — each lands on the intended path. **Low — a duplicate `workflows_ref:` could pass the very check that exists to catch a split pin.** `workflowsRefInput` returned the FIRST match, while the value GitHub actually passes is whichever its own YAML parser keeps — so a caller carrying two, the first agreeing with `uses:` and the second stale, linted clean. The scan now records a second occurrence and reports it as its own error (5), raised before the pair comparison since that comparison can only read one of the two. `--print-pin` fails on it as well, for the reason it already refuses two callers — it returns one pin and cannot choose — rather than handing the freshness watchdog a SHA that may not be in force. Verified both modes fire on a fixture carrying two `workflows_ref:` entries, and that the single-entry repo still lints clean and prints `425c154…`. **Low — the bare `Comfy-Org/github-workflows` Dependabot ignore entry was dead, and condemned by its neighbour's own reasoning.** By the block's stated premise — Dependabot names a reusable by its FULL path — the only `uses:` a bare exact match can name is a repo-**root** composite action, which carries a single pin with no `workflows_ref:` partner: precisely the shape the glob beside it was narrowed to stop swallowing, on the grounds that ignoring it freezes it silently. Verified this repo has no such root-action `uses:` today, so the entry was dead as written; dropped, and the lead paragraph that claimed "Both entries are needed" rewritten. The tradeoff is recorded in the file: if the premise is ever wrong, the bump lands as a split pin and `check-workflow-pins.mjs` fails CI on the Dependabot PR — loud, on a PR nobody has merged, which beats a silent freeze nothing reports at all. ## Review round 17 — cursor-review panel (673888f) All four remaining threads are fixed; none deferred. Each claim was reproduced before being acted on. **Low (4 reviewers, re-raised twice as never-answered) — the pin lint's owning-key walks were quadratic.** `isJobLevelUses` asks "which key owns this line" three times per caller and `workflowsRefInput` a fourth, and each was a fresh backward scan to a shallower line; round 16 also stopped `workflowsRefInput` returning at the first `workflows_ref:` (it has to see a second to report the duplicate), so it rescanned the whole job body per `uses:`. A file with N sibling job-level `uses: Comfy-Org/github-workflows/...` lines under one job key was therefore O(N²) with a `trim()`/`trimStart()` allocation per iteration — and `workflow-pins` runs on `pull_request` against the **merge commit**, so a fork PR could add such a `.github/workflows/*.yml` and push the job toward its 5-minute timeout: the same reachability that got the quadratic `USES_RE` backtracking fixed in round 15, by the same route. The owning-key chain is now built in **one forward pass per file** — the "previous smaller indent" stack, kept as a linked list — so a lookup is O(nesting depth), which YAML bounds at a handful of levels however long the file is; and the `workflows_ref` answer is **memoised per job body**, since it is a property of the job rather than of the `uses:` line that asked. Measured before changing anything and again after, on a single job with N siblings: **0.086 / 0.174 / 0.491 / 1.889 / 7.480 s** at N = 500 / 1000 / 2000 / 4000 / 8000 — ~4× per doubling, genuinely quadratic — against a flat **0.052 / 0.054 / 0.065 / 0.082 / 0.105 s** after. Verified **behaviour-preserving** rather than merely faster, which mattered more here than the speed: old and new produce byte-identical output and exit codes in both `lint` and `--print-pin` modes, over this repo and over six fixtures covering every structural case the earlier rounds fixed — `with:` above `uses:`, an indented root mapping, a `--- # doc` marker above one, a step-level composite action (still ignored), a flush-left `run: |` body quoting a `uses:` (still not read as structure), and a duplicate `workflows_ref:` (still error 5). **Low (2 reviewers) — stripping CR from the watched pathspecs reintroduced a fail-open.** Round 16 moved the strip to extraction and argued it was safe because a mangled path stops resolving and lands on the `missing` branch as a loud `broken` verdict. That only holds for entries actually **probed**, and the probe `continue`s before `ls-tree` for wildcards, for magic `ls-tree` cannot reproduce, and for an empty remainder — and upstream's list is glob- and `:(exclude)`-heavy, so those unprobed shapes are the common ones. Such an entry went straight to `git log`, matched nothing, and silently shrank the backlog toward `state=current` on a pin that is in fact frozen: the exact fail-open this job exists to prevent, reintroduced by the sanitiser. For entries that *were* probed the report was wrong in the other direction — the `missing` sticky issue quoted the **stripped** path and told a human upstream had renamed a path it never wrote. A CR is now its **own `broken` verdict**, decided once at extraction. That is a stronger form of the fix the panel asked for: keeping the raw entry for reporting while classifying CR separately still leaves every consumer needing its own CR reasoning, whereas rejecting the list outright needs none — a CR is never legitimate inside a pathspec, so an entry carrying one means this job can no longer trust the list it is reading, which is `broken` by the same standard as a list it cannot parse at all. The entry is reported with each CR rendered as the literal text `<CR>`, so the raw entry stays visible to whoever reads the sticky issue while the value embedded in `$GITHUB_OUTPUT` is guaranteed single-line and cannot split `broken_reason=` into a forged `state=` record or one carrying no `=`. **The CRLF regression risk was checked against the real extractor** — pulled out of the parsed YAML rather than retyped — because getting it wrong would turn a working job red every day: a fully CRLF upstream file yields byte-identical entries to an LF one and the check stays **quiet**, since awk's `sub(/[[:space:]]+$/, "", entry)` has already trimmed the trailing CR and only an *interior* CR can trigger. The reviewer's interior-CR case, a `:(glob)` entry that `continue`s before `ls-tree` and so was precisely the silently-passed shape, now fails closed and reports as `` `:(glob)prompts/**<CR>::stop-commands::TOKEN` `` — one record, fully behind the ` - ` marker. **Low — the `gh pr list` permanent arm had no not-found shape.** Round 16 anchored it on 401/403 spellings, but a repository the token can no longer see — made private, renamed, or access withdrawn — is not reported that way: `gh` answers `Could not resolve to a Repository with the name '<owner>/<repo>'` through GraphQL, and REST paths answer `HTTP 404`. Permanent and deterministic, yet matching neither alternation, so it fell through to the no-verdict `exit 1` and reddened the scheduled run identically every day while filing nothing — the same fault/verdict conflation that arm was added to remove, arriving from the other end. Both shapes are added, along with `Could not resolve to a User` for a vanished owner, and the reason text no longer asserts permission loss as the only cause. Exercised over 13 real `gh`/GitHub strings, each landing on the intended arm: the three not-found shapes classify permanent; the four pre-existing permanent shapes are unchanged; primary rate limit, secondary rate limit and abuse detection still classify transient and are still tested **first**, which stays load-bearing because GitHub answers those with 403 as well; and HTTP 502, HTTP 503, DNS failure and a client timeout still fall to the no-verdict path. **Nit (3 reviewers, re-raised as never-answered) — the durable issue bodies hardcoded the window lengths.** "3-day grace window" in three places and "past 7 days" in the delivered-note, while the enforced values are `GRACE_SECONDS` and `BUMP_PR_GRACE_SECONDS` on the `assess` step's `env:`. Tuning either knob left the sticky issue — the part that outlives the run — asserting a window the verdict was not reached under. `assess` now publishes `grace_days` and `bump_pr_grace_days`, derived from those same variables exactly as the `grace`-branch log line already did, and written **before any `exit`** so every path that can reach an issue step has them; the three steps read them off `steps.assess.outputs` and interpolate. Derived rather than copied into each issue step's `env:` deliberately: a second copy of the constant is the same staleness bug one level up, just harder to spot. Rendered at the current setting — output unchanged apart from `past 7 days` → `past 7 day(s)`, which matches the `$BUMP_AGE_DAYS day(s)` already in that sentence — and at a hypothetical 5d/10d retune, where all three messages track the knob. The two remaining `3-day` mentions are explanatory code comments about the mechanism, not emitted text, and are left as prose. ## Review round 18 — cursor-review panel (dd45504) Six threads: one Medium, four Low, one Nit. **Five fixed here, the Medium already fixed in a stacked PR, none deferred.** **Already fixed above this branch: `--print-pin` alone cannot prove a bump PR delivers (Medium).** The panel is right — `--print-pin` returns only `workflows_ref` and by design never asserts it equals the PR's own `uses:` SHA, so a bump PR that moved just the input (precisely the split this repo shipped between #135 and #153) passed the `oldest_sha` ancestry test, set `bump_delivers=yes` and muted the watchdog for the full `BUMP_PR_GRACE_SECONDS` while merging it could not advance the workflow-definition pin. That is exactly what **#172** fixes: it runs the script's default LINT mode over the same scratch root, and materialises *every* workflow file so the lint sees the file set the merged tree would. The fix is deliberately **not duplicated here** — #172 stacks above this branch and rewrites these same lines, so a second copy would only conflict with it. **Fixed: the sticky issues were identified by a human-editable field.** Each mode now carries its **own** label — `cursor-review-pin-stale` and `cursor-review-pin-blocked` — and every lookup filters on that label alone. The lookups previously ANDed the shared `cursor-review-pin` label with exact title equality, because one shared label could not tell the two modes apart; so a maintainer who validly retitled the open issue, even just to add context, made every subsequent lookup miss — the next stale run filed a second issue and the renamed one was never closed on recovery. This is also what the block comment above these steps already *claimed* ("at most one issue ever carries each label"), which a single shared label could not deliver. The shared label stays on both as the grouping label a human browses by; it is no longer what finds them. Two smaller consequences fall out: the open/update paths take the **lowest-numbered** open match (the original thread, the one carrying the history) rather than whichever `gh` listed first, and the close paths close **every** open match, so a duplicate that somehow exists is reconciled instead of left falsely open — which the panel asked for on the same thread. **Fixed: a `broken -> stale` transition could leave no alert open at all.** The watchdog-blocked close now also requires the stale-pin issue step (`id: stale_issue`) to have **succeeded** when the verdict is `stale`. The gate stays `!cancelled()` rather than `success()` on purpose — a `stale` verdict exits 1 and must still close the blocked alert, which is the whole reason `success()` was rejected — but because of that, a transient `gh` error in the step filing the *replacement* alert used to close the blocked issue anyway, leaving the run red with **no sticky issue open in either mode** until the next daily run re-filed one. On every other state that step is `skipped`, not failed, so the new clause is scoped to `stale` rather than asserting success unconditionally. **Fixed: only CR was rejected inside `WATCHED_PATHSPECS`, not other control bytes.** The awk extractor trims just leading/trailing `[[:space:]]`, so an entry carrying an interior ESC, BEL, VT or DEL survived the CR-only check — and the consequence is identical for the **unprobed** shapes (wildcard, unreproducible magic, empty remainder), which are the common ones in upstream's glob- and `:(exclude)`-heavy list: the entry skips `ls-tree`, goes straight to `git log`, matches nothing and shrinks the backlog toward `state=current` on a frozen pin. It was also echoed raw into the run log and embedded into `broken_reason`. The `case` now tests the `[[:cntrl:]]` class, and control bytes are rendered as `<CR>`, `<TAB>` or `<0xNN>` so the sticky issue still shows *which* byte upstream wrote while `broken_reason` stays single-line. Fails closed by construction: a locale whose `[[:cntrl:]]` covers more than C0+DEL can only reject *more*, never less. **Fixed: the published day windows were integer-divided.** `$((s / 86400))` truncates, so `grace_days`/`bump_pr_grace_days` only matched the enforced thresholds while both seconds constants stayed exact multiples of 86400 — at a `GRACE_SECONDS` of 129600 the durable issue asserted a "1-day grace window" for a verdict reached under 1.5 days, and anything under a day published a flat `0`. That is a smaller version of the divergence these outputs were added to remove. Both values are **text-only** (nothing compares them numerically; all the arithmetic runs on the seconds constants), so they are now formatted with a trimmed fractional rendering. Chosen over the thread's other option — asserting whole days — because a mis-set knob should not take the watchdog itself offline. **Fixed (Nit): the missing space in "carriage return inside`entry`".** Now part of the widened control-character message, which spaces before `${ctl_entries# }` the way the parallel `missing` reason already did. ## Review round 19 — cursor-review panel (56a738b) Six threads: three Low, three Nit. **Four fixed, two declined with reasoning, none deferred.** The functional change is **three lines**; everything else is comment accuracy, which is most of what this round was about. **Fixed (Low, 2 reviewers): the reopen path revived the duplicate, not the sticky thread.** The two `--state closed` lookups took `max_by(.number)` while every open/update lookup took `min_by(.number)`, so the two sides disagreed about which issue is canonical — and the block comment above them already declares the lowest-numbered match "the original sticky thread, the one carrying the history". A mode that had ever accumulated a duplicate therefore revived the **newest** closed match on its next cycle and orphaned the thread the rest of the block treats as canonical. Both reopen lookups now use `min_by`, so picking a single issue and picking the oldest are the same decision on every path, and the rule is stated once in the block comment and referenced at both reopen sites. The same thread's second half — that the update path touches one issue while the close paths close all of them — is **deliberate and now written down as intended** rather than changed: commenting on one thread while a stale duplicate sits open is cosmetic, whereas leaving a duplicate open after recovery is a live false alert, so the close side is the one that must be exhaustive. **Fixed (Nit): the `grace` log line reintroduced the truncating division.** It still derived its window with `$((GRACE_SECONDS / 86400))` while the durable `grace_days` output goes through `fmt_days`, so at a non-whole-day knob the run log and the sticky issue disagreed about the same number. It now calls `fmt_days "$GRACE_SECONDS"`. At the shipped `GRACE_SECONDS=259200` both renderings are `3-day`, so this is a **no-op today** and a fix for the mis-set case only. **Fixed (Nit): the `[[:cntrl:]]` comment overclaimed.** It asserted the class test "cannot false-positive on a legitimate non-ASCII pathspec". Under a UTF-8 locale glibc's `iswcntrl` is also true for the C1 range U+0080–U+009F, which are legal bytes in a filename, so it can. The comment now states the bound exactly instead of denying it, and names the trade: a spurious `broken` verdict — the watchdog asking a human to look — rather than a missed stale pin, which is the direction this block is built to fail in. Every other code point above U+007F is genuinely unaffected, so that much of the old claim survives, narrowed to what is true. The rendering is documented too: `<0xNN>` carries the **code point**, not the UTF-8 bytes upstream wrote, so U+0085 renders `<0x85>` and not `<0xC2><0x85>`. **Fixed (Nit): the close loops promised more than the lookup can see.** The comments said they close EVERY open match, but `gh issue list --limit 100` returns only the 100 newest-created. The claim is softened to "every open match the lookup returns", with the bound and its consequence stated inline at both loops. Not looped to exhaustion: reaching truncation needs 100+ simultaneously-open issues under a single machine-written mode label, which is a far louder problem than the duplicate accumulation this loop reconciles, and the extra paging is failure surface on a recovery path that must not itself break. **Declined (Low): NUL is invisible to the control-character guard.** Correct on the mechanism — `$(git show ...)` drops NUL before `mapfile` builds `watched`, so the `case` structurally cannot see it — but the byte cannot reach the guard to be caught. NUL is outside YAML's `c-printable` production, so an upstream `bump-cursor-review-callers.yml` carrying one is not a loadable YAML document: upstream's **own** Actions runner would fail to parse its workflow before this watchdog ever clones the repo. Hitting this needs a file simultaneously live enough to be the pin-bump workflow and malformed enough not to load. Rather than add a blob-to-disk probe and its own error-classification arm to the most-reviewed fault-handling path in this job — where every new arm is a fail-open risk of its own — the reasoning is recorded **in the code** beside the guard, so the next reader gets the boundary instead of re-deriving it. Not reachable under current code, so no follow-up was filed. **Declined (Low): the close loops act on a human-writable label.** The finding turns on the grouping label and the closing label being the same; they are not. The close loops filter on the **mode** labels alone (`cursor-review-pin-blocked`, `cursor-review-pin-stale`), while the label that "invites grouping" is the shared `cursor-review-pin`, which nothing closes on — so the invited case, a maintainer grouping a related issue under the shared label, is untouched. Triggering this needs someone to apply a mode label, created by this workflow with the description "cursor-review pin freshness watchdog: cannot run", which reads as machine state rather than a topic tag. Both suggested mitigations also **invert the failure direction**: a body marker is absent from every issue already filed, so the close loops would silently skip them and leave standing exactly the false "the pin is not being watched" alerts this step exists to clear; an author filter has the same shape plus a version-dependent spelling for an App actor, and a filter that silently matches nothing closes nothing. The current blast radius is bounded and reversible — a mislabelled issue is closed with a comment naming the run, reopenable in one click — which is preferable to a filter whose failure mode is a permanently-open false alarm nobody notices. Not filed as a follow-up because the mitigation is not obviously net-positive and needs a design decision rather than blind implementation. ## Provenance - **Authored by:** agent-work loop - **Verified:** Round 19 (`56a738b`) takes four of the panel's six open findings, declines two with reasoning on the threads, and defers none. The functional change is **three lines**; the remainder is comment accuracy. **Each fix was checked against the behaviour it claims.** *The day-window rendering* was measured before and after with the real `fmt_days` body: at the shipped 259200 both the old integer division and `fmt_days` render `3`, so the log line is **unchanged in production**; the divergence appears only at a mis-set knob (129600 → `1` old vs `1.5` new, 43200 → `0` vs `0.5`). *The `fmt_days` call site* was confirmed in scope: `fmt_days` is defined at the top of the same `run:` block that contains the `grace` log line, which is one step spanning both, so the call cannot resolve to an undefined function. *The `min_by` change* was confirmed exhaustive — all five single-issue lookups in the job now read `min_by(.number).number // empty`, with no `max_by` left in the file. **Full local gate on `56a738b`:** `pnpm lint`, `pnpm format:check`, `pnpm typecheck`, `pnpm test` (26 files, 620 tests, 0 failed), `pnpm check:spec-drift` and `pnpm check:workflow-pins` (1 caller, pins agree) all pass; `node scripts/check-public-repo-hygiene.mjs` is clean; `.github/workflows/ci.yml` parses, and **all 21** `run:` blocks in it — extracted from the parsed YAML rather than retyped, with none skipped — pass `bash -n`. **CI status on `56a738b`, read once after the push:** `Test (node 22)`, `Test (node 24)`, `workflow-pins`, `public-repo-hygiene`, `build-check`, `Block do-not-merge`, `CodeRabbit`, `cla-assistant` and both Socket checks all **pass**. Checks are **not fully settled**: the `cursor-review` panel had not been dispatched on this head when this was written, so its next verdict is outstanding and nothing here claims it. The one failure is `sdk-parity (python surface freshness)`, which is **pre-existing, unrelated and not caused by this PR** — it was reproduced this round on a pristine `origin/main` worktree with the identical `ExtractionError: … unrecognized public class(es) RouterRunResult`, `main`'s own scheduled runs fail it (run 35573997014 on `4491e4b`), it is tracked in #156 with a fix open in #167, and this PR changes no line of `scripts/python-surface.mjs` or `scripts/sync-python-surface.mjs`. **Re-read on a later shepherd pass over the same head `56a738b` (no new commits):** the local gate was re-run green in a clean worktree — `npm test` (26 files, 620 tests, 0 failed), `npm run lint`, `npm run typecheck`, `npm run format:check`, and `npm run check:workflow-pins` (1 caller, pins agree). `sdk-parity` is still the only red check and still reproduces byte-identically on this branch's own tree; its fix in **#167 is now APPROVED and MERGEABLE**, so that check is expected to go green here once #167 lands, with no change required on this branch. `origin/main` merges into this branch cleanly (branch is 24 ahead, 2 behind), no `merge_group` run has ever been attempted for this PR, and all **118** review threads are resolved with **0** unresolved. - **Deviations:** Round 19 (`56a738b`) changes **only** `.github/workflows/ci.yml`, entirely inside the `cursor-review-pin-freshness` job this PR adds; no TS/JS surface, no published API, no other workflow, `scripts/check-workflow-pins.mjs` and `.github/dependabot.yml` both untouched this round. Three things go beyond the literal threads. (1) The `min_by`/`max_by` thread also asked for one **duplicate policy** on both sides; that half is deliberately **not** taken — the close paths stay exhaustive while the open/update paths touch a single issue, because a stale duplicate left open after recovery is a live false alert whereas a duplicate carrying a stale body until recovery is cosmetic. The asymmetry is now documented as intended rather than left to be re-derived. (2) The NUL finding is answered with an **in-code note** instead of a probe: NUL is outside YAML's `c-printable`, so a file carrying one is not loadable by upstream's own Actions runner and cannot reach this job. Not reachable under current code, so no follow-up was filed. (3) The human-writable-label finding is **declined** rather than mitigated, because both suggested mitigations (a body marker, an author filter) fail **open** — silently skipping the already-filed issues they would need to close — which is the worse direction for a watchdog; it is recorded on the thread as needing a design decision rather than filed blind. Two notes for whoever lands the stack: **#172** still owns the `--print-pin` delivery Medium and is not duplicated here, and **#173 edits these same close steps** and will need a rebase onto this head — ordinary stack churn, and nothing here was force-pushed. Nothing was deferred and no follow-up was filed this round. A later shepherd pass over this same head changed **no code and pushed no commit**; it corrected four stale factual claims in this description that round 16 had left behind when it dropped the bare Dependabot `ignore` entry — the "Both `ignore` entries are load-bearing" lead, the `grep -c` → `2` verification line, the `grep -n` "4 lines, not 2" judgment call, and the "comment block is used verbatim as supplied" judgment call — all four of which the file now contradicts. No claim of merge-readiness is made here; review approval is still outstanding. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Updated automated dependency management for reusable workflow references. - Synchronized code review workflows with their pinned reusable workflow version. - Added validation to ensure workflow references use full, matching revisions. - Added scheduled monitoring for outdated workflow references, including grace-period handling and status tracking. - Added a command for manually checking workflow pin consistency. - No application functionality or user-facing behavior changed. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- _{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

bsakaguchi added the

pull-request

label 2026-10-31 22:05:06 +00:00

bsakaguchi referenced this issue 2026-10-31 22:05:09 +00:00

[PR #155] [CLOSED] chore: sync vendored Comfy Router spec (cloud@8759533) #157

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

main

chore/sync-router-spec

chore/sync-v2-spec

matt/dplat-1602-dash-less-step-pin

dependabot/npm_and_yarn/npm-minor-and-patch-9e54abe97c

dependabot/github_actions/Comfy-Org/github-workflows/dot-github/workflows/public-repo-hygiene.yml-9a5f2a20caf4e7f9fd59409704423c2ac0189573

dependabot/github_actions/Comfy-Org/github-workflows/dot-github/workflows/cursor-review.yml-9a5f2a20caf4e7f9fd59409704423c2ac0189573

dependabot/github_actions/actions-minor-and-patch-c673fd7efc

matt/be-15270-resync-cursor-review-pins

be-17174-refusal-subject

matt/publish-requires-changelog-section

matt/land-v2-spec-sync-b8907cf

matt/land-router-spec-sync-9794097

be-19976-request-handle-estimate

matt/be-17267-queued-binary-result

mattmiller/be-19941-sse-error-frame

be-19948-head-429-rate-limited

matt/be-19943-sso-required-forbidden

be-19945-bare-message-fallback

matt/be-19930-floor-events-429-pause

mattmiller/be-17634-typed-error-on-unread-body

chore/node-runtime-refresh-2026-09-28

js/codeowners

deepme987/docs/router-params

chore/sync-router-spec-19ea326

chore/sync-router-spec-ec6587d

matt/fix-request-handle-get-kind

chore/sync-router-spec-5a3b668

chore/sync-router-spec-e57bc53

ben/sdk-browser-safe

jacob/be-9479-add-devplat-execution-logs-support-to-sdk

chore/cursor-review-caller

chore/sync-router-spec-a3d6e59

chore/sync-v2-spec-cb3a225

chore/sync-router-spec-2a369ae

chore/sync-v2-spec-8b5e498

chore/sync-router-spec-8b5e498

chore/sync-router-spec-f74cc66

chore/sync-router-spec-2ccc38c

chore/sync-router-spec-31831a9

chore/sync-router-spec-f93534d

chore/sync-router-spec-2b9c2cf

chore/sync-v2-spec-463e218

chore/sync-router-spec-462120c

chore/sync-v2-spec-462120c

chore/sync-v2-spec-d5155ac

chore/sync-router-spec-d5155ac

chore/sync-v2-spec-bd3663b

chore/sync-v2-spec-b886298

chore/sync-v2-spec-375edb1

chore/sync-v2-spec-7c65b1a

chore/sync-v2-spec-61ef440

wei/be-7128-embed-workflow-metadata

chore/sync-v2-spec-4204113

chore/sync-v2-spec-04133db

chore/sync-v2-spec-ab38191

chore/sync-v2-spec-f20e07a

alexis/update-readme

chore/sync-v2-spec-e142a69

v0.5.0

v0.4.0

v0.3.0

v0.2.0

v0.1.9

v0.1.8

v0.1.7

v0.1.6

v0.1.5

v0.1.4

v0.1.3

v0.1.2

v0.1.1

v0.1.0

Labels

Clear labels   

audit:ai-readiness

  

audit:community

  

audit:github

  

audit:health-scoring

  

audit:policy

  

audit:release

  

audit:supply-chain

  

bug

  

cursor-review-pin

  

effort:medium

  

effort:small

  

effort:trivial

  

priority:nice-to-have

  

priority:recommended

  

pull-request

Mirrored from GitHub Pull Request

No labels

audit:ai-readiness

audit:community

audit:github

audit:health-scoring

audit:policy

audit:release

audit:supply-chain

bug

cursor-review-pin

effort:medium

effort:small

effort:trivial

priority:nice-to-have

priority:recommended

pull-request

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Comfy-Org/comfy-typescript-sdk#155

No description provided.