fix(smithy): spawn honors workspace defaultModels at session start

.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.

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,45 +282,132 @@ 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<AgentDefaultsSettings>(() =>
     loadFromStorage(AGENT_DEFAULTS_KEY, DEFAULT_AGENT_DEFAULTS_SETTINGS)
   );
+  const [isLoading, setIsLoading] = useState(true);
+  const [error, setError] = useState<string | null>(null);
+  const debounceTimerRef = useRef<ReturnType<typeof setTimeout> | 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<string, string>;
+        };
+        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<AgentDefaultsSettings>) => {
-    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<AgentDefaultsSettings>) => {
+    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,
   };
 }
 
 // ============================================================================
 // Server-backed Executable Path Settings
 // ============================================================================
 
-const API_BASE = '/api';
 
 /**
  * Response shape from GET /api/settings/agent-defaults

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

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<string, string>;
+    defaultModels?: Record<string, string>;
+    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<typeof createSessionManager>[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<EntityId, AgentEntity>();
+    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<EntityId, AgentEntity>();
+    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<EntityId, AgentEntity>();
+    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<EntityId, AgentEntity>();
+    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<EntityId, AgentEntity>();
+    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<EntityId, AgentEntity>();
+    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');
+  });
+});