diff --git a/changelog.d/fixed-contribute-merged-refs-claim.md b/changelog.d/fixed-contribute-merged-refs-claim.md new file mode 100644 index 0000000000..dd6a684423 --- /dev/null +++ b/changelog.d/fixed-contribute-merged-refs-claim.md @@ -0,0 +1 @@ +- Fix the contribute queue holding an issue as "already claimed by a PR" after a PR that only said `Refs #N` merged. Such a PR landed part of the work and left the issue open for the rest, so the issue now goes back to contributors, as it already did for agents (#7061). A merged `Refs #N` also ranks below every open PR in the claim ledger, so it never hides someone's open PR on the same issue. The contribute-ws `skip:` admission lines are now logged when an item is first withheld or its reason changes, not on every pass. diff --git a/src/docs/contributor-relay.md b/src/docs/contributor-relay.md index 585e467184..7e981d7433 100644 --- a/src/docs/contributor-relay.md +++ b/src/docs/contributor-relay.md @@ -1225,7 +1225,7 @@ Owners and read-write operators can use the Operations page's **Message** action Two admission behaviors are worth knowing when your relay seems idle: -- **Issues already claimed by an open PR — or settled by a recently-merged one — are skipped.** The hub's claim ledger records every open PR that references an issue with a closing keyword (`fixes #N`, `closes owner/repo#N`, …) — including PRs from external authors, not just hive agents ([#3792](https://github.com/hivecommons/hive/pull/3792)). Since [#6867](https://github.com/hivecommons/hive/issues/6867) the same scan also covers **recently-merged** PRs: a fix that merged *without* a closing keyword leaves its issue open, and its claim used to vanish the moment the PR left the open set, so the settled issue went straight back to contributors (and agent dispatch) whose only possible verdict was "already resolved on `main`" — measured downstream at ~34% of contributor sessions. Merged claims are graded by the same evidence tiers as open ones and suppress identically; across tiers, evidence wins, so a merged `Fixes #N` outranks an open `Refs #N`. A claimed issue is silently dropped from the contribute candidate set; if nothing else is admissible the relay receives `task_unavailable` with reason `no_matching_work` (there is no per-issue "claimed by PR #N" message). External claims — like the weaker non-closing `Refs #N` references — now DEFER the hive's own agents for a bounded 72h window, then release the issue even while the PR stays open ([#4929](https://github.com/hivecommons/hive/issues/4929)). They previously never suppressed agent work at all, which let an agent that cannot check for existing PRs re-implement an issue a live PR already covered. Nothing is frozen: the window is a bound — anchored at first observation for an open claim, and at the **merge** for a merged one, so a merged `Refs #N` still releases an epic's remainder — and a red+stale claiming PR defers nothing. +- **Issues already claimed by an open PR — or settled by a recently-merged one — are skipped.** The hub's claim ledger records every open PR that references an issue with a closing keyword (`fixes #N`, `closes owner/repo#N`, …) — including PRs from external authors, not just hive agents ([#3792](https://github.com/hivecommons/hive/pull/3792)). Since [#6867](https://github.com/hivecommons/hive/issues/6867) the same scan also covers **recently-merged** PRs: a fix that merged *without* a closing keyword leaves its issue open, and its claim used to vanish the moment the PR left the open set, so the settled issue went straight back to contributors (and agent dispatch) whose only possible verdict was "already resolved on `main`" — measured downstream at ~34% of contributor sessions. Merged claims are graded by the same evidence tiers as open ones and suppress identically; across tiers, evidence wins, so a merged `Fixes #N` outranks an open `Refs #N`. A merged `Refs #N` (or `Part of #N`) is the exception: it never said the issue was done, so the contribute queue does not count it and the issue goes back to contributors for the rest of the work, as it already does for agents ([#7061](https://github.com/hivecommons/hive/issues/7061)). It also ranks below every open PR, so it never hides someone's open PR on the same issue. A claimed issue is silently dropped from the contribute candidate set; if nothing else is admissible the relay receives `task_unavailable` with reason `no_matching_work` (there is no per-issue "claimed by PR #N" message). External claims — like the weaker non-closing `Refs #N` references — now DEFER the hive's own agents for a bounded 72h window, then release the issue even while the PR stays open ([#4929](https://github.com/hivecommons/hive/issues/4929)). They previously never suppressed agent work at all, which let an agent that cannot check for existing PRs re-implement an issue a live PR already covered. Nothing is frozen: the window is a bound — anchored at first observation for an open claim, and at the **merge** for a merged one, so a merged `Refs #N` still releases an epic's remainder — and a red+stale claiming PR defers nothing. - **A `no_work_needed` verdict's citation becomes a verified claim.** The merged-PR scan above only sees PRs that *reference* the issue; a fix that landed without mentioning it is invisible, and on one hosted spoke 5 of 7 tasks on a contributor ended `no_work_needed` re-discovering settlements the previous agent had already found ([#7871](https://github.com/hivecommons/hive/issues/7871)). The agent does name what settled the issue in its `verdict_reason` ("already merged via #532", "covered by merged PR #867", "already in upstream/main (e6d3de3)"), so the hub now parses that reason for `#N`, `owner/repo#N`, pull URLs and 7–40 hex SHAs (the task's own issue number excluded, at most six candidates), checks each against GitHub, and records the **first that holds up** in the same claim ledger, exactly as the scan would have: a PR merged into the task repo *before the task was dispatched* — or a commit reachable from the default branch — is a strong merged claim; an open PR in the task repo by someone else is a weak external claim — *someone else* is enforced: the hub compares the PR's author to the reporting contributor's GitHub login (case-insensitively) and drops a self-authored open PR, so the least-trusted write path cannot defer an issue by citing its own unrelated PR ([#7890](https://github.com/hivecommons/hive/issues/7890)); a merged PR is a fact whoever authored it. The text alone is never trusted: a reference the API cannot confirm (wrong repo, closed-unmerged, not on `main`, merged after dispatch, or simply fabricated) records nothing and today's cooldown applies. What the API check does *not* establish is relevance — a real but unrelated ref passes, since a settling PR by definition never referenced the issue; that residual is bounded by the claim TTL and voided by newer activity on the issue. Verdict-recovered claims carry `source: "verdict"` and survive the authoritative scan that cannot see their PR, until the ordinary 72h TTL retires them or stronger live evidence for the issue replaces them. The check runs off the contributor's read loop and requires a wired claim ledger and GitHub client; a hive without either behaves as before. - **A closing keyword written as prose counts.** The claim parser used to require GitHub's own trailer shape, `keyword #N` (optionally with a colon), so a merged PR whose body read *"Resolves architect issue #1232 (first incremental step)"* produced **no claim at all** ([#7995](https://github.com/hivecommons/hive/issues/7995)). Since that issue the closing tier accepts the same bounded run of prose the non-closing reference tier already did — at most 40 characters, no `.`, newline or `#` — so `Resolves architect issue #1232` and `Fixes the bug in #12` are closing claims. GitHub will still not auto-close on those, and hive does not pretend it will: a claim here is about whether the **work landed**, and a merged PR that says it resolves an issue is the strongest evidence the hub gets, so it suppresses rather than being released for re-verification after every merge. The prose gap must begin with a space or tab, which is what keeps a Conventional Commits type prefix (`fix:`, `fix(scope):`) from claiming the first issue number in a title. The closing tier remains single-issue, matching GitHub's one-keyword-per-number rule; only the reference tier reads lists. - **A merged fix does not expire on a clock.** A strong claim from a merged PR — or a verified `no_work_needed` verdict — used to leave the ledger 72 hours after the merge, because the settle scan looks back 72h and the ledger TTL matched it. The PR was still merged; only the clock had moved. So the issue went straight back into the offer pool, an agent re-verified "already fixed on main", and the verdict claim it left sat on the same 72h clock — one wasted task cycle every three days for as long as nobody closed the issue ([#8003](https://github.com/hivecommons/hive/issues/8003): projectbluefin/chairlift#55, fixed by #133, re-offered three times with two "please close" agent comments on it). Settled claims are now carried forward across scans for as long as the issue stays open — the 30-day retention bound counts from the last enumeration pass that listed the issue as open, not from the merge, so it only prunes claims whose issue has since closed ([#11580](https://github.com/hivecommons/hive/issues/11580)); a merged *weak* claim (a `Refs #N`, or an external author's PR) is not — it never asserted it finished the issue, and is still released with context as before. And because the hold has by then outlasted every automatic bound, after **7 days** the contribute queue stops describing it as "an open pull request already claims this issue" and withholds it under `merged_claim_stale`, whose prose is the question a maintainer has to answer: *Fixed by merged PR #133 4 days ago; the issue is still open — close it or say what remains.* diff --git a/src/pkg/dashboard/contribute_admission.go b/src/pkg/dashboard/contribute_admission.go index 70878fcd6c..80272c20e4 100644 --- a/src/pkg/dashboard/contribute_admission.go +++ b/src/pkg/dashboard/contribute_admission.go @@ -275,21 +275,33 @@ func observeExternalDependencies(candidate contributorAdmissionCandidate) conver // first since cross-repo `owner/repo#N` closing references key on it. A hub // with no hook wired (tests, or a hive booted without GitHub credentials) // reports no claims and selection proceeds exactly as before. +// +// A merged `Refs #N` claim does not count. The PR landed part of the work and +// deliberately left the issue open for the rest, so the issue goes back to +// the queue, as it already does for agents (#7061). Before this it was held as +// "already claimed by a PR" for the whole merged-claim scan window, and the +// remainder sat unoffered. func (h *ContributeWSHub) issueClaimedByOpenPR(repoFull, repoName string, number int) (ghpkg.IssueClaim, bool) { if h == nil || h.server == nil || h.server.deps == nil || h.server.deps.IssueClaimed == nil { return ghpkg.IssueClaim{}, false } - if claim, ok := h.server.deps.IssueClaimed(repoFull, number); ok { + if claim, ok := h.server.deps.IssueClaimed(repoFull, number); ok && !mergedReferenceClaim(claim) { return claim, true } if repoName != "" && repoName != repoFull { - if claim, ok := h.server.deps.IssueClaimed(repoName, number); ok { + if claim, ok := h.server.deps.IssueClaimed(repoName, number); ok && !mergedReferenceClaim(claim) { return claim, true } } return ghpkg.IssueClaim{}, false } +// mergedReferenceClaim reports whether a claim comes from a merged PR that +// only referenced the issue (`Refs #N`, `Part of #N`). +func mergedReferenceClaim(c ghpkg.IssueClaim) bool { + return c.MergedPR && c.Reference +} + // churnTriagedMarker is the label a maintainer applies to say "I have looked // at the churn on this issue; it is still ordinary work". It clears the churn // guard for that issue and nothing else — the guard's whole purpose is to ask diff --git a/src/pkg/dashboard/contribute_admission_log.go b/src/pkg/dashboard/contribute_admission_log.go new file mode 100644 index 0000000000..2b625250d3 --- /dev/null +++ b/src/pkg/dashboard/contribute_admission_log.go @@ -0,0 +1,61 @@ +package dashboard + +import ( + "strconv" + "strings" + "time" +) + +// admissionSkipLogLimit bounds the per-item memory behind +// admissionSkipChanged. Entries are dropped when their item is admitted, but an +// issue that closes while withheld is never admitted again, so the map is +// cleared outright past this size; the cost is one repeat log per item. +const admissionSkipLogLimit = 4096 + +// logKey is what an admission refusal says about its item, so the skip log +// can tell a repeat from a change. Volatile detail that does not change the +// answer (the dependency record's generation) is left out. +func (d contributorAdmissionDecision) logKey() string { + parts := []string{d.reason} + switch d.reason { + case contributorAdmissionReasonOpenPRClaim, contributorAdmissionReasonMergedClaimStale: + parts = append(parts, d.claim.PRURL, strconv.FormatBool(d.claim.MergedPR), d.claim.Source) + case contributorAdmissionReasonIssueClaim: + parts = append(parts, d.issueClaim.Identity, d.issueClaim.ExpiresAt.UTC().Format(time.RFC3339)) + case contributorAdmissionReasonIssueChurn: + parts = append(parts, d.churn.PRNumbers()...) + case contributorAdmissionReasonWorkflowBlocked, contributorAdmissionReasonLabelSkipped: + parts = append(parts, d.skippedLabel) + case contributorAdmissionReasonDependencyBlocked, contributorAdmissionReasonDependencyUnknown: + parts = append(parts, d.convergence.Reason, d.convergence.ObservedRecord) + parts = append(parts, d.convergence.Blockers...) + } + return strings.Join(parts, "\x00") +} + +// admissionSkipChanged reports whether selectTask should log this refusal: +// true the first time an item is withheld for a reason, and again only when +// the reason or its detail changes. selectTask runs for every connected relay +// every 20-40s, and logging each pass buried the Operations feed in the same +// handful of lines. +func (h *ContributeWSHub) admissionSkipChanged(itemKey string, d contributorAdmissionDecision) bool { + key := d.logKey() + h.admissionSkipMu.Lock() + defer h.admissionSkipMu.Unlock() + if h.admissionSkipLogged[itemKey] == key { + return false + } + if h.admissionSkipLogged == nil || len(h.admissionSkipLogged) >= admissionSkipLogLimit { + h.admissionSkipLogged = make(map[string]string) + } + h.admissionSkipLogged[itemKey] = key + return true +} + +// admissionSkipCleared forgets an item's last logged refusal once it is +// admitted, so withholding it again later is logged again. +func (h *ContributeWSHub) admissionSkipCleared(itemKey string) { + h.admissionSkipMu.Lock() + defer h.admissionSkipMu.Unlock() + delete(h.admissionSkipLogged, itemKey) +} diff --git a/src/pkg/dashboard/contribute_select.go b/src/pkg/dashboard/contribute_select.go index f86664693e..15acb3bf25 100644 --- a/src/pkg/dashboard/contribute_select.go +++ b/src/pkg/dashboard/contribute_select.go @@ -873,6 +873,10 @@ func (h *ContributeWSHub) selectTaskPass(c *ContributorConnection, skippedUnmint issueClaim: issueClaim, claimed: claimed, }) + // An unchanged refusal was already logged on an earlier pass. + if !decision.admitted && !h.admissionSkipChanged(itemKey, decision) { + return + } if !decision.admitted { switch decision.reason { case contributorAdmissionReasonOpenPRClaim: @@ -928,6 +932,7 @@ func (h *ContributeWSHub) selectTaskPass(c *ContributorConnection, skippedUnmint } return } + h.admissionSkipCleared(itemKey) // Yank self-exclusion: an issue this SAME clanker was just yanked off is // briefly skipped for it (yankSelfExcludeSeconds), so the immediate post-yank // reassignment moves the clanker to genuinely different work instead of re- diff --git a/src/pkg/dashboard/contribute_ws.go b/src/pkg/dashboard/contribute_ws.go index 6c04023787..d133bb1a35 100644 --- a/src/pkg/dashboard/contribute_ws.go +++ b/src/pkg/dashboard/contribute_ws.go @@ -975,6 +975,12 @@ type ContributeWSHub struct { unmintableMu sync.Mutex logger *slog.Logger seq int + // admissionSkipLogged maps an item key to the last admission refusal + // selectTask logged for it, so an unchanged refusal is logged once rather + // than on every pass. Its own mutex for the same reason as unmintableMu. + // See contribute_admission_log.go. + admissionSkipLogged map[string]string + admissionSkipMu sync.Mutex // taskGen is the monotonically increasing source of assignment GENERATION tokens // (kubestellar/hive#2568, the Gate). nextTaskGen() hands out a fresh value for // every assignment and every release, so a generation is never reused across the diff --git a/src/pkg/dashboard/contribute_ws_merged_reference_claim_test.go b/src/pkg/dashboard/contribute_ws_merged_reference_claim_test.go new file mode 100644 index 0000000000..b864dc7f7a --- /dev/null +++ b/src/pkg/dashboard/contribute_ws_merged_reference_claim_test.go @@ -0,0 +1,177 @@ +package dashboard + +import ( + "bytes" + "log/slog" + "path/filepath" + "strconv" + "strings" + "testing" + "time" + + ghpkg "github.com/hivecommons/hive/pkg/github" +) + +// A PR that only says `Refs #N` and has merged landed part of the work and +// left the issue open for the rest. On projectbluefin the contribute queue +// kept such issues as "already claimed by a PR" (common#1156 after +// chairlift#633, utah#755 after common#1486, utah-packages#496 after +// utah-packages#631), so nobody was offered the remainder. These tests wire a +// real ClaimLedger in as Dependencies.IssueClaimed, as the hive does. + +func mergedRefIssueHub(t *testing.T, live ...ghpkg.IssueClaim) (*ContributeWSHub, *Server) { + t.Helper() + hub, s := covK2Hub(t) + claimedIssueStatus(s) + ledger := ghpkg.NewClaimLedger(filepath.Join(t.TempDir(), "l.json"), s.logger) + ledger.Reconcile(live, true) + s.deps.IssueClaimed = ledger.Lookup + return hub, s +} + +func dakotaClaim(pr int, mutate func(*ghpkg.IssueClaim)) ghpkg.IssueClaim { + c := ghpkg.IssueClaim{ + Repo: "projectbluefin/dakota", Issue: 353, + PRNumber: pr, PRRepo: "projectbluefin/dakota", + PRURL: "https://github.com/projectbluefin/dakota/pull/" + strconv.Itoa(pr), + PRAuthor: "hivecommons-hive[bot]", + ObservedAt: time.Now(), + } + mutate(&c) + return c +} + +// firstOffered reports the issue ReadyQueue lists first and the one selectTask +// assigns, which must agree. +func firstOffered(t *testing.T, hub *ContributeWSHub) int { + t.Helper() + queue := hub.ReadyQueue(readyQueueDefaultLimit) + if len(queue) == 0 { + t.Fatal("ReadyQueue offered nothing") + } + msg := hub.selectTask(claimedIssueConn()) + if msg == nil || msg.Type != "task_assign" { + t.Fatalf("selectTask = %+v, want a task_assign", msg) + } + if msg.Number != queue[0].Number { + t.Fatalf("selectTask assigned #%d but ReadyQueue lists #%d first", msg.Number, queue[0].Number) + } + return msg.Number +} + +func TestMergedReferenceClaim_ContributeAdmission(t *testing.T) { + merged := time.Now().Add(-2 * time.Hour) + cases := []struct { + name string + claim ghpkg.IssueClaim + want int + }{ + { + // An open PR is someone on the issue now: withheld (#3768, #3980). + name: "open Refs PR holds the issue", + claim: dakotaClaim(400, func(c *ghpkg.IssueClaim) { c.Reference = true }), + want: 360, + }, + { + // A merged closing keyword closes the issue; until the enumeration + // sees it closed (or for a cross-repo close GitHub could not act + // on), it stays withheld exactly as before (#6867, #8003). + name: "merged closing PR still holds the issue", + claim: dakotaClaim(401, func(c *ghpkg.IssueClaim) { + c.Closing, c.MergedPR, c.MergedAt = true, true, merged + }), + want: 360, + }, + { + name: "merged Refs PR returns the issue to the queue", + claim: dakotaClaim(402, func(c *ghpkg.IssueClaim) { + c.Reference, c.MergedPR, c.MergedAt = true, true, merged + }), + want: 353, + }, + { + // The common#1156 shape: a person's PR in another repo. + name: "cross-repo merged Refs PR returns the issue to the queue", + claim: dakotaClaim(633, func(c *ghpkg.IssueClaim) { + c.PRRepo = "projectbluefin/chairlift" + c.PRURL = "https://github.com/projectbluefin/chairlift/pull/633" + c.PRAuthor, c.ExternalAuthor = "thatguywob", true + c.Reference, c.MergedPR, c.MergedAt = true, true, merged + }), + want: 353, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + hub, _ := mergedRefIssueHub(t, tc.claim) + if got := firstOffered(t, hub); got != tc.want { + t.Fatalf("offered #%d, want #%d", got, tc.want) + } + }) + } +} + +// TestMergedReferenceClaim_DoesNotHideAnOpenPR: a person's open `Refs #N` PR +// on the same issue still holds it. The ledger keeps one claim per issue, and +// before this a merged hive-authored `Refs #N` outranked it, so releasing the +// merged claim would have offered an issue someone is working on. +func TestMergedReferenceClaim_DoesNotHideAnOpenPR(t *testing.T) { + hiveMerged := dakotaClaim(631, func(c *ghpkg.IssueClaim) { + c.Reference, c.MergedPR, c.MergedAt = true, true, time.Now().Add(-time.Hour) + }) + personOpen := dakotaClaim(640, func(c *ghpkg.IssueClaim) { + c.PRAuthor, c.ExternalAuthor, c.Reference = "arteeh", true, true + }) + for _, live := range [][]ghpkg.IssueClaim{{hiveMerged, personOpen}, {personOpen, hiveMerged}} { + hub, _ := mergedRefIssueHub(t, live...) + if got := firstOffered(t, hub); got != 360 { + t.Fatalf("scan order #%d,#%d: offered #%d, want #360 while PR #640 is open", + live[0].PRNumber, live[1].PRNumber, got) + } + } +} + +// TestAdmissionSkipLoggedOnce: selectTask runs for every relay every 20-40s, +// and the projectbluefin hive logged the same "already claimed" line for each +// withheld issue on every pass. A refusal is logged when it first appears and +// when it changes, not on repeat. +func TestAdmissionSkipLoggedOnce(t *testing.T) { + hub, s := covK2Hub(t) + claimedIssueStatus(s) + var buf bytes.Buffer + hub.logger = slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelInfo})) + claim, claimed := dakotaClaim(400, func(*ghpkg.IssueClaim) {}), true + s.deps.IssueClaimed = func(repo string, number int) (ghpkg.IssueClaim, bool) { + if number == 353 && claimed { + return claim, true + } + return ghpkg.IssueClaim{}, false + } + skips := func() int { return strings.Count(buf.String(), "skip: issue already claimed by a PR") } + pass := func() *WSMessage { return hub.selectTask(claimedIssueConn()) } + + for i := 0; i < 3; i++ { + pass() + } + if got := skips(); got != 1 { + t.Fatalf("three passes over the same claim logged %d skips, want 1", got) + } + + claim = dakotaClaim(401, func(*ghpkg.IssueClaim) {}) + pass() + pass() + if got := skips(); got != 2 { + t.Fatalf("a different claiming PR should log once more: got %d skips, want 2", got) + } + + // Admitted, then withheld again: a new episode, logged again. + claimed = false + if msg := pass(); msg == nil || msg.Number != 353 { + t.Fatalf("with no claim #353 should be offered, got %+v", msg) + } + claimed = true + pass() + if got := skips(); got != 3 { + t.Fatalf("withholding again after admission should log: got %d skips, want 3", got) + } +} diff --git a/src/pkg/github/prclaims.go b/src/pkg/github/prclaims.go index 97588a0a44..b1ff8b49c0 100644 --- a/src/pkg/github/prclaims.go +++ b/src/pkg/github/prclaims.go @@ -382,6 +382,12 @@ func settledRetained(c IssueClaim, now time.Time) bool { // evidence still wins: a merged `Fixes #N` (the work is done) outranks an open // `Refs #N` (someone is doing related work), so a settled issue is not // re-offered just because a follow-up references it. +// +// A merged `Refs #N` is the exception: it never claimed the issue was done, +// so it ranks below EVERY open claim. Readers release a merged reference +// claim back to the queue (#7061), and if it outranked a person's open +// `Refs #N` PR, that release would hand the issue to someone else while the +// person is still working it. func claimRank(c IssueClaim) int { var tier int switch { @@ -394,6 +400,9 @@ func claimRank(c IssueClaim) int { default: tier = 0 } + if c.Reference && c.MergedPR { + return tier - 2 + } rank := tier * 2 if !c.MergedPR { rank++ diff --git a/src/pkg/github/prclaims_merged_test.go b/src/pkg/github/prclaims_merged_test.go index 2b42d33891..fdfe9992dc 100644 --- a/src/pkg/github/prclaims_merged_test.go +++ b/src/pkg/github/prclaims_merged_test.go @@ -188,6 +188,46 @@ func TestClaimRankOpenBeatsMergedWithinTier(t *testing.T) { } } +// TestClaimRankMergedReferenceBelowEveryOpenClaim: a merged `Refs #N` never +// said the issue was done, and the contribute queue releases it (#7061), so it +// must not shadow an open PR of any grade. The case that mattered: a merged +// hive-authored `Refs #N` used to outrank a person's open `Refs #N` PR. +func TestClaimRankMergedReferenceBelowEveryOpenClaim(t *testing.T) { + open := map[string]IssueClaim{ + "hive closing": {}, + "person closing": {ExternalAuthor: true}, + "hive reference": {Reference: true}, + "person reference": {Reference: true, ExternalAuthor: true}, + } + merged := map[string]IssueClaim{ + "hive reference": {Reference: true, MergedPR: true}, + "person reference": {Reference: true, MergedPR: true, ExternalAuthor: true}, + } + for on, o := range open { + for mn, m := range merged { + if claimRank(o) <= claimRank(m) { + t.Errorf("open %s (%d) must outrank merged %s (%d)", on, claimRank(o), mn, claimRank(m)) + } + } + } + if claimRank(merged["hive reference"]) <= claimRank(merged["person reference"]) { + t.Errorf("merged hive reference (%d) must still outrank merged person reference (%d)", + claimRank(merged["hive reference"]), claimRank(merged["person reference"])) + } + + // Through the ledger, in both scan orders. + hiveMerged := IssueClaim{Repo: "o/r", Issue: 7, PRNumber: 10, PRRepo: "o/r", Reference: true, MergedPR: true, ObservedAt: time.Now()} + personOpen := IssueClaim{Repo: "o/r", Issue: 7, PRNumber: 11, PRRepo: "o/r", Reference: true, ExternalAuthor: true, ObservedAt: time.Now()} + for _, live := range [][]IssueClaim{{hiveMerged, personOpen}, {personOpen, hiveMerged}} { + l := NewClaimLedger(t.TempDir()+"/l.json", testLogger()) + l.Reconcile(live, true) + got, ok := l.Lookup("o/r", 7) + if !ok || got.PRNumber != 11 { + t.Errorf("Lookup after %d,%d = %+v (ok=%v), want the open PR #11", live[0].PRNumber, live[1].PRNumber, got, ok) + } + } +} + // TestFilterClaimedIssuesMergedClaims pins the agent-side consumption of // merged claims (#6867/#7061): a merged strong claim suppresses and never takes // the red+stale release valve (the PR is merged — check state is meaningless),