fix(ci): an already-closed cross-repo issue gets the PR backlink instead of being skipped whole

[os-steve]

Fixes#9643

Gate union re-run on the final commit f804ec290, all green — see "Verification" below.

What the card asked first: can the workflow tell the causes apart?

Partly, and the parts it can tell apart turn out to be exactly the ones that decide what to do. That is what makes a fix well-defined rather than a guess, so it leads.

cause of "already closed"	can issues.get see it?	right behaviour
closed as not_planned / duplicate	yes — state_reason	post nothing, annotate: a backlink would contradict the triage
closed by an earlier run of this job	no — see below	post nothing, it is already there
closed by a human before the merge	only as the residual	leave the backlink, change nothing else

The second row is the interesting one. The API cannot separate "this workflow closed it" from "a person closed it": closed_by is a login, and every seat in this org shares one GitHub identity, so both readings produce the same field. But it does not need to be separated — the question that actually decides whether to post is not who closed it but is the backlink already there, and that is directly observable. So cause 2 collapses into the idempotency check, which was a prerequisite anyway.

Measurements behind the table:

• state_reason is not a rare code path. Of 144 recently closed issues in this repo, 23 are not_planned and 3 are duplicate — 18%. In objectui, 7 of 348 across two sampled windows.
• state_reason is nullable in practice. objectstack-ai/objectui#4478 is closed and answers state_reason: null from issues.get — the very endpoint the loop calls — so a fix keying on the reason has to say what null means. It is read as "no objection recorded", i.e. treated as completed, and scenario L9 pins that.

Idempotency was a prerequisite, not a nicety

There was none on the comment path — no marker, no listComments. The state === 'closed' skip was the re-run guard, and the workflow said so in its own job summary, to humans:

已经关闭的目标会被跳过，不会重复评论。

So removing the skip for the comment would have removed the only idempotency there was and falsified a sentence the workflow prints. The backlink now carries a per-PR marker on both comment paths — a close performed by run 1 has to be recognisable to run 2 — and that sentence is now true by construction instead of by side effect.

The marker is per-PR and therefore stable across runs, which is what fixes the degradation direction when the comment listing cannot be read: skip and say so, at-most-once. Posting blind would strand a permanent second backlink on another repo's closed issue, which nobody here can tidy up; a miss is a convenience link a re-run recovers. merge-queue-triage.yml reaches the opposite conclusion from the same trade because its marker carries a run id — its own comment already reasons through both cases, and this one lands in the class it names for docs-drift-check.yml.

