From 70c76d9a1a7147d8887b6205d78a71289a9ec515 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sat, 4 Apr 2026 18:10:50 -0500 Subject: [PATCH] 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" };