fix(source-control): scaffold ## Related section in pull-request create flow (#487) - #629
Conversation
…te flow (#487) The create flow assembles the PR body from its own template and passes it via `gh pr create --body`, which fully overrides `.github/pull_request_template.md` (cli/cli#10751). The assembled skeleton had `## Summary` / `## Test plan` but no `## Related` section, so PRs in a repo enforcing a pr-issue-linkage-style contract (non-empty `## Related` + native closing keyword) failed the gate on first push. Emit a `## Related` section defaulting to `N/A` (non-empty by default), pairing with the always-present `${CLOSES_LINE}` closing keyword so both halves of the contract are scaffolded up front. Document both scaffolds in the §2.4.1 prose and flag that a bare `Refs #N` opt-out does not satisfy a validator's closing-keyword half. Create-flow half of #462 (repo PR template covers the web/editor path). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 25s —— View job Code Review
Overall: Focused, well-reasoned fix. The change is minimal (adding 4 lines to the body template plus documentation prose), correctness is solid, and security of the shell construction is maintained. One pre-existing issue is worth a follow-up call-out; everything else looks good. CorrectnessTemplate addition ( The
Position (after Opt-out asymmetry documentation ( The prose accurately describes both scaffolds and correctly flags the known gap: §2.4.2's That asymmetry is pre-existing (introduced when §2.4.0's orphan-PR prompt was written, not by this PR) and intentionally deferred here. The inline documentation makes the mismatch visible to maintainers and future contributors, which is the right call for a patch-scoped fix. A follow-up should either:
The PR description already recommends this — flagging here just to keep it visible. Fix this → SecurityNo new concerns introduced. The critical property is preserved: The Plugin versioning & CHANGELOG
CLAUDE.md alignment
Summary: Approve. The fix is correct, the shell construction is safe, and the known gap is properly documented and deferred rather than silently ignored. The only actionable item is the pre-existing |
|
Follow-up filed for the opt-out marker asymmetry noted in this PR's body / create.md §2.4.1: #630. |
Post-green review classification
No blocking findings. Security review confirmed heredoc quoting keeps the new content inert. CI green (16/16). Flipping to ready for review. |
|
Claude finished @kyle-sexton's task in 1m 24s —— View job PR Review
Verdict: Approve. Minimal, correct, and safe. The earlier review findings are all resolved or properly deferred. No new issues found in this pass. Template change —
|
|
Control tower decide-and-log (operator veto window). Digest flagged one plan deviation: #487's plan said to promote the |
main advanced source-control to 0.13.0 while this branch bumped 0.12.0 -> 0.12.1; re-home the branch's Related-section changelog entry above main's 0.13.0 entry and bump the manifest to 0.13.1. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 27s —— View job PR Review
Verdict: Approve. The synchronize event is a single maintenance commit ( What changed since the last review pass (
|
| File | Change |
|---|---|
plugin.json |
"version": "0.12.1" → "0.13.1" |
CHANGELOG.md |
Top-level header updated to ## [0.13.1]; ## [0.13.0] block from main inserted below |
create.md |
Unchanged — identical to the file reviewed in the two prior passes |
Version bump correctness
The re-bump is correct. During the review window, main advanced from 0.12.0 → 0.13.0 (the babysit authorship/classifier refactor). This PR's original commit had targeted 0.12.1. The resolution:
0.13.0— already on main (not part of this PR)0.13.1— this PR's patch fix applied on top → correct semver semantics ✓
The ## [0.13.0] CHANGELOG block is brought in verbatim from main and is not authored by this PR, so no additional review is needed for its content. ✓
Core fix — create.md L174–176
Unchanged from prior review. Re-verified at HEAD:
## Related\nN/Ais inside<<'EOF'(single-quoted heredoc) — completely inert, no expansion ✓## Relatedmatches/^##\s+related$/i;N/Ais non-empty after trim — both halves ofpr-issue-linkagepass ✓- Section position (after
## Test plan, before attribution) matches repo convention established by source-control/repo: PRs fail pr-issue-linkage (missing## Related) — no template scaffolds the required sections #462 ✓
Shell injection defense — create.md L181–189
Unchanged. Quoted heredoc + parameter-expansion concat is intact. ${CLOSES_LINE} from user input remains literal regardless of content. ✓
Prose documentation — create.md L196–201
Unchanged. "Linkage scaffolds — always emitted" block and opt-out asymmetry note are accurate. The Refs #N / validator-gap is documented here and tracked in #630. ✓
CLAUDE.md alignment
- Repo-agnostic, no hardcoded paths ✓
- Plugin-form-safe, no
../reach-outs ✓ - Explicit semver bump with correct patch semantics ✓
- Narrowly scoped to the gap it closes ✓
Pre-existing deferred item: OPTOUT_REGEX asymmetry (§2.4.2 accepts Refs #N; real validator does not) is documented in §2.4.1 and tracked in #630. Out of scope for this patch.
…) (#643) ## Summary `/work-items:work` selection had no rule excluding an issue that already has an open linked PR from the pickable frontier. An issue keeps `status: ready` for its entire window with an open PR (from open through merge/auto-close), so a picker had to manually cross-check `gh pr list` to 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 - **Selection-time frontier filter (`skills/work/SKILL.md`, Step 1).** After `list-frontier` derives 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 same `Closes #N` / native-closing-keyword signal `pr-issue-linkage` enforces); an intentional `Refs #N` opt-out does not exclude its issue. The filter **fails open** when the bound provider exposes no PR host (offline `local-markdown` is never a coordination surface and touches no network tool). - **New GitHub adapter mechanic (`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 no `gh`. It uses `gh pr list --state open --search "<N> in:body"` (truncation-safe per item, no page-size race) and a closing-keyword `jq` test with an exact-number boundary so `#463` does not match `#4630` / `#1463`. - **Retires the interim heuristic.** The prior "Already-in-flight (interim, retire on `#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-enforced `Closes #N`, so keyword-matching is faithful to the ratified "references it as a closing target" scope. - **No seam-contract change.** `CONTRACT.md` and `list-frontier` are 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: ``` issue #435 open-closing-PR: true (open, unassigned, status: ready; open PR #638 closes it) issue #487 open-closing-PR: true (open, unassigned, status: ready; open PR #629 closes it) issue #534 open-closing-PR: true (open, unassigned, status: ready; open PR #634 closes it) issue #463 open-closing-PR: false (no open PR closes it → stays pickable) issue #4630 open-closing-PR: false (exact-number boundary: does not false-match on #463's PR) ``` 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 returns `true` for exactly those and excludes them, while #463 (no open closing PR) returns `false` and remains pickable. `list-frontier` itself was not run end-to-end here because the `work-items` plugin repo binds no tracker (`.work-item-tracker.json` is a consuming-repo artifact) — the per-number filter, which is the added logic, is what is demonstrated above. Closes #463 ## Related - #463 — this issue (open-linked-PR selection filter). - #416 — planning routes through the tracker seam; argues the durable in-review state belongs at the seam, not a GitHub-only label. This PR does NOT implement that durable state. - #498 — seam read-verb coverage; where a durable in-review / container-scoped read would live. This PR does NOT implement that durable state. Note: PR #641 concurrently bumps `work-items` to `0.14.0` (minor, for #478) and also edits `plugin.json` + `CHANGELOG.md`. This PR is a patch bump to `0.13.1` from the current `origin/main` base (`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) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…te (#630) (#651) ## Summary The `pull-request` create flow's §2.4.2 pre-create gate treated a bare `Refs #N` line as a valid closing-keyword opt-out, but the real `pr-issue-linkage` reusable CI workflow does not accept `Refs #N`. A PR body that satisfied the skill's own local gate could therefore still fail the CI gate on push. This is the narrow, same-repo fix (issue #630, option 1): align the skill's local opt-out marker set to the validator by dropping `Refs #N`. ## Fix In `plugins/source-control/skills/pull-request/reference/create.md`: - **§2.4.2** `OPTOUT_REGEX` drops `Refs #N`: `^(Refs #[0-9]+|No related issue:)` → `^No related issue:`. A rationale comment explains why `Refs #N` is excluded. The two abort-message lines no longer list `Refs #N` as an opt-out. - **§2.4.0** orphan-PR prompt reduced from three options to two (`Closes #N` / `No related issue:`); a `Refs #N` link-without-close reference is now explicitly routed to the `## Related` section instead of the closing-keyword line. - **§2.4.1** asymmetry note rewritten: a `Refs #N` line satisfies the closing-keyword half of neither the §2.4.2 gate nor the real validator. - The §2.4.2 "gate passes silently" prose no longer names `Refs #N` as a silent-pass path. `Refs #N` remains valid where it always belonged — as a link-without-close reference inside the `## Related` section (§2.4.1) and in the multi-issue prompt where a primary `Closes #N` already carries the gate. Extending the upstream validator to accept `Refs #N` (option 2) requires a change in `melodic-software/ci-workflows` and is out of scope for this lane. `source-control` bumped 0.13.1 → 0.13.2 (patch — contract-alignment bug fix) with a matching CHANGELOG entry at the top. ## Verification **The real validator's accepted set** — read live from the pinned reusable source (`melodic-software/ci-workflows/.github/workflows/pr-issue-linkage.yml` @ `90f1c54935203fa31b5b3d1f41531228be2c2b7f`, the SHA this repo's `.github/workflows/pr-issue-linkage.yml` pins). Its closing-keyword half accepts exactly two forms: ```js const CLOSING_KEYWORD = /\b(close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s*(?:[\w.-]+\/[\w.-]+)?#\d+\b/i; const NO_ISSUE_MARKER = /\bno (?:linked|related) issue\b/i; ``` `Refs #N` matches neither. **The skill's local gate after this change** (§2.4.2): ```bash KEYWORD_REGEX='^(close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved):? #[0-9]+' # Only `No related issue:` — a bare `Refs #N` links without closing and does NOT # satisfy the real pr-issue-linkage validator's closing-keyword half, so accepting # it here would clear a body the CI gate then rejects. OPTOUT_REGEX='^No related issue:' ``` `Refs #N` no longer matches `OPTOUT_REGEX` — the specific false-accept this issue reports is eliminated. **Marker-set relationship.** The two sets are not literally identical, and this PR does not claim they are: the validator additionally honors `No linked issue`, owner/repo-qualified keywords, and looser-whitespace keyword forms that the line-anchored local gate does not. What now holds is the safety property that was broken: **every body the local gate passes, the validator also passes** (the local gate is a strict safe subset). The one marker the local gate accepted that the validator rejects — `Refs #N` — has been removed, so a "skill-gate-satisfied" body can no longer fail the real CI gate on this axis. The skill only ever emits `No related issue:` (never the sibling `No linked issue` phrasing), so the local gate is deliberately mirrored to what the skill produces, not widened to the validator's full superset. **No stale references remain.** Grep of the whole plugin for `Refs #`: ``` create.md:144 multi-issue prompt — `Refs #Y` links additional issues (primary `Closes #N` carries the gate) create.md:153 new prose — routes link-without-close refs to `## Related` create.md:200 `## Related` section usage (correct) create.md:202 rewritten asymmetry note — `Refs #N` satisfies neither gate create.md:214 new OPTOUT_REGEX rationale comment ``` Every surviving `Refs #` is either legitimate `## Related` / multi-issue linking or prose that now states `Refs #N` does **not** satisfy the gate. No occurrence presents `Refs #N` as a closing-keyword opt-out. Closes #630 ## Related - #630 — the issue this PR closes (option 1, the narrow same-repo fix). - #487 / PR #629 — the sibling work this builds on: #629 added the `## Related` scaffold to the create flow and *documented* this `Refs #N` opt-out asymmetry in §2.4.1 as a known, deliberately-out-of-scope gap; this PR is the follow-up that closes it. 🤖 Generated with a Claude Code implementation subagent (issue #630) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…e-filter rationale (#654) Review-bot findings on the open-linked-PR GraphQL mechanic: - Add the missing `| tr -d '\r'` to the final pipeline (repo Gotchas rule). - Document the `first:20` page bound and its cursor-pagination escape hatch. - Document that `select(.state=="OPEN")` is load-bearing, not redundant with `includeClosedPrs:false`: that arg suppresses only CLOSED (unmerged) PRs and still returns MERGED nodes (verified live against issue #487 / merged PR #629), so the OPEN filter is required to avoid a false in-flight report. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… 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
Agent-driven PRs created through
/source-control:pull-request createassemble their body from the skill's own template and ship it viagh pr create --body, which fully overrides.github/pull_request_template.md(cli/cli#10751). The assembled skeleton had## Summary/## Test planbut no## Relatedsection, so PRs in a repo enforcing apr-issue-linkage-style contract (non-empty## Related+ a native closing keyword) failed the gate on first push and burned a red-CI round-trip. This is the create-flow half of the same gap #462 closed for the web/editor authoring path via the repo PR template.Fix
In
plugins/source-control/skills/pull-request/reference/create.md§2.4.1, the body-assembly template now emits a## Relatedsection defaulting to the literalN/A(non-empty by default; replace withRefs #Nreferences to related-but-not-closed PRs/ADRs/decisions when they exist). It pairs with the already-always-present${CLOSES_LINE}closing keyword — populated by §2.4.0 for every branch shape and asserted by the §2.4.2 pre-create gate — so both halves of the contract are scaffolded up front. Surrounding §2.4.1 prose documents the two scaffolds and flags the opt-out asymmetry: a bareRefs #Ndoes not satisfy a validator's closing-keyword half (only a real keyword or aNo linked issue/No related issue:phrase does).plugin.jsonbumped 0.12.0 → 0.12.1 (patch — contract fix to an existing flow) with a matching CHANGELOG entry.Verification
The exact contract was read from the pinned reusable workflow source (
melodic-software/ci-workflows/.github/workflows/pr-issue-linkage.yml@90f1c54, the SHA this repo's.github/workflows/pr-issue-linkage.ymlpins). It runs two independent checks against the HTML-comment-stripped body:## Relatedsection present (/^##\s+related$/i) AND non-empty after trim./\b(close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s*(?:[\w.-]+\/[\w.-]+)?#\d+\b/iOR the marker/\bno (?:linked|related) issue\b/i.The updated template, assembled with a branch-derived
Closes #N, produces:Mapped against the two checks (simulated with the validator's own regexes):
## Relatedmatches^##\s+related$; extracted section content isN/A→ non-empty → pass.Closes #487matches the closing-keyword regex (match: Closes #487) → pass.Both halves are green on first push, no red-CI round-trip.
Known adjacent gap (out of scope, flagged for a maintainer decision): §2.4.0 offers
Refs #Nas a standalone opt-out and §2.4.2'sOPTOUT_REGEXaccepts it, but the real validator's closing-keyword half does not honor a bareRefs #N— only a real keyword or aNo linked issue/No related issue:phrase. ARefs #N-only PR would still clear this skill's local gate and then failpr-issue-linkage. This PR does not change the opt-out mechanism (that is a contract-shaped decision); it documents the asymmetry in §2.4.1 so the mismatch is visible. Recommend a follow-up to reconcile the opt-out marker set with the validator.Closes #487
Related
## Related) — no template scaffolds the required sections #462 — the sibling half of this gap (repo.github/pull_request_template.mdcovering the web/editor authoring path); this PR covers the/source-control:pull-request createflow half.## Relatedsection plus aCloses #487closing keyword, exactly the two-part skeleton the fix now emits.🤖 Generated with a Claude Code implementation subagent (issue #487)