Skip to content

Commit 9d5991f

Browse files
bek91bek91
committed
fix: slack keep resumed sends in thread
1 parent 0131343 commit 9d5991f

5 files changed

Lines changed: 162 additions & 12 deletions

File tree

‎src/agents/openclaw-tools.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,7 @@ export function createOpenClawTools(
468468
currentChannelId: options?.currentChannelId,
469469
currentChannelProvider: options?.agentChannel,
470470
currentThreadTs: options?.currentThreadTs,
471+
agentThreadId: options?.agentThreadId,
471472
currentMessageId: options?.currentMessageId,
472473
replyToMode: options?.replyToMode,
473474
hasRepliedRef: options?.hasRepliedRef,

‎src/agents/subagent-announce-output.test.ts‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { afterEach, describe, expect, it, vi } from "vitest";
2-
import { __testing, readSubagentOutput } from "./subagent-announce-output.js";
2+
import {
3+
__testing,
4+
buildChildCompletionFindings,
5+
readSubagentOutput,
6+
} from "./subagent-announce-output.js";
37

48
type CallGateway = typeof import("../gateway/call.js").callGateway;
59
type ReadLatestAssistantReply = typeof import("./tools/agent-step.js").readLatestAssistantReply;
@@ -101,3 +105,56 @@ describe("readSubagentOutput", () => {
101105
);
102106
});
103107
});
108+
109+
describe("buildChildCompletionFindings", () => {
110+
it("does not convert ANNOUNCE_SKIP child completions into no-output findings", () => {
111+
const findings = buildChildCompletionFindings([
112+
{
113+
childSessionKey: "agent:main:subagent:silent",
114+
task: "silent task",
115+
createdAt: 1,
116+
frozenResultText: "ANNOUNCE_SKIP",
117+
outcome: { status: "ok" },
118+
},
119+
]);
120+
121+
expect(findings).toBeUndefined();
122+
});
123+
124+
it("keeps failed ANNOUNCE_SKIP child completions visible", () => {
125+
const findings = buildChildCompletionFindings([
126+
{
127+
childSessionKey: "agent:main:subagent:silent",
128+
task: "silent task",
129+
createdAt: 1,
130+
frozenResultText: "ANNOUNCE_SKIP",
131+
outcome: { status: "error", error: "boom" },
132+
},
133+
]);
134+
135+
expect(findings).toContain("status: error: boom");
136+
expect(findings).toContain("ANNOUNCE_SKIP");
137+
});
138+
139+
it("numbers findings contiguously after skipped silent completions", () => {
140+
const findings = buildChildCompletionFindings([
141+
{
142+
childSessionKey: "agent:main:subagent:silent",
143+
task: "silent task",
144+
createdAt: 1,
145+
frozenResultText: "ANNOUNCE_SKIP",
146+
outcome: { status: "ok" },
147+
},
148+
{
149+
childSessionKey: "agent:main:subagent:visible",
150+
task: "visible task",
151+
createdAt: 2,
152+
frozenResultText: "actual output",
153+
outcome: { status: "ok" },
154+
},
155+
]);
156+
157+
expect(findings).toContain("1. visible task");
158+
expect(findings).not.toContain("2. visible task");
159+
});
160+
});

