fix(web): hydrate sessionModels and sessionMcpStatus on resume (#2213)
This commit is contained in:
@@ -168,3 +168,64 @@ describe("useExpandableDiff", () => {
|
||||
expect(result.current.metadata).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("useExpandableDiff — content-load retries", () => {
|
||||
it("retries a transient WebSocket timeout and succeeds on a later attempt", async () => {
|
||||
// A single 5s WS timeout used to set a terminal error and permanently
|
||||
// disable expansion for this mount (the auto-load effect only fires while
|
||||
// there is no error). The load now retries transient failures, so a brief
|
||||
// backend stall recovers instead of killing expansion.
|
||||
requestFileContentMock
|
||||
.mockRejectedValueOnce(new Error("WebSocket request timed out: workspace.file.get"))
|
||||
.mockResolvedValue({ content: "new", is_binary: false });
|
||||
requestFileContentAtRefMock.mockResolvedValue({ content: "old", is_binary: false });
|
||||
const reparsed = consistentMeta();
|
||||
processFileMock.mockReturnValue(reparsed);
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useExpandableDiff({ ...baseProps, fileDiffMetadata: PARTIAL }),
|
||||
);
|
||||
await loadAndSettle(result.current.loadContent);
|
||||
|
||||
expect(requestFileContentMock).toHaveBeenCalledTimes(2);
|
||||
expect(result.current.error).toBeNull();
|
||||
expect(result.current.metadata).toBe(reparsed);
|
||||
expect(result.current.isContentLoaded).toBe(true);
|
||||
});
|
||||
|
||||
it("does not retry a permanent error and falls back to the partial metadata", async () => {
|
||||
// Binary files can never be expanded; retrying would just spin. The load
|
||||
// surfaces the error on the first attempt and keeps the partial metadata.
|
||||
requestFileContentMock.mockRejectedValue(new Error("Cannot expand binary files"));
|
||||
requestFileContentAtRefMock.mockResolvedValue({ content: "old", is_binary: false });
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useExpandableDiff({ ...baseProps, fileDiffMetadata: PARTIAL }),
|
||||
);
|
||||
await loadAndSettle(result.current.loadContent);
|
||||
|
||||
expect(requestFileContentMock).toHaveBeenCalledTimes(1);
|
||||
expect(result.current.error).toBe("Cannot expand binary files");
|
||||
expect(result.current.metadata).toBe(PARTIAL);
|
||||
expect(result.current.canExpand).toBe(false);
|
||||
});
|
||||
|
||||
it("surfaces a terminal error after transient retries are exhausted", async () => {
|
||||
// Four attempts total (initial + 3 retries); when every one times out the
|
||||
// load gives up with the error so the caller falls back to partial metadata.
|
||||
requestFileContentMock.mockRejectedValue(
|
||||
new Error("WebSocket request timed out: workspace.file.get"),
|
||||
);
|
||||
requestFileContentAtRefMock.mockResolvedValue({ content: "old", is_binary: false });
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useExpandableDiff({ ...baseProps, fileDiffMetadata: PARTIAL }),
|
||||
);
|
||||
await loadAndSettle(result.current.loadContent);
|
||||
|
||||
expect(requestFileContentMock).toHaveBeenCalledTimes(4);
|
||||
expect(result.current.error).toContain("timed out");
|
||||
expect(result.current.metadata).toBe(PARTIAL);
|
||||
expect(result.current.canExpand).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -32,6 +32,34 @@ function isFileNotFoundError(error: string): boolean {
|
||||
return /file not found|not found|no such file|does not exist/i.test(error);
|
||||
}
|
||||
|
||||
/**
|
||||
* Number of extra attempts for a transient content-load failure and the base
|
||||
* backoff between them. The two file reads go over the WebSocket, which rejects
|
||||
* with a 5s timeout and no retry of its own; when the backend is briefly slow
|
||||
* (loaded CI runner, reconnect in flight) a single timeout would otherwise set
|
||||
* a terminal error and permanently disable expansion for this mount, since the
|
||||
* auto-load effect only fires while there is no error. Retrying transient
|
||||
* failures here keeps that from happening.
|
||||
*/
|
||||
const CONTENT_LOAD_MAX_RETRIES = 3;
|
||||
const CONTENT_LOAD_RETRY_BASE_MS = 300;
|
||||
|
||||
/**
|
||||
* A transient failure is one worth retrying: the WebSocket client not being
|
||||
* ready yet, or a request that timed out. Permanent failures (binary file,
|
||||
* genuine backend rejections) are surfaced immediately so we fall back to the
|
||||
* partial metadata without spinning.
|
||||
*/
|
||||
function isTransientLoadError(error: string): boolean {
|
||||
return /timed out|websocket client not available|not connected|connection (closed|lost)/i.test(
|
||||
error,
|
||||
);
|
||||
}
|
||||
|
||||
function delay(ms: number): Promise<void> {
|
||||
return new Promise((resolve) => setTimeout(resolve, ms));
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate that a reparsed metadata won't trip @pierre/diffs' iterateOverDiff
|
||||
* trailing-context check (which throws and tears down the renderer). When
|
||||
@@ -114,6 +142,34 @@ async function fetchExpansionContent(
|
||||
return { oldContent, newContent };
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch expansion content, retrying transient failures with linear backoff.
|
||||
* `isCurrent` lets the caller abort the retry loop when the hook's inputs have
|
||||
* changed (a newer request superseded this one); permanent failures rethrow on
|
||||
* the first attempt so we don't spin on a binary file or a real backend error.
|
||||
*/
|
||||
async function fetchExpansionContentWithRetry(
|
||||
sessionId: string,
|
||||
filePath: string,
|
||||
baseRef: string | undefined,
|
||||
repo: string | undefined,
|
||||
isCurrent: () => boolean,
|
||||
) {
|
||||
let lastError: unknown;
|
||||
for (let attempt = 0; attempt <= CONTENT_LOAD_MAX_RETRIES; attempt++) {
|
||||
try {
|
||||
return await fetchExpansionContent(sessionId, filePath, baseRef, repo);
|
||||
} catch (err) {
|
||||
lastError = err;
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
if (!isTransientLoadError(msg) || attempt === CONTENT_LOAD_MAX_RETRIES) throw err;
|
||||
await delay(CONTENT_LOAD_RETRY_BASE_MS * (attempt + 1));
|
||||
if (!isCurrent()) throw err;
|
||||
}
|
||||
}
|
||||
throw lastError;
|
||||
}
|
||||
|
||||
/**
|
||||
* Hook for managing expandable diffs with lazy-loaded file content.
|
||||
*
|
||||
@@ -157,7 +213,13 @@ export function useExpandableDiff({
|
||||
setError(null);
|
||||
|
||||
try {
|
||||
const content = await fetchExpansionContent(sessionId, filePath, baseRef, repo);
|
||||
const content = await fetchExpansionContentWithRetry(
|
||||
sessionId,
|
||||
filePath,
|
||||
baseRef,
|
||||
repo,
|
||||
() => version === requestVersionRef.current,
|
||||
);
|
||||
if (version === requestVersionRef.current) setLoadedContent(content);
|
||||
} catch (err) {
|
||||
const msg = err instanceof Error ? err.message : "Failed to load file content";
|
||||
|
||||
@@ -4,7 +4,12 @@ import {
|
||||
type ModelSelectorOption,
|
||||
usableConfigOptions,
|
||||
} from "@/components/model-config-selector";
|
||||
import type { ConfigOptionEntry } from "@/lib/state/slices/session-runtime/types";
|
||||
import { hasCompleteDynamicConfig, requiredConfigKeys } from "@/components/task/model-selector";
|
||||
import type { Agent, TaskSession } from "@/lib/types/http";
|
||||
import type {
|
||||
ConfigOptionEntry,
|
||||
SessionModelEntry,
|
||||
} from "@/lib/state/slices/session-runtime/types";
|
||||
|
||||
function compactTriggerLabel(
|
||||
modelOptions: ModelSelectorOption[],
|
||||
@@ -178,3 +183,99 @@ describe("task model selector compact label", () => {
|
||||
expect(label).toBe(`${providerModelName} / Off`);
|
||||
});
|
||||
});
|
||||
|
||||
function makeSession(overrides: Partial<TaskSession> = {}): TaskSession {
|
||||
return {
|
||||
id: "session-1",
|
||||
task_id: "task-1",
|
||||
state: "RUNNING",
|
||||
started_at: "2026-01-01T00:00:00Z",
|
||||
updated_at: "2026-01-01T00:00:00Z",
|
||||
...overrides,
|
||||
} as TaskSession;
|
||||
}
|
||||
|
||||
const flatModelId = "claude-opus-4-8";
|
||||
|
||||
function flatModelEntries(): SessionModelEntry[] {
|
||||
return [
|
||||
{ modelId: flatModelId, name: "Claude Opus 4.8" },
|
||||
{ modelId: "claude-sonnet-4-8", name: "Claude Sonnet 4.8" },
|
||||
];
|
||||
}
|
||||
|
||||
describe("hasCompleteDynamicConfig", () => {
|
||||
const noAgents: Agent[] = [];
|
||||
|
||||
it("treats a required model key as satisfied by a flat ACP model list", () => {
|
||||
const session = makeSession({
|
||||
metadata: { runtime_config: { config_options: { model: flatModelId } } },
|
||||
});
|
||||
expect(requiredConfigKeys(session, noAgents)).toEqual(["model"]);
|
||||
|
||||
const hydrated = hasCompleteDynamicConfig(
|
||||
session,
|
||||
{ currentModelId: flatModelId, models: flatModelEntries(), configOptions: [] },
|
||||
noAgents,
|
||||
);
|
||||
|
||||
expect(hydrated).toBe(true);
|
||||
});
|
||||
|
||||
it("still requires the model config option when there is no flat model list", () => {
|
||||
const session = makeSession({
|
||||
metadata: { runtime_config: { config_options: { model: providerModelId } } },
|
||||
});
|
||||
|
||||
const hydrated = hasCompleteDynamicConfig(
|
||||
session,
|
||||
{ currentModelId: providerModelId, models: [], configOptions: [] },
|
||||
noAgents,
|
||||
);
|
||||
|
||||
expect(hydrated).toBe(false);
|
||||
});
|
||||
|
||||
it("hydrates a config-option agent once the model option is present", () => {
|
||||
const session = makeSession({
|
||||
metadata: { runtime_config: { config_options: { model: providerModelId } } },
|
||||
});
|
||||
|
||||
const modelOption: ConfigOptionEntry = {
|
||||
type: "select",
|
||||
id: "model",
|
||||
name: "Model",
|
||||
currentValue: providerModelId,
|
||||
category: "model",
|
||||
options: [{ value: providerModelId, name: providerModelName }],
|
||||
};
|
||||
|
||||
const hydrated = hasCompleteDynamicConfig(
|
||||
session,
|
||||
{ currentModelId: providerModelId, models: [], configOptions: [modelOption] },
|
||||
noAgents,
|
||||
);
|
||||
|
||||
expect(hydrated).toBe(true);
|
||||
});
|
||||
|
||||
it("does not let a flat model list satisfy a non-model required key", () => {
|
||||
const session = makeSession({
|
||||
metadata: {
|
||||
runtime_config: { config_options: { model: flatModelId, reasoning_effort: "high" } },
|
||||
},
|
||||
});
|
||||
|
||||
const hydrated = hasCompleteDynamicConfig(
|
||||
session,
|
||||
{ currentModelId: flatModelId, models: flatModelEntries(), configOptions: [] },
|
||||
noAgents,
|
||||
);
|
||||
|
||||
expect(hydrated).toBe(false);
|
||||
});
|
||||
|
||||
it("is hydrated when there are no required config keys", () => {
|
||||
expect(hasCompleteDynamicConfig(makeSession(), undefined, noAgents)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
} from "@/components/model-config-selector";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import { useToast } from "@/components/toast-provider";
|
||||
import { createDebugLogger, isDebug } from "@/lib/debug/log";
|
||||
import { useAvailableAgents } from "@/hooks/domains/settings/use-available-agents";
|
||||
import { useSettingsData } from "@/hooks/domains/settings/use-settings-data";
|
||||
import { setSessionConfigOption, setSessionModel } from "@/lib/api/domains/session-api";
|
||||
@@ -33,6 +34,8 @@ type ModelSelectorProps = {
|
||||
triggerClassName?: string;
|
||||
};
|
||||
|
||||
const debug = createDebugLogger("model-selector:gate");
|
||||
|
||||
function configValueKeys(value: unknown): string[] {
|
||||
if (!value || typeof value !== "object" || Array.isArray(value)) return [];
|
||||
return Object.entries(value)
|
||||
@@ -40,7 +43,9 @@ function configValueKeys(value: unknown): string[] {
|
||||
.map(([key]) => key);
|
||||
}
|
||||
|
||||
function requiredConfigKeys(session: TaskSession | null, agents: Agent[]): string[] {
|
||||
const MODEL_CONFIG_KEY = "model";
|
||||
|
||||
export function requiredConfigKeys(session: TaskSession | null, agents: Agent[]): string[] {
|
||||
if (!session) return [];
|
||||
const keys = new Set(configValueKeys(session.agent_profile_snapshot?.config_options));
|
||||
for (const agent of agents) {
|
||||
@@ -59,7 +64,7 @@ function requiredConfigKeys(session: TaskSession | null, agents: Agent[]): strin
|
||||
return [...keys];
|
||||
}
|
||||
|
||||
function hasCompleteDynamicConfig(
|
||||
export function hasCompleteDynamicConfig(
|
||||
session: TaskSession | null,
|
||||
sessionModelsData: SessionModelsEntry | undefined,
|
||||
agents: Agent[],
|
||||
@@ -68,7 +73,16 @@ function hasCompleteDynamicConfig(
|
||||
if (required.length === 0) return true;
|
||||
if (!sessionModelsData) return false;
|
||||
const available = new Set(sessionModelsData.configOptions.map((option) => option.id));
|
||||
return required.every((key) => available.has(key));
|
||||
// Flat-model-list agents (e.g. claude-opus) expose their models via the ACP
|
||||
// top-level `models` list and switch with session/set_model rather than a
|
||||
// SessionConfigOption(category="model"). For those the persisted runtime
|
||||
// config still records a "model" key, but it will never appear in
|
||||
// configOptions. Treat that key as satisfied when the session has a flat model
|
||||
// list — the selector renders fine from it.
|
||||
const hasFlatModelList = !!sessionModelsData.models.length;
|
||||
return required.every(
|
||||
(key) => available.has(key) || (key === MODEL_CONFIG_KEY && hasFlatModelList),
|
||||
);
|
||||
}
|
||||
|
||||
function resolveSessionState(
|
||||
@@ -338,6 +352,8 @@ function useModelSelectorState(sessionId: string | null) {
|
||||
configOptions,
|
||||
configBaseline: sessionModelsData?.configBaseline,
|
||||
configHydrated: hasCompleteDynamicConfig(session, sessionModelsData, settingsAgents as Agent[]),
|
||||
requiredKeys: requiredConfigKeys(session, settingsAgents as Agent[]),
|
||||
rawConfigOptionIds: (sessionModelsData?.configOptions ?? []).map((o) => o.id),
|
||||
handleModelChange,
|
||||
handleConfigChange,
|
||||
};
|
||||
@@ -353,6 +369,8 @@ export const ModelSelector = memo(function ModelSelector({
|
||||
configOptions,
|
||||
configBaseline,
|
||||
configHydrated,
|
||||
requiredKeys,
|
||||
rawConfigOptionIds,
|
||||
handleModelChange,
|
||||
handleConfigChange,
|
||||
} = useModelSelectorState(sessionId);
|
||||
@@ -374,6 +392,19 @@ export const ModelSelector = memo(function ModelSelector({
|
||||
[sessionId, handleConfigChange],
|
||||
);
|
||||
|
||||
if (isDebug()) {
|
||||
debug("render", {
|
||||
sessionId: sessionId ?? "",
|
||||
configHydrated,
|
||||
currentModel: currentModel ?? "",
|
||||
hasModelConfig: !!modelConfig,
|
||||
modelOptionsLen: modelOptions.length,
|
||||
configOptionIds: configOptions.map((o) => o.id),
|
||||
rawConfigOptionIds,
|
||||
requiredKeys,
|
||||
willHide: !sessionId || !configHydrated || (!currentModel && !modelConfig),
|
||||
});
|
||||
}
|
||||
if (!sessionId || !configHydrated || (!currentModel && !modelConfig)) return null;
|
||||
|
||||
return (
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { test, expect } from "../../fixtures/docker-test-base";
|
||||
import { E2E_IMAGE_TAG } from "../../fixtures/docker-probe";
|
||||
import { E2E_IMAGE_TAG, removeKandevContainers } from "../../fixtures/docker-probe";
|
||||
import { dockerInspectExists, dockerRemove } from "../../helpers/docker";
|
||||
|
||||
function createStoppedContainer(labels: string[]): string {
|
||||
@@ -17,6 +17,14 @@ test("removes only stopped Kandev-labeled containers and gates daemon-wide clean
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
// This test asserts the *global* count of kandev.managed=true containers the
|
||||
// daemon reports ("2 managed containers"). The containers project shares one
|
||||
// Docker daemon across all specs in the worker and only sweeps managed
|
||||
// containers at worker teardown, so a sibling Docker-executor spec (or one of
|
||||
// its retries) can leave managed containers behind and inflate the count —
|
||||
// the flake seen in CI was "5 managed containers" instead of 2. Sweep any
|
||||
// stray managed containers first so this test counts only its own fixtures.
|
||||
removeKandevContainers();
|
||||
const activeTask = await apiClient.createTask(seedData.workspaceId, "Retain active container", {
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
|
||||
@@ -325,33 +325,46 @@ test.describe("Diff expansion — Pierre Diffs provider", () => {
|
||||
await openChangesTab(testPage);
|
||||
await openExpansionFileDiff(testPage);
|
||||
|
||||
await waitForDiffText(testPage, "HUNK_TOP", 15_000);
|
||||
// Use the default (60s) waitForDiffText timeout, not a tight 15s: on a cold
|
||||
// CI runner the first diff in a shard pays a 30-40s Shiki/regex-engine JIT
|
||||
// (see the "renders Pierre Diffs viewer" test) before any diff text renders.
|
||||
await waitForDiffText(testPage, "HUNK_TOP");
|
||||
|
||||
// Wait for expand buttons to appear in the shadow DOM. They load
|
||||
// asynchronously after full file content is fetched via WebSocket.
|
||||
await testPage.waitForFunction(
|
||||
() => {
|
||||
const container = document.querySelector("diffs-container");
|
||||
const shadow = container?.shadowRoot;
|
||||
if (!shadow) return false;
|
||||
return shadow.querySelectorAll("[data-expand-button]").length >= 3;
|
||||
},
|
||||
null,
|
||||
{ timeout: 20_000 },
|
||||
);
|
||||
|
||||
// Click the middle separator's expand-up button to reveal lines from the
|
||||
// top of the collapsed gap. The middle separator is the only one without
|
||||
// data-separator-first or data-separator-last attributes.
|
||||
await testPage.evaluate(() => {
|
||||
const container = document.querySelector("diffs-container");
|
||||
const shadow = container!.shadowRoot!;
|
||||
const sel =
|
||||
"[data-separator='line-info']:not([data-separator-first]):not([data-separator-last])";
|
||||
const btn = shadow.querySelector<HTMLElement>(`${sel} [data-expand-up]`);
|
||||
if (!btn) throw new Error("Middle separator expand-up button not found");
|
||||
btn.click();
|
||||
});
|
||||
// Expand controls render in two async phases: the separators appear once the
|
||||
// patch is parsed, but their expand buttons are injected only after the full
|
||||
// file content arrives over WebSocket and @pierre/diffs re-parses it (see
|
||||
// components/diff/use-expandable-diff.ts). Waiting for a *count* of
|
||||
// [data-expand-button] elements is insufficient — three buttons can exist
|
||||
// while the middle separator's [data-expand-up] specifically has not been
|
||||
// injected yet, which is the flake that failed CI with "Middle separator
|
||||
// expand-up button not found". The re-parse can also replace the shadow
|
||||
// subtree, so a separate wait-then-click leaves a window where the button
|
||||
// vanishes between the wait passing and the click firing. Find the button
|
||||
// and click it in a single browser evaluation, retried by expect.poll, so
|
||||
// the lookup and the click are atomic per attempt and the click is only
|
||||
// considered done once the collapsed lines are revealed. The timeout is
|
||||
// generous because the WS content fetch + reparse is the slowest phase under
|
||||
// a loaded runner; the 120s per-test budget leaves ample headroom.
|
||||
const middleExpandUpSelector =
|
||||
"[data-separator='line-info']:not([data-separator-first]):not([data-separator-last]) [data-expand-up]";
|
||||
await expect
|
||||
.poll(
|
||||
() =>
|
||||
testPage.evaluate((selector) => {
|
||||
const container = document.querySelector("diffs-container");
|
||||
const shadow = container?.shadowRoot;
|
||||
if (!shadow) return false;
|
||||
// Line 60 sits within the first 20 lines revealed by expanding from
|
||||
// the top of the gap; if it is already present the click landed.
|
||||
if (shadow.textContent?.includes("original_060")) return true;
|
||||
const btn = shadow.querySelector<HTMLElement>(selector);
|
||||
if (!btn) return false;
|
||||
btn.click();
|
||||
return shadow.textContent?.includes("original_060") ?? false;
|
||||
}, middleExpandUpSelector),
|
||||
{ timeout: 40_000 },
|
||||
)
|
||||
.toBe(true);
|
||||
|
||||
// Line 60 is within the first 20 lines revealed by expanding from the top hunk.
|
||||
await expect(testPage.getByText("original_060", { exact: false })).toBeVisible({
|
||||
|
||||
@@ -200,6 +200,19 @@
|
||||
* fetch.success/error means the PR fetch never
|
||||
* fired (look at useActiveTaskPR plumbing).
|
||||
*
|
||||
* Model selector pipeline (bug: selector shows then disappears on relaunch)
|
||||
* [model-selector:ws] session.models_updated handler — payload vs.
|
||||
* existing entry, isEmpty/populated guard, the
|
||||
* stale-empty skip decision (willSkip), and the
|
||||
* partial-update config-options preservation
|
||||
* decision (preserveConfigOptions).
|
||||
* [model-selector:gate] ModelSelector render gate — configHydrated,
|
||||
* currentModel, hasModelConfig, configOptionIds
|
||||
* (usable/filtered), rawConfigOptionIds (unfiltered
|
||||
* store entry), requiredKeys (keys the session's
|
||||
* profile/runtime config demand), and willHide (the
|
||||
* exact gate expression).
|
||||
*
|
||||
* Other
|
||||
* [ws:connection] WS hook mount + status transitions
|
||||
* [dockview:*] layout restore / save / env-switch / session-tabs / task-select
|
||||
|
||||
@@ -5,6 +5,7 @@ import { hydrateState, hydrateUI } from "./hydrator";
|
||||
import { defaultUIState } from "@/lib/state/slices/ui/ui-slice";
|
||||
import { defaultState, mergeInitialState } from "@/lib/state/default-state";
|
||||
import type { AppState } from "@/lib/state/store";
|
||||
import type { MCPAttachmentHistory } from "@/lib/state/slices/session-runtime/types";
|
||||
|
||||
function makeDraft(): AppState {
|
||||
// hydrateUI only touches UI-slice fields; an empty object cast satisfies
|
||||
@@ -347,6 +348,100 @@ describe("hydrateState — sidebar views from user settings", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("hydrateState — session runtime model state", () => {
|
||||
const sessionId = "session-1";
|
||||
const modelId = "claude-opus-4-8";
|
||||
const liveModelId = "live-model";
|
||||
|
||||
it("force-merges sessionModels over live state so the selector survives resume", () => {
|
||||
const result = produce(makeAppDraft(), (draft: Draft<AppState>) => {
|
||||
draft.sessionModels.bySessionId[sessionId] = {
|
||||
currentModelId: liveModelId,
|
||||
models: [{ modelId: liveModelId, name: "Live" }],
|
||||
configOptions: [],
|
||||
};
|
||||
hydrateState(
|
||||
draft,
|
||||
{
|
||||
sessionModels: {
|
||||
bySessionId: {
|
||||
[sessionId]: {
|
||||
currentModelId: modelId,
|
||||
models: [{ modelId, name: "Opus" }],
|
||||
configOptions: [],
|
||||
},
|
||||
},
|
||||
},
|
||||
} as unknown as Partial<AppState>,
|
||||
{ activeSessionId: sessionId, forceMergeSessionId: sessionId },
|
||||
);
|
||||
});
|
||||
|
||||
expect(result.sessionModels.bySessionId[sessionId]).toEqual({
|
||||
currentModelId: modelId,
|
||||
models: [{ modelId, name: "Opus" }],
|
||||
configOptions: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("force-merges sessionMcpStatus over live state", () => {
|
||||
const bootHistory: MCPAttachmentHistory = {
|
||||
version: 1,
|
||||
current: {
|
||||
attachment_attempt_id: "attempt-boot",
|
||||
started_at: "2026-06-11T00:00:00.000Z",
|
||||
servers: [{ name: "fs", status: "connected" }],
|
||||
},
|
||||
};
|
||||
const result = produce(makeAppDraft(), (draft: Draft<AppState>) => {
|
||||
draft.sessionMcpStatus.bySessionId[sessionId] = {
|
||||
version: 1,
|
||||
current: {
|
||||
attachment_attempt_id: "attempt-live",
|
||||
started_at: "2026-06-10T00:00:00.000Z",
|
||||
servers: [{ name: "stale", status: "failed" }],
|
||||
},
|
||||
};
|
||||
hydrateState(
|
||||
draft,
|
||||
{
|
||||
sessionMcpStatus: { bySessionId: { [sessionId]: bootHistory } },
|
||||
} as unknown as Partial<AppState>,
|
||||
{ activeSessionId: sessionId, forceMergeSessionId: sessionId },
|
||||
);
|
||||
});
|
||||
|
||||
expect(result.sessionMcpStatus.bySessionId[sessionId]).toEqual(bootHistory);
|
||||
});
|
||||
|
||||
it("does not overwrite live model state for the active (non-force-merged) session", () => {
|
||||
const result = produce(makeAppDraft(), (draft: Draft<AppState>) => {
|
||||
draft.sessionModels.bySessionId["active-session"] = {
|
||||
currentModelId: liveModelId,
|
||||
models: [],
|
||||
configOptions: [],
|
||||
};
|
||||
hydrateState(
|
||||
draft,
|
||||
{
|
||||
sessionModels: {
|
||||
bySessionId: {
|
||||
"active-session": {
|
||||
currentModelId: "stale-model",
|
||||
models: [],
|
||||
configOptions: [],
|
||||
},
|
||||
},
|
||||
},
|
||||
} as unknown as Partial<AppState>,
|
||||
{ activeSessionId: "active-session" },
|
||||
);
|
||||
});
|
||||
|
||||
expect(result.sessionModels.bySessionId["active-session"].currentModelId).toBe(liveModelId);
|
||||
});
|
||||
});
|
||||
|
||||
describe("hydrateState — system slice", () => {
|
||||
it("leaves the system slice untouched when the caller supplies no system fields", () => {
|
||||
const result = produce(makeAppDraft(), (draft: Draft<AppState>) => {
|
||||
|
||||
@@ -177,6 +177,15 @@ function hydrateSessionRuntime(
|
||||
activeSessionId: string | null,
|
||||
forceMergeSessionId: string | null,
|
||||
): void {
|
||||
// Merge a `{ bySessionId }`-shaped slice, protecting the active session.
|
||||
const mergeBySession = (key: keyof AppState & keyof HydrationState): void => {
|
||||
const source = state[key] as { bySessionId?: Record<string, unknown> } | undefined;
|
||||
if (!source) return;
|
||||
const target = draft[key] as { bySessionId?: Record<string, unknown> } | undefined;
|
||||
if (!target?.bySessionId) return;
|
||||
mergeSessionMap(target.bySessionId, source.bySessionId, activeSessionId, forceMergeSessionId);
|
||||
};
|
||||
|
||||
if (state.terminal) deepMerge(draft.terminal, state.terminal);
|
||||
if (state.shell) {
|
||||
mergeSessionMap(
|
||||
@@ -201,26 +210,14 @@ function hydrateSessionRuntime(
|
||||
forceMergeSessionId,
|
||||
);
|
||||
}
|
||||
if (state.contextWindow) {
|
||||
mergeSessionMap(
|
||||
draft.contextWindow.bySessionId,
|
||||
state.contextWindow?.bySessionId,
|
||||
activeSessionId,
|
||||
forceMergeSessionId,
|
||||
);
|
||||
}
|
||||
mergeBySession("contextWindow");
|
||||
if (state.environmentIdBySessionId) {
|
||||
Object.assign(draft.environmentIdBySessionId, state.environmentIdBySessionId);
|
||||
}
|
||||
mergeBySession("sessionModels");
|
||||
mergeBySession("sessionMcpStatus");
|
||||
if (state.agents) deepMerge(draft.agents, state.agents);
|
||||
if (state.prepareProgress) {
|
||||
mergeSessionMap(
|
||||
draft.prepareProgress.bySessionId,
|
||||
state.prepareProgress?.bySessionId,
|
||||
activeSessionId,
|
||||
forceMergeSessionId,
|
||||
);
|
||||
}
|
||||
mergeBySession("prepareProgress");
|
||||
}
|
||||
|
||||
/** Hydrate UI slices without overwriting active connection state. */
|
||||
|
||||
@@ -7,6 +7,7 @@ import type { SessionModelsPayload } from "@/lib/types/session-runtime-payloads"
|
||||
import { registerSessionModelsHandlers } from "./session-models";
|
||||
|
||||
const providerModelId = "gpt-5.6-sol";
|
||||
const providerModelName = "GPT-5.6 Sol";
|
||||
const reasoningOptionId = "reasoning_effort";
|
||||
const reasoningOptionName = "Reasoning Effort";
|
||||
const optionDescription = "Controls how much reasoning the model performs.";
|
||||
@@ -155,6 +156,183 @@ describe("session.models_updated handler", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("session.models_updated stale-empty guard", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("ignores a fully empty update when a populated entry already exists", () => {
|
||||
const store = makeStore({
|
||||
sessionModels: {
|
||||
bySessionId: {
|
||||
"session-1": {
|
||||
currentModelId: "gpt-5.5",
|
||||
models: [{ modelId: "gpt-5.5", name: "GPT-5.5" }],
|
||||
configOptions: [],
|
||||
},
|
||||
},
|
||||
} as AppState["sessionModels"],
|
||||
});
|
||||
const handler = registerSessionModelsHandlers(store)["session.models_updated"]!;
|
||||
|
||||
handler(makeMessage(makePayload("", { models: [], config_options: [] })));
|
||||
|
||||
expect(store.getState().setSessionModels).not.toHaveBeenCalled();
|
||||
expect(store.getState().sessionModels.bySessionId["session-1"]).toEqual({
|
||||
currentModelId: "gpt-5.5",
|
||||
models: [{ modelId: "gpt-5.5", name: "GPT-5.5" }],
|
||||
configOptions: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("applies an empty update when no entry exists yet", () => {
|
||||
const store = makeStore();
|
||||
const handler = registerSessionModelsHandlers(store)["session.models_updated"]!;
|
||||
|
||||
handler(makeMessage(makePayload("", { models: [], config_options: [] })));
|
||||
|
||||
expect(store.getState().setSessionModels).toHaveBeenCalledWith(
|
||||
"session-1",
|
||||
expect.objectContaining({ currentModelId: "" }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("session.models_updated stale-empty guard — payload-derived classification", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("ignores an empty update even when persisted runtime metadata carries a model", () => {
|
||||
const store = makeStore({
|
||||
taskSessions: {
|
||||
items: {
|
||||
"session-1": makeTaskSession({
|
||||
runtime_config: { model: providerModelId },
|
||||
}),
|
||||
},
|
||||
},
|
||||
sessionModels: {
|
||||
bySessionId: {
|
||||
"session-1": {
|
||||
currentModelId: providerModelId,
|
||||
models: [{ modelId: providerModelId, name: providerModelName }],
|
||||
configOptions: [],
|
||||
},
|
||||
},
|
||||
} as AppState["sessionModels"],
|
||||
});
|
||||
const handler = registerSessionModelsHandlers(store)["session.models_updated"]!;
|
||||
|
||||
handler(makeMessage(makePayload("", { models: [], config_options: [] })));
|
||||
|
||||
expect(store.getState().setSessionModels).not.toHaveBeenCalled();
|
||||
expect(store.getState().sessionModels.bySessionId["session-1"].currentModelId).toBe(
|
||||
providerModelId,
|
||||
);
|
||||
});
|
||||
|
||||
it("ignores a fully empty update when only config options are populated", () => {
|
||||
const configOptions = [
|
||||
{
|
||||
type: "select",
|
||||
id: "model",
|
||||
name: "Model",
|
||||
currentValue: providerModelId,
|
||||
options: [{ value: providerModelId, name: providerModelName }],
|
||||
},
|
||||
];
|
||||
const store = makeStore({
|
||||
sessionModels: {
|
||||
bySessionId: {
|
||||
"session-1": {
|
||||
currentModelId: "",
|
||||
models: [],
|
||||
configOptions,
|
||||
},
|
||||
},
|
||||
} as AppState["sessionModels"],
|
||||
});
|
||||
const handler = registerSessionModelsHandlers(store)["session.models_updated"]!;
|
||||
|
||||
handler(makeMessage(makePayload("", { models: [], config_options: [] })));
|
||||
|
||||
expect(store.getState().setSessionModels).not.toHaveBeenCalled();
|
||||
expect(store.getState().sessionModels.bySessionId["session-1"].configOptions).toEqual(
|
||||
configOptions,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("session.models_updated stale-empty guard — config preservation", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("still applies a populated update over an existing entry", () => {
|
||||
const store = makeStore({
|
||||
sessionModels: {
|
||||
bySessionId: {
|
||||
"session-1": {
|
||||
currentModelId: "gpt-5.5",
|
||||
models: [{ modelId: "gpt-5.5", name: "GPT-5.5" }],
|
||||
configOptions: [],
|
||||
},
|
||||
},
|
||||
} as AppState["sessionModels"],
|
||||
});
|
||||
const handler = registerSessionModelsHandlers(store)["session.models_updated"]!;
|
||||
|
||||
handler(makeMessage(makePayload("gpt-5.6-sol")));
|
||||
|
||||
expect(store.getState().setSessionModels).toHaveBeenCalledWith(
|
||||
"session-1",
|
||||
expect.objectContaining({ currentModelId: "gpt-5.6-sol" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves populated config options when a partial relaunch update drops them", () => {
|
||||
const existingConfigOptions = [
|
||||
{
|
||||
type: "select",
|
||||
id: "model",
|
||||
name: "Model",
|
||||
currentValue: providerModelId,
|
||||
options: [{ value: providerModelId, name: providerModelName }],
|
||||
},
|
||||
];
|
||||
const store = makeStore({
|
||||
sessionModels: {
|
||||
bySessionId: {
|
||||
"session-1": {
|
||||
currentModelId: providerModelId,
|
||||
models: [{ modelId: providerModelId, name: providerModelName }],
|
||||
configOptions: existingConfigOptions,
|
||||
},
|
||||
},
|
||||
} as AppState["sessionModels"],
|
||||
});
|
||||
const handler = registerSessionModelsHandlers(store)["session.models_updated"]!;
|
||||
|
||||
handler(
|
||||
makeMessage(
|
||||
makePayload(providerModelId, {
|
||||
models: [{ model_id: providerModelId, name: providerModelName }],
|
||||
config_options: [],
|
||||
}),
|
||||
),
|
||||
);
|
||||
|
||||
expect(store.getState().setSessionModels).toHaveBeenCalledWith(
|
||||
"session-1",
|
||||
expect.objectContaining({ configOptions: existingConfigOptions }),
|
||||
);
|
||||
expect(store.getState().sessionModels.bySessionId["session-1"].configOptions).toEqual(
|
||||
existingConfigOptions,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("session.models_updated config metadata", () => {
|
||||
it("stores provider descriptions and the persisted config baseline", () => {
|
||||
const store = makeStore();
|
||||
|
||||
@@ -2,6 +2,9 @@ import type { StoreApi } from "zustand";
|
||||
import type { AppState } from "@/lib/state/store";
|
||||
import type { WsHandlers } from "@/lib/ws/handlers/types";
|
||||
import type { SessionModelsPayload } from "@/lib/types/backend";
|
||||
import { createDebugLogger, isDebug } from "@/lib/debug/log";
|
||||
|
||||
const debug = createDebugLogger("model-selector:ws");
|
||||
|
||||
type SessionModelConfigOption = SessionModelsPayload["config_options"][number];
|
||||
|
||||
@@ -80,6 +83,76 @@ function isModelConfigOption(option: SessionModelConfigOption): boolean {
|
||||
return option.id === "model" || option.category === "model";
|
||||
}
|
||||
|
||||
// During an agentctl relaunch (ready -> starting -> ready) the backend can emit
|
||||
// a transient session.models_updated with no models, no current model, and no
|
||||
// config options before the fresh agent reports its real capabilities. Since
|
||||
// setSessionModels is a blind replace, that empty event would wipe the populated
|
||||
// entry and unmount the model selector until a good event arrives. Skip the
|
||||
// overwrite when the incoming payload is fully empty but a populated entry
|
||||
// already exists — the same stale-empty guard used by setSessionCommits.
|
||||
function isEmptyModelsUpdate(
|
||||
currentModelId: string,
|
||||
acpModels: SessionModelsPayload["models"],
|
||||
configOptions: SessionModelsPayload["config_options"],
|
||||
): boolean {
|
||||
return !currentModelId && !acpModels?.length && !configOptions?.length;
|
||||
}
|
||||
|
||||
function hasPopulatedModels(state: AppState, sessionId: string): boolean {
|
||||
const existing = state.sessionModels.bySessionId[sessionId];
|
||||
return (
|
||||
!!existing &&
|
||||
(!!existing.currentModelId || existing.models.length > 0 || existing.configOptions.length > 0)
|
||||
);
|
||||
}
|
||||
|
||||
// During an agentctl relaunch the backend can emit a transient
|
||||
// session.models_updated that still carries models + a current model (so it is
|
||||
// not "fully empty") but reports an empty config_options array before the fresh
|
||||
// agent finishes reporting its dynamic config. Blindly writing that array wipes
|
||||
// the previously-hydrated config options, which flips the model selector's
|
||||
// configHydrated gate to false and unmounts it. Preserve the existing populated
|
||||
// config options (and their baseline) when the payload drops them; a real config
|
||||
// change always re-sends the non-empty array.
|
||||
function shouldPreserveExistingConfigOptions(
|
||||
state: AppState,
|
||||
sessionId: string,
|
||||
payload: SessionModelsPayload,
|
||||
): boolean {
|
||||
if (payload.config_options?.length) return false;
|
||||
const existing = state.sessionModels.bySessionId[sessionId];
|
||||
return !!existing && existing.configOptions.length > 0;
|
||||
}
|
||||
|
||||
function debugModelsUpdate(
|
||||
state: AppState,
|
||||
sessionId: string,
|
||||
payload: SessionModelsPayload,
|
||||
resolved: {
|
||||
currentModelId: string;
|
||||
isEmpty: boolean;
|
||||
populated: boolean;
|
||||
preserveConfigOptions: boolean;
|
||||
},
|
||||
) {
|
||||
if (!isDebug()) return;
|
||||
const existing = state.sessionModels.bySessionId[sessionId];
|
||||
debug("models_updated", {
|
||||
sessionId,
|
||||
payloadCurrentModelId: payload.current_model_id ?? "",
|
||||
resolvedCurrentModelId: resolved.currentModelId,
|
||||
payloadModelsLen: payload.models?.length ?? 0,
|
||||
payloadConfigOptionIds: (payload.config_options ?? []).map((o) => o.id),
|
||||
isEmpty: resolved.isEmpty,
|
||||
existingPopulated: resolved.populated,
|
||||
existingCurrentModelId: existing?.currentModelId ?? "",
|
||||
existingModelsLen: existing?.models.length ?? 0,
|
||||
existingConfigOptionIds: (existing?.configOptions ?? []).map((o) => o.id),
|
||||
willSkip: resolved.isEmpty && resolved.populated,
|
||||
preserveConfigOptions: resolved.preserveConfigOptions,
|
||||
});
|
||||
}
|
||||
|
||||
function clearStaleContextWindow(state: AppState, sessionId: string, currentModelId: string) {
|
||||
const previousModelId = state.sessionModels.bySessionId[sessionId]?.currentModelId ?? "";
|
||||
if (previousModelId && currentModelId && previousModelId !== currentModelId) {
|
||||
@@ -116,9 +189,37 @@ export function registerSessionModelsHandlers(store: StoreApi<AppState>): WsHand
|
||||
const matchesPersisted = payloadMatchesPersistedRuntime(payload, persisted);
|
||||
if (matchesPersisted) hydrated.add(sessionId);
|
||||
const pendingRuntime = matchesPersisted ? {} : persisted;
|
||||
const currentModelId = pendingRuntime.model || resolveCurrentModelId(payload);
|
||||
const payloadCurrentModelId = resolveCurrentModelId(payload);
|
||||
const currentModelId = pendingRuntime.model || payloadCurrentModelId;
|
||||
// Classify emptiness from the incoming payload alone; persisted metadata
|
||||
// must not make a genuinely empty relaunch event look populated.
|
||||
const isEmpty = isEmptyModelsUpdate(payloadCurrentModelId, acpModels, payload.config_options);
|
||||
const populated = hasPopulatedModels(state, sessionId);
|
||||
const preserveConfigOptions = shouldPreserveExistingConfigOptions(state, sessionId, payload);
|
||||
debugModelsUpdate(state, sessionId, payload, {
|
||||
currentModelId,
|
||||
isEmpty,
|
||||
populated,
|
||||
preserveConfigOptions,
|
||||
});
|
||||
if (isEmpty && populated) {
|
||||
return;
|
||||
}
|
||||
clearStaleContextWindow(state, sessionId, currentModelId);
|
||||
|
||||
const existingEntry = state.sessionModels.bySessionId[sessionId];
|
||||
const configOptions = preserveConfigOptions
|
||||
? existingEntry.configOptions
|
||||
: (payload.config_options ?? []).map((o) => ({
|
||||
type: o.type,
|
||||
id: o.id,
|
||||
name: o.name,
|
||||
description: o.description,
|
||||
currentValue: pendingRuntime.configOptions?.[o.id] ?? o.current_value,
|
||||
category: o.category,
|
||||
options: o.options,
|
||||
}));
|
||||
|
||||
state.setSessionModels(sessionId, {
|
||||
currentModelId,
|
||||
models: acpModels.map((m) => ({
|
||||
@@ -128,15 +229,7 @@ export function registerSessionModelsHandlers(store: StoreApi<AppState>): WsHand
|
||||
usageMultiplier: m.usage_multiplier,
|
||||
meta: m.meta,
|
||||
})),
|
||||
configOptions: (payload.config_options ?? []).map((o) => ({
|
||||
type: o.type,
|
||||
id: o.id,
|
||||
name: o.name,
|
||||
description: o.description,
|
||||
currentValue: pendingRuntime.configOptions?.[o.id] ?? o.current_value,
|
||||
category: o.category,
|
||||
options: o.options,
|
||||
})),
|
||||
configOptions,
|
||||
configBaseline:
|
||||
payload.config_baseline ??
|
||||
persisted.baseline ??
|
||||
|
||||
Reference in New Issue
Block a user