diff --git a/src/resources/extensions/gsd/auto-start.ts b/src/resources/extensions/gsd/auto-start.ts index 11e0a621e..34b5467ab 100644 --- a/src/resources/extensions/gsd/auto-start.ts +++ b/src/resources/extensions/gsd/auto-start.ts @@ -80,6 +80,7 @@ import { join } from "node:path"; import { sep as pathSep } from "node:path"; import { resolveProjectRootDbPath } from "./bootstrap/dynamic-tools.js"; +import { resolveDefaultSessionModel } from "./preferences-models.js"; import type { WorktreeResolver } from "./worktree-resolver.js"; export interface BootstrapDeps { @@ -155,12 +156,16 @@ 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). - const startModelSnapshot = ctx.model - ? { - provider: ctx.model.provider, - id: ctx.model.id, - } - : null; + // + // 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. + const preferredModel = resolveDefaultSessionModel(); + const startModelSnapshot = preferredModel + ?? (ctx.model + ? { provider: ctx.model.provider, id: ctx.model.id } + : null); try { // Validate GSD_PROJECT_ID early so the user gets immediate feedback diff --git a/src/resources/extensions/gsd/preferences-models.ts b/src/resources/extensions/gsd/preferences-models.ts index 22e6909b1..40b41d04c 100644 --- a/src/resources/extensions/gsd/preferences-models.ts +++ b/src/resources/extensions/gsd/preferences-models.ts @@ -107,6 +107,52 @@ export function resolveModelWithFallbacksForUnit(unitType: string): ResolvedMode }; } +/** + * Resolve the default session model from GSD preferences. + * + * Used at auto-mode bootstrap to override the session model that was + * determined by settings.json (defaultProvider/defaultModel). When + * PREFERENCES.md (or project preferences) configures an `execution` model + * we treat that as the session default. Falls back through execution → + * planning → first configured model. + * + * Returns `{ provider, id }` parsed from the `provider/model` format, + * or `undefined` if no model preference is configured. + */ +export function resolveDefaultSessionModel(): { provider: string; id: string } | undefined { + const prefs = loadEffectiveGSDPreferences(); + if (!prefs?.preferences.models) return undefined; + + const m = prefs.preferences.models as GSDModelConfigV2; + + // Priority: execution → planning → first configured value + const candidates: Array = [ + m.execution, + m.planning, + m.research, + m.discuss, + m.completion, + m.validation, + m.subagent, + ]; + + for (const cfg of candidates) { + if (!cfg) continue; + const modelStr = typeof cfg === "string" + ? cfg + : cfg.provider && !cfg.model.includes("/") + ? `${cfg.provider}/${cfg.model}` + : cfg.model; + + if (modelStr.includes("/")) { + const slashIdx = modelStr.indexOf("/"); + return { provider: modelStr.slice(0, slashIdx), id: modelStr.slice(slashIdx + 1) }; + } + } + + return undefined; +} + /** * Determines the next fallback model to try when the current model fails. * If the current model is not in the configured list, returns the primary model. 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 3daa00f3f..d18a1ada0 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,8 +7,10 @@ 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)", () => { - const snapshotIdx = source.indexOf("const startModelSnapshot = ctx.model"); - assert.ok(snapshotIdx > -1, "auto-start.ts should snapshot ctx.model at bootstrap start"); + // #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"); + assert.ok(snapshotIdx > -1, "auto-start.ts should snapshot model at bootstrap start"); const firstDiscussIdx = source.indexOf('await showSmartEntry(ctx, pi, base, { step: requestedStepMode });'); assert.ok(firstDiscussIdx > -1, "auto-start.ts should route through showSmartEntry during guided flow"); @@ -26,3 +28,17 @@ test("bootstrapAutoSession restores autoModeStartModel from the early snapshot ( const snapshotRefIdx = source.indexOf("provider: startModelSnapshot.provider", assignmentIdx); 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 + const preferredIdx = source.indexOf("const preferredModel = resolveDefaultSessionModel()"); + assert.ok(preferredIdx > -1, "auto-start.ts should call resolveDefaultSessionModel()"); + + const snapshotIdx = source.indexOf("const startModelSnapshot = preferredModel"); + assert.ok(snapshotIdx > -1, "startModelSnapshot should use preferredModel when available"); + + assert.ok( + preferredIdx < snapshotIdx, + "resolveDefaultSessionModel() must be called 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 088d24079..27177b327 100644 --- a/src/resources/extensions/gsd/tests/model-isolation.test.ts +++ b/src/resources/extensions/gsd/tests/model-isolation.test.ts @@ -1,5 +1,6 @@ /** - * Tests for model config isolation between concurrent instances (#650, #1065). + * Tests for model config isolation between concurrent instances (#650, #1065) + * and GSD preferences override of settings.json defaults (#3517). */ import { describe, it, beforeEach, afterEach } from "node:test"; @@ -155,3 +156,63 @@ describe("session model recovery on error (#1065)", () => { "Recovery should be skipped when no session model was captured"); }); }); + +// ─── GSD Preferences override settings.json (#3517) ───────────────────────── + +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" }; + + const startModelSnapshot = 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"); + }); + + it("falls back to ctx.model when no GSD preferences are configured", () => { + const preferredModel: { provider: string; id: string } | undefined = undefined; + const ctxModel = { provider: "claude-code", id: "claude-sonnet-4-6" }; + + const startModelSnapshot = 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"); + }); + + it("handles null ctx.model with no preferences gracefully", () => { + 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 + ?? (ctxModel ? { provider: ctxModel.provider, id: ctxModel.id } : null); + + assert.equal(startModelSnapshot, null, + "should be null when neither preferences nor ctx.model exist"); + }); + + 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"); + }); +});