mirror of
https://github.com/openclaw/openclaw.git
synced 2026-02-19 18:39:20 -05:00
perf(auto-reply): avoid skill scans for inline directives
This commit is contained in:
@@ -169,27 +169,34 @@ export async function resolveReplyDirectives(params: {
|
||||
surface: command.surface,
|
||||
commandSource: ctx.CommandSource,
|
||||
});
|
||||
const shouldResolveSkillCommands =
|
||||
allowTextCommands && command.commandBodyNormalized.includes("/");
|
||||
const skillCommands = shouldResolveSkillCommands
|
||||
? listSkillCommandsForWorkspace({
|
||||
workspaceDir,
|
||||
cfg,
|
||||
skillFilter,
|
||||
})
|
||||
: [];
|
||||
const reservedCommands = new Set(
|
||||
listChatCommands().flatMap((cmd) =>
|
||||
cmd.textAliases.map((a) => a.replace(/^\//, "").toLowerCase()),
|
||||
),
|
||||
);
|
||||
for (const command of skillCommands) {
|
||||
reservedCommands.add(command.name.toLowerCase());
|
||||
}
|
||||
const configuredAliases = Object.values(cfg.agents?.defaults?.models ?? {})
|
||||
|
||||
const rawAliases = Object.values(cfg.agents?.defaults?.models ?? {})
|
||||
.map((entry) => entry.alias?.trim())
|
||||
.filter((alias): alias is string => Boolean(alias))
|
||||
.filter((alias) => !reservedCommands.has(alias.toLowerCase()));
|
||||
|
||||
// Only load workspace skill commands when we actually need them to filter aliases.
|
||||
// This avoids scanning skills for messages that only use inline directives like /think:/verbose:.
|
||||
const skillCommands =
|
||||
allowTextCommands && rawAliases.length > 0
|
||||
? listSkillCommandsForWorkspace({
|
||||
workspaceDir,
|
||||
cfg,
|
||||
skillFilter,
|
||||
})
|
||||
: [];
|
||||
for (const command of skillCommands) {
|
||||
reservedCommands.add(command.name.toLowerCase());
|
||||
}
|
||||
|
||||
const configuredAliases = rawAliases.filter(
|
||||
(alias) => !reservedCommands.has(alias.toLowerCase()),
|
||||
);
|
||||
const allowStatusDirective = allowTextCommands && command.isAuthorizedSender;
|
||||
let parsedDirectives = parseInlineDirectives(commandText, {
|
||||
modelAliases: configuredAliases,
|
||||
|
||||
@@ -11,12 +11,52 @@ import { createOpenClawTools } from "../../agents/openclaw-tools.js";
|
||||
import { getChannelDock } from "../../channels/dock.js";
|
||||
import { logVerbose } from "../../globals.js";
|
||||
import { resolveGatewayMessageChannel } from "../../utils/message-channel.js";
|
||||
import { listChatCommands } from "../commands-registry.js";
|
||||
import { listSkillCommandsForWorkspace, resolveSkillCommandInvocation } from "../skill-commands.js";
|
||||
import { getAbortMemory } from "./abort.js";
|
||||
import { buildStatusReply, handleCommands } from "./commands.js";
|
||||
import { isDirectiveOnly } from "./directive-handling.js";
|
||||
import { extractInlineSimpleCommand } from "./reply-inline.js";
|
||||
|
||||
const builtinSlashCommands = (() => {
|
||||
const reserved = new Set<string>();
|
||||
for (const command of listChatCommands()) {
|
||||
if (command.nativeName) {
|
||||
reserved.add(command.nativeName.toLowerCase());
|
||||
}
|
||||
for (const alias of command.textAliases) {
|
||||
const trimmed = alias.trim();
|
||||
if (!trimmed.startsWith("/")) {
|
||||
continue;
|
||||
}
|
||||
reserved.add(trimmed.slice(1).toLowerCase());
|
||||
}
|
||||
}
|
||||
for (const name of [
|
||||
"think",
|
||||
"verbose",
|
||||
"reasoning",
|
||||
"elevated",
|
||||
"exec",
|
||||
"model",
|
||||
"status",
|
||||
"queue",
|
||||
]) {
|
||||
reserved.add(name);
|
||||
}
|
||||
return reserved;
|
||||
})();
|
||||
|
||||
function resolveSlashCommandName(commandBodyNormalized: string): string | null {
|
||||
const trimmed = commandBodyNormalized.trim();
|
||||
if (!trimmed.startsWith("/")) {
|
||||
return null;
|
||||
}
|
||||
const match = trimmed.match(/^\/([^\s:]+)(?::|\s|$)/);
|
||||
const name = match?.[1]?.trim().toLowerCase() ?? "";
|
||||
return name ? name : null;
|
||||
}
|
||||
|
||||
export type InlineActionResult =
|
||||
| { kind: "reply"; reply: ReplyPayload | ReplyPayload[] | undefined }
|
||||
| {
|
||||
@@ -135,7 +175,12 @@ export async function handleInlineActions(params: {
|
||||
let directives = initialDirectives;
|
||||
let cleanedBody = initialCleanedBody;
|
||||
|
||||
const shouldLoadSkillCommands = command.commandBodyNormalized.startsWith("/");
|
||||
const slashCommandName = resolveSlashCommandName(command.commandBodyNormalized);
|
||||
const shouldLoadSkillCommands =
|
||||
allowTextCommands &&
|
||||
slashCommandName !== null &&
|
||||
// `/skill …` needs the full skill command list.
|
||||
(slashCommandName === "skill" || !builtinSlashCommands.has(slashCommandName));
|
||||
const skillCommands =
|
||||
shouldLoadSkillCommands && params.skillCommands
|
||||
? params.skillCommands
|
||||
|
||||
@@ -1,36 +1,29 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import type { OpenClawConfig } from "../../config/config.js";
|
||||
import type { FollowupRun, QueueSettings } from "./queue.js";
|
||||
import { defaultRuntime } from "../../runtime.js";
|
||||
import { enqueueFollowupRun, scheduleFollowupDrain } from "./queue.js";
|
||||
|
||||
const COLLECT_SETTINGS: QueueSettings = {
|
||||
mode: "collect",
|
||||
debounceMs: 0,
|
||||
cap: 50,
|
||||
dropPolicy: "summarize",
|
||||
};
|
||||
|
||||
function createSettings(overrides?: Partial<QueueSettings>): QueueSettings {
|
||||
return { ...COLLECT_SETTINGS, ...overrides } as QueueSettings;
|
||||
function createDeferred<T>() {
|
||||
let resolve!: (value: T) => void;
|
||||
let reject!: (reason?: unknown) => void;
|
||||
const promise = new Promise<T>((res, rej) => {
|
||||
resolve = res;
|
||||
reject = rej;
|
||||
});
|
||||
return { promise, resolve, reject };
|
||||
}
|
||||
|
||||
function createRunCollector() {
|
||||
const calls: FollowupRun[] = [];
|
||||
const runFollowup = async (run: FollowupRun) => {
|
||||
calls.push(run);
|
||||
};
|
||||
return { calls, runFollowup };
|
||||
}
|
||||
let previousRuntimeError: typeof defaultRuntime.error;
|
||||
|
||||
async function drainAndWait(params: {
|
||||
key: string;
|
||||
calls: FollowupRun[];
|
||||
runFollowup: (run: FollowupRun) => Promise<void>;
|
||||
count: number;
|
||||
}) {
|
||||
scheduleFollowupDrain(params.key, params.runFollowup);
|
||||
await expect.poll(() => params.calls.length).toBe(params.count);
|
||||
}
|
||||
beforeAll(() => {
|
||||
previousRuntimeError = defaultRuntime.error;
|
||||
defaultRuntime.error = undefined;
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
defaultRuntime.error = previousRuntimeError;
|
||||
});
|
||||
|
||||
function createRun(params: {
|
||||
prompt: string;
|
||||
@@ -66,8 +59,21 @@ function createRun(params: {
|
||||
describe("followup queue deduplication", () => {
|
||||
it("deduplicates messages with same Discord message_id", async () => {
|
||||
const key = `test-dedup-message-id-${Date.now()}`;
|
||||
const { calls, runFollowup } = createRunCollector();
|
||||
const settings = createSettings();
|
||||
const calls: FollowupRun[] = [];
|
||||
const done = createDeferred<void>();
|
||||
const expectedCalls = 1;
|
||||
const runFollowup = async (run: FollowupRun) => {
|
||||
calls.push(run);
|
||||
if (calls.length >= expectedCalls) {
|
||||
done.resolve();
|
||||
}
|
||||
};
|
||||
const settings: QueueSettings = {
|
||||
mode: "collect",
|
||||
debounceMs: 0,
|
||||
cap: 50,
|
||||
dropPolicy: "summarize",
|
||||
};
|
||||
|
||||
// First enqueue should succeed
|
||||
const first = enqueueFollowupRun(
|
||||
@@ -108,14 +114,20 @@ describe("followup queue deduplication", () => {
|
||||
);
|
||||
expect(third).toBe(true);
|
||||
|
||||
await drainAndWait({ key, calls, runFollowup, count: 1 });
|
||||
scheduleFollowupDrain(key, runFollowup);
|
||||
await done.promise;
|
||||
// Should collect both unique messages
|
||||
expect(calls[0]?.prompt).toContain("[Queued messages while agent was busy]");
|
||||
});
|
||||
|
||||
it("deduplicates exact prompt when routing matches and no message id", async () => {
|
||||
const key = `test-dedup-whatsapp-${Date.now()}`;
|
||||
const settings = createSettings();
|
||||
const settings: QueueSettings = {
|
||||
mode: "collect",
|
||||
debounceMs: 0,
|
||||
cap: 50,
|
||||
dropPolicy: "summarize",
|
||||
};
|
||||
|
||||
// First enqueue should succeed
|
||||
const first = enqueueFollowupRun(
|
||||
@@ -156,7 +168,12 @@ describe("followup queue deduplication", () => {
|
||||
|
||||
it("does not deduplicate across different providers without message id", async () => {
|
||||
const key = `test-dedup-cross-provider-${Date.now()}`;
|
||||
const settings = createSettings();
|
||||
const settings: QueueSettings = {
|
||||
mode: "collect",
|
||||
debounceMs: 0,
|
||||
cap: 50,
|
||||
dropPolicy: "summarize",
|
||||
};
|
||||
|
||||
const first = enqueueFollowupRun(
|
||||
key,
|
||||
@@ -183,7 +200,12 @@ describe("followup queue deduplication", () => {
|
||||
|
||||
it("can opt-in to prompt-based dedupe when message id is absent", async () => {
|
||||
const key = `test-dedup-prompt-mode-${Date.now()}`;
|
||||
const settings = createSettings();
|
||||
const settings: QueueSettings = {
|
||||
mode: "collect",
|
||||
debounceMs: 0,
|
||||
cap: 50,
|
||||
dropPolicy: "summarize",
|
||||
};
|
||||
|
||||
const first = enqueueFollowupRun(
|
||||
key,
|
||||
@@ -214,8 +236,21 @@ describe("followup queue deduplication", () => {
|
||||
describe("followup queue collect routing", () => {
|
||||
it("does not collect when destinations differ", async () => {
|
||||
const key = `test-collect-diff-to-${Date.now()}`;
|
||||
const { calls, runFollowup } = createRunCollector();
|
||||
const settings = createSettings();
|
||||
const calls: FollowupRun[] = [];
|
||||
const done = createDeferred<void>();
|
||||
const expectedCalls = 2;
|
||||
const runFollowup = async (run: FollowupRun) => {
|
||||
calls.push(run);
|
||||
if (calls.length >= expectedCalls) {
|
||||
done.resolve();
|
||||
}
|
||||
};
|
||||
const settings: QueueSettings = {
|
||||
mode: "collect",
|
||||
debounceMs: 0,
|
||||
cap: 50,
|
||||
dropPolicy: "summarize",
|
||||
};
|
||||
|
||||
enqueueFollowupRun(
|
||||
key,
|
||||
@@ -236,15 +271,29 @@ describe("followup queue collect routing", () => {
|
||||
settings,
|
||||
);
|
||||
|
||||
await drainAndWait({ key, calls, runFollowup, count: 2 });
|
||||
scheduleFollowupDrain(key, runFollowup);
|
||||
await done.promise;
|
||||
expect(calls[0]?.prompt).toBe("one");
|
||||
expect(calls[1]?.prompt).toBe("two");
|
||||
});
|
||||
|
||||
it("collects when channel+destination match", async () => {
|
||||
const key = `test-collect-same-to-${Date.now()}`;
|
||||
const { calls, runFollowup } = createRunCollector();
|
||||
const settings = createSettings();
|
||||
const calls: FollowupRun[] = [];
|
||||
const done = createDeferred<void>();
|
||||
const expectedCalls = 1;
|
||||
const runFollowup = async (run: FollowupRun) => {
|
||||
calls.push(run);
|
||||
if (calls.length >= expectedCalls) {
|
||||
done.resolve();
|
||||
}
|
||||
};
|
||||
const settings: QueueSettings = {
|
||||
mode: "collect",
|
||||
debounceMs: 0,
|
||||
cap: 50,
|
||||
dropPolicy: "summarize",
|
||||
};
|
||||
|
||||
enqueueFollowupRun(
|
||||
key,
|
||||
@@ -265,7 +314,8 @@ describe("followup queue collect routing", () => {
|
||||
settings,
|
||||
);
|
||||
|
||||
await drainAndWait({ key, calls, runFollowup, count: 1 });
|
||||
scheduleFollowupDrain(key, runFollowup);
|
||||
await done.promise;
|
||||
expect(calls[0]?.prompt).toContain("[Queued messages while agent was busy]");
|
||||
expect(calls[0]?.originatingChannel).toBe("slack");
|
||||
expect(calls[0]?.originatingTo).toBe("channel:A");
|
||||
@@ -273,8 +323,21 @@ describe("followup queue collect routing", () => {
|
||||
|
||||
it("collects Slack messages in same thread and preserves string thread id", async () => {
|
||||
const key = `test-collect-slack-thread-same-${Date.now()}`;
|
||||
const { calls, runFollowup } = createRunCollector();
|
||||
const settings = createSettings();
|
||||
const calls: FollowupRun[] = [];
|
||||
const done = createDeferred<void>();
|
||||
const expectedCalls = 1;
|
||||
const runFollowup = async (run: FollowupRun) => {
|
||||
calls.push(run);
|
||||
if (calls.length >= expectedCalls) {
|
||||
done.resolve();
|
||||
}
|
||||
};
|
||||
const settings: QueueSettings = {
|
||||
mode: "collect",
|
||||
debounceMs: 0,
|
||||
cap: 50,
|
||||
dropPolicy: "summarize",
|
||||
};
|
||||
|
||||
enqueueFollowupRun(
|
||||
key,
|
||||
@@ -297,15 +360,29 @@ describe("followup queue collect routing", () => {
|
||||
settings,
|
||||
);
|
||||
|
||||
await drainAndWait({ key, calls, runFollowup, count: 1 });
|
||||
scheduleFollowupDrain(key, runFollowup);
|
||||
await done.promise;
|
||||
expect(calls[0]?.prompt).toContain("[Queued messages while agent was busy]");
|
||||
expect(calls[0]?.originatingThreadId).toBe("1706000000.000001");
|
||||
});
|
||||
|
||||
it("does not collect Slack messages when thread ids differ", async () => {
|
||||
const key = `test-collect-slack-thread-diff-${Date.now()}`;
|
||||
const { calls, runFollowup } = createRunCollector();
|
||||
const settings = createSettings();
|
||||
const calls: FollowupRun[] = [];
|
||||
const done = createDeferred<void>();
|
||||
const expectedCalls = 2;
|
||||
const runFollowup = async (run: FollowupRun) => {
|
||||
calls.push(run);
|
||||
if (calls.length >= expectedCalls) {
|
||||
done.resolve();
|
||||
}
|
||||
};
|
||||
const settings: QueueSettings = {
|
||||
mode: "collect",
|
||||
debounceMs: 0,
|
||||
cap: 50,
|
||||
dropPolicy: "summarize",
|
||||
};
|
||||
|
||||
enqueueFollowupRun(
|
||||
key,
|
||||
@@ -328,7 +405,8 @@ describe("followup queue collect routing", () => {
|
||||
settings,
|
||||
);
|
||||
|
||||
await drainAndWait({ key, calls, runFollowup, count: 2 });
|
||||
scheduleFollowupDrain(key, runFollowup);
|
||||
await done.promise;
|
||||
expect(calls[0]?.prompt).toBe("one");
|
||||
expect(calls[1]?.prompt).toBe("two");
|
||||
expect(calls[0]?.originatingThreadId).toBe("1706000000.000001");
|
||||
@@ -338,6 +416,8 @@ describe("followup queue collect routing", () => {
|
||||
it("retries collect-mode batches without losing queued items", async () => {
|
||||
const key = `test-collect-retry-${Date.now()}`;
|
||||
const calls: FollowupRun[] = [];
|
||||
const done = createDeferred<void>();
|
||||
const expectedCalls = 1;
|
||||
let attempt = 0;
|
||||
const runFollowup = async (run: FollowupRun) => {
|
||||
attempt += 1;
|
||||
@@ -345,14 +425,22 @@ describe("followup queue collect routing", () => {
|
||||
throw new Error("transient failure");
|
||||
}
|
||||
calls.push(run);
|
||||
if (calls.length >= expectedCalls) {
|
||||
done.resolve();
|
||||
}
|
||||
};
|
||||
const settings: QueueSettings = {
|
||||
mode: "collect",
|
||||
debounceMs: 0,
|
||||
cap: 50,
|
||||
dropPolicy: "summarize",
|
||||
};
|
||||
const settings = createSettings();
|
||||
|
||||
enqueueFollowupRun(key, createRun({ prompt: "one" }), settings);
|
||||
enqueueFollowupRun(key, createRun({ prompt: "two" }), settings);
|
||||
|
||||
scheduleFollowupDrain(key, runFollowup);
|
||||
await expect.poll(() => calls.length).toBe(1);
|
||||
await done.promise;
|
||||
expect(calls[0]?.prompt).toContain("Queued #1\none");
|
||||
expect(calls[0]?.prompt).toContain("Queued #2\ntwo");
|
||||
});
|
||||
@@ -360,6 +448,8 @@ describe("followup queue collect routing", () => {
|
||||
it("retries overflow summary delivery without losing dropped previews", async () => {
|
||||
const key = `test-overflow-summary-retry-${Date.now()}`;
|
||||
const calls: FollowupRun[] = [];
|
||||
const done = createDeferred<void>();
|
||||
const expectedCalls = 1;
|
||||
let attempt = 0;
|
||||
const runFollowup = async (run: FollowupRun) => {
|
||||
attempt += 1;
|
||||
@@ -367,14 +457,22 @@ describe("followup queue collect routing", () => {
|
||||
throw new Error("transient failure");
|
||||
}
|
||||
calls.push(run);
|
||||
if (calls.length >= expectedCalls) {
|
||||
done.resolve();
|
||||
}
|
||||
};
|
||||
const settings: QueueSettings = {
|
||||
mode: "followup",
|
||||
debounceMs: 0,
|
||||
cap: 1,
|
||||
dropPolicy: "summarize",
|
||||
};
|
||||
const settings = createSettings({ mode: "followup", cap: 1 });
|
||||
|
||||
enqueueFollowupRun(key, createRun({ prompt: "first" }), settings);
|
||||
enqueueFollowupRun(key, createRun({ prompt: "second" }), settings);
|
||||
|
||||
scheduleFollowupDrain(key, runFollowup);
|
||||
await expect.poll(() => calls.length).toBe(1);
|
||||
await done.promise;
|
||||
expect(calls[0]?.prompt).toContain("[Queue overflow] Dropped 1 message due to cap.");
|
||||
expect(calls[0]?.prompt).toContain("- first");
|
||||
});
|
||||
|
||||
@@ -6,6 +6,11 @@ import type { OpenClawConfig } from "../../config/config.js";
|
||||
import { saveSessionStore } from "../../config/sessions.js";
|
||||
import { initSessionState } from "./session.js";
|
||||
|
||||
// Perf: session-store locks are exercised elsewhere; most session tests don't need FS lock files.
|
||||
vi.mock("../../agents/session-write-lock.js", () => ({
|
||||
acquireSessionWriteLock: async () => ({ release: async () => {} }),
|
||||
}));
|
||||
|
||||
let suiteRoot = "";
|
||||
let suiteCase = 0;
|
||||
|
||||
@@ -27,6 +32,7 @@ async function makeCaseDir(prefix: string): Promise<string> {
|
||||
|
||||
describe("initSessionState thread forking", () => {
|
||||
it("forks a new session from the parent session file", async () => {
|
||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const root = await makeCaseDir("openclaw-thread-session-");
|
||||
const sessionsDir = path.join(root, "sessions");
|
||||
await fs.mkdir(sessionsDir);
|
||||
@@ -96,6 +102,7 @@ describe("initSessionState thread forking", () => {
|
||||
parentSession?: string;
|
||||
};
|
||||
expect(parsedHeader.parentSession).toBe(parentSessionFile);
|
||||
warn.mockRestore();
|
||||
});
|
||||
|
||||
it("records topic-specific session files when MessageThreadId is present", async () => {
|
||||
|
||||
Reference in New Issue
Block a user