Skip to content

Commit 98d5c46

Browse files
obviyusobviyus
committed
fix(agents): keep compaction notices additive
1 parent 65848d0 commit 98d5c46

8 files changed

Lines changed: 190 additions & 81 deletions

‎extensions/feishu/src/bot.test.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,6 +1019,54 @@ describe("handleFeishuMessage command authorization", () => {
10191019
);
10201020
});
10211021

1022+
it("routes /compact through the standard reply dispatch path (#90185)", async () => {
1023+
mockShouldComputeCommandAuthorized.mockReturnValue(true);
1024+
1025+
const cfg: ClawdbotConfig = {
1026+
channels: {
1027+
feishu: {
1028+
dmPolicy: "open",
1029+
},
1030+
},
1031+
} as ClawdbotConfig;
1032+
1033+
await dispatchMessage({
1034+
cfg,
1035+
event: {
1036+
sender: {
1037+
sender_id: {
1038+
open_id: "ou-command-user",
1039+
},
1040+
},
1041+
message: {
1042+
message_id: "msg-compact-command",
1043+
chat_id: "oc-dm",
1044+
chat_type: "p2p",
1045+
message_type: "text",
1046+
content: JSON.stringify({ text: "/compact" }),
1047+
},
1048+
},
1049+
});
1050+
1051+
expect(mockDispatchReplyFromConfig).toHaveBeenCalledTimes(1);
1052+
const dispatchParams = mockCallArg<{
1053+
ctx: {
1054+
CommandAuthorized?: boolean;
1055+
CommandBody?: string;
1056+
BodyForCommands?: string;
1057+
RawBody?: string;
1058+
MessageSid?: string;
1059+
};
1060+
}>(mockDispatchReplyFromConfig, 0, 0);
1061+
expect(dispatchParams.ctx).toMatchObject({
1062+
CommandAuthorized: true,
1063+
CommandBody: "/compact",
1064+
BodyForCommands: "/compact",
1065+
RawBody: "/compact",
1066+
MessageSid: "msg-compact-command",
1067+
});
1068+
});
1069+
10221070
it("does not enqueue inbound preview text as system events", async () => {
10231071
mockShouldComputeCommandAuthorized.mockReturnValue(false);
10241072

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4589,7 +4589,7 @@ describe("runAgentTurnWithFallback", () => {
45894589
});
45904590
});
45914591

