From 24d93f26a4f7203011bf2ade262f51db63764a0a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 27 Aug 2026 11:17:44 -0700 Subject: [PATCH] fix(ui): preserve session identity through rename and group actions (#130936) * fix(ui): reject stale session renames after replacement * fix(ui): preserve selected session identity through group actions --- docs/web/control-ui.md | 4 + ui/src/components/session-menu-access.ts | 4 +- .../session-organizer-batch-mutations.test.ts | 36 ++-- .../session-organizer-batch-mutations.ts | 4 +- .../session-organizer-operations.runtime.ts | 40 +--- ...sion-management.group-identity.e2e.test.ts | 189 ++++++++++++++++++ ...ion-management.rename-identity.e2e.test.ts | 147 ++++++++++++++ ui/src/e2e/session-management.test-support.ts | 6 +- ui/src/i18n/locales/en.ts | 2 - ui/src/lib/sessions/patch.ts | 2 +- ui/src/pages/chat/chat-pane-base.ts | 5 +- ui/src/pages/chat/chat-pane-header.ts | 2 +- .../pages/chat/chat-pane-session-menu.test.ts | 118 +++++++++++ ui/src/pages/chat/chat-pane-session-menu.ts | 48 +++-- ui/src/pages/chat/chat-pane.test.ts | 69 ------- ui/src/pages/chat/chat-state-route.ts | 10 - ui/src/pages/chat/route-resolution.test.ts | 23 --- .../sessions/sessions-page.groups.test.ts | 106 +++++++--- ui/src/pages/sessions/sessions-page.ts | 39 ++-- .../app-sidebar-cases/interactions.ts | 4 +- .../app-sidebar-cases/new-group-dialog.ts | 72 ++++--- 21 files changed, 678 insertions(+), 252 deletions(-) create mode 100644 ui/src/e2e/session-management.group-identity.e2e.test.ts create mode 100644 ui/src/e2e/session-management.rename-identity.e2e.test.ts diff --git a/docs/web/control-ui.md b/docs/web/control-ui.md index e6354af50bd9..d92432992069 100644 --- a/docs/web/control-ui.md +++ b/docs/web/control-ui.md @@ -250,6 +250,10 @@ The sidebar organizes everything around the agent. The identity row at the top i **Mark as unread** creates a reminder that remains unread while the current chat stays open, including while a run streams or completes. Leave and reopen the session, or choose **Mark as read**, to clear it. +**Rename** in the sidebar, chat header, and Sessions page targets the session you started editing. If that session is deleted and recreated at the same key before you save, the edit is rejected instead of renaming the replacement. Reopen Rename on the current session to try again. Resetting the conversation keeps the same session identity and does not invalidate the edit. + +**New group** from the sidebar, chat header, or Sessions page keeps the original session selection while the dialog is open and the group is being saved. A deleted or replaced session is not moved; an error is shown and the new group remains available. For a sidebar multi-selection, sessions that still exist can move even if another target fails. Paging a selected session out of the visible list does not cancel its move. + ### Session placement A selected session running on a worker shows a quiet **Runs on Cloud** chip in the chat header. Connections with `operator.write` can choose **Move session…** to continue on the Gateway or an eligible paired device, and can use **Stop cloud worker…** through the write-scoped `sessions.reclaim` lifecycle. Moving to a configured cloud profile requires `operator.admin`. Cloud rows are filtered against all execution modes advertised by each profile: the same bundled Crabbox profile is selectable for OpenClaw `worker-turn` and Codex `remote-exec`, while a genuinely single-mode profile stays disabled for the other runtime. Profiles with multiple machine classes show a machine picker; choosing the default omits an override, while choosing a different class on the current profile resizes the session. The confirmation explains that an active turn is interrupted and never replayed; OpenClaw reconciles the workspace before activating the destination. While the durable operation is in progress, the chip shows **Moving to…**. If recovery is blocked, the chip exposes the bounded error after reconnect so the action never fails silently. diff --git a/ui/src/components/session-menu-access.ts b/ui/src/components/session-menu-access.ts index f866810b61a0..ae3d758c4fb5 100644 --- a/ui/src/components/session-menu-access.ts +++ b/ui/src/components/session-menu-access.ts @@ -37,9 +37,7 @@ export function sessionMenuReasons(params: { params: { targets: batchRows.map((row) => ({ key: row.key, - ...(typeof patch.archived === "boolean" && row.sessionId - ? { expectedSessionId: row.sessionId } - : {}), + ...(row.sessionId ? { expectedSessionId: row.sessionId } : {}), })), patch, }, diff --git a/ui/src/components/session-organizer-batch-mutations.test.ts b/ui/src/components/session-organizer-batch-mutations.test.ts index 6cdeed272333..7dbeba3715e1 100644 --- a/ui/src/components/session-organizer-batch-mutations.test.ts +++ b/ui/src/components/session-organizer-batch-mutations.test.ts @@ -248,9 +248,28 @@ describe("patchSessionRows", () => { expect(harness.refreshReplacement).toHaveBeenCalledOnce(); }); - it("uses the legacy-compatible batch Mark as read payload", async () => { + it.each([{ unread: false }, { unread: true }, { category: "Projects" }, { pinned: true }])( + "preserves captured identities for metadata patch %j", + async (patch) => { + const rows = [sessionRow(0), sessionRow(1)]; + const harness = createHarness(); + + await patchSessionRows(harness.host, rows, patch, harness.scope); + + expect(harness.request).toHaveBeenCalledWith("sessions.patchMany", { + targets: rows.map((row) => ({ + key: row.key, + agentId: "main", + expectedSessionId: row.sessionId, + })), + patch, + }); + }, + ); + + it("keeps batch read identity independent of the unread acknowledgement capability", async () => { const rows = [sessionRow(0), sessionRow(1)]; - const harness = createHarness(); + const harness = createHarness({ capabilities: [] }); await patchSessionRows(harness.host, rows, { unread: false }, harness.scope); @@ -258,23 +277,12 @@ describe("patchSessionRows", () => { targets: rows.map((row) => ({ key: row.key, agentId: "main", + expectedSessionId: row.sessionId, })), patch: { unread: false }, }); }); - it("uses the legacy batch read payload when the Gateway lacks the unread contract", async () => { - const rows = [sessionRow(0), sessionRow(1)]; - const harness = createHarness({ capabilities: [] }); - - await patchSessionRows(harness.host, rows, { unread: false }, harness.scope); - - expect(harness.request).toHaveBeenCalledWith("sessions.patchMany", { - targets: rows.map((row) => ({ key: row.key, agentId: "main" })), - patch: { unread: false }, - }); - }); - it("sends no requests or refresh when the mutation scope is already stale", async () => { const harness = createHarness({ current: false }); diff --git a/ui/src/components/session-organizer-batch-mutations.ts b/ui/src/components/session-organizer-batch-mutations.ts index 0c6df52ca4af..a39400024fb8 100644 --- a/ui/src/components/session-organizer-batch-mutations.ts +++ b/ui/src/components/session-organizer-batch-mutations.ts @@ -128,9 +128,7 @@ export async function patchSessionRows( targets: chunkRows.map((row) => ({ key: row.key, agentId: sessionRowAgentId(row, scope), - ...(typeof patch.archived === "boolean" && row.sessionId - ? { expectedSessionId: row.sessionId } - : {}), + ...(row.sessionId ? { expectedSessionId: row.sessionId } : {}), })), patch, }; diff --git a/ui/src/components/session-organizer-operations.runtime.ts b/ui/src/components/session-organizer-operations.runtime.ts index 0cbe01de866b..9d953e5d0b11 100644 --- a/ui/src/components/session-organizer-operations.runtime.ts +++ b/ui/src/components/session-organizer-operations.runtime.ts @@ -427,43 +427,25 @@ export async function createSessionGroup( sessions: readonly SidebarRecentSession[], scope: SidebarSessionMutationScope, ): Promise { + if (sessions.some((session) => !session.sessionId)) { + host.sessionData.publishSessionMutationError(scope, t("common.refresh")); + return "failed"; + } const remembered = await rememberSessionGroup(host, name, scope); if (remembered !== "completed") { return remembered; } - // The dialog no longer blocks, so a captured row can be deleted while the - // catalog write is in flight, and sessions.patch would recreate it. Re-resolve - // every target against the current list, as the Sessions-page path does. - const targets = sessions.flatMap((session) => { - const current = host.findSidebarSessionByKey(session.key); - return current ? [current] : []; - }); - if (targets.length > 0) { - const moved = - targets.length === 1 - ? await patchSession(host, targets[0]!, { category: name }, scope) - : await patchSessions(host, targets, { category: name }, scope); - // Rows that left the list are absent from `targets`, so patching the - // remainder reports success for a selection that was only partly applied. - // Closing on that would leave the skipped rows unaccounted for, so the - // partial outcome is named here; it is terminal, as the group already exists. - if (moved === "completed" && targets.length < sessions.length) { - showToast({ message: t("sessionsView.newGroupMovePartial") }); - } - return moved; + // The Gateway checks the identities captured with the action. A bounded + // roster can page them out or replace a key, so it cannot authorize the move. + if (sessions.length > 0) { + return sessions.length === 1 + ? patchSession(host, sessions[0]!, { category: name }, scope) + : patchSessions(host, sessions, { category: name }, scope); } if (!host.sessionData.isSessionMutationScopeCurrent(scope)) { return "stale"; } - // A header-created group starts empty and needs no notice. Rows that were - // requested but resolved to nothing are a partial outcome: the group landed - // and the moves did not. The sidebar list is a bounded projection, so this is - // not proof the sessions are gone — say so rather than closing on a silent - // non-outcome the operator cannot account for. - if (sessions.length > 0) { - showToast({ message: t("sessionsView.newGroupMoveSkipped") }); - } - // Re-render so the new section shows up. + // A header-created group starts empty and needs no assignment. host.requestUpdate(); return "completed"; } diff --git a/ui/src/e2e/session-management.group-identity.e2e.test.ts b/ui/src/e2e/session-management.group-identity.e2e.test.ts new file mode 100644 index 000000000000..a45ecbee3071 --- /dev/null +++ b/ui/src/e2e/session-management.group-identity.e2e.test.ts @@ -0,0 +1,189 @@ +import { mkdir } from "node:fs/promises"; +import path from "node:path"; +import { expect, it } from "vitest"; +import { + activateSelfRemovingControl, + captureUiProofEnabled, + controlUiSessionUrl, + createSessionManagementE2eSuite, + installMockGateway, + openSessionMenuSubmenu, + requireRecord, + sessionRow, + sessionsListResponse, +} from "./session-management.test-support.ts"; + +const suite = createSessionManagementE2eSuite(); +const proofDir = path.join(process.cwd(), ".artifacts/control-ui-e2e/group-identity-20260827"); + +suite.define(() => { + it.each(["sessions", "sidebar", "selection", "header"] as const)( + "keeps a replacement unchanged during a pending %s new-group assignment", + async (surface) => { + const viewport = { width: 1280, height: 900 }; + const context = await suite.newBrowserContext({ + locale: "en-US", + serviceWorkers: "block", + viewport, + recordVideo: captureUiProofEnabled ? { dir: proofDir, size: viewport } : undefined, + }); + const page = await context.newPage(); + const video = page.video(); + const original = sessionRow("agent:main:group-identity", "Original session", Date.now(), { + sessionId: "original-session", + }); + const survivor = sessionRow("agent:main:group-survivor", "Surviving session", Date.now()); + const replacement = { + ...original, + sessionId: "replacement-session", + label: "Replacement session", + displayName: "Replacement session", + updatedAt: original.updatedAt + 1_000, + }; + const group = "Selected work"; + const batch = surface === "selection"; + const method = batch ? "sessions.patchMany" : "sessions.patch"; + const gateway = await installMockGateway(page, { + deferredMethods: ["sessions.groups.put", method], + methodResponses: { "sessions.list": sessionsListResponse([original, survivor]) }, + sessionKey: original.key, + }); + const capture = async (stage: string) => { + if (captureUiProofEnabled) { + await mkdir(proofDir, { recursive: true }); + await page.screenshot({ + path: path.join(proofDir, `${surface}-${stage}.png`), + animations: "disabled", + fullPage: true, + }); + } + }; + try { + await page.goto( + surface === "sessions" + ? `${suite.server.baseUrl}sessions` + : controlUiSessionUrl(suite.server.baseUrl, original.key), + ); + const row = + surface === "sessions" + ? page.locator(".sessions-table tbody tr", { hasText: original.key }) + : page.locator(`.sidebar-recent-session[data-session-key="${original.key}"]`); + await row.waitFor({ state: "visible" }); + if (batch) { + for (const key of [original.key, survivor.key]) { + await page + .locator(`[data-session-key="${key}"] .sidebar-recent-session__link`) + .click({ modifiers: ["Alt"] }); + } + await expect + .poll(() => page.locator(".sidebar-recent-session--selected").count()) + .toBe(2); + } + if (surface === "header") { + await page.locator(".chat-header-session-menu__trigger").click(); + } else { + await row.hover(); + await row.getByRole("button", { name: "Open session menu" }).click(); + } + await openSessionMenuSubmenu(page, batch ? "Move 2 to group" : "Move to group"); + await activateSelfRemovingControl(page.getByRole("menuitem", { name: "New group…" })); + const input = page.getByLabel("New group name"); + await input.fill(group); + await capture("editing"); + await input.press("Enter"); + await gateway.waitForRequest("sessions.groups.put"); + + // Deletion/recreation changes the durable identity, unlike ordinary reset. + await gateway.setMethodResponse( + "sessions.list", + sessionsListResponse([replacement, survivor]), + ); + await gateway.emitGatewayEvent("sessions.changed", { + ...replacement, + sessionKey: original.key, + reason: "create", + }); + await expect.poll(() => row.textContent()).toContain(replacement.label); + await gateway.resolveDeferred("sessions.groups.put"); + const request = await gateway.waitForRequest(method); + const params = requireRecord(request.params); + const targets = + batch && Array.isArray(params.targets) ? params.targets.map(requireRecord) : [params]; + const acceptedKeys: string[] = []; + // Replay the existing Gateway CAS contract, not the UI's intended result. + // The generic mock otherwise accepts every metadata write unconditionally. + const outcomes = targets.map((target) => { + const current = target.key === original.key ? replacement : survivor; + if (target.expectedSessionId && target.expectedSessionId !== current.sessionId) { + return { + ok: false, + key: target.key, + agentId: target.agentId, + error: { + code: "INVALID_REQUEST", + message: `Session ${String(target.key)} changed before patch. Retry.`, + details: { reason: "session-changed" }, + }, + }; + } + acceptedKeys.push(String(target.key)); + return { ok: true, key: target.key, agentId: target.agentId }; + }); + if (batch) { + await gateway.setMethodResponse(method, { outcomes }); + await gateway.resolveDeferred(method); + } else if (outcomes[0]?.error) { + await gateway.rejectDeferred(method, outcomes[0].error); + } else { + await gateway.resolveDeferred(method); + } + const failure = + surface === "header" + ? page.getByText(/changed before patch\. Retry\./).first() + : page.locator('openclaw-modal-dialog [role="alert"]'); + if (acceptedKeys.includes(original.key)) { + await input.waitFor({ state: "detached" }); + await page + .locator( + `[data-session-section="category:${group}"] [data-session-key="${original.key}"]`, + ) + .waitFor({ state: "visible" }); + } else { + await failure.waitFor({ state: "visible" }); + } + await capture(acceptedKeys.includes(original.key) ? "incorrectly-moved" : "rejected"); + + expect(acceptedKeys).not.toContain(original.key); + expect(targets.find((target) => target.key === original.key)).toMatchObject({ + expectedSessionId: original.sessionId, + }); + expect(await failure.textContent()).toContain("changed before patch. Retry."); + if (batch) { + expect(acceptedKeys).toEqual([survivor.key]); + await page + .locator( + `[data-session-section="category:${group}"] [data-session-key="${survivor.key}"]`, + ) + .waitFor({ state: "visible" }); + } + expect( + await page + .locator( + `[data-session-section="category:${group}"] [data-session-key="${original.key}"]`, + ) + .count(), + ).toBe(0); + if (surface !== "header") { + await page.getByRole("button", { name: "Cancel", exact: true }).click(); + } + await input.waitFor({ state: "detached" }); + await capture("unchanged"); + } finally { + await suite.closeBrowserContext(context); + if (video) { + await video.saveAs(path.join(proofDir, `${surface}.webm`)); + } + } + }, + ); +}); diff --git a/ui/src/e2e/session-management.rename-identity.e2e.test.ts b/ui/src/e2e/session-management.rename-identity.e2e.test.ts new file mode 100644 index 000000000000..b4d6a0976169 --- /dev/null +++ b/ui/src/e2e/session-management.rename-identity.e2e.test.ts @@ -0,0 +1,147 @@ +import { mkdir } from "node:fs/promises"; +import path from "node:path"; +import { expect, it } from "vitest"; +import { + captureUiProofEnabled, + controlUiSessionUrl, + createSessionManagementE2eSuite, + installMockGateway, + requireRecord, + sessionRow, + sessionsListResponse, + waitForPatch, +} from "./session-management.test-support.ts"; + +const suite = createSessionManagementE2eSuite(); +const proofDir = path.join(process.cwd(), ".artifacts/control-ui-e2e/session-identity-20260827"); + +suite.define(() => { + it.each(["sessions", "sidebar", "header"] as const)( + "keeps a replacement session unchanged after a stale %s rename", + async (surface) => { + const viewport = { width: 1280, height: 900 }; + const context = await suite.browser.newContext({ + locale: "en-US", + serviceWorkers: "block", + viewport, + recordVideo: captureUiProofEnabled ? { dir: proofDir, size: viewport } : undefined, + }); + const page = await context.newPage(); + const video = page.video(); + const original = sessionRow( + "agent:main:rename-identity", + "Original session", + Date.parse("2026-08-27T12:00:00.000Z"), + ); + const replacement = { + ...original, + sessionId: "replacement-session", + label: "Replacement session", + displayName: "Replacement session", + updatedAt: original.updatedAt + 1_000, + }; + const gateway = await installMockGateway(page, { + deferredMethods: ["sessions.patch"], + methodResponses: { "sessions.list": sessionsListResponse([original]) }, + sessionKey: original.key, + }); + const capture = async (stage: string) => { + if (captureUiProofEnabled) { + await mkdir(proofDir, { recursive: true }); + await page.screenshot({ + path: path.join(proofDir, `${surface}-${stage}.png`), + animations: "disabled", + fullPage: true, + }); + } + }; + + try { + await page.goto( + surface === "sessions" + ? `${suite.server.baseUrl}sessions` + : controlUiSessionUrl(suite.server.baseUrl, original.key), + ); + const row = + surface === "sessions" + ? page.locator(".sessions-table tbody tr", { hasText: original.key }) + : page.locator(`.sidebar-recent-session[data-session-key="${original.key}"]`); + await row.waitFor({ state: "visible" }); + const openRename = async () => { + if (surface === "header") { + await page.locator(".chat-pane__session-title-button").click(); + } else { + await row.hover(); + await row.getByRole("button", { name: "Open session menu" }).click(); + await page.getByRole("menuitem", { name: "Rename…" }).click(); + } + }; + await openRename(); + const input = page.locator( + surface === "header" + ? ".chat-pane__session-title-input" + : 'openclaw-modal-dialog[label="Rename session"] input', + ); + await expect.poll(() => input.inputValue()).toBe(original.label); + await input.fill("Stale rename"); + await capture("editing"); + + await gateway.setMethodResponse("sessions.list", sessionsListResponse([replacement])); + await gateway.emitGatewayEvent("sessions.changed", { + ...replacement, + sessionKey: original.key, + reason: "create", + }); + await expect.poll(() => row.textContent()).toContain(replacement.label); + await input.press("Enter"); + const request = await waitForPatch(gateway, (params) => params.label === "Stale rename"); + const params = requireRecord(request.params); + // Replay the Gateway's existing metadata CAS contract, covered by + // server.sessions.patch-expected-identity.test.ts, without a new mock seam. + if (params.expectedSessionId && params.expectedSessionId !== replacement.sessionId) { + await gateway.rejectDeferred("sessions.patch", { + code: "INVALID_REQUEST", + message: `Session ${original.key} changed before patch. Retry.`, + details: { reason: "session-changed" }, + }); + } else { + await gateway.resolveDeferred("sessions.patch"); + } + await input.waitFor({ state: "detached" }); + const outcome = page.getByText(/changed before patch\. Retry\.|Stale rename/, { + exact: false, + }); + await outcome.first().waitFor({ state: "visible" }); + await capture("outcome"); + await expect + .poll(() => page.getByText(/changed before patch\. Retry\./).count()) + .toBeGreaterThan(0); + expect(params).toMatchObject({ + key: original.key, + expectedSessionId: original.sessionId, + label: "Stale rename", + }); + expect(await row.textContent()).toContain(replacement.label); + expect(await page.getByText("Stale rename", { exact: true }).count()).toBe(0); + + await openRename(); + await expect.poll(() => input.inputValue()).toBe(replacement.label); + await input.fill("Fresh rename"); + await input.press("Enter"); + const fresh = await waitForPatch(gateway, (next) => next.label === "Fresh rename"); + expect(fresh.params).toMatchObject({ + key: original.key, + expectedSessionId: replacement.sessionId, + label: "Fresh rename", + }); + await expect.poll(() => row.textContent()).toContain("Fresh rename"); + await capture("recovered"); + } finally { + await context.close(); + if (video) { + await video.saveAs(path.join(proofDir, `${surface}.webm`)); + } + } + }, + ); +}); diff --git a/ui/src/e2e/session-management.test-support.ts b/ui/src/e2e/session-management.test-support.ts index f99d4752bf44..b3fc174da21b 100644 --- a/ui/src/e2e/session-management.test-support.ts +++ b/ui/src/e2e/session-management.test-support.ts @@ -186,7 +186,11 @@ export async function openSessionMenuSubmenu(page: Page, name: string): Promise< expect(index).toBeGreaterThanOrEqual(0); await expect .poll(() => - page.locator("openclaw-session-menu > wa-dropdown > wa-dropdown-item:focus").count(), + page + .locator( + ":is(openclaw-session-menu, openclaw-chat-header-session-menu) > wa-dropdown > wa-dropdown-item:focus", + ) + .count(), ) .toBe(1); await page.keyboard.press("Home"); diff --git a/ui/src/i18n/locales/en.ts b/ui/src/i18n/locales/en.ts index 5e83e801e2ba..deff803a1d1a 100644 --- a/ui/src/i18n/locales/en.ts +++ b/ui/src/i18n/locales/en.ts @@ -1251,8 +1251,6 @@ export const en: TranslationMap = { newGroupStale: "Gateway connection replaced before the group was saved. Try again.", newGroupMoveSkipped: "Group created, but the move was skipped because the list changed. Move from the row menu.", - newGroupMovePartial: - "Group created, but some selected sessions were not moved because the list changed. Move them from the row menu.", moveToGroup: "Move session to a group", moveToGroupMenu: "Move to group", moveToGroupMenuCount: "Move {count} to group", diff --git a/ui/src/lib/sessions/patch.ts b/ui/src/lib/sessions/patch.ts index 8921d896cfa3..937e761cac6d 100644 --- a/ui/src/lib/sessions/patch.ts +++ b/ui/src/lib/sessions/patch.ts @@ -28,7 +28,7 @@ export type SessionPatch = { export type SessionPatchOptions = { agentId?: string; - /** Durable identity observed with the row before an archive or restore action. */ + /** Durable identity observed with the row before the action or edit began. */ expectedSessionId?: string; /** Explicit unread marker observed by an automatic read acknowledgement. */ expectedMarkedUnreadAt?: number | null; diff --git a/ui/src/pages/chat/chat-pane-base.ts b/ui/src/pages/chat/chat-pane-base.ts index e2c2a37560e7..ac94238ed983 100644 --- a/ui/src/pages/chat/chat-pane-base.ts +++ b/ui/src/pages/chat/chat-pane-base.ts @@ -13,6 +13,7 @@ import type { ControlUiSessionPullRequest, } from "../../../../src/gateway/control-ui-contract.js"; import type { GatewayBrowserClient } from "../../api/gateway.ts"; +import type { GatewaySessionRow } from "../../api/types.ts"; import { applicationContext } from "../../app/context.ts"; import { observeNativeGateway } from "../../app/native-editor-locality.runtime.ts"; import type { @@ -394,9 +395,9 @@ export abstract class ChatPaneBase extends OpenClawLightDomElement { config: SessionDiscussionPanelConfig; } >(); - protected headerRenameInitialLabel: string | null = null; protected headerRenameInitialValue = ""; - protected headerRenameSessionKey = ""; + protected headerRenameSession: Pick | null = + null; protected headerCopiedTimer: number | null = null; protected composerPrefillAttentionTimer: number | null = null; protected composerPrefillAttentionTarget: HTMLElement | null = null; diff --git a/ui/src/pages/chat/chat-pane-header.ts b/ui/src/pages/chat/chat-pane-header.ts index 954908cb2433..3b34d1b8d3ec 100644 --- a/ui/src/pages/chat/chat-pane-header.ts +++ b/ui/src/pages/chat/chat-pane-header.ts @@ -399,7 +399,7 @@ export abstract class ChatPaneHeader extends ChatPaneDiscussion { ownerViewing, personActivity, catalog, - editing: this.headerEditing && this.headerRenameSessionKey === row?.key, + editing: this.headerEditing && this.headerRenameSession?.key === row?.key, renameValue: this.headerRenameValue, workspaceRoot: workspace.root, workspaceLabel: workspace.label, diff --git a/ui/src/pages/chat/chat-pane-session-menu.test.ts b/ui/src/pages/chat/chat-pane-session-menu.test.ts index 5e5dde4f15f5..833544049955 100644 --- a/ui/src/pages/chat/chat-pane-session-menu.test.ts +++ b/ui/src/pages/chat/chat-pane-session-menu.test.ts @@ -155,4 +155,122 @@ describe("chat pane session menu boundary", () => { { agentId: "main", expectedSessionId: rendered.sessionId }, ); }); + + it.each([ + { action: { kind: "toggle-pin" }, patch: { pinned: true } }, + { action: { kind: "toggle-unread" }, patch: { unread: true } }, + { action: { kind: "set-icon", icon: "🦞" }, patch: { icon: "🦞" } }, + { action: { kind: "move-to-group", category: "Projects" }, patch: { category: "Projects" } }, + ] as const)( + "keeps the original header identity for $action.kind after replacement", + async ({ action, patch: expectedPatch }) => { + const patch = vi.fn(async () => ({})); + const original = { + key: "agent:main:current", + sessionId: "original-session", + kind: "direct", + updatedAt: 0, + } satisfies GatewaySessionRow; + const sessions = createSessionCapabilityFixture({ + patch, + state: { + error: null, + groups: ["Projects"], + result: { + ts: 1, + count: 1, + path: "", + defaults: { modelProvider: null, model: null, contextTokens: null }, + sessions: [{ ...original, sessionId: "replacement-session" }], + }, + }, + }); + const { pane } = createTestChatPane({ + client: createGatewayBrowserClientFixture(), + sessions, + }); + + await pane.handleHeaderSessionAction(action, original); + + expect(patch).toHaveBeenCalledWith(original.key, expectedPatch, { + agentId: "main", + expectedSessionId: original.sessionId, + }); + }, + ); + + it("commits a trimmed label and clears with null", async () => { + const patch = vi.fn(async () => ({})); + const sessions = createSessionCapabilityFixture({ patch }); + const { pane } = createTestChatPane({ client: createGatewayBrowserClientFixture(), sessions }); + const session = { + key: "agent:main:current", + sessionId: "rename-current", + kind: "direct", + updatedAt: 0, + } satisfies GatewaySessionRow; + pane.beginHeaderRename(session); + pane.headerRenameValue = " Renamed session "; + pane.commitHeaderRename(); + expect(patch).toHaveBeenCalledWith( + session.key, + { label: "Renamed session" }, + { agentId: "main", expectedSessionId: session.sessionId }, + ); + + const labeled = { ...session, label: "Renamed session" }; + pane.beginHeaderRename(labeled); + pane.headerRenameValue = " "; + pane.commitHeaderRename(); + expect(patch).toHaveBeenLastCalledWith( + session.key, + { label: null }, + { agentId: "main", expectedSessionId: session.sessionId }, + ); + }); + + it("renames the selected agent's canonical global session", () => { + const patch = vi.fn(async () => ({})); + const sessions = createSessionCapabilityFixture({ patch }); + const { pane, state } = createTestChatPane({ + client: createGatewayBrowserClientFixture(), + sessions, + }); + state.sessionKey = "global"; + state.assistantAgentId = "research"; + const session = { + key: "global", + sessionId: "research-global", + kind: "global", + updatedAt: 0, + } satisfies GatewaySessionRow; + + pane.beginHeaderRename(session); + pane.headerRenameValue = "Research thread"; + pane.commitHeaderRename(); + + expect(patch).toHaveBeenCalledWith( + "global", + { label: "Research thread" }, + { agentId: "research", expectedSessionId: session.sessionId }, + ); + }); + + it("cancels and skips an unchanged generated dashboard title", () => { + const patch = vi.fn(async () => ({})); + const sessions = createSessionCapabilityFixture({ patch }); + const { pane } = createTestChatPane({ client: createGatewayBrowserClientFixture(), sessions }); + const session = { + key: "agent:main:dashboard:generated", + kind: "direct", + displayName: "Generated title", + updatedAt: 0, + } satisfies GatewaySessionRow; + pane.beginHeaderRename(session); + expect(pane.headerRenameValue).toBe("Generated title"); + pane.commitHeaderRename(); + pane.beginHeaderRename(session); + pane.cancelHeaderRename(); + expect(patch).not.toHaveBeenCalled(); + }); }); diff --git a/ui/src/pages/chat/chat-pane-session-menu.ts b/ui/src/pages/chat/chat-pane-session-menu.ts index 782f9e676f1a..8f6b738bbf96 100644 --- a/ui/src/pages/chat/chat-pane-session-menu.ts +++ b/ui/src/pages/chat/chat-pane-session-menu.ts @@ -25,7 +25,7 @@ import { import { showToast } from "../../lib/toast.ts"; import { ChatPaneContext } from "./chat-pane-context.ts"; import { headerPlatformByClient } from "./chat-pane-shared.ts"; -import { patchChatSessionLabel } from "./chat-state-route.ts"; +import { resolveChatAgentId } from "./chat-state-route.ts"; import type { HeaderMenuAction } from "./components/chat-header-session-menu.ts"; import type { ChatPaneHeaderAction } from "./components/chat-pane-header.ts"; import { buildContinueInTerminalCommand } from "./continue-in-terminal-command.ts"; @@ -99,15 +99,15 @@ export abstract class ChatPaneSessionMenu extends ChatPaneContext { gatewayHasActiveRun: candidate.hasActiveRun, }); const session = toActionSession(row); - // Header actions capture a rendered row, but any awaited menu work can - // outlive that row. Resolve at the patch boundary so a deleted no-ID row - // cannot be recreated by a stale sessions.patch request. + // Refresh metadata only on the selected instance; replacements must keep + // the captured ID so the Gateway rejects the stale action. No-ID rows + // still need current-list presence before a metadata patch can create them. const resolveCurrentSession = (notify = false) => { - const currentRow = scope.sessions.state.result?.sessions.find((candidate) => - areUiSessionKeysEquivalent(candidate.key, row.key), + const currentRow = scope.sessions.state.result?.sessions.find( + (candidate) => + areUiSessionKeysEquivalent(candidate.key, row.key) && + (!session.sessionId || candidate.sessionId === session.sessionId), ); - // A durable ID makes the captured row safe: the Gateway rejects a - // deleted or replaced identity. No-ID rows need current-list proof. const resolvedRow = currentRow ?? (session.sessionId ? row : null); if (!resolvedRow && notify) { showToast({ message: t("common.refresh") }); @@ -352,8 +352,8 @@ export abstract class ChatPaneSessionMenu extends ChatPaneContext { ? (normalizeOptionalString(row.displayName) ?? (row.worktree ? undefined : normalizeOptionalString(row.derivedTitle))) : undefined; - this.headerRenameSessionKey = row.key; - this.headerRenameInitialLabel = customLabel; + // The edit belongs to this instance, even if a replacement reuses its key. + this.headerRenameSession = { key: row.key, sessionId: row.sessionId, label: row.label }; // Dashboard titles are generated session text; channel/account decoration // remains display-only and must never become the stored label. this.headerRenameInitialValue = customLabel ?? generatedTitle ?? ""; @@ -368,37 +368,45 @@ export abstract class ChatPaneSessionMenu extends ChatPaneContext { protected cancelHeaderRename(): void { this.headerEditing = false; - this.headerRenameSessionKey = ""; + this.headerRenameSession = null; } protected commitHeaderRename(): void { if (!this.headerEditing) { return; } - const key = this.headerRenameSessionKey; + const session = this.headerRenameSession; + const initialLabel = normalizeOptionalString(session?.label) ?? null; const trimmed = this.headerRenameValue.trim(); const label = trimmed || null; const unchangedGeneratedTitle = - this.headerRenameInitialLabel === null && trimmed === this.headerRenameInitialValue.trim(); - const unchangedLabel = label === this.headerRenameInitialLabel; + initialLabel === null && trimmed === this.headerRenameInitialValue.trim(); + const unchangedLabel = label === initialLabel; this.headerEditing = false; - this.headerRenameSessionKey = ""; + this.headerRenameSession = null; const state = this.state; - if (!key || !state || unchangedGeneratedTitle || unchangedLabel) { + if (!session || !state || unchangedGeneratedTitle || unchangedLabel) { return; } const access = readSessionMethodAccess(this.context.gateway.snapshot, { method: "sessions.patch", - params: { key, label }, + params: { key: session.key, label }, }); if (!access.allowed) { this.publishHeaderError(access.reason); return; } const owner = this.headerOutcomeOwner; - void patchChatSessionLabel(state, this.context.sessions, key, label).catch((error: unknown) => - this.publishHeaderError(error, owner), - ); + void this.context.sessions + .patch( + session.key, + { label }, + { + agentId: resolveChatAgentId(state), + expectedSessionId: session.sessionId, + }, + ) + .catch((error: unknown) => this.publishHeaderError(error, owner)); } protected async loadHeaderMenuData( diff --git a/ui/src/pages/chat/chat-pane.test.ts b/ui/src/pages/chat/chat-pane.test.ts index 608181d55e5a..b9ed86ab38c3 100644 --- a/ui/src/pages/chat/chat-pane.test.ts +++ b/ui/src/pages/chat/chat-pane.test.ts @@ -264,75 +264,6 @@ describe("chat pane header state", () => { expect(showToast).toHaveBeenCalledWith({ message: t("common.refresh") }); }); - it("commits a trimmed label and clears with null", async () => { - const patch = vi.fn(async () => ({})); - const sessions = createSessionCapabilityFixture({ patch }); - const { pane } = createTestChatPane({ client: createGatewayBrowserClientFixture(), sessions }); - const session = { - key: "agent:main:current", - kind: "direct", - updatedAt: 0, - } satisfies GatewaySessionRow; - pane.beginHeaderRename(session); - pane.headerRenameValue = " Renamed session "; - pane.commitHeaderRename(); - expect(patch).toHaveBeenCalledWith( - session.key, - { label: "Renamed session" }, - { agentId: "main" }, - ); - - const labeled = { ...session, label: "Renamed session" }; - pane.beginHeaderRename(labeled); - pane.headerRenameValue = " "; - pane.commitHeaderRename(); - expect(patch).toHaveBeenLastCalledWith(session.key, { label: null }, { agentId: "main" }); - }); - - it("renames the selected agent's canonical global session", () => { - const patch = vi.fn(async () => ({})); - const sessions = createSessionCapabilityFixture({ patch }); - const { pane, state } = createTestChatPane({ - client: createGatewayBrowserClientFixture(), - sessions, - }); - state.sessionKey = "global"; - state.assistantAgentId = "research"; - const session = { - key: "global", - kind: "global", - updatedAt: 0, - } satisfies GatewaySessionRow; - - pane.beginHeaderRename(session); - pane.headerRenameValue = "Research thread"; - pane.commitHeaderRename(); - - expect(patch).toHaveBeenCalledWith( - "global", - { label: "Research thread" }, - { agentId: "research" }, - ); - }); - - it("cancels and skips an unchanged generated dashboard title", () => { - const patch = vi.fn(async () => ({})); - const sessions = createSessionCapabilityFixture({ patch }); - const { pane } = createTestChatPane({ client: createGatewayBrowserClientFixture(), sessions }); - const session = { - key: "agent:main:dashboard:generated", - kind: "direct", - displayName: "Generated title", - updatedAt: 0, - } satisfies GatewaySessionRow; - pane.beginHeaderRename(session); - expect(pane.headerRenameValue).toBe("Generated title"); - pane.commitHeaderRename(); - pane.beginHeaderRename(session); - pane.cancelHeaderRename(); - expect(patch).not.toHaveBeenCalled(); - }); - it("copies the resolved workspace path and branch", async () => { const { pane } = createTestChatPane({ client: createGatewayBrowserClientFixture(), diff --git a/ui/src/pages/chat/chat-state-route.ts b/ui/src/pages/chat/chat-state-route.ts index 697fd1ec3d9c..99abd0213612 100644 --- a/ui/src/pages/chat/chat-state-route.ts +++ b/ui/src/pages/chat/chat-state-route.ts @@ -1,7 +1,6 @@ import { loadLocalAssistantIdentity } from "../../app/assistant-identity.ts"; import { patchSettings } from "../../app/settings.ts"; import { isRenderableControlUiAvatarUrl } from "../../lib/avatar.ts"; -import type { SessionCapability } from "../../lib/sessions/index.ts"; import { areUiSessionKeysEquivalent, isUiGlobalSessionKey, @@ -66,15 +65,6 @@ export function saveRouteSessionSettings(state: ChatPageHost, sessionKey: string state.settings = patchSettings({ sessionKey, lastActiveSessionKey: sessionKey }); } -export function patchChatSessionLabel( - state: ChatPageHost, - sessions: Pick, - sessionKey: string, - label: string | null, -) { - return sessions.patch(sessionKey, { label }, { agentId: resolveChatAgentId(state) }); -} - export function resolveChatAvatarUrl(state: ChatPageHost): string | null { const agentId = resolveChatAgentId(state); if (state.chatAvatarUrl) { diff --git a/ui/src/pages/chat/route-resolution.test.ts b/ui/src/pages/chat/route-resolution.test.ts index 60e184ad814a..6b962e6eb871 100644 --- a/ui/src/pages/chat/route-resolution.test.ts +++ b/ui/src/pages/chat/route-resolution.test.ts @@ -5,7 +5,6 @@ import type { GatewaySessionRow, SessionsListResult } from "../../api/types.ts"; import { INTERNAL_SESSION_PATH_PARAM } from "../../app-route-paths.ts"; import type { ApplicationContext } from "../../app/context.ts"; import { buildCatalogSessionKey } from "../../lib/sessions/catalog-key.ts"; -import type { SessionCapability } from "../../lib/sessions/index.ts"; import { prepareSessionNavigationHandoff } from "../../lib/sessions/navigation-handoff.ts"; import { resolveSessionPreferredFaceForKey, @@ -13,8 +12,6 @@ import { SESSION_NAVIGATION_KEY_PARAM, sessionNavigationTarget, } from "../../lib/sessions/route-navigation.ts"; -import type { ChatPageHost } from "./chat-state-host.ts"; -import { patchChatSessionLabel } from "./chat-state-route.ts"; import { loadChatRoute } from "./route-loader.ts"; const uuid = "12345678-90ab-cdef-1234-567890abcdef"; @@ -108,26 +105,6 @@ function targetLocation(target: ReturnType) { } describe("gateway-backed session route resolution", () => { - it("patches a canonical global session on the selected agent", async () => { - const patch = vi.fn(async () => ({})); - const state = { - sessionKey: "global", - assistantAgentId: "research", - agentsList: null, - hello: null, - } as ChatPageHost; - - const sessions = { patch } as unknown as Pick; - - await patchChatSessionLabel(state, sessions, "global", "Research thread"); - - expect(patch).toHaveBeenCalledWith( - "global", - { label: "Research thread" }, - { agentId: "research" }, - ); - }); - it("resolves a non-default agent's canonical global face from its scoped row", async () => { const globalRow = row({ key: "global", kind: "global", boardFace: "dashboard" }); const { context, list } = contextFor(({ agentId, search }) => diff --git a/ui/src/pages/sessions/sessions-page.groups.test.ts b/ui/src/pages/sessions/sessions-page.groups.test.ts index 288c4e7e0752..f9e9b99f3d81 100644 --- a/ui/src/pages/sessions/sessions-page.groups.test.ts +++ b/ui/src/pages/sessions/sessions-page.groups.test.ts @@ -17,6 +17,7 @@ import { vi.mock("../../components/input-dialog.ts", () => ({ showInputDialog: vi.fn() })); const SESSION_KEY = "agent:main:move-me"; +const SESSION_ID = "original-session"; afterEach(() => { document.body.replaceChildren(); @@ -38,7 +39,7 @@ async function mountGroupsPage(groupsPut: () => Promise { expect(sessions.patch).toHaveBeenCalledWith( SESSION_KEY, { category: "Client work" }, - expect.anything(), + { agentId: undefined, expectedSessionId: SESSION_ID }, ); }); @@ -167,7 +168,7 @@ describe("sessions page new group", () => { // The replacement connection reloads the list before the operator retries. page.result = { count: 1, - sessions: [{ key: SESSION_KEY, archived: false }], + sessions: [{ key: SESSION_KEY, sessionId: SESSION_ID, archived: false }], } as SessionsListResult; await page.requestNewCategory(SESSION_KEY); @@ -180,32 +181,91 @@ describe("sessions page new group", () => { ); }); - it("reports the skipped move when the row left the list during the catalog write", async () => { - let landCatalogWrite!: () => void; - const pending = new Promise((resolve) => { - landCatalogWrite = () => resolve("completed"); - }); - const { page, sessions, submitMessages } = await mountGroupsPage(() => pending); + it.each([ + { change: "paged out", currentId: SESSION_ID, visible: false }, + { change: "reset with the same identity", currentId: SESSION_ID, visible: true }, + { change: "deleted", currentId: undefined, visible: false }, + { change: "replaced", currentId: "replacement-session", visible: true }, + ])( + "lets the Gateway decide a move when the row was $change mid-write", + async ({ currentId, visible }) => { + let landCatalogWrite!: () => void; + const pending = new Promise((resolve) => { + landCatalogWrite = () => resolve("completed"); + }); + const { page, sessions, submitMessages } = await mountGroupsPage(() => pending); + const failure = `Session ${SESSION_KEY} changed before patch. Retry.`; + let moved = false; + vi.mocked(sessions.patch).mockImplementation(async (_key, _patch, options) => { + if (options?.expectedSessionId && options.expectedSessionId !== currentId) { + throw new Error(failure); + } + moved = true; + return { key: SESSION_KEY } as Awaited>; + }); - const created = page.requestNewCategory(SESSION_KEY); - await vi.waitFor(() => expect(sessions.groupsPut).toHaveBeenCalledOnce()); + const created = page.requestNewCategory(SESSION_KEY); + await vi.waitFor(() => expect(sessions.groupsPut).toHaveBeenCalledOnce()); - // The row left this bounded list while the catalog write was in flight; - // patching its key now could recreate an entry the operator removed. - page.result = { count: 0, sessions: [] } as unknown as SessionsListResult; - landCatalogWrite(); - await created; + page.result = { + count: visible ? 1 : 0, + sessions: visible ? [{ key: SESSION_KEY, sessionId: currentId, label: "Updated row" }] : [], + } as SessionsListResult; + landCatalogWrite(); + await created; - expect(sessions.patch).not.toHaveBeenCalled(); - // The group landed and the move did not. Retrying would only re-create the - // group, so the dialog closes — but the partial outcome has to stay visible - // on the page rather than reading as a clean success. - expect(submitMessages).toEqual([null]); - expect(page.error).toBe( - "Group created, but the move was skipped because the list changed. Move from the row menu.", + expect(sessions.patch).toHaveBeenCalledWith( + SESSION_KEY, + { category: "Client work" }, + { agentId: undefined, expectedSessionId: SESSION_ID }, + ); + expect(moved).toBe(currentId === SESSION_ID); + expect(submitMessages).toEqual([currentId === SESSION_ID ? null : failure]); + }, + ); + + it("captures the selected identity before the dialog's lazy load", async () => { + const { page, sessions } = await mountGroupsPage(async () => "completed"); + const pending = page.requestNewCategory(SESSION_KEY); + page.result = { + count: 1, + sessions: [{ key: SESSION_KEY, sessionId: "replacement-session" }], + } as SessionsListResult; + await pending; + expect(sessions.patch).toHaveBeenCalledWith( + SESSION_KEY, + { category: "Client work" }, + { agentId: undefined, expectedSessionId: SESSION_ID }, ); }); + it("creates an empty group without a selected row", async () => { + const { page, sessions, submitMessages } = await mountGroupsPage(async () => "completed"); + await page.requestNewCategory(); + expect(sessions.groupsPut).toHaveBeenCalledWith(["Client work"]); + expect(sessions.patch).not.toHaveBeenCalled(); + expect(submitMessages).toEqual([null]); + }); + + it("does not assign when creating the catalog entry fails", async () => { + const { page, sessions, submitMessages } = await mountGroupsPage(async () => { + throw new Error("Group name rejected"); + }); + await page.requestNewCategory(SESSION_KEY); + expect(sessions.patch).not.toHaveBeenCalled(); + expect(submitMessages).toEqual(["Group name rejected"]); + }); + + it("asks for a refresh instead of starting an unbound move", async () => { + const { page, sessions } = await mountGroupsPage(async () => "completed"); + page.result = { count: 1, sessions: [{ key: SESSION_KEY }] } as SessionsListResult; + await page.requestNewCategory(SESSION_KEY); + expect(showInputDialog).not.toHaveBeenCalled(); + expect(sessions.groupsPut).not.toHaveBeenCalled(); + expect(sessions.patch).not.toHaveBeenCalled(); + expect(page.error).toBe("Refresh"); + }); + it("skips the assignment when the catalog itself reports the write stale", async () => { // The capability retires the write on its own connection epoch, which the // page's scope predicate cannot observe; the assignment must still stop. diff --git a/ui/src/pages/sessions/sessions-page.ts b/ui/src/pages/sessions/sessions-page.ts index 8960c8180fe1..d45fcac4965c 100644 --- a/ui/src/pages/sessions/sessions-page.ts +++ b/ui/src/pages/sessions/sessions-page.ts @@ -995,6 +995,13 @@ class SessionsPage extends OpenClawLightDomElement { } private async requestNewCategory(sessionKey?: string) { + // Capture before loading the dialog: its key may belong to a replacement + // by the time the operator submits or the catalog write completes. + const session = this.result?.sessions.find((row) => row.key === sessionKey); + if (sessionKey && !session?.sessionId) { + this.error = t("common.refresh"); + return; + } await this.withDialogLifecycle(async (signal) => { const showInputDialog = await this.loadInputDialog(); await showInputDialog?.({ @@ -1003,7 +1010,7 @@ class SessionsPage extends OpenClawLightDomElement { label: t("sessionsView.newGroupPrompt"), submitLabel: t("sessionsView.newGroupCreate"), requireValue: true, - submit: (name) => this.writeNewCategory(name, sessionKey), + submit: (name) => this.writeNewCategory(name, session), }); }); } @@ -1013,7 +1020,10 @@ class SessionsPage extends OpenClawLightDomElement { * row moves, and a catalog write that outlived its connection must not be * followed by an assignment issued on the replacement one. */ - private async writeNewCategory(name: string, sessionKey?: string): Promise { + private async writeNewCategory( + name: string, + session?: GatewaySessionRow, + ): Promise { this.error = null; const scope = this.captureRequestScope(); if (!scope) { @@ -1025,21 +1035,15 @@ class SessionsPage extends OpenClawLightDomElement { ? (this.error ?? t("sessionsView.newGroupFailed")) : t("sessionsView.newGroupStale"); } - if (!sessionKey) { + if (!session) { return null; } - // The catalog write is awaited first, so the row can leave this list in - // between. sessions.patch would recreate a store entry for a key the list no - // longer has, so the move is skipped — but this list is a bounded, filtered - // projection, and a plain refresh can page a live row out of it. Skipping - // silently would leave the operator with a new group, an unmoved session and - // nothing explaining why, so the partial outcome is stated and terminal: - // retrying here would only try to create the group that already exists. - if (!this.result?.sessions.some((row) => row.key === sessionKey)) { - this.error = t("sessionsView.newGroupMoveSkipped"); - return null; - } - const assigned = await this.patchSession(sessionKey, { category: name }, scope); + const assigned = await this.patchSession( + session.key, + { category: name }, + scope, + session.sessionId, + ); if (assigned === "failed") { return this.error ?? t("sessionsView.newGroupFailed"); } @@ -1060,7 +1064,8 @@ class SessionsPage extends OpenClawLightDomElement { if (value === null) { return; } - void this.patchSession(row.key, { label: normalizeOptionalString(value) ?? null }); + const patch = { label: normalizeOptionalString(value) ?? null }; + void this.patchSession(row.key, patch, undefined, row.sessionId); } private async patchSession( @@ -1095,7 +1100,7 @@ class SessionsPage extends OpenClawLightDomElement { try { const patched = await scope.sessions.patch(key, patch, { agentId, - ...(typeof patch.archived === "boolean" ? { expectedSessionId } : {}), + ...(expectedSessionId ? { expectedSessionId } : {}), }); if (!this.isRequestScopeCurrent(scope)) { return "stale"; diff --git a/ui/src/test-helpers/app-sidebar-cases/interactions.ts b/ui/src/test-helpers/app-sidebar-cases/interactions.ts index 3cf4d5ef88e4..1bf95bae67fc 100644 --- a/ui/src/test-helpers/app-sidebar-cases/interactions.ts +++ b/ui/src/test-helpers/app-sidebar-cases/interactions.ts @@ -220,8 +220,8 @@ describe("AppSidebar multi-select", () => { await waitForFast(() => expect(harness.patchMany).toHaveBeenCalledOnce()); expect(harness.patchMany).toHaveBeenCalledWith( [ - { key: "agent:main:a", agentId: "main" }, - { key: "agent:main:b", agentId: "main" }, + { key: "agent:main:a", agentId: "main", expectedSessionId: "session:agent:main:a" }, + { key: "agent:main:b", agentId: "main", expectedSessionId: "session:agent:main:b" }, ], { unread: true }, ); diff --git a/ui/src/test-helpers/app-sidebar-cases/new-group-dialog.ts b/ui/src/test-helpers/app-sidebar-cases/new-group-dialog.ts index bc252cd5361d..78a9eb90c1d8 100644 --- a/ui/src/test-helpers/app-sidebar-cases/new-group-dialog.ts +++ b/ui/src/test-helpers/app-sidebar-cases/new-group-dialog.ts @@ -38,8 +38,8 @@ describe("AppSidebar new group dialog", () => { expect(harness.groupsPut).toHaveBeenCalledWith(["Projects"]); expect(harness.patchMany).toHaveBeenCalledWith( [ - { key: "agent:main:a", agentId: "main" }, - { key: "agent:main:b", agentId: "main" }, + { key: "agent:main:a", agentId: "main", expectedSessionId: "session:agent:main:a" }, + { key: "agent:main:b", agentId: "main", expectedSessionId: "session:agent:main:b" }, ], { category: "Projects" }, ); @@ -53,7 +53,7 @@ describe("AppSidebar new group dialog", () => { } }); - it("reports the skipped moves when selected rows leave the list mid-write", async () => { + it("moves captured sessions even when both leave the bounded list mid-write", async () => { const restoreDialogPolyfill = installDialogPolyfill(); const toastHost = document.createElement("openclaw-toast-host"); document.body.append(toastHost); @@ -81,8 +81,8 @@ describe("AppSidebar new group dialog", () => { await submitInputDialog("Projects"); await waitForFast(() => expect(harness.groupsPut).toHaveBeenCalledOnce()); - // Both rows leave the list while the catalog write is still in flight; - // patching their keys now could recreate sessions that were removed. + // Projection absence is not deletion. The Gateway can still apply both + // captured identities, and rejects either one if it was actually removed. harness.publish({ result: { count: 0, sessions: [] } as unknown as SessionsListResult }); landCatalogWrite(); @@ -92,24 +92,23 @@ describe("AppSidebar new group dialog", () => { await waitForFast(() => expect(document.body.querySelector("openclaw-modal-dialog")).toBeNull(), ); - expect(harness.patchMany).not.toHaveBeenCalled(); - expect(harness.patch).not.toHaveBeenCalled(); - // The group landed and the moves did not: that partial outcome has to - // reach the operator instead of closing as a plain success. - expect(toastHost.querySelector(".app-toast__message")?.textContent).toBe( - "Group created, but the move was skipped because the list changed. Move from the row menu.", + expect(harness.patchMany).toHaveBeenCalledWith( + [ + { key: "agent:main:a", agentId: "main", expectedSessionId: "session:agent:main:a" }, + { key: "agent:main:b", agentId: "main", expectedSessionId: "session:agent:main:b" }, + ], + { category: "Projects" }, ); + expect(harness.patch).not.toHaveBeenCalled(); + expect(toastHost.querySelector(".app-toast__message")).toBeNull(); } finally { toastHost.remove(); restoreDialogPolyfill(); } }); - it("reports the partial outcome when only part of the selection leaves the list", async () => { + it("reports the failed target when the Gateway moves only part of the selection", async () => { const restoreDialogPolyfill = installDialogPolyfill(); - const toastHost = document.createElement("openclaw-toast-host"); - document.body.append(toastHost); - await toastHost.updateComplete; try { const { sidebar, harness } = await mountMultiSelect([ "sessions.groups.put", @@ -134,34 +133,43 @@ describe("AppSidebar new group dialog", () => { await submitInputDialog("Projects"); await waitForFast(() => expect(harness.groupsPut).toHaveBeenCalledOnce()); - // Only one of the two selected rows survives the catalog write. Moving the - // survivor is still correct, but the other row was requested and skipped. + const failure = "Session agent:main:a changed before patch. Retry."; + harness.patchMany.mockImplementationOnce(async (targets) => ({ + outcomes: targets.map((target) => + target.key === "agent:main:a" + ? { ok: false, ...target, error: { code: "INVALID_REQUEST", message: failure } } + : { ok: true, ...target }, + ), + })); harness.publish({ result: { count: 1, - sessions: [{ key: "agent:main:b", agentId: "main" }], + sessions: [{ key: "agent:main:b", sessionId: "session:agent:main:b", agentId: "main" }], } as unknown as SessionsListResult, }); landCatalogWrite(); + await waitForFast(() => + expect( + document.body.querySelector("openclaw-modal-dialog [role=alert]")?.textContent, + ).toContain(failure), + ); + expect(harness.patchMany).toHaveBeenCalledWith( + [ + { key: "agent:main:a", agentId: "main", expectedSessionId: "session:agent:main:a" }, + { key: "agent:main:b", agentId: "main", expectedSessionId: "session:agent:main:b" }, + ], + { category: "Projects" }, + ); + expect(harness.patch).not.toHaveBeenCalled(); + expect(harness.refreshReplacement).toHaveBeenCalledOnce(); + document.body + .querySelector('openclaw-modal-dialog button[type="button"]') + ?.click(); await waitForFast(() => expect(document.body.querySelector("openclaw-modal-dialog")).toBeNull(), ); - // The surviving row is patched on its own; the removed key is never sent, - // so patchMany stays out of it. - await waitForFast(() => expect(harness.patch).toHaveBeenCalledOnce()); - expect(harness.patch).toHaveBeenCalledWith( - "agent:main:b", - { category: "Projects" }, - { agentId: "main" }, - ); - expect(harness.patchMany).not.toHaveBeenCalled(); - // A partly applied selection must not close as a plain success. - expect(toastHost.querySelector(".app-toast__message")?.textContent).toBe( - "Group created, but some selected sessions were not moved because the list changed. Move them from the row menu.", - ); } finally { - toastHost.remove(); restoreDialogPolyfill(); } });