From fbcd722cf4d8229212b874fcfdd16c2870fc37f6 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sat, 4 Apr 2026 17:15:43 -0500 Subject: [PATCH 1/2] fix(gsd): prefer PREFERENCES.md over settings.json for session bootstrap model (#3517) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Session bootstrap used ctx.model (from settings.json defaultProvider/defaultModel) as the autoModeStartModel snapshot. When settings.json had a stale provider (e.g. claude-code) but PREFERENCES.md was fully configured for openai-codex, sessions would start with the wrong provider and fail with auth errors. Add resolveDefaultSessionModel() to preferences-models.ts which extracts the default model from GSD preferences (execution → planning → first configured). In auto-start.ts, the preferred model now takes priority over ctx.model when building startModelSnapshot, so PREFERENCES.md always wins over stale settings. --- src/resources/extensions/gsd/auto-start.ts | 17 +++-- .../extensions/gsd/preferences-models.ts | 46 ++++++++++++++ .../tests/auto-start-model-capture.test.ts | 20 +++++- .../gsd/tests/model-isolation.test.ts | 63 ++++++++++++++++++- 4 files changed, 137 insertions(+), 9 deletions(-) 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"); + }); +}); From 70c76d9a1a7147d8887b6205d78a71289a9ec515 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sat, 4 Apr 2026 18:10:50 -0500 Subject: [PATCH 2/2] fix(gsd): handle bare model IDs in resolveDefaultSessionModel (#3517) resolveDefaultSessionModel() previously only returned a result for provider/model format strings, silently ignoring valid bare model IDs like "gpt-5.4". This meant preferences could fail to override stale settings.json defaults when users configured models without explicit provider prefixes. Now accepts sessionProvider param (ctx.model?.provider) to resolve bare IDs. Also handles object configs without explicit provider field. --- src/resources/extensions/gsd/auto-start.ts | 2 +- .../extensions/gsd/preferences-models.ts | 54 +++++++++++++++---- .../tests/auto-start-model-capture.test.ts | 6 ++- .../gsd/tests/model-isolation.test.ts | 13 +++++ 4 files changed, 62 insertions(+), 13 deletions(-) diff --git a/src/resources/extensions/gsd/auto-start.ts b/src/resources/extensions/gsd/auto-start.ts index 34b5467ab..50f137c2b 100644 --- a/src/resources/extensions/gsd/auto-start.ts +++ b/src/resources/extensions/gsd/auto-start.ts @@ -161,7 +161,7 @@ export async function bootstrapAutoSession( // (#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 preferredModel = resolveDefaultSessionModel(ctx.model?.provider); const startModelSnapshot = preferredModel ?? (ctx.model ? { provider: ctx.model.provider, id: ctx.model.id } diff --git a/src/resources/extensions/gsd/preferences-models.ts b/src/resources/extensions/gsd/preferences-models.ts index 40b41d04c..2e4171687 100644 --- a/src/resources/extensions/gsd/preferences-models.ts +++ b/src/resources/extensions/gsd/preferences-models.ts @@ -116,10 +116,19 @@ export function resolveModelWithFallbacksForUnit(unitType: string): ResolvedMode * 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. + * Accepts an optional `sessionProvider` for bare model IDs that don't + * include an explicit provider prefix (e.g. `gpt-5.4` instead of + * `openai-codex/gpt-5.4`). When a bare ID is found and sessionProvider + * is available, the session provider is used. Without sessionProvider, + * bare IDs are still returned with provider set to the bare ID itself + * so downstream resolution (resolveModelId) can match it. + * + * Returns `{ provider, id }` or `undefined` if no model preference is + * configured. */ -export function resolveDefaultSessionModel(): { provider: string; id: string } | undefined { +export function resolveDefaultSessionModel( + sessionProvider?: string, +): { provider: string; id: string } | undefined { const prefs = loadEffectiveGSDPreferences(); if (!prefs?.preferences.models) return undefined; @@ -138,15 +147,38 @@ export function resolveDefaultSessionModel(): { provider: string; id: string } | 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) }; + // Normalize to provider + id from the various config shapes + let provider: string | undefined; + let id: string; + + if (typeof cfg === "string") { + const slashIdx = cfg.indexOf("/"); + if (slashIdx !== -1) { + provider = cfg.slice(0, slashIdx); + id = cfg.slice(slashIdx + 1); + } else { + // Bare model ID (e.g. "gpt-5.4") — use session provider as context + provider = sessionProvider; + id = cfg; + } + } else { + // Object config: { model, provider?, fallbacks? } + if (cfg.provider) { + provider = cfg.provider; + } else if (cfg.model.includes("/")) { + const slashIdx = cfg.model.indexOf("/"); + provider = cfg.model.slice(0, slashIdx); + id = cfg.model.slice(slashIdx + 1); + return { provider, id }; + } else { + provider = sessionProvider; + } + id = cfg.model; + } + + if (provider && id) { + return { provider, id }; } } 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 d18a1ada0..2ffb5bf96 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 @@ -31,9 +31,13 @@ test("bootstrapAutoSession restores autoModeStartModel from the early snapshot ( 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()"); + const preferredIdx = source.indexOf("const preferredModel = resolveDefaultSessionModel("); assert.ok(preferredIdx > -1, "auto-start.ts should call resolveDefaultSessionModel()"); + // Session provider should be passed for bare model ID resolution + 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"); diff --git a/src/resources/extensions/gsd/tests/model-isolation.test.ts b/src/resources/extensions/gsd/tests/model-isolation.test.ts index 27177b327..6dd107b12 100644 --- a/src/resources/extensions/gsd/tests/model-isolation.test.ts +++ b/src/resources/extensions/gsd/tests/model-isolation.test.ts @@ -200,6 +200,19 @@ describe("GSD preferences override settings.json for session model (#3517)", () "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" };