Merge pull request #4067 from jeremymcs/fix/gsd-model-session-override

This commit is contained in:
Jeremy McSpadden 2026-04-12 12:50:12 -05:00 committed by GitHub
commit c3aa3a3bf0
12 changed files with 258 additions and 84 deletions

View file

@ -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<Api> {
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<Api>;
}
function makeRegistry(opts: {
readyProviders?: Set<string>;
byProviderAndId?: Map<string, Model<Api>>;
available?: Model<Api>[];
}): ModelRegistry {
const readyProviders = opts.readyProviders ?? new Set<string>();
const byProviderAndId = opts.byProviderAndId ?? new Map<string, Model<Api>>();
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");
});
});

View file

@ -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 };
}
}

View file

@ -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<ModelSelectionResult> {
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<Api> | null = null;

View file

@ -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

View file

@ -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;

View file

@ -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"];

View file

@ -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<Api> | 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;

View file

@ -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");
}

View file

@ -0,0 +1,36 @@
export interface SessionModelOverride {
provider: string;
id: string;
}
const sessionOverrides = new Map<string, SessionModelOverride>();
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);
}

View file

@ -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",
);
});

View file

@ -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");
});
});

View file

@ -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",
);
});