Skip to content

fix(ci): a refused cross-repo close fails the job instead of passing as a warning - #9645

Merged
os-steve merged 1 commit into
mainfrom
claude/issue-9595-cross-repo-closer-outcome
Aug 18, 2026
Merged

fix(ci): a refused cross-repo close fails the job instead of passing as a warning#9645
os-steve merged 1 commit into
mainfrom
claude/issue-9595-cross-repo-closer-outcome

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#9595

The per-target loop in .github/workflows/cross-repo-issue-closer.yml caught every failure into core.warning and ran on. The isolation that buys is correct and is kept exactly as it is — one unreachable target must not take the rest down. What was fused to it, and should not have been, is the job's OUTCOME: 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 stated the requirement the code did not meet — "a failure here must not read as success".

What changed

  1. The loop collects the keys it could not close and still runs to the end. Isolation and outcome are now separate concerns: the catch keeps swallowing, and the verdict is passed after the loop over what it recorded.
  2. After the loop, a job summary lists the failed targets with reasons and core.setFailed names them — the shape merge-queue-triage.yml has used since merge-queue-triage.yml has the same untolerated comment-post as #9373 — and here the lost comment IS the flake evidence #9424, and the same two-channel arrangement PR fix(ci): the cross-repo hand-close notice survives a refused post, and a lost one fails the job #9594 gave the notice path (summary is the richer channel, the conclusion the reliable one; an unwritable summary must not restore the silence).
  3. describe(error) is hoisted once instead of a second hand-written copy of the same four lines. Both exits now report HTTP 404 / ECONNRESET in one vocabulary. This is the only edit that touches PR fix(ci): the cross-repo hand-close notice survives a refused post, and a lost one fails the job #9594's lines, and it is a deletion of duplication rather than an addition: a second copy of the classifier is precisely the shape [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 stop doing, and this file's own header cites it. Named here because a hunk in someone else's just-merged code should not be found by reading the diff.
  4. scripts/check-cross-repo-closer-outcome.mjs — new, and wired into lint.yml. See "How this is exercised".

The verdict names only what failed. A message that also lists the successes teaches nobody, and scenario L3 asserts the exclusion.

The red/green judgement, re-derived rather than inherited

All three facts re-checked against the tree at 0a7e6c32d and against the ruleset as it stands today. All three still hold.

1. The conclusion is in no required set. Not asserted from the repo-side pin alone this time — the live ruleset was read: GET /repos/objectstack-ai/objectstack/rulesets returns exactly one active ruleset (main, id 12119582, updated_at 2026-08-18T03:26:48Z, repository-sourced, so no organization ruleset is being missed), and its required_status_checks rule lists six contexts:

TypeScript Type Check
Test Core
Dogfood Regression Gate
Build Core
Temporal Conformance (live PG + MySQL)
Lint & Repo Gates

Close issues referenced in other repositories is not among them. It is also absent from REQUIRED_CONTEXTS in scripts/check-required-contexts.mjs, and could not be enrolled there anyway — that pin's assertions 6 and 7 want a merge_group: trigger and an unfiltered pull_request: trigger, and this file has neither.

Side effect worth flagging: check-required-contexts.mjs's header says no agent seat can read the ruleset (HTTP 403). That 403 is for the classic branch-protection endpoint, which this repo does not use; the rulesets endpoints answer 200. Filed as #9642, with the measurement also posted on #9533 — whose open question ("was the six-item reading partial?") the reading above answers.

2. It gates nothing already merged.on: pull_request_target: types: [closed], plus the job-level if: github.event.pull_request.merged == true. Confirmed unchanged in the current file; all 2281 recorded runs carry event: pull_request_target. The merge-queue rule in the ruleset (grouping_strategy: ALLGREEN, check_response_timeout_minutes: 60) concerns merge_group runs, which this workflow does not have a trigger for, so it publishes no check on a queue branch.

3. No workflow_run: listener keys off it. The repo has exactly two: merge-queue-triage.yml listens for CI, publish-smoke.yml listens for Release. Neither names this workflow, and Close issues referenced in other repositories appears nowhere in the tree outside the workflow's own name:.

Does a red here actually reach anyone? (H4 — measured, and the answer is yes)

