Skip to content

Commit 0a7e6c3

Browse files
os-steveclaude
andcommitted
fix(ci): a refused cross-repo close fails the job instead of passing as a warning
cross-repo-issue-closer.yml's per-target loop caught every failure into `core.warning` and ran on. The isolation that buys is correct and is kept unchanged -- one unreachable target must not take the rest down -- but the job's OUTCOME was fused to it, so a refused close left the foreign issue open, put no notice on the PR, and produced a conclusion identical to the ~2270 runs where there was nothing to do at all. The catch's own comment already stated the requirement it did not meet: "a failure here must not read as success". - the loop records the keys it could not close and still runs to the end; - after the loop, a job summary lists them with reasons and `core.setFailed` names them, the shape merge-queue-triage.yml has used since #9424; - `describe(error)` is hoisted once instead of a second hand-written copy, so both exits report `HTTP 404` / `ECONNRESET` in one vocabulary (#9576). The red/green judgement was re-derived rather than inherited from PR #9594: the job's context is absent from the LIVE ruleset (six required contexts, read from `/rulesets/12119582`), it triggers on `pull_request_target: [closed]` so it gates nothing already merged, and the repo's only two `workflow_run:` listeners watch `CI` and `Release`. scripts/check-cross-repo-closer-outcome.mjs is the half PR #9594 could not leave behind: it extracts the SHIPPED script from the YAML and runs it under doubles the way actions/github-script does, ten scenarios over every exit path, with a --self-test that mutates the script seven ways and requires the battery to go red for each. Over the 1176 most recently merged PRs, zero bodies carry a qualified cross-repo closing keyword, so this loop is code nobody has seen run and every fix to it lands unexercised without a harness like this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
1 parent e8dba8a commit 0a7e6c3

3 files changed

Lines changed: 829 additions & 11 deletions

File tree

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

