diff --git a/changelog.d/fixed-pack-agent-follows-hive-gateway.md b/changelog.d/fixed-pack-agent-follows-hive-gateway.md new file mode 100644 index 0000000000..dff199dae0 --- /dev/null +++ b/changelog.d/fixed-pack-agent-follows-hive-gateway.md @@ -0,0 +1 @@ +- A pack agent that an ACMM level adds to a hive whose operator moved their agents onto one inference gateway (e.g. the L2 `reviewer` on a LiteLLM hive) now joins that gateway with its default model instead of the pack's Copilot CLI, and an already-created pack-owned agent is moved there on the next restart. The gateway is the one every operator-picked backend names, when the agents already on it are a majority of the other enabled agents; pack-owned CLI agents such as the L2 `supervisor`/`brainstorm` do not veto the move and follow it, and once moved they count toward that majority so the next level's agent (the L3 `ci-maintainer`) follows too, while a hive with an operator-picked CLI agent or only a minority on a gateway is left as is (#14678). diff --git a/src/crux/config/agent_runtime.go b/src/crux/config/agent_runtime.go index 9f5e8d6b40..c40d59c927 100644 --- a/src/crux/config/agent_runtime.go +++ b/src/crux/config/agent_runtime.go @@ -647,6 +647,12 @@ func (a AgentConfig) BackendIsOperatorOwned() bool { return a.BackendOwner == FieldOwnerOperator } +// BackendIsPackOwned reports whether an ACMM pack wrote this agent's backend, +// as opposed to an operator's pick or a legacy value with no owner. +func (a AgentConfig) BackendIsPackOwned() bool { + return a.BackendOwner == FieldOwnerPack +} + // PauseIsOperatorOwned reports whether an operator explicitly owns this // agent's pause/run state — stamped at dashboard-API create and on an explicit // operator resume — which makes the agent immune to the ACMM pack visibility diff --git a/src/crux/config/leaf_test.go b/src/crux/config/leaf_test.go index 6c49368069..f45ebf1bb0 100644 --- a/src/crux/config/leaf_test.go +++ b/src/crux/config/leaf_test.go @@ -92,6 +92,13 @@ func TestAgentConfigJevAndOwners(t *testing.T) { if a.ModelIsOperatorOwned() { t.Fatal("pack owner is not operator") } + if a.BackendIsPackOwned() { + t.Fatal("operator-owned backend is not pack-owned") + } + a.BackendOwner = FieldOwnerPack + if !a.BackendIsPackOwned() || (AgentConfig{}).BackendIsPackOwned() { + t.Fatal("only a pack-stamped backend is pack-owned") + } } func TestAgentConfigRepos(t *testing.T) { diff --git a/src/pkg/dashboard/api_packs.go b/src/pkg/dashboard/api_packs.go index 436670b45b..dd06400e49 100644 --- a/src/pkg/dashboard/api_packs.go +++ b/src/pkg/dashboard/api_packs.go @@ -147,6 +147,9 @@ func (s *Server) applyPack(level int, forceGovernor bool) (*ApplyPackResult, err // the manager's process table and were re-registered. A repair, not a // creation — it never feeds `created` or isFirstApplyOrExpansion (#5632). var readded []string + // Read once, before this apply creates or moves anything, so the + // gateway the operator chose is judged on their agents alone. + hiveBackends := s.hiveAgentBackends() for _, pa := range pack.Agents { // A deliberately deleted agent is NOT re-created, at any level. The // pack listing it is exactly why deletion did not stick: ApplyPack @@ -156,6 +159,7 @@ func (s *Server) applyPack(level int, forceGovernor bool) (*ApplyPackResult, err tombstoned = append(tombstoned, pa.Name) continue } + hiveGateway := s.hiveGatewayBackend(hiveBackends, pa.Name) if existing, exists := s.deps.Config.Agents[pa.Name]; exists { changed := false @@ -205,6 +209,19 @@ func (s *Server) applyPack(level int, forceGovernor bool) (*ApplyPackResult, err // stick. A pack-owned model is cleared there instead, so the // gateway default applies. packBackend, backendReplaced := s.packAgentBackend(pa) + // A pack agent still sitting on the CLI the pack gave it follows + // the hive's gateway (#14678). Only a backend the pack itself + // wrote moves — never an operator's pick, nor a legacy unowned one. + gw := gatewayPlacement(packBackend, backendReplaced, hiveGateway) + if gw != "" && existing.BackendIsPackOwned() && strings.EqualFold(existing.Backend, packBackend) { + s.logger.Info("moving pack-owned agent onto the hive's inference gateway", + "agent", pa.Name, "backend", existing.Backend, "gateway", gw) + existing.Backend = gw + if !existing.ModelIsOperatorOwned() { + existing.Model = "" + } + changed = true + } onForeignGateway := s.agentOnForeignGatewayBackend(existing.Backend, packBackend) packModel := pa.Model // A hand-set backend other than the pack's (an operator-owned @@ -370,6 +387,12 @@ func (s *Server) applyPack(level int, forceGovernor bool) (*ApplyPackResult, err if backendReplaced { packModel = "" } + // The pack model is an id from the pack CLI's catalogue; on the + // gateway it is cleared so the gateway's default_model applies (#9945). + if gw := gatewayPlacement(packBackend, backendReplaced, hiveGateway); gw != "" { + packBackend = gw + packModel = "" + } agentCfg := config.AgentConfig{ Backend: packBackend, Model: packModel, @@ -1107,10 +1130,6 @@ func (s *Server) ensureCPUTier(level int) { } } -// packAgentBackend is the backend a pack agent is placed on: the pack's own -// choice, unless the spoke's backends allow/deny list (#11310) disallows it, -// in which case the list's fallback ("" = hive default) is used instead and -// replaced reports true. // agentOnForeignGatewayBackend reports whether an agent runs on an // inference-routed backend (litellm/vllm/llm-d or a configured gateway name) // other than the pack's backend, so a pack model — an id from the pack @@ -1122,9 +1141,102 @@ func (s *Server) agentOnForeignGatewayBackend(agentBackend, packBackend string) return config.IsInferenceBackend(agentBackend) || s.deps.Config.Governor.ResolveGateway(agentBackend) != nil } +// packAgentBackend is the backend a pack agent is placed on: the pack's own +// choice, unless the spoke's backends allow/deny list (#11310) disallows it, +// in which case the list's fallback ("" = hive default) is used instead and +// replaced reports true. func (s *Server) packAgentBackend(pa config.PackAgent) (backend string, replaced bool) { if s.deps.Config.BackendAllowed(pa.Backend) { return pa.Backend, false } return s.deps.Config.Backends.Fallback(), true } + +// agentBackendVote is one enabled agent's backend as seen by +// hiveGatewayBackend: the backend and whether an operator picked it. +type agentBackendVote struct { + backend string + operatorOwned bool +} + +// hiveAgentBackends snapshots the backend of every enabled agent, keyed by +// name, for hiveGatewayBackend. +func (s *Server) hiveAgentBackends() map[string]agentBackendVote { + backends := map[string]agentBackendVote{} + for name, agent := range s.deps.Config.Agents { + if !agent.Enabled || s.deps.Config.IsAgentRemoved(name) { + continue + } + backends[name] = agentBackendVote{ + backend: strings.TrimSpace(agent.Backend), + operatorOwned: agent.BackendIsOperatorOwned(), + } + } + return backends +} + +// hiveGatewayBackend is the inference gateway the operator moved the hive +// onto: the one inference backend or configured gateway that every +// operator-picked backend among the enabled agents other than except names, +// when the agents already on that gateway are a majority of all those other +// agents. It is "" when any operator-picked backend is a subscription CLI, +// the hive default or a different gateway, when there are no operator picks, +// or when the gateway hosts at most half of the other agents. except is the +// pack agent being placed, so its own pack CLI placement is not counted +// against the move. +// +// A pack agent a level adds later is placed on this backend instead of its +// pack CLI (#14678): an operator who moved scanner/quality/guide onto a LiteLLM +// gateway found the L2 `reviewer`, added afterwards, on copilot — the only +// agent in the hive still calling an inference provider they had moved off. +// Only operator-owned backends choose the gateway, so the pack's own CLI +// placements (the L2 `supervisor`/`brainstorm`, or a reviewer the pack +// created before this fix) cannot veto the move they are the subject of, and +// the majority bar keeps one agent moved onto a gateway from migrating the +// pack's CLI agents with it. Every agent already on the chosen gateway counts +// toward that majority, whoever placed it there: the pack agents an earlier +// level moved onto it must not dilute the bar for the agent the next level +// adds, or the L3 `ci-maintainer` would land on copilot one level up. +func (s *Server) hiveGatewayBackend(backends map[string]agentBackendVote, except string) string { + shared := "" + for name, vote := range backends { + if strings.EqualFold(name, except) || !vote.operatorOwned { + continue + } + if !s.agentOnForeignGatewayBackend(vote.backend, "") { + return "" + } + if shared != "" && !strings.EqualFold(shared, vote.backend) { + return "" + } + shared = vote.backend + } + if shared == "" || !s.deps.Config.BackendAllowed(shared) { + return "" + } + onGateway, others := 0, 0 + for name, vote := range backends { + if strings.EqualFold(name, except) { + continue + } + others++ + if strings.EqualFold(vote.backend, shared) { + onGateway++ + } + } + if onGateway*2 <= others { + return "" + } + return shared +} + +// gatewayPlacement is the backend a pack-owned agent is moved to so it follows +// the hive's gateway (hiveGatewayBackend), or "" to keep packBackend. Only a +// pack CLI placement is redirected: a pack that itself names a gateway, or a +// placement the spoke's backends list already replaced, is left alone. +func gatewayPlacement(packBackend string, backendReplaced bool, hiveGateway string) string { + if hiveGateway == "" || backendReplaced || !config.IsCLIBackend(packBackend) { + return "" + } + return hiveGateway +} diff --git a/src/pkg/dashboard/pack_gateway_model_test.go b/src/pkg/dashboard/pack_gateway_model_test.go index c64f120d49..29925b655d 100644 --- a/src/pkg/dashboard/pack_gateway_model_test.go +++ b/src/pkg/dashboard/pack_gateway_model_test.go @@ -124,3 +124,214 @@ func TestApplyPackModelOnKeptOrReplacedBackend(t *testing.T) { }) } } + +// gatewayHive is the #14678 shape: an L2 hive whose operator moved the agents +// it already had onto one inference gateway before the pack grew `reviewer`. +func gatewayHive(t *testing.T, backends ...string) *Server { + t.Helper() + srv := newFullServer(t) + for i, name := range []string{"scanner", "quality", "guide"} { + srv.deps.Config.Agents[name] = config.AgentConfig{ + ID: name, Role: name, Enabled: true, + Backend: backends[i%len(backends)], BackendOwner: config.FieldOwnerOperator, + } + } + return srv +} + +// TestApplyPackCreatesAgentOnHiveGateway covers #14678: the L2 `reviewer`, +// added to a hive whose agents all run on a LiteLLM gateway, was created on +// the pack's copilot backend with a copilot model id. +func TestApplyPackCreatesAgentOnHiveGateway(t *testing.T) { + srv := gatewayHive(t, "litellm") + if _, err := srv.ApplyPack(2); err != nil { + t.Fatalf("ApplyPack(2): %v", err) + } + got, ok := srv.deps.Config.Agents["reviewer"] + if !ok { + t.Fatal("L2 pack did not create `reviewer`") + } + if got.Backend != "litellm" || got.Model != "" { + t.Errorf("created reviewer backend=%q model=%q, want litellm with no model so the gateway default_model applies", got.Backend, got.Model) + } + if got.BackendIsOperatorOwned() { + t.Error("created reviewer backend is operator-owned; the operator never picked it") + } +} + +// TestApplyPackMovesPackOwnedAgentOntoHiveGateway: a hive that already got +// the copilot reviewer before the fix is repaired on its next restart. +func TestApplyPackMovesPackOwnedAgentOntoHiveGateway(t *testing.T) { + srv := gatewayHive(t, "litellm") + srv.deps.Config.Agents["reviewer"] = config.AgentConfig{ + ID: "reviewer", Role: "reviewer", Enabled: true, + Backend: "copilot", BackendOwner: config.FieldOwnerPack, + Model: "claude-fable-5", ModelOwner: config.FieldOwnerPack, + } + for i := 0; i < 2; i++ { + if _, err := srv.ApplyPack(2); err != nil { + t.Fatalf("ApplyPack(2) #%d: %v", i, err) + } + got := srv.deps.Config.Agents["reviewer"] + if got.Backend != "litellm" || got.Model != "" { + t.Errorf("apply #%d: reviewer backend=%q model=%q, want litellm with no model", i, got.Backend, got.Model) + } + } +} + +// TestApplyPackKeepsPackBackendWhenGatewayNotShared: the pack backend stays +// whenever "the hive's gateway" is not a single, unambiguous choice, and an +// operator's own copilot pick is never moved. +func TestApplyPackKeepsPackBackendWhenGatewayNotShared(t *testing.T) { + tests := []struct { + name string + backends []string + reviewer *config.AgentConfig + }{ + {"agents disagree on the gateway", []string{"litellm", "vllm"}, nil}, + {"no agent on a gateway", []string{"claude"}, nil}, + {"one agent on a gateway, the rest on a CLI", []string{"litellm", "copilot"}, nil}, + {"operator picked copilot", []string{"litellm"}, &config.AgentConfig{ + ID: "reviewer", Role: "reviewer", Enabled: true, + Backend: "copilot", BackendOwner: config.FieldOwnerOperator, + }}, + {"legacy unowned copilot", []string{"litellm"}, &config.AgentConfig{ + ID: "reviewer", Role: "reviewer", Enabled: true, Backend: "copilot", + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := gatewayHive(t, tt.backends...) + if tt.reviewer != nil { + srv.deps.Config.Agents["reviewer"] = *tt.reviewer + } + if _, err := srv.ApplyPack(2); err != nil { + t.Fatalf("ApplyPack(2): %v", err) + } + if got := srv.deps.Config.Agents["reviewer"].Backend; got != "copilot" { + t.Errorf("reviewer backend = %q, want the pack's copilot", got) + } + }) + } +} + +// TestApplyPackDoesNotMigrateFleetForOneGatewayAgent: an operator who moved a +// single agent onto a gateway has not moved the hive, so the pack-owned CLI +// agents stay where the pack put them (#14678 review). +func TestApplyPackDoesNotMigrateFleetForOneGatewayAgent(t *testing.T) { + srv := newFullServer(t) + for _, name := range []string{"scanner", "quality"} { + srv.deps.Config.Agents[name] = config.AgentConfig{ + ID: name, Role: name, Enabled: true, + Backend: "copilot", BackendOwner: config.FieldOwnerPack, + Model: "claude-fable-5", ModelOwner: config.FieldOwnerPack, + } + } + srv.deps.Config.Agents["guide"] = config.AgentConfig{ + ID: "guide", Role: "guide", Enabled: true, + Backend: "litellm", BackendOwner: config.FieldOwnerOperator, + } + if _, err := srv.ApplyPack(2); err != nil { + t.Fatalf("ApplyPack(2): %v", err) + } + for _, name := range []string{"scanner", "quality", "reviewer"} { + if got := srv.deps.Config.Agents[name].Backend; got != "copilot" { + t.Errorf("%s backend = %q, want the pack's copilot: one gateway agent is not the hive's gateway", name, got) + } + } +} + +// packCLIAgent is an agent the pack created on its own copilot CLI, as the L2 +// pack does for supervisor/brainstorm (and did for reviewer before #14678). +func packCLIAgent(name string) config.AgentConfig { + return config.AgentConfig{ + ID: name, Role: name, Enabled: true, + Backend: "copilot", BackendOwner: config.FieldOwnerPack, + Model: "claude-sonnet-4-6", ModelOwner: config.FieldOwnerPack, + } +} + +// TestApplyPackGatewayWithPreexistingPackCLIAgents is the real L2 roster +// behind #14678: supervisor and brainstorm already exist pack-owned on +// copilot when reviewer arrives, and the operator moved only +// scanner/quality/guide. Pack-owned CLI placements do not veto the move, so +// reviewer is created on the gateway and the two older ones follow it — in +// one apply, even though each sees the other still on copilot. +func TestApplyPackGatewayWithPreexistingPackCLIAgents(t *testing.T) { + tests := []struct { + name string + preexisting []string + }{ + {"reviewer created", []string{"supervisor", "brainstorm"}}, + {"reviewer repaired alongside two others", []string{"supervisor", "brainstorm", "reviewer"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := gatewayHive(t, "litellm") + for _, name := range tt.preexisting { + srv.deps.Config.Agents[name] = packCLIAgent(name) + } + for i := 0; i < 2; i++ { + if _, err := srv.ApplyPack(2); err != nil { + t.Fatalf("ApplyPack(2) #%d: %v", i, err) + } + for _, name := range []string{"supervisor", "brainstorm", "reviewer"} { + got, ok := srv.deps.Config.Agents[name] + if !ok { + t.Fatalf("apply #%d: L2 pack did not create %s", i, name) + } + if got.Backend != "litellm" || got.Model != "" { + t.Errorf("apply #%d: %s backend=%q model=%q, want litellm with no model", i, name, got.Backend, got.Model) + } + } + } + }) + } +} + +// TestApplyPackKeepsPackBackendWhenOperatorPicksAreMinority: three operator +// gateway agents that are at most half of the other enabled agents is not +// "the hive moved", so the pack's CLI placements stay. Four pack CLI agents +// so that every pack agent, which does not count itself, still sees three +// operator picks among six others. +func TestApplyPackKeepsPackBackendWhenOperatorPicksAreMinority(t *testing.T) { + srv := gatewayHive(t, "litellm") + packCLI := []string{"supervisor", "brainstorm", "ci-maintainer", "triage"} + for _, name := range packCLI { + srv.deps.Config.Agents[name] = packCLIAgent(name) + } + if _, err := srv.ApplyPack(2); err != nil { + t.Fatalf("ApplyPack(2): %v", err) + } + for _, name := range append(packCLI, "reviewer") { + if got := srv.deps.Config.Agents[name].Backend; got != "copilot" { + t.Errorf("%s backend = %q, want the pack's copilot: operator picks are not a majority", name, got) + } + } +} + +// TestApplyPackGatewayHoldsAcrossLevels: the pack agents L2 moved onto the +// gateway are pack-owned, so they must count toward the majority when L3 +// places `ci-maintainer` — otherwise three operator picks among six other +// agents is no longer a majority and #14678 reproduces one level up. +func TestApplyPackGatewayHoldsAcrossLevels(t *testing.T) { + srv := gatewayHive(t, "litellm") + if _, err := srv.ApplyPack(2); err != nil { + t.Fatalf("ApplyPack(2): %v", err) + } + for _, name := range []string{"supervisor", "brainstorm", "reviewer"} { + if got := srv.deps.Config.Agents[name]; got.Backend != "litellm" || got.BackendIsOperatorOwned() { + t.Fatalf("after L2: %s backend=%q operator-owned=%v, want pack-owned litellm", name, got.Backend, got.BackendIsOperatorOwned()) + } + } + if _, err := srv.ApplyPack(3); err != nil { + t.Fatalf("ApplyPack(3): %v", err) + } + got, ok := srv.deps.Config.Agents["ci-maintainer"] + if !ok { + t.Fatal("L3 pack did not create `ci-maintainer`") + } + if got.Backend != "litellm" || got.Model != "" { + t.Errorf("ci-maintainer backend=%q model=%q, want litellm with no model: the agents L2 moved onto the gateway count toward the hive's gateway", got.Backend, got.Model) + } +}