From cbcd6a102c929b74640d472c2d5d2edaf0568cc2 Mon Sep 17 00:00:00 2001 From: Marvinthebored Date: Mon, 6 Jul 2026 13:27:27 +0800 Subject: [PATCH] fix(clickclack): reply to a top-level message in-channel, not as a new thread (#100582) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(clickclack): reply to a top-level message in-channel, not as a new thread sendClickClackText routed any replyToId to createThreadReply, and the inbound handler stamps replyToId = on every reply. As a result every reply to a top-level channel message opened its own thread, so the main channel timeline showed nothing and the chat was effectively unusable. Route a bare replyToId to the main channel as a quote-reply (quoted_message_id) instead — matching the reply-to affordance of the Discord/Slack/Telegram channels — and reserve threads for genuine thread context (an explicit threadId or a thread-kind target). DM replies likewise quote-reply in the same conversation. quoted_message_id is omitted when there is no reply context, so plain sends are unchanged. Co-Authored-By: Claude Opus 4.8 * test(clickclack): cover quote payload typing --------- Co-authored-by: Marvinthebored Co-authored-by: Claude Opus 4.8 Co-authored-by: Vincent Koc --- extensions/clickclack/src/http-client.test.ts | 63 +++++++++++ extensions/clickclack/src/http-client.ts | 20 +++- extensions/clickclack/src/outbound.test.ts | 107 ++++++++++++++++++ extensions/clickclack/src/outbound.ts | 19 +++- 4 files changed, 201 insertions(+), 8 deletions(-) create mode 100644 extensions/clickclack/src/outbound.test.ts diff --git a/extensions/clickclack/src/http-client.test.ts b/extensions/clickclack/src/http-client.test.ts index ecee8442cc23..bd1d7591be5d 100644 --- a/extensions/clickclack/src/http-client.test.ts +++ b/extensions/clickclack/src/http-client.test.ts @@ -191,6 +191,69 @@ describe("ClickClack HTTP client", () => { }); }); + it("includes quoted_message_id on a channel message when quoting", async () => { + const fetchMock = vi.fn( + async (_input: string | URL | Request, _init?: RequestInit) => + new Response(JSON.stringify({ message: { id: "msg_q" } }), { + status: 201, + headers: { "Content-Type": "application/json" }, + }), + ); + const client = createClickClackClient({ + baseUrl: "https://clickclack.example", + token: "test-token", + fetch: fetchMock as unknown as typeof fetch, + }); + + await client.createChannelMessage("chn_1", "ack", { quotedMessageId: "msg_root" }); + + expect(requestBodyJson(fetchMock.mock.calls[0]?.[1])).toEqual({ + body: "ack", + quoted_message_id: "msg_root", + }); + }); + + it("omits quoted_message_id on a channel message when not quoting", async () => { + const fetchMock = vi.fn( + async (_input: string | URL | Request, _init?: RequestInit) => + new Response(JSON.stringify({ message: { id: "msg_p" } }), { + status: 201, + headers: { "Content-Type": "application/json" }, + }), + ); + const client = createClickClackClient({ + baseUrl: "https://clickclack.example", + token: "test-token", + fetch: fetchMock as unknown as typeof fetch, + }); + + await client.createChannelMessage("chn_1", "hello"); + + expect(requestBodyJson(fetchMock.mock.calls[0]?.[1])).toEqual({ body: "hello" }); + }); + + it("includes quoted_message_id on a direct message when quoting", async () => { + const fetchMock = vi.fn( + async (_input: string | URL | Request, _init?: RequestInit) => + new Response(JSON.stringify({ message: { id: "msg_dm_q" } }), { + status: 201, + headers: { "Content-Type": "application/json" }, + }), + ); + const client = createClickClackClient({ + baseUrl: "https://clickclack.example", + token: "test-token", + fetch: fetchMock as unknown as typeof fetch, + }); + + await client.createDirectMessage("dcn_1", "ack", { quotedMessageId: "msg_root" }); + + expect(requestBodyJson(fetchMock.mock.calls[0]?.[1])).toEqual({ + body: "ack", + quoted_message_id: "msg_root", + }); + }); + it("rejects activity rows without a channel or conversation target", async () => { const fetchMock = vi.fn(); const client = createClickClackClient({ diff --git a/extensions/clickclack/src/http-client.ts b/extensions/clickclack/src/http-client.ts index 00a780de5c4a..edc9e871738b 100644 --- a/extensions/clickclack/src/http-client.ts +++ b/extensions/clickclack/src/http-client.ts @@ -114,11 +114,18 @@ export function createClickClackClient(options: ClientOptions) { createChannelMessage: async ( channelId: string, body: string, - opts?: { provenance?: ClickClackMessageProvenance }, + opts?: { provenance?: ClickClackMessageProvenance; quotedMessageId?: string }, ): Promise => { const data = await request<{ message: ClickClackMessage }>( `/api/channels/${encodeURIComponent(channelId)}/messages`, - { method: "POST", body: JSON.stringify({ body, ...provenanceFields(opts?.provenance) }) }, + { + method: "POST", + body: JSON.stringify({ + body, + ...(opts?.quotedMessageId ? { quoted_message_id: opts.quotedMessageId } : {}), + ...provenanceFields(opts?.provenance), + }), + }, ); return data.message; }, @@ -184,10 +191,17 @@ export function createClickClackClient(options: ClientOptions) { createDirectMessage: async ( conversationId: string, body: string, + opts?: { quotedMessageId?: string }, ): Promise => { const data = await request<{ message: ClickClackMessage }>( `/api/dms/${encodeURIComponent(conversationId)}/messages`, - { method: "POST", body: JSON.stringify({ body }) }, + { + method: "POST", + body: JSON.stringify({ + body, + ...(opts?.quotedMessageId ? { quoted_message_id: opts.quotedMessageId } : {}), + }), + }, ); return data.message; }, diff --git a/extensions/clickclack/src/outbound.test.ts b/extensions/clickclack/src/outbound.test.ts new file mode 100644 index 000000000000..4ebe35b038b4 --- /dev/null +++ b/extensions/clickclack/src/outbound.test.ts @@ -0,0 +1,107 @@ +// Covers outbound delivery routing: a reply to a top-level message must post to +// the main channel as a quote-reply, not open a per-reply thread. +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { sendClickClackText } from "./outbound.js"; +import type { CoreConfig } from "./types.js"; + +const createChannelMessage = vi.hoisted(() => vi.fn(async () => ({ id: "msg_out" }))); +const createThreadReply = vi.hoisted(() => vi.fn(async () => ({ id: "msg_out" }))); +const createDirectMessage = vi.hoisted(() => vi.fn(async () => ({ id: "msg_out" }))); +const createDirectConversation = vi.hoisted(() => vi.fn(async () => ({ id: "dm_1" }))); + +vi.mock("./accounts.js", () => ({ + resolveClickClackAccount: () => ({ + baseUrl: "https://clickclack.example", + token: "test-token", + workspace: "wsp_1", + }), +})); + +vi.mock("./http-client.js", () => ({ + createClickClackClient: () => ({ + createChannelMessage, + createThreadReply, + createDirectMessage, + createDirectConversation, + }), +})); + +vi.mock("./resolve.js", () => ({ + resolveWorkspaceId: async () => "wsp_1", + resolveChannelId: async (_client: unknown, _workspaceId: string, id: string) => id, +})); + +const cfg = {} as CoreConfig; + +describe("sendClickClackText routing", () => { + beforeEach(() => { + createChannelMessage.mockClear(); + createThreadReply.mockClear(); + createDirectMessage.mockClear(); + createDirectConversation.mockClear(); + }); + + it("delivers a reply to a top-level channel message as an in-channel quote-reply", async () => { + await sendClickClackText({ + cfg, + to: "channel:general", + text: "hi", + replyToId: "msg_root", + }); + + expect(createChannelMessage).toHaveBeenCalledTimes(1); + expect(createChannelMessage).toHaveBeenCalledWith( + "general", + "hi", + expect.objectContaining({ quotedMessageId: "msg_root" }), + ); + expect(createThreadReply).not.toHaveBeenCalled(); + }); + + it("posts a plain channel message when there is no reply context", async () => { + await sendClickClackText({ cfg, to: "channel:general", text: "hi" }); + + expect(createChannelMessage).toHaveBeenCalledWith( + "general", + "hi", + expect.objectContaining({ quotedMessageId: undefined }), + ); + expect(createThreadReply).not.toHaveBeenCalled(); + }); + + it("keeps replies inside a genuine thread (explicit threadId)", async () => { + await sendClickClackText({ + cfg, + to: "channel:general", + text: "hi", + threadId: "msg_thread_root", + replyToId: "msg_root", + }); + + expect(createThreadReply).toHaveBeenCalledWith("msg_thread_root", "hi", expect.anything()); + expect(createChannelMessage).not.toHaveBeenCalled(); + }); + + it("threads when the target itself names a thread", async () => { + await sendClickClackText({ cfg, to: "thread:msg_root", text: "hi" }); + + expect(createThreadReply).toHaveBeenCalledWith("msg_root", "hi", expect.anything()); + expect(createChannelMessage).not.toHaveBeenCalled(); + }); + + it("delivers a DM reply as a quote-reply in the same conversation", async () => { + await sendClickClackText({ + cfg, + to: "dm:usr_1", + text: "hi", + replyToId: "msg_root", + }); + + expect(createDirectMessage).toHaveBeenCalledWith( + "dm_1", + "hi", + expect.objectContaining({ quotedMessageId: "msg_root" }), + ); + expect(createThreadReply).not.toHaveBeenCalled(); + }); +}); diff --git a/extensions/clickclack/src/outbound.ts b/extensions/clickclack/src/outbound.ts index 65edf029a0fa..efe801892554 100644 --- a/extensions/clickclack/src/outbound.ts +++ b/extensions/clickclack/src/outbound.ts @@ -28,10 +28,13 @@ export async function sendClickClackText(params: { const parsed = parseClickClackTarget(params.to); const explicitThreadId = params.threadId == null ? "" : String(params.threadId); const replyToId = params.replyToId == null ? "" : String(params.replyToId); - if (explicitThreadId || replyToId || parsed.kind === "thread") { - // Explicit thread/reply context wins over the target kind so OpenClaw reply - // hooks keep conversations attached to the original ClickClack root. - const rootId = explicitThreadId || replyToId || parsed.id; + if (explicitThreadId || parsed.kind === "thread") { + // Genuine thread context (the inbound message already lived in a thread, or + // the target explicitly names a thread) stays in that thread. A bare reply to + // a top-level message must NOT open a new thread — see the quote-reply paths + // below — otherwise every channel reply spawns its own thread and the main + // timeline goes silent. + const rootId = explicitThreadId || parsed.id; const message = await client.createThreadReply(rootId, params.text, { provenance: params.provenance, }); @@ -39,12 +42,18 @@ export async function sendClickClackText(params: { } if (parsed.kind === "dm") { const dm = await client.createDirectConversation(workspaceId, [parsed.id]); - const message = await client.createDirectMessage(dm.id, params.text); + const message = await client.createDirectMessage(dm.id, params.text, { + quotedMessageId: replyToId || undefined, + }); return { to: params.to, messageId: message.id }; } const channelId = await resolveChannelId(client, workspaceId, parsed.id); + // A reply to a top-level channel message is delivered to the main channel as a + // quote-reply (quoted_message_id), matching the reply-to affordance of the + // Discord/Slack/Telegram channels, instead of opening a per-reply thread. const message = await client.createChannelMessage(channelId, params.text, { provenance: params.provenance, + quotedMessageId: replyToId || undefined, }); return { to: params.to, messageId: message.id }; }