mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 11:55:47 -06:00
fix(ui): surface session sharing errors without leaking across chats (#118135)
This commit is contained in:
committed by
GitHub
parent
82540adc47
commit
3eca723733
@@ -2,6 +2,7 @@
|
||||
import { mkdir } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { chromium, type Browser, type Page } from "playwright";
|
||||
import { expect as expectBrowser } from "playwright/test";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import {
|
||||
canRunPlaywrightChromium,
|
||||
@@ -369,6 +370,49 @@ describeControlUiE2e("Control UI session ownership", () => {
|
||||
expect(await gateway.getRequests("session.visibility.set")).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("keeps rejected visibility-only sharing changes visible after the menu closes", async () => {
|
||||
const context = await browser.newContext({ viewport: { height: 800, width: 1200 } });
|
||||
const currentPage = await context.newPage();
|
||||
page = currentPage;
|
||||
const sessions = draftSessionsList();
|
||||
const ownerSession = sessions.sessions[0];
|
||||
if (!ownerSession) {
|
||||
throw new Error("expected owner draft fixture");
|
||||
}
|
||||
Object.assign(ownerSession, { sharingRole: "owner" });
|
||||
const gateway = await installMockGateway(currentPage, {
|
||||
sessionKey: "agent:main:ada",
|
||||
allowedSessionVisibilities: ["shared", "draft"],
|
||||
deferredMethods: ["session.visibility.set"],
|
||||
featureMethods: ["chat.metadata", "chat.startup", "session.visibility.set"],
|
||||
operatorScopes: ["operator.write"],
|
||||
historyMessages: [{ role: "assistant", content: [{ type: "text", text: "Ready." }] }],
|
||||
methodResponses: { "sessions.list": sessions },
|
||||
});
|
||||
|
||||
await currentPage.goto(`${server?.baseUrl ?? ""}chat`);
|
||||
await currentPage.getByText("Ready.", { exact: true }).waitFor();
|
||||
await currentPage.getByRole("button", { name: "Thread sharing" }).click();
|
||||
const dropdown = currentPage.locator(".chat-pane__sharing-menu");
|
||||
await expect.poll(() => dropdown.getAttribute("open")).not.toBeNull();
|
||||
expect(await dropdown.locator(".chat-pane__sharing-title").count()).toBe(1);
|
||||
await currentPage.getByText("Publish draft", { exact: true }).click();
|
||||
await gateway.waitForRequest("session.visibility.set");
|
||||
await expect.poll(() => dropdown.getAttribute("open")).toBeNull();
|
||||
expect(await gateway.getRequests("session.members.list")).toHaveLength(0);
|
||||
|
||||
const message = "visibility change rejected";
|
||||
await gateway.rejectDeferred("session.visibility.set", {
|
||||
code: "INVALID_REQUEST",
|
||||
message,
|
||||
});
|
||||
const alert = currentPage.getByRole("alert").filter({ hasText: message });
|
||||
await expectBrowser(alert).toBeVisible();
|
||||
|
||||
await currentPage.getByRole("button", { name: "Thread sharing" }).click();
|
||||
await expectBrowser(dropdown.locator(".chat-pane__sharing-status--error")).toBeVisible();
|
||||
});
|
||||
|
||||
it("lets a read-scoped owner inspect sharing but blocks mutations", async () => {
|
||||
const context = await browser.newContext({ viewport: { height: 800, width: 1200 } });
|
||||
const currentPage = await context.newPage();
|
||||
|
||||
@@ -468,7 +468,8 @@ export abstract class ChatPaneHeader extends ChatPaneContext {
|
||||
if (!this.state) {
|
||||
return;
|
||||
}
|
||||
this.state.chatError = error instanceof Error ? error.message : String(error);
|
||||
this.state.lastError = error instanceof Error ? error.message : String(error);
|
||||
this.state.chatError = this.state.lastError;
|
||||
this.state.requestUpdate?.();
|
||||
}
|
||||
|
||||
|
||||
@@ -388,6 +388,8 @@ describe.each(mutations)("chat pane $name mutation connection ownership", (mutat
|
||||
expect(replacement.request).not.toHaveBeenCalled();
|
||||
expect(replacement.sessions.refreshReplacement).not.toHaveBeenCalled();
|
||||
expect(pane.sessionSharingStates.get(replacement.cacheKey)).toBe(replacement.sharingState);
|
||||
expect(state.lastError).toBeNull();
|
||||
expect(state.chatError).toBeNull();
|
||||
},
|
||||
);
|
||||
|
||||
@@ -435,9 +437,53 @@ describe.each(mutations)("chat pane $name mutation connection ownership", (mutat
|
||||
expect(request).toHaveBeenCalledTimes(1);
|
||||
expect(sessions.refreshReplacement).not.toHaveBeenCalled();
|
||||
expect(pane.sessionSharingStates.get(cacheKey)).toBe(replacementState);
|
||||
expect(state.lastError).toBeNull();
|
||||
expect(state.chatError).toBeNull();
|
||||
},
|
||||
);
|
||||
|
||||
it("keeps a previous-session failure out of the newly selected session", async () => {
|
||||
const response = createDeferred<unknown>();
|
||||
const request = vi.fn((method: string) => {
|
||||
if (method !== mutation.method) {
|
||||
throw new Error(`unexpected request: ${method}`);
|
||||
}
|
||||
return response.promise;
|
||||
});
|
||||
const sessions = {
|
||||
refreshReplacement: vi.fn(),
|
||||
} as unknown as SessionCapability;
|
||||
const { pane: testPane, state } = createSharingTestChatPane({
|
||||
client: { request } as unknown as GatewayBrowserClient,
|
||||
sessions,
|
||||
});
|
||||
const pane = testPane as SharingPane;
|
||||
const previous = sessionRow();
|
||||
const selected = {
|
||||
...sessionRow(),
|
||||
key: "agent:main:selected",
|
||||
sessionId: "session-selected",
|
||||
};
|
||||
state.sessionsResult = {
|
||||
...sharingSessionsResult(previous),
|
||||
count: 2,
|
||||
sessions: [previous, selected],
|
||||
};
|
||||
const pending = mutation.invoke(pane, previous);
|
||||
state.sessionKey = selected.key;
|
||||
|
||||
response.reject(new Error(`${mutation.name} failed after session switch`));
|
||||
await pending;
|
||||
|
||||
expect(pane.sessionSharingStates.get(pane.sessionSharingCacheKey(previous.key))).toMatchObject({
|
||||
loading: false,
|
||||
error: `Error: ${mutation.name} failed after session switch`,
|
||||
});
|
||||
expect(state.lastError).toBeNull();
|
||||
expect(state.chatError).toBeNull();
|
||||
expect(sessions.refreshReplacement).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("preserves the current connection failure in the sharing cache", async () => {
|
||||
const request = vi.fn(async (method: string) => {
|
||||
if (method === mutation.method) {
|
||||
@@ -448,7 +494,7 @@ describe.each(mutations)("chat pane $name mutation connection ownership", (mutat
|
||||
const sessions = {
|
||||
refreshReplacement: vi.fn(),
|
||||
} as unknown as SessionCapability;
|
||||
const { pane: testPane } = createSharingTestChatPane({
|
||||
const { pane: testPane, state } = createSharingTestChatPane({
|
||||
client: { request } as unknown as GatewayBrowserClient,
|
||||
sessions,
|
||||
});
|
||||
@@ -461,6 +507,8 @@ describe.each(mutations)("chat pane $name mutation connection ownership", (mutat
|
||||
loading: false,
|
||||
error: `Error: ${mutation.name} failed`,
|
||||
});
|
||||
expect(state.lastError).toBe(`${mutation.name} failed`);
|
||||
expect(state.chatError).toBe(state.lastError);
|
||||
expect(sessions.refreshReplacement).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -136,6 +136,19 @@ export abstract class ChatPaneSharing extends ChatPaneBase {
|
||||
}
|
||||
}
|
||||
|
||||
protected publishSharingFailure(cacheKey: string, sessionKey: string, error: unknown): void {
|
||||
this.setSessionSharingState(cacheKey, {
|
||||
...(this.sessionSharingStates.get(cacheKey) ?? { loading: false }),
|
||||
loading: false,
|
||||
error: String(error),
|
||||
});
|
||||
// Sharing errors stay with their session; the visible page slot belongs
|
||||
// only to the selected session after same-connection navigation.
|
||||
if (areUiSessionKeysEquivalent(this.state?.sessionKey, sessionKey)) {
|
||||
this.publishHeaderError(error);
|
||||
}
|
||||
}
|
||||
|
||||
protected async setSessionVisibility(
|
||||
row: GatewaySessionRow,
|
||||
visibility: SessionVisibility,
|
||||
@@ -181,11 +194,7 @@ export abstract class ChatPaneSharing extends ChatPaneBase {
|
||||
) {
|
||||
return;
|
||||
}
|
||||
this.setSessionSharingState(cacheKey, {
|
||||
...(this.sessionSharingStates.get(cacheKey) ?? { loading: false }),
|
||||
loading: false,
|
||||
error: String(error),
|
||||
});
|
||||
this.publishSharingFailure(cacheKey, currentRow.key, error);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -238,11 +247,7 @@ export abstract class ChatPaneSharing extends ChatPaneBase {
|
||||
) {
|
||||
return;
|
||||
}
|
||||
this.setSessionSharingState(cacheKey, {
|
||||
...(this.sessionSharingStates.get(cacheKey) ?? { loading: false }),
|
||||
loading: false,
|
||||
error: String(error),
|
||||
});
|
||||
this.publishSharingFailure(cacheKey, currentRow.key, error);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -365,6 +365,7 @@ describe("chat pane header state", () => {
|
||||
} satisfies GatewaySessionRow;
|
||||
pane.handleHeaderMenuAction("reveal", session, "/src/openclaw", null);
|
||||
await vi.waitFor(() => expect(state.chatError).toBe("No desktop available."));
|
||||
expect(state.lastError).toBe(state.chatError);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -260,4 +260,34 @@ describe("chat session sharing menu", () => {
|
||||
expect(onOpen).toHaveBeenCalledOnce();
|
||||
expect(onVisibilityChange).toHaveBeenCalledWith("read-only");
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ name: "visibility-only", membersAvailable: false },
|
||||
{ name: "member-enabled", membersAvailable: true },
|
||||
])("shows rejected sharing changes for $name Gateways", ({ membersAvailable }) => {
|
||||
const root = mount(
|
||||
renderChatSessionSharing({
|
||||
session: {
|
||||
key: "agent:main:main",
|
||||
kind: "direct",
|
||||
updatedAt: 1,
|
||||
visibility: "shared",
|
||||
sharingRole: "owner",
|
||||
},
|
||||
state: { loading: false, error: "Visibility update rejected" },
|
||||
allowedVisibilities: ["shared", "read-only"],
|
||||
membersAvailable,
|
||||
onOpen: vi.fn(),
|
||||
onVisibilityChange: vi.fn(),
|
||||
onMemberChange: vi.fn(),
|
||||
}),
|
||||
);
|
||||
|
||||
expect(root.querySelector(".chat-pane__sharing-status--error")?.textContent).toContain(
|
||||
"Visibility update rejected",
|
||||
);
|
||||
expect(root.querySelectorAll(".chat-pane__sharing-title")).toHaveLength(
|
||||
membersAvailable ? 2 : 1,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -172,13 +172,13 @@ export function renderChatSessionSharing(props: ChatSessionSharingProps) {
|
||||
: html`<div class="chat-pane__sharing-status">
|
||||
${t("chat.sessionSharing.noPeople")}
|
||||
</div>`}
|
||||
${props.state?.error
|
||||
? html`<div class="chat-pane__sharing-status chat-pane__sharing-status--error">
|
||||
${props.state.error}
|
||||
</div>`
|
||||
: nothing}
|
||||
`
|
||||
: nothing}
|
||||
${props.state?.error
|
||||
? html`<div class="chat-pane__sharing-status chat-pane__sharing-status--error">
|
||||
${props.state.error}
|
||||
</div>`
|
||||
: nothing}
|
||||
</wa-dropdown>
|
||||
`;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user