refactor(auto-reply): privatize reply helpers (#106600)

This commit is contained in:
Peter Steinberger
2026-07-13 10:12:17 -07:00
committed by GitHub
parent b69e0477fe
commit edbcf11451
16 changed files with 74 additions and 175 deletions
-9
View File
@@ -1211,7 +1211,6 @@ export const KNIP_UNUSED_EXPORT_BASELINE = [
"src/auto-reply/reply/agent-runner-execution.ts: MAX_LIVE_SWITCH_RETRIES",
"src/auto-reply/reply/agent-runner-memory.ts: setAgentRunnerMemoryTestDeps",
"src/auto-reply/reply/agent-runner-session-reset.ts: setAgentRunnerSessionResetTestDeps",
"src/auto-reply/reply/block-reply-pipeline.ts: createBlockReplyPayloadKey",
"src/auto-reply/reply/commands-diagnostics.ts: createDiagnosticsCommandHandler",
"src/auto-reply/reply/commands-login.ts: testing",
"src/auto-reply/reply/commands-mcp.ts: createMcpCommandHandler",
@@ -1219,33 +1218,25 @@ export const KNIP_UNUSED_EXPORT_BASELINE = [
"src/auto-reply/reply/commands-setunset.ts: parseSetUnsetCommand",
"src/auto-reply/reply/commands-setunset.ts: parseSetUnsetCommandAction",
"src/auto-reply/reply/commands-tasks.ts: buildTasksReply",
"src/auto-reply/reply/completion-delivery-policy.ts: resolveCompletionChatType",
"src/auto-reply/reply/delivery-hints.ts: MESSAGE_TOOL_ONLY_DELIVERY_HINT",
"src/auto-reply/reply/delivery-hints.ts: ROOM_EVENT_DELIVERY_HINT",
"src/auto-reply/reply/dispatch-from-config.ts: getDispatcherFinalOutcomeCounts",
"src/auto-reply/reply/dispatch-from-config.ts: testing",
"src/auto-reply/reply/dispatcher-registry.ts: clearAllDispatchers",
"src/auto-reply/reply/get-reply-directives-apply.ts: formatModelOverrideResetEvent",
"src/auto-reply/reply/get-reply-fast-path.ts: markCompleteReplyConfig",
"src/auto-reply/reply/get-reply-fast-path.ts: withFastReplyConfig",
"src/auto-reply/reply/get-reply-run.ts: buildExecOverridePromptHint",
"src/auto-reply/reply/get-reply-run.ts: resolvePromptSessionContextForSystemEvent",
"src/auto-reply/reply/get-reply-run.ts: resolvePromptSilentReplyConversationType",
"src/auto-reply/reply/history.ts: appendHistoryEntry",
"src/auto-reply/reply/inbound-dedupe.ts: buildInboundDedupeKey",
"src/auto-reply/reply/progress-narrator.ts: createProgressNarrator",
"src/auto-reply/reply/prompt-prelude.ts: buildReplyPromptBodies",
"src/auto-reply/reply/queue/cleanup.ts: testing",
"src/auto-reply/reply/queue/drain.ts: resolveFollowupAuthorizationKey",
"src/auto-reply/reply/queue/enqueue.ts: resetRecentQueuedMessageIdDedupe",
"src/auto-reply/reply/reply-payloads-dedupe.ts: getMatchingMessagingToolReplyTargets",
"src/auto-reply/reply/reply-run-registry.ts: __testing",
"src/auto-reply/reply/reply-run-registry.ts: testing",
"src/auto-reply/reply/reply-threading.ts: createReplyToModeFilter",
"src/auto-reply/reply/reply-threading.ts: resolveConfiguredReplyToMode",
"src/auto-reply/reply/reply-threading.ts: resolveReplyToModeWithThreading",
"src/auto-reply/reply/reply-usage-state.ts: clearReplyUsageStateForTest",
"src/auto-reply/reply/session-reset-prompt.ts: buildBareSessionResetPrompt",
"src/auto-reply/reply/stage-sandbox-media.ts: appendScpStderrTail",
"src/auto-reply/reply/stage-sandbox-media.ts: SCP_STDERR_TAIL_CHARS",
"src/auto-reply/reply/stage-sandbox-media.ts: testing",
@@ -1,5 +1,5 @@
import { afterEach, describe, expect, it } from "vitest";
import { getMatchingMessagingToolReplyTargets } from "../auto-reply/reply/reply-payloads-dedupe.js";
import { resolveMessagingToolPayloadDedupe } from "../auto-reply/reply/reply-payloads-dedupe.js";
import { setActivePluginRegistry } from "../plugins/runtime.js";
import { createChannelTestPluginBase, createTestRegistry } from "../test-utils/channel-plugins.js";
import {
@@ -75,13 +75,13 @@ describe("extractMessagingToolSendResult thread evidence", () => {
expect(confirmed.threadImplicit).toBe(true);
expect(confirmed.threadId).toBe("root-1");
const matches = getMatchingMessagingToolReplyTargets({
const decision = resolveMessagingToolPayloadDedupe({
messageProvider: PARTIAL_RESULT_PROVIDER,
originatingTo: "channel:abc",
originatingThreadId: "root-1",
messagingToolSentTargets: [confirmed],
});
expect(matches).toHaveLength(1);
expect(decision.matchingRoute).toBe(true);
});
it("lets an explicit provider-reported thread override pending implicit evidence", () => {
@@ -2,11 +2,7 @@
import { MAX_TIMER_TIMEOUT_MS } from "@openclaw/normalization-core/number-coercion";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { getReplyPayloadMetadata, setReplyPayloadMetadata } from "../reply-payload.js";
import {
createBlockReplyContentKey,
createBlockReplyPayloadKey,
createBlockReplyPipeline,
} from "./block-reply-pipeline.js";
import { createBlockReplyContentKey, createBlockReplyPipeline } from "./block-reply-pipeline.js";
const waitForAbort = (signal: AbortSignal | undefined): Promise<void> =>
new Promise((resolve) => {
@@ -25,48 +21,6 @@ afterEach(() => {
vi.useRealTimers();
});
describe("createBlockReplyPayloadKey", () => {
it("produces different keys for payloads differing only by replyToId", () => {
const a = createBlockReplyPayloadKey({ text: "hello world", replyToId: "post-1" });
const b = createBlockReplyPayloadKey({ text: "hello world", replyToId: "post-2" });
const c = createBlockReplyPayloadKey({ text: "hello world" });
expect(a).not.toBe(b);
expect(a).not.toBe(c);
});
it("produces different keys for payloads with different text", () => {
const a = createBlockReplyPayloadKey({ text: "hello" });
const b = createBlockReplyPayloadKey({ text: "world" });
expect(a).not.toBe(b);
});
it("produces different keys for payloads with different media", () => {
const a = createBlockReplyPayloadKey({ text: "hello", mediaUrl: "file:///a.png" });
const b = createBlockReplyPayloadKey({ text: "hello", mediaUrl: "file:///b.png" });
expect(a).not.toBe(b);
});
it("produces different keys for payloads with different presentation content", () => {
const a = createBlockReplyPayloadKey({
presentation: {
blocks: [{ type: "buttons", buttons: [{ label: "Approve", value: "approve" }] }],
},
});
const b = createBlockReplyPayloadKey({
presentation: {
blocks: [{ type: "buttons", buttons: [{ label: "Reject", value: "reject" }] }],
},
});
expect(a).not.toBe(b);
});
it("trims whitespace from text for key comparison", () => {
const a = createBlockReplyPayloadKey({ text: " hello " });
const b = createBlockReplyPayloadKey({ text: "hello" });
expect(a).toBe(b);
});
});
describe("createBlockReplyContentKey", () => {
it("produces the same key for payloads differing only by replyToId", () => {
const a = createBlockReplyContentKey({ text: "hello world", replyToId: "post-1" });
+1 -1
View File
@@ -49,7 +49,7 @@ export function createAudioAsVoiceBuffer(params: {
}
/** Creates a stable duplicate key for a complete outbound payload. */
export function createBlockReplyPayloadKey(payload: ReplyPayload): string {
function createBlockReplyPayloadKey(payload: ReplyPayload): string {
const reply = resolveSendableOutboundReplyParts(payload);
return JSON.stringify({
statusNotice: isReplyPayloadStatusNotice(payload),
@@ -2,10 +2,16 @@
import { describe, expect, it } from "vitest";
import {
completionRequiresMessageToolDelivery,
resolveCompletionChatType,
shouldRouteCompletionThroughRequesterSession,
} from "./completion-delivery-policy.js";
const chatTypeProbeConfig = {
messages: {
visibleReplies: "message_tool",
groupChat: { visibleReplies: "automatic" },
},
} as const;
describe("completion delivery policy", () => {
it.each([
{
@@ -33,17 +39,23 @@ describe("completion delivery policy", () => {
requesterSessionKey: "agent:main:whatsapp:123@g.us",
expected: "group",
},
])("infers $name", ({ requesterSessionKey, expected }) => {
expect(resolveCompletionChatType({ requesterSessionKey })).toBe(expected);
])("applies the inferred $expected policy for $name", ({ requesterSessionKey, expected }) => {
expect(
completionRequiresMessageToolDelivery({
cfg: chatTypeProbeConfig,
requesterSessionKey,
}),
).toBe(expected === "direct");
});
it("prefers explicit session chat type over key inference", () => {
expect(
resolveCompletionChatType({
completionRequiresMessageToolDelivery({
cfg: chatTypeProbeConfig,
requesterSessionKey: "agent:main:slack:channel:C123",
requesterEntry: { chatType: "direct" },
}),
).toBe("direct");
).toBe(true);
});
it.each([
@@ -55,11 +67,12 @@ describe("completion delivery policy", () => {
{ to: "user:U123", expected: "direct" },
] as const)("falls back to origin target prefix $to", ({ to, expected }) => {
expect(
resolveCompletionChatType({
completionRequiresMessageToolDelivery({
cfg: chatTypeProbeConfig,
requesterSessionKey: "agent:main:opaque:unknown-target",
directOrigin: { channel: "test", to },
}),
).toBe(expected);
).toBe(expected === "direct");
});
it("allows automatic delivery for group and channel completions by default", () => {
@@ -12,7 +12,7 @@ type CompletionDeliverySessionEntry = {
origin?: { chatType?: string | null } | null;
};
export function resolveCompletionChatType(params: {
function resolveCompletionChatType(params: {
requesterSessionKey?: string | null;
targetRequesterSessionKey?: string | null;
requesterEntry?: CompletionDeliverySessionEntry;
@@ -45,14 +45,3 @@ export function getTotalPendingReplies(): number {
}
return total;
}
/**
* Clear all registered dispatchers (for testing).
* WARNING: Only use this in test cleanup!
*/
export function clearAllDispatchers(): void {
if (!process.env.VITEST && process.env.NODE_ENV !== "test") {
throw new Error("clearAllDispatchers() is only available in test environments");
}
activeDispatchers.clear();
}
+1 -1
View File
@@ -48,7 +48,7 @@ export function buildHistoryContext(params: {
}
/** Appends one history entry, enforces per-session limit, and refreshes LRU key order. */
export function appendHistoryEntry<T extends HistoryEntry>(params: {
function appendHistoryEntry<T extends HistoryEntry>(params: {
historyMap: Map<string, T[]>;
historyKey: string;
entry: T;
+7 -7
View File
@@ -7,7 +7,7 @@ import {
createReplyDispatcher,
waitForReplyDispatcherIdle,
} from "./reply-dispatcher.js";
import { createReplyToModeFilter } from "./reply-threading.js";
import { createReplyToModeFilterForChannel } from "./reply-threading.js";
type DeliverPayload = Parameters<Parameters<typeof createReplyDispatcher>[0]["deliver"]>[0];
type DeliverMock = { mock: { calls: unknown[][] } };
@@ -539,25 +539,25 @@ describe("waitForReplyDispatcherIdle", () => {
});
});
describe("createReplyToModeFilter", () => {
describe("createReplyToModeFilterForChannel", () => {
it("handles off/all mode behavior for replyToId", () => {
const cases: Array<{
filter: ReturnType<typeof createReplyToModeFilter>;
filter: ReturnType<typeof createReplyToModeFilterForChannel>;
input: { text: string; replyToId?: string; replyToTag?: boolean };
expectedReplyToId?: string;
}> = [
{
filter: createReplyToModeFilter("off"),
filter: createReplyToModeFilterForChannel("off"),
input: { text: "hi", replyToId: "1" },
expectedReplyToId: undefined,
},
{
filter: createReplyToModeFilter("off", { allowExplicitReplyTagsWhenOff: true }),
filter: createReplyToModeFilterForChannel("off", "slack"),
input: { text: "hi", replyToId: "1", replyToTag: true },
expectedReplyToId: "1",
},
{
filter: createReplyToModeFilter("all"),
filter: createReplyToModeFilterForChannel("all"),
input: { text: "hi", replyToId: "1" },
expectedReplyToId: "1",
},
@@ -568,7 +568,7 @@ describe("createReplyToModeFilter", () => {
});
it("keeps only the first replyToId when mode is first", () => {
const filter = createReplyToModeFilter("first");
const filter = createReplyToModeFilterForChannel("first");
expect(filter({ text: "hi", replyToId: "1" }).replyToId).toBe("1");
expect(filter({ text: "next", replyToId: "1" }).replyToId).toBeUndefined();
});
@@ -269,7 +269,7 @@ export function shouldDedupeMessagingToolRepliesForRoute(params: {
}
/** Finds message-tool sends that target the same channel/account/thread as the source reply. */
export function getMatchingMessagingToolReplyTargets(params: {
function getMatchingMessagingToolReplyTargets(params: {
config?: OpenClawConfig;
messageProvider?: string;
messagingToolSentTargets?: MessagingToolSend[];
+3 -4
View File
@@ -7,7 +7,6 @@ import { afterEach, describe, expect, it } from "vitest";
import type { SessionEntry } from "../../config/sessions.js";
import { loadSessionEntry, upsertSessionEntry } from "../../config/sessions/session-accessor.js";
import {
appendHistoryEntry,
buildHistoryContext,
buildHistoryContextFromEntries,
buildHistoryContextFromMap,
@@ -133,19 +132,19 @@ describe("history helpers", () => {
it("trims history to configured limit", () => {
const historyMap = new Map<string, { sender: string; body: string }[]>();
appendHistoryEntry({
recordPendingHistoryEntryIfEnabled({
historyMap,
historyKey: "group",
limit: 2,
entry: { sender: "A", body: "one" },
});
appendHistoryEntry({
recordPendingHistoryEntryIfEnabled({
historyMap,
historyKey: "group",
limit: 2,
entry: { sender: "B", body: "two" },
});
appendHistoryEntry({
recordPendingHistoryEntryIfEnabled({
historyMap,
historyKey: "group",
limit: 2,
+1 -55
View File
@@ -3,12 +3,7 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest";
import type { OpenClawConfig } from "../../config/config.js";
import { setActivePluginRegistry } from "../../plugins/runtime.js";
import { createTestRegistry } from "../../test-utils/channel-plugins.js";
import {
resolveConfiguredReplyToMode,
resolveReplyDeliveryAccountId,
resolveReplyToMode,
resolveReplyToModeWithThreading,
} from "./reply-threading.js";
import { resolveReplyDeliveryAccountId, resolveReplyToMode } from "./reply-threading.js";
const emptyCfg = {} as OpenClawConfig;
@@ -82,28 +77,6 @@ describe("resolveReplyToMode", () => {
}
});
it("prefers plugin threading adapters over config fallback when available", () => {
expect(
resolveReplyToModeWithThreading(
{
channels: {
slack: {
replyToMode: "off",
},
},
} as OpenClawConfig,
{
resolveReplyToMode: () => "first",
},
{
channel: "slack",
accountId: "acct-1",
chatType: "direct",
},
),
).toBe("first");
});
it("uses registered channel threading adapters for runtime reply-mode resolution", () => {
setActivePluginRegistry(
createTestRegistry([
@@ -166,30 +139,3 @@ describe("resolveReplyToMode", () => {
expect(resolveReplyDeliveryAccountId(emptyCfg, "whatsapp", "personal")).toBe("personal");
});
});
describe("resolveConfiguredReplyToMode", () => {
beforeEach(() => {
setActivePluginRegistry(createTestRegistry());
});
afterEach(() => {
setActivePluginRegistry(createTestRegistry());
});
it("handles top-level, chat-type, and legacy DM fallback without plugin registry access", () => {
const cfg = {
channels: {
slack: {
replyToMode: "off",
replyToModeByChatType: { direct: "all", group: "first" },
dm: { replyToMode: "all" },
},
},
} as OpenClawConfig;
expect(resolveConfiguredReplyToMode(cfg, "slack", "direct")).toBe("all");
expect(resolveConfiguredReplyToMode(cfg, "slack", "group")).toBe("first");
expect(resolveConfiguredReplyToMode(cfg, "slack", "channel")).toBe("off");
expect(resolveConfiguredReplyToMode(cfg, "slack", undefined)).toBe("off");
});
});
+3 -3
View File
@@ -33,7 +33,7 @@ function normalizeReplyToModeChatType(
}
/** Resolve configured reply-to mode from channel and chat-type config. */
export function resolveConfiguredReplyToMode(
function resolveConfiguredReplyToMode(
cfg: OpenClawConfig,
channel?: OriginatingChannelType,
chatType?: string | null,
@@ -62,7 +62,7 @@ export function resolveConfiguredReplyToMode(
}
/** Resolve reply-to mode using channel threading adapter override when present. */
export function resolveReplyToModeWithThreading(
function resolveReplyToModeWithThreading(
cfg: OpenClawConfig,
threading: ChannelThreadingAdapter | undefined,
params: {
@@ -149,7 +149,7 @@ export function createReplyDeliveryContext(
}
/** Create a payload filter that strips reply targets according to reply-to mode. */
export function createReplyToModeFilter(
function createReplyToModeFilter(
mode: ReplyToMode,
opts: { allowExplicitReplyTagsWhenOff?: boolean } = {},
) {
@@ -4,14 +4,23 @@ import path from "node:path";
import { describe, it, expect } from "vitest";
import type { OpenClawConfig } from "../../config/config.js";
import { makeTempWorkspace } from "../../test-helpers/workspace.js";
import {
buildBareSessionResetPrompt,
resolveBareSessionResetPromptState,
} from "./session-reset-prompt.js";
import { resolveBareSessionResetPromptState } from "./session-reset-prompt.js";
describe("buildBareSessionResetPrompt", () => {
it("includes the explicit Session Startup instruction for bare /new and /reset", () => {
const prompt = buildBareSessionResetPrompt();
type ResetPromptParams = Parameters<typeof resolveBareSessionResetPromptState>[0];
async function resolveResetPrompt(params: ResetPromptParams = {}): Promise<string> {
return (await resolveBareSessionResetPromptState(params)).prompt;
}
async function makeBootstrapPendingWorkspace(): Promise<string> {
const workspaceDir = await makeTempWorkspace("openclaw-reset-bootstrap-");
await fs.writeFile(path.join(workspaceDir, "BOOTSTRAP.md"), "ritual", "utf8");
return workspaceDir;
}
describe("resolveBareSessionResetPromptState", () => {
it("includes the explicit Session Startup instruction for bare /new and /reset", async () => {
const prompt = await resolveResetPrompt();
expect(prompt).toContain("Execute your Session Startup sequence now");
expect(prompt).toContain("read the required files before responding to the user");
expect(prompt).toContain("If BOOTSTRAP.md exists in the provided Project Context");
@@ -21,8 +30,9 @@ describe("buildBareSessionResetPrompt", () => {
);
});
it("uses bootstrap-specific wording when bootstrap is still pending", () => {
const prompt = buildBareSessionResetPrompt(undefined, undefined, "full");
it("uses bootstrap-specific wording when bootstrap is still pending", async () => {
const workspaceDir = await makeBootstrapPendingWorkspace();
const prompt = await resolveResetPrompt({ workspaceDir });
expect(prompt).toContain("while bootstrap is still pending for this workspace");
expect(prompt).toContain("Please read BOOTSTRAP.md from the workspace now");
@@ -34,8 +44,9 @@ describe("buildBareSessionResetPrompt", () => {
expect(prompt).not.toContain("Then greet the user in your configured persona");
});
it("uses limited bootstrap wording for constrained reset runs", () => {
const prompt = buildBareSessionResetPrompt(undefined, undefined, "limited");
it("uses limited bootstrap wording for constrained reset runs", async () => {
const workspaceDir = await makeBootstrapPendingWorkspace();
const prompt = await resolveResetPrompt({ workspaceDir, hasBootstrapFileAccess: false });
expect(prompt).toContain("cannot safely complete the full BOOTSTRAP.md workflow here");
expect(prompt).toContain("Never claim complete");
@@ -44,32 +55,31 @@ describe("buildBareSessionResetPrompt", () => {
expect(prompt).not.toContain("Please read BOOTSTRAP.md from the workspace now");
});
it("appends current time line so agents know the date", () => {
it("appends current time line so agents know the date", async () => {
const cfg = {
agents: { defaults: { userTimezone: "America/New_York", timeFormat: "12" } },
} as OpenClawConfig;
// 2026-03-03 14:00 UTC = 2026-03-03 09:00 EST
const nowMs = Date.UTC(2026, 2, 3, 14, 0, 0);
const prompt = buildBareSessionResetPrompt(cfg, nowMs);
const prompt = await resolveResetPrompt({ cfg, nowMs });
expect(prompt).toContain("Current time: Tuesday, March 3rd, 2026 - 9:00 AM (America/New_York)");
expect(prompt).toContain("Reference UTC: 2026-03-03 14:00 UTC");
});
it("does not append a duplicate current time line", () => {
it("does not append a duplicate current time line", async () => {
const nowMs = Date.UTC(2026, 2, 3, 14, 0, 0);
const prompt = buildBareSessionResetPrompt(undefined, nowMs);
const prompt = await resolveResetPrompt({ nowMs });
expect((prompt.match(/Current time:/g) ?? []).length).toBe(1);
});
it("falls back to UTC when no timezone configured", () => {
it("falls back to UTC when no timezone configured", async () => {
const nowMs = Date.UTC(2026, 2, 3, 14, 0, 0);
const prompt = buildBareSessionResetPrompt(undefined, nowMs);
const prompt = await resolveResetPrompt({ nowMs });
expect(prompt).toContain("Current time:");
});
it("resolves shared bare reset prompt state from workspace bootstrap truth", async () => {
const workspaceDir = await makeTempWorkspace("openclaw-reset-bootstrap-");
await fs.writeFile(path.join(workspaceDir, "BOOTSTRAP.md"), "ritual", "utf8");
const workspaceDir = await makeBootstrapPendingWorkspace();
const pending = await resolveBareSessionResetPromptState({ workspaceDir });
expect(pending.bootstrapMode).toBe("full");
+1 -1
View File
@@ -98,7 +98,7 @@ export async function resolveBareSessionResetPromptState(params: {
* know which daily memory files to read during their Session Startup sequence.
* Without this, agents on /new or /reset guess the date from their training cutoff.
*/
export function buildBareSessionResetPrompt(
function buildBareSessionResetPrompt(
cfg?: OpenClawConfig,
nowMs?: number,
bootstrapMode?: BootstrapMode,
+2 -5
View File
@@ -1,10 +1,7 @@
// Restart deferral tests protect queue-depth checks that delay gateway restart
// until in-flight reply deliveries and command work have drained.
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import {
clearAllDispatchers,
getTotalPendingReplies,
} from "../auto-reply/reply/dispatcher-registry.js";
import { getTotalPendingReplies } from "../auto-reply/reply/dispatcher-registry.js";
import { createReplyDispatcher } from "../auto-reply/reply/reply-dispatcher.js";
import { getTotalQueueSize, resetCommandQueueStateForTest } from "../process/command-queue.js";
import { createDeferred } from "../test-utils/deferred.js";
@@ -27,7 +24,7 @@ describe("gateway restart deferral", () => {
afterEach(async () => {
vi.restoreAllMocks();
await flushMicrotasks();
clearAllDispatchers();
expect(getTotalPendingReplies()).toBe(0);
resetCommandQueueStateForTest();
});