Repository navigation
✨ Activity Monitor: Adopt now and Reroute to another clanker for PRs waiting on a contributor's clanker - #15085
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
There was a problem hiding this comment.
Correctness review (head bd9e232)
1. Pickup fallback fires after the target finished the work (medium). reroutePickupLapsed (src/pkg/dashboard/api_repo_item_decision_reroute.go:604-620) treats "not in flight AND not queued AND head unchanged" as a lapse. But ReroutePickedUp (contribute_reroute.go:207-222) only scans InFlight, and a finished task_complete removes the dispatch via CompleteDispatch (contribute_fixqueue.go:504; fixqueue.go:338-355). The reroute prompt tells the target to open a replacement PR when it cannot push (contribute_fixqueue.go fixPRRerouteInstruction), which leaves the original head unchanged — so a target that does exactly what it was told within 30m gets a lapsed=true note, a forced adoption and a second hive copy. TestRepoItemReroutePickupLapseFallsBackToAdoptNow only covers the never-dispatched case. "Neither queued nor in flight" after a dispatch means completed or Resolved (a refusal goes back to the queue via Requeue), so it should end the reroute, not fall back:
if hub != nil && !hub.RerouteQueued(to, repoFull, number) {
// Neither queued nor in flight: the relay completed the task
// (fixed in place or opened a replacement PR) or the PR left 072.
// Either way there is nothing to fall back from.
store.ClearReroute(key, false, now)
return false
}
(replacing the head-comparison block at :616-623; the head check stays useful only as an extra reason to clear.)
2. Fallback timer and queued item are in-memory only (medium). armReroutePickup is called solely from the decision handler (api_repo_item_decision_reroute.go:523) with time.AfterFunc; the fix queue is also process-local. After a dashboard restart the record keeps RerouteTo/RerouteAt (persisted, store.go), the row keeps the rerouted → @X pill, but nothing re-arms the window and the item is gone from the target's queue — so the #15082 guarantee "falls back to Adopt now with an alert" is silently lost. Suggest re-arming on startup (or in the stage-tracker sweep) for every record with RerouteTo != "", with max(0, RerouteAt+timeout-now).
3. Engine still nudges/adopts around the reroute (low). The PR body says the author's hive "must not race the target", but Reroute cancels the author's queue only once (contribute_reroute.go:176). nudgeContributor (automerge_contributor_nudge.go:258-287) checks only nudgeDue and re-enqueues the PR on the author's queue at the renudge interval; adoptionDue likewise ignores rec.Rerouted. Nothing in src/pkg/github/automerge reads the reroute fields. Suggest skipping the nudge (and the grace-based adoption) while rec.Rerouted(obs.HeadSHA) != "".
— hive: agent=adjudicator backend=copilot model=claude-fable-5.1 copilot=1.0.88
4d0c845 to
00b94d9
Compare
…waiting on a contributor's clanker A PR at stage 072/071 waiting on its author's hive offered only Hold / Re-evaluate / Mark needs-human / Open / Copy. Two operator actions end the wait: - Adopt now — hive takes it over: decision `adopt` + `force: true` marks the stage record forced, posts an attributed adoption-requested note, sets adoption/supersede and nudges the sweep, which runs the existing supersede adoption at once, bypassing presence, grace and maintainer_policy (hold still stops it). Marker reads `reason=operator-forced by=@login`; an online author is told the maintainer took it over. Audit `repo_item_decision_adopt_now` (`action=adopt-now`). Toast "Adopted → #N (hive's copy), contributor PR will close when #N merges". - Reroute to another clanker: decision `reroute` + `to: <login>|any` enqueues the PR's follow-up work on the target's contributor fix queue and dispatches it as a fix-pr task_assign interrupt (the 072 nudge path), posts `hive:rerouted to=@x by=@op reason=…`, shows a "rerouted → @x" pill with presence, and falls back to Adopt now when the target has not picked the task up within governor.reroute.pickup_timeout (default 30m), with a system alert and audit entry. Disabled with "no clankers online — use Adopt now" when nothing is online. Both appear in the Needs-human popover options once the nudge grace has elapsed, and "Let hive decide" picks Adopt now (grace-elapsed-adopt-now) then. Go tests for the handlers, the fallback, the tracker payload and the engine bypass; node fixture tests for the menu, consequence line, disabled reasons, confirms and popover. Fixes #15082 Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Signed-off-by: Andrew Anderson <andy@clubanderson.com>
8ab9932 to
d217f4a
Compare
|
@clubanderson this PR is held at Failing checks:
Next: the hive does not push to your branch. Push a new head (or reply here); CI and the review swarm re-run automatically and the PR returns to the normal path. |
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
Fixes #15082
What
A PR waiting on its author's clanker (stage 072
returned-to-contributor/ 071needs-author, e.g. #15038 "Returned to contributor · waiting on @Danathar") offered only Hold / Re-evaluate / Mark needs-human / Open / Copy. The ⋯ row menu (and the per-PR sub-row menu under an issue, #15056) now carries two operator actions, both throughrunRowActionandPOST /api/repos/{o}/{r}/items/{n}/decision:Adopt now — hive takes it over
hive.tracker.confirm.decide:adopt-now); the confirm adds a line when the author's hive is online right now.{"decision":"adopt","force":true}→handleRepoItemAdoptNow: stage recordNoteAdoptionForced(by), attributed<!-- hive:adoption-requested by=<login> force=true -->note,adoption/supersedelabel,SweepNudgeso the automerge sweep runs the existing supersede adoption path (adoptContributorPR/supersedeAdoptedPR, PR 15016/15011) at once. The forced mark bypasses presence, both grace windows andmaintainer_policy(incl.never);holdstill blocks. Marker:hive:adopted … reason=operator-forced by=@login; when the author's hive is online the adoption comment says the maintainer took it over.operatorActionFromBodymapsadopt+force→adopt-now(andadopt→adopt,reroute→reroutewith target); auditrepo_item_decision_adopt_nowcarriesaction=adopt-now.Adopted → #N (hive's copy), contributor PR will close when #N merges.when the sweep lands within the handler's wait, elseAdoption forced: hive opens its copy on the pass now running.Reroute to another clanker
@login · hive · online 12m ago · 1 in hand|idle) plus any available (N online), fromtakeover.clankers[]— the same hub facts as/api/contribute/fleet. Disabled with no clankers online — use Adopt now when none.{"decision":"reroute","to":"<login>|any"}→handleRepoItemReroute→ContributeWSHub.Reroute: reuses the contributor fix queue —fixQueue().Enqueue(target, item)→dispatchFixes→sendFixTask(task_assignkindfix-pr, interrupt priority) →awaitFixTaskRefusal. This is exactly the mechanism the stage-072 nudge uses to offer rework to the author's own relay (ContributeWSHub.Nudge, hive#12436). The item carriesRerouteFrom/RerouteBy/RerouteReasonso the task instruction tells the target whose PR it is and that they either push the fix (if the branch allows) or open a replacement PR carrying the author's commits + sign-off. The PR's return reason (failing check / reviewer's requested changes) plus the operator's comment is the reason shipped.<!-- hive:rerouted to=@X by=@op reason=… -->; row shows arerouted → @Xpill with a presence dot; toastRerouted → @X (dispatched now, preempt)/(queued; N ahead).governor.reroute.pickup_timeout(default 30m): if the target's relay has not taken the task (not in flight) the hive falls back to Adopt now by itself — withdraws the queued item, marks the forced adoption, posts alapsed=truenote, raises a warning system alert +repo_item_reroute_fallbackaudit entry, nudges the sweep.Needs-human popover + Let hive decide
decision_needed(blocker_reason: adoption:<why>, options Adopt now / Reroute / Hold) so both appear indecision_needed.options[](✨ Activity Monitor: needs-human popover says what to decide and offers the actions #14995); Reroute there is a required choice list.grace-elapsed-adopt-now: "Let hive decide" picks Adopt now when the grace already elapsed, and the dashboard runs it as a forced adoption.Tests
api_repo_item_takeover_test.go(force adopt → comment/label/audit/toast, queued form, guards; reroute 409 when none online, fix-pr dispatch + marker + store, pickup-lapse fallback, candidate filtering; trackertakeover{}+ disabled reasons + reroute pill; store marks; delegated rule),automerge_adopt_now_test.go(forced adoption bypasses presence / grace /maintainer_policy: never, wording online vs offline,holdstill stops it), operator-actions ledger mapping.stage_tracker_takeover_ui_test.go— menu items, consequence line, disabled reasons, confirms, reroute popover options, toasts.governor.rerouteblock.Docs
docs/dashboard-sections.md(row actions),docs/labels-and-control-signals.md(operator-forcedandhive:reroutedmarkers),docs/operator-reference.md,changelog.d/added-adopt-now-reroute.md.SPA:
dashboardSetIntervalonly (none added),[hidden]not inline styles, CSS tokens only.