diff --git a/apps/backend/internal/office/agents/dto.go b/apps/backend/internal/office/agents/dto.go index 2ee7e21a82..93823b587a 100644 --- a/apps/backend/internal/office/agents/dto.go +++ b/apps/backend/internal/office/agents/dto.go @@ -50,14 +50,53 @@ type UpdateAgentStatusRequest struct { PauseReason string `json:"pause_reason"` } +// agentResponseBody wraps a models.AgentInstance for API responses, +// shadowing the shared struct's "model" key so it can be omitted for +// Office identities (role != ""). Embedding rather than hand-mirroring +// the 43 JSON-tagged fields on the shared struct keeps this DTO in +// sync automatically as that struct grows; a *string shadow field +// (not string) is required because the embedded struct's Model field +// has no `omitempty` — a plain string would drop the "model" key +// entirely for role=="" rows too, which AC-13 requires unchanged. +type agentResponseBody struct { + *models.AgentInstance + Model *string `json:"model,omitempty"` +} + +// newAgentResponseBody wraps agent for an API response. Model is set +// (and so emitted) only for role=="" rows (execution profiles); nil — +// omitted from JSON — for Office identities. A nil agent stays nil so +// callers serialise "agent": null exactly as before, never a +// zero-valued {} DTO. +func newAgentResponseBody(agent *models.AgentInstance) *agentResponseBody { + if agent == nil { + return nil + } + body := &agentResponseBody{AgentInstance: agent} + if agent.Role == "" { + m := agent.Model + body.Model = &m + } + return body +} + +// newAgentResponseBodies wraps a slice of agents for a list response. +func newAgentResponseBodies(agents []*models.AgentInstance) []*agentResponseBody { + out := make([]*agentResponseBody, len(agents)) + for i, a := range agents { + out[i] = newAgentResponseBody(a) + } + return out +} + // AgentResponse wraps a single agent instance. type AgentResponse struct { - Agent *models.AgentInstance `json:"agent"` + Agent *agentResponseBody `json:"agent"` } // AgentListResponse wraps a list of agent instances. type AgentListResponse struct { - Agents []*models.AgentInstance `json:"agents"` + Agents []*agentResponseBody `json:"agents"` } // UpsertMemoryRequest is the request body for creating/updating agent memory. diff --git a/apps/backend/internal/office/agents/handler.go b/apps/backend/internal/office/agents/handler.go index df2d4e9571..1745955444 100644 --- a/apps/backend/internal/office/agents/handler.go +++ b/apps/backend/internal/office/agents/handler.go @@ -1,7 +1,9 @@ package agents import ( + "encoding/json" "errors" + "io" "net/http" "strings" @@ -19,6 +21,12 @@ const ( ctxKeyAgentCaller = "agent_caller" ) +// maxUpdateAgentBodyBytes bounds the update-agent request body via +// http.MaxBytesReader, writing the 413 response below when exceeded. +// UpdateAgentRequest carries only short scalar fields plus a routing +// override blob, so this is generous relative to any legitimate payload. +const maxUpdateAgentBodyBytes = 64 * 1024 + // Handler provides HTTP handlers for agent routes. type Handler struct { svc *AgentService @@ -120,7 +128,7 @@ func (h *Handler) listAgents(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } - c.JSON(http.StatusOK, AgentListResponse{Agents: agents}) + c.JSON(http.StatusOK, AgentListResponse{Agents: newAgentResponseBodies(agents)}) } func (h *Handler) createAgent(c *gin.Context) { @@ -165,7 +173,7 @@ func (h *Handler) createAgent(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } - c.JSON(http.StatusCreated, AgentResponse{Agent: agent}) + c.JSON(http.StatusCreated, AgentResponse{Agent: newAgentResponseBody(agent)}) } func (h *Handler) getAgent(c *gin.Context) { @@ -174,7 +182,7 @@ func (h *Handler) getAgent(c *gin.Context) { c.JSON(http.StatusNotFound, gin.H{"error": err.Error()}) return } - c.JSON(http.StatusOK, AgentResponse{Agent: agent}) + c.JSON(http.StatusOK, AgentResponse{Agent: newAgentResponseBody(agent)}) } func (h *Handler) updateAgent(c *gin.Context) { @@ -191,9 +199,8 @@ func (h *Handler) updateAgent(c *gin.Context) { c.JSON(http.StatusNotFound, gin.H{"error": err.Error()}) return } - var req UpdateAgentRequest - if err := c.ShouldBindJSON(&req); err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + req, ok := decodeUpdateAgentRequest(c, agent.Role != "") + if !ok { return } if req.Permissions != nil { @@ -212,7 +219,7 @@ func (h *Handler) updateAgent(c *gin.Context) { }) return } - applyAgentUpdates(agent, &req) + applyAgentUpdates(agent, req) if err := h.svc.UpdateAgentInstance(ctx, agent); err != nil { if code := agentValidationErrorCode(err); code != "" { c.JSON(http.StatusBadRequest, gin.H{"error": err.Error(), "code": code}) @@ -221,7 +228,57 @@ func (h *Handler) updateAgent(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } - c.JSON(http.StatusOK, AgentResponse{Agent: agent}) + c.JSON(http.StatusOK, AgentResponse{Agent: newAgentResponseBody(agent)}) +} + +// decodeUpdateAgentRequest buffers the body once so the handler can inspect +// raw keys and then decode the typed request. Office identities reject the +// ignored model key before the request reaches the normal update path. +func decodeUpdateAgentRequest(c *gin.Context, officeAgent bool) (*UpdateAgentRequest, bool) { + c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, maxUpdateAgentBodyBytes) + body, err := io.ReadAll(c.Request.Body) + if err != nil { + var maxBytesError *http.MaxBytesError + if errors.As(err, &maxBytesError) { + c.JSON(http.StatusRequestEntityTooLarge, gin.H{"error": "request body too large"}) + return nil, false + } + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return nil, false + } + if officeAgent { + hasModel, err := requestBodyHasKey(body, "model") + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return nil, false + } + if hasModel { + respondRoutingValidation(c, &routing.ValidationError{ + Field: "model", + Message: "model no longer selects a launched model for an Office agent; " + + "update the agent routing override or workspace tier profiles", + }) + return nil, false + } + } + var req UpdateAgentRequest + if err := json.Unmarshal(body, &req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return nil, false + } + return &req, true +} + +// requestBodyHasKey reports whether the top-level JSON object in body +// carries key, without caring about its value — {"model":"x"}, +// {"model":""}, and {"model":null} must all be detected identically. +func requestBodyHasKey(body []byte, key string) (bool, error) { + raw := map[string]json.RawMessage{} + if err := json.Unmarshal(body, &raw); err != nil { + return false, err + } + _, ok := raw[key] + return ok, nil } // applyRoutingOverride validates the override blob and writes it onto @@ -322,7 +379,7 @@ func (h *Handler) updateAgentStatus(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) return } - c.JSON(http.StatusOK, AgentResponse{Agent: agent}) + c.JSON(http.StatusOK, AgentResponse{Agent: newAgentResponseBody(agent)}) } // -- Instruction handlers -- diff --git a/apps/backend/internal/office/agents/handler_routing_test.go b/apps/backend/internal/office/agents/handler_routing_test.go index ad74eebe7d..51160ac8f3 100644 --- a/apps/backend/internal/office/agents/handler_routing_test.go +++ b/apps/backend/internal/office/agents/handler_routing_test.go @@ -151,6 +151,357 @@ func asString(v interface{}) string { return s } +// newGetRecorder issues a GET against the agents router and returns the +// recorded response. +func newGetRecorder(t *testing.T, svc *AgentService, path string) *httptest.ResponseRecorder { + t.Helper() + gin.SetMode(gin.TestMode) + r := gin.New() + group := r.Group("/api/v1") + RegisterRoutes(group, svc, logger.Default()) + + req := httptest.NewRequest(http.MethodGet, path, nil) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + return rec +} + +// decodeAgentMap decodes body's top-level "agent" key into a plain map, so +// tests can assert on key *presence* rather than on a typed zero value — +// AC-13c requires the "model" key to be absent, not merely empty. +func decodeAgentMap(t *testing.T, body []byte) map[string]interface{} { + t.Helper() + var resp map[string]json.RawMessage + if err := json.Unmarshal(body, &resp); err != nil { + t.Fatalf("decode response envelope: %v", err) + } + var agent map[string]interface{} + if err := json.Unmarshal(resp["agent"], &agent); err != nil { + t.Fatalf("decode agent: %v", err) + } + return agent +} + +// AC-13/AC-13b/AC-13c: the "model" key is omitted entirely (not merely +// empty) for an Office agent (role != "") across every handler that +// serialises an agentResponseBody. +func TestGetAgent_OmitsModelFieldForOfficeAgent(t *testing.T) { + svc, _ := newTestAgentService(t) + ctx := context.Background() + agent := &models.AgentInstance{WorkspaceID: "ws-1", Name: "Worker", Role: models.AgentRoleWorker} + if err := svc.CreateAgentInstance(ctx, agent); err != nil { + t.Fatalf("create agent: %v", err) + } + + rec := newGetRecorder(t, svc, "/api/v1/agents/"+agent.ID) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body=%s", rec.Code, rec.Body.String()) + } + got := decodeAgentMap(t, rec.Body.Bytes()) + if _, ok := got["model"]; ok { + t.Errorf("model key present for Office agent: %+v", got) + } +} + +// AC-13: a legacy execution-profile row (role == "") keeps the "model" key, +// even when the value itself is the empty string. Such rows predate the +// Office identity model and are constructed directly against the +// repository, bypassing CreateAgentInstance's role validation (empty role +// is not a creatable value through the service layer). +func TestGetAgent_IncludesModelFieldForExecutionProfileAgent(t *testing.T) { + svc, repo := newTestAgentService(t) + ctx := context.Background() + agent := &models.AgentInstance{ + ID: "legacy-1", WorkspaceID: "ws-1", Name: "Legacy Profile", Model: "gpt-5", + } + if err := repo.CreateAgentInstance(ctx, agent); err != nil { + t.Fatalf("create legacy agent: %v", err) + } + + rec := newGetRecorder(t, svc, "/api/v1/agents/"+agent.ID) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body=%s", rec.Code, rec.Body.String()) + } + got := decodeAgentMap(t, rec.Body.Bytes()) + if v, ok := got["model"]; !ok || v != "gpt-5" { + t.Errorf("model key = %v (present=%v), want present with gpt-5", v, ok) + } +} + +// AC-13a: the shadow field on agentResponseBody must be a *string, not a +// plain string. This is the regression the type choice guards against: +// a legacy execution-profile row (role == "") with an EMPTY model value +// must still emit the "model" key (present, empty string) — this only +// passes because the shadow field's omitempty is checked against a nil +// *pointer*, not the empty string it points to. A plain `string` field +// with omitempty would silently drop this key when the value is "", +// which is exactly the regression TestGetAgent_IncludesModelFieldForExecutionProfileAgent's +// non-empty "gpt-5" fixture cannot catch. +func TestGetAgent_IncludesEmptyModelKeyForExecutionProfileAgent(t *testing.T) { + svc, repo := newTestAgentService(t) + ctx := context.Background() + agent := &models.AgentInstance{ + ID: "legacy-empty-model", WorkspaceID: "ws-1", Name: "Legacy No Model", Model: "", + } + if err := repo.CreateAgentInstance(ctx, agent); err != nil { + t.Fatalf("create legacy agent: %v", err) + } + + rec := newGetRecorder(t, svc, "/api/v1/agents/"+agent.ID) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body=%s", rec.Code, rec.Body.String()) + } + got := decodeAgentMap(t, rec.Body.Bytes()) + v, ok := got["model"] + if !ok { + t.Fatalf("model key absent for execution-profile agent with empty model: %+v", got) + } + if v != "" { + t.Errorf("model = %v, want empty string", v) + } +} + +// AC-13b: the omission also applies to the list endpoint. +func TestListAgents_OmitsModelFieldForOfficeAgents(t *testing.T) { + svc, _ := newTestAgentService(t) + ctx := context.Background() + agent := &models.AgentInstance{WorkspaceID: "ws-1", Name: "Worker", Role: models.AgentRoleWorker} + if err := svc.CreateAgentInstance(ctx, agent); err != nil { + t.Fatalf("create agent: %v", err) + } + + rec := newGetRecorder(t, svc, "/api/v1/workspaces/ws-1/agents") + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body=%s", rec.Code, rec.Body.String()) + } + var resp map[string]json.RawMessage + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode envelope: %v", err) + } + var list []map[string]interface{} + if err := json.Unmarshal(resp["agents"], &list); err != nil { + t.Fatalf("decode agents: %v", err) + } + if len(list) != 1 { + t.Fatalf("expected 1 agent, got %d", len(list)) + } + if _, ok := list[0]["model"]; ok { + t.Errorf("model key present for Office agent in list: %+v", list[0]) + } +} + +// AC-13b: create returns the same shape as get/list. +func TestCreateAgent_OmitsModelFieldForOfficeAgent(t *testing.T) { + svc, _ := newTestAgentService(t) + gin.SetMode(gin.TestMode) + r := gin.New() + group := r.Group("/api/v1") + RegisterRoutes(group, svc, logger.Default()) + + body := `{"name":"Worker","role":"worker"}` + req := httptest.NewRequest(http.MethodPost, "/api/v1/workspaces/ws-1/agents", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + + if rec.Code != http.StatusCreated { + t.Fatalf("status = %d, body=%s", rec.Code, rec.Body.String()) + } + got := decodeAgentMap(t, rec.Body.Bytes()) + if _, ok := got["model"]; ok { + t.Errorf("model key present for created Office agent: %+v", got) + } +} + +// AC-13b: status transitions return the same shape too. +func TestUpdateAgentStatus_OmitsModelFieldForOfficeAgent(t *testing.T) { + svc, _ := newTestAgentService(t) + ctx := context.Background() + agent := &models.AgentInstance{WorkspaceID: "ws-1", Name: "Worker", Role: models.AgentRoleWorker} + if err := svc.CreateAgentInstance(ctx, agent); err != nil { + t.Fatalf("create agent: %v", err) + } + + gin.SetMode(gin.TestMode) + r := gin.New() + group := r.Group("/api/v1") + RegisterRoutes(group, svc, logger.Default()) + + body := `{"status":"paused","pause_reason":"testing"}` + req := httptest.NewRequest(http.MethodPatch, "/api/v1/agents/"+agent.ID+"/status", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body=%s", rec.Code, rec.Body.String()) + } + got := decodeAgentMap(t, rec.Body.Bytes()) + if _, ok := got["model"]; ok { + t.Errorf("model key present for Office agent after status update: %+v", got) + } +} + +// AC-13b: a routine field-only PATCH also omits "model" from the response. +func TestUpdateAgent_OmitsModelFieldForOfficeAgent(t *testing.T) { + svc, _ := newTestAgentService(t) + ctx := context.Background() + agent := &models.AgentInstance{WorkspaceID: "ws-1", Name: "Worker", Role: models.AgentRoleWorker} + if err := svc.CreateAgentInstance(ctx, agent); err != nil { + t.Fatalf("create agent: %v", err) + } + + rec := newPatchAgentRecorder(t, svc, agent.ID, `{"icon":"robot"}`) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body=%s", rec.Code, rec.Body.String()) + } + got := decodeAgentMap(t, rec.Body.Bytes()) + if _, ok := got["model"]; ok { + t.Errorf("model key present for Office agent after PATCH: %+v", got) + } +} + +// AC-14/AC-14a: PATCH rejects a "model" key for an Office agent with a +// structured 400 naming the field, regardless of the key's value shape — +// a string, an empty string, and a JSON null must all be detected +// identically since requestBodyHasKey only checks key presence. +func TestUpdateAgent_RejectsModelKeyForOfficeAgent(t *testing.T) { + svc, _ := newTestAgentService(t) + ctx := context.Background() + + cases := []struct { + name string + body string + }{ + {"string value", `{"model":"gpt-5"}`}, + {"empty string value", `{"model":""}`}, + {"null value", `{"model":null}`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + agent := &models.AgentInstance{WorkspaceID: "ws-1", Name: "Worker " + tc.name, Role: models.AgentRoleWorker} + if err := svc.CreateAgentInstance(ctx, agent); err != nil { + t.Fatalf("create agent: %v", err) + } + + rec := newPatchAgentRecorder(t, svc, agent.ID, tc.body) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400; body=%s", rec.Code, rec.Body.String()) + } + var resp map[string]interface{} + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode response: %v", err) + } + if resp["field"] != "model" { + t.Errorf("response field = %v, want model", resp["field"]) + } + + stored, err := svc.GetAgentFromConfig(ctx, agent.ID) + if err != nil { + t.Fatalf("get agent: %v", err) + } + if stored.Model != "" { + t.Errorf("model persisted despite 400: %q", stored.Model) + } + }) + } +} + +// TestUpdateAgent_RejectsOversizedBody pins the request-body size cap on +// PATCH /agents/:id: a body larger than maxUpdateAgentBodyBytes must be +// rejected with 413 before JSON decoding runs, not buffered unbounded into +// memory. +func TestUpdateAgent_RejectsOversizedBody(t *testing.T) { + svc, _ := newTestAgentService(t) + ctx := context.Background() + agent := &models.AgentInstance{WorkspaceID: "ws-1", Name: "Worker", Role: models.AgentRoleWorker} + if err := svc.CreateAgentInstance(ctx, agent); err != nil { + t.Fatalf("create agent: %v", err) + } + + oversized := `{"name":"` + strings.Repeat("x", maxUpdateAgentBodyBytes+1) + `"}` + rec := newPatchAgentRecorder(t, svc, agent.ID, oversized) + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, want 413; body=%s", rec.Code, rec.Body.String()) + } + + stored, err := svc.GetAgentFromConfig(ctx, agent.ID) + if err != nil { + t.Fatalf("get agent: %v", err) + } + if stored.Name != "Worker" { + t.Errorf("name changed despite 413: %q", stored.Name) + } +} + +// AC-14c (AC-40 test #4): the model-key rejection uses the same structured +// ValidationError shape (field + details) as applyRoutingOverride, not the +// bare {"error": "..."} shape the pre-existing agent_profile_id rejection +// uses below in the same handler — proving these are genuinely distinct +// response contracts, not incidentally similar ones. +func TestUpdateAgent_ModelRejection_UsesStructuredShapeUnlikeAgentProfileIDRejection(t *testing.T) { + svc, _ := newTestAgentService(t) + ctx := context.Background() + + modelAgent := &models.AgentInstance{WorkspaceID: "ws-1", Name: "Worker Model", Role: models.AgentRoleWorker} + if err := svc.CreateAgentInstance(ctx, modelAgent); err != nil { + t.Fatalf("create agent: %v", err) + } + profileAgent := &models.AgentInstance{WorkspaceID: "ws-1", Name: "Worker Profile", Role: models.AgentRoleWorker} + if err := svc.CreateAgentInstance(ctx, profileAgent); err != nil { + t.Fatalf("create agent: %v", err) + } + + modelRec := newPatchAgentRecorder(t, svc, modelAgent.ID, `{"model":"gpt-5"}`) + profileRec := newPatchAgentRecorder(t, svc, profileAgent.ID, `{"agent_profile_id":"prof-1"}`) + + var modelResp, profileResp map[string]interface{} + if err := json.Unmarshal(modelRec.Body.Bytes(), &modelResp); err != nil { + t.Fatalf("decode model response: %v", err) + } + if err := json.Unmarshal(profileRec.Body.Bytes(), &profileResp); err != nil { + t.Fatalf("decode profile response: %v", err) + } + + if _, ok := modelResp["field"]; !ok { + t.Errorf("model rejection missing structured field key: %+v", modelResp) + } + if _, ok := profileResp["field"]; ok { + t.Errorf("agent_profile_id rejection unexpectedly carries a field key: %+v", profileResp) + } +} + +// AC-14b: the model-key gate is scoped to Office agents (role != "") — +// requestBodyHasKey is only consulted when agent.Role != "". A legacy +// execution-profile row (role == "") never reaches this endpoint +// successfully at all (validateAgentUpdate rejects any empty role, +// independent of this feature), so the gate being scoped correctly is +// what keeps that pre-existing, unrelated 500 as the response instead of +// this feature's 400 "field":"model" shape shadowing it. +func TestUpdateAgent_ModelKeyGateSkipped_ForExecutionProfileAgent(t *testing.T) { + svc, repo := newTestAgentService(t) + ctx := context.Background() + agent := &models.AgentInstance{ + ID: "legacy-2", WorkspaceID: "ws-1", Name: "Legacy Profile", Model: "gpt-5", + } + if err := repo.CreateAgentInstance(ctx, agent); err != nil { + t.Fatalf("create legacy agent: %v", err) + } + + rec := newPatchAgentRecorder(t, svc, agent.ID, `{"model":"claude-opus"}`) + if rec.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500 (pre-existing invalid-role rejection); body=%s", + rec.Code, rec.Body.String()) + } + var resp map[string]interface{} + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode response: %v", err) + } + if resp["field"] == "model" { + t.Errorf("model-key gate fired for role==\"\" agent, want it scoped to Office agents: %+v", resp) + } +} + // TestAgentAuthMiddleware_RejectsCrossWorkspaceToken ensures a token minted // for workspace A cannot access endpoints scoped to workspace B via the // :wsId path parameter. Without this check, an agent in one workspace could diff --git a/apps/backend/internal/office/dashboard/handler_routing_test.go b/apps/backend/internal/office/dashboard/handler_routing_test.go index 3652978756..d4ef029b8c 100644 --- a/apps/backend/internal/office/dashboard/handler_routing_test.go +++ b/apps/backend/internal/office/dashboard/handler_routing_test.go @@ -251,7 +251,7 @@ func TestRouting_PreviewComposesAgentRows(t *testing.T) { fake.preview = []routing.PreviewItem{ { AgentID: "a1", AgentName: "Alice", - TierSource: "inherit", EffectiveTier: "balanced", + TierSource: routing.TierSourceWorkspace, EffectiveTier: "balanced", PrimaryProviderID: "claude-acp", PrimaryModel: "sonnet", FallbackChain: []routing.PreviewProviderModel{ {ProviderID: "codex-acp", Model: "gpt-5", Tier: "balanced"}, @@ -284,6 +284,87 @@ func TestRouting_PreviewComposesAgentRows(t *testing.T) { } } +// AC-18a/AC-20: the preview row's TierSource passes through unmodified, +// including the two new widened values ("role", "workspace") the +// dashboard layer never previously had to carry. +func TestRouting_PreviewRow_WidenedTierSourceValues(t *testing.T) { + cases := []string{ + routing.TierSourceOverride, + routing.TierSourceRole, + routing.TierSourceWorkspace, + } + for _, source := range cases { + t.Run(source, func(t *testing.T) { + deps, fake := newRoutingTestDeps(t) + fake.preview = []routing.PreviewItem{ + {AgentID: "a1", AgentName: "Alice", TierSource: source, EffectiveTier: "balanced"}, + } + req := httptest.NewRequest(http.MethodGet, "/api/v1/office/workspaces/ws-1/routing/preview", nil) + rec := httptest.NewRecorder() + deps.router.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d body = %s", rec.Code, rec.Body.String()) + } + var resp dashboard.RoutingPreviewResponse + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if len(resp.Agents) != 1 || resp.Agents[0].TierSource != source { + t.Fatalf("TierSource = %+v, want %q", resp.Agents, source) + } + }) + } +} + +// AC-20/AC-20a: RouteAttemptDTO.TierSource carries the persisted +// precedence level through the /runs/:id/attempts endpoint. +func TestRouting_AttemptsEndpoint_TierSourceRoundTrips(t *testing.T) { + deps, _ := newRoutingTestDeps(t) + lister := &fakeAttemptLister{byRun: map[string][]models.RouteAttempt{ + "run-1": { + {RunID: "run-1", Seq: 1, ProviderID: "claude-acp", Model: "haiku", + Tier: "economy", TierSource: "role", Outcome: "launched", + StartedAt: time.Now().UTC()}, + }, + }} + deps.svc.SetRouteAttemptLister(lister) + req := httptest.NewRequest(http.MethodGet, "/api/v1/office/runs/run-1/attempts", nil) + rec := httptest.NewRecorder() + deps.router.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d body = %s", rec.Code, rec.Body.String()) + } + var resp dashboard.RouteAttemptsResponse + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if len(resp.Attempts) != 1 || resp.Attempts[0].TierSource != "role" { + t.Fatalf("unexpected attempts: %+v", resp.Attempts) + } +} + +// An attempt row with no recorded tier_source (pre-migration or a +// max-attempts-exceeded terminal row) omits the field entirely rather +// than emitting "tier_source":"" — the DTO field carries omitempty. +func TestRouting_AttemptsEndpoint_EmptyTierSourceOmittedFromJSON(t *testing.T) { + deps, _ := newRoutingTestDeps(t) + lister := &fakeAttemptLister{byRun: map[string][]models.RouteAttempt{ + "run-1": { + {RunID: "run-1", Seq: 1, Outcome: "skipped_max_attempts", StartedAt: time.Now().UTC()}, + }, + }} + deps.svc.SetRouteAttemptLister(lister) + req := httptest.NewRequest(http.MethodGet, "/api/v1/office/runs/run-1/attempts", nil) + rec := httptest.NewRecorder() + deps.router.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d body = %s", rec.Code, rec.Body.String()) + } + if bytes.Contains(rec.Body.Bytes(), []byte(`"tier_source"`)) { + t.Fatalf("expected tier_source to be omitted, got %s", rec.Body.String()) + } +} + func TestRouting_HealthEndpointReturnsRows(t *testing.T) { deps, fake := newRoutingTestDeps(t) fake.healthRows = []models.ProviderHealth{ diff --git a/apps/backend/internal/office/dashboard/routing_dto.go b/apps/backend/internal/office/dashboard/routing_dto.go index 6cd974c047..69df5a86d3 100644 --- a/apps/backend/internal/office/dashboard/routing_dto.go +++ b/apps/backend/internal/office/dashboard/routing_dto.go @@ -31,8 +31,11 @@ type RoutingHealthResponse struct { } // AgentRoutePreview is one row in the workspace routing preview table. -// TierSource is "inherit" when the agent uses the workspace default and -// "override" when its override blob sets a tier explicitly. +// TierSource names the precedence level that supplied EffectiveTier: +// one of "wake_reason", "override", "role", "workspace" (see +// routing.PreviewItem.TierSource / effectiveTier). Always computed +// with an empty run reason, so this never reports "wake_reason" even +// when the agent's own wake-reason policy would shadow it at runtime. // // PrimaryProviderID/PrimaryModel reflect the configured intent (first // entry in the effective provider order). CurrentProviderID/ @@ -72,21 +75,26 @@ type RoutingPreviewResponse struct { // RouteAttemptDTO mirrors models.RouteAttempt with stable JSON tags for // the dashboard run-detail UI. Timestamps are formatted as RFC3339. type RouteAttemptDTO struct { - Seq int `json:"seq"` - ExecutionProfileID string `json:"execution_profile_id,omitempty"` - ProviderID string `json:"provider_id"` - Model string `json:"model,omitempty"` - Tier string `json:"tier"` - Outcome string `json:"outcome"` - ErrorCode string `json:"error_code,omitempty"` - ErrorConfidence string `json:"error_confidence,omitempty"` - AdapterPhase string `json:"adapter_phase,omitempty"` - ClassifierRule string `json:"classifier_rule,omitempty"` - ExitCode *int `json:"exit_code,omitempty"` - RawExcerpt string `json:"raw_excerpt,omitempty"` - ResetHint *string `json:"reset_hint,omitempty"` - StartedAt string `json:"started_at"` - FinishedAt *string `json:"finished_at,omitempty"` + Seq int `json:"seq"` + ExecutionProfileID string `json:"execution_profile_id,omitempty"` + ProviderID string `json:"provider_id"` + Model string `json:"model,omitempty"` + Tier string `json:"tier"` + // TierSource names the precedence level that supplied Tier for this + // attempt: one of "wake_reason", "override", "role", "workspace", + // or "" for attempts recorded before this column existed or that + // never resolved a tier (see models.RouteAttempt.TierSource). + TierSource string `json:"tier_source,omitempty"` + Outcome string `json:"outcome"` + ErrorCode string `json:"error_code,omitempty"` + ErrorConfidence string `json:"error_confidence,omitempty"` + AdapterPhase string `json:"adapter_phase,omitempty"` + ClassifierRule string `json:"classifier_rule,omitempty"` + ExitCode *int `json:"exit_code,omitempty"` + RawExcerpt string `json:"raw_excerpt,omitempty"` + ResetHint *string `json:"reset_hint,omitempty"` + StartedAt string `json:"started_at"` + FinishedAt *string `json:"finished_at,omitempty"` } // RunRouting is embedded on the run-detail response so the routing @@ -130,6 +138,7 @@ func routeAttemptToDTO(a models.RouteAttempt) RouteAttemptDTO { ProviderID: a.ProviderID, Model: a.Model, Tier: a.Tier, + TierSource: a.TierSource, Outcome: string(a.Outcome), ErrorCode: a.ErrorCode, ErrorConfidence: string(a.ErrorConfidence), diff --git a/apps/backend/internal/office/models/models.go b/apps/backend/internal/office/models/models.go index 85ead513e1..f6fc78a769 100644 --- a/apps/backend/internal/office/models/models.go +++ b/apps/backend/internal/office/models/models.go @@ -459,22 +459,28 @@ type Run struct { // Persisted by the routing scheduler dispatcher in // internal/office/scheduler/dispatch_routing.go. type RouteAttempt struct { - RunID string `json:"run_id" db:"run_id"` - Seq int `json:"seq" db:"seq"` - ExecutionProfileID string `json:"execution_profile_id" db:"execution_profile_id"` - ProviderID string `json:"provider_id" db:"provider_id"` - Model string `json:"model" db:"model"` - Tier string `json:"tier" db:"tier"` - Outcome RouteAttemptOutcome `json:"outcome" db:"outcome"` - ErrorCode string `json:"error_code,omitempty" db:"error_code"` - ErrorConfidence ErrorConfidence `json:"error_confidence,omitempty" db:"error_confidence"` - AdapterPhase AdapterPhase `json:"adapter_phase,omitempty" db:"adapter_phase"` - ClassifierRule string `json:"classifier_rule,omitempty" db:"classifier_rule"` - ExitCode *int `json:"exit_code,omitempty" db:"exit_code"` - RawExcerpt string `json:"raw_excerpt,omitempty" db:"raw_excerpt"` - ResetHint *time.Time `json:"reset_hint,omitempty" db:"reset_hint"` - StartedAt time.Time `json:"started_at" db:"started_at"` - FinishedAt *time.Time `json:"finished_at,omitempty" db:"finished_at"` + RunID string `json:"run_id" db:"run_id"` + Seq int `json:"seq" db:"seq"` + ExecutionProfileID string `json:"execution_profile_id" db:"execution_profile_id"` + ProviderID string `json:"provider_id" db:"provider_id"` + Model string `json:"model" db:"model"` + Tier string `json:"tier" db:"tier"` + // TierSource names the precedence level that supplied Tier: one of + // "wake_reason", "override", "role", "workspace". Empty means "not + // recorded" — either a pre-migration row (Tier non-empty) or an + // attempt that never resolved a tier (both empty, e.g. a + // max-attempts-exceeded row) — never interpreted as "workspace". + TierSource string `json:"tier_source,omitempty" db:"tier_source"` + Outcome RouteAttemptOutcome `json:"outcome" db:"outcome"` + ErrorCode string `json:"error_code,omitempty" db:"error_code"` + ErrorConfidence ErrorConfidence `json:"error_confidence,omitempty" db:"error_confidence"` + AdapterPhase AdapterPhase `json:"adapter_phase,omitempty" db:"adapter_phase"` + ClassifierRule string `json:"classifier_rule,omitempty" db:"classifier_rule"` + ExitCode *int `json:"exit_code,omitempty" db:"exit_code"` + RawExcerpt string `json:"raw_excerpt,omitempty" db:"raw_excerpt"` + ResetHint *time.Time `json:"reset_hint,omitempty" db:"reset_hint"` + StartedAt time.Time `json:"started_at" db:"started_at"` + FinishedAt *time.Time `json:"finished_at,omitempty" db:"finished_at"` } // ProviderHealth records the health state of one (workspace, provider, diff --git a/apps/backend/internal/office/repository/sqlite/base_migrations.go b/apps/backend/internal/office/repository/sqlite/base_migrations.go index 5d24492546..1c9d2932c0 100644 --- a/apps/backend/internal/office/repository/sqlite/base_migrations.go +++ b/apps/backend/internal/office/repository/sqlite/base_migrations.go @@ -94,6 +94,7 @@ func (r *Repository) migrateProviderRouting() { provider_order TEXT NOT NULL DEFAULT '[]', provider_profiles TEXT NOT NULL DEFAULT '{}', tier_per_reason TEXT NOT NULL DEFAULT '{}', + role_tiers TEXT NOT NULL DEFAULT '{}', updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP )`) @@ -105,6 +106,7 @@ func (r *Repository) migrateProviderRouting() { provider_id TEXT NOT NULL, model TEXT NOT NULL, tier TEXT NOT NULL, + tier_source TEXT NOT NULL DEFAULT '', outcome TEXT NOT NULL, error_code TEXT, error_confidence TEXT, @@ -123,6 +125,10 @@ func (r *Repository) migrateProviderRouting() { `ALTER TABLE runs ADD COLUMN resolved_execution_profile_id TEXT`) r.migrate.Apply("office_run_route_attempts.execution_profile_id", `ALTER TABLE office_run_route_attempts ADD COLUMN execution_profile_id TEXT NOT NULL DEFAULT ''`) + r.migrate.Apply("office_workspace_routing.role_tiers", + `ALTER TABLE office_workspace_routing ADD COLUMN role_tiers TEXT NOT NULL DEFAULT '{}'`) + r.migrate.Apply("office_run_route_attempts.tier_source", + `ALTER TABLE office_run_route_attempts ADD COLUMN tier_source TEXT NOT NULL DEFAULT ''`) _, _ = r.db.Exec(` CREATE TABLE IF NOT EXISTS office_provider_health ( diff --git a/apps/backend/internal/office/repository/sqlite/role_tiers_test.go b/apps/backend/internal/office/repository/sqlite/role_tiers_test.go new file mode 100644 index 0000000000..4a187bc029 --- /dev/null +++ b/apps/backend/internal/office/repository/sqlite/role_tiers_test.go @@ -0,0 +1,110 @@ +package sqlite_test + +import ( + "context" + "strings" + "testing" + + "github.com/kandev/kandev/internal/office/routing" +) + +// TestUpsertWorkspaceRouting_RoleTiersRoundTrip pins the JSON round-trip +// for the new role_tiers column, including AC-11's drop-empty-values +// behavior on write. +func TestUpsertWorkspaceRouting_RoleTiersRoundTrip(t *testing.T) { + repo := newTestRepo(t) + ctx := context.Background() + in := &routing.WorkspaceConfig{ + Enabled: true, + DefaultTier: routing.TierBalanced, + ProviderOrder: []routing.ProviderID{"claude-acp"}, + ProviderProfiles: map[routing.ProviderID]routing.ProviderProfile{ + "claude-acp": {TierMap: routing.TierMap{Balanced: "sonnet", Economy: "haiku"}}, + }, + RoleTiers: routing.RoleTierMap{ + "qa": routing.TierEconomy, + "devops": routing.TierEconomy, + "worker": "", // AC-11: dropped before persistence + }, + } + if err := repo.UpsertWorkspaceRouting(ctx, "ws-1", in); err != nil { + t.Fatalf("upsert: %v", err) + } + got, err := repo.GetWorkspaceRouting(ctx, "ws-1") + if err != nil { + t.Fatalf("get: %v", err) + } + if got.RoleTiers["qa"] != routing.TierEconomy { + t.Errorf("qa tier = %q, want economy", got.RoleTiers["qa"]) + } + if got.RoleTiers["devops"] != routing.TierEconomy { + t.Errorf("devops tier = %q, want economy", got.RoleTiers["devops"]) + } + if _, ok := got.RoleTiers["worker"]; ok { + t.Errorf("empty-value worker entry survived persistence: %+v", got.RoleTiers) + } +} + +// AC-40 test #1: a literal empty-string role_tiers column (the state a +// pre-migration row lands in via ADD COLUMN ... DEFAULT ”) decodes to an +// empty map with no error. This is the precedent-trap test proving +// decodeRoleTiers's raw=="" special case was necessary — the sibling +// tier_per_reason column only needs COALESCE(..., '{}') because SQL NULL +// is its only "missing" state, but role_tiers can also be a literal ” +// coming from the migration's own DEFAULT clause. +func TestGetWorkspaceRouting_EmptyStringRoleTiersColumn(t *testing.T) { + repo, _ := newTestRepoWithDB(t) + ctx := context.Background() + if _, err := repo.ExecRaw(ctx, ` + INSERT INTO office_workspace_routing + (workspace_id, enabled, default_tier, provider_order, provider_profiles, role_tiers, updated_at) + VALUES (?, 1, 'balanced', '[]', '{}', '', datetime('now')) + `, "ws-empty-roles"); err != nil { + t.Fatalf("insert row with literal empty role_tiers: %v", err) + } + + cfg, err := repo.GetWorkspaceRouting(ctx, "ws-empty-roles") + if err != nil { + t.Fatalf("GetWorkspaceRouting: %v", err) + } + if len(cfg.RoleTiers) != 0 { + t.Errorf("RoleTiers = %+v, want empty map", cfg.RoleTiers) + } +} + +// AC-33: malformed (non-empty, non-JSON) role_tiers content is rejected +// rather than silently swallowed. +func TestGetWorkspaceRouting_MalformedRoleTiersColumn(t *testing.T) { + repo, _ := newTestRepoWithDB(t) + ctx := context.Background() + if _, err := repo.ExecRaw(ctx, ` + INSERT INTO office_workspace_routing + (workspace_id, enabled, default_tier, provider_order, provider_profiles, role_tiers, updated_at) + VALUES (?, 1, 'balanced', '[]', '{}', 'not-json', datetime('now')) + `, "ws-bad-roles"); err != nil { + t.Fatalf("insert row with malformed role_tiers: %v", err) + } + + _, err := repo.GetWorkspaceRouting(ctx, "ws-bad-roles") + if err == nil { + t.Fatal("expected error decoding malformed role_tiers") + } + if !strings.Contains(err.Error(), "role_tiers") { + t.Errorf("error should name role_tiers, got %v", err) + } +} + +// defaultWorkspaceRouting (the never-configured-workspace path) also +// carries a non-nil, empty RoleTiers map. +func TestGetWorkspaceRouting_DefaultOnEmpty_RoleTiers(t *testing.T) { + repo := newTestRepo(t) + ctx := context.Background() + + cfg, err := repo.GetWorkspaceRouting(ctx, "ws-never-configured") + if err != nil { + t.Fatalf("GetWorkspaceRouting: %v", err) + } + if len(cfg.RoleTiers) != 0 { + t.Errorf("RoleTiers = %+v, want empty map", cfg.RoleTiers) + } +} diff --git a/apps/backend/internal/office/repository/sqlite/route_attempts.go b/apps/backend/internal/office/repository/sqlite/route_attempts.go index 7e46793e42..3e4ed3e67e 100644 --- a/apps/backend/internal/office/repository/sqlite/route_attempts.go +++ b/apps/backend/internal/office/repository/sqlite/route_attempts.go @@ -22,11 +22,11 @@ func (r *Repository) AppendRouteAttempt( } _, err := r.db.ExecContext(ctx, r.db.Rebind(` INSERT INTO office_run_route_attempts ( - run_id, seq, execution_profile_id, provider_id, model, tier, outcome, + run_id, seq, execution_profile_id, provider_id, model, tier, tier_source, outcome, error_code, error_confidence, adapter_phase, classifier_rule, exit_code, raw_excerpt, reset_hint, started_at, finished_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - `), a.RunID, a.Seq, a.ExecutionProfileID, a.ProviderID, a.Model, a.Tier, a.Outcome, + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `), a.RunID, a.Seq, a.ExecutionProfileID, a.ProviderID, a.Model, a.Tier, a.TierSource, a.Outcome, nullableString(a.ErrorCode), nullableString(string(a.ErrorConfidence)), nullableString(string(a.AdapterPhase)), nullableString(a.ClassifierRule), a.ExitCode, nullableString(a.RawExcerpt), a.ResetHint, @@ -45,7 +45,7 @@ func (r *Repository) ListRouteAttempts( ctx context.Context, runID string, ) ([]models.RouteAttempt, error) { rows, err := r.ro.QueryxContext(ctx, r.ro.Rebind(` - SELECT run_id, seq, execution_profile_id, provider_id, model, tier, outcome, + SELECT run_id, seq, execution_profile_id, provider_id, model, tier, tier_source, outcome, COALESCE(error_code,'') AS error_code, COALESCE(error_confidence,'') AS error_confidence, COALESCE(adapter_phase,'') AS adapter_phase, diff --git a/apps/backend/internal/office/repository/sqlite/route_attempts_test.go b/apps/backend/internal/office/repository/sqlite/route_attempts_test.go index f361fc2f12..752635e394 100644 --- a/apps/backend/internal/office/repository/sqlite/route_attempts_test.go +++ b/apps/backend/internal/office/repository/sqlite/route_attempts_test.go @@ -102,6 +102,47 @@ func TestAppendAndListRouteAttempts_RoundTrip(t *testing.T) { } } +// TestAppendAndListRouteAttempts_TierSourceRoundTrip pins the JSON-free +// round-trip for the new tier_source column, including the empty-string +// default (a max-attempts-exceeded row, or any attempt appended before +// this column existed, never resolved a tier). +func TestAppendAndListRouteAttempts_TierSourceRoundTrip(t *testing.T) { + repo := newTestRepo(t) + ctx := context.Background() + now := time.Now().UTC().Truncate(time.Second) + + seedRun(t, repo, "run-1", "agent-1", now) + + withSource := &models.RouteAttempt{ + RunID: "run-1", Seq: 1, ProviderID: "claude-acp", Model: "haiku", + Tier: "economy", TierSource: "role", Outcome: "launched", StartedAt: now, + } + if err := repo.AppendRouteAttempt(ctx, withSource); err != nil { + t.Fatalf("append with tier_source: %v", err) + } + withoutSource := &models.RouteAttempt{ + RunID: "run-1", Seq: 2, ProviderID: "claude-acp", Model: "sonnet", + Tier: "balanced", Outcome: "launched", StartedAt: now.Add(time.Minute), + } + if err := repo.AppendRouteAttempt(ctx, withoutSource); err != nil { + t.Fatalf("append without tier_source: %v", err) + } + + attempts, err := repo.ListRouteAttempts(ctx, "run-1") + if err != nil { + t.Fatalf("list: %v", err) + } + if len(attempts) != 2 { + t.Fatalf("expected 2 attempts, got %d", len(attempts)) + } + if attempts[0].TierSource != "role" { + t.Errorf("attempts[0].TierSource = %q, want role", attempts[0].TierSource) + } + if attempts[1].TierSource != "" { + t.Errorf("attempts[1].TierSource = %q, want empty", attempts[1].TierSource) + } +} + func TestListRouteAttempts_EmptyReturnsEmptySlice(t *testing.T) { repo := newTestRepo(t) ctx := context.Background() diff --git a/apps/backend/internal/office/repository/sqlite/workspace_routing.go b/apps/backend/internal/office/repository/sqlite/workspace_routing.go index 9a0a99148d..14b4b10d71 100644 --- a/apps/backend/internal/office/repository/sqlite/workspace_routing.go +++ b/apps/backend/internal/office/repository/sqlite/workspace_routing.go @@ -33,14 +33,15 @@ func (r *Repository) GetWorkspaceRouting( orderRaw string profsRaw string reasonsRaw string + rolesRaw string updatedAt time.Time ) err := r.ro.QueryRowxContext(ctx, r.ro.Rebind(` SELECT enabled, default_tier, provider_order, provider_profiles, - COALESCE(tier_per_reason, '{}'), updated_at + COALESCE(tier_per_reason, '{}'), COALESCE(role_tiers, '{}'), updated_at FROM office_workspace_routing WHERE workspace_id = ? - `), workspaceID).Scan(&enabled, &tier, &orderRaw, &profsRaw, &reasonsRaw, &updatedAt) + `), workspaceID).Scan(&enabled, &tier, &orderRaw, &profsRaw, &reasonsRaw, &rolesRaw, &updatedAt) if errors.Is(err, sql.ErrNoRows) { return defaultWorkspaceRouting(), nil } @@ -60,12 +61,31 @@ func (r *Repository) GetWorkspaceRouting( if err := json.Unmarshal([]byte(reasonsRaw), &cfg.TierPerReason); err != nil { return nil, fmt.Errorf("routing: decode tier_per_reason: %w", err) } + if err := decodeRoleTiers(rolesRaw, &cfg.RoleTiers); err != nil { + return nil, fmt.Errorf("routing: decode role_tiers: %w", err) + } if cfg.ProviderProfiles == nil { cfg.ProviderProfiles = map[routing.ProviderID]routing.ProviderProfile{} } return cfg, nil } +// decodeRoleTiers decodes raw into *out, treating a literal empty +// string as "{}" (no role policy) before unmarshalling (AC-32). The +// adjacent tier_per_reason decode above does not need this: it reads +// COALESCE(tier_per_reason, '{}'), which only substitutes for SQL +// NULL, so a literal ” falls through to json.Unmarshal and errors — +// which happens to be correct there (AC-33 malformed-JSON rejection), +// but AC-32 explicitly names ” as a valid role_tiers value, so +// copying the sibling's COALESCE-only pattern would wrongly reject it. +func decodeRoleTiers(raw string, out *routing.RoleTierMap) error { + if raw == "" { + *out = routing.RoleTierMap{} + return nil + } + return json.Unmarshal([]byte(raw), out) +} + // UpsertWorkspaceRouting persists cfg for workspaceID, replacing any // previously stored row. JSON marshaling errors are surfaced before // touching the DB so a malformed config can never leave a half-written @@ -88,6 +108,10 @@ func (r *Repository) UpsertWorkspaceRouting( if err != nil { return err } + rolesRaw, err := marshalRoleTiers(cfg.RoleTiers) + if err != nil { + return err + } enabled := 0 if cfg.Enabled { enabled = 1 @@ -98,16 +122,17 @@ func (r *Repository) UpsertWorkspaceRouting( } _, err = r.db.ExecContext(ctx, r.db.Rebind(` INSERT INTO office_workspace_routing - (workspace_id, enabled, default_tier, provider_order, provider_profiles, tier_per_reason, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?) + (workspace_id, enabled, default_tier, provider_order, provider_profiles, tier_per_reason, role_tiers, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(workspace_id) DO UPDATE SET enabled = excluded.enabled, default_tier = excluded.default_tier, provider_order = excluded.provider_order, provider_profiles = excluded.provider_profiles, tier_per_reason = excluded.tier_per_reason, + role_tiers = excluded.role_tiers, updated_at = excluded.updated_at - `), workspaceID, enabled, tier, orderRaw, profsRaw, reasonsRaw, time.Now().UTC()) + `), workspaceID, enabled, tier, orderRaw, profsRaw, reasonsRaw, rolesRaw, time.Now().UTC()) if err != nil { return fmt.Errorf("routing: upsert workspace %q: %w", workspaceID, err) } @@ -123,6 +148,7 @@ func defaultWorkspaceRouting() *routing.WorkspaceConfig { ProviderOrder: []routing.ProviderID{}, ProviderProfiles: map[routing.ProviderID]routing.ProviderProfile{}, TierPerReason: routing.TierPerReason{}, + RoleTiers: routing.RoleTierMap{}, } } @@ -215,6 +241,26 @@ func marshalTierPerReason(m routing.TierPerReason) (string, error) { return string(b), nil } +// marshalRoleTiers marshals the role tier policy, normalising nil to +// "{}" and dropping empty-string values (AC-11: an empty value means +// "absent, fall through to default_tier", not "explicitly set to a +// blank tier" — dropping it here keeps GetWorkspaceRouting's read path +// from ever having to special-case a persisted empty-string entry). +func marshalRoleTiers(m routing.RoleTierMap) (string, error) { + out := make(routing.RoleTierMap, len(m)) + for role, tier := range m { + if tier == "" { + continue + } + out[role] = tier + } + b, err := json.Marshal(out) + if err != nil { + return "", fmt.Errorf("routing: marshal role_tiers: %w", err) + } + return string(b), nil +} + // marshalOrder marshals the provider order, normalising nil to "[]" // so the column's NOT NULL default invariant is preserved. func marshalOrder(order []routing.ProviderID) (string, error) { diff --git a/apps/backend/internal/office/routing/provider.go b/apps/backend/internal/office/routing/provider.go index a557af6b99..eb758e1829 100644 --- a/apps/backend/internal/office/routing/provider.go +++ b/apps/backend/internal/office/routing/provider.go @@ -22,8 +22,15 @@ import ( // eligible candidate); when not degraded these equal the primary. // When every provider is skipped, both Current fields are empty. type PreviewItem struct { - AgentID string - AgentName string + AgentID string + AgentName string + // TierSource names the precedence level supplying EffectiveTier: + // one of TierSourceWakeReason, TierSourceOverride, TierSourceRole, + // TierSourceWorkspace (see effectiveTier). Always computed with an + // empty reason (AC-18b), so this never reports TierSourceWakeReason + // even when the agent's own wake-reason policy would shadow it at + // runtime — the frontend surfaces that shadowing separately from + // the agent's TierPerReason data, not from this field. TierSource string EffectiveTier string PrimaryProviderID string @@ -166,14 +173,15 @@ func (p *Provider) ListExecutionProfiles( // - enabled→disabled: parked runs would otherwise sit forever waiting // on a provider that no longer matters. // - enabled→enabled with a material change (provider order, default -// tier, or provider profiles): a user fixing a missing tier mapping -// should unblock blocked_provider_action_required runs immediately; -// without this, those parks persist until the user manually retries. +// tier, role tiers, or provider profiles): a user fixing a missing +// tier mapping should unblock blocked_provider_action_required runs +// immediately; without this, those parks persist until the user +// manually retries. // // "Material" is defined coarsely — any change to ProviderOrder, -// DefaultTier, or ProviderProfiles triggers a clear. False positives -// (clearing when the change couldn't affect any block reason) are -// harmless because runs simply re-dispatch and re-park with the +// DefaultTier, RoleTiers, or ProviderProfiles triggers a clear. False +// positives (clearing when the change couldn't affect any block reason) +// are harmless because runs simply re-dispatch and re-park with the // latest verdict. func (p *Provider) UpdateConfig( ctx context.Context, workspaceID string, cfg WorkspaceConfig, @@ -396,8 +404,8 @@ func shouldClearParked(prev *WorkspaceConfig, next WorkspaceConfig) bool { // routingConfigEqual reports whether two enabled configs are // behaviorally identical for routing decisions. Compares provider -// order, default tier, and per-provider profile maps. The Enabled -// field is intentionally not compared — callers check that. +// order, default tier, per-provider profile maps, and role tiers. The +// Enabled field is intentionally not compared — callers check that. func routingConfigEqual(a, b WorkspaceConfig) bool { if a.DefaultTier != b.DefaultTier { return false @@ -405,9 +413,24 @@ func routingConfigEqual(a, b WorkspaceConfig) bool { if !providerOrderEqual(a.ProviderOrder, b.ProviderOrder) { return false } + if !roleTiersEqual(a.RoleTiers, b.RoleTiers) { + return false + } return providerProfilesEqual(a.ProviderProfiles, b.ProviderProfiles) } +func roleTiersEqual(a, b RoleTierMap) bool { + if len(a) != len(b) { + return false + } + for k, va := range a { + if vb, ok := b[k]; !ok || va != vb { + return false + } + } + return true +} + func providerOrderEqual(a, b []ProviderID) bool { if len(a) != len(b) { return false @@ -567,11 +590,11 @@ func (p *Provider) previewForAgent( ctx context.Context, workspaceID string, agent models.AgentInstance, cfg *WorkspaceConfig, ) (PreviewItem, error) { - tierSource := tierSourceForAgent(agent, cfg) res, err := p.resolver.Resolve(ctx, workspaceID, agent, ResolveOptions{}) if err != nil { return PreviewItem{}, fmt.Errorf("routing: resolve %s: %w", agent.ID, err) } + tierSource := previewTierSource(res, agent, cfg) primaryProvider, primaryProfile, primaryModel := primaryProviderModel(res, cfg) return PreviewItem{ AgentID: agent.ID, @@ -607,17 +630,43 @@ func primaryProviderModel(res *Resolution, cfg *WorkspaceConfig) (string, string return string(first), prof.ExecutionProfileID(res.RequestedTier), prof.TierMap.Model(res.RequestedTier) } -// tierSourceForAgent returns "override" when the agent's settings flip -// TierSource explicitly; "inherit" otherwise. -func tierSourceForAgent(agent models.AgentInstance, _ *WorkspaceConfig) string { - ov, err := ReadAgentOverrides(agent.Settings) - if err != nil { - return "inherit" +// tierSourceForAgent delegates to effectiveTier — the resolve path's +// producer — rather than deciding independently, so preview and resolve +// never diverge on which precedence level supplied the tier (AC-20d). +// Always called with an empty reason (AC-18b): a preview never carries +// run context, so this can never report TierSourceWakeReason. A parse +// error on the agent's settings blob falls back to zero-value overrides +// rather than a fixed string, so the result still lands on one of the +// four widened values instead of the old error-only "inherit" literal. +func tierSourceForAgent(agent models.AgentInstance, cfg *WorkspaceConfig) string { + ov, _ := ReadAgentOverrides(agent.Settings) + if cfg == nil { + cfg = &WorkspaceConfig{} } - if ov.TierSource == TierSourceOverride && ov.Tier != "" { - return "override" + _, source := effectiveTier(cfg, ov, string(agent.Role), "") + return source +} + +// previewTierSource returns the source PreviewItem.TierSource must carry. +// AC-20d requires preview and resolve to share one producer AND never +// disagree — sharing the effectiveTier function alone is not enough, +// because previewForAgent's cfg snapshot and Resolve's own internal +// GetWorkspaceRouting read are two independent reads of the same row: a +// workspace routing write landing between them would otherwise let +// tierSourceForAgent(agent, cfg) answer from a config Resolve never saw. +// res.TierSource is therefore the primary source — it is exactly what +// Resolve computed against the config it actually used. The one +// exception is Resolve's disabled-empty-config early return +// (resolver.go, cfg == nil || (!cfg.Enabled && len(cfg.ProviderOrder) == +// 0)), which returns a Resolution that never calls effectiveTier and so +// carries an empty TierSource; previewForAgent's own contract still +// requires a non-blank preview row in that case, so it falls back to the +// snapshot computation only then. +func previewTierSource(res *Resolution, agent models.AgentInstance, cfg *WorkspaceConfig) string { + if res != nil && res.TierSource != "" { + return res.TierSource } - return "inherit" + return tierSourceForAgent(agent, cfg) } // effectivePreviewTier returns the tier the resolver consumed, falling diff --git a/apps/backend/internal/office/routing/provider_test.go b/apps/backend/internal/office/routing/provider_test.go index ec60c7bcd2..ef1db23e27 100644 --- a/apps/backend/internal/office/routing/provider_test.go +++ b/apps/backend/internal/office/routing/provider_test.go @@ -19,9 +19,22 @@ type fakeProviderRepo struct { byID map[string]*models.AgentInstance clearedParkedFor []string // workspace ids passed to ClearAllParked clearedParkedErr error + + // cfgSeq, when non-nil, makes GetWorkspaceRouting return a different + // config on each successive call (advancing cfgSeqIdx), simulating a + // workspace routing row changing between two reads within a single + // logical operation (AC-20d race coverage). Falls back to cfg once + // the sequence is exhausted. + cfgSeq []*WorkspaceConfig + cfgSeqIdx int } func (f *fakeProviderRepo) GetWorkspaceRouting(_ context.Context, _ string) (*WorkspaceConfig, error) { + if f.cfgSeqIdx < len(f.cfgSeq) { + cfg := f.cfgSeq[f.cfgSeqIdx] + f.cfgSeqIdx++ + return cfg, nil + } return f.cfg, nil } @@ -414,6 +427,61 @@ func TestProvider_PreviewAgentRoundTrips(t *testing.T) { } } +// AC-20d: PreviewItem.TierSource must equal what Resolve actually used, +// never a value computed from a separately-read, potentially stale copy +// of the workspace config. previewForAgent reads the workspace config +// once for tierSourceForAgent and Resolve reads it again internally; +// this simulates a routing config write landing between those two reads +// and asserts the preview reports the same source the live resolve did, +// not the stale snapshot's answer. +func TestProvider_PreviewAgentTierSourceMatchesResolveDespiteConfigRaceBetweenReads(t *testing.T) { + profiles := map[ProviderID]ProviderProfile{ + "claude-acp": { + ExecutionProfileIDs: ExecutionProfileIDs{Frontier: "claude-opus", Balanced: "claude-sonnet", Economy: "claude-haiku"}, + TierMap: TierMap{Frontier: "opus", Balanced: "sonnet", Economy: "haiku"}, + }, + } + stale := &WorkspaceConfig{ + Enabled: true, + DefaultTier: TierBalanced, + ProviderOrder: []ProviderID{"claude-acp"}, + ProviderProfiles: profiles, + RoleTiers: RoleTierMap{"qa": TierFrontier}, + } + fresh := &WorkspaceConfig{ + Enabled: true, + DefaultTier: TierBalanced, + ProviderOrder: []ProviderID{"claude-acp"}, + ProviderProfiles: profiles, + RoleTiers: RoleTierMap{}, // role tier removed between the two reads + } + agent := &models.AgentInstance{ID: "a1", Name: "Alice", WorkspaceID: "ws-1", Role: settingsmodels.AgentRoleQA} + repo := &fakeProviderRepo{ + cfgSeq: []*WorkspaceConfig{stale, fresh}, + agents: []*models.AgentInstance{agent}, + byID: map[string]*models.AgentInstance{"a1": agent}, + } + resolver := NewResolver(&providerResolverAdapter{repo: repo}, nil) + p := NewProvider(repo, nil, resolver, &fakeRetry{}) + + item, err := p.PreviewAgent(context.Background(), "a1") + if err != nil { + t.Fatalf("PreviewAgent: %v", err) + } + if item == nil { + t.Fatal("PreviewAgent returned nil item") + return + } + if item.TierSource != TierSourceWorkspace { + t.Errorf("TierSource = %q, want %q — must match what Resolve actually computed against"+ + " the fresh config, not the stale role_tiers entry read earlier", + item.TierSource, TierSourceWorkspace) + } + if item.EffectiveTier != string(TierBalanced) { + t.Errorf("EffectiveTier = %q, want %q", item.EffectiveTier, TierBalanced) + } +} + func TestProvider_UpdateRejectsInvalid(t *testing.T) { cfg := &WorkspaceConfig{} p, _ := newProviderTest(cfg, nil) @@ -525,6 +593,37 @@ func TestProvider_UpdateClearsParkedWhenExecutionProfileChanges(t *testing.T) { } } +func TestProvider_UpdateClearsParkedOnRoleTiersOnlyChange(t *testing.T) { + prev := &WorkspaceConfig{ + Enabled: true, + DefaultTier: TierBalanced, + ProviderOrder: []ProviderID{"claude-acp"}, + ProviderProfiles: map[ProviderID]ProviderProfile{ + "claude-acp": { + TierMap: TierMap{Frontier: "opus", Balanced: "sonnet"}, + ExecutionProfileIDs: ExecutionProfileIDs{ + Frontier: "claude-opus", Balanced: "claude-sonnet", + }, + }, + }, + RoleTiers: RoleTierMap{"qa": TierFrontier}, + } + p, repo := newProviderTest(prev, nil) + // Only RoleTiers changes; DefaultTier, ProviderOrder, and + // ProviderProfiles stay identical. A run parked because the "qa" + // role resolved to frontier with no usable candidate should unblock + // once that role is remapped to balanced. + next := *prev + next.RoleTiers = RoleTierMap{"qa": TierBalanced} + + if err := p.UpdateConfig(context.Background(), "ws-1", next); err != nil { + t.Fatalf("update config: %v", err) + } + if len(repo.clearedParkedFor) != 1 || repo.clearedParkedFor[0] != "ws-1" { + t.Fatalf("role-tiers-only change did not clear parked runs: %v", repo.clearedParkedFor) + } +} + func TestProvider_UpdateDoesNotClearWhenConfigUnchanged(t *testing.T) { prev := &WorkspaceConfig{ Enabled: true, diff --git a/apps/backend/internal/office/routing/resolver.go b/apps/backend/internal/office/routing/resolver.go index 52339f3c84..c060b85fe6 100644 --- a/apps/backend/internal/office/routing/resolver.go +++ b/apps/backend/internal/office/routing/resolver.go @@ -124,8 +124,14 @@ type BlockReason struct { // fall through to the existing concrete-profile launch path and ignore // the other fields. type Resolution struct { - Enabled bool - RequestedTier Tier + Enabled bool + RequestedTier Tier + // TierSource names the precedence level that supplied + // RequestedTier: one of TierSourceWakeReason, TierSourceOverride, + // TierSourceRole, TierSourceWorkspace, or "" (only when + // RequestedTier is also "", see effectiveTier). This is the sole + // carrier of the resolved source — it is not recomputed downstream. + TierSource string ProviderOrder []ProviderID Candidates []Candidate SkippedDegraded []SkippedCandidate @@ -174,12 +180,17 @@ func (r *Resolver) Resolve( if err != nil { return nil, fmt.Errorf("routing: load agent overrides: %w", err) } - tier := effectiveTier(cfg, ov, opts.Reason) + tier, tierSource := effectiveTier(cfg, ov, string(agent.Role), opts.Reason) order := effectiveOrder(cfg, ov) if len(order) == 0 { return nil, ErrEmptyOrder } - res := &Resolution{Enabled: cfg.Enabled, RequestedTier: tier, ProviderOrder: order} + res := &Resolution{ + Enabled: cfg.Enabled, + RequestedTier: tier, + TierSource: tierSource, + ProviderOrder: order, + } idx, err := r.loadHealthIndex(ctx, workspaceID) if err != nil { return nil, err @@ -414,21 +425,47 @@ func aggregateBlock(skipped []SkippedCandidate) BlockReason { return br } -// effectiveTier resolves the tier for one run. +// effectiveTier resolves the tier for one run and the precedence level +// that supplied it (see Resolution.TierSource and PreviewItem.TierSource). // Order: 1) wake-reason policy (agent override > workspace policy), -// 2) agent tier override, 3) workspace default. The reason argument -// may be empty when the caller has no run context — in that case the -// wake-reason step is skipped. -func effectiveTier(cfg *WorkspaceConfig, ov AgentOverrides, reason string) Tier { +// 2) agent tier override, 3) per-role tier (role's entry in +// cfg.RoleTiers), 4) workspace default. The reason argument may be +// empty when the caller has no run context — in that case the +// wake-reason step is skipped (this is how a preview computation, +// which never carries a reason, is guaranteed to never report +// TierSourceWakeReason — see AC-18b). role is the agent's role string; +// an empty role, or a role absent from (or empty-valued in) +// cfg.RoleTiers, simply never matches the map lookup and falls through. +// A non-empty role outside the seven-value enum is explicitly gated by +// agentRoleSet() (AC-34): role_tiers may persist a matching key for such +// a role (AC-34b permits it on the read path), so the lookup itself +// would otherwise match — the enum check exists specifically to stop +// that from applying. When cfg.DefaultTier is itself empty, the function +// returns ("", "") rather than (DefaultTier, TierSourceWorkspace): an +// empty source must never be interpreted as "workspace" (AC-20c). This +// cannot happen in practice (validateTier rejects an empty default_tier +// and the column default is non-empty), but the fallthrough is pinned +// defensively anyway. +func effectiveTier(cfg *WorkspaceConfig, ov AgentOverrides, role string, reason string) (Tier, string) { if reason != "" { if t := wakeReasonTier(cfg, ov, reason); t != "" { - return t + return t, TierSourceWakeReason } } if ov.TierSource == TierSourceOverride && ov.Tier != "" { - return ov.Tier + return ov.Tier, TierSourceOverride } - return cfg.DefaultTier + if role != "" { + if _, known := agentRoleSet()[role]; known { + if t, ok := cfg.RoleTiers[role]; ok && t != "" { + return t, TierSourceRole + } + } + } + if cfg.DefaultTier == "" { + return "", "" + } + return cfg.DefaultTier, TierSourceWorkspace } // wakeReasonTier returns the tier the wake-reason policy assigns for diff --git a/apps/backend/internal/office/routing/role_tiers_resolve_test.go b/apps/backend/internal/office/routing/role_tiers_resolve_test.go new file mode 100644 index 0000000000..f391d0b5f8 --- /dev/null +++ b/apps/backend/internal/office/routing/role_tiers_resolve_test.go @@ -0,0 +1,234 @@ +package routing_test + +import ( + "context" + "testing" + + settingsmodels "github.com/kandev/kandev/internal/agent/settings/models" + "github.com/kandev/kandev/internal/office/routing" +) + +// agentWithRole builds a resolver-ready agent profile carrying role and an +// optional overrides blob, mirroring agentWithOverrides but adding the Role +// field the widened effectiveTier precedence needs. +func agentWithRole(t *testing.T, role settingsmodels.AgentRole, ov routing.AgentOverrides) settingsmodels.AgentProfile { + t.Helper() + agent := agentWithOverrides(t, ov) + agent.Role = role + return agent +} + +// AC-3/AC-7 style: with no wake reason, no per-agent override, and no +// matching role entry, the workspace default_tier supplies the tier and +// TierSource reports "workspace". +func TestResolve_TierSource_WorkspaceDefault_NoRoleEntry(t *testing.T) { + cfg := twoProviderCfg() + repo := &fakeRepo{cfg: cfg} + r := newResolver(t, repo) + + res, err := r.Resolve(context.Background(), wsID, + agentWithRole(t, settingsmodels.AgentRoleQA, routing.AgentOverrides{}), + routing.ResolveOptions{}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if res.RequestedTier != cfg.DefaultTier { + t.Errorf("RequestedTier = %q, want workspace default %q", res.RequestedTier, cfg.DefaultTier) + } + if res.TierSource != routing.TierSourceWorkspace { + t.Errorf("TierSource = %q, want %q", res.TierSource, routing.TierSourceWorkspace) + } +} + +// AC-11 fallthrough at the resolve layer: a role_tiers entry present but +// set to "" behaves identically to the role being entirely absent from +// the map — falls through to the workspace default. +func TestResolve_TierSource_EmptyRoleEntryFallsThroughToWorkspace(t *testing.T) { + cfg := twoProviderCfg() + cfg.RoleTiers = routing.RoleTierMap{"qa": ""} + repo := &fakeRepo{cfg: cfg} + r := newResolver(t, repo) + + res, err := r.Resolve(context.Background(), wsID, + agentWithRole(t, settingsmodels.AgentRoleQA, routing.AgentOverrides{}), + routing.ResolveOptions{}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if res.RequestedTier != cfg.DefaultTier || res.TierSource != routing.TierSourceWorkspace { + t.Errorf("got (%q,%q), want (%q,%q)", + res.RequestedTier, res.TierSource, cfg.DefaultTier, routing.TierSourceWorkspace) + } +} + +// The new level 3: a role_tiers entry matching the agent's role supplies +// the tier, below any per-agent override and above the workspace default. +func TestResolve_TierSource_RoleTierApplies(t *testing.T) { + cfg := twoProviderCfg() + cfg.RoleTiers = routing.RoleTierMap{"qa": routing.TierEconomy} + repo := &fakeRepo{cfg: cfg} + r := newResolver(t, repo) + + res, err := r.Resolve(context.Background(), wsID, + agentWithRole(t, settingsmodels.AgentRoleQA, routing.AgentOverrides{}), + routing.ResolveOptions{}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if res.RequestedTier != routing.TierEconomy { + t.Errorf("RequestedTier = %q, want economy", res.RequestedTier) + } + if res.TierSource != routing.TierSourceRole { + t.Errorf("TierSource = %q, want %q", res.TierSource, routing.TierSourceRole) + } +} + +// A role_tiers entry for a different role than the agent's own must not +// apply — it falls through to the workspace default exactly as if +// role_tiers were empty. +func TestResolve_TierSource_RoleTierForDifferentRoleDoesNotApply(t *testing.T) { + cfg := twoProviderCfg() + cfg.RoleTiers = routing.RoleTierMap{"devops": routing.TierEconomy} + repo := &fakeRepo{cfg: cfg} + r := newResolver(t, repo) + + res, err := r.Resolve(context.Background(), wsID, + agentWithRole(t, settingsmodels.AgentRoleQA, routing.AgentOverrides{}), + routing.ResolveOptions{}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if res.RequestedTier != cfg.DefaultTier || res.TierSource != routing.TierSourceWorkspace { + t.Errorf("got (%q,%q), want workspace default", res.RequestedTier, res.TierSource) + } +} + +// Level 2 beats level 3: a per-agent tier override outranks the agent's +// role tier. +func TestResolve_TierSource_AgentOverrideBeatsRoleTier(t *testing.T) { + cfg := twoProviderCfg() + cfg.RoleTiers = routing.RoleTierMap{"qa": routing.TierEconomy} + repo := &fakeRepo{cfg: cfg} + r := newResolver(t, repo) + + ov := routing.AgentOverrides{TierSource: routing.TierSourceOverride, Tier: routing.TierFrontier} + res, err := r.Resolve(context.Background(), wsID, + agentWithRole(t, settingsmodels.AgentRoleQA, ov), + routing.ResolveOptions{}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if res.RequestedTier != routing.TierFrontier { + t.Errorf("RequestedTier = %q, want frontier (agent override)", res.RequestedTier) + } + if res.TierSource != routing.TierSourceOverride { + t.Errorf("TierSource = %q, want %q", res.TierSource, routing.TierSourceOverride) + } +} + +// Level 1 beats everything: a matching wake-reason policy outranks both +// the per-agent override and the role tier. +func TestResolve_TierSource_WakeReasonBeatsOverrideAndRoleTier(t *testing.T) { + cfg := twoProviderCfg() + cfg.TierPerReason = routing.TierPerReason{routing.WakeReasonHeartbeat: routing.TierEconomy} + cfg.RoleTiers = routing.RoleTierMap{"qa": routing.TierFrontier} + repo := &fakeRepo{cfg: cfg} + r := newResolver(t, repo) + + ov := routing.AgentOverrides{TierSource: routing.TierSourceOverride, Tier: routing.TierBalanced} + res, err := r.Resolve(context.Background(), wsID, + agentWithRole(t, settingsmodels.AgentRoleQA, ov), + routing.ResolveOptions{Reason: routing.WakeReasonHeartbeat}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if res.RequestedTier != routing.TierEconomy { + t.Errorf("RequestedTier = %q, want economy (wake reason)", res.RequestedTier) + } + if res.TierSource != routing.TierSourceWakeReason { + t.Errorf("TierSource = %q, want %q", res.TierSource, routing.TierSourceWakeReason) + } +} + +// AC-34: an agent whose role is outside the seven-value enum (a row +// written by an older build or by hand) must never consult role_tiers, +// even when the persisted map happens to hold a matching non-enum key +// (AC-34b permits such a key to exist on the read path). Resolution +// falls through to the workspace default exactly as if role_tiers were +// empty for this agent. +func TestResolve_TierSource_NonEnumRoleIgnoresMatchingRoleTiersKey(t *testing.T) { + cfg := twoProviderCfg() + cfg.RoleTiers = routing.RoleTierMap{"legacy-role": routing.TierFrontier} + repo := &fakeRepo{cfg: cfg} + r := newResolver(t, repo) + + res, err := r.Resolve(context.Background(), wsID, + agentWithRole(t, settingsmodels.AgentRole("legacy-role"), routing.AgentOverrides{}), + routing.ResolveOptions{}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if res.RequestedTier != cfg.DefaultTier { + t.Errorf("RequestedTier = %q, want workspace default %q (non-enum role must not match role_tiers)", + res.RequestedTier, cfg.DefaultTier) + } + if res.TierSource != routing.TierSourceWorkspace { + t.Errorf("TierSource = %q, want %q", res.TierSource, routing.TierSourceWorkspace) + } +} + +// AC-6: an agent whose role is the empty string must never consult +// role_tiers at all, regardless of the map's contents. This is the +// strongest form of the guard: role_tiers holds an entry keyed by the +// empty string (unreachable via the write path, since AC-8 restricts +// keys to the seven-value enum, but a resolver read-path lookup does not +// itself know that) to prove the `role != ""` gate in effectiveTier, not +// merely the absence of an empty key in a realistic fixture. +func TestResolve_TierSource_EmptyRoleNeverConsultsRoleTiers(t *testing.T) { + cfg := twoProviderCfg() + cfg.RoleTiers = routing.RoleTierMap{"": routing.TierFrontier} + repo := &fakeRepo{cfg: cfg} + r := newResolver(t, repo) + + res, err := r.Resolve(context.Background(), wsID, + agentWithRole(t, settingsmodels.AgentRole(""), routing.AgentOverrides{}), + routing.ResolveOptions{}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if res.RequestedTier != cfg.DefaultTier { + t.Errorf("RequestedTier = %q, want workspace default %q (empty role must not match role_tiers)", + res.RequestedTier, cfg.DefaultTier) + } + if res.TierSource != routing.TierSourceWorkspace { + t.Errorf("TierSource = %q, want %q", res.TierSource, routing.TierSourceWorkspace) + } +} + +// AC-12a: a disabled workspace with a non-empty provider order still +// resolves the role tier — Enabled only gates automatic fallback, not +// tier resolution. This is the branch that does NOT hit Resolve's +// "disabled and empty order" early return. +func TestResolve_TierSource_RoleTierAppliesWhenDisabled(t *testing.T) { + cfg := twoProviderCfg() + cfg.Enabled = false + cfg.RoleTiers = routing.RoleTierMap{"qa": routing.TierEconomy} + repo := &fakeRepo{cfg: cfg} + r := newResolver(t, repo) + + res, err := r.Resolve(context.Background(), wsID, + agentWithRole(t, settingsmodels.AgentRoleQA, routing.AgentOverrides{}), + routing.ResolveOptions{}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if res.Enabled { + t.Fatalf("expected Enabled=false to be preserved") + } + if res.RequestedTier != routing.TierEconomy { + t.Errorf("RequestedTier = %q, want economy even though disabled", res.RequestedTier) + } + if res.TierSource != routing.TierSourceRole { + t.Errorf("TierSource = %q, want %q", res.TierSource, routing.TierSourceRole) + } +} diff --git a/apps/backend/internal/office/routing/role_tiers_test.go b/apps/backend/internal/office/routing/role_tiers_test.go new file mode 100644 index 0000000000..7d6f9acd5d --- /dev/null +++ b/apps/backend/internal/office/routing/role_tiers_test.go @@ -0,0 +1,191 @@ +package routing_test + +import ( + "errors" + "strings" + "testing" + + "github.com/kandev/kandev/internal/office/routing" +) + +// roleTiersCfg returns a minimal enabled config with one provider mapping +// all three tiers, used as the base for role_tiers validation cases. +func roleTiersCfg() routing.WorkspaceConfig { + return routing.WorkspaceConfig{ + Enabled: true, + DefaultTier: routing.TierBalanced, + ProviderOrder: []routing.ProviderID{"claude-acp"}, + ProviderProfiles: map[routing.ProviderID]routing.ProviderProfile{ + "claude-acp": { + ExecutionProfileIDs: routing.ExecutionProfileIDs{ + Frontier: "claude-opus-profile", + Balanced: "claude-sonnet-profile", + Economy: "claude-haiku-profile", + }, + TierMap: routing.TierMap{ + Frontier: "opus", + Balanced: "sonnet", + Economy: "haiku", + }, + }, + }, + } +} + +// AC-8: an unknown role key is rejected with a per-key ValidationDetail. +func TestValidateWorkspaceConfig_RoleTiers_UnknownRole(t *testing.T) { + cfg := roleTiersCfg() + cfg.RoleTiers = routing.RoleTierMap{"astronaut": routing.TierEconomy} + + err := routing.ValidateWorkspaceConfig(cfg, testKnown) + var ve *routing.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *ValidationError, got %v", err) + } + if ve.Field != "role_tiers" { + t.Fatalf("Field = %q, want role_tiers", ve.Field) + } + if len(ve.Details) != 1 || ve.Details[0].Field != "astronaut" { + t.Fatalf("Details = %+v, want one entry for astronaut", ve.Details) + } +} + +// AC-9: an invalid tier value is rejected. The message must not carry +// "default_tier" or the generic "routing config invalid:" prefix that +// validateTier's own Error() produces — those would misattribute the +// field to the workspace default-tier validator instead of role_tiers. +func TestValidateWorkspaceConfig_RoleTiers_InvalidTierValue(t *testing.T) { + cfg := roleTiersCfg() + cfg.RoleTiers = routing.RoleTierMap{"qa": routing.Tier("nonsense")} + + err := routing.ValidateWorkspaceConfig(cfg, testKnown) + var ve *routing.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *ValidationError, got %v", err) + } + if ve.Field != "role_tiers" { + t.Fatalf("Field = %q, want role_tiers", ve.Field) + } + if len(ve.Details) != 1 || ve.Details[0].Field != "qa" { + t.Fatalf("Details = %+v, want one entry for qa", ve.Details) + } + if strings.Contains(ve.Details[0].Message, "default_tier") { + t.Fatalf("detail message leaked default_tier field: %q", ve.Details[0].Message) + } + full := ve.Error() + if strings.Contains(full, "routing config invalid: default_tier") { + t.Fatalf("rendered error misattributes to default_tier: %q", full) + } +} + +// AC-10/AC-10a: a role tier that is valid but not mapped on any provider +// in the workspace order is rejected, accumulating per-role details the +// same way checkTierPerReasonMapped does (not the single-value shape +// checkTierMapped uses for the agent-override tier). +func TestValidateWorkspaceConfig_RoleTiers_UnmappedTier(t *testing.T) { + cfg := roleTiersCfg() + // claude-acp has no frontier mapping in this variant. + prof := cfg.ProviderProfiles["claude-acp"] + prof.ExecutionProfileIDs.Frontier = "" + prof.TierMap.Frontier = "" + cfg.ProviderProfiles["claude-acp"] = prof + cfg.RoleTiers = routing.RoleTierMap{"ceo": routing.TierFrontier} + + err := routing.ValidateWorkspaceConfig(cfg, testKnown) + var ve *routing.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *ValidationError, got %v", err) + } + if ve.Field != "role_tiers" { + t.Fatalf("Field = %q, want role_tiers", ve.Field) + } + if len(ve.Details) != 1 || ve.Details[0].Field != "ceo" { + t.Fatalf("Details = %+v, want one entry for ceo", ve.Details) + } +} + +// AC-11: an empty tier value means "absent" and is silently tolerated, +// both by the unknown-role/invalid-value check and the unmapped-tier +// check. +func TestValidateWorkspaceConfig_RoleTiers_EmptyValueIsAbsent(t *testing.T) { + cfg := roleTiersCfg() + cfg.RoleTiers = routing.RoleTierMap{"worker": ""} + + if err := routing.ValidateWorkspaceConfig(cfg, testKnown); err != nil { + t.Fatalf("empty role tier value should validate cleanly, got %v", err) + } +} + +// AC-12: role_tiers is validated identically regardless of cfg.Enabled — +// checkRoleTiersMapped is not gated behind the Enabled=true block, unlike +// the provider-order/default-tier checks in validateEnabledRules. +func TestValidateWorkspaceConfig_RoleTiers_ValidatedWhenDisabled(t *testing.T) { + cfg := roleTiersCfg() + cfg.Enabled = false + cfg.RoleTiers = routing.RoleTierMap{"astronaut": routing.TierEconomy} + + err := routing.ValidateWorkspaceConfig(cfg, testKnown) + var ve *routing.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected role_tiers to be validated while disabled, got %v", err) + } + if ve.Field != "role_tiers" { + t.Fatalf("Field = %q, want role_tiers", ve.Field) + } +} + +// AC-40 test #3: two bad role_tiers entries in a single write must +// produce two Details entries, not collapse into one flat error. +func TestValidateWorkspaceConfig_RoleTiers_TwoBadEntriesAccumulate(t *testing.T) { + cfg := roleTiersCfg() + cfg.RoleTiers = routing.RoleTierMap{ + "astronaut": routing.TierEconomy, // unknown role + "qa": routing.Tier("nonsense"), // invalid tier value + } + + err := routing.ValidateWorkspaceConfig(cfg, testKnown) + var ve *routing.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *ValidationError, got %v", err) + } + if len(ve.Details) != 2 { + t.Fatalf("Details = %+v, want 2 entries", ve.Details) + } + fields := map[string]bool{} + for _, d := range ve.Details { + fields[d.Field] = true + } + if !fields["astronaut"] || !fields["qa"] { + t.Fatalf("Details fields = %v, want astronaut and qa", fields) + } +} + +// A fully valid role_tiers map (every AllAgentRoles key, every value +// mapped) must pass cleanly. +func TestValidateWorkspaceConfig_RoleTiers_AllRolesValid(t *testing.T) { + cfg := roleTiersCfg() + cfg.RoleTiers = routing.RoleTierMap{ + "ceo": routing.TierFrontier, + "worker": routing.TierEconomy, + "specialist": routing.TierBalanced, + "assistant": routing.TierEconomy, + "security": routing.TierBalanced, + "qa": routing.TierEconomy, + "devops": routing.TierBalanced, + } + + if err := routing.ValidateWorkspaceConfig(cfg, testKnown); err != nil { + t.Fatalf("all-known-roles config should validate cleanly, got %v", err) + } +} + +// A nil/missing role_tiers map means "no role policy" and must never be +// rejected. +func TestValidateWorkspaceConfig_RoleTiers_NilMapIsValid(t *testing.T) { + cfg := roleTiersCfg() + cfg.RoleTiers = nil + + if err := routing.ValidateWorkspaceConfig(cfg, testKnown); err != nil { + t.Fatalf("nil role_tiers should validate cleanly, got %v", err) + } +} diff --git a/apps/backend/internal/office/routing/tier_source_test.go b/apps/backend/internal/office/routing/tier_source_test.go new file mode 100644 index 0000000000..bdaddaeb47 --- /dev/null +++ b/apps/backend/internal/office/routing/tier_source_test.go @@ -0,0 +1,118 @@ +package routing + +import ( + "encoding/json" + "testing" + + settingsmodels "github.com/kandev/kandev/internal/agent/settings/models" +) + +// agentSettingsWithOverrides marshals ov under the "routing" settings key, +// mirroring the production agent_settings.go shape. +func agentSettingsWithOverrides(t *testing.T, ov AgentOverrides) string { + t.Helper() + if ov.IsZero() { + return "" + } + blob, err := json.Marshal(map[string]interface{}{"routing": ov}) + if err != nil { + t.Fatalf("marshal overrides: %v", err) + } + return string(blob) +} + +// AC-16/AC-20d: tierSourceForAgent delegates to the same effectiveTier +// producer the resolver uses, so a role_tiers entry reports "role" here +// exactly as it would on a live Resolve call. +func TestTierSourceForAgent_RoleTier(t *testing.T) { + cfg := &WorkspaceConfig{ + DefaultTier: TierBalanced, + RoleTiers: RoleTierMap{"qa": TierEconomy}, + } + agent := settingsmodels.AgentProfile{Role: settingsmodels.AgentRoleQA} + + if got := tierSourceForAgent(agent, cfg); got != TierSourceRole { + t.Errorf("tierSourceForAgent = %q, want %q", got, TierSourceRole) + } +} + +// A per-agent override outranks a matching role tier. +func TestTierSourceForAgent_OverrideBeatsRoleTier(t *testing.T) { + cfg := &WorkspaceConfig{ + DefaultTier: TierBalanced, + RoleTiers: RoleTierMap{"qa": TierEconomy}, + } + ov := AgentOverrides{TierSource: TierSourceOverride, Tier: TierFrontier} + agent := settingsmodels.AgentProfile{ + Role: settingsmodels.AgentRoleQA, + Settings: agentSettingsWithOverrides(t, ov), + } + + if got := tierSourceForAgent(agent, cfg); got != TierSourceOverride { + t.Errorf("tierSourceForAgent = %q, want %q", got, TierSourceOverride) + } +} + +// No override, no matching role entry: falls through to the workspace +// default. +func TestTierSourceForAgent_WorkspaceDefault(t *testing.T) { + cfg := &WorkspaceConfig{DefaultTier: TierBalanced} + agent := settingsmodels.AgentProfile{Role: settingsmodels.AgentRoleQA} + + if got := tierSourceForAgent(agent, cfg); got != TierSourceWorkspace { + t.Errorf("tierSourceForAgent = %q, want %q", got, TierSourceWorkspace) + } +} + +// AC-18b: a preview computation never carries wake-reason context. +// tierSourceForAgent always calls effectiveTier with an empty reason, so +// even when the agent's own wake-reason override would shadow the tier +// at runtime, the preview must never report "wake_reason". +func TestTierSourceForAgent_NeverReportsWakeReason(t *testing.T) { + cfg := &WorkspaceConfig{ + DefaultTier: TierBalanced, + TierPerReason: TierPerReason{WakeReasonHeartbeat: TierEconomy}, + } + ov := AgentOverrides{ + TierPerReasonSource: TierPerReasonSourceOverride, + TierPerReason: TierPerReason{WakeReasonHeartbeat: TierFrontier}, + } + agent := settingsmodels.AgentProfile{ + Role: settingsmodels.AgentRoleQA, + Settings: agentSettingsWithOverrides(t, ov), + } + + got := tierSourceForAgent(agent, cfg) + if got == TierSourceWakeReason { + t.Fatalf("tierSourceForAgent reported wake_reason for a reason-less preview") + } + if got != TierSourceWorkspace { + t.Errorf("tierSourceForAgent = %q, want %q", got, TierSourceWorkspace) + } +} + +// A nil cfg (defensive edge case — Provider always constructs a default +// before calling this, but the helper is exported to package-internal +// callers only) must not panic and falls through to the empty-source +// pinned-defensive case in effectiveTier. +func TestTierSourceForAgent_NilConfigDoesNotPanic(t *testing.T) { + agent := settingsmodels.AgentProfile{Role: settingsmodels.AgentRoleQA} + if got := tierSourceForAgent(agent, nil); got != "" { + t.Errorf("tierSourceForAgent(nil cfg) = %q, want empty", got) + } +} + +// A malformed settings blob falls back to zero-value overrides rather +// than erroring the preview (SR-23): the tier source still resolves via +// the normal precedence chain instead of surfacing a fixed sentinel. +func TestTierSourceForAgent_MalformedSettingsFallsBackToZeroOverrides(t *testing.T) { + cfg := &WorkspaceConfig{DefaultTier: TierBalanced} + agent := settingsmodels.AgentProfile{ + Role: settingsmodels.AgentRoleQA, + Settings: "{not valid json", + } + + if got := tierSourceForAgent(agent, cfg); got != TierSourceWorkspace { + t.Errorf("tierSourceForAgent = %q, want %q (fell back to workspace default)", got, TierSourceWorkspace) + } +} diff --git a/apps/backend/internal/office/routing/types.go b/apps/backend/internal/office/routing/types.go index 922bdf10df..6503b27c54 100644 --- a/apps/backend/internal/office/routing/types.go +++ b/apps/backend/internal/office/routing/types.go @@ -12,6 +12,8 @@ import ( "errors" "fmt" "strings" + + settingsmodels "github.com/kandev/kandev/internal/agent/settings/models" ) // Tier is a normalized model strength bucket. Workspace and agent @@ -37,6 +39,19 @@ const ProviderOrderSourceOverride = "override" // that the override blob's Tier replaces the workspace default. const TierSourceOverride = "override" +// Tier-resolution level names for the widened, four-value tier_source +// contract computed by effectiveTier and carried on Resolution and +// RouteAttempt: wake_reason | override | role | workspace. This is a +// different contract from AgentOverrides.TierSource above (still +// inherit | override | "" — a persisted "did the user override" +// marker, not a resolution level) even though both are spelled +// "tier_source" and share the "override" string. See spec AC-18a. +const ( + TierSourceWakeReason = "wake_reason" + TierSourceRole = "role" + TierSourceWorkspace = "workspace" +) + // TierPerReasonSourceOverride is the magic value AgentOverrides uses to // signal that its TierPerReason map replaces the workspace policy // entirely. Any other value (default "") inherits the workspace map. @@ -67,6 +82,26 @@ var AllWakeReasons = []string{ // the agent overrides blob. type TierPerReason map[string]Tier +// AllAgentRoles enumerates the seven role_tiers keys, in the +// settingsmodels.AgentRole constants' declaration order. This order is +// also what the frontend role list follows, so it lives here as the +// single source of truth rather than being duplicated as a literal list. +var AllAgentRoles = []settingsmodels.AgentRole{ + settingsmodels.AgentRoleCEO, + settingsmodels.AgentRoleWorker, + settingsmodels.AgentRoleSpecialist, + settingsmodels.AgentRoleAssistant, + settingsmodels.AgentRoleSecurity, + settingsmodels.AgentRoleQA, + settingsmodels.AgentRoleDevOps, +} + +// RoleTierMap maps an agent role to the tier that role should run at. +// Keys are restricted to AllAgentRoles; values to AllTiers. Empty / +// missing keys mean "no role policy — fall through to the workspace +// default_tier." Stored as JSON on the workspace routing row. +type RoleTierMap map[string]Tier + // ProviderID identifies a CLI provider (the agent type ID from the // agent/registry package). First-class IDs in v1 are restricted by // catalogue.KnownProviders. @@ -224,6 +259,11 @@ type WorkspaceConfig struct { // empty / missing map means "no special policy — every reason uses // the agent's effective tier." TierPerReason TierPerReason `json:"tier_per_reason,omitempty"` + // RoleTiers maps an agent role onto the tier that role should run + // at, applied below TierPerReason and the per-agent override but + // above DefaultTier. An empty / missing map means "no role policy" + // — the default for every workspace. + RoleTiers RoleTierMap `json:"role_tiers,omitempty"` } // AgentOverrides is the routing override blob stored on @@ -296,6 +336,12 @@ func ValidateWorkspaceConfig(cfg WorkspaceConfig, known []ProviderID) error { if err := validateTierPerReason(cfg.TierPerReason, "tier_per_reason"); err != nil { return err } + if err := validateRoleTiers(cfg.RoleTiers, "role_tiers"); err != nil { + return err + } + if err := checkRoleTiersMapped(cfg.RoleTiers, &cfg); err != nil { + return err + } if !cfg.Enabled { return nil } @@ -506,6 +552,109 @@ func wakeReasonSet() map[string]struct{} { return out } +// validateRoleTiers rejects role_tiers keys outside the seven +// AllAgentRoles values and values outside AllTiers. Unlike +// validateTierPerReason (fail-fast on the first bad entry), +// role_tiers accumulates one ValidationDetail per offending key — +// role_tiers is a map that may carry several bad entries in a single +// write, and a single flat error would collapse them into one message +// and hide which of the seven roles is broken. Messages are built +// directly here rather than through validateTier, whose Field is +// hardcoded to "default_tier" and whose Error() text is prefixed with +// "routing config invalid:" — either leaking into a role_tiers message +// would misattribute the offending field. +func validateRoleTiers(m RoleTierMap, field string) error { + if len(m) == 0 { + return nil + } + knownRoles := agentRoleSet() + var details []ValidationDetail + for role, tier := range m { + if _, ok := knownRoles[role]; !ok { + details = append(details, ValidationDetail{ + Field: role, + Message: fmt.Sprintf("unknown role %q", role), + }) + continue + } + if tier == "" { + // AC-11: empty value means "absent"; dropped before + // persistence, not rejected. + continue + } + if !isValidTierValue(tier) { + details = append(details, ValidationDetail{ + Field: role, + Message: fmt.Sprintf("invalid tier %q", string(tier)), + }) + } + } + if len(details) == 0 { + return nil + } + return &ValidationError{ + Field: field, + Message: "role_tiers references unknown roles or invalid tiers", + Details: details, + } +} + +// isValidTierValue reports whether t is one of AllTiers. Unlike +// validateTier, it carries no field/message opinion — callers that need +// a role_tiers-scoped error build it themselves (see validateRoleTiers). +func isValidTierValue(t Tier) bool { + for _, v := range AllTiers { + if v == t { + return true + } + } + return false +} + +// agentRoleSet returns the lookup set used by validateRoleTiers. Built +// from AllAgentRoles so the source of truth stays single. +func agentRoleSet() map[string]struct{} { + out := make(map[string]struct{}, len(AllAgentRoles)) + for _, r := range AllAgentRoles { + out[string(r)] = struct{}{} + } + return out +} + +// checkRoleTiersMapped returns nil when every non-empty tier in +// role_tiers is mapped on at least one provider in cfg.ProviderOrder. +// Mirrors checkTierPerReasonMapped's accumulation shape rather than +// checkTierMapped's single-value shape, for the same reason +// validateRoleTiers does: role_tiers may hold several bad entries at +// once. Checked unconditionally (not gated on cfg.Enabled) because a +// role tier can supply the effective tier for a launch even while +// automatic fallback is disabled (AC-12/AC-12a). +func checkRoleTiersMapped(m RoleTierMap, cfg *WorkspaceConfig) error { + var details []ValidationDetail + for role, tier := range m { + if tier == "" { + continue + } + if tierMappedOnAnyProvider(tier, cfg.ProviderOrder, cfg.ProviderProfiles) { + continue + } + details = append(details, ValidationDetail{ + Field: role, + Message: fmt.Sprintf( + "tier %q is not mapped on any provider in the workspace provider order", + string(tier)), + }) + } + if len(details) == 0 { + return nil + } + return &ValidationError{ + Field: "role_tiers", + Message: "role tier overrides reference unmapped tiers", + Details: details, + } +} + // validateTier returns an error when t is not in AllTiers. Empty tier // is rejected because every config that reaches the validator should // already have a default applied. diff --git a/apps/backend/internal/office/scheduler/dispatch_routing.go b/apps/backend/internal/office/scheduler/dispatch_routing.go index 6d2dafb174..902d3af09e 100644 --- a/apps/backend/internal/office/scheduler/dispatch_routing.go +++ b/apps/backend/internal/office/scheduler/dispatch_routing.go @@ -202,6 +202,7 @@ func (ss *SchedulerService) parkRunBlocked( Seq: seq, ProviderID: string(sk.ProviderID), Tier: string(res.RequestedTier), + TierSource: res.TierSource, Outcome: skipOutcome(sk.Reason), ErrorCode: sk.ErrorCode, RawExcerpt: sk.RawExcerpt, @@ -252,7 +253,7 @@ func (ss *SchedulerService) tryCandidates( candidateLaunch := continuationLaunchContext( launch, prior, run.RouteCycleBaselineSeq, candidate, ) - seq, err := ss.recordAttemptStart(ctx, run, candidate, res.RequestedTier) + seq, err := ss.recordAttemptStart(ctx, run, candidate, res.RequestedTier, res.TierSource) if err != nil { return false, nil, err } @@ -352,10 +353,14 @@ func latestFailedExecutionProfile( } // recordAttemptStart increments the attempt sequence and appends the -// in-flight attempt row with outcome=launched. +// in-flight attempt row with outcome=launched. tierSource is the +// resolution's precedence level (routing.Resolution.TierSource) — +// threaded in from the caller, which has the *routing.Resolution in +// scope, rather than re-derived here (AC-20d names effectiveTier as +// the sole producer). func (ss *SchedulerService) recordAttemptStart( ctx context.Context, run *models.Run, - candidate routing.Candidate, tier routing.Tier, + candidate routing.Candidate, tier routing.Tier, tierSource string, ) (int, error) { seq, err := ss.repo.IncrementRouteAttemptSeq(ctx, run.ID) if err != nil { @@ -369,6 +374,7 @@ func (ss *SchedulerService) recordAttemptStart( ProviderID: string(candidate.ProviderID), Model: candidate.Model, Tier: string(tier), + TierSource: tierSource, Outcome: RouteAttemptOutcomeLaunched, StartedAt: time.Now().UTC(), } diff --git a/apps/backend/internal/office/scheduler/tier_source_dispatch_test.go b/apps/backend/internal/office/scheduler/tier_source_dispatch_test.go new file mode 100644 index 0000000000..0d6789401f --- /dev/null +++ b/apps/backend/internal/office/scheduler/tier_source_dispatch_test.go @@ -0,0 +1,295 @@ +package scheduler_test + +import ( + "context" + "fmt" + "testing" + "time" + + settingsmodels "github.com/kandev/kandev/internal/agent/settings/models" + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/events/bus" + officemodels "github.com/kandev/kandev/internal/office/models" + "github.com/kandev/kandev/internal/office/routing" + "github.com/kandev/kandev/internal/office/scheduler" +) + +// makeAgentWithRole mirrors makeAgent but sets the organisational Role so +// role_tiers precedence tests can exercise the resolver's role level. +func makeAgentWithRole(role settingsmodels.AgentRole) *officemodels.AgentInstance { + agent := makeAgent() + agent.Role = role + return agent +} + +// AC-20d/AC-20e: a launch that resolves at the workspace-default level +// persists tier_source="workspace" on the recorded attempt via +// recordAttemptStart. +func TestDispatch_TierSource_WorkspaceDefault(t *testing.T) { + repo := newTestRepoSched(t) + seedRoutingConfig(t, repo, []routing.ProviderID{"claude-acp", "codex-acp"}) + ss := buildScheduler(t, repo, newFakeTaskStarter()) + run := seedRun(t, repo, `{"task_id":"t-1"}`) + + launched, _, err := ss.DispatchWithRouting(context.Background(), run, makeAgent(), scheduler.LaunchContext{}) + if err != nil { + t.Fatalf("dispatch: %v", err) + } + if !launched { + t.Fatalf("expected launched=true") + } + attempts, err := repo.ListRouteAttempts(context.Background(), run.ID) + if err != nil || len(attempts) != 1 { + t.Fatalf("attempts = %v err=%v, want one attempt", attempts, err) + } + if attempts[0].TierSource != "workspace" { + t.Errorf("TierSource = %q, want workspace", attempts[0].TierSource) + } +} + +// The new role level: an agent whose role has a role_tiers entry +// launches at that tier, and the attempt records tier_source="role". +func TestDispatch_TierSource_RoleTier(t *testing.T) { + repo := newTestRepoSched(t) + profiles := map[routing.ProviderID]routing.ProviderProfile{ + "claude-acp": { + TierMap: routing.TierMap{Balanced: "sonnet", Economy: "haiku"}, + ExecutionProfileIDs: routing.ExecutionProfileIDs{ + Balanced: "claude-sonnet-profile", + Economy: "claude-haiku-profile", + }, + Mode: "default", + }, + } + cfg := &routing.WorkspaceConfig{ + Enabled: true, + DefaultTier: routing.TierBalanced, + ProviderOrder: []routing.ProviderID{"claude-acp"}, + ProviderProfiles: profiles, + RoleTiers: routing.RoleTierMap{"qa": routing.TierEconomy}, + } + if err := repo.UpsertWorkspaceRouting(context.Background(), testWorkspaceID, cfg); err != nil { + t.Fatalf("upsert routing: %v", err) + } + starter := newFakeTaskStarter() + ss := buildScheduler(t, repo, starter) + run := seedRun(t, repo, `{"task_id":"t-1"}`) + + launched, _, err := ss.DispatchWithRouting( + context.Background(), run, makeAgentWithRole(settingsmodels.AgentRoleQA), scheduler.LaunchContext{}) + if err != nil { + t.Fatalf("dispatch: %v", err) + } + if !launched { + t.Fatalf("expected launched=true") + } + got := starter.lastCall().route + if got.Tier != string(routing.TierEconomy) || got.Model != "haiku" { + t.Errorf("route = %+v, want economy/haiku", got) + } + attempts, err := repo.ListRouteAttempts(context.Background(), run.ID) + if err != nil || len(attempts) != 1 { + t.Fatalf("attempts = %v err=%v, want one attempt", attempts, err) + } + if attempts[0].TierSource != "role" { + t.Errorf("TierSource = %q, want role", attempts[0].TierSource) + } +} + +// The top level: a matching wake-reason policy resolves the tier and the +// attempt records tier_source="wake_reason", outranking a role entry. +func TestDispatch_TierSource_WakeReasonBeatsRole(t *testing.T) { + repo := newTestRepoSched(t) + profiles := map[routing.ProviderID]routing.ProviderProfile{ + "claude-acp": { + TierMap: routing.TierMap{Frontier: "opus", Balanced: "sonnet", Economy: "haiku"}, + ExecutionProfileIDs: routing.ExecutionProfileIDs{ + Frontier: "claude-opus-profile", + Balanced: "claude-sonnet-profile", + Economy: "claude-haiku-profile", + }, + Mode: "default", + }, + } + cfg := &routing.WorkspaceConfig{ + Enabled: true, + DefaultTier: routing.TierBalanced, + ProviderOrder: []routing.ProviderID{"claude-acp"}, + ProviderProfiles: profiles, + RoleTiers: routing.RoleTierMap{"qa": routing.TierFrontier}, + TierPerReason: routing.TierPerReason{ + routing.WakeReasonHeartbeat: routing.TierEconomy, + }, + } + if err := repo.UpsertWorkspaceRouting(context.Background(), testWorkspaceID, cfg); err != nil { + t.Fatalf("upsert routing: %v", err) + } + starter := newFakeTaskStarter() + ss := buildScheduler(t, repo, starter) + run := seedRun(t, repo, `{"task_id":"t-1"}`) + run.Reason = routing.WakeReasonHeartbeat + + launched, _, err := ss.DispatchWithRouting( + context.Background(), run, makeAgentWithRole(settingsmodels.AgentRoleQA), scheduler.LaunchContext{}) + if err != nil { + t.Fatalf("dispatch: %v", err) + } + if !launched { + t.Fatalf("expected launched=true") + } + got := starter.lastCall().route + if got.Tier != string(routing.TierEconomy) || got.Model != "haiku" { + t.Errorf("route = %+v, want economy/haiku (wake reason beats role)", got) + } + attempts, err := repo.ListRouteAttempts(context.Background(), run.ID) + if err != nil || len(attempts) != 1 { + t.Fatalf("attempts = %v err=%v, want one attempt", attempts, err) + } + if attempts[0].TierSource != "wake_reason" { + t.Errorf("TierSource = %q, want wake_reason", attempts[0].TierSource) + } +} + +// parkRunBlocked's skip-attempt rows also carry tier_source, propagated +// from the resolution's block-reason path rather than a launch attempt. +func TestDispatch_TierSource_ParkedBlockedRecordsSource(t *testing.T) { + repo := newTestRepoSched(t) + seedRoutingConfig(t, repo, []routing.ProviderID{"claude-acp"}) + starter := newFakeTaskStarter() + starter.failFor["claude-acp"] = fmt.Errorf("missing API key: not authenticated") + ss := buildScheduler(t, repo, starter) + run := seedRun(t, repo, `{"task_id":"t-1"}`) + + launched, parked, err := ss.DispatchWithRouting(context.Background(), run, makeAgent(), scheduler.LaunchContext{}) + if err != nil { + t.Fatalf("dispatch: %v", err) + } + if launched || !parked { + t.Fatalf("expected parked; got launched=%v parked=%v", launched, parked) + } + attempts, err := repo.ListRouteAttempts(context.Background(), run.ID) + if err != nil || len(attempts) != 1 { + t.Fatalf("attempts = %v err=%v, want one attempt", attempts, err) + } + if attempts[0].TierSource != "workspace" { + t.Errorf("TierSource = %q, want workspace", attempts[0].TierSource) + } +} + +// AC-20f/AC-20g: the terminal skipped_max_attempts row parkRunMaxAttempts +// records carries no Tier today, and correspondingly no TierSource — the +// cap fires before the resolver ever runs for this dispatch call. +func TestDispatch_TierSource_MaxAttemptsRowLeavesSourceEmpty(t *testing.T) { + repo := newTestRepoSched(t) + seedRoutingConfig(t, repo, []routing.ProviderID{"claude-acp", "codex-acp"}) + starter := newFakeTaskStarter() + ss := buildScheduler(t, repo, starter) + run := seedRun(t, repo, `{"task_id":"t-1"}`) + + for i := 0; i < scheduler.MaxAttemptsPerRun; i++ { + seq, err := repo.IncrementRouteAttemptSeq(context.Background(), run.ID) + if err != nil { + t.Fatalf("seed seq bump: %v", err) + } + if err := repo.AppendRouteAttempt(context.Background(), &officemodels.RouteAttempt{ + RunID: run.ID, + Seq: seq, + ProviderID: "claude-acp", + Model: "claude-acp-bal", + Tier: "balanced", + TierSource: "workspace", + Outcome: scheduler.RouteAttemptOutcomeFailedProviderUnavail, + StartedAt: time.Now().UTC(), + }); err != nil { + t.Fatalf("seed prior attempt: %v", err) + } + } + + launched, parked, err := ss.DispatchWithRouting(context.Background(), + run, makeAgent(), scheduler.LaunchContext{}) + if err != nil { + t.Fatalf("dispatch: %v", err) + } + if launched || !parked { + t.Fatalf("expected parked at cap; got launched=%v parked=%v", launched, parked) + } + attempts, err := repo.ListRouteAttempts(context.Background(), run.ID) + if err != nil { + t.Fatalf("list: %v", err) + } + last := attempts[len(attempts)-1] + if last.Outcome != scheduler.RouteAttemptOutcomeMaxAttempts { + t.Fatalf("last outcome = %q, want skipped_max_attempts", last.Outcome) + } + if last.TierSource != "" { + t.Errorf("TierSource = %q, want empty on the max-attempts terminal row", last.TierSource) + } +} + +// The route_attempt_appended WS event carries the same TierSource the +// persisted attempt row does — the payload is the raw models.RouteAttempt, +// not a re-derived value, so this pins the wiring rather than the value. +func TestDispatch_TierSource_PublishedOnRouteAttemptAppended(t *testing.T) { + repo := newTestRepoSched(t) + profiles := map[routing.ProviderID]routing.ProviderProfile{ + "claude-acp": { + TierMap: routing.TierMap{Balanced: "sonnet", Economy: "haiku"}, + ExecutionProfileIDs: routing.ExecutionProfileIDs{ + Balanced: "claude-sonnet-profile", + Economy: "claude-haiku-profile", + }, + Mode: "default", + }, + } + cfg := &routing.WorkspaceConfig{ + Enabled: true, + DefaultTier: routing.TierBalanced, + ProviderOrder: []routing.ProviderID{"claude-acp"}, + ProviderProfiles: profiles, + RoleTiers: routing.RoleTierMap{"qa": routing.TierEconomy}, + } + if err := repo.UpsertWorkspaceRouting(context.Background(), testWorkspaceID, cfg); err != nil { + t.Fatalf("upsert routing: %v", err) + } + ss := buildScheduler(t, repo, newFakeTaskStarter()) + + log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "console"}) + if err != nil { + t.Fatalf("logger: %v", err) + } + eb := bus.NewMemoryEventBus(log) + ss.SetEventBus(eb) + + var captured *officemodels.RouteAttempt + if _, err := eb.Subscribe(events.OfficeRouteAttemptAppended, func(_ context.Context, ev *bus.Event) error { + data, ok := ev.Data.(map[string]interface{}) + if !ok { + t.Fatalf("event data type = %T, want map[string]interface{}", ev.Data) + } + attempt, ok := data["attempt"].(officemodels.RouteAttempt) + if !ok { + t.Fatalf("attempt field type = %T, want officemodels.RouteAttempt", data["attempt"]) + } + captured = &attempt + return nil + }); err != nil { + t.Fatalf("subscribe: %v", err) + } + + run := seedRun(t, repo, `{"task_id":"t-1"}`) + launched, _, err := ss.DispatchWithRouting( + context.Background(), run, makeAgentWithRole(settingsmodels.AgentRoleQA), scheduler.LaunchContext{}) + if err != nil { + t.Fatalf("dispatch: %v", err) + } + if !launched { + t.Fatalf("expected launched=true") + } + if captured == nil { + t.Fatal("route_attempt_appended was not published") + } + if captured.TierSource != "role" { + t.Errorf("published attempt TierSource = %q, want role", captured.TierSource) + } +} diff --git a/apps/web/app/office/agents/[id]/components/agent-route-strip.tsx b/apps/web/app/office/agents/[id]/components/agent-route-strip.tsx index 1dadea2a8b..2df49e15c3 100644 --- a/apps/web/app/office/agents/[id]/components/agent-route-strip.tsx +++ b/apps/web/app/office/agents/[id]/components/agent-route-strip.tsx @@ -7,6 +7,7 @@ import { useAppStore } from "@/components/state-provider"; import { useAgentRoute } from "@/hooks/domains/office/use-agent-route"; import { useWorkspaceRouting } from "@/hooks/domains/office/use-workspace-routing"; import { providerLabel } from "../../../workspace/routing/components/provider-order-editor"; +import { TIER_SOURCE_LABEL_KEYS } from "../../../lib/label-keys"; import { useTranslation } from "react-i18next"; // Catalog keys, not copy — module scope freezes a `t()` at the boot locale. @@ -76,9 +77,12 @@ function ConfiguredRow({ preview }: { preview: AgentRoutePreview }) { ))} - - {preview.effective_tier} - +
+ + {preview.effective_tier} + + {t(TIER_SOURCE_LABEL_KEYS[preview.tier_source])} +
); } diff --git a/apps/web/app/office/agents/[id]/components/agent-routing-card.tsx b/apps/web/app/office/agents/[id]/components/agent-routing-card.tsx index 244776a06b..2bef4bb66c 100644 --- a/apps/web/app/office/agents/[id]/components/agent-routing-card.tsx +++ b/apps/web/app/office/agents/[id]/components/agent-routing-card.tsx @@ -11,12 +11,15 @@ import { useAgentRoute } from "@/hooks/domains/office/use-agent-route"; import { useAppStore } from "@/components/state-provider"; import { useWorkspaceRouting } from "@/hooks/domains/office/use-workspace-routing"; import type { + AgentRoutePreview, AgentRoutingOverrides, Tier, + TierPerReason, WorkspaceRouting, } from "@/lib/state/slices/office/types"; import { ProviderOrderEditor } from "../../../workspace/routing/components/provider-order-editor"; import { AgentWakeReasonOverrides } from "./agent-wake-reason-overrides"; +import { WAKE_REASONS } from "../../../workspace/routing/components/wake-reason-info"; import type { TFunction } from "i18next"; import { Trans, useTranslation } from "react-i18next"; import { TIER_NAME_KEYS } from "../../../lib/label-keys"; @@ -90,6 +93,12 @@ export function AgentRoutingCard({ agentId, initial }: Props) { workspaceConfig={workspace.config} knownProviders={workspace.knownProviders} saving={saving} + preview={route.data?.preview} + /> + setOverrides({ ...overrides, tier: t })} /> ) : ( - + )} - + Inherits tier @@ -261,6 +284,52 @@ function InheritedTierHint({ defaultTier }: { defaultTier?: Tier }) { ); } +// AC-17 / AC-17a / AC-17b: names the wake reasons whose policy shadows the +// tier displayed above, whichever level supplied it. The effective map is +// the agent's own tier_per_reason when it overrides the workspace policy +// (an override replaces the map entirely, per docs/specs/office/routing.md) +// and the workspace map otherwise; keys with an empty value do not shadow +// anything and are excluded. +function effectiveWakeReasonMap( + overrides: AgentRoutingOverrides, + workspaceConfig: WorkspaceRouting | undefined, +): TierPerReason { + if (overrides.tier_per_reason_source === "override") { + return overrides.tier_per_reason ?? {}; + } + return workspaceConfig?.tier_per_reason ?? {}; +} + +function TierShadowingNotice({ + overrides, + workspaceConfig, + preview, +}: { + overrides: AgentRoutingOverrides; + workspaceConfig: WorkspaceRouting | undefined; + preview: AgentRoutePreview | undefined; +}) { + const { t } = useTranslation(); + const overriding = overrides.tier_source === "override"; + // AC-18b: a preview never reports wake_reason, so the role/workspace case + // is only reachable once the preview has actually loaded. + const roleOrWorkspace = preview?.tier_source === "role" || preview?.tier_source === "workspace"; + if (!overriding && !roleOrWorkspace) return null; + + const map = effectiveWakeReasonMap(overrides, workspaceConfig); + const reasons = WAKE_REASONS.filter((r) => !!map[r.id]).map((r) => t(r.labelKey)); + if (reasons.length === 0) return null; + + const i18nKey = overriding + ? "office:overrideShadowedByWakeReasons" + : "office:tierShadowedByWakeReasons"; + return ( +

