Skip to content

Commit 064e7f3

Browse files
johanzanderclaude
andauthored
fix: link a PR to its issue on any #N reference, not just closing keywords (#652) (#679)
The digest linked an open PR to its issue only when the PR body used a closing keyword (fixes/closes/resolves/refs #N). The no-auto-close rule forbids those on intermediate PRs -- they say "Part of #N", "tracking #N", or a bare "#N" -- so such a PR was invisible: issue #409 reported In Progress while its approved PR (#490) sat open. Linkage now matches any #N reference, with documented cross-ref phrases (Blocked by, Depends on, Unblocks, Related to, Relationship to, See also, See, Not part of) stripped first so they never link. The merged-PR scan is deliberately narrower (work verbs: fixes/closes/ resolves/refs/part of/tracking/tracks): a merged intermediate PR keeps its issue In Verification (a narrowed scan had flipped #643/#542/#571/#592/#666 back to Backlog/Ready for Dev on the live board), while a merged PR that merely names another issue in prose does not. mergeable is re-queried until it leaves UNKNOWN (budget 6) and emitted null if it never resolves. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 949121f commit 064e7f3

3 files changed

Lines changed: 311 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
88

99
### Fixed
1010

11+
- **Backlog digest now links a PR to its issue on any `#N` reference, not just closing keywords** — a "Part of #N" PR no longer leaves its issue showing In Progress. ([#652](https://github.com/johanzander/bess-manager/issues/652))
1112
- **Beta release changelog merges no longer absorb the new section into the previous one** — the merge is now resolved deterministically instead of by hand. ([#648](https://github.com/johanzander/bess-manager/issues/648))
1213

1314
## [10.1.0] - 2026-08-22

backend/tests/test_backlog_digest.py

Lines changed: 247 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,144 @@ def test_an_issue_with_several_prs_reports_all_of_them(bin_dir: Path) -> None:
268268
assert "pr" not in item
269269

270270

271+
def test_a_part_of_pr_is_linked_to_its_issue(bin_dir: Path) -> None:
272+
"""The no-auto-close rule forbids `Closes #N` on an intermediate PR, so a
273+
beta PR says `Part of #N` (or `tracking #N`, or nothing but a bare `#N`)
274+
instead. A digest that links by closing keyword only therefore makes that
275+
PR invisible: #409 reported In Progress while its PR #490 sat approved,
276+
because `prs_for` resolved to nothing and the column fell through to the
277+
live worktree. Linkage must match any `#N` reference, not just
278+
`fixes/closes/resolves/refs`.
279+
280+
The branch name deliberately carries no issue number, so the association
281+
is proved by the body reference alone -- the headRefName fallback is
282+
exercised by its own test and must not mask this one."""
283+
issue = _issue(409, labels=[{"name": "bug"}])
284+
pr = _pr(
285+
490,
286+
body="Part of #409 — this PR covers PredictionSnapshotStore only.",
287+
headRefName="feat/prediction-snapshot-store",
288+
isDraft=False,
289+
mergeable="MERGEABLE",
290+
)
291+
_write_shim(bin_dir, "gh", _gh_shim([issue], [pr], []))
292+
_write_shim(
293+
bin_dir,
294+
"git",
295+
_git_shim(
296+
_porcelain(
297+
("/repo/wt/409", "feat/issue-409-prediction-snapshot-consolidation")
298+
)
299+
),
300+
)
301+
302+
digest = _run(bin_dir)
303+
item = digest["items"][0]
304+
305+
assert [p["number"] for p in item["prs"]] == [490]
306+
assert item["column"] == "In Review" # was In Progress before the fix
307+
# ...and the PR is no longer reported as belonging to no issue.
308+
assert [o for o in digest["orphans"] if o["kind"] == "pr_no_issue"] == []
309+
310+
311+
def test_a_bare_number_reference_links_a_pr_to_its_issue(bin_dir: Path) -> None:
312+
"""`tracking #N` and a bare `#N` are the other spellings the no-auto-close
313+
rule leaves an intermediate PR with. Any `#N` in the body must link."""
314+
issue = _issue(633, labels=[{"name": "bug"}])
315+
pr = _pr(
316+
634,
317+
body="First of two PRs tracking #633; the issue stays open until the second lands.",
318+
headRefName="feat/split-work-a",
319+
isDraft=False,
320+
)
321+
_write_shim(bin_dir, "gh", _gh_shim([issue], [pr], []))
322+
323+
item = _run(bin_dir)["items"][0]
324+
325+
assert [p["number"] for p in item["prs"]] == [634]
326+
assert item["column"] == "In Review"
327+
328+
329+
def test_a_blocked_by_reference_does_not_link_a_pr_to_its_issue(bin_dir: Path) -> None:
330+
"""`Blocked by #N` is a documented convention -- a PR that waits on issue N
331+
is not part of N's work. The widened `any #N` linkage must not grab it, or
332+
a PR that merely names its blocker would flip the blocked issue to
333+
In Review and clear the PR's orphan status."""
334+
issue = _issue(900, labels=[{"name": "bug"}])
335+
pr = _pr(
336+
901,
337+
body="Blocked by #900 \u2014 landing once the price-provider decision is made.",
338+
headRefName="feat/price-provider-wait",
339+
isDraft=True,
340+
)
341+
_write_shim(bin_dir, "gh", _gh_shim([issue], [pr], []))
342+
343+
digest = _run(bin_dir)
344+
item = digest["items"][0]
345+
346+
assert item["prs"] == []
347+
assert item["column"] == "Backlog"
348+
assert [o for o in digest["orphans"] if o["kind"] == "pr_no_issue"] != []
349+
350+
351+
def test_a_blocked_by_side_reference_still_links_the_prs_own_issue(
352+
bin_dir: Path,
353+
) -> None:
354+
"""Phrase-stripping removes only the blocker reference, not the whole line:
355+
"- Blocked by #900 \u2014 part of #905" links the PR to #905 while leaving
356+
#900 untouched."""
357+
issues = [
358+
_issue(900, labels=[{"name": "bug"}]),
359+
_issue(905, labels=[{"name": "bug"}]),
360+
]
361+
pr = _pr(
362+
901,
363+
body="- Blocked by #900 \u2014 part of #905",
364+
headRefName="feat/prediction-snapshot-store",
365+
isDraft=False,
366+
)
367+
_write_shim(bin_dir, "gh", _gh_shim(issues, [pr], []))
368+
369+
items = {i["number"]: i for i in _run(bin_dir)["items"]}
370+
371+
assert [p["number"] for p in items[905]["prs"]] == [901]
372+
assert items[900]["prs"] == []
373+
374+
375+
@pytest.mark.parametrize(
376+
"body",
377+
[
378+
"Related to #900. Not closing it.",
379+
"Not blocked by #900 anymore \u2014 resuming.",
380+
"Depends on #900.",
381+
"Unblocks #900.",
382+
"Unblocking #900.",
383+
"See also #900.",
384+
"See #900 for the original report.",
385+
"Relationship to #900.",
386+
"Unrelated to #900.",
387+
"Not part of #900 anymore.",
388+
],
389+
)
390+
def test_a_non_work_reference_does_not_link_a_pr_to_its_issue(
391+
bin_dir: Path, body: str
392+
) -> None:
393+
"""Phrases that name an issue without claiming to work on it must not link
394+
-- real bodies say "Related to #403. Not closing it", "unblocks #485",
395+
"unrelated to #402". Linking on them would flip an unrelated issue to
396+
In Review and clear the PR's orphan status."""
397+
issue = _issue(900, labels=[{"name": "bug"}])
398+
pr = _pr(901, body=body, headRefName="feat/price-provider-wait", isDraft=True)
399+
_write_shim(bin_dir, "gh", _gh_shim([issue], [pr], []))
400+
401+
digest = _run(bin_dir)
402+
item = digest["items"][0]
403+
404+
assert item["prs"] == []
405+
assert item["column"] == "Backlog"
406+
assert [o for o in digest["orphans"] if o["kind"] == "pr_no_issue"] != []
407+
408+
271409
def test_open_pr_list_actually_requests_isdraft(bin_dir: Path) -> None:
272410
"""`prs_for` emits `isDraft` on every PR object, but jq can only surface a
273411
field the `gh pr list --json ...` call actually requested -- a fixture
@@ -569,12 +707,61 @@ def test_a_merged_pr_with_the_issue_open_is_in_verification(bin_dir: Path) -> No
569707
this period unnamed, so a fix awaiting real-world confirmation sat in
570708
whatever column it happened to be in."""
571709
issue = _issue(510, labels=[{"name": "bug"}, {"name": "analyzed"}])
572-
merged = [_pr(511, body="Refs #510", headRefName="fix/issue-510")]
710+
merged = [_pr(511, body="Closes #510", headRefName="fix/issue-510")]
573711
_write_shim(bin_dir, "gh", _gh_shim([issue], [], [], merged))
574712

575713
assert _run(bin_dir)["items"][0]["column"] == "In Verification"
576714

577715

716+
def test_a_merged_intermediate_pr_keeps_the_issue_in_verification(
717+
bin_dir: Path,
718+
) -> None:
719+
"""A merged intermediate PR (`Part of #N`) means the work has landed on main
720+
and is awaiting graduation -- In Verification, never re-dispatchable.
721+
722+
This is the no-auto-close contract: beta PRs omit `Closes #N` until the
723+
fix graduates, so `Part of`/`Refs` are how a fix normally reads on merge.
724+
When the merged scan was narrowed to closing keywords only, issues whose
725+
fix had already merged (#643 -> #675, #571 -> #584, #592 -> #619, #666 ->
726+
#672, #542 -> #591) fell through to Backlog / Ready for Dev, so a backlog
727+
pass could re-dispatch an issue whose partial work already landed."""
728+
issue = _issue(517, labels=[{"name": "bug"}, {"name": "analyzed"}])
729+
merged = [_pr(518, body="Part of #517", headRefName="fix/issue-517-a")]
730+
_write_shim(bin_dir, "gh", _gh_shim([issue], [], [], merged))
731+
732+
item = _run(bin_dir)["items"][0]
733+
734+
assert item["merged_pr"] == 518
735+
assert item["merged_prs"] == [518]
736+
assert item["column"] == "In Verification"
737+
738+
739+
def test_a_merged_cross_ref_does_not_move_an_issue_to_in_verification(
740+
bin_dir: Path,
741+
) -> None:
742+
"""The merged scan is deliberately narrower than the open-PR one: it uses
743+
work verbs only (`fixes/closes/resolves/refs/part of/tracking`), never bare
744+
`#N`. A merged PR that merely names another issue -- "Related to #403. Not
745+
closing it -- leaving it open until #456 and #457 are also resolved" --
746+
must not flip that issue to In Verification."""
747+
issue = _issue(403, labels=[{"name": "bug"}])
748+
merged = [
749+
_pr(
750+
453,
751+
body="Related to #403. Not closing it -- leaving it open until "
752+
"#456 and #457 are also resolved.",
753+
headRefName="fix/issue-403-logging",
754+
)
755+
]
756+
_write_shim(bin_dir, "gh", _gh_shim([issue], [], [], merged))
757+
758+
item = _run(bin_dir)["items"][0]
759+
760+
assert item["merged_pr"] is None
761+
assert item["merged_prs"] == []
762+
assert item["column"] != "In Verification"
763+
764+
578765
def test_an_open_pr_outranks_a_merged_one(bin_dir: Path) -> None:
579766
"""A graduation PR still open means the work is In Review, not verified."""
580767
issue = _issue(512, labels=[{"name": "bug"}])
@@ -678,6 +865,65 @@ def test_conflicting_pr_is_reported_on_its_issue(bin_dir: Path) -> None:
678865
assert item["column"] == "In Review"
679866

680867

868+
def test_mergeable_is_requeried_until_it_leaves_unknown(bin_dir: Path) -> None:
869+
"""GitHub computes `mergeable` LAZILY: the first query on a cold PR returns
870+
UNKNOWN and triggers the computation, so a single query reports UNKNOWN as
871+
if it were a verdict (measured on #490: six consecutive UNKNOWN passes).
872+
The digest must re-query until the value settles, exactly as sweep-prs
873+
does. This shim returns UNKNOWN on the first `pr list` and MERGEABLE on
874+
the second, so only a re-query produces the asserted value."""
875+
issue = _issue(801, labels=[{"name": "bug"}])
876+
# `Refs #N` (not `Part of #N`): this test isolates the mergeable retry,
877+
# and `Refs` already links under both the old and new linkage rules.
878+
unknown_pr = _pr(
879+
802, body="Refs #801", headRefName="fix/part-a", mergeable="UNKNOWN"
880+
)
881+
mergeable_pr = dict(unknown_pr, mergeable="MERGEABLE")
882+
counter = bin_dir / "gh_pr_list_calls"
883+
shim = f"""
884+
case "$*" in
885+
*"issue list"*) cat <<'EOF'
886+
{json.dumps([issue])}
887+
EOF
888+
;;
889+
*"pr diff "*"--name-only"*) : ;;
890+
*"pr list"*"--state merged"*) printf '%s\\n' '[]' ;;
891+
*"pr list"*)
892+
if [ -f '{counter}' ]; then
893+
cat <<'EOF'
894+
{json.dumps([mergeable_pr])}
895+
EOF
896+
else
897+
touch '{counter}'
898+
cat <<'EOF'
899+
{json.dumps([unknown_pr])}
900+
EOF
901+
fi
902+
;;
903+
*"project item-list"*) printf '%s\\n' '{{"items": []}}' ;;
904+
*) echo "unexpected gh call: $*" >&2; exit 1 ;;
905+
esac
906+
"""
907+
_write_shim(bin_dir, "gh", shim)
908+
909+
item = _run(bin_dir, MERGE_RETRY_SLEEP="0")["items"][0]
910+
911+
assert item["prs"][0]["mergeable"] == "MERGEABLE"
912+
913+
914+
def test_mergeable_still_unknown_after_retries_is_reported_null(bin_dir: Path) -> None:
915+
"""If GitHub has still not computed `mergeable` inside the retry budget,
916+
the digest must not pass UNKNOWN through as if it were a definite state --
917+
it emits null, so no consumer can read it as a verdict."""
918+
issue = _issue(803, labels=[{"name": "bug"}])
919+
pr = _pr(804, body="Refs #803", headRefName="fix/part-b", mergeable="UNKNOWN")
920+
_write_shim(bin_dir, "gh", _gh_shim([issue], [pr], []))
921+
922+
item = _run(bin_dir, MERGE_RETRY_SLEEP="0")["items"][0]
923+
924+
assert item["prs"][0]["mergeable"] is None
925+
926+
681927
def test_issue_matched_by_two_prs_emits_one_item_with_both_prs(
682928
bin_dir: Path,
683929
) -> None:

scripts/backlog-digest.sh

Lines changed: 63 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,25 @@ fi
5555
issues=$(gh issue list --repo "$repo" --state open --limit 200 \
5656
--json number,title,labels,author,createdAt,updatedAt,comments,body)
5757

58+
# GitHub computes `mergeable` LAZILY: the first query on a cold PR returns
59+
# UNKNOWN and triggers the computation, so a single pass would report UNKNOWN
60+
# as if it were a verdict. Re-query until no open PR is still UNKNOWN -- the
61+
# budget covers the #490 measurement, which stayed UNKNOWN for six consecutive
62+
# passes -- because the digest must never hang on a cold PR. A PR left UNKNOWN
63+
# after the budget is emitted as null in `prs_for` below, so it never
64+
# masquerades as a definite state.
65+
MERGE_RETRY_LIMIT="${MERGE_RETRY_LIMIT:-6}"
66+
MERGE_RETRY_SLEEP="${MERGE_RETRY_SLEEP:-2}"
5867
prs=$(gh pr list --repo "$repo" --state open --limit 100 \
5968
--json number,title,headRefName,mergeable,body,isDraft)
69+
merge_attempts=0
70+
while printf '%s' "$prs" | jq -e 'any(.[]; .mergeable == "UNKNOWN")' >/dev/null 2>&1 \
71+
&& [ "$merge_attempts" -lt "$MERGE_RETRY_LIMIT" ]; do
72+
merge_attempts=$((merge_attempts + 1))
73+
sleep "$MERGE_RETRY_SLEEP"
74+
prs=$(gh pr list --repo "$repo" --state open --limit 100 \
75+
--json number,title,headRefName,mergeable,body,isDraft)
76+
done
6077

6178
# The EXACT half of the collision gate: what every open PR already touches.
6279
# One gh pr diff per open PR, bounded by the WIP limit in practice. The
@@ -110,7 +127,13 @@ merged_prs=$(gh pr list --repo "$repo" --state merged --limit 200 \
110127
| jq -c '[ .[] | {
111128
number,
112129
headRefName,
113-
refs: [ (.body // "") | scan("(?i)(?:fixes|closes|resolves|refs) #([0-9]+)") | .[0] | tonumber ]
130+
# WORK references only -- the closing verbs plus the no-auto-close
131+
# spellings (`Part of`, `tracking`, `Refs`). Deliberately NOT bare `#N`:
132+
# a merged PR body can name other issues without working on them
133+
# ("until #456 and #457 are also resolved"), and a merged PR must not
134+
# flip an unrelated issue to In Verification. Drives both `merged_pr`
135+
# (the column) and `merged_prs` (the visibility list).
136+
refs: [ (.body // "") | scan("(?i)(?:fixes|closes|resolves|refs|part of|tracking|tracks) #([0-9]+)") | .[0] | tonumber ]
114137
} ]')
115138

116139
# Emits, per worktree, a JSON object of {path, branch, locked}. `git worktree
@@ -206,20 +229,45 @@ jq -n \
206229
def resume_count($comments):
207230
[ $comments[]? | select((.body // "") | contains("<!-- resume-handoff -->")) ] | length;
208231
209-
# `refs` joins the closing verbs deliberately. The project rule is that a beta
210-
# or intermediate PR must NOT close the reporters issue -- only the graduation
211-
# PR does -- so an intermediate PR carries `Refs #N` and would otherwise
212-
# associate with nothing at all.
232+
# LINKAGE is any `#N` reference in the body, not just the closing verbs. The
233+
# no-auto-close rule forbids `Closes #N` on an intermediate PR -- it says
234+
# `Part of #N`, `tracking #N`, `Refs #N`, or a bare `#N` -- so a digest that
235+
# links by closing keyword only makes that PR invisible (the #409/#490
236+
# defect). Whether the work has LANDED is the merged-PR scan above, which is
237+
# deliberately narrower (work verbs only) so a merged PR that merely names
238+
# another issue cannot flip it to In Verification; linkage here is the broad
239+
# any-`#N` net for OPEN PRs. The number is bounded on both sides so `#2409`
240+
# does not match issue 409 and `#4095` does not match 409.
241+
#
242+
# Cross-references that name an issue WITHOUT claiming to work on it are
243+
# stripped before matching, so they neither link nor orphan-claim: the
244+
# documented `Blocked by #N` convention, `Depends on`, `Unblocks`,
245+
# `Related to` (and `unrelated to`), `Relationship to`, `Follow-up to`,
246+
# `See also`. Real bodies use these -- "Related to #403. Not closing it",
247+
# "unblocks #485", "unrelated to #402" -- and linking on them would flip an
248+
# unrelated issue to In Review. Stripping the PHRASE, not the whole
249+
# line, keeps a combined reference like "- Blocked by #100 -- part of #409"
250+
# working: only the blocker phrase disappears and #409 still links. The
251+
# remaining test is still any `#N`, so the no-auto-close spellings
252+
# (`Part of #N`, `tracking #N`, `Refs #N`, bare `#N`) all link.
253+
def linkage_body($body):
254+
($body // "")
255+
| gsub("(?i)(blocked by|depends on|unblocks?(?:ing)?|related to|relationship to|follow[- ]?up to|see also|see|not part of) #[0-9]+"; "");
256+
213257
def pr_matches_issue($p; $n):
214-
($p.body // "" | test("(?i)(fixes|closes|resolves|refs) #\($n)\\b"))
258+
(linkage_body($p.body) | test("(?i)(^|[^0-9])#\($n)\\b"))
215259
or ($p.headRefName | test("issue-\($n)(\\D|$)"));
216260
217261
# Returns EVERY matching PR, ascending. Taking `[0]` discarded the rest, and
218262
# with one issue routinely carrying several PRs that meant the column was
219263
# derived from whichever happened to sort first.
220264
def prs_for($n):
221265
[ $prs[] | select(pr_matches_issue(.; $n))
222-
| {number: .number, mergeable: .mergeable, isDraft: .isDraft} ]
266+
| {number: .number,
267+
# A mergeable still UNKNOWN after the retry loop is not a
268+
# verdict -- report null so no consumer reads it as definite.
269+
mergeable: (if .mergeable == "UNKNOWN" then null else .mergeable end),
270+
isDraft: .isDraft} ]
223271
| sort_by(.number);
224272
225273
# Matches a worktree whose path OR branch contains the issue number in a
@@ -457,6 +505,14 @@ jq -n \
457505
last_comment: last_comment(.comments; .author.login),
458506
priority: $prio,
459507
prs: $open_prs,
508+
# Every merged PR whose body references this issue with a WORK verb
509+
# (`fixes/closes/resolves/refs/part of/tracking`) -- the visibility
510+
# list. `merged_pr` above is the first in the order `gh` returns
511+
# (the most recent merge) and drives the In Verification column; this
512+
# plural exposes all of them, sorted, so a merged
513+
# intermediate PR (`Part of #N`, which must not close the issue)
514+
# stays visible even alongside later PRs.
515+
merged_prs: ([ $merged_prs[] | select((.refs | index($i.number)) != null) | .number ] | sort),
460516
merged_pr: $merged_pr,
461517
worktree: ($wt.path // null),
462518
worktree_branch: ($wt.branch // null),

0 commit comments

Comments
 (0)