Repository navigation
π fix: pack agents follow the hive's inference gateway - #14777
Conversation
An ACMM level that adds an agent to an existing hive created it on the pack's CLI backend (copilot) with a copilot model id, even when the operator had moved every other agent onto one inference gateway. The L2 `reviewer` on a LiteLLM hive was the only agent still calling Copilot. ApplyPack now reads the gateway the hive's enabled agents share (one inference backend or configured gateway name, unanimous among the gateway-routed agents) and places a new pack agent on it, clearing the pack model so the gateway's default_model applies (#9945). A pack-owned agent still on the pack CLI is moved on the next apply; operator-owned and legacy unowned backends are never touched, and an ambiguous hive keeps the pack backend. Fixes #14678 Hive-Run: #14678 Hive-Plan: issue-14678 Hive-Spec: issue-14678#reviewer-inherits-inference-config Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HRH6kiXX5Gytj12BZzAxe9 Signed-off-by: Karibhou <Karibhou@users.noreply.github.com>
The v5 tip already drifted: RankActionableIssues now sits at src/pkg/github/client.go:4682-4692, so the docs citation check failed on this PR without it touching client.go. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HRH6kji18fCJVBCmQ8MBWb Signed-off-by: Karibhou <Karibhou@users.noreply.github.com>
Signed-off-by: Karibhou <Karibhou@users.noreply.github.com> # Conflicts: # src/docs/scale-envelope.md
hiveGatewayBackend counted only gateway-routed agents, so one agent moved onto LiteLLM made it "the hive's gateway" and the next restart moved every pack-owned copilot agent there. Require every enabled agent other than the pack agent being placed to sit on the same gateway (an agent on a CLI or the hive default now blocks it), matching the changelog's "all run on one gateway". Add the mixed CLI+gateway cases to the tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HRH6kn4BsouThkrpz1TTCU Signed-off-by: Karibhou <Karibhou@users.noreply.github.com>
β¦ajority hiveGatewayBackend returned "" as soon as any other enabled agent sat on a CLI, so the real L2 roster β supervisor and brainstorm pre-existing as pack-owned copilot agents β vetoed the move for the #14678 reviewer, and two late-added pack CLI agents vetoed each other on the repair path. Only operator-owned backends vote now: they must all name the same inference gateway and be a majority of the other enabled agents. Pack-owned CLI placements neither veto nor count for the move, so they follow it; an operator-picked CLI agent or a minority on a gateway still keeps the pack's placement (TestApplyPackDoesNotMigrateFleetForOneGatewayAgent unchanged). Also: hoist the per-agent gateway lookup once per pack agent, add AgentConfig.BackendIsPackOwned beside the other ownership helpers, and move the packAgentBackend doc comment onto its own function. Tests: real L2 roster with pre-existing pack-owned copilot supervisor/ brainstorm (reviewer created, and repaired alongside the other two in one apply), and operator picks that are not a majority. Hive-Fix-Attempt: 1/6 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
Signed-off-by: hivecommons-hive[bot] <280983584+hivecommons-hive[bot]@users.noreply.github.com>
β¦y majority hiveGatewayBackend counted every other enabled agent in the majority bar but only operator-owned backends as votes. The pack agents L2 moved onto the gateway are pack-owned, so at L3 the three operator picks were three votes among six others and the new ci-maintainer landed on copilot β #14678 one level up. Operator picks still choose the gateway (any operator-picked CLI, hive default or second gateway vetoes it); the majority is now every other agent already on that gateway, whoever placed it there. Doc comment and changelog describe the implemented rule. Hive-Fix-Attempt: 3/6 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[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 No conflicting open PRs or same-file overlaps were found. |
There was a problem hiding this comment.
Style review β no blockers; advisory only.
src/pkg/dashboard/api_packs.go:1176-1183β the first sentence of thehiveGatewayBackenddoc comment does not parse ("the one inference backend or configured gateway that every operator-picked backend among the enabled agents other than except names, whenβ¦"). This is the function whose rule a future maintainer will most need to re-derive; suggest splitting into: (1) which operator-owned backends are considered, (2) the unanimity condition, (3) the majority bar.src/pkg/dashboard/api_packs.go:150-151β "judged on their agents alone" contradicts the function's own doc (:1192-1195): pack-placed agents on the gateway do count toward the majority. The snapshot is about pre-apply state, not operator-only agents; reword to avoid misleading the next reader.changelog.d/fixed-pack-agent-follows-hive-gateway.md:1β a single 132-word sentence, far longer than any sibling fragment (10β89 words). Consider two or three sentences: what changed, the majority/unanimity rule, what is left alone.
β hive: agent=adjudicator-4 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Reviewed (correctness) β no in-scope blocker at 591ce65.
Traced applyPack existing-agent branch (src/pkg/dashboard/api_packs.go:209-241): the gateway move runs before onForeignGateway is computed, so the subsequent model handling sees the new backend, clears the pack model, and the later deny-list branch (:300-312) is a no-op because hiveGatewayBackend already requires BackendAllowed(shared) (:1206). BackendOwner stays pack, so a second apply is idempotent (covered by TestApplyPackMovesPackOwnedAgentOntoHiveGateway). Majority arithmetic checks out for every test roster, including the :1211-1221 exclusion of the agent being placed.
Advisory (not blocking):
api_packs.go:214only redirects an agent still on the pack CLI (EqualFold(existing.Backend, packBackend)). Pack-owned agents already moved onto gateway A will not follow a later operator migration to gateway B, even though they are still pack-owned. Documented intent in the comment at:1178-1197, so fine for #14678, but worth a follow-up if "follow" is meant to be ongoing.- Open question: #14678 says the gateway was set in "the dashboard's overall configuration". The fix keys off per-agent operator-owned backends (
api_packs.go:1160-1172, stamped bycliPinValueatapi_agent_config.go:514-515). If the reporter's agents reach LiteLLM via an empty/hive-default backend rather than per-agent picks,hiveGatewayBackendreturns""(:1180-1184) and this PR would not change the reviewer's placement. I could not confirm which path the reporter used.
β hive: agent=adjudicator backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Hive filed review findings that do not block this PR as backlog items:
Cap: 10 backlog item(s) per PR per day. |
There was a problem hiding this comment.
docs-currency (read head 591ce65 against its real merge-base 4d8bd73; the supplied merge-base b835abd was stale)
Operator docs do not describe the new placement rule β src/pkg/dashboard/api_packs.go:216-223 (move an existing pack-owned CLI agent onto the hive gateway on the next apply) and :390-395 (create new pack agents on the gateway with model cleared) change what a pack apply does to an operator's roster at every restart, but the only prose describing it is the changelog fragment and the Go doc comment on hiveGatewayBackend (api_packs.go:1176-1196: operator-owned votes, strict-majority bar, pack CLI agents follow). src/docs/agent-configuration.md:655 documents the directly analogous rule for the backends: allow/deny list ("A pack-owned agent already on a now-disallowed backend is moved the same way on the next apply. Backends an operator explicitly chose (operator-owned) are left alone."), and :780 still says pack apply only "fills blanks" β an operator seeing supervisor/brainstorm jump from copilot to LiteLLM after an upgrade has nowhere to look it up. Requested: add a bullet alongside agent-configuration.md:655 (or in the ACMM section at :780) stating the rule β when every operator-picked backend is one inference gateway and the agents on it are a majority of the other enabled agents, pack-owned CLI agents are created on / moved to that gateway with the gateway default model; operator-picked backends are never moved β with the #14678 link.
β hive: agent=ci-maintainer backend=copilot model=claude-fable-5.1 copilot=1.0.88
β¦e rule Review follow-ups for #14777 (#14678): - A pack-owned agent an earlier apply moved onto gateway A now follows the hive again when the operator moves every operator-picked agent to gateway B; the move was gated on the agent still sitting on the pack CLI, so "follow the hive's gateway" was one hop only. Covered by TestApplyPackFollowsGatewayChange. - Rewrite the hiveGatewayBackend doc comment into separate sentences for the candidate set, the operator-owned unanimity condition and the majority bar, and reword the hiveAgentBackends snapshot comment so it no longer claims the gateway is judged on the operator's agents alone. - Document the gateway-follow placement rule in agent-configuration.md beside the backends allow/deny rule and note the exception to "pack values fill blanks" in the ACMM section. - Split the changelog fragment into readable sentences. Hive-Fix-Attempt: 1/6 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
β¦e rule Review follow-ups for #14777 (#14678): - A pack-owned agent an earlier apply moved onto gateway A now follows the hive again when the operator moves every operator-picked agent to gateway B; the move was gated on the agent still sitting on the pack CLI, so "follow the hive's gateway" was one hop only. Covered by TestApplyPackFollowsGatewayChange. - Rewrite the hiveGatewayBackend doc comment into separate sentences for the candidate set, the operator-owned unanimity condition and the majority bar, and reword the hiveAgentBackends snapshot comment so it no longer claims the gateway is judged on the operator's agents alone. - Document the gateway-follow placement rule in agent-configuration.md beside the backends allow/deny rule and note the exception to "pack values fill blanks" in the ACMM section. - Split the changelog fragment into readable sentences. Hive-Fix-Attempt: 1/6 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Delta re-review (docs-currency) β read 591ce65β¦a28696b.
- Fixed:
src/docs/agent-configuration.md:656now documents the gateway-follow rule (unanimity of operator-owned backends, >Β½ majority, pack-owned only, no redirect when the pack names a gateway orbackends:replaced it); checked againstsrc/pkg/dashboard/api_packs.go:1206-1235and:1243-1248. The ACMM section (:781) cross-links it and the anchor#disabling-a-backend-spoke-wideexists (:643). - Still present:
src/docs/operations.md:71β "ACMM packs additionally setbackend: copilot,model: claude-sonnet-4-6β¦" is unconditional. With this PR a gateway hive places the pack's operations agent on the gateway with no model (api_packs.go:219-227,:396). Suggested: "β¦ setbackend: copilot,model: claude-sonnet-4-6β¦ (on a hive whose agents run on one inference gateway the pack agent follows that gateway instead, with its default model β see agent-configuration)". - Minor (advisory): the new bullet says a moved agent gets "no
model:", but on the move path an operator-owned model is kept (api_packs.go:224-226); "a pack-ownedmodel:is cleared" would be exact.
β hive: agent=ci-maintainer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Reviewed (style, delta 591ce65β¦a28696b) β prior findings re-checked at a28696b:
api_packs.go:1182hiveGatewayBackenddoc comment β fixed, now parses and matches the two-pass logic.api_packs.go:150snapshot comment β fixed, no longer claims the gateway is judged "on their agents alone".changelog.d/fixed-pack-agent-follows-hive-gateway.md:1β improved (one sentence β three) but still ~150 words in a single bullet; advisory only.
One advisory on the rewritten text: the new hiveGatewayBackend comment (api_packs.go:1182-1191) enumerates every "" outcome yet omits the !s.deps.Config.BackendAllowed(shared) return at api_packs.go:1219. Non-blocking.
β hive: agent=adjudicator-4 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Correctness (delta re-review, head a28696b)
Previous finding api_packs.go:214 (follow only from the pack CLI, not across a later gateway change) is partially fixed: the move condition is now existing.BackendIsPackOwned() && !EqualFold(existing.Backend, gw) (src/pkg/dashboard/api_packs.go:220), so a pack-owned agent on an old gateway is a candidate. But the gateway the hive "moved to" is still gated by hiveGatewayBackend's majority bar, which counts every other candidate β including pack-owned agents still sitting on the old gateway β in the denominator (api_packs.go:1226-1235: others++ for all non-except agents, onGateway*2 <= others β "").
Consequence on the real L3+ roster (packs/level-3.yaml: scanner/quality/guide operator-picked, supervisor/brainstorm/reviewer/ci-maintainer pack-owned): after an earlier apply moved the four pack agents onto litellm, the operator moves their three onto vllm. Each pack agent sees 3 on vllm among 6 others β 3*2 <= 6 β "", nothing moves, and because hiveBackends is a pre-apply snapshot (api_packs.go:153) the next restart sees the same roster β permanently stuck on the gateway the hive left. The new TestApplyPackFollowsGatewayChange (pack_gateway_model_test.go:186-209) only passes because it uses the L2 roster (3 pack-owned β 3 of 5). The new changelog line ("a pack-owned agent β¦ on a gateway the hive has since moved off β is moved there on the next restart") and agent-configuration.md:656 state this unconditionally.
This contradicts the stated design at api_packs.go:1196-1198 ("the pack's own β¦ placements cannot veto the move they are the subject of"). Requested fix: in the majority count, skip candidates that are pack-owned and sit on a gateway other than shared (an earlier apply's placement that would itself follow), and add an L3-roster variant of TestApplyPackFollowsGatewayChange. The pack-CLI-minority rule (TestApplyPackKeepsPackBackendWhenOperatorPicksAreMinority) is unaffected by that change.
Prior open question (api_packs.go:1180, hive-default gateway not detected) is unchanged by the delta; advisory only.
β hive: agent=adjudicator backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Conclusion: the placement logic looks correct to me; what still blocks this PR is DCO, not code.
What I read at head a28696b:
hiveGatewayBackend(src/pkg/dashboard/api_packs.go:1198-1232): unanimity over operator-owned backends excludingexcept, then a strict majority (onGateway*2 <= othersβ"") over all candidates. Checked the cases the comment promises: one operator-moved agent out of three β1*2 <= 2β no move; two of three β4 > 3β move. No operator-owned backends at all βshared == ""β no move, so a pack-only hive is untouched.gatewayPlacement(:1239-1244) only redirects a CLI pack backend (config.IsCLIBackend, defined insrc/crux/config/gateways.go) and leaves allow/deny-replaced placements alone, so it cannot fightpackAgentBackend.- The existing-agent move at
:220-229guards onexisting.BackendIsPackOwned()and is persisted by the pre-existingif changed { s.deps.Config.Agents[pa.Name] = existing; β¦ UpdateConfig }at:320-321, so the struct-copy is written back.BackendOwnerstayspack, so a later operator pick still wins. hiveBackendsis snapshotted once before the loop (:153) as the comment says, so agents moved earlier in the same apply do not shift the bar mid-apply.
Blocker: 3 of the 8 commits have no Signed-off-by (38cd38c6, 591ce65f, a28696b3) β the re-home the PR body asks for has not happened yet. Once the branch is DCO-clean this reads as mergeable to me; I have not approved anything.
β hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
a28696b to
5d76820
Compare
There was a problem hiding this comment.
Read the full applyPack path at this head, not just the hunks. The change looks correct to me; one open question for the maintainer below, not a defect.
What I verified
src/pkg/dashboard/api_packs.go:150snapshotshiveAgentBackends()once before the loop, so an agent this apply moves cannot swing the majority for the next one β the comment matches the code.- The move on existing agents (
api_packs.go:218-227) runs beforeonForeignGatewayis computed at:229, so after the moveagentOnForeignGatewayBackend(gw, packBackend)is true and the existing model-clearing block at:243-249cannot re-apply a pack model. Order is right. BackendOwnerstaysFieldOwnerPackafter a move (not re-stamped), which is what letsTestApplyPackFollowsGatewayChangework and keeps the operator-owned guard at:277-290untouched.hiveGatewayBackendchecksBackendAllowed(shared), so a gateway the spoke'sbackends:deny list blocks is never chosen; and the existing "move off denied backend" branch at:287would otherwise undo it the same apply. No oscillation.- All 9 commits carry
Signed-off-by; #14710 is closed.Sign-off survives the squashshows CANCELLED only on superseded runs.
Open question (not verified either way) β hiveGatewayBackend lets only operatorOwned backends vote (api_packs.go, if β¦ !vote.operatorOwned { continue }). BackendOwner is stamped only by the dashboard API paths (api_agent_config.go:515, :993, api_agent_control.go:207). An agent whose backend: litellm was set by a hand-edited overlay YAML, or through the dashboard before owner stamping existed, has no owner β it counts in others for the majority bar but never establishes shared. In a hive where all gateway agents are in that state (plausibly the #14678 reporter's, on v5.186.4-439 with agents moved "weeks ago"), shared stays "" and the reviewer still lands on copilot. Whether that is the intended conservative boundary ("legacy unowned is never moved" is documented; "legacy unowned does not vote" is not) is a call for @clubanderson or @Karibhou. If it is intended, a one-line note in the agent-configuration.md:656 paragraph would save the next reader the trace.
β hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
#14777 only counted operator-owned backends when choosing the gateway a pack agent follows, so a hive whose agents were put on LiteLLM without a backend_owner stamp (hive.yaml, pre-ownership hives, or POST /api/config/governor/agents) had no gateway and the L2 reviewer stayed on copilot. An unowned inference/gateway backend can only be the operator's choice (packs write CLIs and stamp their gateway moves), so it now counts as an operator pick; unowned CLIs still abstain. The governor add-agent endpoint now stamps an explicitly named backend/model as operator-owned. Fixes #14678 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: MikeSpreitzer <14296719+MikeSpreitzer@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
β¦ts (#15130) #14777 only counted operator-owned backends when choosing the gateway a pack agent follows, so a hive whose agents were put on LiteLLM without a backend_owner stamp (hive.yaml, pre-ownership hives, or POST /api/config/governor/agents) had no gateway and the L2 reviewer stayed on copilot. An unowned inference/gateway backend can only be the operator's choice (packs write CLIs and stamp their gateway moves), so it now counts as an operator pick; unowned CLIs still abstain. The governor add-agent endpoint now stamps an explicitly named backend/model as operator-owned. Fixes #14678 Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: MikeSpreitzer <14296719+MikeSpreitzer@users.noreply.github.com>
Supersedes #14710 (adopted in place, re-homed for DCO repair; original commits by @Karibhou preserved).
Fixes #14678
This branch is #14710's head
591ce65fdfa7as pushed by @Karibhou (forkKaribhou/hive), unchanged. Last lane reason on the original:dco-missing.Hive fix agent: rebase this branch on
v5, fix the failing checks / review asks, and push to THIS branch. Keep the original author's commits and theirSigned-off-by; add your own sign-off on your commits. Do not open another PR.DCO repair required: this adoption has unsigned non-merge commits and the hive cannot repair them by amending the contributor branch. Re-home the work onto a hive-owned branch from
v5: cherry-pick each original commit withgit cherry-pick -s(or squash withgit commit --authorpreserving the original author), ensure every replacement commit hasSigned-off-by, push the DCO-clean branch, open/keep one superseding PR with the same title, then comment on and close #14710. Suggested replacement commit message: