fix(ui): stop warm sidebar refreshes from blanking foreign-owned rows (#129558)

* fix(ui): stop warm sidebar refreshes from blanking foreign-owned rows

The owner-first roster plan (#128767) published its provisional owner-only
window as a full membership replacement on every primary refresh, so on
multi-owner gateways every other user's sessions blinked out of the sidebar
until the shared merge landed - up to once a second under event load. The
provisional phase now publishes only when no roster is on screen (cold start
keeps its fast first paint), and the shared phase merges the owner window
from the initial load's returned rows instead of reading published state;
the mergeExisting option is gone. A warm refresh whose shared phase fails
now keeps the previous roster instead of collapsing to owner-only rows.

The thread and child session lists also rendered rows positionally with
map()+keyed(), so any reorder tore down and rebuilt every shifted row's DOM
(spinners restarting, avatars remounting). Both now use repeat() with key
identity, matching the catalog renderer.

Regression coverage: owner-first-roster.test.ts pins the publish-sequence
invariant and shared-failure retention (both fail pre-fix); the new browser
test proves row DOM identity survives a reorder (fails on map()). The
owner-first tests moved out of index.event-refresh.test.ts, which hit the
max-lines limit.

* test(ui): prove foreign-owned rows survive a warm owner-first refresh

Browser-level regression for the warm-refresh half of this fix: holds the
shared phase deferred after a sessions.changed event and asserts the
foreign-owned row never leaves the DOM. Fails on pre-fix code (the row
count drops to zero the moment the provisional owner window publishes).

* test(anthropic): pin the local retired-profile id to the plugin-sdk constant

The provider-policy artifact keeps this id as a local literal so it never
imports the provider-auth barrel (#129052 regressed dist-less CI checkouts
into 120s jiti compiles of ~2.2k modules); this parity test stops the two
constants from drifting apart.
This commit is contained in:
Peter Steinberger
2026-08-25 16:38:55 -07:00
committed by GitHub
parent 6ad0f79ae4
commit e2c8dae785
6 changed files with 426 additions and 80 deletions
@@ -0,0 +1,12 @@
import { CLAUDE_CLI_PROFILE_ID as SDK_CLAUDE_CLI_PROFILE_ID } from "openclaw/plugin-sdk/provider-auth";
import { describe, expect, it } from "vitest";
import { CLAUDE_CLI_PROFILE_ID } from "./cli-constants.js";
describe("claude cli constants", () => {
it("keeps the local retired profile id aligned with the plugin-sdk constant", () => {
// The provider-policy artifact must stay light, so this plugin carries the
// retired profile id locally instead of importing the provider-auth barrel
// (#129052 regressed dist-less CI checkouts into 120s jiti compiles).
expect(CLAUDE_CLI_PROFILE_ID).toBe(SDK_CLAUDE_CLI_PROFILE_ID);
});
});
@@ -0,0 +1,48 @@
import { afterEach, describe, expect, it } from "vitest";
import type { GatewayBrowserClient } from "../api/gateway.ts";
import "../test-helpers/load-styles.ts";
afterEach(() => document.body.replaceChildren());
describe.runIf("__vitest_browser__" in globalThis)("sidebar session row DOM identity", () => {
it("moves existing row DOM when a new session shifts the list", async () => {
await import("./app-sidebar.ts");
const { createGatewayHarness, createSessionsHarness, createSessionState, mountSidebar } =
await import("../test-helpers/app-sidebar.ts");
const harness = createSessionsHarness("main", ["agent:main:alpha", "agent:main:beta"]);
const { sidebar } = await mountSidebar(
createGatewayHarness({ instanceId: "self-instance" } as GatewayBrowserClient).gateway,
harness.sessions,
);
sidebar.connected = true;
await sidebar.updateComplete;
const rowFor = (key: string) =>
sidebar.querySelector<HTMLElement>(`[data-session-tree="${key}"]`);
const alphaBefore = rowFor("agent:main:alpha");
const betaBefore = rowFor("agent:main:beta");
expect(alphaBefore).not.toBeNull();
expect(betaBefore).not.toBeNull();
// A newly created session sorts first (createdAt desc) and shifts every
// existing row's position; keyed reuse must move their DOM, not rebuild it.
const next = createSessionState("main", [
"agent:main:alpha",
"agent:main:beta",
"agent:main:gamma",
]);
const gamma = next.result?.sessions.find((row) => row.key === "agent:main:gamma");
if (gamma) {
gamma.createdAt = Date.now();
}
harness.publishList(next);
await sidebar.updateComplete;
const rowKeys = Array.from(sidebar.querySelectorAll("[data-session-tree]")).map((row) =>
row.getAttribute("data-session-tree"),
);
expect(rowKeys[0]).toBe("agent:main:gamma");
expect(rowFor("agent:main:alpha")).toBe(alphaBefore);
expect(rowFor("agent:main:beta")).toBe(betaBefore);
});
});
@@ -0,0 +1,117 @@
import { mkdir } from "node:fs/promises";
import path from "node:path";
import type { Page } from "playwright";
import { expect, it } from "vitest";
import { installMockGateway } from "../test-helpers/control-ui-e2e.ts";
import { createControlUiE2eSuite } from "./control-ui-e2e-suite.test-support.ts";
const suite = createControlUiE2eSuite({ name: "Control UI warm owner-first refresh" });
const captureProof = process.env.OPENCLAW_CAPTURE_UI_PROOF === "1";
const proofDir = path.join(process.cwd(), ".artifacts", "control-ui-e2e", "session-owner-warm");
function sessionRow(ownerId: string, key: string, label: string, updatedAt: number) {
const owner = {
type: "human" as const,
id: ownerId,
label: ownerId === "profile-ada" ? "Ada" : "Bob",
};
return {
key,
kind: "direct" as const,
label,
createdActor: owner,
owner: { actor: owner },
updatedAt,
};
}
function rosterOf(sessions: ReturnType<typeof sessionRow>[]) {
return {
count: sessions.length,
owners: sessions.map((session) => session.owner.actor),
defaults: { contextTokens: null, model: null, modelProvider: null },
path: "",
sessions,
ts: 1,
};
}
async function captureSidebar(page: Page, fileName: string) {
if (!captureProof) {
return;
}
await mkdir(proofDir, { recursive: true });
await page.locator(".sidebar-sessions").screenshot({
animations: "disabled",
path: path.join(proofDir, fileName),
});
}
suite.define(() => {
it("keeps foreign-owned rows visible while a warm refresh's shared phase is in flight", async () => {
const context = await suite.browser.newContext({
viewport: { height: 800, width: 1200 },
...(captureProof
? { recordVideo: { dir: proofDir, size: { height: 800, width: 1200 } } }
: {}),
});
const page = await context.newPage();
const adaRow = sessionRow("profile-ada", "agent:main:ada", "Ada research", 2);
const bobRow = sessionRow("profile-bob", "agent:main:bob", "Bob operations", 1);
const sharedRoster = rosterOf([adaRow, bobRow]);
const ownerRoster = rosterOf([adaRow]);
const gateway = await installMockGateway(page, {
presenceUsers: [{ self: true, id: "profile-ada", name: "Ada" }],
sessionKey: "agent:main:ada",
methodResponses: {
"sessions.list": {
cases: [
{ match: { ownerId: "profile-ada" }, response: ownerRoster },
{ response: sharedRoster },
],
},
},
});
try {
await page.goto(`${suite.server?.baseUrl ?? ""}chat`);
const ada = page.locator('[data-session-key="agent:main:ada"]');
const bob = page.locator('[data-session-key="agent:main:bob"]');
await ada.waitFor();
await bob.waitFor();
await captureSidebar(page, "warm-before-event.png");
// Hold the warm refresh open at its vulnerable point: the owner-scoped
// request resolves instantly (mocked response), the shared request stays
// deferred. Pre-#129558 the owner-only publish blanks Bob's row here.
const before = (await gateway.getRequests("sessions.list")).length;
await gateway.deferNext("sessions.list", { ownerId: "profile-ada" });
await gateway.deferNext("sessions.list");
await gateway.emitGatewayEvent("sessions.changed", {
sessionKey: adaRow.key,
key: adaRow.key,
kind: "direct",
reason: "create",
updatedAt: 3,
});
await gateway.waitForRequest("sessions.list", { after: before + 1 });
await gateway.resolveDeferred("sessions.list", ownerRoster);
// The shared phase is still pending; the roster on screen must not shrink
// to the owner window. Sample repeatedly so a transient blank fails loud.
for (let sample = 0; sample < 6; sample += 1) {
await page.waitForTimeout(100);
expect(await bob.count()).toBe(1);
expect(await ada.count()).toBe(1);
}
await captureSidebar(page, "warm-shared-deferred.png");
await gateway.resolveDeferred("sessions.list", sharedRoster);
await expect.poll(() => bob.count()).toBe(1);
expect(await ada.count()).toBe(1);
await captureSidebar(page, "warm-after-merge.png");
} finally {
await context.close();
}
});
});
@@ -583,71 +583,6 @@ describe("event-driven session list refresh", () => {
}
});
it("retains owner and appended shared pages when an event replaces the list", async () => {
vi.useFakeTimers();
const ownerId = "profile-ada";
const ownerTail = {
key: "agent:main:owner-tail",
kind: "direct" as const,
updatedAt: 1,
createdActor: { type: "human" as const, id: ownerId },
};
const ownerHead = {
key: "agent:main:owner-head",
kind: "direct" as const,
updatedAt: 3,
createdActor: { type: "human" as const, id: ownerId },
};
const sharedRows = [
ownerHead,
...Array.from({ length: 119 }, (_, index) => ({
key: `agent:main:shared-${index}`,
kind: "direct" as const,
updatedAt: 119 - index,
createdActor: { type: "human" as const, id: "profile-bob" },
})),
];
const request = vi.fn(
async (method: string, params?: { limit?: number; offset?: number; ownerId?: string }) => {
if (method !== "sessions.list") {
throw new Error(`Unexpected request: ${method}`);
}
if (params?.ownerId === ownerId) {
return sessionsResult(1, [ownerHead, ownerTail]);
}
const offset = params?.offset ?? 0;
return sessionsResult(2, sharedRows.slice(offset, offset + (params?.limit ?? 50)));
},
);
const { sessions, emitEvent } = createHarness(
request as unknown as GatewayBrowserClient["request"],
ownerId,
);
try {
await sessions.refresh({ agentId: "main", limit: 60, force: true });
await sessions.refresh({ agentId: "main", limit: 60, offset: 60, append: true, force: true });
expect(sessions.state.result?.sessions).toHaveLength(121);
emitEvent(sessionChangedEvent(sharedRows[1]!.key));
await vi.advanceTimersByTimeAsync(SESSION_EVENT_REFRESH_DEBOUNCE_MS);
expect(request.mock.calls).toHaveLength(5);
expect(request.mock.calls.map(([, params]) => params)).toEqual([
expect.objectContaining({ ownerId, limit: 60 }),
expect.objectContaining({ limit: 60 }),
expect.objectContaining({ limit: 60, offset: 60 }),
expect.objectContaining({ ownerId, limit: 60 }),
expect.objectContaining({ limit: 120 }),
]);
expect(sessions.state.result?.sessions).toHaveLength(121);
expect(sessions.state.result?.sessions.map((row) => row.key)).toContain(ownerTail.key);
} finally {
sessions.dispose();
vi.useRealTimers();
}
});
it("clears a recreated session's prior deletion before the debounced refresh", async () => {
vi.useFakeTimers();
const key = "agent:main:recreated-thread";
@@ -0,0 +1,224 @@
// @vitest-environment node
import { describe, expect, it, vi } from "vitest";
import type { GatewayBrowserClient, GatewayEventFrame } from "../../api/gateway.ts";
import type { SessionsListResult } from "../../api/types.ts";
import { createSessionCapability } from "./index.ts";
const SESSION_EVENT_REFRESH_DEBOUNCE_MS = 200;
function sessionsResult(
ts: number,
sessions: SessionsListResult["sessions"] = [],
): SessionsListResult {
return {
ts,
path: "",
count: sessions.length,
defaults: { modelProvider: null, model: null, contextTokens: null },
sessions,
};
}
function sessionChangedEvent(key: string): GatewayEventFrame {
return {
type: "event",
event: "sessions.changed",
payload: { sessionKey: key, reason: "create", key, kind: "direct", updatedAt: 1 },
};
}
function createHarness(request: GatewayBrowserClient["request"], ownerId: string) {
const client = { request } as GatewayBrowserClient;
let eventListener: ((event: GatewayEventFrame) => void) | undefined;
const sessions = createSessionCapability({
snapshot: {
client,
phase: "connected",
sessionKey: "agent:main:main",
assistantAgentId: "main",
hello: null,
selfUser: { id: ownerId },
},
subscribe: () => () => undefined,
subscribeEvents(listener) {
eventListener = listener;
return () => {
eventListener = undefined;
};
},
});
return { sessions, emitEvent: (event: GatewayEventFrame) => eventListener?.(event) };
}
describe("owner-first session roster plan", () => {
it("retains owner and appended shared pages when an event replaces the list", async () => {
vi.useFakeTimers();
const ownerId = "profile-ada";
const ownerTail = {
key: "agent:main:owner-tail",
kind: "direct" as const,
updatedAt: 1,
createdActor: { type: "human" as const, id: ownerId },
};
const ownerHead = {
key: "agent:main:owner-head",
kind: "direct" as const,
updatedAt: 3,
createdActor: { type: "human" as const, id: ownerId },
};
const sharedRows = [
ownerHead,
...Array.from({ length: 119 }, (_, index) => ({
key: `agent:main:shared-${index}`,
kind: "direct" as const,
updatedAt: 119 - index,
createdActor: { type: "human" as const, id: "profile-bob" },
})),
];
const request = vi.fn(
async (method: string, params?: { limit?: number; offset?: number; ownerId?: string }) => {
if (method !== "sessions.list") {
throw new Error(`Unexpected request: ${method}`);
}
if (params?.ownerId === ownerId) {
return sessionsResult(1, [ownerHead, ownerTail]);
}
const offset = params?.offset ?? 0;
return sessionsResult(2, sharedRows.slice(offset, offset + (params?.limit ?? 50)));
},
);
const { sessions, emitEvent } = createHarness(
request as unknown as GatewayBrowserClient["request"],
ownerId,
);
try {
await sessions.refresh({ agentId: "main", limit: 60, force: true });
await sessions.refresh({ agentId: "main", limit: 60, offset: 60, append: true, force: true });
expect(sessions.state.result?.sessions).toHaveLength(121);
emitEvent(sessionChangedEvent(sharedRows[1]!.key));
await vi.advanceTimersByTimeAsync(SESSION_EVENT_REFRESH_DEBOUNCE_MS);
expect(request.mock.calls).toHaveLength(5);
expect(request.mock.calls.map(([, params]) => params)).toEqual([
expect.objectContaining({ ownerId, limit: 60 }),
expect.objectContaining({ limit: 60 }),
expect.objectContaining({ limit: 60, offset: 60 }),
expect.objectContaining({ ownerId, limit: 60 }),
expect.objectContaining({ limit: 120 }),
]);
expect(sessions.state.result?.sessions).toHaveLength(121);
expect(sessions.state.result?.sessions.map((row) => row.key)).toContain(ownerTail.key);
} finally {
sessions.dispose();
vi.useRealTimers();
}
});
it("keeps foreign-owned rows published through a warm owner-first refresh", async () => {
vi.useFakeTimers();
const ownerId = "profile-ada";
const ownRow = {
key: "agent:main:ada",
kind: "direct" as const,
updatedAt: 2,
createdActor: { type: "human" as const, id: ownerId },
};
const foreignRow = {
key: "agent:main:bob",
kind: "direct" as const,
updatedAt: 1,
createdActor: { type: "human" as const, id: "profile-bob" },
};
const request = vi.fn(async (method: string, params?: { ownerId?: string }) => {
if (method !== "sessions.list") {
throw new Error(`Unexpected request: ${method}`);
}
return params?.ownerId === ownerId
? sessionsResult(1, [ownRow])
: sessionsResult(2, [ownRow, foreignRow]);
});
const { sessions, emitEvent } = createHarness(
request as unknown as GatewayBrowserClient["request"],
ownerId,
);
try {
await sessions.refresh({ agentId: "main", limit: 60, force: true });
expect(sessions.state.result?.sessions.map((row) => row.key)).toContain(foreignRow.key);
const publishedKeySets: string[][] = [];
const stop = sessions.subscribe((next) => {
if (next.result) {
publishedKeySets.push(next.result.sessions.map((row) => row.key));
}
});
emitEvent(sessionChangedEvent(ownRow.key));
await vi.advanceTimersByTimeAsync(SESSION_EVENT_REFRESH_DEBOUNCE_MS);
stop();
// Cold start fired owner + shared; the warm event refresh fired both again.
expect(request.mock.calls).toHaveLength(4);
expect(publishedKeySets.length).toBeGreaterThan(0);
for (const keys of publishedKeySets) {
expect(keys).toContain(foreignRow.key);
}
} finally {
sessions.dispose();
vi.useRealTimers();
}
});
it("keeps the previous roster when the shared phase of a warm refresh fails", async () => {
vi.useFakeTimers();
const ownerId = "profile-ada";
const ownRow = {
key: "agent:main:ada",
kind: "direct" as const,
updatedAt: 2,
createdActor: { type: "human" as const, id: ownerId },
};
const foreignRow = {
key: "agent:main:bob",
kind: "direct" as const,
updatedAt: 1,
createdActor: { type: "human" as const, id: "profile-bob" },
};
let failShared = false;
const request = vi.fn(async (method: string, params?: { ownerId?: string }) => {
if (method !== "sessions.list") {
throw new Error(`Unexpected request: ${method}`);
}
if (params?.ownerId === ownerId) {
return sessionsResult(1, [ownRow]);
}
if (failShared) {
throw new Error("shared roster unavailable");
}
return sessionsResult(2, [ownRow, foreignRow]);
});
const { sessions, emitEvent } = createHarness(
request as unknown as GatewayBrowserClient["request"],
ownerId,
);
try {
await sessions.refresh({ agentId: "main", limit: 60, force: true });
expect(sessions.state.result?.sessions).toHaveLength(2);
failShared = true;
emitEvent(sessionChangedEvent(ownRow.key));
await vi.advanceTimersByTimeAsync(SESSION_EVENT_REFRESH_DEBOUNCE_MS);
expect(sessions.state.error).not.toBeNull();
expect(sessions.state.result?.sessions.map((row) => row.key)).toEqual([
ownRow.key,
foreignRow.key,
]);
} finally {
sessions.dispose();
vi.useRealTimers();
}
});
});
+25 -15
View File
@@ -42,7 +42,6 @@ type ManagedSessionListRefresh = {
};
type SessionRosterLoadOptions = SessionRefreshOptions & {
mergeExisting?: boolean;
provisional?: boolean;
};
@@ -231,16 +230,18 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) {
}
};
const load = async (options: SessionRosterLoadOptions, publishAfter?: Promise<void>) => {
const load = async (
options: SessionRosterLoadOptions,
ownerFirst?: Promise<SessionsListResult | null>,
): Promise<SessionsListResult | null> => {
const scope = host.connection.capture();
if (!scope) {
return;
return null;
}
const {
append = false,
force: _force,
backgroundHydrate = false,
mergeExisting = false,
provisional = false,
...requestOptions
} = options;
@@ -257,7 +258,10 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) {
lastListOptions = durableListOptions;
hasSeededListOptions = true;
}
if (!backgroundHydrate) {
// A provisional owner window may only paint an empty sidebar faster; once a
// roster is on screen it stays silent so foreign-owned rows never blink out.
const provisionalSilent = provisional && Boolean(host.readState().result);
if (!backgroundHydrate && !provisionalSilent) {
const error = host.observerError();
host.publish(
{ ...host.readState(), loading: true, error, deletedSessions: [] },
@@ -266,18 +270,21 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) {
}
try {
const request = requestSessionList(scope.client, requestOptions);
await Promise.allSettled([request, publishAfter]);
const ownerRows = ownerFirst ? await ownerFirst.catch(() => null) : null;
const result = await request;
if (!host.connection.isCurrent(scope)) {
return;
return null;
}
const currentState = host.readState();
const mergeWithCurrent =
mergeExisting || (append && typeof requestOptions.offset === "number");
if (provisional && currentState.result) {
return result;
}
const merged = result && ownerRows ? appendSessionResults(ownerRows, result) : result;
const mergeWithCurrent = append && typeof requestOptions.offset === "number";
let nextResult =
result && mergeWithCurrent && currentState.result
? appendSessionResults(currentState.result, result)
: reconcileRosterPresentationMetadata(result, currentState.result);
merged && mergeWithCurrent && currentState.result
? appendSessionResults(currentState.result, merged)
: reconcileRosterPresentationMetadata(merged, currentState.result);
if (append && nextResult && !backgroundHydrate) {
const ownerFirstPage =
Boolean(host.snapshot().selfUser?.id.trim()) &&
@@ -347,8 +354,9 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) {
},
error ? "session-observer" : undefined,
);
return result;
} catch (error) {
if (host.connection.isCurrent(scope)) {
if (host.connection.isCurrent(scope) && !(provisional && host.readState().result)) {
const state = host.readState();
host.publish(
{
@@ -360,6 +368,7 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) {
"operation",
);
}
return null;
}
};
@@ -400,7 +409,9 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) {
: DEFAULT_SESSION_LIST_QUERY.limit,
);
// Keep owner-first and shared loads atomic in the existing refresh queue.
// Only the shared phase advances canonical membership and durable options.
// Only the shared phase advances canonical membership and durable options;
// it merges the owner window from the initial load's returned rows, so the
// provisional phase never has to publish to be part of the final roster.
return {
initial: {
...options,
@@ -411,7 +422,6 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) {
shared: {
...options,
limit: sharedLimit,
mergeExisting: true,
},
};
};