From 20aded30f0e94cbefe9dacef44735b46f78d818a Mon Sep 17 00:00:00 2001 From: Adrian Komorek Date: Fri, 8 May 2026 13:25:27 +0200 Subject: [PATCH 1/2] fix(smithy): spawn honors workspace defaultModels at session start MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The session manager's resolveExecutablePath had a 3-tier fallback (agent metadata → workspace defaults → provider built-in) but model resolution skipped the middle tier. Agents with no per-agent model override fell straight to the SDK default (sonnet for Claude Code) regardless of the workspace's "default model" setting in the UI. Concretely: every time the operator restarted sf serve smithy, the director's claude session reverted to sonnet despite the UI showing opus. They had to /model opus manually each time. Three pieces: 1. Extend ServerAgentDefaults with defaultModels and defaultProvider. Persisted server-side via the existing /api/settings/agent-defaults endpoint. Validated on read and write. Backwards compatible with workspaces that have agent-defaults set via just defaultExecutablePaths. 2. New resolveModel(providerName, agentModel?) helper on SessionManagerImpl, mirroring resolveExecutablePath. Used in both startSession and resumeSession so fresh starts and reconnects honor the same precedence: 1. options.model (call-site override) 2. agent metadata model 3. workspace defaults defaultModels[providerName] 4. undefined → provider/SDK built-in default 3. Migrate useAgentDefaultsSettings in smithy-web from localStorage-only to server-backed (mirrors useExecutablePathSettings). The UI now round-trips defaultProvider + defaultModels through the daemon, so operator preference reaches the spawn path. Tests cover all four resolution tiers, per-provider keying (codex default doesn't shadow claude-code agents), the no-settingsService fallback, and persistence through getAgentDefaults/setAgentDefaults. --- .../spawn-honors-workspace-default-model.md | 16 ++ apps/smithy-web/src/api/hooks/useSettings.ts | 109 ++++++++++- .../src/runtime/session-manager.bun.test.ts | 171 ++++++++++++++++++ .../smithy/src/runtime/session-manager.ts | 51 +++++- packages/smithy/src/server/routes/settings.ts | 50 +++++ .../src/services/settings-service.bun.test.ts | 94 ++++++++++ .../smithy/src/services/settings-service.ts | 52 ++++++ 7 files changed, 529 insertions(+), 14 deletions(-) create mode 100644 .changeset/spawn-honors-workspace-default-model.md diff --git a/.changeset/spawn-honors-workspace-default-model.md b/.changeset/spawn-honors-workspace-default-model.md new file mode 100644 index 00000000..6d4ee1ad --- /dev/null +++ b/.changeset/spawn-honors-workspace-default-model.md @@ -0,0 +1,16 @@ +--- +"@stoneforge/smithy": minor +"@stoneforge/smithy-web": minor +--- + +fix(smithy): spawn-time model resolution honors workspace `defaultModels` setting + +The session manager's `resolveExecutablePath` had a 3-tier priority chain (agent metadata → workspace defaults → provider built-in default) but model resolution skipped the middle tier — it only read `agent.metadata.model`, never the workspace-level `defaultModels[providerName]` setting. Operators who set "Default model = Opus" in the smithy-web settings UI would see that reflected in the UI but the daemon would still spawn Claude with the SDK's built-in default (sonnet), forcing them to `/model opus` manually after every restart. + +Two pieces: + +1. **Server-backed `defaultModels` and `defaultProvider`.** `ServerAgentDefaults` (and the `/api/settings/agent-defaults` route) now persist these alongside the existing `defaultExecutablePaths`. `useAgentDefaultsSettings` in smithy-web is migrated from localStorage-only to server-backed, mirroring the existing `useExecutablePathSettings` pattern. + +2. **`resolveModel` helper in session-manager.** New private method analogous to `resolveExecutablePath`: agent metadata wins, falls back to `defaults.defaultModels[providerName]`, finally to `undefined` (provider/SDK default). Used in both `startSession` and `resumeSession` so fresh starts and reconnects honor the same precedence. + +Tests cover all four resolution tiers (agent override, workspace default, no override → undefined, options.model wins), per-provider keying, and the no-settingsService fallback. diff --git a/apps/smithy-web/src/api/hooks/useSettings.ts b/apps/smithy-web/src/api/hooks/useSettings.ts index 22f58c7b..f214c4a9 100644 --- a/apps/smithy-web/src/api/hooks/useSettings.ts +++ b/apps/smithy-web/src/api/hooks/useSettings.ts @@ -110,6 +110,8 @@ const WORKSPACE_KEY = 'settings.workspace'; const STEWARD_SCHEDULES_KEY = 'settings.stewardSchedules'; const AGENT_DEFAULTS_KEY = 'settings.agentDefaults'; +const API_BASE = '/api'; + // ============================================================================ // Helper Functions // ============================================================================ @@ -280,37 +282,125 @@ export function useStewardScheduleSettings() { } /** - * Hook for managing agent default settings (provider & model) + * Hook for managing agent default settings (provider & model). + * + * Server-backed via `/api/settings/agent-defaults`: the daemon reads these + * values at spawn time when an agent has no per-agent override, so they MUST + * round-trip to the server. Local state is a debounced cache to keep the UI + * responsive while the persist call completes. + * + * Falls back to localStorage on initial render so the UI doesn't flash empty + * while the fetch is in flight; the localStorage entry is treated as a hint + * only and is overwritten by the server's authoritative response. */ export function useAgentDefaultsSettings() { const [settings, setSettingsState] = useState(() => loadFromStorage(AGENT_DEFAULTS_KEY, DEFAULT_AGENT_DEFAULTS_SETTINGS) ); + const [isLoading, setIsLoading] = useState(true); + const [error, setError] = useState(null); + const debounceTimerRef = useRef | null>(null); + // Fetch authoritative values from server on mount + useEffect(() => { + let cancelled = false; + async function fetchDefaults() { + try { + const res = await fetch(`${API_BASE}/settings/agent-defaults`, { + headers: { 'Content-Type': 'application/json' }, + }); + if (!res.ok) throw new Error(`HTTP ${res.status}`); + const data = await res.json() as { + defaultProvider?: AgentProvider; + defaultModels?: Record; + }; + if (cancelled) return; + setSettingsState({ + defaultProvider: data.defaultProvider ?? DEFAULT_AGENT_DEFAULTS_SETTINGS.defaultProvider, + defaultModels: data.defaultModels ?? {}, + }); + setIsLoading(false); + } catch (err) { + if (!cancelled) { + setError(String(err)); + setIsLoading(false); + } + } + } + fetchDefaults(); + return () => { cancelled = true; }; + }, []); + + // Mirror local state to localStorage as a hint cache useEffect(() => { saveToStorage(AGENT_DEFAULTS_KEY, settings); }, [settings]); - const setSettings = useCallback((updates: Partial) => { - setSettingsState((prev) => ({ ...prev, ...updates })); + // Persist to server (PUT). Merges with the existing server config so we + // don't clobber `defaultExecutablePaths` or `fallbackChain`. + const persistToServer = useCallback(async (next: AgentDefaultsSettings) => { + try { + setError(null); + // Read current server state to preserve unrelated fields (paths, chain) + const cur = await fetch(`${API_BASE}/settings/agent-defaults`, { + headers: { 'Content-Type': 'application/json' }, + }); + const curBody = cur.ok ? await cur.json() : {}; + const res = await fetch(`${API_BASE}/settings/agent-defaults`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + defaultExecutablePaths: curBody.defaultExecutablePaths ?? {}, + fallbackChain: curBody.fallbackChain, + defaultProvider: next.defaultProvider, + defaultModels: next.defaultModels, + }), + }); + if (!res.ok) { + const errData = await res.json().catch(() => ({ error: { message: 'Unknown error' } })); + throw new Error(errData.error?.message || `HTTP ${res.status}`); + } + } catch (err) { + setError(String(err)); + } }, []); + const debouncedPersist = useCallback((next: AgentDefaultsSettings) => { + if (debounceTimerRef.current) clearTimeout(debounceTimerRef.current); + debounceTimerRef.current = setTimeout(() => persistToServer(next), 400); + }, [persistToServer]); + + const setSettings = useCallback((updates: Partial) => { + setSettingsState((prev) => { + const next = { ...prev, ...updates }; + debouncedPersist(next); + return next; + }); + }, [debouncedPersist]); + const setDefaultModel = useCallback((provider: string, model: string) => { - setSettingsState((prev) => ({ - ...prev, - defaultModels: { ...prev.defaultModels, [provider]: model }, - })); - }, []); + setSettingsState((prev) => { + const next = { + ...prev, + defaultModels: { ...prev.defaultModels, [provider]: model }, + }; + debouncedPersist(next); + return next; + }); + }, [debouncedPersist]); const resetToDefaults = useCallback(() => { setSettingsState(DEFAULT_AGENT_DEFAULTS_SETTINGS); - }, []); + debouncedPersist(DEFAULT_AGENT_DEFAULTS_SETTINGS); + }, [debouncedPersist]); return { settings, setSettings, setDefaultModel, resetToDefaults, + isLoading, + error, }; } @@ -318,7 +408,6 @@ export function useAgentDefaultsSettings() { // Server-backed Executable Path Settings // ============================================================================ -const API_BASE = '/api'; /** * Response shape from GET /api/settings/agent-defaults diff --git a/packages/smithy/src/runtime/session-manager.bun.test.ts b/packages/smithy/src/runtime/session-manager.bun.test.ts index 1210504d..ff3819f0 100644 --- a/packages/smithy/src/runtime/session-manager.bun.test.ts +++ b/packages/smithy/src/runtime/session-manager.bun.test.ts @@ -1433,3 +1433,174 @@ describe('SessionManager executable path tracking', () => { expect(spawner._lastSpawnOptions?.claudePath).toBeUndefined(); }); }); + +// ============================================================================ +// Model resolution: agent metadata → workspace defaults → undefined +// ============================================================================ + +describe('SessionManager model resolution', () => { + /** + * Minimal SettingsService mock returning a configurable agentDefaults blob. + * Only `getAgentDefaults` is exercised by `resolveModel`; other methods can + * stay as no-op stubs. + */ + function createMockSettingsService(defaults: { + defaultExecutablePaths?: Record; + defaultModels?: Record; + fallbackChain?: string[]; + defaultProvider?: string; + }) { + const filled = { + defaultExecutablePaths: defaults.defaultExecutablePaths ?? {}, + ...(defaults.defaultModels ? { defaultModels: defaults.defaultModels } : {}), + ...(defaults.fallbackChain ? { fallbackChain: defaults.fallbackChain } : {}), + ...(defaults.defaultProvider ? { defaultProvider: defaults.defaultProvider } : {}), + }; + return { + getAgentDefaults: () => filled, + // The other methods aren't used by resolveModel; unsafe-cast through + // unknown so tests don't have to stub the entire SettingsService surface. + } as unknown as Parameters[3]; + } + + test('agent.model wins over workspace default', async () => { + const agentWithModel: AgentEntity = { + id: testAgentId, + type: ElementType.ENTITY, + name: 'agent-with-model', + entityType: 'agent', + version: 1, + createdAt: createTimestamp(), + updatedAt: createTimestamp(), + createdBy: testCreatorId, + tags: [], + metadata: { + agent: { + agentRole: 'worker' as const, + workerMode: 'ephemeral' as const, + sessionStatus: 'idle', + model: 'claude-sonnet-4-X', + }, + }, + } as unknown as AgentEntity; + + const agents = new Map(); + agents.set(testAgentId, agentWithModel); + + const spawner = createMockSpawnerService(); + const registry = createMockAgentRegistry(agents); + const api = createMockApi(); + const settingsService = createMockSettingsService({ + defaultModels: { 'claude-code': 'claude-opus-4-X' }, + }); + const sessionManager = createSessionManager(spawner, api, registry, settingsService); + + await sessionManager.startSession(testAgentId, {}); + + expect(spawner._lastSpawnOptions?.model).toBe('claude-sonnet-4-X'); + }); + + test('workspace default model is used when agent has no model override', async () => { + const agents = new Map(); + agents.set(testAgentId, createMockAgent(testAgentId, 'worker')); + + const spawner = createMockSpawnerService(); + const registry = createMockAgentRegistry(agents); + const api = createMockApi(); + const settingsService = createMockSettingsService({ + defaultModels: { 'claude-code': 'claude-opus-4-X' }, + }); + const sessionManager = createSessionManager(spawner, api, registry, settingsService); + + await sessionManager.startSession(testAgentId, {}); + + expect(spawner._lastSpawnOptions?.model).toBe('claude-opus-4-X'); + }); + + test('workspace default is keyed by provider — only matching provider entries apply', async () => { + // Agent has no explicit provider, so it defaults to 'claude-code'. + // Workspace has a 'codex' default but no 'claude-code' default — agent + // must resolve to undefined, NOT pick up the codex value. + const agents = new Map(); + agents.set(testAgentId, createMockAgent(testAgentId, 'worker')); + + const spawner = createMockSpawnerService(); + const registry = createMockAgentRegistry(agents); + const api = createMockApi(); + const settingsService = createMockSettingsService({ + defaultModels: { 'codex': 'gpt-5' }, + }); + const sessionManager = createSessionManager(spawner, api, registry, settingsService); + + await sessionManager.startSession(testAgentId, {}); + + expect(spawner._lastSpawnOptions?.model).toBeUndefined(); + }); + + test('falls back to undefined (provider built-in default) when neither agent nor workspace has a model', async () => { + const agents = new Map(); + agents.set(testAgentId, createMockAgent(testAgentId, 'worker')); + + const spawner = createMockSpawnerService(); + const registry = createMockAgentRegistry(agents); + const api = createMockApi(); + const settingsService = createMockSettingsService({}); + const sessionManager = createSessionManager(spawner, api, registry, settingsService); + + await sessionManager.startSession(testAgentId, {}); + + expect(spawner._lastSpawnOptions?.model).toBeUndefined(); + }); + + test('no settingsService still resolves correctly (workspace tier silently skipped)', async () => { + const agents = new Map(); + agents.set(testAgentId, createMockAgent(testAgentId, 'worker')); + + const spawner = createMockSpawnerService(); + const registry = createMockAgentRegistry(agents); + const api = createMockApi(); + // No settingsService passed. + const sessionManager = createSessionManager(spawner, api, registry); + + await sessionManager.startSession(testAgentId, {}); + + expect(spawner._lastSpawnOptions?.model).toBeUndefined(); + }); + + test('options.model override wins over both agent and workspace defaults', async () => { + const agentWithModel: AgentEntity = { + id: testAgentId, + type: ElementType.ENTITY, + name: 'agent-with-model', + entityType: 'agent', + version: 1, + createdAt: createTimestamp(), + updatedAt: createTimestamp(), + createdBy: testCreatorId, + tags: [], + metadata: { + agent: { + agentRole: 'worker' as const, + workerMode: 'ephemeral' as const, + sessionStatus: 'idle', + model: 'claude-sonnet-4-X', + }, + }, + } as unknown as AgentEntity; + + const agents = new Map(); + agents.set(testAgentId, agentWithModel); + + const spawner = createMockSpawnerService(); + const registry = createMockAgentRegistry(agents); + const api = createMockApi(); + const settingsService = createMockSettingsService({ + defaultModels: { 'claude-code': 'claude-opus-4-X' }, + }); + const sessionManager = createSessionManager(spawner, api, registry, settingsService); + + await sessionManager.startSession(testAgentId, { model: 'claude-haiku-X' }); + + expect(spawner._lastSpawnOptions?.model).toBe('claude-haiku-X'); + }); +}); diff --git a/packages/smithy/src/runtime/session-manager.ts b/packages/smithy/src/runtime/session-manager.ts index bc235466..682a996c 100644 --- a/packages/smithy/src/runtime/session-manager.ts +++ b/packages/smithy/src/runtime/session-manager.ts @@ -563,8 +563,12 @@ export class SessionManagerImpl implements SessionManager { const agentExecutablePath = options?.executablePathOverride ?? (meta as { executablePath?: string }).executablePath; const providerOverride = await this.resolveProvider(providerName, agentExecutablePath); - // Resolve model: use options override, or fall back to agent metadata - const modelOverride = options?.model ?? (meta as { model?: string }).model; + // Resolve model: explicit options override wins, then agent metadata, + // then workspace `defaultModels[providerName]`, then provider built-in + // default. resolveModel handles tiers 2 and 3. + const agentModelMeta = (meta as { model?: string }).model; + const modelOverride = options?.model + ?? this.resolveModel(providerName ?? 'claude-code', agentModelMeta); // Build spawn options. // Pass the resolved executable path as claudePath so the spawner can track @@ -693,8 +697,12 @@ export class SessionManagerImpl implements SessionManager { const providerOverride = await this.resolveProvider(providerName, agentExecutablePath); const resolvedExecutablePath = this.resolveExecutablePath(providerName ?? 'claude-code', agentExecutablePath); - // Resolve model from agent metadata (resume doesn't allow model override) - const modelFromMeta = (meta as { model?: string }).model; + // Resolve model: agent metadata, then workspace `defaultModels[providerName]`, + // then provider built-in default. Resume doesn't allow a per-call override + // (the resumed session is locked to whatever the original session used in + // theory, but Claude Code SDK will honor a fresh --model on resume). + const agentModelMeta = (meta as { model?: string }).model; + const modelFromMeta = this.resolveModel(providerName ?? 'claude-code', agentModelMeta); // Build spawn options with resume const spawnOptions: SpawnOptions = { @@ -1313,6 +1321,41 @@ export class SessionManagerImpl implements SessionManager { return undefined; } + /** + * Resolves the model for a session, mirroring `resolveExecutablePath`'s + * 3-tier priority chain: + * 1. Agent-specific model override (from agent metadata) + * 2. Workspace-wide default from settings (`defaultModels[providerName]`) + * 3. undefined (provider/SDK will use its built-in default) + * + * Without tier 2, an agent with no explicit model falls straight to the SDK + * default (e.g. sonnet for Claude Code) regardless of what the workspace's + * agent-defaults UI shows. This mismatched the operator's expectation that + * setting "default model = opus" in the UI would apply to every agent that + * didn't set its own model. + */ + private resolveModel( + providerName: string, + agentModel?: string + ): string | undefined { + // Priority 1: Agent-specific override + if (agentModel) { + return agentModel; + } + + // Priority 2: Workspace-wide default from settings + if (this.settingsService) { + const defaults = this.settingsService.getAgentDefaults(); + const workspaceModel = defaults.defaultModels?.[providerName]; + if (workspaceModel) { + return workspaceModel; + } + } + + // Priority 3: No custom model — provider/SDK uses its built-in default + return undefined; + } + /** * Creates a fresh provider instance with a custom executable path. * Returns undefined if no custom path is needed (use registry singleton instead). diff --git a/packages/smithy/src/server/routes/settings.ts b/packages/smithy/src/server/routes/settings.ts index 380974e0..f5d7b092 100644 --- a/packages/smithy/src/server/routes/settings.ts +++ b/packages/smithy/src/server/routes/settings.ts @@ -77,6 +77,48 @@ export function createSettingsRoutes(services: Services) { } } + // Validate defaultModels if provided — must be an object of strings + if (body.defaultModels !== undefined && typeof body.defaultModels !== 'object') { + return c.json( + { + error: { + code: 'INVALID_INPUT', + message: 'defaultModels must be an object mapping provider names to model identifiers', + }, + }, + 400 + ); + } + if (body.defaultModels) { + for (const [key, value] of Object.entries(body.defaultModels)) { + if (typeof value !== 'string') { + return c.json( + { + error: { + code: 'INVALID_INPUT', + message: `defaultModels.${key} must be a string, got ${typeof value}`, + }, + }, + 400 + ); + } + } + } + + // Validate defaultProvider if provided — must be a non-empty string + if (body.defaultProvider !== undefined + && (typeof body.defaultProvider !== 'string' || body.defaultProvider.length === 0)) { + return c.json( + { + error: { + code: 'INVALID_INPUT', + message: 'defaultProvider must be a non-empty string', + }, + }, + 400 + ); + } + // Validate fallbackChain if provided — must be an array if (body.fallbackChain !== undefined && !Array.isArray(body.fallbackChain)) { return c.json( @@ -98,6 +140,14 @@ export function createSettingsRoutes(services: Services) { defaults.fallbackChain = body.fallbackChain; } + if (body.defaultModels !== undefined) { + defaults.defaultModels = body.defaultModels; + } + + if (body.defaultProvider !== undefined) { + defaults.defaultProvider = body.defaultProvider; + } + const updated = settingsService.setAgentDefaults(defaults); return c.json(updated); diff --git a/packages/smithy/src/services/settings-service.bun.test.ts b/packages/smithy/src/services/settings-service.bun.test.ts index d299552c..5460c0c7 100644 --- a/packages/smithy/src/services/settings-service.bun.test.ts +++ b/packages/smithy/src/services/settings-service.bun.test.ts @@ -144,6 +144,65 @@ describe('SettingsService', () => { const result = service.getAgentDefaults(); expect(result.fallbackChain).toBeUndefined(); }); + + test('returns stored defaultModels', () => { + service.setSetting(SETTING_KEYS.AGENT_DEFAULTS, { + defaultExecutablePaths: {}, + defaultModels: { + 'claude-code': 'claude-opus-4-X', + 'codex': 'gpt-5', + }, + }); + const result = service.getAgentDefaults(); + expect(result.defaultModels).toEqual({ + 'claude-code': 'claude-opus-4-X', + 'codex': 'gpt-5', + }); + }); + + test('returns stored defaultProvider', () => { + service.setSetting(SETTING_KEYS.AGENT_DEFAULTS, { + defaultExecutablePaths: {}, + defaultProvider: 'opencode', + }); + const result = service.getAgentDefaults(); + expect(result.defaultProvider).toBe('opencode'); + }); + + test('omits defaultModels when missing or empty', () => { + service.setSetting(SETTING_KEYS.AGENT_DEFAULTS, { + defaultExecutablePaths: {}, + defaultModels: {}, + }); + const result = service.getAgentDefaults(); + expect(result.defaultModels).toBeUndefined(); + }); + + test('filters non-string entries from defaultModels on read', () => { + service.setSetting(SETTING_KEYS.AGENT_DEFAULTS, { + defaultExecutablePaths: {}, + defaultModels: { + 'claude-code': 'claude-opus-4-X', + 'invalid': 42, + 'empty': '', + 'codex': 'gpt-5', + }, + }); + const result = service.getAgentDefaults(); + expect(result.defaultModels).toEqual({ + 'claude-code': 'claude-opus-4-X', + 'codex': 'gpt-5', + }); + }); + + test('ignores non-string defaultProvider on read', () => { + service.setSetting(SETTING_KEYS.AGENT_DEFAULTS, { + defaultExecutablePaths: {}, + defaultProvider: 42, + }); + const result = service.getAgentDefaults(); + expect(result.defaultProvider).toBeUndefined(); + }); }); describe('setAgentDefaults', () => { @@ -234,6 +293,41 @@ describe('SettingsService', () => { }); expect(result.fallbackChain).toEqual([]); }); + + test('persists defaultModels and defaultProvider', () => { + const input: ServerAgentDefaults = { + defaultExecutablePaths: {}, + defaultModels: { 'claude-code': 'claude-opus-4-X' }, + defaultProvider: 'claude-code', + }; + const result = service.setAgentDefaults(input); + expect(result.defaultModels).toEqual({ 'claude-code': 'claude-opus-4-X' }); + expect(result.defaultProvider).toBe('claude-code'); + // Round-trip: read it back + const retrieved = service.getAgentDefaults(); + expect(retrieved.defaultModels).toEqual({ 'claude-code': 'claude-opus-4-X' }); + expect(retrieved.defaultProvider).toBe('claude-code'); + }); + + test('filters non-string defaultModels values on write', () => { + const input = { + defaultExecutablePaths: {}, + defaultModels: { + 'claude-code': 'claude-opus-4-X', + 'invalid': 42 as unknown as string, + }, + } as unknown as ServerAgentDefaults; + const result = service.setAgentDefaults(input); + expect(result.defaultModels).toEqual({ 'claude-code': 'claude-opus-4-X' }); + }); + + test('omits defaultProvider when empty string', () => { + const result = service.setAgentDefaults({ + defaultExecutablePaths: {}, + defaultProvider: '', + }); + expect(result.defaultProvider).toBeUndefined(); + }); }); describe('persistence', () => { diff --git a/packages/smithy/src/services/settings-service.ts b/packages/smithy/src/services/settings-service.ts index d77aa9f0..1786b682 100644 --- a/packages/smithy/src/services/settings-service.ts +++ b/packages/smithy/src/services/settings-service.ts @@ -34,6 +34,21 @@ export interface ServerAgentDefaults { defaultExecutablePaths: Record; /** Ordered list of executable names/paths for rate limit fallback. When one hits its limit, the next available is used. */ fallbackChain?: string[]; + /** + * Provider name → default model identifier (e.g. { 'claude-code': 'claude-opus-4-X' }). + * Applied at spawn time when an agent has no per-agent `model` override in its + * metadata. Read by session-manager's `resolveModel`. Same precedence model as + * `defaultExecutablePaths`: agent metadata wins, this fills the gap, provider + * built-in default is last resort. + */ + defaultModels?: Record; + /** + * Default provider name when an agent has no `provider` in its metadata. + * Currently advisory — `resolveProvider` already falls back to 'claude-code' + * when unset, so persisting this aligns the UI's view with the server's + * authoritative default. + */ + defaultProvider?: string; } /** @@ -229,6 +244,25 @@ export function createSettingsService(storage: StorageBackend): SettingsService ); } + // Include defaultModels if it's a valid object of strings + const models = value?.defaultModels; + if (models && typeof models === 'object' && !Array.isArray(models)) { + const sanitized: Record = {}; + for (const [provider, model] of Object.entries(models as Record)) { + if (typeof model === 'string' && model.length > 0) { + sanitized[provider] = model; + } + } + if (Object.keys(sanitized).length > 0) { + result.defaultModels = sanitized; + } + } + + // Include defaultProvider if it's a non-empty string + if (typeof value?.defaultProvider === 'string' && value.defaultProvider.length > 0) { + result.defaultProvider = value.defaultProvider; + } + return result; }, @@ -253,6 +287,24 @@ export function createSettingsService(storage: StorageBackend): SettingsService ); } + // Validate defaultModels — same pattern as defaultExecutablePaths. + if (defaults.defaultModels && typeof defaults.defaultModels === 'object') { + const sanitizedModels: Record = {}; + for (const [provider, model] of Object.entries(defaults.defaultModels)) { + if (typeof model === 'string' && model.length > 0) { + sanitizedModels[provider] = model; + } + } + if (Object.keys(sanitizedModels).length > 0) { + validated.defaultModels = sanitizedModels; + } + } + + // Validate defaultProvider — must be a non-empty string when present. + if (typeof defaults.defaultProvider === 'string' && defaults.defaultProvider.length > 0) { + validated.defaultProvider = defaults.defaultProvider; + } + this.setSetting(SETTING_KEYS.AGENT_DEFAULTS, validated); return validated; }, From f10cb5e218086cf504d9fa254531bf6093e670e5 Mon Sep 17 00:00:00 2001 From: Adrian Komorek Date: Fri, 8 May 2026 14:11:31 +0200 Subject: [PATCH 2/2] fix(smithy): sf agent start CLI also resolves workspace defaultModels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CLI handler for `sf agent start` calls spawner.spawn() directly, bypassing session-manager.startSession where the resolveModel helper lives. So manual `sf agent start ` invocations didn't pick up the workspace defaultModels fallback even after the rest of this PR. Daemon-driven dispatches went through session-manager and worked. Mirror the same precedence chain in the CLI: 1. --model flag 2. agent metadata.model 3. workspace defaults defaultModels[providerName] 4. undefined → provider/SDK built-in default Settings are read directly from the workspace's .stoneforge/stoneforge.db via createSettingsService — no daemon round-trip needed since the CLI runs in the workspace cwd anyway. Verified live: ran `sf agent start --mode interactive` against a workspace with defaultModels.claude-code='claude-opus-4-5-20251101'; ps showed the spawned claude process invoked with --model claude-opus-4-5-20251101. The director's auto-resume after daemon restart was also captured with the same --model flag, confirming both paths now honor the workspace default. --- packages/smithy/src/cli/commands/agent.ts | 26 +++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/packages/smithy/src/cli/commands/agent.ts b/packages/smithy/src/cli/commands/agent.ts index 6d69e943..f2ba6c41 100644 --- a/packages/smithy/src/cli/commands/agent.ts +++ b/packages/smithy/src/cli/commands/agent.ts @@ -950,6 +950,31 @@ async function agentStartHandler( spawnMode = options.mode as 'headless' | 'interactive'; } + // Resolve model with the same precedence as session-manager.resolveModel: + // 1. explicit --model flag + // 2. agent metadata.model (per-agent override) + // 3. workspace defaultModels[providerName] (server-side settings) + // 4. undefined → provider/SDK built-in default + // Without this, manual `sf agent start` calls bypass workspace defaults + // even though daemon-driven dispatches honor them. + const providerName = (meta as { provider?: string }).provider ?? 'claude-code'; + const agentModelMeta = (meta as { model?: string }).model; + let resolvedModel: string | undefined = options.model ?? agentModelMeta; + if (!resolvedModel && stoneforgeDir) { + try { + const { createStorage, initializeSchema } = await import('@stoneforge/quarry'); + const { createSettingsService } = await import('../../services/settings-service.js'); + const dbPath = options.db ?? `${stoneforgeDir}/stoneforge.db`; + const backend = createStorage({ path: dbPath, create: false }); + initializeSchema(backend); + const settingsService = createSettingsService(backend); + const defaults = settingsService.getAgentDefaults(); + resolvedModel = defaults.defaultModels?.[providerName]; + } catch { + // Settings unavailable — fall through to provider built-in default + } + } + // Spawn the agent const result = await spawner.spawn(id as EntityId, agentRole, { initialPrompt: options.prompt, @@ -958,6 +983,7 @@ async function agentStartHandler( workingDirectory: options.workdir, cols: options.cols ? parseInt(options.cols, 10) : undefined, rows: options.rows ? parseInt(options.rows, 10) : undefined, + model: resolvedModel, }); // If task ID is provided, assign the task to this agent