Lines changed: 97 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,21 @@
2929
# attempted, and a refusal that outlives the retries writes the notice into the
3030
# run's job summary and then FAILS the job.
3131
#
32-
# Failing is deliberate, and it is the opposite of what docs-drift-check.yml
33-
# (#9373) chose for its advisory comment. The difference is a property of this
34-
# job, measured rather than inherited:
32+
# The OTHER exit — the per-target close loop, the branch that runs when the
33+
# token IS present — reached the same rule one card later (#9595). Its `catch`
34+
# is unchanged and must stay so: it buys ISOLATION, and one unreachable target
35+
# taking the rest down with it would be a worse failure than the one being
36+
# fixed. What it did NOT buy is a verdict. `core.warning` leaves the job green,
37+
# so a refused close left the foreign issue open, put nothing on the PR, and
38+
# produced a conclusion identical to the ~2270 runs where there was nothing to
39+
# do at all — while the catch's own comment said "a failure here must not read
40+
# as success". Isolation and outcome are now separate: the loop records the
41+
# keys it could not close, runs to the end, and the verdict is passed after it.
42+
#
43+
# Failing is deliberate on BOTH exits, and it is the opposite of what
44+
# docs-drift-check.yml (#9373) chose for its advisory comment. The difference is
45+
# a property of this job, measured rather than inherited — and re-measured for
46+
# the loop on 2026-08-18 rather than carried over from the notice (#9595):
3547
#
3648
# - its conclusion is in no required set — the context name `Close issues
3749
# referenced in other repositories` is absent from the registry in
@@ -44,10 +56,21 @@
4456
# - no `workflow_run:` listener in this repo watches it (the only two listen
4557
# for `CI` and `Release`), so a red starts no fan-out.
4658
#
59+
# The first of those three was re-derived on 2026-08-18 against the LIVE ruleset
60+
# rather than against the repo-side pin alone: `GET /repos/.../rulesets/12119582`
61+
# lists six required contexts (`Lint & Repo Gates`, `TypeScript Type Check`,
62+
# `Test Core`, `Dogfood Regression Gate`, `Build Core`, `Temporal Conformance
63+
# (live PG + MySQL)`) and this job's name is not among them.
64+
#
4765
# A red therefore costs one X on an already-merged PR. A green that delivered
4866
# nothing costs the foreign issues this workflow exists to stop losing — and
4967
# looks exactly like the 2000+ green runs where there was simply nothing to
50-
# report. Same trade merge-queue-triage.yml (#9424) made, for the same reason;
68+
# report. That a red here is READ, meanwhile, is not an assumption either: this
69+
# job's only two script-level failures (2026-08-02, a `SyntaxError` that stopped
70+
# it running at all) were diagnosed and fixed 39 minutes after the first one,
71+
# and the second of them landed on an unrelated author's merge.
72+
#
73+
# Same trade merge-queue-triage.yml (#9424) made, for the same reason;
5174
# docs-drift-check.yml's opposite choice is right THERE because its conclusion
5275
# is a check on a live PR and its comment is a courtesy, neither of which is
5376
# true here.
@@ -121,6 +144,20 @@ jobs:
121144
const prUrl = context.payload.pull_request.html_url;
122145
const thisRepo = `${context.repo.owner}/${context.repo.repo}`;
123146
147+
// ONE vocabulary for a refusal, used by both exits below: `HTTP 503`
148+
// or `ECONNRESET` is what lets a reader separate platform weather
149+
// from a 403/404 that means the credential is wrong. Hoisted rather
150+
// than copied per call site — merge-queue-triage.yml declares the
151+
// identical helper once, and a second hand-written copy here is the
152+
// shape #9576 records as the thing to stop doing.
153+
const describe = (error) => {
154+
const kind = typeof error?.status === 'number'
155+
? `HTTP ${error.status}`
156+
: (error?.code || 'error');
157+
// Octokit messages usually end in a full stop; ours supplies its own.
158+
return `${kind}: ${String(error?.message || '').replace(/\s*\.\s*$/, '')}`;
159+
};
160+
124161
// GitHub's own keyword set, restricted to the qualified
125162
// `owner/repo#N` form — the bare `#N` form already works natively
126163
// and must not be touched here.
@@ -197,11 +234,7 @@ jobs:
197234
`Reported ${targets.size} unclosed cross-repo issue(s) on this pull request.`,
198235
);
199236
} catch (error) {
200-
const kind = typeof error?.status === 'number'
201-
? `HTTP ${error.status}`
202-
: (error?.code || 'error');
203-
// Octokit messages usually end in a full stop; ours supplies its own.
204-
const reason = `${kind}: ${String(error?.message || '').replace(/\s*\.\s*$/, '')}`;
237+
const reason = describe(error);
205238
try {
206239
await core.summary.addRaw([
207240
'## ⚠️ 跨仓库关闭提示没能发到 PR 上',
@@ -234,6 +267,17 @@ jobs:
234267
return;
235268
}
236269
270+
// ISOLATION and OUTCOME are separable, and the loop owns only the
271+
// first (#9595). The `catch` below must keep swallowing — one
272+
// unreachable target must not take the rest down — so the verdict
273+
// is passed AFTER the loop, over the keys it collected. Until this
274+
// split existed the two were fused into `core.warning` alone: a
275+
// refused close left the foreign issue open, put no notice on the
276+
// PR, and handed the run the same green conclusion as the ~2270
277+
// runs that had nothing to do at all. The catch's own comment
278+
// already stated the requirement; only the code was missing.
279+
const failures = [];
280+
237281
for (const [key, t] of targets) {
238282
try {
239283
const { data: issue } = await github.rest.issues.get({
@@ -257,7 +301,49 @@ jobs:
257301
core.info(`Closed ${key}.`);
258302
} catch (error) {
259303
// One unreachable target must not swallow the rest, and a
260-
// failure here must not read as success.
261-
core.warning(`Could not close ${key}: ${error.message}`);
304+
// failure here must not read as success. This half buys the
305+
// first: the loop continues. The failure is RECORDED instead of
306+
// dropped, and the second half is passed below.
307+
const reason = describe(error);
308+
failures.push({ key, reason });
309+
core.warning(`Could not close ${key}: ${reason}`, {
310+
title: 'Cross-repo issue left open',
311+
});
262312
}
263313
}
314+
315+
if (failures.length === 0) {
316+
core.info(`All ${targets.size} cross-repo target(s) closed or already closed.`);
317+
return;
318+
}
319+
320+
// A spent failure IS the report. Announce it in both channels the
321+
// notice branch uses, for the same reasons: the summary carries the
322+
// full list a human needs, `setFailed` carries the conclusion that
323+
// makes anyone open the run at all.
324+
const failedKeys = failures.map((f) => f.key).join(', ');
325+
const failedList = failures.map((f) => `- \`${f.key}\` —— ${f.reason}`).join('\n');
326+
try {
327+
await core.summary.addRaw([
328+
'## ⚠️ 跨仓库 issue 没能自动关闭',
329+
'',
330+
`本次合并声明了 ${targets.size} 个跨仓库关闭目标,其中 ${failures.length} 个被拒绝,仍是 open:`,
331+
'',
332+
failedList,
333+
'',
334+
`- 修复它们的是 ${prUrl} —— 需要手工关闭,并把这条链接留在目标 issue 上。`,
335+
'- 原因排除后可以直接 re-run 本 job:已经关闭的目标会被跳过,不会重复评论。',
336+
'- 401/404 通常意味着 `CROSS_REPO_ISSUE_TOKEN` 对目标仓库没有 `issues: write`,而不是目标不存在 —— GitHub 对无权访问的仓库回 404。',
337+
'',
338+
].join('\n')).write();
339+
} catch (summaryError) {
340+
// Same asymmetry as the notice branch: the summary is the richer
341+
// channel, the conclusion the reliable one. Losing the richer one
342+
// must not restore the silence.
343+
core.info(`Could not write the job summary: ${summaryError.message}`);
344+
}
345+
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.`,
349+
);

‎.github/workflows/lint.yml‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,44 @@ jobs:
785785
- name: Workflow status-function guard
786786
run: pnpm check:workflow-status-functions
787787

788+
# Cross-repo closer outcome contract (#9595, and #9575 before it).
789+
# `cross-repo-issue-closer.yml` carries ~150 lines of inline
790+
# github-script, and it is code nobody has ever seen run: over the 1176
791+
# most recently merged PRs, ZERO bodies carry a qualified cross-repo
792+
# closing keyword, so the branch that closes foreign issues has had no
793+
# target in that whole window. Its defects are therefore found by reading
794+
# — one card per silent exit — and each fix lands as more unexercised
795+
# code. This step is the exercise: the shipped script is extracted from
796+
# the YAML with a real parser (never retyped) and run under doubles the
797+
# way actions/github-script runs it, as one AsyncFunction body. Ten
798+
# scenarios pin the target parse and the outcome of EVERY exit — which of
799+
# setFailed / warning / job summary fires, and which API calls were made.
800+
#
801+
# Assertion 0 is the compile, and it is not theoretical: this job failed
802+
# twice on 2026-08-02 with `SyntaxError: Identifier 'octokit' has already
803+
# been declared`, i.e. a script that never ran at all, on a post-merge
804+
# workflow whose red nothing else in CI can see.
805+
#
806+
# Its --self-test runs first and is the half that stops the battery
807+
# rotting into decoration: it mutates the shipped script seven ways —
808+
# downgrade the verdict to a warning, stop collecting failed keys, break
809+
# out of the loop instead of isolating, drop the same-repo skip, narrow
810+
# the keyword set, drop the already-closed skip, downgrade the notice
811+
# path's verdict — and requires the battery to go RED for each, naming the
812+
# scenario that catches it. A mutation whose anchor no longer exists is a
813+
# failure too, so a rewrite of the workflow cannot leave the mutations
814+
# silently matching nothing.
815+
#
816+
# Invoked as `node` rather than through a `pnpm check:*` alias: that alias
817+
# belongs in root package.json, declared territory of the @changesets/cli
818+
# v3 migration lane (#9465) while it runs. Same shape as the
819+
# release-rehearsal step above; dispatch-gates.mjs derives gate families
820+
# from either spelling. No network, no build; ~0.2 s.
821+
- name: Cross-repo closer outcome contract
822+
run: |
823+
node scripts/check-cross-repo-closer-outcome.mjs --self-test
824+
node scripts/check-cross-repo-closer-outcome.mjs
825+
788826
# Shard positive-attestation gate (#6082). ci.yml's two aggregate gates
789827
# used to decide from one `needs.<matrix>.result` word, which cannot carry
790828
# three shards' verdicts: run 31120902911 read the undocumented

0 commit comments

Comments
 (0)