Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/fixed-contribute-merged-refs-claim.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion src/docs/contributor-relay.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.*
Expand Down
16 changes: 14 additions & 2 deletions src/pkg/dashboard/contribute_admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
61 changes: 61 additions & 0 deletions src/pkg/dashboard/contribute_admission_log.go
Original file line number Diff line number Diff line change
@@ -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)
}
5 changes: 5 additions & 0 deletions src/pkg/dashboard/contribute_select.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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-
Expand Down
6 changes: 6 additions & 0 deletions src/pkg/dashboard/contribute_ws.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading