fix(clickclack): accept omitted nullable managed-channel fields and auto-open the discussion panel (#111685)

* fix(clickclack): accept omitted nullable managed-channel fields and auto-open the discussion panel

* fix(ui): open the discussion when write access arrives after it resolved
This commit is contained in:
Peter Steinberger
2026-07-19 23:10:29 -07:00
committed by GitHub
parent f67e55ac00
commit e67fda8ec8
5 changed files with 213 additions and 59 deletions
@@ -8,6 +8,7 @@ import {
import type { ClickClackDiscussionBinding } from "./binding-store.js";
import { discussionCredentialFingerprint } from "./naming.js";
import { markClickClackDiscussionChannelRevoked } from "./revoked-channel-store.js";
import { assertManagedChannelListContract } from "./service-open.js";
import {
TEST_DESTINATION_IDENTITY,
createHarness,
@@ -36,6 +37,62 @@ describe("ClickClack discussion service contracts", () => {
expect(harness.generationStore.lookup("agent:main:unsupported")).toBeUndefined();
});
it("accepts ordinary channels whose nullable managed fields are omitted", async () => {
const harness = createHarness({ label: "Supported server" });
vi.mocked(harness.channels).mockResolvedValue([
{
id: "chn_general",
route_id: "general-route",
workspace_id: "wsp_team",
name: "general",
kind: "public",
external_managed: false,
created_at: "2026-07-19T00:00:00.000Z",
},
]);
await expect(harness.service.open("agent:main:supported")).resolves.toMatchObject({
state: "open",
});
});
it("rejects a non-boolean managed-contract capability signal", async () => {
const harness = createHarness({ label: "Unsupported server" });
vi.mocked(harness.channels).mockResolvedValue([
{
id: "chn_general",
route_id: "general-route",
workspace_id: "wsp_team",
name: "general",
kind: "public",
external_managed: "false" as unknown as boolean,
created_at: "2026-07-19T00:00:00.000Z",
},
]);
await expect(harness.service.open("agent:main:unsupported-type")).rejects.toThrow(
"ClickClack server does not advertise the managed discussion contract",
);
});
it("accepts a managed list entry whose external URL is omitted", () => {
expect(() =>
assertManagedChannelListContract([
{
id: "chn_managed",
route_id: "managed-route",
workspace_id: "wsp_team",
name: "managed",
kind: "public",
external_managed: true,
external_ref: "openclaw:discussion:example",
sidebar_section: "Sessions",
created_at: "2026-07-19T00:00:00.000Z",
},
]),
).not.toThrow();
});
it("does not retain a generation when channel preflight cannot run", async () => {
const harness = createHarness({ label: "Unavailable preflight" });
const sessionKey = "agent:main:unavailable-preflight";
@@ -57,8 +114,9 @@ describe("ClickClack discussion service contracts", () => {
expect(harness.createChannel).toHaveBeenCalledTimes(1);
});
it("rejects a created channel that omits the managed external URL field", async () => {
it("accepts a managed channel whose absent external URL is omitted", async () => {
const harness = createHarness({ label: "Missing URL field" });
harness.config.channels!.clickclack!.discussions!.controlUrlBase = undefined;
vi.mocked(harness.channels).mockResolvedValue([]);
vi.mocked(harness.createChannel).mockImplementationOnce(async (_workspaceId, input) => ({
id: "chn_incompatible",
@@ -70,11 +128,10 @@ describe("ClickClack discussion service contracts", () => {
created_at: "2026-07-19T00:00:00.000Z",
}));
await expect(harness.service.open("agent:main:missing-url-field")).rejects.toThrow(
"ClickClack server does not support the managed discussion channel contract",
);
expect(harness.updateChannel).toHaveBeenCalledWith("chn_incompatible", { archived: true });
expect(harness.revokedStore.entries()).toHaveLength(1);
await expect(harness.service.open("agent:main:missing-url-field")).resolves.toMatchObject({
state: "open",
});
expect(harness.revokedStore.entries()).toHaveLength(0);
expect(harness.generationStore.lookup("agent:main:missing-url-field")).toBeUndefined();
});
@@ -99,8 +99,23 @@ export function assertChannelPatch(
channel: Awaited<ReturnType<ClickClackClient["updateChannel"]>>,
patch: Parameters<ClickClackClient["updateChannel"]>[1],
): void {
for (const key of ["archived", "external_url", "name", "sidebar_section"] as const) {
if (patch[key] !== undefined && channel[key] !== patch[key]) {
for (const key of [
"archived",
"external_managed",
"external_ref",
"external_url",
"name",
"sidebar_section",
] as const) {
const expected = patch[key];
if (expected === undefined) {
continue;
}
const actual =
key === "external_ref" || key === "external_url" || key === "sidebar_section"
? (channel[key] ?? "")
: channel[key];
if (actual !== expected) {
throw new Error(`ClickClack channel update did not apply ${key}`);
}
}
@@ -112,10 +127,9 @@ function assertManagedChannelContract(
): void {
if (
channel.external_managed !== true ||
channel.external_ref !== expected.externalRef ||
channel.sidebar_section !== expected.section ||
typeof channel.external_url !== "string" ||
channel.external_url !== (expected.externalUrl ?? "")
(channel.external_ref ?? "") !== (expected.externalRef ?? "") ||
(channel.sidebar_section ?? "") !== (expected.section ?? "") ||
(channel.external_url ?? "") !== (expected.externalUrl ?? "")
) {
throw new Error(
`ClickClack server does not support the managed discussion channel contract for ${expected.sessionKey}`,
@@ -130,9 +144,9 @@ export function assertManagedChannelListContract(
channels.some(
(channel) =>
typeof channel.external_managed !== "boolean" ||
typeof channel.external_ref !== "string" ||
typeof channel.external_url !== "string" ||
typeof channel.sidebar_section !== "string",
(channel.external_ref !== undefined && typeof channel.external_ref !== "string") ||
(channel.external_url !== undefined && typeof channel.external_url !== "string") ||
(channel.sidebar_section !== undefined && typeof channel.sidebar_section !== "string"),
)
) {
throw new Error("ClickClack server does not advertise the managed discussion contract");
+1 -1
View File
@@ -4152,8 +4152,8 @@ export const en: TranslationMap = {
show: "Show discussion",
disconnected: "Gateway is disconnected.",
loading: "Loading discussion…",
open: "Open discussion",
opening: "Opening discussion…",
requiresWriteAccess: "Operator write access is required to open this discussion.",
opened: "Session discussion",
openExternal: "Open discussion in a new tab",
frameTitle: "Session discussion",
@@ -27,19 +27,21 @@ function mount(params: {
loadInfo: SessionDiscussionInfoLoader;
openDiscussion: SessionDiscussionOpener;
onStateChange?: SessionDiscussionStateListener;
canOpen?: boolean;
}): DiscussionPanelElement {
const panel = document.createElement("openclaw-session-discussion") as DiscussionPanelElement;
panel.sessionKey = "agent:main:first";
panel.loadInfo = params.loadInfo;
panel.openDiscussion = params.openDiscussion;
panel.onStateChange = params.onStateChange ?? vi.fn();
panel.canOpen = params.canOpen ?? true;
document.body.append(panel);
panels.push(panel);
return panel;
}
describe("session discussion panel", () => {
it("loads once, opens an available discussion, and renders both URLs", async () => {
it("automatically opens an available discussion and renders both URLs", async () => {
const loadInfo = vi.fn<SessionDiscussionInfoLoader>().mockResolvedValue({
state: "available",
});
@@ -50,13 +52,6 @@ describe("session discussion panel", () => {
});
const panel = mount({ loadInfo, openDiscussion });
await vi.waitFor(() => {
expect(panel.querySelector("button")?.textContent).toContain("Open discussion");
});
expect(loadInfo).toHaveBeenCalledTimes(1);
panel.querySelector<HTMLButtonElement>("button")?.click();
await vi.waitFor(() => {
expect(panel.querySelector("iframe")?.getAttribute("src")).toBe(
"https://discussion.example/embed/thread",
@@ -66,12 +61,65 @@ describe("session discussion panel", () => {
);
});
const external = panel.querySelector<HTMLAnchorElement>("a");
expect(loadInfo).toHaveBeenCalledTimes(1);
expect(openDiscussion).toHaveBeenCalledTimes(1);
expect(openDiscussion).toHaveBeenCalledWith("agent:main:first");
expect(external?.href).toBe("https://discussion.example/thread");
expect(external?.target).toBe("_blank");
expect(external?.rel).toBe("noopener");
});
it("shows the opening affordance while auto-open is in flight", async () => {
const openDiscussion = vi
.fn<SessionDiscussionOpener>()
.mockImplementation(() => new Promise(() => {}));
const panel = mount({
loadInfo: vi.fn().mockResolvedValue({ state: "available" }),
openDiscussion,
});
await vi.waitFor(() => {
expect(openDiscussion).toHaveBeenCalledTimes(1);
expect(panel.textContent).toContain("Opening discussion");
});
expect(panel.querySelector("button")).toBeNull();
});
it("does not auto-open without operator write access", async () => {
const openDiscussion = vi.fn<SessionDiscussionOpener>();
const panel = mount({
loadInfo: vi.fn().mockResolvedValue({ state: "available" }),
openDiscussion,
canOpen: false,
});
await vi.waitFor(() => {
expect(panel.textContent).toContain("Operator write access is required");
});
expect(openDiscussion).not.toHaveBeenCalled();
expect(panel.querySelector("button")).toBeNull();
});
it("opens once write access is granted after the discussion resolved", async () => {
const openDiscussion = vi.fn<SessionDiscussionOpener>().mockResolvedValue({
state: "open",
embedUrl: "https://clack.example.com/embed/channel/T1/C1",
});
const panel = mount({
loadInfo: vi.fn().mockResolvedValue({ state: "available" }),
openDiscussion,
canOpen: false,
});
await vi.waitFor(() => {
expect(panel.textContent).toContain("Operator write access is required");
});
expect(openDiscussion).not.toHaveBeenCalled();
panel.canOpen = true;
await vi.waitFor(() => expect(openDiscussion).toHaveBeenCalledTimes(1));
});
it("refetches on session switch and reports a hidden discussion", async () => {
const loadInfo = vi
.fn<SessionDiscussionInfoLoader>()
@@ -91,25 +139,54 @@ describe("session discussion panel", () => {
expect(panel.querySelector("iframe")).toBeNull();
});
it("clears an in-flight open state when the session changes", async () => {
const loadInfo = vi.fn<SessionDiscussionInfoLoader>().mockResolvedValue({
state: "available",
});
const openDiscussion = vi
.fn<SessionDiscussionOpener>()
.mockImplementation(() => new Promise(() => {}));
it("ignores an in-flight open result after the session changes", async () => {
let resolveFirstOpen: ((value: { state: "open"; embedUrl: string }) => void) | undefined;
const loadInfo = vi
.fn<SessionDiscussionInfoLoader>()
.mockResolvedValueOnce({ state: "available" })
.mockResolvedValueOnce({ state: "none" });
const openDiscussion = vi.fn<SessionDiscussionOpener>().mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveFirstOpen = resolve;
}),
);
const panel = mount({ loadInfo, openDiscussion });
await vi.waitFor(() => expect(panel.querySelector("button")?.disabled).toBe(false));
panel.querySelector<HTMLButtonElement>("button")?.click();
await vi.waitFor(() => expect(panel.querySelector("button")?.disabled).toBe(true));
await vi.waitFor(() => expect(openDiscussion).toHaveBeenCalledTimes(1));
panel.sessionKey = "agent:main:second";
await vi.waitFor(() => {
expect(loadInfo).toHaveBeenCalledTimes(2);
expect(panel.querySelector("button")?.disabled).toBe(false);
expect(panel.querySelector("button")?.textContent).toContain("Open discussion");
});
resolveFirstOpen?.({ state: "open", embedUrl: "https://discussion.example/stale" });
await panel.updateComplete;
expect(openDiscussion).toHaveBeenCalledTimes(1);
expect(panel.querySelector("iframe")).toBeNull();
expect(panel.textContent).not.toContain("Opening discussion");
});
it("does not auto-open a superseded available resolution", async () => {
let resolveFirstLoad: ((value: { state: "available" }) => void) | undefined;
const loadInfo = vi
.fn<SessionDiscussionInfoLoader>()
.mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveFirstLoad = resolve;
}),
)
.mockResolvedValueOnce({ state: "none" });
const openDiscussion = vi.fn<SessionDiscussionOpener>();
const panel = mount({ loadInfo, openDiscussion });
await vi.waitFor(() => expect(loadInfo).toHaveBeenCalledTimes(1));
panel.sessionKey = "agent:main:second";
await vi.waitFor(() => expect(loadInfo).toHaveBeenCalledTimes(2));
resolveFirstLoad?.({ state: "available" });
await panel.updateComplete;
expect(openDiscussion).not.toHaveBeenCalled();
});
it("does not render non-HTTP discussion URLs", async () => {
@@ -54,9 +54,20 @@ class SessionDiscussionPanel extends OpenClawLightDomElement {
private requestVersion = 0;
private isCurrentRequest(sessionKey: string, version: number): boolean {
return version === this.requestVersion && sessionKey === this.sessionKey.trim();
}
protected override updated(changed: Map<string, unknown>) {
if (changed.has("sessionKey") || changed.has("loadInfo")) {
void this.refresh();
return;
}
// Gaining write access after an available discussion resolved must still
// open it: refresh() already ran, and without the removed manual button
// nothing else would ever call the opener.
if (changed.has("canOpen") && this.canOpen && this.info?.state === "available") {
void this.open(this.sessionKey.trim(), this.requestVersion);
}
}
@@ -85,40 +96,42 @@ class SessionDiscussionPanel extends OpenClawLightDomElement {
this.loading = true;
try {
const info = await loader(sessionKey);
if (version === this.requestVersion) {
if (this.isCurrentRequest(sessionKey, version)) {
this.publish(sessionKey, info);
this.loading = false;
if (info.state === "available" && this.canOpen) {
await this.open(sessionKey, version);
}
}
} catch (error) {
if (version === this.requestVersion) {
if (this.isCurrentRequest(sessionKey, version)) {
this.error = error instanceof Error ? error.message : String(error);
}
} finally {
if (version === this.requestVersion) {
if (this.isCurrentRequest(sessionKey, version)) {
this.loading = false;
}
}
}
private async open(): Promise<void> {
private async open(sessionKey: string, version: number): Promise<void> {
const opener = this.openDiscussion;
const sessionKey = this.sessionKey.trim();
if (!opener || !sessionKey || this.opening) {
if (!opener || !sessionKey || this.opening || !this.isCurrentRequest(sessionKey, version)) {
return;
}
const version = ++this.requestVersion;
this.opening = true;
this.error = null;
try {
const info = await opener(sessionKey);
if (version === this.requestVersion) {
if (this.isCurrentRequest(sessionKey, version)) {
this.publish(sessionKey, info);
}
} catch (error) {
if (version === this.requestVersion) {
if (this.isCurrentRequest(sessionKey, version)) {
this.error = error instanceof Error ? error.message : String(error);
}
} finally {
if (version === this.requestVersion) {
if (this.isCurrentRequest(sessionKey, version)) {
this.opening = false;
}
}
@@ -181,18 +194,11 @@ class SessionDiscussionPanel extends OpenClawLightDomElement {
return nothing;
}
if (this.info.state === "available") {
return html`
<div class="session-discussion__empty">
<button
class="btn primary"
type="button"
?disabled=${!this.canOpen || this.opening}
@click=${() => void this.open()}
>
${this.opening ? t("chat.sessionDiscussion.opening") : t("chat.sessionDiscussion.open")}
</button>
</div>
`;
return html`<div class="session-discussion__empty">
${this.canOpen
? t("chat.sessionDiscussion.opening")
: t("chat.sessionDiscussion.requiresWriteAccess")}
</div>`;
}
return this.renderOpen(this.info);
}