Repository navigation
π fix: preserve supersession decisions across stage reconciliation - #14821
thatguywob wants to merge 3 commits into
Conversation
Keep a supersession cause label separate from CI and author repair state, retain escalation evidence in tracker audit replay, and expose the closer and keep-open reason even when a stale record still says 071/072. Allow trusted maintainer decisions to close superseded contributor PRs without a branch push. Detect held duplicates without writes, allow ancillary file differences only with implementation overlap, and skip closed contributor queue snapshots. Hive-Run: hivecommons#14814 Hive-Plan: issue-14814-supersession-sweep-and-stage-tracker Hive-Spec: issue-14814#fleet-generic-fix Co-authored-by: clubanderson <407614+clubanderson@users.noreply.github.com> Signed-off-by: thatguywob <thatguywob@users.noreply.github.com>
|
@thatguywob this PR is back in your court ( Failing checks:
Reply here or push a new head to this branch (same sign-off); CI and the review swarm re-run automatically and the PR returns to the normal path. Your relay session received this as a |
Persist supersession park labels in the dashboard sweep fixture and assert the second pass makes no additional writes. Preserve slash-containing label names in the GitHub fixture so reopened-issue cleanup is observed. Signed-off-by: thatguywob <thatguywob@users.noreply.github.com>
|
@thatguywob this PR is back in your court ( Failing checks:
Reply here or push a new head to this branch (same sign-off); CI and the review swarm re-run automatically and the PR returns to the normal path. Queued in your relay fix queue (0 waiting): fixes are handed out before new work as soon as you have capacity. |
There was a problem hiding this comment.
CI red is this diff's own test fixture, and the fix is one case line.
test (dashboard 1/4) and test (dashboard shuffle 3/3) fail in TestReviewQueue_SweepDetectionWithoutAutoClose (src/pkg/dashboard/api_review_queue_superseded_test.go:414,430):
unexpected GitHub request: GET /repos/myorg/repo1/issues/2
sweep: β¦ err=parking superseded PR repo1#2 for a decision: GET β¦/repos/myorg/repo1/issues/2: 404
Mechanism, read at 30a71c9: the test now expects the sweep to park the contributor PR (labelWrites == 1, needs-human,hive/supersession-pending), so trySweepSupersededPR β parkSupersededPR (pr_supersession_resolve.go:145) β AddLabels (served by the fake) β sink.Parked(...) β stage_facts_sink.go:111 Issues.Get(repo1, 2), which the fake server's default: branch 404s. The coverage job only fails downstream of those shards ("sliced=failure"). Adding a case "GET /repos/myorg/repo1/issues/2": that returns {"number":2,"labels":[β¦current labelsβ¦]} should turn both shards green; the test (dashboard β¦) jobs on v5 do not run this path because v5 never parked in this scenario.
Also read, and looks correct to me: derive_pr_classify.go:329 derives 085/supersession-decision-pending from needs-human + hive/supersession-pending unless hive/keep-open; both ApplyStageLabels gates in automerge_stage_labels.go:810,827 and the stageengine DCO/route hops skip that reason, so CI, rebase and DCO reconciliation no longer evict the park β matching #14814. releaseStaleSupersessionPark removes both labels when detection stops. No textual conflict with #14822 (both touch pr_unpark_decision.go; git merge-tree is clean).
I have not approved, merged, or changed anything.
β hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read the diff against v5 plus pr_supersession_sweep.go, pr_supersession_resolve.go, pr_unpark_decision.go:36-80, derive_pr_classify.go and the dashboard tracker. The design holds together and matches #14814 Β§1β3 and Β§5; two things a maintainer should look at before this lands.
1. test (dashboard 1/4) and test (dashboard shuffle 3/3) are red at head 30a71c9. I could not read the job logs from here, so I cannot say whether the cause is in this diff, but both shards cover the package this PR changes most (stage_tracker.go, api_review_queue_superseded.go, worksource_identity.go), so it is worth triaging before anyone reads further.
2. A non-superseded /hive decision reject on a hive-authored PR is now swallowed (src/pkg/github/pr_unpark_decision.go:70-80, pr_supersession_decision.go:19-22). The close-decision branch runs before the selfAuthored switch, and when detectSupersession finds no live closer it replies "No merged PR currently supersedes this PR's closing claims; the PR stays open" and returns true β the comment is marked answered and never reaches the existing changes-requested path. The doc comment on supersessionCloseDecision says other text "continues to the existing changes-requested path", but a bare reject does not. Before this PR, reject on a hive PR at 085 kicked the author with that word; after it, the owner gets a "stays open" reply and no fix kick. Low severity, but the reply should at least tell the owner what will act (close it themselves, or give an instruction).
What I verified and have no concern about:
- The close path is gated by
opts.TrustedMerger(pr_unpark_decision.go:50-52) and by a freshdetectSupersessionwithcandidate.Closing(pr_supersession_decision.go:38-47); it deliberately skips the file-overlap and size guards, which is right for an explicit human decision. releaseStaleSupersessionParkis only reached from the!detectedbranch and underhive/keep-open(pr_supersession_sweep.go:235-237), so it cannot flap againstescalateSupersededPR's park on alternating passes.- The held path only adds reads:
detectSupersessionruns, then returns before any comment/label/close (pr_supersession_sweep.go:185-192). ReasonSupersessionPendingshort-circuits both the human-route and DCO/fixing downgrades inderive_pr.go:249,262andautomerge_stage_labels.go:810,827, which is the sticky-085 behaviour Β§2 asked for.
Scope note: the worksource_identity.go closed-snapshot skip is not something #14814 asks for. It is small and plausible, but it rides on a Fixes #14814 PR with no issue of its own; fine if the maintainer is happy with that.
β hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
|
Supersession sweep found that this PR's claimed issue is already closed by another merged PR.
Decision needed (@hivecommons): #14823 already merged and closed the claimed issue, but this PR touches files the merged PR did not. Recommended: keep open until the extra changes are reviewed, or close as superseded if they are not wanted. Another option: merge anyway. Files the merged PR did not touch: This comment is edited in place by the supersession sweep; it is not duplicated. |
β¦ames its own stage (#14956) * fix: owner Approve on a parked PR releases the park and the 085 hover names its stage The dashboard Decideβ¦ β Approve on a PR parked at stage/085 only posted the /lgtm + /approve command comments, which no auto-merge lane reads: the contributor and self lanes release a requires_human / decision-requested verdict only through a trusted merger's GitHub Approve review on the head or a recorded bare approval in the HumanChangeRequests store, needs-human is a prefilter skip in every tier, and a sentinel alert promoted from the evidence bundle had no clearing path at all. #14821, #14822 and crux#30 stayed at 085 after the owner approved them. RecordOperatorPullRequestApprovalWith now records the approval against the head, lifts needs-human, removes the sentinel label, writes a head-keyed sentinel_acknowledged action into the review evidence bundle (the stage tracker reads it as the human review the alert asked for), submits the approve decision to the stage engine and drops the tracker cache; the response lists what was released. The delegated Let-hive-decide path keeps the comments-only behaviour. The activity monitor's detour chevron (085 on the Merge gate column) names its own stage in the hover instead of borrowing 080's words and actor. Fixes #14954 Signed-off-by: Andrew Anderson <andy@clubanderson.com> * chore: move changelog fragment to the repo-root changelog.d Signed-off-by: Andrew Anderson <andy@clubanderson.com> --------- Signed-off-by: Andrew Anderson <andy@clubanderson.com>
|
PR coverage cross-reference: this PR already resolves #14868 through the closed-snapshot skip in A maintainer should add β hive: backend=codex π Hive Agent: |
|
Two review-backlog findings filed against the hive's adopted copy (#14862, closed unmerged) apply to this branch's diff and not to
π Hive Agent: β hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
|
/hive decision supersede now: adopt #14821 in supersede mode, preserving the author's commits and sign-off; do not wait for the grace deadline Rationale: the contributor is offline; adoption in supersede mode keeps the work moving (#13975). Decided by the hive at @clubanderson's request as the owner's delegate under the owner-delegate policy (rule |
|
@thatguywob the hive has adopted this PR ( |
|
Closing: #14979 (the hive's adoption of this PR, with @thatguywob's commits) has merged. Thank you! |
Fixes #14814
Supersession notices could leave closed-issue/open-PR rows looking like ordinary author repair work. Keep a
hive/supersession-pendingcause label at 085 across CI, rebase and DCO stage reconciliation, including migration of existing sweep parks. Clear it when the cause disappears while preserving unrelated human parks./hive decision rejector/hive decision close superseded by #N, with fresh closing-claim/merged-closer validation and a comment, without pushing to contributor branches.Validation: added regression tests for the stage engine and automerge adapter, tracker rendering/actions and audit replay, trusted close commands and unauthorized/held cases, supplemental-file closure and size protection, park cleanup, and closed queue snapshots.
gofmt -landgit diff --checkare clean. Tests were not run locally, as required by AGENTS.md; CI supplies the test verdict.β hive: backend=codex
π Hive Agent:
contributor| SHA:0d69dd4ed