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 { render, screen } from '@testing-library/react';
|
||||||
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||||
import type { ModelOption } from 'state/useModels';
|
import type { ModelOption } from 'state/useModels';
|
||||||
@@ -127,3 +127,30 @@ describe('ModelSelector display of an unknown model', () => {
|
|||||||
expect(screen.queryByTitle('claude-code/opus')).toBeNull();
|
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 harnessOf = (id: string) => (id.startsWith('claude-code') ? 'claude-code' : 'opencode');
|
||||||
const currentHarness = displayModel ? harnessOf(displayModel) : null;
|
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? */
|
/** Would picking this provider move the conversation to the other sidecar? */
|
||||||
const crossesHarness = (provider: string) => {
|
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);
|
const target = availableModels.find((m) => m.provider === provider);
|
||||||
return !!target && harnessOf(target.id) !== currentHarness;
|
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
|
// 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".
|
// means "one harness on offer", not "you have already sent a message".
|
||||||
const selectableProviders = providers.filter((p) => !crossesHarness(p));
|
const selectableProviders = providers.filter((p) => !crossesHarness(p));
|
||||||
const providerStripLocked = isBusy || selectableProviders.length < 2;
|
const providerStripLocked = isBusy || harnessUnknown || selectableProviders.length < 2;
|
||||||
|
|
||||||
const handleProviderClick = (provider: string) => {
|
const handleProviderClick = (provider: string) => {
|
||||||
if (isBusy || crossesHarness(provider)) return;
|
if (isBusy || crossesHarness(provider)) return;
|
||||||
|
|||||||
@@ -112,19 +112,42 @@ export function useChat(initialSessionId?: string, initialModel?: string | null,
|
|||||||
const cursorRef = useRef(0);
|
const cursorRef = useRef(0);
|
||||||
const sendRef = useRef<(d: Record<string, unknown>) => void>(() => {});
|
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
|
// Get user settings for default model
|
||||||
const { settings } = useSettings();
|
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(() => {
|
useEffect(() => {
|
||||||
if (!initialSessionId && !initialModel) {
|
if (existingSessionId || initialModel) return;
|
||||||
const defaultModel = (projectScoped && settings?.chat?.defaultProjectModel) || settings?.chat?.defaultModel;
|
const defaultModel = (projectScoped && settings?.chat?.defaultProjectModel) || settings?.chat?.defaultModel;
|
||||||
if (defaultModel) setSelectedModel(defaultModel);
|
if (defaultModel) setSelectedModel(defaultModel);
|
||||||
}
|
}, [existingSessionId, initialModel, settings, projectScoped]);
|
||||||
}, [initialSessionId, initialModel, settings, projectScoped]);
|
|
||||||
|
|
||||||
// Sync selectedModel when initialModel changes (e.g. resuming a session)
|
// Sync selectedModel when initialModel changes (e.g. resuming a session)
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
|||||||
Reference in New Issue
Block a user