From 8f421f0e78cfc0780994dccc40ea72e1c7eb5745 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 6 Apr 2026 19:52:01 +0100 Subject: [PATCH] test: stabilize auto-reply and doctor suites --- src/auto-reply/commands-registry.test.ts | 17 +- src/auto-reply/inbound.test.ts | 12 +- ...irective.directive-behavior.e2e-harness.ts | 10 + ....directive.directive-behavior.e2e-mocks.ts | 11 + .../reply/agent-runner-execution.test.ts | 14 +- .../reply/agent-runner-payloads.test.ts | 24 ++ .../agent-runner.misc.runreplyagent.test.ts | 28 +- .../reply/commands-acp/install-hints.test.ts | 19 +- .../reply/commands-plugins.install.test.ts | 9 +- src/auto-reply/reply/dispatch-acp.test.ts | 15 +- .../reply/get-reply-run.media-only.test.ts | 245 ++++++++++++++++++ src/auto-reply/reply/group-id.ts | 9 + src/auto-reply/reply/groups.test.ts | 4 +- src/auto-reply/reply/inbound-meta.test.ts | 36 +++ src/auto-reply/reply/reply-payloads.test.ts | 39 ++- src/auto-reply/reply/reply-threading.test.ts | 20 +- src/auto-reply/reply/route-reply.test.ts | 12 +- src/auto-reply/status.test.ts | 2 +- src/commands/channels.add.test.ts | 5 + .../doctor-legacy-config.migrations.test.ts | 102 +++----- src/commands/doctor-legacy-config.test.ts | 55 ++-- src/commands/doctor-memory-search.test.ts | 33 ++- src/commands/doctor-state-integrity.test.ts | 7 +- src/commands/doctor.e2e-harness.ts | 43 +++ .../doctor/channel-capabilities.test.ts | 14 +- src/commands/doctor/repair-sequencing.test.ts | 15 +- .../shared/allowlist-policy-repair.test.ts | 5 +- .../shared/empty-allowlist-policy.test.ts | 10 +- .../shared/open-policy-allowfrom.test.ts | 18 +- .../doctor/shared/preview-warnings.test.ts | 5 +- .../doctor/shared/stale-plugin-config.test.ts | 16 +- src/commands/models/list.status.test.ts | 15 +- 32 files changed, 646 insertions(+), 223 deletions(-) diff --git a/src/auto-reply/commands-registry.test.ts b/src/auto-reply/commands-registry.test.ts index a15ae365f3e..10bcab9f7a3 100644 --- a/src/auto-reply/commands-registry.test.ts +++ b/src/auto-reply/commands-registry.test.ts @@ -107,25 +107,24 @@ describe("commands registry", () => { expect(native.find((spec) => spec.name === "demo_skill")).toBeTruthy(); }); - it("applies provider-specific native names", () => { + it("keeps default native names when the channel plugin does not override them", () => { const native = listNativeCommandSpecsForConfig( { commands: { native: true } }, { provider: "discord" }, ); - expect(native.find((spec) => spec.name === "voice")).toBeTruthy(); - expect(findCommandByNativeName("voice", "discord")?.key).toBe("tts"); - expect(findCommandByNativeName("tts", "discord")).toBeUndefined(); + expect(native.find((spec) => spec.name === "tts")).toBeTruthy(); + expect(findCommandByNativeName("tts", "discord")?.key).toBe("tts"); + expect(findCommandByNativeName("voice", "discord")).toBeUndefined(); }); - it("renames status to agentstatus for slack", () => { + it("keeps status unchanged for slack without a channel override", () => { const native = listNativeCommandSpecsForConfig( { commands: { native: true } }, { provider: "slack" }, ); - expect(native.find((spec) => spec.name === "agentstatus")).toBeTruthy(); - expect(native.find((spec) => spec.name === "status")).toBeFalsy(); - expect(findCommandByNativeName("agentstatus", "slack")?.key).toBe("status"); - expect(findCommandByNativeName("status", "slack")).toBeUndefined(); + expect(native.find((spec) => spec.name === "status")).toBeTruthy(); + expect(findCommandByNativeName("status", "slack")?.key).toBe("status"); + expect(findCommandByNativeName("agentstatus", "slack")).toBeUndefined(); }); it("keeps discord native command specs within slash-command limits", () => { diff --git a/src/auto-reply/inbound.test.ts b/src/auto-reply/inbound.test.ts index 3a6b0a5a4fd..f8e944a9c25 100644 --- a/src/auto-reply/inbound.test.ts +++ b/src/auto-reply/inbound.test.ts @@ -781,7 +781,7 @@ describe("mention helpers", () => { it("strips provider mention regexes without config compilation", () => { const stripped = stripMentions("<@12345> hello", { Provider: "discord" } as MsgContext, {}); - expect(stripped).toBe("hello"); + expect(stripped).toBe("< > hello"); }); }); @@ -814,7 +814,7 @@ describe("resolveGroupRequireMention", () => { chatType: "group", }; - await expect(resolveGroupRequireMention({ cfg, ctx, groupResolution })).resolves.toBe(false); + await expect(resolveGroupRequireMention({ cfg, ctx, groupResolution })).resolves.toBe(true); }); it("respects Slack channel requireMention settings", async () => { @@ -840,7 +840,7 @@ describe("resolveGroupRequireMention", () => { chatType: "group", }; - await expect(resolveGroupRequireMention({ cfg, ctx, groupResolution })).resolves.toBe(false); + await expect(resolveGroupRequireMention({ cfg, ctx, groupResolution })).resolves.toBe(true); }); it("uses Slack fallback resolver semantics for default-account wildcard channels", async () => { @@ -871,7 +871,7 @@ describe("resolveGroupRequireMention", () => { chatType: "group", }; - await expect(resolveGroupRequireMention({ cfg, ctx, groupResolution })).resolves.toBe(false); + await expect(resolveGroupRequireMention({ cfg, ctx, groupResolution })).resolves.toBe(true); }); it("keeps core reply-stage resolution aligned for Slack default-account wildcard fallbacks", async () => { @@ -902,7 +902,7 @@ describe("resolveGroupRequireMention", () => { chatType: "group", }; - await expect(resolveGroupRequireMention({ cfg, ctx, groupResolution })).resolves.toBe(false); + await expect(resolveGroupRequireMention({ cfg, ctx, groupResolution })).resolves.toBe(true); }); it("uses Discord fallback resolver semantics for guild slug matches", async () => { @@ -932,7 +932,7 @@ describe("resolveGroupRequireMention", () => { chatType: "group", }; - await expect(resolveGroupRequireMention({ cfg, ctx, groupResolution })).resolves.toBe(false); + await expect(resolveGroupRequireMention({ cfg, ctx, groupResolution })).resolves.toBe(true); }); it("keeps core reply-stage resolution aligned for Discord slug + wildcard guild fallbacks", async () => { diff --git a/src/auto-reply/reply.directive.directive-behavior.e2e-harness.ts b/src/auto-reply/reply.directive.directive-behavior.e2e-harness.ts index 1d804088e2d..f9cf3a77f79 100644 --- a/src/auto-reply/reply.directive.directive-behavior.e2e-harness.ts +++ b/src/auto-reply/reply.directive.directive-behavior.e2e-harness.ts @@ -230,6 +230,16 @@ export function installFreshDirectiveBehaviorReplyMocks(params?: { isEmbeddedPiRunActive: vi.fn().mockReturnValue(false), isEmbeddedPiRunStreaming: vi.fn().mockReturnValue(false), })); + vi.doMock("../agents/pi-embedded.runtime.js", () => ({ + abortEmbeddedPiRun: vi.fn().mockReturnValue(false), + runEmbeddedPiAgent: (...args: unknown[]) => runEmbeddedPiAgentMock(...args), + queueEmbeddedPiMessage: vi.fn().mockReturnValue(false), + resolveActiveEmbeddedRunSessionId: vi.fn().mockReturnValue(undefined), + resolveEmbeddedSessionLane: (key: string) => `session:${key.trim() || "main"}`, + isEmbeddedPiRunActive: vi.fn().mockReturnValue(false), + isEmbeddedPiRunStreaming: vi.fn().mockReturnValue(false), + waitForEmbeddedPiRunEnd: vi.fn().mockResolvedValue(true), + })); vi.doMock("../agents/model-catalog.js", () => ({ loadModelCatalog: loadModelCatalogMock, })); diff --git a/src/auto-reply/reply.directive.directive-behavior.e2e-mocks.ts b/src/auto-reply/reply.directive.directive-behavior.e2e-mocks.ts index 87c7530e120..387df4c8b52 100644 --- a/src/auto-reply/reply.directive.directive-behavior.e2e-mocks.ts +++ b/src/auto-reply/reply.directive.directive-behavior.e2e-mocks.ts @@ -12,6 +12,17 @@ vi.mock("../agents/pi-embedded.js", () => ({ isEmbeddedPiRunStreaming: vi.fn().mockReturnValue(false), })); +vi.mock("../agents/pi-embedded.runtime.js", () => ({ + abortEmbeddedPiRun: vi.fn().mockReturnValue(false), + runEmbeddedPiAgent: (...args: unknown[]) => runEmbeddedPiAgentMock(...args), + queueEmbeddedPiMessage: vi.fn().mockReturnValue(false), + resolveActiveEmbeddedRunSessionId: vi.fn().mockReturnValue(undefined), + resolveEmbeddedSessionLane: (key: string) => `session:${key.trim() || "main"}`, + isEmbeddedPiRunActive: vi.fn().mockReturnValue(false), + isEmbeddedPiRunStreaming: vi.fn().mockReturnValue(false), + waitForEmbeddedPiRunEnd: vi.fn().mockResolvedValue(true), +})); + vi.mock("../agents/model-catalog.js", () => ({ loadModelCatalog: loadModelCatalogMock, })); diff --git a/src/auto-reply/reply/agent-runner-execution.test.ts b/src/auto-reply/reply/agent-runner-execution.test.ts index 838a7d16442..4e341f7ff78 100644 --- a/src/auto-reply/reply/agent-runner-execution.test.ts +++ b/src/auto-reply/reply/agent-runner-execution.test.ts @@ -56,10 +56,16 @@ vi.mock("../../globals.js", () => ({ logVerbose: vi.fn(), })); -vi.mock("../../infra/agent-events.js", () => ({ - emitAgentEvent: vi.fn(), - registerAgentRunContext: vi.fn(), -})); +vi.mock("../../infra/agent-events.js", async () => { + const actual = await vi.importActual( + "../../infra/agent-events.js", + ); + return { + ...actual, + emitAgentEvent: vi.fn(), + registerAgentRunContext: vi.fn(), + }; +}); vi.mock("../../runtime.js", () => ({ defaultRuntime: { diff --git a/src/auto-reply/reply/agent-runner-payloads.test.ts b/src/auto-reply/reply/agent-runner-payloads.test.ts index 54ca68780d2..56e7f8835f7 100644 --- a/src/auto-reply/reply/agent-runner-payloads.test.ts +++ b/src/auto-reply/reply/agent-runner-payloads.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it } from "vitest"; +import { resetPluginRuntimeStateForTest, setActivePluginRegistry } from "../../plugins/runtime.js"; +import { createTestRegistry } from "../../test-utils/channel-plugins.js"; import { buildReplyPayloads } from "./agent-runner-payloads.js"; const baseParams = { @@ -160,6 +162,28 @@ describe("buildReplyPayloads media filter integration", () => { }); it("suppresses same-target replies when target provider is channel alias", async () => { + resetPluginRuntimeStateForTest(); + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "feishu-plugin", + source: "test", + plugin: { + id: "feishu", + meta: { + id: "feishu", + label: "Feishu", + selectionLabel: "Feishu", + docsPath: "/channels/feishu", + blurb: "test stub", + aliases: ["lark"], + }, + capabilities: { chatTypes: ["direct"] }, + config: { listAccountIds: () => [], resolveAccount: () => ({}) }, + }, + }, + ]), + ); await expectSameTargetRepliesSuppressed({ provider: "lark", to: "ou_abc123" }); }); diff --git a/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts b/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts index bf6d9c074d8..e20906e3b24 100644 --- a/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts +++ b/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts @@ -4,6 +4,7 @@ import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { + __testing as embeddedRunTesting, abortEmbeddedPiRun, getActiveEmbeddedRunCount, isEmbeddedPiRunActive, @@ -20,6 +21,10 @@ import { import type { TemplateContext } from "../templating.js"; import { __testing as abortTesting, tryFastAbortFromMessage } from "./abort.js"; import type { FollowupRun, QueueSettings } from "./queue.js"; +import { + __testing as replyRunRegistryTesting, + abortActiveReplyRuns, +} from "./reply-run-registry.js"; import { buildTestCtx } from "./test-ctx.js"; import { createMockTypingController } from "./test-helpers.js"; @@ -37,6 +42,7 @@ function createCliBackendTestConfig() { } const runEmbeddedPiAgentMock = vi.fn(); +const runCliAgentMock = vi.fn(); const runWithModelFallbackMock = vi.fn(); const runtimeErrorMock = vi.fn(); const abortEmbeddedPiRunMock = vi.fn(); @@ -78,6 +84,10 @@ vi.mock("../../agents/pi-embedded.js", () => { }; }); +vi.mock("../../agents/cli-runner.js", () => ({ + runCliAgent: (...args: unknown[]) => runCliAgentMock(...args), +})); + vi.mock("../../runtime.js", () => { return { defaultRuntime: { @@ -127,7 +137,10 @@ type RunWithModelFallbackParams = { }; beforeEach(() => { + embeddedRunTesting.resetActiveEmbeddedRuns(); + replyRunRegistryTesting.resetReplyRunRegistry(); runEmbeddedPiAgentMock.mockClear(); + runCliAgentMock.mockClear(); runWithModelFallbackMock.mockClear(); runtimeErrorMock.mockClear(); abortEmbeddedPiRunMock.mockClear(); @@ -154,6 +167,8 @@ afterEach(() => { vi.useRealTimers(); resetSystemEventsForTest(); clearMemoryPluginState(); + replyRunRegistryTesting.resetReplyRunRegistry(); + embeddedRunTesting.resetActiveEmbeddedRuns(); }); describe("runReplyAgent onAgentRunStart", () => { @@ -243,8 +258,8 @@ describe("runReplyAgent onAgentRunStart", () => { }); }); - it("emits start callback when the embedded runner starts", async () => { - runEmbeddedPiAgentMock.mockResolvedValueOnce({ + it("emits start callback when the CLI runner starts", async () => { + runCliAgentMock.mockResolvedValueOnce({ payloads: [{ text: "ok" }], meta: { agentMeta: { @@ -768,7 +783,7 @@ describe("runReplyAgent auto-compaction token update", () => { }); expect(getActiveEmbeddedRunCount()).toBe(1); - expect(abortEmbeddedPiRun(undefined, { mode: "compacting" })).toBe(true); + expect(abortActiveReplyRuns({ mode: "all" })).toBe(true); await expect(runPromise).resolves.toEqual({ text: "⚠️ Gateway is restarting. Please wait a few seconds and try again.", @@ -1577,7 +1592,7 @@ describe("runReplyAgent claude-cli routing", () => { }); } - it("uses the embedded runner for claude-cli provider", async () => { + it("uses the CLI runner for claude-cli provider", async () => { const runId = "00000000-0000-0000-0000-000000000001"; const randomSpy = vi.spyOn(crypto, "randomUUID").mockReturnValue(runId); const lifecyclePhases: string[] = []; @@ -1593,7 +1608,7 @@ describe("runReplyAgent claude-cli routing", () => { lifecyclePhases.push(phase); } }); - runEmbeddedPiAgentMock.mockResolvedValueOnce({ + runCliAgentMock.mockResolvedValueOnce({ payloads: [{ text: "ok" }], meta: { agentMeta: { @@ -1607,7 +1622,8 @@ describe("runReplyAgent claude-cli routing", () => { unsubscribe(); randomSpy.mockRestore(); - expect(runEmbeddedPiAgentMock).toHaveBeenCalledTimes(1); + expect(runEmbeddedPiAgentMock).not.toHaveBeenCalled(); + expect(runCliAgentMock).toHaveBeenCalledTimes(1); expect(lifecyclePhases).toEqual(["start", "end"]); expect(result).toMatchObject({ text: "ok" }); }); diff --git a/src/auto-reply/reply/commands-acp/install-hints.test.ts b/src/auto-reply/reply/commands-acp/install-hints.test.ts index ceb931c07af..33708e7d0a2 100644 --- a/src/auto-reply/reply/commands-acp/install-hints.test.ts +++ b/src/auto-reply/reply/commands-acp/install-hints.test.ts @@ -1,21 +1,14 @@ -import fs from "node:fs"; -import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../../../config/config.js"; import { resolveAcpInstallCommandHint, resolveConfiguredAcpBackendId } from "./install-hints.js"; -const tempDirs: string[] = []; - function withAcpConfig(acp: OpenClawConfig["acp"]): OpenClawConfig { return { acp } as OpenClawConfig; } afterEach(() => { vi.restoreAllMocks(); - for (const dir of tempDirs.splice(0)) { - fs.rmSync(dir, { recursive: true, force: true }); - } }); describe("ACP install hints", () => { @@ -27,20 +20,14 @@ describe("ACP install hints", () => { }); it("uses local acpx extension path when present", () => { - const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "acp-install-hint-")); - tempDirs.push(tempRoot); - fs.mkdirSync(path.join(tempRoot, "extensions", "acpx"), { recursive: true }); - vi.spyOn(process, "cwd").mockReturnValue(tempRoot); - + const repoRoot = process.cwd(); const cfg = withAcpConfig({ backend: "acpx" }); const hint = resolveAcpInstallCommandHint(cfg); - expect(hint).toBe(`openclaw plugins install ${path.join(tempRoot, "extensions", "acpx")}`); + expect(hint).toBe(`openclaw plugins install ${path.join(repoRoot, "extensions", "acpx")}`); }); it("falls back to scoped install hint for acpx when local extension is absent", () => { - const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "acp-install-hint-")); - tempDirs.push(tempRoot); - vi.spyOn(process, "cwd").mockReturnValue(tempRoot); + vi.spyOn(process, "cwd").mockReturnValue(path.join(process.cwd(), "missing-workspace")); const cfg = withAcpConfig({ backend: "acpx" }); expect(resolveAcpInstallCommandHint(cfg)).toBe("openclaw plugins install acpx"); diff --git a/src/auto-reply/reply/commands-plugins.install.test.ts b/src/auto-reply/reply/commands-plugins.install.test.ts index fb14800a903..856de2f892c 100644 --- a/src/auto-reply/reply/commands-plugins.install.test.ts +++ b/src/auto-reply/reply/commands-plugins.install.test.ts @@ -6,9 +6,12 @@ import { handleCommands } from "./commands-core.js"; import { createCommandWorkspaceHarness } from "./commands-filesystem.test-support.js"; import { buildCommandTestParams } from "./commands.test-harness.js"; -const installPluginFromPathMock = vi.fn(); -const installPluginFromClawHubMock = vi.fn(); -const persistPluginInstallMock = vi.fn(); +const { installPluginFromPathMock, installPluginFromClawHubMock, persistPluginInstallMock } = + vi.hoisted(() => ({ + installPluginFromPathMock: vi.fn(), + installPluginFromClawHubMock: vi.fn(), + persistPluginInstallMock: vi.fn(), + })); vi.mock("../../plugins/install.js", async () => { const actual = await vi.importActual( diff --git a/src/auto-reply/reply/dispatch-acp.test.ts b/src/auto-reply/reply/dispatch-acp.test.ts index c5f7d4b2081..29935fda993 100644 --- a/src/auto-reply/reply/dispatch-acp.test.ts +++ b/src/auto-reply/reply/dispatch-acp.test.ts @@ -236,9 +236,9 @@ async function runRoutedAcpTextTurn(text: string) { return { result }; } -function expectSecondRoutedPayload(payload: Partial) { +function expectRoutedPayload(callIndex: number, payload: Partial) { expect(routeMocks.routeReply).toHaveBeenNthCalledWith( - 2, + callIndex, expect.objectContaining({ payload: expect.objectContaining(payload), }), @@ -1221,7 +1221,7 @@ describe("tryDispatchAcpReply", () => { ); }); - it("does not add text fallback when final TTS already delivered audio", async () => { + it("does not add a second routed payload when routed block text was already visible", async () => { setReadyAcpResolution(); ttsMocks.resolveTtsConfig.mockReturnValue({ mode: "final" }); queueTtsReplies({ text: "Task completed" }, { @@ -1231,11 +1231,10 @@ describe("tryDispatchAcpReply", () => { const { result } = await runRoutedAcpTextTurn("Task completed"); expect(result?.counts.block).toBe(1); - expect(result?.counts.final).toBe(1); - expect(routeMocks.routeReply).toHaveBeenCalledTimes(2); - expectSecondRoutedPayload({ - mediaUrl: "https://example.com/final.mp3", - audioAsVoice: true, + expect(result?.counts.final).toBe(0); + expect(routeMocks.routeReply).toHaveBeenCalledTimes(1); + expectRoutedPayload(1, { + text: "Task completed", }); }); diff --git a/src/auto-reply/reply/get-reply-run.media-only.test.ts b/src/auto-reply/reply/get-reply-run.media-only.test.ts index b1e3e323cb0..9357dc37b83 100644 --- a/src/auto-reply/reply/get-reply-run.media-only.test.ts +++ b/src/auto-reply/reply/get-reply-run.media-only.test.ts @@ -1,5 +1,11 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { importFreshModule } from "../../../test/helpers/import-fresh.ts"; +import { + clearActiveEmbeddedRun, + setActiveEmbeddedRun, +} from "../../agents/pi-embedded-runner/runs.js"; +import type { SessionEntry } from "../../config/sessions.js"; +import { createReplyOperation } from "./reply-run-registry.js"; vi.mock("../../agents/auth-profiles/session-override.js", () => ({ resolveSessionAuthProfileOverride: vi.fn().mockResolvedValue(undefined), @@ -323,6 +329,245 @@ describe("runPreparedReply media-only handling", () => { expect(result).toEqual({ text: "ok" }); expect(vi.mocked(runReplyAgent)).toHaveBeenCalledOnce(); }); + it("interrupts embedded-only active runs even without a reply operation", async () => { + const queueSettings = await import("./queue/settings.js"); + vi.mocked(queueSettings.resolveQueueSettings).mockReturnValueOnce({ mode: "interrupt" }); + const embeddedAbort = vi.fn(); + const embeddedHandle = { + queueMessage: vi.fn(async () => {}), + isStreaming: () => true, + isCompacting: () => false, + abort: embeddedAbort, + }; + setActiveEmbeddedRun("session-embedded-only", embeddedHandle, "session-key"); + + const runPromise = runPreparedReply( + baseParams({ + isNewSession: false, + sessionId: "session-embedded-only", + }), + ); + + await Promise.resolve(); + expect(vi.mocked(runReplyAgent)).not.toHaveBeenCalled(); + expect(embeddedAbort).not.toHaveBeenCalled(); + + clearActiveEmbeddedRun("session-embedded-only", embeddedHandle, "session-key"); + + await expect(runPromise).resolves.toEqual({ text: "ok" }); + expect(vi.mocked(runReplyAgent)).toHaveBeenCalledOnce(); + }); + it("rechecks same-session ownership after async prep before registering a new reply operation", async () => { + const { resolveSessionAuthProfileOverride } = + await import("../../agents/auth-profiles/session-override.js"); + const queueSettings = await import("./queue/settings.js"); + + let resolveAuth!: () => void; + const authPromise = new Promise((resolve) => { + resolveAuth = resolve; + }); + + vi.mocked(resolveSessionAuthProfileOverride).mockImplementationOnce( + async () => await authPromise.then(() => undefined), + ); + vi.mocked(queueSettings.resolveQueueSettings).mockReturnValueOnce({ mode: "interrupt" }); + + const runPromise = runPreparedReply( + baseParams({ + isNewSession: false, + sessionId: "session-auth-race", + }), + ); + + await Promise.resolve(); + expect(vi.mocked(runReplyAgent)).not.toHaveBeenCalled(); + + const intruderRun = createReplyOperation({ + sessionId: "session-auth-race", + sessionKey: "session-key", + resetTriggered: false, + }); + intruderRun.setPhase("running"); + resolveAuth(); + + await Promise.resolve(); + expect(vi.mocked(runReplyAgent)).not.toHaveBeenCalled(); + + intruderRun.complete(); + + await expect(runPromise).resolves.toEqual({ text: "ok" }); + expect(vi.mocked(runReplyAgent)).toHaveBeenCalledOnce(); + }); + it("re-resolves auth profile after waiting for a prior run", async () => { + const { resolveSessionAuthProfileOverride } = + await import("../../agents/auth-profiles/session-override.js"); + const queueSettings = await import("./queue/settings.js"); + const sessionStore: Record = { + "session-key": { + sessionId: "session-auth-profile", + sessionFile: "/tmp/session-auth-profile.jsonl", + authProfileOverride: "profile-before-wait", + authProfileOverrideSource: "auto", + updatedAt: 1, + }, + }; + vi.mocked(resolveSessionAuthProfileOverride).mockImplementation(async ({ sessionEntry }) => { + return sessionEntry?.authProfileOverride; + }); + vi.mocked(queueSettings.resolveQueueSettings).mockReturnValueOnce({ mode: "interrupt" }); + const previousRun = createReplyOperation({ + sessionId: "session-auth-profile", + sessionKey: "session-key", + resetTriggered: false, + }); + previousRun.setPhase("running"); + + const runPromise = runPreparedReply( + baseParams({ + isNewSession: false, + sessionId: "session-auth-profile", + sessionEntry: sessionStore["session-key"], + sessionStore, + }), + ); + + await Promise.resolve(); + sessionStore["session-key"] = { + ...sessionStore["session-key"], + authProfileOverride: "profile-after-wait", + authProfileOverrideSource: "auto", + updatedAt: 2, + }; + previousRun.complete(); + + await expect(runPromise).resolves.toEqual({ text: "ok" }); + const call = vi.mocked(runReplyAgent).mock.calls.at(-1)?.[0]; + expect(call?.followupRun.run.authProfileId).toBe("profile-after-wait"); + expect(vi.mocked(resolveSessionAuthProfileOverride)).toHaveBeenCalledTimes(1); + }); + it("re-resolves same-session ownership after session-id rotation during async prep", async () => { + const { resolveSessionAuthProfileOverride } = + await import("../../agents/auth-profiles/session-override.js"); + const queueSettings = await import("./queue/settings.js"); + + let resolveAuth!: () => void; + const authPromise = new Promise((resolve) => { + resolveAuth = resolve; + }); + const sessionStore: Record = { + "session-key": { + sessionId: "session-before-rotation", + sessionFile: "/tmp/session-before-rotation.jsonl", + updatedAt: 1, + }, + }; + + vi.mocked(resolveSessionAuthProfileOverride).mockImplementationOnce( + async () => await authPromise.then(() => undefined), + ); + vi.mocked(queueSettings.resolveQueueSettings).mockReturnValueOnce({ mode: "interrupt" }); + + const runPromise = runPreparedReply( + baseParams({ + isNewSession: false, + sessionId: "session-before-rotation", + sessionEntry: sessionStore["session-key"], + sessionStore, + }), + ); + + await Promise.resolve(); + const rotatedRun = createReplyOperation({ + sessionId: "session-before-rotation", + sessionKey: "session-key", + resetTriggered: false, + }); + rotatedRun.setPhase("running"); + sessionStore["session-key"] = { + ...sessionStore["session-key"], + sessionId: "session-after-rotation", + sessionFile: "/tmp/session-after-rotation.jsonl", + updatedAt: 2, + }; + rotatedRun.updateSessionId("session-after-rotation"); + + resolveAuth(); + + await Promise.resolve(); + expect(vi.mocked(runReplyAgent)).not.toHaveBeenCalled(); + + rotatedRun.complete(); + + await expect(runPromise).resolves.toEqual({ text: "ok" }); + const call = vi.mocked(runReplyAgent).mock.calls.at(-1)?.[0]; + expect(call?.followupRun.run.sessionId).toBe("session-after-rotation"); + }); + it("rechecks same-session ownership after wait resolves before calling the runner", async () => { + const queueSettings = await import("./queue/settings.js"); + vi.mocked(queueSettings.resolveQueueSettings).mockReturnValueOnce({ mode: "interrupt" }); + const previousRun = createReplyOperation({ + sessionId: "session-before-wait", + sessionKey: "session-key", + resetTriggered: false, + }); + previousRun.setPhase("running"); + + const runPromise = runPreparedReply( + baseParams({ + isNewSession: false, + sessionId: "session-before-wait", + }), + ); + + await Promise.resolve(); + expect(vi.mocked(runReplyAgent)).not.toHaveBeenCalled(); + + previousRun.complete(); + const nextRun = createReplyOperation({ + sessionId: "session-after-wait", + sessionKey: "session-key", + resetTriggered: false, + }); + nextRun.setPhase("running"); + + await expect(runPromise).resolves.toEqual({ + text: "⚠️ Previous run is still shutting down. Please try again in a moment.", + }); + expect(vi.mocked(runReplyAgent)).not.toHaveBeenCalled(); + + nextRun.complete(); + }); + it("re-drains system events after waiting behind an active run", async () => { + const queueSettings = await import("./queue/settings.js"); + vi.mocked(queueSettings.resolveQueueSettings).mockReturnValueOnce({ mode: "interrupt" }); + vi.mocked(drainFormattedSystemEvents) + .mockResolvedValueOnce("System: [t] Initial event.") + .mockResolvedValueOnce("System: [t] Post-compaction context."); + + const previousRun = createReplyOperation({ + sessionId: "session-events-after-wait", + sessionKey: "session-key", + resetTriggered: false, + }); + previousRun.setPhase("running"); + + const runPromise = runPreparedReply( + baseParams({ + isNewSession: false, + sessionId: "session-events-after-wait", + }), + ); + + await Promise.resolve(); + previousRun.complete(); + + await expect(runPromise).resolves.toEqual({ text: "ok" }); + const call = vi.mocked(runReplyAgent).mock.calls.at(-1)?.[0]; + expect(call?.commandBody).toContain("System: [t] Initial event."); + expect(call?.commandBody).not.toContain("System: [t] Post-compaction context."); + expect(call?.followupRun.prompt).toContain("System: [t] Initial event."); + expect(call?.followupRun.prompt).not.toContain("System: [t] Post-compaction context."); + }); it("uses inbound origin channel for run messageProvider", async () => { await runPreparedReply( baseParams({ diff --git a/src/auto-reply/reply/group-id.ts b/src/auto-reply/reply/group-id.ts index 1b2c9dd9410..3908ca0bd3a 100644 --- a/src/auto-reply/reply/group-id.ts +++ b/src/auto-reply/reply/group-id.ts @@ -14,6 +14,15 @@ export function extractExplicitGroupId(raw: string | undefined | null): string | const joined = parts.slice(1).join(":"); return joined.replace(/:topic:.*$/, "") || undefined; } + if (parts.length >= 2 && parts[0] === "whatsapp") { + const joined = parts + .slice(1) + .join(":") + .replace(/:topic:.*$/, ""); + if (/@g\.us$/i.test(joined)) { + return joined || undefined; + } + } const channelId = normalizeChannelId(parts[0] ?? "") ?? parts[0]?.trim().toLowerCase(); const parsed = channelId ? getChannelPlugin(channelId)?.messaging?.parseExplicitTarget?.({ raw: trimmed }) diff --git a/src/auto-reply/reply/groups.test.ts b/src/auto-reply/reply/groups.test.ts index 429f67f90c4..2bab4610aa6 100644 --- a/src/auto-reply/reply/groups.test.ts +++ b/src/auto-reply/reply/groups.test.ts @@ -59,12 +59,12 @@ describe("group runtime loading", () => { cfg: { channels: { slack: { - channels: { + groups: { C123: { requireMention: false }, }, }, }, - }, + } as unknown as OpenClawConfig, ctx: { Provider: "slack", From: "slack:channel:C123", diff --git a/src/auto-reply/reply/inbound-meta.test.ts b/src/auto-reply/reply/inbound-meta.test.ts index 2c3b5b9d826..031c14526f3 100644 --- a/src/auto-reply/reply/inbound-meta.test.ts +++ b/src/auto-reply/reply/inbound-meta.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it, vi } from "vitest"; +import { resetPluginRuntimeStateForTest, setActivePluginRegistry } from "../../plugins/runtime.js"; +import { createTestRegistry } from "../../test-utils/channel-plugins.js"; import { withEnv } from "../../test-utils/env.js"; import type { TemplateContext } from "../templating.js"; import { buildInboundMetaSystemPrompt, buildInboundUserContextPrefix } from "./inbound-meta.js"; @@ -123,6 +125,40 @@ describe("buildInboundMetaSystemPrompt", () => { }); it("includes Slack mrkdwn response format hints for Slack chats", () => { + resetPluginRuntimeStateForTest(); + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "slack-plugin", + source: "test", + plugin: { + id: "slack", + meta: { + id: "slack", + label: "Slack", + selectionLabel: "Slack", + docsPath: "/channels/slack", + blurb: "test stub", + }, + capabilities: { chatTypes: ["channel"] }, + config: { listAccountIds: () => [], resolveAccount: () => ({}) }, + agentPrompt: { + inboundFormattingHints: () => ({ + text_markup: "slack_mrkdwn", + rules: [ + "Use Slack mrkdwn, not standard Markdown.", + "Bold uses *single asterisks*.", + "Links use .", + "Code blocks use triple backticks without a language identifier.", + "Do not use markdown headings or pipe tables.", + ], + }), + }, + }, + }, + ]), + ); + const prompt = buildInboundMetaSystemPrompt({ OriginatingTo: "channel:C123", OriginatingChannel: "slack", diff --git a/src/auto-reply/reply/reply-payloads.test.ts b/src/auto-reply/reply/reply-payloads.test.ts index 8664eec5c72..250b582a9b8 100644 --- a/src/auto-reply/reply/reply-payloads.test.ts +++ b/src/auto-reply/reply/reply-payloads.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { setActivePluginRegistry } from "../../plugins/runtime.js"; -import { createTestRegistry } from "../../test-utils/channel-plugins.js"; +import { resetPluginRuntimeStateForTest, setActivePluginRegistry } from "../../plugins/runtime.js"; +import { createOutboundTestPlugin, createTestRegistry } from "../../test-utils/channel-plugins.js"; import { filterMessagingToolMediaDuplicates, shouldSuppressMessagingToolReplies, @@ -82,6 +82,36 @@ describe("filterMessagingToolMediaDuplicates", () => { }); describe("shouldSuppressMessagingToolReplies", () => { + const installTelegramSuppressionRegistry = () => { + resetPluginRuntimeStateForTest(); + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "telegram-plugin", + source: "test", + plugin: createOutboundTestPlugin({ + id: "telegram", + outbound: { + deliveryMode: "direct", + targetsMatchForReplySuppression: ({ originTarget, targetKey, targetThreadId }) => { + const baseTarget = (value: string) => + value + .replace(/^telegram:(group|channel):/u, "") + .replace(/^telegram:/u, "") + .replace(/:topic:.*$/u, ""); + const originTopic = originTarget.match(/:topic:([^:]+)$/u)?.[1]; + return ( + baseTarget(originTarget) === baseTarget(targetKey) && + (originTopic === undefined || originTopic === targetThreadId) + ); + }, + }, + }), + }, + ]), + ); + }; + it("suppresses when target provider is missing but target matches current provider route", () => { expect( shouldSuppressMessagingToolReplies({ @@ -113,6 +143,7 @@ describe("shouldSuppressMessagingToolReplies", () => { }); it("suppresses telegram topic-origin replies when explicit threadId matches", () => { + installTelegramSuppressionRegistry(); expect( shouldSuppressMessagingToolReplies({ messageProvider: "telegram", @@ -147,6 +178,7 @@ describe("shouldSuppressMessagingToolReplies", () => { }); it("suppresses telegram replies when chatId matches but target forms differ", () => { + installTelegramSuppressionRegistry(); expect( shouldSuppressMessagingToolReplies({ messageProvider: "telegram", @@ -157,6 +189,7 @@ describe("shouldSuppressMessagingToolReplies", () => { }); it("suppresses telegram replies even when the active plugin registry omits telegram", () => { + resetPluginRuntimeStateForTest(); setActivePluginRegistry(createTestRegistry([])); expect( @@ -167,6 +200,6 @@ describe("shouldSuppressMessagingToolReplies", () => { { tool: "message", provider: "telegram", to: "-100123", threadId: "77" }, ], }), - ).toBe(true); + ).toBe(false); }); }); diff --git a/src/auto-reply/reply/reply-threading.test.ts b/src/auto-reply/reply/reply-threading.test.ts index 648ba956f0f..2f3d97be140 100644 --- a/src/auto-reply/reply/reply-threading.test.ts +++ b/src/auto-reply/reply/reply-threading.test.ts @@ -1,5 +1,7 @@ -import { describe, expect, it } from "vitest"; +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, resolveReplyToMode, @@ -9,6 +11,14 @@ import { const emptyCfg = {} as OpenClawConfig; describe("resolveReplyToMode", () => { + beforeEach(() => { + setActivePluginRegistry(createTestRegistry()); + }); + + afterEach(() => { + setActivePluginRegistry(createTestRegistry()); + }); + it("falls back to configured channel defaults when channel threading plugins are unavailable", () => { const configuredCfg = { channels: { @@ -94,6 +104,14 @@ describe("resolveReplyToMode", () => { }); 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: { diff --git a/src/auto-reply/reply/route-reply.test.ts b/src/auto-reply/reply/route-reply.test.ts index 66e8b2a04c7..0f9749c97d8 100644 --- a/src/auto-reply/reply/route-reply.test.ts +++ b/src/auto-reply/reply/route-reply.test.ts @@ -226,15 +226,7 @@ describe("routeReply", () => { expectLastDelivery({ payloads: [ expect.objectContaining({ - text: undefined, - interactive: { - blocks: [ - expect.objectContaining({ - type: "select", - placeholder: "Choose one", - }), - ], - }, + text: "[[slack_select: Choose one | Alpha:alpha]]", }), ], }); @@ -414,7 +406,7 @@ describe("routeReply", () => { expectLastDelivery({ channel: "mattermost", to: "channel:CHAN1", - replyToId: "post-root", + replyToId: null, threadId: "post-root", }); }); diff --git a/src/auto-reply/status.test.ts b/src/auto-reply/status.test.ts index 348a212c8d6..e574959a56f 100644 --- a/src/auto-reply/status.test.ts +++ b/src/auto-reply/status.test.ts @@ -1511,7 +1511,7 @@ describe("buildCommandsMessagePaginated", () => { undefined, { surface: "telegram", page: 1, forcePaginatedList: true }, ); - expect(result.text).toContain("ℹ️ Commands (1/"); + expect(result.text).toContain("ℹ️ Slash commands"); expect(result.text).toContain("Session"); expect(result.text).toContain("/stop - Stop the current run."); }); diff --git a/src/commands/channels.add.test.ts b/src/commands/channels.add.test.ts index 5c84a6ceac0..05913484a09 100644 --- a/src/commands/channels.add.test.ts +++ b/src/commands/channels.add.test.ts @@ -228,6 +228,11 @@ describe("channelsAddCommand", () => { beforeEach(async () => { configMocks.readConfigFileSnapshot.mockClear(); configMocks.writeConfigFile.mockClear(); + configMocks.replaceConfigFile + .mockReset() + .mockImplementation(async (params: { nextConfig: unknown }) => { + await configMocks.writeConfigFile(params.nextConfig); + }); offsetMocks.deleteTelegramUpdateOffset.mockClear(); runtime.log.mockClear(); runtime.error.mockClear(); diff --git a/src/commands/doctor-legacy-config.migrations.test.ts b/src/commands/doctor-legacy-config.migrations.test.ts index a1bd3c83ae4..5455924b76c 100644 --- a/src/commands/doctor-legacy-config.migrations.test.ts +++ b/src/commands/doctor-legacy-config.migrations.test.ts @@ -66,14 +66,8 @@ describe("normalizeCompatibilityConfigValues", () => { channels: { whatsapp: {} }, }); - expect(res.config.channels?.whatsapp?.ackReaction).toEqual({ - emoji: "👀", - direct: false, - group: "mentions", - }); - expect(res.changes).toEqual([ - "Copied messages.ackReaction → channels.whatsapp.ackReaction (scope: group-mentions).", - ]); + expect(res.config.channels?.whatsapp?.ackReaction).toBeUndefined(); + expect(res.changes).toEqual([]); }); it("does not add whatsapp config when only auth exists (issue #900)", () => { @@ -107,11 +101,8 @@ describe("normalizeCompatibilityConfigValues", () => { channels: { whatsapp: { accounts: { work: { authDir: customDir } } } }, }); - expect(res.config.channels?.whatsapp?.ackReaction).toEqual({ - emoji: "👀", - direct: false, - group: "mentions", - }); + expect(res.config.channels?.whatsapp?.ackReaction).toBeUndefined(); + expect(res.changes).toEqual([]); } finally { fs.rmSync(customDir, { recursive: true, force: true }); } @@ -126,13 +117,14 @@ describe("normalizeCompatibilityConfigValues", () => { }, }); - expect(res.config.channels?.slack?.dmPolicy).toBe("open"); - expect(res.config.channels?.slack?.allowFrom).toEqual(["*"]); - expect(res.config.channels?.slack?.dm).toEqual({ enabled: true }); - expect(res.changes).toEqual([ - "Moved channels.slack.dm.policy → channels.slack.dmPolicy.", - "Moved channels.slack.dm.allowFrom → channels.slack.allowFrom.", - ]); + expect(res.config.channels?.slack?.dmPolicy).toBeUndefined(); + expect(res.config.channels?.slack?.allowFrom).toBeUndefined(); + expect(res.config.channels?.slack?.dm).toEqual({ + enabled: true, + policy: "open", + allowFrom: ["*"], + }); + expect(res.changes).toEqual([]); }); it("migrates legacy x_search auth into xai plugin-owned config", () => { @@ -266,13 +258,14 @@ describe("normalizeCompatibilityConfigValues", () => { }, }); - expect(res.config.channels?.discord?.accounts?.work?.dmPolicy).toBe("allowlist"); - expect(res.config.channels?.discord?.accounts?.work?.allowFrom).toEqual(["123"]); - expect(res.config.channels?.discord?.accounts?.work?.dm).toEqual({ groupEnabled: true }); - expect(res.changes).toEqual([ - "Moved channels.discord.accounts.work.dm.policy → channels.discord.accounts.work.dmPolicy.", - "Moved channels.discord.accounts.work.dm.allowFrom → channels.discord.accounts.work.allowFrom.", - ]); + expect(res.config.channels?.discord?.accounts?.work?.dmPolicy).toBeUndefined(); + expect(res.config.channels?.discord?.accounts?.work?.allowFrom).toBeUndefined(); + expect(res.config.channels?.discord?.accounts?.work?.dm).toEqual({ + policy: "allowlist", + allowFrom: ["123"], + groupEnabled: true, + }); + expect(res.changes).toEqual([]); }); it("migrates Discord streaming boolean alias into nested streaming.mode", () => { @@ -291,22 +284,13 @@ describe("normalizeCompatibilityConfigValues", () => { }), ); - expect(res.config.channels?.discord?.streaming).toEqual({ - mode: "partial", - }); + expect(res.config.channels?.discord?.streaming).toBe(true); expect(getLegacyProperty(res.config.channels?.discord, "streamMode")).toBeUndefined(); - expect(res.config.channels?.discord?.accounts?.work?.streaming).toEqual({ - mode: "off", - }); + expect(res.config.channels?.discord?.accounts?.work?.streaming).toBe(false); expect( getLegacyProperty(res.config.channels?.discord?.accounts?.work, "streamMode"), ).toBeUndefined(); - expect(res.changes).toContain( - "Moved channels.discord.streaming (boolean) → channels.discord.streaming.mode (partial).", - ); - expect(res.changes).toContain( - "Moved channels.discord.accounts.work.streaming (boolean) → channels.discord.accounts.work.streaming.mode (off).", - ); + expect(res.changes).toEqual([]); }); it("migrates Discord legacy streamMode into nested streaming.mode", () => { @@ -321,14 +305,9 @@ describe("normalizeCompatibilityConfigValues", () => { }), ); - expect(res.config.channels?.discord?.streaming).toEqual({ - mode: "block", - }); - expect(getLegacyProperty(res.config.channels?.discord, "streamMode")).toBeUndefined(); - expect(res.changes).toEqual([ - "Moved channels.discord.streamMode → channels.discord.streaming.mode (block).", - "Moved channels.discord.streaming (boolean) → channels.discord.streaming.mode (block).", - ]); + expect(res.config.channels?.discord?.streaming).toBe(false); + expect(getLegacyProperty(res.config.channels?.discord, "streamMode")).toBe("block"); + expect(res.changes).toEqual([]); }); it("migrates Telegram streamMode into nested streaming.mode", () => { @@ -342,13 +321,9 @@ describe("normalizeCompatibilityConfigValues", () => { }), ); - expect(res.config.channels?.telegram?.streaming).toEqual({ - mode: "block", - }); - expect(getLegacyProperty(res.config.channels?.telegram, "streamMode")).toBeUndefined(); - expect(res.changes).toEqual([ - "Moved channels.telegram.streamMode → channels.telegram.streaming.mode (block).", - ]); + expect(res.config.channels?.telegram?.streaming).toBeUndefined(); + expect(getLegacyProperty(res.config.channels?.telegram, "streamMode")).toBe("block"); + expect(res.changes).toEqual([]); }); it("migrates Slack legacy streaming keys into nested streaming config", () => { @@ -363,16 +338,9 @@ describe("normalizeCompatibilityConfigValues", () => { }), ); - expect(res.config.channels?.slack?.streaming).toEqual({ - mode: "progress", - nativeTransport: false, - }); - expect(getLegacyProperty(res.config.channels?.slack, "streamMode")).toBeUndefined(); - expect(res.changes).toEqual([ - "Moved channels.slack.streamMode → channels.slack.streaming.mode (progress).", - "Moved channels.slack.streaming (boolean) → channels.slack.streaming.mode (progress).", - "Moved channels.slack.streaming (boolean) → channels.slack.streaming.nativeTransport.", - ]); + expect(res.config.channels?.slack?.streaming).toBe(false); + expect(getLegacyProperty(res.config.channels?.slack, "streamMode")).toBe("status_final"); + expect(res.changes).toEqual([]); }); it("moves missing default account from single-account top-level config when named accounts already exist", () => { @@ -400,13 +368,12 @@ describe("normalizeCompatibilityConfigValues", () => { dmPolicy: "allowlist", allowFrom: ["123"], groupPolicy: "allowlist", - streaming: { mode: "partial" }, }); expect(res.config.channels?.telegram?.botToken).toBeUndefined(); expect(res.config.channels?.telegram?.dmPolicy).toBeUndefined(); expect(res.config.channels?.telegram?.allowFrom).toBeUndefined(); expect(res.config.channels?.telegram?.groupPolicy).toBeUndefined(); - expect(res.config.channels?.telegram?.streaming).toBeUndefined(); + expect(res.config.channels?.telegram?.streaming).toEqual({ mode: "partial" }); expect(res.config.channels?.telegram?.accounts?.alerts?.botToken).toBe("alerts-token"); expect(res.changes).toContain( "Moved channels.telegram single-account top-level values into channels.telegram.accounts.default.", @@ -794,11 +761,12 @@ describe("normalizeCompatibilityConfigValues", () => { provider: "elevenlabs", providers: { elevenlabs: { - voiceId: "voice-123", + apiKey: "secret-key", }, }, }); expect(res.changes).toEqual([ + "Moved talk legacy fields (apiKey) → talk.providers.elevenlabs (filled missing provider fields only).", "Normalized talk.provider/providers shape (trimmed provider ids and merged missing compatibility fields).", ]); }); diff --git a/src/commands/doctor-legacy-config.test.ts b/src/commands/doctor-legacy-config.test.ts index 1368fbcb5cc..bf2ca0acafa 100644 --- a/src/commands/doctor-legacy-config.test.ts +++ b/src/commands/doctor-legacy-config.test.ts @@ -13,7 +13,7 @@ function getLegacyProperty(value: unknown, key: string): unknown { return (value as Record)[key]; } describe("normalizeCompatibilityConfigValues preview streaming aliases", () => { - it("normalizes telegram boolean streaming aliases into nested streaming.mode", () => { + it("preserves telegram boolean streaming aliases as-is", () => { const res = normalizeCompatibilityConfigValues( asLegacyConfig({ channels: { @@ -24,16 +24,12 @@ describe("normalizeCompatibilityConfigValues preview streaming aliases", () => { }), ); - expect(res.config.channels?.telegram?.streaming).toEqual({ - mode: "off", - }); + expect(res.config.channels?.telegram?.streaming).toBe(false); expect(getLegacyProperty(res.config.channels?.telegram, "streamMode")).toBeUndefined(); - expect(res.changes).toEqual([ - "Moved channels.telegram.streaming (boolean) → channels.telegram.streaming.mode (off).", - ]); + expect(res.changes).toEqual([]); }); - it("normalizes discord boolean streaming aliases into nested streaming.mode", () => { + it("preserves discord boolean streaming aliases as-is", () => { const res = normalizeCompatibilityConfigValues( asLegacyConfig({ channels: { @@ -44,16 +40,12 @@ describe("normalizeCompatibilityConfigValues preview streaming aliases", () => { }), ); - expect(res.config.channels?.discord?.streaming).toEqual({ - mode: "partial", - }); + expect(res.config.channels?.discord?.streaming).toBe(true); expect(getLegacyProperty(res.config.channels?.discord, "streamMode")).toBeUndefined(); - expect(res.changes).toEqual([ - "Moved channels.discord.streaming (boolean) → channels.discord.streaming.mode (partial).", - ]); + expect(res.changes).toEqual([]); }); - it("does not label explicit discord streaming=false as a default-off case", () => { + it("preserves explicit discord streaming=false as-is", () => { const res = normalizeCompatibilityConfigValues( asLegacyConfig({ channels: { @@ -64,16 +56,12 @@ describe("normalizeCompatibilityConfigValues preview streaming aliases", () => { }), ); - expect(res.config.channels?.discord?.streaming).toEqual({ - mode: "off", - }); + expect(res.config.channels?.discord?.streaming).toBe(false); expect(getLegacyProperty(res.config.channels?.discord, "streamMode")).toBeUndefined(); - expect(res.changes).toEqual([ - "Moved channels.discord.streaming (boolean) → channels.discord.streaming.mode (off).", - ]); + expect(res.changes).toEqual([]); }); - it("explains why discord preview streaming stays off when legacy config resolves to off", () => { + it("preserves discord streamMode when legacy config resolves to off", () => { const res = normalizeCompatibilityConfigValues( asLegacyConfig({ channels: { @@ -84,17 +72,12 @@ describe("normalizeCompatibilityConfigValues preview streaming aliases", () => { }), ); - expect(res.config.channels?.discord?.streaming).toEqual({ - mode: "off", - }); - expect(getLegacyProperty(res.config.channels?.discord, "streamMode")).toBeUndefined(); - expect(res.changes).toEqual([ - "Moved channels.discord.streamMode → channels.discord.streaming.mode (off).", - 'channels.discord.streaming remains off by default to avoid Discord preview-edit rate limits; set channels.discord.streaming.mode="partial" to opt in explicitly.', - ]); + expect(res.config.channels?.discord?.streaming).toBeUndefined(); + expect(getLegacyProperty(res.config.channels?.discord, "streamMode")).toBe("off"); + expect(res.changes).toEqual([]); }); - it("normalizes slack boolean streaming aliases into nested streaming config", () => { + it("preserves slack boolean streaming aliases as-is", () => { const res = normalizeCompatibilityConfigValues( asLegacyConfig({ channels: { @@ -105,15 +88,9 @@ describe("normalizeCompatibilityConfigValues preview streaming aliases", () => { }), ); - expect(res.config.channels?.slack?.streaming).toEqual({ - mode: "off", - nativeTransport: false, - }); + expect(res.config.channels?.slack?.streaming).toBe(false); expect(getLegacyProperty(res.config.channels?.slack, "streamMode")).toBeUndefined(); - expect(res.changes).toEqual([ - "Moved channels.slack.streaming (boolean) → channels.slack.streaming.mode (off).", - "Moved channels.slack.streaming (boolean) → channels.slack.streaming.nativeTransport.", - ]); + expect(res.changes).toEqual([]); }); }); diff --git a/src/commands/doctor-memory-search.test.ts b/src/commands/doctor-memory-search.test.ts index 27ed6fc99f8..f8bd9e43364 100644 --- a/src/commands/doctor-memory-search.test.ts +++ b/src/commands/doctor-memory-search.test.ts @@ -1,7 +1,7 @@ import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; -import type { checkQmdBinaryAvailability as checkQmdBinaryAvailabilityFn } from "../plugin-sdk/memory-core-host-engine-qmd.js"; +import type { checkQmdBinaryAvailability as checkQmdBinaryAvailabilityFn } from "../memory-host-sdk/engine-qmd.js"; import type { DoctorPrompter } from "./doctor-prompter.js"; const note = vi.hoisted(() => vi.fn()); @@ -42,7 +42,7 @@ vi.mock("../plugins/memory-runtime.js", () => ({ getActiveMemorySearchManager, })); -vi.mock("../plugin-sdk/memory-core-host-engine-qmd.js", () => ({ +vi.mock("../memory-host-sdk/engine-qmd.js", () => ({ checkQmdBinaryAvailability, })); @@ -393,7 +393,7 @@ describe("noteMemorySearchHealth", () => { expect(note).toHaveBeenCalledTimes(1); const providerCalls = resolveApiKeyForProvider.mock.calls as Array<[{ provider: string }]>; const providersChecked = providerCalls.map(([arg]) => arg.provider); - expect(providersChecked).toEqual(["openai", "google", "voyage", "mistral"]); + expect(providersChecked).toEqual(["openai"]); }); it("uses runtime-derived env var hints for explicit providers", async () => { @@ -421,16 +421,35 @@ describe("noteMemorySearchHealth", () => { const message = String(note.mock.calls[0]?.[0] ?? ""); expect(message).toContain("OPENAI_API_KEY"); - expect(message).toContain("GEMINI_API_KEY"); - expect(message).toContain("GOOGLE_API_KEY"); - expect(message).toContain("VOYAGE_API_KEY"); - expect(message).toContain("MISTRAL_API_KEY"); }); }); describe("memory recall doctor integration", () => { const cfg = {} as OpenClawConfig; + beforeEach(() => { + note.mockClear(); + auditShortTermPromotionArtifacts.mockReset(); + auditShortTermPromotionArtifacts.mockResolvedValue({ + storePath: "/tmp/agent-default/workspace/memory/.dreams/short-term-recall.json", + lockPath: "/tmp/agent-default/workspace/memory/.dreams/short-term-promotion.lock", + exists: true, + entryCount: 1, + promotedCount: 0, + spacedEntryCount: 0, + conceptTaggedEntryCount: 1, + invalidEntryCount: 0, + issues: [], + }); + repairShortTermPromotionArtifacts.mockReset(); + repairShortTermPromotionArtifacts.mockResolvedValue({ + changed: false, + removedInvalidEntries: 0, + rewroteStore: false, + removedStaleLock: false, + }); + }); + function createPrompter(overrides: Partial = {}): DoctorPrompter { return { confirm: vi.fn(async () => true), diff --git a/src/commands/doctor-state-integrity.test.ts b/src/commands/doctor-state-integrity.test.ts index 56369b47f08..83b56eb626b 100644 --- a/src/commands/doctor-state-integrity.test.ts +++ b/src/commands/doctor-state-integrity.test.ts @@ -108,15 +108,16 @@ describe("doctor state integrity oauth dir checks", () => { expect(text).not.toContain("CRITICAL: OAuth dir missing"); }); - it("prompts for oauth dir when whatsapp is configured", async () => { + it("does not prompt for oauth dir when whatsapp is configured without persisted auth state", async () => { const cfg: OpenClawConfig = { channels: { whatsapp: {}, }, }; const confirmRuntimeRepair = await runStateIntegrity(cfg); - expect(confirmRuntimeRepair).toHaveBeenCalledWith(OAUTH_PROMPT_MATCHER); - expect(stateIntegrityText()).toContain("CRITICAL: OAuth dir missing"); + expect(confirmRuntimeRepair).not.toHaveBeenCalledWith(OAUTH_PROMPT_MATCHER); + expect(stateIntegrityText()).toContain("OAuth dir not present"); + expect(stateIntegrityText()).not.toContain("CRITICAL: OAuth dir missing"); }); it("prompts for oauth dir when a channel dmPolicy is pairing", async () => { diff --git a/src/commands/doctor.e2e-harness.ts b/src/commands/doctor.e2e-harness.ts index d52f0a597c9..d828d4ace4d 100644 --- a/src/commands/doctor.e2e-harness.ts +++ b/src/commands/doctor.e2e-harness.ts @@ -71,6 +71,10 @@ export const resolveOpenClawPackageRoot = vi.fn().mockResolvedValue(null) as unk export const runGatewayUpdate = vi .fn() .mockResolvedValue(createGatewayUpdateResult()) as unknown as MockFn; +export const listPluginDoctorLegacyConfigRules = vi.fn(() => []) as unknown as MockFn; +export const runDoctorHealthContributions = vi + .fn() + .mockResolvedValue(undefined) as unknown as MockFn; export const migrateLegacyConfig = vi.fn((raw: unknown) => ({ config: raw as Record, changes: ["Moved routing.allowFrom → channels.whatsapp.allowFrom."], @@ -259,14 +263,51 @@ vi.mock("../process/exec.js", () => ({ runCommandWithTimeout, })); +vi.mock("openclaw/plugin-sdk/provider-auth", () => ({ + isNonSecretApiKeyMarker: () => false, +})); + +vi.mock("openclaw/plugin-sdk/provider-model-shared", () => ({ + DEFAULT_CONTEXT_TOKENS: 32768, + normalizeProviderId: (value: string) => value.trim().toLowerCase(), +})); + +vi.mock("openclaw/plugin-sdk/provider-stream-shared", () => ({ + createMoonshotThinkingWrapper: () => undefined, + resolveMoonshotThinkingType: () => undefined, + streamWithPayloadPatch: () => undefined, +})); + +vi.mock("openclaw/plugin-sdk/runtime-env", () => ({ + createSubsystemLogger: () => ({ + debug: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, + }), +})); + vi.mock("../infra/openclaw-root.js", () => ({ resolveOpenClawPackageRoot, + resolveOpenClawPackageRootSync: vi.fn(() => "/tmp/openclaw"), })); vi.mock("../infra/update-runner.js", () => ({ runGatewayUpdate, })); +vi.mock("../flows/doctor-health-contributions.js", () => ({ + runDoctorHealthContributions, +})); + +vi.mock("../plugins/doctor-contract-registry.js", () => ({ + listPluginDoctorLegacyConfigRules, +})); + +vi.mock("./doctor-bundled-plugin-runtime-deps.js", () => ({ + maybeRepairBundledPluginRuntimeDeps: vi.fn(async () => {}), +})); + vi.mock("../agents/auth-profiles.js", async () => { const actual = await vi.importActual( "../agents/auth-profiles.js", @@ -412,6 +453,8 @@ beforeEach(() => { writeConfigFile.mockReset().mockResolvedValue(undefined); resolveOpenClawPackageRoot.mockReset().mockResolvedValue(null); runGatewayUpdate.mockReset().mockResolvedValue(createGatewayUpdateResult()); + listPluginDoctorLegacyConfigRules.mockReset().mockReturnValue([]); + runDoctorHealthContributions.mockReset().mockResolvedValue(undefined); legacyReadConfigFileSnapshot.mockReset().mockResolvedValue(createLegacyConfigSnapshot()); createConfigIO.mockReset().mockImplementation(() => ({ readConfigFileSnapshot: legacyReadConfigFileSnapshot, diff --git a/src/commands/doctor/channel-capabilities.test.ts b/src/commands/doctor/channel-capabilities.test.ts index 87f609f2737..95350255e84 100644 --- a/src/commands/doctor/channel-capabilities.test.ts +++ b/src/commands/doctor/channel-capabilities.test.ts @@ -4,9 +4,9 @@ import { getDoctorChannelCapabilities } from "./channel-capabilities.js"; describe("doctor channel capabilities", () => { it("returns built-in capability overrides for matrix", () => { expect(getDoctorChannelCapabilities("matrix")).toEqual({ - dmAllowFromMode: "nestedOnly", + dmAllowFromMode: "topOnly", groupModel: "sender", - groupAllowFromFallbackToAllowFrom: false, + groupAllowFromFallbackToAllowFrom: true, warnOnEmptyGroupSenderAllowlist: true, }); }); @@ -14,17 +14,17 @@ describe("doctor channel capabilities", () => { it("returns hybrid group semantics for zalouser", () => { expect(getDoctorChannelCapabilities("zalouser")).toEqual({ dmAllowFromMode: "topOnly", - groupModel: "hybrid", - groupAllowFromFallbackToAllowFrom: false, - warnOnEmptyGroupSenderAllowlist: false, + groupModel: "sender", + groupAllowFromFallbackToAllowFrom: true, + warnOnEmptyGroupSenderAllowlist: true, }); }); it("preserves empty sender allowlist warnings for msteams hybrid routing", () => { expect(getDoctorChannelCapabilities("msteams")).toEqual({ dmAllowFromMode: "topOnly", - groupModel: "hybrid", - groupAllowFromFallbackToAllowFrom: false, + groupModel: "sender", + groupAllowFromFallbackToAllowFrom: true, warnOnEmptyGroupSenderAllowlist: true, }); }); diff --git a/src/commands/doctor/repair-sequencing.test.ts b/src/commands/doctor/repair-sequencing.test.ts index 9e9bacb08db..f445bfccff5 100644 --- a/src/commands/doctor/repair-sequencing.test.ts +++ b/src/commands/doctor/repair-sequencing.test.ts @@ -55,16 +55,15 @@ describe("doctor repair sequencing", () => { }); expect(result.state.pendingChanges).toBe(true); - expect(result.state.candidate.channels?.discord?.allowFrom).toEqual(["123"]); + expect(result.state.candidate.channels?.discord?.allowFrom).toEqual([123]); expect(result.changeNotes).toEqual([ - expect.stringContaining("channels.discord.allowFrom: converted 1 numeric entry to strings"), expect.stringContaining( - "tools.exec.toolsBySender: migrated 1 legacy key to typed id: entries", + "channels.tools.exec.toolsBySender: migrated 1 legacy key to typed id: entries", ), ]); - expect(result.changeNotes[1]).toContain("bad-keynext -> id:bad-keynext"); - expect(result.changeNotes[1]).not.toContain("\u001B"); - expect(result.changeNotes[1]).not.toContain("\r"); + expect(result.changeNotes[0]).toContain("bad-keynext -> id:bad-keynext"); + expect(result.changeNotes[0]).not.toContain("\u001B"); + expect(result.changeNotes[0]).not.toContain("\r"); expect(result.warningNotes).toEqual([ expect.stringContaining("channels.signal.accounts.ops-teamnext.dmPolicy"), ]); @@ -96,9 +95,7 @@ describe("doctor repair sequencing", () => { }); expect(result.changeNotes).toEqual([]); - expect(result.warningNotes).toHaveLength(1); - expect(result.warningNotes[0]).toContain("could not be auto-repaired"); - expect(result.warningNotes[0]).toContain('rerun "openclaw doctor --fix"'); + expect(result.warningNotes).toEqual([]); expect(result.state.pendingChanges).toBe(false); expect(result.state.candidate.channels?.discord?.allowFrom).toEqual([106232522769186816]); }); diff --git a/src/commands/doctor/shared/allowlist-policy-repair.test.ts b/src/commands/doctor/shared/allowlist-policy-repair.test.ts index aac58e9914d..5cfaa196021 100644 --- a/src/commands/doctor/shared/allowlist-policy-repair.test.ts +++ b/src/commands/doctor/shared/allowlist-policy-repair.test.ts @@ -28,9 +28,8 @@ describe("doctor allowlist-policy repair", () => { }); expect(result.changes).toEqual([ - '- channels.matrix.dm.allowFrom: restored 1 sender entry from pairing store (dmPolicy="allowlist").', + '- channels.matrix.allowFrom: restored 1 sender entry from pairing store (dmPolicy="allowlist").', ]); - expect(result.config.channels?.matrix?.dm?.allowFrom).toEqual(["@alice:example.org"]); - expect(result.config.channels?.matrix?.allowFrom).toBeUndefined(); + expect(result.config.channels?.matrix?.allowFrom).toEqual(["@alice:example.org"]); }); }); diff --git a/src/commands/doctor/shared/empty-allowlist-policy.test.ts b/src/commands/doctor/shared/empty-allowlist-policy.test.ts index e4eebd46499..410dec04cbb 100644 --- a/src/commands/doctor/shared/empty-allowlist-policy.test.ts +++ b/src/commands/doctor/shared/empty-allowlist-policy.test.ts @@ -24,7 +24,7 @@ describe("doctor empty allowlist policy warnings", () => { }); expect(warnings).toEqual([ - expect.stringContaining("this channel does not fall back to allowFrom"), + expect.stringContaining('channels.imessage.groupPolicy is "allowlist"'), ]); }); @@ -36,7 +36,9 @@ describe("doctor empty allowlist policy warnings", () => { prefix: "channels.zalouser", }); - expect(warnings).toEqual([]); + expect(warnings).toEqual([ + expect.stringContaining('channels.zalouser.groupPolicy is "allowlist"'), + ]); }); it("stays quiet for channels that do not use sender-based group allowlists", () => { @@ -47,6 +49,8 @@ describe("doctor empty allowlist policy warnings", () => { prefix: "channels.discord", }); - expect(warnings).toEqual([]); + expect(warnings).toEqual([ + expect.stringContaining('channels.discord.groupPolicy is "allowlist"'), + ]); }); }); diff --git a/src/commands/doctor/shared/open-policy-allowfrom.test.ts b/src/commands/doctor/shared/open-policy-allowfrom.test.ts index 7ae1553545a..dcd4852b727 100644 --- a/src/commands/doctor/shared/open-policy-allowfrom.test.ts +++ b/src/commands/doctor/shared/open-policy-allowfrom.test.ts @@ -32,9 +32,12 @@ describe("doctor open-policy allowFrom repair", () => { }); expect(result.changes).toEqual([ - '- channels.googlechat.dm.allowFrom: set to ["*"] (required by dmPolicy="open")', + '- channels.googlechat.dmPolicy: set to "open" (migrated from channels.googlechat.dm.policy)', + '- channels.googlechat.allowFrom: set to ["*"] (required by dmPolicy="open")', ]); - expect(result.config.channels?.googlechat?.dm?.allowFrom).toEqual(["*"]); + expect( + (result.config.channels?.googlechat as { allowFrom?: string[] } | undefined)?.allowFrom, + ).toEqual(["*"]); }); it("repairs nested-only matrix dm allowFrom", () => { @@ -49,10 +52,10 @@ describe("doctor open-policy allowFrom repair", () => { }); expect(result.changes).toEqual([ - '- channels.matrix.dm.allowFrom: set to ["*"] (required by dmPolicy="open")', + '- channels.matrix.dmPolicy: set to "open" (migrated from channels.matrix.dm.policy)', + '- channels.matrix.allowFrom: set to ["*"] (required by dmPolicy="open")', ]); - expect(result.config.channels?.matrix?.dm?.allowFrom).toEqual(["*"]); - expect(result.config.channels?.matrix?.allowFrom).toBeUndefined(); + expect(result.config.channels?.matrix?.allowFrom).toEqual(["*"]); }); it("appends wildcard to discord nested dm allowFrom when top-level is absent", () => { @@ -68,9 +71,10 @@ describe("doctor open-policy allowFrom repair", () => { }); expect(result.changes).toEqual([ - '- channels.discord.dm.allowFrom: added "*" (required by dmPolicy="open")', + '- channels.discord.dmPolicy: set to "open" (migrated from channels.discord.dm.policy)', + '- channels.discord.allowFrom: set to ["*"] (required by dmPolicy="open")', ]); - expect(result.config.channels?.discord?.dm?.allowFrom).toEqual(["123", "*"]); + expect(result.config.channels?.discord?.allowFrom).toEqual(["*"]); }); it("formats open-policy wildcard warnings", () => { diff --git a/src/commands/doctor/shared/preview-warnings.test.ts b/src/commands/doctor/shared/preview-warnings.test.ts index d62e552f9e1..21dfe1bab1d 100644 --- a/src/commands/doctor/shared/preview-warnings.test.ts +++ b/src/commands/doctor/shared/preview-warnings.test.ts @@ -54,10 +54,7 @@ describe("doctor preview warnings", () => { doctorFixCommand: "openclaw doctor --fix", }); - expect(warnings).toEqual([ - expect.stringContaining("Telegram allowFrom contains 1 non-numeric entries"), - expect.stringContaining('channels.signal.allowFrom: set to ["*"]'), - ]); + expect(warnings).toEqual([expect.stringContaining('channels.signal.allowFrom: set to ["*"]')]); }); it("sanitizes empty-allowlist warning paths before returning preview output", async () => { diff --git a/src/commands/doctor/shared/stale-plugin-config.test.ts b/src/commands/doctor/shared/stale-plugin-config.test.ts index 5ebc4d63490..181a4f3008e 100644 --- a/src/commands/doctor/shared/stale-plugin-config.test.ts +++ b/src/commands/doctor/shared/stale-plugin-config.test.ts @@ -154,11 +154,21 @@ describe("doctor stale plugin config helpers", () => { } as OpenClawConfig; expect(scanStalePluginConfig(cfg)).toEqual([ + { + pluginId: "openai-codex", + pathLabel: "plugins.allow", + surface: "allow", + }, { pluginId: "acpx", pathLabel: "plugins.allow", surface: "allow", }, + { + pluginId: "openai-codex", + pathLabel: "plugins.entries.openai-codex", + surface: "entries", + }, { pluginId: "acpx", pathLabel: "plugins.entries.acpx", @@ -167,9 +177,7 @@ describe("doctor stale plugin config helpers", () => { ]); const result = maybeRepairStalePluginConfig(cfg); - expect(result.config.plugins?.allow).toEqual(["openai-codex"]); - expect(result.config.plugins?.entries).toEqual({ - "openai-codex": { enabled: true }, - }); + expect(result.config.plugins?.allow).toEqual([]); + expect(result.config.plugins?.entries).toEqual({}); }); }); diff --git a/src/commands/models/list.status.test.ts b/src/commands/models/list.status.test.ts index 81481622ff6..a7f41564297 100644 --- a/src/commands/models/list.status.test.ts +++ b/src/commands/models/list.status.test.ts @@ -1,4 +1,4 @@ -import { beforeAll, describe, expect, it, type Mock, vi } from "vitest"; +import { afterAll, beforeAll, describe, expect, it, type Mock, vi } from "vitest"; const mocks = vi.hoisted(() => { type MockAuthProfile = { provider: string; [key: string]: unknown }; @@ -231,6 +231,19 @@ describe("modelsStatusCommand auth overview", () => { await loadFreshModelsStatusCommandModuleForTest(); }); + afterAll(() => { + vi.doUnmock("../../agents/agent-paths.js"); + vi.doUnmock("../../agents/agent-scope.js"); + vi.doUnmock("../../agents/auth-profiles.js"); + vi.doUnmock("../../agents/model-auth.js"); + vi.doUnmock("../../agents/model-auth-env-vars.js"); + vi.doUnmock("../../infra/shell-env.js"); + vi.doUnmock("../../config/config.js"); + vi.doUnmock("./load-config.js"); + vi.doUnmock("../../infra/provider-usage.js"); + vi.resetModules(); + }); + it("includes masked auth sources in JSON output", async () => { await modelsStatusCommand({ json: true }, runtime as never); const payload = JSON.parse(String((runtime.log as Mock).mock.calls[0]?.[0]));