🐛 suppress claims on already-done issues

changelog.d/fixed-10706.md

@@ -0,0 +1 @@
+- suppress claims on already-done issues (#10706)

src/cmd/hive/claimswire.go

@@ -202,6 +202,15 @@ func recordAgentKickClaims(dashSrv *dashboard.Server, org, agentName string, iss
 		if org != "" && !strings.Contains(repo, "/") {
 			repo = org + "/" + repo
 		}
+		ctx, cancel := context.WithTimeout(context.Background(), claimsGitHubTimeout)
+		refused, _ := dashSrv.RefuseAlreadyDoneClaim(ctx, repo, ref.Number)
+		cancel()
+		if refused {
+			if logger != nil {
+				logger.Info("issue-claims: agent claim refused for already-done issue", "agent", agentName, "issue", claims.Key(repo, ref.Number))
+			}
+			continue
+		}
 		res, err := ledger.Claim(claims.Request{
 			Repo: repo, Issue: ref.Number,
 			Holder: agentName, HolderID: agentName, Kind: claims.KindAgent,

src/cmd/hive/main.go

@@ -8486,6 +8486,12 @@ func applyDuplicatePRGuard(
 	actionable *github.ActionableResult,
 	logger *slog.Logger,
 ) {
+	// hive/already-done is display state, not a hold, but it is also positive
+	// evidence that a merged fix already covered the item. Keep it out of new
+	// kick prompts until a human removes the label.
+	if withheld := github.FilterAlreadyDoneIssues(actionable, logger); withheld > 0 {
+		logger.Info("already-done guard applied", "withheld", withheld)
+	}
 	// #8380: a LIVE issue claim covers an issue the same way an open PR does,
 	// for every kick prompt (the scanner's included). The claim fields are
 	// only ever set while governor.claims.enabled is on, so with the feature

src/docs/api-reference.md

@@ -390,10 +390,10 @@ Read the result from `GET /api/kick/{agent}/status`, which returns `status` of `
 
 | Method | Path | Auth | Purpose | Source |
 |---|---|---|---|---|
-| `GET` | `/api/claims` | Dashboard auth | Every live worker claim (`{enabled, claims:[…]}`); `enabled:false` when `claims.enabled: false` | `pkg/dashboard/claims_api.go:53` |
-| `GET` | `/api/claims/{owner}/{repo}/{number}` | Dashboard auth | `{held, claim}` for one issue | `pkg/dashboard/claims_api.go:54` |
-| `POST` | `/api/claims/{owner}/{repo}/{number}` | Signed-in / hub-proxied / GitHub-token caller claims as a **human** under their login; a verified owner token claims as the owner | Body `{force, ttl_s, session}`. 200 with `outcome` `claimed`/`renewed`/`taken_over`; 409 with `outcome` `held` (same rank, retry with `force`) or `refused` (higher rank holds it) and a `hint` | `pkg/dashboard/claims_api.go:55` |
-| `DELETE` | `/api/claims/{owner}/{repo}/{number}` | Same identity rules | Body `{force, reason}`. Releases your claim or a lower-ranked holder's; `force` with a verified owner role releases regardless of holder | `pkg/dashboard/claims_api.go:56` |
+| `GET` | `/api/claims` | Dashboard auth | Every live worker claim (`{enabled, claims:[…]}`); `enabled:false` when `claims.enabled: false` | `pkg/dashboard/claims_api.go:56` |
+| `GET` | `/api/claims/{owner}/{repo}/{number}` | Dashboard auth | `{held, claim}` for one issue | `pkg/dashboard/claims_api.go:57` |
+| `POST` | `/api/claims/{owner}/{repo}/{number}` | Signed-in / hub-proxied / GitHub-token caller claims as a **human** under their login; a verified owner token claims as the owner | Body `{force, ttl_s, session}`. 200 with `outcome` `claimed`/`renewed`/`taken_over`; 409 with `outcome` `held` (same rank, retry with `force`) or `refused` (higher rank holds it) and a `hint` | `pkg/dashboard/claims_api.go:58` |
+| `DELETE` | `/api/claims/{owner}/{repo}/{number}` | Same identity rules | Body `{force, reason}`. Releases your claim or a lower-ranked holder's; `force` with a verified owner role releases regardless of holder | `pkg/dashboard/claims_api.go:59` |
 
 ### `/api/v1` contributor subpaths
 

src/docs/labels-and-control-signals.md

@@ -66,7 +66,7 @@ The "respect hold labels" text in policy templates names the enforced set (`hold
 | `needs-rebase` | PRs | Scanner/kick annotation | Fills mergeability annotation in kick data. | Informational | Scanner/Hive | PR status processing | Remove after rebase | Fixed label (`src/pkg/scheduler/pr_annotations.go:117-144`) |
 | `hive/covered-by-pr` | Issues | PR-claim label sync and dashboard | Open PR is verified as related; issue remains actionable. Labels are synced only for actionable issues, so stale labels can remain on held/exempt/filtered issues. | Display-only | Hive | Claim sync after enumeration | Hive removes when actionable issue no longer has open PR evidence | Fixed label (`src/pkg/github/prclaims.go:1380-1435`) |
 | `hive/likely-done` | Issues | PR-claim label sync and dashboard | Merged PR is verified as related while issue remains open; issue remains actionable until GitHub/operator closes or confirms. | Display-only | Hive | Claim sync after enumeration | Hive removes when actionable evidence no longer says likely done | Fixed label (`src/pkg/github/prclaims.go:1380-1435`) |
-| `hive/already-done` | Issues | Contributor queue and dashboard | Contributor already-done verdict/confirmation; default contributor skip label and done band. Spoke agents can still receive the issue unless another gate also filters it. | Contributor hard skip; dashboard display | Hive/contributor flow | Contributor admission | Human removes or config changes; optional close behavior | `hub.contribute_already_done_label`, `contribute_close_already_done` (`src/pkg/config/config.go:4442-4572`, `src/pkg/config/config.go:4585-4685`) |
+| `hive/already-done` | Issues | Contributor queue, agent kicks, claims, and dashboard | Contributor already-done verdict/confirmation; default contributor skip label and done band. It is display state, not a hold, but Hive soft-suppresses contributor offers, agent kick claims, and new worker claims while the label remains. | Soft suppress; dashboard display | Hive/contributor flow | Contributor admission, actionable filtering, claim create | Human removes or config changes; optional close behavior | `hub.contribute_already_done_label`, `contribute_close_already_done`, fixed label (`src/pkg/config/config.go:4701-4924`, `src/pkg/github/issue_claims.go`, `src/pkg/dashboard/claims_api.go`) |
 | `blocked` | Issues | Contributor queue and dashboard issue bands | Always included in contributor skip patterns and default waiting band. Does not by itself stop spoke-agent enumeration unless also exempt/held/filtered. | Contributor hard skip; display | Human | Contributor admission/dashboard render | Remove label | `hub.contribute_skip_labels`, dashboard bands (`src/pkg/config/config.go:4585-4685`, `src/docs/dashboard.md:105-140`) |
 | `needs-direction` | Issues | Enumeration, escalation labels, dashboard issue bands and un-park sweep | ADR-0019 escalation marker for work that needs a maintainer direction decision before continuing. Filtered from enumeration like `needs-human`: no agent kick, kick claim or contributor offer. The un-park sweep keeps the same "What to reply" notice as `needs-human`/`needs-decision` and accepts the same `/hive approve` or `/hive decision <text>` commands. See [Maintainer commands](maintainer-commands.md). | Hard suppress until directed | Hive/human | Dashboard render and un-park sweep | Human removes, or a maintainer replies `/hive approve` / `/hive decision <text>`, which also adds `approved-direction` and never touches `hold` (`src/pkg/github/issue_unpark_command.go`) | Fixed escalation label (`src/pkg/github/labels.go`, `src/docs/adr/0019-escalation-over-stalling.md`) |
 | `needs-decision` | Issues | Enumeration, contributor relay, dashboard issue bands and un-park sweep | Relay can apply it when a maintainer decision is needed; filtered from enumeration like `needs-human`, so no agent kick, kick claim or contributor offer. The un-park sweep keeps the "What to reply" notice and command handling documented in [Maintainer commands](maintainer-commands.md). | Contributor hard skip; display | Relay/Hive/human | Contributor admission/dashboard render | Human removes, or a maintainer replies `/hive approve` / `/hive decision <text>`, which also adds `approved-direction` and never touches `hold` (`src/pkg/github/issue_unpark_command.go`); empty config disables relay application | `hub.contribute_needs_decision_label` (`src/pkg/config/config.go:4397-4408`, `src/pkg/config/config.go:4645-4685`) |

src/pkg/dashboard/claims_api.go

@@ -9,7 +9,10 @@ import (
 	"strings"
 	"time"
 
+	gh "github.com/google/go-github/v72/github"
+
 	"github.com/hivecommons/hive/pkg/claims"
+	ghpkg "github.com/hivecommons/hive/pkg/github"
 	"github.com/hivecommons/hive/pkg/hooks"
 	"github.com/hivecommons/hive/pkg/worksource"
 )
@@ -290,6 +293,10 @@ func (s *Server) handleClaimCreate(w http.ResponseWriter, r *http.Request) {
 		jsonError(w, "Sign in with GitHub (or use an owner token) to claim an issue.", http.StatusUnauthorized)
 		return
 	}
+	if refused, msg := s.RefuseAlreadyDoneClaim(r.Context(), repo, number); refused {
+		jsonError(w, msg, http.StatusConflict)
+		return
+	}
 	var body claimRequestBody
 	if r.Body != nil {
 		_ = json.NewDecoder(r.Body).Decode(&body)
@@ -313,6 +320,90 @@ func (s *Server) handleClaimCreate(w http.ResponseWriter, r *http.Request) {
 	writeClaimsJSON(w, status, claimResultJSON(res))
 }
 
+const alreadyDoneClaimRefusalMarker = "<!-- hive:claim-refused already_done -->"
+
+// RefuseAlreadyDoneClaim refuses to create a new worker claim on an issue Hive
+// has already marked as fixed. This is a soft suppression: removing the display
+// label makes the issue claimable again, and a GitHub read failure fails open
+// so a transient API problem does not break otherwise-valid claims.
+func (s *Server) RefuseAlreadyDoneClaim(ctx context.Context, repo string, number int) (bool, string) {
+	if s == nil || s.deps == nil || s.deps.GHClient == nil || number <= 0 || repo == "" {
+		return false, ""
+	}
+	gh := s.deps.GHClient.GoGitHub()
+	if gh == nil {
+		return false, ""
+	}
+	owner, name, ok := strings.Cut(repo, claimsRepoSep)
+	if !ok || strings.TrimSpace(owner) == "" || strings.TrimSpace(name) == "" {
+		return false, ""
+	}
+	issue, _, err := gh.Issues.Get(ctx, owner, name, number)
+	if err != nil {
+		if s.logger != nil {
+			s.logger.Warn("[claims] already-done claim guard failed open", "repo", repo, "issue", number, "error", err)
+		}
+		return false, ""
+	}
+	label, ok := s.alreadyDoneLabelOn(issue.Labels)
+	if !ok {
+		return false, ""
+	}
+	claim, hasClaim := s.mergedIssueClaim(repo, number)
+	body := alreadyDoneClaimRefusalComment(label, claim, hasClaim)
+	alreadyCommented := false
+	if ok, err := s.deps.GHClient.IssueCommentsContain(ctx, repo, number, alreadyDoneClaimRefusalMarker); err == nil {
+		alreadyCommented = ok
+	}
+	if !alreadyCommented {
+		if err := s.deps.GHClient.CreateIssueComment(ctx, repo, number, body); err != nil && s.logger != nil {
+			s.logger.Warn("[claims] already-done claim refusal comment failed", "repo", repo, "issue", number, "error", err)
+		}
+	}
+	return true, "issue is marked " + label + "; not claimable until the label is removed"
+}
+
+func (s *Server) alreadyDoneLabelOn(labels []*gh.Label) (string, bool) {
+	want := ghpkg.AlreadyDoneLabel
+	if s != nil && s.deps != nil && s.deps.Config != nil {
+		want = s.deps.Config.Hub.ContributeAlreadyDoneLabelOrDefault()
+	}
+	for _, label := range labels {
+		name := strings.TrimSpace(label.GetName())
+		if strings.EqualFold(name, ghpkg.AlreadyDoneLabel) || strings.EqualFold(name, want) {
+			return name, true
+		}
+	}
+	return "", false
+}
+
+func (s *Server) mergedIssueClaim(repo string, number int) (ghpkg.IssueClaim, bool) {
+	if s == nil || s.deps == nil || s.deps.IssueClaimed == nil {
+		return ghpkg.IssueClaim{}, false
+	}
+	if claim, ok := s.deps.IssueClaimed(repo, number); ok && claim.MergedPR {
+		return claim, true
+	}
+	if _, name, ok := strings.Cut(repo, claimsRepoSep); ok {
+		if claim, ok := s.deps.IssueClaimed(name, number); ok && claim.MergedPR {
+			return claim, true
+		}
+	}
+	return ghpkg.IssueClaim{}, false
+}
+
+func alreadyDoneClaimRefusalComment(label string, claim ghpkg.IssueClaim, hasClaim bool) string {
+	target := "a merged fix"
+	if hasClaim && claim.PRNumber > 0 {
+		target = fmt.Sprintf("merged PR #%d", claim.PRNumber)
+		if claim.PRURL != "" {
+			target = fmt.Sprintf("merged PR #%d (%s)", claim.PRNumber, claim.PRURL)
+		}
+	}
+	return fmt.Sprintf("%s\nNot claiming this issue because it is marked `%s`; %s already fixed it. Remove `%s` (or mark `%s`) if work remains.",
+		alreadyDoneClaimRefusalMarker, label, target, label, ghpkg.VerifiedOpenLabel)
+}
+
 func (s *Server) handleClaimRelease(w http.ResponseWriter, r *http.Request) {
 	repo, number, err := claimPathRef(r)
 	if err != nil {

src/pkg/dashboard/claims_api_test.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/hivecommons/hive/pkg/claims"
+	ghpkg "github.com/hivecommons/hive/pkg/github"
 	"github.com/hivecommons/hive/pkg/hooks"
 )
 
@@ -69,6 +70,27 @@ func TestClaims_SelectTaskSkipsHumanClaimedIssue(t *testing.T) {
 	}
 }
 
+func TestClaims_SelectTaskSkipsAlreadyDoneIssue(t *testing.T) {
+	hub, s, l := claimsHub(t)
+	done := intgIssue(1, "already fixed", "someone", nil)
+	done["labels"] = []any{ghpkg.AlreadyDoneLabel}
+	free := intgIssue(2, "free", "someone", nil)
+	free["labels"] = []any{"bug"}
+	setStatusIssues(s, done, free)
+	c := &ContributorConnection{
+		profile:  &ContributorProfile{GitHubUsername: "worker", ContributorID: "c-worker", TrustTier: "contributor"},
+		lastPong: time.Now(),
+	}
+
+	msg := hub.selectTask(c)
+	if msg == nil || msg.Type != "task_assign" || msg.Number != 2 {
+		t.Fatalf("expected #2 (the not-already-done issue), got %+v", msg)
+	}
+	if _, ok := l.Lookup("myorg/repo1", 1); ok {
+		t.Fatal("already-done issue was claimed")
+	}
+}
+
 func TestClaims_TakeoverPreemptsOnlyTheDisplacedSession(t *testing.T) {
 	hub, s, l := claimsHub(t)
 	setStatusIssues(s,
@@ -203,6 +225,51 @@ func TestClaims_APIRoutes(t *testing.T) {
 	}
 }
 
+func TestClaims_APIRouteRefusesAlreadyDoneIssue(t *testing.T) {
+	_, s, l := claimsHub(t)
+	var posted string
+	ghSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.Method == http.MethodGet && r.URL.Path == "/repos/o/r/issues/5":
+			_, _ = w.Write([]byte(`{"number":5,"labels":[{"name":"hive/already-done"}]}`))
+		case r.Method == http.MethodGet && r.URL.Path == "/repos/o/r/issues/5/comments":
+			_, _ = w.Write([]byte(`[]`))
+		case r.Method == http.MethodPost && r.URL.Path == "/repos/o/r/issues/5/comments":
+			var body map[string]string
+			_ = json.NewDecoder(r.Body).Decode(&body)
+			posted = body["body"]
+			w.WriteHeader(http.StatusCreated)
+			_, _ = w.Write([]byte(`{"id":1}`))
+		default:
+			t.Fatalf("unexpected GitHub request %s %s", r.Method, r.URL.Path)
+		}
+	}))
+	t.Cleanup(ghSrv.Close)
+	s.deps.GHClient = ghpkg.NewClientForTest(ghSrv.URL, "o", []string{"r"}, s.logger)
+	s.deps.IssueClaimed = func(repo string, number int) (ghpkg.IssueClaim, bool) {
+		if repo == "o/r" && number == 5 {
+			return ghpkg.IssueClaim{Repo: repo, Issue: number, PRNumber: 10617, PRURL: "https://github.com/hivecommons/hive/pull/10617", MergedPR: true}, true
+		}
+		return ghpkg.IssueClaim{}, false
+	}
+
+	req := httptest.NewRequest(http.MethodPost, "/api/claims/o/r/5", strings.NewReader(""))
+	req.Header.Set("X-Hive-User", "alice")
+	w := httptest.NewRecorder()
+	s.mux.ServeHTTP(w, req)
+	if w.Code != http.StatusConflict {
+		t.Fatalf("claim code=%d body=%s, want 409", w.Code, w.Body.String())
+	}
+	if _, ok := l.Lookup("o/r", 5); ok {
+		t.Fatal("already-done issue was claimed")
+	}
+	for _, want := range []string{"hive:claim-refused", "hive/already-done", "#10617", "https://github.com/hivecommons/hive/pull/10617"} {
+		if !strings.Contains(posted, want) {
+			t.Fatalf("refusal comment missing %q: %q", want, posted)
+		}
+	}
+}
+
 // A relay whose socket is down has no connection to yank, but its lease would
 // otherwise let it RESUME the item it lost on reconnect (#4260 resumability).
 // Takeover must revoke that lease too.

src/pkg/github/issue_claims.go

@@ -188,6 +188,35 @@ func FilterLiveIssueClaims(result *ActionableResult, now time.Time, logger *slog
 	return withheld
 }
 
+// FilterAlreadyDoneIssues removes issues marked hive/already-done from the
+// actionable set. The label is not a hard hold: it does not park the issue in
+// the hold list or require an operator unhold, but it does mean Hive already has
+// enough evidence that new contributor offers and agent kicks are duplicate
+// work until a human removes the display label.
+func FilterAlreadyDoneIssues(result *ActionableResult, logger *slog.Logger) int {
+	if result == nil || len(result.Issues.Items) == 0 {
+		return 0
+	}
+	kept := make([]Issue, 0, len(result.Issues.Items))
+	withheld := 0
+	for _, issue := range result.Issues.Items {
+		if !issueHasLabel(issue.Labels, AlreadyDoneLabel) {
+			kept = append(kept, issue)
+			continue
+		}
+		withheld++
+		if logger != nil {
+			logger.Info("withholding issue: already-done label",
+				"repo", issue.Repo, "issue", issue.Number, "label", AlreadyDoneLabel)
+		}
+	}
+	if withheld == 0 {
+		return 0
+	}
+	result.Issues = IssueResultFromItems(kept)
+	return withheld
+}
+
 // The dashboard-facing seam for issue claims (hivecommons/hive#8380).
 //
 // pkg/dashboard already imports this package for the Issue envelope that

src/pkg/github/issue_claims_test.go

@@ -311,6 +311,29 @@ func TestFilterLiveIssueClaims(t *testing.T) {
 	}
 }
 
+func TestFilterAlreadyDoneIssues(t *testing.T) {
+	items := []Issue{
+		{Repo: "widgets", Number: 1, Labels: []string{AlreadyDoneLabel}},
+		{Repo: "widgets", Number: 2, Labels: []string{LikelyDoneLabel}},
+		{Repo: "widgets", Number: 3},
+	}
+	result := &ActionableResult{Issues: IssueResultFromItems(items)}
+	if n := FilterAlreadyDoneIssues(result, slog.New(slog.NewTextHandler(io.Discard, nil))); n != 1 {
+		t.Fatalf("withheld %d, want 1", n)
+	}
+	if len(result.Issues.Items) != 2 || result.Issues.Count != 2 {
+		t.Fatalf("kept %d/%d, want 2", len(result.Issues.Items), result.Issues.Count)
+	}
+	for _, issue := range result.Issues.Items {
+		if issue.Number == 1 {
+			t.Fatal("already-done issue #1 was not withheld")
+		}
+	}
+	if n := FilterAlreadyDoneIssues(nil, nil); n != 0 {
+		t.Fatalf("nil result withheld %d", n)
+	}
+}
+
 // The SetIssueClaims setting is read live and a zero ttl takes the default.
 func TestIssueClaimsSetting(t *testing.T) {
 	var c *Client

src/pkg/github/prclaims.go

@@ -102,6 +102,8 @@ const (
 	// LikelyDoneLabel marks an open issue with an API-verified merged PR relation
 	// that is not yet a resolved/closed issue.
 	LikelyDoneLabel = "hive/likely-done"
+	// AlreadyDoneLabel marks an issue that Hive verified is already fixed.
+	AlreadyDoneLabel = "hive/already-done"
 	// VerifiedOpenLabel records that verification found the issue still open/not
 	// done, suppressing renewed likely-done labeling from the same stale claim.
 	VerifiedOpenLabel = "hive/verified-open"