mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-09 05:33:59 +00:00
fix(clickclack): address review — docs, lint lanes, activity coalescing
- docs: document agentActivity option, the non-inherited agent_activity:write scope, default-off and best-effort degradation behavior - activity: type the enqueue catch parameter as unknown (lint lane) - activity: treat toolCallId as opaque; normalize only itemId lane prefixes - activity: skip identical/stale-shorter commentary snapshots so no redundant PATCHes queue - http-client: fail fast when createActivityMessage has no channel or conversation target; narrow mocked request bodies before JSON.parse
This commit is contained in:
@@ -130,6 +130,30 @@ bit:
|
||||
Keep the trust bit off if you only use the default `agent` reply mode; it is
|
||||
not needed there.
|
||||
|
||||
## Agent activity rows
|
||||
|
||||
By default a ClickClack channel shows nothing while an agent turn runs; only the final reply lands. Set `agentActivity: true` on an account to publish durable `agent_commentary` and `agent_tool` message rows while the turn is in progress:
|
||||
|
||||
```json5
|
||||
{
|
||||
channels: {
|
||||
clickclack: {
|
||||
enabled: true,
|
||||
token: { source: "env", provider: "default", id: "CLICKCLACK_BOT_TOKEN" },
|
||||
workspace: "default",
|
||||
agentActivity: true,
|
||||
},
|
||||
},
|
||||
}
|
||||
```
|
||||
|
||||
Requirements and behavior:
|
||||
|
||||
- **Off by default.** Stock setups and older ClickClack servers are untouched.
|
||||
- **Requires the `agent_activity:write` token scope.** This scope is separate from `bot:write` and is not inherited by it; create the bot token with `--scopes bot:write,agent_activity:write` (or grant the scope to an existing token) before enabling the option.
|
||||
- **Best-effort degradation.** If the token lacks `agent_activity:write` or the server rejects activity writes, failures are logged and the final reply still delivers normally; no activity rows appear.
|
||||
- Rows are grouped per turn (`turn_id`), coalesced so one logical step is one row, and tool rows use the same progress formatting as Discord/Slack/Telegram (tool name plus command detail).
|
||||
|
||||
## Targets
|
||||
|
||||
- `channel:<name-or-id>` sends to a workspace channel. Bare targets default to `channel:`.
|
||||
@@ -153,8 +177,9 @@ ClickClack token scopes are enforced by the ClickClack API.
|
||||
- `bot:read`: read workspace/channel/message/thread/DM/realtime/profile data.
|
||||
- `bot:write`: `bot:read` plus channel messages, thread replies, DMs, and uploads.
|
||||
- `bot:admin`: `bot:write` plus channel creation.
|
||||
- `agent_activity:write`: durable agent activity rows (`agent_commentary` / `agent_tool`). Not inherited by `bot:write` or `bot:admin`; required only when `agentActivity: true` is set.
|
||||
|
||||
OpenClaw only needs `bot:write` for normal agent chat.
|
||||
OpenClaw only needs `bot:write` for normal agent chat. Add `agent_activity:write` when enabling [agent activity rows](#agent-activity-rows).
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
|
||||
@@ -71,6 +71,26 @@ describe("createClickClackActivityPublisher", () => {
|
||||
expect(updateMessageBody).toHaveBeenCalledWith("msg_1", "First and second");
|
||||
});
|
||||
|
||||
it("skips redundant PATCHes for identical or stale-shorter commentary snapshots", async () => {
|
||||
const { client, createActivityMessage, updateMessageBody } = createClientMock();
|
||||
const publisher = createClickClackActivityPublisher({
|
||||
client,
|
||||
target: { channelId: "chn_1" },
|
||||
turnId: "msg_turn",
|
||||
flushMs: 10,
|
||||
});
|
||||
|
||||
publisher.onItemEvent({ itemId: "c1", kind: "preamble", progressText: "First and second" });
|
||||
await vi.advanceTimersByTimeAsync(20);
|
||||
// Identical snapshot and a stale shorter frame must not queue new flushes.
|
||||
publisher.onItemEvent({ itemId: "c1", kind: "preamble", progressText: "First and second" });
|
||||
publisher.onItemEvent({ itemId: "c1", kind: "preamble", progressText: "First" });
|
||||
await publisher.finalize();
|
||||
|
||||
expect(createActivityMessage).toHaveBeenCalledTimes(1);
|
||||
expect(updateMessageBody).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("opens a new durable row for each commentary segment (item id)", async () => {
|
||||
const { client, createActivityMessage } = createClientMock();
|
||||
const publisher = createClickClackActivityPublisher({
|
||||
@@ -98,10 +118,18 @@ describe("createClickClackActivityPublisher", () => {
|
||||
turnId: "msg_turn",
|
||||
});
|
||||
|
||||
publisher.onItemEvent({ toolCallId: "tool:toolu_1", kind: "tool", name: "exec" });
|
||||
// The runtime emits one opaque toolCallId across all frames of a call;
|
||||
// the lane prefix (tool:/command:) lives on itemId only.
|
||||
publisher.onItemEvent({
|
||||
itemId: "tool:toolu_1",
|
||||
toolCallId: "toolu_1",
|
||||
kind: "tool",
|
||||
name: "exec",
|
||||
});
|
||||
await publisher.finalize();
|
||||
publisher.onItemEvent({
|
||||
toolCallId: "command:toolu_1",
|
||||
itemId: "command:toolu_1",
|
||||
toolCallId: "toolu_1",
|
||||
kind: "command",
|
||||
name: "exec",
|
||||
progressText: "ls -la",
|
||||
|
||||
@@ -132,7 +132,7 @@ export function createClickClackActivityPublisher(params: {
|
||||
let chain: Promise<void> = Promise.resolve();
|
||||
|
||||
const enqueue = (work: () => Promise<void>): Promise<void> => {
|
||||
chain = chain.then(work).catch((error) => {
|
||||
chain = chain.then(work).catch((error: unknown) => {
|
||||
params.onError?.(error);
|
||||
});
|
||||
return chain;
|
||||
@@ -188,10 +188,13 @@ export function createClickClackActivityPublisher(params: {
|
||||
commentaryByItem.set(key, segment);
|
||||
}
|
||||
// Snapshots are cumulative per item; never shrink the row body on a
|
||||
// shorter (stale or whitespace-normalized) frame.
|
||||
if (text.length >= segment.body.length) {
|
||||
segment.body = text;
|
||||
// shorter (stale or whitespace-normalized) frame, and skip identical
|
||||
// snapshots entirely so out-of-order frames cannot queue redundant
|
||||
// PATCHes.
|
||||
if (text.length < segment.body.length || text === segment.body) {
|
||||
return;
|
||||
}
|
||||
segment.body = text;
|
||||
segment.dirty = true;
|
||||
if (!segment.timer) {
|
||||
segment.timer = setTimeout(() => {
|
||||
@@ -202,11 +205,16 @@ export function createClickClackActivityPublisher(params: {
|
||||
};
|
||||
|
||||
const toolRowKey = (payload: ClickClackItemEventPayload): string => {
|
||||
// The same call can surface with lane-prefixed ids (`tool:X`,
|
||||
// `command:X`) on some frames and the bare id on others; strip the prefix
|
||||
// from whichever identifier we use so all frames share one key.
|
||||
const base = payload.toolCallId?.trim() || payload.itemId?.trim() || "";
|
||||
return base.replace(/^(tool|command):/, "");
|
||||
// toolCallId is an opaque identifier: use it untouched when present.
|
||||
// itemId carries a lane/lifecycle prefix (`tool:X`, `command:X`) on some
|
||||
// frames and appears bare on others, so normalize only itemId to give
|
||||
// all frames of one call a shared key.
|
||||
const toolCallId = payload.toolCallId?.trim();
|
||||
if (toolCallId) {
|
||||
return toolCallId;
|
||||
}
|
||||
const itemId = payload.itemId?.trim() ?? "";
|
||||
return itemId.replace(/^(tool|command):/, "");
|
||||
};
|
||||
|
||||
const handleDiscreteItem = (payload: ClickClackItemEventPayload): void => {
|
||||
|
||||
@@ -4,6 +4,14 @@ import { createClickClackClient } from "./http-client.js";
|
||||
|
||||
const LOOPBACK_RESPONSE_BYTES = 18 * 1024 * 1024;
|
||||
|
||||
function requestBodyJson(init: RequestInit | undefined): unknown {
|
||||
const body = init?.body;
|
||||
if (typeof body !== "string") {
|
||||
throw new Error("expected string request body");
|
||||
}
|
||||
return JSON.parse(body);
|
||||
}
|
||||
|
||||
async function listenLoopbackServer(server: Server): Promise<number> {
|
||||
return await new Promise((resolve, reject) => {
|
||||
server.once("error", reject);
|
||||
@@ -176,13 +184,27 @@ describe("ClickClack HTTP client", () => {
|
||||
expect.objectContaining({ method: "POST" }),
|
||||
);
|
||||
const init = fetchMock.mock.calls[0]?.[1];
|
||||
expect(JSON.parse(String(init?.body))).toEqual({
|
||||
expect(requestBodyJson(init)).toEqual({
|
||||
body: "ran bash",
|
||||
kind: "agent_tool",
|
||||
turn_id: "t1",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects activity rows without a channel or conversation target", async () => {
|
||||
const fetchMock = vi.fn();
|
||||
const client = createClickClackClient({
|
||||
baseUrl: "https://clickclack.example",
|
||||
token: "test-token",
|
||||
fetch: fetchMock as unknown as typeof fetch,
|
||||
});
|
||||
|
||||
await expect(
|
||||
client.createActivityMessage({ body: "orphan row", kind: "agent_commentary" }),
|
||||
).rejects.toThrow("createActivityMessage requires a channelId or conversationId");
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("routes DM activity rows through the conversation create path", async () => {
|
||||
const fetchMock = vi.fn(
|
||||
async () =>
|
||||
@@ -231,6 +253,6 @@ describe("ClickClack HTTP client", () => {
|
||||
expect.objectContaining({ method: "PATCH" }),
|
||||
);
|
||||
const init = fetchMock.mock.calls[0]?.[1];
|
||||
expect(JSON.parse(String(init?.body))).toEqual({ body: "longer" });
|
||||
expect(requestBodyJson(init)).toEqual({ body: "longer" });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -127,6 +127,9 @@ export function createClickClackClient(options: ClientOptions) {
|
||||
kind: "agent_commentary" | "agent_tool";
|
||||
turnId?: string;
|
||||
}): Promise<ClickClackMessage> => {
|
||||
if (!params.channelId && !params.conversationId) {
|
||||
throw new Error("createActivityMessage requires a channelId or conversationId");
|
||||
}
|
||||
const path = params.channelId
|
||||
? `/api/channels/${encodeURIComponent(params.channelId)}/messages`
|
||||
: `/api/dms/${encodeURIComponent(params.conversationId ?? "")}/messages`;
|
||||
|
||||
Reference in New Issue
Block a user