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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/fixed-14814-supersession-stage.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Keep superseded contributor PRs at the decision stage despite CI or rebase failures, show the closing PR and keep-open reason in the stage tracker, and let trusted maintainers close them with `/hive decision reject` or `/hive decision close superseded by #N`. Supplemental tests, docs and changelog files no longer block duplicate recommendations when implementation files overlap; held PRs remain visible without sweep writes, and closed issue snapshots cannot be offered as contributor work.
4 changes: 2 additions & 2 deletions src/cmd/hive/main.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package main

import (
Expand Down Expand Up @@ -9530,8 +9530,8 @@
if dashSrv == nil {
return
}
detail := fmt.Sprintf("repo=%s, pr=%d, issue=%s#%d, closer_pr=%d, action=%s",
event.Repo, event.Number, event.IssueRepo, event.Issue, event.CloserPR, event.Action)
detail := fmt.Sprintf("repo=%s, pr=%d, issue=%s#%d, closer_pr=%d, action=%s, reason=%s",
event.Repo, event.Number, event.IssueRepo, event.Issue, event.CloserPR, event.Action, event.Reason)
dashSrv.AuditLogRecord("system", github.SupersessionSweepAuditPrefix+event.Action, detail, "", event.Repo, event.Number)
},
})
Expand Down
14 changes: 14 additions & 0 deletions src/docs/maintainer-commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,20 @@ specific to that queue flow. A swarm `changes_requested` or `reject` verdict
is not overridden by a human approval; the head must be fixed or merged
manually.

For a PR whose closing claims were superseded by another merged PR, a trusted
maintainer with repository write access can reply `/hive decision reject` or
`/hive decision close superseded by #N`. Hive rechecks that the claimed issues
are closed and a different merged PR closed them; an explicit `#N` must match
that closer. It comments and closes the PR with `hive/superseded`, including
PRs opened by contributors, without pushing to their branches. Hold and
`do-not-merge` gates still prevent the close. When nothing supersedes the PR,
a bare `reject` keeps its ordinary meaning (changes requested, nudging the
author), while an explicit `close superseded by #N` is answered with why the
PR stays open. The sweep's
`hive/supersession-pending` cause label keeps the unresolved decision at 085
through CI or rebase failures; reopening the issue, rewriting the claim, or
choosing `hive/keep-open` clears the sweep's park on its next pass.

