[PR #163] [MERGED] ci(cursor-review): daily watchdog for a stale github-workflows pin #162

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: matt/be-15270-resync-cursor-review-pins ← Head: matt/be-15923-cursor-review-pin-freshness

📝 Commits (1)

00a6ece ci(cursor-review): daily watchdog for a stale github-workflows pin

📊 Changes

2 files changed (+431 additions, -65 deletions)

View changed files

📝 .github/workflows/ci.yml (+276 -15)
📝 scripts/check-workflow-pins.mjs (+155 -50)

📄 Description

STACKED — merging lands on matt/be-15270-resync-cursor-review-pins (owned by #153, @mattmillerai), NOT main. #153 must merge first; this branch is based on it and its diff here is only this PR's two files.

ELI-5

There are two ways the cursor-review pin can be wrong. It can be inconsistent — the uses: SHA and the workflows_ref: input naming different commits — and #153 adds a CI lint for that. Or it can be perfectly consistent and just old, frozen months behind upstream with nobody bumping it. Nothing catches the second one, which is exactly how this repo sat three weeks behind with no signal anywhere: the upstream bump fleet only visits repos on its roster, and this repo had silently fallen off it.

This PR adds a daily robot that checks the second thing. Once a day it reads the pinned commit, asks Comfy-Org/github-workflows "has anything a caller can actually see landed since then?", and if the answer has been yes for more than three days and no bump PR has shown up, it goes red and opens one GitHub issue that stays open until the pin is current again.

What this adds

A schedule-only cursor-review-pin-freshness job at the end of .github/workflows/ci.yml, on the existing 17 7 * * * cron. It never runs on a PR or a push, on purpose: the failure mode it watches for is that nothing happens, so it must not need a PR to notice.

Its verdict is one of four, and each was run locally against the real upstream repo before this PR opened:

verdict	meaning	result
`current`	nothing consumer-visible landed upstream since the pin	pass
`grace`	the oldest commit this repo hasn't picked up is under 3 days old	pass
`delivered`	past grace, but a PR is open on `ci/bump-cursor-review`	pass, `::notice::` + a comment on the sticky issue if one is open
`stale`	past grace and no bump PR exists	fail, opens/comments the sticky issue

Sticky-issue handling copies sdk-parity exactly: exact-title lookup off gh issue list --state open --limit 100 (not the eventually-consistent search API), gh issue create on first failure, gh issue comment on repeats, gh issue close on the first run that finds the pin current again.

scripts/check-workflow-pins.mjs grows a --print-pin <reusable> mode and its caller scan moves into a shared findCallers(). The watchdog reads the pin through that rather than grepping the YAML a second time, so the two jobs cannot disagree about which line is the pin. --print-pin enforces 40-hex but deliberately does not assert the two pins are equal — that is the lint's assertion and workflow-pins' red build, and reddening a second job for one split pin would just double the noise.

Judgment calls / deviations from the plan

1. The grace clock runs on the OLDEST undelivered commit, not the newest upstream one. This is the one place I did not implement the plan as written, and I think shipping it as written would have shipped a watchdog that never fires. The plan timed git log -1 … — the newest watched commit. But the fleet delivers the whole backlog in one bump, so the real question is how long the first owed commit has been sitting there. Timing the newest restarts the 3-day window on every upstream push. Measured 2026-09-19 over the last 25 consumer-visible upstream commits: median gap between them is 0.39 days, and only 4 of 24 gaps reach 3 days. A frozen caller would therefore be silently re-forgiven roughly every nine hours, and the job would fire only during a rare quiet week — the exact invisibility it exists to end. The issue body still quotes the newest commit (it is what a bump moves the pin to), plus the oldest and the count behind.

2. The sticky issue does not name the ops repo or its roster file path. The plan asked the remedy text to point at rosters/cursor-review.json in the ops repo. This repo is public and public-repo-hygiene default-denies Comfy-Org/<repo> references outside a known-public allowlist; upstream's own bump-cursor-review-callers.yml header refuses to name that repo for the same reason ("the specific home repo is intentionally not named here — this file is public"). The body names the CURSOR_REVIEW_CALLERS secret and the roster: N caller(s) audit line, which are enough to act on, and says the canonical copy lives in the private ops repo without naming it.

3. The sticky-issue step is gated on the verdict (steps.assess.outputs.state == 'stale'), not on bare failure(). A clone failure or a GitHub API blip reddens the scheduled run as a step annotation, but must not file an issue asserting a cause nothing established. Consequence, stated plainly: an infrastructure failure of this job is visible only in the Actions tab, not as an issue.

Verification

Run act-free, as the plan asked, by extracting the real run: bodies out of ci.yml with a YAML parser and executing them against the real Comfy-Org/github-workflows — read-only throughout (git clone, git log, gh pr list, gh issue list); every mutating gh call was exercised against a stub that printed its arguments rather than writing anything.

case	pin	verdict	exit
upstream tip `cae1c78`	current	`current`	0
`425c154` (this branch's pin, 2 commits behind, newest 1d old)	within grace	`grace`	0
`5803646` (2026-09-07, 11 commits behind, oldest 10d old), real 3-day grace, no bump PR	stale	`stale`	1
same pin, `gh` stubbed to report an open bump PR	delivered	`delivered`	0
a 40-hex SHA not on `github-workflows` main	ancestry guard	`::error::`	1

Also verified separately:

The :(exclude)scripts/check-pr-size/*_test.go pathspec is load-bearing, not decoration. Upstream commit d88f28d touches only scripts/check-pr-size/main_test.go; with the exclusion git log over that range returns empty, without it the commit is reported. Without that exclusion a test-only upstream commit would redden this job permanently with no bump that could ever clear it.
--print-pin edge cases: non-40-hex workflows_ref (main) → exit 1 with a pointed message; split pin → prints the workflows_ref and exits 0 while node scripts/check-workflow-pins.mjs exits 1, which is the intended division of labour; unknown reusable name → exit 1; missing argument → exit 1.
Issue steps: stale-with-no-open-issue renders gh issue create, stale-with-open-issue renders gh issue comment, delivered-with-open-issue comments, delivered-with-no-open-issue is a correct no-op, close-with-open-issue closes, close-with-no-open-issue is a correct no-op.
The premise, re-checked against github-workflows@main rather than taken on trust: cursor-review.yml only emits a ::warning:: for a non-40-hex workflows_ref and runs no ancestry or currency check at all (the stricter shape+ancestry guard the org README describes belongs to pr-risk.yml, not this reusable), and that README states in its own words that "what is still not machine-checked is that the pin is the current one". The WATCHED_PATHSPECS list this job mirrors was read out of upstream's bump-cursor-review-callers.yml and matches character-for-character, and the fleet's stable branch really is ci/bump-cursor-review.
Repo checks, all green on this branch: pnpm test 620 passed / 26 files, pnpm lint (oxlint) clean, pnpm format:check clean, pnpm typecheck clean, node scripts/check-workflow-pins.mjs 1 caller checked all pins agree, node scripts/check-public-repo-hygiene.mjs no internal-only references, node scripts/check-spec-drift.mjs in sync.

Residual

The first scheduled run after merge may open the issue, and that is the watchdog working, not a defect. As of 2026-09-19 this repo's pin 425c154 is 2 consumer-visible commits behind with the oldest 24h old, so today's verdict is grace. That commit crosses the 3-day line on ~2026-09-21; unless the bump fleet reaches this repo before then, the run on or after that date will go stale and file the issue. The fix is the enrolment work the issue body's remedy 1 describes (confirm this repo is on the CURSOR_REVIEW_CALLERS roster and re-dispatch the fleet) — that enrolment is NOT done by this PR and is not verifiable from here: the roster is an Actions secret on another repo with no read-back API (gh secret get does not exist), so I could not check whether this repo is on it. This PR makes the condition visible; it does not repair it.
An infrastructure failure of this job files nothing (deviation 3 above). If the upstream clone or a gh call fails, the scheduled run is red in the Actions tab with a step annotation and no issue is opened. Closing that would need a second, differently-titled sticky issue for "the watchdog itself could not run", which is a larger design question than this change.
ci/bump-cursor-review is matched by branch name only. gh pr list --head ci/bump-cursor-review does not assert the PR's author or that it actually moves the pins, so an unrelated PR pushed to that branch name would read as a delivery and suppress the failure. The branch is fleet-owned and force-reset on every run, so this is judged acceptable rather than fixed here.
No upstream-side change is made. The org-wide version of this check — a freshness signal the reusable gives every caller, rather than one this repo carries alone — belongs in Comfy-Org/github-workflows and is out of scope here; every other caller repo remains unguarded. I did not sweep the other caller repos to count how many are affected, because the roster that names them is the unreadable secret above.
scripts/check-workflow-pins.mjs has no unit test. That matches this repo's convention (no script under scripts/ has one; they are exercised by running them in CI), and both modes are run in CI by the workflow-pins and cursor-review-pin-freshness jobs. Worth its own change if the parser grows further.
Stacked. This PR is based on matt/be-15270-resync-cursor-review-pins (#153) and must not be merged before it. If #153 changes, rebase this branch rather than merging it into main.

Provenance

Authored by: agent-work loop
Verified: pnpm test: 620 passed, 0 failed (26 files); pnpm lint (oxlint): clean; pnpm format:check (oxfmt): clean, 84 files; pnpm typecheck (tsc --noEmit): clean; node scripts/check-workflow-pins.mjs: 1 caller checked, pins agree; node scripts/check-public-repo-hygiene.mjs: no findings; node scripts/check-spec-drift.mjs: in sync; plus the five-verdict and edge-case matrix above, run locally against the real upstream repo
Deviations: grace clock anchored on the oldest undelivered commit rather than the newest upstream one (deviation 1, with the measurement that motivated it); sticky-issue body does not name the private ops repo or its roster file path (deviation 2); sticky-issue step gated on the verdict rather than bare failure() (deviation 3)

_{🔄 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/163 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/19/2026 **Status:** ✅ Merged **Merged:** 9/20/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `matt/be-15270-resync-cursor-review-pins` ← **Head:** `matt/be-15923-cursor-review-pin-freshness` --- ### 📝 Commits (1) - [`00a6ece`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/00a6eceaf02f780e5c96d21dcab4b00be5fadd2f) ci(cursor-review): daily watchdog for a stale github-workflows pin ### 📊 Changes **2 files changed** (+431 additions, -65 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/ci.yml` (+276 -15) 📝 `scripts/check-workflow-pins.mjs` (+155 -50) </details> ### 📄 Description **STACKED — merging lands on `matt/be-15270-resync-cursor-review-pins` (owned by #153, @mattmillerai), NOT `main`.** #153 must merge first; this branch is based on it and its diff here is only this PR's two files. ## ELI-5 There are two ways the cursor-review pin can be wrong. It can be *inconsistent* — the `uses:` SHA and the `workflows_ref:` input naming different commits — and #153 adds a CI lint for that. Or it can be perfectly consistent and just **old**, frozen months behind upstream with nobody bumping it. Nothing catches the second one, which is exactly how this repo sat three weeks behind with no signal anywhere: the upstream bump fleet only visits repos on its roster, and this repo had silently fallen off it. This PR adds a daily robot that checks the second thing. Once a day it reads the pinned commit, asks `Comfy-Org/github-workflows` "has anything a caller can actually see landed since then?", and if the answer has been yes for more than three days *and* no bump PR has shown up, it goes red and opens one GitHub issue that stays open until the pin is current again. ## What this adds A schedule-only `cursor-review-pin-freshness` job at the end of `.github/workflows/ci.yml`, on the existing `17 7 * * *` cron. It never runs on a PR or a push, on purpose: the failure mode it watches for is that *nothing happens*, so it must not need a PR to notice. Its verdict is one of four, and each was run locally against the real upstream repo before this PR opened: | verdict | meaning | result | | --- | --- | --- | | `current` | nothing consumer-visible landed upstream since the pin | pass | | `grace` | the oldest commit this repo hasn't picked up is under 3 days old | pass | | `delivered` | past grace, but a PR is open on `ci/bump-cursor-review` | pass, `::notice::` + a comment on the sticky issue if one is open | | `stale` | past grace and no bump PR exists | **fail**, opens/comments the sticky issue | Sticky-issue handling copies `sdk-parity` exactly: exact-title lookup off `gh issue list --state open --limit 100` (not the eventually-consistent search API), `gh issue create` on first failure, `gh issue comment` on repeats, `gh issue close` on the first run that finds the pin current again. `scripts/check-workflow-pins.mjs` grows a `--print-pin <reusable>` mode and its caller scan moves into a shared `findCallers()`. The watchdog reads the pin through that rather than grepping the YAML a second time, so the two jobs cannot disagree about which line is the pin. `--print-pin` enforces 40-hex but deliberately does **not** assert the two pins are equal — that is the lint's assertion and `workflow-pins`' red build, and reddening a second job for one split pin would just double the noise. ## Judgment calls / deviations from the plan **1. The grace clock runs on the OLDEST undelivered commit, not the newest upstream one.** This is the one place I did not implement the plan as written, and I think shipping it as written would have shipped a watchdog that never fires. The plan timed `git log -1 …` — the *newest* watched commit. But the fleet delivers the whole backlog in one bump, so the real question is how long the *first* owed commit has been sitting there. Timing the newest restarts the 3-day window on every upstream push. Measured 2026-09-19 over the last 25 consumer-visible upstream commits: **median gap between them is 0.39 days, and only 4 of 24 gaps reach 3 days**. A frozen caller would therefore be silently re-forgiven roughly every nine hours, and the job would fire only during a rare quiet week — the exact invisibility it exists to end. The issue body still quotes the newest commit (it is what a bump moves the pin *to*), plus the oldest and the count behind. **2. The sticky issue does not name the ops repo or its roster file path.** The plan asked the remedy text to point at `rosters/cursor-review.json` in the ops repo. This repo is public and `public-repo-hygiene` default-denies `Comfy-Org/<repo>` references outside a known-public allowlist; upstream's own `bump-cursor-review-callers.yml` header refuses to name that repo for the same reason ("the specific home repo is intentionally not named here — this file is public"). The body names the `CURSOR_REVIEW_CALLERS` secret and the `roster: N caller(s)` audit line, which are enough to act on, and says the canonical copy lives in the private ops repo without naming it. **3. The sticky-issue step is gated on the verdict (`steps.assess.outputs.state == 'stale'`), not on bare `failure()`.** A clone failure or a GitHub API blip reddens the scheduled run as a step annotation, but must not file an issue asserting a cause nothing established. Consequence, stated plainly: an *infrastructure* failure of this job is visible only in the Actions tab, not as an issue. ## Verification Run `act`-free, as the plan asked, by extracting the real `run:` bodies out of `ci.yml` with a YAML parser and executing them against the real `Comfy-Org/github-workflows` — read-only throughout (`git clone`, `git log`, `gh pr list`, `gh issue list`); every mutating `gh` call was exercised against a stub that printed its arguments rather than writing anything. | case | pin | verdict | exit | | --- | --- | --- | --- | | upstream tip `cae1c78` | current | `current` | 0 | | `425c154` (this branch's pin, 2 commits behind, newest 1d old) | within grace | `grace` | 0 | | `5803646` (2026-09-07, 11 commits behind, oldest 10d old), real 3-day grace, no bump PR | stale | `stale` | **1** | | same pin, `gh` stubbed to report an open bump PR | delivered | `delivered` | 0 | | a 40-hex SHA not on `github-workflows` main | ancestry guard | `::error::` | **1** | Also verified separately: - **The `:(exclude)scripts/check-pr-size/*_test.go` pathspec is load-bearing, not decoration.** Upstream commit `d88f28d` touches only `scripts/check-pr-size/main_test.go`; with the exclusion `git log` over that range returns empty, without it the commit is reported. Without that exclusion a test-only upstream commit would redden this job permanently with no bump that could ever clear it. - **`--print-pin` edge cases:** non-40-hex `workflows_ref` (`main`) → exit 1 with a pointed message; split pin → prints the `workflows_ref` and exits 0 while `node scripts/check-workflow-pins.mjs` exits 1, which is the intended division of labour; unknown reusable name → exit 1; missing argument → exit 1. - **Issue steps:** stale-with-no-open-issue renders `gh issue create`, stale-with-open-issue renders `gh issue comment`, delivered-with-open-issue comments, delivered-with-no-open-issue is a correct no-op, close-with-open-issue closes, close-with-no-open-issue is a correct no-op. - **The premise, re-checked against `github-workflows@main` rather than taken on trust:** `cursor-review.yml` only emits a `::warning::` for a non-40-hex `workflows_ref` and runs no ancestry or currency check at all (the stricter shape+ancestry guard the org README describes belongs to `pr-risk.yml`, not this reusable), and that README states in its own words that "what is still not machine-checked is that the pin is the *current* one". The `WATCHED_PATHSPECS` list this job mirrors was read out of upstream's `bump-cursor-review-callers.yml` and matches character-for-character, and the fleet's stable branch really is `ci/bump-cursor-review`. - **Repo checks, all green on this branch:** `pnpm test` 620 passed / 26 files, `pnpm lint` (oxlint) clean, `pnpm format:check` clean, `pnpm typecheck` clean, `node scripts/check-workflow-pins.mjs` 1 caller checked all pins agree, `node scripts/check-public-repo-hygiene.mjs` no internal-only references, `node scripts/check-spec-drift.mjs` in sync. ## Residual - **The first scheduled run after merge may open the issue, and that is the watchdog working, not a defect.** As of 2026-09-19 this repo's pin `425c154` is 2 consumer-visible commits behind with the oldest 24h old, so today's verdict is `grace`. That commit crosses the 3-day line on ~2026-09-21; unless the bump fleet reaches this repo before then, the run on or after that date will go `stale` and file the issue. The fix is the enrolment work the issue body's remedy 1 describes (confirm this repo is on the `CURSOR_REVIEW_CALLERS` roster and re-dispatch the fleet) — **that enrolment is NOT done by this PR and is not verifiable from here**: the roster is an Actions secret on another repo with no read-back API (`gh secret get` does not exist), so I could not check whether this repo is on it. This PR makes the condition visible; it does not repair it. - **An infrastructure failure of this job files nothing** (deviation 3 above). If the upstream clone or a `gh` call fails, the scheduled run is red in the Actions tab with a step annotation and no issue is opened. Closing that would need a second, differently-titled sticky issue for "the watchdog itself could not run", which is a larger design question than this change. - **`ci/bump-cursor-review` is matched by branch name only.** `gh pr list --head ci/bump-cursor-review` does not assert the PR's author or that it actually moves the pins, so an unrelated PR pushed to that branch name would read as a delivery and suppress the failure. The branch is fleet-owned and force-reset on every run, so this is judged acceptable rather than fixed here. - **No upstream-side change is made.** The org-wide version of this check — a freshness signal the reusable gives every caller, rather than one this repo carries alone — belongs in `Comfy-Org/github-workflows` and is out of scope here; every other caller repo remains unguarded. I did not sweep the other caller repos to count how many are affected, because the roster that names them is the unreadable secret above. - **`scripts/check-workflow-pins.mjs` has no unit test.** That matches this repo's convention (no script under `scripts/` has one; they are exercised by running them in CI), and both modes are run in CI by the `workflow-pins` and `cursor-review-pin-freshness` jobs. Worth its own change if the parser grows further. - **Stacked.** This PR is based on `matt/be-15270-resync-cursor-review-pins` (#153) and must not be merged before it. If #153 changes, rebase this branch rather than merging it into `main`. ## Provenance - **Authored by:** agent-work loop - **Verified:** `pnpm test`: 620 passed, 0 failed (26 files); `pnpm lint` (oxlint): clean; `pnpm format:check` (oxfmt): clean, 84 files; `pnpm typecheck` (tsc --noEmit): clean; `node scripts/check-workflow-pins.mjs`: 1 caller checked, pins agree; `node scripts/check-public-repo-hygiene.mjs`: no findings; `node scripts/check-spec-drift.mjs`: in sync; plus the five-verdict and edge-case matrix above, run locally against the real upstream repo - **Deviations:** grace clock anchored on the oldest undelivered commit rather than the newest upstream one (deviation 1, with the measurement that motivated it); sticky-issue body does not name the private ops repo or its roster file path (deviation 2); sticky-issue step gated on the verdict rather than bare `failure()` (deviation 3) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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