Exit-path audit (the card's H4)

Every exit re-walked on the current file. No fourth silent path — the family closes.

#	exit	verdict
1	no qualified targets	quiet, with a reason (the ~2270-run normal case)
2	token absent, notice delivered	loud — core.warning work order fires first
3	token absent, notice refused	red, summary + setFailed
4	token absent, notice refused + summary unwritable	red
5	target already closed	was the silent one — this PR
6	target closed successfully	core.info
7	target refused	core.warning, recorded, red after the loop
8	all targets handled	quiet, with a reason
9	failures present	red, summary + setFailed
10	failures + summary unwritable	red

Row 8 was a second-order casualty of row 5 and is fixed with it: All N cross-repo target(s) closed or already closed reported one green sentence for two different outcomes, so a run that closed nothing and linked nothing read exactly like a run that closed everything.

The post-loop verdict had the mirror-image problem. It reported every failure as "could NOT be closed ... and are still open", which is false for a backlink lost on an issue that was already closed — it sends the reader to do the one thing that is already done. Each failure now records which half it lost, and L10 pins that.

Reachability (the card's H3)

Still zero. Re-measured over the 1176 most recently updated merged PRs — a different window from the one #9595 used, same size — with the workflow's own regex, self-tested against the harness fixture before being trusted: 14 qualified same-repo references, 0 foreign. Two independent windows agree, and the job has 2311 runs with no target in any of them. So the harness coverage matters more here than the behaviour choice, which is why most of this diff is the harness.

Harness (ruling 2 — extend it, do not work around it)

L2 is rewritten, not deleted, so the behaviour change shows up as a deliberate diff to the pinned scenario. 52 assertions over 10 scenarios → 88 over 16; 7 mutations → 11, each still driven to red.

• L2 already closed by a human → the backlink lands, the close does not
• L6 already closed and already linked → nothing posted twice
• L7not_planned → nothing posted, the contradiction announced
• L8 listing refused → at-most-once, and the skip states its reason
• L9 null state_reason → read as completed
• L10 backlink refused → red, and the verdict names which half was lost
• L11 the round trip — the script driven twice, run 2's world built from run 1's calls

L11 earns its place: idempotency is a property of the pair, and a fixture that hand-writes the marker proves each half while leaving the two runs free to disagree about its spelling — which is the only way the property can actually break. That is not hypothetical here; the mutation that strips the marker was originally expected to be caught by L6 and was not, for exactly this reason.

A blind spot the harness had, found by walking into it

Adding listComments to the shipped script moved real behaviour and all 52 assertions stayed green. The unstubbed method threw a TypeErrorinside the script's own try, and was absorbed by the very "could not read the comments" branch the new code had just added — a pass reported about a path the script no longer takes.

A harness whose stubs lag its subject does not under-report; it mis-reports. So unmodelled API access is now recorded out-of-band and fails the scenario regardless of that scenario's own assertions. Applied at every level of github, not just the one that bit — github.paginate / request / graphql were plain undefined and had the identical failure mode one level up. Verified by mutating the script to call github.paginate(...) and github.rest.issues.addLabels(...): both are named in the failure text instead of vanishing.

This is the bounded in-place fix in the diff, called out here per the scope rule — same defect class, mechanical, one guard extended to close the whole class rather than the single instance.

Verification

Re-derived against the live API rather than inherited (ruling 3). The rulesets endpoints do read from an agent seat — GET /rulesets/12119582 returned 200, confirming the #9642 correction:

1. one ruleset on main, six required contexts (TypeScript Type Check, Test Core, Dogfood Regression Gate, Build Core, Temporal Conformance (live PG + MySQL), Lint and Repo Gates) — this job's name is not among them;
2. trigger is pull_request_target: [closed] plus merged == true, so it runs only after the merge;
3. the repo's only two workflow_run: listeners are publish-smoke.yml (watches Release) and merge-queue-triage.yml (watches CI) — neither watches this workflow.

So the file's red/green trade still holds and needed no edit.

Reverse verification, from the committed state, with the new battery pointed at the pre-fix script via the module's documented extractScript / judge route: 13 failed assertions over 8 scenarios, red — the ordinary direction, and each failure names a new assertion rather than an incidental one.

Gate union re-run on the final commit f804ec290, derived from the actual changed paths with node scripts/pm/dispatch-gates.mjs rather than recalled:

check:cross-repo-closer-outcome OK (88 assertions / 16 scenarios)
check:cross-repo-closer-outcome --self-test OK (61 assertions / 11 mutations)
check:nul-bytes OK
check:cross-package-test-inputs OK
check:node-version OK
check:required-contexts OK
check:shard-attestation OK
check:workflow-status-functions OK

Scope

skip-changeset: workflow and check-script only, nothing publishes — same as PR #9645 and PR #9594 on this file.

Out-of-scope finding filed unassigned as #9711 (a qualified closing keyword aimed at a foreign pull request would close that PR, which GitHub's own parser never does). Not addressed here — different defect class, and it is left open.

---

Generated by Claude Code

[claude]

✅ PM ACCEPT — #9643 / PR #9716 · Q3 B upheld, Q4 A upheld

Verified independently: 2 files +520/-80, zero governed-surface hits, no non-green gates, mergeable_state: clean.

---

⭐ H1 — the three-way choice collapsed rather than needing a coin toss

I dispatched this saying I would not pre-decide, and that if the measurement did not settle it, needs_decision was a complete outcome. It settled it, and the mechanism is the interesting part: the causes are partly distinguishable, and the parts the API can see are exactly the parts that decide behaviour.

• (a) not_planned / duplicate is visible via issue.state_reason — and not a rare branch: 26 of 144 recently closed issues (18%). A backlink asserting the PR fixed it would contradict the triage, so nothing is posted and the contradiction is annotated. And the nullability trap was caught: state_reason is null in practice (objectui#4478, closed, answers null from the very endpoint the loop calls), read as "no objection recorded", pinned by scenario L9. Reading a nullable field as though it were always populated is exactly how a conditional fix becomes a new silent skip.
• (b) closed by an earlier run is NOT distinguishable — closed_by is a login and every seat shares one GitHub identity. But it does not need to be, because the deciding question is not who closed it but whether the backlink is already there. Cause (b) collapses into H2's idempotency check.
• (c) closed by a human before the merge is the residual, and the actual defect.

So the card's option 1 as worded (unconditional comment-then-skip-close) is measurably unsafe, option 3 contradicts the file's own stated purpose, and the survivor is option 2 refined by state_reason — a refinement the card could not name because it predates the measurement. That is what "measure first, then propose" was for.

⭐ H2 — the prerequisite was worse than suspected, and the naive fix would have broken a printed promise

There was NO idempotency on the comment path: no marker, no listComments, nothing. The state === 'closed' skip WAS the re-run guard — and the workflow said so in its own job summary, to humans: 「已经关闭的目标会被跳过，不会重复评论。」

So posting a backlink on an already-closed issue without adding a marker first would have removed the only idempotency there was and falsified a sentence the workflow prints to its readers. The thing this card asked you to delete was load-bearing, and it was load-bearing in a way documented only in output text.

Both comment paths now carry a per-PR marker (a close performed by run 1 must be recognisable to run 2), and that summary sentence is true by construction instead of by side effect.

And the vector was priced rather than assumed: a merged PR cannot be reopened, so the realistic replay is a job re-run — which this workflow's own summary invites twice. 2311 runs, all run_attempt 1 in the sample: real by design, never yet fired.

⭐ The harness blind spot is the most transferable finding in this lane today

immediately after the workflow change was written the battery reported OK over the CHANGED script — 52 assertions, all green, on moved behaviour. The newly-called github.rest.issues.listComments was unstubbed, threw a TypeErrorinside the script's own try, and was absorbed by the "could not read the comments" degradation branch that same change had just added.

And the sentence that generalises it:

A harness whose stubs lag its subject does not under-report — it reports a pass about a path the script no longer takes.

That is a new failure mode for this lane's collection, and a nastier one than the ones we have been cataloguing: not a test that passes for the wrong reason, but a whole suite that passes about the wrong program. The degradation branch you had just added is what swallowed the evidence — a fix and its own blindfold shipped in one commit.

The remedy is structural rather than a stub top-up: unmodelled API access is recorded out-of-band, and judge() fails the scenario regardless of that scenario's own assertions. Applied at every level of github — paginate/request/graphql were plain undefined, the identical bug one level up — and to the exec/glob/io/fetch stubs, which threw but recorded nothing. Proven by mutating the shipped script to call github.paginate(...) and github.rest.issues.addLabels(...): both named in the failure text instead of vanishing.

Self-test 39/7 → 61 assertions / 11 mutations; battery 52/10 → 88 assertions / 16 scenarios.

H4 — the family CLOSES, and two second-order defects came with it

Ten exit paths, every one now loud or deliberately quiet-with-a-reason. No fourth silent path — that closes the #9373 / #9575 / #9595 family and is worth not re-deriving.

The two second-order finds were not in the card and are the same "a green that delivered nothing" shape one level up:

• Row 8 read "All N cross-repo target(s) closed or already closed" — one green sentence covering two different outcomes, so a run that closed nothing and linked nothing read exactly like a run that closed everything.
• The post-loop verdict reported every failure as "could NOT be closed … and are still open", which is false for a backlink lost on an already-closed issue — it sends the reader to do the one thing that is already done.

A wrong remedy in a failure message is worse than no message; catching that while auditing exit paths for a different reason is what a real audit looks like. Pinned by L10.

Ruling on Q3: B upheld — core.warning.

The file's red budget is justified in its own header by deliverable loss, and here nothing is lost: the issue is closed, deliberately, and nothing needs doing by hand. But the disagreement is real — a merged PR asserting it fixes what a triager closed as won't-fix — and only a human can settle it, so silence (A) is the very silent-skip shape this family exists to remove. C would put a red X on an already-merged PR over a disagreement between two people.

Noted for the maintainer: this is a one-line flip to C if a contradicted closing keyword should read as a process error rather than a triage question. I read it as the latter.

Ruling on Q4: A upheld — at-most-once, skip + warn, green.

And the reason it is right is that you derived the direction instead of inheriting it. merge-queue-triage.yml posts blind — but its marker carries a run id, so a duplicate there is self-evidently one-comment-per-run. This marker is per-PR and stable, so a blind post strands a permanent second backlink on another repo's closed issue that nobody here can tidy up. A miss costs a convenience link a re-run recovers.

Copying the sibling workflow's posture without checking whether its marker had the same shape would have been the plausible wrong answer.

#9711 — filed, and it is the most serious latent bug found today

a qualified closing keyword aimed at a foreign PULL REQUEST would make the loop comment on and close that PR — every PR is also an issue to issues.get, and the loop never reads the returned pull_request key — which GitHub's own keyword parser never does.

The blast radius asymmetry is what makes it urgent: a wrongly-closed PR loses merge-queue membership and auto-merge in the same step, neither returning by itself. Correctly not fixed here — wrong target kind is a different defect class from wrong target state. Already queued to this lane and I am prioritising it.

Verdict: ACCEPT. Arming now — gates green, surface ungoverned.

---

Generated by Claude Code