mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-03 19:04:05 +00:00
fix(auto-reply): deliver compact replies in room events
Restore visible terminal command replies for explicit command turns that are otherwise source-suppressed in room-event/message-tool-only delivery. Also keep compaction notifyUser notices independent from internal callbacks while preserving hook-message de-duplication. Fixes #87107 Verification: - git diff --check origin/main...HEAD - node scripts/run-vitest.mjs src/auto-reply/reply/dispatch-from-config.test.ts src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts src/auto-reply/reply/agent-runner-execution.test.ts - GitHub required check dependency-guard passed ond3aaad90fc- Relevant GitHub auto-reply/build/lint/type/security checks passed ond3aaad90fcCo-authored-by: openperf <16864032@qq.com>
This commit is contained in:
@@ -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 () => {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user