‎src/agents/subagent-announce-output.ts‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -447,17 +447,27 @@ export function buildChildCompletionFindings(
447447

448448
const sections: string[] = [];
449449
for (const [index, child] of sorted.entries()) {
450+
const resultText = child.frozenResultText?.trim();
451+
const outcome = describeSubagentOutcome(child.outcome);
452+
if (
453+
child.outcome?.status === "ok" &&
454+
resultText &&
455+
(isAnnounceSkip(resultText) || isSilentReplyText(resultText, SILENT_REPLY_TOKEN))
456+
) {
457+
continue;
458+
}
450459
const title =
451460
child.label?.trim() ||
452461
child.task.trim() ||
453462
child.childSessionKey.trim() ||
454463
`child ${index + 1}`;
455-
const resultText = child.frozenResultText?.trim();
456-
const outcome = describeSubagentOutcome(child.outcome);
464+
const displayIndex = sections.length + 1;
457465
sections.push(
458-
[`${index + 1}. ${title}`, `status: ${outcome}`, formatUntrustedChildResult(resultText)].join(
459-
"\n",
460-
),
466+
[
467+
`${displayIndex}. ${title}`,
468+
`status: ${outcome}`,
469+
formatUntrustedChildResult(resultText),
470+
].join("\n"),
461471
);
462472
}
463473

‎src/agents/tools/message-tool.test.ts‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@ import type { ChannelMessageCapability } from "../../channels/plugins/message-ca
44
import type { ChannelMessageActionName, ChannelPlugin } from "../../channels/plugins/types.js";
55
import type { MessageActionRunResult } from "../../infra/outbound/message-action-runner.js";
66
type CreateMessageTool = typeof import("./message-tool.js").createMessageTool;
7+
type CreateOpenClawTools = typeof import("../openclaw-tools.js").createOpenClawTools;
78
type ResetPluginRuntimeStateForTest =
89
typeof import("../../plugins/runtime.js").resetPluginRuntimeStateForTest;
910
type SetActivePluginRegistry = typeof import("../../plugins/runtime.js").setActivePluginRegistry;
1011
type CreateTestRegistry = typeof import("../../test-utils/channel-plugins.js").createTestRegistry;
1112

1213
let createMessageTool: CreateMessageTool;
14+
let createOpenClawTools: CreateOpenClawTools;
1315
let resetPluginRuntimeStateForTest: ResetPluginRuntimeStateForTest;
1416
let setActivePluginRegistry: SetActivePluginRegistry;
1517
let createTestRegistry: CreateTestRegistry;
@@ -154,6 +156,7 @@ beforeAll(async () => {
154156
await import("../../plugins/runtime.js"));
155157
({ createTestRegistry } = await import("../../test-utils/channel-plugins.js"));
156158
({ createMessageTool } = await import("./message-tool.js"));
159+
({ createOpenClawTools } = await import("../openclaw-tools.js"));
157160
});
158161

159162
beforeEach(() => {
@@ -358,6 +361,79 @@ describe("message tool agent routing", () => {
358361
expect(call?.agentId).toBe("alpha");
359362
expect(call?.sessionKey).toBe("agent:alpha:main");
360363
});
364+
365+
it("uses agentThreadId as ambient thread context when currentThreadTs is absent", async () => {
366+
mockSendResult({ channel: "slack", to: "channel:C123" });
367+
368+
const tool = createMessageTool({
369+
agentSessionKey: "agent:main:slack:channel:c123:thread:111.222",
370+
config: {} as never,
371+
currentChannelProvider: "slack",
372+
currentChannelId: "channel:C123",
373+
agentThreadId: "111.222",
374+
runMessageAction: mocks.runMessageAction as never,
375+
});
376+
377+
await tool.execute("1", {
378+
action: "send",
379+
channel: "slack",
380+
message: "stay in thread",
381+
});
382+
383+
const call = mocks.runMessageAction.mock.calls[0]?.[0];
384+
expect(call?.toolContext?.currentThreadTs).toBe("111.222");
385+
expect(call?.toolContext?.replyToMode).toBe("all");
386+
});
387+
388+
it("keeps explicit reply mode opt-out when agentThreadId is present", async () => {
389+
mockSendResult({ channel: "slack", to: "channel:C123" });
390+
391+
const tool = createMessageTool({
392+
agentSessionKey: "agent:main:slack:channel:c123:thread:111.222",
393+
config: {} as never,
394+
currentChannelProvider: "slack",
395+
currentChannelId: "channel:C123",
396+
agentThreadId: "111.222",
397+
replyToMode: "off",
398+
runMessageAction: mocks.runMessageAction as never,
399+
});
400+
401+
await tool.execute("1", {
402+
action: "send",
403+
channel: "slack",
404+
message: "send at channel level",
405+
});
406+
407+
const call = mocks.runMessageAction.mock.calls[0]?.[0];
408+
expect(call?.toolContext?.currentThreadTs).toBe("111.222");
409+
expect(call?.toolContext?.replyToMode).toBe("off");
410+
});
411+
412+
it("forwards agentThreadId through createOpenClawTools to the message tool", async () => {
413+
mockSendResult({ channel: "slack", to: "channel:C123" });
414+
415+
const tool = createOpenClawTools({
416+
agentSessionKey: "agent:main:slack:channel:c123:thread:111.222",
417+
config: {} as never,
418+
agentChannel: "slack",
419+
currentChannelId: "channel:C123",
420+
agentThreadId: "111.222",
421+
}).find((candidate) => candidate.name === "message");
422+
423+
if (!tool) {
424+
throw new Error("message tool not found");
425+
}
426+
427+
await tool.execute("1", {
428+
action: "send",
429+
channel: "slack",
430+
message: "stay in thread",
431+
});
432+
433+
const call = mocks.runMessageAction.mock.calls[0]?.[0];
434+
expect(call?.toolContext?.currentThreadTs).toBe("111.222");
435+
expect(call?.toolContext?.replyToMode).toBe("all");
436+
});
361437
});
362438

