diff --git a/changelog.d/fixed-hive-merge-evicts-tracker-row.md b/changelog.d/fixed-hive-merge-evicts-tracker-row.md new file mode 100644 index 0000000000..32b53b70a3 --- /dev/null +++ b/changelog.d/fixed-hive-merge-evicts-tracker-row.md @@ -0,0 +1 @@ +- Fixed the stage tracker and the governor's cached actionable snapshot keeping a pull request the hive had merged or closed itself listed as open until GitHub's `closed` webhook came back round through the hub, the closed-row sweep ran on the next status publish, or the next full enumeration replaced the snapshot. On a hosted hive in webhook cadence hivecommons/docs#469 was merged by the automerge sweep at 18:25 and still read "Sentinel alert - needs human" an hour later, with the governor re-evaluating it as merge-eligible seven more times from the cached snapshot while the hub relay was dropping deliveries. Every hive merge path records a `pr_merged` / `pr_closed` audit event; that record now applies the same closed snapshot a webhook delivery would, so the row leaves both stores the moment the hive performs the write, with no GitHub call and no dependence on the webhook round-trip (hivecommons/hive#14420). diff --git a/src/cmd/hive/github_client_config.go b/src/cmd/hive/github_client_config.go index 70bec3a4d1..8676cd2b19 100644 --- a/src/cmd/hive/github_client_config.go +++ b/src/cmd/hive/github_client_config.go @@ -331,6 +331,10 @@ func (b *boot) applyGitHubClientDashboardHooks(client *github.Client) { // #10927/#10933: the agent card's "what is it on" and stale warning // read each agent's last hive-mediated write. dashboard.RecordAgentLastAction(rec, time.Now()) + // A PR the hive merged or closed itself leaves the tracker and the + // governor's cached snapshot now, instead of waiting for GitHub's + // `closed` delivery to come back round through the hub. + evictHiveClosedPR(b.dashSrv, rec, time.Now()) }) client.SetPRTerminalObservedHook(func(obs github.PRTerminalObservation) { ts := obs.ObservedAt.UTC().Format(time.RFC3339) @@ -429,3 +433,20 @@ func holdMigrationFailedAlertMessage(hiveID, reportPath string) string { } return msg } + +// evictHiveClosedPR removes a pull request from the tracker and the cached +// actionable snapshot the moment the hive records having merged or closed it +// (AuditActionPRMerged / AuditActionPRClosed). The audit record is the one +// point every merge path (sweep, queue, relay, forge) passes through, so it +// is where the hive's own write stops depending on the webhook round-trip. +func evictHiveClosedPR(srv *dashboard.Server, rec github.AuditRecord, now time.Time) { + if srv == nil || rec.Target <= 0 || rec.Repo == "" { + return + } + switch rec.Action { + case github.AuditActionPRMerged: + srv.EvictHiveClosedItem(rec.Repo, rec.Target, true, true, rec.Action, now) + case github.AuditActionPRClosed: + srv.EvictHiveClosedItem(rec.Repo, rec.Target, true, false, rec.Action, now) + } +} diff --git a/src/cmd/hive/github_client_config_evict_test.go b/src/cmd/hive/github_client_config_evict_test.go new file mode 100644 index 0000000000..bfa315e150 --- /dev/null +++ b/src/cmd/hive/github_client_config_evict_test.go @@ -0,0 +1,78 @@ +package main + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/hivecommons/hive/pkg/dashboard" + "github.com/hivecommons/hive/pkg/github" +) + +// The attribution audit sink is the one point every hive merge path passes +// through; a pr_merged or pr_closed record there must take the PR off the +// dashboard's listed items, and anything else must leave them alone. +func TestEvictHiveClosedPRFromAuditRecord(t *testing.T) { + now := time.Date(2026, 10, 26, 18, 25, 46, 0, time.UTC) + newSrv := func() *dashboard.Server { + srv := dashboard.NewServer(0, restoreTestLogger()) + srv.UpdateStatus(&dashboard.StatusPayload{Repos: []dashboard.FrontendRepo{{ + Name: "docs", Full: "hivecommons/docs", PRs: 2, + OpenPrs: []any{ + github.PullRequest{Repo: "hivecommons/docs", Number: 469, State: "open", UpdatedAt: now.Add(-time.Hour)}, + github.PullRequest{Repo: "hivecommons/docs", Number: 470, State: "open", UpdatedAt: now.Add(-time.Hour)}, + }, + }}}) + return srv + } + listed := func(srv *dashboard.Server) []int { + req := httptest.NewRequest(http.MethodGet, "/api/status", nil) + rec := httptest.NewRecorder() + srv.Handler().ServeHTTP(rec, req) + var out struct { + Repos []struct { + OpenPrs []struct{ Number int } `json:"openPrs"` + } `json:"repos"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("status body is not JSON: %v\n%s", err, rec.Body.String()) + } + var nums []int + for _, r := range out.Repos { + for _, pr := range r.OpenPrs { + nums = append(nums, pr.Number) + } + } + return nums + } + + cases := []struct { + name string + rec github.AuditRecord + want []int + }{ + {"pr_merged evicts", github.AuditRecord{Action: github.AuditActionPRMerged, Repo: "hivecommons/docs", Target: 469}, []int{470}}, + {"pr_closed evicts", github.AuditRecord{Action: github.AuditActionPRClosed, Repo: "hivecommons/docs", Target: 470}, []int{469}}, + {"other action ignored", github.AuditRecord{Action: github.AuditActionAgentPRCreated, Repo: "hivecommons/docs", Target: 469}, []int{469, 470}}, + {"no target ignored", github.AuditRecord{Action: github.AuditActionPRMerged, Repo: "hivecommons/docs"}, []int{469, 470}}, + {"no repo ignored", github.AuditRecord{Action: github.AuditActionPRMerged, Target: 469}, []int{469, 470}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + srv := newSrv() + evictHiveClosedPR(srv, tc.rec, now) + got := listed(srv) + if len(got) != len(tc.want) { + t.Fatalf("listed PRs = %v, want %v", got, tc.want) + } + for i := range got { + if got[i] != tc.want[i] { + t.Fatalf("listed PRs = %v, want %v", got, tc.want) + } + } + }) + } + evictHiveClosedPR(nil, github.AuditRecord{Action: github.AuditActionPRMerged, Repo: "hivecommons/docs", Target: 469}, now) +} diff --git a/src/pkg/dashboard/hive_write_evict.go b/src/pkg/dashboard/hive_write_evict.go new file mode 100644 index 0000000000..709e5368ea --- /dev/null +++ b/src/pkg/dashboard/hive_write_evict.go @@ -0,0 +1,46 @@ +package dashboard + +import ( + "strings" + "time" + + "github.com/hivecommons/hive/pkg/github" +) + +// hiveWriteDeliveryPrefix marks a tracker eviction that came from the hive's +// own write rather than a GitHub delivery, in the item's patch audit trail. +const hiveWriteDeliveryPrefix = "hive-write:" + +// EvictHiveClosedItem drops an issue or pull request the hive itself has just +// closed or merged from the cached actionable snapshot and the published +// status, through the same patch path a `closed` webhook delivery takes. +// +// A merge the hive performs is the one close it never has to be told about, +// yet before this the row's departure depended entirely on the delivery +// coming back round through the hub (single-shot, lost across hub rollouts +// and spoke stalls), on the closed-row sweep (only on status publish, every +// 15 minutes in webhook cadence) or on the next full enumeration. +// hivecommons/docs#469 was merged by the automerge sweep at 18:25 and still +// read "Sentinel alert - needs human" on the tracker an hour later, with the +// governor re-evaluating it as merge-eligible seven more times from the +// cached snapshot. action is the audit action that recorded the write +// (pr_merged, pr_closed, ...), kept on the patch for audit. It reports +// whether a listed row was removed. +func (s *Server) EvictHiveClosedItem(repo string, number int, isPR, merged bool, action string, at time.Time) bool { + if s == nil || number <= 0 || strings.TrimSpace(repo) == "" { + return false + } + if at.IsZero() { + at = time.Now() + } + snap := github.WebhookItemSnapshot{ + Repo: repo, Number: number, IsPR: isPR, State: "closed", Merged: merged, + UpdatedAt: at, FetchedAt: at, DeliveryID: hiveWriteDeliveryPrefix + action, + } + evicted := s.applyWebhookItemSnapshots([]github.WebhookItemSnapshot{snap}) > 0 + if evicted && s.logger != nil { + s.logger.Info("stage tracker evicted row the hive itself closed", + "repo", repo, "number", number, "pr", isPR, "merged", merged, "action", action) + } + return evicted +} diff --git a/src/pkg/dashboard/hive_write_evict_test.go b/src/pkg/dashboard/hive_write_evict_test.go new file mode 100644 index 0000000000..abf3c512d8 --- /dev/null +++ b/src/pkg/dashboard/hive_write_evict_test.go @@ -0,0 +1,121 @@ +package dashboard + +import ( + "testing" + "time" + + "github.com/hivecommons/hive/pkg/github" +) + +// hivecommons/docs#469 as the hosted hive's status listed it on 2026-10-26: +// a dependabot PR, open on the tracker, merged by the automerge sweep at +// 18:25:46Z and still listed an hour later. +var ( + docs469ListedAt = time.Date(2026, 10, 26, 18, 0, 0, 0, time.UTC) + docs469MergedAt = time.Date(2026, 10, 26, 18, 25, 46, 0, time.UTC) +) + +func docs469Fixture(t *testing.T) *Server { + t.Helper() + s := newTestServer() + status := minimalPayload() + status.HiveID = "hv-test" + status.Repos = []FrontendRepo{{ + Name: "docs", Full: "hivecommons/docs", PRs: 1, + OpenPrs: []any{ + github.PullRequest{Repo: "hivecommons/docs", Number: 469, Title: "build(deps): bump mkdocs", Author: "dependabot[bot]", + State: "open", HeadSHA: "abc123", UpdatedAt: docs469ListedAt}, + }, + }} + s.status = status + return s +} + +func trackerListsPR(t *testing.T, s *Server, repo string, number int) bool { + t.Helper() + s.statusMu.RLock() + cur := s.status + s.statusMu.RUnlock() + for _, it := range s.buildStageTracker(cur, s.stageStore, docs469MergedAt.Add(time.Hour)).Items { + if it.Kind == "pr" && it.Number == number && it.Repo == repo { + return true + } + } + return false +} + +// The hive's own merge must take the row off the tracker and out of the +// governor's cached snapshot at once, without a webhook delivery, a closed +// sweep or a re-enumeration. +func TestHiveMergeEvictsTrackerRowWithoutWebhook(t *testing.T) { + s := docs469Fixture(t) + var patched []github.WebhookItemSnapshot + s.deps = &Dependencies{ApplyWebhookItems: func(snaps []github.WebhookItemSnapshot) { + patched = append(patched, snaps...) + }} + if !trackerListsPR(t, s, "hivecommons/docs", 469) { + t.Fatal("fixture: docs#469 must be listed before the merge") + } + + if !s.EvictHiveClosedItem("hivecommons/docs", 469, true, true, github.AuditActionPRMerged, docs469MergedAt) { + t.Fatal("EvictHiveClosedItem reported no row removed") + } + if trackerListsPR(t, s, "hivecommons/docs", 469) { + t.Fatal("docs#469 still listed on the tracker after the hive merged it") + } + s.statusMu.RLock() + prs := s.status.Repos[0].PRs + s.statusMu.RUnlock() + if prs != 0 { + t.Fatalf("repo PR count = %d after eviction, want 0", prs) + } + if len(patched) != 1 { + t.Fatalf("governor snapshot received %d patches, want 1", len(patched)) + } + got := patched[0] + if got.Repo != "hivecommons/docs" || got.Number != 469 || !got.IsPR || got.Open() || !got.Merged { + t.Fatalf("governor patch = %+v, want closed+merged PR hivecommons/docs#469", got) + } + if got.DeliveryID != hiveWriteDeliveryPrefix+github.AuditActionPRMerged { + t.Fatalf("patch DeliveryID = %q, want hive-write audit marker", got.DeliveryID) + } + if !got.UpdatedAt.Equal(docs469MergedAt) { + t.Fatalf("patch UpdatedAt = %v, want merge instant %v", got.UpdatedAt, docs469MergedAt) + } + + // The eviction is remembered for replay, so a status publish from an + // enumeration that began before the merge cannot bring the row back. + stale := docs469Fixture(t) + stale.statusMu.RLock() + staleStatus := stale.status + stale.statusMu.RUnlock() + s.UpdateStatus(staleStatus) + if trackerListsPR(t, s, "hivecommons/docs", 469) { + t.Fatal("stale publish resurrected docs#469 after the hive merged it") + } +} + +// A second call for the same merge, an unknown repo, or an item the status +// never listed is a no-op rather than an error. +func TestHiveMergeEvictIsIdempotentAndIgnoresUnlisted(t *testing.T) { + s := docs469Fixture(t) + if !s.EvictHiveClosedItem("hivecommons/docs", 469, true, true, github.AuditActionPRMerged, docs469MergedAt) { + t.Fatal("first eviction removed nothing") + } + if s.EvictHiveClosedItem("hivecommons/docs", 469, true, true, github.AuditActionPRMerged, docs469MergedAt) { + t.Fatal("second eviction of the same PR reported a removal") + } + if s.EvictHiveClosedItem("hivecommons/other", 1, true, true, github.AuditActionPRMerged, docs469MergedAt) { + t.Fatal("eviction for an unlisted repo reported a removal") + } + if s.EvictHiveClosedItem("", 469, true, true, github.AuditActionPRMerged, docs469MergedAt) { + t.Fatal("eviction with an empty repo reported a removal") + } + if s.EvictHiveClosedItem("hivecommons/docs", 0, true, true, github.AuditActionPRMerged, docs469MergedAt) { + t.Fatal("eviction with number 0 reported a removal") + } + var nilSrv *Server + if nilSrv.EvictHiveClosedItem("hivecommons/docs", 469, true, true, github.AuditActionPRMerged, docs469MergedAt) { + t.Fatal("nil server reported a removal") + } +}