fix(ui): preserve sidebar collapsed state when group rename/delete fails (#106152)

* fix(ui): preserve sidebar collapsed state when group rename/delete fails

* test(ui): cover group rename/delete rejection error publish

* fix(ui): harden session group mutations

* fix(ui): guard group mutations across reconnects

* fix(ui): keep session group result internal

* style(ui): format sidebar tests

* fix(ui): preserve confirmed group mutations

* chore: keep release notes out of contributor PR

* style(ui): refresh session test formatting

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
wuqxuan
2026-07-13 21:13:58 +08:00
committed by GitHub
parent 93b3f4c45b
commit 94f018caad
5 changed files with 394 additions and 38 deletions

View File

@@ -26,6 +26,8 @@ import {
type LobsterLogoVisitDetail,
} from "./lobster-pet.ts";
type SessionGroupMutationResult = Awaited<ReturnType<SessionCapability["groupsRename"]>>;
// Keep the attention widget inert: it fires its own health RPCs (cron.list,
// models.authStatus) on connect, which would interleave with the nth-call
// assertions on the shared mocked client below. It has its own test file.
@@ -140,6 +142,8 @@ function createSessionsHarness(agentId: string, keys: string[]) {
let canonicalListRevision = 1;
const listeners = new Set<(next: SessionState) => void>();
const groupsPut = vi.fn(() => Promise.resolve());
const groupsRename = vi.fn(() => Promise.resolve<SessionGroupMutationResult>("completed"));
const groupsDelete = vi.fn(() => Promise.resolve<SessionGroupMutationResult>("completed"));
const patch = vi.fn(() => Promise.resolve(null));
const deleteMany = vi.fn(() =>
Promise.resolve({ deleted: [] as string[], errors: [], preservedWorktrees: [] }),
@@ -158,6 +162,8 @@ function createSessionsHarness(agentId: string, keys: string[]) {
subscribeCreated: () => () => undefined,
groupsLoad: () => Promise.resolve(),
groupsPut,
groupsRename,
groupsDelete,
patch,
deleteMany,
refresh: () => Promise.resolve(),
@@ -171,6 +177,8 @@ function createSessionsHarness(agentId: string, keys: string[]) {
return {
sessions,
groupsPut,
groupsRename,
groupsDelete,
patch,
deleteMany,
publish,
@@ -1794,7 +1802,6 @@ describe("AppSidebar custom group reordering", () => {
expect(harness.groupsPut).toHaveBeenCalledWith(["Gamma", "Alpha", "Beta"]);
});
});
describe("AppSidebar catalog session rows", () => {
const catalogList = (
sessions: Array<Record<string, unknown>>,
@@ -1979,3 +1986,130 @@ describe("AppSidebar catalog session rows", () => {
}
});
});
describe("AppSidebar group mutation collapsed state", () => {
const COLLAPSED_STORAGE_KEY = "openclaw:sidebar:sessions:collapsed-sections";
async function mountCollapsedGroup(options: {
groupsRename?: () => Promise<SessionGroupMutationResult>;
groupsDelete?: () => Promise<SessionGroupMutationResult>;
}) {
localStorage.setItem(COLLAPSED_STORAGE_KEY, JSON.stringify(["category:Alpha"]));
const gatewayHarness = createGatewayHarness({} as GatewayBrowserClient);
const harness = createSessionsHarness("main", ["agent:main:main"]);
if (options.groupsRename) {
harness.groupsRename.mockImplementation(options.groupsRename);
}
if (options.groupsDelete) {
harness.groupsDelete.mockImplementation(options.groupsDelete);
}
const { sidebar } = await mountSidebar(gatewayHarness.gateway, harness.sessions);
sidebar.connected = true;
harness.publish({ groups: ["Alpha"] });
await sidebar.updateComplete;
return { sidebar, harness, gatewayHarness };
}
async function openGroupMenu(sidebar: SidebarLifecycleState) {
const actions = sidebar.querySelector<HTMLButtonElement>(
'[data-session-section="category:Alpha"] .sidebar-session-group-actions',
);
if (!actions) {
throw new Error("expected group actions trigger");
}
actions.click();
await sidebar.updateComplete;
const menu = sidebar.querySelector(".sidebar-session-group-menu");
if (!menu) {
throw new Error("expected group menu");
}
return menu;
}
it("keeps collapsed keys when group rename is rejected", async () => {
const { sidebar, harness } = await mountCollapsedGroup({
groupsRename: () => Promise.reject(new Error("rename failed")),
});
const promptSpy = vi.spyOn(window, "prompt").mockReturnValue("Beta");
const menu = await openGroupMenu(sidebar);
const rename = menu.querySelectorAll<HTMLButtonElement>(".session-menu__item")[0];
rename?.click();
await vi.waitFor(() => expect(harness.groupsRename).toHaveBeenCalledWith("Alpha", "Beta"));
await Promise.resolve();
await Promise.resolve();
expect(localStorage.getItem(COLLAPSED_STORAGE_KEY)).toBe(JSON.stringify(["category:Alpha"]));
promptSpy.mockRestore();
});
it("rewrites collapsed keys only after group rename succeeds", async () => {
const { sidebar, harness } = await mountCollapsedGroup({
groupsRename: () => Promise.resolve("completed"),
});
const promptSpy = vi.spyOn(window, "prompt").mockReturnValue("Beta");
const menu = await openGroupMenu(sidebar);
menu.querySelectorAll<HTMLButtonElement>(".session-menu__item")[0]?.click();
await vi.waitFor(() => expect(harness.groupsRename).toHaveBeenCalledWith("Alpha", "Beta"));
await Promise.resolve();
await Promise.resolve();
expect(JSON.parse(localStorage.getItem(COLLAPSED_STORAGE_KEY) ?? "[]")).toEqual([
"category:Beta",
]);
promptSpy.mockRestore();
});
it("ignores a stale group rename after its Gateway reconnects with the same client", async () => {
let resolveRename!: (result: SessionGroupMutationResult) => void;
const rename = new Promise<SessionGroupMutationResult>((resolve) => {
resolveRename = resolve;
});
const { sidebar, harness, gatewayHarness } = await mountCollapsedGroup({
groupsRename: () => rename,
});
const promptSpy = vi.spyOn(window, "prompt").mockReturnValue("Beta");
const menu = await openGroupMenu(sidebar);
menu.querySelectorAll<HTMLButtonElement>(".session-menu__item")[0]?.click();
await vi.waitFor(() => expect(harness.groupsRename).toHaveBeenCalledWith("Alpha", "Beta"));
gatewayHarness.publish({ connected: false });
gatewayHarness.publish({ connected: true });
resolveRename("stale");
await Promise.resolve();
await Promise.resolve();
expect(localStorage.getItem(COLLAPSED_STORAGE_KEY)).toBe(JSON.stringify(["category:Alpha"]));
promptSpy.mockRestore();
});
it("keeps collapsed keys when group delete is rejected", async () => {
const { sidebar, harness } = await mountCollapsedGroup({
groupsDelete: () => Promise.reject(new Error("delete failed")),
});
const confirmSpy = vi.spyOn(window, "confirm").mockReturnValue(true);
const menu = await openGroupMenu(sidebar);
const items = menu.querySelectorAll<HTMLButtonElement>(".session-menu__item");
items[items.length - 1]?.click();
await vi.waitFor(() => expect(harness.groupsDelete).toHaveBeenCalledWith("Alpha"));
await Promise.resolve();
await Promise.resolve();
expect(localStorage.getItem(COLLAPSED_STORAGE_KEY)).toBe(JSON.stringify(["category:Alpha"]));
confirmSpy.mockRestore();
});
it("drops collapsed keys only after group delete succeeds", async () => {
const { sidebar, harness } = await mountCollapsedGroup({
groupsDelete: () => Promise.resolve("completed"),
});
const confirmSpy = vi.spyOn(window, "confirm").mockReturnValue(true);
const menu = await openGroupMenu(sidebar);
const items = menu.querySelectorAll<HTMLButtonElement>(".session-menu__item");
items[items.length - 1]?.click();
await vi.waitFor(() => expect(harness.groupsDelete).toHaveBeenCalledWith("Alpha"));
await Promise.resolve();
await Promise.resolve();
expect(JSON.parse(localStorage.getItem(COLLAPSED_STORAGE_KEY) ?? "[]")).toEqual([]);
confirmSpy.mockRestore();
});
});

View File

@@ -1616,40 +1616,65 @@ class AppSidebar extends OpenClawLightDomContentsElement {
private renameSessionGroupFromMenu(group: string) {
const context = this.context;
if (!context || !this.connected) {
const client = context?.gateway.snapshot.client;
if (!context || !client || !this.connected || !context.gateway.snapshot.connected) {
return;
}
const next = window.prompt(t("sessionsView.renameGroupPrompt"), group)?.trim();
if (!next || next === group) {
return;
}
// The gateway renames the catalog entry and repoints member sessions.
void context.sessions.groupsRename(group, next).finally(() => {
const from = `category:${group}`;
if (this.collapsedSessionSections.has(from)) {
const collapsed = new Set(this.collapsedSessionSections);
collapsed.delete(from);
collapsed.add(`category:${next}`);
this.saveCollapsedSessionSections(collapsed);
}
this.requestUpdate();
});
// Collapse keys follow the confirmed Gateway rename only and stay scoped
// to the connection that issued it; stale completions must not rewrite storage.
void context.sessions.groupsRename(group, next).then(
(outcome) => {
if (outcome !== "completed" || !this.isCurrentSessionGroupMutation(context, client)) {
return;
}
const from = `category:${group}`;
if (this.collapsedSessionSections.has(from)) {
const collapsed = new Set(this.collapsedSessionSections);
collapsed.delete(from);
collapsed.add(`category:${next}`);
this.saveCollapsedSessionSections(collapsed);
}
this.requestUpdate();
},
() => undefined,
);
}
private deleteSessionGroupFromMenu(group: string) {
const context = this.context;
if (!context || !this.connected) {
const client = context?.gateway.snapshot.client;
if (!context || !client || !this.connected || !context.gateway.snapshot.connected) {
return;
}
if (!window.confirm(t("sessionsView.deleteGroupConfirm", { group }))) {
return;
}
void context.sessions.groupsDelete(group).finally(() => {
const collapsed = new Set(this.collapsedSessionSections);
collapsed.delete(`category:${group}`);
this.saveCollapsedSessionSections(collapsed);
this.requestUpdate();
});
void context.sessions.groupsDelete(group).then(
(outcome) => {
if (outcome !== "completed" || !this.isCurrentSessionGroupMutation(context, client)) {
return;
}
const collapsed = new Set(this.collapsedSessionSections);
collapsed.delete(`category:${group}`);
this.saveCollapsedSessionSections(collapsed);
this.requestUpdate();
},
() => undefined,
);
}
private isCurrentSessionGroupMutation(
context: ApplicationContext<RouteId>,
client: GatewayBrowserClient,
) {
const snapshot = context.gateway.snapshot;
return (
this.context === context && this.connected && snapshot.connected && snapshot.client === client
);
}
private saveCollapsedSessionSections(sections: ReadonlySet<string>) {

View File

@@ -17,6 +17,7 @@ const chromiumExecutablePath = resolvePlaywrightChromiumExecutablePath(chromium.
const chromiumAvailable = canRunPlaywrightChromium(chromiumExecutablePath);
const allowMissingChromium = process.env.OPENCLAW_UI_E2E_ALLOW_MISSING_CHROMIUM === "1";
const describeControlUiE2e = chromiumAvailable || !allowMissingChromium ? describe : describe.skip;
const collapsedSessionSectionsStorageKey = "openclaw:sidebar:sessions:collapsed-sections";
let browser: Browser;
let server: ControlUiE2eServer;
@@ -823,6 +824,85 @@ describeControlUiE2e("Control UI session management mocked Gateway E2E", () => {
}
});
it("preserves a collapsed sidebar group when its rename is rejected", async () => {
const baseTime = Date.parse("2026-07-01T16:00:00.000Z");
const context = await browser.newContext({
locale: "en-US",
serviceWorkers: "block",
viewport: { height: 900, width: 1280 },
});
const page = await context.newPage();
await page.addInitScript(
({ key, value }) => {
try {
if (localStorage.getItem(key) === null) {
localStorage.setItem(key, value);
}
} catch {
// The opaque initial document has no storage; the app origin does.
}
},
{
key: collapsedSessionSectionsStorageKey,
value: JSON.stringify(["category:Research"]),
},
);
const gateway = await installMockGateway(page, {
deferredMethods: ["sessions.groups.rename"],
featureMethods: ["chat.metadata", "chat.startup", "sessions.groups.list"],
methodResponses: {
"sessions.list": sessionsListResponse([
sessionRow("agent:main:main", "Main", baseTime),
sessionRow("agent:main:paper", "Paper", baseTime - 60_000, {
category: "Research",
}),
]),
},
sessionGroups: ["Research"],
sessionKey: "agent:main:main",
});
const pageErrors: string[] = [];
page.on("pageerror", (error) => pageErrors.push(error.message));
try {
await page.goto(`${server.baseUrl}chat`);
const researchGroup = page.locator('[data-session-section="category:Research"]');
await researchGroup.waitFor({ state: "visible", timeout: 10_000 });
await expect.poll(() => researchGroup.locator(".sidebar-recent-session").count()).toBe(0);
await researchGroup.locator(".sidebar-recent-sessions__head").hover();
await researchGroup.getByRole("button", { name: "Group options for Research" }).click();
page.once("dialog", (dialog) => void dialog.accept("Projects"));
await page.getByRole("menuitem", { name: "Rename group…" }).click();
await gateway.waitForRequest("sessions.groups.rename");
await gateway.rejectDeferred("sessions.groups.rename", {
code: "INVALID_REQUEST",
message: "rejected group rename",
});
await page.evaluate(
() =>
new Promise<void>((resolve) => {
requestAnimationFrame(() => requestAnimationFrame(() => resolve()));
}),
);
expect(
await page.evaluate((key) => localStorage.getItem(key), collapsedSessionSectionsStorageKey),
).toBe(JSON.stringify(["category:Research"]));
await researchGroup.waitFor({ state: "visible" });
expect(await page.locator('[data-session-section="category:Projects"]').count()).toBe(0);
expect(pageErrors).toEqual([]);
await page.reload();
await researchGroup.waitFor({ state: "visible", timeout: 10_000 });
await expect.poll(() => researchGroup.locator(".sidebar-recent-session").count()).toBe(0);
expect(
await page.evaluate((key) => localStorage.getItem(key), collapsedSessionSectionsStorageKey),
).toBe(JSON.stringify(["category:Research"]));
} finally {
await context.close();
}
});
it("pages sidebar sessions and supports complete drag-managed groups", async () => {
const baseTime = Date.parse("2026-07-01T16:00:00.000Z");
const context = await browser.newContext({

View File

@@ -160,6 +160,99 @@ describe("createSessionCapability", () => {
sessions.dispose();
});
it("publishes state.error when group rename is rejected", async () => {
const request = vi.fn(async (method: string) => {
if (method === "sessions.groups.rename") {
throw new Error("rename failed");
}
throw new Error(`Unexpected request: ${method}`);
});
const client = { request } as unknown as GatewayBrowserClient;
const { gateway } = createGatewayHarness(client, ["sessions.groups.rename"]);
const sessions = createSessionCapability(gateway);
await expect(sessions.groupsRename("Alpha", "Beta")).rejects.toThrow("rename failed");
expect(sessions.state.error).toBe("Error: rename failed");
sessions.dispose();
});
it("publishes state.error when group delete is rejected", async () => {
const request = vi.fn(async (method: string) => {
if (method === "sessions.groups.delete") {
throw new Error("delete failed");
}
throw new Error(`Unexpected request: ${method}`);
});
const client = { request } as unknown as GatewayBrowserClient;
const { gateway } = createGatewayHarness(client, ["sessions.groups.delete"]);
const sessions = createSessionCapability(gateway);
await expect(sessions.groupsDelete("Alpha")).rejects.toThrow("delete failed");
expect(sessions.state.error).toBe("Error: delete failed");
sessions.dispose();
});
it("reports a group rename as stale after a same-client reconnect", async () => {
const renamed = deferred<{ groups: Array<{ name: string }> }>();
const request = vi.fn(async (method: string) => {
if (method === "sessions.groups.rename") {
return await renamed.promise;
}
if (method === "sessions.subscribe") {
return {};
}
if (method === "sessions.list") {
return sessionsResult([], 2);
}
throw new Error(`Unexpected request: ${method}`);
});
const client = { request } as unknown as GatewayBrowserClient;
const { gateway, publish } = createGatewayHarness(client, ["sessions.groups.rename"]);
const sessions = createSessionCapability(gateway);
const operation = sessions.groupsRename("Alpha", "Beta");
publish(false);
publish(true);
renamed.resolve({ groups: [{ name: "Beta" }] });
await expect(operation).resolves.toBe("stale");
expect(sessions.state.groups).toEqual([]);
sessions.dispose();
});
it.each(["rename", "delete"] as const)(
"keeps a confirmed group %s completed when its row refresh outlives the connection",
async (operation) => {
const refreshed = deferred<SessionsListResult>();
const method = operation === "rename" ? "sessions.groups.rename" : "sessions.groups.delete";
const request = vi.fn(async (requestedMethod: string) => {
if (requestedMethod === method) {
return { groups: [{ name: operation === "rename" ? "Beta" : "Other" }] };
}
if (requestedMethod === "sessions.list") {
return await refreshed.promise;
}
throw new Error(`Unexpected request: ${requestedMethod}`);
});
const client = { request } as unknown as GatewayBrowserClient;
const { gateway, publish } = createGatewayHarness(client, [method]);
const sessions = createSessionCapability(gateway);
const mutation =
operation === "rename"
? sessions.groupsRename("Alpha", "Beta")
: sessions.groupsDelete("Alpha");
await vi.waitFor(() =>
expect(request).toHaveBeenCalledWith("sessions.list", expect.any(Object)),
);
publish(false);
refreshed.resolve(sessionsResult([], 2));
await expect(mutation).resolves.toBe("completed");
sessions.dispose();
},
);
it("does not probe for a group catalog when the method is explicitly absent", async () => {
const request = vi.fn();
const client = { request } as unknown as GatewayBrowserClient;

View File

@@ -62,6 +62,8 @@ export type SessionState = {
groups: readonly string[];
};
type SessionGroupMutationResult = "completed" | "stale";
export type SessionListOptions = {
agentId?: string;
activeMinutes?: number;
@@ -240,10 +242,10 @@ export type SessionCapability = {
groupsLoad: () => Promise<void>;
/** Replaces the gateway-owned group catalog (order included). */
groupsPut: (names: readonly string[]) => Promise<void>;
/** Renames a group; the gateway repoints member sessions server-side. */
groupsRename: (from: string, to: string) => Promise<void>;
/** Deletes a group; the gateway clears member categories server-side. */
groupsDelete: (name: string) => Promise<void>;
/** Renames a group; stale means the initiating connection retired before reconciliation. */
groupsRename: (from: string, to: string) => Promise<SessionGroupMutationResult>;
/** Deletes a group; stale means the initiating connection retired before reconciliation. */
groupsDelete: (name: string) => Promise<SessionGroupMutationResult>;
subscribeCreated: (listener: (key: string) => void) => () => void;
subscribe: (listener: (state: SessionState) => void) => () => void;
dispose: () => void;
@@ -994,30 +996,52 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil
}
};
const groupsRename = async (from: string, to: string) => {
const groupsRename = async (from: string, to: string): Promise<SessionGroupMutationResult> => {
const scope = captureConnection();
if (!scope) {
return;
return "stale";
}
const result = await scope.client.request("sessions.groups.rename", { name: from, to });
if (!isCurrentConnection(scope)) {
return;
try {
const result = await scope.client.request("sessions.groups.rename", { name: from, to });
if (!isCurrentConnection(scope)) {
return "stale";
}
publishGroups(readGroupNames(result));
// The mutation response is the commit point. Reconcile member rows in
// the background so a later disconnect cannot downgrade confirmed work.
void refresh({ ...lastListOptions, force: true });
return "completed";
} catch (error) {
if (!isCurrentConnection(scope)) {
return "stale";
}
publish({ ...state, error: String(error) });
throw error;
}
publishGroups(readGroupNames(result));
await refresh({ ...lastListOptions, force: true });
};
const groupsDelete = async (name: string) => {
const groupsDelete = async (name: string): Promise<SessionGroupMutationResult> => {
const scope = captureConnection();
if (!scope) {
return;
return "stale";
}
const result = await scope.client.request("sessions.groups.delete", { name });
if (!isCurrentConnection(scope)) {
return;
try {
const result = await scope.client.request("sessions.groups.delete", { name });
if (!isCurrentConnection(scope)) {
return "stale";
}
publishGroups(readGroupNames(result));
// See groupsRename: collapsed-state consumers must observe confirmed
// completion before an unrelated refresh can outlive the connection.
void refresh({ ...lastListOptions, force: true });
return "completed";
} catch (error) {
if (!isCurrentConnection(scope)) {
return "stale";
}
publish({ ...state, error: String(error) });
throw error;
}
publishGroups(readGroupNames(result));
await refresh({ ...lastListOptions, force: true });
};
const patch = async (