Skip to content

Commit ae53d02

Browse files
openperfopenperf
committed
fix(auto-reply): narrow room_event source-suppression bypass for explicit command turns; deliver /compact + notifyUser independently of internal callbacks
Three distinct silent-delivery failures of /compact and compaction notices on 2026.5.22 (#87107): 1. dispatch-from-config previously rejected the deliverDespiteSourceReplySuppression marker for every room_event, so a marked authorized /compact reply (Feishu group / WebChat room-event) was silently dropped. Narrow the bypass to honor the marker when the current ctx represents an explicit command turn (isExplicitCommandTurn: native or authorized text-slash); ambient marked runtime failure notices and sendPolicy: deny stay suppressed. 2. handleInlineActions terminal replies (/compact, /status, skill tool blocked / error replies) did not carry the marker, so the same suppress branch silently dropped them under messages.visibleReplies: "message_tool" configs. Wrap every { kind: "reply", reply } exit in a local helper that calls markReplyPayloadForSourceSuppressionDelivery. 3. The compaction-event handler in runAgentTurnWithFallback gated notifyUser on the absence of onCompactionStart / onCompactionEnd callbacks, conflating internal Control-UI callbacks with the opt-in user-channel notice. Drop the callback predicates from the gate so notifyUser fires whenever configured; keep hookMessages overlap suppression (same user-channel audience). Refs #87107
1 parent 7a381b8 commit ae53d02

6 files changed

Lines changed: 179 additions & 23 deletions

‎src/auto-reply/reply/agent-runner-execution.test.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4125,8 +4125,9 @@ describe("runAgentTurnWithFallback", () => {
41254125
});
41264126
});
41274127

