fix: preserve Step thinking effort preferences

apps/cli/src/ui/interactive-mode.ts

@@ -95,7 +95,6 @@ import {
 	type SessionEntry,
 	SessionImportFileNotFoundError,
 	SessionManager,
-	STEP_PROVIDER_ID,
 	type StepLoginHost,
 	sessionEntryToContextMessages,
 	setRegisteredThemes,
@@ -143,7 +142,7 @@ import {
 	TuiMainScreen,
 	visibleWidth,
 } from "@step-harness/pi-tui";
-import type { AuthEvent, AuthPrompt, ImageContent } from "@step-harness/providers";
+import { type AuthEvent, type AuthPrompt, clampThinkingLevel, type ImageContent } from "@step-harness/providers";
 import type { AssistantMessage, Message, Model, Usage } from "@step-harness/providers/compat";
 import chalk from "chalk";
 import { spawn } from "child_process";
@@ -4775,12 +4774,11 @@ export class InteractiveMode {
 			};
 			const availableLevels = this.session.getAvailableThinkingLevels();
 			const globalDefault = this.settingsManager.getDefaultThinkingLevel() ?? DEFAULT_THINKING_LEVEL;
-			// Step models default to their highest supported effort, so mark that as
-			// the default rather than the global level (which may not be selectable).
-			const defaultMarker =
-				this.session.model?.provider === STEP_PROVIDER_ID
-					? (availableLevels[availableLevels.length - 1] ?? globalDefault)
-					: globalDefault;
+			const model = this.session.model;
+			const savedDefault = model
+				? (this.settingsManager.getModelThinkingLevel(model.provider, model.id) ?? globalDefault)
+				: globalDefault;
+			const defaultMarker = model ? clampThinkingLevel(model, savedDefault) : savedDefault;
 			const selector = new ThinkingSelectorComponent(
 				this.session.thinkingLevel ?? DEFAULT_THINKING_LEVEL,
 				availableLevels,

apps/cli/test/thinking-default-marker.test.ts

@@ -0,0 +1,60 @@
+import type { ThinkingLevel } from "@step-harness/agent-core";
+import type { Model } from "@step-harness/providers";
+import { beforeAll, describe, expect, it, vi } from "vitest";
+import { SettingsManager } from "../../../packages/coding-agent/src/core/settings-manager.ts";
+import { stepThinkingLevelMap } from "../../../packages/coding-agent/src/features/step-provider/index.ts";
+import { initTheme } from "../../../packages/coding-agent/src/theme/theme.ts";
+import { stripAnsi } from "../../../packages/coding-agent/src/utils/ansi.ts";
+import { stepModel } from "../../../packages/coding-agent/test/utilities.ts";
+import { InteractiveMode } from "../src/ui/interactive-mode.ts";
+import type { ThinkingSelectorComponent } from "../src/ui/view/dialogs/thinking-selector.ts";
+
+const showThinkingSelector = Reflect.get(InteractiveMode.prototype, "showThinkingSelector") as (this: object) => void;
+
+function renderSelector(settingsManager: SettingsManager, model: Model<any>, levels: ThinkingLevel[]): string[] {
+	let selector!: ThinkingSelectorComponent;
+	showThinkingSelector.call({
+		session: { model, thinkingLevel: "high", getAvailableThinkingLevels: () => levels },
+		settingsManager,
+		selectThinkingLevel: vi.fn(),
+		showSelector(factory: (done: () => void) => { component: ThinkingSelectorComponent }) {
+			selector = factory(() => {}).component;
+		},
+	});
+	return selector.render(100).map(stripAnsi);
+}
+
+describe("thinking selector default marker", () => {
+	beforeAll(() => initTheme("dark"));
+
+	it("marks the saved global default for Step models", () => {
+		const lines = renderSelector(
+			SettingsManager.inMemory({ defaultThinkingLevel: "medium" }),
+			stepModel({ thinkingLevelMap: stepThinkingLevelMap(["low", "medium", "high"]) }),
+			["low", "medium", "high"],
+		);
+		expect(lines.some((line) => line.includes("medium") && line.includes("default"))).toBe(true);
+		expect(lines.some((line) => line.includes("high") && line.includes("default"))).toBe(false);
+	});
+
+	it("marks the model-specific default ahead of the global default", () => {
+		const lines = renderSelector(
+			SettingsManager.inMemory({
+				defaultThinkingLevel: "high",
+				modelThinkingLevels: { "step/step-5-preview": "low" },
+			}),
+			stepModel({ thinkingLevelMap: stepThinkingLevelMap(["low", "medium", "high"]) }),
+			["low", "medium", "high"],
+		);
+		expect(lines.some((line) => line.includes("low") && line.includes("default"))).toBe(true);
+	});
+
+	it("marks the supported level used when the saved default must be clamped", () => {
+		const lines = renderSelector(
+			SettingsManager.inMemory({ defaultThinkingLevel: "medium" }),
+			stepModel({ thinkingLevelMap: stepThinkingLevelMap(["low", "high"]) }),
+			["low", "high"],
+		);
+		expect(lines.some((line) => line.includes("high") && line.includes("default"))).toBe(true);
+	});
+});

packages/coding-agent/src/features/step.ts

@@ -1,5 +1,5 @@
 import process from "node:process";
-import { type Api, getSupportedThinkingLevels, type Model } from "@step-harness/providers";
+import { type Api, clampThinkingLevel, type Model } from "@step-harness/providers";
 import type {
 	ExtensionAPI,
 	ExtensionCommandContext,
@@ -54,14 +54,13 @@ export interface StepExtensionOptions {
  * carries `reasoning_effort_support_list` on some profiles (e.g. step_plan); when
  * it does not (e.g. platform), fetch the per-model detail from the domain-root
  * `/v1`. Mutates the model in place — the session's active model is the same
- * object the picker reads. When `applyDefault` is set (fresh activation, not a
- * restore), also default the level to the highest supported effort.
+ * object the picker reads. Keep the session's resolved thinking preference;
+ * capability discovery may only clamp a level the active model cannot support.
  */
 async function enrichStepModelEffort(
 	model: Model<Api> | undefined,
 	ctx: ExtensionContext,
 	pi: ExtensionAPI,
-	applyDefault: boolean,
 ): Promise<void> {
 	try {
 		if (!model || model.provider !== STEP_PROVIDER_ID) return;
@@ -80,11 +79,12 @@ async function enrichStepModelEffort(
 				}
 			}
 		}
-		if (applyDefault) {
-			const supported = getSupportedThinkingLevels(model).filter((level) => level !== "off");
-			const highest = supported[supported.length - 1];
-			if (highest) pi.setThinkingLevel(highest);
-		}
+		// Discovery may finish after a model switch or a user effort change. Only
+		// validate the still-active model, using the latest session preference.
+		if (ctx.model !== model) return;
+		const selected = pi.getThinkingLevel();
+		const supported = clampThinkingLevel(model, selected);
+		if (supported !== selected) pi.setThinkingLevel(supported);
 	} catch {
 		// Best-effort; the model keeps its existing thinking-level defaults.
 	}
@@ -100,12 +100,11 @@ export function createStepExtension(options: StepExtensionOptions = {}): Extensi
 		// Fire-and-forget; failures leave the model's defaults untouched.
 		// (The initial model catalog is refreshed by the launcher before the
 		// session's model is resolved, so it is already available here.)
-		pi.on("session_start", (event, ctx) => {
-			const fresh = event.reason === "startup" || event.reason === "new";
-			void enrichStepModelEffort(ctx.model, ctx, pi, fresh);
+		pi.on("session_start", (_event, ctx) => {
+			void enrichStepModelEffort(ctx.model, ctx, pi);
 		});
 		pi.on("model_select", (event, ctx) => {
-			void enrichStepModelEffort(event.model, ctx, pi, event.source !== "restore");
+			void enrichStepModelEffort(event.model, ctx, pi);
 		});
 		// Declarative plugins (including StepPage) contribute MCP servers. The
 		// bridge is loaded as part of the Step product extension so ordinary Pi

packages/coding-agent/test/step-thinking-preferences.test.ts

@@ -0,0 +1,201 @@
+import { mkdirSync, mkdtempSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import type { ThinkingLevel } from "@step-harness/agent-core";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { parseArgs } from "../src/cli/args.ts";
+import type { AgentSession } from "../src/core/agent-session.ts";
+import { AuthStorage } from "../src/core/auth-storage.ts";
+import type { ExtensionAPI, ExtensionContext } from "../src/core/extensions/types.ts";
+import { createAgentSession } from "../src/core/sdk.ts";
+import { SessionManager } from "../src/core/session-manager.ts";
+import { SettingsManager } from "../src/core/settings-manager.ts";
+import { createStepExtension } from "../src/features/step.ts";
+import { stepThinkingLevelMap } from "../src/features/step-provider/index.ts";
+import { createStepSettingsManager } from "../src/step/settings-manager.ts";
+import { createInMemoryModelRegistry, getModelRuntime } from "./model-runtime-test-utils.ts";
+import { createTestResourceLoader, stepModel } from "./utilities.ts";
+
+const roots: string[] = [];
+const sessions: AgentSession[] = [];
+function fixture() {
+	const root = mkdtempSync(join(tmpdir(), "step-thinking-preferences-"));
+	roots.push(root);
+	const cwd = join(root, "project");
+	const agentDir = join(root, "agent");
+	mkdirSync(cwd, { recursive: true });
+	mkdirSync(agentDir, { recursive: true });
+	return { root, cwd, agentDir };
+}
+async function makeSession(f: ReturnType<typeof fixture>, settings: SettingsManager, thinkingLevel?: ThinkingLevel) {
+	const registry = await createInMemoryModelRegistry(AuthStorage.inMemory());
+	const result = await createAgentSession({
+		cwd: f.cwd,
+		agentDir: f.agentDir,
+		settingsManager: settings,
+		sessionManager: SessionManager.inMemory(f.cwd),
+		resourceLoader: createTestResourceLoader(),
+		modelRuntime: getModelRuntime(registry),
+		model: stepModel({ thinkingLevelMap: stepThinkingLevelMap(["low", "medium", "high"]) }),
+		thinkingLevel,
+		noTools: "all",
+	});
+	sessions.push(result.session);
+	return result.session;
+}
+function stepEffortHooks(session: AgentSession) {
+	const on = vi.fn();
+	createStepExtension({ permission: { env: {} } })({
+		registerProvider: vi.fn(),
+		registerCommand: vi.fn(),
+		sendUserMessage: vi.fn(),
+		on,
+		setThinkingLevel: (level: ThinkingLevel) => session.setThinkingLevel(level),
+		getThinkingLevel: () => session.thinkingLevel,
+	} as unknown as ExtensionAPI);
+	const ctx = {
+		get model() {
+			return session.model;
+		},
+		get thinkingLevel() {
+			return session.thinkingLevel;
+		},
+		modelRegistry: {
+			getApiKeyAndHeaders: async () => ({ ok: true, apiKey: "test-key" }),
+		},
+	} as unknown as ExtensionContext;
+	return {
+		async start(reason = "startup") {
+			// The first registered session_start handler is the real effort enrichment hook.
+			await on.mock.calls.find(([name]) => name === "session_start")![1]({ type: "session_start", reason }, ctx);
+		},
+		async select() {
+			await on.mock.calls.find(([name]) => name === "model_select")![1](
+				{ type: "model_select", model: session.model, source: "set" },
+				ctx,
+			);
+		},
+	};
+}
+afterEach(() => {
+	vi.restoreAllMocks();
+	for (const session of sessions.splice(0)) session.dispose();
+	for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true });
+});
+
+describe("Step thinking preferences", () => {
+	it("CLI parsing and the core SDK honor --thinking medium before the Step hook", async () => {
+		const f = fixture();
+		const parsed = parseArgs(["--thinking", "medium"]);
+		expect(parsed.thinking).toBe("medium");
+		const session = await makeSession(f, SettingsManager.inMemory({ defaultThinkingLevel: "high" }), parsed.thinking);
+		expect(session.thinkingLevel).toBe("medium");
+	});
+
+	it("Step startup must preserve the explicit --thinking medium option", async () => {
+		const f = fixture();
+		const parsed = parseArgs(["--thinking", "medium"]);
+		const session = await makeSession(f, SettingsManager.inMemory({ defaultThinkingLevel: "high" }), parsed.thinking);
+		expect(session.thinkingLevel).toBe("medium");
+		await stepEffortHooks(session).start();
+		expect(session.thinkingLevel).toBe("medium");
+	});
+
+	it("a saved default survives disk roundtrip and must still apply after startup", async () => {
+		const f = fixture();
+		const saved = createStepSettingsManager(f.cwd, f.agentDir);
+		saved.setDefaultThinkingLevel("medium");
+		await saved.flush();
+		const reloaded = createStepSettingsManager(f.cwd, f.agentDir);
+		expect(reloaded.getDefaultThinkingLevel()).toBe("medium");
+		const session = await makeSession(f, reloaded);
+		expect(session.thinkingLevel).toBe("medium");
+		await stepEffortHooks(session).start();
+		expect(reloaded.getDefaultThinkingLevel()).toBe("medium");
+		expect(session.thinkingLevel).toBe("medium");
+	});
+
+	it("Step startup must preserve a saved per-model thinking preference", async () => {
+		const f = fixture();
+		const saved = createStepSettingsManager(f.cwd, f.agentDir);
+		saved.setDefaultThinkingLevel("high");
+		saved.setModelThinkingLevel("step", "step-5-preview", "low");
+		await saved.flush();
+		const reloaded = createStepSettingsManager(f.cwd, f.agentDir);
+		const session = await makeSession(f, reloaded);
+		expect(session.thinkingLevel).toBe("low");
+		await stepEffortHooks(session).start();
+		expect(session.thinkingLevel).toBe("low");
+	});
+
+	it("model_select must preserve the effort already resolved by the session", async () => {
+		const f = fixture();
+		const session = await makeSession(f, SettingsManager.inMemory(), "medium");
+		await stepEffortHooks(session).select();
+		expect(session.thinkingLevel).toBe("medium");
+	});
+
+	it("an explicit resume lifecycle does not trigger the override", async () => {
+		const f = fixture();
+		const session = await makeSession(f, SettingsManager.inMemory(), "medium");
+		await stepEffortHooks(session).start("resume");
+		expect(session.thinkingLevel).toBe("medium");
+	});
+
+	it("preserves a newer user choice while capability discovery is pending", async () => {
+		const f = fixture();
+		const session = await makeSession(f, SettingsManager.inMemory(), "medium");
+		const model = session.model!;
+		delete model.thinkingLevelMap;
+		let respond!: (response: Response) => void;
+		const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation(
+			() =>
+				new Promise((resolve) => {
+					respond = resolve;
+				}),
+		);
+		await stepEffortHooks(session).start();
+		await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledOnce());
+		session.setThinkingLevel("low");
+		respond(new Response(JSON.stringify({ reasoning_effort_support_list: ["low", "medium", "high"] })));
+		await vi.waitFor(() => expect(model.thinkingLevelMap).toBeDefined());
+		expect(session.thinkingLevel).toBe("low");
+	});
+
+	it("does not change the active model's effort when an earlier model's discovery finishes", async () => {
+		const f = fixture();
+		const session = await makeSession(f, SettingsManager.inMemory(), "medium");
+		const previousModel = session.model!;
+		delete previousModel.thinkingLevelMap;
+		let respond!: (response: Response) => void;
+		const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation(
+			() =>
+				new Promise((resolve) => {
+					respond = resolve;
+				}),
+		);
+		await stepEffortHooks(session).start();
+		await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledOnce());
+		session.agent.state.model = stepModel({
+			id: "other-model",
+			thinkingLevelMap: stepThinkingLevelMap(["low", "medium"]),
+		});
+		session.setThinkingLevel("low");
+		respond(new Response(JSON.stringify({ reasoning_effort_support_list: ["high"] })));
+		await vi.waitFor(() => expect(previousModel.thinkingLevelMap).toBeDefined());
+		expect(session.thinkingLevel).toBe("low");
+	});
+
+	it("clamps only unsupported choices after discovering the active model's capabilities", async () => {
+		const f = fixture();
+		const session = await makeSession(f, SettingsManager.inMemory(), "medium");
+		const model = session.model!;
+		delete model.thinkingLevelMap;
+		vi.spyOn(globalThis, "fetch").mockResolvedValue(
+			new Response(JSON.stringify({ reasoning_effort_support_list: ["low", "high"] })),
+		);
+		await stepEffortHooks(session).start();
+		await vi.waitFor(() => expect(model.thinkingLevelMap).toBeDefined());
+		expect(session.thinkingLevel).toBe("high");
+	});
+});