Skip to content

fix(ci): a cross-repo closing keyword aimed at a foreign PULL REQUEST is refused, not honoured#9759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-steve merged 1 commit into from
Aug 19, 2026
+246 −15
Merged

fix(ci): a cross-repo closing keyword aimed at a foreign PULL REQUEST is refused, not honoured #9759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 100 additions & 10 deletions .github/workflows/cross-repo-issue-closer.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -307,6 +307,13 @@ jobs:
// already stated the requirement; only the code was missing.
const failures = [];

// Targets that are not issues AT ALL. Kept apart from `failures`
// above because the two need opposite remedies: a failure is an API
// refusal a re-run can clear, this one is a defect in the merged
// PR's own body that no re-run will ever change. Fusing them would
// send the reader off to re-run a job that is going to refuse again.
const malformed = [];

// The backlink this PR leaves, identified so a SECOND run can see
// it. The marker is scoped to one pull request and is therefore
// STABLE across runs — one PR leaves one backlink, however many
Expand DownExpand Up@@ -334,6 +341,47 @@ jobs:
owner: t.owner, repo: t.repo, issue_number: t.number,
});

// A pull request is ALSO an issue to this endpoint. Every PR
// answers `GET /repos/{owner}/{repo}/issues/{N}` with `state`,
// `state_reason` and the rest, plus a `pull_request` key that
// nothing here used to read (#9711) — the key is ABSENT on a
// real issue, present as an object on a PR, so its truthiness
// is the whole test. The target regex takes any `owner/repo#N`
// and PR and issue numbers share one sequence, so a body saying
// `Fixes owner/repo#4500` where 4500 is a pull request would
// have made this loop comment on that PR and CLOSE it.
//
// GitHub's own closing-keyword parser — the behaviour this
// workflow exists to carry across repository boundaries — never
// closes a pull request; `Fixes #N` aimed at a PR leaves a
// reference and nothing else. So the keyword is a MALFORMED
// instruction, and it is refused OUT LOUD rather than skipped:
// this file's ten exit paths were arrived at by fixing one
// quiet one after another, and a quiet `continue` would have
// been the eleventh. The refusal is also the conservative half
// of an asymmetry — closing a pull request drops its
// merge-queue membership and any armed auto-merge in the same
// step, and neither comes back by itself.
//
// Placed BEFORE the `state === 'closed'` branch on purpose: a
// merged pull request reads `state: 'closed'` with
// `state_reason: null` from this very endpoint (measured on
// objectstack-ai/objectstack#9143), which that branch reads as
// "no objection recorded" and answers with a backlink comment.
// A guard sitting after it would still write on somebody else's
// pull request.
if (issue.pull_request) {
malformed.push({ key, url: issue.pull_request.html_url });
core.warning(
`${key} is a PULL REQUEST, not an issue — ${prUrl} declares it FIXED, but GitHub's own `
+ `closing-keyword parser never closes a pull request, so this workflow does not either. `
+ `Nothing was commented on and nothing was closed. Re-running cannot fix a keyword that `
+ `can never fire: point the reference at the issue it meant.`,
{ title: 'Cross-repo closing keyword names a pull request' },
);
continue;
}

// "Already closed" is NOT one situation, and the three causes
// that reach here do not deserve the same treatment (#9643).
// Until this split existed all three were skipped whole, which
Expand DownExpand Up@@ -433,7 +481,7 @@ jobs:
}
}