4128-
it("prefers onCompactionEnd callback over default notice when notifyUser is enabled", async () => {
4128+
it("fires both notifyUser notices alongside onCompactionStart / onCompactionEnd callbacks (#87107)", async () => {
41294129
const onBlockReply = vi.fn();
4130+
const onCompactionStart = vi.fn();
41304131
const onCompactionEnd = vi.fn();
41314132
state.runEmbeddedAgentMock.mockImplementationOnce(async (params: EmbeddedAgentParams) => {
41324133
await params.onAgentEvent?.({ stream: "compaction", data: { phase: "start" } });
@@ -4156,7 +4157,7 @@ describe("runAgentTurnWithFallback", () => {
41564157
Provider: "whatsapp",
41574158
MessageSid: "msg",
41584159
} as unknown as TemplateContext,
4159-
opts: { onBlockReply, onCompactionEnd },
4160+
opts: { onBlockReply, onCompactionStart, onCompactionEnd },
41604161
typingSignals: createMockTypingSignaler(),
41614162
blockReplyPipeline: null,
41624163
blockStreamingEnabled: false,
@@ -4173,14 +4174,19 @@ describe("runAgentTurnWithFallback", () => {
41734174
});
41744175

41754176
expect(result.kind).toBe("success");
4177+
// Internal callbacks (Control UI etc.) and the user-channel notifyUser
4178+
// notices are independent audiences; both must fire when opted in.
4179+
expect(onCompactionStart).toHaveBeenCalledTimes(1);
41764180
expect(onCompactionEnd).toHaveBeenCalledTimes(1);
4177-
// The start notice still fires (no onCompactionStart callback provided),
4178-
// but the completion notice is suppressed in favor of the callback.
4179-
expect(onBlockReply).toHaveBeenCalledTimes(1);
4181+
expect(onBlockReply).toHaveBeenCalledTimes(2);
41804182
expectBlockReplyCall(onBlockReply, 0, {
41814183
text: "🧹 Compacting context...",
41824184
isCompactionNotice: true,
41834185
});
4186+
expectBlockReplyCall(onBlockReply, 1, {
4187+
text: "🧹 Compaction complete",
4188+
isCompactionNotice: true,
4189+
});
41844190
});
41854191

41864192
it("emits an incomplete compaction notice when compaction ends without completing", async () => {

‎src/auto-reply/reply/agent-runner-execution.ts‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2426,17 +2426,18 @@ export async function runAgentTurnWithFallback(params: {
24262426
const phase = readStringValue(evt.data.phase) ?? "";
24272427
const hookMessages = readCompactionHookMessages(evt.data.messages);
24282428
if (phase === "start") {
2429-
// Keep custom compaction callbacks active, but gate the
2430-
// fallback user-facing notice behind explicit opt-in.
2429+
// Three independent audiences: internal callbacks
2430+
// (Control UI) fire regardless; hookMessages deliver
2431+
// plugin-authored user-channel text (overlap with the
2432+
// default notice, so they suppress it); notifyUser is
2433+
// the opt-in user-channel notice. Internal callbacks
2434+
// must not suppress the user notice — see #87107.
24312435
if (params.opts?.onCompactionStart) {
24322436
await params.opts.onCompactionStart();
24332437
}
24342438
if (hookMessages.length > 0) {
24352439
await sendCompactionHookMessages(hookMessages);
2436-
} else if (
2437-
!params.opts?.onCompactionStart &&
2438-
shouldNotifyUserAboutCompaction
2439-
) {
2440+
} else if (shouldNotifyUserAboutCompaction) {
24402441
// Send directly via opts.onBlockReply (bypassing the
24412442
// pipeline) so the notice does not cause final payloads
24422443
// to be discarded on non-streaming model paths.
@@ -2452,10 +2453,7 @@ export async function runAgentTurnWithFallback(params: {
24522453
}
24532454
if (hookMessages.length > 0) {
24542455
await sendCompactionHookMessages(hookMessages);
2455-
} else if (
2456-
!params.opts?.onCompactionEnd &&
2457-
shouldNotifyUserAboutCompaction
2458-
) {
2456+
} else if (shouldNotifyUserAboutCompaction) {
24592457
await sendCompactionNotice("end");
24602458
}
24612459
} else if (hookMessages.length > 0) {

‎src/auto-reply/reply/dispatch-from-config.test.ts‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7547,6 +7547,79 @@ describe("sendPolicy deny — suppress delivery, not processing (#53328)", () =>
75477547
expect(dispatcher.sendToolResult).not.toHaveBeenCalled();
75487548
});
75497549

7550+
it("delivers marked explicit command terminal replies in room events (#87107)", async () => {
7551+
setNoAbort();
7552+
sessionStoreMocks.currentEntry = {
7553+
sessionId: "s1",
7554+
updatedAt: 0,
7555+
sendPolicy: "allow",
7556+
};
7557+
const dispatcher = createDispatcher();
7558+
const commandReply = setReplyPayloadMetadata(
7559+
{ text: "⚙️ Compacted (76k → 934 tokens)" },
7560+
{ deliverDespiteSourceReplySuppression: true },
7561+
);
7562+
const replyResolver = vi.fn(async () => commandReply satisfies ReplyPayload);
7563+
const ctx = buildTestCtx({
7564+
ChatType: "group",
7565+
InboundEventKind: "room_event",
7566+
SessionKey: "test:session",
7567+
CommandSource: "text",
7568+
CommandAuthorized: true,
7569+
CommandBody: "/compact",
7570+
});
7571+
7572+
const result = await dispatchReplyFromConfig({
7573+
ctx,
7574+
cfg: emptyConfig,
7575+
dispatcher,
7576+
replyResolver,
7577+
replyOptions: {
7578+
sourceReplyDeliveryMode: "message_tool_only",
7579+
},
7580+
});
7581+
7582+
expect(replyResolver).toHaveBeenCalledTimes(1);
7583+
expect(result.queuedFinal).toBe(true);
7584+
expect(dispatcher.sendFinalReply).toHaveBeenCalledWith(commandReply);
7585+
});
7586+
7587+
it("delivers marked /compact reply in room event when CommandSource is undefined (#87107)", async () => {
7588+
setNoAbort();
7589+
sessionStoreMocks.currentEntry = {
7590+
sessionId: "s1",
7591+
updatedAt: 0,
7592+
sendPolicy: "allow",
7593+
};
7594+
const dispatcher = createDispatcher();
7595+
const commandReply = setReplyPayloadMetadata(
7596+
{ text: "⚙️ Compacted (76k → 934 tokens)" },
7597+
{ deliverDespiteSourceReplySuppression: true },
7598+
);
7599+
const replyResolver = vi.fn(async () => commandReply satisfies ReplyPayload);
7600+
const ctx = buildTestCtx({
7601+
ChatType: "group",
7602+
InboundEventKind: "room_event",
7603+
SessionKey: "test:session",
7604+
CommandAuthorized: true,
7605+
CommandBody: "/compact",
7606+
});
7607+
7608+
const result = await dispatchReplyFromConfig({
7609+
ctx,
7610+
cfg: emptyConfig,
7611+
dispatcher,
7612+
replyResolver,
7613+
replyOptions: {
7614+
sourceReplyDeliveryMode: "message_tool_only",
7615+
},
7616+
});
7617+
7618+
expect(replyResolver).toHaveBeenCalledTimes(1);
7619+
expect(result.queuedFinal).toBe(true);
7620+
expect(dispatcher.sendFinalReply).toHaveBeenCalledWith(commandReply);
7621+
});
7622+
75507623
it("mirrors internal source reply payloads into the active transcript", async () => {
75517624
setNoAbort();
75527625
sessionStoreMocks.currentEntry = {

‎src/auto-reply/reply/dispatch-from-config.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2702,11 +2702,18 @@ export async function dispatchReplyFromConfig(
27022702
let routedFinalCount = 0;
27032703
let attemptedFinalDelivery = false;
27042704
let finalDeliveryFailed = false;
2705+
// Explicit command turns (native or authorized text-slash like /compact) are
2706+
// user-initiated, so a marked terminal reply for the command bypasses
2707+
// room_event suppression. Ambient marked notices (no CommandTurn) stay
2708+
// suppressed in room_event. sendPolicy: deny still suppresses everything.
2709+
// Uses the same helper as the source-reply visibility policy so the bypass
2710+
// and the policy stay aligned.
2711+
const explicitCommandTurnCtx = isExplicitSourceReplyCommand(ctx, cfg);
27052712
const shouldDeliverDespiteSourceReplySuppression = (reply: ReplyPayload) =>
27062713
suppressAutomaticSourceDelivery &&
2707-
ctx.InboundEventKind !== "room_event" &&
27082714
!sendPolicyDenied &&
2709-
getReplyPayloadMetadata(reply)?.deliverDespiteSourceReplySuppression === true;
2715+
getReplyPayloadMetadata(reply)?.deliverDespiteSourceReplySuppression === true &&
2716+
(ctx.InboundEventKind !== "room_event" || explicitCommandTurnCtx);
27102717
for (const reply of replies) {
27112718
throwIfDispatchOperationAborted();
27122719
// Suppress reasoning payloads from channel delivery — channels using this

‎src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import path from "node:path";
44
import { beforeEach, describe, expect, it, vi } from "vitest";
55
import type { SkillCommandSpec } from "../../agents/skills.js";
66
import type { SessionEntry } from "../../config/sessions.js";
7+
import { getReplyPayloadMetadata } from "../reply-payload.js";
78
import type { TemplateContext } from "../templating.js";
89
import { clearInlineDirectives } from "./get-reply-directives-utils.js";
910
import { handleInlineActions } from "./get-reply-inline-actions.js";
@@ -1131,4 +1132,49 @@ describe("handleInlineActions", () => {
11311132
);
11321133
expect(toolExecute).toHaveBeenCalled();
11331134
});
1135+
1136+
it("marks command-handler terminal replies with deliverDespiteSourceReplySuppression so they are not dropped under message_tool_only delivery (#87107)", async () => {
1137+
const typing = createTypingController();
1138+
handleCommandsMock.mockResolvedValueOnce({
1139+
shouldContinue: false,
1140+
reply: { text: "⚙️ Compacted (76k → 934 tokens)" },
1141+
});
1142+
1143+
const ctx = buildTestCtx({
1144+
Body: "/compact",
1145+
CommandBody: "/compact",
1146+
});
1147+
1148+
const result = await handleInlineActions(
1149+
createHandleInlineActionsInput({
1150+
ctx,
1151+
typing,
1152+
cleanedBody: "/compact",
1153+
command: {
1154+
isAuthorizedSender: true,
1155+
senderId: "sender-1",
1156+
senderIsOwner: true,
1157+
abortKey: "sender-1",
1158+
rawBodyNormalized: "/compact",
1159+
commandBodyNormalized: "/compact",
1160+
},
1161+
overrides: {
1162+
cfg: { commands: { text: true } },
1163+
allowTextCommands: true,
1164+
},
1165+
}),
1166+
);
1167+
1168+
expect(result.kind).toBe("reply");
1169+
if (result.kind !== "reply") {
1170+
throw new Error("expected reply");
1171+
}
1172+
expect(result.reply).toEqual({ text: "⚙️ Compacted (76k → 934 tokens)" });
1173+
// Reply must carry deliverDespiteSourceReplySuppression so dispatch-from-config
1174+
// does not silently `continue` past it when sourceReplyDeliveryMode is
1175+
// "message_tool_only" (Feishu group / WebChat default).
1176+
expect(
1177+
getReplyPayloadMetadata(result.reply as object)?.deliverDespiteSourceReplySuppression,
1178+
).toBe(true);
1179+
});
11341180
});

‎src/auto-reply/reply/get-reply-inline-actions.ts‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
normalizeOptionalLowercaseString,
1313
normalizeOptionalString,
1414
} from "../../shared/string-coerce.js";
15+
import { markReplyPayloadForSourceSuppressionDelivery } from "../reply-payload.js";
1516
import {
1617
listReservedChatSlashCommandNames,
1718
resolveSkillCommandInvocation,
@@ -127,6 +128,23 @@ export type InlineActionResult =
127128
cleanedBody: string;
128129
};
129130

131+
// Command / skill-dispatch handlers ("/compact", "/status", tool-not-available
132+
// errors, etc.) emit system-meta feedback for an explicit user action; they
133+
// are not assistant source content. Mark them so dispatch-from-config does
134+
// not silently drop them when sourceReplyDeliveryMode === "message_tool_only"
135+
// (the default for many channels and group chats). See #87107.
136+
function markCommandReplyForDelivery(
137+
reply: ReplyPayload | ReplyPayload[] | undefined,
138+
): ReplyPayload | ReplyPayload[] | undefined {
139+
if (!reply) {
140+
return reply;
141+
}
142+
if (Array.isArray(reply)) {
143+
return reply.map((payload) => markReplyPayloadForSourceSuppressionDelivery(payload));
144+
}
145+
return markReplyPayloadForSourceSuppressionDelivery(reply);
146+
}
147+
130148
function extractTextFromToolResult(result: unknown): string | null {
131149
if (!result || typeof result !== "object") {
132150
return null;
@@ -307,7 +325,12 @@ export async function handleInlineActions(params: {
307325
const tool = authorizedTools.find((candidate) => candidate.name === dispatch.toolName);
308326
if (!tool) {
309327
typing.cleanup();
310-
return { kind: "reply", reply: { text: `❌ Tool not available: ${dispatch.toolName}` } };
328+
return {
329+
kind: "reply",
330+
reply: markCommandReplyForDelivery({
331+
text: `❌ Tool not available: ${dispatch.toolName}`,
332+
}),
333+
};
311334
}
312335

313336
const toolCallId = `cmd_${generateSecureToken(8)}`;
@@ -323,16 +346,19 @@ export async function handleInlineActions(params: {
323346
typing.cleanup();
324347
return {
325348
kind: "reply",
326-
reply: { text: `❌ Tool call blocked: ${blockedReason}` },
349+
reply: markCommandReplyForDelivery({ text: `❌ Tool call blocked: ${blockedReason}` }),
327350
};
328351
}
329352
const text = extractTextFromToolResult(result) ?? "✅ Done.";
330353
typing.cleanup();
331-
return { kind: "reply", reply: { text } };
354+
return { kind: "reply", reply: markCommandReplyForDelivery({ text }) };
332355
} catch (err) {
333356
const message = formatErrorMessage(err);
334357
typing.cleanup();
335-
return { kind: "reply", reply: { text: `❌ ${message}` } };
358+
return {
359+
kind: "reply",
360+
reply: markCommandReplyForDelivery({ text: `❌ ${message}` }),
361+
};
336362
}
337363
}
338364

@@ -494,7 +520,7 @@ export async function handleInlineActions(params: {
494520
if (inlineResult.reply) {
495521
if (!inlineCommand.cleaned) {
496522
typing.cleanup();
497-
return { kind: "reply", reply: inlineResult.reply };
523+
return { kind: "reply", reply: markCommandReplyForDelivery(inlineResult.reply) };
498524
}
499525
await sendInlineReply(inlineResult.reply);
500526
}
@@ -558,7 +584,7 @@ export async function handleInlineActions(params: {
558584
const commandResult = await runCommands(command);
559585
if (!commandResult.shouldContinue) {
560586
typing.cleanup();
561-
return { kind: "reply", reply: commandResult.reply };
587+
return { kind: "reply", reply: markCommandReplyForDelivery(commandResult.reply) };
562588
}
563589
if (command.commandBodyNormalized !== commandBodyBeforeRun) {
564590
cleanedBody = command.commandBodyNormalized;

0 commit comments

Comments
 (0)