On a **hive-authored** PR (`author/hive`) parked at `stage/085-needs-decision`
the same trusted human has a second lever
([#13041](https://github.com/hivecommons/hive/issues/13041)): a GitHub
Expand Down
8 changes: 6 additions & 2 deletions src/pkg/dashboard/api_review_queue_superseded.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func supersededGraceStates(entries []AuditEntry, org string, grace time.Duration
}
key := ghpkg.ReviewQueueKey(repo, e.Target)
switch e.Action {
case supersessionDetectedAuditAction, supersessionGraceAuditAction:
case supersessionDetectedAuditAction, supersessionGraceAuditAction, ghpkg.SupersessionSweepAuditPrefix + ghpkg.SupersessionActionEscalated:
case supersessionCommentedAuditAction:
if detected[key] {
info := out[key]
Expand All @@ -104,6 +104,7 @@ func supersededGraceStates(entries []AuditEntry, org string, grace time.Duration
}
info := ghpkg.ReviewQueueSuperseded{
Reason: ghpkg.ReviewQueueSupersededReason,
KeepReason: fields["reason"],
Issue: fields["issue"],
CloserRepo: repo,
CloserPR: closerPR,
Expand All @@ -117,7 +118,10 @@ func supersededGraceStates(entries []AuditEntry, org string, grace time.Duration
if previous, ok := out[key]; ok && previous.CloserRepo == info.CloserRepo && previous.CloserPR == info.CloserPR && previous.Issue == info.Issue {
info.GraceStartedAt, info.GraceEndsAt = previous.GraceStartedAt, previous.GraceEndsAt
}
if e.Action == supersessionDetectedAuditAction {
if e.Action != supersessionGraceAuditAction {
if e.Action == ghpkg.SupersessionSweepAuditPrefix+ghpkg.SupersessionActionEscalated {
info.GraceStartedAt, info.GraceEndsAt = time.Time{}, time.Time{}
}
detected[key] = true
} else if info.GraceStartedAt.IsZero() {
info.GraceStartedAt, info.GraceEndsAt = at.UTC(), at.Add(grace).UTC()
Expand Down
34 changes: 33 additions & 1 deletion src/pkg/dashboard/api_review_queue_superseded_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (

"github.com/hivecommons/hive/pkg/config"
ghpkg "github.com/hivecommons/hive/pkg/github"
"github.com/hivecommons/hive/pkg/stageengine/prstage"
"github.com/hivecommons/hive/pkg/stageengine/stageenginetest"
)

// useRingSupersessionAudit points the queue's on-disk audit scan at an empty
Expand Down Expand Up @@ -325,6 +327,8 @@ func TestSupersededDetectionReplay(t *testing.T) {
start time.Time
}{
{"detection only", []AuditEntry{detection}, true, time.Time{}},
{"escalation retains decision", []AuditEntry{detection, entry("supersession-sweep-commented-needs-human", now)}, true, time.Time{}},
{"escalation cancels grace", []AuditEntry{detection, grace, entry("supersession-sweep-commented-needs-human", now)}, true, time.Time{}},
{"plain notice", []AuditEntry{detection, notice, repeat}, true, time.Time{}},
{"grace starts at notice", []AuditEntry{detection, grace}, true, now.Add(time.Hour)},
{"repeated detection preserves grace", []AuditEntry{detection, grace, repeat, entry(supersessionGraceAuditAction, now.Add(4*time.Hour))}, true, now.Add(time.Hour)},
Expand Down Expand Up @@ -368,11 +372,17 @@ func TestReviewQueue_SweepDetectionWithoutAutoClose(t *testing.T) {
useRingSupersessionAudit(t)
s := reviewQueueTestServer(t, filepath.Join(t.TempDir(), "missing.json"))
var notice string
var labels []string
var labelWrites int
var writes int
api := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.Method + " " + r.URL.Path {
case "GET /repos/myorg/repo1/pulls":
fmt.Fprint(w, `[{"number":2,"body":"Fixes #3","user":{"login":"alice"}}]`)
wireLabels := make([]map[string]string, 0, len(labels))
for _, label := range labels {
wireLabels = append(wireLabels, map[string]string{"name": label})
}
json.NewEncoder(w).Encode([]map[string]any{{"number": 2, "body": "Fixes #3", "user": map[string]string{"login": "alice"}, "labels": wireLabels}})
case "GET /repos/myorg/repo1/issues/3":
fmt.Fprint(w, `{"number":3,"state":"closed"}`)
case "POST /graphql":
Expand All @@ -385,6 +395,12 @@ func TestReviewQueue_SweepDetectionWithoutAutoClose(t *testing.T) {
} else {
json.NewEncoder(w).Encode([]map[string]any{{"id": 1, "body": notice}})
}
case "POST /repos/myorg/repo1/issues/2/labels":
if err := json.NewDecoder(r.Body).Decode(&labels); err != nil {
t.Error(err)
}
labelWrites++
fmt.Fprint(w, `[]`)
case "POST /repos/myorg/repo1/issues/2/comments":
var in struct {
Body string `json:"body"`
Expand All @@ -403,6 +419,10 @@ func TestReviewQueue_SweepDetectionWithoutAutoClose(t *testing.T) {
defer api.Close()
client := ghpkg.NewClient("token", "myorg", []string{"repo1"}, slog.Default(), api.URL)
client.SetAppBotLogin("hive-app[bot]")
// The park goes through the stage engine; record it instead of letting the
// default sink re-read the PR and reconcile against the shared label store.
facts := &stageenginetest.Recorder{}
client.SetStageFactSink(facts)
for pass := 0; pass < 2; pass++ {
res, err := client.SweepSupersededOpenPRs(context.Background(), ghpkg.SupersessionSweepOptions{
ACMMLevelForRepo: func(string) int { return 6 },
Expand Down Expand Up @@ -440,4 +460,16 @@ func TestReviewQueue_SweepDetectionWithoutAutoClose(t *testing.T) {
if writes != 1 {
t.Fatalf("notice writes = %d, want 1", writes)
}
if labelWrites != 1 || strings.Join(labels, ",") != "needs-human,"+prstage.SupersessionPendingLabel {
t.Fatalf("park labels = %v, writes = %d; want one supersession park", labels, labelWrites)
}
var parked int
for _, f := range facts.All() {
if f.Method == "Parked" && f.Ref.Number == 2 && f.Label == "needs-human" {
parked++
}
}
if parked != 1 {
t.Fatalf("Parked facts for PR 2 = %d, want 1: %+v", parked, facts.All())
}
}
34 changes: 28 additions & 6 deletions src/pkg/dashboard/stage_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,13 @@ type StageTrackerHop struct {

// StageTrackerItem is one open issue or PR on the tracker.
type StageTrackerItem struct {
Repo string `json:"repo"`
Number int `json:"number"`
Kind string `json:"kind"`
Title string `json:"title"`
URL string `json:"url,omitempty"`
Author string `json:"author,omitempty"`
Superseded *github.ReviewQueueSuperseded `json:"superseded,omitempty"`
Repo string `json:"repo"`
Number int `json:"number"`
Kind string `json:"kind"`
Title string `json:"title"`
URL string `json:"url,omitempty"`
Author string `json:"author,omitempty"`
// Head is the PR head SHA the lane last evaluated ("" for issues), so
// a "waiting for a verdict" caption can name the revision it waits on.
Head string `json:"head,omitempty"`
Expand Down Expand Up @@ -1304,6 +1305,15 @@ func (s *Server) buildStageTracker(status *StatusPayload, store *stagelabels.Sto
return resp
}
resp.HiveID = status.HiveID
qualifyRepo := stageTrackerRepoQualifier(status)
superseded := map[string]github.ReviewQueueSuperseded{}
entries := append([]AuditEntry(nil), s.supersessionAuditEntries(now)...)
for i := range entries {
entries[i].Repo = qualifyRepo(entries[i].Repo)
}
for key, info := range supersededGraceStates(entries, "", s.supersessionGracePeriod()) {
superseded[key] = info
}
var cfg *config.Config
if s.deps != nil {
cfg = s.deps.Config
Expand Down Expand Up @@ -1496,6 +1506,18 @@ func (s *Server) buildStageTracker(status *StatusPayload, store *stagelabels.Sto
it.DeferredModifiers = append([]string{modSentinel}, it.DeferredModifiers...)
}
it.LinkedIssues = stageTrackerPRLinkedIssues(pr, repoName, status.GitHubBaseURL)
info, found := superseded[github.ReviewQueueKey(qualifyRepo(repoName), pr.Number)]
if !found {
info, found = superseded[github.ReviewQueueKey(repoName, pr.Number)]
}
if found && !labelSet(pr.Labels)[github.SupersessionKeepOpenLabel] {
it.Superseded = &info
it.Caption = fmt.Sprintf("Superseded by #%d — decide", info.CloserPR)
if info.KeepReason != "" {
it.Caption += ": " + info.KeepReason
}
it.Actor = "maintainer"
}
add(it)
}
for _, repo := range status.Repos {
Expand Down
62 changes: 62 additions & 0 deletions src/pkg/dashboard/stage_tracker_supersession_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package dashboard

import (
"strings"
"testing"
"time"

"github.com/hivecommons/hive/pkg/github"
"github.com/hivecommons/hive/pkg/stagelabels"
"github.com/hivecommons/hive/pkg/worksource"
)

func TestTrackerSupersessionSurvivesEscalationAndStaleRepairStage(t *testing.T) {
useRingSupersessionAudit(t)
s := newTestServer()
now := time.Now().UTC()
status := minimalPayload()
status.Repos = []FrontendRepo{{Name: "hive", Full: "hivecommons/hive", OpenPrs: []any{
FrontendPR{PullRequest: github.PullRequest{Number: 14795, Title: "duplicate", Author: "contributor", Labels: []string{"stage/072-returned-to-contributor", "sentinel-alert"}, LinkedIssues: []github.PRLinkedIssue{{Number: 14789, State: "closed"}}}},
}}}
detail := "repo=hive, pr=14795, issue=hivecommons/hive#14789, closer_pr=14791, reason=extra implementation files"
s.audit.LogRecordAt(now.Add(-time.Minute).Format(time.RFC3339), "system", supersessionDetectedAuditAction, detail, "", "hive", 14795)
s.audit.LogRecordAt(now.Format(time.RFC3339), "system", github.SupersessionSweepAuditPrefix+github.SupersessionActionEscalated, detail, "", "hive", 14795)
store := stagelabels.NewStore("", nil)
store.Observe(stagelabels.Key("hivecommons/hive", 14795), stagelabels.ReturnedToContributor, "check-failing:unit")
resp := s.buildStageTracker(status, store, now)
if len(resp.Items) != 1 {
t.Fatalf("items=%+v", resp.Items)
}
it := resp.Items[0]
if it.Superseded == nil || it.Superseded.CloserPR != 14791 || !strings.Contains(it.Caption, "Superseded by #14791 — decide") || !strings.Contains(it.Caption, "extra implementation files") {
t.Fatalf("supersession lost to repair stage: %+v", it)
}
if it.Actor != "maintainer" {
t.Fatal(it.Actor)
}
s.audit.LogRecord("owner", supersessionKeepOpenAction, "", "", "hive", 14795)
if it := s.buildStageTracker(status, store, now).Items[0]; it.Superseded != nil {
t.Fatalf("kept-open still needs decision: %+v", it)
}
}

func TestClosedSnapshotIsNeverActionable(t *testing.T) {
raw := []any{map[string]any{"number": 1, "state": "closed"}, map[string]any{"number": 2, "state": "open"}}
var offered []string
forEachActionableIssue(nil, "test", "o/r", raw, func(_ map[string]any, ref worksource.Ref) { offered = append(offered, ref.Key()) })
if len(offered) != 1 || offered[0] != "o/r#2" {
t.Fatalf("offered=%v", offered)
}
}

func TestSupersededRepairRowHasDecisionActionAndHeadline(t *testing.T) {
html := indexHTML(t)
script := stageTrackerRowActionsPreamble(t, html) + "\n" + jsFunc(t, html, "stageTrackerHeadline") + "\n" + jsFunc(t, html, "stageTrackerCurrentStageHtml") + `
const item = {kind:'pr',repo:'o/r',number:12,stage:72,superseded:{closer_pr:22}};
assert.strictEqual(stageTrackerHeadline(item), 'Superseded by #22 — decide');
assert.ok(stageTrackerCurrentStageHtml(item, [], null, 0).includes('Superseded by #22'));
assert.strictEqual(stageTrackerDecisionTarget(item), item);
assert.ok(stageTrackerRowActions(item, {canHold:true}).some(a => a.id === 'decide'));
`
runStageTrackerRowActionsScript(t, script)
}
4 changes: 3 additions & 1 deletion src/pkg/dashboard/static/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -13927,6 +13927,7 @@ <h3 class="faq-group-title">Where to get help</h3>
var _stageTrackerLabelQuery = '';

function stageTrackerHeadline(item) {
if (item.superseded && item.superseded.closer_pr) return 'Superseded by #' + item.superseded.closer_pr + ' — decide';
const n = Number(item.stage) || 0;
const mods = Array.isArray(item.modifiers) ? item.modifiers : [];
const holdLabels = stageTrackerHoldLabels(item);
Expand Down Expand Up @@ -14190,6 +14191,7 @@ <h3 class="faq-group-title">Where to get help</h3>
// Compact strips retain their column alignment; the current step gets a
// full-width caption so detours and the next actor never need a hover.
function stageTrackerCurrentStageHtml(item, stages, range, nowMs) {
if (item.superseded && item.superseded.closer_pr) return `<p class="ham-current-stage">${esc(stageTrackerHeadline(item) + (item.superseded.keep_reason ? " · " + item.superseded.keep_reason : ""))}</p>`;
const active = stageTrackerSegments(item, stages, nowMs).find(sg => sg.state === 'active'
&& (!range || range.from == null || sg.number >= range.from)
&& (!range || range.to == null || sg.number <= range.to));
Expand Down Expand Up @@ -15828,7 +15830,7 @@ <h2 class="config-title" id="activity-monitor-labels-title">All labels</h2>
function stageTrackerDecisionTarget(item) {
if (!item) return null;
const mods = Array.isArray(item.modifiers) ? item.modifiers : [];
const parked = it => (Array.isArray(it.modifiers) ? it.modifiers : []).indexOf('needs-decision') >= 0 || STAGE_TRACKER_DECISION_STAGES.indexOf(Number(it.stage) || 0) >= 0;
const parked = it => !!it.superseded || (Array.isArray(it.modifiers) ? it.modifiers : []).indexOf('needs-decision') >= 0 || STAGE_TRACKER_DECISION_STAGES.indexOf(Number(it.stage) || 0) >= 0;
if (item.kind === 'pr') return parked(item) ? item : null;
for (const lp of (Array.isArray(item.linked_prs) ? item.linked_prs : [])) {
const pr = { repo: item.repo, number: lp.number, kind: 'pr', title: lp.title || '', url: lp.url || '', stage: lp.stage || 0, modifiers: lp.modifiers || [], deferred_modifiers: lp.deferred_modifiers || [], author: lp.author || '',
Expand Down
4 changes: 4 additions & 0 deletions src/pkg/dashboard/worksource_identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package dashboard
import (
"encoding/json"
"log/slog"
"strings"

ghpkg "github.com/hivecommons/hive/pkg/github"
"github.com/hivecommons/hive/pkg/worksource"
Expand Down Expand Up @@ -104,6 +105,9 @@ func forEachActionableIssue(logger *slog.Logger, logTag string, repoFull string,
}
continue
}
if state, _ := issue["state"].(string); strings.EqualFold(state, "closed") {
continue
}
ref := refFromIssueMap(repoFull, issue)
if ref.Key() == "" {
if logger != nil {
Expand Down
4 changes: 2 additions & 2 deletions src/pkg/github/automerge/automerge_stage_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,7 @@ func (c *Engine) ApplyStageLabels(ctx context.Context) (StageLabelResult, error)
obs.Reason = replacement.Reason
}
var route stagelabels.Route
if verdict.Human && !verdict.Terminal {
if verdict.Human && !verdict.Terminal && verdict.Reason != prstage.ReasonSupersessionPending {
var evidence stagelabels.Evidence
route, evidence = c.routeHuman(ctx, tracker, key, obs, verdict.Stage, now)
pass.Route = &evidence
Expand All @@ -824,7 +824,7 @@ func (c *Engine) ApplyStageLabels(ctx context.Context) (StageLabelResult, error)
// the label state, which is reset below for informational no-dco and
// is unknown for heads with nothing to sign.
dcoRead := false
if !verdict.Terminal && verdict.Stage != stagelabels.None {
if !verdict.Terminal && verdict.Stage != stagelabels.None && verdict.Reason != prstage.ReasonSupersessionPending {
// The commit check is the one author-side gate decided here,
// before CI or a verdict: an unsigned head goes straight back
// to its author (hivecommons/hive#12542).
Expand Down
37 changes: 37 additions & 0 deletions src/pkg/github/automerge/automerge_stage_supersession_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package automerge

import (
"context"
"slices"
"testing"
"time"

"github.com/hivecommons/hive/pkg/stageengine/prstage"
"github.com/hivecommons/hive/pkg/stagelabels"
)

func TestApplyStageLabelsKeepsSupersessionParkOverConflictAndUnsignedHead(t *testing.T) {
api, closeAPI := seededStageAPI(t)
defer closeAPI()
clock := &contributorLaneClock{at: time.Date(2026, 10, 28, 0, 0, 0, 0, time.UTC)}
e, _, store := newStageEngine(t, apiURLOf(t, api), clock)
api.commits = map[int][]string{14795: {"unsigned commit"}}
pr := nudgePR(14795, "head", "needs-human", prstage.SupersessionPendingLabel, "needs-rebase")
e.recordStage(stageLaneTrusted, "acme/widget", "acme", "widget", pr, "check-failing:unit", false, nil)
e.recordStage(stageLaneContributor, "acme/widget", "acme", "widget", pr, "conflicting", false, nil)
if _, err := e.ApplyStageLabels(context.Background()); err != nil {
t.Fatal(err)
}
rec, ok := store.Get(stageKey("acme", "widget", 14795))
if !ok || rec.Stage != stagelabels.NeedsDecision || rec.Reason != prstage.ReasonSupersessionPending {
t.Fatalf("record=%+v", rec)
}
for _, added := range api.added {
if slices.Contains(added, stagelabels.ReturnedToContributor.Label()) || slices.Contains(added, stagelabels.NeedsAuthor.Label()) {
t.Fatalf("sent to author: %v", api.added)
}
}
if api.commitReads != 0 {
t.Fatalf("superseded head read for repair: %d", api.commitReads)
}
}
Loading
Loading