Merge pull request #4138 from jeremymcs/claude/investigate-issue-4122-6Vi1I
fix(gsd): preserve custom-model selection on /gsd auto bootstrap (#4122)
This commit is contained in:
commit
24f51fd76b
4 changed files with 199 additions and 6 deletions
|
|
@ -83,7 +83,11 @@ import { join } from "node:path";
|
|||
import { sep as pathSep } from "node:path";
|
||||
|
||||
import { resolveProjectRootDbPath } from "./bootstrap/dynamic-tools.js";
|
||||
import { resolveDefaultSessionModel, resolveDynamicRoutingConfig } from "./preferences-models.js";
|
||||
import {
|
||||
isCustomProvider,
|
||||
resolveDefaultSessionModel,
|
||||
resolveDynamicRoutingConfig,
|
||||
} from "./preferences-models.js";
|
||||
import type { WorktreeResolver } from "./worktree-resolver.js";
|
||||
import { getSessionModelOverride } from "./session-model-override.js";
|
||||
|
||||
|
|
@ -274,8 +278,18 @@ export async function bootstrapAutoSession(
|
|||
//
|
||||
// This preserves #3517 defaults while honoring explicit runtime model
|
||||
// selection for subsequent /gsd runs in the same session.
|
||||
//
|
||||
// Exception (#4122): when the session provider is a custom provider declared
|
||||
// in ~/.gsd/agent/models.json (Ollama, vLLM, OpenAI-compatible proxy, etc.),
|
||||
// PREFERENCES.md is skipped entirely. PREFERENCES.md cannot reference custom
|
||||
// providers, so honoring it would silently reroute auto-mode to a built-in
|
||||
// provider the user is not logged into and surface as "Not logged in · Please
|
||||
// run /login" before pausing and resetting to claude-code/claude-sonnet-4-6.
|
||||
const manualSessionOverride = getSessionModelOverride(ctx.sessionManager.getSessionId());
|
||||
const preferredModel = resolveDefaultSessionModel(ctx.model?.provider);
|
||||
const sessionProviderIsCustom = isCustomProvider(ctx.model?.provider);
|
||||
const preferredModel = sessionProviderIsCustom
|
||||
? null
|
||||
: resolveDefaultSessionModel(ctx.model?.provider);
|
||||
// Validate the preferred model against the live registry + provider auth so
|
||||
// an unconfigured PREFERENCES.md entry (no API key / OAuth) can't become the
|
||||
// start-model snapshot. Without this, every subsequent unit would try to
|
||||
|
|
|
|||
|
|
@ -7,6 +7,8 @@
|
|||
*/
|
||||
|
||||
import { existsSync, readFileSync, writeFileSync } from "node:fs";
|
||||
import { homedir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import type { DynamicRoutingConfig } from "./model-router.js";
|
||||
import { defaultRoutingConfig } from "./model-router.js";
|
||||
import type { TokenProfile, InlineLevel } from "./types.js";
|
||||
|
|
@ -185,6 +187,45 @@ export function resolveDefaultSessionModel(
|
|||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true if `provider` is defined as a custom provider in the user's
|
||||
* `~/.gsd/agent/models.json` (Ollama, vLLM, LM Studio, OpenAI-compatible
|
||||
* proxies, etc.).
|
||||
*
|
||||
* Used by auto-mode bootstrap to decide whether the session model
|
||||
* (set via `/gsd model`) should override `PREFERENCES.md`. Custom providers
|
||||
* are never reachable from `PREFERENCES.md` (which only knows built-in
|
||||
* providers), so when the user has explicitly selected one, it must take
|
||||
* priority — otherwise auto-mode tries to start the built-in provider from
|
||||
* PREFERENCES.md and fails with "Not logged in · Please run /login" (#4122).
|
||||
*
|
||||
* Reads models.json directly with a lightweight JSON parse to avoid
|
||||
* pulling in the full model-registry at this call site. Falls back to
|
||||
* `~/.pi/agent/models.json` for parity with `resolveModelsJsonPath()`.
|
||||
* Any read or parse error yields `false` (treat as not-custom) so a
|
||||
* malformed models.json never breaks the session bootstrap.
|
||||
*/
|
||||
export function isCustomProvider(provider: string | undefined): boolean {
|
||||
if (!provider) return false;
|
||||
const candidates = [
|
||||
join(homedir(), ".gsd", "agent", "models.json"),
|
||||
join(homedir(), ".pi", "agent", "models.json"),
|
||||
];
|
||||
for (const path of candidates) {
|
||||
if (!existsSync(path)) continue;
|
||||
try {
|
||||
const raw = readFileSync(path, "utf-8");
|
||||
const parsed = JSON.parse(raw) as { providers?: Record<string, unknown> };
|
||||
if (parsed?.providers && Object.prototype.hasOwnProperty.call(parsed.providers, provider)) {
|
||||
return true;
|
||||
}
|
||||
} catch {
|
||||
// Ignore — malformed models.json must not break bootstrap.
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
|
|
|
|||
|
|
@ -33,8 +33,12 @@ test("bootstrapAutoSession checks manual session override before preferences", (
|
|||
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()");
|
||||
const preferredIdx = source.indexOf("const preferredModel = ");
|
||||
assert.ok(preferredIdx > -1, "auto-start.ts should build preferredModel");
|
||||
assert.ok(
|
||||
source.indexOf("resolveDefaultSessionModel(") > -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)");
|
||||
|
|
@ -47,6 +51,51 @@ test("bootstrapAutoSession checks manual session override before preferences", (
|
|||
manualIdx < snapshotIdx && preferredIdx < snapshotIdx,
|
||||
"manual override and preference fallback must be resolved before building startModelSnapshot",
|
||||
);
|
||||
|
||||
// The validated preferred model must still appear as one of the snapshot
|
||||
// sources so PREFERENCES.md continues to win over a stale settings.json
|
||||
// default for built-in providers.
|
||||
const snapshotBlock = source.slice(snapshotIdx, snapshotIdx + 400);
|
||||
assert.ok(
|
||||
snapshotBlock.includes("validatedPreferredModel") || snapshotBlock.includes("preferredModel"),
|
||||
"startModelSnapshot must still consider preferredModel for built-in providers",
|
||||
);
|
||||
});
|
||||
|
||||
test("bootstrapAutoSession prefers session model over PREFERENCES.md when provider is custom (#4122)", () => {
|
||||
// Custom providers (Ollama, vLLM, OpenAI-compatible proxies) live in
|
||||
// ~/.gsd/agent/models.json, not PREFERENCES.md. When the user picks one
|
||||
// via /gsd model, that selection must win over any preferredModel from
|
||||
// PREFERENCES.md, otherwise auto-mode tries to start a built-in provider
|
||||
// the user is not logged into and pauses with "Not logged in".
|
||||
const customCheckIdx = source.indexOf("isCustomProvider(ctx.model?.provider)");
|
||||
assert.ok(
|
||||
customCheckIdx > -1,
|
||||
"auto-start.ts should call isCustomProvider() to detect custom-model sessions",
|
||||
);
|
||||
|
||||
// sessionProviderIsCustom must gate preferredModel resolution so that when the
|
||||
// session provider is custom, preferredModel is null and PREFERENCES.md is
|
||||
// skipped entirely — the snapshot then falls through to ctx.model.
|
||||
const gateIdx = source.indexOf("sessionProviderIsCustom");
|
||||
assert.ok(gateIdx > -1, "auto-start.ts should bind sessionProviderIsCustom");
|
||||
|
||||
const preferredIdx = source.indexOf("const preferredModel = ");
|
||||
assert.ok(preferredIdx > -1, "auto-start.ts should build preferredModel");
|
||||
|
||||
const preferredBlock = source.slice(preferredIdx, preferredIdx + 200);
|
||||
assert.ok(
|
||||
preferredBlock.includes("sessionProviderIsCustom"),
|
||||
"preferredModel must be gated on sessionProviderIsCustom so PREFERENCES.md is skipped for custom providers",
|
||||
);
|
||||
|
||||
const snapshotIdx = source.indexOf("const startModelSnapshot = ");
|
||||
assert.ok(snapshotIdx > -1, "auto-start.ts should build startModelSnapshot");
|
||||
|
||||
assert.ok(
|
||||
customCheckIdx < preferredIdx && preferredIdx < snapshotIdx,
|
||||
"isCustomProvider() must be evaluated before preferredModel, which must be resolved before startModelSnapshot",
|
||||
);
|
||||
});
|
||||
|
||||
test("bootstrapAutoSession validates preferred model against live registry auth (#unconfigured-models)", () => {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
/**
|
||||
* Tests for model config isolation between concurrent instances (#650, #1065)
|
||||
* and session-scoped model precedence behavior.
|
||||
* Tests for model config isolation between concurrent instances (#650, #1065),
|
||||
* session-scoped model precedence behavior including manual session override,
|
||||
* GSD preferences override of settings.json defaults (#3517), and custom
|
||||
* provider precedence over PREFERENCES.md when set via `/gsd model` (#4122).
|
||||
*/
|
||||
|
||||
import { describe, it, beforeEach, afterEach } from "node:test";
|
||||
|
|
@ -214,3 +216,90 @@ describe("manual session model override precedence", () => {
|
|||
"should be null when no model source is available");
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Custom provider session model wins over PREFERENCES.md (#4122) ─────────
|
||||
|
||||
describe("custom provider session model overrides PREFERENCES.md (#4122)", () => {
|
||||
// Mirrors the auto-start.ts logic:
|
||||
// sessionProviderIsCustom && ctx.model
|
||||
// ? ctx.model
|
||||
// : (preferredModel ?? ctx.model ?? null)
|
||||
function selectStartModel(args: {
|
||||
ctxModel: { provider: string; id: string } | null;
|
||||
preferredModel: { provider: string; id: string } | undefined;
|
||||
sessionProviderIsCustom: boolean;
|
||||
}): { provider: string; id: string } | null {
|
||||
const { ctxModel, preferredModel, sessionProviderIsCustom } = args;
|
||||
if (sessionProviderIsCustom && ctxModel) {
|
||||
return { provider: ctxModel.provider, id: ctxModel.id };
|
||||
}
|
||||
return preferredModel
|
||||
?? (ctxModel ? { provider: ctxModel.provider, id: ctxModel.id } : null);
|
||||
}
|
||||
|
||||
it("custom provider from /gsd model wins over PREFERENCES.md built-in default", () => {
|
||||
// User runs `/gsd model ollama/llama3.1:8b`, then `/gsd auto`.
|
||||
// PREFERENCES.md still has the project-template claude-code default.
|
||||
const ctxModel = { provider: "ollama", id: "llama3.1:8b" };
|
||||
const preferredModel = { provider: "claude-code", id: "claude-sonnet-4-6" };
|
||||
|
||||
const snapshot = selectStartModel({
|
||||
ctxModel,
|
||||
preferredModel,
|
||||
sessionProviderIsCustom: true,
|
||||
});
|
||||
|
||||
assert.equal(snapshot?.provider, "ollama",
|
||||
"custom-provider session model must win over PREFERENCES.md");
|
||||
assert.equal(snapshot?.id, "llama3.1:8b",
|
||||
"custom-provider session model id must be preserved");
|
||||
assert.notEqual(snapshot?.provider, "claude-code",
|
||||
"claude-code from PREFERENCES.md must NOT be selected when session is custom");
|
||||
});
|
||||
|
||||
it("built-in session provider still defers to PREFERENCES.md (#3517 preserved)", () => {
|
||||
// ctx.model is a built-in provider (claude-code) but PREFERENCES.md has
|
||||
// an explicit openai-codex preference. PREFERENCES.md should still win.
|
||||
const ctxModel = { provider: "claude-code", id: "claude-sonnet-4-6" };
|
||||
const preferredModel = { provider: "openai-codex", id: "gpt-5.4" };
|
||||
|
||||
const snapshot = selectStartModel({
|
||||
ctxModel,
|
||||
preferredModel,
|
||||
sessionProviderIsCustom: false,
|
||||
});
|
||||
|
||||
assert.equal(snapshot?.provider, "openai-codex",
|
||||
"PREFERENCES.md must still win when session provider is built-in");
|
||||
assert.equal(snapshot?.id, "gpt-5.4");
|
||||
});
|
||||
|
||||
it("custom provider with no PREFERENCES.md still uses ctx.model", () => {
|
||||
const ctxModel = { provider: "vllm", id: "qwen2.5-coder:32b" };
|
||||
|
||||
const snapshot = selectStartModel({
|
||||
ctxModel,
|
||||
preferredModel: undefined,
|
||||
sessionProviderIsCustom: true,
|
||||
});
|
||||
|
||||
assert.equal(snapshot?.provider, "vllm");
|
||||
assert.equal(snapshot?.id, "qwen2.5-coder:32b");
|
||||
});
|
||||
|
||||
it("null ctx.model with custom flag falls through to preferredModel", () => {
|
||||
// Defensive: sessionProviderIsCustom can only be true if ctx.model exists,
|
||||
// but verify the guard works if that invariant is ever broken.
|
||||
const preferredModel = { provider: "claude-code", id: "claude-sonnet-4-6" };
|
||||
|
||||
const snapshot = selectStartModel({
|
||||
ctxModel: null,
|
||||
preferredModel,
|
||||
sessionProviderIsCustom: true,
|
||||
});
|
||||
|
||||
assert.equal(snapshot?.provider, "claude-code",
|
||||
"should fall back to preferredModel when ctx.model is null");
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue