[PR #173] [MERGED] ci(cursor-review): prove the pin moved before closing the stale-pin issue #171

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: matt/be-15270-resync-cursor-review-pins ← Head: matt/be-16376-record-pin-on-stale-issue

📝 Commits (10+)

b7ea71e ci(cursor-review): prove the pin moved before closing the stale-pin issue
d320b45 Merge base 'matt/be-15270-resync-cursor-review-pins' into the pin-marker branch
1e95954 ci(cursor-review): stop the movement proof failing open, and fence the manual run
9d5b324 ci(cursor-review): stop a second dispatch cancelling the first mid-mutation
fac9c52 Merge base 'matt/be-15270-resync-cursor-review-pins' into the pin-marker branch
33ed546 ci(cursor-review): scope the pin marker to the alarm cycle, hold when unproven
af95676 Merge base 'matt/be-15270-resync-cursor-review-pins' into the pin-marker branch
6a6d5b2 ci(cursor-review): re-serialise sdk-parity, and key the stale report on the bump state
6db283f Merge base 'matt/be-15270-resync-cursor-review-pins' into the pin-marker branch
723e52f ci(cursor-review): scope the dedup read, and stop the reports asserting stale facts

📊 Changes

1 file changed (+830 additions, -56 deletions)

View changed files

📝 .github/workflows/ci.yml (+830 -56)

📄 Description

STACKED — merging lands on matt/be-15270-resync-cursor-review-pins (owned by @mattmillerai, PR #153), NOT main. The cursor-review-pin-freshness job this changes exists only on that branch; main's ci.yml has no such job. PRs #169 and #170 stack on the same base.

ELI-5

The pin-freshness watchdog opens a sticky issue when this repo's Comfy-Org/github-workflows pin goes stale, and closed that issue again as soon as nothing was "overdue". But "nothing overdue" is measured purely from the age of the oldest upstream commit this repo has not picked up — and that commit disappears from the walk, with the pin untouched, whenever upstream narrows the list of paths it watches. So the watchdog could close its own issue on a completely frozen pin. This makes it record the pin in every report it files and refuse to close unless the pin it sees now is a different one.

What changed

1. Every stale report records the pin. The body the stale step writes now ends with an  HTML comment — invisible to a reader. That body is re-commented once per report rather than once per run — keyed on the pin, the bump-PR state and the date basis of the grace verdict, since the report's premise and its step-1 remediation both branch on BUMP_PR/BUMP_DELIVERS, so the same frozen pin produces a materially different report once a bump PR opens against it, stops carrying the owed commit, or ages back out of delivered. A sibling  marker records that key; it is a separate marker precisely so the close step's movement proof, which matches cursor-review-pin(-held|-unknown)?: against an exact 40-hex string, cannot see it. The fourth field is the FUTURE_DATED selector, because the report's overdue paragraph branches on it as well — without it, a frozen pin whose oldest owed commit turns out to be future-dated kept a durable report giving the other diagnosis. The key omits the bump PR's age and the day-varying upstream facts (NEWEST_SHA, NEWEST_DATE, BEHIND_COUNT), which move whenever upstream lands a watched commit, pin or no pin, and so would defeat the suppression on exactly the long-frozen pins it exists for. Those are handled in the body instead: it labels both a snapshot of one run, and step 3 no longer names NEWEST_SHA as the SHA to hand-bump to — it sends the reader to the newest daily run's log for the current tip. That was the one with teeth: a maintainer following a suppressed, weeks-old report would move the pin to a SHA still far behind, and the close step reads any movement off the recorded marker, so it would then close the alarm asserting a bump landed. Deduping at all matters because a frozen pin, the very state this watchdog exists to report, otherwise accrued one whole report a day on the same thread the close step must --paginate in full. On reopen it is posted before the reopen, so an open issue always carries the current cycle's marker. The last marker recorded in the current alarm cycle — at or after the moment the issue was last closed — is the pin as of the most recent report. This dedup read is scoped to that same cycle, and by the same anchor, as the close step's movement-proof read: unscoped, the two steps disagreed about which markers exist, so a hand-reopened alarm (or one whose reopen run died after posting) was suppressed on the previous cycle's markers — left with a stale Run URL and backlog count, a ::notice:: claiming a report was already there, and a close step that then found nothing in-cycle and held it open an extra day. A failed read still falls through to commenting, and now says why in a ::warning:: rather than swallowing the error, since a persistent failure silently reverts the step to commenting once per run — the unbounded growth the dedup exists to stop. No escaping is needed: assess asserts the value is 40-hex before writing it to $GITHUB_OUTPUT, so it cannot break out of the comment.

2. The grace close proves movement first. current still closes unconditionally. grace now reads the recorded pin back and branches four ways: a different pin closes with "The pin moved from x to y since it was last recorded on this issue, so a bump landed"; an equal pin does NOT close — it comments why the backlog shrank without the pin moving, emits a ::warning:: and exits 0 (the verdict is still grace, i.e. passing) — and what it is willing to claim there depends on whether this alarm cycle ever recorded the pin while the backlog was still overdue. Only the stale step's plain cursor-review-pin: marker is written in that state — -held and -unknown are both written from grace, i.e. after the backlog shrank — so the test is over the whole cycle's records: with a plain marker for this pin in the cycle, upstream narrowing WATCHED_PATHSPECS is named as the likely cause; without one, the comment says only that the pin has not moved since it was first sighted and states outright that a bump landing before that is indistinguishable from a narrowed watch list. Two rounds were needed to get this right. Byte-identical markers first made the no-baseline case assert the other case's cause as fact; splitting out -unknown fixed the run that reads it directly but not the one after, because that run's -held marker — written by the weak branch itself — read as a real predecessor and laundered the chain back to the strong claim one cycle later. Keying on the cycle's plain markers rather than the newest marker's form is what actually closes it: a -held record is still honest that the pin has not moved since it, but it cannot vouch for a predecessor it never had. This matters on exactly the ship-day input the branch exists for, where a bump that lands and reaches grace is what gets recorded with no baseline; no marker in this cycle does NOT close either — there is nothing to compare against, and that is precisely the state in which a real bump and a narrowed WATCHED_PATHSPECS cannot be told apart, so it RECORDS the current pin as a distinct  marker, emits a ::warning:: and holds the issue one more cycle; it cannot stick open, because that marker is what closes the issue tomorrow if the pin moved; a failed read is UNKNOWN, which is neither "moved" nor "no marker", so it does NOT close either — it emits a ::warning:: carrying gh's stderr and leaves the issue open for the next scheduled run to read again.

The equal-pin branch comments once per pin, not once per run: it ends its comment with a sibling  marker and skips the comment when that is already the newest marker on the thread. The ::warning:: still fires on every held run, so nothing goes unreported. A later stale report writes a plain marker that is newer, so a fresh entry into the state is still announced. The movement comparison reads the newest marker of any of the three kinds, so a held or unknown record counts as a record; the three forms cannot cross-match, and the -report marker stays invisible to both greps.

A marker only counts if it was recorded in the current alarm cycle — at or after the moment the issue was last closed. Unscoped, the “different pin” test proves inequality rather than “a bump landed since the alarm came back”, and two ordinary sequences reach it: a run dying between the reopen step's two API calls, and a maintainer reopening a wrongly-closed alarm by hand. Both leave an open issue whose newest marker is a previous cycle's pin, so the next grace run closes the alarm claiming a bump landed — on a pin nothing has touched. Scoped, both land on the no-marker branch and hold. The anchor is the last closed event rather than the last reopened one precisely because the report is posted before the reopen: anchoring on the reopen would exclude the very marker the reopen just wrote. A reopened issue therefore reads its comments only; the filing body is read only when the issue has never been closed, since otherwise it belongs to an earlier cycle.

current still closes with no movement proof, and that is a decision rather than the same hole one door over: current means nothing is owed at all, so this issue's premise — a backlog frozen past the grace window — is gone whatever removed it. A narrowed WATCHED_PATHSPECS can reach current on a frozen pin, and if it does, this watchdog has nothing left to say; a pin that is wrong rather than merely behind is workflow-pins' beat. The issue body says as much ("closes itself on the first daily run that finds the pin moved, or nothing owed").

The read is gh api on the issue and gh api --paginate on its comments, deliberately not gh issue view --json comments — that returns only the first 100 comments, which a sticky issue commented on daily passes inside about three months, after which the newest marker would fall off the end and the step would compare against a stale one.

3. workflow_dispatch, kept permanently. The job is if: github.event_name == 'schedule' and on: had no manual trigger, so nothing about this job — verdict or sticky-issue behaviour — is reachable from any PR run, and a change to it could previously only be validated by waiting a day. It is also the only on-demand way to re-run the watchdog after a bump lands. sdk-parity's issue steps gate on github.event_name == 'schedule' themselves and are untouched, so a manual run does its network check and touches no issue. Dropping cancellation for schedule, however, did reach sdk-parity: that job runs on the cron, does the same non-atomic list-then-mutate against a title-matched sticky issue, and had no job-level group of its own, so it had been relying on run-level cancellation to stay single-instance. It now carries its own sdk-parity-${{ github.event_name == 'schedule' && 'schedule' || github.run_id }} group with cancel-in-progress: false. Only scheduled runs share it — github.run_id is unique per run, so PR and push runs keep going fully in parallel. Because that group no longer cancels in progress, the job also gained timeout-minutes: 15: scripts/sync-python-surface.mjs is the one step here that reaches the network and fetches with no AbortSignal, and undici's headersTimeout/bodyTimeout are inactivity-based, so without a bound a trickling response would hold the group for the six-hour default and queue every later scheduled run behind it.

This job does touch its issues on a manual run — that is the point of having one — so it carries two guards of its own. Its actions/checkout is pinned to github.event.repository.default_branch, not the dispatched ref. The ref is caller-chosen and the pin is read out of the working tree, so without that pin a run dispatched from a bump branch (or from a branch used to test the job, which the on: comment recommends) would assess a pin that has not landed, close the real alarm on it, and write that unlanded pin into the marker governing every later close. It costs nothing the manual run is for: GitHub runs the workflow FILE from the dispatched ref either way, so the job's own YAML and inline shell are still exercised end to end, sticky issue included — only the tree the pin is read out of comes from where the pin ships. It does not exercise an uncommitted change to scripts/check-workflow-pins.mjs, which is invoked out of the checkout; that script is covered by workflow-pins on ordinary PR runs. And overlapping runs are serialised by a PAIR of concurrency settings, neither of which is sufficient alone. Every issue step is a non-atomic list-then-mutate (gh issue list, then create/reopen/comment/close) against one shared sticky issue, so overlapping runs could file duplicates, or have the slower one reopen a thread the faster one just correctly closed. Dispatch vs cron: the workflow-level key includes github.event_name, which stops a push cancelling the cron but also let a manual run execute concurrently with it; the job now carries its own group WITHOUT the event name, which queues rather than cancels, since cancelling would abandon a half-applied mutation. Dispatch vs dispatch: two dispatches of the same ref DO land in the identical run-level group, and a job-level group cannot override run-level cancellation — so the second run would have cancelled the first outright, mid-mutation, which is precisely the state the job group was added to avoid. The workflow-level concurrency therefore drops cancellation for the two events that reach this job (cancel-in-progress: ${{ github.event_name != 'workflow_dispatch' && github.event_name != 'schedule' }}); every other event keeps true unchanged, since on a PR or a push only the newest commit's verdict matters. Cron vs its own re-run: schedule is on that list even though the 07:17 cron cannot overlap itself under a 10-minute job timeout — “Re-run all jobs” keeps the original schedule event and ref, so a re-run lands in the identical group and would cancel an in-flight cron run mid-mutation, exactly like the dispatch case. A re-run is a deliberate act taken when the cron run looked wrong, which is precisely when it is most likely to still be running. The cost is that two same-ref runs of these events also run the ordinary test matrix twice instead of superseding — and on schedule, not even that, since test is skipped there. A manual dispatch or re-run is a rare deliberate act, and duplicated runner minutes are cheaper than a corrupted alarm.

current, delivered and the assess step are unchanged.

4. Merged the base branch, three times. Round 12 (9d84ec9). It rewrote the same watchdog step this PR is changing, so the two conflicted. Both conflicts were resolved keeping both sides, and two of base's fixes that this PR's rewrite had silently undone are re-applied on top of the new four-branch structure: the paragraph break (a "" element inside a single printf '%s ' list is joined with a space, emitting one extra SPACE rather than the blank LINE markdown needs — each grace branch now builds its second paragraph separately and joins with printf '%s\n' "$premise" "" "$para2", so all four carry the fix, not just the one base had), and the de-hardcoded default branch (base resolves it once from refs/remotes/origin/HEAD; this PR's rewrite had reintroduced a literal github-workflows@main in the shared premise and in the frozen-pin comment's WATCHED_PATHSPECS path, both now $UPSTREAM_BRANCH). Base's per-verdict step1 remediation and this PR's pin marker are independent additions to the stale-report body and both survive. dependabot.yml and check-workflow-pins.mjs merged clean and are identical to base.

Round 18 (673888f). Same step, same kind of conflict; this branch's movement-proof rewrite was kept and base's one substantive change carried into it (the grace window is quoted from $GRACE_DAYS rather than spelled 3-day by hand).

Latest. Base moved the watchdog's issue lookups off exact-title matching and onto per-mode labels (cursor-review-pin-stale / cursor-review-pin-blocked, both created idempotently and both applied at gh issue create), because a title is human-editable and a maintainer retitling the sticky issue made every later lookup miss; and it made every CLOSE path close every open match rather than only the first. That lands on the same Close the stale-pin issue step this PR rewrites, so it conflicted again. Resolved keeping both: the sticky thread is now found by MODE_LABEL with min_by(.number) — the same lookup the open/update path uses, which matters here because the movement proof has to be read off exactly the thread the stale step records its markers on — and the title compare is gone. That last part is load-bearing rather than cosmetic: base had already dropped TITLE from this step's env:, so keeping this branch's select(.title == env.TITLE) would have left env.TITLE unset and the lookup silently matching nothing, i.e. an alarm that never closes. A missing open match now exits early instead of falling into a marker read that has no thread to read; current still closes unconditionally and grace still runs all four marker branches, three of which hold the issue open; and the two branches that DO close fall through to base's close-every-open-match loop instead of closing the one issue alone. The sibling steps (stale-report open/update, bump-PR note, watchdog-blocked open and close) merged clean, so this PR's once-per-pin dedup and its comment-before-reopen ordering now sit on top of base's label lookups.

How this was tested

workflow_dispatch made the job runnable on a branch, so the paths below were exercised against the real workflow, on real runs, against real issue #171.

#	Case	Run	Outcome
1	`stale` on this PR's branch	35589137312	Opened issue #171; its body ends with `<!-- cursor-review-pin: 425c154… -->` (change 1, live)
2	`grace`, recorded pin EQUAL	35589289709	#171 stayed OPEN, got the "backlog shrank without the pin moving" comment, and the run log carries the matching `::warning::`
3	`grace`, recorded pin DIFFERENT	35589391062	#171 CLOSED with "The pin moved from `425c154…` to `ebfd9e4…` since the last stale report, so a bump landed"

Runs 2 and 3 came from two throwaway branches (GRACE_SECONDS raised to one year to force a grace verdict, and for run 3 the caller pin rewritten to an older SHA so the recorded pin differed). Both branches are deleted; this PR's branch carries neither edit — GRACE_SECONDS is still "259200" # 3 days and the caller pin is still 425c154ce5049324ee23ad10e106baeed4cfa31b.

The author filter on the comment read was verified live against #171's real github-actions[bot] comments, and the exact two-call read pipeline returns the recorded marker. That live check used the original .user.type == "Bot" spelling; the review round replaced it with .user.login == "github-actions[bot]" (see below), which is strictly narrower over the same #171 comments.

Run 3's quoted message ("…since the last stale report") is what that run really emitted; the review round reworded it to "…since it was last recorded on this issue", because the read can legitimately fall back to the marker in the issue body, which is the filing report rather than the last one.

The ticket's premise that upstream narrows its watched list was checked against the two upstream commits it names: Comfy-Org/github-workflows@90da1d5 (#324) is "exclude tests/ and README.md from the agents-md, coderabbit-config, groom and cursor-review fleets", and @422cfa4 (#302) adds WATCHED_EXEC to five fleets. Both move the watched set under a frozen pin, which is exactly the failure mode this closes.

Local, on this branch: node scripts/check-workflow-pins.mjs (1 caller, pins agree), node scripts/check-public-repo-hygiene.mjs (clean), pnpm lint, pnpm format:check, pnpm typecheck, pnpm test (26 files, 620 tests, all passing), plus shellcheck clean on both rewritten step scripts and a stubbed-gh harness driving all four grace branches.

On capability: this change adds paths that decline to close an issue rather than paths that refuse a caller, and both outcomes of the equal/differing decision were produced on live runs rather than asserted by a test. The review round does narrow one thing on purpose — a workflow_dispatch run no longer judges the dispatcher's own tree — and that is the point of the fix, not a side effect; the manual run's actual purpose (exercising the job and its sticky-issue behaviour on demand) is preserved, because the dispatched ref still supplies the workflow file that runs.

Judgment calls

Comments are filtered to one author identity (select(.user.login == "github-actions[bot]")), which the plan did not ask for. Anyone can comment on an issue in a public repo, and this marker is the movement proof — an unfiltered read would let a drive-by comment carrying a different 40-hex value close the very issue this check exists to hold open. This started as .user.type == "Bot"; the review round narrowed it, because type authenticates by actor class and every GitHub App installed on this public repo is a Bot, so that spelling trusted all of them — including a compromised one — rather than just the identity this job posts under. The body is read unfiltered because this job opens it. If the workflow token ever stops posting as github-actions[bot], the filter finds nothing. On an issue that has never been closed the read still falls back to the marker in the issue body — a genuine report, the filing one. On a reopened issue the body is out of scope by the rule above, so the read finds nothing and lands on the no-marker branch, which records the pin and holds. Both degrade safely rather than going wrong. (Two earlier drafts got this backwards in opposite directions; it is now stated per case.)
The two reads are joined with &&, not ; as the plan sketched, so a failure of either surfaces as a non-zero status instead of being masked by the other succeeding.
?per_page=100 on the comments call, to cut the number of paginated requests on a long-lived sticky issue.
The not-moved branch comments once per pin, not once per run. The plan had it commenting on every such run, which meant a genuinely frozen pin — precisely the state this branch exists to sit in — accruing an identical comment every day indefinitely. That is not only noise: the same thread is what the next run must --paginate in full, so the read grew without bound and got steadily likelier to hit the unreadable branch, which now holds the issue open. The reason stays visible on the thread via the one comment, and every held run still annotates its own log.
The ::warning:: carrying gh's stderr escapes % before flattening line breaks. Workflow-command messages are percent-decoded by the runner, so an unescaped percent-encoded URL in stderr would be mangled or would inject extra log lines.

Residual

The no-marker and read-failure branches were not exercised on a live run. Cases 2 and 3 above are live; the "no marker to compare against" case and the "read failed" case were verified only against a local harness with a stubbed gh, because no marker-less sticky issue existed and manufacturing one meant opening a decoy issue on a public repo. Both now close nothing — the conservative direction — but nothing has run them against the real GitHub API. The same applies to everything added since those three workflow_dispatch runs, which predate it: the once-per-pin dedup on both the held-open branch and the stale step, the default-branch checkout, and the cycle-scoped marker read (including its issues/<n>/timeline call, whose shape is asserted only against a stub).
Issue #171 carries verification artifacts. It was opened by run 1 (a true finding — the pin really is stale, and #153 is the fix), closed by run 3's scratch branch, then reopened by hand with a comment explaining that the close was a branch-run artifact. Its two grace comments and the one-year grace window they were judged against come from those scratch branches, not from a scheduled run. Anyone reading that thread should skip the first four comments.
delivered is untouched and still suspends the watchdog on the strength of an open bump PR, bounded only by BUMP_PR_GRACE_SECONDS. That path has its own open question and is explicitly out of scope here.
The upstream investigation's own notes were not readable from this environment, so the failure mode above is established from the two upstream commits quoted in this PR and from the job's own code, not from that write-up.
The cursor-review-pin-report: and cursor-review-pin-unknown: markers, the cycle-scoped dedup read, and sdk-parity's new concurrency group and timeout have not run live. All are verified by a YAML parse, bash -n, shellcheck, and — for the markers and the dedup — the real blocks extracted from the parsed workflow and driven through a stubbed gh. GitHub's run-level concurrency cannot be exercised from a worktree, and a timeout-minutes can only be observed by wedging a run.
Base PR #153's per-mode-label lookups have no backfill, and that is not fixable here. Issue #171 is open today carrying only the legacy cursor-review-pin label, so once #153 merges every --label "$MODE_LABEL" lookup misses it: the stale path files a duplicate and neither close loop can ever close #171. The label scheme is entirely the base's (20+ sites; this PR's base...HEAD diff has one such lookup, inherited into the close step by a merge resolution), so it has to be fixed on #153 — a one-shot backfill or a legacy title fallback. Raised by 3 of 6 round-19 reviewers, triaged on that thread and recorded as a follow-up.
sdk-parity fails on every run of this branch (including run 1 above). That is pre-existing drift against the Python SDK, tracked by PR #167, and is unrelated to this change.

Provenance

Authored by: agent-work loop
Verified: review round on head 31f9a66, which this round did not move: no commit was pushed, and the only change is to this description. On arrival nothing was outstanding on any mechanical axis — mergeable: MERGEABLE with the base tip 56a738b already an ancestor (nothing to merge), all 27 review threads resolved, every PR check passing, and no merge-queue run on any gh-readonly-queue/.../pr-173-* ref, so no ejected merged result to reproduce. The two newest commits (723e52f, 31f9a66) were pushed AFTER the last panel round, which ran on 6a6d5b2, so they carry no panel findings of their own; rather than take the prior round's word for them they were re-verified from scratch here. The whole local gate was re-run in a clean isolated worktree: pnpm test 620 passed / 0 failed (26 files); pnpm lint, format:check and typecheck clean; check-spec-drift.mjs, check-workflow-pins.mjs (1 caller, pins agree) and check-public-repo-hygiene.mjs clean; ci.yml parses (6 jobs) and bash -n plus shellcheck -S warning are clean across all 21 run blocks — independently reproducing every count the prior round claimed. The real baseline/held_reported case blocks were then extracted verbatim from the PARSED workflow and driven through 11 thread shapes. That confirms the laundering chain the last commit exists to close: -unknown → -held → -held now holds at baseline=unproven at runs 3 AND 4, where keying on the newest marker's form had flipped run 3 to proven; a plain in-cycle cursor-review-pin: marker for THIS pin is the only record that selects proven; cursor-review-pin-report: cannot vouch as a plain marker; and held_reported still suppresses only on an exact -held: $PIN newest marker. No regression — every shape that closed before still closes, and the shapes where recorded != $PIN never reach the baseline test at all. Two stale claims in this description were corrected: 1 carried a duplicated clause, and 2 quoted the PRE-REWORD close message ("…since the last stale report") as current behaviour when the code says "…since it was last recorded on this issue" — a drift ## How this was tested already records for the run-3 table row, but which had not been carried into the behaviour summary. Prior round (head 31f9a66): review round on head 31f9a66. On arrival this PR had nothing outstanding on the mechanical axes: mergeable: MERGEABLE with the base already merged in at 56a738b, all 27 review threads resolved, every PR check passing, and no merge-queue run on any gh-readonly-queue/.../pr-173-* ref (so no ejected merged result to reproduce). The round-20 findings remain addressed as described below. This round's one change came from adversarial self-review of that fix and is its own commit: the no-marker branch's -unknown marker stopped the NEXT run asserting the WATCHED_PATHSPECS cause without a baseline, but the run after that one saw the -held marker the weak branch had just written, scored it baseline=proven and asserted the cause anyway — the same wrong claim, one cycle later, on the ship-day input where a landed bump is what gets recorded with no baseline. baseline is now computed over the whole cycle's records (proven iff a plain cursor-review-pin: marker for this pin was filed in this cycle) instead of from the newest marker's form, and the marker-writing comment no longer claims a -held record can upgrade the chain — see 2. Comparison is a case glob over the already-read $reports, not | grep -q, whose SIGPIPE/141 under the set -o pipefail in force would read a match as a miss. No branch that closed before closes now; held_reported, the four branches and the marker greps are unchanged. Prior round (head 723e52f) addressed all seven open round-20 panel findings, none deferred and none judged wrong. Three Medium: the stale step's dedup read took the whole thread while the close step scoped its read to the alarm cycle, so a hand-reopened alarm was suppressed on the previous cycle's markers (both reads are now scoped by the same closed anchor — see 1); REPORT_KEY did not cover the FUTURE_DATED branch, and the body's day-varying upstream facts were being carried as current by a suppressed report, step 3 included (key extended, body relabelled a snapshot, step 3 now points at the run log — see 1); and the no-marker branch recorded a -held marker byte-identical to the held branch's (distinct -unknown form — completed by this round's fix above). Three Low and a Nit: sdk-parity had no timeout-minutes under its new non-cancelling group (now 15 — see 3); the dedup read's 2>/dev/null hid a broken dedup (stderr captured, ::warning:: emitted); and two comment-accuracy fixes (the !cancelled() note contradicted the concurrency block in the same diff, and "once per pin" is "once per report" now). It changed .github/workflows/ci.yml only. Local, in a clean isolated worktree (base already an ancestor, nothing to merge): pnpm test: 620 passed, 0 failed (26 files); pnpm lint / format:check / typecheck: clean; node scripts/check-spec-drift.mjs, node scripts/check-workflow-pins.mjs (1 caller, pins agree) and node scripts/check-public-repo-hygiene.mjs: clean. On the workflow itself: ci.yml parses, and bash -n and shellcheck -S warning are clean across all 21 run blocks. The real baseline/held_reported block was extracted from the parsed workflow and driven through 11 thread shapes — all three marker forms, the laundering chain at runs 2/3/4, a plain report filed on a different pin, a re-report after a hold, and the -report marker (which must not vouch as a plain one) — with every expected verdict. No live workflow_dispatch run was made this round or the last; the three runs listed under How this was tested predate everything added since.
Deviations: this round's baseline-provenance fix is verified by the extracted-block harness described above and by bash -n/shellcheck, not on a live run — the held-open branch it corrects is reached only from a scheduled grace verdict on an alarm whose cycle carries no plain marker, which cannot be staged without a decoy issue on a public repo. Likewise the no-marker branch, the read-failure branches, the report dedup on both the stale and held-open paths, the default-branch checkout, the cycle-scoped marker reads, the cursor-review-pin-report: and cursor-review-pin-unknown: markers, and sdk-parity's new concurrency group and timeout are verified locally (YAML parse, bash -n, shellcheck, greps and case branches driven directly) rather than on a live run — see ## Residual. The issues/<n>/timeline read in particular has not been exercised against the real API — and this round added a second such call, in the stale step — while GitHub's run-level cancellation cannot be exercised from a worktree at all. CI on this PR runs no test/lint gates whatsoever, because the base is not the default branch (CodeRabbit reports "reviews are disabled for this base branch"), so the local results above are the only functional evidence for this round — the green check list on this PR is the cursor-review panel and policy checks, not a test run. One known defect is knowingly left unfixed here because it is the base's, not this PR's: the missing mode-label backfill described under ## Residual, which must be fixed on #153 before that branch merges.

_{🔄 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/173 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/21/2026 **Status:** ✅ Merged **Merged:** 10/9/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `matt/be-15270-resync-cursor-review-pins` ← **Head:** `matt/be-16376-record-pin-on-stale-issue` --- ### 📝 Commits (10+) - [`b7ea71e`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/b7ea71e9e1a2f1cd69599fde6c37bf283112602a) ci(cursor-review): prove the pin moved before closing the stale-pin issue - [`d320b45`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/d320b4509f62b92de7c65f0a30f17bbc9d618862) Merge base 'matt/be-15270-resync-cursor-review-pins' into the pin-marker branch - [`1e95954`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/1e959541b4e0c14aa95ebe606682610100e36d3e) ci(cursor-review): stop the movement proof failing open, and fence the manual run - [`9d5b324`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/9d5b324d004b0ad0478cd256c86722ff2e4e0cd2) ci(cursor-review): stop a second dispatch cancelling the first mid-mutation - [`fac9c52`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/fac9c5260fea58b6730f5a26f470ce3def73116c) Merge base 'matt/be-15270-resync-cursor-review-pins' into the pin-marker branch - [`33ed546`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/33ed546c0af32a738ba1b8e565d43421f18fc7da) ci(cursor-review): scope the pin marker to the alarm cycle, hold when unproven - [`af95676`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/af95676dc67925545cfdf831f8f5884f352777e8) Merge base 'matt/be-15270-resync-cursor-review-pins' into the pin-marker branch - [`6a6d5b2`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/6a6d5b20272116f9727fe0a946cde89fda1ba733) ci(cursor-review): re-serialise sdk-parity, and key the stale report on the bump state - [`6db283f`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/6db283fe79543fa62dbed48f4d39f39de75e7f17) Merge base 'matt/be-15270-resync-cursor-review-pins' into the pin-marker branch - [`723e52f`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/723e52f2d48983551b04199b11eea3d13db46fef) ci(cursor-review): scope the dedup read, and stop the reports asserting stale facts ### 📊 Changes **1 file changed** (+830 additions, -56 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/ci.yml` (+830 -56) </details> ### 📄 Description **STACKED — merging lands on `matt/be-15270-resync-cursor-review-pins` (owned by @mattmillerai, PR #153), NOT `main`.** The `cursor-review-pin-freshness` job this changes exists only on that branch; `main`'s `ci.yml` has no such job. PRs #169 and #170 stack on the same base. ## ELI-5 The pin-freshness watchdog opens a sticky issue when this repo's `Comfy-Org/github-workflows` pin goes stale, and closed that issue again as soon as nothing was "overdue". But "nothing overdue" is measured purely from the age of the oldest upstream commit this repo has not picked up — and that commit disappears from the walk, with the pin untouched, whenever upstream narrows the list of paths it watches. So the watchdog could close its own issue on a completely frozen pin. This makes it record the pin in every report it files and refuse to close unless the pin it sees now is a different one. ## What changed **1. Every stale report records the pin.** The body the stale step writes now ends with an `` HTML comment — invisible to a reader. That body is re-commented **once per report** rather than once per run — keyed on the pin, the bump-PR state *and* the date basis of the grace verdict, since the report's premise and its step-1 remediation both branch on `BUMP_PR`/`BUMP_DELIVERS`, so the same frozen pin produces a materially different report once a bump PR opens against it, stops carrying the owed commit, or ages back out of `delivered`. A sibling `` marker records that key; it is a separate marker precisely so the close step's movement proof, which matches `cursor-review-pin(-held|-unknown)?:` against an exact 40-hex string, cannot see it. The fourth field is the `FUTURE_DATED` selector, because the report's `overdue` paragraph branches on it as well — without it, a frozen pin whose oldest owed commit turns out to be future-dated kept a durable report giving the other diagnosis. The key omits the bump PR's *age* **and** the day-varying upstream facts (`NEWEST_SHA`, `NEWEST_DATE`, `BEHIND_COUNT`), which move whenever *upstream* lands a watched commit, pin or no pin, and so would defeat the suppression on exactly the long-frozen pins it exists for. Those are handled in the **body** instead: it labels both a snapshot of one run, and step 3 no longer names `NEWEST_SHA` as the SHA to hand-bump to — it sends the reader to the newest daily run's log for the current tip. That was the one with teeth: a maintainer following a suppressed, weeks-old report would move the pin to a SHA still far behind, and the close step reads *any* movement off the recorded marker, so it would then close the alarm asserting a bump landed. Deduping at all matters because a frozen pin, the very state this watchdog exists to report, otherwise accrued one whole report a day on the same thread the close step must `--paginate` in full. On reopen it is posted *before* the reopen, so an open issue always carries the current cycle's marker. The last marker recorded **in the current alarm cycle** — at or after the moment the issue was last closed — is the pin as of the most recent report. This dedup read is scoped to that same cycle, and by the same anchor, as the close step's movement-proof read: unscoped, the two steps disagreed about which markers exist, so a hand-reopened alarm (or one whose reopen run died after posting) was suppressed on the *previous* cycle's markers — left with a stale Run URL and backlog count, a `::notice::` claiming a report was already there, and a close step that then found nothing in-cycle and held it open an extra day. A failed read still falls through to commenting, and now says why in a `::warning::` rather than swallowing the error, since a persistent failure silently reverts the step to commenting once per *run* — the unbounded growth the dedup exists to stop. No escaping is needed: `assess` asserts the value is 40-hex before writing it to `$GITHUB_OUTPUT`, so it cannot break out of the comment. **2. The `grace` close proves movement first.** `current` still closes unconditionally. `grace` now reads the recorded pin back and branches four ways: a *different* pin closes with "The pin moved from `x` to `y` since it was last recorded on this issue, so a bump landed"; an *equal* pin does NOT close — it comments why the backlog shrank without the pin moving, emits a `::warning::` and exits 0 (the verdict is still `grace`, i.e. passing) — and *what it is willing to claim there depends on whether this alarm cycle ever recorded the pin while the backlog was still overdue*. Only the stale step's plain `cursor-review-pin:` marker is written in that state — `-held` and `-unknown` are both written from `grace`, i.e. after the backlog shrank — so the test is over the whole cycle's records: with a plain marker for this pin in the cycle, upstream narrowing `WATCHED_PATHSPECS` is named as the likely cause; without one, the comment says only that the pin has not moved *since it was first sighted* and states outright that a bump landing before that is indistinguishable from a narrowed watch list. Two rounds were needed to get this right. Byte-identical markers first made the no-baseline case assert the other case's cause as fact; splitting out `-unknown` fixed the run that reads it directly but not the one after, because that run's `-held` marker — written by the weak branch itself — read as a real predecessor and laundered the chain back to the strong claim one cycle later. Keying on the cycle's plain markers rather than the newest marker's form is what actually closes it: a `-held` record is still honest that the pin has not moved since it, but it cannot vouch for a predecessor it never had. This matters on exactly the ship-day input the branch exists for, where a bump that lands and reaches `grace` is what gets recorded with no baseline; *no marker in this cycle* does NOT close either — there is nothing to compare against, and that is precisely the state in which a real bump and a narrowed `WATCHED_PATHSPECS` cannot be told apart, so it RECORDS the current pin as a distinct `` marker, emits a `::warning::` and holds the issue one more cycle; it cannot stick open, because that marker is what closes the issue tomorrow if the pin moved; a *failed read* is UNKNOWN, which is neither "moved" nor "no marker", so it does NOT close either — it emits a `::warning::` carrying gh's stderr and leaves the issue open for the next scheduled run to read again. The equal-pin branch comments **once per pin**, not once per run: it ends its comment with a sibling `` marker and skips the comment when that is already the newest marker on the thread. The `::warning::` still fires on every held run, so nothing goes unreported. A later stale report writes a plain marker that is newer, so a fresh entry into the state is still announced. The movement comparison reads the newest marker of *any of the three* kinds, so a held or unknown record counts as a record; the three forms cannot cross-match, and the `-report` marker stays invisible to both greps. **A marker only counts if it was recorded in the current alarm cycle** — at or after the moment the issue was last closed. Unscoped, the “different pin” test proves *inequality* rather than “a bump landed since the alarm came back”, and two ordinary sequences reach it: a run dying between the reopen step's two API calls, and a maintainer reopening a wrongly-closed alarm by hand. Both leave an open issue whose newest marker is a *previous* cycle's pin, so the next `grace` run closes the alarm claiming a bump landed — on a pin nothing has touched. Scoped, both land on the no-marker branch and hold. The anchor is the last `closed` event rather than the last `reopened` one precisely because the report is posted *before* the reopen: anchoring on the reopen would exclude the very marker the reopen just wrote. A reopened issue therefore reads its comments only; the filing **body** is read only when the issue has never been closed, since otherwise it belongs to an earlier cycle. `current` still closes with no movement proof, and that is a decision rather than the same hole one door over: `current` means nothing is owed **at all**, so this issue's premise — a backlog frozen past the grace window — is gone whatever removed it. A narrowed `WATCHED_PATHSPECS` can reach `current` on a frozen pin, and if it does, this watchdog has nothing left to say; a pin that is *wrong* rather than merely behind is `workflow-pins`' beat. The issue body says as much ("closes itself on the first daily run that finds the pin moved, or nothing owed"). The read is `gh api` on the issue and `gh api --paginate` on its comments, deliberately not `gh issue view --json comments` — that returns only the first 100 comments, which a sticky issue commented on daily passes inside about three months, after which the newest marker would fall off the end and the step would compare against a stale one. **3. `workflow_dispatch`, kept permanently.** The job is `if: github.event_name == 'schedule'` and `on:` had no manual trigger, so nothing about this job — verdict or sticky-issue behaviour — is reachable from any PR run, and a change to it could previously only be validated by waiting a day. It is also the only on-demand way to re-run the watchdog after a bump lands. `sdk-parity`'s issue steps gate on `github.event_name == 'schedule'` themselves and are untouched, so a manual run does its network check and touches no issue. Dropping cancellation for `schedule`, however, *did* reach `sdk-parity`: that job runs on the cron, does the same non-atomic list-then-mutate against a title-matched sticky issue, and had no job-level group of its own, so it had been relying on run-level cancellation to stay single-instance. It now carries its own `sdk-parity-${{ github.event_name == 'schedule' && 'schedule' || github.run_id }}` group with `cancel-in-progress: false`. Only scheduled runs share it — `github.run_id` is unique per run, so PR and push runs keep going fully in parallel. Because that group no longer cancels in progress, the job also gained `timeout-minutes: 15`: `scripts/sync-python-surface.mjs` is the one step here that reaches the network and fetches with no `AbortSignal`, and undici's `headersTimeout`/`bodyTimeout` are *inactivity*-based, so without a bound a trickling response would hold the group for the six-hour default and queue every later scheduled run behind it. This job *does* touch its issues on a manual run — that is the point of having one — so it carries two guards of its own. **Its `actions/checkout` is pinned to `github.event.repository.default_branch`, not the dispatched ref.** The ref is caller-chosen and the pin is read out of the working tree, so without that pin a run dispatched from a bump branch (or from a branch used to test the job, which the `on:` comment recommends) would assess a pin that has not landed, close the real alarm on it, and write that unlanded pin into the marker governing every later close. It costs nothing the manual run is for: GitHub runs the workflow FILE from the dispatched ref either way, so the job's own YAML and inline shell are still exercised end to end, sticky issue included — only the tree the pin is read out of comes from where the pin ships. It does *not* exercise an uncommitted change to `scripts/check-workflow-pins.mjs`, which is invoked out of the checkout; that script is covered by `workflow-pins` on ordinary PR runs. **And overlapping runs are serialised by a PAIR of `concurrency` settings**, neither of which is sufficient alone. Every issue step is a non-atomic list-then-mutate (`gh issue list`, then `create`/`reopen`/`comment`/`close`) against one shared sticky issue, so overlapping runs could file duplicates, or have the slower one reopen a thread the faster one just correctly closed. *Dispatch vs cron:* the workflow-level key includes `github.event_name`, which stops a push cancelling the cron but also let a manual run execute *concurrently* with it; the job now carries its own group WITHOUT the event name, which queues rather than cancels, since cancelling would abandon a half-applied mutation. *Dispatch vs dispatch:* two dispatches of the same ref DO land in the identical run-level group, and a job-level group cannot override run-level cancellation — so the second run would have cancelled the first outright, mid-mutation, which is precisely the state the job group was added to avoid. The workflow-level `concurrency` therefore drops cancellation for the two events that reach this job (`cancel-in-progress: ${{ github.event_name != 'workflow_dispatch' && github.event_name != 'schedule' }}`); every other event keeps `true` unchanged, since on a PR or a push only the newest commit's verdict matters. *Cron vs its own re-run:* `schedule` is on that list even though the 07:17 cron cannot overlap itself under a 10-minute job timeout — “Re-run all jobs” keeps the original `schedule` event **and** ref, so a re-run lands in the identical group and would cancel an in-flight cron run mid-mutation, exactly like the dispatch case. A re-run is a deliberate act taken when the cron run looked wrong, which is precisely when it is most likely to still be running. The cost is that two same-ref runs of these events also run the ordinary test matrix twice instead of superseding — and on `schedule`, not even that, since `test` is skipped there. A manual dispatch or re-run is a rare deliberate act, and duplicated runner minutes are cheaper than a corrupted alarm. `current`, `delivered` and the `assess` step are unchanged. **4. Merged the base branch, three times.** *Round 12 (`9d84ec9`).* It rewrote the same watchdog step this PR is changing, so the two conflicted. Both conflicts were resolved keeping both sides, and two of base's fixes that this PR's rewrite had silently undone are re-applied on top of the new four-branch structure: the **paragraph break** (a `""` element inside a single `printf '%s '` list is joined with a space, emitting one extra SPACE rather than the blank LINE markdown needs — each grace branch now builds its second paragraph separately and joins with `printf '%s\n' "$premise" "" "$para2"`, so all four carry the fix, not just the one base had), and the **de-hardcoded default branch** (base resolves it once from `refs/remotes/origin/HEAD`; this PR's rewrite had reintroduced a literal `github-workflows@main` in the shared premise and in the frozen-pin comment's `WATCHED_PATHSPECS` path, both now `$UPSTREAM_BRANCH`). Base's per-verdict `step1` remediation and this PR's pin marker are independent additions to the stale-report body and both survive. `dependabot.yml` and `check-workflow-pins.mjs` merged clean and are identical to base. *Round 18 (`673888f`).* Same step, same kind of conflict; this branch's movement-proof rewrite was kept and base's one substantive change carried into it (the grace window is quoted from `$GRACE_DAYS` rather than spelled `3-day` by hand). *Latest.* Base moved the watchdog's issue lookups off exact-title matching and onto per-mode labels (`cursor-review-pin-stale` / `cursor-review-pin-blocked`, both created idempotently and both applied at `gh issue create`), because a title is human-editable and a maintainer retitling the sticky issue made every later lookup miss; and it made every CLOSE path close **every** open match rather than only the first. That lands on the same `Close the stale-pin issue` step this PR rewrites, so it conflicted again. Resolved keeping both: the sticky thread is now found by `MODE_LABEL` with `min_by(.number)` — the same lookup the open/update path uses, which matters here because the movement proof has to be read off exactly the thread the stale step records its markers on — and the title compare is gone. That last part is load-bearing rather than cosmetic: base had already dropped `TITLE` from this step's `env:`, so keeping this branch's `select(.title == env.TITLE)` would have left `env.TITLE` unset and the lookup silently matching nothing, i.e. an alarm that never closes. A missing open match now exits early instead of falling into a marker read that has no thread to read; `current` still closes unconditionally and `grace` still runs all four marker branches, three of which hold the issue open; and the two branches that DO close fall through to base's close-every-open-match loop instead of closing the one issue alone. The sibling steps (stale-report open/update, bump-PR note, watchdog-blocked open and close) merged clean, so this PR's once-per-pin dedup and its comment-before-reopen ordering now sit on top of base's label lookups. ## How this was tested `workflow_dispatch` made the job runnable on a branch, so the paths below were exercised against the real workflow, on real runs, against real issue #171. | # | Case | Run | Outcome | | --- | --- | --- | --- | | 1 | `stale` on this PR's branch | [35589137312](https://github.com/Comfy-Org/comfy-typescript-sdk/actions/runs/35589137312) | Opened issue #171; its body ends with `` (change 1, live) | | 2 | `grace`, recorded pin EQUAL | [35589289709](https://github.com/Comfy-Org/comfy-typescript-sdk/actions/runs/35589289709) | #171 stayed OPEN, got the "backlog shrank without the pin moving" comment, and the run log carries the matching `::warning::` | | 3 | `grace`, recorded pin DIFFERENT | [35589391062](https://github.com/Comfy-Org/comfy-typescript-sdk/actions/runs/35589391062) | #171 CLOSED with "The pin moved from `425c154…` to `ebfd9e4…` since the last stale report, so a bump landed" | Runs 2 and 3 came from two throwaway branches (`GRACE_SECONDS` raised to one year to force a `grace` verdict, and for run 3 the caller pin rewritten to an older SHA so the recorded pin differed). Both branches are deleted; this PR's branch carries neither edit — `GRACE_SECONDS` is still `"259200" # 3 days` and the caller pin is still `425c154ce5049324ee23ad10e106baeed4cfa31b`. The author filter on the comment read was verified live against #171's real `github-actions[bot]` comments, and the exact two-call read pipeline returns the recorded marker. That live check used the original `.user.type == "Bot"` spelling; the review round replaced it with `.user.login == "github-actions[bot]"` (see below), which is strictly narrower over the same #171 comments. Run 3's quoted message ("…since the last stale report") is what that run really emitted; the review round reworded it to "…since it was last recorded on this issue", because the read can legitimately fall back to the marker in the issue *body*, which is the filing report rather than the last one. The ticket's premise that upstream narrows its watched list was checked against the two upstream commits it names: `Comfy-Org/github-workflows@90da1d5` (#324) is *"exclude tests/ and README.md from the agents-md, coderabbit-config, groom and cursor-review fleets"*, and `@422cfa4` (#302) adds `WATCHED_EXEC` to five fleets. Both move the watched set under a frozen pin, which is exactly the failure mode this closes. Local, on this branch: `node scripts/check-workflow-pins.mjs` (1 caller, pins agree), `node scripts/check-public-repo-hygiene.mjs` (clean), `pnpm lint`, `pnpm format:check`, `pnpm typecheck`, `pnpm test` (26 files, 620 tests, all passing), plus `shellcheck` clean on both rewritten step scripts and a stubbed-`gh` harness driving all four `grace` branches. On capability: this change adds paths that decline to *close* an issue rather than paths that refuse a caller, and both outcomes of the equal/differing decision were produced on live runs rather than asserted by a test. The review round does narrow one thing on purpose — a `workflow_dispatch` run no longer judges the dispatcher's own tree — and that is the point of the fix, not a side effect; the manual run's actual purpose (exercising the job and its sticky-issue behaviour on demand) is preserved, because the dispatched ref still supplies the workflow file that runs. ## Judgment calls - **Comments are filtered to one author identity** (`select(.user.login == "github-actions[bot]")`), which the plan did not ask for. Anyone can comment on an issue in a public repo, and this marker is the movement proof — an unfiltered read would let a drive-by comment carrying a different 40-hex value close the very issue this check exists to hold open. This started as `.user.type == "Bot"`; the review round narrowed it, because `type` authenticates by actor *class* and every GitHub App installed on this public repo is a `Bot`, so that spelling trusted all of them — including a compromised one — rather than just the identity this job posts under. The body is read unfiltered because this job opens it. If the workflow token ever stops posting as `github-actions[bot]`, the filter finds nothing. On an issue that has never been closed the read still falls back to the marker in the issue **body** — a genuine report, the filing one. On a reopened issue the body is out of scope by the rule above, so the read finds nothing and lands on the no-marker branch, which records the pin and holds. Both degrade safely rather than going wrong. (Two earlier drafts got this backwards in opposite directions; it is now stated per case.) - **The two reads are joined with `&&`, not `;`** as the plan sketched, so a failure of *either* surfaces as a non-zero status instead of being masked by the other succeeding. - **`?per_page=100`** on the comments call, to cut the number of paginated requests on a long-lived sticky issue. - **The not-moved branch comments once per pin, not once per run.** The plan had it commenting on every such run, which meant a genuinely frozen pin — precisely the state this branch exists to sit in — accruing an identical comment every day indefinitely. That is not only noise: the same thread is what the next run must `--paginate` in full, so the read grew without bound and got steadily likelier to hit the unreadable branch, which now holds the issue open. The reason stays visible on the thread via the one comment, and every held run still annotates its own log. - **The `::warning::` carrying gh's stderr escapes `%` before flattening line breaks.** Workflow-command messages are percent-decoded by the runner, so an unescaped percent-encoded URL in stderr would be mangled or would inject extra log lines. ## Residual - **The no-marker and read-failure branches were not exercised on a live run.** Cases 2 and 3 above are live; the "no marker to compare against" case and the "read failed" case were verified only against a local harness with a stubbed `gh`, because no marker-less sticky issue existed and manufacturing one meant opening a decoy issue on a public repo. Both now close **nothing** — the conservative direction — but nothing has run them against the real GitHub API. The same applies to everything added since those three `workflow_dispatch` runs, which predate it: the once-per-pin dedup on both the held-open branch and the stale step, the default-branch checkout, and the cycle-scoped marker read (including its `issues/<n>/timeline` call, whose shape is asserted only against a stub). - **Issue #171 carries verification artifacts.** It was opened by run 1 (a true finding — the pin really is stale, and #153 is the fix), closed by run 3's scratch branch, then reopened by hand with a comment explaining that the close was a branch-run artifact. Its two `grace` comments and the one-year grace window they were judged against come from those scratch branches, not from a scheduled run. Anyone reading that thread should skip the first four comments. - **`delivered` is untouched and still suspends the watchdog on the strength of an open bump PR**, bounded only by `BUMP_PR_GRACE_SECONDS`. That path has its own open question and is explicitly out of scope here. - **The upstream investigation's own notes were not readable from this environment**, so the failure mode above is established from the two upstream commits quoted in this PR and from the job's own code, not from that write-up. - **The `cursor-review-pin-report:` and `cursor-review-pin-unknown:` markers, the cycle-scoped dedup read, and `sdk-parity`'s new concurrency group and timeout have not run live.** All are verified by a YAML parse, `bash -n`, `shellcheck`, and — for the markers and the dedup — the real blocks extracted from the parsed workflow and driven through a stubbed `gh`. GitHub's run-level concurrency cannot be exercised from a worktree, and a `timeout-minutes` can only be observed by wedging a run. - **Base PR #153's per-mode-label lookups have no backfill, and that is not fixable here.** Issue #171 is open today carrying only the legacy `cursor-review-pin` label, so once #153 merges every `--label "$MODE_LABEL"` lookup misses it: the stale path files a duplicate and neither close loop can ever close #171. The label scheme is entirely the base's (20+ sites; this PR's `base...HEAD` diff has one such lookup, inherited into the close step by a merge resolution), so it has to be fixed on #153 — a one-shot backfill or a legacy title fallback. Raised by 3 of 6 round-19 reviewers, triaged on that thread and recorded as a follow-up. - **`sdk-parity` fails on every run of this branch** (including run 1 above). That is pre-existing drift against the Python SDK, tracked by PR #167, and is unrelated to this change. ## Provenance - **Authored by:** agent-work loop - **Verified:** review round on head `31f9a66`, which this round did not move: no commit was pushed, and the only change is to this description. On arrival nothing was outstanding on any mechanical axis — `mergeable: MERGEABLE` with the base tip `56a738b` already an ancestor (nothing to merge), all 27 review threads resolved, every PR check passing, and no merge-queue run on any `gh-readonly-queue/.../pr-173-*` ref, so no ejected merged result to reproduce. The two newest commits (`723e52f`, `31f9a66`) were pushed AFTER the last panel round, which ran on `6a6d5b2`, so they carry no panel findings of their own; rather than take the prior round's word for them they were re-verified from scratch here. The whole local gate was re-run in a clean isolated worktree: `pnpm test` 620 passed / 0 failed (26 files); `pnpm lint`, `format:check` and `typecheck` clean; `check-spec-drift.mjs`, `check-workflow-pins.mjs` (1 caller, pins agree) and `check-public-repo-hygiene.mjs` clean; `ci.yml` parses (6 jobs) and `bash -n` plus `shellcheck -S warning` are clean across **all 21** `run` blocks — independently reproducing every count the prior round claimed. The real `baseline`/`held_reported` `case` blocks were then extracted verbatim from the PARSED workflow and driven through 11 thread shapes. That confirms the laundering chain the last commit exists to close: `-unknown` → `-held` → `-held` now holds at `baseline=unproven` at runs 3 AND 4, where keying on the newest marker's form had flipped run 3 to `proven`; a plain in-cycle `cursor-review-pin:` marker for THIS pin is the only record that selects `proven`; `cursor-review-pin-report:` cannot vouch as a plain marker; and `held_reported` still suppresses only on an exact `-held: $PIN` newest marker. No regression — every shape that closed before still closes, and the shapes where `recorded != $PIN` never reach the baseline test at all. Two stale claims in this description were corrected: **1** carried a duplicated clause, and **2** quoted the PRE-REWORD close message ("…since the last stale report") as current behaviour when the code says "…since it was last recorded on this issue" — a drift `## How this was tested` already records for the run-3 table row, but which had not been carried into the behaviour summary. Prior round (head `31f9a66`): review round on head `31f9a66`. On arrival this PR had nothing outstanding on the mechanical axes: `mergeable: MERGEABLE` with the base already merged in at `56a738b`, all 27 review threads resolved, every PR check passing, and no merge-queue run on any `gh-readonly-queue/.../pr-173-*` ref (so no ejected merged result to reproduce). The round-20 findings remain addressed as described below. This round's one change came from adversarial self-review of that fix and is its own commit: the no-marker branch's `-unknown` marker stopped the NEXT run asserting the `WATCHED_PATHSPECS` cause without a baseline, but the run after that one saw the `-held` marker the weak branch had just written, scored it `baseline=proven` and asserted the cause anyway — the same wrong claim, one cycle later, on the ship-day input where a landed bump is what gets recorded with no baseline. `baseline` is now computed over the whole cycle's records (proven iff a plain `cursor-review-pin:` marker for this pin was filed in this cycle) instead of from the newest marker's form, and the marker-writing comment no longer claims a `-held` record can upgrade the chain — see **2**. Comparison is a `case` glob over the already-read `$reports`, not `| grep -q`, whose SIGPIPE/141 under the `set -o pipefail` in force would read a match as a miss. No branch that closed before closes now; `held_reported`, the four branches and the marker greps are unchanged. Prior round (head `723e52f`) addressed all seven open round-20 panel findings, none deferred and none judged wrong. Three Medium: the stale step's dedup read took the whole thread while the close step scoped its read to the alarm cycle, so a hand-reopened alarm was suppressed on the previous cycle's markers (both reads are now scoped by the same `closed` anchor — see **1**); `REPORT_KEY` did not cover the `FUTURE_DATED` branch, and the body's day-varying upstream facts were being carried as current by a suppressed report, step 3 included (key extended, body relabelled a snapshot, step 3 now points at the run log — see **1**); and the no-marker branch recorded a `-held` marker byte-identical to the held branch's (distinct `-unknown` form — completed by this round's fix above). Three Low and a Nit: `sdk-parity` had no `timeout-minutes` under its new non-cancelling group (now 15 — see **3**); the dedup read's `2>/dev/null` hid a broken dedup (stderr captured, `::warning::` emitted); and two comment-accuracy fixes (the `!cancelled()` note contradicted the `concurrency` block in the same diff, and "once per pin" is "once per report" now). It changed `.github/workflows/ci.yml` only. Local, in a clean isolated worktree (base already an ancestor, nothing to merge): `pnpm test`: 620 passed, 0 failed (26 files); `pnpm lint` / `format:check` / `typecheck`: clean; `node scripts/check-spec-drift.mjs`, `node scripts/check-workflow-pins.mjs` (1 caller, pins agree) and `node scripts/check-public-repo-hygiene.mjs`: clean. On the workflow itself: `ci.yml` parses, and `bash -n` and `shellcheck -S warning` are clean across **all 21** `run` blocks. The real `baseline`/`held_reported` block was extracted from the parsed workflow and driven through 11 thread shapes — all three marker forms, the laundering chain at runs 2/3/4, a plain report filed on a different pin, a re-report after a hold, and the `-report` marker (which must not vouch as a plain one) — with every expected verdict. No live `workflow_dispatch` run was made this round or the last; the three runs listed under **How this was tested** predate everything added since. - **Deviations:** this round's baseline-provenance fix is verified by the extracted-block harness described above and by `bash -n`/`shellcheck`, not on a live run — the held-open branch it corrects is reached only from a scheduled `grace` verdict on an alarm whose cycle carries no plain marker, which cannot be staged without a decoy issue on a public repo. Likewise the no-marker branch, the read-failure branches, the report dedup on both the stale and held-open paths, the default-branch checkout, the cycle-scoped marker reads, the `cursor-review-pin-report:` and `cursor-review-pin-unknown:` markers, and `sdk-parity`'s new concurrency group and timeout are verified locally (YAML parse, `bash -n`, `shellcheck`, greps and `case` branches driven directly) rather than on a live run — see `## Residual`. The `issues/<n>/timeline` read in particular has not been exercised against the real API — and this round added a *second* such call, in the stale step — while GitHub's run-level cancellation cannot be exercised from a worktree at all. CI on this PR runs no test/lint gates whatsoever, because the base is not the default branch (CodeRabbit reports "reviews are disabled for this base branch"), so the local results above are the only functional evidence for this round — the green check list on this PR is the cursor-review panel and policy checks, not a test run. One known defect is knowingly left unfixed here because it is the base's, not this PR's: the missing mode-label backfill described under `## Residual`, which must be fixed on #153 before that branch merges. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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