363439
describe("message tool explicit target guard", () => {

‎src/agents/tools/message-tool.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { getRuntimeConfig } from "../../config/config.js";
1717
import type { OpenClawConfig } from "../../config/types.openclaw.js";
1818
import { GATEWAY_CLIENT_IDS, GATEWAY_CLIENT_MODES } from "../../gateway/protocol/client-info.js";
1919
import { getToolResult, runMessageAction } from "../../infra/outbound/message-action-runner.js";
20+
import { stringifyRouteThreadId } from "../../plugin-sdk/channel-route.js";
2021
import { POLL_CREATION_PARAM_DEFS, SHARED_POLL_CREATION_PARAM_NAMES } from "../../poll-params.js";
2122
import { normalizeAccountId } from "../../routing/session-key.js";
2223
import { normalizeOptionalString } from "../../shared/string-coerce.js";
@@ -513,6 +514,7 @@ type MessageToolOptions = {
513514
currentChannelId?: string;
514515
currentChannelProvider?: string;
515516
currentThreadTs?: string;
517+
agentThreadId?: string | number;
516518
currentMessageId?: string | number;
517519
replyToMode?: "off" | "first" | "all" | "batched";
518520
hasRepliedRef?: { value: boolean };
@@ -706,6 +708,10 @@ export function createMessageTool(options?: MessageToolOptions): AnyAgentTool {
706708
options?.resolveCommandSecretRefsViaGateway ?? resolveCommandSecretRefsViaGateway;
707709
const runMessageActionForTool = options?.runMessageAction ?? runMessageAction;
708710
const agentAccountId = resolveAgentAccountId(options?.agentAccountId);
711+
const currentThreadTs =
712+
options?.currentThreadTs ??
713+
(options?.agentThreadId != null ? stringifyRouteThreadId(options.agentThreadId) : undefined);
714+
const replyToMode = options?.replyToMode ?? (currentThreadTs ? "all" : undefined);
709715
const resolvedAgentId = options?.agentSessionKey
710716
? resolveSessionAgentId({
711717
sessionKey: options.agentSessionKey,
@@ -717,7 +723,7 @@ export function createMessageTool(options?: MessageToolOptions): AnyAgentTool {
717723
cfg: options.config,
718724
currentChannelProvider: options.currentChannelProvider,
719725
currentChannelId: options.currentChannelId,
720-
currentThreadTs: options.currentThreadTs,
726+
currentThreadTs,
721727
currentMessageId: options.currentMessageId,
722728
currentAccountId: agentAccountId,
723729
sessionKey: options.agentSessionKey,
@@ -731,7 +737,7 @@ export function createMessageTool(options?: MessageToolOptions): AnyAgentTool {
731737
config: options?.config,
732738
currentChannel: options?.currentChannelProvider,
733739
currentChannelId: options?.currentChannelId,
734-
currentThreadTs: options?.currentThreadTs,
740+
currentThreadTs,
735741
currentMessageId: options?.currentMessageId,
736742
currentAccountId: agentAccountId,
737743
sessionKey: options?.agentSessionKey,
@@ -834,16 +840,16 @@ export function createMessageTool(options?: MessageToolOptions): AnyAgentTool {
834840
const toolContext =
835841
options?.currentChannelId ||
836842
options?.currentChannelProvider ||
837-
options?.currentThreadTs ||
843+
currentThreadTs ||
838844
hasCurrentMessageId ||
839-
options?.replyToMode ||
845+
replyToMode ||
840846
options?.hasRepliedRef
841847
? {
842848
currentChannelId: options?.currentChannelId,
843849
currentChannelProvider: options?.currentChannelProvider,
844-
currentThreadTs: options?.currentThreadTs,
850+
currentThreadTs,
845851
currentMessageId: options?.currentMessageId,
846-
replyToMode: options?.replyToMode,
852+
replyToMode,
847853
hasRepliedRef: options?.hasRepliedRef,
848854
// Direct tool invocations should not add cross-context decoration.
849855
// The agent is composing a message, not forwarding from another chat.

0 commit comments

Comments
 (0)