Skip to content

Commit 92d4986

Browse files
committed
fix(resume): keep stored session model unless flags override
Resumed sessions lose their stored provider:model to the launch default on both resume paths. The stored identity now restores unless --provider or --model was passed; the override rides a parse-time flag, never a value comparison.
1 parent 83c2aed commit 92d4986

3 files changed

Lines changed: 263 additions & 0 deletions

File tree

‎src/config/index.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,13 @@ export interface Config {
650650
* id; `"pick"` opens the interactive picker. Omitted for a fresh session.
651651
*/
652652
resumeMode?: "id" | "pick";
653+
/**
654+
* True when this invocation passed --provider and/or --model. Resume keeps
655+
* the stored session's model unless this is set; the override is a
656+
* parse-time fact, never inferred by comparing launch values against the
657+
* stored record.
658+
*/
659+
modelOverride?: boolean;
653660

654661
// Deprecated no-op retained for CLI compatibility.
655662
noWorkflow: boolean;
@@ -1181,6 +1188,9 @@ export async function loadConfig(
11811188
}
11821189
: {}),
11831190
...(resumePicker ? { resumePicker: true } : {}),
1191+
...(provider !== undefined || model !== undefined
1192+
? { modelOverride: true as const }
1193+
: {}),
11841194
...(settings?.defaultProvider !== undefined
11851195
? { globalDefaultProvider: settings.defaultProvider }
11861196
: {}),

‎src/tui/session-start.test.ts‎

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ import { describe, expect, test } from "bun:test";
22

33
import { withMockedModuleDuring } from "../../tests/helpers/mock-module.js";
44
import { setActiveRun, clearActiveRun } from "../session/active-run.js";
5+
import type { RunStateHandle } from "../session/active-run.js";
6+
import type { Config } from "../config/index.js";
57
import type { RunState } from "../session/state.js";
8+
import type { Telemetry } from "../telemetry/index.js";
69