+ {t(i18nKey, { list: reasons.join(", ") })} +

+ ); +} + function computeNextOrder( on: boolean, current: string[] | undefined, diff --git a/apps/web/app/office/lib/label-keys.ts b/apps/web/app/office/lib/label-keys.ts index d841a73874..ba93ad6467 100644 --- a/apps/web/app/office/lib/label-keys.ts +++ b/apps/web/app/office/lib/label-keys.ts @@ -3,6 +3,7 @@ import type { OfficeTaskStatus, ProjectStatus, Tier, + TierSource, } from "@/lib/state/slices/office/types"; /** @@ -58,6 +59,18 @@ export const TIER_NAME_KEYS: Record = { economy: "office:tierNameEconomy", }; +/** + * Names the precedence level that supplied an agent's effective tier: a + * matching wake-reason policy, a per-agent override, the agent's role entry, + * or the workspace default. Wire values are never shown raw (AC-16, AC-16a). + */ +export const TIER_SOURCE_LABEL_KEYS: Record = { + wake_reason: "office:tierSourceWakeReason", + override: "office:tierSourceOverride", + role: "office:tierSourceRole", + workspace: "office:tierSourceWorkspace", +}; + /** * Routine concurrency policies. The row and the expanded detail used to render * the wire value with `policy.replace(/_/g, " ")`, which showed the identifier diff --git a/apps/web/app/office/workspace/routing/components/agent-preview-table.tsx b/apps/web/app/office/workspace/routing/components/agent-preview-table.tsx index 131a32a2ff..8ed88456ef 100644 --- a/apps/web/app/office/workspace/routing/components/agent-preview-table.tsx +++ b/apps/web/app/office/workspace/routing/components/agent-preview-table.tsx @@ -5,6 +5,7 @@ import { Badge } from "@kandev/ui/badge"; import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@kandev/ui/table"; import type { AgentRoutePreview } from "@/lib/state/slices/office/types"; import { providerLabel } from "./provider-order-editor"; +import { TIER_SOURCE_LABEL_KEYS } from "../../../lib/label-keys"; import { useTranslation } from "react-i18next"; type Props = { @@ -67,7 +68,7 @@ function PreviewRow({ a }: { a: AgentRoutePreview }) { {a.effective_tier} - {a.tier_source} + {t(TIER_SOURCE_LABEL_KEYS[a.tier_source])} diff --git a/apps/web/app/office/workspace/routing/components/index.ts b/apps/web/app/office/workspace/routing/components/index.ts index 4dc93377c1..49ab1767a4 100644 --- a/apps/web/app/office/workspace/routing/components/index.ts +++ b/apps/web/app/office/workspace/routing/components/index.ts @@ -4,4 +4,5 @@ export { ProviderOrderEditor, providerLabel } from "./provider-order-editor"; export { ProviderTierMapping } from "./provider-tier-mapping"; export { AgentPreviewTable } from "./agent-preview-table"; export { ProviderHealthBanner } from "./provider-health-banner"; -export { WakeReasonTierCard } from "./wake-reason-tier-card"; +export { WakeReasonTierCard, firstProviderWithTier } from "./wake-reason-tier-card"; +export { RoleTierCard } from "./role-tier-card"; diff --git a/apps/web/app/office/workspace/routing/components/role-tier-card.tsx b/apps/web/app/office/workspace/routing/components/role-tier-card.tsx new file mode 100644 index 0000000000..7935e00eee --- /dev/null +++ b/apps/web/app/office/workspace/routing/components/role-tier-card.tsx @@ -0,0 +1,195 @@ +"use client"; + +import { Card, CardContent, CardHeader, CardTitle } from "@kandev/ui/card"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@kandev/ui/select"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; +import { Badge } from "@kandev/ui/badge"; +import { IconInfoCircle, IconAlertTriangle } from "@tabler/icons-react"; +import type { + AgentRole, + RoleMeta, + RoleTierMap, + Tier, + WorkspaceRouting, +} from "@/lib/state/slices/office/types"; +import { providerLabel } from "./provider-order-editor"; +import { TIER_NAME_KEYS } from "../../../lib/label-keys"; +import { firstProviderWithTier } from "./wake-reason-tier-card"; +import { useTranslation } from "react-i18next"; + +// USE_WORKSPACE_DEFAULT is the sentinel used in the per-role tier dropdown +// to mean "no role entry — use the workspace default tier." Persisting this +// removes the role's key from the workspace's role_tiers map. +const USE_WORKSPACE_DEFAULT = "__workspace_default__" as const; + +type Props = { + roles: RoleMeta[]; + config: WorkspaceRouting; + value: RoleTierMap; + onChange: (next: RoleTierMap) => void; + disabled?: boolean; +}; + +// RoleTierCard renders the per-role tier policy table on the workspace +// routing settings page: the third of four precedence levels, below a +// wake-reason policy and a per-agent override, above the workspace default. +// See docs/specs/office-agent-tier-routing/spec.md. +export function RoleTierCard({ roles, config, value, onChange, disabled }: Props) { + const { t } = useTranslation(); + const handleRowChange = (role: AgentRole, tier: Tier | typeof USE_WORKSPACE_DEFAULT) => { + const next: RoleTierMap = { ...value }; + if (tier === USE_WORKSPACE_DEFAULT) { + delete next[role]; + } else { + next[role] = tier; + } + onChange(next); + }; + if (roles.length === 0) return null; + return ( + + + {t("office:roleTierPolicy")} +

+ {t("office:setADefaultTierForEveryAgentOfA")} +

+ + + {roles.map((role) => ( + handleRowChange(role.id as AgentRole, tier)} + /> + ))} + + + ); +} + +type RowProps = { + role: RoleMeta; + tier: Tier | undefined; + config: WorkspaceRouting; + disabled?: boolean; + onChange: (tier: Tier | typeof USE_WORKSPACE_DEFAULT) => void; +}; + +function RoleRow({ role, tier, config, disabled, onChange }: RowProps) { + const labelId = `role-tier-label-${role.id}`; + return ( +
+
+ + +
+ +
+ ); +} + +function RowLabel({ role, labelId }: { role: RoleMeta; labelId: string }) { + const { t } = useTranslation(); + return ( +
+ + {role.label} + + + + + + + {role.description || t("office:roleTierRowInfo")} + + +
+ ); +} + +// Catalog keys, not copy — module scope freezes a `t()` at the boot locale. +const TIER_OPTIONS: Array<{ value: Tier; labelKey: string }> = [ + { value: "frontier", labelKey: "office:tierFrontierQualified" }, + { value: "balanced", labelKey: "office:tierBalancedQualified" }, + { value: "economy", labelKey: "office:tierEconomyQualified" }, +]; + +function TierSelect({ + tier, + onChange, + disabled, + ariaLabelledBy, +}: { + tier: Tier | undefined; + onChange: (tier: Tier | typeof USE_WORKSPACE_DEFAULT) => void; + disabled?: boolean; + ariaLabelledBy: string; +}) { + const { t } = useTranslation(); + const selected = tier ?? USE_WORKSPACE_DEFAULT; + return ( + + ); +} + +// ResolvedRow shows "Currently maps to: / " so users see +// the concrete (provider, model) their selection produces. When the +// selected tier is not mapped on any provider in the order we surface a +// warning chip directing them to the provider tier mapping section. +function ResolvedRow({ tier, config }: { tier: Tier | undefined; config: WorkspaceRouting }) { + const { t } = useTranslation(); + if (!tier) return null; + const match = firstProviderWithTier(tier, config); + if (!match) { + return ( +

+ + {/* One key: the tier appeared twice in one sentence, so two fragments + would have frozen both the order and the repetition. */} + {t("office:noProviderHasTierMapped", { tier: t(TIER_NAME_KEYS[tier]) })} +

