From 9dbfeb7e61a66bf63b62b5ca2db81cbf2550ff12 Mon Sep 17 00:00:00 2001 From: wangmiao0668000666 Date: Tue, 28 Jul 2026 13:50:51 +0800 Subject: [PATCH] fix(channels): keep healthy accounts' message actions when one credential SecretRef fails (#110329) * fix(channels): isolate unavailable account discovery Co-authored-by: wangmiao0668000666 * refactor(channels): remove stale discovery exports * test(pr): provide ripgrep fixture command --------- Co-authored-by: Peter Steinberger --- docs/gateway/secrets.md | 1 + extensions/googlechat/src/accounts.ts | 50 ++++++++++++---- extensions/googlechat/src/actions.test.ts | 42 +++++++------- extensions/googlechat/src/actions.ts | 28 +-------- extensions/googlechat/src/channel-base.ts | 12 +++- .../googlechat/src/channel.deps.runtime.ts | 6 +- extensions/googlechat/src/channel.ts | 25 +------- .../googlechat/src/message-tool-api.test.ts | 58 +++++++++++++++++++ extensions/googlechat/src/message-tool-api.ts | 19 ++++++ .../mattermost/src/channel-config-shared.ts | 6 +- .../src/channel.message-tool.test.ts | 36 ++++++++++++ extensions/mattermost/src/channel.ts | 12 ++-- .../src/mattermost/accounts.test.ts | 41 ++++++++++++- .../mattermost/src/mattermost/accounts.ts | 46 +++++++++++---- .../src/mattermost/directory.test.ts | 22 ++++++- .../mattermost/src/mattermost/directory.ts | 4 +- .../src/mattermost/interactions.test.ts | 1 - .../mattermost/src/mattermost/monitor.test.ts | 2 - extensions/mattermost/src/secret-input.ts | 4 +- extensions/mattermost/src/setup-core.ts | 8 +-- .../mattermost/src/setup.accounts.runtime.ts | 2 +- extensions/mattermost/src/setup.test.ts | 27 +++++---- extensions/slack/src/account-inspect.test.ts | 26 +++++++++ extensions/slack/src/account-inspect.ts | 15 ++++- .../src/message-actions.secretref.test.ts | 31 ++++++++++ extensions/slack/src/message-actions.ts | 13 +++-- extensions/zalo/src/accounts.test.ts | 34 ++++++++++- extensions/zalo/src/accounts.ts | 24 +++++--- extensions/zalo/src/actions.test.ts | 24 ++++++++ extensions/zalo/src/actions.ts | 10 +++- extensions/zalo/src/channel.ts | 14 ++++- extensions/zalo/src/secret-input.ts | 3 +- extensions/zalo/src/setup-surface.ts | 14 +---- extensions/zalo/src/token.ts | 53 +++++++++-------- extensions/zalo/src/types.ts | 2 + .../pr-review-artifact-validation.test.ts | 2 + 36 files changed, 529 insertions(+), 188 deletions(-) create mode 100644 extensions/googlechat/src/message-tool-api.test.ts create mode 100644 extensions/googlechat/src/message-tool-api.ts create mode 100644 extensions/mattermost/src/channel.message-tool.test.ts create mode 100644 extensions/slack/src/message-actions.secretref.test.ts diff --git a/docs/gateway/secrets.md b/docs/gateway/secrets.md index ec56a868d9a6..0bf56cd878f5 100644 --- a/docs/gateway/secrets.md +++ b/docs/gateway/secrets.md @@ -25,6 +25,7 @@ Plaintext credentials remain agent-readable when they sit in files the agent can - Reload validates each mapped owner independently, then publishes one atomic snapshot. Healthy owners refresh. An eligible failed owner keeps its last-known-good value and becomes stale only when its ref identities, provider definitions, and complete non-secret owner contract are unchanged; a changed or new failed owner becomes cold. A strict failure rejects the reload and preserves the active snapshot. - Policy violations (for example an OAuth-mode auth profile combined with SecretRef input) fail activation before the runtime swap. - Runtime requests read only the active in-memory snapshot. Model-provider SecretRef credentials pass through auth storage and stream options as process-local sentinels until egress. Outbound delivery paths (Discord reply/thread delivery, Telegram action sends) also read that snapshot and do not re-resolve refs per send. +- Read-only channel capability discovery evaluates accounts independently. A configured-but-unavailable account does not hide healthy sibling accounts' message actions, while direct sends through the unavailable account still fail closed. This keeps secret-provider outages off hot request paths. diff --git a/extensions/googlechat/src/accounts.ts b/extensions/googlechat/src/accounts.ts index cf8a015c3b69..5f05166fc8ab 100644 --- a/extensions/googlechat/src/accounts.ts +++ b/extensions/googlechat/src/accounts.ts @@ -9,7 +9,12 @@ import { import { safeParseJsonWithSchema, safeParseWithSchema } from "openclaw/plugin-sdk/extension-shared"; import { mergePairLoopGuardConfig } from "openclaw/plugin-sdk/pair-loop-guard-runtime"; import { tryReadSecretFileSync } from "openclaw/plugin-sdk/secret-file-runtime"; -import { isSecretRef } from "openclaw/plugin-sdk/secret-input"; +import { + coerceSecretRef, + isSecretRef, + resolveSecretInputString, + type SecretInputStringResolutionMode, +} from "openclaw/plugin-sdk/secret-input"; import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; import { resolveUserPath } from "openclaw/plugin-sdk/text-utility-runtime"; import { z } from "zod"; @@ -114,8 +119,10 @@ function parseServiceAccount(value: unknown): Record | null { } function resolveCredentialsFromConfig(params: { + cfg: OpenClawConfig; accountId: string; account: GoogleChatAccountConfig; + mode: SecretInputStringResolutionMode; }): { credentials?: Record; credentialsFile?: string; @@ -129,10 +136,17 @@ function resolveCredentialsFromConfig(params: { return { credentials: inline, source: "inline", status: "available" }; } - if (isSecretRef(account.serviceAccount)) { - throw new Error( - `channels.googlechat.accounts.${accountId}.serviceAccount: unresolved SecretRef "${account.serviceAccount.source}:${account.serviceAccount.provider}:${account.serviceAccount.id}". Resolve this command against an active gateway runtime snapshot before reading it.`, - ); + const serviceAccountRef = coerceSecretRef(account.serviceAccount, params.cfg.secrets?.defaults); + if (serviceAccountRef) { + const resolved = resolveSecretInputString({ + value: account.serviceAccount, + defaults: params.cfg.secrets?.defaults, + path: `channels.googlechat.accounts.${accountId}.serviceAccount`, + mode: params.mode, + }); + return resolved.status === "configured_unavailable" + ? { source: "none", status: "configured_unavailable" } + : { source: "none", status: "missing" }; } const file = normalizeOptionalString(account.serviceAccountFile); @@ -191,9 +205,10 @@ function resolveCredentialsFromConfig(params: { return { source: "none", status: "missing" }; } -export function resolveGoogleChatAccount(params: { +function resolveGoogleChatAccountWithMode(params: { cfg: OpenClawConfig; accountId?: string | null; + mode: SecretInputStringResolutionMode; }): ResolvedGoogleChatAccount { const accountId = normalizeAccountId( params.accountId ?? params.cfg.channels?.["googlechat"]?.defaultAccount, @@ -202,7 +217,12 @@ export function resolveGoogleChatAccount(params: { const merged = mergeGoogleChatAccountConfig(params.cfg, accountId); const accountEnabled = merged.enabled !== false; const enabled = baseEnabled && accountEnabled; - const credentials = resolveCredentialsFromConfig({ accountId, account: merged }); + const credentials = resolveCredentialsFromConfig({ + cfg: params.cfg, + accountId, + account: merged, + mode: params.mode, + }); return { accountId, @@ -217,8 +237,16 @@ export function resolveGoogleChatAccount(params: { }; } -export function listEnabledGoogleChatAccounts(cfg: OpenClawConfig): ResolvedGoogleChatAccount[] { - return listGoogleChatAccountIds(cfg) - .map((accountId) => resolveGoogleChatAccount({ cfg, accountId })) - .filter((account) => account.enabled); +export function resolveGoogleChatAccount(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): ResolvedGoogleChatAccount { + return resolveGoogleChatAccountWithMode({ ...params, mode: "strict" }); +} + +export function inspectGoogleChatAccount(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): ResolvedGoogleChatAccount { + return resolveGoogleChatAccountWithMode({ ...params, mode: "inspect" }); } diff --git a/extensions/googlechat/src/actions.test.ts b/extensions/googlechat/src/actions.test.ts index 5c7e747b7b8c..66d79c3d0fe5 100644 --- a/extensions/googlechat/src/actions.test.ts +++ b/extensions/googlechat/src/actions.test.ts @@ -1,13 +1,15 @@ // Googlechat tests cover actions plugin behavior. import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; -const listEnabledGoogleChatAccounts = vi.hoisted(() => vi.fn()); +const inspectGoogleChatAccount = vi.hoisted(() => vi.fn()); +const listGoogleChatAccountIds = vi.hoisted(() => vi.fn()); const resolveGoogleChatAccount = vi.hoisted(() => vi.fn()); const sendGoogleChatMessage = vi.hoisted(() => vi.fn()); const resolveGoogleChatOutboundSpace = vi.hoisted(() => vi.fn()); vi.mock("./accounts.js", () => ({ - listEnabledGoogleChatAccounts, + inspectGoogleChatAccount, + listGoogleChatAccountIds, resolveGoogleChatAccount, })); @@ -68,16 +70,16 @@ describe("googlechat message actions", () => { } it("describes only send actions when enabled accounts exist", () => { - listEnabledGoogleChatAccounts.mockReturnValueOnce([]); + listGoogleChatAccountIds.mockReturnValueOnce([]); expect(googlechatMessageActions.describeMessageTool?.({ cfg: {} as never })).toBeNull(); - listEnabledGoogleChatAccounts.mockReturnValueOnce([ - { - enabled: true, - credentialSource: "service-account", - config: {}, - }, - ]); + listGoogleChatAccountIds.mockReturnValueOnce(["default"]); + inspectGoogleChatAccount.mockReturnValueOnce({ + enabled: true, + credentialSource: "inline", + tokenStatus: "available", + config: {}, + }); expect(googlechatMessageActions.describeMessageTool?.({ cfg: {} as never })).toEqual({ actions: ["send"], @@ -87,23 +89,23 @@ describe("googlechat message actions", () => { }); it("does not expose actions for configured-unavailable file credentials", () => { - listEnabledGoogleChatAccounts.mockReturnValueOnce([ - { - enabled: true, - credentialSource: "file", - tokenStatus: "configured_unavailable", - config: {}, - }, - ]); + listGoogleChatAccountIds.mockReturnValueOnce(["default"]); + inspectGoogleChatAccount.mockReturnValueOnce({ + enabled: true, + credentialSource: "file", + tokenStatus: "configured_unavailable", + config: {}, + }); expect(googlechatMessageActions.describeMessageTool?.({ cfg: {} as never })).toBeNull(); }); it("keeps account-scoped discovery send-only", () => { - resolveGoogleChatAccount.mockImplementation( + inspectGoogleChatAccount.mockImplementation( ({ accountId: _accountId }: { accountId?: string | null }) => ({ enabled: true, - credentialSource: "service-account", + credentialSource: "inline", + tokenStatus: "available", config: {}, }), ); diff --git a/extensions/googlechat/src/actions.ts b/extensions/googlechat/src/actions.ts index e16ef710ff6c..50fd4e49a425 100644 --- a/extensions/googlechat/src/actions.ts +++ b/extensions/googlechat/src/actions.ts @@ -5,23 +5,14 @@ import { readStringParam, } from "openclaw/plugin-sdk/channel-actions"; import type { ChannelMessageActionAdapter } from "openclaw/plugin-sdk/channel-contract"; -import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { extractToolSend } from "openclaw/plugin-sdk/tool-send"; -import { listEnabledGoogleChatAccounts, resolveGoogleChatAccount } from "./accounts.js"; +import { resolveGoogleChatAccount } from "./accounts.js"; import { sendGoogleChatMessage } from "./api.js"; +import { describeGoogleChatMessageTool } from "./message-tool-api.js"; import { resolveGoogleChatOutboundSpace } from "./targets.js"; const providerId = "googlechat"; -function listEnabledAccounts(cfg: OpenClawConfig) { - return listEnabledGoogleChatAccounts(cfg).filter( - (account) => - account.enabled && - account.credentialSource !== "none" && - account.tokenStatus !== "configured_unavailable", - ); -} - const OUTBOUND_MEDIA_KEYS = ["media", "mediaUrl", "path", "filePath", "fileUrl"] as const; const STRUCTURED_ATTACHMENT_MEDIA_KEYS = [...OUTBOUND_MEDIA_KEYS, "url"] as const; @@ -47,20 +38,7 @@ function hasGoogleChatOutboundAttachment(params: Record): boole } export const googlechatMessageActions: ChannelMessageActionAdapter = { - describeMessageTool: ({ cfg, accountId }) => { - const accounts = accountId - ? [resolveGoogleChatAccount({ cfg, accountId })].filter( - (account) => - account.enabled && - account.credentialSource !== "none" && - account.tokenStatus !== "configured_unavailable", - ) - : listEnabledAccounts(cfg); - if (accounts.length === 0) { - return null; - } - return { actions: ["send"] }; - }, + describeMessageTool: describeGoogleChatMessageTool, supportsAction: ({ action }) => action === "send", extractToolSend: ({ args }) => { return extractToolSend(args, "sendMessage"); diff --git a/extensions/googlechat/src/channel-base.ts b/extensions/googlechat/src/channel-base.ts index 8a2a77ec9169..d48dd0cebbc4 100644 --- a/extensions/googlechat/src/channel-base.ts +++ b/extensions/googlechat/src/channel-base.ts @@ -9,6 +9,7 @@ import type { ChannelPlugin } from "openclaw/plugin-sdk/channel-core"; import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime"; import { type GoogleChatConfigAccessorAccount, + inspectGoogleChatAccount, listGoogleChatAccountIds, resolveDefaultGoogleChatAccountId, resolveGoogleChatConfigAccessorAccount, @@ -71,6 +72,12 @@ const googleChatConfigAdapter = createScopedChannelConfigAdapter< resolveDefaultTo: (account) => account.config.defaultTo, }); +function isGoogleChatAccountConfigured(account: ResolvedGoogleChatAccount): boolean { + return account.tokenStatus + ? account.tokenStatus !== "missing" + : account.credentialSource !== "none"; +} + type GoogleChatPluginBase = Pick< ChannelPlugin, | "id" @@ -112,11 +119,12 @@ export function createGoogleChatPluginBase( ...(params.configSchema ? { configSchema: params.configSchema } : {}), config: { ...googleChatConfigAdapter, - isConfigured: (account) => account.credentialSource !== "none", + inspectAccount: adaptScopedAccountAccessor(inspectGoogleChatAccount), + isConfigured: isGoogleChatAccountConfigured, describeAccount: (account) => describeAccountSnapshot({ account, - configured: account.credentialSource !== "none", + configured: isGoogleChatAccountConfigured(account), extra: { credentialSource: account.credentialSource, tokenStatus: account.tokenStatus, diff --git a/extensions/googlechat/src/channel.deps.runtime.ts b/extensions/googlechat/src/channel.deps.runtime.ts index 5d0a8fe76c36..d4ddde7a91ae 100644 --- a/extensions/googlechat/src/channel.deps.runtime.ts +++ b/extensions/googlechat/src/channel.deps.runtime.ts @@ -9,11 +9,7 @@ export { type ChannelStatusIssue, type OpenClawConfig, } from "../runtime-api.js"; -export { - listGoogleChatAccountIds, - resolveGoogleChatAccount, - type ResolvedGoogleChatAccount, -} from "./accounts.js"; +export { resolveGoogleChatAccount, type ResolvedGoogleChatAccount } from "./accounts.js"; export { isGoogleChatSpaceTarget, isGoogleChatUserTarget, diff --git a/extensions/googlechat/src/channel.ts b/extensions/googlechat/src/channel.ts index b3691db4dedc..bbb2598ed535 100644 --- a/extensions/googlechat/src/channel.ts +++ b/extensions/googlechat/src/channel.ts @@ -27,9 +27,7 @@ import { GoogleChatConfigSchema, isGoogleChatSpaceTarget, isGoogleChatUserTarget, - listGoogleChatAccountIds, normalizeGoogleChatTarget, - resolveGoogleChatAccount, resolveGoogleChatOutboundSessionRoute, type ChannelMessageActionAdapter, type ChannelStatusIssue, @@ -41,6 +39,7 @@ import { } from "./doctor-contract.js"; import { collectGoogleChatMutableAllowlistWarnings } from "./doctor.js"; import { startGoogleChatGatewayAccount } from "./gateway.js"; +import { describeGoogleChatMessageTool } from "./message-tool-api.js"; import { collectRuntimeConfigAssignments, secretTargetRegistryEntries } from "./secret-contract.js"; const loadGoogleChatChannelRuntime = createLazyRuntimeNamedExport( @@ -49,27 +48,7 @@ const loadGoogleChatChannelRuntime = createLazyRuntimeNamedExport( ); const googlechatActions: ChannelMessageActionAdapter = { - describeMessageTool: ({ cfg, accountId }) => { - const accounts = accountId - ? [resolveGoogleChatAccount({ cfg, accountId })].filter( - (account) => - account.enabled && - account.credentialSource !== "none" && - account.tokenStatus !== "configured_unavailable", - ) - : listGoogleChatAccountIds(cfg) - .map((id) => resolveGoogleChatAccount({ cfg, accountId: id })) - .filter( - (account) => - account.enabled && - account.credentialSource !== "none" && - account.tokenStatus !== "configured_unavailable", - ); - if (accounts.length === 0) { - return null; - } - return { actions: ["send"] }; - }, + describeMessageTool: describeGoogleChatMessageTool, supportsAction: ({ action }) => action === "send", extractToolSend: ({ args }) => extractToolSend(args, "sendMessage"), handleAction: async (ctx) => { diff --git a/extensions/googlechat/src/message-tool-api.test.ts b/extensions/googlechat/src/message-tool-api.test.ts new file mode 100644 index 000000000000..7c754d7ab568 --- /dev/null +++ b/extensions/googlechat/src/message-tool-api.test.ts @@ -0,0 +1,58 @@ +// Google Chat tests cover account-isolated message-tool discovery. +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { inspectGoogleChatAccount, resolveGoogleChatAccount } from "./accounts.js"; +import { describeGoogleChatMessageTool } from "./message-tool-api.js"; + +const unresolvedRef = { + source: "env", + provider: "default", + id: "OPENCLAW_TEST_MISSING_GOOGLE_CHAT_SERVICE_ACCOUNT", +} as const; + +function buildTwoAccountConfig(): OpenClawConfig { + return { + channels: { + googlechat: { + accounts: { + broken: { serviceAccount: unresolvedRef }, + healthy: { + serviceAccount: + '{"client_email":"proof@example.iam.gserviceaccount.com","private_key":"proof-key"}', + }, + }, + }, + }, + } as OpenClawConfig; +} + +describe("Google Chat message-tool SecretRef inspection", () => { + afterEach(() => vi.unstubAllEnvs()); + + it("keeps healthy account actions when one account credential is unavailable", () => { + const cfg = buildTwoAccountConfig(); + expect(describeGoogleChatMessageTool({ cfg })).toEqual({ actions: ["send"] }); + expect(describeGoogleChatMessageTool({ cfg, accountId: "broken" })).toBeNull(); + }); + + it("keeps direct account resolution strict", () => { + expect(() => + resolveGoogleChatAccount({ cfg: buildTwoAccountConfig(), accountId: "broken" }), + ).toThrow(/unresolved SecretRef/); + }); + + it("does not fall through an unavailable configured ref to the environment", () => { + vi.stubEnv( + "GOOGLE_CHAT_SERVICE_ACCOUNT", + '{"client_email":"fallback@example.iam.gserviceaccount.com","private_key":"fallback"}', + ); + const account = inspectGoogleChatAccount({ + cfg: { channels: { googlechat: { serviceAccount: unresolvedRef } } } as OpenClawConfig, + }); + expect(account).toMatchObject({ + credentialSource: "none", + tokenStatus: "configured_unavailable", + }); + expect(account.credentials).toBeUndefined(); + }); +}); diff --git a/extensions/googlechat/src/message-tool-api.ts b/extensions/googlechat/src/message-tool-api.ts new file mode 100644 index 000000000000..e2cdee695b49 --- /dev/null +++ b/extensions/googlechat/src/message-tool-api.ts @@ -0,0 +1,19 @@ +// Google Chat message-tool discovery stays read-only and account-isolated. +import type { ChannelMessageActionAdapter } from "openclaw/plugin-sdk/channel-contract"; +import { inspectGoogleChatAccount, listGoogleChatAccountIds } from "./accounts.js"; + +export function describeGoogleChatMessageTool({ + cfg, + accountId, +}: Parameters>[0]) { + const accounts = accountId + ? [inspectGoogleChatAccount({ cfg, accountId })] + : listGoogleChatAccountIds(cfg).map((listedAccountId) => + inspectGoogleChatAccount({ cfg, accountId: listedAccountId }), + ); + const hasAvailableAccount = accounts.some( + (account) => + account.enabled && account.credentialSource !== "none" && account.tokenStatus === "available", + ); + return hasAvailableAccount ? { actions: ["send" as const] } : null; +} diff --git a/extensions/mattermost/src/channel-config-shared.ts b/extensions/mattermost/src/channel-config-shared.ts index 66c4c9ddf70d..c3b018b77e76 100644 --- a/extensions/mattermost/src/channel-config-shared.ts +++ b/extensions/mattermost/src/channel-config-shared.ts @@ -65,7 +65,10 @@ export const mattermostConfigAdapter = createScopedChannelConfigAdapter { + const cfg = { + channels: { + mattermost: { + accounts: { + broken: { + botToken: { + source: "env", + provider: "default", + id: "OPENCLAW_TEST_MISSING_MATTERMOST_TOKEN", + }, + baseUrl: "https://mm.example.com", + }, + healthy: { botToken: "healthy-token", baseUrl: "https://mm.example.com" }, + }, + }, + }, + } as OpenClawConfig; + + it("keeps healthy account actions discoverable", () => { + expect(mattermostPlugin.actions?.describeMessageTool({ cfg })?.actions).toEqual( + expect.arrayContaining(["send", "react"]), + ); + }); + + it("does not advertise actions for the unavailable selected account", () => { + expect( + mattermostPlugin.actions?.describeMessageTool({ cfg, accountId: "broken" })?.actions, + ).toEqual([]); + }); +}); diff --git a/extensions/mattermost/src/channel.ts b/extensions/mattermost/src/channel.ts index f8b734121bd5..65d920142388 100644 --- a/extensions/mattermost/src/channel.ts +++ b/extensions/mattermost/src/channel.ts @@ -1,3 +1,4 @@ +import { adaptScopedAccountAccessor } from "openclaw/plugin-sdk/channel-config-helpers"; // Mattermost plugin module implements channel behavior. import type { ChannelMessageActionAdapter, @@ -54,6 +55,7 @@ import { MattermostChannelConfigSchema } from "./config-surface.js"; import { mattermostDoctor } from "./doctor.js"; import { resolveMattermostGroupRequireMention } from "./group-mentions.js"; import { + inspectMattermostAccount, listMattermostAccountIds, resolveDefaultMattermostAccountId, resolveMattermostAccount, @@ -153,9 +155,9 @@ function describeMattermostMessageTool({ >[0]): ChannelMessageToolDiscovery { const enabledAccounts = ( accountId - ? [resolveMattermostAccount({ cfg, accountId })] + ? [inspectMattermostAccount({ cfg, accountId })] : listMattermostAccountIds(cfg).map((listedAccountId) => - resolveMattermostAccount({ cfg, accountId: listedAccountId }), + inspectMattermostAccount({ cfg, accountId: listedAccountId }), ) ) .filter((account) => account.enabled) @@ -188,9 +190,9 @@ function hasConfiguredMattermostDirectoryAccount({ accountId, }: Pick): boolean { const accounts = accountId - ? [resolveMattermostAccount({ cfg, accountId })] + ? [inspectMattermostAccount({ cfg, accountId })] : listMattermostAccountIds(cfg).map((listedAccountId) => - resolveMattermostAccount({ cfg, accountId: listedAccountId }), + inspectMattermostAccount({ cfg, accountId: listedAccountId }), ); return accounts.some((account) => Boolean(account.enabled && account.botToken?.trim() && account.baseUrl?.trim()), @@ -810,6 +812,7 @@ export const mattermostPlugin: ChannelPlugin = create configSchema: MattermostChannelConfigSchema, config: { ...mattermostConfigAdapter, + inspectAccount: adaptScopedAccountAccessor(inspectMattermostAccount), isConfigured: isMattermostConfigured, describeAccount: describeMattermostAccount, }, @@ -895,6 +898,7 @@ export const mattermostPlugin: ChannelPlugin = create configured: Boolean(account.botToken && account.baseUrl), extra: { botTokenSource: account.botTokenSource, + botTokenStatus: account.botTokenStatus, baseUrl: account.baseUrl, dmPolicy: account.config.dmPolicy ?? "pairing", connected: runtime?.connected ?? false, diff --git a/extensions/mattermost/src/mattermost/accounts.test.ts b/extensions/mattermost/src/mattermost/accounts.test.ts index 939ef00bfbf1..148fb95e7bf2 100644 --- a/extensions/mattermost/src/mattermost/accounts.test.ts +++ b/extensions/mattermost/src/mattermost/accounts.test.ts @@ -1,7 +1,8 @@ // Mattermost tests cover accounts plugin behavior. -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../../runtime-api.js"; import { + inspectMattermostAccount, listMattermostAccountIds, resolveDefaultMattermostAccountId, resolveMattermostAccount, @@ -104,6 +105,44 @@ describe("resolveDefaultMattermostAccountId", () => { }); }); +describe("Mattermost account SecretRef inspection", () => { + afterEach(() => vi.unstubAllEnvs()); + + const unresolvedRef = { + source: "env" as const, + provider: "default", + id: "OPENCLAW_TEST_MISSING_MATTERMOST_TOKEN", + }; + + it("keeps direct account resolution strict", () => { + expect(() => + resolveMattermostAccount({ + cfg: { + channels: { + mattermost: { botToken: unresolvedRef, baseUrl: "https://mm.example.com" }, + }, + }, + }), + ).toThrow(/unresolved SecretRef/); + }); + + it("does not fall through an unavailable configured ref to the environment", () => { + vi.stubEnv("MATTERMOST_BOT_TOKEN", "lower-precedence-token"); + const account = inspectMattermostAccount({ + cfg: { + channels: { + mattermost: { botToken: unresolvedRef, baseUrl: "https://mm.example.com" }, + }, + }, + }); + expect(account).toMatchObject({ + botToken: undefined, + botTokenSource: "config", + botTokenStatus: "configured_unavailable", + }); + }); +}); + describe("resolveMattermostReplyToMode", () => { it("uses configured defaultAccount when accountId is omitted", () => { const cfg: OpenClawConfig = { diff --git a/extensions/mattermost/src/mattermost/accounts.ts b/extensions/mattermost/src/mattermost/accounts.ts index 7e74b09ea980..e35754968c82 100644 --- a/extensions/mattermost/src/mattermost/accounts.ts +++ b/extensions/mattermost/src/mattermost/accounts.ts @@ -14,7 +14,7 @@ import { } from "openclaw/plugin-sdk/channel-outbound"; import type { BlockStreamingCoalesceConfig } from "openclaw/plugin-sdk/config-contracts"; import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; -import { normalizeResolvedSecretInputString, normalizeSecretInputString } from "../secret-input.js"; +import { resolveSecretInputString, type SecretInputStringResolutionMode } from "../secret-input.js"; import type { MattermostAccountConfig, MattermostChatMode, @@ -26,6 +26,7 @@ import type { OpenClawConfig } from "./runtime-api.js"; type MattermostTokenSource = "env" | "config" | "none"; type MattermostBaseUrlSource = "env" | "config" | "none"; +type MattermostCredentialStatus = "available" | "configured_unavailable" | "missing"; export type ResolvedMattermostAccount = { accountId: string; @@ -34,6 +35,7 @@ export type ResolvedMattermostAccount = { botToken?: string; baseUrl?: string; botTokenSource: MattermostTokenSource; + botTokenStatus?: MattermostCredentialStatus; baseUrlSource: MattermostBaseUrlSource; config: MattermostAccountConfig; chatmode?: MattermostChatMode; @@ -76,10 +78,10 @@ function resolveMattermostRequireMention(config: MattermostAccountConfig): boole return config.requireMention; } -export function resolveMattermostAccount(params: { +function resolveMattermostAccountWithMode(params: { cfg: OpenClawConfig; accountId?: string | null; - allowUnresolvedSecretRef?: boolean; + mode: SecretInputStringResolutionMode; }): ResolvedMattermostAccount { const accountId = normalizeAccountId( params.accountId ?? resolveDefaultMattermostAccountId(params.cfg), @@ -92,18 +94,25 @@ export function resolveMattermostAccount(params: { const allowEnv = accountId === DEFAULT_ACCOUNT_ID; const envToken = allowEnv ? process.env.MATTERMOST_BOT_TOKEN?.trim() : undefined; const envUrl = allowEnv ? process.env.MATTERMOST_URL?.trim() : undefined; - const configToken = params.allowUnresolvedSecretRef - ? normalizeSecretInputString(merged.botToken) - : normalizeResolvedSecretInputString({ - value: merged.botToken, - path: `channels.mattermost.accounts.${accountId}.botToken`, - }); + const configToken = resolveSecretInputString({ + value: merged.botToken, + path: `channels.mattermost.accounts.${accountId}.botToken`, + mode: params.mode, + }); const configUrl = merged.baseUrl?.trim(); - const botToken = configToken || envToken; + const botToken = + configToken.status === "available" + ? configToken.value + : configToken.status === "missing" + ? envToken + : undefined; const baseUrl = normalizeMattermostBaseUrl(configUrl || envUrl); const requireMention = resolveMattermostRequireMention(merged); - const botTokenSource: MattermostTokenSource = configToken ? "config" : envToken ? "env" : "none"; + const botTokenSource: MattermostTokenSource = + configToken.status !== "missing" ? "config" : envToken ? "env" : "none"; + const botTokenStatus: MattermostCredentialStatus = + configToken.status !== "missing" ? configToken.status : envToken ? "available" : "missing"; const baseUrlSource: MattermostBaseUrlSource = configUrl ? "config" : envUrl ? "env" : "none"; return { @@ -113,6 +122,7 @@ export function resolveMattermostAccount(params: { botToken, baseUrl, botTokenSource, + botTokenStatus, baseUrlSource, config: merged, chatmode: merged.chatmode, @@ -126,6 +136,20 @@ export function resolveMattermostAccount(params: { }; } +export function resolveMattermostAccount(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): ResolvedMattermostAccount { + return resolveMattermostAccountWithMode({ ...params, mode: "strict" }); +} + +export function inspectMattermostAccount(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): ResolvedMattermostAccount { + return resolveMattermostAccountWithMode({ ...params, mode: "inspect" }); +} + /** * Resolve the effective replyToMode for a given chat type. * Direct messages stay flat unless explicitly opted into a per-chat-type mode. diff --git a/extensions/mattermost/src/mattermost/directory.test.ts b/extensions/mattermost/src/mattermost/directory.test.ts index a7293de03be4..cf05d351f1e7 100644 --- a/extensions/mattermost/src/mattermost/directory.test.ts +++ b/extensions/mattermost/src/mattermost/directory.test.ts @@ -18,7 +18,7 @@ const { vi.mock("./accounts.js", () => { return { listMattermostAccountIds: listMattermostAccountIdsMock, - resolveMattermostAccount: resolveMattermostAccountMock, + inspectMattermostAccount: resolveMattermostAccountMock, }; }); @@ -42,6 +42,26 @@ describe("mattermost directory", () => { vi.clearAllMocks(); }); + it("skips an unavailable account while retaining a healthy directory client", async () => { + const client = { + token: "token-healthy", + request: vi.fn().mockResolvedValueOnce([]), + }; + listMattermostAccountIdsMock.mockReturnValue(["broken", "healthy"]); + resolveMattermostAccountMock.mockImplementation(({ accountId }) => + accountId === "broken" + ? { enabled: true, botToken: undefined, baseUrl: "https://chat.example.com" } + : { enabled: true, botToken: "token-healthy", baseUrl: "https://chat.example.com" }, + ); + createMattermostClientMock.mockReturnValue(client); + fetchMattermostMeMock.mockResolvedValue({ id: "me-1" }); + + await expect( + listMattermostDirectoryGroups({ cfg: {} as never, runtime: {} as never }), + ).resolves.toEqual([]); + expect(createMattermostClientMock).toHaveBeenCalledOnce(); + }); + it("deduplicates channels across enabled accounts and skips failing accounts", async () => { const clientA = { token: "token-a", diff --git a/extensions/mattermost/src/mattermost/directory.ts b/extensions/mattermost/src/mattermost/directory.ts index c69244f23059..2b1a256a1a4d 100644 --- a/extensions/mattermost/src/mattermost/directory.ts +++ b/extensions/mattermost/src/mattermost/directory.ts @@ -1,7 +1,7 @@ // Mattermost plugin module implements directory behavior. import { isPrivateNetworkOptInEnabled } from "openclaw/plugin-sdk/ssrf-runtime"; import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime"; -import { listMattermostAccountIds, resolveMattermostAccount } from "./accounts.js"; +import { inspectMattermostAccount, listMattermostAccountIds } from "./accounts.js"; import { createMattermostClient, fetchMattermostMe, @@ -23,7 +23,7 @@ function buildClient(params: { cfg: OpenClawConfig; accountId?: string | null; }): MattermostClient | null { - const account = resolveMattermostAccount({ cfg: params.cfg, accountId: params.accountId }); + const account = inspectMattermostAccount({ cfg: params.cfg, accountId: params.accountId }); if (!account.enabled || !account.botToken || !account.baseUrl) { return null; } diff --git a/extensions/mattermost/src/mattermost/interactions.test.ts b/extensions/mattermost/src/mattermost/interactions.test.ts index 0ac5909f2d41..46ecede6af68 100644 --- a/extensions/mattermost/src/mattermost/interactions.test.ts +++ b/extensions/mattermost/src/mattermost/interactions.test.ts @@ -311,7 +311,6 @@ describe("resolveInteractionCallbackUrl", () => { const account = resolveMattermostAccount({ cfg, accountId: "acct", - allowUnresolvedSecretRef: true, }); const url = resolveInteractionCallbackUrl(account.accountId, { gateway: cfg.gateway, diff --git a/extensions/mattermost/src/mattermost/monitor.test.ts b/extensions/mattermost/src/mattermost/monitor.test.ts index 0931a5df61b3..5515c386a2d4 100644 --- a/extensions/mattermost/src/mattermost/monitor.test.ts +++ b/extensions/mattermost/src/mattermost/monitor.test.ts @@ -226,7 +226,6 @@ describe("shouldUpdateMattermostDraftToolProgress", () => { }, }, accountId: "default", - allowUnresolvedSecretRef: true, }); return shouldUpdateMattermostDraftToolProgress(account); } @@ -273,7 +272,6 @@ describe("shouldSuppressMattermostDefaultToolProgressMessages", () => { }, }, accountId: "default", - allowUnresolvedSecretRef: true, }); return shouldSuppressMattermostDefaultToolProgressMessages(account); } diff --git a/extensions/mattermost/src/secret-input.ts b/extensions/mattermost/src/secret-input.ts index 11ca969cb8d4..bcc24b572131 100644 --- a/extensions/mattermost/src/secret-input.ts +++ b/extensions/mattermost/src/secret-input.ts @@ -3,6 +3,6 @@ export type { SecretInput } from "openclaw/plugin-sdk/secret-input"; export { buildSecretInputSchema, hasConfiguredSecretInput, - normalizeResolvedSecretInputString, - normalizeSecretInputString, + resolveSecretInputString, } from "openclaw/plugin-sdk/secret-input"; +export type { SecretInputStringResolutionMode } from "openclaw/plugin-sdk/secret-input"; diff --git a/extensions/mattermost/src/setup-core.ts b/extensions/mattermost/src/setup-core.ts index be4eb837328e..40a47dfdaabf 100644 --- a/extensions/mattermost/src/setup-core.ts +++ b/extensions/mattermost/src/setup-core.ts @@ -13,7 +13,7 @@ import { } from "openclaw/plugin-sdk/setup"; import { createSetupInputPresenceValidator } from "openclaw/plugin-sdk/setup-runtime"; import { - resolveMattermostAccount, + inspectMattermostAccount, type ResolvedMattermostAccount, } from "./setup.accounts.runtime.js"; import { normalizeMattermostBaseUrl } from "./setup.client.runtime.js"; @@ -33,11 +33,7 @@ export function isMattermostConfigured(account: ResolvedMattermostAccount): bool } export function resolveMattermostAccountWithSecrets(cfg: OpenClawConfig, accountId: string) { - return resolveMattermostAccount({ - cfg, - accountId, - allowUnresolvedSecretRef: true, - }); + return inspectMattermostAccount({ cfg, accountId }); } export function applyMattermostSetupConfigPatch(params: { diff --git a/extensions/mattermost/src/setup.accounts.runtime.ts b/extensions/mattermost/src/setup.accounts.runtime.ts index d1a45872ac35..f7d9aed737cb 100644 --- a/extensions/mattermost/src/setup.accounts.runtime.ts +++ b/extensions/mattermost/src/setup.accounts.runtime.ts @@ -1,2 +1,2 @@ // Mattermost plugin module implements setup.accounts behavior. -export { resolveMattermostAccount, type ResolvedMattermostAccount } from "./mattermost/accounts.js"; +export { inspectMattermostAccount, type ResolvedMattermostAccount } from "./mattermost/accounts.js"; diff --git a/extensions/mattermost/src/setup.test.ts b/extensions/mattermost/src/setup.test.ts index 38a007e00218..a73804287ef8 100644 --- a/extensions/mattermost/src/setup.test.ts +++ b/extensions/mattermost/src/setup.test.ts @@ -13,13 +13,8 @@ const resolveMattermostAccount = vi.hoisted(() => vi.fn()); const normalizeMattermostBaseUrl = vi.hoisted(() => vi.fn((value: string | undefined) => value)); const hasConfiguredSecretInput = vi.hoisted(() => vi.fn((value: unknown) => Boolean(value))); -vi.mock("./setup.accounts.runtime.js", () => ({ - listMattermostAccountIds: vi.fn((cfg: OpenClawConfig) => { - const accounts = cfg.channels?.mattermost?.accounts; - const ids = accounts ? Object.keys(accounts) : []; - return ids.length > 0 ? ids : [DEFAULT_ACCOUNT_ID]; - }), - resolveMattermostAccount: (params: Parameters[0]) => { +vi.mock("./setup.accounts.runtime.js", () => { + const resolveAccount = (params: Parameters[0]) => { const mocked = resolveMattermostAccount(params); return ( mocked ?? { @@ -32,12 +27,23 @@ vi.mock("./setup.accounts.runtime.js", () => ({ baseUrl: normalizeMattermostBaseUrl(params.cfg.channels?.mattermost?.baseUrl), botTokenSource: typeof params.cfg.channels?.mattermost?.botToken === "string" ? "config" : "none", + botTokenStatus: + typeof params.cfg.channels?.mattermost?.botToken === "string" ? "available" : "missing", baseUrlSource: params.cfg.channels?.mattermost?.baseUrl ? "config" : "none", config: params.cfg.channels?.mattermost ?? {}, } ); - }, -})); + }; + return { + listMattermostAccountIds: vi.fn((cfg: OpenClawConfig) => { + const accounts = cfg.channels?.mattermost?.accounts; + const ids = accounts ? Object.keys(accounts) : []; + return ids.length > 0 ? ids : [DEFAULT_ACCOUNT_ID]; + }), + inspectMattermostAccount: resolveAccount, + resolveMattermostAccount: resolveAccount, + }; +}); vi.mock("./setup.client.runtime.js", () => ({ normalizeMattermostBaseUrl, @@ -125,7 +131,7 @@ describe("mattermost setup", () => { ).toBe(false); }); - it("resolves accounts with unresolved secret refs allowed", () => { + it("inspects accounts without resolving secret refs", () => { resolveMattermostAccount.mockReturnValue({ accountId: "default" }); const cfg = { channels: { mattermost: {} } }; @@ -136,7 +142,6 @@ describe("mattermost setup", () => { expect(resolveMattermostAccount).toHaveBeenCalledWith({ cfg, accountId: "default", - allowUnresolvedSecretRef: true, }); }); diff --git a/extensions/slack/src/account-inspect.test.ts b/extensions/slack/src/account-inspect.test.ts index 854e26fd33b7..f9724ce6973f 100644 --- a/extensions/slack/src/account-inspect.test.ts +++ b/extensions/slack/src/account-inspect.test.ts @@ -83,4 +83,30 @@ describe("inspectSlackAccount", () => { userTokenStatus: "missing", }); }); + + it("does not fall through an unavailable configured ref to environment tokens", () => { + const account = inspectSlackAccount({ + cfg: { + channels: { + slack: { + botToken: { + source: "env", + provider: "default", + id: "OPENCLAW_TEST_MISSING_SLACK_BOT_TOKEN", + }, + appToken: "test-app-token", + }, + }, + } as OpenClawConfig, + envBotToken: "xoxb-lower-precedence", + envAppToken: "", + envUserToken: "", + }); + + expect(account.botToken).toBeUndefined(); + expect(account).toMatchObject({ + botTokenSource: "config", + botTokenStatus: "configured_unavailable", + }); + }); }); diff --git a/extensions/slack/src/account-inspect.ts b/extensions/slack/src/account-inspect.ts index 2be79d196785..f815a7766927 100644 --- a/extensions/slack/src/account-inspect.ts +++ b/extensions/slack/src/account-inspect.ts @@ -66,6 +66,15 @@ function inspectSlackToken(value: unknown): { }; } +function selectInspectedSlackToken( + configured: ReturnType, + envToken: string | undefined, +): string | undefined { + // A configured SecretRef remains authoritative while unavailable; read-only + // inspection must not make a lower-precedence environment token look active. + return configured.status === "missing" ? envToken : configured.token; +} + export function inspectSlackAccount(params: { cfg: OpenClawConfig; accountId?: string | null; @@ -100,10 +109,10 @@ export function inspectSlackAccount(params: { ? normalizeSecretInputString(params.envUserToken ?? process.env.SLACK_USER_TOKEN) : undefined; - const botToken = configBot.token ?? envBot; - const appToken = configApp.token ?? envApp; + const botToken = selectInspectedSlackToken(configBot, envBot); + const appToken = selectInspectedSlackToken(configApp, envApp); const signingSecret = configSigningSecret.token; - const userToken = configUser.token ?? envUser; + const userToken = selectInspectedSlackToken(configUser, envUser); const relayConfigured = isRelayMode && Boolean(normalizeOptionalString(merged.relay?.url)) && diff --git a/extensions/slack/src/message-actions.secretref.test.ts b/extensions/slack/src/message-actions.secretref.test.ts new file mode 100644 index 000000000000..887e441b5290 --- /dev/null +++ b/extensions/slack/src/message-actions.secretref.test.ts @@ -0,0 +1,31 @@ +// Slack tests cover account-isolated message-tool discovery. +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; +import { describe, expect, it } from "vitest"; +import { listSlackMessageActions } from "./message-actions.js"; + +describe("Slack message actions with an unavailable account SecretRef", () => { + const cfg = { + channels: { + slack: { + accounts: { + broken: { + botToken: { + source: "env", + provider: "default", + id: "OPENCLAW_TEST_MISSING_SLACK_BOT_TOKEN", + }, + }, + healthy: { botToken: "xoxb-healthy" }, + }, + }, + }, + } as OpenClawConfig; + + it("keeps healthy account actions discoverable", () => { + expect(listSlackMessageActions(cfg)).toContain("send"); + }); + + it("does not advertise actions for the unavailable selected account", () => { + expect(listSlackMessageActions(cfg, "broken")).toEqual([]); + }); +}); diff --git a/extensions/slack/src/message-actions.ts b/extensions/slack/src/message-actions.ts index 7acfae8371dc..39a6cbc55b63 100644 --- a/extensions/slack/src/message-actions.ts +++ b/extensions/slack/src/message-actions.ts @@ -3,7 +3,8 @@ import { createActionGate } from "openclaw/plugin-sdk/channel-actions"; import type { ChannelMessageActionName } from "openclaw/plugin-sdk/channel-contract"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { extractToolSend, type ChannelToolSend } from "openclaw/plugin-sdk/tool-send"; -import { listEnabledSlackAccounts, resolveSlackAccount } from "./accounts.js"; +import { inspectSlackAccount } from "./account-inspect.js"; +import { listSlackAccountIds } from "./accounts.js"; import { normalizeSlackThreadTsCandidate, resolveSlackThreadTsValue } from "./thread-ts.js"; export function listSlackMessageActions( @@ -11,13 +12,17 @@ export function listSlackMessageActions( accountId?: string | null, ): ChannelMessageActionName[] { const accounts = ( - accountId ? [resolveSlackAccount({ cfg, accountId })] : listEnabledSlackAccounts(cfg) + accountId + ? [inspectSlackAccount({ cfg, accountId })] + : listSlackAccountIds(cfg).map((listedAccountId) => + inspectSlackAccount({ cfg, accountId: listedAccountId }), + ) ).filter( (account) => account.enabled && (account.identity === "user" - ? account.userTokenSource !== "none" - : account.botTokenSource !== "none"), + ? account.userTokenStatus === "available" + : account.botTokenStatus === "available"), ); if (accounts.length === 0) { return []; diff --git a/extensions/zalo/src/accounts.test.ts b/extensions/zalo/src/accounts.test.ts index 618b8f743e29..a1390a3c31e5 100644 --- a/extensions/zalo/src/accounts.test.ts +++ b/extensions/zalo/src/accounts.test.ts @@ -1,7 +1,7 @@ // Zalo tests cover accounts plugin behavior. -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { - listEnabledZaloAccounts, + inspectZaloAccount, listZaloAccountIds, resolveDefaultZaloAccountId, resolveZaloAccount, @@ -91,6 +91,34 @@ describe("resolveZaloAccount", () => { expect(listZaloAccountIds(cfg)).toEqual(["default", "work"]); expect(resolveDefaultZaloAccountId(cfg)).toBe("default"); - expect(listEnabledZaloAccounts(cfg).map((account) => account.accountId)).toEqual(["default"]); + expect(resolveZaloAccount({ cfg, accountId: "default" }).enabled).toBe(true); + }); +}); + +describe("Zalo account SecretRef inspection", () => { + afterEach(() => vi.unstubAllEnvs()); + + const unresolvedRef = { + source: "env" as const, + provider: "default", + id: "OPENCLAW_TEST_MISSING_ZALO_TOKEN", + }; + + it("keeps direct account resolution strict", () => { + expect(() => + resolveZaloAccount({ cfg: { channels: { zalo: { botToken: unresolvedRef } } } }), + ).toThrow(/unresolved SecretRef/); + }); + + it("does not fall through an unavailable configured ref to the environment", () => { + vi.stubEnv("ZALO_BOT_TOKEN", "lower-precedence-token"); + const account = inspectZaloAccount({ + cfg: { channels: { zalo: { botToken: unresolvedRef } } }, + }); + expect(account).toMatchObject({ + token: "", + tokenSource: "config", + tokenStatus: "configured_unavailable", + }); }); }); diff --git a/extensions/zalo/src/accounts.ts b/extensions/zalo/src/accounts.ts index 631a4530a3bc..1757f274b4a8 100644 --- a/extensions/zalo/src/accounts.ts +++ b/extensions/zalo/src/accounts.ts @@ -3,6 +3,7 @@ import { createAccountListHelpers } from "openclaw/plugin-sdk/account-helpers"; import { normalizeAccountId } from "openclaw/plugin-sdk/account-id"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; +import type { SecretInputStringResolutionMode } from "./secret-input.js"; import { resolveZaloToken } from "./token.js"; import type { ResolvedZaloAccount, ZaloAccountConfig, ZaloConfig } from "./types.js"; @@ -21,10 +22,10 @@ const { }); export { listZaloAccountIds, resolveDefaultZaloAccountId }; -export function resolveZaloAccount(params: { +function resolveZaloAccountWithMode(params: { cfg: OpenClawConfig; accountId?: string | null; - allowUnresolvedSecretRef?: boolean; + mode: SecretInputStringResolutionMode; }): ResolvedZaloAccount { const accountId = normalizeAccountId( params.accountId ?? (params.cfg.channels?.zalo as ZaloConfig | undefined)?.defaultAccount, @@ -36,7 +37,7 @@ export function resolveZaloAccount(params: { const tokenResolution = resolveZaloToken( params.cfg.channels?.zalo as ZaloConfig | undefined, accountId, - { allowUnresolvedSecretRef: params.allowUnresolvedSecretRef }, + { mode: params.mode }, ); return { @@ -45,12 +46,21 @@ export function resolveZaloAccount(params: { enabled, token: tokenResolution.token, tokenSource: tokenResolution.source, + tokenStatus: tokenResolution.status, config: merged, }; } -export function listEnabledZaloAccounts(cfg: OpenClawConfig): ResolvedZaloAccount[] { - return listZaloAccountIds(cfg) - .map((accountId) => resolveZaloAccount({ cfg, accountId })) - .filter((account) => account.enabled); +export function resolveZaloAccount(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): ResolvedZaloAccount { + return resolveZaloAccountWithMode({ ...params, mode: "strict" }); +} + +export function inspectZaloAccount(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): ResolvedZaloAccount { + return resolveZaloAccountWithMode({ ...params, mode: "inspect" }); } diff --git a/extensions/zalo/src/actions.test.ts b/extensions/zalo/src/actions.test.ts index 9780f153119f..77c41e6cdb52 100644 --- a/extensions/zalo/src/actions.test.ts +++ b/extensions/zalo/src/actions.test.ts @@ -33,6 +33,30 @@ describe("zaloMessageActions.describeMessageTool", () => { expect(zaloMessageActions.supportsAction?.({ action: "send" })).toBe(true); expect(zaloMessageActions.supportsAction?.({ action: "react" })).toBe(false); }); + + it("keeps healthy account actions when one account SecretRef is unavailable", () => { + const cfg = { + channels: { + zalo: { + accounts: { + broken: { + botToken: { + source: "env", + provider: "default", + id: "OPENCLAW_TEST_MISSING_ZALO_TOKEN", + }, + }, + healthy: { botToken: "healthy-token" }, + }, + }, + }, + } as OpenClawConfig; + expect(zaloMessageActions.describeMessageTool?.({ cfg })).toEqual({ + actions: ["send"], + capabilities: [], + }); + expect(zaloMessageActions.describeMessageTool?.({ cfg, accountId: "broken" })).toBeNull(); + }); }); describe("zaloMessageActions.handleAction", () => { diff --git a/extensions/zalo/src/actions.ts b/extensions/zalo/src/actions.ts index bc7322d70553..3105e6422fa8 100644 --- a/extensions/zalo/src/actions.ts +++ b/extensions/zalo/src/actions.ts @@ -7,7 +7,7 @@ import type { import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { createLazyRuntimeNamedExport } from "openclaw/plugin-sdk/lazy-runtime"; import { extractToolSend } from "openclaw/plugin-sdk/tool-send"; -import { listEnabledZaloAccounts, resolveZaloAccount } from "./accounts.js"; +import { inspectZaloAccount, listZaloAccountIds } from "./accounts.js"; const loadZaloActionsRuntime = createLazyRuntimeNamedExport( () => import("./actions.runtime.js"), @@ -19,8 +19,12 @@ const ZALO_ACTIONS = new Set(["send"]); function listEnabledAccounts(cfg: OpenClawConfig, accountId?: string | null) { return ( - accountId ? [resolveZaloAccount({ cfg, accountId })] : listEnabledZaloAccounts(cfg) - ).filter((account) => account.enabled && account.tokenSource !== "none"); + accountId + ? [inspectZaloAccount({ cfg, accountId })] + : listZaloAccountIds(cfg).map((listedAccountId) => + inspectZaloAccount({ cfg, accountId: listedAccountId }), + ) + ).filter((account) => account.enabled && account.tokenStatus === "available"); } export const zaloMessageActions: ChannelMessageActionAdapter = { diff --git a/extensions/zalo/src/channel.ts b/extensions/zalo/src/channel.ts index 923d647b7183..5955a6317cf6 100644 --- a/extensions/zalo/src/channel.ts +++ b/extensions/zalo/src/channel.ts @@ -43,6 +43,7 @@ import { sanitizeAssistantVisibleText, } from "openclaw/plugin-sdk/text-chunking"; import { + inspectZaloAccount, listZaloAccountIds, resolveDefaultZaloAccountId, resolveZaloAccount, @@ -131,6 +132,10 @@ const zaloMessageAdapter = defineChannelMessageAdapter({ }, }); +function isZaloAccountConfigured(account: ResolvedZaloAccount): boolean { + return account.tokenStatus ? account.tokenStatus !== "missing" : Boolean(account.token?.trim()); +} + const zaloConfigAdapter = createScopedChannelConfigAdapter({ sectionKey: "zalo", listAccountIds: listZaloAccountIds, @@ -206,14 +211,16 @@ export const zaloPlugin: ChannelPlugin = configSchema: buildChannelConfigSchema(ZaloConfigSchema), config: { ...zaloConfigAdapter, - isConfigured: (account) => Boolean(account.token?.trim()), + inspectAccount: adaptScopedAccountAccessor(inspectZaloAccount), + isConfigured: isZaloAccountConfigured, describeAccount: (account): ChannelAccountSnapshot => describeWebhookAccountSnapshot({ account, - configured: Boolean(account.token?.trim()), + configured: isZaloAccountConfigured(account), mode: account.config.webhookUrl ? "webhook" : "polling", extra: { tokenSource: account.tokenSource, + tokenStatus: account.tokenStatus, }, }), }, @@ -252,7 +259,7 @@ export const zaloPlugin: ChannelPlugin = probeAccount: async ({ account, timeoutMs }) => await (await loadZaloChannelRuntime()).probeZaloAccount({ account, timeoutMs }), resolveAccountSnapshot: ({ account }) => { - const configured = Boolean(account.token?.trim()); + const configured = isZaloAccountConfigured(account); return { accountId: account.accountId, name: account.name, @@ -260,6 +267,7 @@ export const zaloPlugin: ChannelPlugin = configured, extra: { tokenSource: account.tokenSource, + tokenStatus: account.tokenStatus, mode: account.config.webhookUrl ? "webhook" : "polling", dmPolicy: account.config.dmPolicy ?? "pairing", }, diff --git a/extensions/zalo/src/secret-input.ts b/extensions/zalo/src/secret-input.ts index f293a399aefd..f58732e0e954 100644 --- a/extensions/zalo/src/secret-input.ts +++ b/extensions/zalo/src/secret-input.ts @@ -1,6 +1,7 @@ // Zalo plugin module implements secret input behavior. export { buildSecretInputSchema, - normalizeResolvedSecretInputString, normalizeSecretInputString, + resolveSecretInputString, } from "openclaw/plugin-sdk/secret-input"; +export type { SecretInputStringResolutionMode } from "openclaw/plugin-sdk/secret-input"; diff --git a/extensions/zalo/src/setup-surface.ts b/extensions/zalo/src/setup-surface.ts index e60a0d43dff0..d7ccb37d9498 100644 --- a/extensions/zalo/src/setup-surface.ts +++ b/extensions/zalo/src/setup-surface.ts @@ -11,7 +11,7 @@ import { type SecretInput, createSetupTranslator, } from "openclaw/plugin-sdk/setup"; -import { resolveZaloAccount } from "./accounts.js"; +import { inspectZaloAccount } from "./accounts.js"; import { noteZaloTokenHelp, promptZaloAllowFrom } from "./setup-allow-from.js"; import { zaloDmPolicy } from "./setup-core.js"; @@ -110,11 +110,7 @@ export const zaloSetupWizard: ChannelSetupWizard = { unconfiguredScore: 10, includeStatusLine: true, resolveConfigured: ({ cfg, accountId }) => { - const account = resolveZaloAccount({ - cfg, - accountId, - allowUnresolvedSecretRef: true, - }); + const account = inspectZaloAccount({ cfg, accountId }); return ( Boolean(account.token) || hasConfiguredSecretInput(account.config.botToken) || @@ -125,11 +121,7 @@ export const zaloSetupWizard: ChannelSetupWizard = { credentials: [], finalize: async ({ cfg, accountId, forceAllowFrom, options, prompter }) => { let next = cfg; - const resolvedAccount = resolveZaloAccount({ - cfg: next, - accountId, - allowUnresolvedSecretRef: true, - }); + const resolvedAccount = inspectZaloAccount({ cfg: next, accountId }); const accountConfigured = Boolean(resolvedAccount.token); const allowEnv = accountId === DEFAULT_ACCOUNT_ID; const hasConfigToken = Boolean( diff --git a/extensions/zalo/src/token.ts b/extensions/zalo/src/token.ts index 10d1cf636f83..e24bbb12649f 100644 --- a/extensions/zalo/src/token.ts +++ b/extensions/zalo/src/token.ts @@ -3,11 +3,12 @@ import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "openclaw/plugin-sdk/acco import type { BaseTokenResolution } from "openclaw/plugin-sdk/channel-contract"; import { tryReadSecretFileSync } from "openclaw/plugin-sdk/core"; import { resolveAccountEntry } from "openclaw/plugin-sdk/routing"; -import { normalizeResolvedSecretInputString, normalizeSecretInputString } from "./secret-input.js"; -import type { ZaloConfig } from "./types.js"; +import { resolveSecretInputString, type SecretInputStringResolutionMode } from "./secret-input.js"; +import type { ZaloConfig, ZaloTokenStatus } from "./types.js"; type ZaloTokenResolution = BaseTokenResolution & { source: "env" | "config" | "configFile" | "none"; + status: ZaloTokenStatus; }; function readTokenFromFile(tokenFile: string | undefined): string { @@ -17,7 +18,7 @@ function readTokenFromFile(tokenFile: string | undefined): string { export function resolveZaloToken( config: ZaloConfig | undefined, accountId?: string | null, - options?: { allowUnresolvedSecretRef?: boolean }, + options?: { mode?: SecretInputStringResolutionMode }, ): ZaloTokenResolution { const resolvedAccountId = normalizeAccountId(accountId ?? config?.defaultAccount); const isDefaultAccount = resolvedAccountId === DEFAULT_ACCOUNT_ID; @@ -29,50 +30,54 @@ export function resolveZaloToken( const accountHasBotToken = Boolean(accountConfig && Object.hasOwn(accountConfig, "botToken")); if (accountConfig && accountHasBotToken) { - const token = options?.allowUnresolvedSecretRef - ? normalizeSecretInputString(accountConfig.botToken) - : normalizeResolvedSecretInputString({ - value: accountConfig.botToken, - path: `channels.zalo.accounts.${resolvedAccountId}.botToken`, - }); - if (token) { - return { token, source: "config" }; + const token = resolveSecretInputString({ + value: accountConfig.botToken, + path: `channels.zalo.accounts.${resolvedAccountId}.botToken`, + mode: options?.mode, + }); + if (token.status === "available") { + return { token: token.value, source: "config", status: "available" }; + } + if (token.status === "configured_unavailable") { + return { token: "", source: "config", status: "configured_unavailable" }; } const fileToken = readTokenFromFile(accountConfig.tokenFile); if (fileToken) { - return { token: fileToken, source: "configFile" }; + return { token: fileToken, source: "configFile", status: "available" }; } } if (!accountHasBotToken) { const fileToken = readTokenFromFile(accountConfig?.tokenFile); if (fileToken) { - return { token: fileToken, source: "configFile" }; + return { token: fileToken, source: "configFile", status: "available" }; } } if (!accountHasBotToken) { - const token = options?.allowUnresolvedSecretRef - ? normalizeSecretInputString(baseConfig?.botToken) - : normalizeResolvedSecretInputString({ - value: baseConfig?.botToken, - path: "channels.zalo.botToken", - }); - if (token) { - return { token, source: "config" }; + const token = resolveSecretInputString({ + value: baseConfig?.botToken, + path: "channels.zalo.botToken", + mode: options?.mode, + }); + if (token.status === "available") { + return { token: token.value, source: "config", status: "available" }; + } + if (token.status === "configured_unavailable") { + return { token: "", source: "config", status: "configured_unavailable" }; } const fileToken = readTokenFromFile(baseConfig?.tokenFile); if (fileToken) { - return { token: fileToken, source: "configFile" }; + return { token: fileToken, source: "configFile", status: "available" }; } } if (isDefaultAccount) { const envToken = process.env.ZALO_BOT_TOKEN?.trim(); if (envToken) { - return { token: envToken, source: "env" }; + return { token: envToken, source: "env", status: "available" }; } } - return { token: "", source: "none" }; + return { token: "", source: "none", status: "missing" }; } diff --git a/extensions/zalo/src/types.ts b/extensions/zalo/src/types.ts index 60ec0c842084..85d448cd4020 100644 --- a/extensions/zalo/src/types.ts +++ b/extensions/zalo/src/types.ts @@ -40,6 +40,7 @@ export type ZaloConfig = { } & ZaloAccountConfig; type ZaloTokenSource = "env" | "config" | "configFile" | "none"; +export type ZaloTokenStatus = "available" | "configured_unavailable" | "missing"; export type ResolvedZaloAccount = { accountId: string; @@ -47,5 +48,6 @@ export type ResolvedZaloAccount = { enabled: boolean; token: string; tokenSource: ZaloTokenSource; + tokenStatus?: ZaloTokenStatus; config: ZaloAccountConfig; }; diff --git a/test/scripts/pr-review-artifact-validation.test.ts b/test/scripts/pr-review-artifact-validation.test.ts index 7e09b6fa848a..54a8b0a0d8b8 100644 --- a/test/scripts/pr-review-artifact-validation.test.ts +++ b/test/scripts/pr-review-artifact-validation.test.ts @@ -114,6 +114,7 @@ function runValidation( 'fixture_root="$2"', 'enter_worktree() { cd "$fixture_root"; }', 'require_artifact() { [ -s "$1" ]; }', + 'rg() { case " $* " in *" -F "*) grep "$@";; *) grep -E "$@";; esac; }', options.guardFailure ? "review_guard() { REVIEW_MODE=pr; echo 'review head guard failed'; return 1; }" : `review_guard() { REVIEW_MODE=${options.mode ?? "pr"}; }`, @@ -139,6 +140,7 @@ function runReviewShellFunction(fixtureRoot: string, invocation: string) { 'fixture_root="$2"', 'enter_worktree() { cd "$fixture_root"; }', 'require_artifact() { [ -s "$1" ]; }', + 'rg() { case " $* " in *" -F "*) grep "$@";; *) grep -E "$@";; esac; }', "mark_pr_operation_side_effects_started() { :; }", invocation, ].join("\n"),