From 293b4aa92af2ea3c4d588287d3bb549cd2989116 Mon Sep 17 00:00:00 2001 From: Danathar Date: Sun, 4 Oct 2026 20:06:42 +0000 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=90=9B=20fix(claims):=20escalate=20in?= =?UTF-8?q?stead=20of=20re-claiming=20after=202=20no-progress=20agent=20cl?= =?UTF-8?q?aims?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The claim ledger now remembers each agent's claims per issue (with how each ended), and a scheduler in-flight gate withholds a free issue whose last escalate_after_claims claims by one agent were followed by no linked or referencing PR, referencing commit, label/assignee change or close/reopen. The issue is labelled needs-human and gets one comment listing the claims (ADR-0019). Timeline read failures fail open. NOT FOR MERGE AS-IS: agent kick claims are recorded for every issue in a kick's list (up to governor.kick_limits.max_issues, default 100), so with the default threshold of 2 this would label the whole untouched backlog needs-human about four hours after deploy. See the issue comment. Hive-Run: hivecommons/hive#10527 Hive-Plan: hivecommons/hive#10527 (Proposed fix) Hive-Spec: hivecommons/hive#10527#acceptance Signed-off-by: Danathar --- changelog.d/fixed-10527-claim-escalation.md | 1 + src/cmd/hive/claimescalation.go | 215 +++++++++++++++++++ src/cmd/hive/claimescalation_test.go | 215 +++++++++++++++++++ src/cmd/hive/main.go | 6 +- src/docs/labels-and-control-signals.md | 2 +- src/docs/operator-reference.md | 2 +- src/pkg/claims/claims.go | 218 +++++++++++++++++++- src/pkg/claims/comments.go | 38 +++- src/pkg/claims/stall_test.go | 166 +++++++++++++++ src/pkg/config/claims.go | 23 +++ src/pkg/config/claims_config_test.go | 19 ++ src/pkg/github/issue_timeline.go | 66 ++++++ src/pkg/github/issue_timeline_test.go | 43 ++++ 13 files changed, 997 insertions(+), 17 deletions(-) create mode 100644 changelog.d/fixed-10527-claim-escalation.md create mode 100644 src/cmd/hive/claimescalation.go create mode 100644 src/cmd/hive/claimescalation_test.go create mode 100644 src/pkg/claims/stall_test.go create mode 100644 src/pkg/github/issue_timeline.go create mode 100644 src/pkg/github/issue_timeline_test.go diff --git a/changelog.d/fixed-10527-claim-escalation.md b/changelog.d/fixed-10527-claim-escalation.md new file mode 100644 index 0000000000..b238ccedc8 --- /dev/null +++ b/changelog.d/fixed-10527-claim-escalation.md @@ -0,0 +1 @@ +- An agent no longer re-claims the same issue forever when nothing moves between its claims ([#10527](https://github.com/hivecommons/hive/issues/10527)). An agent claim lapses after `agent_ttl_s`, the issue went back into the work lists, and the next kick claimed it again, so an issue only a person could unblock showed as "claimed" for hours (#10512: `scanner` claimed at 15:50Z and again at 17:57Z with no commit, PR or label change in between). The claim ledger now remembers each agent's claims on an issue. When one agent's last `governor.claims.escalate_after_claims` claims (default `2`; negative turns the gate off) on a free issue were followed by no linked or referencing pull request, no referencing commit, no label or assignee change and no close/reopen, the issue is left out of every kick instead of being claimed a third time, labelled `needs-human`, and given one comment listing the claims and how each ended (ADR-0019). The claim machinery's own `claimed`/`preempted:*` labels and the gate's own `needs-human` do not count as progress; a person removing `needs-human` (or `/hive approve`) does, so the next claim goes ahead. If the issue timeline cannot be read, the claim is allowed. diff --git a/src/cmd/hive/claimescalation.go b/src/cmd/hive/claimescalation.go new file mode 100644 index 0000000000..4bd93489fd --- /dev/null +++ b/src/cmd/hive/claimescalation.go @@ -0,0 +1,215 @@ +package main + +import ( + "context" + "fmt" + "log/slog" + "strings" + "sync" + "time" + + "github.com/hivecommons/hive/pkg/claims" + "github.com/hivecommons/hive/pkg/config" + "github.com/hivecommons/hive/pkg/escalation" + "github.com/hivecommons/hive/pkg/github" + "github.com/hivecommons/hive/pkg/scheduler" +) + +// Claim escalation gate (hivecommons/hive#10527). +// +// An agent claim expires after agent_ttl_s and the issue goes back into the +// work lists, so nothing stopped the same agent re-claiming an issue forever: +// each claim looked like progress while nothing moved. The gate sits in the +// scheduler's in-flight seam, in front of the kick that would record the next +// claim. When one agent's last escalate_after_claims claims on a free issue +// were followed by no linked PR, referencing commit, label or assignee change +// and no close/reopen, the issue is withheld from every work list, labelled +// needs-human, and given one comment summarising the claims (ADR-0019). The +// needs-human label then keeps it out of the issue enumeration until a person +// removes it — which is itself progress, so the next claim goes ahead. + +// claimProgressRecheck bounds how long a "nothing moved" verdict, or a failed +// timeline read, is reused before the timeline is read again. A verdict that +// something moved holds for as long as the run it was read for. +const claimProgressRecheck = 5 * time.Minute + +// claimEscalationGate decides, per issue, whether a stalled run of agent +// claims withholds it. +type claimEscalationGate struct { + ctx context.Context + ledger *claims.Ledger + org string + threshold int + // events reads the issue timeline after since. + events func(ctx context.Context, repo string, issue int, since time.Time) ([]github.IssueEvent, error) + // escalate applies the needs-human label and posts the summary; called + // once per stalled run. + escalate func(claims.Stall) + now func() time.Time + logger *slog.Logger + + mu sync.Mutex + verdicts map[string]progressVerdict +} + +// progressVerdict caches one timeline read for one stalled run. +type progressVerdict struct { + since time.Time // Since of the run the verdict was read for + allow bool + until time.Time // zero: valid until the run changes +} + +// claimEscalationLookup returns the gate as a scheduler in-flight lookup, or +// nil when the ledger is off or escalate_after_claims is negative. +func claimEscalationLookup(ctx context.Context, cfg *config.Config, ledger *claims.Ledger, client func() *github.Client, logger *slog.Logger) scheduler.InflightLookup { + if ledger == nil || cfg == nil || client == nil { + return nil + } + threshold := cfg.Governor.Claims.EscalationThreshold() + if threshold <= 0 { + return nil + } + g := &claimEscalationGate{ + ctx: ctx, + ledger: ledger, + org: cfg.Project.Org, + threshold: threshold, + events: func(ctx context.Context, repo string, issue int, since time.Time) ([]github.IssueEvent, error) { + gh := client() + if gh == nil { + return nil, github.ErrNoGitHubClient + } + return gh.IssueTimelineSince(ctx, repo, issue, since) + }, + escalate: func(s claims.Stall) { + go func() { + gh := client() + if gh == nil { + return + } + cctx, cancel := context.WithTimeout(ctx, claimsGitHubTimeout) + defer cancel() + if err := gh.AddLabels(cctx, s.Repo, s.Issue, []string{escalation.NeedsHumanLabel}); err != nil && logger != nil { + logger.Warn("issue-claims: escalation label failed", "issue", s.Key(), "error", err) + } + if err := gh.CreateIssueComment(cctx, s.Repo, s.Issue, claims.EscalationComment(s, escalation.NeedsHumanLabel)); err != nil && logger != nil { + logger.Warn("issue-claims: escalation comment failed", "issue", s.Key(), "error", err) + } + }() + }, + now: time.Now, + logger: logger, + } + return g.lookup +} + +// lookup is the scheduler.InflightLookup. An issue is withheld only when a +// stalled run exists and the timeline shows nothing moved since it began; a +// timeline read that fails lets the issue through, so a GitHub outage never +// parks work on its own. +func (g *claimEscalationGate) lookup(issue github.Issue) (string, bool) { + if issue.Number <= 0 { + return "", false + } + repo := issue.Repo + if g.org != "" && !strings.Contains(repo, "/") { + repo = g.org + "/" + repo + } + stall, ok := g.ledger.Stall(repo, issue.Number, g.threshold) + if !ok { + return "", false + } + since := stall.Since() + held := fmt.Sprintf("escalated to %s: %d claims by %s moved nothing since %s", + escalation.NeedsHumanLabel, len(stall.Attempts), stall.Holder, since.UTC().Format(time.RFC3339)) + now := g.now() + if v, ok := g.verdict(stall.Key(), since, now); ok { + if v.allow { + return "", false + } + return held, true + } + + ctx, cancel := context.WithTimeout(g.ctx, claimsGitHubTimeout) + events, err := g.events(ctx, repo, issue.Number, since) + cancel() + if err != nil { + if g.logger != nil { + g.logger.Warn("issue-claims: could not read timeline for escalation check, allowing the claim", + "issue", stall.Key(), "holder", stall.Holder, "error", err) + } + g.remember(stall.Key(), progressVerdict{since: since, allow: true, until: now.Add(claimProgressRecheck)}) + return "", false + } + if why, moved := claimProgress(events); moved { + g.remember(stall.Key(), progressVerdict{since: since, allow: true}) + if g.logger != nil { + g.logger.Debug("issue-claims: re-claim allowed, issue moved", "issue", stall.Key(), "holder", stall.Holder, "progress", why) + } + return "", false + } + g.remember(stall.Key(), progressVerdict{since: since, until: now.Add(claimProgressRecheck)}) + if g.ledger.MarkEscalated(stall) { + if g.logger != nil { + g.logger.Info("issue-claims: escalating instead of re-claiming", + "issue", stall.Key(), "holder", stall.Holder, "claims", len(stall.Attempts), "since", since) + } + g.escalate(stall) + } + return held, true +} + +func (g *claimEscalationGate) verdict(key string, since, now time.Time) (progressVerdict, bool) { + g.mu.Lock() + defer g.mu.Unlock() + v, ok := g.verdicts[key] + if !ok || !v.since.Equal(since) || (!v.until.IsZero() && !now.Before(v.until)) { + return progressVerdict{}, false + } + return v, true +} + +func (g *claimEscalationGate) remember(key string, v progressVerdict) { + g.mu.Lock() + defer g.mu.Unlock() + if g.verdicts == nil { + g.verdicts = map[string]progressVerdict{} + } + g.verdicts[key] = v +} + +// claimProgress reports the first timeline event that counts as the issue +// moving: a pull request referencing or linked to it, a commit referencing it, +// an assignee change, a close/reopen, or a label change. The claim machinery's +// own labels (claimed, preempted:*) and the gate's own needs-human label being +// ADDED do not count — otherwise every re-claim, and every escalation, would +// read as progress. needs-human being REMOVED does count: that is a person +// sending the issue back. +func claimProgress(events []github.IssueEvent) (string, bool) { + for _, e := range events { + switch e.Event { + case "cross-referenced": + if e.SourcePR > 0 { + return fmt.Sprintf("pull request #%d referenced it", e.SourcePR), true + } + case "connected": + return "a pull request was linked", true + case "referenced": + return "commit " + e.CommitID + " referenced it", true + case "assigned", "unassigned": + return "assignees changed", true + case "closed", "reopened": + return "it was " + e.Event, true + case "labeled", "unlabeled": + if strings.EqualFold(e.Label, claims.LabelClaimed) || + strings.HasPrefix(strings.ToLower(e.Label), claims.LabelPreemptedPrefix) { + continue + } + if e.Event == "labeled" && strings.EqualFold(e.Label, escalation.NeedsHumanLabel) { + continue + } + return fmt.Sprintf("label %q %s", e.Label, e.Event), true + } + } + return "", false +} diff --git a/src/cmd/hive/claimescalation_test.go b/src/cmd/hive/claimescalation_test.go new file mode 100644 index 0000000000..deff24a65d --- /dev/null +++ b/src/cmd/hive/claimescalation_test.go @@ -0,0 +1,215 @@ +package main + +import ( + "context" + "errors" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/hivecommons/hive/pkg/claims" + "github.com/hivecommons/hive/pkg/github" +) + +// escalationFixture is a ledger on a test clock plus a gate whose timeline +// reads and escalations are recorded instead of hitting GitHub. +type escalationFixture struct { + t *testing.T + ledger *claims.Ledger + now time.Time + gate *claimEscalationGate + timeline []github.IssueEvent // returned for every read, filtered by since + readErr error + reads int + escalated []claims.Stall +} + +func newEscalationFixture(t *testing.T) *escalationFixture { + t.Helper() + f := &escalationFixture{t: t, now: time.Date(2026, 10, 4, 15, 50, 0, 0, time.UTC)} + l, err := claims.New(filepath.Join(t.TempDir(), "claims.json"), claims.DefaultPolicy(), claims.Hooks{}) + if err != nil { + t.Fatal(err) + } + l.SetNow(func() time.Time { return f.now }) + f.ledger = l + f.gate = &claimEscalationGate{ + ctx: context.Background(), ledger: l, org: "myorg", threshold: 2, + events: func(_ context.Context, repo string, issue int, since time.Time) ([]github.IssueEvent, error) { + f.reads++ + if repo != "myorg/repo" || issue != 10512 { + t.Fatalf("timeline read for %s#%d", repo, issue) + } + if f.readErr != nil { + return nil, f.readErr + } + var out []github.IssueEvent + for _, e := range f.timeline { + if e.At.After(since) { + out = append(out, e) + } + } + return out, nil + }, + escalate: func(s claims.Stall) { f.escalated = append(f.escalated, s) }, + now: func() time.Time { return f.now }, + } + return f +} + +// kickClaim records the claim an agent kick naming the issue makes, then +// lets it lapse with nothing done — the #10512 pattern. +func (f *escalationFixture) kickClaim(agent string) { + f.t.Helper() + res, err := f.ledger.Claim(claims.Request{Repo: "myorg/repo", Issue: 10512, Holder: agent, HolderID: agent, Kind: claims.KindAgent}) + if err != nil || res.Outcome != claims.OutcomeClaimed { + f.t.Fatalf("claim by %s: outcome=%s err=%v", agent, res.Outcome, err) + } + f.now = f.now.Add(claims.DefaultAgentTTL + 7*time.Minute) +} + +func (f *escalationFixture) event(e github.IssueEvent) { + e.At = f.now.Add(-time.Minute) + f.timeline = append(f.timeline, e) +} + +// lookup asks the gate about the issue the way the scheduler does, with the +// bare repo name a default config's enumeration carries. +func (f *escalationFixture) lookup() (string, bool) { + return f.gate.lookup(github.Issue{Repo: "repo", Number: 10512}) +} + +func TestClaimEscalationRefusesThirdNoProgressClaim(t *testing.T) { + f := newEscalationFixture(t) + f.kickClaim("scanner") + if _, held := f.lookup(); held || f.reads != 0 { + t.Fatalf("one prior claim: held=%v reads=%d, want free without a timeline read", held, f.reads) + } + f.kickClaim("scanner") + // The claim machinery's own label churn and comments are not progress. + f.event(github.IssueEvent{Event: "unlabeled", Label: claims.LabelClaimed}) + f.event(github.IssueEvent{Event: "commented"}) + + holder, held := f.lookup() + if !held || !strings.Contains(holder, "needs-human") || !strings.Contains(holder, "scanner") { + t.Fatalf("third no-progress claim: held=%v holder=%q, want withheld and escalated", held, holder) + } + if len(f.escalated) != 1 || f.escalated[0].Holder != "scanner" || len(f.escalated[0].Attempts) != 2 { + t.Fatalf("escalations=%+v, want one for scanner's two claims", f.escalated) + } + + // The scheduler asks several times per cycle, and the stale enumeration + // keeps offering the issue until needs-human shows up in it: one + // escalation, one timeline read per recheck window. + f.event(github.IssueEvent{Event: "labeled", Label: "needs-human"}) + for range 3 { + if _, held := f.lookup(); !held { + t.Fatal("escalated issue offered again") + } + } + f.now = f.now.Add(claimProgressRecheck) + if _, held := f.lookup(); !held { + t.Fatal("the gate's own needs-human label counted as progress") + } + if len(f.escalated) != 1 || f.reads != 2 { + t.Fatalf("escalations=%d reads=%d, want 1 escalation and 2 reads", len(f.escalated), f.reads) + } + if _, ok := f.ledger.Lookup("myorg/repo", 10512); ok { + t.Fatal("refused claim left a live claim behind") + } + + // A person removing needs-human is progress: the next claim goes ahead. + f.now = f.now.Add(claimProgressRecheck) + f.event(github.IssueEvent{Event: "unlabeled", Label: "needs-human"}) + if _, held := f.lookup(); held { + t.Fatal("issue still withheld after needs-human was removed") + } +} + +func TestClaimEscalationAllowsClaimsWithAPullRequestBetween(t *testing.T) { + f := newEscalationFixture(t) + f.kickClaim("scanner") + f.event(github.IssueEvent{Event: "cross-referenced", SourcePR: 10530}) + f.kickClaim("scanner") + + if holder, held := f.lookup(); held { + t.Fatalf("claims with a PR between withheld: %q", holder) + } + if len(f.escalated) != 0 { + t.Fatalf("escalated despite a PR: %+v", f.escalated) + } + // The verdict holds for the run: no second read. + if _, held := f.lookup(); held || f.reads != 1 { + t.Fatalf("held=%v reads=%d, want free on one read", held, f.reads) + } + + // Two more claims with nothing between them form a new run, which + // escalates: the PR before it does not excuse it forever. + f.kickClaim("scanner") + f.kickClaim("scanner") + if _, held := f.lookup(); !held || len(f.escalated) != 1 { + t.Fatalf("held=%v escalations=%d, want the new no-progress run escalated", held, len(f.escalated)) + } +} + +func TestClaimEscalationSkipsWhileClaimedAndAcrossAgents(t *testing.T) { + f := newEscalationFixture(t) + f.kickClaim("ci-maintainer") + f.kickClaim("scanner") + if _, held := f.lookup(); held || f.reads != 0 { + t.Fatalf("one claim each by two agents: held=%v reads=%d", held, f.reads) + } + if _, err := f.ledger.Claim(claims.Request{Repo: "myorg/repo", Issue: 10512, Holder: "scanner", Kind: claims.KindAgent}); err != nil { + t.Fatal(err) + } + if _, held := f.lookup(); held || f.reads != 0 { + t.Fatalf("live claim: held=%v reads=%d — the claims lookup owns a held issue", held, f.reads) + } +} + +func TestClaimEscalationFailsOpenOnTimelineError(t *testing.T) { + f := newEscalationFixture(t) + f.kickClaim("scanner") + f.kickClaim("scanner") + f.readErr = errors.New("502 bad gateway") + if _, held := f.lookup(); held || len(f.escalated) != 0 { + t.Fatalf("timeline error: held=%v escalations=%d, want the claim allowed", held, len(f.escalated)) + } + if _, held := f.lookup(); held || f.reads != 1 { + t.Fatalf("held=%v reads=%d, want the failed read reused within the recheck window", held, f.reads) + } + f.readErr = nil + f.now = f.now.Add(claimProgressRecheck) + if _, held := f.lookup(); !held || len(f.escalated) != 1 { + t.Fatalf("after recovery: held=%v escalations=%d, want escalated", held, len(f.escalated)) + } +} + +func TestClaimProgress(t *testing.T) { + cases := []struct { + name string + event github.IssueEvent + moved bool + }{ + {"linked pr", github.IssueEvent{Event: "cross-referenced", SourcePR: 7}, true}, + {"issue cross-reference", github.IssueEvent{Event: "cross-referenced"}, false}, + {"development link", github.IssueEvent{Event: "connected"}, true}, + {"commit", github.IssueEvent{Event: "referenced", CommitID: "abc"}, true}, + {"assignee", github.IssueEvent{Event: "assigned"}, true}, + {"closed", github.IssueEvent{Event: "closed"}, true}, + {"other label", github.IssueEvent{Event: "labeled", Label: "priority/high"}, true}, + {"claimed label", github.IssueEvent{Event: "labeled", Label: "claimed"}, false}, + {"preempted label", github.IssueEvent{Event: "unlabeled", Label: "preempted:alice"}, false}, + {"needs-human added", github.IssueEvent{Event: "labeled", Label: "Needs-Human"}, false}, + {"needs-human removed", github.IssueEvent{Event: "unlabeled", Label: "needs-human"}, true}, + {"comment", github.IssueEvent{Event: "commented"}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if _, moved := claimProgress([]github.IssueEvent{tc.event}); moved != tc.moved { + t.Fatalf("claimProgress(%+v) moved=%v, want %v", tc.event, moved, tc.moved) + } + }) + } +} diff --git a/src/cmd/hive/main.go b/src/cmd/hive/main.go index cc7c76febb..06e1db3a83 100644 --- a/src/cmd/hive/main.go +++ b/src/cmd/hive/main.go @@ -3347,8 +3347,12 @@ func (b *boot) bootSupervision() { // the pr-request watcher narrates opened PRs into the session. // #8380: the worker-claim ledger is a second in-flight source — an issue a // human, contributor or another agent holds is withheld from kicks too. + // #10527: a free issue an agent already claimed escalate_after_claims + // times with nothing moving is withheld and escalated instead of + // re-claimed. b.sched.SetInflightLookup(composeInflight(b.dashSrv.LinearSessionHolder, - claimsInflightLookup(b.issueClaims, b.cfg.Project.Org))) + claimsInflightLookup(b.issueClaims, b.cfg.Project.Org), + claimEscalationLookup(b.ctx, b.cfg, b.issueClaims, func() *github.Client { return b.ghClient }, b.logger))) // #9584: close answered question issues unless the author objects (default off). b.questionAutoclose.Store(wireQuestionAutoclose(b.ctx, b.cfg, b.sched, func() *github.Client { return b.ghClient }, b.logger)) // The pr-request watcher's PR-opened hook (Linear session narration, the diff --git a/src/docs/labels-and-control-signals.md b/src/docs/labels-and-control-signals.md index b0aff05fa1..105c6b363f 100644 --- a/src/docs/labels-and-control-signals.md +++ b/src/docs/labels-and-control-signals.md @@ -41,7 +41,7 @@ The "respect hold labels" text in policy templates names the enforced set (`hold | `hold` from SHA-hold | Issues | SHA-hold sweep | Human-filed primary-repo `kind/bug` without a 7-40 hex SHA gets `hold` and a notice carrying ``; once a SHA appears in body/comments, Hive removes `hold` only if the current hold is its own (newest `hold` event labeled by the App bot, paired with an App-authored notice at or after it). A human's hold, a re-hold, or another subsystem's hold stays. | Hard issue hold | Hive | Eval tick SHA sweep | Add SHA evidence; sweep removes its own `hold`, a human removes any other | Primary repo and SHA-hold config (`src/pkg/github/client.go:2467-2532`, `src/pkg/github/client.go:2582-2604`) | | `do-not-merge` and `do-not-merge*` | Issues, PRs | Exempt filter, merge sweeps, task-list sweep | Permanently exempt; exact match is case-insensitive but prefix matching follows Go `strings.HasPrefix` on the original label. | Hard suppress | Human | Enumeration/sweeps | Human removes | Built-in `PermanentExemptLabels` (`src/pkg/github/client.go:750-755`, `src/pkg/github/client.go:2030-2048`) | | `governor.labels.exempt` entries | Issues, PRs | Exempt filter | Same exempt behavior as `do-not-merge`; wins over required-label admission. Defaults include `nightly-tests`, `LFX`, `meta-tracker`, `auto-qa-tuning-report`, `adopters`, `changes-requested`, `waiting-on-author`. | Hard suppress | Operator/dashboard | Enumeration/sweeps | Remove label or config entry | `governor.labels.exempt` (`src/pkg/config/config.go:3388-3396`, `src/pkg/config/config.go:5800-5808`) | -| `needs-human` | Issues | Enumeration, task-list sweep and un-park sweep | Issue is filtered, not held: no agent kick or contributor offer; task-list sweep can add it when a merged PR leaves human remainder work (never on a held issue). The un-park sweep keeps a "What to reply" comment on every parked issue listing the commands. See [Maintainer commands](maintainer-commands.md). | Hard suppress | Hive or human | Enumeration/task-list sweep | Human removes, or a maintainer with write/maintain/admin access replies `/hive approve` or `/hive decision ` on the issue, which also adds `approved-direction` and never touches `hold` (`src/pkg/github/issue_unpark_command.go`) | Fixed label (`src/pkg/github/client.go:1008-1024`, `src/pkg/github/task_list_sweep.go:611-641`, `src/pkg/github/task_list_sweep.go:797-815`) | +| `needs-human` | Issues | Enumeration, task-list sweep, claim escalation gate and un-park sweep | Issue is filtered, not held: no agent kick or contributor offer; task-list sweep can add it when a merged PR leaves human remainder work (never on a held issue), and the claim escalation gate adds it instead of a third no-progress claim by the same agent (`governor.claims.escalate_after_claims`, [#10527](https://github.com/hivecommons/hive/issues/10527)). The un-park sweep keeps a "What to reply" comment on every parked issue listing the commands. See [Maintainer commands](maintainer-commands.md). | Hard suppress | Hive or human | Enumeration/task-list sweep/kick building | Human removes, or a maintainer with write/maintain/admin access replies `/hive approve` or `/hive decision ` on the issue, which also adds `approved-direction` and never touches `hold` (`src/pkg/github/issue_unpark_command.go`) | Fixed label (`src/pkg/github/client.go:1008-1024`, `src/pkg/github/task_list_sweep.go:611-641`, `src/pkg/github/task_list_sweep.go:797-815`, `src/cmd/hive/claimescalation.go`) | | `needs-human` | PRs | Escalation ledger and reviewer lane | Applied when red-CI fix budget is exhausted; stops automated fix dispatch and sends the PR to the human/reviewer lane. It is not a PR enumeration filter. | Hard fix-dispatch gate | Hive | Escalation pass/kick building | Human removes after root cause or reviewer un-escalates; budget resets after grace | Fixed label (`src/pkg/escalation/escalation.go:400-470`, `src/pkg/escalation/escalation.go:1185-1205`) | | `hive/advisory` | Issues | Standing meta classifier | Hive advisory report is never actionable and never appears on the Hold list. | Hard exclusion | Hive | Before holds/exempts | Remove label, but exact advisory title still excludes | Fixed label/name (`src/pkg/github/standing_issues.go:1-57`, `src/pkg/github/client.go:983-1001`) | | `approved-direction` | Agent-filed issues | #5117 gate, ranking, kick tag | A PR opened after the label is present avoids #5117 hold; the issue ranks ahead of unacknowledged hive-filed backlog. | Gate input + soft ranking | Human | PR creation and issue ranking | Remove label; human assignee/comment can still acknowledge for #5117 | Fixed `HumanAckLabel` (`src/pkg/github/pr_self_authorization.go:14-226`, `src/pkg/github/client.go:2740-2832`) | diff --git a/src/docs/operator-reference.md b/src/docs/operator-reference.md index 3bb9d9dd1b..162d9f0b09 100644 --- a/src/docs/operator-reference.md +++ b/src/docs/operator-reference.md @@ -74,7 +74,7 @@ Top-level YAML keys accepted by `config.Config`: | `contribute.help_links` | Defaults to Hive contributor docs and the Hive issue tracker. | Up to 5 `{label, url}` links shown on `/contribute` Onboarding, Operations, and printed once by relays on connect. URLs must be absolute `http://` or `https://`; labels are plain text and length-capped. Prefer the Hive Commons Discord redirect (`https://hivecommons.dev/discord`) over channel URLs for contributors who may not already be server members. | | `variables` | Trusted variable resolver definitions. | Env-only substitution works without this block. | | `classification` | Go-consumed subset of `hive-project.yaml`'s `classification:` block — today only `review_bots`, the external review-bot logins whose inline threads on hive-authored PRs the hive addresses and resolves itself. | Off unless `review_bots.logins` names a bot. The same key in `hive-project.yaml` is read when `hive.yaml` has none. See [review-bot-threads.md](review-bot-threads.md). | -| `claims` | The worker-claim ledger ([#8380](https://github.com/hivecommons/hive/issues/8380)): who is on an issue right now, ranked human > agent > contributor > external, so a person can take an issue over from a hive agent or a relay contributor and the displaced holder is told to stop. | Default ON. `enabled: false` turns recording, enforcement and `/api/claims` off. `human_ttl_s` 4h, `agent_ttl_s` 2h, `contributor_ttl_s` 30m, `max_ttl_s` 24h; `comment: false` / `label: false` suppress the GitHub issue comment / `claimed` label mirror. Driven from `hivectl claim` — see [hivectl.md](hivectl.md#claim--unclaim--claims--issue-claims). | +| `claims` | The worker-claim ledger ([#8380](https://github.com/hivecommons/hive/issues/8380)): who is on an issue right now, ranked human > agent > contributor > external, so a person can take an issue over from a hive agent or a relay contributor and the displaced holder is told to stop. | Default ON. `enabled: false` turns recording, enforcement and `/api/claims` off. `human_ttl_s` 4h, `agent_ttl_s` 2h, `contributor_ttl_s` 30m, `max_ttl_s` 24h; `comment: false` / `label: false` suppress the GitHub issue comment / `claimed` label mirror. `escalate_after_claims` (default `2`, negative turns it off): once one agent's last that-many claims on a free issue were followed by no linked PR, referencing commit, label or assignee change and no close/reopen, the issue is withheld from every kick, labelled `needs-human` and given one comment listing the claims, instead of being claimed again ([#10527](https://github.com/hivecommons/hive/issues/10527)). Driven from `hivectl claim` — see [hivectl.md](hivectl.md#claim--unclaim--claims--issue-claims). | | `removed_agents` | Persistent tombstones for deleted agents. | Dashboard/overlay-owned; do not seed casually. | ## Notable fields diff --git a/src/pkg/claims/claims.go b/src/pkg/claims/claims.go index d53a53c741..3320b28a56 100644 --- a/src/pkg/claims/claims.go +++ b/src/pkg/claims/claims.go @@ -123,6 +123,46 @@ func (c Claim) Expired(now time.Time) bool { return !now.Before(c.ExpiresAt) } // Key builds the canonical work-item key. func Key(repo string, issue int) string { return repo + "#" + fmt.Sprint(issue) } +// Attempt is one agent claim on an issue, remembered after the claim itself +// ends so the escalation gate (#10527) can tell an agent re-claiming an issue +// it already held, with nothing moving in between, from a healthy long fix. +type Attempt struct { + // Holder is the agent that claimed. + Holder string `json:"holder"` + // ClaimedAt is when the claim was taken. + ClaimedAt time.Time `json:"claimed_at"` + // EndedAt is when the claim ended; zero while it is live. + EndedAt time.Time `json:"ended_at,omitempty"` + // Ended is how it ended: "expired", a release reason, or "taken over by + // ". + Ended string `json:"ended,omitempty"` +} + +// AttemptRetention is how long an agent's claim attempts are remembered. A +// run of claims spread over more than this is not treated as one stall. +const AttemptRetention = 7 * 24 * time.Hour + +// Stall is the most recent run of claims one agent made on an issue that is +// free again — the shape the escalation gate (#10527) checks for progress. +type Stall struct { + Repo string + Issue int + Holder string + // Attempts are the holder's trailing claims on the issue, oldest first. + Attempts []Attempt +} + +// Key is the canonical "owner/repo#N" key of the stalled issue. +func (s Stall) Key() string { return Key(s.Repo, s.Issue) } + +// Since is when the run began. Progress on the issue after it breaks the run. +func (s Stall) Since() time.Time { + if len(s.Attempts) == 0 { + return time.Time{} + } + return s.Attempts[0].ClaimedAt +} + // Request is one attempt to claim an issue. type Request struct { Repo string @@ -264,11 +304,17 @@ func (p Policy) ttlFor(k Kind, requested time.Duration) time.Duration { type Ledger struct { mu sync.Mutex claims map[string]Claim - path string - policy Policy - hooks Hooks - now func() time.Time - hive string + // attempts holds every agent claim per issue key, oldest first, pruned + // after AttemptRetention. + attempts map[string][]Attempt + // escalated maps an issue key to the Since of the last stall escalated + // on it, so one run is escalated once however often it is observed. + escalated map[string]time.Time + path string + policy Policy + hooks Hooks + now func() time.Time + hive string } // ErrInvalid is returned for requests missing a repo, issue, holder or kind. @@ -282,11 +328,13 @@ func New(path string, policy Policy, hooks Hooks) (*Ledger, error) { policy = DefaultPolicy() } l := &Ledger{ - claims: map[string]Claim{}, - path: path, - policy: policy, - hooks: hooks, - now: time.Now, + claims: map[string]Claim{}, + attempts: map[string][]Attempt{}, + escalated: map[string]time.Time{}, + path: path, + policy: policy, + hooks: hooks, + now: time.Now, } err := l.load() return l, err @@ -340,6 +388,7 @@ func (l *Ledger) Claim(req Request) (Result, error) { case !held: l.claims[key] = next res = Result{Outcome: OutcomeClaimed, Claim: next} + l.recordAttemptLocked(next) case sameHolder(prev, req): prev.ExpiresAt = now.Add(ttl) l.claims[key] = prev @@ -350,6 +399,8 @@ func (l *Ledger) Claim(req Request) (Result, error) { l.claims[key] = next p := prev res = Result{Outcome: OutcomeTakenOver, Claim: next, Previous: &p} + l.endAttemptLocked(prev, "taken over by "+req.Holder, now) + l.recordAttemptLocked(next) case req.Kind.rank() == prev.Kind.rank(): p := prev res = Result{Outcome: OutcomeHeld, Claim: prev, Previous: &p} @@ -412,7 +463,8 @@ func (l *Ledger) Release(repo string, issue int, by string, byKind Kind, reason return Claim{}, false, nil } l.mu.Lock() - expired := l.expireLocked(l.now()) + now := l.now() + expired := l.expireLocked(now) key := Key(strings.TrimSpace(repo), issue) c, ok := l.claims[key] if !ok { @@ -431,6 +483,7 @@ func (l *Ledger) Release(repo string, issue int, by string, byKind Kind, reason return c, false, fmt.Errorf("claims: %s is held by %s (%s)", key, c.Holder, c.Kind) } delete(l.claims, key) + l.endAttemptLocked(c, reason, now) err := l.saveLocked() hooks := l.hooks l.mu.Unlock() @@ -452,6 +505,7 @@ func (l *Ledger) ForceRelease(repo string, issue int, reason string) (Claim, boo c, ok := l.claims[key] if ok { delete(l.claims, key) + l.endAttemptLocked(c, reason, l.now()) _ = l.saveLocked() } hooks := l.hooks @@ -471,6 +525,7 @@ func (l *Ledger) ReleaseByHolderID(holderID, reason string, onlyKey ...string) [ return nil } l.mu.Lock() + now := l.now() var out []Claim for k, c := range l.claims { if c.HolderID != holderID { @@ -481,6 +536,7 @@ func (l *Ledger) ReleaseByHolderID(holderID, reason string, onlyKey ...string) [ } out = append(out, c) delete(l.claims, k) + l.endAttemptLocked(c, reason, now) } if len(out) > 0 { _ = l.saveLocked() @@ -596,16 +652,140 @@ func (l *Ledger) expireLocked(now time.Time) []Claim { if c.Expired(now) { dropped = append(dropped, c) delete(l.claims, k) + l.endAttemptLocked(c, "expired", c.ExpiresAt) } } + l.pruneLocked(now) return dropped } +// ---- attempt history (#10527) ---- + +// recordAttemptLocked remembers a new agent claim. Only agent claims are +// counted: the gate exists to stop the hive's own agents spinning on an issue, +// and a person's claim is never second-guessed. +func (l *Ledger) recordAttemptLocked(c Claim) { + if c.Kind != KindAgent { + return + } + key := c.Key() + l.attempts[key] = append(l.attempts[key], Attempt{Holder: c.Holder, ClaimedAt: c.ClaimedAt}) +} + +// endAttemptLocked records how an agent claim ended on its attempt entry. +func (l *Ledger) endAttemptLocked(c Claim, reason string, at time.Time) { + if c.Kind != KindAgent { + return + } + list := l.attempts[c.Key()] + for i := len(list) - 1; i >= 0; i-- { + if list[i].Holder == c.Holder && list[i].ClaimedAt.Equal(c.ClaimedAt) { + if list[i].EndedAt.IsZero() { + list[i].EndedAt, list[i].Ended = at, reason + } + return + } + } +} + +// pruneLocked forgets attempts and escalations older than AttemptRetention. +func (l *Ledger) pruneLocked(now time.Time) { + cutoff := now.Add(-AttemptRetention) + for k, list := range l.attempts { + keep := list[:0] + for _, a := range list { + if a.ClaimedAt.After(cutoff) { + keep = append(keep, a) + } + } + if len(keep) == 0 { + delete(l.attempts, k) + continue + } + l.attempts[k] = keep + } + for k, since := range l.escalated { + if !since.After(cutoff) { + delete(l.escalated, k) + } + } +} + +// Stall reports the most recent run of threshold claims one agent made on the +// issue, when the issue has no live claim. Of several agents with such a run +// it returns the one whose run began last: if anything moved since that run +// began, it moved during every other agent's run too. threshold <= 0, a live +// claim, or no agent with threshold remembered claims reports false. Lapsed +// claims are expired first, so the run's last attempt carries its end. +func (l *Ledger) Stall(repo string, issue, threshold int) (Stall, bool) { + if l == nil || threshold <= 0 { + return Stall{}, false + } + repo = strings.TrimSpace(repo) + key := Key(repo, issue) + l.mu.Lock() + expired := l.expireLocked(l.now()) + if len(expired) > 0 { + _ = l.saveLocked() + } + var best Stall + if _, held := l.claims[key]; !held { + best = l.stallLocked(repo, issue, threshold) + } + hooks := l.hooks + l.mu.Unlock() + fireExpired(hooks, expired) + return best, best.Attempts != nil +} + +func (l *Ledger) stallLocked(repo string, issue, threshold int) Stall { + byHolder := map[string][]Attempt{} + for _, a := range l.attempts[Key(repo, issue)] { + byHolder[a.Holder] = append(byHolder[a.Holder], a) + } + var best Stall + for holder, list := range byHolder { + if len(list) < threshold { + continue + } + run := append([]Attempt(nil), list[len(list)-threshold:]...) + if best.Attempts == nil || run[0].ClaimedAt.After(best.Since()) || + (run[0].ClaimedAt.Equal(best.Since()) && holder < best.Holder) { + best = Stall{Repo: repo, Issue: issue, Holder: holder, Attempts: run} + } + } + return best +} + +// MarkEscalated records that the stall s was escalated and reports whether +// this call was the first for that run, so the escalation fires once per run +// however many times the gate observes it. A later run on the same issue (it +// began after s) escalates again. +func (l *Ledger) MarkEscalated(s Stall) bool { + if l == nil || len(s.Attempts) == 0 { + return false + } + key := s.Key() + since := s.Since() + l.mu.Lock() + defer l.mu.Unlock() + if prev, ok := l.escalated[key]; ok && prev.Equal(since) { + return false + } + l.escalated[key] = since + _ = l.saveLocked() + return true +} + // ---- persistence ---- type persisted struct { Version int `json:"version"` Claims []Claim `json:"claims"` + // Attempts and Escalated carry the #10527 escalation history; both are + // absent from files written before it, which load as no history. + Attempts map[string][]Attempt `json:"attempts,omitempty"` + Escalated map[string]time.Time `json:"escalated,omitempty"` } const persistVersion = 1 @@ -631,6 +811,16 @@ func (l *Ledger) load() error { } l.claims[c.Key()] = c } + for k, list := range p.Attempts { + for _, a := range list { + if a.Holder != "" && !a.ClaimedAt.IsZero() { + l.attempts[k] = append(l.attempts[k], a) + } + } + } + for k, since := range p.Escalated { + l.escalated[k] = since + } return nil } @@ -646,6 +836,12 @@ func (l *Ledger) saveLocked() error { p.Claims = append(p.Claims, c) } sort.Slice(p.Claims, func(i, j int) bool { return p.Claims[i].Key() < p.Claims[j].Key() }) + if len(l.attempts) > 0 { + p.Attempts = l.attempts + } + if len(l.escalated) > 0 { + p.Escalated = l.escalated + } data, err := json.MarshalIndent(p, "", " ") if err != nil { return err diff --git a/src/pkg/claims/comments.go b/src/pkg/claims/comments.go index 548a7f9152..f28a3c464f 100644 --- a/src/pkg/claims/comments.go +++ b/src/pkg/claims/comments.go @@ -18,9 +18,10 @@ const ( // Marker names used in the HTML comments the hub writes so tooling can parse // them without scraping prose. const ( - markerClaim = "hive:claim" - markerPreempt = "hive:preempt" - markerRelease = "hive:release" + markerClaim = "hive:claim" + markerPreempt = "hive:preempt" + markerRelease = "hive:release" + markerEscalation = "hive:claim-escalation" ) // PreemptedLabel is the label a displaced holder should watch for. @@ -69,6 +70,37 @@ func ReleaseComment(c Claim, reason string) string { return b.String() } +// EscalationComment renders the comment posted instead of another claim when +// an agent's last claims on an issue moved nothing (#10527, ADR-0019). label +// is the label the issue is parked with. +func EscalationComment(s Stall, label string) string { + agent := Claim{Holder: s.Holder, Kind: KindAgent} + var b strings.Builder + fmt.Fprintf(&b, "\n", + markerEscalation, s.Holder, KindAgent, len(s.Attempts), s.Since().UTC().Format(time.RFC3339)) + fmt.Fprintf(&b, "🚨 Not claiming this again: %s claimed it %d times and nothing moved in between — no linked pull request, referencing commit, label or assignee change, or close/reopen since %s UTC.\n\n", + mention(agent), len(s.Attempts), s.Since().UTC().Format("2006-01-02 15:04")) + b.WriteString("| Claimed | How it ended |\n| --- | --- |\n") + for _, a := range s.Attempts { + ended := "no end recorded" + if !a.EndedAt.IsZero() { + ended = fmt.Sprintf("%s at %s UTC", tableCell(a.Ended), a.EndedAt.UTC().Format("2006-01-02 15:04")) + } + fmt.Fprintf(&b, "| %s UTC | %s |\n", a.ClaimedAt.UTC().Format("2006-01-02 15:04"), ended) + } + fmt.Fprintf(&b, "\nLabelled `%s` and left out of every work list. Removing the label (or replying `/hive approve`) counts as progress, so the next claim goes ahead.", label) + return b.String() +} + +// tableCell keeps a free-text reason inside one markdown table cell. +func tableCell(s string) string { + s = strings.Join(strings.Fields(s), " ") + if s == "" { + return "ended" + } + return strings.ReplaceAll(s, "|", `\|`) +} + func mention(c Claim) string { switch c.Kind { case KindHuman, KindExternal, KindContributor: diff --git a/src/pkg/claims/stall_test.go b/src/pkg/claims/stall_test.go new file mode 100644 index 0000000000..98664cbffa --- /dev/null +++ b/src/pkg/claims/stall_test.go @@ -0,0 +1,166 @@ +package claims + +import ( + "path/filepath" + "strings" + "testing" + "time" +) + +// claimAndLapse records an agent claim at *now and moves the clock past its +// expiry, the way an agent kick's claim runs out with nothing done. +func claimAndLapse(t *testing.T, l *Ledger, now *time.Time, holder string) { + t.Helper() + if res, err := l.Claim(Request{Repo: "o/r", Issue: 5, Holder: holder, HolderID: holder, Kind: KindAgent}); err != nil || res.Outcome != OutcomeClaimed { + t.Fatalf("claim by %s: outcome=%s err=%v", holder, res.Outcome, err) + } + *now = now.Add(DefaultAgentTTL + time.Minute) +} + +func TestStallNeedsThresholdAgentClaimsOnAFreeIssue(t *testing.T) { + l, now := newTestLedger(t, Hooks{}) + first := *now + claimAndLapse(t, l, now, "scanner") + if _, ok := l.Stall("o/r", 5, 2); ok { + t.Fatal("one claim reported as a stall at threshold 2") + } + second := *now + if _, err := l.Claim(Request{Repo: "o/r", Issue: 5, Holder: "scanner", HolderID: "scanner", Kind: KindAgent}); err != nil { + t.Fatal(err) + } + if _, ok := l.Stall("o/r", 5, 2); ok { + t.Fatal("stall reported while the second claim is still live") + } + *now = now.Add(DefaultAgentTTL + time.Minute) + + s, ok := l.Stall("o/r", 5, 2) + if !ok || s.Holder != "scanner" || len(s.Attempts) != 2 || !s.Since().Equal(first) || !s.Attempts[1].ClaimedAt.Equal(second) { + t.Fatalf("Stall=%+v ok=%v, want scanner run since %v", s, ok, first) + } + for _, a := range s.Attempts { + if a.Ended != "expired" || !a.EndedAt.Equal(a.ClaimedAt.Add(DefaultAgentTTL)) { + t.Fatalf("attempt end not recorded: %+v", a) + } + } + if _, ok := l.Stall("o/r", 5, 3); ok { + t.Fatal("two claims reported as a stall at threshold 3") + } + if _, ok := l.Stall("o/r", 5, 0); ok { + t.Fatal("threshold 0 must disable the check") + } +} + +func TestStallIgnoresHumanClaimsAndCountsPerAgent(t *testing.T) { + l, now := newTestLedger(t, Hooks{}) + if _, err := l.Claim(Request{Repo: "o/r", Issue: 5, Holder: "alice", Kind: KindHuman}); err != nil { + t.Fatal(err) + } + *now = now.Add(DefaultHumanTTL + time.Minute) + claimAndLapse(t, l, now, "ci-maintainer") + claimAndLapse(t, l, now, "scanner") + if s, ok := l.Stall("o/r", 5, 2); ok { + t.Fatalf("claims by three different holders reported as a stall: %+v", s) + } + claimAndLapse(t, l, now, "ci-maintainer") + s, ok := l.Stall("o/r", 5, 2) + if !ok || s.Holder != "ci-maintainer" { + t.Fatalf("Stall=%+v ok=%v, want ci-maintainer", s, ok) + } +} + +func TestStallRecordsTakeoverAsTheAttemptEnd(t *testing.T) { + l, now := newTestLedger(t, Hooks{}) + if _, err := l.Claim(Request{Repo: "o/r", Issue: 5, Holder: "scanner", Kind: KindAgent}); err != nil { + t.Fatal(err) + } + *now = now.Add(time.Minute) + if res, _ := l.Claim(Request{Repo: "o/r", Issue: 5, Holder: "alice", Kind: KindHuman}); res.Outcome != OutcomeTakenOver { + t.Fatalf("outcome=%s want taken_over", res.Outcome) + } + if _, _, err := l.Release("o/r", 5, "alice", KindHuman, "released by alice"); err != nil { + t.Fatal(err) + } + claimAndLapse(t, l, now, "scanner") + s, ok := l.Stall("o/r", 5, 2) + if !ok || s.Attempts[0].Ended != "taken over by alice" { + t.Fatalf("Stall=%+v ok=%v, want first attempt ended by takeover", s, ok) + } +} + +func TestMarkEscalatedOncePerRunAndPersisted(t *testing.T) { + path := filepath.Join(t.TempDir(), "claims.json") + now := time.Date(2026, 10, 4, 12, 0, 0, 0, time.UTC) + l, err := New(path, DefaultPolicy(), Hooks{}) + if err != nil { + t.Fatal(err) + } + l.SetNow(func() time.Time { return now }) + claimAndLapse(t, l, &now, "scanner") + claimAndLapse(t, l, &now, "scanner") + s, ok := l.Stall("o/r", 5, 2) + if !ok { + t.Fatal("no stall after two lapsed claims") + } + if !l.MarkEscalated(s) { + t.Fatal("first MarkEscalated = false") + } + if l.MarkEscalated(s) { + t.Fatal("same run escalated twice") + } + + reloaded, err := New(path, DefaultPolicy(), Hooks{}) + if err != nil { + t.Fatal(err) + } + reloaded.SetNow(func() time.Time { return now }) + rs, ok := reloaded.Stall("o/r", 5, 2) + if !ok || !rs.Since().Equal(s.Since()) || rs.Attempts[1].Ended != "expired" { + t.Fatalf("reloaded Stall=%+v ok=%v, want %+v", rs, ok, s) + } + if reloaded.MarkEscalated(rs) { + t.Fatal("escalation forgotten across a restart: the same run would be escalated again") + } + + // A later run (it begins after the escalated one) escalates again. + claimAndLapse(t, reloaded, &now, "scanner") + next, ok := reloaded.Stall("o/r", 5, 2) + if !ok || !next.Since().After(s.Since()) { + t.Fatalf("next Stall=%+v ok=%v", next, ok) + } + if !reloaded.MarkEscalated(next) { + t.Fatal("a new run was suppressed as already escalated") + } +} + +func TestStallForgetsAttemptsPastRetention(t *testing.T) { + l, now := newTestLedger(t, Hooks{}) + claimAndLapse(t, l, now, "scanner") + *now = now.Add(AttemptRetention) + claimAndLapse(t, l, now, "scanner") + if s, ok := l.Stall("o/r", 5, 2); ok { + t.Fatalf("claims more than AttemptRetention apart reported as one run: %+v", s) + } +} + +func TestEscalationComment(t *testing.T) { + at := time.Date(2026, 10, 4, 15, 50, 0, 0, time.UTC) + s := Stall{Repo: "o/r", Issue: 5, Holder: "scanner", Attempts: []Attempt{ + {Holder: "scanner", ClaimedAt: at, EndedAt: at.Add(2 * time.Hour), Ended: "expired"}, + {Holder: "scanner", ClaimedAt: at.Add(127 * time.Minute), EndedAt: at.Add(247 * time.Minute), Ended: "a | b"}, + }} + got := EscalationComment(s, "needs-human") + for _, want := range []string{ + "", + "agent `scanner` claimed it 2 times", + "| 2026-10-04 15:50 UTC | expired at 2026-10-04 17:50 UTC |", + `| 2026-10-04 17:57 UTC | a \| b at 2026-10-04 19:57 UTC |`, + "Labelled `needs-human`", + } { + if !strings.Contains(got, want) { + t.Fatalf("EscalationComment missing %q:\n%s", want, got) + } + } + if strings.Contains(got, "` marker (or `` on takeover) and a `claimed` label, so a relay that only reads GitHub can still see it was preempted. +- **An issue somebody has *claimed* is not offered — and a clanker already on it is pulled off when a higher rank claims it.** The PR-claim ledger above only knows about work that has reached a pull request. Since [#8380](https://github.com/hivecommons/hive/issues/8380) the hub also keeps a **worker-claim ledger**: who is on an issue *right now*, ranked **human > agent > contributor > external**. A person claims from the terminal (`hivectl claim owner/repo#N`) or the API (`POST /api/claims/{owner}/{repo}/{number}`); a governor kick *lists* each issue it hands one of the hive's own agents — no GitHub comment, no claim, just a 30-minute hold that keeps your session and other agents off it while the kick is live — and the hub records the **agent** claim on the agent's first start signal on that issue (its own comment, label or claim request through `hive-open-issue`, or a `hive-open-pr` request naming it; [#10527](https://github.com/hivecommons/hive/issues/10527)); and the relay records a **contributor** claim for your session the moment it leases you a task, releasing it when the task completes, is abandoned, is yanked or its lease expires. `selectTask` excludes every issue another holder claims exactly as it excludes leased items, and the scheduler withholds them from kicks under *In Flight*. Precedence is enforced on write: a higher rank **takes over** and the displaced holder is told to stop — for a relay session that means the hub yanks that one task (`task_revoke`, generation bumped so a late completion is fenced) and hands the session its next admissible item in the same call, leaving your other sessions alone; for an agent or an external bot it means a `🔁` comment addressed to it plus a `preempted:` label on the issue. Same rank warns unless forced; a lower rank is refused. Claims lapse on a per-kind TTL (`claims.human_ttl_s` 4h, `agent_ttl_s` 2h, `contributor_ttl_s` 30m — the relay lease length — capped by `max_ttl_s` 24h) and `claims.enabled: false` turns the whole mechanism off. Every transition is mirrored onto the issue as a comment carrying a `` marker (or `` on takeover) and a `claimed` label, so a relay that only reads GitHub can still see it was preempted. - **Claims expire.** Ledger entries live 72 hours — refreshed while the PR stays open, or anchored at the merge for a merged claim, so a settled issue is suppressed for 72h after its fix lands and nothing is stranded forever. An **open** claiming PR that goes red on a required check and stale releases the issue back to the queue; merged claims never take that valve — a fix already on `main` cannot go red. - **A repository the App cannot mint a token for is excluded, not fatal.** Since C4 the `task_assign` token is scoped to the chosen item's repository, so GitHub's `422 There is at least one repository that does not exist or is not accessible to the parent installation` means *that repo* — renamed, deleted, or removed from the installation — and nothing about the tier or the App key. It used to fail the whole selection with `token_mint_failed`, and because selection is deterministic the same doomed item was chosen on every 30 s retry: one stale entry in the project repo list (a renamed repo) wedged a hive for every contributor while `actionable_items` sat at 19 ([#7869](https://github.com/hivecommons/hive/issues/7869)). Now the claim is rolled back, the repo is excluded from selection for `mintFailureRepoCooldown` (10 min, so the fleet is not re-discovering it every retry, but the operator's fix takes effect without a restart), and the *same* call selects again from the remaining candidates (bounded at three such skips). If nothing else is admissible the reason is `repo_unmintable` rather than `no_matching_work` or `token_mint_failed`, and the hub log names the repo with a pointer at the repo list. Non-repo mint failures (5xx, bad key, an unaccepted permission) keep the `token_mint_failed` contract and exclude nothing — they would recur for any candidate. diff --git a/src/docs/operator-reference.md b/src/docs/operator-reference.md index 162d9f0b09..db7533495a 100644 --- a/src/docs/operator-reference.md +++ b/src/docs/operator-reference.md @@ -74,7 +74,7 @@ Top-level YAML keys accepted by `config.Config`: | `contribute.help_links` | Defaults to Hive contributor docs and the Hive issue tracker. | Up to 5 `{label, url}` links shown on `/contribute` Onboarding, Operations, and printed once by relays on connect. URLs must be absolute `http://` or `https://`; labels are plain text and length-capped. Prefer the Hive Commons Discord redirect (`https://hivecommons.dev/discord`) over channel URLs for contributors who may not already be server members. | | `variables` | Trusted variable resolver definitions. | Env-only substitution works without this block. | | `classification` | Go-consumed subset of `hive-project.yaml`'s `classification:` block — today only `review_bots`, the external review-bot logins whose inline threads on hive-authored PRs the hive addresses and resolves itself. | Off unless `review_bots.logins` names a bot. The same key in `hive-project.yaml` is read when `hive.yaml` has none. See [review-bot-threads.md](review-bot-threads.md). | -| `claims` | The worker-claim ledger ([#8380](https://github.com/hivecommons/hive/issues/8380)): who is on an issue right now, ranked human > agent > contributor > external, so a person can take an issue over from a hive agent or a relay contributor and the displaced holder is told to stop. | Default ON. `enabled: false` turns recording, enforcement and `/api/claims` off. `human_ttl_s` 4h, `agent_ttl_s` 2h, `contributor_ttl_s` 30m, `max_ttl_s` 24h; `comment: false` / `label: false` suppress the GitHub issue comment / `claimed` label mirror. `escalate_after_claims` (default `2`, negative turns it off): once one agent's last that-many claims on a free issue were followed by no linked PR, referencing commit, label or assignee change and no close/reopen, the issue is withheld from every kick, labelled `needs-human` and given one comment listing the claims, instead of being claimed again ([#10527](https://github.com/hivecommons/hive/issues/10527)). Driven from `hivectl claim` — see [hivectl.md](hivectl.md#claim--unclaim--claims--issue-claims). | +| `claims` | The worker-claim ledger ([#8380](https://github.com/hivecommons/hive/issues/8380)): who is on an issue right now, ranked human > agent > contributor > external, so a person can take an issue over from a hive agent or a relay contributor and the displaced holder is told to stop. | Default ON. `enabled: false` turns recording, enforcement and `/api/claims` off. `human_ttl_s` 4h, `agent_ttl_s` 2h, `contributor_ttl_s` 30m, `max_ttl_s` 24h; `comment: false` / `label: false` suppress the GitHub issue comment / `claimed` label mirror. A governor kick only *lists* the issues it names (30-minute hold, no comment, not a claim); the agent claim, with its comment, is recorded on the agent's first start signal on an issue — its own comment, label or claim request through `hive-open-issue`, or a `hive-open-pr` request naming it. `escalate_after_claims` (default `2`, negative turns it off) counts those claims only: once one agent's last that-many claims on a free issue were followed by no linked PR, referencing commit, label or assignee change and no close/reopen, the issue is withheld from every kick, labelled `needs-human` and given one comment listing the claims, instead of being claimed again. A `hive/likely-done` issue an agent already started on after the merged PR (its verification) is withheld from kicks until a person comments, labels, assigns, closes or reopens it, so it is verified once rather than every cycle ([#10527](https://github.com/hivecommons/hive/issues/10527)). Driven from `hivectl claim` — see [hivectl.md](hivectl.md#claim--unclaim--claims--issue-claims). | | `removed_agents` | Persistent tombstones for deleted agents. | Dashboard/overlay-owned; do not seed casually. | ## Notable fields diff --git a/src/pkg/claims/claims.go b/src/pkg/claims/claims.go index 3320b28a56..6926d51ba6 100644 --- a/src/pkg/claims/claims.go +++ b/src/pkg/claims/claims.go @@ -40,8 +40,8 @@ const ( // KindHuman is a person acting directly: dashboard, hivectl, an `/claim` // comment, or their own IDE/CLI session running under their login. KindHuman Kind = "human" - // KindAgent is one of this hive's own agents, claimed by the hub when the - // scheduler kicks it with the issue in IssueRefs. + // KindAgent is one of this hive's own agents, claimed by the hub on the + // agent's first start signal on an issue a kick listed to it (#10527). KindAgent Kind = "agent" // KindContributor is a relay contributor ("clanker") the hub dispatched // the issue to over the contribute WebSocket. @@ -163,6 +163,35 @@ func (s Stall) Since() time.Time { return s.Attempts[0].ClaimedAt } +// Listing is a kick naming an issue to one of the hive's agents (#10527). +// +// A kick lists up to governor.kick_limits.max_issues issues, and the agent +// starts on at most a few of them, so a listing is not a claim: it posts no +// GitHub comment and is not an Attempt. It holds the issue back from other +// agents and relay contributors for DefaultListedTTL, and lets the agent's +// first start signal on the issue (a comment, a label or claim request, a +// pull-request request) turn into a real claim for as long as the agent's +// claim TTL after the kick. +type Listing struct { + Repo string `json:"repo"` + Issue int `json:"issue"` + Holder string `json:"holder"` + // ListedAt is when the kick was delivered. + ListedAt time.Time `json:"listed_at"` + // ExpiresAt is when the hold on other workers lapses. + ExpiresAt time.Time `json:"expires_at"` +} + +// Key is the canonical "owner/repo#N" key of the listed issue. +func (li Listing) Key() string { return Key(li.Repo, li.Issue) } + +// Expired reports whether the listing's hold has lapsed at now. +func (li Listing) Expired(now time.Time) bool { return !now.Before(li.ExpiresAt) } + +// DefaultListedTTL is how long a kick listing holds an issue back from other +// workers. Short on purpose: it covers the kick being live, not the work. +const DefaultListedTTL = 30 * time.Minute + // Request is one attempt to claim an issue. type Request struct { Repo string @@ -310,11 +339,13 @@ type Ledger struct { // escalated maps an issue key to the Since of the last stall escalated // on it, so one run is escalated once however often it is observed. escalated map[string]time.Time - path string - policy Policy - hooks Hooks - now func() time.Time - hive string + // listed holds the kick listings (#10527) by issue key. + listed map[string]Listing + path string + policy Policy + hooks Hooks + now func() time.Time + hive string } // ErrInvalid is returned for requests missing a repo, issue, holder or kind. @@ -331,6 +362,7 @@ func New(path string, policy Policy, hooks Hooks) (*Ledger, error) { claims: map[string]Claim{}, attempts: map[string][]Attempt{}, escalated: map[string]time.Time{}, + listed: map[string]Listing{}, path: path, policy: policy, hooks: hooks, @@ -408,6 +440,9 @@ func (l *Ledger) Claim(req Request) (Result, error) { p := prev res = Result{Outcome: OutcomeRefused, Claim: prev, Previous: &p} } + if res.Outcome.Changed() { + delete(l.listed, key) + } var saveErr error if res.Outcome.Changed() || len(expired) > 0 { saveErr = l.saveLocked() @@ -592,6 +627,12 @@ func (l *Ledger) HeldKeys(exceptHolderID string) map[string]bool { } out[k] = true } + for k, li := range l.listed { + if li.Expired(now) || (exceptHolderID != "" && li.Holder == exceptHolderID) { + continue + } + out[k] = true + } return out } @@ -709,6 +750,105 @@ func (l *Ledger) pruneLocked(now time.Time) { delete(l.escalated, k) } } + window := l.listingWindow() + for k, li := range l.listed { + if li.Expired(now) && !now.Before(li.ListedAt.Add(window)) { + delete(l.listed, k) + } + } +} + +// ---- kick listings (#10527) ---- + +// listingWindow is how long after a kick the listed agent's first start +// signal still becomes a claim: the agent claim TTL, the hold a kick used to +// take on every issue it named. +func (l *Ledger) listingWindow() time.Duration { + return l.policy.ttlFor(KindAgent, 0) +} + +// MarkListed records that a delivered kick named the issue to the agent +// holder. It posts nothing and counts as no claim. An issue under a live claim +// or a live listing for another agent is left alone and reports false. +func (l *Ledger) MarkListed(repo string, issue int, holder string) (Listing, bool) { + repo, holder = strings.TrimSpace(repo), strings.TrimSpace(holder) + if l == nil || repo == "" || issue <= 0 || holder == "" { + return Listing{}, false + } + l.mu.Lock() + now := l.now() + expired := l.expireLocked(now) + key := Key(repo, issue) + _, held := l.claims[key] + prev, listed := l.listed[key] + var li Listing + ok := !held && (!listed || prev.Expired(now) || prev.Holder == holder) + if ok { + li = Listing{Repo: repo, Issue: issue, Holder: holder, ListedAt: now, ExpiresAt: now.Add(DefaultListedTTL)} + l.listed[key] = li + } + if ok || len(expired) > 0 { + _ = l.saveLocked() + } + hooks := l.hooks + l.mu.Unlock() + fireExpired(hooks, expired) + return li, ok +} + +// Listed returns the live kick listing on an issue, if any. +func (l *Ledger) Listed(repo string, issue int) (Listing, bool) { + if l == nil { + return Listing{}, false + } + l.mu.Lock() + defer l.mu.Unlock() + li, ok := l.listed[Key(strings.TrimSpace(repo), issue)] + if !ok || li.Expired(l.now()) { + return Listing{}, false + } + return li, true +} + +// ClaimOnStart turns the agent's first start signal on an issue into a real +// agent claim — comment, label and Attempt included — when a kick listed the +// issue to that agent within the listing window, or renews the agent's own +// live claim. Any other start signal (a comment on an issue nobody kicked the +// agent with, a request about its own pull request) reports false and +// changes nothing. +func (l *Ledger) ClaimOnStart(repo string, issue int, agent string) (Result, bool, error) { + repo, agent = strings.TrimSpace(repo), strings.TrimSpace(agent) + if l == nil || repo == "" || issue <= 0 || agent == "" { + return Result{}, false, nil + } + l.mu.Lock() + now := l.now() + key := Key(repo, issue) + c, held := l.claims[key] + li, listed := l.listed[key] + ok := (held && !c.Expired(now) && c.Kind == KindAgent && c.Holder == agent) || + (listed && li.Holder == agent && now.Before(li.ListedAt.Add(l.listingWindow()))) + l.mu.Unlock() + if !ok { + return Result{}, false, nil + } + res, err := l.Claim(Request{Repo: repo, Issue: issue, Holder: agent, HolderID: agent, Kind: KindAgent}) + return res, true, err +} + +// LastAttempt returns the most recent agent claim recorded on the issue, by +// any agent, within AttemptRetention. +func (l *Ledger) LastAttempt(repo string, issue int) (Attempt, bool) { + if l == nil { + return Attempt{}, false + } + l.mu.Lock() + defer l.mu.Unlock() + list := l.attempts[Key(strings.TrimSpace(repo), issue)] + if len(list) == 0 { + return Attempt{}, false + } + return list[len(list)-1], true } // Stall reports the most recent run of threshold claims one agent made on the @@ -786,6 +926,8 @@ type persisted struct { // absent from files written before it, which load as no history. Attempts map[string][]Attempt `json:"attempts,omitempty"` Escalated map[string]time.Time `json:"escalated,omitempty"` + // Listed carries the kick listings (#10527). + Listed []Listing `json:"listed,omitempty"` } const persistVersion = 1 @@ -821,6 +963,11 @@ func (l *Ledger) load() error { for k, since := range p.Escalated { l.escalated[k] = since } + for _, li := range p.Listed { + if li.Repo != "" && li.Issue > 0 && li.Holder != "" { + l.listed[li.Key()] = li + } + } return nil } @@ -842,6 +989,10 @@ func (l *Ledger) saveLocked() error { if len(l.escalated) > 0 { p.Escalated = l.escalated } + for _, li := range l.listed { + p.Listed = append(p.Listed, li) + } + sort.Slice(p.Listed, func(i, j int) bool { return p.Listed[i].Key() < p.Listed[j].Key() }) data, err := json.MarshalIndent(p, "", " ") if err != nil { return err diff --git a/src/pkg/claims/listing_test.go b/src/pkg/claims/listing_test.go new file mode 100644 index 0000000000..c52aab48af --- /dev/null +++ b/src/pkg/claims/listing_test.go @@ -0,0 +1,149 @@ +package claims + +import ( + "path/filepath" + "testing" + "time" +) + +func TestMarkListedHoldsWithoutClaiming(t *testing.T) { + var claimed int + l, now := newTestLedger(t, Hooks{OnClaimed: func(Claim, Outcome) { claimed++ }}) + li, ok := l.MarkListed("o/r", 5, "scanner") + if !ok || li.Holder != "scanner" || !li.ExpiresAt.Equal(now.Add(DefaultListedTTL)) { + t.Fatalf("MarkListed = %+v, %v", li, ok) + } + if claimed != 0 { + t.Fatal("a kick listing fired OnClaimed (the 🔒 comment)") + } + if _, held := l.Lookup("o/r", 5); held { + t.Fatal("a kick listing created a claim") + } + if _, ok := l.LastAttempt("o/r", 5); ok { + t.Fatal("a kick listing counted as a claim attempt") + } + if keys := l.HeldKeys("relay#s"); !keys["o/r#5"] { + t.Fatalf("listing not excluded for a relay contributor: %v", keys) + } + if keys := l.HeldKeys("scanner"); keys["o/r#5"] { + t.Fatal("listing excluded for the agent it was listed to") + } + if _, ok := l.MarkListed("o/r", 5, "quality"); ok { + t.Fatal("a second agent's kick displaced a live listing") + } + + *now = now.Add(DefaultListedTTL) + if _, ok := l.Listed("o/r", 5); ok { + t.Fatal("listing still holds after DefaultListedTTL") + } + if keys := l.HeldKeys(""); keys["o/r#5"] { + t.Fatal("lapsed listing still excluded") + } + if li, ok := l.MarkListed("o/r", 5, "quality"); !ok || li.Holder != "quality" { + t.Fatalf("lapsed listing not replaced: %+v %v", li, ok) + } +} + +func TestMarkListedLeavesClaimedIssuesAlone(t *testing.T) { + l, _ := newTestLedger(t, Hooks{}) + if _, err := l.Claim(Request{Repo: "o/r", Issue: 5, Holder: "alice", Kind: KindHuman}); err != nil { + t.Fatal(err) + } + if _, ok := l.MarkListed("o/r", 5, "scanner"); ok { + t.Fatal("listed an issue a person claims") + } + if _, ok := l.MarkListed("", 5, "scanner"); ok { + t.Fatal("listed without a repo") + } + var nilLedger *Ledger + if _, ok := nilLedger.MarkListed("o/r", 5, "scanner"); ok { + t.Fatal("nil ledger listed") + } +} + +func TestClaimOnStartClaimsOnlyListedIssues(t *testing.T) { + var outcomes []Outcome + l, now := newTestLedger(t, Hooks{OnClaimed: func(_ Claim, o Outcome) { outcomes = append(outcomes, o) }}) + if _, started, _ := l.ClaimOnStart("o/r", 5, "scanner"); started { + t.Fatal("start signal on an issue no kick listed recorded a claim") + } + l.MarkListed("o/r", 5, "scanner") + if _, started, _ := l.ClaimOnStart("o/r", 5, "quality"); started { + t.Fatal("another agent's start signal claimed a listing") + } + + // The start signal can come after the listing's short hold lapsed, as + // long as it is within the agent claim TTL of the kick. + *now = now.Add(DefaultListedTTL + time.Minute) + res, started, err := l.ClaimOnStart("o/r", 5, "scanner") + if err != nil || !started || res.Outcome != OutcomeClaimed || res.Claim.Kind != KindAgent { + t.Fatalf("first start signal: res=%+v started=%v err=%v", res, started, err) + } + if _, ok := l.Listed("o/r", 5); ok { + t.Fatal("listing left behind once the claim was recorded") + } + a, ok := l.LastAttempt("o/r", 5) + if !ok || a.Holder != "scanner" || !a.ClaimedAt.Equal(*now) { + t.Fatalf("LastAttempt = %+v, %v", a, ok) + } + + // A second signal renews: no second 🔒 comment, no second attempt. + *now = now.Add(time.Minute) + if res, started, _ := l.ClaimOnStart("o/r", 5, "scanner"); !started || res.Outcome != OutcomeRenewed { + t.Fatalf("second start signal: %+v started=%v", res, started) + } + if len(outcomes) != 2 || outcomes[0] != OutcomeClaimed || outcomes[1] != OutcomeRenewed { + t.Fatalf("OnClaimed outcomes = %v, want [claimed renewed]", outcomes) + } + if s, _ := l.Stall("o/r", 5, 1); len(s.Attempts) > 0 { + t.Fatalf("live claim reported as a stall: %+v", s) + } +} + +func TestClaimOnStartAfterTheListingWindowRecordsNothing(t *testing.T) { + l, now := newTestLedger(t, Hooks{}) + l.MarkListed("o/r", 5, "scanner") + *now = now.Add(DefaultAgentTTL) + if _, started, _ := l.ClaimOnStart("o/r", 5, "scanner"); started { + t.Fatal("start signal past the agent claim TTL of the kick recorded a claim") + } + // Pruned once the window has passed. + l.Expire() + if _, ok := l.MarkListed("o/r", 5, "quality"); !ok { + t.Fatal("stale listing blocked a new one") + } +} + +func TestListedIssuesNeverStall(t *testing.T) { + l, now := newTestLedger(t, Hooks{}) + for range 5 { + l.MarkListed("o/r", 5, "scanner") + *now = now.Add(DefaultAgentTTL + time.Minute) + } + if s, ok := l.Stall("o/r", 5, 2); ok { + t.Fatalf("issues only listed, never started, reported as a stall: %+v", s) + } +} + +func TestListingsPersist(t *testing.T) { + path := filepath.Join(t.TempDir(), "claims.json") + now := time.Date(2026, 10, 5, 12, 0, 0, 0, time.UTC) + l, err := New(path, DefaultPolicy(), Hooks{}) + if err != nil { + t.Fatal(err) + } + l.SetNow(func() time.Time { return now }) + l.MarkListed("o/r", 5, "scanner") + + reloaded, err := New(path, DefaultPolicy(), Hooks{}) + if err != nil { + t.Fatal(err) + } + reloaded.SetNow(func() time.Time { return now.Add(time.Minute) }) + if li, ok := reloaded.Listed("o/r", 5); !ok || li.Holder != "scanner" { + t.Fatalf("listing lost across restart: %+v %v", li, ok) + } + if res, started, err := reloaded.ClaimOnStart("o/r", 5, "scanner"); err != nil || !started || res.Outcome != OutcomeClaimed { + t.Fatalf("start after restart: %+v started=%v err=%v", res, started, err) + } +} diff --git a/src/pkg/github/agent_start_hook.go b/src/pkg/github/agent_start_hook.go new file mode 100644 index 0000000000..a7894291b5 --- /dev/null +++ b/src/pkg/github/agent_start_hook.go @@ -0,0 +1,63 @@ +package github + +// Agent start signals (hivecommons/hive#10527). +// +// A governor kick lists many issues and the agent starts on few of them, so +// the hive no longer claims an issue for being listed. It claims it when the +// agent's own relay request first shows it working the issue: a comment, +// label or claim request on the issue (hive-open-issue), or a pull-request +// request naming it (hive-open-pr --issues). The relays below report those +// requests through AgentStartHook once the request is authorized, so the +// agent name is the verified requester, not text the agent wrote. + +// Start signals reported through AgentStartHook. +const ( + AgentStartSignalComment = "comment" + AgentStartSignalLabel = "label" + AgentStartSignalClaim = "claim" + AgentStartSignalPRRequest = "pr_request" +) + +// AgentStartHook is told that agent's own request shows it working issue +// number in repo. repo is spelled as the request spelled it (bare or +// owner/repo). +type AgentStartHook func(agent, repo string, number int, signal string) + +// SetAgentStartHook installs (or with nil, removes) the start-signal hook. +// Safe to call before or after the watchers start. +func (c *Client) SetAgentStartHook(fn AgentStartHook) { + if c == nil { + return + } + if fn == nil { + c.agentStartHook.Store(nil) + return + } + c.agentStartHook.Store(&fn) +} + +// notifyAgentStart reports a start signal to the installed hook, if any. +func (c *Client) notifyAgentStart(agent, repo string, number int, signal string) { + if c == nil || agent == "" || repo == "" || number <= 0 { + return + } + if hook := c.agentStartHook.Load(); hook != nil && *hook != nil { + (*hook)(agent, repo, number, signal) + } +} + +// issueRequestStartSignal maps a successful issue-request kind to the start +// signal it is, or "" for kinds that are not one: creating an issue, closing +// one, and asking for a review do not show the agent working an existing +// issue. +func issueRequestStartSignal(kind string) string { + switch kind { + case "comment": + return AgentStartSignalComment + case "label": + return AgentStartSignalLabel + case "claim": + return AgentStartSignalClaim + } + return "" +} diff --git a/src/pkg/github/agent_start_hook_test.go b/src/pkg/github/agent_start_hook_test.go new file mode 100644 index 0000000000..3a239bcde0 --- /dev/null +++ b/src/pkg/github/agent_start_hook_test.go @@ -0,0 +1,116 @@ +package github + +import ( + "context" + "testing" +) + +// startCall captures one AgentStartHook invocation. +type startCall struct { + agent string + repo string + number int + signal string +} + +func recordStarts(c *Client) *[]startCall { + var calls []startCall + c.SetAgentStartHook(func(agent, repo string, number int, signal string) { + calls = append(calls, startCall{agent: agent, repo: repo, number: number, signal: signal}) + }) + return &calls +} + +// #10527: an agent's comment through the issue relay is its start signal on +// that issue. +func TestAgentStartHook_FiresOnRelayComment(t *testing.T) { + commented := 0 + srv := newIssueMockServer(t, "", nil, nil, &commented) + defer srv.Close() + c := issueTestClient(t, srv.URL) + calls := recordStarts(c) + dir := withIssueDir(t) + + if _, err := WriteIssueRequest(dir, IssueRequest{Kind: "comment", Repo: "o/r", Number: 41, Body: "verified on v5", Agent: "scanner"}); err != nil { + t.Fatal(err) + } + if _, err := WriteIssueRequest(dir, IssueRequest{Repo: "o/r", Title: "new finding", Agent: "scanner"}); err != nil { + t.Fatal(err) + } + c.ProcessIssueRequestsOnce(context.Background()) + + if commented != 1 { + t.Fatalf("expected 1 comment, got %d", commented) + } + want := startCall{agent: "scanner", repo: "o/r", number: 41, signal: AgentStartSignalComment} + if len(*calls) != 1 || (*calls)[0] != want { + t.Fatalf("start signals = %+v, want only %+v (creating an issue is not one)", *calls, want) + } +} + +// A request the relay refuses never reports a start. +func TestAgentStartHook_SilentOnDeniedRequest(t *testing.T) { + srv := newIssueMockServer(t, "", nil, nil, nil) + defer srv.Close() + c := testClient(t, srv.URL) + calls := recordStarts(c) + dir := withIssueDir(t) + if _, err := WriteIssueRequest(dir, IssueRequest{Kind: "comment", Repo: "o/r", Number: 41, Body: "b", Agent: "scanner"}); err != nil { + t.Fatal(err) + } + c.ProcessIssueRequestsOnce(context.Background()) + if len(*calls) != 0 { + t.Fatalf("denied request reported start signals: %+v", *calls) + } +} + +// A hive-open-pr request naming issues is a start signal on each of them. +func TestAgentStartHook_FiresOnPRRequestIssues(t *testing.T) { + created := 0 + srv := newPRMockServer(t, "", &created) + defer srv.Close() + c := testClient(t, srv.URL) + calls := recordStarts(c) + + dir := t.TempDir() + old := prRequestDirForTest + prRequestDirForTest = dir + defer func() { prRequestDirForTest = old }() + + if _, err := WritePRRequest(dir, PRRequest{Repo: "o/r", Head: "scanner/fix-1", Title: "🐛 fix: thing", Body: "Fixes #1\nRefs #2", Agent: "scanner", IssueN: []int{1, 2}}); err != nil { + t.Fatal(err) + } + c.ProcessPRRequestsOnce(context.Background()) + + if len(*calls) != 2 || (*calls)[0].number != 1 || (*calls)[1].number != 2 || (*calls)[0].signal != AgentStartSignalPRRequest { + t.Fatalf("start signals = %+v, want #1 and #2 as pr_request", *calls) + } +} + +func TestAgentStartHook_SetAndClear(t *testing.T) { + var nilClient *Client + nilClient.SetAgentStartHook(func(string, string, int, string) {}) + nilClient.notifyAgentStart("a", "o/r", 1, AgentStartSignalComment) + + c := &Client{} + calls := recordStarts(c) + c.notifyAgentStart("a", "o/r", 0, AgentStartSignalComment) + c.notifyAgentStart("", "o/r", 1, AgentStartSignalComment) + c.notifyAgentStart("a", "o/r", 1, AgentStartSignalLabel) + c.SetAgentStartHook(nil) + c.notifyAgentStart("a", "o/r", 2, AgentStartSignalLabel) + if len(*calls) != 1 || (*calls)[0].number != 1 { + t.Fatalf("calls = %+v, want one for #1", *calls) + } +} + +func TestIssueRequestStartSignal(t *testing.T) { + for kind, want := range map[string]string{ + "comment": AgentStartSignalComment, "label": AgentStartSignalLabel, "claim": AgentStartSignalClaim, + "issue": "", "close": "", "request_review": "", + } { + if got := issueRequestStartSignal(kind); got != want { + t.Errorf("issueRequestStartSignal(%q) = %q, want %q", kind, got, want) + } + } +} diff --git a/src/pkg/github/client.go b/src/pkg/github/client.go index f544c14708..2a6dc85e6c 100644 --- a/src/pkg/github/client.go +++ b/src/pkg/github/client.go @@ -195,6 +195,9 @@ type Client struct { prOpenedDetailHook atomic.Pointer[PROpenedDetailHook] prTerminalObservedHook atomic.Pointer[PRTerminalObservedHook] prRepoPolicyGate atomic.Pointer[PRRepoPolicyGate] + // agentStartHook is told when an agent's own relay request shows it + // working an issue (#10527). See agent_start_hook.go. + agentStartHook atomic.Pointer[AgentStartHook] // mergeAuthz gates merge requests from the merge-request watcher against the // per-agent ACMM merge-policy (CanMerge) + forge-resistance AND the merge // TARGET (pinned SHA + governor merge-eligible membership; see diff --git a/src/pkg/github/issue_request_watcher.go b/src/pkg/github/issue_request_watcher.go index 51efbc00ba..96a2a3af66 100644 --- a/src/pkg/github/issue_request_watcher.go +++ b/src/pkg/github/issue_request_watcher.go @@ -574,6 +574,10 @@ func (c *Client) handleOneIssueRequest(ctx context.Context, path string, nowFn f return } + if signal := issueRequestStartSignal(kind); signal != "" { + c.notifyAgentStart(req.Agent, req.Repo, req.Number, signal) + } + action := AuditActionAgentIssueCreated switch kind { case "comment": diff --git a/src/pkg/github/issue_timeline.go b/src/pkg/github/issue_timeline.go index 209a3ec75c..5f3b1815d9 100644 --- a/src/pkg/github/issue_timeline.go +++ b/src/pkg/github/issue_timeline.go @@ -2,13 +2,15 @@ package github import ( "context" + "strings" "time" gh "github.com/google/go-github/v72/github" ) // IssueEvent is one entry of an issue's timeline, reduced to the fields the -// claim escalation gate (#10527) reads to decide whether anything moved. +// claim escalation and verify-once gates (#10527) read to decide whether +// anything moved. type IssueEvent struct { // Event is GitHub's timeline event name: "labeled", "cross-referenced", // "referenced", "assigned", "closed", ... @@ -22,6 +24,13 @@ type IssueEvent struct { SourcePR int // CommitID is the referencing commit on a referenced event. CommitID string + // Actor is the login that caused the event (the commenter on a + // "commented" event); empty when GitHub names none. + Actor string + // ActorIsBot reports that Actor is a bot or GitHub App account, so the + // verify-once gate (#10527) can tell the hive's own comments from a + // reporter or maintainer acting. + ActorIsBot bool } // IssueTimelineSince returns the issue's timeline events that happened after @@ -62,5 +71,13 @@ func issueEventFrom(t *gh.Timeline, since time.Time) (IssueEvent, bool) { if src := t.GetSource(); src != nil && src.Issue != nil && src.Issue.IsPullRequest() { ev.SourcePR = src.Issue.GetNumber() } + actor := t.GetActor() + if actor == nil { + actor = t.GetUser() + } + if actor != nil { + ev.Actor = actor.GetLogin() + ev.ActorIsBot = strings.EqualFold(actor.GetType(), "Bot") || strings.HasSuffix(strings.ToLower(ev.Actor), "[bot]") + } return ev, true } diff --git a/src/pkg/github/issue_timeline_test.go b/src/pkg/github/issue_timeline_test.go index ecc524e91f..025d39badd 100644 --- a/src/pkg/github/issue_timeline_test.go +++ b/src/pkg/github/issue_timeline_test.go @@ -27,6 +27,12 @@ func TestIssueEventFrom(t *testing.T) { true, IssueEvent{Event: "unlabeled", At: since.Add(time.Hour), Label: "needs-human"}}, {"commit reference", &gh.Timeline{Event: gh.Ptr("referenced"), CreatedAt: at(time.Second), CommitID: gh.Ptr("abc123")}, true, IssueEvent{Event: "referenced", At: since.Add(time.Second), CommitID: "abc123"}}, + {"human comment", &gh.Timeline{Event: gh.Ptr("commented"), CreatedAt: at(time.Minute), Actor: &gh.User{Login: gh.Ptr("clubanderson"), Type: gh.Ptr("User")}}, + true, IssueEvent{Event: "commented", At: since.Add(time.Minute), Actor: "clubanderson"}}, + {"app comment names its user", &gh.Timeline{Event: gh.Ptr("commented"), CreatedAt: at(time.Minute), User: &gh.User{Login: gh.Ptr("hivecommons-hive[bot]")}}, + true, IssueEvent{Event: "commented", At: since.Add(time.Minute), Actor: "hivecommons-hive[bot]", ActorIsBot: true}}, + {"bot-typed actor", &gh.Timeline{Event: gh.Ptr("labeled"), CreatedAt: at(time.Minute), Label: &gh.Label{Name: gh.Ptr("bug")}, Actor: &gh.User{Login: gh.Ptr("hivecommons-hive"), Type: gh.Ptr("Bot")}}, + true, IssueEvent{Event: "labeled", At: since.Add(time.Minute), Label: "bug", Actor: "hivecommons-hive", ActorIsBot: true}}, {"at since is not after it", &gh.Timeline{Event: gh.Ptr("labeled"), CreatedAt: at(0), Label: &gh.Label{Name: gh.Ptr("bug")}}, false, IssueEvent{}}, {"before since", &gh.Timeline{Event: gh.Ptr("assigned"), CreatedAt: at(-time.Minute)}, false, IssueEvent{}}, {"no timestamp", &gh.Timeline{Event: gh.Ptr("committed")}, false, IssueEvent{}}, diff --git a/src/pkg/github/pr_request_watcher.go b/src/pkg/github/pr_request_watcher.go index 81b31e160e..ba25d37e8a 100644 --- a/src/pkg/github/pr_request_watcher.go +++ b/src/pkg/github/pr_request_watcher.go @@ -414,6 +414,11 @@ func (c *Client) handleOnePRRequest(ctx context.Context, path string, nowFn func c.rejectPRRequest(path, req, "body", reason, nowFn) return } + // #10527: an authorized PR request naming an issue is the agent's start + // signal on it, whether or not the claim gates below let the PR open. + for _, n := range req.IssueN { + c.notifyAgentStart(req.Agent, req.Repo, n, AgentStartSignalPRRequest) + } // Validate claims while the request is still at the server-side choke point. // Agents cannot bypass this by invoking a different CLI: direct POST /pulls From 0e4bcb33f53dc53df58e59d2619a902acc4945dd Mon Sep 17 00:00:00 2001 From: scanner Date: Mon, 5 Oct 2026 20:36:04 -0400 Subject: [PATCH 3/3] docs: shift scale-envelope client.go citation for the 3-line Client struct addition Signed-off-by: scanner --- src/docs/scale-envelope.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/docs/scale-envelope.md b/src/docs/scale-envelope.md index 419f4567c7..d59bb060e6 100644 --- a/src/docs/scale-envelope.md +++ b/src/docs/scale-envelope.md @@ -34,7 +34,7 @@ ones to: 2. **Truncation must preserve the prioritization signal, not cut an arbitrary slice.** This holds today and is easy to assume backwards: - Issues are sorted **oldest first** before any cap is applied - (`src/pkg/github/client.go:3241-3249`, descending `AgeMinutes`). + (`src/pkg/github/client.go:3244-3252`, descending `AgeMinutes`). - PRs are sorted by **review class then oldest-first within class** (`SortPullRequestsForReview`, `src/pkg/github/review_priority.go:181`; fixes → refactors/docs → tests).