From 9a5235e1408a5cf19d366ed71bcc639ebc464d96 Mon Sep 17 00:00:00 2001 From: deseltrus Date: Sat, 4 Apr 2026 07:21:25 +0200 Subject: [PATCH 1/5] fix: pass persist option through extension API setModel wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ExtensionAPI.setModel wrapper in loader.ts accepts only the model parameter, silently dropping the options argument. This means every pi.setModel(model, { persist: false }) call from any extension becomes runtime.setModel(model) — the persist guard is lost and transient model switches always overwrite the user's saved default in settings.json. --- packages/pi-coding-agent/src/core/extensions/loader.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/pi-coding-agent/src/core/extensions/loader.ts b/packages/pi-coding-agent/src/core/extensions/loader.ts index 016f05448a..7bd2a6f75f 100644 --- a/packages/pi-coding-agent/src/core/extensions/loader.ts +++ b/packages/pi-coding-agent/src/core/extensions/loader.ts @@ -567,8 +567,8 @@ function createExtensionAPI( return runtime.getCommands(); }, - setModel(model) { - return runtime.setModel(model); + setModel(model, options) { + return runtime.setModel(model, options); }, getThinkingLevel() { From 5bee2bd59bedff2fb824218e3d28aa1428cad8d6 Mon Sep 17 00:00:00 2001 From: deseltrus Date: Sat, 4 Apr 2026 06:50:17 +0200 Subject: [PATCH 2/5] fix: prevent transient model switches from persisting thinking level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _applyModelChange() calls setThinkingLevel() unconditionally, which always writes the effective thinking level to settings.json. When auto-mode dispatches use setModel({ persist: false }), the model default is correctly protected, but the thinking level default is not. If a transient model switch triggers thinking level clamping (e.g. xhigh → high), the clamped level is persisted and every subsequent session starts with the wrong thinking level. Extract _applyThinkingLevel(level, options?) as a private method that respects the persist option from _applyModelChange. The public setThinkingLevel(level) API is unchanged — it delegates to _applyThinkingLevel without options, preserving the existing persist behavior for user-initiated changes. --- .../pi-coding-agent/src/core/agent-session.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/pi-coding-agent/src/core/agent-session.ts b/packages/pi-coding-agent/src/core/agent-session.ts index 33ae7ba8b4..df2bfd128e 100644 --- a/packages/pi-coding-agent/src/core/agent-session.ts +++ b/packages/pi-coding-agent/src/core/agent-session.ts @@ -1660,7 +1660,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"); } @@ -1746,6 +1746,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); @@ -1756,7 +1768,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"); From 1310c0ef75f9d4d93f95c05df8f378e54a818d75 Mon Sep 17 00:00:00 2001 From: deseltrus Date: Sat, 4 Apr 2026 06:42:46 +0200 Subject: [PATCH 3/5] fix(gsd): prevent auto-mode setModel calls from persisting to settings.json Two setModel() calls in auto.ts omit { persist: false }, causing transient model switches during auto-mode to silently overwrite the user's saved default model in settings.json. Every subsequent session then starts with the wrong model. Adds a structural regression test that fails if any setModel call in auto.ts is missing persist: false. --- src/resources/extensions/gsd/auto.ts | 4 +-- .../gsd/tests/model-isolation.test.ts | 35 +++++++++++++++++-- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/resources/extensions/gsd/auto.ts b/src/resources/extensions/gsd/auto.ts index aa42dc5453..e5b89abef3 100644 --- a/src/resources/extensions/gsd/auto.ts +++ b/src/resources/extensions/gsd/auto.ts @@ -939,7 +939,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) }); @@ -1705,7 +1705,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" }); diff --git a/src/resources/extensions/gsd/tests/model-isolation.test.ts b/src/resources/extensions/gsd/tests/model-isolation.test.ts index 74950e1a36..4b40c9656e 100644 --- a/src/resources/extensions/gsd/tests/model-isolation.test.ts +++ b/src/resources/extensions/gsd/tests/model-isolation.test.ts @@ -5,12 +5,15 @@ * provider precedence over PREFERENCES.md when set via `/gsd model` (#4122). */ -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 { @@ -303,3 +306,31 @@ describe("custom provider session model overrides PREFERENCES.md (#4122)", () => }); }); +// ─── 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.", + ); + } +}); From 4771825c76fc1b572c321c54daf9305f1f6d7d6c Mon Sep 17 00:00:00 2001 From: deseltrus Date: Sun, 5 Apr 2026 06:56:45 +0200 Subject: [PATCH 4/5] test: cover transient setModel persistence seams Add targeted structural regressions for the two persistence leaks behind the auto-mode settings overwrite bug. The new tests pin the ExtensionAPI setModel wrapper to forward options and assert that transient model switches do not persist thinking-level defaults when persist:false is used. --- .../core/agent-session-model-switch.test.ts | 22 +++++++++++++++++++ .../extensions/loader-model-options.test.ts | 17 ++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 packages/pi-coding-agent/src/core/extensions/loader-model-options.test.ts diff --git a/packages/pi-coding-agent/src/core/agent-session-model-switch.test.ts b/packages/pi-coding-agent/src/core/agent-session-model-switch.test.ts index f86dac6cab..d80959a53c 100644 --- a/packages/pi-coding-agent/src/core/agent-session-model-switch.test.ts +++ b/packages/pi-coding-agent/src/core/agent-session-model-switch.test.ts @@ -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", + ); +}); diff --git a/packages/pi-coding-agent/src/core/extensions/loader-model-options.test.ts b/packages/pi-coding-agent/src/core/extensions/loader-model-options.test.ts new file mode 100644 index 0000000000..c7afb25426 --- /dev/null +++ b/packages/pi-coding-agent/src/core/extensions/loader-model-options.test.ts @@ -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", + ); +}); From ebe61b7fc5c8205f84ba3ca94a6c9734a587aa0c Mon Sep 17 00:00:00 2001 From: deseltrus Date: Sun, 5 Apr 2026 18:18:45 +0200 Subject: [PATCH 5/5] fix: gate thinking-level session history entry behind persist option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback: appendThinkingLevelChange() was recording durable entries even for transient (persist:false) model switches. On session resume, replay would call the public setThinkingLevel() which always persists — silently overwriting the user's saved default. Move appendThinkingLevelChange inside the persist guard so transient thinking-level changes leave no trace in session history. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../core/agent-session-model-switch.test.ts | 29 +++++++++++++++++-- .../pi-coding-agent/src/core/agent-session.ts | 12 ++++++-- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/packages/pi-coding-agent/src/core/agent-session-model-switch.test.ts b/packages/pi-coding-agent/src/core/agent-session-model-switch.test.ts index d80959a53c..adcafd396d 100644 --- a/packages/pi-coding-agent/src/core/agent-session-model-switch.test.ts +++ b/packages/pi-coding-agent/src/core/agent-session-model-switch.test.ts @@ -32,12 +32,37 @@ test("agent-session: transient model switches pass persist options into thinking }); 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 + 1200); + + assert.ok( + window.includes("if (options?.persist !== false)"), + "_applyThinkingLevel should gate all persistence (settings AND session history) behind persist check", + ); + assert.ok( + window.includes("this.settingsManager.setDefaultThinkingLevel(effectiveLevel)"), + "settings persistence should be inside the persist guard", + ); +}); + +test("agent-session: transient thinking-level changes do not leak into session history (#3486 review)", () => { + // Verifies jeremymcs' review finding: appendThinkingLevelChange must NOT run + // when persist:false, because session resume replays these entries via the + // public setThinkingLevel() which always persists — defeating the isolation. const start = source.indexOf("private _applyThinkingLevel("); assert.ok(start >= 0, "missing _applyThinkingLevel"); const window = source.slice(start, start + 900); + // appendThinkingLevelChange must be INSIDE the persist guard, not before it + const persistGuardIdx = window.indexOf("if (options?.persist !== false)"); + const appendIdx = window.indexOf("this.sessionManager.appendThinkingLevelChange(effectiveLevel)"); + assert.ok(persistGuardIdx >= 0, "missing persist guard in _applyThinkingLevel"); + assert.ok(appendIdx >= 0, "missing appendThinkingLevelChange call"); 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", + appendIdx > persistGuardIdx, + "appendThinkingLevelChange must be inside the persist:false guard — " + + "otherwise transient thinking-level changes survive in session history " + + "and get replayed on resume via setThinkingLevel() which always persists", ); }); diff --git a/packages/pi-coding-agent/src/core/agent-session.ts b/packages/pi-coding-agent/src/core/agent-session.ts index df2bfd128e..6525df86b9 100644 --- a/packages/pi-coding-agent/src/core/agent-session.ts +++ b/packages/pi-coding-agent/src/core/agent-session.ts @@ -1767,9 +1767,15 @@ export class AgentSession { this.agent.setThinkingLevel(effectiveLevel); if (isChanging) { - this.sessionManager.appendThinkingLevelChange(effectiveLevel); - if (options?.persist !== false && (this.supportsThinking() || effectiveLevel !== "off")) { - this.settingsManager.setDefaultThinkingLevel(effectiveLevel); + // Only record in session history when persisting — transient switches + // (auto-mode dispatches with persist:false) must not leave durable + // thinking_level_change entries, because session resume replays them + // via the public setThinkingLevel() path which always persists. + if (options?.persist !== false) { + this.sessionManager.appendThinkingLevelChange(effectiveLevel); + if (this.supportsThinking() || effectiveLevel !== "off") { + this.settingsManager.setDefaultThinkingLevel(effectiveLevel); + } } this._emitSessionStateChanged("set_thinking_level"); }