710
describe("createTUICrashGuard", () => {
811
test("finalizeOnCrash writes live session id and provider:model after bindLiveSession", async () => {
@@ -99,3 +102,218 @@ describe("createTUICrashGuard", () => {
99102
expect(ran).toBe(true);
100103
});
101104
});
105+
106+
interface CapturedSave {
107+
cwd: string;
108+
sessionId: string;
109+
state: RunState;
110+
}
111+
112+
function storedRunState(overrides: Partial<RunState> = {}): RunState {
113+
return {
114+
status: "running",
115+
turnsUsed: 7,
116+
task: "stored task",
117+
startedAt: 111,
118+
model: "stored-p:stored-m",
119+
...overrides,
120+
};
121+
}
122+
123+
function launchConfig(overrides: Partial<Config> = {}): Config {
124+
return {
125+
configured: true,
126+
apiKey: "key",
127+
baseURL: "https://example.test",
128+
model: "launch-m",
129+
providerName: "launch-p",
130+
cwd: "/cwd",
131+
task: "",
132+
dangerouslySkipPermissions: false,
133+
anthropicCachePrompt: false,
134+
skipPermissionsFromSettings: false,
135+
auto: true,
136+
command: "tui",
137+
globalSettingsPath: "/settings",
138+
providers: [],
139+
mcpServerEntries: [],
140+
sessionId: "launch-session",
141+
noWorkflow: false,
142+
...overrides,
143+
} as Config;
144+
}
145+
146+
async function runPrepareTUISession(
147+
config: Config,
148+
opts: { pickSessionId?: string; stored?: RunState | null },
149+
): Promise<{
150+
prepared: Awaited<
151+
ReturnType<typeof import("./session-start.js").prepareTUISession>
152+
>;
153+
saves: CapturedSave[];
154+
activeRuns: RunStateHandle[];
155+
}> {
156+
const saves: CapturedSave[] = [];
157+
const activeRuns: RunStateHandle[] = [];
158+
const stored = opts.stored === undefined ? storedRunState() : opts.stored;
159+
const prepared = await withMockedModuleDuring(
160+
import.meta.resolve("../session/assemble-runtime.js"),
161+
(real: typeof import("../session/assemble-runtime.js")) => ({
162+
...real,
163+
assembleInferenceBase: async () => ({}),
164+
assembleSessionTrust: async () => ({}),
165+
}),
166+
async () =>
167+
withMockedModuleDuring(
168+
import.meta.resolve("./pick-session.js"),
169+
(real: typeof import("./pick-session.js")) => ({
170+
...real,
171+
pickSession: async () =>
172+
opts.pickSessionId === undefined
173+
? null
174+
: {
175+
sessionId: opts.pickSessionId,
176+
task: "picked task",
177+
startedAt: 1,
178+
updatedAt: 2,
179+
status: "running" as const,
180+
},
181+
}),
182+
async () =>
183+
withMockedModuleDuring(
184+
import.meta.resolve("../session/state.js"),
185+
(real: typeof import("../session/state.js")) => ({
186+
...real,
187+
loadState: async () =>
188+
stored === null
189+
? { kind: "missing" as const }
190+
: { kind: "ok" as const, state: stored },
191+
saveState: async (
192+
cwd: string,
193+
sessionId: string,
194+
state: RunState,
195+
) => {
196+
saves.push({ cwd, sessionId, state });
197+
},
198+
}),
199+
async () =>
200+
withMockedModuleDuring(
201+
import.meta.resolve("../session/index.js"),
202+
(real: typeof import("../session/index.js")) => ({
203+
...real,
204+
initSessionDir: async () => "/dir",
205+
sessionContextDir: () => "/workdir",
206+
}),
207+
async () =>
208+
withMockedModuleDuring(
209+
import.meta.resolve("../session/active-run.js"),
210+
(real: typeof import("../session/active-run.js")) => ({
211+
...real,
212+
setActiveRun: (handle: RunStateHandle) => {
213+
activeRuns.push(handle);
214+
},
215+
}),
216+
async () => {
217+
const { prepareTUISession } =
218+
await import("./session-start.js");
219+
return prepareTUISession(config, {} as Telemetry);
220+
},
221+
),
222+
),
223+
),
224+
),
225+
);
226+
return { prepared, saves, activeRuns };
227+
}
228+
229+
describe("prepareTUISession resume model", () => {
230+
test("picker resume without flags restores the stored provider:model", async () => {
231+
const { prepared, saves } = await runPrepareTUISession(
232+
launchConfig({ resumePicker: true }),
233+
{ pickSessionId: "picked-session" },
234+
);
235+
236+
expect(prepared?.config.providerName).toBe("stored-p");
237+
expect(prepared?.config.model).toBe("stored-m");
238+
expect(saves).toHaveLength(1);
239+
expect(saves[0]?.state.model).toBe("stored-p:stored-m");
240+
});
241+
242+
test("id resume without flags restores the stored provider:model", async () => {
243+
const { prepared, saves, activeRuns } = await runPrepareTUISession(
244+
launchConfig({ resumeMode: "id", sessionId: "resume-id" }),
245+
{},
246+
);
247+
248+
expect(prepared?.config.providerName).toBe("stored-p");
249+
expect(prepared?.config.model).toBe("stored-m");
250+
expect(prepared?.resumeSeed.storedModel).toEqual({
251+
providerName: "stored-p",
252+
model: "stored-m",
253+
});
254+
expect(saves).toHaveLength(1);
255+
expect(saves[0]?.sessionId).toBe("resume-id");
256+
expect(saves[0]?.state.model).toBe("stored-p:stored-m");
257+
expect(activeRuns).toHaveLength(1);
258+
expect(activeRuns[0]?.model).toBe("stored-p:stored-m");
259+
});
260+
261+
test("explicit flags win over the stored model on both resume branches", async () => {
262+
for (const config of [
263+
launchConfig({ resumePicker: true, modelOverride: true }),
264+
launchConfig({
265+
resumeMode: "id",
266+
sessionId: "resume-id",
267+
modelOverride: true,
268+
}),
269+
]) {
270+
const { prepared, saves } = await runPrepareTUISession(config, {
271+
pickSessionId: "picked-session",
272+
});
273+
274+
expect(prepared?.config.providerName).toBe("launch-p");
275+
expect(prepared?.config.model).toBe("launch-m");
276+
expect(saves).toHaveLength(1);
277+
expect(saves[0]?.state.model).toBe("launch-p:launch-m");
278+
}
279+
});
280+
281+
test("legacy model-less records keep the launch default", async () => {
282+
const { model: _dropped, ...legacy } = storedRunState();
283+
const { prepared, saves } = await runPrepareTUISession(
284+
launchConfig({ resumePicker: true }),
285+
{ pickSessionId: "picked-session", stored: legacy },
286+
);
287+
288+
expect(prepared?.config.providerName).toBe("launch-p");
289+
expect(prepared?.config.model).toBe("launch-m");
290+
expect(prepared?.resumeSeed.storedModel).toBeUndefined();
291+
expect(saves).toHaveLength(1);
292+
expect(saves[0]?.state.model).toBe("launch-p:launch-m");
293+
});
294+
295+
test("resolveResumeSeed carries the stored model and tolerates malformed values", async () => {
296+
const { resolveResumeSeed } = await import("./session-start.js");
297+
298+
expect(resolveResumeSeed(null)).toEqual({
299+
turnsUsed: 0,
300+
mcpServers: [],
301+
activatedTools: [],
302+
});
303+
expect(resolveResumeSeed(storedRunState()).storedModel).toEqual({
304+
providerName: "stored-p",
305+
model: "stored-m",
306+
});
307+
expect(
308+
resolveResumeSeed(storedRunState({ model: "stored-p:org:model-v2" }))
309+
.storedModel,
310+
).toEqual({ providerName: "stored-p", model: "org:model-v2" });
311+
for (const model of ["nocolon", ":empty-provider", "provider:", ""]) {
312+
expect(
313+
resolveResumeSeed(storedRunState({ model })).storedModel,
314+
).toBeUndefined();
315+
}
316+
const { model: _dropped, ...legacy } = storedRunState();
317+
expect(resolveResumeSeed(legacy).storedModel).toBeUndefined();
318+
});
319+
});

‎src/tui/session-start.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ export interface ResumeSeed {
5050
// Present only when the prior run stamped an Anthropic-protocol cache write.
5151
lastCacheWriteAt?: number;
5252
cacheWriteModel?: string;
53+
// The prior run's resolved provider:model, split for restore. Absent for a
54+
// fresh run and for legacy records that predate the model field.
55+
storedModel?: { providerName: string; model: string };
5356
}
5457

5558
const FRESH_RESUME_SEED: ResumeSeed = {
@@ -58,6 +61,23 @@ const FRESH_RESUME_SEED: ResumeSeed = {
5861
activatedTools: [],
5962
};
6063

64+
/**
65+
* Split a run.json `provider:model` identity on its first colon so model ids
66+
* containing colons survive. Absent or malformed values yield undefined and
67+
* the caller keeps the launch default; this never throws.
68+
*/
69+
function splitStoredModel(
70+
value: string | undefined,
71+
): { providerName: string; model: string } | undefined {
72+
if (value === undefined) return undefined;
73+
const colon = value.indexOf(":");
74+
if (colon <= 0 || colon === value.length - 1) return undefined;
75+
return {
76+
providerName: value.slice(0, colon),
77+
model: value.slice(colon + 1),
78+
};
79+
}
80+
6181
/**
6282
* Fold a resumed session's run.json into a concrete seed once, at the
6383
* resume boundary, so every downstream reader (the run sink, the
@@ -68,10 +88,12 @@ const FRESH_RESUME_SEED: ResumeSeed = {
6888
*/
6989
export function resolveResumeSeed(pickedState: RunState | null): ResumeSeed {
7090
if (pickedState === null) return FRESH_RESUME_SEED;
91+
const storedModel = splitStoredModel(pickedState.model);
7192
return {
7293
turnsUsed: pickedState.turnsUsed,
7394
mcpServers: pickedState.mcpServers ?? [],
7495
activatedTools: pickedState.activatedTools ?? [],
96+
...(storedModel !== undefined ? { storedModel } : {}),
7597
...(pickedState.lastCacheWriteAt !== undefined
7698
? {
7799
lastCacheWriteAt: pickedState.lastCacheWriteAt,
@@ -290,6 +312,19 @@ export async function prepareTUISession(
290312
}
291313
}
292314

315+
// A resume without explicit --provider/--model keeps the stored session's
316+
// model: the launch default would otherwise clobber it on both resume
317+
// branches above. An explicit flag wins, so the restore is gated on the
318+
// parse-time signal rather than any value comparison. Fresh runs and
319+
// legacy model-less records carry no storedModel and keep the default.
320+
if (config.modelOverride !== true && resumeSeed.storedModel !== undefined) {
321+
config = {
322+
...config,
323+
providerName: resumeSeed.storedModel.providerName,
324+
model: resumeSeed.storedModel.model,
325+
};
326+
}
327+
293328
const workdir = sessionContextDir(config.cwd, sessionId);
294329
await initSessionDir(config.cwd, sessionId);
295330

0 commit comments

Comments
 (0)