From 9afc4ef28e7d8bb092604e2e9b009417b3aa3292 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 25 Aug 2026 15:15:20 -0700 Subject: [PATCH] fix(ui): protect replacement sessions during delete confirmation (#129628) --- .../e2e/session-management.delete.e2e.test.ts | 69 +++++++++++++++++++ ui/src/pages/sessions/sessions-page.test.ts | 54 +++++++++++++++ ui/src/pages/sessions/sessions-page.ts | 8 +-- 3 files changed, 127 insertions(+), 4 deletions(-) diff --git a/ui/src/e2e/session-management.delete.e2e.test.ts b/ui/src/e2e/session-management.delete.e2e.test.ts index 2df9d2766b4b..dcb2aa537406 100644 --- a/ui/src/e2e/session-management.delete.e2e.test.ts +++ b/ui/src/e2e/session-management.delete.e2e.test.ts @@ -253,6 +253,75 @@ suite.define(() => { } }); + it("keeps bulk deletion fenced to the session selected before confirmation", async () => { + const key = "agent:main:confirmed"; + const original = sessionRow(key, "Original session", 1, { + sessionId: "confirmed-session", + }); + const replacement = sessionRow(key, "Replacement session", 2, { + sessionId: "replacement-session", + }); + const context = await suite.browser.newContext({ + locale: "en-US", + serviceWorkers: "block", + viewport: { height: 900, width: 1280 }, + recordVideo: captureUiProofEnabled + ? { dir: uiProofArtifactDir, size: { height: 900, width: 1280 } } + : undefined, + }); + const page = await context.newPage(); + const proofVideo = page.video(); + const gateway = await installMockGateway(page, { + methodResponses: { + "sessions.delete": { ok: true, deleted: true }, + "sessions.list": sessionsListResponse([original]), + }, + sessionKey: "agent:main:main", + }); + + try { + await page.goto(`${suite.server.baseUrl}sessions`); + const replacementLabel = page.locator(".sessions-table").getByText("Replacement session", { + exact: true, + }); + await page.getByRole("checkbox", { name: `Select session: ${key}` }).check(); + await page.locator(".data-table-bulk-bar").getByRole("button", { name: "Delete" }).click(); + const confirmModal = await waitForConfirmModal(page); + await captureUiProof(page, "sessions-bulk-delete-original-confirm.png"); + + await gateway.setMethodResponse("sessions.list", sessionsListResponse([replacement])); + await gateway.emitGatewayEvent("sessions.changed", { + ...replacement, + reason: "update", + sessionKey: key, + }); + await replacementLabel.waitFor(); + await gateway.deferNext("sessions.delete"); + await confirmModal.getByRole("button", { name: "Delete", exact: true }).click(); + + const request = await gateway.waitForRequest("sessions.delete"); + expect(request).toMatchObject({ + params: { expectedSessionId: "confirmed-session", key }, + }); + await gateway.rejectDeferred("sessions.delete", { + code: "INVALID_REQUEST", + message: `Session ${key} changed before deletion. Retry.`, + }); + await expect + .poll(() => page.locator(".sessions-error[role=alert]").textContent()) + .toContain("changed before deletion. Retry."); + await replacementLabel.waitFor(); + await captureUiProof(page, "sessions-bulk-delete-replacement-protected.png"); + } finally { + await context.close(); + if (proofVideo) { + await proofVideo.saveAs( + path.join(uiProofArtifactDir, "sessions-bulk-delete-replaced.webm"), + ); + } + } + }); + it("rejects deleting a same-key replacement after the in-app confirm", async () => { const key = "agent:main:research"; const context = await suite.browser.newContext({ diff --git a/ui/src/pages/sessions/sessions-page.test.ts b/ui/src/pages/sessions/sessions-page.test.ts index e2320e966692..97e62df02136 100644 --- a/ui/src/pages/sessions/sessions-page.test.ts +++ b/ui/src/pages/sessions/sessions-page.test.ts @@ -581,6 +581,60 @@ describe("sessions page lifecycle", () => { expect(page.selectedKeys).toEqual(new Set()); }); + it.each([ + { + scenario: "the selected row is replaced by an archived generation", + originalArchived: false, + replacement: { sessionId: "replacement-session", archived: true }, + }, + { + scenario: "the selected row disappears from the roster", + originalArchived: false, + replacement: null, + }, + { + scenario: "an archived selection is replaced by an active generation", + originalArchived: true, + replacement: { sessionId: "replacement-session", archived: false }, + }, + ])( + "preserves confirmed deletion identity when $scenario", + async ({ originalArchived, replacement }) => { + const key = "agent:main:confirmed"; + const confirmation = deferred(); + vi.mocked(showConfirmDialog).mockReturnValueOnce(confirmation.promise); + const sessions = createSessions({ + deleteMany: vi.fn(async () => ({ deleted: [], errors: [], preservedWorktrees: [] })), + }); + const page = await createPage( + createContext(createGateway({} as GatewayBrowserClient).gateway, sessions), + ); + page.result = { + count: 1, + sessions: [{ key, sessionId: "confirmed-session", archived: originalArchived }], + } as SessionsListResult; + page.selectedKeys = new Set([key]); + + const deleting = page.deleteSelected(); + expect(showConfirmDialog).toHaveBeenCalledOnce(); + page.result = { + count: replacement ? 1 : 0, + sessions: replacement ? [{ key, ...replacement }] : [], + } as SessionsListResult; + confirmation.resolve(true); + await deleting; + + expect(sessions.deleteMany).toHaveBeenCalledWith([ + { + key, + agentId: undefined, + expectedSessionId: "confirmed-session", + ...(originalArchived ? { archivedOnly: true } : {}), + }, + ]); + }, + ); + it("adopts a managed snapshot that arrives under the bulk-delete lock after its tail refresh", async () => { const deleted = deferred<{ deleted: string[]; diff --git a/ui/src/pages/sessions/sessions-page.ts b/ui/src/pages/sessions/sessions-page.ts index 7da245689ebe..e5f333a3acf0 100644 --- a/ui/src/pages/sessions/sessions-page.ts +++ b/ui/src/pages/sessions/sessions-page.ts @@ -670,6 +670,9 @@ class SessionsPage extends OpenClawLightDomElement { if (!scope) { return; } + // Snapshot identity and archive authority before a replacement can change them. + const rowsByKey = new Map(this.result?.sessions.map((row) => [row.key, row]) ?? []); + const rows = keys.map((key) => rowsByKey.get(key) ?? { key }); const message = t( keys.length === 1 ? "sessionsView.deleteSelectedConfirmOne" @@ -686,10 +689,7 @@ class SessionsPage extends OpenClawLightDomElement { ) { return; } - const rowsByKey = new Map(this.result?.sessions.map((row) => [row.key, row]) ?? []); - // Only current row state may opt into write-scoped archive deletion. - // Unknown selections stay unflagged and therefore admin-only. - await this.deleteSessions(keys.map((key) => rowsByKey.get(key) ?? { key })); + await this.deleteSessions(rows); } private async deleteSessions(