mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-10 23:26:13 +00:00
fix(clickclack): reply to a top-level message in-channel, not as a new thread (#100582)
* 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 = <triggering message id> 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 <noreply@anthropic.com> * test(clickclack): cover quote payload typing --------- Co-authored-by: Marvinthebored <marvinthebored@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
@@ -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({
|
||||
|
||||
@@ -114,11 +114,18 @@ export function createClickClackClient(options: ClientOptions) {
|
||||
createChannelMessage: async (
|
||||
channelId: string,
|
||||
body: string,
|
||||
opts?: { provenance?: ClickClackMessageProvenance },
|
||||
opts?: { provenance?: ClickClackMessageProvenance; quotedMessageId?: string },
|
||||
): Promise<ClickClackMessage> => {
|
||||
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<ClickClackMessage> => {
|
||||
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;
|
||||
},
|
||||
|
||||
107
extensions/clickclack/src/outbound.test.ts
Normal file
107
extensions/clickclack/src/outbound.test.ts
Normal file
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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 };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user