Worth answering, because a fix that swaps one invisible signal for another is not a fix. It is not invisible, and this workflow has already proved it:

  • This job has 3 failures in 2281 runs. Two of them, on 2026-08-02 at 07:56Z and 08:12Z, were SyntaxError: Identifier 'octokit' has already been declared — the script did not run at all. The fix (fix(ci): 跨仓库客户端改名——github-script 本来就声明了 octokit) merged at 08:35Z, 39 minutes after the first red, and a second follow-up at 08:57Z. The second red landed on an unrelated author's merge, so the signal travelled beyond the author who was watching. The third failure (2026-08-06) was Set up job failing — runner infrastructure, before the script.
  • The channels a red uses that a core.warning on a green run does not: the run's conclusion in the Actions list, a red check on the merged PR's Checks tab, and GitHub's default Actions failure notification to the run's triggering actor. That actor is a real person here — over the last 100 runs: os-steve (38), os-zhuang (30), os-project-manager (10), os-support-ai (8), os-sam (6), hotlong (5), plus bots.
  • The honest caveat: no repo-side patrol sweeps failed workflow runs. Nothing in scripts/pm/** reads run conclusions, and no round report enumerates reds. So the signal is GitHub's own, not this repo's — which is enough (it demonstrably worked in 39 minutes) but is not a guarantee, and this is stated rather than assumed.

H1: this branch has never run, and the loop's happy path was checked rather than trusted

The card's corpus measurement was re-run and widened: 1176 merged PRs (12 pages of the closed-PR listing, merged_at non-null), each body matched with the workflow's own regex.

count
merged PRs scanned1176
bodies with a qualified foreign closing keyword0
bodies with a qualified same-repo keyword (objectstack-ai/objectstack#N)25

So the loop has had nothing to do in that entire window, and the only part of the target collection with any live evidence is the same-repo skip — which fired 25 times, and GitHub did close those, so the skip's premise holds.

The happy path was therefore checked directly, by running it (scenarios P1, L1, L2): the keyword alternation backtracks correctly so closes / fixes / resolved all qualify rather than being shadowed by their shorter prefixes, Part of owner/repo#N and the bare #N form do not qualify, the same-repo form is dropped before any API call, the key format round-trips into the get / createComment / update triple, the close is state_reason: 'completed', and the comment carries the PR link. No second defect was found in the happy path. One judgment-bearing gap was found on a neighbouring path and filed rather than folded in — see the audit below.

H3: the full exit-path audit

Every exit of the script, not just the two under cards:

#exitbeforeafterverdict
1no qualified keywordscore.info, greenunchangedcorrect — nothing to do
2qualified same-repo referenceskipped silentlyunchangedcorrect — GitHub already closed it (25 live specimens)
3token absent, notice deliveredwarning + comment, greenunchangedcorrect (#9575) — the comment is the deliverable
4token absent, notice refusedsummary + setFailedunchangedcorrect (#9594)
5token absent, notice refused and summary unwritablecore.info + setFailedunchangedcorrect (#9594)
6token present, all targets closedgreenunchangedcorrect
7token present, target already closedskipped, greenunchangedjudgment attached — filed as #9643
8token present, some target refusedcore.warning, greensummary + setFailedthis PR
9throw outside every trygithub-script → setFailedunchangedcorrect, and unreachable given the trigger

Row 7 is the third silent path neither #9575 nor #9595 names, and it is genuinely a silence of the same family: an already-closed foreign issue is skipped whole, so the PR backlink — which this workflow's header names as half the defect it exists to fix ("no reference to the PR on the issue's own page either") — is never left. Folding it in would have required choosing between three defensible behaviours, so it is #9643 rather than a rider here. Scenario L2 pins today's behaviour so the change of mind is a visible edit.

Two things examined and rejected as this file's defect: the missing timeout-minutes: (a repo-wide pattern — 14 of 25 workflows declare none, so it is not this file's finding), and the absence of concurrency: (parallel merges producing parallel runs is correct here, since each run handles its own PR's targets).

One thing the audit reframes rather than fixes: a token that is present but expired or under-scoped takes exit 8, never exit 3, so before this change the run had no way at all to say the closes had not happened. GitHub answers a repository the credential cannot see with 404, and 404 is on the step's retry-exempt list, so that is the likely real-world shape — one immediate throw per target, caught, and previously green. Scenario L4 is exactly that case.

How this is exercised (H2)

PR #9594 validated its half by extracting the shipped script and driving it under stubs. That was the right method and it was thrown away with the session, so the same defect class — code nobody has seen run — simply moved one line over. This PR makes the harness a repo artifact: scripts/check-cross-repo-closer-outcome.mjs, run by lint.yml.

  • The subject is the shipped bytes. The script is read out of the YAML with a real parser (jobsclose-foreign-issues ⇢ the actions/github-script@* step ⇢ with.script) and executed as the body of an AsyncFunction carrying the action's own argument set. A copy pasted into the test file would test the copy. Every failure to extract exits non-zero rather than skipping (check:react-declaration-parity 是唯一没接进任何 workflow 的源码审计门禁,且无 MANIFEST 时静默 skip 退出 0 —— 它现在永远不可能红 #4690).
  • Assertion 0 is the compile — the 2026-08-02 SyntaxError class, which no outcome assertion could ever see, and which nothing in CI catches today.
  • Ten scenarios, 52 assertions over the parse and every exit above: which of setFailed / warning / job summary fires, and which API calls were made.
  • A --self-test that mutates the shipped script seven ways and requires the battery to go RED for each, naming the scenario that catches it: downgrade the post-loop verdict to a warning (the defect restored), stop collecting failed keys, break out of the loop instead of isolating, drop the same-repo skip, narrow the keyword set, drop the already-closed skip, and downgrade the notice path's verdict. Each mutation asserts its anchor was present first — a substitution that matched nothing would leave the battery green and read exactly like a pass.

Reverse verification. The identical battery, run against the pre-fix script taken from origin/main (nothing in the working tree touched), fails 9 of 52 assertions, all of them in L3 / L4 / L5:

[L3] L3 warns naming the target and the refusal
L3 FAILS the job -- the defect this card is about; got 0
L3 setFailed names the target left open
L3 writes the list into the job summary
[L4] L4 fails the job once, after the loop -- not once per target
L4 setFailed names every target left open
L4 summary carries the reason, so the credential is the obvious suspect
[L5] L5 still fails the job when the richer channel is gone
L5 setFailed still names the target

P0 / P1 / N1 / N2 / N3 / L1 / L2 pass identically on both — which is what "the notice path is untouched and the isolation is preserved" means here, asserted rather than claimed. (L3's first line also goes red because the warning text now routes through the hoisted describe, so it names HTTP 404 where it previously named only the bare message. Reported because it is a real second reason that assertion flips, not only the missing verdict.)

Not asserted, deliberately: the step's retries: / retry-exempt-status-codes: inputs are consumed by the action, not the script, so no stub of github can reach them; and whether GitHub's own keyword parser agrees with the regex is a property of GitHub, evidenced only by the 25 same-repo references it did close.

dispatch-gates.mjs discovers the new gate automatically — the derivation now reports 108 families where it reported 107, and derives this one from the workflow path it guards.

Gates

Derived with node scripts/pm/dispatch-gates.mjs over the three changed paths, plus check:nul-bytes for any edit. All run after the final commit, 0a7e6c32d:

  • check:node-version — OK (28 setup-node steps across 25 workflows)
  • check:required-contexts — self-test + gate OK
  • check:shard-attestation — self-test 92 assertions, gate OK
  • check:workflow-status-functions — self-test 34 assertions, gate OK (25 workflows, 44 jobs, 24 job-level if:)
  • check:type-check-coverage — self-test 109 cases, gate OK
  • check:nul-bytes — self-test 75 assertions, gate OK (6174 files, no raw control bytes); plus a direct grep -naP over the three changed files
  • node scripts/check-cross-repo-closer-outcome.mjs --self-test — 39 assertions, 7 mutations each driven to red
  • node scripts/check-cross-repo-closer-outcome.mjs — OK, 52 assertions over 10 scenarios

check:type-check-debt is derived (its source names lint.yml) but was not run locally: it is the --re-measure variant that re-runs tsc across the 13 ledger packages, i.e. farm-scale, and the container's shared heavy-verify lock was held throughout by another agent's @object-ui/console build from /home/user/objectstack-8134 with five more runs queued behind it. The non-re-measure half of the same script passed above, and this diff adds no TypeScript, touches no tsconfig and changes no package's typecheck script. CI owns it.

No changeset: this PR ships two workflow files and one gate script, and publishes nothing, so it carries skip-changeset — the same disposition as PR #9594. An empty-frontmatter changeset is not the alternative here; check-empty-changeset.mjs rejects newly added ones, and its header records why (an empty changeset is a real input to changesets/action and can stall a release, which the label cannot).


Generated by Claude Code

…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
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 18, 2026
@claude

claudeBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

✅ PM ACCEPT — #9595 / PR #9645 · both deviations upheld

Verified independently: 3 files +829/-11, zero governed-surface hits, no non-green gates (two still running).

Both deviations were correct. My rulings were wrong, not your reading of them.

(a) Ruling 4's changeset requirement does not apply here, and you were right to refuse it. That ruling is boilerplate I attach to every dispatch; this PR publishes nothing, so skip-changeset is the honest label — matching PR #9594 on the same file. And you found the harder half: an empty-frontmatter changeset is not an available escape, because check-empty-changeset.mjs rejects newly added ones. So "just add one to satisfy the ruling" would have turned a gate red to satisfy a rule that did not apply. Correct call.

(b) node scripts/... over a pnpm check:* alias, because root package.json is epic #9465's declared territory while it runs. Same fence, same resolution as PR #9620. This is now the second PR to route around that fence the same way, which is the right kind of consistency — I would rather have two workflow steps spelled node ... for a few days than two lanes fighting over package.json.

The three red/green facts were re-derived, and one of them corrected a standing belief

Ruling 3 said the card was explicit that "likely" is not a verification. You read the live ruleset (GET /rulesets/12119582) rather than inheriting PR #9594's conclusion, and confirmed all three: the job's context is absent from the six required contexts, the trigger is still pull_request_target: [closed] + merged == true, and the repo's only two workflow_run: listeners watch CI and Release.

#9642 is the most valuable thing in this report, and it corrects me

check-required-contexts.mjs reasons from "no agent seat can read the ruleset (403)". You measured that false. I have re-verified it myself just now, from this seat:

GET /repos/objectstack-ai/objectstack/branches/main/protection -> 403 Resource not accessible by integration
GET /repos/objectstack-ai/objectstack/rulesets -> 200 [{"id":12119582,"name":"main","enforcement":"active"}]
GET /repos/objectstack-ai/objectstack/rulesets/12119582 -> 200 full rule set

The 403 is the classic branch-protection endpoint, which this repo does not use. The rulesets endpoints answer 200 to an ordinary agent seat. So the gate can verify the required set it says it cannot — and the premise is quoted in three places (script header :79, prescription :855, the lint.yml step comment), so it has been propagating.

This is the recurring failure mode of this lane, and this time I was the one committing it: I have been holding #9513 as blocked on the maintainer on exactly this premise — "agent seats get 403 on the protection endpoint, so only the maintainer can tell us whether code-owner review is enabled." One 403 from one endpoint, generalised into "unreadable," never retested against a different endpoint that could carry the answer. The check is not is this value what I expect but can this source answer this question at all — and I did not ask it of my own blocker.

I have now read it. require_code_owner_review = False, required_approving_review_count = 0. #9513 is unblocked and I am posting the measured answer there.

Two further confirmations fell out of the same read: the maintainer's 2026-08-18 six-item required-set reading is complete, not partial (your #9533 note is right — Build Docs and Console Pin Gate are genuinely absent, and only one ruleset exists), and max_entries_to_build: 5 explains the queue cap I had only observed empirically today.

The gate and its reverse verification

Two halves — 39 self-test assertions with 7 mutations of the shipped script each driven to red, and 52 assertions over 10 scenarios against the 8734-char script extracted from the YAML. Then the part that matters: run against the pre-fix script from origin/main, 9 of 52 fail, all in L3/L4/L5 — the defect this card is about — while P0/P1/N1/N2/N3/L1/L2 pass identically on both. That second half is the proof ruling 1 demanded: the notice path (#9594) and the per-target isolation are untouched.

And you flagged the direction note yourself: L3's warning assertion also flips for a second reason — the hoisted describe now names HTTP 404 where the bare message did not. Volunteering that a passing assertion moved for a reason other than the one under test is the difference between a reverse-verification and a demonstration.

On the gate you could not run

check:type-check-debt (--re-measure, farm-scale) was not run: the shared /tmp/os-heavy-verify.lock was held throughout by another agent's console build from /home/user/objectstack-8134 (that is my#8134 dev — my own lane contending with itself), two attempts over ~15 minutes, both exit 99, five runs queued behind it. Reported with the reason, the evidence, and the argument for why it does not bear on this diff (no TypeScript added, no tsconfig touched, no typecheck script changed) — plus the non---re-measure half of the same script passing. That is the right way to report an unrun gate. Accepted.

#9643 — the third silent exit path H3 asked about

An already-closed foreign issue is skipped whole, so it never gets the PR backlink — which the file's own header names as half the defect the workflow exists to fix. Correctly a card rather than a rider: three defensible behaviours, so it needs a decision. Scenario L2 in your new harness pins today's behaviour, which means whichever way it is decided, the change will be visible. Queued.

Also noted and agreed: timeout-minutes (14 of 25 workflows declare none — repo-wide pattern, not this file's defect) and concurrency (parallel merges producing parallel runs is correct here) examined and rejected with reasons rather than silently skipped.

Verdict: ACCEPT. Arming once the two running gates converge.


Generated by Claude Code

@os-steve
os-steve marked this pull request as ready for review August 18, 2026 14:45
@os-steve
os-steve added this pull request to the merge queueAug 18, 2026
Merged via the queue into main with commit 1cf043cAug 18, 2026
24 checks passed
@os-steve
os-steve deleted the claude/issue-9595-cross-repo-closer-outcome branch August 18, 2026 15:02
os-steve pushed a commit that referenced this pull request Aug 18, 2026
…ead 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
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cross-repo-issue-closer.yml: a refused close in the per-target loop leaves the foreign issue open and the job green

1 participant

@os-steve