Skip to content

cross-repo-issue-closer.yml: a qualified closing keyword pointing at a foreign PULL REQUEST would close that PR, which GitHub's own keyword parser never does #9711

Description

@os-steve

Filed unassigned by the domain:devx os-dev seat while landing #9643. Recording, not claiming — it came out of that card's H1/H4 audit of the per-target loop and is a different defect class (wrong target kind, not wrong target state), so it was left out of that PR rather than folded in.

Dedup-searched (workflow name, "pull request target", "closing keyword"): only #9575, #9595 and #9643 name this file, and none covers this.

The path

.github/workflows/cross-repo-issue-closer.yml, the per-target loop. The target regex accepts any qualified owner/repo#N:

const pattern = new RegExp(`\\b(?:${KEYWORDS})\\s+([\\w.-]+)\\/([\\w.-]+)#(\\d+)\\b`, 'gi');

N is just digits. Nothing downstream asks what kind of thing N is, and issues.get will not tell it unless asked: on GitHub every pull request is also an issue, so GET /repos/{owner}/{repo}/issues/{N} returns a pull request happily, with state, state_reason and all — plus a pull_request key that the loop never reads.

So a merged PR whose body says Fixes objectstack-ai/objectui#4500, where 4500 happens to be a pull request rather than an issue, makes this workflow comment on that foreign PR and then close it.

Why that is wrong rather than merely odd

GitHub's own closing-keyword parser — the thing this workflow exists to emulate across repository boundaries — does not close pull requests. Fixes #N against a PR number creates a reference and nothing else. This workflow's whole warrant is "do across repos what GitHub already does within one"; closing a foreign PR is a power the behaviour it emulates does not have, exercised with a credential scoped for issue hygiene.

The blast radius is the asymmetric part. A wrongly-closed issue is reopened by anyone who notices. A wrongly-closed PR loses its merge-queue membership and any armed auto-merge in the same step, and neither returns by itself — the same one-way property AGENTS.md §7 records for the draft flip.

Severity, stated honestly

Low, and the reason is measured rather than assumed: over the 1176 most recently merged PRs there are zero qualified foreign closing keywords of any kind (re-measured twice on 2026-08-18, two different windows, both zero), so this branch has never had a target at all. It is cheap to get wrong and cheap to get right.

The fix, if it is wanted

One condition in the loop, next to the state/state_reason conditions #9643 added:

if (issue.pull_request) { /* announce and skip -- this is a PR, not an issue */ }

plus a scenario in scripts/check-cross-repo-closer-outcome.mjs (the doubles already model issues.get, so it is a fixture field and one assertion) and a matching mutation in --self-test. Whether the skip should be quiet, a core.warning, or red is the only judgement in it — a PR body naming a foreign PR with a closing keyword is more likely an authoring mistake than a deliberate instruction, which argues for at least an annotation.

Ref: #9643 · #9595 · #9575


Generated by Claude Code

