diff --git a/packages/pi-coding-agent/src/core/model-resolver-initial-model-auth.test.ts b/packages/pi-coding-agent/src/core/model-resolver-initial-model-auth.test.ts new file mode 100644 index 000000000..e7d5fb46a --- /dev/null +++ b/packages/pi-coding-agent/src/core/model-resolver-initial-model-auth.test.ts @@ -0,0 +1,78 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import type { Api, Model } from "@gsd/pi-ai"; +import type { ModelRegistry } from "./model-registry.js"; +import { findInitialModel } from "./model-resolver.js"; + +function makeModel(provider: string, id: string): Model { + return { + id, + name: id, + provider, + api: "openai-responses", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 128000, + maxTokens: 8192, + } as Model; +} + +function makeRegistry(opts: { + readyProviders?: Set; + byProviderAndId?: Map>; + available?: Model[]; +}): ModelRegistry { + const readyProviders = opts.readyProviders ?? new Set(); + const byProviderAndId = opts.byProviderAndId ?? new Map>(); + const available = opts.available ?? []; + + return { + find: (provider: string, modelId: string) => byProviderAndId.get(`${provider}/${modelId}`), + getAvailable: async () => available, + isProviderRequestReady: (provider: string) => readyProviders.has(provider), + } as unknown as ModelRegistry; +} + +describe("findInitialModel auth gating for saved defaults", () => { + it("uses saved default when provider is request-ready", async () => { + const saved = makeModel("anthropic", "claude-opus-4-6"); + const registry = makeRegistry({ + readyProviders: new Set(["anthropic"]), + byProviderAndId: new Map([[`anthropic/claude-opus-4-6`, saved]]), + available: [saved], + }); + + const result = await findInitialModel({ + scopedModels: [], + isContinuing: false, + defaultProvider: "anthropic", + defaultModelId: "claude-opus-4-6", + modelRegistry: registry, + }); + + assert.equal(result.model?.provider, "anthropic"); + assert.equal(result.model?.id, "claude-opus-4-6"); + }); + + it("skips saved default when provider is not request-ready and falls back to available", async () => { + const staleDefault = makeModel("anthropic", "claude-opus-4-6"); + const fallback = makeModel("openai", "gpt-5.4"); + const registry = makeRegistry({ + readyProviders: new Set(["openai"]), + byProviderAndId: new Map([[`anthropic/claude-opus-4-6`, staleDefault]]), + available: [fallback], + }); + + const result = await findInitialModel({ + scopedModels: [], + isContinuing: false, + defaultProvider: "anthropic", + defaultModelId: "claude-opus-4-6", + modelRegistry: registry, + }); + + assert.equal(result.model?.provider, "openai"); + assert.equal(result.model?.id, "gpt-5.4"); + }); +}); diff --git a/packages/pi-coding-agent/src/core/model-resolver.ts b/packages/pi-coding-agent/src/core/model-resolver.ts index 3e3b266f7..1a85c2bfa 100644 --- a/packages/pi-coding-agent/src/core/model-resolver.ts +++ b/packages/pi-coding-agent/src/core/model-resolver.ts @@ -504,27 +504,31 @@ export async function findInitialModel(options: { // 3. Try saved default from settings if (defaultProvider && defaultModelId) { - const found = modelRegistry.find(defaultProvider, defaultModelId); - if (found) { - // Check if the provider's recommended default is a higher-capability variant - // of the saved model (e.g. saved "claude-opus-4-6" vs recommended "claude-opus-4-6-extended"). - // If so, prefer the recommended variant to avoid using a smaller context window (#1125). - const recommendedId = defaultModelPerProvider[defaultProvider as KnownProvider]; - if (recommendedId && recommendedId !== defaultModelId && recommendedId.startsWith(defaultModelId)) { - const recommended = modelRegistry.find(defaultProvider, recommendedId); - if (recommended) { - model = recommended; - if (defaultThinkingLevel) { - thinkingLevel = defaultThinkingLevel; + // Guard against stale settings defaults: only use the saved provider/model + // if the provider is actually request-ready (auth/OAuth/CLI ready). + if (modelRegistry.isProviderRequestReady(defaultProvider)) { + const found = modelRegistry.find(defaultProvider, defaultModelId); + if (found) { + // Check if the provider's recommended default is a higher-capability variant + // of the saved model (e.g. saved "claude-opus-4-6" vs recommended "claude-opus-4-6-extended"). + // If so, prefer the recommended variant to avoid using a smaller context window (#1125). + const recommendedId = defaultModelPerProvider[defaultProvider as KnownProvider]; + if (recommendedId && recommendedId !== defaultModelId && recommendedId.startsWith(defaultModelId)) { + const recommended = modelRegistry.find(defaultProvider, recommendedId); + if (recommended) { + model = recommended; + if (defaultThinkingLevel) { + thinkingLevel = defaultThinkingLevel; + } + return { model, thinkingLevel, fallbackMessage: undefined }; } - return { model, thinkingLevel, fallbackMessage: undefined }; } + model = found; + if (defaultThinkingLevel) { + thinkingLevel = defaultThinkingLevel; + } + return { model, thinkingLevel, fallbackMessage: undefined }; } - model = found; - if (defaultThinkingLevel) { - thinkingLevel = defaultThinkingLevel; - } - return { model, thinkingLevel, fallbackMessage: undefined }; } } diff --git a/src/resources/extensions/gsd/auto-model-selection.ts b/src/resources/extensions/gsd/auto-model-selection.ts index ce33bda61..c369d23a7 100644 --- a/src/resources/extensions/gsd/auto-model-selection.ts +++ b/src/resources/extensions/gsd/auto-model-selection.ts @@ -14,6 +14,7 @@ import { classifyUnitComplexity, tierLabel } from "./complexity-classifier.js"; import { resolveModelForComplexity, escalateTier, getEligibleModels, loadCapabilityOverrides, adjustToolSet, filterToolsForProvider } from "./model-router.js"; import { getLedger, getProjectTotals } from "./metrics.js"; import { unitPhaseLabel } from "./auto-dashboard.js"; +import { getSessionModelOverride } from "./session-model-override.js"; export interface ModelSelectionResult { /** Routing metadata for metrics recording */ @@ -72,8 +73,15 @@ export async function selectAndApplyModel( /** When false (interactive/guided-flow), skip dynamic routing and use the session model. * Dynamic routing only applies in auto-mode where cost optimization is expected. (#3962) */ isAutoMode = true, + /** Explicit /gsd model pin captured at bootstrap for long-running auto loops. */ + sessionModelOverride?: { provider: string; id: string } | null, ): Promise { - const modelConfig = resolvePreferredModelConfig(unitType, autoModeStartModel, isAutoMode); + const effectiveSessionModelOverride = sessionModelOverride === undefined + ? getSessionModelOverride(ctx.sessionManager.getSessionId()) + : (sessionModelOverride ?? undefined); + const modelConfig = effectiveSessionModelOverride + ? undefined + : resolvePreferredModelConfig(unitType, autoModeStartModel, isAutoMode); let routing: { tier: string; modelDowngraded: boolean } | null = null; let appliedModel: Model | null = null; diff --git a/src/resources/extensions/gsd/auto-start.ts b/src/resources/extensions/gsd/auto-start.ts index 5856bd0b9..345f0b9bc 100644 --- a/src/resources/extensions/gsd/auto-start.ts +++ b/src/resources/extensions/gsd/auto-start.ts @@ -85,6 +85,7 @@ import { sep as pathSep } from "node:path"; import { resolveProjectRootDbPath } from "./bootstrap/dynamic-tools.js"; import { resolveDefaultSessionModel, resolveDynamicRoutingConfig } from "./preferences-models.js"; import type { WorktreeResolver } from "./worktree-resolver.js"; +import { getSessionModelOverride } from "./session-model-override.js"; export interface BootstrapDeps { shouldUseWorktreeIsolation: () => boolean; @@ -266,12 +267,17 @@ export async function bootstrapAutoSession( // Capture the user's session model before guided-flow dispatch can apply a // phase-specific planning model for a discuss turn (#2829). // - // GSD PREFERENCES.md takes priority over the session model from settings.json - // (#3517). The session model (ctx.model) comes from findInitialModel() which - // reads defaultProvider/defaultModel from ~/.gsd/agent/settings.json. When - // the user has explicit model preferences in PREFERENCES.md, those should win. + // Precedence: + // 1) Explicit session override via /gsd model (this session) + // 2) GSD model preferences from PREFERENCES.md + // 3) Current session model from settings/session restore + // + // This preserves #3517 defaults while honoring explicit runtime model + // selection for subsequent /gsd runs in the same session. + const manualSessionOverride = getSessionModelOverride(ctx.sessionManager.getSessionId()); const preferredModel = resolveDefaultSessionModel(ctx.model?.provider); - const startModelSnapshot = preferredModel + const startModelSnapshot = manualSessionOverride + ?? preferredModel ?? (ctx.model ? { provider: ctx.model.provider, id: ctx.model.id } : null); @@ -731,6 +737,7 @@ export async function bootstrapAutoSession( id: startModelSnapshot.id, }; } + s.manualSessionModelOverride = manualSessionOverride ?? null; // Apply worker model override from parallel orchestrator (#worker-model). // GSD_WORKER_MODEL is injected by the coordinator when parallel.worker_model diff --git a/src/resources/extensions/gsd/auto/loop-deps.ts b/src/resources/extensions/gsd/auto/loop-deps.ts index ff63d8a3e..c68a68c3a 100644 --- a/src/resources/extensions/gsd/auto/loop-deps.ts +++ b/src/resources/extensions/gsd/auto/loop-deps.ts @@ -211,6 +211,8 @@ export interface LoopDeps { verbose: boolean, startModel: { provider: string; id: string } | null, retryContext?: { isRetry: boolean; previousTier?: string }, + isAutoMode?: boolean, + sessionModelOverride?: { provider: string; id: string } | null, ) => Promise<{ routing: { tier: string; modelDowngraded: boolean } | null; appliedModel: { provider: string; id: string } | null; diff --git a/src/resources/extensions/gsd/auto/phases.ts b/src/resources/extensions/gsd/auto/phases.ts index a3591e6ca..71283447c 100644 --- a/src/resources/extensions/gsd/auto/phases.ts +++ b/src/resources/extensions/gsd/auto/phases.ts @@ -1183,6 +1183,8 @@ export async function runUnitPhase( s.verbose, s.autoModeStartModel, sidecarItem ? undefined : { isRetry, previousTier }, + undefined, + s.manualSessionModelOverride, ); s.currentUnitRouting = modelResult.routing as AutoSession["currentUnitRouting"]; diff --git a/src/resources/extensions/gsd/auto/session.ts b/src/resources/extensions/gsd/auto/session.ts index ff69877b9..426713411 100644 --- a/src/resources/extensions/gsd/auto/session.ts +++ b/src/resources/extensions/gsd/auto/session.ts @@ -111,6 +111,8 @@ export class AutoSession { // ── Model state ────────────────────────────────────────────────────────── autoModeStartModel: StartModel | null = null; + /** Explicit /gsd model pin captured at bootstrap (session-scoped policy override). */ + manualSessionModelOverride: StartModel | null = null; currentUnitModel: Model | null = null; /** Fully-qualified model ID (provider/id) set after selectAndApplyModel + hook overrides (#2899). */ currentDispatchedModelId: string | null = null; @@ -222,6 +224,7 @@ export class AutoSession { // Model this.autoModeStartModel = null; + this.manualSessionModelOverride = null; this.currentUnitModel = null; this.currentDispatchedModelId = null; this.originalModelId = null; diff --git a/src/resources/extensions/gsd/commands/handlers/core.ts b/src/resources/extensions/gsd/commands/handlers/core.ts index 8855a4e2a..51aaec2bc 100644 --- a/src/resources/extensions/gsd/commands/handlers/core.ts +++ b/src/resources/extensions/gsd/commands/handlers/core.ts @@ -8,6 +8,7 @@ import { ensurePreferencesFile, handlePrefs, handlePrefsMode, handlePrefsWizard import { runEnvironmentChecks } from "../../doctor-environment.js"; import { deriveState } from "../../state.js"; import { handleCmux } from "../../commands-cmux.js"; +import { setSessionModelOverride } from "../../session-model-override.js"; import { projectRoot } from "../context.js"; import { formattedShortcutPair } from "../../shortcut-defs.js"; @@ -336,6 +337,17 @@ async function handleModel(trimmedArgs: string, ctx: ExtensionCommandContext, pi return; } + // /gsd model is an explicit per-session pin for GSD dispatches. + // This is captured at auto bootstrap so it survives internal session + // switches during /gsd auto and /gsd next runs. + const sessionId = ctx.sessionManager?.getSessionId?.(); + if (sessionId) { + setSessionModelOverride(sessionId, { + provider: targetModel.provider, + id: targetModel.id, + }); + } + ctx.ui.notify(`Model: ${targetModel.provider}/${targetModel.id}`, "info"); } diff --git a/src/resources/extensions/gsd/session-model-override.ts b/src/resources/extensions/gsd/session-model-override.ts new file mode 100644 index 000000000..3494c4da7 --- /dev/null +++ b/src/resources/extensions/gsd/session-model-override.ts @@ -0,0 +1,36 @@ +export interface SessionModelOverride { + provider: string; + id: string; +} + +const sessionOverrides = new Map(); + +function normalizeSessionId(sessionId: string): string { + return typeof sessionId === "string" ? sessionId.trim() : ""; +} + +export function setSessionModelOverride( + sessionId: string, + override: SessionModelOverride, +): void { + const key = normalizeSessionId(sessionId); + if (!key) return; + sessionOverrides.set(key, { + provider: override.provider, + id: override.id, + }); +} + +export function getSessionModelOverride( + sessionId: string, +): SessionModelOverride | undefined { + const key = normalizeSessionId(sessionId); + if (!key) return undefined; + return sessionOverrides.get(key); +} + +export function clearSessionModelOverride(sessionId: string): void { + const key = normalizeSessionId(sessionId); + if (!key) return; + sessionOverrides.delete(key); +} diff --git a/src/resources/extensions/gsd/tests/auto-start-model-capture.test.ts b/src/resources/extensions/gsd/tests/auto-start-model-capture.test.ts index 2ffb5bf96..e1b090845 100644 --- a/src/resources/extensions/gsd/tests/auto-start-model-capture.test.ts +++ b/src/resources/extensions/gsd/tests/auto-start-model-capture.test.ts @@ -7,9 +7,8 @@ const sourcePath = join(import.meta.dirname, "..", "auto-start.ts"); const source = readFileSync(sourcePath, "utf-8"); test("bootstrapAutoSession snapshots ctx.model before guided-flow entry (#2829)", () => { - // #3517 changed the snapshot to prefer GSD preferences, but the ordering - // guarantee still holds: the snapshot must be built before guided-flow. - const snapshotIdx = source.indexOf("const startModelSnapshot = preferredModel"); + // The snapshot ordering guarantee still holds: build snapshot before guided-flow. + const snapshotIdx = source.indexOf("const startModelSnapshot = manualSessionOverride"); assert.ok(snapshotIdx > -1, "auto-start.ts should snapshot model at bootstrap start"); const firstDiscussIdx = source.indexOf('await showSmartEntry(ctx, pi, base, { step: requestedStepMode });'); @@ -29,8 +28,11 @@ test("bootstrapAutoSession restores autoModeStartModel from the early snapshot ( assert.ok(snapshotRefIdx > -1, "autoModeStartModel should be restored from startModelSnapshot"); }); -test("bootstrapAutoSession prefers GSD PREFERENCES.md over settings.json for start model (#3517)", () => { - // resolveDefaultSessionModel() should be called before the snapshot is built +test("bootstrapAutoSession checks manual session override before preferences", () => { + const manualIdx = source.indexOf("const manualSessionOverride = getSessionModelOverride("); + assert.ok(manualIdx > -1, "auto-start.ts should read session model override first"); + + // resolveDefaultSessionModel() should still be called for fallback behavior const preferredIdx = source.indexOf("const preferredModel = resolveDefaultSessionModel("); assert.ok(preferredIdx > -1, "auto-start.ts should call resolveDefaultSessionModel()"); @@ -38,11 +40,11 @@ test("bootstrapAutoSession prefers GSD PREFERENCES.md over settings.json for sta const withProviderIdx = source.indexOf("resolveDefaultSessionModel(ctx.model?.provider)"); assert.ok(withProviderIdx > -1, "auto-start.ts should pass ctx.model?.provider for bare ID resolution"); - const snapshotIdx = source.indexOf("const startModelSnapshot = preferredModel"); - assert.ok(snapshotIdx > -1, "startModelSnapshot should use preferredModel when available"); + const snapshotIdx = source.indexOf("const startModelSnapshot = manualSessionOverride"); + assert.ok(snapshotIdx > -1, "startModelSnapshot should prefer manual session override"); assert.ok( - preferredIdx < snapshotIdx, - "resolveDefaultSessionModel() must be called before building startModelSnapshot", + manualIdx < snapshotIdx && preferredIdx < snapshotIdx, + "manual override and preference fallback must be resolved before building startModelSnapshot", ); }); diff --git a/src/resources/extensions/gsd/tests/model-isolation.test.ts b/src/resources/extensions/gsd/tests/model-isolation.test.ts index 6dd107b12..8b86a20b8 100644 --- a/src/resources/extensions/gsd/tests/model-isolation.test.ts +++ b/src/resources/extensions/gsd/tests/model-isolation.test.ts @@ -1,6 +1,6 @@ /** * Tests for model config isolation between concurrent instances (#650, #1065) - * and GSD preferences override of settings.json defaults (#3517). + * and session-scoped model precedence behavior. */ import { describe, it, beforeEach, afterEach } from "node:test"; @@ -157,75 +157,60 @@ describe("session model recovery on error (#1065)", () => { }); }); -// ─── GSD Preferences override settings.json (#3517) ───────────────────────── +// ─── Manual session model override precedence ─────────────────────────────── -describe("GSD preferences override settings.json for session model (#3517)", () => { - it("preferredModel takes priority over ctx.model when both are available", () => { - // Simulates auto-start.ts logic: preferredModel ?? ctx.model snapshot - const preferredModel = { provider: "openai-codex", id: "gpt-5.4" }; - const ctxModel = { provider: "claude-code", id: "claude-sonnet-4-6" }; +describe("manual session model override precedence", () => { + it("manual session override takes priority over preferences and ctx.model", () => { + const manualSessionOverride = { provider: "openai-codex", id: "gpt-5.4" }; + const preferredModel = { provider: "anthropic", id: "claude-sonnet-4-6" }; + const ctxModel = { provider: "claude-code", id: "claude-opus-4-6" }; - const startModelSnapshot = preferredModel + const startModelSnapshot = manualSessionOverride + ?? preferredModel ?? { provider: ctxModel.provider, id: ctxModel.id }; - assert.equal(startModelSnapshot.provider, "openai-codex", - "preferredModel provider should win over ctx.model"); - assert.equal(startModelSnapshot.id, "gpt-5.4", - "preferredModel id should win over ctx.model"); + assert.equal(startModelSnapshot.provider, "openai-codex"); + assert.equal(startModelSnapshot.id, "gpt-5.4"); }); - it("falls back to ctx.model when no GSD preferences are configured", () => { + it("falls back to preferences when no manual override is active", () => { + const manualSessionOverride: { provider: string; id: string } | undefined = undefined; + const preferredModel = { provider: "anthropic", id: "claude-sonnet-4-6" }; + const ctxModel = { provider: "claude-code", id: "claude-opus-4-6" }; + + const startModelSnapshot = manualSessionOverride + ?? preferredModel + ?? { provider: ctxModel.provider, id: ctxModel.id }; + + assert.equal(startModelSnapshot.provider, "anthropic"); + assert.equal(startModelSnapshot.id, "claude-sonnet-4-6"); + }); + + it("falls back to ctx.model when no manual override or preferences are configured", () => { + const manualSessionOverride: { provider: string; id: string } | undefined = undefined; const preferredModel: { provider: string; id: string } | undefined = undefined; - const ctxModel = { provider: "claude-code", id: "claude-sonnet-4-6" }; + const ctxModel = { provider: "claude-code", id: "claude-opus-4-6" }; - const startModelSnapshot = preferredModel + const startModelSnapshot = manualSessionOverride + ?? preferredModel ?? { provider: ctxModel.provider, id: ctxModel.id }; - assert.equal(startModelSnapshot.provider, "claude-code", - "should fall back to ctx.model provider when no preferences"); - assert.equal(startModelSnapshot.id, "claude-sonnet-4-6", - "should fall back to ctx.model id when no preferences"); + assert.equal(startModelSnapshot.provider, "claude-code"); + assert.equal(startModelSnapshot.id, "claude-opus-4-6"); }); - it("handles null ctx.model with no preferences gracefully", () => { + it("handles null ctx.model with no override or preferences gracefully", () => { + const manualSessionOverride: { provider: string; id: string } | undefined = undefined; const preferredModel: { provider: string; id: string } | undefined = undefined; // Use a function to prevent TS from narrowing to `never` in the ternary function getCtxModel(): { provider: string; id: string } | null { return null; } const ctxModel = getCtxModel(); - const startModelSnapshot = preferredModel + const startModelSnapshot = manualSessionOverride + ?? preferredModel ?? (ctxModel ? { provider: ctxModel.provider, id: ctxModel.id } : null); assert.equal(startModelSnapshot, null, - "should be null when neither preferences nor ctx.model exist"); - }); - - it("bare model ID uses session provider when available", () => { - // Simulates: PREFERENCES.md has "gpt-5.4" (no provider), session is openai-codex - const preferredModel = { provider: "openai-codex", id: "gpt-5.4" }; // from resolveDefaultSessionModel("openai-codex") - const ctxModel = { provider: "openai-codex", id: "claude-sonnet-4-6" }; - - const startModelSnapshot = preferredModel - ?? { provider: ctxModel.provider, id: ctxModel.id }; - - assert.equal(startModelSnapshot.provider, "openai-codex"); - assert.equal(startModelSnapshot.id, "gpt-5.4", - "bare model ID from preferences should still override ctx.model"); - }); - - it("stale settings.json does not leak when preferences are set", () => { - // Scenario: settings.json has claude-code, PREFERENCES.md has openai-codex - const settingsJsonDefault = { provider: "claude-code", id: "claude-sonnet-4-6" }; - const preferencesModel = { provider: "openai-codex", id: "gpt-5.4" }; - - // auto-start.ts captures preferredModel first, which preempts settingsJsonDefault - const startModelSnapshot = preferencesModel ?? settingsJsonDefault; - - assert.equal(startModelSnapshot.provider, "openai-codex", - "PREFERENCES.md must override stale settings.json provider"); - assert.equal(startModelSnapshot.id, "gpt-5.4", - "PREFERENCES.md must override stale settings.json model"); - assert.notEqual(startModelSnapshot.provider, settingsJsonDefault.provider, - "settings.json provider must NOT leak through"); + "should be null when no model source is available"); }); }); diff --git a/src/resources/extensions/gsd/tests/session-model-override.test.ts b/src/resources/extensions/gsd/tests/session-model-override.test.ts new file mode 100644 index 000000000..a6ca1a31b --- /dev/null +++ b/src/resources/extensions/gsd/tests/session-model-override.test.ts @@ -0,0 +1,35 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { + clearSessionModelOverride, + getSessionModelOverride, + setSessionModelOverride, +} from "../session-model-override.js"; + +const phasesSource = readFileSync(join(import.meta.dirname, "..", "auto", "phases.ts"), "utf-8"); + +test("setSessionModelOverride stores provider/model for the session", () => { + const sessionId = `session-override-${Date.now()}`; + setSessionModelOverride(sessionId, { provider: "openai-codex", id: "gpt-5.4" }); + + const override = getSessionModelOverride(sessionId); + assert.equal(override?.provider, "openai-codex"); + assert.equal(override?.id, "gpt-5.4"); +}); + +test("clearSessionModelOverride removes the session override", () => { + const sessionId = `session-clear-${Date.now()}`; + setSessionModelOverride(sessionId, { provider: "anthropic", id: "claude-sonnet-4-6" }); + clearSessionModelOverride(sessionId); + assert.equal(getSessionModelOverride(sessionId), undefined); +}); + +test("auto dispatch threads manual session model override into selectAndApplyModel", () => { + assert.ok( + phasesSource.includes("s.manualSessionModelOverride"), + "auto/phases.ts should pass s.manualSessionModelOverride into selectAndApplyModel", + ); +});