From 8fa7eebd235355522c8104166b4f1f959b4e2f10 Mon Sep 17 00:00:00 2001 From: Cristina Poncela Cubeiro <140309543+cristinaponcela@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:49:28 +0200 Subject: [PATCH] fix: persist default to scoped if non-empty --- .../coding-agent/src/core/agent-session.ts | 17 +++ .../src/modes/interactive/interactive-mode.ts | 1 + .../coding-agent/test/model-resolver.test.ts | 101 +++++++++++++++++- 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 60e278d34..ea69b84ef 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -1644,6 +1644,7 @@ export class AgentSession { this.sessionManager.appendModelChange(model.provider, model.id); if (options.persist) { this.settingsManager.setDefaultModelAndProvider(model.provider, model.id); + this._addPersistedDefaultToNonEmptyScope(model); } // Apply thinking level for the new model. @@ -1654,6 +1655,20 @@ export class AgentSession { await this._emitModelSelect(model, previousModel, "set"); } + private _addPersistedDefaultToNonEmptyScope(model: Model): void { + if (this._scopedModels.length === 0) return; + if (this._scopedModels.some((scoped) => modelsAreEqual(scoped.model, model))) return; + + this._scopedModels = [...this._scopedModels, { model }]; + + const enabledModels = this.settingsManager.getEnabledModels(); + if (!enabledModels?.length) return; + + const modelReference = `${model.provider}/${model.id}`; + if (enabledModels.some((pattern) => pattern.toLowerCase() === modelReference.toLowerCase())) return; + this.settingsManager.setEnabledModels([...enabledModels, modelReference]); + } + /** * Cycle to next/previous model. * Uses scoped models (from --models flag) if available, otherwise all available models. @@ -1696,6 +1711,7 @@ export class AgentSession { this.sessionManager.appendModelChange(next.model.provider, next.model.id); if (options.persist) { this.settingsManager.setDefaultModelAndProvider(next.model.provider, next.model.id); + this._addPersistedDefaultToNonEmptyScope(next.model); } // Apply thinking level for the new model. @@ -1730,6 +1746,7 @@ export class AgentSession { this.sessionManager.appendModelChange(nextModel.provider, nextModel.id); if (options.persist) { this.settingsManager.setDefaultModelAndProvider(nextModel.provider, nextModel.id); + this._addPersistedDefaultToNonEmptyScope(nextModel); } // Apply thinking level for the new model. diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index acf96ae2f..e9d888a6c 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -4960,6 +4960,7 @@ export class InteractiveMode { const selectModel = async (model: Model, persist: boolean) => { try { await this.session.setModel(model, { persist }); + this.updateAvailableProviderCount(); this.footer.invalidate(); this.updateEditorBorderColor(); done(); diff --git a/packages/coding-agent/test/model-resolver.test.ts b/packages/coding-agent/test/model-resolver.test.ts index 6488b7ae8..8f37b7054 100644 --- a/packages/coding-agent/test/model-resolver.test.ts +++ b/packages/coding-agent/test/model-resolver.test.ts @@ -1,6 +1,13 @@ +import { mkdirSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { Agent } from "@earendil-works/pi-agent-core"; import type { Model } from "@earendil-works/pi-ai"; +import { getModel, streamSimple } from "@earendil-works/pi-ai/compat"; import { getBuiltinModels, getBuiltinProviders } from "@earendil-works/pi-ai/providers/all"; -import { describe, expect, test, vi } from "vitest"; +import { afterEach, describe, expect, test, vi } from "vitest"; +import { AgentSession } from "../src/core/agent-session.ts"; +import { AuthStorage } from "../src/core/auth-storage.ts"; import { defaultModelPerProvider, findInitialModel, @@ -9,6 +16,10 @@ import { resolveModelScope, resolveModelScopeWithDiagnostics, } from "../src/core/model-resolver.ts"; +import { SessionManager } from "../src/core/session-manager.ts"; +import { SettingsManager } from "../src/core/settings-manager.ts"; +import { createModelRegistry, getModelRuntime } from "./model-runtime-test-utils.ts"; +import { createTestResourceLoader } from "./utilities.ts"; // Mock models for testing const mockModels: Model<"anthropic-messages">[] = [ @@ -814,4 +825,92 @@ describe("default model selection", () => { expect(result.model?.provider).toBe("spark-two"); expect(result.model?.id).toBe("deepseek-v4-flash"); }); + + describe("persisted default model scoping", () => { + const tempDirs: string[] = []; + const sonnet = getModel("anthropic", "claude-sonnet-4-5")!; + const opus = getModel("anthropic", "claude-opus-4-8")!; + + afterEach(() => { + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } + }); + + async function createSession(options: { scoped: boolean; persistedScope?: string[] }) { + const tempDir = join(tmpdir(), `pi-default-scope-${Date.now()}-${Math.random().toString(36).slice(2)}`); + mkdirSync(tempDir, { recursive: true }); + tempDirs.push(tempDir); + + const settingsManager = SettingsManager.create(tempDir, tempDir); + if (options.persistedScope) { + settingsManager.setEnabledModels(options.persistedScope); + } + + const authStorage = AuthStorage.inMemory({ anthropic: { type: "api_key", key: "test-key" } }); + const modelRuntime = getModelRuntime(await createModelRegistry(authStorage, join(tempDir, "models.json"))); + const agent = new Agent({ + initialState: { + model: sonnet, + systemPrompt: "test", + tools: [], + }, + streamFn: streamSimple, + }); + const session = new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(tempDir), + settingsManager, + cwd: tempDir, + modelRuntime, + resourceLoader: createTestResourceLoader(), + scopedModels: options.scoped ? [{ model: sonnet }] : [], + }); + + return { session, settingsManager }; + } + + test("adds a persisted default to an existing scoped model list", async () => { + const { session, settingsManager } = await createSession({ + scoped: true, + persistedScope: [`${sonnet.provider}/${sonnet.id}`], + }); + + await session.setModel(opus, { persist: true }); + + expect(settingsManager.getDefaultProvider()).toBe(opus.provider); + expect(settingsManager.getDefaultModel()).toBe(opus.id); + expect(session.scopedModels.map((scoped) => `${scoped.model.provider}/${scoped.model.id}`)).toEqual([ + `${sonnet.provider}/${sonnet.id}`, + `${opus.provider}/${opus.id}`, + ]); + expect(settingsManager.getEnabledModels()).toEqual([ + `${sonnet.provider}/${sonnet.id}`, + `${opus.provider}/${opus.id}`, + ]); + }); + + test("does not create a scoped model list when all models are available", async () => { + const { session, settingsManager } = await createSession({ scoped: false }); + + await session.setModel(opus, { persist: true }); + + expect(session.scopedModels).toEqual([]); + expect(settingsManager.getEnabledModels()).toBeUndefined(); + }); + + test("keeps session-only model changes out of scope", async () => { + const { session, settingsManager } = await createSession({ + scoped: true, + persistedScope: [`${sonnet.provider}/${sonnet.id}`], + }); + + await session.setModel(opus, { persist: false }); + + expect(session.scopedModels.map((scoped) => `${scoped.model.provider}/${scoped.model.id}`)).toEqual([ + `${sonnet.provider}/${sonnet.id}`, + ]); + expect(settingsManager.getEnabledModels()).toEqual([`${sonnet.provider}/${sonnet.id}`]); + }); + }); });