4592-
it("delivers compaction hook messages without duplicating notifyUser notices", async () => {
4592+
it("delivers compaction hook messages alongside notifyUser notices (#90185)", async () => {
45934593
const onBlockReply = vi.fn();
45944594
state.runEmbeddedAgentMock.mockImplementationOnce(async (params: EmbeddedAgentParams) => {
45954595
await params.onAgentEvent?.({
@@ -4639,19 +4639,31 @@ describe("runAgentTurnWithFallback", () => {
46394639
});
46404640

46414641
expect(result.kind).toBe("success");
4642-
expect(onBlockReply).toHaveBeenCalledTimes(2);
4642+
expect(onBlockReply).toHaveBeenCalledTimes(4);
46434643
expectBlockReplyCall(onBlockReply, 0, {
46444644
text: "Hook before",
46454645
replyToId: "msg",
46464646
replyToCurrent: true,
46474647
isCompactionNotice: true,
46484648
});
46494649
expectBlockReplyCall(onBlockReply, 1, {
4650+
text: "🧹 Compacting context...",
4651+
replyToId: "msg",
4652+
replyToCurrent: true,
4653+
isCompactionNotice: true,
4654+
});
4655+
expectBlockReplyCall(onBlockReply, 2, {
46504656
text: "Hook after",
46514657
replyToId: "msg",
46524658
replyToCurrent: true,
46534659
isCompactionNotice: true,
46544660
});
4661+
expectBlockReplyCall(onBlockReply, 3, {
4662+
text: "🧹 Compaction complete",
4663+
replyToId: "msg",
4664+
replyToCurrent: true,
4665+
isCompactionNotice: true,
4666+
});
46554667
});
46564668

46574669
it("fires both notifyUser notices alongside onCompactionStart / onCompactionEnd callbacks (#87107)", async () => {

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

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2550,28 +2550,24 @@ export async function runAgentTurnWithFallback(params: {
25502550
summary: readStringValue(evt.data.summary),
25512551
});
25522552
}
2553-
// Track auto-compaction and notify higher layers.
25542553
if (evt.stream === "compaction") {
25552554
const phase = readStringValue(evt.data.phase) ?? "";
25562555
const hookMessages = readCompactionHookMessages(evt.data.messages);
2556+
const sendCompactionUserNotices = async (
2557+
noticePhase: "start" | "end" | "incomplete",
2558+
) => {
2559+
if (hookMessages.length > 0) {
2560+
await sendCompactionHookMessages(hookMessages);
2561+
}
2562+
if (notifyUserAboutCompaction) {
2563+
await sendCompactionNotice(noticePhase);
2564+
}
2565+
};
25572566
if (phase === "start") {
2558-
// Three independent audiences: internal callbacks
2559-
// (Control UI) fire regardless; hookMessages deliver
2560-
// plugin-authored user-channel text (overlap with the
2561-
// default notice, so they suppress it); notifyUser is
2562-
// the opt-in user-channel notice. Internal callbacks
2563-
// must not suppress the user notice — see #87107.
25642567
if (params.opts?.onCompactionStart) {
25652568
await params.opts.onCompactionStart();
25662569
}
2567-
if (hookMessages.length > 0) {
2568-
await sendCompactionHookMessages(hookMessages);
2569-
} else if (notifyUserAboutCompaction) {
2570-
// Send directly via opts.onBlockReply (bypassing the
2571-
// pipeline) so the notice does not cause final payloads
2572-
// to be discarded on non-streaming model paths.
2573-
await sendCompactionNotice("start");
2574-
}
2570+
await sendCompactionUserNotices("start");
25752571
}
25762572
if (phase === "end") {
25772573
const completed = evt.data?.completed === true;
@@ -2580,15 +2576,9 @@ export async function runAgentTurnWithFallback(params: {
25802576
if (params.opts?.onCompactionEnd) {
25812577
await params.opts.onCompactionEnd();
25822578
}
2583-
if (hookMessages.length > 0) {
2584-
await sendCompactionHookMessages(hookMessages);
2585-
} else if (notifyUserAboutCompaction) {
2586-
await sendCompactionNotice("end");
2587-
}
2588-
} else if (hookMessages.length > 0) {
2589-
await sendCompactionHookMessages(hookMessages);
2590-
} else if (notifyUserAboutCompaction) {
2591-
await sendCompactionNotice("incomplete");
2579+
await sendCompactionUserNotices("end");
2580+
} else {
2581+
await sendCompactionUserNotices("incomplete");
25922582
}
25932583
}
25942584
}

‎src/auto-reply/reply/followup-runner.test.ts‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3565,6 +3565,76 @@ describe("createFollowupRunner messaging delivery and dedupe", () => {
35653565
});
35663566
});
35673567

3568+
it("routes queued compaction hook messages alongside notifyUser notices (#90185)", async () => {
3569+
runEmbeddedAgentMock.mockImplementationOnce(
3570+
async (args: {
3571+
onAgentEvent?: (evt: { stream: string; data: Record<string, unknown> }) => Promise<void>;
3572+
}) => {
3573+
await args.onAgentEvent?.({
3574+
stream: "compaction",
3575+
data: { phase: "start", messages: ["Hook before"] },
3576+
});
3577+
await args.onAgentEvent?.({
3578+
stream: "compaction",
3579+
data: { phase: "end", completed: true, messages: ["Hook after"] },
3580+
});
3581+
return { payloads: [], meta: {} };
3582+
},
3583+
);
3584+
const runner = createFollowupRunner({
3585+
typing: createMockTypingController(),
3586+
typingMode: "instant",
3587+
defaultModel: "openai/gpt-5.5",
3588+
});
3589+
3590+
await runner(
3591+
createQueuedRun({
3592+
originatingChannel: "discord",
3593+
originatingTo: "channel:C1",
3594+
messageId: "current-msg-1",
3595+
run: {
3596+
config: {
3597+
channels: { discord: { replyToMode: "all" } },
3598+
agents: { defaults: { compaction: { notifyUser: true } } },
3599+
},
3600+
messageProvider: "discord",
3601+
},
3602+
}),
3603+
);
3604+
3605+
expect(routeReplyMock).toHaveBeenCalledTimes(4);
3606+
expect(
3607+
requireRecord(requireMockCallArg(routeReplyMock, 0).payload, "hook start"),
3608+
).toMatchObject({
3609+
text: "Hook before",
3610+
replyToId: "current-msg-1",
3611+
replyToCurrent: true,
3612+
isCompactionNotice: true,
3613+
});
3614+
expect(
3615+
requireRecord(requireMockCallArg(routeReplyMock, 1).payload, "notice start"),
3616+
).toMatchObject({
3617+
text: "🧹 Compacting context...",
3618+
replyToId: "current-msg-1",
3619+
replyToCurrent: true,
3620+
isCompactionNotice: true,
3621+
});
3622+
expect(requireRecord(requireMockCallArg(routeReplyMock, 2).payload, "hook end")).toMatchObject({
3623+
text: "Hook after",
3624+
replyToId: "current-msg-1",
3625+
replyToCurrent: true,
3626+
isCompactionNotice: true,
3627+
});
3628+
expect(
3629+
requireRecord(requireMockCallArg(routeReplyMock, 3).payload, "notice end"),
3630+
).toMatchObject({
3631+
text: "🧹 Compaction complete",
3632+
replyToId: "current-msg-1",
3633+
replyToCurrent: true,
3634+
isCompactionNotice: true,
3635+
});
3636+
});
3637+
35683638
it("applies reply-to mode filtering to queued compaction notices", async () => {
35693639
runPreflightCompactionIfNeededMock.mockImplementationOnce(
35703640
async (params: {

‎src/auto-reply/reply/followup-runner.ts‎

Lines changed: 12 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -231,24 +231,28 @@ async function forwardFollowupProgressEvent(params: {
231231
if (evt.stream === "compaction") {
232232
const phase = readStringValue(evt.data.phase) ?? "";
233233
const hookMessages = readCompactionHookMessages(evt.data.messages);
234-
if (phase === "start" && emitChannelProgress) {
235-
await opts?.onCompactionStart?.();
236-
}
237-
if (phase === "start") {
234+
const sendCompactionUserNotices = async (noticePhase: "start" | "end" | "incomplete") => {
238235
const hookPayload = createCompactionHookNoticePayload({
239236
messages: hookMessages,
240237
currentMessageId: params.currentMessageId,
241238
});
242239
if (hookPayload) {
243240
await params.onCompactionNoticePayload?.(hookPayload);
244-
} else if (params.notifyUserAboutCompaction === true) {
241+
}
242+
if (params.notifyUserAboutCompaction === true) {
245243
await params.onCompactionNoticePayload?.(
246244
createCompactionNoticePayload({
247-
phase: "start",
245+
phase: noticePhase,
248246
currentMessageId: params.currentMessageId,
249247
}),
250248
);
251249
}
250+
};
251+
if (phase === "start" && emitChannelProgress) {
252+
await opts?.onCompactionStart?.();
253+
}
254+
if (phase === "start") {
255+
await sendCompactionUserNotices("start");
252256
}
253257
if (phase === "end" && evt.data?.completed === true) {
254258
params.onCompactionComplete?.();
@@ -258,35 +262,9 @@ async function forwardFollowupProgressEvent(params: {
258262
if (evt.data?.willRetry === true) {
259263
return;
260264
}
261-
const hookPayload = createCompactionHookNoticePayload({
262-
messages: hookMessages,
263-
currentMessageId: params.currentMessageId,
264-
});
265-
if (hookPayload) {
266-
await params.onCompactionNoticePayload?.(hookPayload);
267-
} else if (params.notifyUserAboutCompaction === true) {
268-
await params.onCompactionNoticePayload?.(
269-
createCompactionNoticePayload({
270-
phase: "end",
271-
currentMessageId: params.currentMessageId,
272-
}),
273-
);
274-
}
265+
await sendCompactionUserNotices("end");
275266
} else if (phase === "end") {
276-
const hookPayload = createCompactionHookNoticePayload({
277-
messages: hookMessages,
278-
currentMessageId: params.currentMessageId,
279-
});
280-
if (hookPayload) {
281-
await params.onCompactionNoticePayload?.(hookPayload);
282-
} else if (params.notifyUserAboutCompaction === true) {
283-
await params.onCompactionNoticePayload?.(
284-
createCompactionNoticePayload({
285-
phase: "incomplete",
286-
currentMessageId: params.currentMessageId,
287-
}),
288-
);
289-
}
267+
await sendCompactionUserNotices("incomplete");
290268
}
291269
}
292270
}

‎src/auto-reply/reply/get-reply-native-slash-fast-path.test.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ describe("maybeResolveNativeSlashCommandFastReply", () => {
3232
handleCommandsMock.mockReset();
3333
});
3434

35-
it("marks native /compact terminal replies for delivery under message_tool_only (#87107)", async () => {
35+
it("marks native /compact terminal replies for delivery under message_tool_only (#90185)", async () => {
3636
handleCommandsMock.mockResolvedValueOnce({
3737
shouldContinue: false,
3838
reply: { text: "⚙️ Compaction skipped: no real conversation messages yet • Context 12.1k" },
@@ -50,6 +50,7 @@ describe("maybeResolveNativeSlashCommandFastReply", () => {
5050
kind: "native",
5151
source: "native",
5252
authorized: true,
53+
commandName: "compact",
5354
body: "/compact",
5455
},
5556
});
@@ -82,9 +83,10 @@ describe("maybeResolveNativeSlashCommandFastReply", () => {
8283
if (!result.handled) {
8384
throw new Error("expected handled");
8485
}
85-
expect(
86-
getReplyPayloadMetadata(result.reply as object)?.deliverDespiteSourceReplySuppression,
87-
).toBe(true);
86+
if (!result.reply || Array.isArray(result.reply)) {
87+
throw new Error("expected single reply payload");
88+
}
89+
expect(getReplyPayloadMetadata(result.reply)?.deliverDespiteSourceReplySuppression).toBe(true);
8890
expect(typing.cleanup).toHaveBeenCalledTimes(1);
8991
});
9092
});

‎src/auto-reply/reply/get-reply-native-slash-fast-path.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,10 @@ export async function maybeResolveNativeSlashCommandFastReply(params: {
296296
skillFilter: params.skillFilter,
297297
});
298298
if (inlineActionResult.kind === "reply") {
299-
return { handled: true, reply: inlineActionResult.reply };
299+
return {
300+
handled: true,
301+
reply: markCommandReplyForDelivery(inlineActionResult.reply),
302+
};
300303
}
301304
return { handled: false };
302305
}

0 commit comments

Comments
 (0)