fix: stop sending tool summaries to channels

This commit is contained in:
Peter Steinberger
2026-01-25 11:54:20 +00:00
parent b6581e77f6
commit 875b018ea1
13 changed files with 100 additions and 101 deletions

View File

@@ -599,7 +599,7 @@ export async function runEmbeddedPiAgent(
verboseLevel: params.verboseLevel, verboseLevel: params.verboseLevel,
reasoningLevel: params.reasoningLevel, reasoningLevel: params.reasoningLevel,
toolResultFormat: resolvedToolResultFormat, toolResultFormat: resolvedToolResultFormat,
inlineToolResultsAllowed: !params.onPartialReply && !params.onToolResult, inlineToolResultsAllowed: false,
}); });
log.debug( log.debug(

View File

@@ -138,6 +138,38 @@ describe("dispatchReplyFromConfig", () => {
); );
}); });
it("does not provide onToolResult when routing cross-provider", async () => {
mocks.tryFastAbortFromMessage.mockResolvedValue({
handled: false,
aborted: false,
});
mocks.routeReply.mockClear();
const cfg = {} as ClawdbotConfig;
const dispatcher = createDispatcher();
const ctx = buildTestCtx({
Provider: "slack",
OriginatingChannel: "telegram",
OriginatingTo: "telegram:999",
});
const replyResolver = async (
_ctx: MsgContext,
opts: GetReplyOptions | undefined,
_cfg: ClawdbotConfig,
) => {
expect(opts?.onToolResult).toBeUndefined();
return { text: "hi" } satisfies ReplyPayload;
};
await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver });
expect(mocks.routeReply).toHaveBeenCalledWith(
expect.objectContaining({
payload: expect.objectContaining({ text: "hi" }),
}),
);
});
it("fast-aborts without calling the reply resolver", async () => { it("fast-aborts without calling the reply resolver", async () => {
mocks.tryFastAbortFromMessage.mockResolvedValue({ mocks.tryFastAbortFromMessage.mockResolvedValue({
handled: true, handled: true,

View File

@@ -206,6 +206,7 @@ export async function dispatchReplyFromConfig(params: {
const sendPayloadAsync = async ( const sendPayloadAsync = async (
payload: ReplyPayload, payload: ReplyPayload,
abortSignal?: AbortSignal, abortSignal?: AbortSignal,
mirror?: boolean,
): Promise<void> => { ): Promise<void> => {
// TypeScript doesn't narrow these from the shouldRouteToOriginating check, // TypeScript doesn't narrow these from the shouldRouteToOriginating check,
// but they're guaranteed non-null when this function is called. // but they're guaranteed non-null when this function is called.
@@ -220,6 +221,7 @@ export async function dispatchReplyFromConfig(params: {
threadId: ctx.MessageThreadId, threadId: ctx.MessageThreadId,
cfg, cfg,
abortSignal, abortSignal,
mirror,
}); });
if (!result.ok) { if (!result.ok) {
logVerbose(`dispatch-from-config: route-reply failed: ${result.error ?? "unknown error"}`); logVerbose(`dispatch-from-config: route-reply failed: ${result.error ?? "unknown error"}`);
@@ -268,24 +270,6 @@ export async function dispatchReplyFromConfig(params: {
ctx, ctx,
{ {
...params.replyOptions, ...params.replyOptions,
onToolResult: (payload: ReplyPayload) => {
const run = async () => {
const ttsPayload = await maybeApplyTtsToPayload({
payload,
cfg,
channel: ttsChannel,
kind: "tool",
inboundAudio,
ttsAuto: sessionTtsAuto,
});
if (shouldRouteToOriginating) {
await sendPayloadAsync(ttsPayload);
} else {
dispatcher.sendToolResult(ttsPayload);
}
};
return run();
},
onBlockReply: (payload: ReplyPayload, context) => { onBlockReply: (payload: ReplyPayload, context) => {
const run = async () => { const run = async () => {
const ttsPayload = await maybeApplyTtsToPayload({ const ttsPayload = await maybeApplyTtsToPayload({
@@ -297,7 +281,7 @@ export async function dispatchReplyFromConfig(params: {
ttsAuto: sessionTtsAuto, ttsAuto: sessionTtsAuto,
}); });
if (shouldRouteToOriginating) { if (shouldRouteToOriginating) {
await sendPayloadAsync(ttsPayload, context?.abortSignal); await sendPayloadAsync(ttsPayload, context?.abortSignal, false);
} else { } else {
dispatcher.sendBlockReply(ttsPayload); dispatcher.sendBlockReply(ttsPayload);
} }

View File

@@ -379,6 +379,23 @@ describe("routeReply", () => {
}), }),
); );
}); });
it("skips mirror data when mirror is false", async () => {
mocks.deliverOutboundPayloads.mockResolvedValue([]);
await routeReply({
payload: { text: "hi" },
channel: "slack",
to: "channel:C123",
sessionKey: "agent:main:main",
mirror: false,
cfg: {} as never,
});
expect(mocks.deliverOutboundPayloads).toHaveBeenCalledWith(
expect.objectContaining({
mirror: undefined,
}),
);
});
}); });
const emptyRegistry = createRegistry([]); const emptyRegistry = createRegistry([]);

View File

@@ -33,6 +33,8 @@ export type RouteReplyParams = {
cfg: ClawdbotConfig; cfg: ClawdbotConfig;
/** Optional abort signal for cooperative cancellation. */ /** Optional abort signal for cooperative cancellation. */
abortSignal?: AbortSignal; abortSignal?: AbortSignal;
/** Mirror reply into session transcript (default: true when sessionKey is set). */
mirror?: boolean;
}; };
export type RouteReplyResult = { export type RouteReplyResult = {
@@ -118,14 +120,15 @@ export async function routeReply(params: RouteReplyParams): Promise<RouteReplyRe
replyToId: resolvedReplyToId ?? null, replyToId: resolvedReplyToId ?? null,
threadId: resolvedThreadId, threadId: resolvedThreadId,
abortSignal, abortSignal,
mirror: params.sessionKey mirror:
? { params.mirror !== false && params.sessionKey
sessionKey: params.sessionKey, ? {
agentId: resolveSessionAgentId({ sessionKey: params.sessionKey, config: cfg }), sessionKey: params.sessionKey,
text, agentId: resolveSessionAgentId({ sessionKey: params.sessionKey, config: cfg }),
mediaUrls, text,
} mediaUrls,
: undefined, }
: undefined,
}); });
const last = results.at(-1); const last = results.at(-1);

View File

@@ -34,15 +34,13 @@ vi.mock("../auto-reply/dispatch.js", async (importOriginal) => {
beforeEach(() => { beforeEach(() => {
dispatchMock.mockReset().mockImplementation(async (params) => { dispatchMock.mockReset().mockImplementation(async (params) => {
if ("dispatcher" in params && params.dispatcher) { if ("dispatcher" in params && params.dispatcher) {
params.dispatcher.sendToolResult({ text: "tool update" });
params.dispatcher.sendFinalReply({ text: "final reply" }); params.dispatcher.sendFinalReply({ text: "final reply" });
return { queuedFinal: true, counts: { tool: 1, block: 0, final: 1 } }; return { queuedFinal: true, counts: { tool: 0, block: 0, final: 1 } };
} }
if ("dispatcherOptions" in params && params.dispatcherOptions) { if ("dispatcherOptions" in params && params.dispatcherOptions) {
const { dispatcher, markDispatchIdle } = createReplyDispatcherWithTyping( const { dispatcher, markDispatchIdle } = createReplyDispatcherWithTyping(
params.dispatcherOptions, params.dispatcherOptions,
); );
dispatcher.sendToolResult({ text: "tool update" });
dispatcher.sendFinalReply({ text: "final reply" }); dispatcher.sendFinalReply({ text: "final reply" });
await dispatcher.waitForIdle(); await dispatcher.waitForIdle();
markDispatchIdle(); markDispatchIdle();
@@ -53,7 +51,7 @@ beforeEach(() => {
}); });
describe("discord native commands", () => { describe("discord native commands", () => {
it("streams tool results for native slash commands", { timeout: 60_000 }, async () => { it("skips tool results for native slash commands", { timeout: 60_000 }, async () => {
const { ChannelType } = await import("@buape/carbon"); const { ChannelType } = await import("@buape/carbon");
const { createDiscordNativeCommand } = await import("./monitor.js"); const { createDiscordNativeCommand } = await import("./monitor.js");
@@ -97,8 +95,7 @@ describe("discord native commands", () => {
expect(dispatchMock).toHaveBeenCalledTimes(1); expect(dispatchMock).toHaveBeenCalledTimes(1);
expect(reply).toHaveBeenCalledTimes(1); expect(reply).toHaveBeenCalledTimes(1);
expect(followUp).toHaveBeenCalledTimes(1); expect(followUp).toHaveBeenCalledTimes(0);
expect(reply.mock.calls[0]?.[0]?.content).toContain("tool"); expect(reply.mock.calls[0]?.[0]?.content).toContain("final");
expect(followUp.mock.calls[0]?.[0]?.content).toContain("final");
}); });
}); });

View File

@@ -277,15 +277,12 @@ describe("monitorIMessageProvider", () => {
expect(ctx.SessionKey).toBe("agent:main:imessage:group:2"); expect(ctx.SessionKey).toBe("agent:main:imessage:group:2");
}); });
it("prefixes tool and final replies with responsePrefix", async () => { it("prefixes final replies with responsePrefix", async () => {
config = { config = {
...config, ...config,
messages: { responsePrefix: "PFX" }, messages: { responsePrefix: "PFX" },
}; };
replyMock.mockImplementation(async (_ctx, opts) => { replyMock.mockResolvedValue({ text: "final reply" });
await opts?.onToolResult?.({ text: "tool update" });
return { text: "final reply" };
});
const run = monitorIMessageProvider(); const run = monitorIMessageProvider();
await waitForSubscribe(); await waitForSubscribe();
@@ -307,9 +304,8 @@ describe("monitorIMessageProvider", () => {
closeResolve?.(); closeResolve?.();
await run; await run;
expect(sendMock).toHaveBeenCalledTimes(2); expect(sendMock).toHaveBeenCalledTimes(1);
expect(sendMock.mock.calls[0][1]).toBe("PFX tool update"); expect(sendMock.mock.calls[0][1]).toBe("PFX final reply");
expect(sendMock.mock.calls[1][1]).toBe("PFX final reply");
}); });
it("defaults to dmPolicy=pairing behavior when allowFrom is empty", async () => { it("defaults to dmPolicy=pairing behavior when allowFrom is empty", async () => {

View File

@@ -210,12 +210,9 @@ describe("monitorSignalProvider tool results", () => {
); );
}); });
it("sends tool summaries with responsePrefix", async () => { it("skips tool summaries with responsePrefix", async () => {
const abortController = new AbortController(); const abortController = new AbortController();
replyMock.mockImplementation(async (_ctx, opts) => { replyMock.mockResolvedValue({ text: "final reply" });
await opts?.onToolResult?.({ text: "tool update" });
return { text: "final reply" };
});
streamMock.mockImplementation(async ({ onEvent }) => { streamMock.mockImplementation(async ({ onEvent }) => {
const payload = { const payload = {
@@ -243,9 +240,8 @@ describe("monitorSignalProvider tool results", () => {
await flush(); await flush();
expect(sendMock).toHaveBeenCalledTimes(2); expect(sendMock).toHaveBeenCalledTimes(1);
expect(sendMock.mock.calls[0][1]).toBe("PFX tool update"); expect(sendMock.mock.calls[0][1]).toBe("PFX final reply");
expect(sendMock.mock.calls[1][1]).toBe("PFX final reply");
}); });
it("replies with pairing code when dmPolicy is pairing and no allowFrom is set", async () => { it("replies with pairing code when dmPolicy is pairing and no allowFrom is set", async () => {

View File

@@ -23,11 +23,8 @@ beforeEach(() => {
}); });
describe("monitorSlackProvider tool results", () => { describe("monitorSlackProvider tool results", () => {
it("sends tool summaries with responsePrefix", async () => { it("skips tool summaries with responsePrefix", async () => {
replyMock.mockImplementation(async (_ctx, opts) => { replyMock.mockResolvedValue({ text: "final reply" });
await opts?.onToolResult?.({ text: "tool update" });
return { text: "final reply" };
});
const controller = new AbortController(); const controller = new AbortController();
const run = monitorSlackProvider({ const run = monitorSlackProvider({
@@ -55,9 +52,8 @@ describe("monitorSlackProvider tool results", () => {
controller.abort(); controller.abort();
await run; await run;
expect(sendMock).toHaveBeenCalledTimes(2); expect(sendMock).toHaveBeenCalledTimes(1);
expect(sendMock.mock.calls[0][1]).toBe("PFX tool update"); expect(sendMock.mock.calls[0][1]).toBe("PFX final reply");
expect(sendMock.mock.calls[1][1]).toBe("PFX final reply");
}); });
it("drops events with mismatched api_app_id", async () => { it("drops events with mismatched api_app_id", async () => {
@@ -130,10 +126,7 @@ describe("monitorSlackProvider tool results", () => {
}, },
}; };
replyMock.mockImplementation(async (_ctx, opts) => { replyMock.mockResolvedValue({ text: "final reply" });
await opts?.onToolResult?.({ text: "tool update" });
return { text: "final reply" };
});
const controller = new AbortController(); const controller = new AbortController();
const run = monitorSlackProvider({ const run = monitorSlackProvider({
@@ -161,9 +154,8 @@ describe("monitorSlackProvider tool results", () => {
controller.abort(); controller.abort();
await run; await run;
expect(sendMock).toHaveBeenCalledTimes(2); expect(sendMock).toHaveBeenCalledTimes(1);
expect(sendMock.mock.calls[0][1]).toBe("tool update"); expect(sendMock.mock.calls[0][1]).toBe("final reply");
expect(sendMock.mock.calls[1][1]).toBe("final reply");
}); });
it("preserves RawBody without injecting processed room history", async () => { it("preserves RawBody without injecting processed room history", async () => {

View File

@@ -300,7 +300,7 @@ describe("createTelegramBot", () => {
expect.objectContaining({ message_thread_id: 99 }), expect.objectContaining({ message_thread_id: 99 }),
); );
}); });
it("streams tool summaries for native slash commands", async () => { it("skips tool summaries for native slash commands", async () => {
onSpy.mockReset(); onSpy.mockReset();
sendMessageSpy.mockReset(); sendMessageSpy.mockReset();
commandSpy.mockReset(); commandSpy.mockReset();
@@ -338,9 +338,8 @@ describe("createTelegramBot", () => {
match: "on", match: "on",
}); });
expect(sendMessageSpy).toHaveBeenCalledTimes(2); expect(sendMessageSpy).toHaveBeenCalledTimes(1);
expect(sendMessageSpy.mock.calls[0]?.[1]).toContain("tool update"); expect(sendMessageSpy.mock.calls[0]?.[1]).toContain("final reply");
expect(sendMessageSpy.mock.calls[1]?.[1]).toContain("final reply");
}); });
it("dedupes duplicate message updates by update_id", async () => { it("dedupes duplicate message updates by update_id", async () => {
onSpy.mockReset(); onSpy.mockReset();

View File

@@ -217,15 +217,12 @@ describe("createTelegramBot", () => {
expect(call[2]?.reply_to_message_id).toBeUndefined(); expect(call[2]?.reply_to_message_id).toBeUndefined();
} }
}); });
it("prefixes tool and final replies with responsePrefix", async () => { it("prefixes final replies with responsePrefix", async () => {
onSpy.mockReset(); onSpy.mockReset();
sendMessageSpy.mockReset(); sendMessageSpy.mockReset();
const replySpy = replyModule.__replySpy as unknown as ReturnType<typeof vi.fn>; const replySpy = replyModule.__replySpy as unknown as ReturnType<typeof vi.fn>;
replySpy.mockReset(); replySpy.mockReset();
replySpy.mockImplementation(async (_ctx, opts) => { replySpy.mockResolvedValue({ text: "final reply" });
await opts?.onToolResult?.({ text: "tool result" });
return { text: "final reply" };
});
loadConfig.mockReturnValue({ loadConfig.mockReturnValue({
channels: { channels: {
telegram: { dmPolicy: "open", allowFrom: ["*"] }, telegram: { dmPolicy: "open", allowFrom: ["*"] },
@@ -245,9 +242,8 @@ describe("createTelegramBot", () => {
getFile: async () => ({ download: async () => new Uint8Array() }), getFile: async () => ({ download: async () => new Uint8Array() }),
}); });
expect(sendMessageSpy).toHaveBeenCalledTimes(2); expect(sendMessageSpy).toHaveBeenCalledTimes(1);
expect(sendMessageSpy.mock.calls[0][1]).toBe("PFX tool result"); expect(sendMessageSpy.mock.calls[0][1]).toBe("PFX final reply");
expect(sendMessageSpy.mock.calls[1][1]).toBe("PFX final reply");
}); });
it("honors replyToMode=all for threaded replies", async () => { it("honors replyToMode=all for threaded replies", async () => {
onSpy.mockReset(); onSpy.mockReset();

View File

@@ -865,15 +865,12 @@ describe("createTelegramBot", () => {
} }
}); });
it("prefixes tool and final replies with responsePrefix", async () => { it("prefixes final replies with responsePrefix", async () => {
onSpy.mockReset(); onSpy.mockReset();
sendMessageSpy.mockReset(); sendMessageSpy.mockReset();
const replySpy = replyModule.__replySpy as unknown as ReturnType<typeof vi.fn>; const replySpy = replyModule.__replySpy as unknown as ReturnType<typeof vi.fn>;
replySpy.mockReset(); replySpy.mockReset();
replySpy.mockImplementation(async (_ctx, opts) => { replySpy.mockResolvedValue({ text: "final reply" });
await opts?.onToolResult?.({ text: "tool result" });
return { text: "final reply" };
});
loadConfig.mockReturnValue({ loadConfig.mockReturnValue({
channels: { channels: {
telegram: { dmPolicy: "open", allowFrom: ["*"] }, telegram: { dmPolicy: "open", allowFrom: ["*"] },
@@ -893,9 +890,8 @@ describe("createTelegramBot", () => {
getFile: async () => ({ download: async () => new Uint8Array() }), getFile: async () => ({ download: async () => new Uint8Array() }),
}); });
expect(sendMessageSpy).toHaveBeenCalledTimes(2); expect(sendMessageSpy).toHaveBeenCalledTimes(1);
expect(sendMessageSpy.mock.calls[0][1]).toBe("PFX tool result"); expect(sendMessageSpy.mock.calls[0][1]).toBe("PFX final reply");
expect(sendMessageSpy.mock.calls[1][1]).toBe("PFX final reply");
}); });
it("honors replyToMode=all for threaded replies", async () => { it("honors replyToMode=all for threaded replies", async () => {
@@ -2288,7 +2284,7 @@ describe("createTelegramBot", () => {
); );
}); });
it("streams tool summaries for native slash commands", async () => { it("skips tool summaries for native slash commands", async () => {
onSpy.mockReset(); onSpy.mockReset();
sendMessageSpy.mockReset(); sendMessageSpy.mockReset();
commandSpy.mockReset(); commandSpy.mockReset();
@@ -2326,9 +2322,8 @@ describe("createTelegramBot", () => {
match: "on", match: "on",
}); });
expect(sendMessageSpy).toHaveBeenCalledTimes(2); expect(sendMessageSpy).toHaveBeenCalledTimes(1);
expect(sendMessageSpy.mock.calls[0]?.[1]).toContain("tool update"); expect(sendMessageSpy.mock.calls[0]?.[1]).toContain("final reply");
expect(sendMessageSpy.mock.calls[1]?.[1]).toContain("final reply");
}); });
it("dedupes duplicate message updates by update_id", async () => { it("dedupes duplicate message updates by update_id", async () => {

View File

@@ -105,7 +105,7 @@ describe("web auto-reply", () => {
vi.useRealTimers(); vi.useRealTimers();
}); });
it("sends tool summaries immediately with responsePrefix", async () => { it("skips tool summaries and sends final reply with responsePrefix", async () => {
setLoadConfigMock(() => ({ setLoadConfigMock(() => ({
channels: { whatsapp: { allowFrom: ["*"] } }, channels: { whatsapp: { allowFrom: ["*"] } },
messages: { messages: {
@@ -125,15 +125,7 @@ describe("web auto-reply", () => {
return { close: vi.fn() }; return { close: vi.fn() };
}; };
const resolver = vi const resolver = vi.fn().mockResolvedValue({ text: "final" });
.fn()
.mockImplementation(
async (_ctx, opts?: { onToolResult?: (r: { text: string }) => Promise<void> }) => {
await opts?.onToolResult?.({ text: "🧩 tool1" });
await opts?.onToolResult?.({ text: "🧩 tool2" });
return { text: "final" };
},
);
await monitorWebChannel(false, listenerFactory, false, resolver); await monitorWebChannel(false, listenerFactory, false, resolver);
expect(capturedOnMessage).toBeDefined(); expect(capturedOnMessage).toBeDefined();
@@ -149,7 +141,7 @@ describe("web auto-reply", () => {
}); });
const replies = reply.mock.calls.map((call) => call[0]); const replies = reply.mock.calls.map((call) => call[0]);
expect(replies).toEqual(["🦞 🧩 tool1", "🦞 🧩 tool2", "🦞 final"]); expect(replies).toEqual(["🦞 final"]);
resetLoadConfigMock(); resetLoadConfigMock();
}); });
it("uses identity.name for messagePrefix when set", async () => { it("uses identity.name for messagePrefix when set", async () => {