diff --git a/.changes/source-control/work-resolve-issue-issue-13068-41767948125-1-d1c6f5cb.md b/.changes/source-control/work-resolve-issue-issue-13068-41767948125-1-d1c6f5cb.md new file mode 100644 index 00000000000..48c401e2f8c --- /dev/null +++ b/.changes/source-control/work-resolve-issue-issue-13068-41767948125-1-d1c6f5cb.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- **`pull-request` `create` writes the `Siblings:` line when a pull request closes an issue that earlier pull requests delivered part of** ([#13068](https://github.com/melodic-software/claude-code-plugins/issues/13068)). The new `scripts/siblings-line.sh` lists the merged pull requests whose body has a `Refs: #N` line for an issue the pull request closes and that close no issue, the 10 newest. §2.4.0 then writes `Siblings: #, #` directly below the closing line, so pr-verify judges the diff only for what those pull requests left. With none, no line is written. diff --git a/plugins/source-control/skills/pull-request/reference/create.md b/plugins/source-control/skills/pull-request/reference/create.md index 4e6c6372b43..8844715b4fe 100644 --- a/plugins/source-control/skills/pull-request/reference/create.md +++ b/plugins/source-control/skills/pull-request/reference/create.md @@ -218,6 +218,24 @@ on the tracker-bound path, the `refs` item's `ISSUE_BODY` serves. Persist chosen line(s) into `${CLOSES_LINE}`. NEVER wrap a closing keyword in an HTML comment: `` still parses and auto-closes the issue on merge. Fenced code blocks ARE inert, so example snippets are safe. +**Siblings of a closing pull request.** Once `${CLOSES_LINE}` is final, when it closes one or more +GitHub issues, earlier pull requests may have delivered part of one with `Refs: #N`. Name them so +pr-verify judges this diff only for what they left (the pr-verify section of the marketplace's +[pr-pipeline convention](https://raw.githubusercontent.com/melodic-software/claude-code-plugins/main/docs/conventions/pr-pipeline/README.md)). +Run it once with every issue number `${CLOSES_LINE}` closes: + +```bash +SIBLINGS_LINE=$(bash "/scripts/siblings-line.sh" ...) +``` + +It prints one `Siblings: #, #` line for the merged pull requests in this repository whose +body has a `Refs: #N` line of its own and that close no issue, the 10 newest, since pr-verify +credits none from a body naming more; with none it prints nothing. §2.4.1 writes a non-empty +`${SIBLINGS_LINE}` directly below `${CLOSES_LINE}`. When it exits 2, a read failed: tell the user +and write no line. The list comes from GitHub search, which can lag a merge by a few minutes. +pr-verify checks each named pull request's author, base and edits itself. Under §2.7 run it from +`$WT`, since `gh` reads the repository from the current directory. + ### 2.4.1 Push and assemble PR body **Merge the base first when the PR will go ready.** Resolve §2.4.3's open state before the push @@ -390,7 +408,7 @@ Section content is prose a reviewer reads: plain language, bottom line first, no **Left out.** The section that records verification (`Verification`, or `Test plan` under the portable default) ends with one `Left out:` line naming what was not run or checked, each with its reason (for example, `Left out: end-to-end suite, needs a staging environment`), so a reviewer can tell a gap from an omission; when everything relevant was run, it says `Left out: nothing`. When that section closes the body, the §2.4.3 attribution line still follows it as the body's footer. -**Write the body to a file with the Write tool.** Compose the body as literal text: `${CLOSES_LINE}`; when §2.7's `--hold` gave a line, that line on its own line directly below `${CLOSES_LINE}`, before the first `##` block, so the §2.4.2 gates check the body the PR opens with; a blank line when either is non-empty; then one `## ` block per entry in `${SECTION_ORDER[@]}` with real content, leaving out an optional heading that has none (`Related` carries `${REFS_LINES}` when non-empty, else `N/A`), then the `## Acceptance criteria` block when a plan supplied one and no scaffold heading holds it, then the `## Remaining` block for a `Refs:` link, then the ad hoc `## Related` block when the rule above calls for one. Leave the attribution line out; §2.4.3 appends it after the gates. Write it to an absolute path outside the working tree (the session's scratchpad directory when one is listed, else the system temp directory), for example `/pr-body-.md`, and pass that path as `BODY_FILE` to every call below. +**Write the body to a file with the Write tool.** Compose the body as literal text: `${CLOSES_LINE}`; `${SIBLINGS_LINE}` (§2.4.0) when non-empty, at the first column directly below it; when §2.7's `--hold` gave a line, that line on its own line below those, before the first `##` block, so the §2.4.2 gates check the body the PR opens with; a blank line when any is non-empty; then one `## ` block per entry in `${SECTION_ORDER[@]}` with real content, leaving out an optional heading that has none (`Related` carries `${REFS_LINES}` when non-empty, else `N/A`), then the `## Acceptance criteria` block when a plan supplied one and no scaffold heading holds it, then the `## Remaining` block for a `Refs:` link, then the ad hoc `## Related` block when the rule above calls for one. Leave the attribution line out; §2.4.3 appends it after the gates. Write it to an absolute path outside the working tree (the session's scratchpad directory when one is listed, else the system temp directory), for example `/pr-body-.md`, and pass that path as `BODY_FILE` to every call below. **Why a file, not a shell variable:** the Write tool's content is a JSON string no shell parses, so a `$(...)`, `${...}` or backtick typed into the orphan-PR or multi-issue prompt stays literal, and a long body never pushes a Bash command past the tool's length cap (inline `--body` calls were rejected for length on an unattended lane). The §2.4.2 gates read the same file `--body-file` hands to `gh`. @@ -645,7 +663,7 @@ BRANCH=$(git -C "$WT" branch --show-current) - `--draft ''` is §2.4.3's request level, the same flag the normal `create` path takes, for example `Draft: merge hold unread`. - `--hold ''` takes a line starting `Hold:`, which §2.4.1's body composition places - directly below `${CLOSES_LINE}` in `$BODY_FILE` before the §2.4.2 gates run, so the PR opens + below `${CLOSES_LINE}` and any `Siblings:` line in `$BODY_FILE` before the §2.4.2 gates run, so the PR opens with the hold before `ready --unattended` can flip it. A value not starting `Hold:` is refused: STOP and report it. diff --git a/plugins/source-control/skills/pull-request/scripts/siblings-line.sh b/plugins/source-control/skills/pull-request/scripts/siblings-line.sh new file mode 100755 index 00000000000..ab2bf438199 --- /dev/null +++ b/plugins/source-control/skills/pull-request/scripts/siblings-line.sh @@ -0,0 +1,80 @@ +#!/usr/bin/env bash +# siblings-line.sh — Print the Siblings: line for a pull request that closes issues. +# +# When several pull requests deliver one issue, the earlier ones carry a +# `Refs: #` line and close nothing, and the last one closes the issue +# and names them on a first-column `Siblings: #, #` line. This lists the +# merged pull requests in the repository whose body has a `Refs: #` +# line of its own, for any issue given, and that close no issue, and prints +# one line naming them in ascending order. pr-verify credits none when a body +# names more than 10, so the line keeps the 10 newest. With none, it prints +# nothing. pr-verify's resolve-pr applies the credit checks (author, base +# branch, edits), so this does not. +# +# Usage: +# siblings-line.sh ... [--repo ] +# +# Exit codes: +# 0 the line, or nothing when no pull request matches +# 1 invalid argument +# 2 a gh read failed; no line was printed +# 5 prerequisite missing (gh, jq) + +# portability-scope: the GitHub implementation of create's Siblings: line; the read is a gh call. +set -uo pipefail + +usage() { + sed -n '2,/^$/p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' + exit 0 +} + +die() { + printf 'siblings-line: %s\n' "$2" >&2 + exit "$1" +} + +ISSUES=() REPO="" +while (($# > 0)); do + case "$1" in + -h | --help) usage ;; + --repo) + (($# >= 2)) || die 1 "--repo needs a value" + REPO="$2" + shift 2 + ;; + -*) die 1 "unknown flag $(printf '%q' "$1") (use --help)" ;; + *) + [[ "$1" =~ ^[1-9][0-9]{0,8}$ ]] || die 1 "not an issue number: $(printf '%q' "$1")" + ISSUES+=("$1") + shift + ;; + esac +done +((${#ISSUES[@]} > 0)) || die 1 "an issue number is required (use --help)" + +command -v gh >/dev/null 2>&1 || die 5 "gh not found on PATH" +command -v jq >/dev/null 2>&1 || die 5 "jq not found on PATH" + +repo_args=() +[[ -n "$REPO" ]] && repo_args=(--repo "$REPO") + +found='[]' +for issue in "${ISSUES[@]}"; do + # The search narrows the list; the jq filter below decides, since search + # matches words, not the line. + prs=$(gh pr list ${repo_args[@]+"${repo_args[@]}"} --state merged --limit 100 \ + --search "\"Refs: #$issue\" in:body" --json number,body,closingIssuesReferences) || + die 2 "gh pr list failed for #$issue" + found=$(jq -c --arg n "$issue" --argjson found "$found" ' + $found + [.[] + | select((.closingIssuesReferences // []) | length == 0) + | select(.body // "" | test("(?m)^Refs:[ \\t]*#" + $n + "[ \\t]*\\r?$")) + | .number] + ' <<<"$prs") || die 2 "cannot parse the pull request list for #$issue" +done + +numbers=$(jq -r 'unique | .[-10:] | map("#\(.)") | join(", ")' <<<"$found") || + die 2 "cannot build the line" + +[[ -n "$numbers" ]] && printf 'Siblings: %s\n' "$numbers" +exit 0 diff --git a/plugins/source-control/skills/pull-request/scripts/siblings-line.test.sh b/plugins/source-control/skills/pull-request/scripts/siblings-line.test.sh new file mode 100755 index 00000000000..5936899deae --- /dev/null +++ b/plugins/source-control/skills/pull-request/scripts/siblings-line.test.sh @@ -0,0 +1,102 @@ +#!/usr/bin/env bash +# Regression tests for siblings-line.sh, black-box with a stubbed `gh` on PATH. + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPT="$SCRIPT_DIR/siblings-line.sh" +TEST_TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TEST_TMPDIR"' EXIT + +# shellcheck source=../../../scripts/test-helpers.sh +source "$SCRIPT_DIR/../../../scripts/test-helpers.sh" + +command -v jq >/dev/null 2>&1 || skip_suite "jq not installed" + +STUB_DIR="$TEST_TMPDIR/stubs" +mkdir -p "$STUB_DIR" +# The merged pull request list comes from $TEST_TMPDIR/prs.json; a missing +# file fails the call. +cat >"$STUB_DIR/gh" <>"$TEST_TMPDIR/calls" +case "\$1 \$2" in + "pr list") [[ -f "$TEST_TMPDIR/prs.json" ]] || exit 1; cat "$TEST_TMPDIR/prs.json" ;; + *) echo "gh-stub: unrouted \$*" >&2; exit 1 ;; +esac +STUB_EOF +chmod +x "$STUB_DIR/gh" + +# entry [closing-refs-json] +entry() { + jq -n --argjson n "$1" --arg b "$2" --argjson c "${3:-[]}" \ + '{number: $n, body: $b, closingIssuesReferences: $c}' +} + +prs() { + jq -s . >"$TEST_TMPDIR/prs.json" +} + +run() { + : >"$TEST_TMPDIR/calls" + PATH="$STUB_DIR:$PATH" bash "$SCRIPT" "$@" +} + +entry 12974 $'Refs: #12918\n\n## Why\n\nPart one.' | prs +out=$(run 12918) +assert_exit "a merged Refs: pull request exits 0" 0 "$?" +assert_eq "a merged Refs: pull request is named on a Siblings: line" "Siblings: #12974" "$out" +calls=$(cat "$TEST_TMPDIR/calls") +assert_contains "only merged pull requests are listed" "$calls" "--state merged" +assert_contains "the search names the Refs: line" "$calls" '"Refs: #12918" in:body' + +printf '[]' >"$TEST_TMPDIR/prs.json" +out=$(run 12918) +assert_exit "no matching pull request exits 0" 0 "$?" +assert_silent "no matching pull request writes no Siblings: line" "$out" + +{ + entry 30 $'Refs: #7\r\n' + entry 20 $'## Why\n\nRefs: #7 \n' +} | prs +assert_eq "several siblings are listed oldest first" "Siblings: #20, #30" "$(run 7)" + +{ + entry 21 "Closes #7" '[{"number":7}]' + entry 22 $'Refs: #7\nCloses #8' '[{"number":8}]' + entry 23 "Refs: #77" + entry 24 "See Refs: #7 for context" + entry 25 "Refs #7" + entry 26 "Refs: #7, #8" +} | prs +out=$(run 7) +assert_silent "a closing, mid-line, other-issue or colon-less Refs is no sibling" "$out" + +for n in $(seq 101 112); do entry "$n" "Refs: #7"; done | prs +assert_eq "at most 10 siblings are named, the newest" \ + "Siblings: #103, #104, #105, #106, #107, #108, #109, #110, #111, #112" "$(run 7)" + +{ + entry 40 "Refs: #7" + entry 41 "Refs: #8" + entry 42 "Refs: #9" +} | prs +assert_eq "several closed issues share one Siblings: line" "Siblings: #40, #41" "$(run 7 8)" +assert_eq "one search runs per issue" "2" "$(grep -c 'pr list' "$TEST_TMPDIR/calls")" + +rm -f "$TEST_TMPDIR/prs.json" +out=$(run 7 2>/dev/null) +assert_exit "a failed gh read exits 2" 2 "$?" +assert_silent "a failed gh read prints no line" "$out" + +printf '[]' >"$TEST_TMPDIR/prs.json" +run 7 --repo acme/app >/dev/null +assert_contains "--repo reaches gh" "$(cat "$TEST_TMPDIR/calls")" "--repo acme/app" + +run >/dev/null 2>&1 +assert_exit "no issue number exits 1" 1 "$?" + +run 'x7' >/dev/null 2>&1 +assert_exit "a non-numeric issue exits 1" 1 "$?" + +[[ $FAILED -eq 0 ]] || exit 1