Merge pull request #3518 from jeremymcs/fix/bootstrap-prefer-gsd-preferences

fix(gsd): prefer PREFERENCES.md over settings.json for bootstrap model

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(ctx.model?.provider);
+  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

src/resources/extensions/gsd/preferences-models.ts

@@ -107,6 +107,84 @@ 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.
+ *
+ * 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(
+  sessionProvider?: string,
+): { 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;
+
+    // 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 };
+    }
+  }
+
+  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.

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,21 @@ 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()");
+
+  // 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");
+
+  assert.ok(
+    preferredIdx < snapshotIdx,
+    "resolveDefaultSessionModel() must be called before building startModelSnapshot",
+  );
+});

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