chat: stop reassigning a live session to fable
three defects, one shape: the code has two names for "which session is this", and every guard checked the one no caller uses. `fable` is the FIRST entry in the claude-code catalogue, so it is what ModelSelector picks whenever it decides a conversation needs a default. On a key without fable entitlement that lands as a 429, after a real mid-session switch — the reported header read OPUSPLAN → FABLE. 1. the settings default was applied to RESUMED chats. the guard was `!initialSessionId`, and no chat caller passes that when resuming — the id arrives as `options.resumeSessionId`, and ChatPanelWrapper goes as far as `resume ? undefined : sessionKey`. so a user's default model was written over sessions that already had one. both names are checked now. 2. `hasStarted` was `!!preloadedMessages?.length`, which is FALSE for the first frames of a resumed chat — before its messages arrive. ModelSelector reads "not started" as permission to pick a default, so a reload reassigned live sessions during that window. a session id is known on the first render and a chat that has one is by definition not new. 3. harness lock. `crossesHarness` returned false when the current harness was unknown, which reads as "nothing crosses" and is the opposite of what is known — with no current harness, everything might. so a started session offered a switch to opencode while its model was still resolving, a switch that cannot work: it hands a `ses_…` id to `claude --resume` and dispatches to the wrong sidecar. unknown is not permission; the strip locks until the answer arrives. the settings themselves were never wrong — chat.defaultModel reads claude-code/opus on the affected server. nothing was ever reading it for the sessions that got reassigned.
This commit is contained in:
Binary file not shown.
|
After Width: | Height: | Size: 15 KiB |
Binary file not shown.
|
After Width: | Height: | Size: 118 KiB |
Binary file not shown.
|
After Width: | Height: | Size: 12 KiB |
@@ -1,4 +1,4 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { describe, expect, mock, test } from 'bun:test';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||
import type { ModelOption } from 'state/useModels';
|
||||
@@ -127,3 +127,30 @@ describe('ModelSelector display of an unknown model', () => {
|
||||
expect(screen.queryByTitle('claude-code/opus')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('a live session whose model has not resolved yet', () => {
|
||||
// Reported 2026-08-19 from a fresh server: a chat running `opusplan` came back as `fable` after a
|
||||
// reload, and the header showed a real mid-session switch (`OPUSPLAN → FABLE`) followed by a 429 on a
|
||||
// key with no fable entitlement. `fable` is simply the FIRST entry in the claude-code catalogue, so it
|
||||
// is whatever this component picks when it decides a conversation needs a default.
|
||||
test('does not invent a model for a conversation that has started', () => {
|
||||
const onModelChange = mock(() => {});
|
||||
mount({ selectedModel: null, model: null, hasStarted: true, onModelChange });
|
||||
// The old behaviour was `onModelChange('claude-code/fable')` — a confident wrong answer about a
|
||||
// session it knows nothing about. An unset picker is the honest one.
|
||||
expect(onModelChange).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('still picks a default for a composer that has NOT started', () => {
|
||||
const onModelChange = mock(() => {});
|
||||
mount({ selectedModel: null, model: null, hasStarted: false, onModelChange });
|
||||
expect(onModelChange).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Unknown harness is not permission to switch harness: handing a `ses_…` id to `claude --resume`
|
||||
// dispatches to the wrong sidecar, which is why this is pinned rather than left to read nicely.
|
||||
test('offers no harness switch while the harness is unknown', () => {
|
||||
mount({ selectedModel: null, model: null, hasStarted: true });
|
||||
expect(screen.queryByRole('button', { name: /^openai$/i })).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -113,9 +113,22 @@ export function ModelSelector({
|
||||
const harnessOf = (id: string) => (id.startsWith('claude-code') ? 'claude-code' : 'opencode');
|
||||
const currentHarness = displayModel ? harnessOf(displayModel) : null;
|
||||
|
||||
/*
|
||||
A started conversation whose model has NOT resolved yet.
|
||||
|
||||
`crossesHarness` used to return false here, which reads as "nothing crosses the harness" and is the
|
||||
opposite of what is known: with no current harness, EVERY provider might cross it. So a live session
|
||||
offered a switch to opencode during the window before its model arrived — a switch that cannot work,
|
||||
because it hands a `ses_…` id to `claude --resume` and dispatches to the wrong sidecar.
|
||||
|
||||
Unknown is not permission. The strip locks until the answer arrives.
|
||||
*/
|
||||
const harnessUnknown = hasStarted && !currentHarness;
|
||||
|
||||
/** Would picking this provider move the conversation to the other sidecar? */
|
||||
const crossesHarness = (provider: string) => {
|
||||
if (!hasStarted || !currentHarness) return false;
|
||||
if (!hasStarted) return false;
|
||||
if (!currentHarness) return true; // unknown — refuse rather than guess
|
||||
const target = availableModels.find((m) => m.provider === provider);
|
||||
return !!target && harnessOf(target.id) !== currentHarness;
|
||||
};
|
||||
@@ -123,7 +136,7 @@ export function ModelSelector({
|
||||
// The provider strip still collapses to a label when there is nothing to choose between — but now that
|
||||
// means "one harness on offer", not "you have already sent a message".
|
||||
const selectableProviders = providers.filter((p) => !crossesHarness(p));
|
||||
const providerStripLocked = isBusy || selectableProviders.length < 2;
|
||||
const providerStripLocked = isBusy || harnessUnknown || selectableProviders.length < 2;
|
||||
|
||||
const handleProviderClick = (provider: string) => {
|
||||
if (isBusy || crossesHarness(provider)) return;
|
||||
|
||||
@@ -112,19 +112,42 @@ export function useChat(initialSessionId?: string, initialModel?: string | null,
|
||||
const cursorRef = useRef(0);
|
||||
const sendRef = useRef<(d: Record<string, unknown>) => void>(() => {});
|
||||
|
||||
// Track if session has started (first message sent)
|
||||
const [hasStarted, setHasStarted] = useState(!!preloadedMessages?.length);
|
||||
/*
|
||||
Has this conversation already happened?
|
||||
|
||||
`!!preloadedMessages?.length` alone was wrong for the window that matters. A resumed chat renders
|
||||
before its messages arrive, so for those first frames `hasStarted` was FALSE on a conversation with a
|
||||
hundred turns behind it — and `ModelSelector` treats "not started" as permission to pick a default.
|
||||
The catalogue's first entry is `fable`, so that is what a reload reassigned live sessions to, and the
|
||||
change propagated to the running session as a real model switch (`OPUSPLAN → FABLE` in the header,
|
||||
followed by a 429 on a key with no fable entitlement).
|
||||
|
||||
A session id is the earlier and more reliable signal: it is known on the first render, and a chat that
|
||||
has one is by definition not new.
|
||||
*/
|
||||
const [hasStarted, setHasStarted] = useState(!!preloadedMessages?.length || !!resumeSessionId || !!initialSessionId);
|
||||
|
||||
// Get user settings for default model
|
||||
const { settings } = useSettings();
|
||||
|
||||
// Set default model from settings when starting a new chat (no initialSessionId, no initialModel)
|
||||
/*
|
||||
The settings default, for a genuinely NEW chat only.
|
||||
|
||||
This guarded on `initialSessionId`, which no chat caller ever passes when resuming — the id arrives as
|
||||
`options.resumeSessionId`, and `ChatPanelWrapper` goes as far as `resume ? undefined : sessionKey`.
|
||||
So the guard was true for every resumed conversation and the user's default was applied on top of a
|
||||
session that already had a model of its own.
|
||||
|
||||
Both names mean "this conversation exists", so both are checked. They are separate parameters for
|
||||
reasons that have nothing to do with this question.
|
||||
*/
|
||||
const existingSessionId = initialSessionId ?? resumeSessionId;
|
||||
|
||||
useEffect(() => {
|
||||
if (!initialSessionId && !initialModel) {
|
||||
if (existingSessionId || initialModel) return;
|
||||
const defaultModel = (projectScoped && settings?.chat?.defaultProjectModel) || settings?.chat?.defaultModel;
|
||||
if (defaultModel) setSelectedModel(defaultModel);
|
||||
}
|
||||
}, [initialSessionId, initialModel, settings, projectScoped]);
|
||||
}, [existingSessionId, initialModel, settings, projectScoped]);
|
||||
|
||||
// Sync selectedModel when initialModel changes (e.g. resuming a session)
|
||||
useEffect(() => {
|
||||
|
||||
Reference in New Issue
Block a user