if (failures.length === 0) {
if (failures.length === 0 && malformed.length === 0) {
core.info(`All ${targets.size} cross-repo target(s) handled: closed, or already closed and linked.`);
return;
}
Expand All@@ -442,10 +490,23 @@ jobs:
// notice branch uses, for the same reasons: the summary carries the
// full list a human needs, `setFailed` carries the conclusion that
// makes anyone open the run at all.
//
// Both classes are red, and each says what to do about ITSELF: a
// refusal is retried, a malformed reference is rewritten. Green
// with an annotation was the alternative for the malformed class,
// and this file's own history rejects it — 2334 runs, every one of
// them `run_attempt` 1 and 99 of the last 100 green, so a green run
// of this job has never been opened by anybody. An annotation on a
// green post-merge run is the silent path with extra steps, and a
// declared close that can never happen is exactly what #9595 made
// red for.
const failedKeys = failures.map((f) => f.key).join(', ');
const failedList = failures.map((f) => `- \`${f.key}\` —— ${f.lost} 未完成：${f.reason}`).join('\n');
try {
await core.summary.addRaw([
const malformedKeys = malformed.map((m) => m.key).join(', ');
const malformedList = malformed.map((m) => `- \`${m.key}\` —— 这个编号是一个 pull request：${m.url}`).join('\n');
const summaryLines = [];
if (failures.length > 0) {
summaryLines.push(
'## ⚠️ 跨仓库 issue 没能收口',
'',
`本次合并声明了 ${targets.size} 个跨仓库关闭目标，其中 ${failures.length} 个没有完成：`,
Expand All@@ -456,16 +517,45 @@ jobs:
'- 原因排除后可以直接 re-run 本 job：已经关闭的目标不会被重复关闭，已经留下过本 PR 反链的目标也不会被重复评论。',
'- 401/404 通常意味着 `CROSS_REPO_ISSUE_TOKEN` 对目标仓库没有 `issues: write`，而不是目标不存在 —— GitHub 对无权访问的仓库回 404。',
'',
].join('\n')).write();
);
}
if (malformed.length > 0) {
summaryLines.push(
'## ⛔ 关闭关键字指向的是 pull request，不是 issue',
'',
`本次合并声明的 ${targets.size} 个跨仓库关闭目标里，有 ${malformed.length} 个的编号是 pull request：`,
'',
malformedList,
'',
'- 它们没有被评论，也没有被关闭。GitHub 自己的关闭关键字解析器从不关闭 pull request，本工作流也不会 —— 关掉一个 PR 会在同一步里丢掉它的 merge queue 成员资格和已经武装的 auto-merge，两者都不会自己回来。',
`- 这不是 re-run 能解决的故障：写错的是 ${prUrl} 的正文。把引用改成它真正想关的那个 issue 编号，或者手工关掉那个 issue。`,
'',
);
}
try {
await core.summary.addRaw(summaryLines.join('\n')).write();
} catch (summaryError) {
// Same asymmetry as the notice branch: the summary is the richer
// channel, the conclusion the reliable one. Losing the richer one
// must not restore the silence.
core.info(`Could not write the job summary: ${summaryError.message}`);
}
core.setFailed(
`${failures.length} of ${targets.size} cross-repo target(s) were NOT finished by this merge: `
+ `${failedKeys}. Finish them by hand (fixed by ${prUrl}) — an open one needs closing, an `
+ `already-closed one needs this PR's link on it — or re-run this job once the cause is `
+ `cleared. The full list, with what each one is missing, is in this run's job summary.`,
);
const verdict = [];
if (failures.length > 0) {
verdict.push(
`${failures.length} of ${targets.size} cross-repo target(s) were NOT finished by this merge: `
+ `${failedKeys}. Finish them by hand (fixed by ${prUrl}) — an open one needs closing, an `
+ `already-closed one needs this PR's link on it — or re-run this job once the cause is `
+ `cleared. The full list, with what each one is missing, is in this run's job summary.`,
);
}
if (malformed.length > 0) {
verdict.push(
`${malformed.length} of ${targets.size} cross-repo closing keyword(s) in this merged pull `
+ `request name a PULL REQUEST rather than an issue: ${malformedKeys}. GitHub's own keyword `
+ `parser never closes a pull request and this workflow does not either, so nothing was `
+ `commented on and nothing was closed. Re-running this job will refuse them again — what `
+ `has to change is the reference in ${prUrl}'s body.`,
);
}
core.setFailed(verdict.join(' '));
151 changes: 146 additions & 5 deletions scripts/check-cross-repo-closer-outcome.mjs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -56,7 +56,12 @@
// ## What is asserted, and what is deliberately not
//
// Asserted: the target parse (which keyword spellings qualify, which forms do
// not, that a same-repo reference is skipped), and the OUTCOME of every exit --
// not, that a same-repo reference is skipped), the target KIND (#9711: a pull
// request is also an issue to `issues.get`, so `owner/repo#N` naming a PR
// reaches the loop like anything else -- the scenarios pin that it is REFUSED
// rather than closed, and that the refusal happens before the already-closed
// branch can leave a comment on somebody else's pull request), and the OUTCOME
// of every exit --
// which of `core.setFailed` / `core.warning` / a job summary fires, and which
// API calls were made. Those are the properties both cards are about.
//
Expand DownExpand Up@@ -198,7 +203,21 @@ function makeDoubles({ body, token, issues = {}, prCommentError = null, summaryE
// `state_reason` is nullable on a real closed issue -- objectui#4478
// answers `null` from this very endpoint -- so the default models that
// rather than inventing a value the API does not promise.
return { data: { state: t.state ?? 'open', state_reason: t.stateReason ?? null } };
//
// `pull_request` is how the same endpoint says the number is a PULL
// REQUEST, and it is modelled as ABSENCE rather than as `undefined`
// because that is what was measured: objectstack-ai/objectstack#9716 (a
// PR) answers `{ url, html_url, diff_url, patch_url, merged_at }`, and
// #9711 (an issue) carries no such key at all. A fixture that always
// spelled the key would let a truthiness test pass here that the real
// API would fail. (#9711)
return {
data: {
state: t.state ?? 'open',
state_reason: t.stateReason ?? null,
...(t.pullRequest ? { pull_request: t.pullRequest } : {}),
},
};
},
async listComments({ owner, repo, issue_number: n }) {
const key = `${owner}/${repo}#${n}`;
Expand DownExpand Up@@ -384,6 +403,27 @@ const FOREIGN = ['objectstack-ai/objectui#456', 'my-org/some.repo#22', 'third/pa

/** The target the already-closed scenarios (L2, L6-L9) work on. */
const CLOSED_TARGET = 'objectstack-ai/objectui#456';

/** The target the PULL REQUEST scenarios (L12, L13) work on (#9711). */
const PR_TARGET = 'my-org/some.repo#22';

/**
* The `pull_request` key GitHub returns when an issue number is a pull request.
*
* Copied from a real response rather than invented: `GET /repos/
* objectstack-ai/objectstack/issues/9716` answers exactly these five fields,
* and `.../issues/9711` -- an issue -- answers with the key absent. The loop
* reads only its truthiness, but the shape is pinned so that a change in what
* the script reads out of it (`html_url` reaches the job summary) fails here
* instead of printing `undefined` into somebody's run.
*/
const PR_KEY = {
url: 'https://api.github.com/repos/my-org/some.repo/pulls/22',
html_url: 'https://github.com/my-org/some.repo/pull/22',
diff_url: 'https://github.com/my-org/some.repo/pull/22.diff',
patch_url: 'https://github.com/my-org/some.repo/pull/22.patch',
merged_at: null,
};
const PR_URL = 'https://github.com/objectstack-ai/objectstack/pull/1234';

/**
Expand DownExpand Up@@ -730,6 +770,84 @@ export const SCENARIOS = [
t(r.log.warning.length === 0, 'L11 run 2 warns about nothing -- an idempotent re-run is a normal outcome'),
],
},
{
id: 'L12',
name: 'the target is an OPEN pull request -- refused out loud, never closed (#9711)',
scenario: () => ({
body: MIXED_BODY,
token: 'pat',
issues: { [PR_TARGET]: { state: 'open', pullRequest: PR_KEY } },
}),
check: (r, t) => [
t(r.calls.get.includes(PR_TARGET), 'L12 reads the target -- the kind is only knowable from the response'),
t(
!r.calls.update.some((u) => u.key === PR_TARGET),
"L12 does NOT close the pull request -- GitHub's own keyword parser never closes one, and this "
+ 'workflow exists to carry that behaviour across repos, not to exceed it. A closed PR also loses '
+ 'its merge-queue membership and any armed auto-merge in the same step, and neither returns by itself',
),
t(!r.calls.comment.some((c) => c.key === PR_TARGET), 'L12 posts nothing on the foreign pull request either'),
t(
!r.calls.list.includes(PR_TARGET),
'L12 does not even list its comments -- the target KIND settles it before idempotency can matter',
),
t(
r.log.warning.some((w) => w.message.includes(PR_TARGET) && /PULL REQUEST/.test(w.message)),
'L12 ANNOUNCES the refusal and names the target -- a quiet `continue` here would have been the '
+ "eleventh silent exit in a file whose ten known ones were each found by somebody reading it",
),
t(
r.log.failed.length === 1,
`L12 fails the job: the merged body declares a close that can never fire, and this run is the only `
+ `thing that will ever know; got ${r.log.failed.length}`,
),
t((r.log.failed[0] ?? '').includes(PR_TARGET), 'L12 setFailed names the malformed target'),
t(
!/re-run this job once the cause is cleared/.test(r.log.failed[0] ?? ''),
'L12 verdict does not send the reader to re-run -- unlike every other red path here the refusal is '
+ 'deterministic, and only the PR body can change it',
),
t((r.log.summary[0] ?? '').includes(PR_TARGET), 'L12 summary carries it too, with the PR url'),
t(r.calls.update.length === 2, `L12 still closes the other two targets -- isolation holds; got ${r.calls.update.length}`),
t(r.threw === null, 'L12 does not let anything escape the script'),
],
},
{
id: 'L13',
name: 'the target is a MERGED pull request -- refused BEFORE the already-closed branch comments on it (#9711)',
scenario: () => ({
body: MIXED_BODY,
token: 'pat',
// A merged pull request answers `state: 'closed'` with
// `state_reason: null` from the issues endpoint -- measured on
// objectstack-ai/objectstack#9143. That is the very fixture L9 uses to
// prove a closed ISSUE gets its backlink, which makes this scenario the
// ORDERING test: a `pull_request` guard placed after the state branch
// would sail past it and comment on somebody else's pull request.
issues: {
[PR_TARGET]: {
state: 'closed',
stateReason: null,
comments: [],
pullRequest: { ...PR_KEY, merged_at: '2026-08-16T14:56:58Z' },
},
},
}),
check: (r, t) => [
t(
!r.calls.comment.some((c) => c.key === PR_TARGET),
'L13 leaves NO backlink on a merged pull request -- the path L9 pins for a closed issue must not be reached here',
),
t(
!r.calls.list.includes(PR_TARGET),
'L13 never reaches the idempotency check, which is what proves the kind guard runs FIRST',
),
t(!r.calls.update.some((u) => u.key === PR_TARGET), 'L13 does not touch its state'),
t(r.log.failed.length === 1, `L13 fails the job for the same reason L12 does, got ${r.log.failed.length}`),
t((r.log.failed[0] ?? '').includes(PR_TARGET), 'L13 setFailed names it'),
t(r.calls.update.length === 2, `L13 still closes the other two, got ${r.calls.update.length}`),
],
},
];

// ── The battery ─────────────────────────────────────────────────────────────
Expand DownExpand Up@@ -850,9 +968,11 @@ const MUTATIONS = [
{
id: 'M1',
what: 'the post-loop verdict is downgraded back to a warning (the #9595 defect, restored)',
from: 'core.setFailed(\n `${failures.length} of ${targets.size}',
to: 'core.warning(\n `${failures.length} of ${targets.size}',
expect: ['L3', 'L4', 'L5'],
from: "core.setFailed(verdict.join(' '));",
to: "core.warning(verdict.join(' '));",
// Since #9711 the verdict carries both red classes, so this one mutation
// now has to be caught by an API refusal AND by a malformed target.
expect: ['L3', 'L4', 'L5', 'L12', 'L13'],
},
{
id: 'M2',
Expand DownExpand Up@@ -922,6 +1042,27 @@ const MUTATIONS = [
to: 'failures.push({ key, reason });',
expect: ['L10'],
},
{
id: 'M12',
what: 'the pull-request guard is dropped, so a keyword aimed at a foreign PR comments on it and CLOSES it (#9711)',
from: 'if (issue.pull_request) {',
to: 'if (false) {',
expect: ['L12', 'L13'],
},
{
id: 'M13',
what: 'a refused pull-request target stops being recorded, so the run refuses it and still reports green',
from: 'malformed.push({ key, url: issue.pull_request.html_url });',
to: '',
expect: ['L12', 'L13'],
},
{
id: 'M14',
what: 'the refusal is downgraded to an info line, so nothing annotates the run it happened in',
from: 'core.warning(\n `${key} is a PULL REQUEST',
to: 'core.info(\n `${key} is a PULL REQUEST',
expect: ['L12'],
},
{
id: 'M7',
what: 'the notice path loses its verdict (the #9594 half, restored to its defect)',
Expand Down
Loading