From 1f55bd9c79fa3058635217d46ce1fc38f7855ed1 Mon Sep 17 00:00:00 2001 From: Justin Chu Date: Thu, 11 Jun 2026 11:45:41 -0700 Subject: [PATCH 1/4] fix: agent model selection persistence, display, and provider routing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five root causes along the same data path: - db/schema: agents.model was added via raw ALTER but missing from the drizzle schema, so select() never returned it — UI selections looked unsaved. Declare model (+ context_window_*) in the schema. - config paths: web API wrote role/lead model config to internal project storage while gateway/spawn read from project cwd (or process.cwd()). Unify on config.cwd ?? project.subpath('.') everywhere. - orchestrator auto-spawn passed no cwd; new ModelConfig(undefined) threw and silently skipped runtime/model resolution, landing agents on the adapter default provider. Add config-dir fallback and pass cwd + autoResolve from the orchestrator. - restartAgent respawned with model: undefined and no runtime, dropping the persisted selection. Pass agent.model and agent.runtimeName. - AcpAdapter session init only applied the model via the 'model' configOption; runtimes without it silently ignored the selection while the UI still displayed it. Prefer unstable_setSessionModel, fall back to configOptions. Also thread runtime through setAgentModel (UI dropdowns list models across runtimes), persist auto-resolved runtime names, and group the detail-panel model select by runtime. Co-Authored-By: Claude Fable 5 --- packages/server/src/agents/AcpAdapter.ts | 19 +++++- packages/server/src/agents/AgentManager.ts | 44 ++++++++++++-- packages/server/src/api/HttpServer.ts | 5 +- packages/server/src/api/routes/agents.ts | 2 +- packages/server/src/cli/gateway.ts | 11 ++-- packages/server/src/db/schema.ts | 3 + .../server/src/orchestrator/Orchestrator.ts | 4 ++ packages/server/src/storage/SqliteStore.ts | 9 ++- .../server/tests/agents/agent-manager.test.ts | 33 ++++++++++ packages/server/tests/storage/sqlite.test.ts | 24 ++++++++ packages/shared/src/core/types.ts | 2 + .../web/src/components/AgentDetailPanel.tsx | 60 +++++++++++++------ packages/web/src/lib/api.ts | 4 +- packages/web/src/pages/Agents.tsx | 6 +- 14 files changed, 187 insertions(+), 39 deletions(-) diff --git a/packages/server/src/agents/AcpAdapter.ts b/packages/server/src/agents/AcpAdapter.ts index 021102ff..c4e33114 100644 --- a/packages/server/src/agents/AcpAdapter.ts +++ b/packages/server/src/agents/AcpAdapter.ts @@ -600,8 +600,23 @@ export class AcpAdapter extends AgentAdapter { // Set model if specified and different from default if (session.model && result.models?.currentModelId !== session.model) { - // Use configOptions for model setting - const modelConfigOption = result.configOptions?.find( + let applied = false; + // Prefer the standard session model API when the runtime advertises + // the requested model — configOptions alone silently drops the + // selection on runtimes that don't expose a 'model' config option + if (result.models?.availableModels?.some((m: { modelId: string }) => m.modelId === session.model)) { + try { + await session.connection.unstable_setSessionModel({ + sessionId: result.sessionId, + modelId: session.model, + }); + applied = true; + } catch { + // Fall through to configOptions + } + } + // Fallback: use configOptions for model setting + const modelConfigOption = applied ? undefined : result.configOptions?.find( (opt: { id: string }) => opt.id === 'model' ); if (modelConfigOption) { diff --git a/packages/server/src/agents/AgentManager.ts b/packages/server/src/agents/AgentManager.ts index 3e72d6d3..7f45f80b 100644 --- a/packages/server/src/agents/AgentManager.ts +++ b/packages/server/src/agents/AgentManager.ts @@ -297,7 +297,11 @@ export class AgentManager { let resolvedRuntime = opts.runtime; try { const { ModelConfig } = await import('./ModelConfig.js'); - const mc = new ModelConfig(opts.cwd); + const { FD_HOME } = await import('../cli/constants.js'); + // opts.cwd may be absent (e.g. orchestrator auto-spawn) — fall back to + // internal project storage so role model config is still resolved + const configDir = opts.cwd ?? join(FD_HOME, 'projects', opts.projectName ?? this.projectName); + const mc = new ModelConfig(configDir); const enabledModels = mc.getRoleEnabledModelsWithDiscovery(opts.role); const disabledRts: string[] = (opts as any).disabledRuntimes ?? []; const activeModels = enabledModels.filter(m => m.enabled && !disabledRts.includes(m.runtime)); @@ -349,7 +353,7 @@ export class AgentManager { // 6. Spawn via adapter // For Claude Code runtime, inject role instructions via _meta.systemPrompt (append mode) // This provides stronger guidance than AGENTS.md alone - const isClaudeCode = opts.runtime === 'claude' || opts.runtime === 'claude-agent'; + const isClaudeCode = resolvedRuntime === 'claude' || resolvedRuntime === 'claude-agent'; try { const meta = await this.adapter.spawn({ agentId: newId, @@ -369,6 +373,11 @@ export class AgentManager { this.store.updateAgentStatus(newId, 'idle'); if (displayModel) this.store.updateAgentModel(newId, displayModel); else if (meta.model) this.store.updateAgentModel(newId, meta.model); + // Persist the resolved runtime when it was auto-resolved (not passed in) + if (resolvedRuntime && resolvedRuntime !== opts.runtime) { + this.store.updateAgentRuntimeName(newId, resolvedRuntime); + agent.runtimeName = resolvedRuntime; + } agent.acpSessionId = meta.sessionId; agent.status = 'idle'; @@ -538,14 +547,34 @@ export class AgentManager { this.audit(agentId, agent.role, 'agent:steer', `Steered ${agent.role}`, { messageLength: message.length }); } - async setAgentModel(agentId: AgentId, model: string): Promise { + async setAgentModel(agentId: AgentId, model: string, runtime?: string): Promise { const agent = this.store.getAgent(agentId); if (!agent) throw new Error(`Agent not found: ${agentId}`); // Persist model to DB this.store.updateAgentModel(agentId, model); - // Also update live session if available + + // The model may belong to a different runtime than the agent currently + // runs on (the UI lists models across all runtimes). Resolve the owning + // runtime so restarts route to the right provider. + let targetRuntime = runtime; + if (!targetRuntime) { + try { + const { modelRegistry } = await import('./ModelRegistry.js'); + const current = agent.runtimeName ?? undefined; + const owners = modelRegistry.getRuntimes().filter(rt => + modelRegistry.getModels(rt).some(m => m.modelId === model)); + // Prefer the agent's current runtime when it also offers the model + targetRuntime = (current && owners.includes(current)) ? current : owners[0]; + } catch { /* registry unavailable — keep current runtime */ } + } + const runtimeChanged = !!targetRuntime && targetRuntime !== (agent.runtimeName ?? undefined); + if (runtimeChanged) this.store.updateAgentRuntimeName(agentId, targetRuntime!); + + // Update the live session only when the model belongs to the current + // runtime — a foreign model ID can't be applied to a running session + // and takes effect on the next restart instead. const sessionId = this.agentToSession.get(agentId) ?? agent.acpSessionId; - if (sessionId && typeof (this.adapter as any).setModel === 'function') { + if (sessionId && !runtimeChanged && typeof (this.adapter as any).setModel === 'function') { try { await (this.adapter as any).setModel(sessionId, model); } catch { /* live session may not support it — that's OK, persisted for next spawn */ } @@ -581,7 +610,10 @@ export class AgentManager { agentId, role: agent.role, cwd: process.cwd(), - model: undefined, + // Respawn with the agent's persisted model/runtime — otherwise the + // restart silently lands on the adapter's default provider + model: agent.model ?? undefined, + runtime: agent.runtimeName ?? undefined, systemPrompt, }); diff --git a/packages/server/src/api/HttpServer.ts b/packages/server/src/api/HttpServer.ts index f736893c..982ab769 100644 --- a/packages/server/src/api/HttpServer.ts +++ b/packages/server/src/api/HttpServer.ts @@ -73,7 +73,10 @@ export function createHttpServer(deps: HttpServerDeps): Server { let mc = modelCfgCache.get(projName); if (!mc) { const { ModelConfig: MC } = await import('../agents/ModelConfig.js'); - mc = new MC(fd.project.subpath('.')); + // Must match the directory agent spawn paths read from (project cwd, + // falling back to internal storage) — otherwise UI model selections + // are written to a config.yaml that spawn never reads. + mc = new MC(fd.status().config.cwd ?? fd.project.subpath('.')); modelCfgCache.set(projName, mc); } return mc; diff --git a/packages/server/src/api/routes/agents.ts b/packages/server/src/api/routes/agents.ts index d127659d..6b75ca2e 100644 --- a/packages/server/src/api/routes/agents.ts +++ b/packages/server/src/api/routes/agents.ts @@ -173,7 +173,7 @@ export async function handleAgentRoutes( try { const body = await readBody(); if (!body.model) { json(400, { error: 'Missing required field: model' }); return true; } - await am.setAgentModel(agentId as import('@flightdeck-ai/shared').AgentId, body.model); + await am.setAgentModel(agentId as import('@flightdeck-ai/shared').AgentId, body.model, body.runtime); json(200, { success: true }); } catch (e: unknown) { json(500, { error: `Failed to set agent model: ${e instanceof Error ? e.message : String(e)}` }); diff --git a/packages/server/src/cli/gateway.ts b/packages/server/src/cli/gateway.ts index b44a3447..07814b15 100644 --- a/packages/server/src/cli/gateway.ts +++ b/packages/server/src/cli/gateway.ts @@ -441,8 +441,10 @@ export async function startGateway(deps: GatewayDeps): Promise { } } - // Read per-role runtime config from .flightdeck/config.yaml in project cwd - const projectCwd = fd.status().config.cwd ?? process.cwd(); + // Read per-role runtime config from .flightdeck/config.yaml in project cwd, + // falling back to internal project storage (same resolution as the web API + // writes — never process.cwd(), which depends on where the server started). + const projectCwd = fd.status().config.cwd ?? fd.project.subpath('.'); const { ModelConfig } = await import('../agents/ModelConfig.js'); const modelConfig = new ModelConfig(projectCwd); const leadRoleConfig = modelConfig.getRoleConfig('lead'); @@ -591,8 +593,9 @@ export async function startGateway(deps: GatewayDeps): Promise { const profile = fd.status().config.governance; console.error(`\n── Hot-register project: ${name} (profile: ${profile}) ──`); - // ModelConfig — read from project cwd, not internal storage - const projectCwd = fd.status().config.cwd ?? process.cwd(); + // ModelConfig — read from project cwd, falling back to internal storage + // (same resolution as the web API writes — never process.cwd()) + const projectCwd = fd.status().config.cwd ?? fd.project.subpath('.'); const { ModelConfig } = await import('../agents/ModelConfig.js'); const modelConfig = new ModelConfig(projectCwd); const leadRoleConfig = modelConfig.getRoleConfig('lead'); diff --git a/packages/server/src/db/schema.ts b/packages/server/src/db/schema.ts index 5142524a..b33520a2 100644 --- a/packages/server/src/db/schema.ts +++ b/packages/server/src/db/schema.ts @@ -51,6 +51,9 @@ export const agents = sqliteTable('agents', { currentSpecId: text('current_spec_id'), costAccumulated: real('cost_accumulated').notNull().default(0), lastHeartbeat: text('last_heartbeat'), + model: text('model'), + contextWindowTokens: integer('context_window_tokens'), + contextWindowLimit: integer('context_window_limit'), }, (table) => [ index('idx_agents_status').on(table.status), index('idx_agents_role').on(table.role), diff --git a/packages/server/src/orchestrator/Orchestrator.ts b/packages/server/src/orchestrator/Orchestrator.ts index 487305aa..0cfaed23 100644 --- a/packages/server/src/orchestrator/Orchestrator.ts +++ b/packages/server/src/orchestrator/Orchestrator.ts @@ -715,6 +715,10 @@ Continuation behavior: model: task.model, runtime: task.runtime, task: task.id as string, + cwd: this.config.cwd, + // Unattended spawn: fall back to the role's configured default + // model when the task doesn't pin one, instead of erroring + autoResolve: true, }; this.agentManager.spawnAgent(spawnOpts as any).then(agent => { log('Orchestrator', `Auto-spawned agent ${agent.id} for task "${truncate(task.title, 50)}"`); diff --git a/packages/server/src/storage/SqliteStore.ts b/packages/server/src/storage/SqliteStore.ts index cd7817cb..aab91e33 100644 --- a/packages/server/src/storage/SqliteStore.ts +++ b/packages/server/src/storage/SqliteStore.ts @@ -441,6 +441,13 @@ export class SqliteStore extends EventEmitter { this._db.run(sql`UPDATE agents SET model = ${model} WHERE id = ${agentId}`); } + updateAgentRuntimeName(agentId: AgentId, runtimeName: string): void { + this._db.update(agents) + .set({ runtimeName }) + .where(eq(agents.id, agentId)) + .run(); + } + updateTaskDescription(taskId: TaskId, description: string): void { this._db.update(tasks) .set({ description, updatedAt: new Date().toISOString() }) @@ -513,7 +520,7 @@ export class SqliteStore extends EventEmitter { currentSpecId: (row.currentSpecId ?? null) as SpecId | null, costAccumulated: row.costAccumulated, lastHeartbeat: (row.lastHeartbeat ?? null) as string | null, - model: (row as any).model ?? undefined, + model: row.model ?? undefined, }; } diff --git a/packages/server/tests/agents/agent-manager.test.ts b/packages/server/tests/agents/agent-manager.test.ts index 5b1b7411..9b6d9f43 100644 --- a/packages/server/tests/agents/agent-manager.test.ts +++ b/packages/server/tests/agents/agent-manager.test.ts @@ -128,6 +128,39 @@ describe('AgentManager', () => { expect(restarted.acpSessionId).toBe('mock-session-2'); }); + it('restartAgent re-spawns with the persisted model and runtime', async () => { + // Regression: restart used to pass model: undefined and no runtime, + // silently re-routing the agent to the adapter's default provider. + const agent = await manager.spawnAgent({ role: 'worker', cwd: '/tmp', autoResolve: true, model: 'gpt-4', runtime: 'codex' }); + await manager.restartAgent(agent.id); + + expect(adapter.spawnCalls).toHaveLength(2); + expect(adapter.spawnCalls[1].model).toBe('gpt-4'); + expect(adapter.spawnCalls[1].runtime).toBe('codex'); + }); + + it('setAgentModel persists the model so it survives reads', async () => { + const agent = await manager.spawnAgent({ role: 'worker', cwd: '/tmp', autoResolve: true }); + await manager.setAgentModel(agent.id, 'claude-sonnet-4.6'); + expect(store.getAgent(agent.id)!.model).toBe('claude-sonnet-4.6'); + }); + + it('setAgentModel with explicit runtime updates the agent runtime for respawn routing', async () => { + const agent = await manager.spawnAgent({ role: 'worker', cwd: '/tmp', autoResolve: true, runtime: 'codex' }); + await manager.setAgentModel(agent.id, 'claude-sonnet-4.6', 'claude'); + const updated = store.getAgent(agent.id)!; + expect(updated.model).toBe('claude-sonnet-4.6'); + expect(updated.runtimeName).toBe('claude'); + }); + + it('spawnAgent without cwd still resolves and does not crash model resolution', async () => { + // Regression: orchestrator auto-spawn passes no cwd; new ModelConfig(undefined) + // threw and silently skipped runtime/model resolution. + const agent = await manager.spawnAgent({ role: 'worker', autoResolve: true } as any); + expect(agent.status).toBe('idle'); + expect(adapter.spawnCalls).toHaveLength(1); + }); + it('terminateAgent throws for unknown agent', async () => { await expect(manager.terminateAgent('nonexistent' as AgentId)) .rejects.toThrow('Agent not found'); diff --git a/packages/server/tests/storage/sqlite.test.ts b/packages/server/tests/storage/sqlite.test.ts index 142348c5..0bf08cc5 100644 --- a/packages/server/tests/storage/sqlite.test.ts +++ b/packages/server/tests/storage/sqlite.test.ts @@ -95,6 +95,30 @@ describe('SqliteStore', () => { expect(store.getAgent('agent-test' as AgentId)!.status).toBe('busy'); }); + it('persists and reads back agent model and runtime name', () => { + // Regression: agents.model was written via raw SQL but missing from the + // drizzle schema, so select() never returned it and the UI showed the + // selection as unsaved. + const agent: Agent = { + id: 'agent-model' as AgentId, + role: 'worker', + runtime: 'acp', + runtimeName: 'copilot', + acpSessionId: null, + status: 'idle', + currentSpecId: null, + costAccumulated: 0, + lastHeartbeat: null, + }; + store.insertAgent(agent); + store.updateAgentModel('agent-model' as AgentId, 'claude-sonnet-4.6'); + expect(store.getAgent('agent-model' as AgentId)!.model).toBe('claude-sonnet-4.6'); + expect(store.listAgents()[0].model).toBe('claude-sonnet-4.6'); + + store.updateAgentRuntimeName('agent-model' as AgentId, 'claude'); + expect(store.getAgent('agent-model' as AgentId)!.runtimeName).toBe('claude'); + }); + it('tracks costs', () => { const entry: CostEntry = { agentId: 'agent-1' as AgentId, diff --git a/packages/shared/src/core/types.ts b/packages/shared/src/core/types.ts index 99777f00..e7bf1748 100644 --- a/packages/shared/src/core/types.ts +++ b/packages/shared/src/core/types.ts @@ -96,6 +96,8 @@ export interface Agent { currentSpecId: SpecId | null; costAccumulated: number; lastHeartbeat: string | null; + /** Concrete model ID the agent runs on (persisted across restarts). */ + model?: string; } export interface CostEntry { diff --git a/packages/web/src/components/AgentDetailPanel.tsx b/packages/web/src/components/AgentDetailPanel.tsx index e050469f..f3f4ff9f 100644 --- a/packages/web/src/components/AgentDetailPanel.tsx +++ b/packages/web/src/components/AgentDetailPanel.tsx @@ -128,8 +128,8 @@ export function AgentDetailPanel({ if (agentOutputData?.lines?.length) setHistoricalOutput(agentOutputData.lines.join('\n')); }, [agentOutputData]); - // Model dropdown state - const [availableModels, setAvailableModels] = useState([]); + // Model dropdown state (grouped by runtime so provider routing is preserved) + const [modelGroups, setModelGroups] = useState>([]); const [modelLoading, setModelLoading] = useState(false); const config = STATUS_CONFIG[agent.status] ?? { color: 'var(--color-text-tertiary)', label: agent.status }; @@ -145,16 +145,18 @@ export function AgentDetailPanel({ ); useEffect(() => { if (!modelsData) return; - const models: string[] = []; + const groups: Array<{ runtime: string; models: string[] }> = []; for (const runtime of Object.keys(modelsData as Record)) { const runtimeModels = (modelsData as Record)[runtime]; if (Array.isArray(runtimeModels)) { + const models: string[] = []; for (const m of runtimeModels) { if (m.modelId && !models.includes(m.modelId)) models.push(m.modelId); } + if (models.length) groups.push({ runtime, models }); } } - setAvailableModels(models); + setModelGroups(groups); }, [modelsData]); // Auto-scroll chat @@ -218,10 +220,10 @@ export function AgentDetailPanel({ setSending(false); }; - const handleModelChange = async (model: string) => { + const handleModelChange = async (model: string, runtime?: string) => { setModelLoading(true); try { - await api.setAgentModel(projectName, agent.id, model); + await api.setAgentModel(projectName, agent.id, model, runtime); globalMutate((key: unknown) => Array.isArray(key) && key[0] === 'agents'); } catch (err) { console.error('Failed to set model:', err); @@ -440,19 +442,39 @@ export function AgentDetailPanel({
Model - + {(() => { + const sep = '\u001F'; + const agentRuntime = agent.runtimeName ?? ''; + const knownCombo = modelGroups.some(g => + g.models.includes(agent.model ?? '') && (!agentRuntime || g.runtime === agentRuntime)); + const currentValue = agent.model + ? `${knownCombo ? agentRuntime : ''}${sep}${agent.model}` + : ''; + return ( + + ); + })()}
{currentTask && ( diff --git a/packages/web/src/lib/api.ts b/packages/web/src/lib/api.ts index 3e17bc48..e3a61b56 100644 --- a/packages/web/src/lib/api.ts +++ b/packages/web/src/lib/api.ts @@ -104,8 +104,8 @@ export const api = { get<{ agentId: string; lines: string[]; totalLines: number }>(projectPath(project, `/agents/${encodeURIComponent(agentId)}/output?tail=${tail ?? 100}`)), sendAgentMessage: (project: string, agentId: string, message: string, urgent?: boolean) => post<{ ok: boolean }>(projectPath(project, `/agents/${encodeURIComponent(agentId)}/${urgent ? 'interrupt' : 'send'}`), { message }), - setAgentModel: (project: string, agentId: string, model: string) => - put<{ success: boolean }>(projectPath(project, `/agents/${encodeURIComponent(agentId)}/model`), { model }), + setAgentModel: (project: string, agentId: string, model: string, runtime?: string) => + put<{ success: boolean }>(projectPath(project, `/agents/${encodeURIComponent(agentId)}/model`), { model, ...(runtime ? { runtime } : {}) }), getAvailableModels: (project: string) => get>(projectPath(project, '/models/available')), testRuntime: (project: string, runtimeId: string) => post<{ success: boolean; installed: boolean; version?: string; message: string }>(projectPath(project, `/runtimes/${runtimeId}/test`), {}), hibernateAgent: (project: string, agentId: string) => diff --git a/packages/web/src/pages/Agents.tsx b/packages/web/src/pages/Agents.tsx index 93684fed..48083c48 100644 --- a/packages/web/src/pages/Agents.tsx +++ b/packages/web/src/pages/Agents.tsx @@ -105,9 +105,9 @@ function AgentModelDropdown({ agent, projectName, onChanged }: { agent: Agent; p return result; }, [modelsData, agent.runtimeName, agent.runtime]); - const selectModel = async (model: string) => { + const selectModel = async (model: string, runtime: string) => { setLoading(true); - try { await api.setAgentModel(projectName, agent.id, model); onChanged(); } catch (err) { console.error(err); } + try { await api.setAgentModel(projectName, agent.id, model, runtime); onChanged(); } catch (err) { console.error(err); } setLoading(false); setOpen(false); }; @@ -130,7 +130,7 @@ function AgentModelDropdown({ agent, projectName, onChanged }: { agent: Agent; p
{g.runtime}
{g.models.map(m => (