From dab0e97c222c4dc3faebfd3f7fef219ea965041a Mon Sep 17 00:00:00 2001 From: Tars Date: Sun, 8 Mar 2026 04:30:53 +0800 Subject: [PATCH 01/10] fix(models): support minimax-portal coding plan vlm routing for image tool (openclaw#33953) Verified: - pnpm install --frozen-lockfile - pnpm build - pnpm check - pnpm test:macmini Co-authored-by: tars90percent <252094836+tars90percent@users.noreply.github.com> --- CHANGELOG.md | 1 + .../minimax-vlm.normalizes-api-key.test.ts | 11 ++ src/agents/minimax-vlm.ts | 8 ++ .../models-config.providers.nvidia.test.ts | 19 ++- src/agents/models-config.providers.ts | 9 +- src/agents/tools/image-tool.test.ts | 26 ++++ src/agents/tools/image-tool.ts | 8 +- src/media-understanding/defaults.test.ts | 14 ++ src/media-understanding/defaults.ts | 2 + .../providers/image.test.ts | 133 ++++++++++++++++++ src/media-understanding/providers/image.ts | 4 +- .../providers/index.test.ts | 8 ++ src/media-understanding/providers/index.ts | 3 +- .../providers/minimax/index.ts | 6 + src/telegram/sticker-cache.ts | 17 +-- 15 files changed, 246 insertions(+), 23 deletions(-) create mode 100644 src/media-understanding/providers/image.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 71a864bdd7a..89a67e187d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -134,6 +134,7 @@ Docs: https://docs.openclaw.ai - Routing/legacy route guard tightening: require legacy session-key channel hints to match the saved delivery channel before inheriting external routing metadata, preventing custom namespaced keys like `agent::work:` from inheriting stale non-webchat routes. - Gateway/internal client routing continuity: prevent webchat/TUI/UI turns from inheriting stale external reply routes by requiring explicit `deliver: true` for external delivery, keeping main-session external inheritance scoped to non-Webchat/UI clients, and honoring configured `session.mainKey` when identifying main-session continuity. (from #35321, #34635, #35356) Thanks @alexyyyander and @Octane0411. - Security/auth labels: remove token and API-key snippets from user-facing auth status labels so `/status` and `/models` do not expose credential fragments. (#33262) thanks @cu1ch3n. +- Models/MiniMax portal vision routing: add `MiniMax-VL-01` to the `minimax-portal` provider, route portal image understanding through the MiniMax VLM endpoint, and align media auto-selection plus Telegram sticker description with the shared portal image provider path. (#33953) Thanks @tars90percent. - Auth/credential semantics: align profile eligibility + probe diagnostics with SecretRef/expiry rules and harden browser download atomic writes. (#33733) thanks @joshavant. - Security/audit denyCommands guidance: suggest likely exact node command IDs for unknown `gateway.nodes.denyCommands` entries so ineffective denylist entries are easier to correct. (#29713) thanks @liquidhorizon88-bot. - Agents/overload failover handling: classify overloaded provider failures separately from rate limits/status timeouts, add short overload backoff before retry/failover, record overloaded prompt/assistant failures as transient auth-profile cooldowns (with probeable same-provider fallback) instead of treating them like persistent auth/billing failures, and keep one-shot cron retry classification aligned so overloaded fallback summaries still count as transient retries. diff --git a/src/agents/minimax-vlm.normalizes-api-key.test.ts b/src/agents/minimax-vlm.normalizes-api-key.test.ts index faa33b8682c..146f90bbb62 100644 --- a/src/agents/minimax-vlm.normalizes-api-key.test.ts +++ b/src/agents/minimax-vlm.normalizes-api-key.test.ts @@ -45,3 +45,14 @@ describe("minimaxUnderstandImage apiKey normalization", () => { await runNormalizationCase("minimax-\u0417\u2502test-key"); }); }); + +describe("isMinimaxVlmModel", () => { + it("only matches the canonical MiniMax VLM model id", async () => { + const { isMinimaxVlmModel } = await import("./minimax-vlm.js"); + + expect(isMinimaxVlmModel("minimax", "MiniMax-VL-01")).toBe(true); + expect(isMinimaxVlmModel("minimax-portal", "MiniMax-VL-01")).toBe(true); + expect(isMinimaxVlmModel("minimax-portal", "custom-vision")).toBe(false); + expect(isMinimaxVlmModel("openai", "MiniMax-VL-01")).toBe(false); + }); +}); diff --git a/src/agents/minimax-vlm.ts b/src/agents/minimax-vlm.ts index c167936189e..6a86dcc87a2 100644 --- a/src/agents/minimax-vlm.ts +++ b/src/agents/minimax-vlm.ts @@ -6,6 +6,14 @@ type MinimaxBaseResp = { status_msg?: string; }; +export function isMinimaxVlmProvider(provider: string): boolean { + return provider === "minimax" || provider === "minimax-portal"; +} + +export function isMinimaxVlmModel(provider: string, modelId: string): boolean { + return isMinimaxVlmProvider(provider) && modelId.trim() === "MiniMax-VL-01"; +} + function coerceApiHost(params: { apiHost?: string; modelBaseUrl?: string; diff --git a/src/agents/models-config.providers.nvidia.test.ts b/src/agents/models-config.providers.nvidia.test.ts index 02086283c84..fe61b343369 100644 --- a/src/agents/models-config.providers.nvidia.test.ts +++ b/src/agents/models-config.providers.nvidia.test.ts @@ -71,10 +71,9 @@ describe("MiniMax implicit provider (#15275)", () => { "minimax-portal:default": { type: "oauth", provider: "minimax-portal", - oauth: { - access: "token", - expires: Date.now() + 60_000, - }, + access: "token", + refresh: "refresh-token", + expires: Date.now() + 60_000, }, }, }, @@ -87,6 +86,18 @@ describe("MiniMax implicit provider (#15275)", () => { const providers = await resolveImplicitProviders({ agentDir }); expect(providers?.["minimax-portal"]?.authHeader).toBe(true); }); + + it("should include minimax portal provider when MINIMAX_OAUTH_TOKEN is configured", async () => { + const agentDir = mkdtempSync(join(tmpdir(), "openclaw-test-")); + await withEnvAsync({ MINIMAX_OAUTH_TOKEN: "portal-token" }, async () => { + const providers = await resolveImplicitProviders({ agentDir }); + expect(providers?.["minimax-portal"]).toBeDefined(); + expect(providers?.["minimax-portal"]?.authHeader).toBe(true); + expect(providers?.["minimax-portal"]?.models?.some((m) => m.id === "MiniMax-VL-01")).toBe( + true, + ); + }); + }); }); describe("vLLM provider", () => { diff --git a/src/agents/models-config.providers.ts b/src/agents/models-config.providers.ts index a7d42fb7696..985b82c6ef2 100644 --- a/src/agents/models-config.providers.ts +++ b/src/agents/models-config.providers.ts @@ -771,6 +771,12 @@ function buildMinimaxPortalProvider(): ProviderConfig { api: "anthropic-messages", authHeader: true, models: [ + buildMinimaxModel({ + id: MINIMAX_DEFAULT_VISION_MODEL_ID, + name: "MiniMax VL 01", + reasoning: false, + input: ["text", "image"], + }), buildMinimaxTextModel({ id: MINIMAX_DEFAULT_MODEL_ID, name: "MiniMax M2.5", @@ -1116,8 +1122,9 @@ export async function resolveImplicitProviders(params: { providers.minimax = { ...buildMinimaxProvider(), apiKey: minimaxKey }; } + const minimaxPortalEnvKey = resolveEnvApiKeyVarName("minimax-portal"); const minimaxOauthProfile = listProfilesForProvider(authStore, "minimax-portal"); - if (minimaxOauthProfile.length > 0) { + if (minimaxPortalEnvKey || minimaxOauthProfile.length > 0) { providers["minimax-portal"] = { ...buildMinimaxPortalProvider(), apiKey: MINIMAX_OAUTH_MARKER, diff --git a/src/agents/tools/image-tool.test.ts b/src/agents/tools/image-tool.test.ts index 66f985c1cac..78a7754e84a 100644 --- a/src/agents/tools/image-tool.test.ts +++ b/src/agents/tools/image-tool.test.ts @@ -273,6 +273,32 @@ describe("image tool implicit imageModel config", () => { }); }); + it("pairs minimax-portal primary with MiniMax-VL-01 (and fallbacks) when auth exists", async () => { + await withTempAgentDir(async (agentDir) => { + await writeAuthProfiles(agentDir, { + version: 1, + profiles: { + "minimax-portal:default": { + type: "oauth", + provider: "minimax-portal", + access: "oauth-test", + refresh: "refresh-test", + expires: Date.now() + 60_000, + }, + }, + }); + vi.stubEnv("OPENAI_API_KEY", "openai-test"); + vi.stubEnv("ANTHROPIC_API_KEY", "anthropic-test"); + const cfg: OpenClawConfig = { + agents: { defaults: { model: { primary: "minimax-portal/MiniMax-M2.5" } } }, + }; + expect(resolveImageModelConfigForTool({ cfg, agentDir })).toEqual( + createDefaultImageFallbackExpectation("minimax-portal/MiniMax-VL-01"), + ); + expect(createImageTool({ config: cfg, agentDir })).not.toBeNull(); + }); + }); + it("pairs zai primary with glm-4.6v (and fallbacks) when auth exists", async () => { await withTempAgentDir(async (agentDir) => { vi.stubEnv("ZAI_API_KEY", "zai-test"); diff --git a/src/agents/tools/image-tool.ts b/src/agents/tools/image-tool.ts index 3046098ab4f..c1e9537d8c5 100644 --- a/src/agents/tools/image-tool.ts +++ b/src/agents/tools/image-tool.ts @@ -3,7 +3,7 @@ import { Type } from "@sinclair/typebox"; import type { OpenClawConfig } from "../../config/config.js"; import { resolveUserPath } from "../../utils.js"; import { loadWebMedia } from "../../web/media.js"; -import { minimaxUnderstandImage } from "../minimax-vlm.js"; +import { isMinimaxVlmModel, isMinimaxVlmProvider, minimaxUnderstandImage } from "../minimax-vlm.js"; import { coerceImageAssistantText, coerceImageModelConfig, @@ -110,8 +110,8 @@ export function resolveImageModelConfigForTool(params: { let preferred: string | null = null; // MiniMax users: always try the canonical vision model first when auth exists. - if (primary.provider === "minimax" && providerOk) { - preferred = "minimax/MiniMax-VL-01"; + if (isMinimaxVlmProvider(primary.provider) && providerOk) { + preferred = `${primary.provider}/MiniMax-VL-01`; } else if (providerOk && providerVisionFromConfig) { preferred = providerVisionFromConfig; } else if (primary.provider === "zai" && providerOk) { @@ -229,7 +229,7 @@ async function runImagePrompt(params: { }); // MiniMax VLM only supports a single image; use the first one. - if (model.provider === "minimax") { + if (isMinimaxVlmModel(model.provider, model.id)) { const first = params.images[0]; const imageDataUrl = `data:${first.mimeType};base64,${first.base64}`; const text = await minimaxUnderstandImage({ diff --git a/src/media-understanding/defaults.test.ts b/src/media-understanding/defaults.test.ts index f7bc540b104..1670d4bdf6a 100644 --- a/src/media-understanding/defaults.test.ts +++ b/src/media-understanding/defaults.test.ts @@ -1,8 +1,10 @@ import { describe, expect, it } from "vitest"; import { AUTO_AUDIO_KEY_PROVIDERS, + AUTO_IMAGE_KEY_PROVIDERS, AUTO_VIDEO_KEY_PROVIDERS, DEFAULT_AUDIO_MODELS, + DEFAULT_IMAGE_MODELS, } from "./defaults.js"; describe("DEFAULT_AUDIO_MODELS", () => { @@ -22,3 +24,15 @@ describe("AUTO_VIDEO_KEY_PROVIDERS", () => { expect(AUTO_VIDEO_KEY_PROVIDERS).toContain("moonshot"); }); }); + +describe("AUTO_IMAGE_KEY_PROVIDERS", () => { + it("includes minimax-portal auto key resolution", () => { + expect(AUTO_IMAGE_KEY_PROVIDERS).toContain("minimax-portal"); + }); +}); + +describe("DEFAULT_IMAGE_MODELS", () => { + it("includes the MiniMax portal vision default", () => { + expect(DEFAULT_IMAGE_MODELS["minimax-portal"]).toBe("MiniMax-VL-01"); + }); +}); diff --git a/src/media-understanding/defaults.ts b/src/media-understanding/defaults.ts index cac7dbf5271..a7c0d76d021 100644 --- a/src/media-understanding/defaults.ts +++ b/src/media-understanding/defaults.ts @@ -46,6 +46,7 @@ export const AUTO_IMAGE_KEY_PROVIDERS = [ "anthropic", "google", "minimax", + "minimax-portal", "zai", ] as const; export const AUTO_VIDEO_KEY_PROVIDERS = ["google", "moonshot"] as const; @@ -54,6 +55,7 @@ export const DEFAULT_IMAGE_MODELS: Record = { anthropic: "claude-opus-4-6", google: "gemini-3-flash-preview", minimax: "MiniMax-VL-01", + "minimax-portal": "MiniMax-VL-01", zai: "glm-4.6v", }; export const CLI_OUTPUT_MAX_BUFFER = 5 * MB; diff --git a/src/media-understanding/providers/image.test.ts b/src/media-understanding/providers/image.test.ts new file mode 100644 index 00000000000..948f4c74d11 --- /dev/null +++ b/src/media-understanding/providers/image.test.ts @@ -0,0 +1,133 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const completeMock = vi.fn(); +const minimaxUnderstandImageMock = vi.fn(); +const ensureOpenClawModelsJsonMock = vi.fn(async () => {}); +const getApiKeyForModelMock = vi.fn(async () => ({ + apiKey: "oauth-test", + source: "test", + mode: "oauth", +})); +const requireApiKeyMock = vi.fn((auth: { apiKey?: string }) => auth.apiKey ?? ""); +const setRuntimeApiKeyMock = vi.fn(); +const discoverModelsMock = vi.fn(); + +vi.mock("@mariozechner/pi-ai", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + complete: completeMock, + }; +}); + +vi.mock("../../agents/minimax-vlm.js", () => ({ + isMinimaxVlmProvider: (provider: string) => + provider === "minimax" || provider === "minimax-portal", + isMinimaxVlmModel: (provider: string, modelId: string) => + (provider === "minimax" || provider === "minimax-portal") && modelId === "MiniMax-VL-01", + minimaxUnderstandImage: minimaxUnderstandImageMock, +})); + +vi.mock("../../agents/models-config.js", () => ({ + ensureOpenClawModelsJson: ensureOpenClawModelsJsonMock, +})); + +vi.mock("../../agents/model-auth.js", () => ({ + getApiKeyForModel: getApiKeyForModelMock, + requireApiKey: requireApiKeyMock, +})); + +vi.mock("../../agents/pi-model-discovery-runtime.js", () => ({ + discoverAuthStorage: () => ({ + setRuntimeApiKey: setRuntimeApiKeyMock, + }), + discoverModels: discoverModelsMock, +})); + +describe("describeImageWithModel", () => { + beforeEach(() => { + vi.clearAllMocks(); + minimaxUnderstandImageMock.mockResolvedValue("portal ok"); + discoverModelsMock.mockReturnValue({ + find: vi.fn(() => ({ + provider: "minimax-portal", + id: "MiniMax-VL-01", + input: ["text", "image"], + baseUrl: "https://api.minimax.io/anthropic", + })), + }); + }); + + it("routes minimax-portal image models through the MiniMax VLM endpoint", async () => { + const { describeImageWithModel } = await import("./image.js"); + + const result = await describeImageWithModel({ + cfg: {}, + agentDir: "/tmp/openclaw-agent", + provider: "minimax-portal", + model: "MiniMax-VL-01", + buffer: Buffer.from("png-bytes"), + fileName: "image.png", + mime: "image/png", + prompt: "Describe the image.", + timeoutMs: 1000, + }); + + expect(result).toEqual({ + text: "portal ok", + model: "MiniMax-VL-01", + }); + expect(ensureOpenClawModelsJsonMock).toHaveBeenCalled(); + expect(getApiKeyForModelMock).toHaveBeenCalled(); + expect(requireApiKeyMock).toHaveBeenCalled(); + expect(setRuntimeApiKeyMock).toHaveBeenCalledWith("minimax-portal", "oauth-test"); + expect(minimaxUnderstandImageMock).toHaveBeenCalledWith({ + apiKey: "oauth-test", + prompt: "Describe the image.", + imageDataUrl: `data:image/png;base64,${Buffer.from("png-bytes").toString("base64")}`, + modelBaseUrl: "https://api.minimax.io/anthropic", + }); + expect(completeMock).not.toHaveBeenCalled(); + }); + + it("uses generic completion for non-canonical minimax-portal image models", async () => { + discoverModelsMock.mockReturnValue({ + find: vi.fn(() => ({ + provider: "minimax-portal", + id: "custom-vision", + input: ["text", "image"], + baseUrl: "https://api.minimax.io/anthropic", + })), + }); + completeMock.mockResolvedValue({ + role: "assistant", + api: "anthropic-messages", + provider: "minimax-portal", + model: "custom-vision", + stopReason: "stop", + timestamp: Date.now(), + content: [{ type: "text", text: "generic ok" }], + }); + + const { describeImageWithModel } = await import("./image.js"); + + const result = await describeImageWithModel({ + cfg: {}, + agentDir: "/tmp/openclaw-agent", + provider: "minimax-portal", + model: "custom-vision", + buffer: Buffer.from("png-bytes"), + fileName: "image.png", + mime: "image/png", + prompt: "Describe the image.", + timeoutMs: 1000, + }); + + expect(result).toEqual({ + text: "generic ok", + model: "custom-vision", + }); + expect(completeMock).toHaveBeenCalledOnce(); + expect(minimaxUnderstandImageMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/media-understanding/providers/image.ts b/src/media-understanding/providers/image.ts index d0dc13c0086..79c36292f0c 100644 --- a/src/media-understanding/providers/image.ts +++ b/src/media-understanding/providers/image.ts @@ -1,6 +1,6 @@ import type { Api, Context, Model } from "@mariozechner/pi-ai"; import { complete } from "@mariozechner/pi-ai"; -import { minimaxUnderstandImage } from "../../agents/minimax-vlm.js"; +import { isMinimaxVlmModel, minimaxUnderstandImage } from "../../agents/minimax-vlm.js"; import { getApiKeyForModel, requireApiKey } from "../../agents/model-auth.js"; import { ensureOpenClawModelsJson } from "../../agents/models-config.js"; import { coerceImageAssistantText } from "../../agents/tools/image-tool.helpers.js"; @@ -40,7 +40,7 @@ export async function describeImageWithModel( authStorage.setRuntimeApiKey(model.provider, apiKey); const base64 = params.buffer.toString("base64"); - if (model.provider === "minimax") { + if (isMinimaxVlmModel(model.provider, model.id)) { const text = await minimaxUnderstandImage({ apiKey, prompt: params.prompt ?? "Describe the image.", diff --git a/src/media-understanding/providers/index.test.ts b/src/media-understanding/providers/index.test.ts index 430e89e84a6..9294d44acd5 100644 --- a/src/media-understanding/providers/index.test.ts +++ b/src/media-understanding/providers/index.test.ts @@ -24,4 +24,12 @@ describe("media-understanding provider registry", () => { expect(provider?.id).toBe("moonshot"); expect(provider?.capabilities).toEqual(["image", "video"]); }); + + it("registers the minimax portal provider", () => { + const registry = buildMediaUnderstandingRegistry(); + const provider = getMediaUnderstandingProvider("minimax-portal", registry); + + expect(provider?.id).toBe("minimax-portal"); + expect(provider?.capabilities).toEqual(["image"]); + }); }); diff --git a/src/media-understanding/providers/index.ts b/src/media-understanding/providers/index.ts index 5aef51790a2..0ceaa78fd80 100644 --- a/src/media-understanding/providers/index.ts +++ b/src/media-understanding/providers/index.ts @@ -4,7 +4,7 @@ import { anthropicProvider } from "./anthropic/index.js"; import { deepgramProvider } from "./deepgram/index.js"; import { googleProvider } from "./google/index.js"; import { groqProvider } from "./groq/index.js"; -import { minimaxProvider } from "./minimax/index.js"; +import { minimaxPortalProvider, minimaxProvider } from "./minimax/index.js"; import { mistralProvider } from "./mistral/index.js"; import { moonshotProvider } from "./moonshot/index.js"; import { openaiProvider } from "./openai/index.js"; @@ -16,6 +16,7 @@ const PROVIDERS: MediaUnderstandingProvider[] = [ googleProvider, anthropicProvider, minimaxProvider, + minimaxPortalProvider, moonshotProvider, mistralProvider, zaiProvider, diff --git a/src/media-understanding/providers/minimax/index.ts b/src/media-understanding/providers/minimax/index.ts index 6fa6ebf351a..c9a7936f4d3 100644 --- a/src/media-understanding/providers/minimax/index.ts +++ b/src/media-understanding/providers/minimax/index.ts @@ -6,3 +6,9 @@ export const minimaxProvider: MediaUnderstandingProvider = { capabilities: ["image"], describeImage: describeImageWithModel, }; + +export const minimaxPortalProvider: MediaUnderstandingProvider = { + id: "minimax-portal", + capabilities: ["image"], + describeImage: describeImageWithModel, +}; diff --git a/src/telegram/sticker-cache.ts b/src/telegram/sticker-cache.ts index 26fb33ee538..be8966b1eb5 100644 --- a/src/telegram/sticker-cache.ts +++ b/src/telegram/sticker-cache.ts @@ -12,6 +12,7 @@ import type { OpenClawConfig } from "../config/config.js"; import { STATE_DIR } from "../config/paths.js"; import { logVerbose } from "../globals.js"; import { loadJsonFile, saveJsonFile } from "../infra/json-file.js"; +import { AUTO_IMAGE_KEY_PROVIDERS, DEFAULT_IMAGE_MODELS } from "../media-understanding/defaults.js"; import { resolveAutoImageModel } from "../media-understanding/runner.js"; const CACHE_FILE = path.join(STATE_DIR, "telegram", "sticker-cache.json"); @@ -142,7 +143,6 @@ export function getCacheStats(): { count: number; oldestAt?: string; newestAt?: const STICKER_DESCRIPTION_PROMPT = "Describe this sticker image in 1-2 sentences. Focus on what the sticker depicts (character, object, action, emotion). Be concise and objective."; -const VISION_PROVIDERS = ["openai", "anthropic", "google", "minimax"] as const; let imageRuntimePromise: Promise< typeof import("../media-understanding/providers/image-runtime.js") > | null = null; @@ -198,14 +198,7 @@ export async function describeStickerImage(params: DescribeStickerParams): Promi if (entries.length === 0) { return undefined; } - const defaultId = - provider === "openai" - ? "gpt-5-mini" - : provider === "anthropic" - ? "claude-opus-4-6" - : provider === "google" - ? "gemini-3-flash-preview" - : "MiniMax-VL-01"; + const defaultId = DEFAULT_IMAGE_MODELS[provider]; const preferred = entries.find((entry) => entry.id === defaultId); return preferred ?? entries[0]; }; @@ -213,14 +206,16 @@ export async function describeStickerImage(params: DescribeStickerParams): Promi let resolved = null as { provider: string; model?: string } | null; if ( activeModel && - VISION_PROVIDERS.includes(activeModel.provider as (typeof VISION_PROVIDERS)[number]) && + AUTO_IMAGE_KEY_PROVIDERS.includes( + activeModel.provider as (typeof AUTO_IMAGE_KEY_PROVIDERS)[number], + ) && (await hasProviderKey(activeModel.provider)) ) { resolved = activeModel; } if (!resolved) { - for (const provider of VISION_PROVIDERS) { + for (const provider of AUTO_IMAGE_KEY_PROVIDERS) { if (!(await hasProviderKey(provider))) { continue; } From 2bcd56cfacea3166a6a8d0985c662427b2b6b7e6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 19:36:02 +0000 Subject: [PATCH 02/10] refactor: unify DM pairing challenge flows --- .../bluebubbles/src/monitor-processing.ts | 38 ++++---- extensions/feishu/src/bot.ts | 29 +++--- extensions/googlechat/src/monitor-access.ts | 29 +++--- extensions/irc/src/inbound.ts | 26 +++--- extensions/nextcloud-talk/src/inbound.ts | 29 +++--- extensions/zalo/src/monitor.ts | 30 +++---- extensions/zalouser/src/monitor.ts | 34 ++++--- src/discord/monitor/agent-components.ts | 43 +++++---- src/discord/monitor/dm-command-decision.ts | 19 ++-- src/imessage/monitor/monitor-provider.ts | 50 +++++------ src/line/bot-handlers.ts | 67 +++++++------- src/pairing/pairing-challenge.test.ts | 90 +++++++++++++++++++ src/plugin-sdk/bluebubbles.ts | 1 + src/plugin-sdk/feishu.ts | 1 + src/plugin-sdk/googlechat.ts | 1 + src/plugin-sdk/irc.ts | 1 + src/plugin-sdk/nextcloud-talk.ts | 1 + src/plugin-sdk/zalo.ts | 1 + src/plugin-sdk/zalouser.ts | 1 + src/telegram/dm-access.ts | 64 ++++++------- src/web/inbound/access-control.ts | 42 ++++----- 21 files changed, 356 insertions(+), 241 deletions(-) create mode 100644 src/pairing/pairing-challenge.test.ts diff --git a/extensions/bluebubbles/src/monitor-processing.ts b/extensions/bluebubbles/src/monitor-processing.ts index a1c316429e4..a80f22df853 100644 --- a/extensions/bluebubbles/src/monitor-processing.ts +++ b/extensions/bluebubbles/src/monitor-processing.ts @@ -4,6 +4,7 @@ import { createScopedPairingAccess, createReplyPrefixOptions, evictOldHistoryKeys, + issuePairingChallenge, logAckFailure, logInboundDrop, logTypingFailure, @@ -595,25 +596,24 @@ export async function processMessage( } if (accessDecision.decision === "pairing") { - const { code, created } = await pairing.upsertPairingRequest({ - id: message.senderId, + await issuePairingChallenge({ + channel: "bluebubbles", + senderId: message.senderId, + senderIdLine: `Your BlueBubbles sender id: ${message.senderId}`, meta: { name: message.senderName }, - }); - runtime.log?.(`[bluebubbles] pairing request sender=${message.senderId} created=${created}`); - if (created) { - logVerbose(core, runtime, `bluebubbles pairing request sender=${message.senderId}`); - try { - await sendMessageBlueBubbles( - message.senderId, - core.channel.pairing.buildPairingReply({ - channel: "bluebubbles", - idLine: `Your BlueBubbles sender id: ${message.senderId}`, - code, - }), - { cfg: config, accountId: account.accountId }, - ); + upsertPairingRequest: pairing.upsertPairingRequest, + onCreated: () => { + runtime.log?.(`[bluebubbles] pairing request sender=${message.senderId} created=true`); + logVerbose(core, runtime, `bluebubbles pairing request sender=${message.senderId}`); + }, + sendPairingReply: async (text) => { + await sendMessageBlueBubbles(message.senderId, text, { + cfg: config, + accountId: account.accountId, + }); statusSink?.({ lastOutboundAt: Date.now() }); - } catch (err) { + }, + onReplyError: (err) => { logVerbose( core, runtime, @@ -622,8 +622,8 @@ export async function processMessage( runtime.error?.( `[bluebubbles] pairing reply failed sender=${message.senderId}: ${String(err)}`, ); - } - } + }, + }); return; } diff --git a/extensions/feishu/src/bot.ts b/extensions/feishu/src/bot.ts index 3540036c8a6..13a130b3d79 100644 --- a/extensions/feishu/src/bot.ts +++ b/extensions/feishu/src/bot.ts @@ -6,6 +6,7 @@ import { createScopedPairingAccess, DEFAULT_GROUP_HISTORY_LIMIT, type HistoryEntry, + issuePairingChallenge, normalizeAgentId, recordPendingHistoryEntryIfEnabled, resolveOpenProviderRuntimeGroupPolicy, @@ -1101,29 +1102,29 @@ export async function handleFeishuMessage(params: { if (isDirect && dmPolicy !== "open" && !dmAllowed) { if (dmPolicy === "pairing") { - const { code, created } = await pairing.upsertPairingRequest({ - id: ctx.senderOpenId, + await issuePairingChallenge({ + channel: "feishu", + senderId: ctx.senderOpenId, + senderIdLine: `Your Feishu user id: ${ctx.senderOpenId}`, meta: { name: ctx.senderName }, - }); - if (created) { - log(`feishu[${account.accountId}]: pairing request sender=${ctx.senderOpenId}`); - try { + upsertPairingRequest: pairing.upsertPairingRequest, + onCreated: () => { + log(`feishu[${account.accountId}]: pairing request sender=${ctx.senderOpenId}`); + }, + sendPairingReply: async (text) => { await sendMessageFeishu({ cfg, to: `chat:${ctx.chatId}`, - text: core.channel.pairing.buildPairingReply({ - channel: "feishu", - idLine: `Your Feishu user id: ${ctx.senderOpenId}`, - code, - }), + text, accountId: account.accountId, }); - } catch (err) { + }, + onReplyError: (err) => { log( `feishu[${account.accountId}]: pairing reply failed for ${ctx.senderOpenId}: ${String(err)}`, ); - } - } + }, + }); } else { log( `feishu[${account.accountId}]: blocked unauthorized sender ${ctx.senderOpenId} (dmPolicy=${dmPolicy})`, diff --git a/extensions/googlechat/src/monitor-access.ts b/extensions/googlechat/src/monitor-access.ts index daecea59f8a..bb5b6de9211 100644 --- a/extensions/googlechat/src/monitor-access.ts +++ b/extensions/googlechat/src/monitor-access.ts @@ -1,6 +1,7 @@ import { GROUP_POLICY_BLOCKED_LABEL, createScopedPairingAccess, + issuePairingChallenge, isDangerousNameMatchingEnabled, resolveAllowlistProviderRuntimeGroupPolicy, resolveDefaultGroupPolicy, @@ -311,27 +312,27 @@ export async function applyGoogleChatInboundAccessPolicy(params: { if (access.decision !== "allow") { if (access.decision === "pairing") { - const { code, created } = await pairing.upsertPairingRequest({ - id: senderId, + await issuePairingChallenge({ + channel: "googlechat", + senderId, + senderIdLine: `Your Google Chat user id: ${senderId}`, meta: { name: senderName || undefined, email: senderEmail }, - }); - if (created) { - logVerbose(`googlechat pairing request sender=${senderId}`); - try { + upsertPairingRequest: pairing.upsertPairingRequest, + onCreated: () => { + logVerbose(`googlechat pairing request sender=${senderId}`); + }, + sendPairingReply: async (text) => { await sendGoogleChatMessage({ account, space: spaceId, - text: core.channel.pairing.buildPairingReply({ - channel: "googlechat", - idLine: `Your Google Chat user id: ${senderId}`, - code, - }), + text, }); statusSink?.({ lastOutboundAt: Date.now() }); - } catch (err) { + }, + onReplyError: (err) => { logVerbose(`pairing reply failed for ${senderId}: ${String(err)}`); - } - } + }, + }); } else { logVerbose(`Blocked unauthorized Google Chat sender ${senderId} (dmPolicy=${dmPolicy})`); } diff --git a/extensions/irc/src/inbound.ts b/extensions/irc/src/inbound.ts index 6c03ebadf02..a3a9e32c06e 100644 --- a/extensions/irc/src/inbound.ts +++ b/extensions/irc/src/inbound.ts @@ -3,6 +3,7 @@ import { createScopedPairingAccess, dispatchInboundReplyWithBase, formatTextWithAttachmentLinks, + issuePairingChallenge, logInboundDrop, isDangerousNameMatchingEnabled, readStoreAllowFromForDmPolicy, @@ -208,28 +209,25 @@ export async function handleIrcInbound(params: { }).allowed; if (!dmAllowed) { if (dmPolicy === "pairing") { - const { code, created } = await pairing.upsertPairingRequest({ - id: senderDisplay.toLowerCase(), + await issuePairingChallenge({ + channel: CHANNEL_ID, + senderId: senderDisplay.toLowerCase(), + senderIdLine: `Your IRC id: ${senderDisplay}`, meta: { name: message.senderNick || undefined }, - }); - if (created) { - try { - const reply = core.channel.pairing.buildPairingReply({ - channel: CHANNEL_ID, - idLine: `Your IRC id: ${senderDisplay}`, - code, - }); + upsertPairingRequest: pairing.upsertPairingRequest, + sendPairingReply: async (text) => { await deliverIrcReply({ - payload: { text: reply }, + payload: { text }, target: message.senderNick, accountId: account.accountId, sendReply: params.sendReply, statusSink, }); - } catch (err) { + }, + onReplyError: (err) => { runtime.error?.(`irc: pairing reply failed for ${senderDisplay}: ${String(err)}`); - } - } + }, + }); } runtime.log?.(`irc: drop DM sender ${senderDisplay} (dmPolicy=${dmPolicy})`); return; diff --git a/extensions/nextcloud-talk/src/inbound.ts b/extensions/nextcloud-talk/src/inbound.ts index 1657cbd9113..081029782f8 100644 --- a/extensions/nextcloud-talk/src/inbound.ts +++ b/extensions/nextcloud-talk/src/inbound.ts @@ -3,6 +3,7 @@ import { createScopedPairingAccess, dispatchInboundReplyWithBase, formatTextWithAttachmentLinks, + issuePairingChallenge, logInboundDrop, readStoreAllowFromForDmPolicy, resolveDmGroupAccessWithCommandGate, @@ -173,26 +174,20 @@ export async function handleNextcloudTalkInbound(params: { } else { if (access.decision !== "allow") { if (access.decision === "pairing") { - const { code, created } = await pairing.upsertPairingRequest({ - id: senderId, + await issuePairingChallenge({ + channel: CHANNEL_ID, + senderId, + senderIdLine: `Your Nextcloud user id: ${senderId}`, meta: { name: senderName || undefined }, - }); - if (created) { - try { - await sendMessageNextcloudTalk( - roomToken, - core.channel.pairing.buildPairingReply({ - channel: CHANNEL_ID, - idLine: `Your Nextcloud user id: ${senderId}`, - code, - }), - { accountId: account.accountId }, - ); + upsertPairingRequest: pairing.upsertPairingRequest, + sendPairingReply: async (text) => { + await sendMessageNextcloudTalk(roomToken, text, { accountId: account.accountId }); statusSink?.({ lastOutboundAt: Date.now() }); - } catch (err) { + }, + onReplyError: (err) => { runtime.error?.(`nextcloud-talk: pairing reply failed for ${senderId}: ${String(err)}`); - } - } + }, + }); } runtime.log?.(`nextcloud-talk: drop DM sender ${senderId} (reason=${access.reason})`); return; diff --git a/extensions/zalo/src/monitor.ts b/extensions/zalo/src/monitor.ts index b276019879e..33692f27bbb 100644 --- a/extensions/zalo/src/monitor.ts +++ b/extensions/zalo/src/monitor.ts @@ -7,6 +7,7 @@ import type { import { createScopedPairingAccess, createReplyPrefixOptions, + issuePairingChallenge, resolveDirectDmAuthorizationOutcome, resolveSenderCommandAuthorizationWithRuntime, resolveOutboundMediaUrls, @@ -414,31 +415,30 @@ async function processMessageWithPipeline(params: { } if (directDmOutcome === "unauthorized") { if (dmPolicy === "pairing") { - const { code, created } = await pairing.upsertPairingRequest({ - id: senderId, + await issuePairingChallenge({ + channel: "zalo", + senderId, + senderIdLine: `Your Zalo user id: ${senderId}`, meta: { name: senderName ?? undefined }, - }); - - if (created) { - logVerbose(core, runtime, `zalo pairing request sender=${senderId}`); - try { + upsertPairingRequest: pairing.upsertPairingRequest, + onCreated: () => { + logVerbose(core, runtime, `zalo pairing request sender=${senderId}`); + }, + sendPairingReply: async (text) => { await sendMessage( token, { chat_id: chatId, - text: core.channel.pairing.buildPairingReply({ - channel: "zalo", - idLine: `Your Zalo user id: ${senderId}`, - code, - }), + text, }, fetcher, ); statusSink?.({ lastOutboundAt: Date.now() }); - } catch (err) { + }, + onReplyError: (err) => { logVerbose(core, runtime, `zalo pairing reply failed for ${senderId}: ${String(err)}`); - } - } + }, + }); } else { logVerbose( core, diff --git a/extensions/zalouser/src/monitor.ts b/extensions/zalouser/src/monitor.ts index fc3e07c564e..670e0992c0e 100644 --- a/extensions/zalouser/src/monitor.ts +++ b/extensions/zalouser/src/monitor.ts @@ -8,6 +8,7 @@ import { createTypingCallbacks, createScopedPairingAccess, createReplyPrefixOptions, + issuePairingChallenge, resolveOutboundMediaUrls, mergeAllowlist, resolveMentionGatingWithBypass, @@ -262,32 +263,27 @@ async function processMessage( const allowed = senderAllowedForCommands; if (!allowed) { if (dmPolicy === "pairing") { - const { code, created } = await pairing.upsertPairingRequest({ - id: senderId, + await issuePairingChallenge({ + channel: "zalouser", + senderId, + senderIdLine: `Your Zalo user id: ${senderId}`, meta: { name: senderName || undefined }, - }); - - if (created) { - logVerbose(core, runtime, `zalouser pairing request sender=${senderId}`); - try { - await sendMessageZalouser( - chatId, - core.channel.pairing.buildPairingReply({ - channel: "zalouser", - idLine: `Your Zalo user id: ${senderId}`, - code, - }), - { profile: account.profile }, - ); + upsertPairingRequest: pairing.upsertPairingRequest, + onCreated: () => { + logVerbose(core, runtime, `zalouser pairing request sender=${senderId}`); + }, + sendPairingReply: async (text) => { + await sendMessageZalouser(chatId, text, { profile: account.profile }); statusSink?.({ lastOutboundAt: Date.now() }); - } catch (err) { + }, + onReplyError: (err) => { logVerbose( core, runtime, `zalouser pairing reply failed for ${senderId}: ${String(err)}`, ); - } - } + }, + }); } else { logVerbose( core, diff --git a/src/discord/monitor/agent-components.ts b/src/discord/monitor/agent-components.ts index ecf7325338a..f0aa6264720 100644 --- a/src/discord/monitor/agent-components.ts +++ b/src/discord/monitor/agent-components.ts @@ -35,7 +35,7 @@ import { logVerbose } from "../../globals.js"; import { enqueueSystemEvent } from "../../infra/system-events.js"; import { logDebug, logError } from "../../logger.js"; import { getAgentScopedMediaLocalRoots } from "../../media/local-roots.js"; -import { buildPairingReply } from "../../pairing/pairing-messages.js"; +import { issuePairingChallenge } from "../../pairing/pairing-challenge.js"; import { upsertChannelPairingRequest } from "../../pairing/pairing-store.js"; import { resolveAgentRoute } from "../../routing/resolve-route.js"; import { createNonExitingRuntime, type RuntimeEnv } from "../../runtime.js"; @@ -519,28 +519,37 @@ async function ensureDmComponentAuthorized(params: { } if (dmPolicy === "pairing") { - const { code, created } = await upsertChannelPairingRequest({ + const pairingResult = await issuePairingChallenge({ channel: "discord", - id: user.id, - accountId: ctx.accountId, + senderId: user.id, + senderIdLine: `Your Discord user id: ${user.id}`, meta: { tag: formatDiscordUserTag(user), name: user.username, }, + upsertPairingRequest: async ({ id, meta }) => + await upsertChannelPairingRequest({ + channel: "discord", + id, + accountId: ctx.accountId, + meta, + }), + sendPairingReply: async (text) => { + await interaction.reply({ + content: text, + ...replyOpts, + }); + }, }); - try { - await interaction.reply({ - content: created - ? buildPairingReply({ - channel: "discord", - idLine: `Your Discord user id: ${user.id}`, - code, - }) - : "Pairing already requested. Ask the bot owner to approve your code.", - ...replyOpts, - }); - } catch { - // Interaction may have expired + if (!pairingResult.created) { + try { + await interaction.reply({ + content: "Pairing already requested. Ask the bot owner to approve your code.", + ...replyOpts, + }); + } catch { + // Interaction may have expired + } } return false; } diff --git a/src/discord/monitor/dm-command-decision.ts b/src/discord/monitor/dm-command-decision.ts index a0f64fdfb4b..d5b533bfdaa 100644 --- a/src/discord/monitor/dm-command-decision.ts +++ b/src/discord/monitor/dm-command-decision.ts @@ -1,3 +1,4 @@ +import { issuePairingChallenge } from "../../pairing/pairing-challenge.js"; import { upsertChannelPairingRequest } from "../../pairing/pairing-store.js"; import type { DiscordDmCommandAccess } from "./dm-command-auth.js"; @@ -19,17 +20,25 @@ export async function handleDiscordDmCommandDecision(params: { if (params.dmAccess.decision === "pairing") { const upsertPairingRequest = params.upsertPairingRequest ?? upsertChannelPairingRequest; - const { code, created } = await upsertPairingRequest({ + const result = await issuePairingChallenge({ channel: "discord", - id: params.sender.id, - accountId: params.accountId, + senderId: params.sender.id, + senderIdLine: `Your Discord user id: ${params.sender.id}`, meta: { tag: params.sender.tag, name: params.sender.name, }, + upsertPairingRequest: async ({ id, meta }) => + await upsertPairingRequest({ + channel: "discord", + id, + accountId: params.accountId, + meta, + }), + sendPairingReply: async () => {}, }); - if (created) { - await params.onPairingCreated(code); + if (result.created && result.code) { + await params.onPairingCreated(result.code); } return false; } diff --git a/src/imessage/monitor/monitor-provider.ts b/src/imessage/monitor/monitor-provider.ts index ffc15a4df0a..1ea35b60d95 100644 --- a/src/imessage/monitor/monitor-provider.ts +++ b/src/imessage/monitor/monitor-provider.ts @@ -30,7 +30,7 @@ import { resolveIMessageRemoteAttachmentRoots, } from "../../media/inbound-path-policy.js"; import { kindFromMime } from "../../media/mime.js"; -import { buildPairingReply } from "../../pairing/pairing-messages.js"; +import { issuePairingChallenge } from "../../pairing/pairing-challenge.js"; import { readChannelAllowFromStore, upsertChannelPairingRequest, @@ -288,36 +288,36 @@ export async function monitorIMessageProvider(opts: MonitorIMessageOpts = {}): P if (!sender) { return; } - const { code, created } = await upsertChannelPairingRequest({ + await issuePairingChallenge({ channel: "imessage", - id: decision.senderId, - accountId: accountInfo.accountId, + senderId: decision.senderId, + senderIdLine: `Your iMessage sender id: ${decision.senderId}`, meta: { sender: decision.senderId, chatId: chatId ? String(chatId) : undefined, }, - }); - if (created) { - logVerbose(`imessage pairing request sender=${decision.senderId}`); - try { - await sendMessageIMessage( - sender, - buildPairingReply({ - channel: "imessage", - idLine: `Your iMessage sender id: ${decision.senderId}`, - code, - }), - { - client, - maxBytes: mediaMaxBytes, - accountId: accountInfo.accountId, - ...(chatId ? { chatId } : {}), - }, - ); - } catch (err) { + upsertPairingRequest: async ({ id, meta }) => + await upsertChannelPairingRequest({ + channel: "imessage", + id, + accountId: accountInfo.accountId, + meta, + }), + onCreated: () => { + logVerbose(`imessage pairing request sender=${decision.senderId}`); + }, + sendPairingReply: async (text) => { + await sendMessageIMessage(sender, text, { + client, + maxBytes: mediaMaxBytes, + accountId: accountInfo.accountId, + ...(chatId ? { chatId } : {}), + }); + }, + onReplyError: (err) => { logVerbose(`imessage pairing reply failed for ${decision.senderId}: ${String(err)}`); - } - } + }, + }); return; } diff --git a/src/line/bot-handlers.ts b/src/line/bot-handlers.ts index 8cf9be9d79f..2772965a8e6 100644 --- a/src/line/bot-handlers.ts +++ b/src/line/bot-handlers.ts @@ -24,8 +24,8 @@ import { warnMissingProviderGroupPolicyFallbackOnce, } from "../config/runtime-group-policy.js"; import { danger, logVerbose } from "../globals.js"; +import { issuePairingChallenge } from "../pairing/pairing-challenge.js"; import { resolvePairingIdLabel } from "../pairing/pairing-labels.js"; -import { buildPairingReply } from "../pairing/pairing-messages.js"; import { readChannelAllowFromStore, upsertChannelPairingRequest, @@ -237,15 +237,6 @@ async function sendLinePairingReply(params: { context: LineHandlerContext; }): Promise { const { senderId, replyToken, context } = params; - const { code, created } = await upsertChannelPairingRequest({ - channel: "line", - id: senderId, - accountId: context.account.accountId, - }); - if (!created) { - return; - } - logVerbose(`line pairing request sender=${senderId}`); const idLabel = (() => { try { return resolvePairingIdLabel("line"); @@ -253,30 +244,42 @@ async function sendLinePairingReply(params: { return "lineUserId"; } })(); - const text = buildPairingReply({ + await issuePairingChallenge({ channel: "line", - idLine: `Your ${idLabel}: ${senderId}`, - code, - }); - try { - if (replyToken) { - await replyMessageLine(replyToken, [{ type: "text", text }], { + senderId, + senderIdLine: `Your ${idLabel}: ${senderId}`, + upsertPairingRequest: async ({ id, meta }) => + await upsertChannelPairingRequest({ + channel: "line", + id, accountId: context.account.accountId, - channelAccessToken: context.account.channelAccessToken, - }); - return; - } - } catch (err) { - logVerbose(`line pairing reply failed for ${senderId}: ${String(err)}`); - } - try { - await pushMessageLine(`line:${senderId}`, text, { - accountId: context.account.accountId, - channelAccessToken: context.account.channelAccessToken, - }); - } catch (err) { - logVerbose(`line pairing reply failed for ${senderId}: ${String(err)}`); - } + meta, + }), + onCreated: () => { + logVerbose(`line pairing request sender=${senderId}`); + }, + sendPairingReply: async (text) => { + if (replyToken) { + try { + await replyMessageLine(replyToken, [{ type: "text", text }], { + accountId: context.account.accountId, + channelAccessToken: context.account.channelAccessToken, + }); + return; + } catch (err) { + logVerbose(`line pairing reply failed for ${senderId}: ${String(err)}`); + } + } + try { + await pushMessageLine(`line:${senderId}`, text, { + accountId: context.account.accountId, + channelAccessToken: context.account.channelAccessToken, + }); + } catch (err) { + logVerbose(`line pairing reply failed for ${senderId}: ${String(err)}`); + } + }, + }); } async function shouldProcessLineEvent( diff --git a/src/pairing/pairing-challenge.test.ts b/src/pairing/pairing-challenge.test.ts new file mode 100644 index 00000000000..cb447499005 --- /dev/null +++ b/src/pairing/pairing-challenge.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it, vi } from "vitest"; +import { issuePairingChallenge } from "./pairing-challenge.js"; + +describe("issuePairingChallenge", () => { + it("creates and sends a pairing reply when request is newly created", async () => { + const sent: string[] = []; + + const result = await issuePairingChallenge({ + channel: "telegram", + senderId: "123", + senderIdLine: "Your Telegram user id: 123", + upsertPairingRequest: async () => ({ code: "ABCD", created: true }), + sendPairingReply: async (text) => { + sent.push(text); + }, + }); + + expect(result).toEqual({ created: true, code: "ABCD" }); + expect(sent).toHaveLength(1); + expect(sent[0]).toContain("ABCD"); + }); + + it("does not send a reply when request already exists", async () => { + const sendPairingReply = vi.fn(async () => {}); + + const result = await issuePairingChallenge({ + channel: "telegram", + senderId: "123", + senderIdLine: "Your Telegram user id: 123", + upsertPairingRequest: async () => ({ code: "ABCD", created: false }), + sendPairingReply, + }); + + expect(result).toEqual({ created: false }); + expect(sendPairingReply).not.toHaveBeenCalled(); + }); + + it("supports custom reply text builder", async () => { + const sent: string[] = []; + + await issuePairingChallenge({ + channel: "line", + senderId: "u1", + senderIdLine: "Your line id: u1", + upsertPairingRequest: async () => ({ code: "ZXCV", created: true }), + buildReplyText: ({ code }) => `custom ${code}`, + sendPairingReply: async (text) => { + sent.push(text); + }, + }); + + expect(sent).toEqual(["custom ZXCV"]); + }); + + it("calls onCreated and forwards meta to upsert", async () => { + const onCreated = vi.fn(); + const upsert = vi.fn(async () => ({ code: "1111", created: true })); + + await issuePairingChallenge({ + channel: "discord", + senderId: "42", + senderIdLine: "Your Discord user id: 42", + meta: { name: "alice" }, + upsertPairingRequest: upsert, + onCreated, + sendPairingReply: async () => {}, + }); + + expect(upsert).toHaveBeenCalledWith({ id: "42", meta: { name: "alice" } }); + expect(onCreated).toHaveBeenCalledWith({ code: "1111" }); + }); + + it("captures reply errors through onReplyError", async () => { + const onReplyError = vi.fn(); + + const result = await issuePairingChallenge({ + channel: "signal", + senderId: "+1555", + senderIdLine: "Your Signal sender id: +1555", + upsertPairingRequest: async () => ({ code: "9999", created: true }), + sendPairingReply: async () => { + throw new Error("send failed"); + }, + onReplyError, + }); + + expect(result).toEqual({ created: true, code: "9999" }); + expect(onReplyError).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/plugin-sdk/bluebubbles.ts b/src/plugin-sdk/bluebubbles.ts index bb67d56878e..8cf9b38b916 100644 --- a/src/plugin-sdk/bluebubbles.ts +++ b/src/plugin-sdk/bluebubbles.ts @@ -86,6 +86,7 @@ export type { WizardPrompter } from "../wizard/prompts.js"; export { isAllowedParsedChatSender } from "./allow-from.js"; export { readBooleanParam } from "./boolean-param.js"; export { createScopedPairingAccess } from "./pairing-access.js"; +export { issuePairingChallenge } from "../pairing/pairing-challenge.js"; export { resolveRequestUrl } from "./request-url.js"; export { buildComputedAccountStatusSnapshot, diff --git a/src/plugin-sdk/feishu.ts b/src/plugin-sdk/feishu.ts index 360623d9e9c..4a21071e4c6 100644 --- a/src/plugin-sdk/feishu.ts +++ b/src/plugin-sdk/feishu.ts @@ -57,6 +57,7 @@ export type { WizardPrompter } from "../wizard/prompts.js"; export { buildAgentMediaPayload } from "./agent-media-payload.js"; export { readJsonFileWithFallback } from "./json-store.js"; export { createScopedPairingAccess } from "./pairing-access.js"; +export { issuePairingChallenge } from "../pairing/pairing-challenge.js"; export { createPersistentDedupe } from "./persistent-dedupe.js"; export { buildBaseChannelStatusSummary, diff --git a/src/plugin-sdk/googlechat.ts b/src/plugin-sdk/googlechat.ts index e7b96355608..204e0affd3c 100644 --- a/src/plugin-sdk/googlechat.ts +++ b/src/plugin-sdk/googlechat.ts @@ -63,6 +63,7 @@ export { formatDocsLink } from "../terminal/links.js"; export type { WizardPrompter } from "../wizard/prompts.js"; export { resolveInboundRouteEnvelopeBuilderWithRuntime } from "./inbound-envelope.js"; export { createScopedPairingAccess } from "./pairing-access.js"; +export { issuePairingChallenge } from "../pairing/pairing-challenge.js"; export { extractToolSend } from "./tool-send.js"; export { resolveWebhookPath } from "./webhook-path.js"; export type { WebhookInFlightLimiter } from "./webhook-request-guards.js"; diff --git a/src/plugin-sdk/irc.ts b/src/plugin-sdk/irc.ts index afc9428bb05..1983130d193 100644 --- a/src/plugin-sdk/irc.ts +++ b/src/plugin-sdk/irc.ts @@ -60,6 +60,7 @@ export { export { formatDocsLink } from "../terminal/links.js"; export type { WizardPrompter } from "../wizard/prompts.js"; export { createScopedPairingAccess } from "./pairing-access.js"; +export { issuePairingChallenge } from "../pairing/pairing-challenge.js"; export { dispatchInboundReplyWithBase } from "./inbound-reply-dispatch.js"; export type { OutboundReplyPayload } from "./reply-payload.js"; export { diff --git a/src/plugin-sdk/nextcloud-talk.ts b/src/plugin-sdk/nextcloud-talk.ts index 03116a7864b..e6f1f3a9bcd 100644 --- a/src/plugin-sdk/nextcloud-talk.ts +++ b/src/plugin-sdk/nextcloud-talk.ts @@ -84,6 +84,7 @@ export { resolveAccountWithDefaultFallback, } from "./account-resolution.js"; export { createScopedPairingAccess } from "./pairing-access.js"; +export { issuePairingChallenge } from "../pairing/pairing-challenge.js"; export { createPersistentDedupe } from "./persistent-dedupe.js"; export type { OutboundReplyPayload } from "./reply-payload.js"; export { diff --git a/src/plugin-sdk/zalo.ts b/src/plugin-sdk/zalo.ts index 852c6f17f0c..a498e050034 100644 --- a/src/plugin-sdk/zalo.ts +++ b/src/plugin-sdk/zalo.ts @@ -67,6 +67,7 @@ export { evaluateSenderGroupAccess } from "./group-access.js"; export type { SenderGroupAccessDecision } from "./group-access.js"; export { resolveInboundRouteEnvelopeBuilderWithRuntime } from "./inbound-envelope.js"; export { createScopedPairingAccess } from "./pairing-access.js"; +export { issuePairingChallenge } from "../pairing/pairing-challenge.js"; export { buildChannelSendResult } from "./channel-send-result.js"; export type { OutboundReplyPayload } from "./reply-payload.js"; export { diff --git a/src/plugin-sdk/zalouser.ts b/src/plugin-sdk/zalouser.ts index d0c75742ef0..7558c7f750c 100644 --- a/src/plugin-sdk/zalouser.ts +++ b/src/plugin-sdk/zalouser.ts @@ -57,6 +57,7 @@ export { resolveSenderCommandAuthorization } from "./command-auth.js"; export { resolveChannelAccountConfigBasePath } from "./config-paths.js"; export { loadOutboundMediaFromUrl } from "./outbound-media.js"; export { createScopedPairingAccess } from "./pairing-access.js"; +export { issuePairingChallenge } from "../pairing/pairing-challenge.js"; export { buildChannelSendResult } from "./channel-send-result.js"; export type { OutboundReplyPayload } from "./reply-payload.js"; export { diff --git a/src/telegram/dm-access.ts b/src/telegram/dm-access.ts index 1c68dd43d69..26734b69602 100644 --- a/src/telegram/dm-access.ts +++ b/src/telegram/dm-access.ts @@ -2,7 +2,7 @@ import type { Message } from "@grammyjs/types"; import type { Bot } from "grammy"; import type { DmPolicy } from "../config/types.js"; import { logVerbose } from "../globals.js"; -import { buildPairingReply } from "../pairing/pairing-messages.js"; +import { issuePairingChallenge } from "../pairing/pairing-challenge.js"; import { upsertChannelPairingRequest } from "../pairing/pairing-store.js"; import { withTelegramApiErrorLogging } from "./api-logging.js"; import { resolveSenderAllowMatch, type NormalizedAllowFrom } from "./bot-access.js"; @@ -70,42 +70,46 @@ export async function enforceTelegramDmAccess(params: { if (dmPolicy === "pairing") { try { const telegramUserId = sender.userId ?? sender.candidateId; - const { code, created } = await upsertChannelPairingRequest({ + await issuePairingChallenge({ channel: "telegram", - id: telegramUserId, - accountId, + senderId: telegramUserId, + senderIdLine: `Your Telegram user id: ${telegramUserId}`, meta: { username: sender.username || undefined, firstName: sender.firstName, lastName: sender.lastName, }, + upsertPairingRequest: async ({ id, meta }) => + await upsertChannelPairingRequest({ + channel: "telegram", + id, + accountId, + meta, + }), + onCreated: () => { + logger.info( + { + chatId: String(chatId), + senderUserId: sender.userId ?? undefined, + username: sender.username || undefined, + firstName: sender.firstName, + lastName: sender.lastName, + matchKey: allowMatch.matchKey ?? "none", + matchSource: allowMatch.matchSource ?? "none", + }, + "telegram pairing request", + ); + }, + sendPairingReply: async (text) => { + await withTelegramApiErrorLogging({ + operation: "sendMessage", + fn: () => bot.api.sendMessage(chatId, text), + }); + }, + onReplyError: (err) => { + logVerbose(`telegram pairing reply failed for chat ${chatId}: ${String(err)}`); + }, }); - if (created) { - logger.info( - { - chatId: String(chatId), - senderUserId: sender.userId ?? undefined, - username: sender.username || undefined, - firstName: sender.firstName, - lastName: sender.lastName, - matchKey: allowMatch.matchKey ?? "none", - matchSource: allowMatch.matchSource ?? "none", - }, - "telegram pairing request", - ); - await withTelegramApiErrorLogging({ - operation: "sendMessage", - fn: () => - bot.api.sendMessage( - chatId, - buildPairingReply({ - channel: "telegram", - idLine: `Your Telegram user id: ${telegramUserId}`, - code, - }), - ), - }); - } } catch (err) { logVerbose(`telegram pairing reply failed for chat ${chatId}: ${String(err)}`); } diff --git a/src/web/inbound/access-control.ts b/src/web/inbound/access-control.ts index 2363434f34c..a01e27fb6e0 100644 --- a/src/web/inbound/access-control.ts +++ b/src/web/inbound/access-control.ts @@ -5,7 +5,7 @@ import { warnMissingProviderGroupPolicyFallbackOnce, } from "../../config/runtime-group-policy.js"; import { logVerbose } from "../../globals.js"; -import { buildPairingReply } from "../../pairing/pairing-messages.js"; +import { issuePairingChallenge } from "../../pairing/pairing-challenge.js"; import { upsertChannelPairingRequest } from "../../pairing/pairing-store.js"; import { readStoreAllowFromForDmPolicy, @@ -171,28 +171,30 @@ export async function checkInboundAccessControl(params: { if (suppressPairingReply) { logVerbose(`Skipping pairing reply for historical DM from ${candidate}.`); } else { - const { code, created } = await upsertChannelPairingRequest({ + await issuePairingChallenge({ channel: "whatsapp", - id: candidate, - accountId: account.accountId, + senderId: candidate, + senderIdLine: `Your WhatsApp phone number: ${candidate}`, meta: { name: (params.pushName ?? "").trim() || undefined }, - }); - if (created) { - logVerbose( - `whatsapp pairing request sender=${candidate} name=${params.pushName ?? "unknown"}`, - ); - try { - await params.sock.sendMessage(params.remoteJid, { - text: buildPairingReply({ - channel: "whatsapp", - idLine: `Your WhatsApp phone number: ${candidate}`, - code, - }), - }); - } catch (err) { + upsertPairingRequest: async ({ id, meta }) => + await upsertChannelPairingRequest({ + channel: "whatsapp", + id, + accountId: account.accountId, + meta, + }), + onCreated: () => { + logVerbose( + `whatsapp pairing request sender=${candidate} name=${params.pushName ?? "unknown"}`, + ); + }, + sendPairingReply: async (text) => { + await params.sock.sendMessage(params.remoteJid, { text }); + }, + onReplyError: (err) => { logVerbose(`whatsapp pairing reply failed for ${candidate}: ${String(err)}`); - } - } + }, + }); } return { allowed: false, From 7242777d63efc4849b0ef9dc95701cd40717781f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 19:44:56 +0000 Subject: [PATCH 03/10] refactor: unify account list/default scaffolding --- extensions/bluebubbles/src/accounts.ts | 43 ++-------- extensions/googlechat/src/accounts.ts | 44 ++--------- extensions/irc/src/accounts.test.ts | 78 +++++++++++++++++++ extensions/irc/src/accounts.ts | 48 ++---------- extensions/matrix/src/matrix/accounts.ts | 50 ++---------- .../mattermost/src/mattermost/accounts.ts | 43 ++-------- extensions/nextcloud-talk/src/accounts.ts | 42 +++------- extensions/zalo/src/accounts.ts | 42 ++-------- extensions/zalouser/src/accounts.ts | 44 ++--------- src/plugin-sdk/bluebubbles.ts | 1 + src/plugin-sdk/googlechat.ts | 1 + src/plugin-sdk/irc.ts | 1 + src/plugin-sdk/matrix.ts | 1 + src/plugin-sdk/mattermost.ts | 1 + src/plugin-sdk/nextcloud-talk.ts | 1 + src/plugin-sdk/zalo.ts | 1 + src/plugin-sdk/zalouser.ts | 1 + 17 files changed, 143 insertions(+), 299 deletions(-) create mode 100644 extensions/irc/src/accounts.test.ts diff --git a/extensions/bluebubbles/src/accounts.ts b/extensions/bluebubbles/src/accounts.ts index 4b86c6d0364..d7c5a281473 100644 --- a/extensions/bluebubbles/src/accounts.ts +++ b/extensions/bluebubbles/src/accounts.ts @@ -1,9 +1,5 @@ -import { - DEFAULT_ACCOUNT_ID, - normalizeAccountId, - normalizeOptionalAccountId, -} from "openclaw/plugin-sdk/account-id"; -import type { OpenClawConfig } from "openclaw/plugin-sdk/bluebubbles"; +import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "openclaw/plugin-sdk/account-id"; +import { createAccountListHelpers, type OpenClawConfig } from "openclaw/plugin-sdk/bluebubbles"; import { hasConfiguredSecretInput, normalizeSecretInputString } from "./secret-input.js"; import { normalizeBlueBubblesServerUrl, type BlueBubblesAccountConfig } from "./types.js"; @@ -16,36 +12,11 @@ export type ResolvedBlueBubblesAccount = { baseUrl?: string; }; -function listConfiguredAccountIds(cfg: OpenClawConfig): string[] { - const accounts = cfg.channels?.bluebubbles?.accounts; - if (!accounts || typeof accounts !== "object") { - return []; - } - return Object.keys(accounts).filter(Boolean); -} - -export function listBlueBubblesAccountIds(cfg: OpenClawConfig): string[] { - const ids = listConfiguredAccountIds(cfg); - if (ids.length === 0) { - return [DEFAULT_ACCOUNT_ID]; - } - return ids.toSorted((a, b) => a.localeCompare(b)); -} - -export function resolveDefaultBlueBubblesAccountId(cfg: OpenClawConfig): string { - const preferred = normalizeOptionalAccountId(cfg.channels?.bluebubbles?.defaultAccount); - if ( - preferred && - listBlueBubblesAccountIds(cfg).some((accountId) => normalizeAccountId(accountId) === preferred) - ) { - return preferred; - } - const ids = listBlueBubblesAccountIds(cfg); - if (ids.includes(DEFAULT_ACCOUNT_ID)) { - return DEFAULT_ACCOUNT_ID; - } - return ids[0] ?? DEFAULT_ACCOUNT_ID; -} +const { + listAccountIds: listBlueBubblesAccountIds, + resolveDefaultAccountId: resolveDefaultBlueBubblesAccountId, +} = createAccountListHelpers("bluebubbles"); +export { listBlueBubblesAccountIds, resolveDefaultBlueBubblesAccountId }; function resolveAccountConfig( cfg: OpenClawConfig, diff --git a/extensions/googlechat/src/accounts.ts b/extensions/googlechat/src/accounts.ts index f597efbece4..d864eb3ff37 100644 --- a/extensions/googlechat/src/accounts.ts +++ b/extensions/googlechat/src/accounts.ts @@ -1,10 +1,6 @@ -import { - DEFAULT_ACCOUNT_ID, - normalizeAccountId, - normalizeOptionalAccountId, -} from "openclaw/plugin-sdk/account-id"; +import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "openclaw/plugin-sdk/account-id"; import { isSecretRef } from "openclaw/plugin-sdk/googlechat"; -import type { OpenClawConfig } from "openclaw/plugin-sdk/googlechat"; +import { createAccountListHelpers, type OpenClawConfig } from "openclaw/plugin-sdk/googlechat"; import type { GoogleChatAccountConfig } from "./types.config.js"; export type GoogleChatCredentialSource = "file" | "inline" | "env" | "none"; @@ -22,37 +18,11 @@ export type ResolvedGoogleChatAccount = { const ENV_SERVICE_ACCOUNT = "GOOGLE_CHAT_SERVICE_ACCOUNT"; const ENV_SERVICE_ACCOUNT_FILE = "GOOGLE_CHAT_SERVICE_ACCOUNT_FILE"; -function listConfiguredAccountIds(cfg: OpenClawConfig): string[] { - const accounts = cfg.channels?.["googlechat"]?.accounts; - if (!accounts || typeof accounts !== "object") { - return []; - } - return Object.keys(accounts).filter(Boolean); -} - -export function listGoogleChatAccountIds(cfg: OpenClawConfig): string[] { - const ids = listConfiguredAccountIds(cfg); - if (ids.length === 0) { - return [DEFAULT_ACCOUNT_ID]; - } - return ids.toSorted((a, b) => a.localeCompare(b)); -} - -export function resolveDefaultGoogleChatAccountId(cfg: OpenClawConfig): string { - const channel = cfg.channels?.["googlechat"]; - const preferred = normalizeOptionalAccountId(channel?.defaultAccount); - if ( - preferred && - listGoogleChatAccountIds(cfg).some((accountId) => normalizeAccountId(accountId) === preferred) - ) { - return preferred; - } - const ids = listGoogleChatAccountIds(cfg); - if (ids.includes(DEFAULT_ACCOUNT_ID)) { - return DEFAULT_ACCOUNT_ID; - } - return ids[0] ?? DEFAULT_ACCOUNT_ID; -} +const { + listAccountIds: listGoogleChatAccountIds, + resolveDefaultAccountId: resolveDefaultGoogleChatAccountId, +} = createAccountListHelpers("googlechat"); +export { listGoogleChatAccountIds, resolveDefaultGoogleChatAccountId }; function resolveAccountConfig( cfg: OpenClawConfig, diff --git a/extensions/irc/src/accounts.test.ts b/extensions/irc/src/accounts.test.ts new file mode 100644 index 00000000000..59a72d7cbcb --- /dev/null +++ b/extensions/irc/src/accounts.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it } from "vitest"; +import { listIrcAccountIds, resolveDefaultIrcAccountId } from "./accounts.js"; +import type { CoreConfig } from "./types.js"; + +function asConfig(value: unknown): CoreConfig { + return value as CoreConfig; +} + +describe("listIrcAccountIds", () => { + it("returns default when no accounts are configured", () => { + expect(listIrcAccountIds(asConfig({}))).toEqual(["default"]); + }); + + it("normalizes, deduplicates, and sorts configured account ids", () => { + const cfg = asConfig({ + channels: { + irc: { + accounts: { + "Ops Team": {}, + "ops-team": {}, + Work: {}, + }, + }, + }, + }); + + expect(listIrcAccountIds(cfg)).toEqual(["ops-team", "work"]); + }); +}); + +describe("resolveDefaultIrcAccountId", () => { + it("prefers configured defaultAccount when it matches", () => { + const cfg = asConfig({ + channels: { + irc: { + defaultAccount: "Ops Team", + accounts: { + default: {}, + "ops-team": {}, + }, + }, + }, + }); + + expect(resolveDefaultIrcAccountId(cfg)).toBe("ops-team"); + }); + + it("falls back to default when configured defaultAccount is missing", () => { + const cfg = asConfig({ + channels: { + irc: { + defaultAccount: "missing", + accounts: { + default: {}, + work: {}, + }, + }, + }, + }); + + expect(resolveDefaultIrcAccountId(cfg)).toBe("default"); + }); + + it("falls back to first sorted account when default is absent", () => { + const cfg = asConfig({ + channels: { + irc: { + accounts: { + zzz: {}, + aaa: {}, + }, + }, + }, + }); + + expect(resolveDefaultIrcAccountId(cfg)).toBe("aaa"); + }); +}); diff --git a/extensions/irc/src/accounts.ts b/extensions/irc/src/accounts.ts index 3f9640925c8..d61499c4d39 100644 --- a/extensions/irc/src/accounts.ts +++ b/extensions/irc/src/accounts.ts @@ -1,10 +1,9 @@ import { readFileSync } from "node:fs"; +import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "openclaw/plugin-sdk/account-id"; import { - DEFAULT_ACCOUNT_ID, - normalizeAccountId, - normalizeOptionalAccountId, -} from "openclaw/plugin-sdk/account-id"; -import { normalizeResolvedSecretInputString } from "openclaw/plugin-sdk/irc"; + createAccountListHelpers, + normalizeResolvedSecretInputString, +} from "openclaw/plugin-sdk/irc"; import type { CoreConfig, IrcAccountConfig, IrcNickServConfig } from "./types.js"; const TRUTHY_ENV = new Set(["true", "1", "yes", "on"]); @@ -54,19 +53,9 @@ function parseListEnv(value?: string): string[] | undefined { return parsed.length > 0 ? parsed : undefined; } -function listConfiguredAccountIds(cfg: CoreConfig): string[] { - const accounts = cfg.channels?.irc?.accounts; - if (!accounts || typeof accounts !== "object") { - return []; - } - const ids = new Set(); - for (const key of Object.keys(accounts)) { - if (key.trim()) { - ids.add(normalizeAccountId(key)); - } - } - return [...ids]; -} +const { listAccountIds: listIrcAccountIds, resolveDefaultAccountId: resolveDefaultIrcAccountId } = + createAccountListHelpers("irc", { normalizeAccountId }); +export { listIrcAccountIds, resolveDefaultIrcAccountId }; function resolveAccountConfig(cfg: CoreConfig, accountId: string): IrcAccountConfig | undefined { const accounts = cfg.channels?.irc?.accounts; @@ -165,29 +154,6 @@ function resolveNickServConfig(accountId: string, nickserv?: IrcNickServConfig): return merged; } -export function listIrcAccountIds(cfg: CoreConfig): string[] { - const ids = listConfiguredAccountIds(cfg); - if (ids.length === 0) { - return [DEFAULT_ACCOUNT_ID]; - } - return ids.toSorted((a, b) => a.localeCompare(b)); -} - -export function resolveDefaultIrcAccountId(cfg: CoreConfig): string { - const preferred = normalizeOptionalAccountId(cfg.channels?.irc?.defaultAccount); - if ( - preferred && - listIrcAccountIds(cfg).some((accountId) => normalizeAccountId(accountId) === preferred) - ) { - return preferred; - } - const ids = listIrcAccountIds(cfg); - if (ids.includes(DEFAULT_ACCOUNT_ID)) { - return DEFAULT_ACCOUNT_ID; - } - return ids[0] ?? DEFAULT_ACCOUNT_ID; -} - export function resolveIrcAccount(params: { cfg: CoreConfig; accountId?: string | null; diff --git a/extensions/matrix/src/matrix/accounts.ts b/extensions/matrix/src/matrix/accounts.ts index bdb6d90cf13..52fba376200 100644 --- a/extensions/matrix/src/matrix/accounts.ts +++ b/extensions/matrix/src/matrix/accounts.ts @@ -1,8 +1,5 @@ -import { - DEFAULT_ACCOUNT_ID, - normalizeAccountId, - normalizeOptionalAccountId, -} from "openclaw/plugin-sdk/account-id"; +import { normalizeAccountId } from "openclaw/plugin-sdk/account-id"; +import { createAccountListHelpers } from "openclaw/plugin-sdk/matrix"; import { hasConfiguredSecretInput } from "../secret-input.js"; import type { CoreConfig, MatrixConfig } from "../types.js"; import { resolveMatrixConfigForAccount } from "./client.js"; @@ -35,44 +32,11 @@ export type ResolvedMatrixAccount = { config: MatrixConfig; }; -function listConfiguredAccountIds(cfg: CoreConfig): string[] { - const accounts = cfg.channels?.matrix?.accounts; - if (!accounts || typeof accounts !== "object") { - return []; - } - // Normalize and de-duplicate keys so listing and resolution use the same semantics - return [ - ...new Set( - Object.keys(accounts) - .filter(Boolean) - .map((id) => normalizeAccountId(id)), - ), - ]; -} - -export function listMatrixAccountIds(cfg: CoreConfig): string[] { - const ids = listConfiguredAccountIds(cfg); - if (ids.length === 0) { - // Fall back to default if no accounts configured (legacy top-level config) - return [DEFAULT_ACCOUNT_ID]; - } - return ids.toSorted((a, b) => a.localeCompare(b)); -} - -export function resolveDefaultMatrixAccountId(cfg: CoreConfig): string { - const preferred = normalizeOptionalAccountId(cfg.channels?.matrix?.defaultAccount); - if ( - preferred && - listMatrixAccountIds(cfg).some((accountId) => normalizeAccountId(accountId) === preferred) - ) { - return preferred; - } - const ids = listMatrixAccountIds(cfg); - if (ids.includes(DEFAULT_ACCOUNT_ID)) { - return DEFAULT_ACCOUNT_ID; - } - return ids[0] ?? DEFAULT_ACCOUNT_ID; -} +const { + listAccountIds: listMatrixAccountIds, + resolveDefaultAccountId: resolveDefaultMatrixAccountId, +} = createAccountListHelpers("matrix", { normalizeAccountId }); +export { listMatrixAccountIds, resolveDefaultMatrixAccountId }; function resolveAccountConfig(cfg: CoreConfig, accountId: string): MatrixConfig | undefined { const accounts = cfg.channels?.matrix?.accounts; diff --git a/extensions/mattermost/src/mattermost/accounts.ts b/extensions/mattermost/src/mattermost/accounts.ts index e8a3f5d9572..1de9a09bca8 100644 --- a/extensions/mattermost/src/mattermost/accounts.ts +++ b/extensions/mattermost/src/mattermost/accounts.ts @@ -1,9 +1,5 @@ -import { - DEFAULT_ACCOUNT_ID, - normalizeAccountId, - normalizeOptionalAccountId, -} from "openclaw/plugin-sdk/account-id"; -import type { OpenClawConfig } from "openclaw/plugin-sdk/mattermost"; +import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "openclaw/plugin-sdk/account-id"; +import { createAccountListHelpers, type OpenClawConfig } from "openclaw/plugin-sdk/mattermost"; import { normalizeResolvedSecretInputString, normalizeSecretInputString } from "../secret-input.js"; import type { MattermostAccountConfig, MattermostChatMode } from "../types.js"; import { normalizeMattermostBaseUrl } from "./client.js"; @@ -28,36 +24,11 @@ export type ResolvedMattermostAccount = { blockStreamingCoalesce?: MattermostAccountConfig["blockStreamingCoalesce"]; }; -function listConfiguredAccountIds(cfg: OpenClawConfig): string[] { - const accounts = cfg.channels?.mattermost?.accounts; - if (!accounts || typeof accounts !== "object") { - return []; - } - return Object.keys(accounts).filter(Boolean); -} - -export function listMattermostAccountIds(cfg: OpenClawConfig): string[] { - const ids = listConfiguredAccountIds(cfg); - if (ids.length === 0) { - return [DEFAULT_ACCOUNT_ID]; - } - return ids.toSorted((a, b) => a.localeCompare(b)); -} - -export function resolveDefaultMattermostAccountId(cfg: OpenClawConfig): string { - const preferred = normalizeOptionalAccountId(cfg.channels?.mattermost?.defaultAccount); - if ( - preferred && - listMattermostAccountIds(cfg).some((accountId) => normalizeAccountId(accountId) === preferred) - ) { - return preferred; - } - const ids = listMattermostAccountIds(cfg); - if (ids.includes(DEFAULT_ACCOUNT_ID)) { - return DEFAULT_ACCOUNT_ID; - } - return ids[0] ?? DEFAULT_ACCOUNT_ID; -} +const { + listAccountIds: listMattermostAccountIds, + resolveDefaultAccountId: resolveDefaultMattermostAccountId, +} = createAccountListHelpers("mattermost"); +export { listMattermostAccountIds, resolveDefaultMattermostAccountId }; function resolveAccountConfig( cfg: OpenClawConfig, diff --git a/extensions/nextcloud-talk/src/accounts.ts b/extensions/nextcloud-talk/src/accounts.ts index c2d9d8f40f0..74bb45cfd8b 100644 --- a/extensions/nextcloud-talk/src/accounts.ts +++ b/extensions/nextcloud-talk/src/accounts.ts @@ -1,11 +1,8 @@ import { readFileSync } from "node:fs"; import { + createAccountListHelpers, DEFAULT_ACCOUNT_ID, normalizeAccountId, - normalizeOptionalAccountId, -} from "openclaw/plugin-sdk/account-id"; -import { - listConfiguredAccountIds as listConfiguredAccountIdsFromSection, resolveAccountWithDefaultFallback, } from "openclaw/plugin-sdk/nextcloud-talk"; import { normalizeResolvedSecretInputString } from "./secret-input.js"; @@ -32,37 +29,18 @@ export type ResolvedNextcloudTalkAccount = { config: NextcloudTalkAccountConfig; }; -function listConfiguredAccountIds(cfg: CoreConfig): string[] { - return listConfiguredAccountIdsFromSection({ - accounts: cfg.channels?.["nextcloud-talk"]?.accounts as Record | undefined, - normalizeAccountId, - }); -} +const { + listAccountIds: listNextcloudTalkAccountIdsInternal, + resolveDefaultAccountId: resolveDefaultNextcloudTalkAccountId, +} = createAccountListHelpers("nextcloud-talk", { + normalizeAccountId, +}); +export { resolveDefaultNextcloudTalkAccountId }; export function listNextcloudTalkAccountIds(cfg: CoreConfig): string[] { - const ids = listConfiguredAccountIds(cfg); + const ids = listNextcloudTalkAccountIdsInternal(cfg); debugAccounts("listNextcloudTalkAccountIds", ids); - if (ids.length === 0) { - return [DEFAULT_ACCOUNT_ID]; - } - return ids.toSorted((a, b) => a.localeCompare(b)); -} - -export function resolveDefaultNextcloudTalkAccountId(cfg: CoreConfig): string { - const preferred = normalizeOptionalAccountId(cfg.channels?.["nextcloud-talk"]?.defaultAccount); - if ( - preferred && - listNextcloudTalkAccountIds(cfg).some( - (accountId) => normalizeAccountId(accountId) === preferred, - ) - ) { - return preferred; - } - const ids = listNextcloudTalkAccountIds(cfg); - if (ids.includes(DEFAULT_ACCOUNT_ID)) { - return DEFAULT_ACCOUNT_ID; - } - return ids[0] ?? DEFAULT_ACCOUNT_ID; + return ids; } function resolveAccountConfig( diff --git a/extensions/zalo/src/accounts.ts b/extensions/zalo/src/accounts.ts index c4cb8930cca..205a6b94474 100644 --- a/extensions/zalo/src/accounts.ts +++ b/extensions/zalo/src/accounts.ts @@ -1,45 +1,13 @@ -import { - DEFAULT_ACCOUNT_ID, - normalizeAccountId, - normalizeOptionalAccountId, -} from "openclaw/plugin-sdk/account-id"; -import type { OpenClawConfig } from "openclaw/plugin-sdk/zalo"; +import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "openclaw/plugin-sdk/account-id"; +import { createAccountListHelpers, type OpenClawConfig } from "openclaw/plugin-sdk/zalo"; import { resolveZaloToken } from "./token.js"; import type { ResolvedZaloAccount, ZaloAccountConfig, ZaloConfig } from "./types.js"; export type { ResolvedZaloAccount }; -function listConfiguredAccountIds(cfg: OpenClawConfig): string[] { - const accounts = (cfg.channels?.zalo as ZaloConfig | undefined)?.accounts; - if (!accounts || typeof accounts !== "object") { - return []; - } - return Object.keys(accounts).filter(Boolean); -} - -export function listZaloAccountIds(cfg: OpenClawConfig): string[] { - const ids = listConfiguredAccountIds(cfg); - if (ids.length === 0) { - return [DEFAULT_ACCOUNT_ID]; - } - return ids.toSorted((a, b) => a.localeCompare(b)); -} - -export function resolveDefaultZaloAccountId(cfg: OpenClawConfig): string { - const zaloConfig = cfg.channels?.zalo as ZaloConfig | undefined; - const preferred = normalizeOptionalAccountId(zaloConfig?.defaultAccount); - if ( - preferred && - listZaloAccountIds(cfg).some((accountId) => normalizeAccountId(accountId) === preferred) - ) { - return preferred; - } - const ids = listZaloAccountIds(cfg); - if (ids.includes(DEFAULT_ACCOUNT_ID)) { - return DEFAULT_ACCOUNT_ID; - } - return ids[0] ?? DEFAULT_ACCOUNT_ID; -} +const { listAccountIds: listZaloAccountIds, resolveDefaultAccountId: resolveDefaultZaloAccountId } = + createAccountListHelpers("zalo"); +export { listZaloAccountIds, resolveDefaultZaloAccountId }; function resolveAccountConfig( cfg: OpenClawConfig, diff --git a/extensions/zalouser/src/accounts.ts b/extensions/zalouser/src/accounts.ts index ebf4182f15e..5ebec2d2c93 100644 --- a/extensions/zalouser/src/accounts.ts +++ b/extensions/zalouser/src/accounts.ts @@ -1,43 +1,13 @@ -import { - DEFAULT_ACCOUNT_ID, - normalizeAccountId, - normalizeOptionalAccountId, -} from "openclaw/plugin-sdk/account-id"; -import type { OpenClawConfig } from "openclaw/plugin-sdk/zalouser"; +import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "openclaw/plugin-sdk/account-id"; +import { createAccountListHelpers, type OpenClawConfig } from "openclaw/plugin-sdk/zalouser"; import type { ResolvedZalouserAccount, ZalouserAccountConfig, ZalouserConfig } from "./types.js"; import { checkZaloAuthenticated, getZaloUserInfo } from "./zalo-js.js"; -function listConfiguredAccountIds(cfg: OpenClawConfig): string[] { - const accounts = (cfg.channels?.zalouser as ZalouserConfig | undefined)?.accounts; - if (!accounts || typeof accounts !== "object") { - return []; - } - return Object.keys(accounts).filter(Boolean); -} - -export function listZalouserAccountIds(cfg: OpenClawConfig): string[] { - const ids = listConfiguredAccountIds(cfg); - if (ids.length === 0) { - return [DEFAULT_ACCOUNT_ID]; - } - return ids.toSorted((a, b) => a.localeCompare(b)); -} - -export function resolveDefaultZalouserAccountId(cfg: OpenClawConfig): string { - const zalouserConfig = cfg.channels?.zalouser as ZalouserConfig | undefined; - const preferred = normalizeOptionalAccountId(zalouserConfig?.defaultAccount); - if ( - preferred && - listZalouserAccountIds(cfg).some((accountId) => normalizeAccountId(accountId) === preferred) - ) { - return preferred; - } - const ids = listZalouserAccountIds(cfg); - if (ids.includes(DEFAULT_ACCOUNT_ID)) { - return DEFAULT_ACCOUNT_ID; - } - return ids[0] ?? DEFAULT_ACCOUNT_ID; -} +const { + listAccountIds: listZalouserAccountIds, + resolveDefaultAccountId: resolveDefaultZalouserAccountId, +} = createAccountListHelpers("zalouser"); +export { listZalouserAccountIds, resolveDefaultZalouserAccountId }; function resolveAccountConfig( cfg: OpenClawConfig, diff --git a/src/plugin-sdk/bluebubbles.ts b/src/plugin-sdk/bluebubbles.ts index 8cf9b38b916..bbe87260f86 100644 --- a/src/plugin-sdk/bluebubbles.ts +++ b/src/plugin-sdk/bluebubbles.ts @@ -45,6 +45,7 @@ export { applyAccountNameToChannelSection, migrateBaseNameToDefaultAccount, } from "../channels/plugins/setup-helpers.js"; +export { createAccountListHelpers } from "../channels/plugins/account-helpers.js"; export { collectBlueBubblesStatusIssues } from "../channels/plugins/status-issues/bluebubbles.js"; export type { BaseProbeResult, diff --git a/src/plugin-sdk/googlechat.ts b/src/plugin-sdk/googlechat.ts index 204e0affd3c..40ebe2db47b 100644 --- a/src/plugin-sdk/googlechat.ts +++ b/src/plugin-sdk/googlechat.ts @@ -32,6 +32,7 @@ export { applyAccountNameToChannelSection, migrateBaseNameToDefaultAccount, } from "../channels/plugins/setup-helpers.js"; +export { createAccountListHelpers } from "../channels/plugins/account-helpers.js"; export type { ChannelAccountSnapshot, ChannelMessageActionAdapter, diff --git a/src/plugin-sdk/irc.ts b/src/plugin-sdk/irc.ts index 1983130d193..31321801522 100644 --- a/src/plugin-sdk/irc.ts +++ b/src/plugin-sdk/irc.ts @@ -7,6 +7,7 @@ export { deleteAccountFromConfigSection, setAccountEnabledInConfigSection, } from "../channels/plugins/config-helpers.js"; +export { createAccountListHelpers } from "../channels/plugins/account-helpers.js"; export { buildChannelConfigSchema } from "../channels/plugins/config-schema.js"; export { formatPairingApproveHint } from "../channels/plugins/helpers.js"; export type { diff --git a/src/plugin-sdk/matrix.ts b/src/plugin-sdk/matrix.ts index b029062e28a..5cb039af0c2 100644 --- a/src/plugin-sdk/matrix.ts +++ b/src/plugin-sdk/matrix.ts @@ -39,6 +39,7 @@ export { } from "../channels/plugins/onboarding/helpers.js"; export { PAIRING_APPROVED_MESSAGE } from "../channels/plugins/pairing-message.js"; export { applyAccountNameToChannelSection } from "../channels/plugins/setup-helpers.js"; +export { createAccountListHelpers } from "../channels/plugins/account-helpers.js"; export type { BaseProbeResult, ChannelDirectoryEntry, diff --git a/src/plugin-sdk/mattermost.ts b/src/plugin-sdk/mattermost.ts index 9ad22e60284..ca4af538a78 100644 --- a/src/plugin-sdk/mattermost.ts +++ b/src/plugin-sdk/mattermost.ts @@ -37,6 +37,7 @@ export { applyAccountNameToChannelSection, migrateBaseNameToDefaultAccount, } from "../channels/plugins/setup-helpers.js"; +export { createAccountListHelpers } from "../channels/plugins/account-helpers.js"; export type { BaseProbeResult, ChannelAccountSnapshot, diff --git a/src/plugin-sdk/nextcloud-talk.ts b/src/plugin-sdk/nextcloud-talk.ts index e6f1f3a9bcd..a7b86724a4f 100644 --- a/src/plugin-sdk/nextcloud-talk.ts +++ b/src/plugin-sdk/nextcloud-talk.ts @@ -28,6 +28,7 @@ export { promptSingleChannelSecretInput, } from "../channels/plugins/onboarding/helpers.js"; export { applyAccountNameToChannelSection } from "../channels/plugins/setup-helpers.js"; +export { createAccountListHelpers } from "../channels/plugins/account-helpers.js"; export type { ChannelGroupContext, ChannelSetupInput } from "../channels/plugins/types.js"; export type { ChannelPlugin } from "../channels/plugins/types.plugin.js"; export { createReplyPrefixOptions } from "../channels/reply-prefix.js"; diff --git a/src/plugin-sdk/zalo.ts b/src/plugin-sdk/zalo.ts index a498e050034..c11739dfd4a 100644 --- a/src/plugin-sdk/zalo.ts +++ b/src/plugin-sdk/zalo.ts @@ -25,6 +25,7 @@ export { applyAccountNameToChannelSection, migrateBaseNameToDefaultAccount, } from "../channels/plugins/setup-helpers.js"; +export { createAccountListHelpers } from "../channels/plugins/account-helpers.js"; export type { BaseProbeResult, BaseTokenResolution, diff --git a/src/plugin-sdk/zalouser.ts b/src/plugin-sdk/zalouser.ts index 7558c7f750c..a9d32b3010d 100644 --- a/src/plugin-sdk/zalouser.ts +++ b/src/plugin-sdk/zalouser.ts @@ -25,6 +25,7 @@ export { applyAccountNameToChannelSection, migrateBaseNameToDefaultAccount, } from "../channels/plugins/setup-helpers.js"; +export { createAccountListHelpers } from "../channels/plugins/account-helpers.js"; export type { BaseProbeResult, ChannelAccountSnapshot, From 2ee8b807f8782e5dc201728c617ff6c164b30ece Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 19:47:41 +0000 Subject: [PATCH 04/10] refactor: dedupe discord account inspect config merge --- src/discord/account-inspect.test.ts | 126 ++++++++++++++++++++++++++++ src/discord/account-inspect.ts | 22 ++--- src/discord/accounts.ts | 11 ++- 3 files changed, 138 insertions(+), 21 deletions(-) create mode 100644 src/discord/account-inspect.test.ts diff --git a/src/discord/account-inspect.test.ts b/src/discord/account-inspect.test.ts new file mode 100644 index 00000000000..0e8303635f9 --- /dev/null +++ b/src/discord/account-inspect.test.ts @@ -0,0 +1,126 @@ +import { describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../config/config.js"; +import { inspectDiscordAccount } from "./account-inspect.js"; + +function asConfig(value: unknown): OpenClawConfig { + return value as OpenClawConfig; +} + +describe("inspectDiscordAccount", () => { + it("prefers account token over channel token and strips Bot prefix", () => { + const inspected = inspectDiscordAccount({ + cfg: asConfig({ + channels: { + discord: { + token: "Bot channel-token", + accounts: { + work: { + token: "Bot account-token", + }, + }, + }, + }, + }), + accountId: "work", + }); + + expect(inspected.token).toBe("account-token"); + expect(inspected.tokenSource).toBe("config"); + expect(inspected.tokenStatus).toBe("available"); + expect(inspected.configured).toBe(true); + }); + + it("reports configured_unavailable for unresolved configured secret input", () => { + const inspected = inspectDiscordAccount({ + cfg: asConfig({ + channels: { + discord: { + accounts: { + work: { + token: { source: "env", id: "DISCORD_TOKEN" }, + }, + }, + }, + }, + }), + accountId: "work", + }); + + expect(inspected.token).toBe(""); + expect(inspected.tokenSource).toBe("config"); + expect(inspected.tokenStatus).toBe("configured_unavailable"); + expect(inspected.configured).toBe(true); + }); + + it("does not fall back when account token key exists but is missing", () => { + const inspected = inspectDiscordAccount({ + cfg: asConfig({ + channels: { + discord: { + token: "Bot channel-token", + accounts: { + work: { + token: "", + }, + }, + }, + }, + }), + accountId: "work", + }); + + expect(inspected.token).toBe(""); + expect(inspected.tokenSource).toBe("none"); + expect(inspected.tokenStatus).toBe("missing"); + expect(inspected.configured).toBe(false); + }); + + it("falls back to channel token when account token is absent", () => { + const inspected = inspectDiscordAccount({ + cfg: asConfig({ + channels: { + discord: { + token: "Bot channel-token", + accounts: { + work: {}, + }, + }, + }, + }), + accountId: "work", + }); + + expect(inspected.token).toBe("channel-token"); + expect(inspected.tokenSource).toBe("config"); + expect(inspected.tokenStatus).toBe("available"); + expect(inspected.configured).toBe(true); + }); + + it("allows env token only for default account", () => { + const defaultInspected = inspectDiscordAccount({ + cfg: asConfig({}), + accountId: "default", + envToken: "Bot env-default", + }); + const namedInspected = inspectDiscordAccount({ + cfg: asConfig({ + channels: { + discord: { + accounts: { + work: {}, + }, + }, + }, + }), + accountId: "work", + envToken: "Bot env-work", + }); + + expect(defaultInspected.token).toBe("env-default"); + expect(defaultInspected.tokenSource).toBe("env"); + expect(defaultInspected.configured).toBe(true); + expect(namedInspected.token).toBe(""); + expect(namedInspected.tokenSource).toBe("none"); + expect(namedInspected.configured).toBe(false); + }); +}); diff --git a/src/discord/account-inspect.ts b/src/discord/account-inspect.ts index 0ece2072744..53357ffd636 100644 --- a/src/discord/account-inspect.ts +++ b/src/discord/account-inspect.ts @@ -1,9 +1,12 @@ import type { OpenClawConfig } from "../config/config.js"; import type { DiscordAccountConfig } from "../config/types.discord.js"; import { hasConfiguredSecretInput, normalizeSecretInputString } from "../config/types.secrets.js"; -import { resolveAccountEntry } from "../routing/account-lookup.js"; import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "../routing/session-key.js"; -import { resolveDefaultDiscordAccountId } from "./accounts.js"; +import { + mergeDiscordAccountConfig, + resolveDefaultDiscordAccountId, + resolveDiscordAccountConfig, +} from "./accounts.js"; export type DiscordCredentialStatus = "available" | "configured_unavailable" | "missing"; @@ -18,21 +21,6 @@ export type InspectedDiscordAccount = { config: DiscordAccountConfig; }; -function resolveDiscordAccountConfig( - cfg: OpenClawConfig, - accountId: string, -): DiscordAccountConfig | undefined { - return resolveAccountEntry(cfg.channels?.discord?.accounts, accountId); -} - -function mergeDiscordAccountConfig(cfg: OpenClawConfig, accountId: string): DiscordAccountConfig { - const { accounts: _ignored, ...base } = (cfg.channels?.discord ?? {}) as DiscordAccountConfig & { - accounts?: unknown; - }; - const account = resolveDiscordAccountConfig(cfg, accountId) ?? {}; - return { ...base, ...account }; -} - function inspectDiscordTokenValue(value: unknown): { token: string; tokenSource: "config"; diff --git a/src/discord/accounts.ts b/src/discord/accounts.ts index 33731b4260d..75eeff40b3e 100644 --- a/src/discord/accounts.ts +++ b/src/discord/accounts.ts @@ -19,18 +19,21 @@ const { listAccountIds, resolveDefaultAccountId } = createAccountListHelpers("di export const listDiscordAccountIds = listAccountIds; export const resolveDefaultDiscordAccountId = resolveDefaultAccountId; -function resolveAccountConfig( +export function resolveDiscordAccountConfig( cfg: OpenClawConfig, accountId: string, ): DiscordAccountConfig | undefined { return resolveAccountEntry(cfg.channels?.discord?.accounts, accountId); } -function mergeDiscordAccountConfig(cfg: OpenClawConfig, accountId: string): DiscordAccountConfig { +export function mergeDiscordAccountConfig( + cfg: OpenClawConfig, + accountId: string, +): DiscordAccountConfig { const { accounts: _ignored, ...base } = (cfg.channels?.discord ?? {}) as DiscordAccountConfig & { accounts?: unknown; }; - const account = resolveAccountConfig(cfg, accountId) ?? {}; + const account = resolveDiscordAccountConfig(cfg, accountId) ?? {}; return { ...base, ...account }; } @@ -41,7 +44,7 @@ export function createDiscordActionGate(params: { const accountId = normalizeAccountId(params.accountId); return createAccountActionGate({ baseActions: params.cfg.channels?.discord?.actions, - accountActions: resolveAccountConfig(params.cfg, accountId)?.actions, + accountActions: resolveDiscordAccountConfig(params.cfg, accountId)?.actions, }); } From b0ac284dae4863a6daaad345032cbb27d7a2521f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 19:53:31 +0000 Subject: [PATCH 05/10] refactor: share setup account config patch helper --- extensions/bluebubbles/src/channel.ts | 12 ---- extensions/googlechat/src/channel.ts | 38 ++-------- extensions/mattermost/src/channel.ts | 50 ++++--------- extensions/zalo/src/channel.ts | 55 ++++----------- extensions/zalouser/src/channel.ts | 36 ++-------- src/channels/plugins/setup-helpers.test.ts | 81 ++++++++++++++++++++++ src/channels/plugins/setup-helpers.ts | 50 +++++++++++++ src/plugin-sdk/googlechat.ts | 1 + src/plugin-sdk/mattermost.ts | 1 + src/plugin-sdk/zalo.ts | 1 + src/plugin-sdk/zalouser.ts | 1 + 11 files changed, 176 insertions(+), 150 deletions(-) create mode 100644 src/channels/plugins/setup-helpers.test.ts diff --git a/extensions/bluebubbles/src/channel.ts b/extensions/bluebubbles/src/channel.ts index 741f93d3ae0..8e3ae2ea6a5 100644 --- a/extensions/bluebubbles/src/channel.ts +++ b/extensions/bluebubbles/src/channel.ts @@ -256,18 +256,6 @@ export const bluebubblesPlugin: ChannelPlugin = { channelKey: "bluebubbles", }) : namedConfig; - if (accountId === DEFAULT_ACCOUNT_ID) { - return applyBlueBubblesConnectionConfig({ - cfg: next, - accountId, - patch: { - serverUrl: input.httpUrl, - password: input.password, - webhookPath: input.webhookPath, - }, - onlyDefinedFields: true, - }); - } return applyBlueBubblesConnectionConfig({ cfg: next, accountId, diff --git a/extensions/googlechat/src/channel.ts b/extensions/googlechat/src/channel.ts index 6dd896e9f00..333f5c74ac5 100644 --- a/extensions/googlechat/src/channel.ts +++ b/extensions/googlechat/src/channel.ts @@ -1,5 +1,6 @@ import { applyAccountNameToChannelSection, + applySetupAccountConfigPatch, buildChannelConfigSchema, DEFAULT_ACCOUNT_ID, deleteAccountFromConfigSection, @@ -345,37 +346,12 @@ export const googlechatPlugin: ChannelPlugin = { ...(webhookPath ? { webhookPath } : {}), ...(webhookUrl ? { webhookUrl } : {}), }; - if (accountId === DEFAULT_ACCOUNT_ID) { - return { - ...next, - channels: { - ...next.channels, - googlechat: { - ...next.channels?.["googlechat"], - enabled: true, - ...configPatch, - }, - }, - } as OpenClawConfig; - } - return { - ...next, - channels: { - ...next.channels, - googlechat: { - ...next.channels?.["googlechat"], - enabled: true, - accounts: { - ...next.channels?.["googlechat"]?.accounts, - [accountId]: { - ...next.channels?.["googlechat"]?.accounts?.[accountId], - enabled: true, - ...configPatch, - }, - }, - }, - }, - } as OpenClawConfig; + return applySetupAccountConfigPatch({ + cfg: next, + channelKey: "googlechat", + accountId, + patch: configPatch, + }); }, }, outbound: { diff --git a/extensions/mattermost/src/channel.ts b/extensions/mattermost/src/channel.ts index b9f5a3bc85d..c413e1790f9 100644 --- a/extensions/mattermost/src/channel.ts +++ b/extensions/mattermost/src/channel.ts @@ -1,5 +1,6 @@ import { applyAccountNameToChannelSection, + applySetupAccountConfigPatch, buildChannelConfigSchema, DEFAULT_ACCOUNT_ID, deleteAccountFromConfigSection, @@ -449,43 +450,18 @@ export const mattermostPlugin: ChannelPlugin = { channelKey: "mattermost", }) : namedConfig; - if (accountId === DEFAULT_ACCOUNT_ID) { - return { - ...next, - channels: { - ...next.channels, - mattermost: { - ...next.channels?.mattermost, - enabled: true, - ...(input.useEnv - ? {} - : { - ...(token ? { botToken: token } : {}), - ...(baseUrl ? { baseUrl } : {}), - }), - }, - }, - }; - } - return { - ...next, - channels: { - ...next.channels, - mattermost: { - ...next.channels?.mattermost, - enabled: true, - accounts: { - ...next.channels?.mattermost?.accounts, - [accountId]: { - ...next.channels?.mattermost?.accounts?.[accountId], - enabled: true, - ...(token ? { botToken: token } : {}), - ...(baseUrl ? { baseUrl } : {}), - }, - }, - }, - }, - }; + const patch = input.useEnv + ? {} + : { + ...(token ? { botToken: token } : {}), + ...(baseUrl ? { baseUrl } : {}), + }; + return applySetupAccountConfigPatch({ + cfg: next, + channelKey: "mattermost", + accountId, + patch, + }); }, }, gateway: { diff --git a/extensions/zalo/src/channel.ts b/extensions/zalo/src/channel.ts index b6a7f7d0486..54dce454693 100644 --- a/extensions/zalo/src/channel.ts +++ b/extensions/zalo/src/channel.ts @@ -6,6 +6,7 @@ import type { } from "openclaw/plugin-sdk/zalo"; import { applyAccountNameToChannelSection, + applySetupAccountConfigPatch, buildBaseAccountStatusSnapshot, buildChannelConfigSchema, buildTokenChannelStatusSummary, @@ -243,47 +244,19 @@ export const zaloPlugin: ChannelPlugin = { channelKey: "zalo", }) : namedConfig; - if (accountId === DEFAULT_ACCOUNT_ID) { - return { - ...next, - channels: { - ...next.channels, - zalo: { - ...next.channels?.zalo, - enabled: true, - ...(input.useEnv - ? {} - : input.tokenFile - ? { tokenFile: input.tokenFile } - : input.token - ? { botToken: input.token } - : {}), - }, - }, - } as OpenClawConfig; - } - return { - ...next, - channels: { - ...next.channels, - zalo: { - ...next.channels?.zalo, - enabled: true, - accounts: { - ...next.channels?.zalo?.accounts, - [accountId]: { - ...next.channels?.zalo?.accounts?.[accountId], - enabled: true, - ...(input.tokenFile - ? { tokenFile: input.tokenFile } - : input.token - ? { botToken: input.token } - : {}), - }, - }, - }, - }, - } as OpenClawConfig; + const patch = input.useEnv + ? {} + : input.tokenFile + ? { tokenFile: input.tokenFile } + : input.token + ? { botToken: input.token } + : {}; + return applySetupAccountConfigPatch({ + cfg: next, + channelKey: "zalo", + accountId, + patch, + }); }, }, pairing: { diff --git a/extensions/zalouser/src/channel.ts b/extensions/zalouser/src/channel.ts index 41327f1fe7e..2de31e9aa3e 100644 --- a/extensions/zalouser/src/channel.ts +++ b/extensions/zalouser/src/channel.ts @@ -10,6 +10,7 @@ import type { } from "openclaw/plugin-sdk/zalouser"; import { applyAccountNameToChannelSection, + applySetupAccountConfigPatch, buildChannelSendResult, buildBaseAccountStatusSnapshot, buildChannelConfigSchema, @@ -329,35 +330,12 @@ export const zalouserPlugin: ChannelPlugin = { channelKey: "zalouser", }) : namedConfig; - if (accountId === DEFAULT_ACCOUNT_ID) { - return { - ...next, - channels: { - ...next.channels, - zalouser: { - ...next.channels?.zalouser, - enabled: true, - }, - }, - } as OpenClawConfig; - } - return { - ...next, - channels: { - ...next.channels, - zalouser: { - ...next.channels?.zalouser, - enabled: true, - accounts: { - ...next.channels?.zalouser?.accounts, - [accountId]: { - ...next.channels?.zalouser?.accounts?.[accountId], - enabled: true, - }, - }, - }, - }, - } as OpenClawConfig; + return applySetupAccountConfigPatch({ + cfg: next, + channelKey: "zalouser", + accountId, + patch: {}, + }); }, }, messaging: { diff --git a/src/channels/plugins/setup-helpers.test.ts b/src/channels/plugins/setup-helpers.test.ts new file mode 100644 index 00000000000..df4609fc76f --- /dev/null +++ b/src/channels/plugins/setup-helpers.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../../config/config.js"; +import { DEFAULT_ACCOUNT_ID } from "../../routing/session-key.js"; +import { applySetupAccountConfigPatch } from "./setup-helpers.js"; + +function asConfig(value: unknown): OpenClawConfig { + return value as OpenClawConfig; +} + +describe("applySetupAccountConfigPatch", () => { + it("patches top-level config for default account and enables channel", () => { + const next = applySetupAccountConfigPatch({ + cfg: asConfig({ + channels: { + zalo: { + webhookPath: "/old", + enabled: false, + }, + }, + }), + channelKey: "zalo", + accountId: DEFAULT_ACCOUNT_ID, + patch: { webhookPath: "/new", botToken: "tok" }, + }); + + expect(next.channels?.zalo).toMatchObject({ + enabled: true, + webhookPath: "/new", + botToken: "tok", + }); + }); + + it("patches named account config and enables both channel and account", () => { + const next = applySetupAccountConfigPatch({ + cfg: asConfig({ + channels: { + zalo: { + enabled: false, + accounts: { + work: { botToken: "old", enabled: false }, + }, + }, + }, + }), + channelKey: "zalo", + accountId: "work", + patch: { botToken: "new" }, + }); + + expect(next.channels?.zalo).toMatchObject({ + enabled: true, + accounts: { + work: { enabled: true, botToken: "new" }, + }, + }); + }); + + it("normalizes account id and preserves other accounts", () => { + const next = applySetupAccountConfigPatch({ + cfg: asConfig({ + channels: { + zalo: { + accounts: { + personal: { botToken: "personal-token" }, + }, + }, + }, + }), + channelKey: "zalo", + accountId: "Work Team", + patch: { botToken: "work-token" }, + }); + + expect(next.channels?.zalo).toMatchObject({ + accounts: { + personal: { botToken: "personal-token" }, + "work-team": { enabled: true, botToken: "work-token" }, + }, + }); + }); +}); diff --git a/src/channels/plugins/setup-helpers.ts b/src/channels/plugins/setup-helpers.ts index 72b3163a62e..5045c431d60 100644 --- a/src/channels/plugins/setup-helpers.ts +++ b/src/channels/plugins/setup-helpers.ts @@ -120,6 +120,56 @@ export function migrateBaseNameToDefaultAccount(params: { } as OpenClawConfig; } +export function applySetupAccountConfigPatch(params: { + cfg: OpenClawConfig; + channelKey: string; + accountId: string; + patch: Record; +}): OpenClawConfig { + const accountId = normalizeAccountId(params.accountId); + const channels = params.cfg.channels as Record | undefined; + const channelConfig = channels?.[params.channelKey]; + const base = + typeof channelConfig === "object" && channelConfig + ? (channelConfig as Record & { + accounts?: Record>; + }) + : undefined; + if (accountId === DEFAULT_ACCOUNT_ID) { + return { + ...params.cfg, + channels: { + ...params.cfg.channels, + [params.channelKey]: { + ...base, + enabled: true, + ...params.patch, + }, + }, + } as OpenClawConfig; + } + + const accounts = base?.accounts ?? {}; + return { + ...params.cfg, + channels: { + ...params.cfg.channels, + [params.channelKey]: { + ...base, + enabled: true, + accounts: { + ...accounts, + [accountId]: { + ...accounts[accountId], + enabled: true, + ...params.patch, + }, + }, + }, + }, + } as OpenClawConfig; +} + type ChannelSectionRecord = Record & { accounts?: Record>; }; diff --git a/src/plugin-sdk/googlechat.ts b/src/plugin-sdk/googlechat.ts index 40ebe2db47b..0b84d803d5f 100644 --- a/src/plugin-sdk/googlechat.ts +++ b/src/plugin-sdk/googlechat.ts @@ -30,6 +30,7 @@ export { export { PAIRING_APPROVED_MESSAGE } from "../channels/plugins/pairing-message.js"; export { applyAccountNameToChannelSection, + applySetupAccountConfigPatch, migrateBaseNameToDefaultAccount, } from "../channels/plugins/setup-helpers.js"; export { createAccountListHelpers } from "../channels/plugins/account-helpers.js"; diff --git a/src/plugin-sdk/mattermost.ts b/src/plugin-sdk/mattermost.ts index ca4af538a78..4df37da8dd8 100644 --- a/src/plugin-sdk/mattermost.ts +++ b/src/plugin-sdk/mattermost.ts @@ -35,6 +35,7 @@ export { } from "../channels/plugins/onboarding/helpers.js"; export { applyAccountNameToChannelSection, + applySetupAccountConfigPatch, migrateBaseNameToDefaultAccount, } from "../channels/plugins/setup-helpers.js"; export { createAccountListHelpers } from "../channels/plugins/account-helpers.js"; diff --git a/src/plugin-sdk/zalo.ts b/src/plugin-sdk/zalo.ts index c11739dfd4a..011b95e43cb 100644 --- a/src/plugin-sdk/zalo.ts +++ b/src/plugin-sdk/zalo.ts @@ -23,6 +23,7 @@ export { export { PAIRING_APPROVED_MESSAGE } from "../channels/plugins/pairing-message.js"; export { applyAccountNameToChannelSection, + applySetupAccountConfigPatch, migrateBaseNameToDefaultAccount, } from "../channels/plugins/setup-helpers.js"; export { createAccountListHelpers } from "../channels/plugins/account-helpers.js"; diff --git a/src/plugin-sdk/zalouser.ts b/src/plugin-sdk/zalouser.ts index a9d32b3010d..e5c076f8ca2 100644 --- a/src/plugin-sdk/zalouser.ts +++ b/src/plugin-sdk/zalouser.ts @@ -23,6 +23,7 @@ export { } from "../channels/plugins/onboarding/helpers.js"; export { applyAccountNameToChannelSection, + applySetupAccountConfigPatch, migrateBaseNameToDefaultAccount, } from "../channels/plugins/setup-helpers.js"; export { createAccountListHelpers } from "../channels/plugins/account-helpers.js"; From b9e7521463287714b736481c5683ff9ab937a761 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 19:57:22 +0000 Subject: [PATCH 06/10] refactor: unify directory config entry extraction --- extensions/googlechat/src/channel.ts | 35 ++++------ extensions/zalo/src/channel.ts | 20 ++---- .../plugins/directory-config-helpers.test.ts | 39 +++++++++++ .../plugins/directory-config-helpers.ts | 65 +++++++++++++++++++ src/plugin-sdk/googlechat.ts | 4 ++ src/plugin-sdk/zalo.ts | 1 + 6 files changed, 129 insertions(+), 35 deletions(-) create mode 100644 src/channels/plugins/directory-config-helpers.test.ts create mode 100644 src/channels/plugins/directory-config-helpers.ts diff --git a/extensions/googlechat/src/channel.ts b/extensions/googlechat/src/channel.ts index 333f5c74ac5..c7527652f55 100644 --- a/extensions/googlechat/src/channel.ts +++ b/extensions/googlechat/src/channel.ts @@ -6,6 +6,8 @@ import { deleteAccountFromConfigSection, formatPairingApproveHint, getChatChannelMeta, + listDirectoryGroupEntriesFromMapKeys, + listDirectoryUserEntriesFromAllowFrom, migrateBaseNameToDefaultAccount, missingTargetError, normalizeAccountId, @@ -243,34 +245,23 @@ export const googlechatPlugin: ChannelPlugin = { cfg: cfg, accountId, }); - const q = query?.trim().toLowerCase() || ""; - const allowFrom = account.config.dm?.allowFrom ?? []; - const peers = Array.from( - new Set( - allowFrom - .map((entry) => String(entry).trim()) - .filter((entry) => Boolean(entry) && entry !== "*") - .map((entry) => normalizeGoogleChatTarget(entry) ?? entry), - ), - ) - .filter((id) => (q ? id.toLowerCase().includes(q) : true)) - .slice(0, limit && limit > 0 ? limit : undefined) - .map((id) => ({ kind: "user", id }) as const); - return peers; + return listDirectoryUserEntriesFromAllowFrom({ + allowFrom: account.config.dm?.allowFrom, + query, + limit, + normalizeId: (entry) => normalizeGoogleChatTarget(entry) ?? entry, + }); }, listGroups: async ({ cfg, accountId, query, limit }) => { const account = resolveGoogleChatAccount({ cfg: cfg, accountId, }); - const groups = account.config.groups ?? {}; - const q = query?.trim().toLowerCase() || ""; - const entries = Object.keys(groups) - .filter((key) => key && key !== "*") - .filter((key) => (q ? key.toLowerCase().includes(q) : true)) - .slice(0, limit && limit > 0 ? limit : undefined) - .map((id) => ({ kind: "group", id }) as const); - return entries; + return listDirectoryGroupEntriesFromMapKeys({ + groups: account.config.groups, + query, + limit, + }); }, }, resolver: { diff --git a/extensions/zalo/src/channel.ts b/extensions/zalo/src/channel.ts index 54dce454693..9de72910ab7 100644 --- a/extensions/zalo/src/channel.ts +++ b/extensions/zalo/src/channel.ts @@ -17,6 +17,7 @@ import { formatAllowFromLowercase, formatPairingApproveHint, migrateBaseNameToDefaultAccount, + listDirectoryUserEntriesFromAllowFrom, normalizeAccountId, isNumericTargetId, PAIRING_APPROVED_MESSAGE, @@ -196,19 +197,12 @@ export const zaloPlugin: ChannelPlugin = { self: async () => null, listPeers: async ({ cfg, accountId, query, limit }) => { const account = resolveZaloAccount({ cfg: cfg, accountId }); - const q = query?.trim().toLowerCase() || ""; - const peers = Array.from( - new Set( - (account.config.allowFrom ?? []) - .map((entry) => String(entry).trim()) - .filter((entry) => Boolean(entry) && entry !== "*") - .map((entry) => entry.replace(/^(zalo|zl):/i, "")), - ), - ) - .filter((id) => (q ? id.toLowerCase().includes(q) : true)) - .slice(0, limit && limit > 0 ? limit : undefined) - .map((id) => ({ kind: "user", id }) as const); - return peers; + return listDirectoryUserEntriesFromAllowFrom({ + allowFrom: account.config.allowFrom, + query, + limit, + normalizeId: (entry) => entry.replace(/^(zalo|zl):/i, ""), + }); }, listGroups: async () => [], }, diff --git a/src/channels/plugins/directory-config-helpers.test.ts b/src/channels/plugins/directory-config-helpers.test.ts new file mode 100644 index 00000000000..c4f5370cc4a --- /dev/null +++ b/src/channels/plugins/directory-config-helpers.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; +import { + listDirectoryGroupEntriesFromMapKeys, + listDirectoryUserEntriesFromAllowFrom, +} from "./directory-config-helpers.js"; + +describe("listDirectoryUserEntriesFromAllowFrom", () => { + it("normalizes, deduplicates, filters, and limits user ids", () => { + const entries = listDirectoryUserEntriesFromAllowFrom({ + allowFrom: ["", "*", " user:Alice ", "user:alice", "user:Bob", "user:Carla"], + normalizeId: (entry) => entry.replace(/^user:/i, "").toLowerCase(), + query: "a", + limit: 2, + }); + + expect(entries).toEqual([ + { kind: "user", id: "alice" }, + { kind: "user", id: "carla" }, + ]); + }); +}); + +describe("listDirectoryGroupEntriesFromMapKeys", () => { + it("extracts normalized group ids from map keys", () => { + const entries = listDirectoryGroupEntriesFromMapKeys({ + groups: { + "*": {}, + " Space/A ": {}, + "space/b": {}, + }, + normalizeId: (entry) => entry.toLowerCase().replace(/\s+/g, ""), + }); + + expect(entries).toEqual([ + { kind: "group", id: "space/a" }, + { kind: "group", id: "space/b" }, + ]); + }); +}); diff --git a/src/channels/plugins/directory-config-helpers.ts b/src/channels/plugins/directory-config-helpers.ts new file mode 100644 index 00000000000..b431bc14a8b --- /dev/null +++ b/src/channels/plugins/directory-config-helpers.ts @@ -0,0 +1,65 @@ +import type { ChannelDirectoryEntry } from "./types.js"; + +function resolveDirectoryQuery(query?: string | null): string { + return query?.trim().toLowerCase() || ""; +} + +function resolveDirectoryLimit(limit?: number | null): number | undefined { + return typeof limit === "number" && limit > 0 ? limit : undefined; +} + +function applyDirectoryQueryAndLimit( + ids: string[], + params: { query?: string | null; limit?: number | null }, +): string[] { + const q = resolveDirectoryQuery(params.query); + const limit = resolveDirectoryLimit(params.limit); + const filtered = ids.filter((id) => (q ? id.toLowerCase().includes(q) : true)); + return typeof limit === "number" ? filtered.slice(0, limit) : filtered; +} + +function toDirectoryEntries(kind: "user" | "group", ids: string[]): ChannelDirectoryEntry[] { + return ids.map((id) => ({ kind, id }) as const); +} + +export function listDirectoryUserEntriesFromAllowFrom(params: { + allowFrom?: readonly unknown[]; + query?: string | null; + limit?: number | null; + normalizeId?: (entry: string) => string | null | undefined; +}): ChannelDirectoryEntry[] { + const ids = Array.from( + new Set( + (params.allowFrom ?? []) + .map((entry) => String(entry).trim()) + .filter((entry) => Boolean(entry) && entry !== "*") + .map((entry) => { + const normalized = params.normalizeId ? params.normalizeId(entry) : entry; + return typeof normalized === "string" ? normalized.trim() : ""; + }) + .filter(Boolean), + ), + ); + return toDirectoryEntries("user", applyDirectoryQueryAndLimit(ids, params)); +} + +export function listDirectoryGroupEntriesFromMapKeys(params: { + groups?: Record; + query?: string | null; + limit?: number | null; + normalizeId?: (entry: string) => string | null | undefined; +}): ChannelDirectoryEntry[] { + const ids = Array.from( + new Set( + Object.keys(params.groups ?? {}) + .map((entry) => entry.trim()) + .filter((entry) => Boolean(entry) && entry !== "*") + .map((entry) => { + const normalized = params.normalizeId ? params.normalizeId(entry) : entry; + return typeof normalized === "string" ? normalized.trim() : ""; + }) + .filter(Boolean), + ), + ); + return toDirectoryEntries("group", applyDirectoryQueryAndLimit(ids, params)); +} diff --git a/src/plugin-sdk/googlechat.ts b/src/plugin-sdk/googlechat.ts index 0b84d803d5f..59bc1849119 100644 --- a/src/plugin-sdk/googlechat.ts +++ b/src/plugin-sdk/googlechat.ts @@ -14,6 +14,10 @@ export { deleteAccountFromConfigSection, setAccountEnabledInConfigSection, } from "../channels/plugins/config-helpers.js"; +export { + listDirectoryGroupEntriesFromMapKeys, + listDirectoryUserEntriesFromAllowFrom, +} from "../channels/plugins/directory-config-helpers.js"; export { buildChannelConfigSchema } from "../channels/plugins/config-schema.js"; export { resolveGoogleChatGroupRequireMention } from "../channels/plugins/group-mentions.js"; export { formatPairingApproveHint } from "../channels/plugins/helpers.js"; diff --git a/src/plugin-sdk/zalo.ts b/src/plugin-sdk/zalo.ts index 011b95e43cb..5732a4e138c 100644 --- a/src/plugin-sdk/zalo.ts +++ b/src/plugin-sdk/zalo.ts @@ -8,6 +8,7 @@ export { deleteAccountFromConfigSection, setAccountEnabledInConfigSection, } from "../channels/plugins/config-helpers.js"; +export { listDirectoryUserEntriesFromAllowFrom } from "../channels/plugins/directory-config-helpers.js"; export { buildChannelConfigSchema } from "../channels/plugins/config-schema.js"; export { formatPairingApproveHint } from "../channels/plugins/helpers.js"; export type { From 95fe282a17335ca140fec75b4c960abd69a03dea Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 20:01:41 +0000 Subject: [PATCH 07/10] refactor: unify channel status snapshot base fields --- extensions/discord/src/channel.ts | 15 +++++------ extensions/googlechat/src/channel.ts | 39 ++++++++++++++-------------- extensions/mattermost/src/channel.ts | 37 +++++++++++++------------- extensions/slack/src/channel.ts | 15 +++++------ src/plugin-sdk/discord.ts | 5 +++- src/plugin-sdk/googlechat.ts | 1 + src/plugin-sdk/mattermost.ts | 1 + src/plugin-sdk/slack.ts | 1 + 8 files changed, 60 insertions(+), 54 deletions(-) diff --git a/extensions/discord/src/channel.ts b/extensions/discord/src/channel.ts index 04f8b5ab3a8..5744bb3d946 100644 --- a/extensions/discord/src/channel.ts +++ b/extensions/discord/src/channel.ts @@ -1,5 +1,6 @@ import { applyAccountNameToChannelSection, + buildComputedAccountStatusSnapshot, buildChannelConfigSchema, buildTokenChannelStatusSummary, collectDiscordAuditChannelIds, @@ -398,16 +399,17 @@ export const discordPlugin: ChannelPlugin = { resolveConfiguredFromCredentialStatuses(account) ?? Boolean(account.token?.trim()); const app = runtime?.application ?? (probe as { application?: unknown })?.application; const bot = runtime?.bot ?? (probe as { bot?: unknown })?.bot; - return { + const base = buildComputedAccountStatusSnapshot({ accountId: account.accountId, name: account.name, enabled: account.enabled, configured, + runtime, + probe, + }); + return { + ...base, ...projectCredentialSnapshotFields(account), - running: runtime?.running ?? false, - lastStartAt: runtime?.lastStartAt ?? null, - lastStopAt: runtime?.lastStopAt ?? null, - lastError: runtime?.lastError ?? null, connected: runtime?.connected ?? false, reconnectAttempts: runtime?.reconnectAttempts, lastConnectedAt: runtime?.lastConnectedAt ?? null, @@ -415,10 +417,7 @@ export const discordPlugin: ChannelPlugin = { lastEventAt: runtime?.lastEventAt ?? null, application: app ?? undefined, bot: bot ?? undefined, - probe, audit, - lastInboundAt: runtime?.lastInboundAt ?? null, - lastOutboundAt: runtime?.lastOutboundAt ?? null, }; }, }, diff --git a/extensions/googlechat/src/channel.ts b/extensions/googlechat/src/channel.ts index c7527652f55..8026334d21a 100644 --- a/extensions/googlechat/src/channel.ts +++ b/extensions/googlechat/src/channel.ts @@ -1,6 +1,7 @@ import { applyAccountNameToChannelSection, applySetupAccountConfigPatch, + buildComputedAccountStatusSnapshot, buildChannelConfigSchema, DEFAULT_ACCOUNT_ID, deleteAccountFromConfigSection, @@ -504,25 +505,25 @@ export const googlechatPlugin: ChannelPlugin = { lastProbeAt: snapshot.lastProbeAt ?? null, }), probeAccount: async ({ account }) => probeGoogleChat(account), - buildAccountSnapshot: ({ account, runtime, probe }) => ({ - accountId: account.accountId, - name: account.name, - enabled: account.enabled, - configured: account.credentialSource !== "none", - credentialSource: account.credentialSource, - audienceType: account.config.audienceType, - audience: account.config.audience, - webhookPath: account.config.webhookPath, - webhookUrl: account.config.webhookUrl, - running: runtime?.running ?? false, - lastStartAt: runtime?.lastStartAt ?? null, - lastStopAt: runtime?.lastStopAt ?? null, - lastError: runtime?.lastError ?? null, - lastInboundAt: runtime?.lastInboundAt ?? null, - lastOutboundAt: runtime?.lastOutboundAt ?? null, - dmPolicy: account.config.dm?.policy ?? "pairing", - probe, - }), + buildAccountSnapshot: ({ account, runtime, probe }) => { + const base = buildComputedAccountStatusSnapshot({ + accountId: account.accountId, + name: account.name, + enabled: account.enabled, + configured: account.credentialSource !== "none", + runtime, + probe, + }); + return { + ...base, + credentialSource: account.credentialSource, + audienceType: account.config.audienceType, + audience: account.config.audience, + webhookPath: account.config.webhookPath, + webhookUrl: account.config.webhookUrl, + dmPolicy: account.config.dm?.policy ?? "pairing", + }; + }, }, gateway: { startAccount: async (ctx) => { diff --git a/extensions/mattermost/src/channel.ts b/extensions/mattermost/src/channel.ts index c413e1790f9..d1a4135a92b 100644 --- a/extensions/mattermost/src/channel.ts +++ b/extensions/mattermost/src/channel.ts @@ -1,6 +1,7 @@ import { applyAccountNameToChannelSection, applySetupAccountConfigPatch, + buildComputedAccountStatusSnapshot, buildChannelConfigSchema, DEFAULT_ACCOUNT_ID, deleteAccountFromConfigSection, @@ -392,24 +393,24 @@ export const mattermostPlugin: ChannelPlugin = { } return await probeMattermost(baseUrl, token, timeoutMs); }, - buildAccountSnapshot: ({ account, runtime, probe }) => ({ - accountId: account.accountId, - name: account.name, - enabled: account.enabled, - configured: Boolean(account.botToken && account.baseUrl), - botTokenSource: account.botTokenSource, - baseUrl: account.baseUrl, - running: runtime?.running ?? false, - connected: runtime?.connected ?? false, - lastConnectedAt: runtime?.lastConnectedAt ?? null, - lastDisconnect: runtime?.lastDisconnect ?? null, - lastStartAt: runtime?.lastStartAt ?? null, - lastStopAt: runtime?.lastStopAt ?? null, - lastError: runtime?.lastError ?? null, - probe, - lastInboundAt: runtime?.lastInboundAt ?? null, - lastOutboundAt: runtime?.lastOutboundAt ?? null, - }), + buildAccountSnapshot: ({ account, runtime, probe }) => { + const base = buildComputedAccountStatusSnapshot({ + accountId: account.accountId, + name: account.name, + enabled: account.enabled, + configured: Boolean(account.botToken && account.baseUrl), + runtime, + probe, + }); + return { + ...base, + botTokenSource: account.botTokenSource, + baseUrl: account.baseUrl, + connected: runtime?.connected ?? false, + lastConnectedAt: runtime?.lastConnectedAt ?? null, + lastDisconnect: runtime?.lastDisconnect ?? null, + }; + }, }, setup: { resolveAccountId: ({ accountId }) => normalizeAccountId(accountId), diff --git a/extensions/slack/src/channel.ts b/extensions/slack/src/channel.ts index 2589a577689..39951538cf3 100644 --- a/extensions/slack/src/channel.ts +++ b/extensions/slack/src/channel.ts @@ -1,5 +1,6 @@ import { applyAccountNameToChannelSection, + buildComputedAccountStatusSnapshot, buildChannelConfigSchema, DEFAULT_ACCOUNT_ID, deleteAccountFromConfigSection, @@ -443,19 +444,17 @@ export const slackPlugin: ChannelPlugin = { "botTokenStatus", "appTokenStatus", ])) ?? isSlackAccountConfigured(account); - return { + const base = buildComputedAccountStatusSnapshot({ accountId: account.accountId, name: account.name, enabled: account.enabled, configured, - ...projectCredentialSnapshotFields(account), - running: runtime?.running ?? false, - lastStartAt: runtime?.lastStartAt ?? null, - lastStopAt: runtime?.lastStopAt ?? null, - lastError: runtime?.lastError ?? null, + runtime, probe, - lastInboundAt: runtime?.lastInboundAt ?? null, - lastOutboundAt: runtime?.lastOutboundAt ?? null, + }); + return { + ...base, + ...projectCredentialSnapshotFields(account), }; }, }, diff --git a/src/plugin-sdk/discord.ts b/src/plugin-sdk/discord.ts index d0408c604bf..458bebabdc5 100644 --- a/src/plugin-sdk/discord.ts +++ b/src/plugin-sdk/discord.ts @@ -43,4 +43,7 @@ export { unbindThreadBindingsBySessionKey, } from "../discord/monitor/thread-bindings.js"; -export { buildTokenChannelStatusSummary } from "./status-helpers.js"; +export { + buildComputedAccountStatusSnapshot, + buildTokenChannelStatusSummary, +} from "./status-helpers.js"; diff --git a/src/plugin-sdk/googlechat.ts b/src/plugin-sdk/googlechat.ts index 59bc1849119..7bdc7fb63f8 100644 --- a/src/plugin-sdk/googlechat.ts +++ b/src/plugin-sdk/googlechat.ts @@ -18,6 +18,7 @@ export { listDirectoryGroupEntriesFromMapKeys, listDirectoryUserEntriesFromAllowFrom, } from "../channels/plugins/directory-config-helpers.js"; +export { buildComputedAccountStatusSnapshot } from "./status-helpers.js"; export { buildChannelConfigSchema } from "../channels/plugins/config-schema.js"; export { resolveGoogleChatGroupRequireMention } from "../channels/plugins/group-mentions.js"; export { formatPairingApproveHint } from "../channels/plugins/helpers.js"; diff --git a/src/plugin-sdk/mattermost.ts b/src/plugin-sdk/mattermost.ts index 4df37da8dd8..88ecfa70571 100644 --- a/src/plugin-sdk/mattermost.ts +++ b/src/plugin-sdk/mattermost.ts @@ -38,6 +38,7 @@ export { applySetupAccountConfigPatch, migrateBaseNameToDefaultAccount, } from "../channels/plugins/setup-helpers.js"; +export { buildComputedAccountStatusSnapshot } from "./status-helpers.js"; export { createAccountListHelpers } from "../channels/plugins/account-helpers.js"; export type { BaseProbeResult, diff --git a/src/plugin-sdk/slack.ts b/src/plugin-sdk/slack.ts index debb4b75fea..18cf529ca45 100644 --- a/src/plugin-sdk/slack.ts +++ b/src/plugin-sdk/slack.ts @@ -24,6 +24,7 @@ export { } from "../channels/plugins/normalize/slack.js"; export { extractSlackToolSend, listSlackMessageActions } from "../slack/message-actions.js"; export { buildSlackThreadingToolContext } from "../slack/threading-tool-context.js"; +export { buildComputedAccountStatusSnapshot } from "./status-helpers.js"; export { resolveDefaultGroupPolicy, From 30d091b2fb2e37f7fe967b78403dbdcb1287a350 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 20:04:44 +0000 Subject: [PATCH 08/10] refactor: share thread binding id parser --- src/channels/thread-binding-id.test.ts | 43 +++++++++++++++++++ src/channels/thread-binding-id.ts | 15 +++++++ .../monitor/thread-bindings.manager.ts | 24 +++-------- src/telegram/thread-bindings.ts | 21 ++------- 4 files changed, 67 insertions(+), 36 deletions(-) create mode 100644 src/channels/thread-binding-id.test.ts create mode 100644 src/channels/thread-binding-id.ts diff --git a/src/channels/thread-binding-id.test.ts b/src/channels/thread-binding-id.test.ts new file mode 100644 index 00000000000..ad336b291bb --- /dev/null +++ b/src/channels/thread-binding-id.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from "vitest"; +import { resolveThreadBindingConversationIdFromBindingId } from "./thread-binding-id.js"; + +describe("resolveThreadBindingConversationIdFromBindingId", () => { + it("returns the conversation id for matching account-prefixed binding ids", () => { + expect( + resolveThreadBindingConversationIdFromBindingId({ + accountId: "default", + bindingId: "default:thread-123", + }), + ).toBe("thread-123"); + }); + + it("returns undefined when binding id is missing or account prefix does not match", () => { + expect( + resolveThreadBindingConversationIdFromBindingId({ + accountId: "default", + bindingId: undefined, + }), + ).toBeUndefined(); + expect( + resolveThreadBindingConversationIdFromBindingId({ + accountId: "default", + bindingId: "work:thread-123", + }), + ).toBeUndefined(); + }); + + it("trims whitespace and rejects empty ids after the account prefix", () => { + expect( + resolveThreadBindingConversationIdFromBindingId({ + accountId: "default", + bindingId: " default:group-1:topic:99 ", + }), + ).toBe("group-1:topic:99"); + expect( + resolveThreadBindingConversationIdFromBindingId({ + accountId: "default", + bindingId: "default: ", + }), + ).toBeUndefined(); + }); +}); diff --git a/src/channels/thread-binding-id.ts b/src/channels/thread-binding-id.ts new file mode 100644 index 00000000000..c9db30e3637 --- /dev/null +++ b/src/channels/thread-binding-id.ts @@ -0,0 +1,15 @@ +export function resolveThreadBindingConversationIdFromBindingId(params: { + accountId: string; + bindingId?: string; +}): string | undefined { + const bindingId = params.bindingId?.trim(); + if (!bindingId) { + return undefined; + } + const prefix = `${params.accountId}:`; + if (!bindingId.startsWith(prefix)) { + return undefined; + } + const conversationId = bindingId.slice(prefix.length).trim(); + return conversationId || undefined; +} diff --git a/src/discord/monitor/thread-bindings.manager.ts b/src/discord/monitor/thread-bindings.manager.ts index 9592962f368..386d1adbc8c 100644 --- a/src/discord/monitor/thread-bindings.manager.ts +++ b/src/discord/monitor/thread-bindings.manager.ts @@ -1,4 +1,5 @@ import { Routes } from "discord-api-types/v10"; +import { resolveThreadBindingConversationIdFromBindingId } from "../../channels/thread-binding-id.js"; import { logVerbose } from "../../globals.js"; import { registerSessionBindingAdapter, @@ -157,22 +158,6 @@ function toSessionBindingRecord( }; } -function resolveThreadIdFromBindingId(params: { - accountId: string; - bindingId?: string; -}): string | undefined { - const bindingId = params.bindingId?.trim(); - if (!bindingId) { - return undefined; - } - const prefix = `${params.accountId}:`; - if (!bindingId.startsWith(prefix)) { - return undefined; - } - const threadId = bindingId.slice(prefix.length).trim(); - return threadId || undefined; -} - export function createThreadBindingManager( params: { accountId?: string; @@ -617,7 +602,10 @@ export function createThreadBindingManager( return binding ? toSessionBindingRecord(binding, { idleTimeoutMs, maxAgeMs }) : null; }, touch: (bindingId, at) => { - const threadId = resolveThreadIdFromBindingId({ accountId, bindingId }); + const threadId = resolveThreadBindingConversationIdFromBindingId({ + accountId, + bindingId, + }); if (!threadId) { return; } @@ -631,7 +619,7 @@ export function createThreadBindingManager( }); return removed.map((entry) => toSessionBindingRecord(entry, { idleTimeoutMs, maxAgeMs })); } - const threadId = resolveThreadIdFromBindingId({ + const threadId = resolveThreadBindingConversationIdFromBindingId({ accountId, bindingId: input.bindingId, }); diff --git a/src/telegram/thread-bindings.ts b/src/telegram/thread-bindings.ts index 3357375b822..68218e9045d 100644 --- a/src/telegram/thread-bindings.ts +++ b/src/telegram/thread-bindings.ts @@ -1,6 +1,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { resolveThreadBindingConversationIdFromBindingId } from "../channels/thread-binding-id.js"; import { formatThreadBindingDurationLabel } from "../channels/thread-bindings-messages.js"; import { resolveStateDir } from "../config/paths.js"; import { logVerbose } from "../globals.js"; @@ -312,22 +313,6 @@ async function persistBindingsToDisk(params: { }); } -function resolveThreadIdFromBindingId(params: { - accountId: string; - bindingId?: string; -}): string | undefined { - const bindingId = params.bindingId?.trim(); - if (!bindingId) { - return undefined; - } - const prefix = `${params.accountId}:`; - if (!bindingId.startsWith(prefix)) { - return undefined; - } - const conversationId = bindingId.slice(prefix.length).trim(); - return conversationId || undefined; -} - function normalizeTimestampMs(raw: unknown): number { if (typeof raw !== "number" || !Number.isFinite(raw)) { return Date.now(); @@ -575,7 +560,7 @@ export function createTelegramThreadBindingManager( : null; }, touch: (bindingId, at) => { - const conversationId = resolveThreadIdFromBindingId({ + const conversationId = resolveThreadBindingConversationIdFromBindingId({ accountId, bindingId, }); @@ -598,7 +583,7 @@ export function createTelegramThreadBindingManager( }), ); } - const conversationId = resolveThreadIdFromBindingId({ + const conversationId = resolveThreadBindingConversationIdFromBindingId({ accountId, bindingId: input.bindingId, }); From 3ec81709d72d01ad50884ef63b18a8feca33a607 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 20:10:54 +0000 Subject: [PATCH 09/10] refactor: unify shared utility normalization helpers --- src/agents/tools/sessions-resolution.ts | 7 ++-- src/auto-reply/reply/commands-acp/shared.ts | 2 +- .../reply/commands-subagents/shared.ts | 5 ++- src/memory/embeddings-mistral.ts | 14 ++++---- src/memory/embeddings-model-normalize.test.ts | 34 +++++++++++++++++++ src/memory/embeddings-model-normalize.ts | 16 +++++++++ src/memory/embeddings-ollama.ts | 14 ++++---- src/memory/embeddings-openai.ts | 14 ++++---- src/memory/embeddings-voyage.ts | 14 ++++---- src/pairing/setup-code.ts | 29 +++++++++------- src/sessions/session-id.test.ts | 14 ++++++++ src/sessions/session-id.ts | 5 +++ 12 files changed, 115 insertions(+), 53 deletions(-) create mode 100644 src/memory/embeddings-model-normalize.test.ts create mode 100644 src/memory/embeddings-model-normalize.ts create mode 100644 src/sessions/session-id.test.ts create mode 100644 src/sessions/session-id.ts diff --git a/src/agents/tools/sessions-resolution.ts b/src/agents/tools/sessions-resolution.ts index 7eb730da09c..c2ba83c3001 100644 --- a/src/agents/tools/sessions-resolution.ts +++ b/src/agents/tools/sessions-resolution.ts @@ -1,6 +1,7 @@ import type { OpenClawConfig } from "../../config/config.js"; import { callGateway } from "../../gateway/call.js"; import { isAcpSessionKey, normalizeMainKey } from "../../routing/session-key.js"; +import { looksLikeSessionId } from "../../sessions/session-id.js"; function normalizeKey(value?: string) { const trimmed = value?.trim(); @@ -112,11 +113,7 @@ export async function isResolvedSessionVisibleToRequester(params: { }); } -const SESSION_ID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; - -export function looksLikeSessionId(value: string): boolean { - return SESSION_ID_RE.test(value.trim()); -} +export { looksLikeSessionId }; export function looksLikeSessionKey(value: string): boolean { const raw = value.trim(); diff --git a/src/auto-reply/reply/commands-acp/shared.ts b/src/auto-reply/reply/commands-acp/shared.ts index 2fe4710ce76..2b0571b332f 100644 --- a/src/auto-reply/reply/commands-acp/shared.ts +++ b/src/auto-reply/reply/commands-acp/shared.ts @@ -31,7 +31,7 @@ export const ACP_INSTALL_USAGE = "Usage: /acp install"; export const ACP_DOCTOR_USAGE = "Usage: /acp doctor"; export const ACP_SESSIONS_USAGE = "Usage: /acp sessions"; export const ACP_STEER_OUTPUT_LIMIT = 800; -export const SESSION_ID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; +export { SESSION_ID_RE } from "../../../sessions/session-id.js"; export type AcpAction = | "spawn" diff --git a/src/auto-reply/reply/commands-subagents/shared.ts b/src/auto-reply/reply/commands-subagents/shared.ts index 818120edb34..ec96437e645 100644 --- a/src/auto-reply/reply/commands-subagents/shared.ts +++ b/src/auto-reply/reply/commands-subagents/shared.ts @@ -18,6 +18,7 @@ import { parseDiscordTarget } from "../../../discord/targets.js"; import { callGateway } from "../../../gateway/call.js"; import { formatTimeAgo } from "../../../infra/format-time/format-relative.ts"; import { parseAgentSessionKey } from "../../../routing/session-key.js"; +import { looksLikeSessionId } from "../../../sessions/session-id.js"; import { extractTextFromChatContent } from "../../../shared/chat-content.js"; import { formatDurationCompact, @@ -75,8 +76,6 @@ export const RECENT_WINDOW_MINUTES = 30; const SUBAGENT_TASK_PREVIEW_MAX = 110; export const STEER_ABORT_SETTLE_TIMEOUT_MS = 5_000; -const SESSION_ID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; - function compactLine(value: string) { return value.replace(/\s+/g, " ").trim(); } @@ -345,7 +344,7 @@ export async function resolveFocusTargetSession(params: { const attempts: Array> = []; attempts.push({ key: token }); - if (SESSION_ID_RE.test(token)) { + if (looksLikeSessionId(token)) { attempts.push({ sessionId: token }); } attempts.push({ label: token }); diff --git a/src/memory/embeddings-mistral.ts b/src/memory/embeddings-mistral.ts index 7d9f2bb3dfe..0347c2b017c 100644 --- a/src/memory/embeddings-mistral.ts +++ b/src/memory/embeddings-mistral.ts @@ -1,4 +1,5 @@ import type { SsrFPolicy } from "../infra/net/ssrf.js"; +import { normalizeEmbeddingModelWithPrefixes } from "./embeddings-model-normalize.js"; import { createRemoteEmbeddingProvider, resolveRemoteEmbeddingClient, @@ -16,14 +17,11 @@ export const DEFAULT_MISTRAL_EMBEDDING_MODEL = "mistral-embed"; const DEFAULT_MISTRAL_BASE_URL = "https://api.mistral.ai/v1"; export function normalizeMistralModel(model: string): string { - const trimmed = model.trim(); - if (!trimmed) { - return DEFAULT_MISTRAL_EMBEDDING_MODEL; - } - if (trimmed.startsWith("mistral/")) { - return trimmed.slice("mistral/".length); - } - return trimmed; + return normalizeEmbeddingModelWithPrefixes({ + model, + defaultModel: DEFAULT_MISTRAL_EMBEDDING_MODEL, + prefixes: ["mistral/"], + }); } export async function createMistralEmbeddingProvider( diff --git a/src/memory/embeddings-model-normalize.test.ts b/src/memory/embeddings-model-normalize.test.ts new file mode 100644 index 00000000000..dc0581b82fe --- /dev/null +++ b/src/memory/embeddings-model-normalize.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; +import { normalizeEmbeddingModelWithPrefixes } from "./embeddings-model-normalize.js"; + +describe("normalizeEmbeddingModelWithPrefixes", () => { + it("returns default model when input is blank", () => { + expect( + normalizeEmbeddingModelWithPrefixes({ + model: " ", + defaultModel: "fallback-model", + prefixes: ["openai/"], + }), + ).toBe("fallback-model"); + }); + + it("strips the first matching prefix", () => { + expect( + normalizeEmbeddingModelWithPrefixes({ + model: "openai/text-embedding-3-small", + defaultModel: "fallback-model", + prefixes: ["openai/"], + }), + ).toBe("text-embedding-3-small"); + }); + + it("keeps explicit model names when no prefix matches", () => { + expect( + normalizeEmbeddingModelWithPrefixes({ + model: "voyage-4-large", + defaultModel: "fallback-model", + prefixes: ["voyage/"], + }), + ).toBe("voyage-4-large"); + }); +}); diff --git a/src/memory/embeddings-model-normalize.ts b/src/memory/embeddings-model-normalize.ts new file mode 100644 index 00000000000..85fcf5b16ce --- /dev/null +++ b/src/memory/embeddings-model-normalize.ts @@ -0,0 +1,16 @@ +export function normalizeEmbeddingModelWithPrefixes(params: { + model: string; + defaultModel: string; + prefixes: string[]; +}): string { + const trimmed = params.model.trim(); + if (!trimmed) { + return params.defaultModel; + } + for (const prefix of params.prefixes) { + if (trimmed.startsWith(prefix)) { + return trimmed.slice(prefix.length); + } + } + return trimmed; +} diff --git a/src/memory/embeddings-ollama.ts b/src/memory/embeddings-ollama.ts index 03e8a4de60b..4c9326df874 100644 --- a/src/memory/embeddings-ollama.ts +++ b/src/memory/embeddings-ollama.ts @@ -2,6 +2,7 @@ import { resolveEnvApiKey } from "../agents/model-auth.js"; import { formatErrorMessage } from "../infra/errors.js"; import type { SsrFPolicy } from "../infra/net/ssrf.js"; import { normalizeOptionalSecretInput } from "../utils/normalize-secret-input.js"; +import { normalizeEmbeddingModelWithPrefixes } from "./embeddings-model-normalize.js"; import type { EmbeddingProvider, EmbeddingProviderOptions } from "./embeddings.js"; import { buildRemoteBaseUrlPolicy, withRemoteHttpResponse } from "./remote-http.js"; import { resolveMemorySecretInputString } from "./secret-input.js"; @@ -28,14 +29,11 @@ function sanitizeAndNormalizeEmbedding(vec: number[]): number[] { } function normalizeOllamaModel(model: string): string { - const trimmed = model.trim(); - if (!trimmed) { - return DEFAULT_OLLAMA_EMBEDDING_MODEL; - } - if (trimmed.startsWith("ollama/")) { - return trimmed.slice("ollama/".length); - } - return trimmed; + return normalizeEmbeddingModelWithPrefixes({ + model, + defaultModel: DEFAULT_OLLAMA_EMBEDDING_MODEL, + prefixes: ["ollama/"], + }); } function resolveOllamaApiBase(configuredBaseUrl?: string): string { diff --git a/src/memory/embeddings-openai.ts b/src/memory/embeddings-openai.ts index af8184f4452..0ea4156c489 100644 --- a/src/memory/embeddings-openai.ts +++ b/src/memory/embeddings-openai.ts @@ -1,4 +1,5 @@ import type { SsrFPolicy } from "../infra/net/ssrf.js"; +import { normalizeEmbeddingModelWithPrefixes } from "./embeddings-model-normalize.js"; import { createRemoteEmbeddingProvider, resolveRemoteEmbeddingClient, @@ -21,14 +22,11 @@ const OPENAI_MAX_INPUT_TOKENS: Record = { }; export function normalizeOpenAiModel(model: string): string { - const trimmed = model.trim(); - if (!trimmed) { - return DEFAULT_OPENAI_EMBEDDING_MODEL; - } - if (trimmed.startsWith("openai/")) { - return trimmed.slice("openai/".length); - } - return trimmed; + return normalizeEmbeddingModelWithPrefixes({ + model, + defaultModel: DEFAULT_OPENAI_EMBEDDING_MODEL, + prefixes: ["openai/"], + }); } export async function createOpenAiEmbeddingProvider( diff --git a/src/memory/embeddings-voyage.ts b/src/memory/embeddings-voyage.ts index faf9fe1c85e..b078ebdb21a 100644 --- a/src/memory/embeddings-voyage.ts +++ b/src/memory/embeddings-voyage.ts @@ -1,4 +1,5 @@ import type { SsrFPolicy } from "../infra/net/ssrf.js"; +import { normalizeEmbeddingModelWithPrefixes } from "./embeddings-model-normalize.js"; import { resolveRemoteEmbeddingBearerClient } from "./embeddings-remote-client.js"; import { fetchRemoteEmbeddingVectors } from "./embeddings-remote-fetch.js"; import type { EmbeddingProvider, EmbeddingProviderOptions } from "./embeddings.js"; @@ -19,14 +20,11 @@ const VOYAGE_MAX_INPUT_TOKENS: Record = { }; export function normalizeVoyageModel(model: string): string { - const trimmed = model.trim(); - if (!trimmed) { - return DEFAULT_VOYAGE_EMBEDDING_MODEL; - } - if (trimmed.startsWith("voyage/")) { - return trimmed.slice("voyage/".length); - } - return trimmed; + return normalizeEmbeddingModelWithPrefixes({ + model, + defaultModel: DEFAULT_VOYAGE_EMBEDDING_MODEL, + prefixes: ["voyage/"], + }); } export async function createVoyageEmbeddingProvider( diff --git a/src/pairing/setup-code.ts b/src/pairing/setup-code.ts index 247abd38cc8..ef9dda897ca 100644 --- a/src/pairing/setup-code.ts +++ b/src/pairing/setup-code.ts @@ -155,6 +155,16 @@ function pickTailnetIPv4( return pickIPv4Matching(networkInterfaces, isTailnetIPv4); } +function resolveGatewayTokenFromEnv(env: NodeJS.ProcessEnv): string | undefined { + return env.OPENCLAW_GATEWAY_TOKEN?.trim() || env.CLAWDBOT_GATEWAY_TOKEN?.trim() || undefined; +} + +function resolveGatewayPasswordFromEnv(env: NodeJS.ProcessEnv): string | undefined { + return ( + env.OPENCLAW_GATEWAY_PASSWORD?.trim() || env.CLAWDBOT_GATEWAY_PASSWORD?.trim() || undefined + ); +} + function resolveAuth(cfg: OpenClawConfig, env: NodeJS.ProcessEnv): ResolveAuthResult { const mode = cfg.gateway?.auth?.mode; const defaults = cfg.secrets?.defaults; @@ -166,13 +176,12 @@ function resolveAuth(cfg: OpenClawConfig, env: NodeJS.ProcessEnv): ResolveAuthRe value: cfg.gateway?.auth?.password, defaults, }).ref; + const envToken = resolveGatewayTokenFromEnv(env); + const envPassword = resolveGatewayPasswordFromEnv(env); const token = - env.OPENCLAW_GATEWAY_TOKEN?.trim() || - env.CLAWDBOT_GATEWAY_TOKEN?.trim() || - (tokenRef ? undefined : normalizeSecretInputString(cfg.gateway?.auth?.token)); + envToken || (tokenRef ? undefined : normalizeSecretInputString(cfg.gateway?.auth?.token)); const password = - env.OPENCLAW_GATEWAY_PASSWORD?.trim() || - env.CLAWDBOT_GATEWAY_PASSWORD?.trim() || + envPassword || (passwordRef ? undefined : normalizeSecretInputString(cfg.gateway?.auth?.password)); if (mode === "password") { @@ -208,9 +217,7 @@ async function resolveGatewayTokenSecretRef( if (!ref) { return cfg; } - const hasTokenEnvCandidate = Boolean( - env.OPENCLAW_GATEWAY_TOKEN?.trim() || env.CLAWDBOT_GATEWAY_TOKEN?.trim(), - ); + const hasTokenEnvCandidate = Boolean(resolveGatewayTokenFromEnv(env)); if (hasTokenEnvCandidate) { return cfg; } @@ -258,9 +265,7 @@ async function resolveGatewayPasswordSecretRef( if (!ref) { return cfg; } - const hasPasswordEnvCandidate = Boolean( - env.OPENCLAW_GATEWAY_PASSWORD?.trim() || env.CLAWDBOT_GATEWAY_PASSWORD?.trim(), - ); + const hasPasswordEnvCandidate = Boolean(resolveGatewayPasswordFromEnv(env)); if (hasPasswordEnvCandidate) { return cfg; } @@ -270,7 +275,7 @@ async function resolveGatewayPasswordSecretRef( } if (mode !== "password") { const hasTokenCandidate = - Boolean(env.OPENCLAW_GATEWAY_TOKEN?.trim() || env.CLAWDBOT_GATEWAY_TOKEN?.trim()) || + Boolean(resolveGatewayTokenFromEnv(env)) || hasConfiguredSecretInput(cfg.gateway?.auth?.token, cfg.secrets?.defaults); if (hasTokenCandidate) { return cfg; diff --git a/src/sessions/session-id.test.ts b/src/sessions/session-id.test.ts new file mode 100644 index 00000000000..1fb3021a242 --- /dev/null +++ b/src/sessions/session-id.test.ts @@ -0,0 +1,14 @@ +import { describe, expect, it } from "vitest"; +import { SESSION_ID_RE, looksLikeSessionId } from "./session-id.js"; + +describe("session-id", () => { + it("matches canonical UUID session ids", () => { + expect(SESSION_ID_RE.test("123e4567-e89b-12d3-a456-426614174000")).toBe(true); + expect(looksLikeSessionId(" 123e4567-e89b-12d3-a456-426614174000 ")).toBe(true); + }); + + it("rejects non-session-id values", () => { + expect(SESSION_ID_RE.test("agent:main:main")).toBe(false); + expect(looksLikeSessionId("session-label")).toBe(false); + }); +}); diff --git a/src/sessions/session-id.ts b/src/sessions/session-id.ts new file mode 100644 index 00000000000..475d017832b --- /dev/null +++ b/src/sessions/session-id.ts @@ -0,0 +1,5 @@ +export const SESSION_ID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +export function looksLikeSessionId(value: string): boolean { + return SESSION_ID_RE.test(value.trim()); +} From f17f2f918c71a274d7af884b8590d03ccb3cdb33 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 20:42:33 +0000 Subject: [PATCH 10/10] fix(gateway): order bootstrap cache clear after embedded run wait Landed from #38873 by @MumuTW. Co-authored-by: MumuTW --- CHANGELOG.md | 1 + src/gateway/server-methods/sessions.ts | 3 ++- ...sessions.gateway-server-sessions-a.test.ts | 23 +++++++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89a67e187d3..20437930ab9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -272,6 +272,7 @@ Docs: https://docs.openclaw.ai - Agents/OpenAI WS reconnect retry accounting: avoid double retry scheduling when reconnect failures emit both `error` and `close`, so retry budgets track actual reconnect attempts instead of exhausting early. (#39133) Thanks @scoootscooob. - Daemon/Windows schtasks runtime detection: use locale-invariant `Last Run Result` running codes (`0x41301`/`267009`) as the primary running signal so `openclaw node status` no longer misreports active tasks as stopped on non-English Windows locales. (#39076) Thanks @ademczuk. - Usage/token count formatting: round near-million token counts to millions (`1.0m`) instead of `1000k`, with explicit boundary coverage for `999_499` and `999_500`. (#39129) Thanks @CurryMessi. +- Gateway/session bootstrap cache invalidation ordering: clear bootstrap snapshots only after active embedded-run shutdown wait completes, preventing dying runs from repopulating stale cache between `/new`/`sessions.reset` turns. (#38873) Thanks @MumuTW. ## 2026.3.2 diff --git a/src/gateway/server-methods/sessions.ts b/src/gateway/server-methods/sessions.ts index 8200031ae7c..bd8f6b57ac2 100644 --- a/src/gateway/server-methods/sessions.ts +++ b/src/gateway/server-methods/sessions.ts @@ -207,14 +207,15 @@ async function ensureSessionRuntimeCleanup(params: { queueKeys.add(params.sessionId); } clearSessionQueues([...queueKeys]); - clearBootstrapSnapshot(params.target.canonicalKey); stopSubagentsForRequester({ cfg: params.cfg, requesterSessionKey: params.target.canonicalKey }); if (!params.sessionId) { + clearBootstrapSnapshot(params.target.canonicalKey); await closeTrackedBrowserTabs(); return undefined; } abortEmbeddedPiRun(params.sessionId); const ended = await waitForEmbeddedPiRunEnd(params.sessionId, 15_000); + clearBootstrapSnapshot(params.target.canonicalKey); if (ended) { await closeTrackedBrowserTabs(); return undefined; diff --git a/src/gateway/server.sessions.gateway-server-sessions-a.test.ts b/src/gateway/server.sessions.gateway-server-sessions-a.test.ts index 3780174cee0..3837247c9bc 100644 --- a/src/gateway/server.sessions.gateway-server-sessions-a.test.ts +++ b/src/gateway/server.sessions.gateway-server-sessions-a.test.ts @@ -23,6 +23,10 @@ const sessionCleanupMocks = vi.hoisted(() => ({ stopSubagentsForRequester: vi.fn(() => ({ stopped: 0 })), })); +const bootstrapCacheMocks = vi.hoisted(() => ({ + clearBootstrapSnapshot: vi.fn(), +})); + const sessionHookMocks = vi.hoisted(() => ({ triggerInternalHook: vi.fn(async () => {}), })); @@ -68,6 +72,14 @@ vi.mock("../auto-reply/reply/abort.js", async () => { }; }); +vi.mock("../agents/bootstrap-cache.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + clearBootstrapSnapshot: bootstrapCacheMocks.clearBootstrapSnapshot, + }; +}); + vi.mock("../hooks/internal-hooks.js", async () => { const actual = await vi.importActual( "../hooks/internal-hooks.js", @@ -204,6 +216,7 @@ describe("gateway server sessions", () => { beforeEach(() => { sessionCleanupMocks.clearSessionQueues.mockClear(); sessionCleanupMocks.stopSubagentsForRequester.mockClear(); + bootstrapCacheMocks.clearBootstrapSnapshot.mockReset(); sessionHookMocks.triggerInternalHook.mockClear(); subagentLifecycleHookMocks.runSubagentEnded.mockClear(); subagentLifecycleHookState.hasSubagentEndedHook = true; @@ -926,6 +939,10 @@ describe("gateway server sessions", () => { test("sessions.reset aborts active runs and clears queues", async () => { await seedActiveMainSession(); + const waitCallCountAtSnapshotClear: number[] = []; + bootstrapCacheMocks.clearBootstrapSnapshot.mockImplementation(() => { + waitCallCountAtSnapshotClear.push(embeddedRunMock.waitCalls.length); + }); embeddedRunMock.activeIds.add("sess-main"); embeddedRunMock.waitResults.set("sess-main", true); @@ -947,6 +964,7 @@ describe("gateway server sessions", () => { ["main", "agent:main:main", "sess-main"], "sess-main", ); + expect(waitCallCountAtSnapshotClear).toEqual([1]); expect(browserSessionTabMocks.closeTrackedBrowserTabsForSessions).toHaveBeenCalledTimes(1); expect(browserSessionTabMocks.closeTrackedBrowserTabsForSessions).toHaveBeenCalledWith({ sessionKeys: expect.arrayContaining(["main", "agent:main:main", "sess-main"]), @@ -1163,6 +1181,10 @@ describe("gateway server sessions", () => { test("sessions.reset returns unavailable when active run does not stop", async () => { const { dir, storePath } = await seedActiveMainSession(); + const waitCallCountAtSnapshotClear: number[] = []; + bootstrapCacheMocks.clearBootstrapSnapshot.mockImplementation(() => { + waitCallCountAtSnapshotClear.push(embeddedRunMock.waitCalls.length); + }); embeddedRunMock.activeIds.add("sess-main"); embeddedRunMock.waitResults.set("sess-main", false); @@ -1180,6 +1202,7 @@ describe("gateway server sessions", () => { ["main", "agent:main:main", "sess-main"], "sess-main", ); + expect(waitCallCountAtSnapshotClear).toEqual([1]); expect(browserSessionTabMocks.closeTrackedBrowserTabsForSessions).not.toHaveBeenCalled(); const store = JSON.parse(await fs.readFile(storePath, "utf-8")) as Record<