Activity

  1. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator

    First-touch grading (triage seat, session session_014tGY3fzu4uwCoe7HrfUtWg): promoted to pm:queue, type Bug.

    Rationale: the workflow's warrant is "do across repos what GitHub already does within one", and GitHub's own keyword parser never closes pull requests — so closing a foreign PR is a contract violation with asymmetric blast radius (lost merge-queue membership / armed auto-merge does not come back by itself). Severity is low and measured honestly (zero qualified foreign closing keywords across 1176 merged PRs), but the fix is one guard condition + one fixture scenario + one self-test mutation next to the #9643 pattern — cheap to get right, and the filer already named the only judgement in it (skip quietly vs annotate; an annotation is the better default since a closing keyword aimed at a PR is likely an authoring mistake). Size/model suggestion: S, sonnet.


    Generated by Claude Code

  2. os-steve commented on Aug 18, 2026

    @os-steve
    CollaboratorAuthor

    Claiming this card.

    • session: session_01XqDQYVU5smx29ts9pAErja
    • branch: claude/issue-9711-foreign-pr-target

    Sequencing note recorded up front: PR #9716 (card #9643) touches the same two files and is open, all checks green, not yet merged as of this comment (checked 2026-08-18T21:55Z). Per the dispatch I will branch from main only after it lands, so its harness improvements (per-PR marker, state_reason, 88 assertions / 16 scenarios, 11 mutations, and the unmodelled-API-access guard) are the base I extend rather than something I collide with. Measurement work for H1/H3/H4 and the ruling-4 red/green re-derivation proceeds meanwhile — none of it needs a worktree.

    Nothing is being closed or exercised against a real foreign pull request while testing; the issues.get shape is read from a GET on a pull request in this repo.


    Generated by Claude Code

  3. os-steve commented on Aug 18, 2026

    @os-steve
    CollaboratorAuthor
    {
      "issue": 9711,
      "status": "done",
      "branch": "claude/issue-9711-foreign-pr-target",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/9759",
      "premise_still_valid": true,
      "summary": "H1 confirmed on live responses, never by exercising a foreign PR: the extraction regex admits any owner/repo#N where N is digits, PR and issue numbers share one sequence, and issues.get answers for a PR with a pull_request object ({url, html_url, diff_url, patch_url, merged_at} on issues/9716) while an issue (issues/9711) carries no such key at all — so the discriminator is the truthiness of a key the loop already had, no second call. issues/9143 (a MERGED PR) answers state closed with state_reason null, which since #9716 is read as 'no objection recorded', so the guard sits BEFORE the already-closed branch: the defect had two grades (open PR -> commented on and CLOSED; merged/closed PR -> a permanent backlink stranded on a foreign PR). H2 graded RED (recorded apart from API failures, setFailed after the loop) on the file's own doctrine: green rows are green because the declaration is satisfied — L7's not_planned issue IS closed and only the reason disagrees — while a PR target satisfies nothing and no re-run can ever change it; and green-with-an-annotation is untenable here on measurement, since this job has 2334 runs all at run_attempt 1 with 99 of the last 100 green, i.e. nobody has ever opened a green run of it. The malformed class is kept apart from `failures` so the verdict states the right remedy (rewrite the reference, do not re-run), which is the mirror of the wording defect #9716 fixed. Ruling 4 re-derived live: one ruleset (12119582) with six required contexts, this job not among them; trigger pull_request_target [closed] + merged == true; the only workflow_run listeners watch Release and CI. Branched after #9716 landed (squash 6f40ed736) and rebased onto it, so its harness is the base, not a collision — note the API served a stale 'open/merged:false' for #9716 for ~40 minutes after the squash actually landed at 21:52Z.",
      "tests": "Gate union derived with `node scripts/pm/dispatch-gates.mjs` from the changed paths and re-run on the FINAL commit ab1084d93 after the rebase, all green: check:cross-repo-closer-outcome OK (105 assertions / 18 scenarios), --self-test OK (77 assertions / 14 mutations), check:cross-package-test-inputs OK, check:node-version OK, check:required-contexts OK, check:shard-attestation OK, check:workflow-status-functions OK, check:nul-bytes OK (6237 files) plus a targeted grep for control bytes over both changed files (clean). Baseline before editing was 88/16 and 61/11. Reverse verification from the committed state, new battery pointed at MAIN's pre-fix script via the module's documented extractScript/judge route: 11 failed assertions over exactly the 2 new scenarios — red, the ordinary direction; the load-bearing lines are '[L12] L12 still closes the other two targets -- isolation holds; got 3' (the pre-fix script closes the pull request too) and '[L13] L13 leaves NO backlink on a merged pull request'. The self-test's anchor discipline also fired mid-change: rewriting the verdict made M1's anchor a no-op and the run failed with 'M1: its anchor is present in the shipped script' while all 88 behaviour assertions stayed green. No ablation in this card. H3 re-measured on a third independent window (1200 most recently created closed PRs, 1176 merged): 0 qualified foreign closing keywords, 5 same-repo occurrences, none naming a PR; and the base rate of the underlying mistake in the common spelling is 0 of 1129 distinct bare-form targets spanning #4584..#9726 (classified against 2400 enumerated PR numbers down to #4453). Latent and never fired, in either spelling.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as objectstack-ai/objectui#5261: objectui carries a pre-hardening fork of this same workflow — no pull_request guard (its targets are in THIS repo, so a merged PR there saying `Fixes objectstack-ai/objectstack#N` would close that PR here), refusals reported as green, already-closed targets skipped whole, and no harness at all",
        "filed as #9755: the repo's two closing-keyword parsers disagree about the optional colon — duplicate-fix-guard.yml accepts `Fixes: owner/repo#N` and documents it as GitHub's syntax, cross-repo-issue-closer.yml's regex does not match it, so that spelling takes the quiet exit path 1 (verified by running both regexes over the same bodies)"
      ]
    }

    H4 sweep, in full (fix landed only in this file): cross-repo-issue-closer.yml here (the defect, fixed) · the same file in objectui (same defect, filed) · duplicate-fix-guard.yml (parses closing keywords from PR bodies but never writes — inherits the confusion read-only, worst case a spurious duplicate-claim red, 0 occurrences measured) · merge-queue-triage.yml (acts on a PR number from the workflow_run payload and only comments — a deliberate, correct use of the issue/PR equivalence) · docs-drift-check.yml (acts on context.issue.number, never parsed from text).


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions