Skip to content

Commit 6f40ed7

Browse files
os-steveclaude
andauthored
fix(ci): an already-closed cross-repo issue gets the PR backlink instead of being skipped whole (#9716)
* fix(ci): an already-closed cross-repo issue gets the PR backlink instead of being skipped whole The per-target loop skipped an already-closed issue entirely, and the skip covered two calls of which only one is redundant. Re-closing a closed issue is a no-op worth avoiding; dropping the BACKLINK is the half this workflow's own header calls the defect it exists to fix -- "no reference to the PR on the issue's own page either, so the next reader has no way to find the fix". "Already closed" is three situations and the API can separate the ones that matter: not_planned / duplicate -> post nothing, annotate. A comment claiming this PR fixed it contradicts the triage decision recorded on the issue. Measured: 26 of 144 recently closed issues in this repo carry one of those reasons, so this is not a theoretical branch. `state_reason` is nullable in practice (objectui#4478 answers null from `issues.get`), and null is read as "no objection recorded". closed by an earlier run of this job -> post nothing, it is already there. The API cannot tell this from a human close -- `closed_by` is a login and every seat here shares one identity -- so the backlink now carries a per-PR marker on both paths and the loop looks for it. closed by hand before the merge -> leave the backlink, change nothing else. Idempotency was a prerequisite rather than a nicety: there was none on the comment path, and the skip WAS the re-run guard -- the job summary told people "already-closed targets are skipped, so they are not re-commented". That sentence is now true by construction instead of by side effect. The marker is per-PR and therefore stable across runs, which 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. merge-queue-triage.yml reaches the opposite conclusion from the same trade because its marker carries a run id. The post-loop verdict no longer reports every failure as "could NOT be closed and is still open" -- a lost backlink on an already-closed issue sends the reader to do the one thing already done. Each failure records which half it lost. Harness (#9645): L2 is rewritten rather than deleted, plus L6-L11 for the re-run, the triage contradiction, the unreadable listing, the null reason, the backlink-refused verdict, and a two-run ROUND TRIP that feeds run 1's output into run 2. 52 assertions over 10 scenarios -> 88 over 16; 7 mutations -> 11. The harness also grew a guard it needed: adding `listComments` moved real behaviour and all 52 assertions stayed GREEN, because the unstubbed method threw inside the script's own try and was absorbed by the degradation branch the new code had just added. Unmodelled API calls are now recorded out-of-band and fail the scenario regardless of its own assertions. Refs #9595, #9575, #4482 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja * test(ci): close the unmodelled-API blind spot at every level of the harness doubles The guard added with the backlink fix covered `github.rest.issues.*` only, which leaves the identical bug one level up: `github.paginate`, `github.request` and `github.graphql` are all reachable from a github-script body and were plain `undefined`, so a call to one throws a TypeError inside the script's own try and is absorbed by whatever degradation that catch implements. The top-level `exec`/`glob`/`io`/`fetch` stubs threw but recorded nothing, so the same absorption applied to them. Every level now records into one sink before throwing, and `judge` already fails the scenario on the record. Verified by mutating the shipped script to call `github.paginate(...)` and `github.rest.issues.addLabels(...)`: both are named by the failure text instead of vanishing into a degradation branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2065e31 commit 6f40ed7

2 files changed

Lines changed: 520 additions & 80 deletions

File tree

‎.github/workflows/cross-repo-issue-closer.yml‎

Lines changed: 139 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,35 @@
4040
# as success". Isolation and outcome are now separate: the loop records the
4141
# keys it could not close, runs to the end, and the verdict is passed after it.
4242
#
43+
# The loop's LAST silent exit was the already-closed target (#9643). It was
44+
# skipped whole, and the skip covered two calls of which only one is redundant:
45+
# re-closing a closed issue is a no-op worth avoiding, but the BACKLINK is the
46+
# half this file's own first paragraph calls the defect — "no way to find the
47+
# fix" — and a wholesale skip dropped it and reported green.
48+
#
49+
# So the state is no longer the question; the CAUSE is, because "already
50+
# closed" is three situations and they want three answers:
51+
#
52+
# closed as not_planned/duplicate -> post NOTHING and annotate. A comment
53+
# saying this PR fixed it contradicts the triage decision recorded on the
54+
# issue; two people have said opposite things and only a human settles it.
55+
# `state_reason` is the one cause the API states outright — and it is
56+
# nullable, so a null is read as "no objection recorded", i.e. completed.
57+
# closed by an EARLIER run of this job -> post nothing, it is already there.
58+
# The API cannot tell this from the next case: `closed_by` is a login and
59+
# every seat in this org shares one identity. The per-PR comment marker is
60+
# the only evidence, so the backlink now carries one on BOTH paths — a
61+
# close performed by run 1 must be recognisable to run 2.
62+
# closed BY HAND before the merge -> leave the backlink, change nothing else.
63+
# This is the ordinary sequence and the reason the file exists.
64+
#
65+
# The marker is per-PR and therefore STABLE across runs, which is what fixes
66+
# the degradation direction when the comment listing cannot be read: skip and
67+
# say so (at-most-once). Posting blind would strand a permanent second backlink
68+
# on another repo's closed issue, which nobody here can tidy up, and a miss is
69+
# a convenience link that a re-run recovers. merge-queue-triage.yml reaches the
70+
# OPPOSITE conclusion from the same trade because its marker carries a run id.
71+
#
4372
# Failing is deliberate on BOTH exits, and it is the opposite of what
4473
# docs-drift-check.yml (#9373) chose for its advisory comment. The difference is
4574
# a property of this job, measured rather than inherited — and re-measured for
@@ -278,21 +307,113 @@ jobs:
278307
// already stated the requirement; only the code was missing.
279308
const failures = [];
280309
310+
// The backlink this PR leaves, identified so a SECOND run can see
311+
// it. The marker is scoped to one pull request and is therefore
312+
// STABLE across runs — one PR leaves one backlink, however many
313+
// times the job runs. That is deliberate, and it decides the
314+
// degradation direction below: merge-queue-triage.yml's marker
315+
// carries a run id, so a duplicate there is self-evidently one
316+
// comment per run and posting blind is safe; a stable marker
317+
// strands a permanent second copy instead, which is the reason
318+
// docs-drift-check.yml (#9423) chose the opposite. This one is in
319+
// the second class.
320+
const backlinkMarker =
321+
`<!-- cross-repo-issue-closer:${thisRepo}#${context.payload.pull_request.number} -->`;
322+
const backlink = (lead) =>
323+
`${lead}\n\n` +
324+
`(跨仓库的关闭关键字不会自动生效,本条由 \`cross-repo-issue-closer\` 工作流代为收口。)\n\n` +
325+
`---\n_Generated by [Claude Code](https://claude.ai/code)_\n\n${backlinkMarker}`;
326+
281327
for (const [key, t] of targets) {
328+
// What a refusal from here costs, for the verdict after the loop.
329+
// Until the issue has been read there is no way to know it is
330+
// already closed, so a refusal costs both halves.
331+
let lost = 'close and backlink';
282332
try {
283333
const { data: issue } = await github.rest.issues.get({
284334
owner: t.owner, repo: t.repo, issue_number: t.number,
285335
});
336+
337+
// "Already closed" is NOT one situation, and the three causes
338+
// that reach here do not deserve the same treatment (#9643).
339+
// Until this split existed all three were skipped whole, which
340+
// dropped the backlink — the half of the defect the header
341+
// names — on every one of them, and reported green.
286342
if (issue.state === 'closed') {
287-
core.info(`${key} is already closed — skipping.`);
343+
// (a) Closed as not-planned or as a duplicate. A comment
344+
// saying this PR fixed it would CONTRADICT the triage
345+
// decision on the issue, so nothing is posted. Two people
346+
// have said opposite things and only a human can settle
347+
// it, hence an annotation rather than a silent skip.
348+
// `state_reason` is the only cause the API states
349+
// outright; it is nullable (a real closed issue answers
350+
// `null` here), and null is deliberately read as "no
351+
// objection recorded", i.e. as `completed`.
352+
if (issue.state_reason === 'not_planned' || issue.state_reason === 'duplicate') {
353+
core.warning(
354+
`${key} is closed as \`${issue.state_reason}\`, but this PR's body declares it FIXED. `
355+
+ `No backlink was left: a comment saying ${prUrl} closed it would contradict that `
356+
+ `triage decision. Someone has to decide which of the two is right.`,
357+
{ title: 'Cross-repo close contradicts the issue triage' },
358+
);
359+
continue;
360+
}
361+
362+
// (b) Closed by an EARLIER run of this job — a re-run, which
363+
// this workflow's own job summary invites. The marker is
364+
// the only evidence that separates it from (c): the API
365+
// cannot, because `closed_by` is a login and every seat
366+
// here shares one identity, so "the workflow did it" and
367+
// "a human did it" read the same.
368+
let alreadyLinked = false;
369+
try {
370+
// One page is the bound. Overflowing it costs a DUPLICATE
371+
// backlink, never a lost one, and the stable marker makes
372+
// the pair self-evident.
373+
const existing = await github.rest.issues.listComments({
374+
owner: t.owner, repo: t.repo, issue_number: t.number, per_page: 100,
375+
});
376+
alreadyLinked = existing.data.some((c) => (c.body ?? '').includes(backlinkMarker));
377+
} catch (error) {
378+
// AT-MOST-ONCE, stated: the marker is stable, so posting
379+
// without knowing strands a second permanent backlink on
380+
// somebody ELSE's repo, which nobody here can tidy up. A
381+
// miss costs a convenience link on an already-closed issue
382+
// and a re-run recovers it. Loud either way — this is a
383+
// skip with a reason, not a silent one.
384+
core.warning(
385+
`${key} is already closed, but its comments could not be read `
386+
+ `(${describe(error)}), so this run could not tell whether ${prUrl} is already `
387+
+ `linked there. No backlink was posted — re-run this job to leave it.`,
388+
{ title: 'Cross-repo backlink skipped, not confirmed' },
389+
);
390+
continue;
391+
}
392+
if (alreadyLinked) {
393+
core.info(`${key} is already closed and already carries this PR's backlink — nothing to do.`);
394+
continue;
395+
}
396+
397+
// (c) Closed by a human before this PR merged — the ordinary
398+
// sequence the header is about (v17 verification, #4482).
399+
// The close is genuinely redundant; the backlink is the
400+
// whole remaining value, so leave it and touch nothing
401+
// else.
402+
lost = 'backlink';
403+
await github.rest.issues.createComment({
404+
owner: t.owner, repo: t.repo, issue_number: t.number,
405+
body: backlink(
406+
`本 issue 在本次合并之前就已经是 closed 状态,这里只补上修复它的 PR:` +
407+
`${thisRepo} 的 ${prUrl}。本工作流没有改动它的状态。`,
408+
),
409+
});
410+
core.info(`${key} was already closed — left the backlink to ${prUrl}, did not re-close it.`);
288411
continue;
289412
}
413+
290414
await github.rest.issues.createComment({
291415
owner: t.owner, repo: t.repo, issue_number: t.number,
292-
body:
293-
`已由 ${thisRepo} 的 ${prUrl} 修复并合并。\n\n` +
294-
`(跨仓库的关闭关键字不会自动生效,本条由 \`cross-repo-issue-closer\` 工作流代为收口。)\n\n` +
295-
`---\n_Generated by [Claude Code](https://claude.ai/code)_`,
416+
body: backlink(`已由 ${thisRepo} 的 ${prUrl} 修复并合并。`),
296417
});
297418
await github.rest.issues.update({
298419
owner: t.owner, repo: t.repo, issue_number: t.number,
@@ -305,15 +426,15 @@ jobs:
305426
// first: the loop continues. The failure is RECORDED instead of
306427
// dropped, and the second half is passed below.
307428
const reason = describe(error);
308-
failures.push({ key, reason });
309-
core.warning(`Could not close ${key}: ${reason}`, {
310-
title: 'Cross-repo issue left open',
429+
failures.push({ key, reason, lost });
430+
core.warning(`Could not finish ${key} — ${lost} not delivered: ${reason}`, {
431+
title: 'Cross-repo issue left unfinished',
311432
});
312433
}
313434
}
314435
315436
if (failures.length === 0) {
316-
core.info(`All ${targets.size} cross-repo target(s) closed or already closed.`);
437+
core.info(`All ${targets.size} cross-repo target(s) handled: closed, or already closed and linked.`);
317438
return;
318439
}
319440
@@ -322,17 +443,17 @@ jobs:
322443
// full list a human needs, `setFailed` carries the conclusion that
323444
// makes anyone open the run at all.
324445
const failedKeys = failures.map((f) => f.key).join(', ');
325-
const failedList = failures.map((f) => `- \`${f.key}\` —— ${f.reason}`).join('\n');
446+
const failedList = failures.map((f) => `- \`${f.key}\` —— ${f.lost} 未完成:${f.reason}`).join('\n');
326447
try {
327448
await core.summary.addRaw([
328-
'## ⚠️ 跨仓库 issue 没能自动关闭',
449+
'## ⚠️ 跨仓库 issue 没能收口',
329450
'',
330-
`本次合并声明了 ${targets.size} 个跨仓库关闭目标,其中 ${failures.length} 个被拒绝,仍是 open:`,
451+
`本次合并声明了 ${targets.size} 个跨仓库关闭目标,其中 ${failures.length} 个没有完成:`,
331452
'',
332453
failedList,
333454
'',
334-
`- 修复它们的是 ${prUrl} —— 需要手工关闭,并把这条链接留在目标 issue 上。`,
335-
'- 原因排除后可以直接 re-run 本 job:已经关闭的目标会被跳过,不会重复评论。',
455+
`- 修复它们的是 ${prUrl} —— 需要手工收口:还是 open 的要关掉,已经关掉的要把这条链接留在上面。`,
456+
'- 原因排除后可以直接 re-run 本 job:已经关闭的目标不会被重复关闭,已经留下过本 PR 反链的目标也不会被重复评论。',
336457
'- 401/404 通常意味着 `CROSS_REPO_ISSUE_TOKEN` 对目标仓库没有 `issues: write`,而不是目标不存在 —— GitHub 对无权访问的仓库回 404。',
337458
'',
338459
].join('\n')).write();
@@ -343,7 +464,8 @@ jobs:
343464
core.info(`Could not write the job summary: ${summaryError.message}`);
344465
}
345466
core.setFailed(
346-
`${failures.length} of ${targets.size} cross-repo issue(s) could NOT be closed by this merge and `
347-
+ `are still open: ${failedKeys}. Close them by hand (fixed by ${prUrl}), or re-run this job once `
348-
+ `the cause is cleared. The full list with reasons is in this run's job summary.`,
467+
`${failures.length} of ${targets.size} cross-repo target(s) were NOT finished by this merge: `
468+
+ `${failedKeys}. Finish them by hand (fixed by ${prUrl}) — an open one needs closing, an `
469+
+ `already-closed one needs this PR's link on it — or re-run this job once the cause is `
470+
+ `cleared. The full list, with what each one is missing, is in this run's job summary.`,
349471
);

0 commit comments

Comments
 (0)