+ ); + } + return ( +

+ {t("office:currentlyMapsTo")}{" "} + + + + {providerLabel(match.providerId)} / {match.model} + + + + {t("office:typicalMappingAtLaunchTimeThe")} + + +

+ ); +} diff --git a/apps/web/app/office/workspace/routing/components/wake-reason-tier-card.tsx b/apps/web/app/office/workspace/routing/components/wake-reason-tier-card.tsx index 5528fad6e4..69a5094178 100644 --- a/apps/web/app/office/workspace/routing/components/wake-reason-tier-card.tsx +++ b/apps/web/app/office/workspace/routing/components/wake-reason-tier-card.tsx @@ -188,7 +188,9 @@ function ResolvedRow({ tier, config }: { tier: Tier | undefined; config: Workspa ); } -function firstProviderWithTier( +// Exported for reuse by RoleTierCard's own ResolvedRow (same "currently +// maps to" lookup, one level down the tier precedence chain). +export function firstProviderWithTier( tier: Tier, config: WorkspaceRouting, ): { providerId: string; model: string } | null { diff --git a/apps/web/app/office/workspace/routing/page.tsx b/apps/web/app/office/workspace/routing/page.tsx index 192517912e..78d112ef1d 100644 --- a/apps/web/app/office/workspace/routing/page.tsx +++ b/apps/web/app/office/workspace/routing/page.tsx @@ -12,6 +12,8 @@ import { useRoutingPreview } from "@/hooks/domains/office/use-routing-preview"; import type { ProviderProfile, ExecutionProfileSummary, + RoleMeta, + RoleTierMap, Tier, TierPerReason, WorkspaceRouting, @@ -22,6 +24,7 @@ import { ProviderHealthBanner, ProviderOrderEditor, ProviderTierMapping, + RoleTierCard, RoutingEnableCard, WakeReasonTierCard, } from "./components"; @@ -29,6 +32,13 @@ import { useTranslation } from "react-i18next"; const DEFAULT_PROFILE: ProviderProfile = { tier_map: {} }; +// Stable fallback: a Zustand selector that returns a fresh `[]` on every call +// (when `office.meta` hasn't loaded yet) breaks useSyncExternalStore's +// reference-equality snapshot check and infinite-loops into React error #185 +// ("Maximum update depth exceeded"), caught by the route error boundary. See +// EMPTY_PREVIEW / EMPTY_HEALTH in the sibling hooks for the same pattern. +const EMPTY_ROLES: RoleMeta[] = []; + function emptyConfig(): WorkspaceRouting { return { enabled: false, @@ -42,6 +52,7 @@ function emptyConfig(): WorkspaceRouting { export default function ProviderRoutingPage() { const { t } = useTranslation(); const workspaceId = useAppStore((s) => s.workspaces.activeId); + const roles = useAppStore((s) => s.office.meta?.roles ?? EMPTY_ROLES); const routing = useWorkspaceRouting(workspaceId); const health = useProviderHealth(workspaceId); const preview = useRoutingPreview(workspaceId); @@ -85,6 +96,7 @@ export default function ProviderRoutingPage() { void; + roles: RoleMeta[]; knownProviders: string[]; executionProfiles: ExecutionProfileSummary[]; saving: boolean; @@ -117,6 +130,7 @@ type PageBodyProps = { function PageBody({ draft, setDraft, + roles, knownProviders, executionProfiles, saving, @@ -133,6 +147,7 @@ function PageBody({ // Named `tier`, not `t`: a parameter called `t` shadows the translate function. const setTier = (tier: Tier) => setDraft({ ...draft, default_tier: tier }); const setTierPerReason = (m: TierPerReason) => setDraft({ ...draft, tier_per_reason: m }); + const setRoleTiers = (m: RoleTierMap) => setDraft({ ...draft, role_tiers: m }); const setOrder = (next: string[]) => { const profiles = { ...draft.provider_profiles }; for (const p of next) { @@ -159,6 +174,13 @@ function PageBody({ onChange={setTier} disabled={sectionsDisabled} /> + ; + role_tiers?: Record; }; known_providers: string[]; execution_profiles: Array<{ @@ -306,6 +307,7 @@ export class OfficeApiClient { execution_profile_ids?: { frontier?: string; balanced?: string; economy?: string }; } >; + role_tiers?: Record; }, ): Promise> { return this.request("PUT", `/workspaces/${wsId}/routing`, cfg); diff --git a/apps/web/e2e/tests/office/mobile-provider-routing-profiles.spec.ts b/apps/web/e2e/tests/office/mobile-provider-routing-profiles.spec.ts index 5772c6f49f..e7e1115528 100644 --- a/apps/web/e2e/tests/office/mobile-provider-routing-profiles.spec.ts +++ b/apps/web/e2e/tests/office/mobile-provider-routing-profiles.spec.ts @@ -34,4 +34,46 @@ test.describe("Office execution profile routing on mobile", () => { await expect(testPage.getByRole("option").first()).toBeVisible(); await assertNoDocumentHorizontalOverflow(testPage, "provider routing profile selector"); }); + + test("edits and saves a role tier on mobile", async ({ + backend, + apiClient, + officeApi, + officeSeed, + testPage, + }) => { + await backend.restart({ KANDEV_MOCK_PROVIDERS: "claude-acp" }); + const configured = await balancedExecutionProfileRouting( + apiClient, + officeApi, + officeSeed.workspaceId, + ["claude-acp"], + ); + await officeApi.updateRouting(officeSeed.workspaceId, { ...configured, enabled: false }); + + await testPage.goto("/office/workspace/routing"); + const roleTier = testPage.getByRole("combobox", { name: "CEO", exact: true }); + await expect(roleTier).toBeVisible(); + const roleTierBox = await roleTier.boundingBox(); + expect(roleTierBox?.width).toBeGreaterThanOrEqual(44); + expect(roleTierBox?.height).toBeGreaterThanOrEqual(44); + + const infoButton = testPage.getByRole("button", { name: "More info about CEO", exact: true }); + const infoButtonBox = await infoButton.boundingBox(); + expect(infoButtonBox?.width).toBeGreaterThanOrEqual(44); + expect(infoButtonBox?.height).toBeGreaterThanOrEqual(44); + + await roleTier.click(); + await testPage.getByRole("option", { name: /Balanced/ }).click(); + await testPage.getByRole("button", { name: "Save", exact: true }).click(); + + const saved = await officeApi.getRouting(officeSeed.workspaceId); + expect(saved.config.role_tiers?.ceo).toBe("balanced"); + + await testPage.reload(); + await expect(testPage.getByRole("combobox", { name: "CEO", exact: true })).toContainText( + "Balanced", + ); + await assertNoDocumentHorizontalOverflow(testPage, "role tier editor"); + }); }); diff --git a/apps/web/e2e/tests/office/office-routing-agent-override.spec.ts b/apps/web/e2e/tests/office/office-routing-agent-override.spec.ts index f924933510..efd1c11c3c 100644 --- a/apps/web/e2e/tests/office/office-routing-agent-override.spec.ts +++ b/apps/web/e2e/tests/office/office-routing-agent-override.spec.ts @@ -113,3 +113,103 @@ test.describe("Office provider routing — agent override", () => { expect(attempts.every((a) => a.provider_id === "claude-acp")).toBe(true); }); }); + +/** + * Per-role tier routing (AC-13/13b/14/14b/16a/18) and the removal of the + * `model` field from Office agent identity (AC-8a). + * + * The seeded office agent (`officeSeed.agentId`) is always created with + * Role: "ceo" (see internal/office/onboarding/service.go), so a + * `role_tiers: {ceo: ...}` entry targets it directly without a second + * agent. + * + * checkRoleTiersMapped (internal/office/routing/types.go) rejects a + * role_tiers entry whose tier has no provider mapping it via + * execution_profile_ids, so the "economy" tier below reuses the same + * execution profile the balanced-tier helper already created — nothing + * requires a distinct profile per tier, only a non-empty mapping. + */ +test.describe("Office provider routing — role tiers", () => { + test.beforeEach(async ({ apiClient, officeSeed }) => { + await apiClient.e2eReset(officeSeed.workspaceId, [officeSeed.workflowId]); + }); + + test.afterAll(async ({ backend }) => { + await backend.restart(); + }); + + test("role tier overrides the workspace default and renders its translated source label", async ({ + backend, + apiClient, + officeApi, + officeSeed, + testPage, + }) => { + await backend.restart({ KANDEV_MOCK_PROVIDERS: "claude-acp" }); + + const configured = await balancedExecutionProfileRouting( + apiClient, + officeApi, + officeSeed.workspaceId, + ["claude-acp"], + ); + const balancedProfile = configured.provider_profiles["claude-acp"]; + await officeApi.updateRouting(officeSeed.workspaceId, { + ...configured, + role_tiers: { ceo: "economy" }, + provider_profiles: { + "claude-acp": { + tier_map: { ...balancedProfile.tier_map, economy: balancedProfile.tier_map.balanced }, + execution_profile_ids: { + ...balancedProfile.execution_profile_ids, + economy: balancedProfile.execution_profile_ids?.balanced, + }, + }, + }, + }); + + // API: the seeded CEO agent has no per-agent override and no + // wake-reason policy applies, so it resolves the role tier, not the + // workspace default ("balanced"). + const route = await officeApi.getAgentRoute(officeSeed.agentId); + expect(route.preview.tier_source).toBe("role"); + expect(route.preview.effective_tier).toBe("economy"); + + // UI: the resolved-routes preview table renders the widened + // tier_source value as its translated label, not the raw wire value. + await testPage.goto("/office/workspace/routing"); + const row = testPage.getByTestId(`preview-row-${officeSeed.agentId}`); + await expect(row).toBeVisible(); + await expect(row).toContainText("economy"); + await expect(row).toContainText("from role"); + }); + + test("PATCH rejects a model key for an Office-identity agent with the structured shape", async ({ + backend, + request, + officeSeed, + }) => { + await backend.restart({ KANDEV_MOCK_PROVIDERS: "claude-acp" }); + + const modelRes = await request.patch( + `${backend.baseUrl}/api/v1/office/agents/${officeSeed.agentId}`, + { data: { model: "some-model" } }, + ); + expect(modelRes.status()).toBe(400); + const modelBody = await modelRes.json(); + expect(modelBody.field).toBe("model"); + expect(modelBody.error).toBeTruthy(); + + // Contrast: the agent_profile_id rejection uses the older bare + // {"error": ...} shape with no "field" key — the model rejection + // above is deliberately more structured (AC-8a). + const profileRes = await request.patch( + `${backend.baseUrl}/api/v1/office/agents/${officeSeed.agentId}`, + { data: { agent_profile_id: "some-profile" } }, + ); + expect(profileRes.status()).toBe(400); + const profileBody = await profileRes.json(); + expect(profileBody.field).toBeUndefined(); + expect(profileBody.error).toBeTruthy(); + }); +}); diff --git a/apps/web/lib/state/slices/office/office-routing.test.ts b/apps/web/lib/state/slices/office/office-routing.test.ts index e103bf6983..b00ca89dc0 100644 --- a/apps/web/lib/state/slices/office/office-routing.test.ts +++ b/apps/web/lib/state/slices/office/office-routing.test.ts @@ -98,7 +98,7 @@ describe("office routing store actions", () => { { agent_id: "a-1", agent_name: "CEO", - tier_source: "inherit", + tier_source: "workspace", effective_tier: "balanced", fallback_chain: [], missing: [], diff --git a/apps/web/lib/state/slices/office/routing-types.ts b/apps/web/lib/state/slices/office/routing-types.ts index fef2b4f802..607b9528e0 100644 --- a/apps/web/lib/state/slices/office/routing-types.ts +++ b/apps/web/lib/state/slices/office/routing-types.ts @@ -1,9 +1,11 @@ // --- Provider routing types --- // -// Split out of types.ts (which grows past the file-length lint limit easily -// given how many office domains it hosts) — provider routing is a -// self-contained concern with no dependencies on the rest of the office -// slice types, so it splits cleanly. +// Split out of `types.ts` to keep that file under the repo's 600-line cap +// (see apps/web/AGENTS.md "Code-quality limits"). Re-exported from +// `types.ts` so existing `@/lib/state/slices/office/types` imports keep +// working unchanged. + +import type { AgentRole } from "@/lib/types/agent-profile"; export type Tier = "frontier" | "balanced" | "economy"; @@ -56,12 +58,23 @@ export type WakeReason = "heartbeat" | "routine_trigger" | "budget_alert"; export type TierPerReason = Partial>; +// Per-role tier policy: workspace-level default for every agent of a given +// role, overridable per-agent and shadowed by tier_per_reason. See +// docs/specs/office-agent-tier-routing/spec.md. +export type RoleTierMap = Partial>; + +// The four precedence levels a resolved tier can come from, in +// highest-to-lowest priority order: a matching wake-reason policy, a +// per-agent override, the agent's role entry, or the workspace default. +export type TierSource = "wake_reason" | "override" | "role" | "workspace"; + export type WorkspaceRouting = { enabled: boolean; provider_order: string[]; default_tier: Tier; provider_profiles: Record; tier_per_reason?: TierPerReason; + role_tiers?: RoleTierMap; }; export type AgentRoutingOverrides = { @@ -106,6 +119,9 @@ export type RouteAttempt = { provider_id: string; model?: string; tier: Tier | ""; + // Empty for attempts recorded before this column existed, or that never + // resolved a tier (e.g. a max-attempts-exceeded row). + tier_source?: TierSource | ""; outcome: RouteAttemptOutcome; error_code?: RoutingErrorCode; error_confidence?: "high" | "medium" | "low"; @@ -128,7 +144,7 @@ export type ProviderModelPair = { export type AgentRoutePreview = { agent_id: string; agent_name: string; - tier_source: "inherit" | "override"; + tier_source: TierSource; effective_tier: Tier; // primary_* reflects configured intent — first entry in the // effective provider order, even when that provider is currently diff --git a/apps/web/lib/state/slices/office/types.ts b/apps/web/lib/state/slices/office/types.ts index a97f8a0a47..55a2f527ac 100644 --- a/apps/web/lib/state/slices/office/types.ts +++ b/apps/web/lib/state/slices/office/types.ts @@ -459,9 +459,10 @@ export type OfficeMeta = { }; // --- Provider routing types --- -// Extracted to routing-types.ts to keep this file under the file-length -// lint limit; re-exported here so existing `from ".../office/types"` -// imports keep working unchanged. +// +// Defined in `./routing-types` (kept out of this file to stay under the +// 600-line cap) and re-exported here so existing +// `@/lib/state/slices/office/types` imports keep working unchanged. export type { Tier, @@ -471,6 +472,8 @@ export type { ExecutionProfileSummary, WakeReason, TierPerReason, + RoleTierMap, + TierSource, WorkspaceRouting, AgentRoutingOverrides, ProviderHealthState, @@ -486,6 +489,7 @@ export type { RunAttemptsState, AgentRoutingSliceState, } from "./routing-types"; + import type { AgentRouteData, AgentRoutePreview, diff --git a/apps/web/src/locales/en/office.json b/apps/web/src/locales/en/office.json index 7f1b87d2d0..ea02965023 100644 --- a/apps/web/src/locales/en/office.json +++ b/apps/web/src/locales/en/office.json @@ -355,6 +355,7 @@ "inherit": "Inherit", "inheritFromProjectWorkspace": "Inherit from project/workspace", "inheritFromWorkspace": "Inherit from workspace", + "inheritsTierFromRole": "Inherits <1>{{tier}} from role.", "inheritsTierFromWorkspace": "Inherits <1>{{tier}} from workspace.", "inheritsWorkspacePolicyList": "Inherits workspace policy: {{list}}.", "inheritsWorkspacePolicyNoneSet": "Inherits workspace policy (no wake-reason tiers set yet).", @@ -504,6 +505,7 @@ "outgoingDatabaseFilesystem": "Outgoing (Database → Filesystem)", "output": "Output", "outputTokens": "Output tokens", + "overrideShadowedByWakeReasons": "Your tier override is only used for runs that do not match a wake-reason policy. This agent's wake-reason tiers already cover: {{list}}.", "overrideTheWorkspaceTierOrProvider": "Override the workspace tier or provider order for this agent.", "overrideWakeReasonTiers": "Override wake-reason tiers", "overrideWakeReasonTiersHelp": "This is rare. Override only if this agent specifically needs different behaviour for its background work; for example, a security-critical agent that must use Frontier even for routine checks. Most agents should inherit the workspace policy.", @@ -630,6 +632,8 @@ "roleQa": "QA", "roleSecurity": "Security", "roleSpecialist": "Specialist", + "roleTierPolicy": "Role tier policy", + "roleTierRowInfo": "Sets the default model tier for every agent with this role, unless a wake-reason policy or a per-agent override applies first.", "roleWorker": "Worker", "routeAttempts": "Route attempts", "routeBlocked": "Blocked", @@ -719,6 +723,7 @@ "sessionId": "Session ID", "sessionIdAfter": "Session ID (after)", "sessionIdBefore": "Session ID (before)", + "setADefaultTierForEveryAgentOfA": "Set a default tier for every agent of a given role. Falls back to the workspace default tier when a role has no entry here; a wake-reason policy or a per-agent override still takes priority over this.", "setSpendingLimitsPerAgentOr": "Set spending limits per agent or project. Agents pause when limits are exceeded.", "setupTierAgentProfiles": "Setup tier agent profiles", "setupTitle": "Setup", @@ -834,6 +839,11 @@ "tierNameBalanced": "balanced", "tierNameEconomy": "economy", "tierNameFrontier": "frontier", + "tierShadowedByWakeReasons": "The tier shown above is only used for runs that do not match a wake-reason policy. This workspace's wake-reason tiers already cover: {{list}}.", + "tierSourceOverride": "from override", + "tierSourceRole": "from role", + "tierSourceWakeReason": "from wake reason", + "tierSourceWorkspace": "from workspace", "timezone": "Timezone", "title": "Title", "titleForAutoCreatedTasksUse": "Title for auto-created tasks. Use {{name}} and {{date}} as placeholders.", @@ -863,6 +873,7 @@ "useAgentSNormalTier": "Use agent's normal tier", "usedBy": "USED BY", "useQuery": "Use “{{query}}”", + "useWorkspaceDefaultTier": "Use workspace default tier", "usingThisAgentsWakeReasonOverrides": "Using this agent's wake-reason overrides.", "variables": "Variables", "waiting": "Waiting…", diff --git a/apps/web/src/locales/pseudo/office.json b/apps/web/src/locales/pseudo/office.json index a55ce259ed..5d1e853151 100644 --- a/apps/web/src/locales/pseudo/office.json +++ b/apps/web/src/locales/pseudo/office.json @@ -355,6 +355,7 @@ "inherit": "Ĩńĥēŕĩţ", "inheritFromProjectWorkspace": "Ĩńĥēŕĩţ ƒŕōḿ ƥŕōĵēćţ/ŵōŕķśƥàćē", "inheritFromWorkspace": "Ĩńĥēŕĩţ ƒŕōḿ ŵōŕķśƥàćē", + "inheritsTierFromRole": "Ĩńĥēŕĩţś <1>{{tier}} ƒŕōḿ ŕōĺē.", "inheritsTierFromWorkspace": "Ĩńĥēŕĩţś <1>{{tier}} ƒŕōḿ ŵōŕķśƥàćē.", "inheritsWorkspacePolicyList": "Ĩńĥēŕĩţś ŵōŕķśƥàćē ƥōĺĩćŷ: {{list}}.", "inheritsWorkspacePolicyNoneSet": "Ĩńĥēŕĩţś ŵōŕķśƥàćē ƥōĺĩćŷ (ńō ŵàķē-ŕēàśōń ţĩēŕś śēţ ŷēţ).", @@ -504,6 +505,7 @@ "outgoingDatabaseFilesystem": "Ōũţĝōĩńĝ (Ďàţàƀàśē → Ƒĩĺēśŷśţēḿ)", "output": "Ōũţƥũţ", "outputTokens": "Ōũţƥũţ ţōķēńś", + "overrideShadowedByWakeReasons": "Ŷōũŕ ţĩēŕ ōvēŕŕĩďē ĩś ōńĺŷ ũśēď ƒōŕ ŕũńś ţĥàţ ďō ńōţ ḿàţćĥ à ŵàķē-ŕēàśōń ƥōĺĩćŷ. Ţĥĩś àĝēńţ'ś ŵàķē-ŕēàśōń ţĩēŕś àĺŕēàďŷ ćōvēŕ: {{list}}.", "overrideTheWorkspaceTierOrProvider": "Ōvēŕŕĩďē ţĥē ŵōŕķśƥàćē ţĩēŕ ōŕ ƥŕōvĩďēŕ ōŕďēŕ ƒōŕ ţĥĩś àĝēńţ.", "overrideWakeReasonTiers": "Ōvēŕŕĩďē ŵàķē-ŕēàśōń ţĩēŕś", "overrideWakeReasonTiersHelp": "Ţĥĩś ĩś ŕàŕē. Ōvēŕŕĩďē ōńĺŷ ĩƒ ţĥĩś àĝēńţ śƥēćĩƒĩćàĺĺŷ ńēēďś ďĩƒƒēŕēńţ ƀēĥàvĩōũŕ ƒōŕ ĩţś ƀàćķĝŕōũńď ŵōŕķ; ƒōŕ ēxàḿƥĺē, à śēćũŕĩţŷ-ćŕĩţĩćàĺ àĝēńţ ţĥàţ ḿũśţ ũśē Ƒŕōńţĩēŕ ēvēń ƒōŕ ŕōũţĩńē ćĥēćķś. Ḿōśţ àĝēńţś śĥōũĺď ĩńĥēŕĩţ ţĥē ŵōŕķśƥàćē ƥōĺĩćŷ.", @@ -630,6 +632,8 @@ "roleQa": "QÀ", "roleSecurity": "Śēćũŕĩţŷ", "roleSpecialist": "Śƥēćĩàĺĩśţ", + "roleTierPolicy": "Ŕōĺē ţĩēŕ ƥōĺĩćŷ", + "roleTierRowInfo": "Śēţś ţĥē ďēƒàũĺţ ḿōďēĺ ţĩēŕ ƒōŕ ēvēŕŷ àĝēńţ ŵĩţĥ ţĥĩś ŕōĺē, ũńĺēśś à ŵàķē-ŕēàśōń ƥōĺĩćŷ ōŕ à ƥēŕ-àĝēńţ ōvēŕŕĩďē àƥƥĺĩēś ƒĩŕśţ.", "roleWorker": "Ŵōŕķēŕ", "routeAttempts": "Ŕōũţē àţţēḿƥţś", "routeBlocked": "Ɓĺōćķēď", @@ -719,6 +723,7 @@ "sessionId": "Śēśśĩōń ĨĎ", "sessionIdAfter": "Śēśśĩōń ĨĎ (àƒţēŕ)", "sessionIdBefore": "Śēśśĩōń ĨĎ (ƀēƒōŕē)", + "setADefaultTierForEveryAgentOfA": "Śēţ à ďēƒàũĺţ ţĩēŕ ƒōŕ ēvēŕŷ àĝēńţ ōƒ à ĝĩvēń ŕōĺē. Ƒàĺĺś ƀàćķ ţō ţĥē ŵōŕķśƥàćē ďēƒàũĺţ ţĩēŕ ŵĥēń à ŕōĺē ĥàś ńō ēńţŕŷ ĥēŕē; à ŵàķē-ŕēàśōń ƥōĺĩćŷ ōŕ à ƥēŕ-àĝēńţ ōvēŕŕĩďē śţĩĺĺ ţàķēś ƥŕĩōŕĩţŷ ōvēŕ ţĥĩś.", "setSpendingLimitsPerAgentOr": "Śēţ śƥēńďĩńĝ ĺĩḿĩţś ƥēŕ àĝēńţ ōŕ ƥŕōĵēćţ. Àĝēńţś ƥàũśē ŵĥēń ĺĩḿĩţś àŕē ēxćēēďēď.", "setupTierAgentProfiles": "Śēţũƥ ţĩēŕ àĝēńţ ƥŕōƒĩĺēś", "setupTitle": "Śēţũƥ", @@ -834,6 +839,11 @@ "tierNameBalanced": "ƀàĺàńćēď", "tierNameEconomy": "ēćōńōḿŷ", "tierNameFrontier": "ƒŕōńţĩēŕ", + "tierShadowedByWakeReasons": "Ţĥē ţĩēŕ śĥōŵń àƀōvē ĩś ōńĺŷ ũśēď ƒōŕ ŕũńś ţĥàţ ďō ńōţ ḿàţćĥ à ŵàķē-ŕēàśōń ƥōĺĩćŷ. Ţĥĩś ŵōŕķśƥàćē'ś ŵàķē-ŕēàśōń ţĩēŕś àĺŕēàďŷ ćōvēŕ: {{list}}.", + "tierSourceOverride": "ƒŕōḿ ōvēŕŕĩďē", + "tierSourceRole": "ƒŕōḿ ŕōĺē", + "tierSourceWakeReason": "ƒŕōḿ ŵàķē ŕēàśōń", + "tierSourceWorkspace": "ƒŕōḿ ŵōŕķśƥàćē", "timezone": "Ţĩḿēźōńē", "title": "Ţĩţĺē", "titleForAutoCreatedTasksUse": "Ţĩţĺē ƒōŕ àũţō-ćŕēàţēď ţàśķś. Ũśē {{name}} àńď {{date}} àś ƥĺàćēĥōĺďēŕś.", @@ -863,6 +873,7 @@ "useAgentSNormalTier": "Ũśē àĝēńţ'ś ńōŕḿàĺ ţĩēŕ", "usedBy": "ŨŚĒĎ ƁŶ", "useQuery": "Ũśē “{{query}}”", + "useWorkspaceDefaultTier": "Ũśē ŵōŕķśƥàćē ďēƒàũĺţ ţĩēŕ", "usingThisAgentsWakeReasonOverrides": "Ũśĩńĝ ţĥĩś àĝēńţ'ś ŵàķē-ŕēàśōń ōvēŕŕĩďēś.", "variables": "Vàŕĩàƀĺēś", "waiting": "Ŵàĩţĩńĝ…", diff --git a/apps/web/src/locales/pt-pt/office.json b/apps/web/src/locales/pt-pt/office.json index 89bec991e2..f454f36b43 100644 --- a/apps/web/src/locales/pt-pt/office.json +++ b/apps/web/src/locales/pt-pt/office.json @@ -355,6 +355,7 @@ "inherit": "Herdar", "inheritFromProjectWorkspace": "Herdar do projeto ou da área de trabalho", "inheritFromWorkspace": "Herdar da área de trabalho", + "inheritsTierFromRole": "Herda <1>{{tier}} da função.", "inheritsTierFromWorkspace": "Herda <1>{{tier}} da área de trabalho.", "inheritsWorkspacePolicyList": "Herda a política da área de trabalho: {{list}}.", "inheritsWorkspacePolicyNoneSet": "Herda a política da área de trabalho (ainda não há escalões definidos por motivo de despertar).", @@ -504,6 +505,7 @@ "outgoingDatabaseFilesystem": "A enviar (base de dados → sistema de ficheiros)", "output": "Saída", "outputTokens": "Tokens de saída", + "overrideShadowedByWakeReasons": "A sua substituição de escalão só é usada em execuções que não correspondam a uma política de motivo de despertar. Os escalões por motivo de despertar deste agente já cobrem: {{list}}.", "overrideTheWorkspaceTierOrProvider": "Substitua o escalão ou a ordem de fornecedores da área de trabalho para este agente.", "overrideWakeReasonTiers": "Substituir os escalões por motivo de despertar", "overrideWakeReasonTiersHelp": "Isto é raro. Substitua apenas se este agente precisar mesmo de um comportamento diferente no trabalho em segundo plano; por exemplo, um agente crítico para a segurança que tenha de usar Frontier mesmo em verificações de rotina. A maioria dos agentes deve herdar a política da área de trabalho.", @@ -630,6 +632,8 @@ "roleQa": "QA", "roleSecurity": "Segurança", "roleSpecialist": "Especialista", + "roleTierPolicy": "Política de escalão por função", + "roleTierRowInfo": "Define o escalão de modelo predefinido para todos os agentes com esta função, salvo se uma política de motivo de despertar ou uma substituição por agente se aplicar primeiro.", "roleWorker": "Trabalhador", "routeAttempts": "Tentativas de encaminhamento", "routeBlocked": "Bloqueada", @@ -719,6 +723,7 @@ "sessionId": "ID da sessão", "sessionIdAfter": "ID da sessão (depois)", "sessionIdBefore": "ID da sessão (antes)", + "setADefaultTierForEveryAgentOfA": "Defina um escalão predefinido para todos os agentes de uma determinada função. Recorre ao escalão predefinido da área de trabalho quando uma função não tem aqui nenhuma entrada; uma política de motivo de despertar ou uma substituição por agente continua a ter prioridade sobre isto.", "setSpendingLimitsPerAgentOr": "Defina limites de despesa por agente ou por projeto. Os agentes são pausados quando os limites são excedidos.", "setupTierAgentProfiles": "Configurar os perfis de agente por escalão", "setupTitle": "Configuração", @@ -834,6 +839,11 @@ "tierNameBalanced": "Balanced", "tierNameEconomy": "Economy", "tierNameFrontier": "Frontier", + "tierShadowedByWakeReasons": "O escalão apresentado acima só é usado em execuções que não correspondam a uma política de motivo de despertar. Os escalões por motivo de despertar desta área de trabalho já cobrem: {{list}}.", + "tierSourceOverride": "por substituição", + "tierSourceRole": "por função", + "tierSourceWakeReason": "por motivo de despertar", + "tierSourceWorkspace": "por área de trabalho", "timezone": "Fuso horário", "title": "Título", "titleForAutoCreatedTasksUse": "Título das tarefas criadas automaticamente. Use {{name}} e {{date}} como marcadores.", @@ -863,6 +873,7 @@ "useAgentSNormalTier": "Usar o escalão normal do agente", "usedBy": "USADO POR", "useQuery": "Usar “{{query}}”", + "useWorkspaceDefaultTier": "Usar o escalão predefinido da área de trabalho", "usingThisAgentsWakeReasonOverrides": "A usar as substituições por motivo de despertar deste agente.", "variables": "Variáveis", "waiting": "À espera…", diff --git a/apps/web/src/locales/zh-cn/office.json b/apps/web/src/locales/zh-cn/office.json index 616d2c3d2c..4f429a24b0 100644 --- a/apps/web/src/locales/zh-cn/office.json +++ b/apps/web/src/locales/zh-cn/office.json @@ -355,6 +355,7 @@ "inherit": "继承", "inheritFromProjectWorkspace": "从项目/工作区继承", "inheritFromWorkspace": "从工作区继承", + "inheritsTierFromRole": "从角色继承 <1>{{tier}}。", "inheritsTierFromWorkspace": "从工作区继承 <1>{{tier}}。", "inheritsWorkspacePolicyList": "继承工作区策略：{{list}}。", "inheritsWorkspacePolicyNoneSet": "继承工作区策略（尚未设置唤醒原因层级）。", @@ -504,6 +505,7 @@ "outgoingDatabaseFilesystem": "传出（数据库 → 文件系统）", "output": "输出", "outputTokens": "输出令牌", + "overrideShadowedByWakeReasons": "你的层级覆盖仅用于不匹配任何唤醒原因策略的运行。此智能体的唤醒原因层级已覆盖：{{list}}。", "overrideTheWorkspaceTierOrProvider": "覆盖此智能体的工作区层级或提供商顺序。", "overrideWakeReasonTiers": "覆盖唤醒原因层级", "overrideWakeReasonTiersHelp": "较少使用。仅当此智能体的后台工作需要不同行为时覆盖；例如安全关键智能体即使例行检查也必须使用 Frontier。多数智能体应继承工作区策略。", @@ -630,6 +632,8 @@ "roleQa": "QA", "roleSecurity": "安全", "roleSpecialist": "专家", + "roleTierPolicy": "角色层级策略", + "roleTierRowInfo": "为具有此角色的每个智能体设置默认模型层级，除非唤醒原因策略或按智能体的覆盖优先生效。", "roleWorker": "工作者", "routeAttempts": "路由尝试", "routeBlocked": "已阻塞", @@ -719,6 +723,7 @@ "sessionId": "会话 ID", "sessionIdAfter": "会话 ID（之后）", "sessionIdBefore": "会话 ID（之前）", + "setADefaultTierForEveryAgentOfA": "为具有给定角色的每个智能体设置默认层级。当某角色在此处没有条目时，将回退到工作区默认层级；唤醒原因策略或按智能体的覆盖仍优先于此设置。", "setSpendingLimitsPerAgentOr": "按智能体或项目设置支出限额。超出限额时智能体会暂停。", "setupTierAgentProfiles": "设置层级智能体配置", "setupTitle": "初始设置", @@ -834,6 +839,11 @@ "tierNameBalanced": "balanced", "tierNameEconomy": "economy", "tierNameFrontier": "frontier", + "tierShadowedByWakeReasons": "上方显示的层级仅用于不匹配任何唤醒原因策略的运行。此工作区的唤醒原因层级已覆盖：{{list}}。", + "tierSourceOverride": "来自覆盖", + "tierSourceRole": "来自角色", + "tierSourceWakeReason": "来自唤醒原因", + "tierSourceWorkspace": "来自工作区", "timezone": "时区", "title": "标题", "titleForAutoCreatedTasksUse": "自动创建任务的标题。可用 {{name}} 与 {{date}} 作为占位符。", @@ -863,6 +873,7 @@ "useAgentSNormalTier": "使用智能体的常规层级", "usedBy": "使用者", "useQuery": "使用“{{query}}”", + "useWorkspaceDefaultTier": "使用工作区默认层级", "usingThisAgentsWakeReasonOverrides": "正在使用此智能体的唤醒原因覆盖。", "variables": "变量", "waiting": "等待中…", diff --git a/apps/web/src/locales/zh-hk/office.json b/apps/web/src/locales/zh-hk/office.json index 8610882871..c61d4c3564 100644 --- a/apps/web/src/locales/zh-hk/office.json +++ b/apps/web/src/locales/zh-hk/office.json @@ -355,6 +355,7 @@ "inherit": "繼承", "inheritFromProjectWorkspace": "從項目/工作區繼承", "inheritFromWorkspace": "從工作區繼承", + "inheritsTierFromRole": "從角色繼承 <1>{{tier}}。", "inheritsTierFromWorkspace": "從工作區繼承 <1>{{tier}}。", "inheritsWorkspacePolicyList": "繼承工作區策略：{{list}}。", "inheritsWorkspacePolicyNoneSet": "繼承工作區策略（尚未設定喚醒原因層級）。", @@ -504,6 +505,7 @@ "outgoingDatabaseFilesystem": "傳出（數據庫 → 檔案系統）", "output": "輸出", "outputTokens": "輸出權杖", + "overrideShadowedByWakeReasons": "你的層級覆蓋僅用於不匹配任何喚醒原因策略的執行。此代理程式的喚醒原因層級已覆蓋：{{list}}。", "overrideTheWorkspaceTierOrProvider": "覆蓋此代理程式的工作區層級或提供者順序。", "overrideWakeReasonTiers": "覆蓋喚醒原因層級", "overrideWakeReasonTiersHelp": "較少使用。僅當此代理程式的後台工作需要不同行為時覆蓋；例如安全關鍵代理程式即使例行檢查也必須使用 Frontier。多數代理程式應繼承工作區策略。", @@ -630,6 +632,8 @@ "roleQa": "QA", "roleSecurity": "安全", "roleSpecialist": "專家", + "roleTierPolicy": "角色層級策略", + "roleTierRowInfo": "為具有此角色的每個代理程式設定預設模型層級，除非喚醒原因策略或按代理程式的覆蓋優先生效。", "roleWorker": "工作者", "routeAttempts": "路由嘗試", "routeBlocked": "已阻塞", @@ -719,6 +723,7 @@ "sessionId": "工作階段 ID", "sessionIdAfter": "工作階段 ID（之後）", "sessionIdBefore": "工作階段 ID（之前）", + "setADefaultTierForEveryAgentOfA": "為具有給定角色的每個代理程式設定預設層級。當某角色在此處沒有條目時，將後備到工作區預設層級；喚醒原因策略或按代理程式的覆蓋仍優先於此設定。", "setSpendingLimitsPerAgentOr": "按代理程式或項目設定支出限額。超出限額時代理程式會暫停。", "setupTierAgentProfiles": "設定層級代理程式設定檔", "setupTitle": "初始設置", @@ -834,6 +839,11 @@ "tierNameBalanced": "balanced", "tierNameEconomy": "economy", "tierNameFrontier": "frontier", + "tierShadowedByWakeReasons": "上方顯示的層級僅用於不匹配任何喚醒原因策略的執行。此工作區的喚醒原因層級已覆蓋：{{list}}。", + "tierSourceOverride": "來自覆蓋", + "tierSourceRole": "來自角色", + "tierSourceWakeReason": "來自喚醒原因", + "tierSourceWorkspace": "來自工作區", "timezone": "時區", "title": "標題", "titleForAutoCreatedTasksUse": "自動建立任務的標題。可用 {{name}} 與 {{date}} 作為佔位符。", @@ -863,6 +873,7 @@ "useAgentSNormalTier": "使用代理程式的常規層級", "usedBy": "用戶", "useQuery": "使用「{{query}}」", + "useWorkspaceDefaultTier": "使用工作區預設層級", "usingThisAgentsWakeReasonOverrides": "正在使用此代理程式的喚醒原因覆蓋。", "variables": "變量", "waiting": "等待中…", diff --git a/apps/web/src/locales/zh-tw/office.json b/apps/web/src/locales/zh-tw/office.json index 275e27ab78..319fbb6c6f 100644 --- a/apps/web/src/locales/zh-tw/office.json +++ b/apps/web/src/locales/zh-tw/office.json @@ -355,6 +355,7 @@ "inherit": "繼承", "inheritFromProjectWorkspace": "從專案/工作區繼承", "inheritFromWorkspace": "從工作區繼承", + "inheritsTierFromRole": "從角色繼承 <1>{{tier}}。", "inheritsTierFromWorkspace": "從工作區繼承 <1>{{tier}}。", "inheritsWorkspacePolicyList": "繼承工作區策略：{{list}}。", "inheritsWorkspacePolicyNoneSet": "繼承工作區策略（尚未設定喚醒原因層級）。", @@ -504,6 +505,7 @@ "outgoingDatabaseFilesystem": "傳出（資料庫 → 檔案系統）", "output": "輸出", "outputTokens": "輸出權杖", + "overrideShadowedByWakeReasons": "你的層級覆蓋僅用於不匹配任何喚醒原因策略的執行。此代理程式的喚醒原因層級已覆蓋：{{list}}。", "overrideTheWorkspaceTierOrProvider": "覆蓋此代理程式的工作區層級或提供者順序。", "overrideWakeReasonTiers": "覆蓋喚醒原因層級", "overrideWakeReasonTiersHelp": "較少使用。僅當此代理程式的後台工作需要不同行為時覆蓋；例如安全關鍵代理程式即使例行檢查也必須使用 Frontier。多數代理程式應繼承工作區策略。", @@ -630,6 +632,8 @@ "roleQa": "QA", "roleSecurity": "安全", "roleSpecialist": "專家", + "roleTierPolicy": "角色層級策略", + "roleTierRowInfo": "為具有此角色的每個代理程式設定預設模型層級，除非喚醒原因策略或按代理程式的覆蓋優先生效。", "roleWorker": "工作者", "routeAttempts": "路由嘗試", "routeBlocked": "已阻塞", @@ -719,6 +723,7 @@ "sessionId": "工作階段 ID", "sessionIdAfter": "工作階段 ID（之後）", "sessionIdBefore": "工作階段 ID（之前）", + "setADefaultTierForEveryAgentOfA": "為具有給定角色的每個代理程式設定預設層級。當某角色在此處沒有條目時，將備援到工作區預設層級；喚醒原因策略或按代理程式的覆蓋仍優先於此設定。", "setSpendingLimitsPerAgentOr": "按代理程式或專案設定支出限額。超出限額時代理程式會暫停。", "setupTierAgentProfiles": "設定層級代理程式設定檔", "setupTitle": "初始設置", @@ -834,6 +839,11 @@ "tierNameBalanced": "balanced", "tierNameEconomy": "economy", "tierNameFrontier": "frontier", + "tierShadowedByWakeReasons": "上方顯示的層級僅用於不匹配任何喚醒原因策略的執行。此工作區的喚醒原因層級已覆蓋：{{list}}。", + "tierSourceOverride": "來自覆蓋", + "tierSourceRole": "來自角色", + "tierSourceWakeReason": "來自喚醒原因", + "tierSourceWorkspace": "來自工作區", "timezone": "時區", "title": "標題", "titleForAutoCreatedTasksUse": "自動建立任務的標題。可用 {{name}} 與 {{date}} 作為佔位符。", @@ -863,6 +873,7 @@ "useAgentSNormalTier": "使用代理程式的常規層級", "usedBy": "使用者", "useQuery": "使用「{{query}}」", + "useWorkspaceDefaultTier": "使用工作區預設層級", "usingThisAgentsWakeReasonOverrides": "正在使用此代理程式的喚醒原因覆蓋。", "variables": "變數", "waiting": "等待中…", diff --git a/docs/specs/INDEX.md b/docs/specs/INDEX.md index 1795eaa9d3..b5f9997798 100644 --- a/docs/specs/INDEX.md +++ b/docs/specs/INDEX.md @@ -22,6 +22,7 @@ The office umbrella covers kandev's autonomous-agent product surface: workspaces | [scheduler](office/scheduler.md) | draft | | [runtime](office/runtime.md) | draft | | [routing](office/routing.md) | archived | +| [per-agent + per-role tier selection](office-agent-tier-routing/spec.md) | draft | | [costs](office/costs.md) | in-progress | | [dashboard](office/dashboard.md) | draft | | [live-updates](office/live-updates.md) | draft | diff --git a/docs/specs/office-agent-tier-routing/spec.md b/docs/specs/office-agent-tier-routing/spec.md new file mode 100644 index 0000000000..096916b4a7 --- /dev/null +++ b/docs/specs/office-agent-tier-routing/spec.md @@ -0,0 +1,924 @@ +--- +status: draft +created: 2026-08-20 +owner: nova28 +umbrella: office +extends: docs/specs/office/routing.md +--- + +# Office per-agent and per-role tier selection + +Office agents in one workspace must be able to run on different model tiers. Today +they can — a per-agent tier override already exists end to end — but the capability is +undiscoverable, the org has no way to express a tier as a property of a role, and the +Office agent record still carries a `model` field that routing ignores. The result is +an operator who configures a Critic on `opus[1m]`, watches it run on `sonnet`, and has +nothing in the product that explains the difference. + +This spec extends `docs/specs/office/routing.md`. That spec remains authoritative for +tiers, provider order, execution profiles, provider health, and wake-reason policy. +Nothing here changes those contracts except where explicitly named in +[Precedence](#precedence-contract). + +## Verified current state + +Verified 2026-08-20 against `~/.kandev/data/kandev.db` (workspace +`95542bf3-37e9-4fbc-9dbe-e2c774a5e7f6`) and the tree at `0b10edfa2`. + +### The reported symptom reproduces + +`office_run_route_attempts` holds ten attempts; every one resolved +`tier=balanced`, and the five most recent `claude-acp` launches used +`execution_profile_id=b139adcd-…` (profile "Sonnet", `model=sonnet`). + +`office_workspace_routing` for that workspace: + +```text +enabled = 1 +default_tier = balanced +provider_order = ["claude-acp","codex-acp"] +tier_per_reason = {"budget_alert":"economy","heartbeat":"economy","routine_trigger":"economy"} +``` + +### Why it happened — the override exists and was never set + +| Agent | `role` | `agent_profiles.model` | `settings` | Ran as | +|---|---|---|---|---| +| CEO | `ceo` | `opus[1m]` | `{"routing":{"provider_order_source":"inherit","tier_source":"inherit"}}` | sonnet | +| Critic | `specialist` | `opus[1m]` | `{}` | sonnet | +| Analyst | `specialist` | `sonnet` | `{}` | sonnet | +| Tech Lead | `specialist` | `sonnet` | `{}` | sonnet | +| Product Manager | `assistant` | `sonnet` | `{}` | sonnet | + +No agent carried `tier_source: "override"`. Every run therefore fell through to the +workspace default, which is the documented and correct behaviour. + +A per-agent tier override is already implemented on every layer: + +- **Type + validation** — `routing.AgentOverrides{TierSource, Tier}` and + `ValidateAgentOverridesAgainstWorkspace` (`internal/office/routing/types.go`). +- **Resolution** — `effectiveTier()` (`internal/office/routing/resolver.go`) already + honours `ov.TierSource == "override"` above the workspace default. +- **Persistence** — `routing.WriteAgentOverrides` onto `agent_profiles.settings`. +- **API** — `PATCH /office/agents/:id` with a `routing` body key + (`internal/office/agents/handler.go` → `applyRoutingOverride`). +- **UI** — "Override workspace tier" toggle plus a tier toggle group in + `apps/web/app/office/agents/[id]/components/agent-routing-card.tsx`. + +**The card's premise that no per-agent override exists is incorrect.** The defect is +discoverability, the absent role lever, and the misleading `model` field. + +### A role → tier map cannot solve the reported problem + +The card proposes a `role -> tier` map and calls it the better option. The verified +org makes that insufficient on its own: + +| `role` | agents | +|---|---| +| `ceo` | CEO | +| `assistant` | Product Manager | +| `specialist` | **Analyst, Tech Lead, Critic** | + +Analyst and Critic share `role = specialist`. The card's motivating case is exactly +"the Critic must not run the same model as the Analyst it checks", and a role → tier +map cannot express that. Roles are a fixed seven-value enum +(`ceo, worker, specialist, assistant, security, qa, devops`; +`internal/agent/settings/models/agent_attributes.go:16-24`) and are not user-extensible. + +Per-role is therefore specified here as a **bulk default**, never as the mechanism that +satisfies the Critic case. Per-agent remains the precise lever. + +### The `model` field really is ignored + +`models.AgentInstance` is a type alias for `settingsmodels.AgentProfile` +(`internal/office/models/models.go:57`) — Office identities and execution profiles are +rows in the same `agent_profiles` table, separated by `role != ''` and +`workspace_id != ''`. + +At launch the resolver takes the tier's execution-profile ID and reads **that** row's +`Model`. It explicitly refuses an Office identity in that position: +`resolveExecutionProfile` returns `profile %q is an Office agent identity, not an +execution profile` when `profile.Role != ""` (`resolver.go`). So an Office agent's own +`model` column can never reach a launch. + +Scope of the misleading surface, verified: + +- **Not shown in Settings → Agents.** `filterGlobalProfiles` + (`internal/agent/settings/controller/agent_crud.go:99`) drops rows with + `WorkspaceID != ""`, and all five Office identities are workspace-scoped. +- **Not shown on the Office agent page.** `agent-route-strip.tsx` and `agent-card.tsx` + render `preview.current_model` / `preview.primary_model` — the resolved model. +- **Is exposed by the API.** `AgentResponse` returns `*models.AgentInstance` verbatim, + and `AgentProfile.Model` carries `json:"model"`, so `GET /office/agents/:id` + advertises `"model": "opus[1m]"` for the Critic. `PATCH` already rejects + `agent_profile_id` with *"agent_profile_id no longer selects an Office runtime"*, + but says nothing about `model`. + +The accurate defect statement is therefore: **the Office agent API advertises a `model` +field that Office routing ignores**, not "the UI displays it". + +## Precedence contract + +### Deviation from the card's stated acceptance — read this first + +The card's acceptance asks for `per-agent > per-role > tier_per_reason > workspace`. +This spec deliberately specifies `tier_per_reason > per-agent > per-role > workspace` +instead. Reasons, in order of weight: + +1. **The card's own goal does not need the reversal.** The Critic and Analyst both ran + with reason `task_assigned`, for which no `tier_per_reason` key exists. The frozen + spec already guarantees that case falls through to the agent's effective tier + (`docs/specs/office/routing.md`, final wake-reason AC). Per-agent already wins where + the card needs it to win. +2. **The reversal breaks a shipped, documented guarantee.** + `docs/specs/office/routing.md` states: *"the resolver picks the Economy tier model + regardless of the agent's default tier"* for `tier_per_reason.heartbeat = economy`. + Reversing precedence silently voids that line. +3. **It would regress cost control.** `tier_per_reason` exists to cheap-out predictable + background work. Under the card's order, one agent pinned to `frontier` runs + `opus[1m]` on every heartbeat forever — the exact blowup the feature prevents. The + verified workspace maps all three wake reasons to `economy`. +4. **The card's intent already has a supported expression.** An agent that must stay on + Frontier even for heartbeats sets its own `tier_per_reason` override + (`TierPerReasonSource = "override"`), which is shipped, validated, and documented. + +This is a contract decision made under the board's "prefer a defensible decision" +rule rather than parking the card. **Spec Review: if this reasoning is rejected, the +correct disposition is NEEDS RETHINK back to Spec, not a Build-time edit.** + +### The contract + +For one run, the effective tier is the first of these that yields a non-empty tier: + +1. **Wake-reason policy** — agent `tier_per_reason` override when + `tier_per_reason_source == "override"`, otherwise workspace `tier_per_reason`, + keyed by the run's reason. Skipped entirely when the run reason is empty or absent + from the map. *(unchanged)* +2. **Per-agent tier** — `routing.tier` when `tier_source == "override"` and `tier` is + non-empty. *(unchanged)* +3. **Per-role tier** — the workspace `role_tiers` entry for the agent's `role`. *(new)* +4. **Workspace `default_tier`.** *(unchanged)* + +Only step 3 is added. Steps 1, 2 and 4 keep their current order and semantics. + +## Data model + +One new column on the existing `office_workspace_routing` row. No new table, and no +column on `agent_profiles` — keeping routing policy in one place, per the card's own +recommendation and consistent with how `tier_per_reason` is stored. + +```text +office_workspace_routing + role_tiers TEXT NOT NULL DEFAULT '{}' -- JSON map: role -> tier +``` + +- Keys are restricted to the seven `AgentRole` values. Any other key is rejected. +- Values are restricted to `frontier | balanced | economy`. +- An entry whose value is the empty string is treated as absent and is dropped before + persistence. +- `{}` means "no role policy" and is the default for every existing and new workspace. +- The Go field is `WorkspaceConfig.RoleTiers` (`internal/office/routing/types.go`, + beside `TierPerReason`), tagged `json:"role_tiers,omitempty"`. The workspace routing + PUT binds directly into `WorkspaceConfig` — there is no separate request DTO — so the + wire contract and the in-memory contract are the same type. + +Migration is additive with a default, so existing rows need no backfill and behaviour +is unchanged until an operator writes a map. + +## Acceptance criteria + +Written EARS-style. Each is observable through the API, the database, or the UI. + +### Per-role resolution + +- **AC-1** — GIVEN workspace `role_tiers = {"specialist":"frontier"}` and an agent with + `role = specialist` and no tier override, WHEN a run launches with a reason carrying + no wake-reason policy, THEN the resolved tier is `frontier` and + `office_run_route_attempts.tier` records `frontier`. +- **AC-2** — GIVEN the same config and an agent with `role = assistant` absent from + `role_tiers`, WHEN a run launches, THEN the resolved tier is the workspace + `default_tier`. +- **AC-3** — GIVEN `role_tiers = {"specialist":"economy"}` and a `specialist` agent + whose settings carry `tier_source = "override"`, `tier = "frontier"`, WHEN a run + launches with no wake-reason policy, THEN the resolved tier is `frontier` — the + per-agent override outranks the role entry. +- **AC-4** — GIVEN `role_tiers = {"specialist":"frontier"}`, workspace + `tier_per_reason = {"heartbeat":"economy"}`, and a `specialist` agent, WHEN a + heartbeat run launches, THEN the resolved tier is `economy` — the wake-reason policy + outranks the role entry. +- **AC-5** — GIVEN `role_tiers = {"specialist":"frontier"}` and a `specialist` agent + carrying `tier_source = "override"`, `tier = "balanced"`, and workspace + `tier_per_reason = {"heartbeat":"economy"}`, WHEN a heartbeat run launches, THEN the + resolved tier is `economy`, demonstrating the full four-level order in one case. +- **AC-6** — GIVEN an agent whose `role` is the empty string, WHEN a run launches, + THEN `role_tiers` is not consulted and resolution proceeds to `default_tier`. +- **AC-7** — GIVEN `role_tiers = {}`, WHEN any run launches, THEN the resolved tier is + identical to the tier resolved before this feature existed, for every agent in the + workspace. + +### Validation + +- **AC-8** — WHEN a workspace routing write carries a `role_tiers` key outside the + seven `AgentRole` values, THEN the write is rejected with HTTP 400 and a + `ValidationError` whose `Field` is `role_tiers` and whose `Details` carry one + `ValidationDetail` per offending key. +- **AC-8a** — The rejection's **wire shape** is the structured 400 that + `respondRoutingValidation` (`internal/office/agents/handler.go`) and the dashboard + routing handler already emit, and no other: + + ```json + {"error": "", "field": "", "details": [...]} + ``` + + `ValidationDetail` is `{ProviderID, Field, Message}` (`internal/office/routing/types.go`) + — there is **no** role member. The offending role therefore goes in + `ValidationDetail.Field` and the reason in `ValidationDetail.Message`, with + `ProviderID` left empty for `role_tiers` entries. This is pinned because "`Details` + name the offending key" does not by itself say which member carries it, and a builder + cannot read that off the struct. +- **AC-9** — WHEN a write carries a `role_tiers` value outside + `frontier | balanced | economy`, THEN the write is rejected with HTTP 400 in the + AC-8a shape, and the offending **role** and **value** both appear in the response — + the role in `ValidationDetail.Field`, the value quoted in `ValidationDetail.Message`. + The message must contain neither the string `default_tier` nor the prefix + `routing config invalid:`; AC-10a explains why both are live risks rather than + stylistic notes. +- **AC-10** — WHEN a write carries a `role_tiers` entry whose tier is mapped by no + provider in the workspace `provider_order`, THEN the write is rejected with HTTP 400 + rather than failing at launch. The structural precedent is + **`checkTierPerReasonMapped`**, *not* `checkTierMapped`: `role_tiers` is a **map** and + may hold several bad entries at once, so it needs the `[]ValidationDetail` + accumulation `checkTierPerReasonMapped` performs. `checkTierMapped` validates a + **single** value (`ov.Tier`) and returns `Field: "routing.tier"` with no `Details` at + all — copying it collapses N bad entries into one message and reports the wrong field. + Both are called from `ValidateAgentOverridesAgainstWorkspace`, so naming that function + alone does not pick the right one; this AC names the callee deliberately. +- **AC-10a** — `role_tiers` validation **requires a new field-parameterised validator**, + because the obvious existing one cannot satisfy AC-8. `validateTier(t Tier)` + (`internal/office/routing/types.go`) hardcodes `Field: "default_tier"` in the + `ValidationError` it returns and carries neither the role nor any `Details`. It is + already reused for the per-agent override (`validateTier(ov.Tier)`), so the + wrong-field pattern is **established in this repo** — reusing it for `role_tiers` + looks correct and is not. Two consequences: + 1. The `role_tiers` value validator takes the field name as a **parameter**, exactly + as `validateTierPerReason(m TierPerReason, field string)` does, and emits + `Field: "role_tiers"`. + 2. `validateTier`'s error must not be embedded in the message verbatim. + `ValidationError.Error()` renders as + `routing config invalid: default_tier: invalid tier "x"`, and + `validateTierPerReason` wraps exactly that string into its own `Message` — so even + the correct structural sibling leaks `default_tier` into user-facing text today. A + `role_tiers` message that inherits that wording violates AC-9. +- **AC-11** — WHEN a write carries a `role_tiers` entry with an empty-string value, + THEN the entry is dropped and the persisted map omits that key; the write succeeds. +- **AC-12** — GIVEN routing is disabled for the workspace (`enabled = 0`), WHEN a + `role_tiers` write arrives, THEN it is validated and persisted exactly as when + enabled; `enabled` gates automatic fallback, not tier selection. +- **AC-12a** — GIVEN `enabled = 0` and a non-empty `role_tiers`, WHEN a run launches, + THEN the role level still participates in tier resolution. `Resolve` computes the + effective tier before it branches on `Enabled`, and the single evaluated provider + (`provider_order[0]`) is looked up at the role-supplied tier. + +### The ignored `model` field + +- **AC-13** — WHEN `GET /office/agents/:id` returns an Office identity + (`role != ''`), THEN the `model` field is **omitted from the response body**. It is + not emitted as an empty string, and it is not emitted alongside a marker flag. + Omission is chosen over a marker so no consumer can read a value that has no effect. + For a row with `role == ''` (an execution profile) the field is unchanged. +- **AC-13a** — The omission is implemented as a **dedicated response DTO in + `internal/office/agents`** that **embeds** `models.AgentInstance` and **shadows** the + `model` key with its own field: + + ```go + type agentResponseBody struct { + *models.AgentInstance + Model *string `json:"model,omitempty"` + } + ``` + + Go resolves the JSON name collision by depth: the outer `Model` (depth 0) wins over + the embedded `AgentProfile.Model` (depth 1), so the embedded field never reaches the + wire. Embedding a type alias compiles, and the embedded field is referred to by the + alias name as written — `agentResponseBody{AgentInstance: p}`. + + The projection sets `Model` **only when `role == ''`** (pointing it at the row's + value) and leaves it **nil** for an Office identity, so `omitempty` drops the key. + This is what lets AC-13's two halves hold in one type, without the spec having to + assert which rows reach which handler. + + **The shadow field is `*string`, not `string`, and this is load-bearing.** With a + plain `string`, an execution profile (`role == ''`) whose `model` is the empty string + would have its `model` key **dropped** by `omitempty` — a silent shape change on + exactly the rows AC-13 promises are unchanged, because the shared struct tags `Model` + as `json:"model"` with **no** `omitempty` and therefore emits `"model":""` today. A + `*string` distinguishes "absent" (nil) from "present and empty" (non-nil), so the DTO + is shape-preserving by construction whether or not an empty `model` is reachable in + practice. Verified against the real tag set: with `*string`, `role != ''` emits no + `model` key and `role == ''` emits `"model":""` for an empty value, matching the + unwrapped struct. + + **Embedding is mandated over a field-by-field projection.** + `settingsmodels.AgentProfile` carries **43 JSON-tagged fields**, several computed or + `db:"-"`, with inconsistent `omitempty` usage. Hand-mirroring them is 43 chances to + drop a field or mistype a tag, and every such slip is a silent response-shape change + for execution-profile and kanban consumers. Embedding copies zero tags and so cannot + drift when a field is later added to the shared struct — which the + `RouteAttemptDTO` / `routeAttemptToDTO` pattern in + `internal/office/dashboard/routing_dto.go` does not give us here, because that DTO + mirrors a 17-field model this feature owns, not a 43-field struct shared with two + other subsystems. + + Two alternatives remain **explicitly forbidden**: a custom `MarshalJSON` on + `settingsmodels.AgentProfile` (a type alias shared with execution profiles and kanban + — it would silently change their JSON too), and a `map[string]any` post-process (it + drops compile-time field checking). The shared struct and its `json:"model"` tag are + not edited. + + **The existing wrappers change element type, not shape.** `AgentResponse.Agent` + becomes `*agentResponseBody` and `AgentListResponse.Agents` becomes + `[]*agentResponseBody` (`internal/office/agents/dto.go`). Their JSON keys stay `agent` + and `agents`; no new wrapper type is introduced and no envelope key is renamed. A nil + agent stays a nil **wrapper field**, serialising as `"agent": null` exactly as today — + do not substitute a zero-valued DTO, which would serialise as `{}` because a nil + embedded pointer contributes no promoted fields. + + **Key ORDER changes and that is accepted.** Embedding emits the shadowing `Model` + where the outer field is declared, so for `role == ''` the `model` key moves to the + end of the object. The key SET and every value are identical. JSON object member + order is not semantically significant, no in-repo consumer depends on it, and this is + why AC-13c compares decoded maps rather than bytes. Earlier revisions of this AC said + "byte-identical"; that was never achievable by either mechanism and is corrected here. +- **AC-13c** — The shape-preservation property AC-13a asserts is **observable, not + merely asserted**. A Go test in `internal/office/agents` marshals the **same** + `models.AgentInstance` value twice — once directly, once through the response DTO — + decodes both into `map[string]any`, and asserts: + - for a row with `role != ''`: the DTO map equals the direct map **with the `model` + key deleted** (so no other key is added, dropped, or altered), and the DTO map + contains no `model` key; + - for a row with `role == ''`: the DTO map **equals the direct map exactly**, + including a `model` key whose value is `""` when the row's model is empty. + + The comparison is over decoded maps, never raw bytes, per AC-13a's key-order note. + This AC exists because a mechanism whose whole job is "change one key and nothing + else" is worthless without a test that can see "nothing else"; it is the standing + guard for AC-13, AC-13a and AC-13b together. +- **AC-13b** — The omission applies to **every Office-identity agent payload the + `internal/office/agents` handler emits**, not only `GET /office/agents/:id`. Five + handlers emit the agent struct: `listAgents` (via `AgentListResponse`), `createAgent` + (201), `getAgent`, `updateAgent`, and `updateAgentStatus`. All five return the DTO. + A response from any of the five that still carries a `model` key for a row with + `role != ''` fails this AC. +- **AC-14** — WHEN `PATCH /office/agents/:id` carries a `model` field for an Office + identity, THEN the request is rejected with HTTP 400 and a message directing the + caller to workspace tier profiles or the agent routing override. The rejection is + *worded* like the existing `agent_profile_id` rejection, but is **not implemented the + same way** — see AC-14a for why the mechanisms necessarily differ, and AC-14c for the + response shape, which AC-14a does not cover. +- **AC-14c** — The rejection's **wire shape** is the AC-8a structured 400, produced by + `respondRoutingValidation` with + `&routing.ValidationError{Field: "model", Message: ""}` — and + **not** the bare `gin.H{"error": ...}` form the neighbouring `agent_profile_id` + rejection uses. Two rejections in one handler function therefore carry different + envelopes, and that is intended rather than an oversight: the `agent_profile_id` bare + form is the older shape, `respondRoutingValidation` is already called by + `applyRoutingOverride` a few lines away in the same file, and AC-8/AC-9/AC-10 commit + the rest of this feature to the structured form. A caller that receives this 400 can + then read `field == "model"` instead of pattern-matching prose. + + **AC-14's "worded like" governs the sentence only, never the envelope.** The existing + `agent_profile_id` rejection is **not** changed to match: rewriting it is a separate + API change this feature is not authorised to make. +- **AC-14a** — `UpdateAgentRequest` has no `model` member, and Gin's `ShouldBindJSON` + uses stdlib `encoding/json`, which ignores unknown keys — so today a `model` key in a + PATCH body is silently discarded. Detection is therefore implemented by buffering the + request body **once** with `io.ReadAll(c.Request.Body)`, then decoding those same + bytes twice: once into `map[string]json.RawMessage` to test for **presence of the + `"model"` key specifically**, and once into `UpdateAgentRequest`. The handler must + therefore stop calling `c.ShouldBindJSON`, which consumes the body and would leave the + second decode empty; binding from the buffered bytes replaces it, and the existing + malformed-JSON 400 behaviour is preserved. + + **On precedent — read this before going looking for one.** Three handlers already + buffer a Gin request body with `io.ReadAll(c.Request.Body)`: + `internal/office/routines/handler.go`, `internal/office/channels/handler.go` and + `internal/system/frontenderrors/handler.go`. They are precedent for **the buffering + step only**. **None of them does key-presence detection**, and copying any of them + wholesale produces the wrong thing: + - `routines/handler.go` buffers for HMAC verification, then unmarshals to + `map[string]interface{}` to build webhook variables — no typed struct, no presence + test. + - `channels/handler.go` buffers for signature verification and treats the body as raw + text; it never JSON-decodes it at all. + - `frontenderrors/handler.go` decodes into a typed struct with + **`DisallowUnknownFields()`**, then decodes a second time expecting `io.EOF` — a + trailing-garbage guard, and it uses the very technique forbidden two paragraphs + below. It is the closest-looking and the most misleading of the three. + + The `map[string]json.RawMessage` presence check exists nowhere in this backend today. + **It is new code**, and the spec says so rather than sending a builder to find a + pattern that is not there. + + `json.Decoder.DisallowUnknownFields` is **explicitly forbidden**: it would reject + every unknown key on this endpoint, a far broader breaking API change than this + feature is authorised to make. That `frontenderrors/handler.go` uses it is not a + licence — that endpoint accepts one closed payload shape; this one does not. The + existing `agent_profile_id` check stays a plain nil-check on the declared + `AgentProfileID *string` field; that path is unchanged. +- **AC-14b** — Presence, not value, triggers the rejection. `{"model": "opus[1m]"}`, + `{"model": ""}` and `{"model": null}` are **all rejected** with the same HTTP 400, + because each proves the caller believes the field is honoured. A body that omits the + `model` key entirely is accepted. For a row with `role == ''` the key is not rejected. +- **AC-15** — GIVEN an existing Office identity row with a non-empty `model` value, + WHEN this feature ships, THEN that stored value is left untouched in the database and + continues to have no effect on routing. No destructive migration runs. + +### Discoverability + +- **AC-16** — GIVEN an agent inherits its tier (no per-agent override, no role entry), + WHEN the operator views that agent's routing card, THEN the card names the tier in + force **and** the level that supplied it. The four wire values are exactly + `wake_reason | override | role | workspace` (AC-18); the card renders their + translated display labels. Wire values and display labels are distinct — the wire + value is never shown raw and is never translated. +- **AC-16a** — The same rule binds the **workspace routing preview table** + (`app/office/workspace/routing/components/agent-preview-table.tsx`), which today + renders `{a.tier_source}` raw in a user-facing cell. WHEN that table renders a row, + THEN it shows the translated display label for the row's `tier_source`, never the raw + wire value. Without this AC the widening ships the new literals `role` and + `workspace` untranslated into a shipped table, violating AC-16's own principle and + AC-19's five-locale requirement on a surface AC-16 does not reach. +- **AC-17** — GIVEN a per-agent tier override is shadowed by a wake-reason policy for + some reason, WHEN the operator views the agent's routing card, THEN the card states + that the override does not apply to those reasons and names them. +- **AC-17a** — The reasons named are the keys of the agent's **effective** wake-reason + map, which is the same map `wakeReasonTier` consults: WHEN + `tier_per_reason_source == "override"`, the effective map is the agent's own + `tier_per_reason` **and the workspace map is not consulted at all** (an override + replaces it entirely, per `docs/specs/office/routing.md`); OTHERWISE the effective map + is the workspace `tier_per_reason`. The card never shows the union of the two, and + never shows the workspace keys when an override is in force. Keys whose value is + empty are excluded, since they do not shadow anything. GIVEN the effective map is + empty, THEN the card shows no shadowing notice at all. +- **AC-17b** — The shadowing notice is **not** limited to a per-agent override. A + role-supplied tier is shadowed by wake-reason policy identically (AC-4), and AC-18b + guarantees a preview never reports `wake_reason`, so without this AC an agent on a + role tier would see a card asserting the role tier is in force while its heartbeat + runs actually take Economy, with nothing said. WHEN the agent's effective tier comes + from the **role** level or the **workspace default**, and the effective wake-reason + map is non-empty, THEN the card states that the named reasons do not use the + displayed tier, using the same notice and the same key set as AC-17a. No new data + path is required: `agent-routing-card.tsx` already holds the workspace map via + `useWorkspaceRouting` and the agent's own map via the persisted `overrides` blob. +- **AC-18** — WHEN the routing preview (`PreviewItem`) is produced for an agent, THEN + `TierSource` reports exactly one of `wake_reason | override | role | workspace`, + widening the current two-valued `override | inherit`. +- **AC-18a** — The value `override` keeps its current meaning and spelling, so an + existing consumer testing `tier_source === "override"` is unaffected. The value + `inherit` is **removed from the computed preview field only** and replaced by `role` + or `workspace`. + + **The widening is scoped by TYPE, not by file.** Two unrelated fields are both spelled + `tier_source`, and exactly one of them changes. Several files contain both, so + "update file X" is not a safe instruction: + + | | Type | Widens? | + |---|---|---| + | **Computed preview** — CHANGES | Go `routing.PreviewItem.TierSource` → Go `dashboard.AgentRoutePreview.TierSource` → TS `AgentRoutePreview.tier_source` | **YES** — becomes `wake_reason \| override \| role \| workspace` | + | **Persisted override** — UNCHANGED | Go `routing.AgentOverrides.TierSource` (`json:"tier_source,omitempty"`) → TS `AgentRoutingOverrides.tier_source` | **NO** — stays `inherit \| override \| ""` | + + **Sites that MUST change** (the computed chain, in dataflow order): + 1. `internal/office/routing/provider.go#tierSourceForAgent` — today the **only** site + that emits the literal `"inherit"`; it must instead yield `role` / `workspace`, + which requires the agent's role and the workspace `role_tiers` map. Per **AC-20d** + it does not decide this itself: it delegates to the widened `effectiveTier`, so + that preview and audit share one producer. "Sole producer" describes the situation + before this feature, not after it. + 2. `internal/office/routing/provider.go` where the result is set on `PreviewItem`. + 3. `internal/office/dashboard/handler_routing.go` where it is copied into + `AgentRoutePreview` (`previewItemsToDTOs`). + 4. `internal/office/dashboard/routing_dto.go` — the doc comment on `AgentRoutePreview` + currently states the two-valued contract and becomes false. + 5. TS `AgentRoutePreview.tier_source` in `lib/state/slices/office/types.ts`. + 6. `agent-preview-table.tsx`, per AC-16a. + 7. Tests fixturing the computed value — **both** of them, Go and TypeScript: + `internal/office/dashboard/handler_routing_test.go`, and + `apps/web/lib/state/slices/office/office-routing.test.ts`, whose + `setRoutingPreview` case fixtures a preview row with `tier_source: "inherit"`. + That TS fixture is the **computed** preview, not the persisted override, so it + stops compiling the moment item 5 lands. It is easy to mistake for a + MUST-NOT-CHANGE site because the same file name pattern appears in that list + below; it is not one. + + This list is maintained by hand and has been wrong before. It is a starting point, + not a proof of completeness — **AC-18c is the proof**. Work the list, then run the + typecheck gate and treat whatever it reports as in scope. + + **Sites that MUST NOT change** (persisted state — changing these is a regression): + - `routing.AgentOverrides.TierSource` and TS `AgentRoutingOverrides.tier_source`. + - `agent-routing-card.tsx` where it **writes** `tier_source: on ? "override" : + "inherit"` and reads `overrides.tier_source !== "override"` — that is the persisted + override blob being PATCHed, not the preview. + - `e2e/helpers/office-api-client.ts`, whose `tier_source` sits inside `overrides`. + - `internal/office/onboarding/service.go#writeAgentInheritMarkers`, which stamps + `TierSource: "inherit"` as the CEO's onboarding marker. + - `e2e/tests/office/office-routing-disabled.spec.ts`, which asserts + `route.overrides.tier_source === "inherit"`. + + A stored settings blob containing `tier_source: "inherit"` keeps its persisted + spelling and continues to mean "not an override". +- **AC-18b** — GIVEN a preview is produced through `Provider.Preview` / + `Provider.PreviewAgent`, WHEN resolution runs, THEN it runs with an empty run reason, + so the wake-reason level cannot apply and `TierSource` never reports `wake_reason` in + a preview. AC-17 (override case) and AC-17b (role / workspace case) are what surface + wake-reason shadowing in that view; between them every level a preview can report + carries a shadowing notice. +- **AC-18c** — WHEN the `tier_source` widening has been applied, THEN + `pnpm --filter @kandev/web typecheck` passes with no error mentioning `tier_source`. + + This AC exists because AC-18a's site list is maintained by hand and has been + incomplete in successive reviews — the union of "files that mention `tier_source`" is + not knowable by reading, and the type checker knows it exactly. Narrowing + `AgentRoutePreview.tier_source` from `"inherit" | "override"` to + `"wake_reason" | "override" | "role" | "workspace"` makes every stale `"inherit"` + fixture or comparison a compile error, so the gate is exhaustive by construction where + a list can only ever be a best effort. + + Any site the gate reports is **in scope for this feature**, whether or not AC-18a + names it — with one exception that is a genuine failure, not a site to update: an + error on a MUST-NOT-CHANGE site from AC-18a means the **persisted** override type was + widened by mistake. Fix that by reverting the persisted type, never by widening the + fixture to match. + + This mirrors the shape AC-19 already uses — delegate completeness to a checker that + can see everything, rather than to a list that cannot. AC-13c is the same idea for + the agent-payload half. +- **AC-19** — WHEN new user-facing copy is added for AC-16 through AC-18, THEN it is + routed through `t()` / `` and present in all five locales + (`en`, `pt-pt`, `zh-cn`, `zh-hk`, `zh-tw`), with `pnpm run i18n:check` passing. + +### Observability + +- **AC-20** — WHEN a run resolves its tier, THEN the supplying level is persisted on + the route attempt in a new `office_run_route_attempts.tier_source` column + (`TEXT NOT NULL DEFAULT ''`), holding one of + `wake_reason | override | role | workspace`. +- **AC-20c** — `''` means **"the supplying level is not recorded"**, and it is never + interpreted as `workspace`. It arises from exactly two causes, which a consumer + separates by reading the sibling `tier` column: + + | `tier` | `tier_source` | meaning | + |---|---|---| + | non-empty | `''` | row written before this column shipped | + | `''` | `''` | the attempt never resolved a tier | + | non-empty | non-empty | normal post-migration row | + + The second case is real and is **not** a pre-migration artefact: the + `max_attempts_exceeded` attempt appended by + `internal/office/scheduler/dispatch_routing.go` sets no `Tier` today and will + likewise set no `TierSource`. The invariant this AC fixes is therefore + **`tier_source` is non-empty only when `tier` is non-empty**; the converse does not + hold, because legacy rows carry a tier and no source. A migration that back-fills + `tier_source` for legacy rows is forbidden — the level that produced those tiers was + never recorded and cannot be reconstructed. + + **The widened `effectiveTier` preserves this invariant at its own fallthrough.** Its + last step returns `cfg.DefaultTier`; when that value is empty it returns an empty tier + and an **empty source**, never `workspace`. A source is reported only when a level + actually supplied a tier. The case is defensive rather than live — `validateTier` + rejects an empty tier and the column is declared + `default_tier TEXT NOT NULL DEFAULT 'balanced'`, so no supported write reaches it — + but AC-20c states its invariant without qualification, so the fallthrough is pinned + here rather than left for the builder to decide. +- **AC-20b** — The stored value is **surfaced, not write-only**. `tier_source` is added + to `dashboard.RouteAttemptDTO` as `json:"tier_source,omitempty"`, mapped in + `routeAttemptToDTO` beside the existing `Tier` field. It therefore reaches both + payloads that already carry `RouteAttemptDTO` — `GET /runs/:id/attempts` + (`RouteAttemptsResponse`) and the run-detail response (`RunRouting.Attempts`) — with + no new endpoint. The `omitempty` tag means a row whose level is unrecorded (`''`, + either cause in AC-20c) omits the key rather than reporting a false level. + + The **TypeScript client contract widens with it**: `RouteAttempt` in + `apps/web/lib/state/slices/office/types.ts` gains + `tier_source?: "wake_reason" | "override" | "role" | "workspace"`. That one type backs + every consumer the field must reach — `RouteAttemptsResponse` for + `GET /runs/:id/attempts` and the run-detail `attempts` array (both in + `apps/web/lib/api/domains/office-runs-api.ts`), and the WS payload + (`apps/web/lib/ws/handlers/office.ts`). Without this the field is on the wire but + unreadable from TypeScript, and "surfaced, not write-only" is false in practice. + Note this is the **route-attempt** `tier_source`, a third type spelled the same way; + AC-18a's table governs the other two and is unaffected. + + **The `route_attempt_appended` WS event gains the field too, and that is intended.** + `publishRouteAttemptAppended` (`internal/office/scheduler/routing_events.go`) + serialises `models.RouteAttempt` **whole** as `payload["attempt"]`, so the field + AC-20f adds to that struct appears on the event automatically, at all three publish + sites. This is additive and `omitempty`-guarded, so no existing consumer breaks. Build + must **not** suppress it: the two suppression routes available — dropping the JSON tag + AC-20f specifies verbatim, or introducing a separate WS-only DTO — are both forbidden + here, the first because it contradicts AC-20f and the second because it would create + the second producer AC-20d exists to prevent. + + **Explicitly out of scope for this AC:** any new **UI rendering** of the value — no + control, no column, no badge. The field becomes available to the run-detail UI and to + WS consumers; presenting it is a later change. +- **AC-20d** — The value has **one producer, shared by the resolve path and the preview + path**, so an agent's audit record and its preview can never disagree about the level. + `effectiveTier` (`internal/office/routing/resolver.go`) is widened to return the tier + **and** its source — it already receives `cfg` and `ov`, and gains the agent's `role` + for the new level — and `tierSourceForAgent` (`provider.go`, today the preview-only + producer of the literal `"inherit"`) is reimplemented to delegate to it rather than + deciding independently. Two independent producers are **explicitly forbidden**: that + is the defect this AC exists to prevent. +- **AC-20e** — The source is carried out of resolution on + `routing.Resolution` as a new `TierSource string` field beside the existing + `RequestedTier` (`resolver.go`). `Resolution` is the only carrier; the source is not + threaded through `ResolveOptions`, not recomputed by the scheduler, and not re-derived + from the agent at write time. +- **AC-20f** — The persistence path is, in order: + 1. `internal/office/models/models.go` — `RouteAttempt` gains a `TierSource string` + field beside `Tier`, carrying the JSON tag `tier_source,omitempty` and the db tag + `tier_source`: + + ```go + TierSource string `json:"tier_source,omitempty" db:"tier_source"` + ``` + 2. `internal/office/repository/sqlite/base_migrations.go` — the additive column is + added with the replayable form already used one line above for this same table: + `r.migrate.Apply("office_run_route_attempts.tier_source", "ALTER TABLE + office_run_route_attempts ADD COLUMN tier_source TEXT NOT NULL DEFAULT ''")`. + **The migration for this column does not live in `workspace_routing.go`** — that + file holds the `office_workspace_routing` read/write SQL only, and the `role_tiers` + migration likewise belongs in `base_migrations.go`. + 3. `internal/office/repository/sqlite/route_attempts.go` — `tier_source` is added to + the `AppendRouteAttempt` INSERT column list and its parameter list, and to the + `ListRouteAttempts` SELECT list so `StructScan` populates it. + 4. `internal/office/scheduler/dispatch_routing.go` — the **three** + `AppendRouteAttempt` call sites set `TierSource` wherever they already set `Tier` + — and they do **not** divide evenly. Each of the three needs a different edit: + + | site | `res` in scope? | sets `Tier` today? | edit | + |---|---|---|---| + | `parkRunMaxAttempts` | **no** | no | **none** — sets neither field, per AC-20c | + | `parkRunBlocked` | yes (`res *routing.Resolution` parameter) | yes, `Tier: string(res.RequestedTier)` | add `TierSource: res.TierSource` beside it | + | `recordAttemptStart` | **no** — receives a bare `tier routing.Tier` | yes, from that parameter | **thread** a source parameter, supplied at the call site | + + So the split is **one no-op, one direct read, one threaded parameter** — there is + no second direct-read site, and a builder who goes looking for one is looking for + something that does not exist. `parkRunMaxAttempts` in particular takes no + `*routing.Resolution` at all; threading one into it to "complete the pattern" is + forbidden by this AC and by AC-20g. + + For the threaded site: `recordAttemptStart`'s signature today ends in a bare + `tier routing.Tier` parameter and gains a source parameter alongside it, supplied + from `res.TierSource` at its call site, where `res` **is** already in scope. + Deriving the source inside `recordAttemptStart` instead is forbidden by AC-20d. +- **AC-20g** — Two nearby sites are **explicitly NOT write sites** and must not be + changed: the `prior = append(prior, models.RouteAttempt{…})` in + `dispatch_routing.go` is an in-memory mirror used for fallback bookkeeping by + `latestFailedExecutionProfile`, not a persistence call; and the attempt-finalisation + path updates outcome columns on an existing row and does not write `tier_source`. +- **AC-20a** — `office_run_route_attempts` carries no `agent_id` column; attribution is + via `run_id` to the owning run. This feature adds no agent column, so a + "which tier did agent X get" query remains a join through `runs`. + +## Forced-to-invent pass + +Decisions a builder would otherwise have to guess. Each is a contract. + +### Ordering and tiebreak + +- **AC-21** — The four precedence levels are evaluated in the fixed order in + [The contract](#the-contract). The order is not configurable. +- **AC-22** — `role_tiers` is a map keyed by a unique role, so no two entries can + apply to one agent and **no tiebreak is required**. An agent has exactly one `role` + (a single non-null column), so at most one entry matches. +- **AC-23** — WHEN `role_tiers` is serialised to JSON for persistence or API response, + THEN keys are emitted sorted ascending by the role string using byte order, so a + round-trip is byte-stable and diffs are reviewable. `map` iteration order must not + reach the wire. +- **AC-24** — WHEN the routing settings UI lists roles, THEN they are ordered by the + declaration order of the `AgentRole` constants + (`ceo, worker, specialist, assistant, security, qa, devops`), not alphabetically and + not by map order. + +### Idempotency and retry + +- **AC-25** — WHEN the same `role_tiers` payload is written twice, THEN the second + write succeeds and leaves the persisted value byte-identical. `updated_at` still + advances; it records the write, not a value change. +- **AC-26** — A `role_tiers` write is a whole-map replacement, not a merge. WHEN a + payload omits a role that is currently mapped, THEN that role's entry is deleted. + This matches how `tier_per_reason` and `provider_order` already behave. +- **AC-26a** — An **absent `role_tiers` key** on a routing write means the same as + `{}`: the stored map is cleared. It does not mean "preserve the current value". This + follows the endpoint's existing whole-config semantics — `TierPerReason` is likewise + `omitempty`, unmarshals to nil when the key is absent, and is written unconditionally + by the `office_workspace_routing` upsert — and making one field alone + absent-means-preserve would be a new and surprising rule on a PUT where every other + field is replaced. The consequence is that a client which does not yet know about + `role_tiers` erases it, so both in-repo writers are updated to round-trip the field: + the workspace routing page, and the `cfg` parameter type of the routing-config PUT + helper in `apps/web/e2e/helpers/office-api-client.ts`. (This is distinct from that + file's `tier_source`, which sits inside `overrides` and stays untouched per AC-18a.) +- **AC-27** — WHEN a routing write fails validation, THEN no part of it is persisted; + `role_tiers` and every other field are written in one transaction. +- **AC-28** — Tier resolution is a pure read. Retrying a failed launch re-resolves from + current config; a tier is never cached on the run and never frozen at enqueue time. + A config change between attempt N and attempt N+1 therefore takes effect on N+1, and + the two attempts may legitimately record different tiers. + +### Concurrency + +- **AC-29** — GIVEN two callers write `role_tiers` for the same workspace + concurrently, WHEN both succeed, THEN the last committed write wins in full and the + row is never left holding a mixture of the two maps. `office_workspace_routing` is + keyed by `workspace_id`, so this is a single-row update. +- **AC-30** — GIVEN a `role_tiers` write commits while a run is mid-resolution, WHEN + that resolution completes, THEN it uses whichever value its own read observed and is + never retried on that basis, and the run is not aborted. Both halves are observable: + the resolved tier is recorded on the route attempt, and a retry would appear as a + second attempt row. + + *Design note, not an acceptance criterion:* no lock is taken across the launch. That + is a statement about the implementation, not an outcome any test, API response, or DB + query can witness — this section's preamble promises every AC is observable, and as an + AC clause it would not have been. +- **AC-31** — GIVEN an agent's `role` changes while it has queued runs, WHEN those runs + launch, THEN each resolves against the role in force at its own resolution time. Role + changes are not applied retroactively to already-resolved runs. + +### Nil, empty and error behaviour + +- **AC-32** — WHEN the `role_tiers` column holds `''`, `'{}'`, or SQL `NULL`, THEN it + decodes to an empty map and resolution proceeds to `default_tier`. None of the three + is an error. +- **AC-32a** — **AC-32 outranks AC-33 on the empty string.** `''` is simultaneously + "empty" under AC-32 and "JSON that fails to decode" under AC-33, because + `json.Unmarshal([]byte(""), ...)` returns *unexpected end of JSON input*. AC-32 wins + because it names `''` explicitly; AC-33 governs only **non-empty** bytes that fail to + parse (`{`, `not json`, a truncated object). + + **The adjacent `tier_per_reason` decode does NOT behave this way, and copying it is + the failure mode.** `loadWorkspaceRouting` + (`internal/office/repository/sqlite/workspace_routing.go`) selects + `COALESCE(tier_per_reason, '{}')` and unmarshals the result. `COALESCE` substitutes + for SQL `NULL` **only** — a literal `''` passes through untouched and then errors in + `json.Unmarshal`. The pattern sitting one line from where `role_tiers` will be read + therefore implements **AC-33's behaviour on AC-32's input**. The `role_tiers` decode + needs an explicit empty-string short-circuit the precedent lacks: treat a zero-length + raw value as `{}` **before** unmarshalling, then unmarshal. + + This is a defect in the precedent as a template, not in `tier_per_reason` as shipped — + that column is also `NOT NULL DEFAULT '{}'` and no supported write puts `''` in it. + `role_tiers` inherits the same protection, so this too is defensive. It is specified + because AC-32 states its contract unconditionally, and a builder extending the + neighbouring line would break it without ever seeing a failure. +- **AC-33** — WHEN the `role_tiers` column holds JSON that fails to decode, THEN + loading the workspace routing config returns an error and the launch is refused with + the existing blocked-route path. It does **not** silently fall back to + `default_tier`: a corrupt policy must be visible, not quietly ignored. +- **AC-34** — WHEN an agent's `role` holds a value not in the seven-value enum (a row + written by an older build or by hand), THEN `role_tiers` is not consulted for it, + resolution proceeds to `default_tier`, and the launch is not refused. +- **AC-34a** — WHEN a persisted `role_tiers` map holds an entry with an empty-string + tier (bypassing AC-11, e.g. written by hand), THEN that entry is treated as absent at + resolution time and the agent falls through to `default_tier`. It is not an error and + does not refuse the launch; AC-33's refusal is reserved for JSON that fails to decode. +- **AC-34b** — WHEN a persisted `role_tiers` map holds a key outside the seven-value + enum, THEN that entry is ignored at resolution time rather than refusing the launch. + AC-8 keeps such keys out on the write path; the read path stays permissive so a role + removed from the enum in a future build cannot brick every launch in the workspace. +- **AC-35** — WHEN the role-supplied tier is mapped by no provider in the effective + order at launch time, THEN the existing `missing_model_mapping` skip path applies + unchanged; this feature adds no new blocked-route status. + +### Defaults and boundaries + +- **AC-36** — The default value of `role_tiers` is `{}` for every workspace, existing + and new. Onboarding does not seed it. +- **AC-37** — `role_tiers` holds at most seven entries, bounded by the enum. A payload + with more keys than the enum has values is rejected by AC-8 before size matters, so + no separate length limit is specified. +- **AC-38** — Writing `role_tiers` for a role that currently has no agents is valid and + persists; the map is a policy, not a join, and it applies when such an agent is later + created. +- **AC-39** — WHEN an agent's per-agent tier override is cleared, THEN its effective + tier falls to the role entry if one exists, and to `default_tier` otherwise, with no + further operator action. + +## Precedent citations + +This spec cites in-repo code as precedent about a dozen times. Three review rounds have +now each found at least one citation where **copying the named precedent produces +behaviour that violates an AC in this same spec**: AC-14a's three body-buffering +handlers (round 3), then AC-32a's `tier_per_reason` decode and AC-10a's `validateTier` +(round 4). Those instances are fixed above. This section exists to close the class. + +**The standing rule.** Every precedent named in this spec is cited for a **named step**, +never wholesale, and each citation states what copying it produces. A citation reading +only "matching X" or "as Y already does" is incomplete — treat it as a defect in this +spec to be routed back, not as licence to copy X or Y. + +Why prose alone is not enough: all three rounds' traps were already *adjacent to* text +telling the builder to be careful, and a warning is only read by a builder who suspects +there is something to look for. The precedents in question look correct, compile, and +pass their own existing tests. So the guarantee is delegated to checks that go red. + +- **AC-40** — For every AC whose behaviour a cited precedent would violate, there is a + test **that the precedent fails**. This feature ships at minimum these four: + 1. a decode test feeding the `role_tiers` column a literal `''` and asserting an empty + map and **no error** (AC-32/AC-32a) — fails if the `COALESCE`-only pattern is + copied; + 2. a validation test asserting the rejection for a bad `role_tiers` **value** carries + `field == "role_tiers"` and a message containing neither `default_tier` nor + `routing config invalid:` (AC-9/AC-10a) — fails if `validateTier`'s error is + returned or wrapped verbatim; + 3. a validation test asserting that **two** bad `role_tiers` entries produce **two** + `Details` entries (AC-8/AC-10) — fails if `checkTierMapped`'s single-value shape is + copied; + 4. a handler test asserting the PATCH `model` rejection body carries + `field == "model"` (AC-14c) — fails if the bare `gin.H{"error": ...}` form is copied + from the `agent_profile_id` rejection. + + Four is a floor, not a ceiling: any further precedent the builder chooses to follow + earns the same treatment. AC-40 is the same delegation AC-18c and AC-13c already make + — move the guarantee off a list a human maintains and onto a check a machine runs. + +## Out of scope + +Named exclusions. Each is a deliberate contract, not an omission. + +- **Reversing the wake-reason / per-agent precedence.** Justified in + [Deviation](#deviation-from-the-cards-stated-acceptance--read-this-first). If it is + wanted, it is a separate change to `docs/specs/office/routing.md`. +- **User-defined roles.** `AgentRole` stays a fixed seven-value enum. Making roles + extensible would give the Critic case a role-based answer, but it is a much larger + change to Office identity and is not attempted here. +- **A `tier` column on `agent_profiles`.** The card offers it as an alternative; + rejected so routing policy stays in `office_workspace_routing`, matching + `tier_per_reason`. +- **Removing or backfilling the `model` column on Office identity rows.** AC-15 keeps + the stored value. Dropping the column touches the shared `agent_profiles` table used + by execution profiles and kanban, and is not in this scope. +- **Per-agent or per-role provider *order* defaults.** Only tier selection gains a role + level. `provider_order` keeps its existing two levels. +- **Per-project, per-task, or per-skill tier selection.** +- **Automatic tier suggestions**, or any heuristic that picks a tier for a role without + the operator saying so. +- **Changing which model IDs the tiers map to.** `provider_profiles` is untouched. +- **Cost reporting or budget enforcement changes** arising from role-differentiated + tiers. + +## Surfaces touched (E2E decision input) + +- **Backend** — `internal/office/routing/{types,resolver,provider}.go` (incl. + `provider.go#tierSourceForAgent`, which after AC-20d delegates to the widened + `effectiveTier` rather than deciding the source itself), + `internal/office/repository/sqlite/workspace_routing.go` (`role_tiers` read/write SQL), + `internal/office/repository/sqlite/base_migrations.go` (**both** additive migrations — + `office_workspace_routing.role_tiers` and `office_run_route_attempts.tier_source`; + this is where the replayable `ALTER` precedent for these tables lives), + `internal/office/models/models.go` (`RouteAttempt.TierSource`, AC-20f), + `internal/office/repository/sqlite/route_attempts.go` (INSERT + SELECT, AC-20f), + `internal/office/scheduler/dispatch_routing.go` (three `AppendRouteAttempt` sites, + which need **three different edits** — see AC-20f's table), + `internal/office/dashboard/handler_routing.go`, + the AC-40 precedent-trap tests (decode in + `internal/office/repository/sqlite`, validation in `internal/office/routing`, the + PATCH rejection in `internal/office/agents`), + `internal/office/dashboard/routing_dto.go` (`AgentRoutePreview` doc comment + + `RouteAttemptDTO` per AC-20b), `internal/office/agents/handler.go` and a new + embedding response DTO in `internal/office/agents` (AC-13a) with its shape-preservation + test (AC-13c). **Not edited but affected:** + `internal/office/scheduler/routing_events.go` needs no change yet publishes the new + field automatically, because it serialises `models.RouteAttempt` whole (AC-20b). +- **API** — workspace routing GET/PUT gains `role_tiers`; all five Office-identity agent + payloads drop `model` (AC-13b); `PATCH /office/agents/:id` rejects a `model` key + (AC-14); `GET /runs/:id/attempts` and the run-detail payload gain `tier_source` + (AC-20b); the `route_attempt_appended` WS event gains it additively as a consequence, + which AC-20b accepts rather than suppresses. +- **Frontend** — `app/office/workspace/routing/` gains a role-tier card; + `agent-preview-table.tsx` renders a translated source label (AC-16a); + `app/office/agents/[id]/components/agent-routing-card.tsx` and `agent-route-strip.tsx` + gain the four-level source label; TS `AgentRoutePreview` in + `lib/state/slices/office/types.ts` widens and TS `RouteAttempt` in the same file gains + `tier_source?` (AC-20b) — TS `AgentRoutingOverrides` does **not** (AC-18a); + `lib/state/slices/office/office-routing.test.ts` updates its `setRoutingPreview` + fixture (AC-18a item 7); TS `WorkspaceRouting` gains `role_tiers` and the + routing-config PUT helper in `e2e/helpers/office-api-client.ts` gains it too (AC-26a). + This list is indicative; **AC-18c's typecheck gate is authoritative** for the + `tier_source` widening. +- **Explicitly NOT touched** (persisted-state sites, per AC-18a) — + `internal/office/onboarding/service.go#writeAgentInheritMarkers`; the `tier_source` + **inside the `overrides` blob** in `e2e/helpers/office-api-client.ts`; and the + persisted-override reads/writes in `agent-routing-card.tsx`. Note the same e2e helper + *is* edited elsewhere, for an unrelated field: its routing-config PUT `cfg` type gains + `role_tiers` per AC-26a. The untouched thing is the override `tier_source`, not the + file. +- **i18n** — new copy in five locales (AC-19), including the four source labels and the + AC-16a table label. + +User-visible UI changes in the Office routing settings and the agent routing card mean +**E2E coverage is warranted**, scoped to the existing Office routing specs +(`apps/web/e2e/tests/office/office-routing-*.spec.ts`) rather than a new suite.