mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
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 <chisel@psiclawops.dev>
This commit is contained in:
@@ -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<string, unknown>;
|
||||
|
||||
function memoryStore<T>(): PluginStateSyncKeyedStore<T> {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<ClickClackClient["createChannel"]>[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<string, PluginStateSyncKeyedStore<unknown>>();
|
||||
const openSyncKeyedStore = (<T>(options: OpenKeyedStoreOptions) => {
|
||||
const created = createPluginStateSyncKeyedStoreForTests<T>("clickclack", {
|
||||
...options,
|
||||
env,
|
||||
});
|
||||
stores.set(options.namespace, created as PluginStateSyncKeyedStore<unknown>);
|
||||
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");
|
||||
});
|
||||
});
|
||||
@@ -82,6 +82,7 @@ export function createHarness(
|
||||
gatewayEvents?: Pick<OpenClawPluginGatewayEvents, "onSessionsChanged">;
|
||||
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<ClickClackClient["updateChannel"]>[1]) => ({
|
||||
async (
|
||||
_channelId: string,
|
||||
patch: Parameters<ClickClackClient["updateChannel"]>[1],
|
||||
): Promise<ClickClackChannel> => ({
|
||||
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",
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -164,6 +164,7 @@ export type ClickClackChannel = {
|
||||
sidebar_section?: string;
|
||||
display_title?: string;
|
||||
archived?: boolean;
|
||||
archived_at?: string | null;
|
||||
created_at: string;
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user