From e247f2fe619a4baa34ce1d7b7b681d4a43bbc713 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sun, 12 Apr 2026 11:44:16 -0500 Subject: [PATCH] fix(gsd): honor /gsd model as session override across dispatch --- .../extensions/gsd/auto-model-selection.ts | 10 ++- src/resources/extensions/gsd/auto-start.ts | 17 ++-- .../extensions/gsd/auto/loop-deps.ts | 2 + src/resources/extensions/gsd/auto/phases.ts | 2 + src/resources/extensions/gsd/auto/session.ts | 3 + .../extensions/gsd/commands/handlers/core.ts | 9 ++ .../extensions/gsd/session-model-override.ts | 36 ++++++++ .../tests/auto-start-model-capture.test.ts | 20 +++-- .../gsd/tests/model-isolation.test.ts | 87 ++++++++----------- .../gsd/tests/session-model-override.test.ts | 35 ++++++++ 10 files changed, 155 insertions(+), 66 deletions(-) create mode 100644 src/resources/extensions/gsd/session-model-override.ts create mode 100644 src/resources/extensions/gsd/tests/session-model-override.test.ts 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 4f8fc82e0..ba314b8a1 100644 --- a/src/resources/extensions/gsd/auto/session.ts +++ b/src/resources/extensions/gsd/auto/session.ts @@ -107,6 +107,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; @@ -214,6 +216,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 e6824815c..b64dcb7c5 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 { formatShortcut } from "../../files.js"; @@ -301,6 +302,14 @@ 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. + setSessionModelOverride(ctx.sessionManager.getSessionId(), { + 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", + ); +});