fix(work-items): use GitHub computed close-linkage for open-linked-PR filter (#654)

[kyle-sexton]

Summary

The work-items GitHub adapter's "Open linked PRs" mechanic (shipped in #463 / PR #643) decided
whether an issue already had an open closing PR by running a closing-keyword jq regex over the
raw PR body. GitHub treats fenced code blocks and HTML comments as inert for auto-close
linkage, but the raw-body regex did not: a PR body carrying a fenced Closes #<N> example matched
true and dropped the still-open, still-pickable issue #<N> from the /work-items:work
frontier. #643 shipped a partial gsub fence-stripper alongside the regex, but it recognized only
exactly-three backtick/tilde fences and silently missed four-or-more-backtick and indented fences —
exactly the fragile hand-roll #654 flagged as not-small.

Fix

Replace the entire raw-body heuristic (the gh pr list --search "<N> in:body" prefilter, the
closing-keyword jq test, and the partial gsub fence-stripper) with GitHub's own computed
close-linkage, queried directly against the target issue number via the GraphQL
Issue.closedByPullRequestsReferences connection, keeping only OPEN-state PR nodes. This is the
root-cause fix #654 pointed toward: the signal is now GitHub's parsed issue linkage — the same set
GitHub renders in the issue sidebar and acts on at merge time — rather than a text match this
mechanic has to keep fence-, word-, and number-boundary-aware itself.

Consequences folded into the mechanic's rationale in the README:

• Fenced code blocks and HTML comments are inert for free — no fence-tracking heuristic to
  maintain, and the fence-blindness work-items open-linked-PR filter: closing-keyword regex matches fenced-code-block examples #654 filed is closed.
• No word-boundary / number-boundary guards — #463 cannot collide with #4630 / #1463
  because the reference is GitHub's parsed linkage, not a regex over raw text.
• Base-branch correctness (behavior change, called out explicitly): GitHub forms the close-link
  only for a PR that targets the default branch. A Closes #<N> on a non-default-base PR is
  ignored by GitHub and now no longer excludes its issue — where the retired raw-body regex counted
  it regardless of base. This is more correct per GitHub's real merge-time auto-close semantics.
• Opt-out (Refs #<num>) stays intrinsic — a reference-without-closing never enters the
  closing linkage, so it correctly does not exclude its issue.

Scope is held to the work-items adapter. The sibling fence-blind matcher noted in #654 — the
pre-create gate KEYWORD_REGEX in plugins/source-control/skills/pull-request/reference/create.md
— is not touched here: it is a different plugin, and it gates a PR body before the PR exists,
so it cannot use this issue-side GraphQL linkage. Whether it should adopt a different fence-aware
approach is left as a forward-reference follow-up, not folded into this adapter patch.

Verification

Empirical research proving Issue.closedByPullRequestsReferences is queryable via gh api graphql
and returns GitHub's genuine auto-close linkage (open-state filtered) — not body text matches.

Schema exists and is queryable (live introspection, gh api graphql):

closedByPullRequestsReferences args:
  includeClosedPrs  Boolean = false  "Include closed PRs in results"
  first/last/after/before, orderByState, userLinkedOnly

Positive cases — real prose closing keywords link, filtered to open PRs (the exact mechanic
this PR ships, run live against this repo):

issue 642  -> true    (open issue; open PR #666 carries prose `Closes #642`)
issue 654  -> false   (this issue; no open closing PR)
issue 487  -> false   (closed issue; its closing PR #629 is MERGED, so filtered out by state=="OPEN")
issue 644  -> false   (baseline before this PR)
issue 667  -> false   (baseline before this PR)

Negative isolation — fenced and HTML-comment closing keywords are excluded. This PR body is
itself the live experiment. It carries a real Closes #654 in prose (positive control: an open
issue closed via prose, in this draft PR, at this moment), plus two deliberately inert
fixtures — a closing keyword for open issue #644 only inside a fenced code block, and one for
open issue #667 only inside an HTML comment:

inert fenced fixture (must NOT link #644):
Closes #644

Because #644 and #667 are open issues (baselines false above) and this same draft PR does link
#654 from prose, the only difference for #644/#667 is that their keyword sits in a fence / comment.
Live query results against this PR are recorded below.

Live results (this PR is #668, base branch main):

# PR-side — what GitHub links THIS PR as closing:
pullRequest(668).closingIssuesReferences  ->  [654]        # ONLY 654. #644 (fenced) and #667 (comment) excluded.

# Issue-side — the exact mechanic this PR ships:
issue(654).closedByPullRequestsReferences  ->  [{668, OPEN}]   # real prose `Closes #654` links (positive control)
issue(644).closedByPullRequestsReferences  ->  []              # fenced `Closes #644` inert — #668 absent
issue(667).closedByPullRequestsReferences  ->  []              # HTML-comment `Fixes #667` inert — #668 absent

Conclusion: within one PR body at one instant, GitHub linked the prose closing keyword (#654) and
excluded the identical keyword when it sat inside a fenced code block (#644) or an HTML comment
(#667). Since #644/#667 are open issues that this same draft PR would otherwise be capable of
linking, the fence / comment is the sole cause of exclusion. Both the fenced-example bug #654
reported and HTML-comment inertness are handled by closedByPullRequestsReferences with no
heuristic of our own.

Closes #654

Related

• work-items open-linked-PR filter: closing-keyword regex matches fenced-code-block examples #654 — this issue (fence-blindness in the open-linked-PR closing-keyword regex).
• work-items(work): status: ready issues with an open PR stay pickable — no in-review state (re-pick risk) #463 / PR fix(work-items): exclude open-linked-PR issues from work frontier (#463) #643 — shipped the "Open linked PRs" mechanic (and its partial gsub fence-stripper)
  this PR replaces.

🤖 Generated with a Claude Code implementation subagent (issue #654)

[claude]

Claude finished @kyle-sexton's task in 2m 10s —— View job

---

PR Review — fix(work-items): use GitHub computed close-linkage for open-linked-PR filter

• Gather context and understand request
• Read changed files
• Analyze diff for correctness, security, and alignment
• Post review feedback

---

Summary

This PR replaces the raw-body jq regex heuristic for detecting open closing PRs with a call to GitHub's GraphQL Issue.closedByPullRequestsReferences connection. The fix is architecturally sound — using GitHub's own computed linkage eliminates the entire class of fence/comment/word-boundary errors that plagued the regex approach. The three changed files are all in scope and appropriate.

---

Findings

1. Silent pagination cap not documented (minor)

README.md L176

closedByPullRequestsReferences(first:20, includeClosedPrs:false) { nodes { number state } }

first:20 silently caps the result. If an issue ever has more than 20 open PRs targeting it for closure, the mechanic returns a false negative and the issue stays pickable. In practice this is nearly impossible, but the README's rationale section calls out several other precision properties (fences, number-boundary, base-branch) without mentioning the pagination bound. A one-line --limit style callout (mirroring the "List items" --limit note) would make the tradeoff explicit.

---

2. select(.state=="OPEN") is redundant with includeClosedPrs:false (very minor)

README.md L180

--jq '[.data.repository.issue.closedByPullRequestsReferences.nodes[] | select(.state=="OPEN")] | any'

includeClosedPrs:false already excludes non-open PRs from the connection, so the select(.state=="OPEN") filter is redundant. This is defensively correct and harmless — a MERGED node cannot appear when includeClosedPrs:false — but it introduces mild inconsistency with the prose ("Keep only the OPEN-state nodes") that implies a runtime filter is the guard, when the API parameter already handles it. Not a bug; call it out only in case the reader draws a false inference about the API behavior.

---

3. Missing | tr -d '\r' on the final gh api graphql output (minor, Windows)

README.md L170–180

The OWNER_REPO line already carries | tr -d '\r' (good), but the final gh api graphql --jq '...' | any output does not. The README's own "Gotchas" section says "end every parsing pipeline with | tr -d '\r'". On Windows/Git Bash, --jq output is still piped through the gh process and can emit true\r / false\r, which would cause a caller comparing against the literal true/false to silently mismatch. The OWNER_REPO command in the same snippet is correctly guarded; this one is not.

---

What looks good

• The fix is the right fix. Issue.closedByPullRequestsReferences is exactly the right signal — it's the same linkage GitHub renders in the sidebar and acts on at merge time. No fence-tracking heuristic, no word/number-boundary guards, no keyword allow-list to maintain.
• Base-branch behavior change is called out explicitly in the CHANGELOG and the rationale bullet, which is exactly the right treatment for a behavior change in a patch.
• -F n=<N> (typed Int) vs -f (string) is used correctly — typed GraphQL parameter prevents any injection vector.
• gh repo view for owner/repo — not hardcoded, consistent with the plugin's repo-agnostic design requirement from CLAUDE.md.
• Version bump is correct — 0.14.0 → 0.14.1 (patch) for a bug fix, matching semver and the repo's versioning convention.
• CHANGELOG entry is concise, accurate, and includes the behavior-change callout.

---

Verdict

The core change is correct and a clear improvement. Findings 2 and 3 are minor; finding 3 (the missing | tr -d '\r') is the most actionable on Windows environments. Finding 1 is a documentation gap, not a correctness risk for any real repo. Overall this is ready to merge with the | tr -d '\r' gap acknowledged.

[claude]

Claude finished @kyle-sexton's task in 2m 14s —— View job

---

• Gather context and understand request
• Read changed files
• Analyze diff for correctness, security, and alignment
• Post review feedback

---

Review — fix(work-items): use GitHub computed close-linkage for open-linked-PR filter

The synchronize event was commit 60a69d1 ("reword CHANGELOG to satisfy typos spell-check") — a pure prose fix. The core implementation (a6a7567) is unchanged since the previous review pass, so the three prior findings carry forward.

---

Findings

1. | tr -d '\r' missing from the GraphQL pipeline (minor, actionable on Windows)

README.md L180

  --jq '[.data.repository.issue.closedByPullRequestsReferences.nodes[] | select(.state=="OPEN")] | any'

The OWNER_REPO assignment above it correctly ends with | tr -d '\r', but the gh api graphql pipeline does not. The "Gotchas" section (L253–254) says "end every parsing pipeline with | tr -d '\r'" — this snippet violates that rule. On Windows/Git Bash, --jq output from gh can arrive as true\r / false\r; a caller comparing the output against the literal false would silently mismatch. Fix: append | tr -d '\r' after the closing ' of the --jq argument. Fix this →

2. Silent first:20 pagination cap not documented (minor)

README.md L175

closedByPullRequestsReferences(first:20, includeClosedPrs:false) { nodes { number state } }

first:20 silently caps results; an issue with more than 20 open closing PRs would produce a false negative and stay pickable. The "List items" section explicitly calls out --limit truncation with a note; the same transparency would be consistent here. Very unlikely in practice, but the rationale block already enumerates several other precision properties — this one should be named too. Fix this →

3. select(.state=="OPEN") is redundant with includeClosedPrs:false (very minor, no action required)

README.md L180

includeClosedPrs:false already excludes MERGED/CLOSED nodes from the connection, so the select(.state=="OPEN") filter is harmless but redundant. The rationale prose says "Keep only the OPEN-state nodes" which implies a runtime filter is the guard, when the API parameter is the actual guard. Not a bug — called out only so the reader doesn't draw a false inference about what the API returns.

---

What looks good

• The fix is the right fix. Issue.closedByPullRequestsReferences is exactly the right signal — the same linkage GitHub renders in the issue sidebar and acts on at merge. No fence heuristic, no word/number-boundary regex, no keyword allow-list.
• Base-branch behavior change documented. Called out in both the CHANGELOG and the rationale bullet — correct treatment for a semantic change in a patch.
• -F n=<N> (typed Int) vs -f (string) used correctly — prevents any injection via the number field.
• gh repo view for owner/repo — not hardcoded; consistent with the plugin's repo-agnostic design requirement.
• Version bump is correct — 0.14.0 → 0.14.1 (patch) for a bug fix.
• CHANGELOG entry is accurate, concise, and includes the behavior-change callout (and now passes spell-check).

---

Verdict

Finding 1 (missing | tr -d '\r') is the most actionable item and a real inconsistency with the file's own documented rule. Finding 2 is a documentation gap. Finding 3 needs no action. The core change is correct and a clear improvement — ready to merge once Finding 1 is resolved.

[claude]

Claude finished @kyle-sexton's task in 1m 54s —— View job

---

• Gather context and read changed files
• Check if previous review findings were addressed
• Post updated review

---

Review — follow-up pass on 361a300

Commit 361a300 ("docs(work-items): address review — tr -d '\r', pagination bound, state-filter rationale") addresses all three findings from the previous pass. Walking through them:

Finding 1 — | tr -d '\r' missing (was: actionable)

README.md L181 — Fixed. | tr -d '\r' is now appended to the gh api graphql pipeline, consistent with the file's own Gotchas rule.

Finding 2 — first:20 pagination cap undocumented (was: minor)

README.md L191–193 — Fixed. The prose now names the bound, acknowledges cursor pagination as the escape hatch, and draws the explicit analogy to the "List items" --limit note.

Finding 3 — select(.state=="OPEN") called redundant (was: very minor / no action required)

The previous review was wrong on this one. L187–190 now documents that includeClosedPrs:false suppresses only CLOSED (unmerged) PRs in GitHub's GraphQL tri-state model (OPEN / CLOSED / MERGED), so MERGED nodes still appear in the connection and the select(.state=="OPEN") is genuinely load-bearing — not redundant. The commit message notes this was verified live against issue #487 / merged PR #629. This is a net improvement over the prior state: the filter is not only correct, it's now explained with the non-obvious API behavior that makes it necessary.

---

No new findings

The three changed files are all in scope and look correct:

• plugin.json — version 0.14.0 → 0.14.1 (patch, correct for a bug fix)
• CHANGELOG.md — entry is accurate, concise, and calls out the base-branch behavior change
• README.md — all three prior findings resolved; select(.state=="OPEN") clarified as load-bearing; command matches the rationale prose

The core fix remains sound: Issue.closedByPullRequestsReferences is the right signal, the Int-typed -F n=<N> prevents injection, and gh repo view keeps the adapter repo-agnostic per CLAUDE.md requirements.

Verdict: ready to merge.

[kyle-sexton]

Thanks — addressed in 361a3009:

• Finding 3 (missing | tr -d '\r') — fixed; the final pipeline now ends with | tr -d '\r' per the Gotchas rule.
• Finding 1 (first:20 silent cap) — documented; added the page-bound note plus the cursor-pagination escape hatch (GraphQL analogue of the --limit note).
• Finding 2 (select(.state=="OPEN") redundant with includeClosedPrs:false) — respectfully, this is not redundant, and removing it would be a correctness regression. includeClosedPrs:false suppresses only CLOSED (unmerged) PRs; it still returns MERGED nodes. Verified live against issue source-control:pull-request: emit ## Related + closing-keyword skeleton in create flow #487 (its only closing PR, fix(source-control): scaffold ## Related section in pull-request create flow (#487) #629, is MERGED):

closedByPullRequestsReferences(first:20, includeClosedPrs:false).nodes  ->  [{"number":629,"state":"MERGED"}]
... | any                                    ->  true    # WITHOUT the state filter: false in-flight report
... | select(.state=="OPEN") | any           ->  false   # WITH the filter: correct

So the filter is load-bearing (an issue whose only closing PR merged to a non-default base, or that was reopened after a merge, must stay pickable). I kept it and documented the semantics inline instead of removing it.

[kyle-sexton]

Post-green review classification

3 review rounds, all findings resolved by the worker itself before this pass:

Finding	Severity	Classification	Action
Missing | tr -d '\r' on GraphQL pipeline output	minor, Windows-actionable	VALID	Fixed
first:20 pagination cap undocumented	minor	VALID	Fixed — documented with the "List items" --limit precedent
select(.state=="OPEN") called redundant	very minor	INVALID (review's own error)	Corrected — verified live against #487/#629 that MERGED nodes DO appear even with includeClosedPrs:false, making the filter genuinely load-bearing, not redundant

Verdict from the final review pass: "ready to merge." CI green (16/16). Flipping to ready — worker completed all fix rounds but the final draft→ready step wasn't executed.

[claude]

Claude finished @kyle-sexton's task in 1m 42s —— View job

---

• Gather context and review history
• Read changed files at HEAD (361a3009)
• Verify all prior findings are resolved
• Post final review verdict

---

Final Review — fix(work-items): use GitHub computed close-linkage for open-linked-PR filter

This is a clean-slate pass at HEAD 361a3009 (the ready_for_review trigger). Three prior rounds of review ran on this PR; all findings were resolved. I found nothing new.

---

Prior findings — confirmed resolved

Finding 1 — | tr -d '\r' missing from the GraphQL pipeline

README.md L181 — Fixed. The gh api graphql pipeline now ends with | tr -d '\r', consistent with the file's own Gotchas rule and with the OWNER_REPO line directly above it. ✓

Finding 2 — first:20 pagination cap undocumented

README.md L193–194 — Fixed. The prose explicitly names the bound, acknowledges cursor pagination as the escape hatch, and draws the "List items" --limit analogy. ✓

Finding 3 — select(.state=="OPEN") called redundant (prior review was wrong)

README.md L187–190 — The filter is genuinely load-bearing. includeClosedPrs:false suppresses only CLOSED (unmerged) PRs in GitHub's tri-state model; MERGED nodes still appear and must be dropped by the select. The inline rationale and kyle's live verification against issue #487 / merged PR #629 confirm this. The filter is correctly present and now explained. ✓

---

No new findings

Walking the three files at HEAD:

• plugin.json — 0.14.0 → 0.14.1 (patch). Correct semver for a bug fix. ✓
• CHANGELOG.md — Entry is accurate and concise; behavior change (default-branch requirement) is explicitly called out. ✓
• README.md — The "Open linked PRs" section is coherent end-to-end:
  • OWNER_REPO shell splitting (${OWNER_REPO% *} / ${OWNER_REPO#* }) is correct for a "owner name" space-delimited string.
  • -F n=<N> (typed Int) vs -f (string) for owner/repo is correct and prevents any injection vector.
  • The query='...' is in single quotes, so $owner/$repo/$n are not expanded by the shell — they remain GraphQL variable references. ✓
  • jq behavior on an empty nodes array: [] | any → false. ✓
  • The four rationale bullets (fences, number-boundary, base-branch, opt-out) are accurate and complete.
  • gh repo view for owner/repo — not hardcoded; satisfies the plugin-form repo-agnostic requirement from CLAUDE.md. ✓

---

Verdict

All findings resolved. Implementation is correct. Documentation is thorough and internally consistent. Ready to merge.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 361a3009aa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 56s —— View job

---

• Gather context and read changed files at HEAD (cf5e079)
• Verify the first:20 → first:100 pagination fix and new documentation
• Post review verdict

---

Review — follow-up pass on cf5e079

Commit cf5e079 ("docs(work-items): request connection max (first:100) for close-linkage page") addresses the Codex P2 finding: because closedByPullRequestsReferences retains MERGED nodes, first:20 could be exhausted by a long merge/reopen history, leaving the currently-open closing PR off the page and causing a false-negative.

The fix is correct. Walking the change:

• README.md L175 — first:20 → first:100. 100 is the GitHub GraphQL first/last cap, so this requests the maximum the connection allows. ✓
• README.md L191–196 — The new prose correctly explains why the bound matters: the connection counts every ever-linked closing PR (including MERGED), not just open ones, so a merge/reopen cycle consumes slots ahead of the current open PR. The cursor-pagination escape hatch is kept for the >100 case. ✓

No new findings. The full command at HEAD is coherent end-to-end: first:100, includeClosedPrs:false, select(.state=="OPEN") (load-bearing, documented), | tr -d '\r' (Windows guard), -F n=<N> (typed Int). All prior findings remain resolved.

---

Verdict

Clean and correct. Ready to merge.