Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,25 @@ test("agent-session: explicit model switches cancel retry before applying new mo
"retry cancellation must happen before applying the new model to prevent stale provider retries",
);
});

test("agent-session: transient model switches pass persist options into thinking-level application", () => {
const start = source.indexOf("private async _applyModelChange(");
assert.ok(start >= 0, "missing _applyModelChange");
const window = source.slice(start, start + 900);

assert.ok(
window.includes("this._applyThinkingLevel(thinkingLevel, options);"),
"_applyModelChange should forward persist options when applying thinking level during model switches",
);
});

test("agent-session: transient model switches do not persist thinking level defaults", () => {
const start = source.indexOf("private _applyThinkingLevel(");
assert.ok(start >= 0, "missing _applyThinkingLevel");
const window = source.slice(start, start + 900);

assert.ok(
window.includes("options?.persist !== false && (this.supportsThinking() || effectiveLevel !== \"off\")"),
"_applyThinkingLevel should skip settings persistence when persist:false is used for transient model switches",
);
});
16 changes: 14 additions & 2 deletions packages/pi-coding-agent/src/core/agent-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1642,7 +1642,7 @@ export class AgentSession {
if (options?.persist !== false) {
this.settingsManager.setDefaultModelAndProvider(model.provider, model.id);
}
this.setThinkingLevel(thinkingLevel);
this._applyThinkingLevel(thinkingLevel, options);
await this._emitModelSelect(model, previousModel, source);
this._emitSessionStateChanged("set_model");
}
Expand Down Expand Up @@ -1728,6 +1728,18 @@ export class AgentSession {
* Saves to session and settings only if the level actually changes.
*/
setThinkingLevel(level: ThinkingLevel): void {
this._applyThinkingLevel(level);
}

/**
* Internal thinking level setter that respects persist option.
* When called from _applyModelChange with { persist: false }, the thinking
* level is applied in-memory and appended to the session log, but the
* default in settings.json is left untouched. This prevents transient
* model switches (auto-mode dispatches) from silently overwriting the
* user's saved thinking level preference.
*/
private _applyThinkingLevel(level: ThinkingLevel, options?: { persist?: boolean }): void {
const availableLevels = this.getAvailableThinkingLevels();
const effectiveLevel = availableLevels.includes(level) ? level : this._clampThinkingLevel(level, availableLevels);

Expand All @@ -1738,7 +1750,7 @@ export class AgentSession {

if (isChanging) {
this.sessionManager.appendThinkingLevelChange(effectiveLevel);
if (this.supportsThinking() || effectiveLevel !== "off") {
if (options?.persist !== false && (this.supportsThinking() || effectiveLevel !== "off")) {
this.settingsManager.setDefaultThinkingLevel(effectiveLevel);
}
this._emitSessionStateChanged("set_thinking_level");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import test from "node:test";
import assert from "node:assert/strict";
import { readFileSync } from "node:fs";
import { join } from "node:path";

const source = readFileSync(join(process.cwd(), "packages/pi-coding-agent/src/core/extensions/loader.ts"), "utf-8");

test("loader: extension API forwards setModel options to runtime", () => {
const start = source.indexOf("setModel(model, options) {");
assert.ok(start >= 0, "missing ExtensionAPI setModel wrapper");
const window = source.slice(start, start + 140);

assert.ok(
window.includes("return runtime.setModel(model, options);"),
"ExtensionAPI setModel wrapper should forward options such as persist:false to the runtime",
);
});
4 changes: 2 additions & 2 deletions packages/pi-coding-agent/src/core/extensions/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -561,8 +561,8 @@ function createExtensionAPI(
return runtime.getCommands();
},

setModel(model) {
return runtime.setModel(model);
setModel(model, options) {
return runtime.setModel(model, options);
},

getThinkingLevel() {
Expand Down
4 changes: 2 additions & 2 deletions src/resources/extensions/gsd/auto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ export async function stopAuto(
s.originalModelProvider,
s.originalModelId,
);
if (original) await pi.setModel(original);
if (original) await pi.setModel(original, { persist: false });
}
} catch (e) {
debugLog("stop-cleanup-model", { error: e instanceof Error ? e.message : String(e) });
Expand Down Expand Up @@ -1458,7 +1458,7 @@ export async function dispatchHookUnit(
const match = resolveModelId(hookModel, availableModels, ctx.model?.provider);
if (match) {
try {
await pi.setModel(match);
await pi.setModel(match, { persist: false });
} catch (err) {
/* non-fatal */
logWarning("dispatch", `hook model set failed: ${err instanceof Error ? err.message : String(err)}`, { file: "auto.ts" });
Expand Down
36 changes: 34 additions & 2 deletions src/resources/extensions/gsd/tests/model-isolation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@
* and GSD preferences override of settings.json defaults (#3517).
*/

import { describe, it, beforeEach, afterEach } from "node:test";
import { describe, it, test, beforeEach, afterEach } from "node:test";
import assert from "node:assert/strict";
import { mkdirSync, writeFileSync, rmSync, existsSync, readFileSync } from "node:fs";
import { join } from "node:path";
import { join, dirname } from "node:path";
import { fileURLToPath } from "node:url";
import { tmpdir } from "node:os";

const __dirname = dirname(fileURLToPath(import.meta.url));

// ─── Test helpers ─────────────────────────────────────────────────────────────

function makeTmpDir(suffix: string): string {
Expand Down Expand Up @@ -229,3 +232,32 @@ describe("GSD preferences override settings.json for session model (#3517)", ()
"settings.json provider must NOT leak through");
});
});

// ─── Structural guard: no setModel calls in auto.ts should persist ────────────

test("all setModel calls in auto.ts use persist: false", () => {
// Regression test: auto-mode's transient model switches must never persist
// to settings.json. If they do, the user's chosen default model is silently
// overwritten and every subsequent session starts with the wrong model.
// See #650 for the original model-bleed report.
const autoSrc = readFileSync(
join(__dirname, "..", "auto.ts"),
"utf-8",
);

// Find all setModel calls (pi.setModel or await pi.setModel)
const setModelCalls = autoSrc.match(/\.setModel\([^)]*\)/g) ?? [];
assert.ok(
setModelCalls.length > 0,
"Expected at least one setModel call in auto.ts",
);

for (const call of setModelCalls) {
assert.ok(
call.includes("persist: false"),
`setModel call in auto.ts is missing { persist: false }: ${call}\n` +
"Auto-mode model switches are transient — they must not overwrite " +
"the user's saved default in settings.json.",
);
}
});
Loading