Skip to content

fix(ci): the cross-repo hand-close notice survives a refused post, and a lost one fails the job - #9594

Merged
os-steve merged 1 commit into
mainfrom
claude/issue-9575-closer-notice-guard
Aug 18, 2026
Merged

fix(ci): the cross-repo hand-close notice survives a refused post, and a lost one fails the job#9594
os-steve merged 1 commit into
mainfrom
claude/issue-9575-closer-notice-guard

Conversation

@claude

@claudeclaudeBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Fixes#9575

The degraded-path notice in .github/workflows/cross-repo-issue-closer.yml — the comment that tells a human which foreign issues a merge left open — was a single unguarded issues.createComment. actions/github-script routes any throw from the inline script to main().catch(handleError)core.setFailed, so one transient answer from the comments endpoint destroyed the notice, failed the job, and left Unhandled error: HttpError in its place on a post-merge run nobody opens.

What changed

  1. The transient-retry policy is declared, not hand-writtenretries: 3 and retry-exempt-status-codes: 400,401,404,422 on the step. This is the third instance of the family and the second to take the declared route; a third hand-written copy of the classifier that PR ci(docs-drift): tolerate transient API failures when delivering the advisory, loudly #9423 introduced is exactly what [finding] docs-drift-check.yml hand-maintains a transient-status classifier that actions/github-script already accepts as a declared input #9576 records as the thing to avoid. 403 is dropped from the action's default exempt list (400,401,403,404,422) because GitHub answers a secondary rate limit with 403 as well as with 429. The price, stated rather than hidden: a genuine permission denial — a cross-repo PAT without issues: write on a sibling repo — now takes four attempts per target to fail instead of one. It still fails, and the loop still names the target.
  2. The work order is announced before it is delivered, and it names the targets. The core.warning moved above the await and now lists the issue keys rather than only counting them.
  3. A refusal that outlives the retries writes the whole notice into the run's job summary, and then fails the job.

The guarded per-target loop is behaviourally unchanged — it keeps its try / catch + core.warning, and the step-level retries only changes how many times a target is asked. Scenarios D and E below assert that, and they pass identically before and after the change.

Why this job goes red on undelivered, when the first sibling chose green

Neither predecessor's answer was inherited; the properties were measured on this job.

  • Its conclusion is in no required set, and could not be. The context name Close issues referenced in other repositories is absent from the registry in scripts/check-required-contexts.mjs, and that pin's assertions 6 and 7 require a merge_group: trigger and an unfiltered pull_request: trigger — this file has neither. Its trigger is pull_request_target: [closed].
  • It runs only after the merge (if: github.event.pull_request.merged == true), so its conclusion gates nothing that has not already happened.
  • No workflow_run: listener watches it. The only two in the repo listen for CI and Release, so a red starts no fan-out and costs one X on an already-merged PR.
  • A green that delivered nothing is indistinguishable from the ~2268 green runs where there was simply nothing to report — and the thing lost is a work order, not a courtesy. The file's own comment says it: "Someone has to close these by hand, and this comment is the only thing that will tell them so."

That matches the answer PR #9574 reached for merge-queue-triage.yml and deliberately differs from PR #9423's for docs-drift-check.yml, where the conclusion is a check on a live PR and the comment genuinely is advisory, so a red there costs a reader's attention for nothing.

Reachability: LATENT, not live — and two premises corrected

  • The branch is unreachable today. Sampled job logs print the credential state on every run: CROSS_REPO_ISSUE_TOKEN: configured at 2026-08-04T23:49Z, 08-08T23:55Z, 08-12T23:32Z, 08-16T23:53Z and 08-18T12:18Z. It was genuinely absent on 2026-08-02T08:12Z, the day the workflow landed (CROSS_REPO_TOKEN: empty in that run's env group), so the branch is reachable in principle, and would become live again the moment the secret is rotated out or a fork of the repo runs without it.
  • The payload path is dormant too. Scanning the 489 most recent merged PR bodies with the workflow's own regex found zero qualified cross-repo closing keywords. Both conditions must hold at once for the notice to be posted at all.
  • The card's mitigation claim is false, and the loss was total, not partial. The card said "the core.warning on the next line still names the count". It does not run: it sits after the await, so a throw from the post skipped it. Reverse verification against the pre-fix script confirms an empty warning list on that path (scenario B1 below). Before this change a refused post lost the list, the count and the notice together.
  • The card's line numbers and the asymmetry it describes both still held at the branch point (1c6da6eaf): the notice at line 98, the guarded loop at line 127.
  • Not from a live incident: this workflow has 3 failed runs out of 2271, none of them from this call site — two on 2026-08-02 with SyntaxError: Identifier 'octokit' has already been declared (noticed and fixed within 39 minutes) and one on 2026-08-06 with Failed to resolve action download info. Error: Service Unavailable, which is runner infrastructure failing before the script ran.

How this was validated

A real transient cannot be triggered, so nothing here claims to have seen one live.

The shipped script was extracted from the YAML with a Python yaml parser (never retyped) and executed under stubs the way github-script does — as the body of an AsyncFunction whose scope carries github, context, core. Fifteen assertions over five scenarios, all passing on this branch:

  • A token absent, delivery succeeds → notice posted, both foreign targets listed, the same-repo reference skipped, warning names the targets, job green.
  • B token absent, createComment throws HTTP 503 → warning still names the targets, the notice is in the job summary, setFailed names the reason and the targets, and nothing escapes the script.
  • C same, plus an unwritable job summary → still fails with the target list, no throw.
  • D token present → both foreign issues commented and closed, same-repo reference skipped.
  • E token present, first target unreachable → the second is still closed, the first is a warning, job green.

Running the identical scenarios against the pre-fix script from origin/main fails 7 of the 15 in the predicted direction — A3, B1-B4, C1, C2 — while D and E pass unchanged, which is what "the loop is untouched" means here.

The retries: inputs are consumed by the action, not by the script, so this harness does not exercise them. Their acceptance on the pinned version is evidenced instead by the action echoing its own defaults into every run of this job: retries: 0 and retry-exempt-status-codes: 400,401,403,404,422 appear in the with: group of run 32136149301.

Gates

Derived from the changed path with node scripts/pm/dispatch-gates.mjs .github/workflows/cross-repo-issue-closer.yml, plus check:nul-bytes for any edit. All run after the final commit, a365e56b:

  • check:node-version — OK (27 setup-node steps across 24 workflows)
  • check:required-contexts — self-test 83 assertions, gate OK
  • check:shard-attestation — self-test 92 assertions, gate OK
  • check:workflow-status-functions — OK (24 workflows, 43 jobs, 24 job-level if:)
  • check:nul-bytes — self-test 75 assertions, gate OK (6165 files, no raw control bytes)

No changeset: this PR ships only a workflow file and publishes nothing, so it carries skip-changeset.


Generated by Claude Code

…ed post, and a lost one fails the job
The degraded-path notice in cross-repo-issue-closer.yml was a single unguarded
`issues.createComment`. github-script routes any throw to
`main().catch(handleError)` -> `core.setFailed`, so a transient answer lost the
notice AND the `core.warning` that sat after the `await` — the run said
`Unhandled error: HttpError` and nothing about the foreign issues left open.
- declare the transient-retry policy on the step (`retries: 3`,
`retry-exempt-status-codes: 400,401,404,422`) instead of hand-writing a third
copy of the classifier (#9576);
- announce the work order BEFORE delivery, naming the targets, not the count;
- on a refusal that outlives the retries, write the notice into the job summary
and then fail the job — this job's conclusion is in no required set, runs
post-merge and has no `workflow_run:` listener, so red costs one X while a
green that delivered nothing is indistinguishable from having nothing to say.
The guarded per-target loop is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 18, 2026
@os-steve
os-steve marked this pull request as ready for review August 18, 2026 12:44
@os-steve
os-steve added this pull request to the merge queueAug 18, 2026
Merged via the queue into main with commit f902d0fAug 18, 2026
23 checks passed
@os-steve
os-steve deleted the claude/issue-9575-closer-notice-guard branch August 18, 2026 13:22
os-steve added a commit that referenced this pull request Aug 18, 2026
…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
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cdsize/mskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

1 participant

@os-steve