From 0f65e5e02f2489e5c0edca65139965d2ae46bf62 Mon Sep 17 00:00:00 2001 From: ragesaq <11304287+ragesaq@users.noreply.github.com> Date: Wed, 5 Aug 2026 17:48:41 +0100 Subject: [PATCH] fix: preserve ClickClack discussion state contracts (#118851) Why: managed discussion reconciliation must tolerate current ClickClack response shapes without persisting undefined plugin state or stale display titles. Co-authored-by: Chisel --- .../discussions/durable-room-proof.test.ts | 2 +- .../src/discussions/service-open.ts | 4 +- .../service-state-persistence.test.ts | 93 +++++++++++++++++++ .../src/discussions/service-test-support.ts | 27 +++--- .../src/discussions/service.test.ts | 7 +- .../clickclack/src/discussions/service.ts | 10 +- extensions/clickclack/src/types.ts | 1 + 7 files changed, 123 insertions(+), 21 deletions(-) create mode 100644 extensions/clickclack/src/discussions/service-state-persistence.test.ts diff --git a/extensions/clickclack/src/discussions/durable-room-proof.test.ts b/extensions/clickclack/src/discussions/durable-room-proof.test.ts index 9e8c7d3951c0..59cd8ecc9f76 100644 --- a/extensions/clickclack/src/discussions/durable-room-proof.test.ts +++ b/extensions/clickclack/src/discussions/durable-room-proof.test.ts @@ -12,7 +12,7 @@ import { setClickClackRuntime } from "../runtime.js"; import type { ClickClackChannel, ClickClackMessage, CoreConfig } from "../types.js"; import { ClickClackDiscussionService } from "./service.js"; -type RemoteChannel = ClickClackChannel & { archived_at: string | null }; +type RemoteChannel = ClickClackChannel; type RemotePatch = Record; function memoryStore(): PluginStateSyncKeyedStore { diff --git a/extensions/clickclack/src/discussions/service-open.ts b/extensions/clickclack/src/discussions/service-open.ts index 211689c2d0ce..51bb74debd47 100644 --- a/extensions/clickclack/src/discussions/service-open.ts +++ b/extensions/clickclack/src/discussions/service-open.ts @@ -482,7 +482,9 @@ export async function openClickClackDiscussionBinding( section: currentSection, archived: false, label: currentLabel, - displayTitle: "display_title" in currentChannel ? currentChannel.display_title : undefined, + ...(currentChannel.display_title !== undefined + ? { displayTitle: currentChannel.display_title } + : {}), }; try { store.set(sessionKey, nextBinding); diff --git a/extensions/clickclack/src/discussions/service-state-persistence.test.ts b/extensions/clickclack/src/discussions/service-state-persistence.test.ts new file mode 100644 index 000000000000..bbd91ca1c9b4 --- /dev/null +++ b/extensions/clickclack/src/discussions/service-state-persistence.test.ts @@ -0,0 +1,93 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import type { PluginRuntime } from "openclaw/plugin-sdk/core"; +import type { + OpenKeyedStoreOptions, + PluginStateSyncKeyedStore, +} from "openclaw/plugin-sdk/plugin-state-runtime"; +import { + createPluginStateSyncKeyedStoreForTests, + resetPluginStateStoreForTests, +} from "openclaw/plugin-sdk/plugin-state-test-runtime"; +import { describe, expect, it, vi } from "vitest"; +import type { ClickClackClient } from "../http-client.js"; +import type { ClickClackChannel } from "../types.js"; +import type { ClickClackDiscussionBinding } from "./binding-store.js"; +import { createHarness, testExternalRef } from "./service-test-support.js"; + +function legacyCreateResponse( + input: Parameters[1], +): ClickClackChannel { + const response: ClickClackChannel = { + id: "chn_discussion", + route_id: "discussion-route", + workspace_id: "wsp_team", + ...input, + kind: "public", + created_at: "2026-07-19T00:00:00.000Z", + }; + Reflect.deleteProperty(response, "display_title"); + return response; +} + +describe("ClickClack discussion state persistence", () => { + it("persists legacy create responses through the production plugin-state store", async () => { + resetPluginStateStoreForTests(); + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-clickclack-state-")); + const env = { ...process.env, OPENCLAW_STATE_DIR: stateDir }; + const stores = new Map>(); + const openSyncKeyedStore = ((options: OpenKeyedStoreOptions) => { + const created = createPluginStateSyncKeyedStoreForTests("clickclack", { + ...options, + env, + }); + stores.set(options.namespace, created as PluginStateSyncKeyedStore); + return created; + }) as PluginRuntime["state"]["openSyncKeyedStore"]; + + try { + const harness = createHarness({ label: "Persisted legacy title" }, { openSyncKeyedStore }); + const sessionKey = "agent:main:persisted-legacy-title"; + vi.mocked(harness.createChannel).mockImplementationOnce(async (_workspaceId, input) => + legacyCreateResponse(input), + ); + + await expect(harness.service.open(sessionKey)).resolves.toMatchObject({ state: "open" }); + + const binding = stores + .get("discussion-bindings") + ?.lookup(sessionKey) as ClickClackDiscussionBinding; + expect(binding).toMatchObject({ channelId: "chn_discussion" }); + expect(binding).not.toHaveProperty("displayTitle"); + } finally { + resetPluginStateStoreForTests(); + fs.rmSync(stateDir, { recursive: true, force: true }); + } + }); + + it("clears stale display title confirmation when a patch response omits the field", async () => { + const harness = createHarness({ label: "Original title" }); + const sessionKey = "agent:main:stale-title-confirmation"; + await harness.service.open(sessionKey); + expect(harness.store.lookup(sessionKey)).toMatchObject({ displayTitle: "Original title" }); + + harness.setSessionEntry({ label: "Updated title" }); + vi.mocked(harness.updateChannel).mockImplementationOnce(async (_channelId, patch) => ({ + id: "chn_discussion", + route_id: "discussion-route", + workspace_id: "wsp_team", + name: patch.name ?? "updated-title", + kind: "public", + external_managed: true, + external_ref: testExternalRef(sessionKey), + external_url: "https://control.example/control/chat/main/stale-title-confirmation", + sidebar_section: "Sessions", + created_at: "2026-07-19T00:00:00.000Z", + })); + + await harness.service.reconcile(sessionKey); + + expect(harness.store.lookup(sessionKey)).not.toHaveProperty("displayTitle"); + }); +}); diff --git a/extensions/clickclack/src/discussions/service-test-support.ts b/extensions/clickclack/src/discussions/service-test-support.ts index b03e14d001e6..295fe69bdb54 100644 --- a/extensions/clickclack/src/discussions/service-test-support.ts +++ b/extensions/clickclack/src/discussions/service-test-support.ts @@ -82,6 +82,7 @@ export function createHarness( gatewayEvents?: Pick; startTimer?: boolean; maxRetainedDetachedBindings?: number; + openSyncKeyedStore?: PluginRuntime["state"]["openSyncKeyedStore"]; } = {}, ) { let sessionEntry = entry; @@ -92,15 +93,17 @@ export function createHarness( const runtime = createPluginRuntimeMock({ config: { current: vi.fn(() => config) }, state: { - openSyncKeyedStore: vi.fn((storeOptions: { namespace: string }) => { - if (storeOptions.namespace === "discussion-binding-generations") { - return generationStore; - } - if (storeOptions.namespace === "discussion-revoked-channels") { - return revokedStore; - } - return store; - }) as unknown as PluginRuntime["state"]["openSyncKeyedStore"], + openSyncKeyedStore: + options.openSyncKeyedStore ?? + (vi.fn((storeOptions: { namespace: string }) => { + if (storeOptions.namespace === "discussion-binding-generations") { + return generationStore; + } + if (storeOptions.namespace === "discussion-revoked-channels") { + return revokedStore; + } + return store; + }) as unknown as PluginRuntime["state"]["openSyncKeyedStore"]), }, agent: { session: { @@ -121,7 +124,10 @@ export function createHarness( }), ); const updateChannel = vi.fn( - async (_channelId: string, patch: Parameters[1]) => ({ + async ( + _channelId: string, + patch: Parameters[1], + ): Promise => ({ id: "chn_discussion", route_id: "discussion-route", workspace_id: "wsp_team", @@ -132,7 +138,6 @@ export function createHarness( external_url: patch.external_url ?? "https://control.example/control/chat/main", sidebar_section: patch.sidebar_section ?? "Projects", ...(patch.display_title !== undefined ? { display_title: patch.display_title } : {}), - archived: patch.archived ?? false, created_at: "2026-07-19T00:00:00.000Z", }), ); diff --git a/extensions/clickclack/src/discussions/service.test.ts b/extensions/clickclack/src/discussions/service.test.ts index 82893277669c..ec618db4603e 100644 --- a/extensions/clickclack/src/discussions/service.test.ts +++ b/extensions/clickclack/src/discussions/service.test.ts @@ -208,9 +208,7 @@ describe("ClickClack discussion service", () => { legacyCreateResponse(input), ); await legacy.service.open("agent:main:unconfirmed-title"); - expect(legacy.store.lookup("agent:main:unconfirmed-title")).toMatchObject({ - displayTitle: undefined, - }); + expect(legacy.store.lookup("agent:main:unconfirmed-title")).not.toHaveProperty("displayTitle"); }); it("backfills an unchanged title after a sibling confirms server support", async () => { @@ -560,7 +558,6 @@ describe("ClickClack discussion service", () => { external_ref: externalRef, external_url: "https://control.example/control/chat/main/release-planning-12345678", sidebar_section: "Projects", - archived: false, created_at: "2026-07-19T00:00:00.000Z", }, ]); @@ -574,7 +571,6 @@ describe("ClickClack discussion service", () => { external_ref: patch.external_ref, external_url: patch.external_url, sidebar_section: patch.sidebar_section, - archived: patch.archived, created_at: "2026-07-19T00:00:00.000Z", })); @@ -585,6 +581,7 @@ describe("ClickClack discussion service", () => { "chn_recovered", expect.objectContaining({ external_ref: externalRef, external_managed: true }), ); + expect(harness.store.lookup(sessionKey)).not.toHaveProperty("displayTitle"); expect(opened).toEqual({ state: "open", embedUrl: diff --git a/extensions/clickclack/src/discussions/service.ts b/extensions/clickclack/src/discussions/service.ts index e1553f7a2ccf..cb5c8cbebb0b 100644 --- a/extensions/clickclack/src/discussions/service.ts +++ b/extensions/clickclack/src/discussions/service.ts @@ -466,13 +466,17 @@ export class ClickClackDiscussionService { ) { return; } - this.#store.set(sessionKey, { + const nextBinding: ClickClackDiscussionBinding = { ...latestBinding, externalUrl, label, section, - displayTitle: "display_title" in updated ? updated.display_title : undefined, - }); + ...(updated.display_title !== undefined ? { displayTitle: updated.display_title } : {}), + }; + if (updated.display_title === undefined) { + delete nextBinding.displayTitle; + } + this.#store.set(sessionKey, nextBinding); } #refreshSessionAttachment( diff --git a/extensions/clickclack/src/types.ts b/extensions/clickclack/src/types.ts index 51f59565a202..9809e200f96d 100644 --- a/extensions/clickclack/src/types.ts +++ b/extensions/clickclack/src/types.ts @@ -164,6 +164,7 @@ export type ClickClackChannel = { sidebar_section?: string; display_title?: string; archived?: boolean; + archived_at?: string | null; created_at: string; };