Repository navigation
cross-repo-issue-closer.yml: an already-closed foreign issue is skipped whole, so it never gets the backlink the workflow exists to leave #9643
Description
Filed unassigned by the domain:devx os-dev seat while landing #9595. Recording, not claiming — this came out of that card's H3 exit-path audit of the file. It is the one exit path where a judgment is attached, so it is a card rather than a rider on that PR. Dedup-searched (workflow name, "already closed", "backlink"): only #9575 and #9595 name this file, and neither covers this path.
The path
.github/workflows/cross-repo-issue-closer.yml, the per-target loop:
const { data: issue } = await github.rest.issues.get({ ... });
if (issue.state === 'closed') {
core.info(`${key} is already closed — skipping.`);
continue;
}
await github.rest.issues.createComment({ ... }); // the backlink
await github.rest.issues.update({ ... }); // the closeThe skip covers both calls, but only one of them is redundant. Closing an already-closed issue is a no-op worth avoiding. Leaving the PR link on it is not: the file's own header names the missing backlink as half the defect the workflow exists to fix —
and leaves that issue open forever — with no reference to the PR on the issue's own page either, so the next reader has no way to find the fix
— and that second clause is exactly what the skip drops.
Why the case is realistic rather than theoretical
The scenario the header cites is the scenario that produces it. v17 verification (#4482) left objectui#4475 and objectui#4478 open, and both had to be closed by hand. A human closing a foreign issue by hand before the fixing PR merges is the ordinary sequence — the fix ships, someone tidies the tracker, then the merge happens. The loop then finds state === 'closed', skips, and the issue keeps no pointer to the PR that fixed it. The workflow reports a clean green run about it.
Same for a repeat run: re-running the job after a partial failure (now that #9595 makes such a failure red and re-runnable) hits this path for every target that succeeded the first time, which is correct for the close and, again, silently correct-looking for the backlink.
The judgment this needs, which is why it is not folded into #9595
Three defensible answers, and they are not obviously ranked:
- Comment, then skip the close. The backlink always lands; the cost is a comment on an issue somebody already closed, which some readers experience as noise on a settled thread.
- Comment only when no comment from this workflow is already there. Needs an idempotency marker in the comment body (
merge-queue-triage.ymluses a per-run HTML marker; here it would have to be per-PR) and alistCommentscall per target — more API surface, and a listing that cannot be read has to degrade in a stated direction. - Leave it. Defensible if the intended reading is "an already-closed issue has an owner who knows about it", but then the header's own reasoning should say so, because today it argues the opposite.
Whichever wins, the core.info line should probably say what was skipped and what that costs, rather than only that something was skipped.
Scenario L2 in scripts/check-cross-repo-closer-outcome.mjs currently pins today's behaviour (no comment, no close, the other targets still handled) and would be the place to move the pin.
Ref: #4482 · #9575 · #9595 · PR #9594
Generated by Claude Code
Activity
os-steve commentedon Aug 18, 2026
Claiming this card.
session_01XqDQYVU5smx29ts9pAErjaclaude/issue-9643-closed-issue-backlink../objectstack-9643(branched from currentmain, which carries PR fix(ci): a refused cross-repo close fails the job instead of passing as a warning #9645 /1cf043c48)Plan: measure H1 (can the workflow distinguish the causes of "already closed"?), H2 (backlink idempotency), H3 (is the branch reachable), H4 (exit-path audit) before proposing a behaviour. Scenario
L2inscripts/check-cross-repo-closer-outcome.mjsis the pin that moves if the behaviour changes.Generated by Claude Code
Generated by Claude Code
claude commentedon Aug 18, 2026
{ "issue": 9643, "status": "done", "branch": "claude/issue-9643-closed-issue-backlink", "pr": "https://github.com/objectstack-ai/objectstack/pull/9716", "premise_still_valid": true, "summary": "H1 LEADS, because it decides whether a fix is well-defined at all: the causes are PARTLY distinguishable, and the parts the API can see are exactly the parts that decide behaviour — so a conditional fix IS well-defined and the three-way choice collapses rather than needing a coin toss. (a) not_planned/duplicate is VISIBLE via issue.state_reason, and it is not a rare branch: 26 of 144 recently closed issues in this repo (18%) carry one of those reasons. A backlink claiming the PR fixed it would contradict the triage, so nothing is posted and an annotation names the contradiction. state_reason is NULLABLE in practice — objectui#4478 is closed and answers null from issues.get, the very endpoint the loop calls — so null is read as 'no objection recorded' i.e. completed, pinned by new scenario L9. (b) closed by an EARLIER run of this job is NOT distinguishable from a human close through the API: closed_by is a login and every seat here shares one GitHub identity, so both readings produce the same field. 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: the backlink now lands, the close is skipped. Net: the card's option 1 as worded (unconditional comment-then-skip-close) is measurably unsafe, option 3 (leave it) contradicts the file's own stated purpose, and the survivor is option 2 REFINED by state_reason — a refinement the card could not have named because it predates the measurement.", "tests": "All at final commit f804ec290 (= pushed remote tip; verified in sync, worktree clean). Gate set DERIVED from the actual changed paths via `node scripts/pm/dispatch-gates.mjs`, not recalled. UNION GREEN at f804ec290: check:cross-repo-closer-outcome OK (88 assertions over 16 scenarios, up from 52/10) · --self-test OK (61 assertions, 11 mutations each driven to red, up from 39/7) · check:nul-bytes OK (6202 files) plus a direct `grep -naP '[\\x00-\\x08\\x0b\\x0c\\x0e-\\x1f\\x7f]'` over both changed files, exit 1 = none · check:cross-package-test-inputs OK · check:node-version OK · check:required-contexts OK · check:shard-attestation OK · check:workflow-status-functions OK. main has since advanced 23 commits but touched NEITHER changed file (verified against the authoritative `git ls-remote` tip 6cb88d9f2, not a stale local ref), so the union still describes the tree under test. REVERSE VERIFICATION, from the COMMITTED state, new battery pointed at the pre-fix tree via the module's documented extractScript/judge route: 13 failed assertions over 8 scenarios (L1 L2 L6 L7 L8 L9 L10 L11) — direction RED, the ordinary one, and every failure names a NEW assertion rather than an incidental one. HARNESS BLIND SPOT, measured rather than assumed and the most transferable result here: 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 TypeError INSIDE the script's own try, and was absorbed by the 'could not read the comments' degradation branch that same change had just added. A harness whose stubs lag its subject does not under-report, it reports a pass about a path the script no longer takes. Fixed structurally: 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 are named in the failure text instead of vanishing into a degradation branch.", "open_questions": [ { "question": "H2 — IDEMPOTENCY, and it was a hard prerequisite exactly as suspected. 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. The backlink now carries a per-PR marker on BOTH comment paths (a close performed by run 1 must be recognisable to run 2), and that summary sentence is now true by construction instead of by side effect. On the repeat vector: a merged PR cannot be reopened, so the realistic replay is a job RE-RUN — which this workflow's own summary text explicitly invites twice. Measured: 2311 runs, all run_attempt 1 in the 100-run sample, so the vector is real by design and has never yet fired.", "options": ["No decision needed — reporting the prerequisite finding and how it was discharged"], "recommendation": "Nothing to decide. Flagged because 'the skip IS the idempotency mechanism, and the file documents it as such' is the finding that made the naive version of the fix unsafe." }, { "question": "H4 — FULL EXIT-PATH TABLE for the current file after #9594 and #9645. No fourth silent path: the family CLOSES. Ten exits, every one now loud or deliberately quiet-with-a-reason — (1) no qualified targets: quiet + reason, the ~2270-run normal case; (2) token absent, notice delivered: loud, the core.warning work order fires first; (3) notice refused: RED, summary + setFailed; (4) notice refused + summary unwritable: RED; (5) target already closed: WAS the silent one, this card; (6) target closed OK: core.info; (7) target refused: core.warning + recorded + RED after the loop; (8) all targets handled: quiet + reason; (9) failures present: RED; (10) failures + summary unwritable: RED. Two SECOND-ORDER defects travelled with row 5 and are fixed alongside it — neither was in the card, and both are the same 'green that delivered nothing' shape one level up. Row 8's line 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. And 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. Each failure now records which half it lost, pinned by new scenario L10.", "options": ["No decision needed — this is the audit result the card asked for"], "recommendation": "Nothing to decide. Stating it explicitly because 'no fourth path exists' closes the #9373/#9575/#9595 family and is worth not re-deriving." }, { "question": "How loud should the not_planned/duplicate contradiction be? A merged PR asserts it FIXES an issue a triager closed as won't-fix — two humans asserting opposite things. I chose core.warning (annotation, job stays green).", "options": [ "A: core.info — quiet. Cheapest, but it is precisely the silent-skip shape this whole card family exists to remove.", "B: core.warning (chosen) — an annotation naming the target and the reason; green, because nothing was LOST (the issue is closed, deliberately) and nothing needs doing by hand.", "C: core.setFailed — red. Consistent with the file's 'a green that delivered nothing' doctrine, but that doctrine is warranted where the workflow's OWN deliverable was lost, which this is not; it would also put an X on an already-merged PR over a disagreement between two people." ], "recommendation": "B. The file's red budget is justified in its header by deliverable loss, and here nothing is lost — but the disagreement is real and only a human can settle it, so silence (A) is wrong too. One-line flip to C if the maintainer reads a contradicted closing keyword as a process error rather than a triage question." }, { "question": "When the comment listing cannot be read on an already-closed target, the fix skips the backlink (AT-MOST-ONCE) and warns, staying green. merge-queue-triage.yml does the opposite (posts blind, at-least-once).", "options": [ "A: skip + warn, green (chosen) — the marker is per-PR and STABLE across runs, 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 that a re-run recovers.", "B: post blind — copies merge-queue-triage.yml, whose marker carries a RUN id, so a duplicate there is self-evidently one-comment-per-run.", "C: skip + setFailed — red on transient platform weather, for a link that may well already be there." ], "recommendation": "A. The direction is DERIVED from the marker's stability rather than inherited: merge-queue-triage.yml's own comment reasons through both cases and names docs-drift-check.yml (#9423) as the stable-marker precedent — which is the class this marker is in." } ], "out_of_scope_findings": [ "filed as #9711: 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. Blast radius is asymmetric: a wrongly-closed PR loses merge-queue membership and auto-merge in the same step, neither returning by itself. Unassigned, labelled domain:devx + finding, no pm:queue. Different defect class from this card (wrong target KIND, not wrong target STATE), so deliberately not fixed here." ] }Generated by Claude Code