Repository navigation
fix(sessions): switch the client when /resume changes provider or model - #1902
diyaavirmani wants to merge 1 commit into
Conversation
applySession set the provider and model labels to the resumed session's values but never created a client for them, so after an in-app /resume or a pick in the session picker the next turns were still served by the previous model while the UI, the saved session and /stats recorded them under the resumed one. Startup --resume only looked right because init creates the client afterwards and overwrites the labels. applySession now leaves provider and model alone. The in-app paths go through a new resumeSession that applies the conversation and then calls the same programmatic switch the mode toggle uses (handleModelSelect(provider, model, true)). That covers a new provider, a new model on the same provider, and a provider that is no longer configured, where handleModelSelect reports the error and the current client and labels stay. The conversation is applied first, so autosave never pairs the old messages with the new labels in between. handleModelSelect takes the history to size-check as an optional fourth argument. /resume passes the resumed messages; without it the overflow warning measured the conversation being left behind and could tell the user to /clear a session that fits. /resume <id> now awaits the switch, so the command only completes, and a queued prompt only starts, once the new client is in place. Startup --resume is unchanged. Closes Nano-Collective#1856.
nc-review: comments — 1 nit@diyaavirmani — a few things worth a look, none blocking. The change correctly routes the in-app ⚪ nit · The same-provider branch of if (sameProvider) {
if (client) {
client.setModel(selectedModel);
setCurrentModel(selectedModel);
The different-provider branch does not have this problem: Worth a guard in the same-provider branch — e.g. check 🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with |
Description
Closes #1856.
applySessionset the provider and model labels to the resumed session's values but never created a client for them. After an in-app/resume <id>, or a pick in the session picker, the next turns were still served by the previous model, while the UI, the saved session and/statsrecorded them under the resumed one. Startup--resumeonly looked right because init creates the client afterwards and overwrites the labels.As suggested on the issue, the in-app paths now go through the existing programmatic switch,
handleModelSelect(provider, model, true), the one the mode toggle uses.Change:
useAppHandlers.tsx:applySessionno longer sets the provider or model. Whoever owns the client sets them: init at startup, or the switch below.resumeSessionapplies the conversation, then callshandleModelSelect(session.provider, session.model, true, session.messages)./resume <id>and the session picker both use it.useModeHandlers.tsx:handleModelSelecttakes the history to size-check as an optional fourth argument (default: the current messages, so other callers behave as before). Without it, resuming a small session from a long conversation could warn "History exceeds model-b's window. Run /compact or /clear", which measured the conversation being left behind, and/clearwould wipe the session just resumed.session-handler.ts:/resume <id>awaits the switch, so the command only completes, and a queued prompt only starts, once the new client is in place.What
handleModelSelectalready handles:setModelon the live clientStartup
--continue/--resume <id>is unchanged, since a switch there would race client init. The docs (session-management.md) now say which paths switch and which keep the startup provider.Type of Change
Changeset
pnpm changeset) describing this change for the changelogTesting
Automated Tests
.spec.ts/tsxfilespnpm test:allcompletes successfully)New tests:
useAppHandlers.spec.tsx:handleModelSelectwith the session's provider, model,trueand its messages, after the conversation is applied/resume <id>does the same, and doesn't signal completion until the switch resolvesuseModeHandlers.spec.tsx:pnpm run test:allpasses on macOS with Node 22.23.3: format, lint, types, 9,834 AVA tests, knip and audit. Semgrep isn't installed here, so the script skipped that step.The issue's "same provider and model creates no client" case is the existing
handleModelSelect short-circuits when provider and model are unchangedtest.With the source changes reverted, the three resume tests and the history test fail. The two provider tests pass either way: they pin down the
handleModelSelectbehaviour that resume now relies on.Manual Testing
End to end through the real interactive TUI on macOS with Node 22.23.3, driven through a pseudo-terminal:
provA(modelsmodel-a,model-a2) always answers "answer served by provider A".provB(model-b) always answers "answer served by provider B".--provider provA --model model-a, resume a saved session, send one prompt.modelin its body. Also checked: the label on the reply, the provider/model autosave wrote to the session file, and the/statsledger.f5d821796111c064/resume <id>of aprovB/model-bsessionmodel-b; statsprovB/model-bmodel-b; session and statsprovB/model-bmodel-b); label, session and statsprovB/model-bmodel-b; statsprovB/model-bmodel-b; session and statsprovB/model-bmodel-b); label, session and statsprovB/model-b/resume <id>of aprovGone/model-xsession (provider not configured)model-x; statsprovGone/model-xmodel-x; session and statsprovGone/model-xprovA/model-a/resume <id>of aprovA/model-a2session (same provider)model-a, labelledmodel-a2; statsprovA/model-a2model-a, labelledmodel-a2; session and statsprovA/model-a2model-a2; label, session and statsprovA/model-a2nanocoder --resume <id>of theprovBsessionprovA/model-aNotes on the run:
/exit(the session file still holds the original 2 messages), so its saved-session result isn't evidence either way. Main and the release PR do save it.Checklist
handleModelSelect's existing error message)