fix(ui): protect replacement sessions during delete confirmation (#129628)

This commit is contained in:
Peter Steinberger
2026-08-25 15:15:20 -07:00
committed by GitHub
parent 2f20142d69
commit 9afc4ef28e
3 changed files with 127 additions and 4 deletions
@@ -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({
@@ -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<boolean>();
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[];
+4 -4
View File
@@ -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(