From f8a57fe47b42d3df74e96807680beb3c45bb4cd8 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 8 Apr 2026 22:53:05 +0100 Subject: [PATCH] test: move inline directive collisions to pure tests --- ...ng-mixed-messages-acks-immediately.test.ts | 132 +----------------- .../reply/get-reply-directive-aliases.test.ts | 58 ++++++++ .../reply/get-reply-directive-aliases.ts | 29 ++++ src/auto-reply/reply/get-reply-directives.ts | 22 +-- 4 files changed, 93 insertions(+), 148 deletions(-) create mode 100644 src/auto-reply/reply/get-reply-directive-aliases.test.ts create mode 100644 src/auto-reply/reply/get-reply-directive-aliases.ts diff --git a/src/auto-reply/reply.directive.directive-behavior.applies-inline-reasoning-mixed-messages-acks-immediately.test.ts b/src/auto-reply/reply.directive.directive-behavior.applies-inline-reasoning-mixed-messages-acks-immediately.test.ts index ca3773dad49..456d2547270 100644 --- a/src/auto-reply/reply.directive.directive-behavior.applies-inline-reasoning-mixed-messages-acks-immediately.test.ts +++ b/src/auto-reply/reply.directive.directive-behavior.applies-inline-reasoning-mixed-messages-acks-immediately.test.ts @@ -1,14 +1,10 @@ import "./reply.directive.directive-behavior.e2e-mocks.js"; -import fs from "node:fs/promises"; -import path from "node:path"; import { beforeAll, describe, expect, it } from "vitest"; -import { loadSessionStore, resolveSessionKey, saveSessionStore } from "../config/sessions.js"; +import { loadSessionStore } from "../config/sessions.js"; import { installDirectiveBehaviorE2EHooks, - makeEmbeddedTextResult, makeWhatsAppDirectiveConfig, replyText, - replyTexts, sessionStorePath, withTempHome, } from "./reply.directive.directive-behavior.e2e-harness.js"; @@ -16,17 +12,6 @@ import { runEmbeddedPiAgentMock } from "./reply.directive.directive-behavior.e2e let getReplyFromConfig: typeof import("./reply.js").getReplyFromConfig; -async function writeSkill(params: { workspaceDir: string; name: string; description: string }) { - const { workspaceDir, name, description } = params; - const skillDir = path.join(workspaceDir, "skills", name); - await fs.mkdir(skillDir, { recursive: true }); - await fs.writeFile( - path.join(skillDir, "SKILL.md"), - `---\nname: ${name}\ndescription: ${description}\n---\n\n# ${name}\n`, - "utf-8", - ); -} - async function runThinkDirectiveAndGetText(home: string): Promise { const res = await getReplyFromConfig( { Body: "/think", From: "+1222", To: "+1222", CommandAuthorized: true }, @@ -39,62 +24,6 @@ async function runThinkDirectiveAndGetText(home: string): Promise { - const shouldEmit = agentParams.shouldEmitToolResult; - expect(shouldEmit?.()).toBe(params.shouldEmitBefore); - const store = loadSessionStore(storePath); - const entry = store[sessionKey] ?? { - sessionId: "s", - updatedAt: Date.now(), - }; - store[sessionKey] = { - ...entry, - verboseLevel: params.toggledVerboseLevel, - updatedAt: Date.now(), - }; - await saveSessionStore(storePath, store); - expect(shouldEmit?.()).toBe(!params.shouldEmitBefore); - return makeEmbeddedTextResult("done"); - }); - - if (params.seedVerboseOn) { - await getReplyFromConfig( - { Body: "/verbose on", From: ctx.From, To: ctx.To, CommandAuthorized: true }, - {}, - makeRunConfig(params.home, storePath), - ); - } - - const res = await getReplyFromConfig(ctx, {}, makeRunConfig(params.home, storePath)); - return { res }; -} - describe("directive behavior", () => { installDirectiveBehaviorE2EHooks(); @@ -133,30 +62,6 @@ describe("directive behavior", () => { expect(runEmbeddedPiAgentMock).not.toHaveBeenCalled(); }); }); - it("updates tool verbose during in-flight runs for toggle on/off", async () => { - await withTempHome(async (home) => { - for (const testCase of [ - { - shouldEmitBefore: false, - toggledVerboseLevel: "on" as const, - }, - { - shouldEmitBefore: true, - toggledVerboseLevel: "off" as const, - seedVerboseOn: true, - }, - ]) { - runEmbeddedPiAgentMock.mockClear(); - const { res } = await runInFlightVerboseToggleCase({ - home, - ...testCase, - }); - const texts = replyTexts(res); - expect(texts).toContain("done"); - expect(runEmbeddedPiAgentMock).toHaveBeenCalledOnce(); - } - }); - }); it("covers think status", async () => { await withTempHome(async (home) => { const text = await runThinkDirectiveAndGetText(home); @@ -192,41 +97,6 @@ describe("directive behavior", () => { expect(runEmbeddedPiAgentMock).not.toHaveBeenCalled(); }); }); - it("treats skill commands as reserved for model aliases", async () => { - await withTempHome(async (home) => { - const workspace = path.join(home, "openclaw"); - await writeSkill({ - workspaceDir: workspace, - name: "demo-skill", - description: "Demo skill", - }); - - await getReplyFromConfig( - { - Body: "/demo_skill", - From: "+1222", - To: "+1222", - CommandAuthorized: true, - }, - {}, - makeWhatsAppDirectiveConfig( - home, - { - model: "anthropic/claude-opus-4-6", - workspace, - models: { - "anthropic/claude-opus-4-6": { alias: "demo_skill" }, - }, - }, - { session: { store: sessionStorePath(home) } }, - ), - ); - - expect(runEmbeddedPiAgentMock).toHaveBeenCalled(); - const prompt = runEmbeddedPiAgentMock.mock.calls[0]?.[0]?.prompt ?? ""; - expect(prompt).toContain('Use the "demo-skill" skill'); - }); - }); it("reports invalid queue options and current queue settings", async () => { await withTempHome(async (home) => { const invalidRes = await getReplyFromConfig( diff --git a/src/auto-reply/reply/get-reply-directive-aliases.test.ts b/src/auto-reply/reply/get-reply-directive-aliases.test.ts new file mode 100644 index 00000000000..4063432f83a --- /dev/null +++ b/src/auto-reply/reply/get-reply-directive-aliases.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../../config/config.js"; +import { parseInlineDirectives } from "./directive-handling.parse.js"; +import { + reserveSkillCommandNames, + resolveConfiguredDirectiveAliases, +} from "./get-reply-directive-aliases.js"; + +function configWithModelAlias(alias: string): OpenClawConfig { + return { + agents: { + defaults: { + models: { + "anthropic/claude-opus-4-6": { alias }, + }, + }, + }, + } as unknown as OpenClawConfig; +} + +describe("reply directive aliases", () => { + it("does not expose skill command names as inline model aliases", () => { + const reservedCommands = new Set(); + const cfg = configWithModelAlias("demo_skill"); + + expect( + parseInlineDirectives("/demo_skill", { + modelAliases: resolveConfiguredDirectiveAliases({ + cfg, + commandTextHasSlash: true, + reservedCommands, + }), + }), + ).toEqual(expect.objectContaining({ hasModelDirective: true, cleaned: "" })); + + reserveSkillCommandNames({ + reservedCommands, + skillCommands: [ + { + name: "demo_skill", + skillName: "demo-skill", + description: "Demo skill", + sourceFilePath: "/tmp/demo/SKILL.md", + }, + ], + }); + + expect( + parseInlineDirectives("/demo_skill", { + modelAliases: resolveConfiguredDirectiveAliases({ + cfg, + commandTextHasSlash: true, + reservedCommands, + }), + }), + ).toEqual(expect.objectContaining({ hasModelDirective: false, cleaned: "/demo_skill" })); + }); +}); diff --git a/src/auto-reply/reply/get-reply-directive-aliases.ts b/src/auto-reply/reply/get-reply-directive-aliases.ts new file mode 100644 index 00000000000..e403e8682bf --- /dev/null +++ b/src/auto-reply/reply/get-reply-directive-aliases.ts @@ -0,0 +1,29 @@ +import type { SkillCommandSpec } from "../../agents/skills.js"; +import type { OpenClawConfig } from "../../config/config.js"; +import { + normalizeLowercaseStringOrEmpty, + normalizeOptionalString, +} from "../../shared/string-coerce.js"; + +export function reserveSkillCommandNames(params: { + reservedCommands: Set; + skillCommands: SkillCommandSpec[]; +}) { + for (const command of params.skillCommands) { + params.reservedCommands.add(normalizeLowercaseStringOrEmpty(command.name)); + } +} + +export function resolveConfiguredDirectiveAliases(params: { + cfg: OpenClawConfig; + commandTextHasSlash: boolean; + reservedCommands: Set; +}) { + if (!params.commandTextHasSlash) { + return []; + } + return Object.values(params.cfg.agents?.defaults?.models ?? {}) + .map((entry) => normalizeOptionalString(entry.alias)) + .filter((alias): alias is string => Boolean(alias)) + .filter((alias) => !params.reservedCommands.has(normalizeLowercaseStringOrEmpty(alias))); +} diff --git a/src/auto-reply/reply/get-reply-directives.ts b/src/auto-reply/reply/get-reply-directives.ts index e617898b547..d3476c15017 100644 --- a/src/auto-reply/reply/get-reply-directives.ts +++ b/src/auto-reply/reply/get-reply-directives.ts @@ -19,6 +19,10 @@ import type { GetReplyOptions, ReplyPayload } from "../types.js"; import { resolveBlockStreamingChunking } from "./block-streaming.js"; import { buildCommandContext } from "./commands-context.js"; import { type InlineDirectives, parseInlineDirectives } from "./directive-handling.parse.js"; +import { + reserveSkillCommandNames, + resolveConfiguredDirectiveAliases, +} from "./get-reply-directive-aliases.js"; import { applyInlineDirectiveOverrides } from "./get-reply-directives-apply.js"; import { clearExecInlineDirectives, clearInlineDirectives } from "./get-reply-directives-utils.js"; import { shouldUseReplyFastTestRuntime } from "./get-reply-fast-path.js"; @@ -75,20 +79,6 @@ function resolveDirectiveCommandText(params: { ctx: MsgContext; sessionCtx: Temp }; } -function resolveConfiguredDirectiveAliases(params: { - cfg: OpenClawConfig; - commandTextHasSlash: boolean; - reservedCommands: Set; -}) { - if (!params.commandTextHasSlash) { - return []; - } - return Object.values(params.cfg.agents?.defaults?.models ?? {}) - .map((entry) => normalizeOptionalString(entry.alias)) - .filter((alias): alias is string => Boolean(alias)) - .filter((alias) => !params.reservedCommands.has(normalizeLowercaseStringOrEmpty(alias))); -} - export type ReplyDirectiveContinuation = { commandSource: string; command: ReturnType; @@ -267,9 +257,7 @@ export async function resolveReplyDirectives(params: { skillFilter, }) : []; - for (const command of skillCommands) { - reservedCommands.add(normalizeLowercaseStringOrEmpty(command.name)); - } + reserveSkillCommandNames({ reservedCommands, skillCommands }); const configuredAliases = rawAliases.filter( (alias) => !reservedCommands.has(normalizeLowercaseStringOrEmpty(alias)),