fix(work-items): exclude open-linked-PR issues from work frontier (#463)#643
Conversation
/work-items:work selection had no rule excluding an issue that already has an open linked PR from the pickable frontier, so an issue kept `status: ready` for its entire open-PR window and a picker had to hand-cross-check `gh pr list` to avoid starting a duplicate branch. Add a selection-time frontier filter (tiers 2-3) that drops a candidate with an open PR closing it, routed through a new GitHub adapter "Open linked PRs" mechanic (closing-keyword linkage authoritative; `Refs #N` opt-out does not exclude), failing open when the bound provider exposes no PR host. Retires the interim in-flight heuristic from the execute-step staleness pre-check. Durable seam-level in-review state remains deferred to the tracker-seam layer (#416/#498). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 59s —— View job PR Review
SummarySolid, well-scoped fix. The architecture seam is respected (adapter-specific tooling stays in the adapter README, the skill core only consumes a boolean), the tier scoping is correct (recurring tiers 1 and 4 are intentionally excluded — they originate from the schedule, not Issues1. Missing word boundary — The jq regex has no lookbehind before the keyword alternatives:
The fix is to add a negative lookbehind for a word character: jq uses Oniguruma, which supports variable-width lookbehinds — this is valid. The consequence of a false positive is an eligible issue being silently dropped from the pickable frontier for the duration of the spuriously-matched PR's lifetime. That's a subtle bug because it produces no error; the picker just doesn't see the item. Fix this → 2. The README claims GitHub's search orders by relevance, not recency, so the exact-match PR may or may not rank in the first 100. The claim "no page-size race" is only accurate under the implicit assumption that fewer than 100 open PRs mention the bare number — an assumption that breaks at low issue numbers. Tightening the GitHub search to include the keyword would reduce the result set dramatically: gh pr list --state open \
--search "\"closes #<N>\" OR \"fixes #<N>\" OR \"resolves #<N>\" in:body" \
--json number,body --limit 100 | tr -d '\r' \
| jq --arg n "<N>" 'any(.[]; .body | test(...))'Or simply raise the limit to Alternatively, the README comment should be corrected to drop the "truncation-safe" claim and note the small-N caveat explicitly. Fix this → Positive Observations
|
…se positive The `clos(?:e|es|ed)` regex fragment in the "Open linked PRs" adapter mechanic tripped the `typos` spell-checker on the standalone `clos` token. Fully spelling the alternation (close|closes|closed|fix|fixes| fixed|resolve|resolves|resolved) is semantically identical (verified equivalent against live PR data), removes the false positive at the root without a suppression pragma or a managed-config edit, and matches the sibling "PR closing-keyword mechanics" section's keyword phrasing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 37s —— View job PR Review (updated after
|
|
Claude finished @kyle-sexton's task in 2m 55s —— View job PR Review
SummaryThis PR is well-structured and the core logic is sound. Architecture seam discipline is maintained (adapter mechanics stay in the adapter README, the skill core reads only a boolean), tier scoping is correct (tiers 1 and 4 are recurring schedule items, not Issues1. Word boundary missing — The The fix is a single addition — a negative lookbehind for a word character before the keyword group: jq uses Oniguruma, which supports variable-width lookbehinds, so 2.
Three acceptable resolutions (any one suffices):
Positive Observations
VerdictBoth open issues are minor and would be rare in practice. The word boundary ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16866ff220
ℹ️ 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".
…463) Two real findings from the PR review, both in the "Open linked PRs" adapter mechanic: 1. Missing word boundary — the keyword alternation had no lookbehind, so `fix` matched inside ordinary words (a body reading `prefixes #463` spuriously matched `fixes #463` and silently dropped the issue from the frontier). Add `(?<!\w)` (Oniguruma variable-width lookbehind, valid in jq). Verified: crafted `prefixes`/`suffix`/`unfixes`/`postfix` bodies now return false; real closing keywords still return true. 2. Inaccurate "truncation-safe per item" claim — `--search "<N> in:body"` is a coarse prefilter matching every PR mentioning the digit, so a small `<N>` (e.g. #5) can exceed the page cap and truncate the real closing PR (GitHub sorts by relevance, not recency). Raise the limit to 1000 (exceeds any realistic repo's open-PR count) and reword the prose to describe the prefilter/superset accurately instead of overclaiming. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 37s —— View job PR Review
SummaryBoth issues raised in prior reviews have been addressed in the latest commit (
|
| Issue | Status |
|---|---|
Word boundary: fix matched inside prefixes / suffix |
Fixed — (?<!\w) added |
--limit 100 inaccurate for small issue numbers |
Fixed — raised to 1000, prose corrected |
Observations on the full PR
- Seam discipline maintained.
ghstays entirely in the adapter README; the skill core at SKILL.md line 97 reads only the open-closing-PR boolean. No seam-contract change, no new verb. - Tier scoping correct. The open-PR filter applies to tiers 2–3 (frontier candidates) only. Tiers 1 and 4 are recurring schedule items that originate from
recurring-schedule.json, notlist-frontieroutput, so they are correctly not subject to the filter. - Exact-number boundary correct.
(?![0-9])prevents#463matching#4630; the literal#in the pattern prevents#1463matching (the#precedes1, not4). Both cases are documented accurately. Refs #Nopt-out preserved. Keyword-only matching means an intentional opt-out does not exclude its issue — consistent withpr-issue-linkage.- Interim heuristic retired cleanly. The Step 4 "Already-in-flight (interim)" bullet is gone; the replacement operates at a better level (frontier exclusion rather than pre-check skip), which is exactly the retirement the original bullet anticipated.
- Fail-open documented and positioned correctly. The
local-markdowncarve-out is consistent between the skill (SKILL.md line 97) and the README. - Version bump correct. Patch bump
0.13.0→0.13.1for a filter addition with no seam-contract change is appropriate. CHANGELOG entry follows the existing format.
Verdict
No remaining issues. The two correctness findings from prior reviews are fixed, the escaping is right, and the prose now accurately documents both boundaries and the prefilter semantics. Ready to merge.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d04aa1f32
ℹ️ 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".
Review findings addressed (commit
|
…#463) The open-linked-PR frontier filter ran its closing-keyword regex over the raw PR body, so a `Closes #<N>` shown only inside a fenced example snippet matched and spuriously dropped a still-ready issue from the pickable frontier. GitHub treats fenced code blocks as inert (they never auto-close an issue), per the pull-request create contract, so scanning them contradicts real closing linkage. Strip fenced blocks (triple-backtick and ~~~, via a backreferenced gsub) before the keyword test, and document the guard alongside the existing word- and number-boundary guards. Addresses the round-3 Codex P2 finding. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 17s —— View job PR Review
|
| Component | Meaning |
|---|---|
(?s) |
DOTALL — . matches newlines, needed for multi-line blocks |
(\x60{3}|~{3}) |
Captures exactly three backticks or three tildes as the fence delimiter |
.*? |
Lazily matches everything until the closing delimiter |
\1 |
Backreference — closing delimiter must match opening (``` ↔ ```, ~~~ ↔ ~~~) |
This is correct. Oniguruma (jq's regex engine) supports backreferences in gsub patterns. The DOTALL + lazy + backreference combination is exactly how you'd write a content-safe fence stripper: the backreference prevents ``` from closing a ~~~ block, and lazy .*? stops at the first matching close rather than the last, which handles multiple sequential blocks correctly.
The prose update ("Three guards keep the match precise") accurately documents the three guards in order (gsub → (?<!\w) → (?![0-9])).
Known scope limits (not blockers)
4+ backtick fences not stripped. (\x60{3}|~{3}) matches exactly three characters. A fence opened with ``````, `````, etc. won't be matched. The Codex finding specifically called out standard triple-backtick and ~~~ fences; GitHub's own docs show these as the common forms. The four-backtick form is an edge case used primarily to nest triple-backtick examples inside a code block (meta-documentation), which is unlikely in a normal PR body.
Inline code not stripped. A body containing `Closes #<N>` (single-backtick inline code) is not affected by the gsub and would still match the keyword test. The Codex finding was scoped to fenced blocks (the citation was the fenced-block contract rule), and inline-code false positives would require someone to write the exact keyword phrase in inline code in a PR body — vanishingly rare in practice.
Both are edge cases that don't affect real-world PR bodies. The implementation matches the scope of the Codex finding.
Full PR status
All findings across all prior review rounds are now resolved:
| Finding | Status |
|---|---|
Word boundary: fix matched inside prefixes / suffix |
Fixed in 0d04aa1 — (?<!\w) added |
--limit 100 inaccurate for small issue numbers |
Fixed in 0d04aa1 — raised to 1000, prose corrected |
| Fenced code blocks not stripped before keyword scan (Codex P2) | Fixed in 828669a — gsub guard added |
Architecture seam discipline, tier scoping (tiers 2–3 only), Refs #N opt-out, (?![0-9]) number boundary, interim heuristic retirement, fail-open for local-markdown, and version bump (0.13.1 patch) were all confirmed correct in the prior review. Nothing in 828669a touches those.
Verdict
The fix is technically sound and closes the last open finding. Ready to merge.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 828669a67b
ℹ️ 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".
… filter (#654) (#668) ## 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 #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 HTML-comment fixture (must NOT link #667): Fixes #667 --> ```text 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 - #654 — this issue (fence-blindness in the open-linked-PR closing-keyword regex). - #463 / PR #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) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Summary
/work-items:workselection had no rule excluding an issue that already has an open linked PR from the pickable frontier. An issue keepsstatus: readyfor its entire window with an open PR (from open through merge/auto-close), so a picker had to manually cross-checkgh pr listto avoid re-picking work already in flight and starting a duplicate branch. This ships the ratified in-repo open-linked-PR selection filter (operator decision, 2026-07-19): an issue with an open linked PR is not pickable.Fix
skills/work/SKILL.md, Step 1). Afterlist-frontierderives the frontier, tiers 2–3 candidates are additionally filtered to drop any number that has an open PR targeting it for closure — so an in-flight item leaves the pickable set instead of being re-picked. The closing-keyword linkage is authoritative (the sameCloses #N/ native-closing-keyword signalpr-issue-linkageenforces); an intentionalRefs #Nopt-out does not exclude its issue. The filter fails open when the bound provider exposes no PR host (offlinelocal-markdownis never a coordination surface and touches no network tool).tools/work-item-tracker/adapters/github/README.md, "Open linked PRs"). The provider mechanics stay in the adapter per this repo's seam/adapter separation — the skill core inlines nogh. It usesgh pr list --state open --search "<N> in:body"(truncation-safe per item, no page-size race) and a closing-keywordjqtest with an exact-number boundary so#463does not match#4630/#1463.#463)" bullet in the execute-step staleness pre-check is removed — the frontier itself now excludes in-flight items, which is exactly the retirement that bullet anticipated. Scope-narrowed to the closing-keyword signal (the interim branch-pattern-only match is dropped): a standard-flow PR always carries the gate-enforcedCloses #N, so keyword-matching is faithful to the ratified "references it as a closing target" scope.CONTRACT.mdandlist-frontierare untouched; the addition is a README-documented adapter mechanic, not a new seam verb. The durable seam-level in-review state is explicitly NOT built here (see Related).Verification
The adapter mechanic was exercised against live repo data (read-only).
<N>substituted per row:Each of #435 / #487 / #534 is a genuine frontier-eligible candidate (open, unassigned,
status: ready) that under the old logic stayed pickable despite an open PR already in flight; the filter returnstruefor exactly those and excludes them, while #463 (no open closing PR) returnsfalseand remains pickable.list-frontieritself was not run end-to-end here because thework-itemsplugin repo binds no tracker (.work-item-tracker.jsonis a consuming-repo artifact) — the per-number filter, which is the added logic, is what is demonstrated above.Closes #463
Related
status: readyissues with an open PR stay pickable — no in-review state (re-pick risk) #463 — this issue (open-linked-PR selection filter).Note: PR #641 concurrently bumps
work-itemsto0.14.0(minor, for #478) and also editsplugin.json+CHANGELOG.md. This PR is a patch bump to0.13.1from the currentorigin/mainbase (0.13.0) — a different version slot, not a race — but whichever merges second will need a mechanical rebase on those two files (the same-plugin concurrency #464 treats as an awareness note, not a block).🤖 Generated with a Claude Code implementation subagent (issue #463)