diff --git a/src/main/model-dispatch.ts b/src/main/model-dispatch.ts index 43b6baaf..734e0eda 100644 --- a/src/main/model-dispatch.ts +++ b/src/main/model-dispatch.ts @@ -14,25 +14,27 @@ import { inferTaskProfile } from "./agent-role-helpers"; /** * Resolves which (provider, modelId) to use for an agent at run dispatch. * - * The rule is spelled out in the design discussion: + * Rules: * - * 1. `explicit` strategy (or a CEO role): - * use agent.connectorId + agent.model verbatim. CEO is always pinned - * to whatever the user chose on the Agent form. + * 1. `explicit` strategy, or a CEO role (`reportsTo IS NULL` / + * `roleLevel === 'c_suite'`): use the agent's own connectorId + + * model verbatim. * - * 2. Only one enabled (provider, modelId) pair exists anywhere: - * use it. Whether it's the CEO's pick or a lone API-key connector, - * single-model installs should not exercise the dispatcher at all. + * 2. The enabled (connector, model) universe has exactly one pair: + * use it. Single-model installs bypass scoring entirely. * - * 3. `auto` strategy with multiple candidates: - * infer the agent's task profile (domain ∈ reasoning|coding|writing, - * required tier ∈ trivial|standard|hard), filter enabled candidates - * to those whose capability row clears both the domain score and the - * tier floor, and pick the cheapest among them. + * 3. `auto` strategy with multiple candidates: infer the agent's + * task profile (domain ∈ reasoning|coding|writing, required tier + * ∈ trivial|standard|hard), keep candidates that clear the domain + * score floor and the tier rank floor, and pick the cheapest. * - * 4. No candidate clears the bar in `auto`: - * fall back to the CEO's (connectorId, model). Runs must not fail - * because the capability matrix is under-seeded — No regressions. + * 4. `auto` strategy with no qualifying candidate: fail. Silently + * routing to the CEO's pair hid three distinct problems — a + * missing capability row, a mis-inferred profile, a misconfigured + * model pool — and made cost attribution meaningless. The caller + * converts the `ModelPickError` into a visible eligibility + * failure so operators see the real issue instead of "the agent + * ran on an unexpected model for a week". */ export interface EnabledModel { @@ -46,17 +48,19 @@ export interface AgentAssignment { modelId: string | null; } +/** + * Tagged union returned by `pickModelForAgent`. Callers must branch on + * `ok` — success yields a dispatch-ready assignment; failure yields a + * reason code that `getRunEligibilityFailure`'s surrounding handler + * turns into a user-visible eligibility error. + */ +export type ModelPickResult = + | { ok: true; assignment: AgentAssignment } + | { ok: false; reason: "NO_QUALIFYING_CANDIDATE"; message: string }; + export interface PickModelDeps { getConnector: (id: ConnectorId) => ConnectorRecord; getDisabledModels: (companyId: string, connectorId: string) => Array<{ id: string; label: string }>; - /** - * Thin projection of the CEO for fallback. Returns `null` when the - * company has no CEO yet (pre-bootstrap). The picker only needs enough - * to route a run, so we don't demand the full AgentRecord — keeping the - * surface narrow lets callers use the existing `db.getCeoForCompany` - * together with a single `getAgent` lookup for the fallback pair. - */ - getCeoAssignment: (companyId: string) => { connectorId: ConnectorId; modelId: string | null } | null; listModelCapabilities: () => ModelCapability[]; readConnectors: () => ConnectorRecord[]; } @@ -147,14 +151,15 @@ export function pickFromCandidates( } /** - * The one-call entry point that run dispatch uses. Returns the pair to - * burn into the run record; callers pass it straight into `createRun` and - * the worker payload. + * The one-call entry point that run dispatch uses. Returns either a + * dispatch-ready assignment or a structured failure so callers can + * translate the miss into an eligibility error — no more silent CEO + * fallback. */ export function pickModelForAgent( agent: AgentRecord, deps: PickModelDeps, -): AgentAssignment { +): ModelPickResult { const strategy = agent.modelStrategy ?? "explicit"; // CEO is pinned to the user's choice. The canonical CEO marker is @@ -164,7 +169,10 @@ export function pickModelForAgent( // seniority enum. We also accept the enum for completeness. const isCeo = agent.reportsTo === null || agent.roleLevel === "c_suite"; if (strategy === "explicit" || isCeo) { - return { connectorId: agent.connectorId, modelId: agent.model ?? null }; + return { + ok: true, + assignment: { connectorId: agent.connectorId, modelId: agent.model ?? null }, + }; } const enabled = listEnabledModels(agent.companyId, deps); @@ -172,7 +180,7 @@ export function pickModelForAgent( // Single-model install: there's nothing to pick from. Use it. if (enabled.length === 1) { const only = enabled[0]!; - return { connectorId: only.connectorId, modelId: only.modelId }; + return { ok: true, assignment: { connectorId: only.connectorId, modelId: only.modelId } }; } // Empty universe → fall back to whatever is on the agent row. This can @@ -180,23 +188,27 @@ export function pickModelForAgent( // higher up in run dispatch will still block the run, but we return // something sensible so the caller's contract stays total. if (enabled.length === 0) { - return { connectorId: agent.connectorId, modelId: agent.model ?? null }; + return { + ok: true, + assignment: { connectorId: agent.connectorId, modelId: agent.model ?? null }, + }; } const profile = inferTaskProfile(agent.role, agent.title, agent.roleLevel); const winner = pickFromCandidates(profile, enabled, deps.listModelCapabilities()); if (!winner) { - // Fall back to the CEO's explicit pair. If there is no CEO (fresh - // company, board-only), fall back to agent's own row so we never - // throw. The upstream readiness check owns whether to let the run - // actually start. - const ceo = deps.getCeoAssignment(agent.companyId); - if (ceo) return ceo; - return { connectorId: agent.connectorId, modelId: agent.model ?? null }; + return { + ok: false, + reason: "NO_QUALIFYING_CANDIDATE", + message: `No enabled model clears the ${profile.requiredTier} ${profile.domain} bar for agent ${agent.name}. Enable a qualifying model, adjust the capability catalog, or switch this agent to modelStrategy=explicit.`, + }; } - return { connectorId: winner.connectorId, modelId: winner.modelId }; + return { + ok: true, + assignment: { connectorId: winner.connectorId, modelId: winner.modelId }, + }; } /** diff --git a/src/main/orchestration/heartbeat.ts b/src/main/orchestration/heartbeat.ts index 8de6f3be..3abb4224 100644 --- a/src/main/orchestration/heartbeat.ts +++ b/src/main/orchestration/heartbeat.ts @@ -19,6 +19,7 @@ export type HeartbeatSkipReason = | "budget_blocked" | "connector_not_ready" | "connector_model_not_allowed" + | "model_pick_failed" | "max_concurrent_runs" | "cooldown" | "wake_on_demand_disabled" @@ -71,18 +72,25 @@ export function createHeartbeat( logger.warn(`[heartbeat] Refusing cross-company run for agent ${agentId}: ${agent.companyId} !== ${companyId}`); return SKIPPED("cross_company"); } - const assignment = pickModelForAgent(agent, { + const pick = pickModelForAgent(agent, { getConnector: (id) => db.getConnector(id), getDisabledModels: (id) => db.getDisabledModels(agent.companyId, id), - getCeoAssignment: (cid) => { - const ceo = db.getCeoForCompany(cid); - if (!ceo) return null; - const full = db.getAgent(ceo.id); - return { connectorId: full.connectorId, modelId: full.model ?? null }; - }, listModelCapabilities: () => db.modelCapabilities.listModelCapabilities(), readConnectors: () => db.connectorsCrud.readConnectors(), }); + if (!pick.ok) { + logger.warn(`[heartbeat] Model pick failed for agent ${agent.name}: ${pick.message}`); + db.addActivity({ + companyId, + actor: "system", + action: "heartbeat.model_pick_failed", + entityType: "agent", + entityId: agent.id, + detail: pick.message, + }); + return SKIPPED("model_pick_failed"); + } + const assignment = pick.assignment; const connector = db.getConnector(assignment.connectorId); const effectiveAgentForEligibility: AgentRecord = { ...agent, model: assignment.modelId }; const eligibilityFailure = getRunEligibilityFailure(effectiveAgentForEligibility, connector, null); diff --git a/src/main/orchestration/task-runner.ts b/src/main/orchestration/task-runner.ts index 69eba253..df59db89 100644 --- a/src/main/orchestration/task-runner.ts +++ b/src/main/orchestration/task-runner.ts @@ -47,18 +47,16 @@ export function createTaskRunner( if (agent.companyId !== task.companyId) { return fail("AGENT_COMPANY_MISMATCH", "The assigned agent does not belong to the task company."); } - const assignment = pickModelForAgent(agent, { + const pick = pickModelForAgent(agent, { getConnector: (id) => db.getConnector(id), getDisabledModels: (id) => db.getDisabledModels(agent.companyId, id), - getCeoAssignment: (cid) => { - const ceo = db.getCeoForCompany(cid); - if (!ceo) return null; - const full = db.getAgent(ceo.id); - return { connectorId: full.connectorId, modelId: full.model ?? null }; - }, listModelCapabilities: () => db.modelCapabilities.listModelCapabilities(), readConnectors: () => db.connectorsCrud.readConnectors(), }); + if (!pick.ok) { + return fail("MODEL_PICK_FAILED", pick.message); + } + const assignment = pick.assignment; const connector = db.getConnector(assignment.connectorId); const effectiveAgentForEligibility: AgentRecord = { ...agent, model: assignment.modelId }; const eligibilityFailure = getRunEligibilityFailure(effectiveAgentForEligibility, connector, task.projectId ?? null); diff --git a/tests/unit/model-dispatch.test.ts b/tests/unit/model-dispatch.test.ts index 0d2cf6b9..906e4bca 100644 --- a/tests/unit/model-dispatch.test.ts +++ b/tests/unit/model-dispatch.test.ts @@ -78,7 +78,6 @@ function makeConnector(id: string): ConnectorRecord { function makeDeps(options: { enabledModels: Array<{ connectorId: string; provider: string; modelId: string }>; capabilities: ModelCapability[]; - ceoAssignment?: { connectorId: string; modelId: string | null } | null; }): PickModelDeps { const connectorsById = new Map(); for (const entry of options.enabledModels) { @@ -93,13 +92,6 @@ function makeDeps(options: { return c; }, getDisabledModels: () => [], - getCeoAssignment: () => - options.ceoAssignment - ? { - connectorId: options.ceoAssignment.connectorId as ConnectorId, - modelId: options.ceoAssignment.modelId, - } - : null, listModelCapabilities: () => options.capabilities, readConnectors: () => Array.from(connectorsById.values()), }; @@ -155,8 +147,11 @@ describe("pickModelForAgent", () => { ], capabilities: [claudeOpus], }); - const assignment = pickModelForAgent(agent, deps); - expect(assignment).toEqual({ connectorId: "anthropic-api", modelId: "claude-opus-4-7" }); + const pick = pickModelForAgent(agent, deps); + expect(pick.ok).toBe(true); + if (pick.ok) { + expect(pick.assignment).toEqual({ connectorId: "anthropic-api", modelId: "claude-opus-4-7" }); + } }); it("CEO role always takes the user-picked pair regardless of modelStrategy", () => { @@ -164,6 +159,7 @@ describe("pickModelForAgent", () => { modelStrategy: "auto", roleLevel: "c_suite", role: "CEO", + reportsTo: "some-parent", connectorId: "anthropic-api" as ConnectorId, model: "claude-opus-4-7", }); @@ -174,15 +170,12 @@ describe("pickModelForAgent", () => { ], capabilities: [claudeOpus, llama], }); - const assignment = pickModelForAgent(agent, deps); - expect(assignment.modelId).toBe("claude-opus-4-7"); + const pick = pickModelForAgent(agent, deps); + expect(pick.ok).toBe(true); + if (pick.ok) expect(pick.assignment.modelId).toBe("claude-opus-4-7"); }); it("CEO is identified by reportsTo === null even when roleLevel stays at the schema default 'ic'", () => { - // Regression test for the v39 migration gap: CEO agents created before - // the explicit role_level hookup have roleLevel = 'ic' in the DB. The - // dispatcher must still recognise them via the DB invariant used by - // getCeoForCompany: reports_to IS NULL. const agent = makeAgent({ modelStrategy: "auto", roleLevel: "ic", @@ -198,8 +191,38 @@ describe("pickModelForAgent", () => { ], capabilities: [claudeOpus, llama], }); - const assignment = pickModelForAgent(agent, deps); - expect(assignment.modelId).toBe("claude-opus-4-7"); + const pick = pickModelForAgent(agent, deps); + expect(pick.ok).toBe(true); + if (pick.ok) expect(pick.assignment.modelId).toBe("claude-opus-4-7"); + }); + + it("auto strategy with no qualifying candidate fails explicitly (no silent CEO fallback)", () => { + const agent = makeAgent({ + modelStrategy: "auto", + roleLevel: "ic", + reportsTo: "manager-1", + role: "Senior Architect", + title: "Principal Architect", + connectorId: "fireworks" as ConnectorId, + model: "llama-70b", + }); + // Two enabled models, both scoring too low for a hard reasoning agent. + const deps = makeDeps({ + enabledModels: [ + { connectorId: "fireworks", provider: "fireworks", modelId: "llama-70b" }, + { connectorId: "fireworks-b", provider: "fireworks", modelId: "llama-7b" }, + ], + capabilities: [ + llama, + { provider: "fireworks", modelId: "llama-7b", reasoning: 2, coding: 2, writing: 2, tier: "trivial", costPerMtokOut: 0.5, updatedAt: "" }, + ], + }); + const pick = pickModelForAgent(agent, deps); + expect(pick.ok).toBe(false); + if (!pick.ok) { + expect(pick.reason).toBe("NO_QUALIFYING_CANDIDATE"); + expect(pick.message).toContain(agent.name); + } }); });