diff --git a/src/auto-reply/reply/agent-runner-execution.test.ts b/src/auto-reply/reply/agent-runner-execution.test.ts index a83e0b278f2..2f44a9d9409 100644 --- a/src/auto-reply/reply/agent-runner-execution.test.ts +++ b/src/auto-reply/reply/agent-runner-execution.test.ts @@ -4250,8 +4250,9 @@ describe("runAgentTurnWithFallback", () => { }); }); - it("prefers onCompactionEnd callback over default notice when notifyUser is enabled", async () => { + it("fires both notifyUser notices alongside onCompactionStart / onCompactionEnd callbacks (#87107)", async () => { const onBlockReply = vi.fn(); + const onCompactionStart = vi.fn(); const onCompactionEnd = vi.fn(); state.runEmbeddedAgentMock.mockImplementationOnce(async (params: EmbeddedAgentParams) => { await params.onAgentEvent?.({ stream: "compaction", data: { phase: "start" } }); @@ -4281,7 +4282,7 @@ describe("runAgentTurnWithFallback", () => { Provider: "whatsapp", MessageSid: "msg", } as unknown as TemplateContext, - opts: { onBlockReply, onCompactionEnd }, + opts: { onBlockReply, onCompactionStart, onCompactionEnd }, typingSignals: createMockTypingSignaler(), blockReplyPipeline: null, blockStreamingEnabled: false, @@ -4298,14 +4299,19 @@ describe("runAgentTurnWithFallback", () => { }); expect(result.kind).toBe("success"); + // Internal callbacks (Control UI etc.) and the user-channel notifyUser + // notices are independent audiences; both must fire when opted in. + expect(onCompactionStart).toHaveBeenCalledTimes(1); expect(onCompactionEnd).toHaveBeenCalledTimes(1); - // The start notice still fires (no onCompactionStart callback provided), - // but the completion notice is suppressed in favor of the callback. - expect(onBlockReply).toHaveBeenCalledTimes(1); + expect(onBlockReply).toHaveBeenCalledTimes(2); expectBlockReplyCall(onBlockReply, 0, { text: "๐Ÿงน Compacting context...", isCompactionNotice: true, }); + expectBlockReplyCall(onBlockReply, 1, { + text: "๐Ÿงน Compaction complete", + isCompactionNotice: true, + }); }); it("emits an incomplete compaction notice when compaction ends without completing", async () => { diff --git a/src/auto-reply/reply/agent-runner-execution.ts b/src/auto-reply/reply/agent-runner-execution.ts index a76b54ce033..6a74c41197d 100644 --- a/src/auto-reply/reply/agent-runner-execution.ts +++ b/src/auto-reply/reply/agent-runner-execution.ts @@ -2437,17 +2437,18 @@ export async function runAgentTurnWithFallback(params: { const phase = readStringValue(evt.data.phase) ?? ""; const hookMessages = readCompactionHookMessages(evt.data.messages); if (phase === "start") { - // Keep custom compaction callbacks active, but gate the - // fallback user-facing notice behind explicit opt-in. + // Three independent audiences: internal callbacks + // (Control UI) fire regardless; hookMessages deliver + // plugin-authored user-channel text (overlap with the + // default notice, so they suppress it); notifyUser is + // the opt-in user-channel notice. Internal callbacks + // must not suppress the user notice โ€” see #87107. if (params.opts?.onCompactionStart) { await params.opts.onCompactionStart(); } if (hookMessages.length > 0) { await sendCompactionHookMessages(hookMessages); - } else if ( - !params.opts?.onCompactionStart && - shouldNotifyUserAboutCompaction - ) { + } else if (shouldNotifyUserAboutCompaction) { // Send directly via opts.onBlockReply (bypassing the // pipeline) so the notice does not cause final payloads // to be discarded on non-streaming model paths. @@ -2463,10 +2464,7 @@ export async function runAgentTurnWithFallback(params: { } if (hookMessages.length > 0) { await sendCompactionHookMessages(hookMessages); - } else if ( - !params.opts?.onCompactionEnd && - shouldNotifyUserAboutCompaction - ) { + } else if (shouldNotifyUserAboutCompaction) { await sendCompactionNotice("end"); } } else if (hookMessages.length > 0) { diff --git a/src/auto-reply/reply/dispatch-from-config.test.ts b/src/auto-reply/reply/dispatch-from-config.test.ts index ac7a9992abe..dccb570085d 100644 --- a/src/auto-reply/reply/dispatch-from-config.test.ts +++ b/src/auto-reply/reply/dispatch-from-config.test.ts @@ -7547,6 +7547,79 @@ describe("sendPolicy deny โ€” suppress delivery, not processing (#53328)", () => expect(dispatcher.sendToolResult).not.toHaveBeenCalled(); }); + it("delivers marked explicit command terminal replies in room events (#87107)", async () => { + setNoAbort(); + sessionStoreMocks.currentEntry = { + sessionId: "s1", + updatedAt: 0, + sendPolicy: "allow", + }; + const dispatcher = createDispatcher(); + const commandReply = setReplyPayloadMetadata( + { text: "โš™๏ธ Compacted (76k โ†’ 934 tokens)" }, + { deliverDespiteSourceReplySuppression: true }, + ); + const replyResolver = vi.fn(async () => commandReply satisfies ReplyPayload); + const ctx = buildTestCtx({ + ChatType: "group", + InboundEventKind: "room_event", + SessionKey: "test:session", + CommandSource: "text", + CommandAuthorized: true, + CommandBody: "/compact", + }); + + const result = await dispatchReplyFromConfig({ + ctx, + cfg: emptyConfig, + dispatcher, + replyResolver, + replyOptions: { + sourceReplyDeliveryMode: "message_tool_only", + }, + }); + + expect(replyResolver).toHaveBeenCalledTimes(1); + expect(result.queuedFinal).toBe(true); + expect(dispatcher.sendFinalReply).toHaveBeenCalledWith(commandReply); + }); + + it("delivers marked /compact reply in room event when CommandSource is undefined (#87107)", async () => { + setNoAbort(); + sessionStoreMocks.currentEntry = { + sessionId: "s1", + updatedAt: 0, + sendPolicy: "allow", + }; + const dispatcher = createDispatcher(); + const commandReply = setReplyPayloadMetadata( + { text: "โš™๏ธ Compacted (76k โ†’ 934 tokens)" }, + { deliverDespiteSourceReplySuppression: true }, + ); + const replyResolver = vi.fn(async () => commandReply satisfies ReplyPayload); + const ctx = buildTestCtx({ + ChatType: "group", + InboundEventKind: "room_event", + SessionKey: "test:session", + CommandAuthorized: true, + CommandBody: "/compact", + }); + + const result = await dispatchReplyFromConfig({ + ctx, + cfg: emptyConfig, + dispatcher, + replyResolver, + replyOptions: { + sourceReplyDeliveryMode: "message_tool_only", + }, + }); + + expect(replyResolver).toHaveBeenCalledTimes(1); + expect(result.queuedFinal).toBe(true); + expect(dispatcher.sendFinalReply).toHaveBeenCalledWith(commandReply); + }); + it("mirrors internal source reply payloads into the active transcript", async () => { setNoAbort(); sessionStoreMocks.currentEntry = { diff --git a/src/auto-reply/reply/dispatch-from-config.ts b/src/auto-reply/reply/dispatch-from-config.ts index 9ddf2df5ca3..d39fd7194c7 100644 --- a/src/auto-reply/reply/dispatch-from-config.ts +++ b/src/auto-reply/reply/dispatch-from-config.ts @@ -2702,11 +2702,18 @@ export async function dispatchReplyFromConfig( let routedFinalCount = 0; let attemptedFinalDelivery = false; let finalDeliveryFailed = false; + // Explicit command turns (native or authorized text-slash like /compact) are + // user-initiated, so a marked terminal reply for the command bypasses + // room_event suppression. Ambient marked notices (no CommandTurn) stay + // suppressed in room_event. sendPolicy: deny still suppresses everything. + // Uses the same helper as the source-reply visibility policy so the bypass + // and the policy stay aligned. + const explicitCommandTurnCtx = isExplicitSourceReplyCommand(ctx, cfg); const shouldDeliverDespiteSourceReplySuppression = (reply: ReplyPayload) => suppressAutomaticSourceDelivery && - ctx.InboundEventKind !== "room_event" && !sendPolicyDenied && - getReplyPayloadMetadata(reply)?.deliverDespiteSourceReplySuppression === true; + getReplyPayloadMetadata(reply)?.deliverDespiteSourceReplySuppression === true && + (ctx.InboundEventKind !== "room_event" || explicitCommandTurnCtx); for (const reply of replies) { throwIfDispatchOperationAborted(); // Suppress reasoning payloads from channel delivery โ€” channels using this diff --git a/src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts b/src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts index 11c40e240bc..36bdb0db337 100644 --- a/src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts +++ b/src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts @@ -4,6 +4,7 @@ import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; import type { SkillCommandSpec } from "../../agents/skills.js"; import type { SessionEntry } from "../../config/sessions.js"; +import { getReplyPayloadMetadata } from "../reply-payload.js"; import type { TemplateContext } from "../templating.js"; import { clearInlineDirectives } from "./get-reply-directives-utils.js"; import { handleInlineActions } from "./get-reply-inline-actions.js"; @@ -1131,4 +1132,49 @@ describe("handleInlineActions", () => { ); expect(toolExecute).toHaveBeenCalled(); }); + + it("marks command-handler terminal replies with deliverDespiteSourceReplySuppression so they are not dropped under message_tool_only delivery (#87107)", async () => { + const typing = createTypingController(); + handleCommandsMock.mockResolvedValueOnce({ + shouldContinue: false, + reply: { text: "โš™๏ธ Compacted (76k โ†’ 934 tokens)" }, + }); + + const ctx = buildTestCtx({ + Body: "/compact", + CommandBody: "/compact", + }); + + const result = await handleInlineActions( + createHandleInlineActionsInput({ + ctx, + typing, + cleanedBody: "/compact", + command: { + isAuthorizedSender: true, + senderId: "sender-1", + senderIsOwner: true, + abortKey: "sender-1", + rawBodyNormalized: "/compact", + commandBodyNormalized: "/compact", + }, + overrides: { + cfg: { commands: { text: true } }, + allowTextCommands: true, + }, + }), + ); + + expect(result.kind).toBe("reply"); + if (result.kind !== "reply") { + throw new Error("expected reply"); + } + expect(result.reply).toEqual({ text: "โš™๏ธ Compacted (76k โ†’ 934 tokens)" }); + // Reply must carry deliverDespiteSourceReplySuppression so dispatch-from-config + // does not silently `continue` past it when sourceReplyDeliveryMode is + // "message_tool_only" (Feishu group / WebChat default). + expect( + getReplyPayloadMetadata(result.reply as object)?.deliverDespiteSourceReplySuppression, + ).toBe(true); + }); }); diff --git a/src/auto-reply/reply/get-reply-inline-actions.ts b/src/auto-reply/reply/get-reply-inline-actions.ts index b953b2730cf..c464f140965 100644 --- a/src/auto-reply/reply/get-reply-inline-actions.ts +++ b/src/auto-reply/reply/get-reply-inline-actions.ts @@ -12,6 +12,7 @@ import { normalizeOptionalLowercaseString, normalizeOptionalString, } from "../../shared/string-coerce.js"; +import { markReplyPayloadForSourceSuppressionDelivery } from "../reply-payload.js"; import { listReservedChatSlashCommandNames, resolveSkillCommandInvocation, @@ -127,6 +128,23 @@ export type InlineActionResult = cleanedBody: string; }; +// Command / skill-dispatch handlers ("/compact", "/status", tool-not-available +// errors, etc.) emit system-meta feedback for an explicit user action; they +// are not assistant source content. Mark them so dispatch-from-config does +// not silently drop them when sourceReplyDeliveryMode === "message_tool_only" +// (the default for many channels and group chats). See #87107. +function markCommandReplyForDelivery( + reply: ReplyPayload | ReplyPayload[] | undefined, +): ReplyPayload | ReplyPayload[] | undefined { + if (!reply) { + return reply; + } + if (Array.isArray(reply)) { + return reply.map((payload) => markReplyPayloadForSourceSuppressionDelivery(payload)); + } + return markReplyPayloadForSourceSuppressionDelivery(reply); +} + function extractTextFromToolResult(result: unknown): string | null { if (!result || typeof result !== "object") { return null; @@ -307,7 +325,12 @@ export async function handleInlineActions(params: { const tool = authorizedTools.find((candidate) => candidate.name === dispatch.toolName); if (!tool) { typing.cleanup(); - return { kind: "reply", reply: { text: `โŒ Tool not available: ${dispatch.toolName}` } }; + return { + kind: "reply", + reply: markCommandReplyForDelivery({ + text: `โŒ Tool not available: ${dispatch.toolName}`, + }), + }; } const toolCallId = `cmd_${generateSecureToken(8)}`; @@ -323,16 +346,19 @@ export async function handleInlineActions(params: { typing.cleanup(); return { kind: "reply", - reply: { text: `โŒ Tool call blocked: ${blockedReason}` }, + reply: markCommandReplyForDelivery({ text: `โŒ Tool call blocked: ${blockedReason}` }), }; } const text = extractTextFromToolResult(result) ?? "โœ… Done."; typing.cleanup(); - return { kind: "reply", reply: { text } }; + return { kind: "reply", reply: markCommandReplyForDelivery({ text }) }; } catch (err) { const message = formatErrorMessage(err); typing.cleanup(); - return { kind: "reply", reply: { text: `โŒ ${message}` } }; + return { + kind: "reply", + reply: markCommandReplyForDelivery({ text: `โŒ ${message}` }), + }; } } @@ -494,7 +520,7 @@ export async function handleInlineActions(params: { if (inlineResult.reply) { if (!inlineCommand.cleaned) { typing.cleanup(); - return { kind: "reply", reply: inlineResult.reply }; + return { kind: "reply", reply: markCommandReplyForDelivery(inlineResult.reply) }; } await sendInlineReply(inlineResult.reply); } @@ -558,7 +584,7 @@ export async function handleInlineActions(params: { const commandResult = await runCommands(command); if (!commandResult.shouldContinue) { typing.cleanup(); - return { kind: "reply", reply: commandResult.reply }; + return { kind: "reply", reply: markCommandReplyForDelivery(commandResult.reply) }; } if (command.commandBodyNormalized !== commandBodyBeforeRun) { cleanedBody = command.commandBodyNormalized;