fix(gsd): prefer PREFERENCES.md over settings.json for session bootstrap model (#3517)
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.
This commit is contained in:
parent
099e6f3120
commit
fbcd722cf4
4 changed files with 137 additions and 9 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<string | GSDPhaseModelConfig | undefined> = [
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue