diff --git a/docs/channels/clickclack.md b/docs/channels/clickclack.md index 7b47d022a743..1143233d348d 100644 --- a/docs/channels/clickclack.md +++ b/docs/channels/clickclack.md @@ -231,25 +231,26 @@ no account selector, so multiple enabled discussion accounts are rejected rather than choosing one by configuration order. Opening a discussion creates a public ClickClack channel marked as externally -managed. The plugin keeps the session label, category, and archive state in -sync. Restoring a session restores its channel; clearing the session category -moves the channel back to the configured default section. Deleting an -OpenClaw session archives the ClickClack channel instead of deleting it, so its -history remains available. The plugin reconciles bindings when discussion RPCs -are used and approximately once per minute while any bindings exist. +managed. The plugin keeps the session label and category in sync, but channel +lifecycle remains independent. Clearing the session category +moves the channel back to the configured default section. Archiving, resetting, +or deleting an OpenClaw session never archives or replaces the ClickClack +channel. ClickClack owns channel archive and restore independently. The plugin +reconciles bindings when discussion RPCs are used and approximately once per +minute while any bindings exist. Inbound messages in a managed channel use a deterministic side session under the same agent id as the attached main session. The side agent is told which main session to observe and can use `sessions_history` and `session_status` (`changesSince` is useful for incremental checks). It uses `sessions_send` only when people in the discussion ask it to relay or steer the main session. -The binding, managed ownership reference, and side-session peer identity include -the concrete OpenClaw session id along with the pinned ClickClack server and -channel. Resetting a reusable session key or retargeting an account revokes the -old channel locally, archives it when the old credential remains usable, and -cannot reuse its side transcript. Messages arriving through an -archived, reset, disabled, or retargeted binding are dropped instead of falling -back to the account's normal channel routing. Released bindings leave a durable +The binding separates durable room identity from its replaceable session +attachment. The side-session peer identity and scoped grant include the exact +concrete OpenClaw session id, so resetting a reusable session key rotates the +attachment and cannot reuse the old side transcript. The ClickClack channel id, +URL, history, and ownership reference remain unchanged. Messages arriving +through an inactive, disabled, or retargeted attachment are dropped instead of +falling back to the account's normal channel routing. Released bindings leave a durable revoked-channel marker so delayed realtime events remain fail-closed. Remote ownership is keyed by ClickClack server and channel id, so renaming the local account cannot turn a managed channel into an ordinary one. @@ -270,22 +271,22 @@ before persisting a binding. If a create response is lost, the next open adopts the channel by its server-enforced `external_ref` instead of creating another. Until that outcome is reconciled, the pending reservation quarantines otherwise-unbound events in the destination workspace. The coarse reconciler -adopts the channel when the same session is still live or archives it after a -reset; it clears the reservation when no remote channel was created. +adopts the channel when the logical session is active, including after its +concrete session id changes; it clears the reservation when no remote channel +was created. That reference contains a durable per-OpenClaw-installation namespace plus a -hash of the session key, concrete session id, ClickClack destination, and durable +hash of the session key, ClickClack destination, and durable binding generation. Separate gateways cannot adopt each other's channels, -reset sessions cannot inherit old channel history, and an account or workspace +while concrete session resets keep the same channel. An account or workspace round trip cannot re-adopt a previous channel. Bindings are also pinned to the configured ClickClack server URL and are invalidated if the account is retargeted. Changing or removing `controlUrlBase` updates or clears the managed channel link on the next reconciliation pass. Changing -`discussions.workspace` archives and releases the old binding before a channel -can be opened in the new workspace when the old workspace credential remains -configured. If the token was replaced with a workspace-scoped credential that -cannot access the old workspace, OpenClaw records the old channel as revoked and -releases the binding without trying the replacement token; archive that leftover -channel from ClickClack. +`discussions.workspace` releases the old attachment before a channel can be +opened in the new workspace. It never archives the old room. If the token was +replaced with a workspace-scoped credential that cannot access the old +workspace, OpenClaw records the old channel as revoked and releases the binding +without trying the replacement token. The attached main session also receives a pull-only `discussion` tool. It reads the latest messages and recent thread replies as one escaped, attributed record diff --git a/extensions/clickclack/README.md b/extensions/clickclack/README.md index 4dbdfcbb15bc..f0cfce1a0dab 100644 --- a/extensions/clickclack/README.md +++ b/extensions/clickclack/README.md @@ -99,9 +99,10 @@ support and return the managed-channel fields used by this integration. ``` Opening a session discussion creates a public, externally managed channel and -stores its binding in the ClickClack plugin's SQLite state. Session archive, -restore, label, category, and deletion changes are reflected in the channel; -deletion archives the channel and never deletes its messages. `workspace` +stores its binding in the ClickClack plugin's SQLite state. Session label and +category changes remain reflected in the channel, but session archive, restore, +reset, and deletion never archive or replace it. ClickClack owns channel archive +and restore independently. `workspace` defaults to the account workspace, and `section` defaults to `Sessions`. `controlUrlBase` adds a link back to `/chat?session=` in the OpenClaw Control UI. @@ -123,12 +124,11 @@ at its safer default `tree`. A second host-side policy blocks session discovery and alternate targets; the side-agent prompt is not the authorization boundary. The agent still needs those three tools in its effective tool allowlist. -The binding, managed ownership reference, and side-session identity include the -concrete OpenClaw session id as well as the pinned server and channel. Resetting -a reusable session key, replacing a binding, or retargeting it therefore revokes -the old channel locally, archives it when the old credential remains usable, and -starts a fresh channel and side transcript. -Messages arriving through an archived, reset, disabled, or retargeted managed +The binding separates durable room identity from a replaceable exact-session +attachment. Resetting a reusable session key keeps the ClickClack channel, +history, URL, and ownership reference while rotating the side-session identity +and scoped grant. The previous side session cannot access the new main session. +Messages arriving through an inactive, disabled, or retargeted managed binding are dropped instead of falling back to the account's normal channel routing. Released bindings leave a durable revoked-channel marker so delayed realtime events remain fail-closed. Remote ownership is keyed by ClickClack @@ -144,15 +144,12 @@ lifecycle reconciliation pass. If a channel-create response is lost, the pending ownership reservation temporarily quarantines otherwise-unbound events in that workspace. The same -coarse reconciler then adopts the created channel or clears/archives the -ambiguous attempt; a reset cannot make the old channel fall through to ordinary -routing. +coarse reconciler then adopts the created channel or clears the ambiguous +attempt; a reset cannot duplicate or archive the durable room. -When a workspace move keeps the original workspace credential configured, the -plugin archives the old channel before release. If the token is replaced with a -workspace-scoped credential that cannot access the old workspace, OpenClaw -releases the binding into the revoked-channel marker without trying the new token -against the old channel; archive that leftover channel from ClickClack. +Workspace and account moves release local attachment authority without +archiving the old channel. A workspace-scoped replacement token is never tried +against the old channel. The main session gets a read-only `discussion` tool that pulls the latest channel messages, including recent thread replies. The pull uses bounded diff --git a/extensions/clickclack/src/discussions/binding-generation.ts b/extensions/clickclack/src/discussions/binding-generation.ts index 938bac2a82c1..5b3b6dbc6389 100644 --- a/extensions/clickclack/src/discussions/binding-generation.ts +++ b/extensions/clickclack/src/discussions/binding-generation.ts @@ -3,6 +3,8 @@ import type { PluginRuntime } from "openclaw/plugin-sdk/core"; import type { PluginStateSyncKeyedStore } from "openclaw/plugin-sdk/plugin-state-runtime"; type DiscussionBindingGeneration = { + accountId?: string; + credentialFingerprint?: string; destinationIdentity: string; generation: string; pending?: { @@ -49,16 +51,34 @@ function getGenerationStore( export function reserveDiscussionBindingGeneration(params: { runtime: PluginRuntime; sessionKey: string; + accountId: string; + credentialFingerprint: string; destinationIdentity: string; createGeneration?: () => string; }): string { const store = getGenerationStore(params.runtime); const existing = store.lookup(params.sessionKey); - if (existing?.destinationIdentity === params.destinationIdentity) { + const existingAccountId = existing?.accountId ?? existing?.pending?.accountId; + const existingCredentialFingerprint = + existing?.credentialFingerprint ?? existing?.pending?.credentialFingerprint; + if ( + existing?.destinationIdentity === params.destinationIdentity && + existingAccountId === params.accountId && + existingCredentialFingerprint === params.credentialFingerprint + ) { + if (!existing.accountId || !existing.credentialFingerprint) { + store.register(params.sessionKey, { + ...existing, + accountId: params.accountId, + credentialFingerprint: params.credentialFingerprint, + }); + } return existing.generation; } const generation = (params.createGeneration ?? randomUUID)(); store.register(params.sessionKey, { + accountId: params.accountId, + credentialFingerprint: params.credentialFingerprint, destinationIdentity: params.destinationIdentity, generation, }); @@ -94,6 +114,12 @@ export function recordPendingDiscussionOpen(params: { if (!existing || existing.generation !== params.generation) { throw new Error("ClickClack discussion generation changed before channel creation"); } + if ( + existing.accountId !== params.pending.accountId || + existing.credentialFingerprint !== params.pending.credentialFingerprint + ) { + throw new Error("ClickClack discussion ownership changed before channel creation"); + } store.register(params.sessionKey, { ...existing, pending: params.pending }); } @@ -107,6 +133,25 @@ export function listPendingDiscussionOpens(runtime: PluginRuntime): PendingDiscu ); } +/** Stops destination-wide quarantine after the exact remote channel is known. */ +export function clearPendingDiscussionOpen(params: { + runtime: PluginRuntime; + sessionKey: string; + expectedGeneration: string; +}): void { + const store = getGenerationStore(params.runtime); + const existing = store.lookup(params.sessionKey); + if (!existing || existing.generation !== params.expectedGeneration || !existing.pending) { + return; + } + store.register(params.sessionKey, { + accountId: existing.accountId ?? existing.pending.accountId, + credentialFingerprint: existing.credentialFingerprint ?? existing.pending.credentialFingerprint, + destinationIdentity: existing.destinationIdentity, + generation: existing.generation, + }); +} + export function hasPendingDiscussionOpenForDestination(params: { runtime: PluginRuntime; serverBaseUrl: string; diff --git a/extensions/clickclack/src/discussions/binding-store.ts b/extensions/clickclack/src/discussions/binding-store.ts index d076dba5fa0f..3560c6dabf54 100644 --- a/extensions/clickclack/src/discussions/binding-store.ts +++ b/extensions/clickclack/src/discussions/binding-store.ts @@ -24,18 +24,6 @@ export type ClickClackDiscussionBinding = { displayTitle?: string; }; -export function bindingMatchesSessionIncarnation( - runtime: PluginRuntime, - sessionKey: string, - binding: ClickClackDiscussionBinding, -): boolean { - const entry = runtime.agent.session.getSessionEntry({ - sessionKey, - readConsistency: "latest", - }); - return Boolean(entry && binding.sessionId && entry.sessionId === binding.sessionId); -} - export function bindingMatchesActiveSessionIncarnation( runtime: PluginRuntime, sessionKey: string, @@ -53,6 +41,32 @@ export function bindingMatchesActiveSessionIncarnation( ); } +/** + * Refresh the replaceable session attachment without changing the durable room identity. + * The store registers persisted state before reindexing, so a failed write leaves the + * previous attachment authoritative in both persistence and memory. + */ +export function attachBindingToCurrentActiveSession(params: { + runtime: PluginRuntime; + store: ClickClackDiscussionBindingStore; + sessionKey: string; + binding: ClickClackDiscussionBinding; +}): ClickClackDiscussionBinding | undefined { + const entry = params.runtime.agent.session.getSessionEntry({ + sessionKey: params.sessionKey, + readConsistency: "latest", + }); + if (!entry?.sessionId || entry.archivedAt !== undefined) { + return undefined; + } + if (entry.sessionId === params.binding.sessionId) { + return params.binding; + } + const attached = { ...params.binding, sessionId: entry.sessionId }; + params.store.set(params.sessionKey, attached); + return attached; +} + const DISCUSSION_BINDINGS_NAMESPACE = "discussion-bindings"; const MAX_DISCUSSION_BINDINGS = 10_000; const storesByRuntime = new WeakMap(); diff --git a/extensions/clickclack/src/discussions/durable-room-proof.test.ts b/extensions/clickclack/src/discussions/durable-room-proof.test.ts new file mode 100644 index 000000000000..9e8c7d3951c0 --- /dev/null +++ b/extensions/clickclack/src/discussions/durable-room-proof.test.ts @@ -0,0 +1,297 @@ +import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; +import type { AddressInfo } from "node:net"; +import { createPluginRuntimeMock } from "openclaw/plugin-sdk/channel-test-helpers"; +import type { OpenClawPluginSessionsChangedEvent, PluginRuntime } from "openclaw/plugin-sdk/core"; +import type { PluginStateSyncKeyedStore } from "openclaw/plugin-sdk/plugin-state-runtime"; +import { buildAgentSessionKey, resolveAgentRoute } from "openclaw/plugin-sdk/routing"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { resolveClickClackAccount } from "../accounts.js"; +import { createClickClackClient } from "../http-client.js"; +import { handleClickClackInbound } from "../inbound.js"; +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 RemotePatch = Record; + +function memoryStore(): PluginStateSyncKeyedStore { + const values = new Map(); + return { + register: (key, value) => values.set(key, { value, createdAt: Date.now() }), + registerIfAbsent(key, value) { + if (values.has(key)) { + return false; + } + values.set(key, { value, createdAt: Date.now() }); + return true; + }, + lookup: (key) => values.get(key)?.value, + consume(key) { + const value = values.get(key)?.value; + values.delete(key); + return value; + }, + delete: (key) => values.delete(key), + entries: () => + Array.from(values, ([key, entry]) => ({ + key, + value: entry.value, + createdAt: entry.createdAt, + })), + clear: () => values.clear(), + }; +} + +async function readJson(req: IncomingMessage): Promise> { + const chunks: Buffer[] = []; + for await (const chunk of req) { + chunks.push(typeof chunk === "string" ? Buffer.from(chunk) : chunk); + } + const text = Buffer.concat(chunks).toString("utf8"); + return text ? (JSON.parse(text) as Record) : {}; +} + +function sendJson(res: ServerResponse, status: number, body: unknown): void { + res.writeHead(status, { "content-type": "application/json" }); + res.end(JSON.stringify(body)); +} + +async function startMockClickClack() { + const patches: RemotePatch[] = []; + const messages: ClickClackMessage[] = []; + const channels: RemoteChannel[] = [ + { + id: "chn_general", + route_id: "general-route", + workspace_id: "wsp_team", + name: "general", + kind: "public", + external_managed: false, + external_ref: "", + external_url: "", + sidebar_section: "", + archived: false, + archived_at: null, + created_at: "2026-08-04T00:00:00.000Z", + }, + ]; + let createCount = 0; + async function handleRequest(req: IncomingMessage, res: ServerResponse): Promise { + const url = new URL(req.url ?? "/", "http://127.0.0.1"); + if (req.headers.authorization !== "Bearer proof-token") { + sendJson(res, 401, { error: "unauthorized" }); + return; + } + if (req.method === "GET" && url.pathname === "/api/workspaces") { + sendJson(res, 200, { + workspaces: [ + { + id: "wsp_team", + route_id: "team-route", + slug: "team", + name: "Team", + created_at: "2026-08-04T00:00:00.000Z", + }, + ], + }); + return; + } + if (req.method === "GET" && url.pathname === "/api/workspaces/wsp_team/channels") { + sendJson(res, 200, { channels }); + return; + } + if (req.method === "POST" && url.pathname === "/api/workspaces/wsp_team/channels") { + createCount += 1; + const input = await readJson(req); + const channel = { + id: "chn_durable", + route_id: "durable-route", + workspace_id: "wsp_team", + ...input, + kind: "public", + archived: false, + archived_at: null, + created_at: "2026-08-04T00:00:01.000Z", + } as RemoteChannel; + channels.push(channel); + sendJson(res, 200, { channel }); + return; + } + if (req.method === "PATCH" && url.pathname === "/api/channels/chn_durable") { + const patch = await readJson(req); + patches.push(patch); + Object.assign(channels[1]!, patch); + sendJson(res, 200, { channel: channels[1] }); + return; + } + if (req.method === "GET" && url.pathname === "/api/channels/chn_durable/messages") { + sendJson(res, 200, { messages, oldest_seq: 1, has_older: false }); + return; + } + sendJson(res, 404, { error: `unexpected ${req.method} ${url.pathname}` }); + } + const server = createServer((req, res) => { + void handleRequest(req, res).catch(() => { + sendJson(res, 500, { error: "mock ClickClack request failed" }); + }); + }); + await new Promise((resolve) => { + server.listen(0, "127.0.0.1", resolve); + }); + const address = server.address() as AddressInfo; + return { + baseUrl: `http://127.0.0.1:${address.port}`, + channels, + messages, + patches, + get createCount() { + return createCount; + }, + close: async () => + await new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }), + }; +} + +describe("ClickClack durable room real-behavior proof", () => { + const cleanups: Array<() => Promise> = []; + + afterEach(async () => { + await Promise.allSettled(cleanups.splice(0).map(async (cleanup) => await cleanup())); + }); + + it("keeps one remote room and delivers inbound after archive, reset, deletion, and recreation", async () => { + const remote = await startMockClickClack(); + cleanups.push(remote.close); + const sessionKey = "agent:research:proof-room"; + let sessionEntry: + | { sessionId: string; label: string; updatedAt: number; archivedAt?: number } + | undefined = { + sessionId: "session-original", + label: "Durable proof room", + updatedAt: 1, + }; + const config = { + channels: { + clickclack: { + enabled: true, + baseUrl: remote.baseUrl, + token: "proof-token", + workspace: "team", + discussions: { enabled: true, workspace: "team", section: "Sessions" }, + allowFrom: ["*"], + }, + }, + } satisfies CoreConfig; + const stores = new Map>(); + const runtime = createPluginRuntimeMock({ + config: { current: vi.fn(() => config) }, + state: { + openSyncKeyedStore: vi.fn(({ namespace }: { namespace: string }) => { + const existing = stores.get(namespace); + if (existing) { + return existing; + } + const created = memoryStore(); + stores.set(namespace, created); + return created; + }) as unknown as PluginRuntime["state"]["openSyncKeyedStore"], + }, + agent: { + session: { getSessionEntry: vi.fn(() => sessionEntry) }, + }, + channel: { + routing: { + resolveAgentRoute: vi.fn( + (params: Parameters[0]) => + resolveAgentRoute(params), + ), + buildAgentSessionKey: vi.fn( + (params: Parameters[0]) => + buildAgentSessionKey(params), + ), + }, + }, + } as unknown as PluginRuntime); + setClickClackRuntime(runtime); + const eventHandlers = new Set<(event: OpenClawPluginSessionsChangedEvent) => void>(); + const service = new ClickClackDiscussionService(runtime, { + clientFactory: (account) => + createClickClackClient({ baseUrl: account.apiEndpoint, token: account.token }), + installationId: "11111111-2222-4333-8444-555555555555", + bindingGenerationFactory: () => "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee", + gatewayEvents: { + onSessionsChanged(handler) { + eventHandlers.add(handler); + return () => eventHandlers.delete(handler); + }, + }, + startTimer: false, + }); + cleanups.push(async () => service.cleanup()); + const emit = async (event: OpenClawPluginSessionsChangedEvent) => { + for (const handler of eventHandlers) { + handler(event); + } + await new Promise((resolve) => { + setTimeout(resolve, 300); + }); + }; + + await service.open(sessionKey); + const originalExternalRef = remote.channels[1]?.external_ref; + remote.messages.push({ + id: "msg_history", + workspace_id: "wsp_team", + channel_id: "chn_durable", + author_id: "usr_owner", + thread_root_id: "msg_history", + body: "durable history marker", + body_format: "markdown", + created_at: "2026-08-04T00:00:02.000Z", + author: { + id: "usr_owner", + kind: "human", + display_name: "Owner", + handle: "owner", + avatar_url: "", + created_at: "2026-08-04T00:00:00.000Z", + }, + }); + + sessionEntry = { ...sessionEntry!, archivedAt: 2 }; + await emit({ sessionKey, reason: "archive" }); + sessionEntry = { sessionId: "session-reset", label: "Durable proof room", updatedAt: 3 }; + await emit({ sessionKey, reason: "reset" }); + sessionEntry = undefined; + await emit({ sessionKey, reason: "delete" }); + sessionEntry = { sessionId: "session-recreated", label: "Durable proof room", updatedAt: 4 }; + await emit({ sessionKey, reason: "create" }); + + const history = await service.readLatestMessages(sessionKey, 30); + await handleClickClackInbound({ + account: resolveClickClackAccount({ cfg: config }), + config, + message: { + ...remote.messages[0]!, + id: "msg_inbound", + body: "continue after recreation", + }, + }); + + const managed = remote.channels.filter((channel) => channel.external_managed === true); + expect(remote.createCount).toBe(1); + expect(managed).toHaveLength(1); + expect(managed[0]).toMatchObject({ + id: "chn_durable", + external_ref: originalExternalRef, + archived: false, + archived_at: null, + }); + expect(remote.patches.every((patch) => !("archived" in patch))).toBe(true); + expect(history.text).toContain("durable history marker"); + expect(runtime.channel.inbound.dispatch).toHaveBeenCalledOnce(); + }); +}); diff --git a/extensions/clickclack/src/discussions/naming.ts b/extensions/clickclack/src/discussions/naming.ts index 4ffe55fb1b3d..f67e07a81f9c 100644 --- a/extensions/clickclack/src/discussions/naming.ts +++ b/extensions/clickclack/src/discussions/naming.ts @@ -80,12 +80,11 @@ export function isDiscussionSessionKey(sessionKey: string): boolean { export function discussionExternalRef( installationId: string, mainSessionKey: string, - sessionId: string, destinationIdentity: string, bindingGeneration: string, ): string { return `openclaw:${installationId}:${shortSessionHash( - [mainSessionKey, sessionId, destinationIdentity, bindingGeneration].join("\0"), + [mainSessionKey, destinationIdentity, bindingGeneration].join("\0"), )}`; } diff --git a/extensions/clickclack/src/discussions/routing.ts b/extensions/clickclack/src/discussions/routing.ts index 48e104bf1fc8..3718a83eb699 100644 --- a/extensions/clickclack/src/discussions/routing.ts +++ b/extensions/clickclack/src/discussions/routing.ts @@ -2,8 +2,9 @@ import type { PluginRuntime } from "openclaw/plugin-sdk/core"; import type { CoreConfig } from "../types.js"; import { hasPendingDiscussionOpenForDestination } from "./binding-generation.js"; import { - bindingMatchesActiveSessionIncarnation, + attachBindingToCurrentActiveSession, getClickClackDiscussionBindingStore, + type ClickClackDiscussionBinding, } from "./binding-store.js"; import { resolveDiscussionBindingAccount } from "./eligibility.js"; import { discussionSessionKey } from "./naming.js"; @@ -44,26 +45,37 @@ export function resolveClickClackDiscussionRoute(params: { if (matched.binding.serverBaseUrl !== params.serverBaseUrl.replace(/\/+$/u, "")) { return { state: "revoked" }; } - if (matched.binding.archived) { - return { state: "revoked" }; - } if (resolveDiscussionBindingAccount(params.config, matched.binding).state !== "active") { return { state: "revoked" }; } - if ( - !bindingMatchesActiveSessionIncarnation(params.runtime, matched.sessionKey, matched.binding) - ) { + let binding: ClickClackDiscussionBinding | undefined; + try { + binding = attachBindingToCurrentActiveSession({ + runtime: params.runtime, + store, + sessionKey: matched.sessionKey, + binding: matched.binding, + }); + } catch (error) { + params.runtime.logging + .getChildLogger({ plugin: "clickclack", feature: "discussions" }) + .warn( + `discussion attachment refresh failed for channel ${params.channelId}: ${String(error)}`, + ); + return { state: "revoked" }; + } + if (!binding) { return { state: "revoked" }; } const sessionKey = discussionSessionKey({ runtime: params.runtime, - agentId: matched.binding.agentId, + agentId: binding.agentId, mainSessionKey: matched.sessionKey, - sessionId: matched.binding.sessionId, + sessionId: binding.sessionId, accountId: params.accountId, - serverBaseUrl: matched.binding.serverBaseUrl, - channelId: matched.binding.channelId, - externalRef: matched.binding.externalRef, + serverBaseUrl: binding.serverBaseUrl, + channelId: binding.channelId, + externalRef: binding.externalRef, }); if (!sessionKey) { return { state: "revoked" }; @@ -71,7 +83,7 @@ export function resolveClickClackDiscussionRoute(params: { return { state: "active", route: { - agentId: matched.binding.agentId, + agentId: binding.agentId, sessionKey, systemPrompt: [ "You are the side agent for a ClickClack discussion attached to an OpenClaw session.", diff --git a/extensions/clickclack/src/discussions/service-contract.test.ts b/extensions/clickclack/src/discussions/service-contract.test.ts index 991c9077544b..773a493ffe0b 100644 --- a/extensions/clickclack/src/discussions/service-contract.test.ts +++ b/extensions/clickclack/src/discussions/service-contract.test.ts @@ -1,5 +1,4 @@ import { describe, expect, it, vi } from "vitest"; -import type { ClickClackClient } from "../http-client.js"; import type { ClickClackMessage } from "../types.js"; import { recordPendingDiscussionOpen, @@ -191,31 +190,30 @@ describe("ClickClack discussion service contracts", () => { expect(harness.generationStore.lookup("agent:main:missing-url-field")).toBeUndefined(); }); - it("retains incompatible channel recovery state when archival fails", async () => { - const harness = createHarness({ label: "Incompatible archival failure" }); - const sessionKey = "agent:main:incompatible-archive-failure"; + it("retains incompatible channel recovery state without mutating the room", async () => { + const harness = createHarness({ label: "Incompatible recovery" }); + const sessionKey = "agent:main:incompatible-recovery"; vi.mocked(harness.channels).mockResolvedValue([]); vi.mocked(harness.createChannel).mockImplementationOnce(async (_workspaceId, input) => ({ - id: "chn_incompatible_archive_failure", - route_id: "incompatible-archive-failure-route", + id: "chn_incompatible_recovery", + route_id: "incompatible-recovery-route", workspace_id: "wsp_team", ...input, external_url: undefined, kind: "public", created_at: "2026-07-19T00:00:00.000Z", })); - vi.mocked(harness.updateChannel).mockRejectedValueOnce(new Error("archive unavailable")); - await expect(harness.service.open(sessionKey)).rejects.toThrow( "managed discussion channel contract", ); expect(harness.generationStore.lookup(sessionKey)).toMatchObject({ - pending: expect.objectContaining({ sessionId: "session-id" }), + generation: expect.any(String), }); + expect(harness.generationStore.lookup(sessionKey)).not.toHaveProperty("pending"); }); - it("archives a newly created channel whose route id is missing", async () => { + it("quarantines a newly created channel whose route id is missing", async () => { const harness = createHarness({ label: "Missing route" }); vi.mocked(harness.createChannel).mockImplementationOnce(async (_workspaceId, input) => ({ id: "chn_route_less", @@ -229,31 +227,30 @@ describe("ClickClack discussion service contracts", () => { await expect(harness.service.open("agent:main:missing-route")).rejects.toThrow( "ClickClack discussion channel is missing its route id", ); - expect(harness.updateChannel).toHaveBeenCalledWith("chn_route_less", { archived: true }); + expect(harness.updateChannel).not.toHaveBeenCalled(); expect(harness.revokedStore.entries()).toHaveLength(1); - expect(harness.generationStore.lookup("agent:main:missing-route")).toBeUndefined(); + expect(harness.generationStore.lookup("agent:main:missing-route")).toBeDefined(); }); - it("retains route-less channel recovery state when archival fails", async () => { - const harness = createHarness({ label: "Route-less archival failure" }); - const sessionKey = "agent:main:route-less-archive-failure"; + it("retains route-less channel recovery state without mutating the room", async () => { + const harness = createHarness({ label: "Route-less recovery" }); + const sessionKey = "agent:main:route-less-recovery"; vi.mocked(harness.createChannel).mockImplementationOnce(async (_workspaceId, input) => ({ - id: "chn_route_less_archive_failure", + id: "chn_route_less_recovery", route_id: "", workspace_id: "wsp_team", ...input, kind: "public", created_at: "2026-07-19T00:00:00.000Z", })); - vi.mocked(harness.updateChannel).mockRejectedValueOnce(new Error("archive unavailable")); - await expect(harness.service.open(sessionKey)).rejects.toThrow( "ClickClack discussion channel is missing its route id", ); expect(harness.generationStore.lookup(sessionKey)).toMatchObject({ - pending: expect.objectContaining({ sessionId: "session-id" }), + generation: expect.any(String), }); + expect(harness.generationStore.lookup(sessionKey)).not.toHaveProperty("pending"); }); it("rejects ambiguous multi-account discussion configuration", async () => { @@ -468,7 +465,7 @@ describe("ClickClack discussion service contracts", () => { expect(harness.updateChannel).not.toHaveBeenCalled(); }); - it("archives and releases a binding when its configured workspace changes", async () => { + it("releases a binding without archiving when its configured workspace changes", async () => { const harness = createHarness({ label: "Workspace retarget" }); const sessionKey = "agent:main:workspace-retarget"; await harness.service.open(sessionKey); @@ -479,54 +476,34 @@ describe("ClickClack discussion service contracts", () => { ); expect(harness.updateChannel).not.toHaveBeenCalled(); await harness.service.reconcile(sessionKey); - expect(harness.updateChannel).toHaveBeenCalledWith("chn_discussion", { archived: true }); + expect(harness.updateChannel).not.toHaveBeenCalled(); harness.config.channels!.clickclack!.discussions!.workspace = "team"; expect(await harness.service.info(sessionKey)).toEqual({ state: "available" }); }); - it("retains a stale binding for retry when archival fails", async () => { + it("releases a stale binding locally without mutating the room", async () => { const harness = createHarness({ label: "Retry cleanup" }); const sessionKey = "agent:main:cleanup-retry"; await harness.service.open(sessionKey); harness.config.channels!.clickclack!.discussions!.workspace = "other-team"; - vi.mocked(harness.updateChannel).mockRejectedValueOnce(new Error("temporary outage")); - - await expect(harness.service.open(sessionKey)).rejects.toThrow("temporary outage"); + await expect(harness.service.open(sessionKey)).rejects.toThrow( + "ClickClack discussions workspace not found: other-team", + ); expect(harness.createChannel).toHaveBeenCalledTimes(1); + expect(harness.updateChannel).not.toHaveBeenCalled(); harness.config.channels!.clickclack!.discussions!.workspace = "team"; - expect(await harness.service.info(sessionKey)).toMatchObject({ state: "open" }); + expect(await harness.service.info(sessionKey)).toEqual({ state: "available" }); }); - it("serializes stale info cleanup before a replacement open", async () => { + it("serializes local stale cleanup before a replacement open", async () => { const harness = createHarness({ label: "Concurrent cleanup" }); const sessionKey = "agent:main:concurrent-cleanup"; await harness.service.open(sessionKey); harness.config.channels!.clickclack!.discussions!.workspace = "wsp_team"; - let releaseArchive: (() => void) | undefined; - const archiveGate = new Promise((resolve) => { - releaseArchive = resolve; - }); - const defaultUpdate = vi.mocked(harness.updateChannel).getMockImplementation() as - | (( - ...args: Parameters - ) => ReturnType) - | undefined; - if (!defaultUpdate) { - throw new Error("expected update implementation"); - } - vi.mocked(harness.updateChannel).mockImplementationOnce(async (...args) => { - await archiveGate; - return await defaultUpdate(...args); - }); - - const info = harness.service.info(sessionKey); - await vi.waitFor(() => expect(harness.updateChannel).toHaveBeenCalledTimes(1)); - const open = harness.service.open(sessionKey); - releaseArchive?.(); - - expect(await info).toEqual({ state: "available" }); - expect(await open).toMatchObject({ state: "open" }); + expect(await harness.service.info(sessionKey)).toEqual({ state: "available" }); + expect(await harness.service.open(sessionKey)).toMatchObject({ state: "open" }); expect(harness.createChannel).toHaveBeenCalledTimes(2); + expect(harness.updateChannel).not.toHaveBeenCalled(); expect(harness.store.lookup(sessionKey)).toMatchObject({ workspaceRef: "wsp_team" }); }); @@ -543,7 +520,7 @@ describe("ClickClack discussion service contracts", () => { expect(harness.createChannel).not.toHaveBeenCalled(); }); - it("archives the remote channel when binding persistence fails", async () => { + it("keeps the remote channel quarantined when binding persistence fails", async () => { const harness = createHarness({ label: "Persistence failure" }); harness.store.register = vi.fn(() => { throw new Error("SQLITE_FULL: database is full"); @@ -553,24 +530,23 @@ describe("ClickClack discussion service contracts", () => { "SQLITE_FULL", ); expect(harness.createChannel).toHaveBeenCalledTimes(1); - expect(harness.updateChannel).toHaveBeenCalledWith("chn_discussion", { archived: true }); + expect(harness.updateChannel).not.toHaveBeenCalled(); expect(harness.revokedStore.entries()).toHaveLength(1); - expect(harness.generationStore.lookup("agent:main:persistence-failure")).toBeUndefined(); + expect(harness.generationStore.lookup("agent:main:persistence-failure")).toBeDefined(); }); - it("retains the reservation when binding persistence and archival both fail", async () => { + it("retains the reservation when binding persistence fails", async () => { const harness = createHarness({ label: "Persistence and archive failure" }); const sessionKey = "agent:main:persistence-archive-failure"; harness.store.register = vi.fn(() => { throw new Error("SQLITE_FULL: database is full"); }); - vi.mocked(harness.updateChannel).mockRejectedValueOnce(new Error("archive unavailable")); - await expect(harness.service.open(sessionKey)).rejects.toThrow("SQLITE_FULL"); expect(harness.generationStore.lookup(sessionKey)).toMatchObject({ - pending: expect.objectContaining({ sessionId: "session-id" }), + generation: expect.any(String), }); + expect(harness.generationStore.lookup(sessionKey)).not.toHaveProperty("pending"); expect(harness.revokedStore.entries()).toHaveLength(1); }); @@ -585,6 +561,8 @@ describe("ClickClack discussion service contracts", () => { const generation = reserveDiscussionBindingGeneration({ runtime: harness.runtime, sessionKey, + accountId: binding.accountId, + credentialFingerprint: binding.credentialFingerprint, destinationIdentity: TEST_DESTINATION_IDENTITY, createGeneration: () => "interrupted-commit-generation", }); @@ -610,6 +588,42 @@ describe("ClickClack discussion service contracts", () => { expect(harness.revokedStore.entries()).toHaveLength(0); }); + it("rejects a pending open when ownership changes after reservation", () => { + const harness = createHarness({ label: "Ownership changed" }); + const sessionKey = "agent:main:ownership-changed"; + const credentialFingerprint = discussionCredentialFingerprint("original-token"); + const generation = reserveDiscussionBindingGeneration({ + runtime: harness.runtime, + sessionKey, + accountId: "account-original", + credentialFingerprint, + destinationIdentity: TEST_DESTINATION_IDENTITY, + createGeneration: () => "ownership-generation", + }); + + expect(() => + recordPendingDiscussionOpen({ + runtime: harness.runtime, + sessionKey, + generation, + pending: { + accountId: "account-replacement", + serverBaseUrl: "https://clickclack.example", + workspaceId: "wsp_team", + sessionId: "session-replacement", + externalRef: "openclaw:discussion:ownership-generation", + credentialFingerprint: discussionCredentialFingerprint("replacement-token"), + }, + }), + ).toThrow("ClickClack discussion ownership changed before channel creation"); + expect(harness.generationStore.lookup(sessionKey)).toEqual({ + accountId: "account-original", + credentialFingerprint, + destinationIdentity: TEST_DESTINATION_IDENTITY, + generation, + }); + }); + it("lets a durable revocation marker override a surviving binding", async () => { const harness = createHarness({ label: "Revoked binding" }); const sessionKey = "agent:main:revoked-binding"; @@ -720,6 +734,61 @@ describe("ClickClack discussion service contracts", () => { ); }); + it("records attachment persistence failures while reading discussion history", async () => { + const harness = createHarness({ sessionId: "session-old", label: "History reset" }); + const sessionKey = "agent:main:history-reset"; + await harness.service.open(sessionKey); + harness.setSessionEntry({ sessionId: "session-new", label: "History reset" }); + harness.store.register = vi.fn(() => { + throw new Error("SQLITE_FULL"); + }); + + expect(await harness.service.readLatestMessages(sessionKey, 30)).toEqual({ + text: "No discussion is bound to this session.", + }); + expect(harness.store.lookup(sessionKey)).toMatchObject({ sessionId: "session-old" }); + const loggerCall = vi + .mocked(harness.runtime.logging.getChildLogger) + .mock.calls.findIndex( + ([context]) => context?.plugin === "clickclack" && context.feature === "discussions", + ); + const logger = vi.mocked(harness.runtime.logging.getChildLogger).mock.results[loggerCall] + ?.value; + expect(logger?.warn).toHaveBeenCalledWith( + `discussion attachment refresh failed for ${sessionKey}: Error: SQLITE_FULL`, + ); + }); + + it("keeps the previous attachment when reconciliation cannot persist a reset", async () => { + const harness = createHarness({ sessionId: "session-old", label: "Reconcile reset" }); + const sessionKey = "agent:main:reconcile-reset"; + await harness.service.open(sessionKey); + harness.updateChannel.mockClear(); + harness.setSessionEntry({ sessionId: "session-new", label: "Reconcile reset" }); + const register = vi.fn(() => { + throw new Error("SQLITE_FULL"); + }); + harness.store.register = register; + + await expect(harness.service.info(sessionKey)).resolves.toMatchObject({ state: "open" }); + await expect(harness.service.open(sessionKey)).resolves.toMatchObject({ state: "open" }); + await expect(harness.service.reconcile(sessionKey)).resolves.toBeUndefined(); + + expect(harness.store.lookup(sessionKey)).toMatchObject({ sessionId: "session-old" }); + expect(register).toHaveBeenCalledTimes(3); + expect(harness.updateChannel).not.toHaveBeenCalled(); + const loggerCall = vi + .mocked(harness.runtime.logging.getChildLogger) + .mock.calls.findIndex( + ([context]) => context?.plugin === "clickclack" && context.feature === "discussions", + ); + const logger = vi.mocked(harness.runtime.logging.getChildLogger).mock.results[loggerCall] + ?.value; + expect(logger?.warn).toHaveBeenCalledWith( + `discussion attachment refresh failed for ${sessionKey}: Error: SQLITE_FULL`, + ); + }); + it("quotes untrusted message and author fields without forgeable transcript lines", async () => { const harness = createHarness({ label: "Support" }); const sessionKey = "agent:main:quoted-support"; diff --git a/extensions/clickclack/src/discussions/service-events.test.ts b/extensions/clickclack/src/discussions/service-events.test.ts index fb446d500d9d..f15874513d88 100644 --- a/extensions/clickclack/src/discussions/service-events.test.ts +++ b/extensions/clickclack/src/discussions/service-events.test.ts @@ -3,6 +3,7 @@ import type { OpenClawPluginSessionsChangedEvent, } from "openclaw/plugin-sdk/core"; import { describe, expect, it, vi } from "vitest"; +import type { ClickClackDiscussionBinding } from "./binding-store.js"; import { createHarness } from "./service-test-support.js"; function createGatewayEventsHarness() { @@ -84,6 +85,57 @@ describe("ClickClack discussion session events", () => { } }); + it("keeps one durable room through archive, reset, deletion, and recreation events", async () => { + vi.useFakeTimers(); + const gateway = createGatewayEventsHarness(); + const harness = createHarness( + { sessionId: "session-original", label: "Durable event room" }, + { gatewayEvents: gateway.gatewayEvents }, + ); + const sessionKey = "agent:main:event-durable-room"; + try { + await harness.service.open(sessionKey); + const originalBinding = harness.store.lookup(sessionKey) as + | ClickClackDiscussionBinding + | undefined; + if (!originalBinding) { + throw new Error("expected persisted binding"); + } + harness.updateChannel.mockClear(); + + harness.setSessionEntry({ + sessionId: "session-original", + label: "Durable event room", + archivedAt: 123, + }); + gateway.emit({ sessionKey, reason: "archive" }); + await vi.advanceTimersByTimeAsync(250); + + harness.setSessionEntry({ sessionId: "session-reset", label: "Durable event room" }); + gateway.emit({ sessionKey, reason: "reset" }); + await vi.advanceTimersByTimeAsync(250); + + harness.setSessionEntry(undefined); + gateway.emit({ sessionKey, reason: "delete" }); + await vi.advanceTimersByTimeAsync(250); + + harness.setSessionEntry({ sessionId: "session-recreated", label: "Durable event room" }); + gateway.emit({ sessionKey, reason: "create" }); + await vi.advanceTimersByTimeAsync(250); + + expect(harness.createChannel).toHaveBeenCalledOnce(); + expect(harness.updateChannel).not.toHaveBeenCalled(); + expect(harness.store.lookup(sessionKey)).toMatchObject({ + sessionId: "session-recreated", + channelId: originalBinding.channelId, + externalRef: originalBinding.externalRef, + }); + } finally { + harness.service.cleanup(); + vi.useRealTimers(); + } + }); + it("debounces sessions.changed bursts per bound session", async () => { vi.useFakeTimers(); const gateway = createGatewayEventsHarness(); diff --git a/extensions/clickclack/src/discussions/service-open.ts b/extensions/clickclack/src/discussions/service-open.ts index 3ba16c60ef00..d98ca111de19 100644 --- a/extensions/clickclack/src/discussions/service-open.ts +++ b/extensions/clickclack/src/discussions/service-open.ts @@ -8,6 +8,7 @@ import { } from "../http-client.js"; import type { CoreConfig, ResolvedClickClackAccount } from "../types.js"; import { + clearPendingDiscussionOpen, clearDiscussionBindingGeneration, listPendingDiscussionOpens, recordPendingDiscussionOpen, @@ -173,7 +174,7 @@ export async function openClickClackDiscussionBinding( ): Promise { const { account, runtime, sessionKey, store } = params; const entry = runtime.agent.session.getSessionEntry({ sessionKey, readConsistency: "latest" }); - if (!entry) { + if (!entry || entry.archivedAt !== undefined) { return undefined; } if (!entry.sessionId?.trim()) { @@ -202,7 +203,6 @@ export async function openClickClackDiscussionBinding( unresolved && (unresolved.accountId !== account.accountId || unresolved.credentialFingerprint !== credentialFingerprint || - unresolved.sessionId !== entry.sessionId || unresolved.serverBaseUrl !== serverBaseUrl || unresolved.workspaceId !== workspace.id) ) { @@ -227,7 +227,6 @@ export async function openClickClackDiscussionBinding( config.session?.mainKey, label, ); - const archived = entry.archivedAt !== undefined; return await params.withChannelMutationLock(async () => { if (!store.hasCapacity(sessionKey)) { throw new Error("ClickClack discussion binding capacity is exhausted"); @@ -238,13 +237,14 @@ export async function openClickClackDiscussionBinding( const bindingGeneration = reserveDiscussionBindingGeneration({ runtime, sessionKey, + accountId: account.accountId, + credentialFingerprint, destinationIdentity, createGeneration: params.bindingGenerationFactory, }); const externalRef = discussionExternalRef( params.installationId, sessionKey, - entry.sessionId, destinationIdentity, bindingGeneration, ); @@ -304,7 +304,7 @@ export async function openClickClackDiscussionBinding( serverBaseUrl, channelId: adopted.id, }); - resolved = await client.updateChannel(adopted.id, { ...managedFields, archived }); + resolved = await client.updateChannel(adopted.id, managedFields); } else { resolved = await client.createChannel(workspace.id, { ...managedFields, kind: "public" }); markClickClackDiscussionChannelIdentityRevoked({ @@ -339,7 +339,7 @@ export async function openClickClackDiscussionBinding( serverBaseUrl, channelId: recovered.id, }); - resolved = await client.updateChannel(recovered.id, { ...managedFields, archived }); + resolved = await client.updateChannel(recovered.id, managedFields); break; } if (definitiveNoCreate) { @@ -374,99 +374,126 @@ export async function openClickClackDiscussionBinding( displayTitle, }); if (adopted) { - assertChannelPatch(resolved, { ...managedFields, archived }); + assertChannelPatch(resolved, managedFields); } } catch (error) { - try { - const updated = await client.updateChannel(resolved.id, { archived: true }); - assertChannelPatch(updated, { archived: true }); - clearDiscussionBindingGeneration({ - runtime, - sessionKey, - expectedGeneration: bindingGeneration, - }); - } catch (archiveError) { - params.warn( - `failed to archive incompatible discussion channel ${resolved.id}: ${String(archiveError)}`, - ); - } + clearPendingDiscussionOpen({ + runtime, + sessionKey, + expectedGeneration: bindingGeneration, + }); + params.warn(`incompatible discussion channel remains quarantined: ${resolved.id}`); throw error; } if (!resolved.route_id) { + clearPendingDiscussionOpen({ + runtime, + sessionKey, + expectedGeneration: bindingGeneration, + }); + params.warn(`route-less discussion channel remains quarantined: ${resolved.id}`); + throw new Error("ClickClack discussion channel is missing its route id"); + } + const channel = resolved; + const currentEntry = runtime.agent.session.getSessionEntry({ + sessionKey, + readConsistency: "latest", + }); + if (!currentEntry?.sessionId || currentEntry.archivedAt !== undefined) { + clearPendingDiscussionOpen({ + runtime, + sessionKey, + expectedGeneration: bindingGeneration, + }); + params.warn(`unattached discussion channel remains quarantined: ${channel.id}`); + throw new Error("OpenClaw session became inactive while opening its ClickClack discussion"); + } + const currentLabel = resolveDiscussionLabel(currentEntry, sessionKey, agentId); + const currentDisplayTitle = + currentLabel === fallback ? "" : truncateDiscussionDisplayTitle(currentLabel); + const currentSection = currentEntry.category?.trim() || account.discussions.section; + const currentExternalUrl = + controlSessionUrl( + account.discussions.controlUrlBase, + sessionKey, + account.agentId ?? "main", + config.session?.mainKey, + currentLabel, + ) ?? ""; + let currentChannel = channel; + if ( + currentEntry.sessionId !== entry.sessionId || + currentLabel !== label || + currentDisplayTitle !== displayTitle || + currentSection !== section || + currentExternalUrl !== (externalUrl ?? "") + ) { try { - const updated = await client.updateChannel(resolved.id, { archived: true }); - assertChannelPatch(updated, { archived: true }); - clearDiscussionBindingGeneration({ + for (let attempt = 0; attempt < CHANNEL_NAME_MUTATION_ATTEMPTS; attempt += 1) { + const latestManagedFields = { + ...managedFields, + name: await resolveAvailableChannelName({ + client, + workspaceId: workspace.id, + label: currentLabel, + sessionKey, + agentId, + ownChannelId: channel.id, + }), + external_url: currentExternalUrl, + sidebar_section: currentSection, + display_title: currentDisplayTitle, + }; + try { + currentChannel = await client.updateChannel(channel.id, latestManagedFields); + assertChannelPatch(currentChannel, latestManagedFields); + break; + } catch (error) { + if ( + !isClickClackChannelNameConflict(error) || + attempt === CHANNEL_NAME_MUTATION_ATTEMPTS - 1 + ) { + throw error; + } + } + } + } catch (error) { + clearPendingDiscussionOpen({ runtime, sessionKey, expectedGeneration: bindingGeneration, }); - } catch (archiveError) { - params.warn( - `failed to archive route-less discussion channel ${resolved.id}: ${String(archiveError)}`, - ); + params.warn(`unattached discussion channel remains quarantined: ${channel.id}`); + throw error; } - throw new Error("ClickClack discussion channel is missing its route id"); - } - let channel = resolved; - if (!adopted && archived) { - channel = await client.updateChannel(resolved.id, { archived: true }); - assertChannelPatch(channel, { archived: true }); } const nextBinding: ClickClackDiscussionBinding = { accountId: account.accountId, agentId, - sessionId: entry.sessionId, + sessionId: currentEntry.sessionId, serverBaseUrl, credentialFingerprint, externalRef, - externalUrl: externalUrl ?? "", + externalUrl: currentExternalUrl, workspaceRef: account.discussions.workspace, workspaceId: workspace.id, channelId: channel.id, channelRouteId: channel.route_id, workspaceRouteId: workspace.route_id, - section, - archived, - label, - displayTitle: "display_title" in channel ? channel.display_title : undefined, + section: currentSection, + archived: false, + label: currentLabel, + displayTitle: "display_title" in currentChannel ? currentChannel.display_title : undefined, }; - const currentEntry = runtime.agent.session.getSessionEntry({ - sessionKey, - readConsistency: "latest", - }); - if (!currentEntry || currentEntry.sessionId !== entry.sessionId) { - try { - const updated = await client.updateChannel(channel.id, { archived: true }); - assertChannelPatch(updated, { archived: true }); - clearDiscussionBindingGeneration({ - runtime, - sessionKey, - expectedGeneration: bindingGeneration, - }); - } catch (archiveError) { - params.warn( - `failed to archive superseded discussion channel ${channel.id}: ${String(archiveError)}`, - ); - } - throw new Error("OpenClaw session changed while opening its ClickClack discussion"); - } try { store.set(sessionKey, nextBinding); } catch (error) { - try { - const updated = await client.updateChannel(channel.id, { archived: true }); - assertChannelPatch(updated, { archived: true }); - clearDiscussionBindingGeneration({ - runtime, - sessionKey, - expectedGeneration: bindingGeneration, - }); - } catch (archiveError) { - params.warn( - `failed to archive unbound discussion channel ${channel.id}: ${String(archiveError)}`, - ); - } + clearPendingDiscussionOpen({ + runtime, + sessionKey, + expectedGeneration: bindingGeneration, + }); + params.warn(`unbound discussion channel remains quarantined: ${channel.id}`); throw error; } params.finalizePendingBinding(sessionKey, nextBinding); diff --git a/extensions/clickclack/src/discussions/service-test-support.ts b/extensions/clickclack/src/discussions/service-test-support.ts index c07a86474ae4..46383f734920 100644 --- a/extensions/clickclack/src/discussions/service-test-support.ts +++ b/extensions/clickclack/src/discussions/service-test-support.ts @@ -192,11 +192,10 @@ export function createHarness( }; } -export function testExternalRef(sessionKey: string, sessionId = "session-id"): string { +export function testExternalRef(sessionKey: string): string { return discussionExternalRef( TEST_INSTALLATION_ID, sessionKey, - sessionId, TEST_DESTINATION_IDENTITY, TEST_BINDING_GENERATION, ); diff --git a/extensions/clickclack/src/discussions/service.test.ts b/extensions/clickclack/src/discussions/service.test.ts index f45089297eeb..feb5c85c375f 100644 --- a/extensions/clickclack/src/discussions/service.test.ts +++ b/extensions/clickclack/src/discussions/service.test.ts @@ -341,7 +341,7 @@ describe("ClickClack discussion service", () => { expect(harness.createChannel).not.toHaveBeenCalled(); }); - it("maps archive, label, category, restore, and deletion state to channel patches", async () => { + it("keeps the room durable across session archive, restore, and deletion", async () => { const harness = createHarness({ label: "Original", category: "Projects" }); const sessionKey = "agent:main:work"; await harness.service.open(sessionKey); @@ -352,26 +352,22 @@ describe("ClickClack discussion service", () => { archivedAt: 123, }); await harness.service.reconcile(sessionKey); - expect(harness.updateChannel).toHaveBeenLastCalledWith("chn_discussion", { - archived: true, - display_title: "Renamed Session", - name: "renamed-session", - sidebar_section: "Incidents", - }); + expect(harness.updateChannel).not.toHaveBeenCalled(); + expect(harness.store.lookup(sessionKey)).toMatchObject({ channelId: "chn_discussion" }); harness.setSessionEntry({ label: "Renamed Session" }); await harness.service.reconcile(sessionKey); expect(harness.updateChannel).toHaveBeenLastCalledWith("chn_discussion", { - archived: false, + display_title: "Renamed Session", + name: "renamed-session", sidebar_section: "Sessions", }); + harness.updateChannel.mockClear(); harness.setSessionEntry(undefined); await harness.service.reconcile(sessionKey); - expect(harness.updateChannel).toHaveBeenLastCalledWith("chn_discussion", { - archived: true, - }); - expect(await harness.service.info(sessionKey)).toEqual({ state: "available" }); + expect(harness.updateChannel).not.toHaveBeenCalled(); + expect(await harness.service.info(sessionKey)).toMatchObject({ state: "open" }); }); it("renames a fallback discussion when displayName arrives", async () => { @@ -389,58 +385,68 @@ describe("ClickClack discussion service", () => { expect(harness.store.lookup(sessionKey)).toMatchObject({ label: "Generated Session Title" }); }); - it("does not return a binding removed while info or open reconciles a deleted session", async () => { + it("keeps a durable room visible after its attached session is deleted", async () => { const infoHarness = createHarness({ label: "Info deletion" }); const infoKey = "agent:main:deleted-info"; await infoHarness.service.open(infoKey); infoHarness.setSessionEntry(undefined); - expect(await infoHarness.service.info(infoKey)).toEqual({ state: "available" }); + expect(await infoHarness.service.info(infoKey)).toMatchObject({ state: "open" }); const openHarness = createHarness({ label: "Open deletion" }); const openKey = "agent:main:deleted-open"; await openHarness.service.open(openKey); openHarness.setSessionEntry(undefined); - expect(await openHarness.service.open(openKey)).toEqual({ state: "available" }); + expect(await openHarness.service.open(openKey)).toMatchObject({ state: "open" }); }); - it("archives and replaces a binding when the session key gets a new incarnation", async () => { + it("rotates the session attachment without replacing the durable room", async () => { const harness = createHarness({ sessionId: "session-old", label: "Resettable" }); const sessionKey = "agent:main:resettable"; await harness.service.open(sessionKey); - const oldRef = testExternalRef(sessionKey, "session-old"); + const externalRef = testExternalRef(sessionKey); harness.setSessionEntry({ sessionId: "session-new", label: "Resettable" }); expect((await harness.service.readLatestMessages(sessionKey, 30)).text).toBe( - "No discussion is bound to this session.", + "The bound discussion has no messages yet.", ); expect(await harness.service.open(sessionKey)).toMatchObject({ state: "open" }); - const newRef = testExternalRef(sessionKey, "session-new"); - expect(newRef).not.toBe(oldRef); - expect(harness.updateChannel).toHaveBeenCalledWith("chn_discussion", { archived: true }); - expect(harness.createChannel).toHaveBeenCalledTimes(2); - expect(harness.createChannel).toHaveBeenLastCalledWith( - "wsp_team", - expect.objectContaining({ external_ref: newRef }), - ); + expect(harness.updateChannel).not.toHaveBeenCalled(); + expect(harness.createChannel).toHaveBeenCalledTimes(1); expect(harness.store.lookup(sessionKey)).toMatchObject({ sessionId: "session-new", - externalRef: newRef, + externalRef, + channelId: "chn_discussion", }); }); - it("archives an unbound channel when the session resets during open", async () => { + it("attaches a channel to the current incarnation when the session resets during open", async () => { const harness = createHarness({ sessionId: "session-old", label: "Reset race" }); const sessionKey = "agent:main:reset-race"; vi.mocked(harness.runtime.agent.session.getSessionEntry) .mockReturnValueOnce({ sessionId: "session-old", label: "Reset race", updatedAt: 1 }) - .mockReturnValue({ sessionId: "session-new", label: "Reset race", updatedAt: 2 }); + .mockReturnValue({ + sessionId: "session-new", + label: "Current reset race", + category: "Current sessions", + updatedAt: 2, + }); - await expect(harness.service.open(sessionKey)).rejects.toThrow( - "OpenClaw session changed while opening", + await expect(harness.service.open(sessionKey)).resolves.toMatchObject({ state: "open" }); + expect(harness.updateChannel).toHaveBeenCalledWith( + "chn_discussion", + expect.objectContaining({ + name: "current-reset-race", + display_title: "Current reset race", + sidebar_section: "Current sessions", + }), ); - expect(harness.updateChannel).toHaveBeenCalledWith("chn_discussion", { archived: true }); - expect(harness.store.lookup(sessionKey)).toBeUndefined(); + expect(harness.createChannel).toHaveBeenCalledTimes(1); + expect(harness.store.lookup(sessionKey)).toMatchObject({ + sessionId: "session-new", + label: "Current reset race", + section: "Current sessions", + }); }); it("suffixes a desired name when its slug already exists", async () => { @@ -456,7 +462,6 @@ describe("ClickClack discussion service", () => { created_at: "2026-07-19T00:00:00.000Z", }, ]); - await harness.service.open("agent:main:duplicate-label"); expect(harness.createChannel).toHaveBeenCalledWith( @@ -843,7 +848,7 @@ describe("ClickClack discussion service", () => { expect(harness.revokedStore.entries()).toHaveLength(1); }); - it("archives an ambiguous create after the session incarnation changes", async () => { + it("adopts an ambiguous create after the session incarnation changes", async () => { const harness = createHarness({ sessionId: "old-session", label: "Ambiguous reset" }); const sessionKey = "agent:main:ambiguous-reset"; const general = await harness.channels().then((channels) => channels[0]!); @@ -869,19 +874,117 @@ describe("ClickClack discussion service", () => { created_at: "2026-07-19T00:00:00.000Z", }, ]); + vi.mocked(harness.updateChannel).mockImplementationOnce(async (channelId, patch) => ({ + id: channelId, + route_id: "ambiguous-old-route", + workspace_id: "wsp_team", + name: patch.name ?? "ambiguous-reset", + kind: "public", + external_managed: patch.external_managed ?? true, + external_ref: patch.external_ref ?? externalRef, + external_url: patch.external_url, + sidebar_section: patch.sidebar_section, + archived: false, + created_at: "2026-07-19T00:00:00.000Z", + })); await harness.service.open(sessionKey); - expect(harness.updateChannel).toHaveBeenCalledWith("chn_ambiguous_old", { archived: true }); - expect(harness.createChannel).toHaveBeenCalledTimes(2); - expect(harness.createChannel.mock.calls[1]?.[1].external_ref).not.toBe(externalRef); - expect(harness.store.lookup(sessionKey)).toMatchObject({ sessionId: "new-session" }); + expect(harness.updateChannel).toHaveBeenCalledWith( + "chn_ambiguous_old", + expect.objectContaining({ external_ref: externalRef }), + ); + expect(harness.createChannel).toHaveBeenCalledTimes(1); + expect(harness.store.lookup(sessionKey)).toMatchObject({ + sessionId: "new-session", + externalRef, + channelId: "chn_ambiguous_old", + }); expect(harness.generationStore.lookup(sessionKey)).toBeUndefined(); + expect(harness.revokedStore.entries()).toHaveLength(0); + }); + + it("reattaches an ambiguous room after the session is archived and restored", async () => { + const harness = createHarness({ sessionId: "old-session", label: "Archived during create" }); + const sessionKey = "agent:main:archived-ambiguous-open"; + const general = await harness.channels().then((channels) => channels[0]!); + vi.mocked(harness.channels) + .mockResolvedValueOnce([general]) + .mockRejectedValueOnce(new Error("relist unavailable")); + vi.mocked(harness.createChannel).mockRejectedValueOnce(new Error("connection lost")); + await expect(harness.service.open(sessionKey)).rejects.toThrow("connection lost"); + const externalRef = harness.createChannel.mock.calls[0]?.[1].external_ref; + harness.setSessionEntry({ + sessionId: "new-session", + label: "Archived during create", + archivedAt: 1, + }); + vi.mocked(harness.channels).mockResolvedValue([ + { + id: "chn_archived_ambiguous", + route_id: "archived-ambiguous-route", + workspace_id: "wsp_team", + name: "archived-during-create", + kind: "public", + external_managed: true, + external_ref: externalRef, + external_url: "", + sidebar_section: "Sessions", + archived: false, + created_at: "2026-07-19T00:00:00.000Z", + }, + ]); + + await harness.service.reconcileAll(); + + expect(harness.createChannel).toHaveBeenCalledTimes(1); + expect(harness.updateChannel).not.toHaveBeenCalled(); + expect(harness.generationStore.lookup(sessionKey)).toMatchObject({ + generation: expect.any(String), + }); + expect(harness.generationStore.lookup(sessionKey)).not.toHaveProperty("pending"); expect(harness.revokedStore.entries()).toHaveLength(1); + + harness.setSessionEntry({ + sessionId: "new-session", + label: "Archived during create", + }); + vi.mocked(harness.updateChannel).mockImplementationOnce(async (channelId, patch) => ({ + id: channelId, + route_id: "archived-ambiguous-route", + workspace_id: "wsp_team", + name: patch.name ?? "archived-during-create", + kind: "public", + external_managed: patch.external_managed ?? true, + external_ref: patch.external_ref ?? externalRef, + external_url: patch.external_url, + sidebar_section: patch.sidebar_section, + archived: false, + created_at: "2026-07-19T00:00:00.000Z", + })); + + await expect(harness.service.open(sessionKey)).resolves.toMatchObject({ state: "open" }); + + expect(harness.createChannel).toHaveBeenCalledTimes(1); + expect(harness.updateChannel).toHaveBeenCalledWith( + "chn_archived_ambiguous", + expect.objectContaining({ external_ref: externalRef }), + ); + expect(harness.store.lookup(sessionKey)).toMatchObject({ + sessionId: "new-session", + externalRef, + channelId: "chn_archived_ambiguous", + }); + expect(harness.generationStore.lookup(sessionKey)).toBeUndefined(); + expect(harness.revokedStore.entries()).toHaveLength(0); }); it("reconciles an ambiguous create after discussions are disabled", async () => { - const harness = createHarness({ label: "Disable during create" }); + const generations = ["disabled-generation", "replacement-generation"]; + const harness = createHarness( + { label: "Disable during create" }, + { bindingGenerationFactory: () => generations.shift() ?? "unexpected-generation" }, + ); const sessionKey = "agent:main:disable-during-create"; const general = await harness.channels().then((channels) => channels[0]!); vi.mocked(harness.channels) @@ -909,9 +1012,21 @@ describe("ClickClack discussion service", () => { await harness.service.reconcileAll(); - expect(harness.updateChannel).toHaveBeenCalledWith("chn_disabled_pending", { archived: true }); - expect(harness.generationStore.lookup(sessionKey)).toBeUndefined(); + expect(harness.updateChannel).not.toHaveBeenCalled(); + expect(harness.generationStore.lookup(sessionKey)).toMatchObject({ + generation: expect.any(String), + }); + expect(harness.generationStore.lookup(sessionKey)).not.toHaveProperty("pending"); expect(harness.revokedStore.entries()).toHaveLength(1); + + harness.config.channels!.clickclack!.token = "replacement-token"; + harness.config.channels!.clickclack!.discussions!.enabled = true; + + await expect(harness.service.open(sessionKey)).resolves.toMatchObject({ state: "open" }); + + expect(harness.updateChannel).not.toHaveBeenCalled(); + expect(harness.createChannel).toHaveBeenCalledTimes(2); + expect(harness.createChannel.mock.calls[1]?.[1].external_ref).not.toBe(externalRef); }); it("does not recurse while replacing the account for an ambiguous open", async () => { @@ -951,7 +1066,7 @@ describe("ClickClack discussion service", () => { await expect(harness.service.open(sessionKey)).resolves.toMatchObject({ state: "open" }); - expect(harness.updateChannel).toHaveBeenCalledWith("chn_old_account", { archived: true }); + expect(harness.updateChannel).not.toHaveBeenCalled(); expect(harness.createChannel).toHaveBeenCalledTimes(2); expect(harness.store.lookup(sessionKey)).toMatchObject({ accountId: "replacement", @@ -959,7 +1074,7 @@ describe("ClickClack discussion service", () => { }); }); - it("rejects adoption when the server ignores the requested lifecycle state", async () => { + it("does not create or adopt a room for an archived main session", async () => { const harness = createHarness({ label: "Recovered Name", archivedAt: 123 }); const sessionKey = "agent:main:recover-stale"; const externalRef = testExternalRef(sessionKey); @@ -992,9 +1107,8 @@ describe("ClickClack discussion service", () => { created_at: "2026-07-19T00:00:00.000Z", })); - await expect(harness.service.open(sessionKey)).rejects.toThrow( - "ClickClack channel update did not apply archived", - ); - expect(harness.generationStore.lookup(sessionKey)).toBeUndefined(); + await expect(harness.service.open(sessionKey)).resolves.toEqual({ state: "available" }); + expect(harness.channels).not.toHaveBeenCalled(); + expect(harness.updateChannel).not.toHaveBeenCalled(); }); }); diff --git a/extensions/clickclack/src/discussions/service.ts b/extensions/clickclack/src/discussions/service.ts index 105d8373943a..a755c7c480dc 100644 --- a/extensions/clickclack/src/discussions/service.ts +++ b/extensions/clickclack/src/discussions/service.ts @@ -12,13 +12,14 @@ import { } from "../http-client.js"; import type { CoreConfig, ResolvedClickClackAccount } from "../types.js"; import { + clearPendingDiscussionOpen, clearDiscussionBindingGeneration, listPendingDiscussionOpens, type PendingDiscussionOpen, } from "./binding-generation.js"; import { + attachBindingToCurrentActiveSession, getClickClackDiscussionBindingStore, - bindingMatchesSessionIncarnation, type ClickClackDiscussionBinding, type ClickClackDiscussionBindingStore, } from "./binding-store.js"; @@ -54,7 +55,6 @@ import { const RECONCILE_INTERVAL_MS = 60_000; const CHANNEL_NAME_MUTATION_ATTEMPTS = 4; - type DiscussionServiceOptions = { clientFactory?: (account: ResolvedClickClackAccount) => ClickClackClient; installationId?: string; @@ -280,7 +280,8 @@ export class ClickClackDiscussionService { if (resolved.state !== "active") { return { text: "No discussion is bound to this session." }; } - if (!bindingMatchesSessionIncarnation(this.#runtime, sessionKey, binding)) { + const attached = this.#refreshSessionAttachment(sessionKey, binding); + if (!attached) { return { text: "No discussion is bound to this session." }; } if ( @@ -293,12 +294,12 @@ export class ClickClackDiscussionService { return { text: "No discussion is bound to this session." }; } const history = await this.#clientFactory(resolved.account).latestChannelMessages( - binding.channelId, + attached.channelId, limit, ); const text = formatDiscussionHistory(history); return { - binding, + binding: attached, text: text || "The bound discussion has no messages yet.", }; } @@ -351,23 +352,21 @@ export class ClickClackDiscussionService { `ClickClack discussion account is no longer configured: ${binding.accountId}`, ); } + const attached = this.#refreshSessionAttachment(sessionKey, binding); + if (!attached) { + return; + } + const currentBinding = attached; const entry = this.#runtime.agent.session.getSessionEntry({ sessionKey, readConsistency: "latest", }); - if (entry && (!binding.sessionId || entry.sessionId !== binding.sessionId)) { - await this.#archiveAndDeleteBinding(sessionKey, binding, account); + if (!entry || entry.archivedAt !== undefined) { return; } - const archived = entry ? entry.archivedAt !== undefined : true; - const deleted = entry === undefined; - const fallback = fallbackDiscussionLabel(sessionKey, binding.agentId); - const label = entry - ? resolveDiscussionLabel(entry, sessionKey, binding.agentId) - : binding.label; - const section = entry?.category?.trim() || account.discussions.section; - // Binding ownership follows global session routing; unscoped link decoration - // follows the ClickClack account agent so reconciliation keeps the URL stable. + const fallback = fallbackDiscussionLabel(sessionKey, currentBinding.agentId); + const label = resolveDiscussionLabel(entry, sessionKey, currentBinding.agentId); + const section = entry.category?.trim() || account.discussions.section; const externalUrl = controlSessionUrl( account.discussions.controlUrlBase, @@ -377,46 +376,35 @@ export class ClickClackDiscussionService { label, ) ?? ""; const patch: { - archived?: boolean; display_title?: string; external_url?: string; name?: string; sidebar_section?: string; } = {}; - if (archived !== binding.archived) { - patch.archived = archived; - } - const labelChanged = label !== binding.label; + const labelChanged = label !== currentBinding.label; const desiredDisplayTitle = label === fallback ? "" : truncateDiscussionDisplayTitle(label); - // Confirmation piggybacks on normal opens/renames, so a workspace backfills after any channel - // round-trips a title. Legacy servers never confirm, avoiding retry spam; a fully dormant - // workspace may wait until its next titled open. Scoped to this binding's server+account so a - // confirmation from a previous deployment cannot arm backfill against a legacy server. const serverSupportsDisplayTitle = this.#store .entries() .some( ({ binding: candidate }) => candidate.displayTitle !== undefined && - candidate.serverBaseUrl === binding.serverBaseUrl && - candidate.accountId === binding.accountId, + candidate.serverBaseUrl === currentBinding.serverBaseUrl && + candidate.accountId === currentBinding.accountId, ); const shouldBackfillDisplayTitle = desiredDisplayTitle !== "" && - binding.displayTitle !== desiredDisplayTitle && + currentBinding.displayTitle !== desiredDisplayTitle && serverSupportsDisplayTitle; if (labelChanged || shouldBackfillDisplayTitle) { patch.display_title = desiredDisplayTitle; } - if (section !== binding.section) { + if (section !== currentBinding.section) { patch.sidebar_section = section; } - if (externalUrl !== binding.externalUrl) { + if (externalUrl !== currentBinding.externalUrl) { patch.external_url = externalUrl; } if (Object.keys(patch).length === 0 && !labelChanged) { - if (deleted) { - this.#revokeAndDeleteBinding(sessionKey, binding); - } return; } const client = this.#clientFactory(account); @@ -426,14 +414,14 @@ export class ClickClackDiscussionService { for (let attempt = 0; attempt < CHANNEL_NAME_MUTATION_ATTEMPTS; attempt += 1) { patch.name = await resolveAvailableChannelName({ client, - workspaceId: binding.workspaceId, + workspaceId: currentBinding.workspaceId, label, sessionKey, - agentId: binding.agentId, - ownChannelId: binding.channelId, + agentId: currentBinding.agentId, + ownChannelId: currentBinding.channelId, }); try { - const renamed = await client.updateChannel(binding.channelId, patch); + const renamed = await client.updateChannel(currentBinding.channelId, patch); assertChannelPatch(renamed, patch); return renamed; } catch (error) { @@ -448,16 +436,11 @@ export class ClickClackDiscussionService { throw new Error("ClickClack discussion channel name retries were exhausted"); }); } else { - updated = await client.updateChannel(binding.channelId, patch); + updated = await client.updateChannel(currentBinding.channelId, patch); assertChannelPatch(updated, patch); } - if (deleted) { - this.#revokeAndDeleteBinding(sessionKey, binding); - return; - } this.#store.set(sessionKey, { - ...binding, - archived, + ...currentBinding, externalUrl, label, section, @@ -465,6 +448,25 @@ export class ClickClackDiscussionService { }); } + #refreshSessionAttachment( + sessionKey: string, + binding: ClickClackDiscussionBinding, + ): ClickClackDiscussionBinding | undefined { + try { + return attachBindingToCurrentActiveSession({ + runtime: this.#runtime, + store: this.#store, + sessionKey, + binding, + }); + } catch (error) { + this.#logger().warn( + `discussion attachment refresh failed for ${sessionKey}: ${String(error)}`, + ); + return undefined; + } + } + async #reconcilePendingOpen( pending: PendingDiscussionOpen, options: { allowRetry?: boolean } = {}, @@ -497,7 +499,8 @@ export class ClickClackDiscussionService { const retryAccount = activeAccounts.length === 1 ? activeAccounts[0] : undefined; if ( options.allowRetry !== false && - entry?.sessionId === pending.sessionId && + entry?.sessionId && + entry.archivedAt === undefined && retryAccount && normalizedServerBaseUrl(retryAccount) === pending.serverBaseUrl && discussionCredentialFingerprint(retryAccount.token) === pending.credentialFingerprint @@ -528,8 +531,12 @@ export class ClickClackDiscussionService { serverBaseUrl: pending.serverBaseUrl, channelId: channel.id, }); - const updated = await client.updateChannel(channel.id, { archived: true }); - assertChannelPatch(updated, { archived: true }); + clearPendingDiscussionOpen({ + runtime: this.#runtime, + sessionKey: pending.sessionKey, + expectedGeneration: pending.generation, + }); + return; } clearDiscussionBindingGeneration({ runtime: this.#runtime, @@ -542,41 +549,9 @@ export class ClickClackDiscussionService { sessionKey: string, binding: ClickClackDiscussionBinding, ): Promise { - // Clear the durable interrupted-open reservation before releasing ownership. - // A crash after this point can retry archival, but can never re-adopt the old channel. + // Release local routing authority only. The ClickClack room remains durable, + // and its lifecycle remains owned by ClickClack. clearDiscussionBindingGeneration({ runtime: this.#runtime, sessionKey }); - const boundAccount = resolveClickClackAccount({ - cfg: this.#currentConfig(), - accountId: binding.accountId, - }); - if ( - !boundAccount.configured || - binding.serverBaseUrl !== normalizedServerBaseUrl(boundAccount) || - !binding.credentialFingerprint || - binding.credentialFingerprint !== discussionCredentialFingerprint(boundAccount.token) - ) { - this.#revokeAndDeleteBinding(sessionKey, binding); - return; - } - // Eligibility checks revoke routing/tool authority immediately, while the - // durable binding remains as the retry record until archival is verified. - const updated = await this.#clientFactory(boundAccount).updateChannel(binding.channelId, { - archived: true, - }); - assertChannelPatch(updated, { archived: true }); - this.#revokeAndDeleteBinding(sessionKey, binding); - } - - async #archiveAndDeleteBinding( - sessionKey: string, - binding: ClickClackDiscussionBinding, - account: ResolvedClickClackAccount, - ): Promise { - clearDiscussionBindingGeneration({ runtime: this.#runtime, sessionKey }); - const updated = await this.#clientFactory(account).updateChannel(binding.channelId, { - archived: true, - }); - assertChannelPatch(updated, { archived: true }); this.#revokeAndDeleteBinding(sessionKey, binding); } diff --git a/extensions/clickclack/src/discussions/tool-policy.test.ts b/extensions/clickclack/src/discussions/tool-policy.test.ts index 811b224eb70a..37f25bcef53d 100644 --- a/extensions/clickclack/src/discussions/tool-policy.test.ts +++ b/extensions/clickclack/src/discussions/tool-policy.test.ts @@ -3,7 +3,10 @@ import type { PluginRuntime } from "openclaw/plugin-sdk/core"; import type { PluginStateSyncKeyedStore } from "openclaw/plugin-sdk/plugin-state-runtime"; import { describe, expect, it, vi } from "vitest"; import type { CoreConfig } from "../types.js"; -import { getClickClackDiscussionBindingStore } from "./binding-store.js"; +import { + attachBindingToCurrentActiveSession, + getClickClackDiscussionBindingStore, +} from "./binding-store.js"; import { discussionSessionKey } from "./naming.js"; import { markClickClackDiscussionChannelRevoked } from "./revoked-channel-store.js"; import { enforceClickClackDiscussionToolTarget } from "./tool-policy.js"; @@ -169,7 +172,7 @@ describe("ClickClack discussion session tool policy", () => { expect(run("sessions_send", { sessionKey: mainSessionKey, message: "x" })?.block).toBe(true); }); - it("revokes the target capability for a synchronized archived binding", () => { + it("ignores legacy session-derived archive metadata on a durable room", () => { const { bindingStore, mainSessionKey, run } = setup(); const binding = bindingStore.get(mainSessionKey); if (!binding) { @@ -177,8 +180,8 @@ describe("ClickClack discussion session tool policy", () => { } bindingStore.set(mainSessionKey, { ...binding, archived: true }); - expect(run("sessions_history", { sessionKey: mainSessionKey })?.block).toBe(true); - expect(run("sessions_send", { sessionKey: mainSessionKey, message: "x" })?.block).toBe(true); + expect(run("sessions_history", { sessionKey: mainSessionKey })).toBeUndefined(); + expect(run("sessions_send", { sessionKey: mainSessionKey, message: "x" })).toBeUndefined(); }); it("lets a durable channel tombstone override a surviving binding", () => { @@ -253,6 +256,32 @@ describe("ClickClack discussion session tool policy", () => { ).toBe(mainSessionKey); }); + it("keeps the old attachment authoritative when reset persistence fails", () => { + const { bindingStore, mainSessionKey, run, runtime, store } = setup(); + const previous = bindingStore.get(mainSessionKey); + if (!previous) { + throw new Error("expected binding"); + } + vi.mocked(runtime.agent.session.getSessionEntry).mockReturnValue({ + sessionId: "replacement-session-id", + updatedAt: 2, + }); + store.register = vi.fn(() => { + throw new Error("SQLITE_FULL"); + }); + + expect(() => + attachBindingToCurrentActiveSession({ + runtime, + store: bindingStore, + sessionKey: mainSessionKey, + binding: previous, + }), + ).toThrow("SQLITE_FULL"); + expect(bindingStore.get(mainSessionKey)).toEqual(previous); + expect(run("sessions_history", { sessionKey: mainSessionKey })?.block).toBe(true); + }); + it("uses a different side-session identity after a server retarget", () => { const { mainSessionKey, runtime, sideSessionKey } = setup(); const retargeted = discussionSessionKey({ diff --git a/extensions/clickclack/src/discussions/tool-policy.ts b/extensions/clickclack/src/discussions/tool-policy.ts index 3cec8291fa6b..fa8bc56fcd4a 100644 --- a/extensions/clickclack/src/discussions/tool-policy.ts +++ b/extensions/clickclack/src/discussions/tool-policy.ts @@ -38,7 +38,6 @@ export function isClickClackDiscussionSessionTarget(params: { serverBaseUrl: matched.binding.serverBaseUrl, channelId: matched.binding.channelId, }) && - !matched.binding.archived && bindingMatchesActiveSessionIncarnation(params.runtime, matched.sessionKey, matched.binding) && resolveDiscussionBindingAccount(params.runtime.config.current() as CoreConfig, matched.binding) .state === "active" diff --git a/extensions/clickclack/src/inbound.mention-gating.test.ts b/extensions/clickclack/src/inbound.mention-gating.test.ts index 06ff4d302e33..88f83755fa11 100644 --- a/extensions/clickclack/src/inbound.mention-gating.test.ts +++ b/extensions/clickclack/src/inbound.mention-gating.test.ts @@ -3,7 +3,10 @@ import type { PluginRuntime } from "openclaw/plugin-sdk/core"; import type { PluginStateSyncKeyedStore } from "openclaw/plugin-sdk/plugin-state-runtime"; import { buildAgentSessionKey, resolveAgentRoute } from "openclaw/plugin-sdk/routing"; import { describe, expect, it, vi } from "vitest"; -import { getClickClackDiscussionBindingStore } from "./discussions/binding-store.js"; +import { + getClickClackDiscussionBindingStore, + type ClickClackDiscussionBinding, +} from "./discussions/binding-store.js"; import { handleClickClackInbound } from "./inbound.js"; import { setClickClackRuntime } from "./runtime.js"; import type { ClickClackMessage, CoreConfig, ResolvedClickClackAccount } from "./types.js"; @@ -150,6 +153,72 @@ function createMessage(overrides: Partial = {}): ClickClackMe } describe("ClickClack inbound mention gating", () => { + it("records attachment persistence failures before dropping inbound delivery", async () => { + const runtime = createRuntime(); + setClickClackRuntime(runtime); + const mainSessionKey = "agent:research:main"; + const bindingStore = getClickClackDiscussionBindingStore(runtime); + bindingStore.set(mainSessionKey, { + accountId: "default", + agentId: "research", + sessionId: "old-session-id", + serverBaseUrl: "http://127.0.0.1:8080", + externalRef: "openclaw:test:research", + externalUrl: "", + workspaceRef: "wsp_1", + workspaceId: "wsp_1", + channelId: "chn_1", + channelRouteId: "discussion-route", + workspaceRouteId: "workspace-route", + section: "Sessions", + archived: false, + label: "Research", + }); + const persisted = runtime.state.openSyncKeyedStore({ + namespace: "discussion-bindings", + maxEntries: 10_000, + overflowPolicy: "reject-new", + }); + persisted.register = vi.fn(() => { + throw new Error("SQLITE_FULL"); + }); + + await handleClickClackInbound({ + account: createAgentAccount({ + replyMode: "model", + discussions: { enabled: true, workspace: "wsp_1", section: "Sessions" }, + }), + config: { + channels: { + clickclack: { + enabled: true, + baseUrl: "http://127.0.0.1:8080", + token: "test-token-placeholder", + workspace: "wsp_1", + discussions: { enabled: true, workspace: "wsp_1" }, + }, + }, + } satisfies CoreConfig, + message: createMessage({ channel_id: "chn_1", body: "Old discussion" }), + }); + + expect(runtime.channel.inbound.dispatch).not.toHaveBeenCalled(); + expect(bindingStore.get(mainSessionKey)).toMatchObject({ sessionId: "old-session-id" }); + expect(runtime.logging.getChildLogger).toHaveBeenCalledWith({ + plugin: "clickclack", + feature: "discussions", + }); + const loggerCall = vi + .mocked(runtime.logging.getChildLogger) + .mock.calls.findIndex( + ([context]) => context?.plugin === "clickclack" && context.feature === "discussions", + ); + const logger = vi.mocked(runtime.logging.getChildLogger).mock.results[loggerCall]?.value; + expect(logger?.warn).toHaveBeenCalledWith( + "discussion attachment refresh failed for channel chn_1: Error: SQLITE_FULL", + ); + }); + it("rejects an unmentioned group message when mention gating is enabled", async () => { const runtime = createRuntime(); setClickClackRuntime(runtime); diff --git a/extensions/clickclack/src/inbound.test.ts b/extensions/clickclack/src/inbound.test.ts index aa9a96454b73..480fe7439b87 100644 --- a/extensions/clickclack/src/inbound.test.ts +++ b/extensions/clickclack/src/inbound.test.ts @@ -669,7 +669,7 @@ describe("handleClickClackInbound", () => { expect(dispatch.mock.calls[0]?.[0].ctxPayload.GroupSystemPrompt).toContain("sessions_send"); }); - it("drops an old bound channel after the main session is replaced", async () => { + it("rotates an old attachment before dispatch after the main session is replaced", async () => { const runtime = createRuntime(); setClickClackRuntime(runtime); const mainSessionKey = "agent:research:main"; @@ -710,10 +710,15 @@ describe("handleClickClackInbound", () => { }); expect(runtime.llm.complete).not.toHaveBeenCalled(); - expect(runtime.channel.inbound.dispatch).not.toHaveBeenCalled(); + expect(runtime.channel.inbound.dispatch).toHaveBeenCalledTimes(1); + expect(getClickClackDiscussionBindingStore(runtime).get(mainSessionKey)).toMatchObject({ + sessionId: "session-id", + channelId: "chn_1", + externalRef: "openclaw:test:research", + }); }); - it("drops inbound delivery for an archived managed discussion", async () => { + it("ignores legacy session-derived archive metadata on a durable room", async () => { const runtime = createRuntime(); setClickClackRuntime(runtime); getClickClackDiscussionBindingStore(runtime).set("agent:research:main", { @@ -753,7 +758,7 @@ describe("handleClickClackInbound", () => { }); expect(runtime.llm.complete).not.toHaveBeenCalled(); - expect(runtime.channel.inbound.dispatch).not.toHaveBeenCalled(); + expect(runtime.channel.inbound.dispatch).toHaveBeenCalledTimes(1); }); it("drops inbound delivery as soon as the main session is archived", async () => { @@ -906,6 +911,8 @@ describe("handleClickClackInbound", () => { const generation = reserveDiscussionBindingGeneration({ runtime, sessionKey, + accountId: "default", + credentialFingerprint: "test-fingerprint", destinationIdentity: "http://127.0.0.1:8080\0wsp_1", createGeneration: () => "pending-generation", });