From e1ec95dcbfb133873e9c8579dc46671eaedf813f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 8 Aug 2026 21:14:16 -0700 Subject: [PATCH] refactor(auth): dedupe profile upserts and approval resolvers (#120831) * refactor(auth): dedupe profile upserts and approval resolvers * test(auth): mock canonical locked upsert * test(auth): mock locked upsert during onboarding --- extensions/anthropic/register.runtime.ts | 11 +-- extensions/cloudflare-ai-gateway/index.ts | 13 +-- .../src/monitor/exec-approvals.test.ts | 3 +- .../discord/src/monitor/exec-approvals.ts | 3 +- extensions/github-copilot/login.ts | 13 +-- .../src/approval-card-click.test.ts | 2 +- .../googlechat/src/approval-card-click.ts | 2 +- .../imessage/src/approval-gateway-types.ts | 5 ++ .../imessage/src/approval-polls.test.ts | 6 +- extensions/imessage/src/approval-polls.ts | 12 ++- .../imessage/src/approval-reaction-poller.ts | 2 +- .../imessage/src/approval-reactions.test.ts | 12 ++- extensions/imessage/src/approval-reactions.ts | 12 ++- .../imessage/src/approval-resolver.test.ts | 49 ------------ extensions/imessage/src/approval-resolver.ts | 43 ---------- .../imessage/src/monitor/monitor-provider.ts | 2 +- .../matrix/src/exec-approval-resolver.test.ts | 80 ------------------- .../matrix/src/exec-approval-resolver.ts | 29 ------- .../matrix/monitor/reaction-events.test.ts | 10 ++- .../src/matrix/monitor/reaction-events.ts | 8 +- extensions/pixverse/onboard.ts | 13 +-- .../signal/src/approval-reactions.test.ts | 8 +- extensions/signal/src/approval-reactions.ts | 10 ++- extensions/signal/src/approval-resolver.ts | 29 ------- .../events/interactions.block-actions.ts | 4 +- .../src/monitor/events/interactions.test.ts | 12 +-- extensions/telegram/src/bot-deps.ts | 38 +++++++-- ...bot-handlers.callback-approvals.runtime.ts | 13 ++- .../bot.create-telegram-bot.test-harness.ts | 6 +- extensions/telegram/src/bot.test.ts | 18 ++--- .../src/exec-approval-resolver.test.ts | 77 ------------------ .../telegram/src/exec-approval-resolver.ts | 49 ------------ .../whatsapp/src/approval-reactions.test.ts | 9 ++- extensions/whatsapp/src/approval-reactions.ts | 10 ++- .../whatsapp/src/approval-resolver.test.ts | 46 ----------- extensions/whatsapp/src/approval-resolver.ts | 29 ------- extensions/xiaomi/index.ts | 13 +-- extensions/zai/index.ts | 13 +-- src/agents/auth-profiles.ts | 1 + src/agents/auth-profiles/profiles.ts | 2 +- .../auth-profiles/upsert-with-lock.test.ts | 44 ++++++++++ src/agents/auth-profiles/upsert-with-lock.ts | 12 +++ .../auth-choice.apply.plugin-provider.test.ts | 1 + src/commands/auth-choice.test.ts | 7 ++ src/commands/models/auth.test.ts | 1 + src/commands/models/auth.ts | 13 +-- src/commands/onboard-auth.test.ts | 7 ++ src/infra/approval-gateway-resolver.test.ts | 31 +++++++ src/infra/approval-gateway-resolver.ts | 26 ++++-- src/plugin-sdk/provider-auth-api-key.ts | 6 +- src/plugins/provider-api-key-auth.ts | 13 +-- src/plugins/provider-auth-choice.ts | 12 +-- src/plugins/provider-auth-helpers.ts | 17 ++-- 53 files changed, 283 insertions(+), 624 deletions(-) create mode 100644 extensions/imessage/src/approval-gateway-types.ts delete mode 100644 extensions/imessage/src/approval-resolver.test.ts delete mode 100644 extensions/imessage/src/approval-resolver.ts delete mode 100644 extensions/matrix/src/exec-approval-resolver.test.ts delete mode 100644 extensions/matrix/src/exec-approval-resolver.ts delete mode 100644 extensions/signal/src/approval-resolver.ts delete mode 100644 extensions/telegram/src/exec-approval-resolver.test.ts delete mode 100644 extensions/telegram/src/exec-approval-resolver.ts delete mode 100644 extensions/whatsapp/src/approval-resolver.test.ts delete mode 100644 extensions/whatsapp/src/approval-resolver.ts create mode 100644 src/agents/auth-profiles/upsert-with-lock.test.ts diff --git a/extensions/anthropic/register.runtime.ts b/extensions/anthropic/register.runtime.ts index f1a2e5d614c4..9f7385bf5926 100644 --- a/extensions/anthropic/register.runtime.ts +++ b/extensions/anthropic/register.runtime.ts @@ -22,9 +22,9 @@ import { type OpenClawConfig as ProviderAuthConfig, type ProviderAuthResult, suggestOAuthProfileIdForLegacyDefault, - upsertAuthProfileWithLock, validateAnthropicSetupToken, } from "openclaw/plugin-sdk/provider-auth"; +import { upsertAuthProfileWithLockOrThrow } from "openclaw/plugin-sdk/provider-auth-api-key"; import { buildOpenAICompatibleProviderCatalog } from "openclaw/plugin-sdk/provider-catalog-live-runtime"; import { buildManifestModelProviderConfig, @@ -90,7 +90,6 @@ function classifyAnthropicFailoverDescriptor(value: string | undefined) { return undefined; } } -type UpsertAuthProfileParams = Parameters[0]; const DEFAULT_ANTHROPIC_MODEL = "anthropic/claude-opus-5"; const ANTHROPIC_OPUS_48_MODEL_ID = "claude-opus-4-8"; const ANTHROPIC_OPUS_48_DOT_MODEL_ID = "claude-opus-4.8"; @@ -196,14 +195,6 @@ const CLAUDE_CLI_CANONICAL_ALLOWLIST_REFS = CLAUDE_CLI_DEFAULT_ALLOWLIST_REFS.ma : ref, ); -async function upsertAuthProfileWithLockOrThrow(params: UpsertAuthProfileParams): Promise { - const updated = await upsertAuthProfileWithLock(params); - if (!updated) { - throw new Error( - "Failed to update auth profile store; the auth store lock may be busy. Wait a moment and retry.", - ); - } -} function normalizeAnthropicSetupTokenInput(value: string): string { return value.replaceAll(/\s+/g, "").trim(); } diff --git a/extensions/cloudflare-ai-gateway/index.ts b/extensions/cloudflare-ai-gateway/index.ts index 1ffa444a8c2d..1476c8163ced 100644 --- a/extensions/cloudflare-ai-gateway/index.ts +++ b/extensions/cloudflare-ai-gateway/index.ts @@ -11,9 +11,9 @@ import { listProfilesForProvider, normalizeApiKeyInput, normalizeOptionalSecretInput, - upsertAuthProfileWithLock, validateApiKeyInput, } from "openclaw/plugin-sdk/provider-auth"; +import { upsertAuthProfileWithLockOrThrow } from "openclaw/plugin-sdk/provider-auth-api-key"; import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; import { buildCloudflareAiGatewayCatalogProvider } from "./catalog-provider.js"; import { CLOUDFLARE_AI_GATEWAY_DEFAULT_MODEL_REF } from "./models.js"; @@ -23,17 +23,6 @@ import { wrapCloudflareAiGatewayProviderStream } from "./stream-wrappers.js"; const PROVIDER_ID = "cloudflare-ai-gateway"; const PROVIDER_ENV_VAR = "CLOUDFLARE_AI_GATEWAY_API_KEY"; const PROFILE_ID = "cloudflare-ai-gateway:default"; -type UpsertAuthProfileParams = Parameters[0]; - -async function upsertAuthProfileWithLockOrThrow(params: UpsertAuthProfileParams): Promise { - const updated = await upsertAuthProfileWithLock(params); - if (!updated) { - throw new Error( - "Failed to update auth profile store; the auth store lock may be busy. Wait a moment and retry.", - ); - } -} - function readRequiredTextInput(value: unknown): string { return typeof value === "string" ? value.trim() : ""; } diff --git a/extensions/discord/src/monitor/exec-approvals.test.ts b/extensions/discord/src/monitor/exec-approvals.test.ts index 3784a3063685..5f775d0bc59f 100644 --- a/extensions/discord/src/monitor/exec-approvals.test.ts +++ b/extensions/discord/src/monitor/exec-approvals.test.ts @@ -271,8 +271,9 @@ describe("discord exec approval monitor helpers", () => { approvalId: "abc", approvalKind, decision: "allow-once", + channel: "discord", + senderId: "default", gatewayUrl: "ws://127.0.0.1:18789", - clientDisplayName: "Discord approval (default)", }); }, ); diff --git a/extensions/discord/src/monitor/exec-approvals.ts b/extensions/discord/src/monitor/exec-approvals.ts index 9f1f6dac8bb1..71e85ea26992 100644 --- a/extensions/discord/src/monitor/exec-approvals.ts +++ b/extensions/discord/src/monitor/exec-approvals.ts @@ -204,8 +204,9 @@ export function createDiscordExecApprovalButtonContext(params: { approvalId, approvalKind, decision, + channel: "discord", + senderId: params.accountId, gatewayUrl: params.gatewayUrl, - clientDisplayName: `Discord approval (${params.accountId})`, }); return { ok: true, resolution }; } catch (err) { diff --git a/extensions/github-copilot/login.ts b/extensions/github-copilot/login.ts index 09b569ac4ca7..ee0ae90807cc 100644 --- a/extensions/github-copilot/login.ts +++ b/extensions/github-copilot/login.ts @@ -12,8 +12,8 @@ import { applyAuthProfileConfig, ensureAuthProfileStore, normalizeGithubCopilotDomain, - upsertAuthProfileWithLock, } from "openclaw/plugin-sdk/provider-auth"; +import { upsertAuthProfileWithLockOrThrow } from "openclaw/plugin-sdk/provider-auth-api-key"; import { readProviderJsonResponse } from "openclaw/plugin-sdk/provider-http"; import type { RuntimeEnv } from "openclaw/plugin-sdk/runtime"; import { fetchWithSsrFGuard, type SsrFPolicy } from "openclaw/plugin-sdk/ssrf-runtime"; @@ -63,8 +63,6 @@ type DeviceTokenResponse = const GITHUB_DEVICE_ACCESS_DENIED = Symbol("github-device-access-denied"); const GITHUB_DEVICE_EXPIRED = Symbol("github-device-expired"); -type UpsertAuthProfileParams = Parameters[0]; - class GitHubDeviceFlowError extends Error { readonly kind: symbol; constructor(kind: symbol, message: string) { @@ -74,15 +72,6 @@ class GitHubDeviceFlowError extends Error { } } -async function upsertAuthProfileWithLockOrThrow(params: UpsertAuthProfileParams): Promise { - const updated = await upsertAuthProfileWithLock(params); - if (!updated) { - throw new Error( - "Failed to update auth profile store; the auth store lock may be busy. Wait a moment and retry.", - ); - } -} - function isGitHubDeviceAccessDeniedError(err: unknown): boolean { return err instanceof GitHubDeviceFlowError && err.kind === GITHUB_DEVICE_ACCESS_DENIED; } diff --git a/extensions/googlechat/src/approval-card-click.test.ts b/extensions/googlechat/src/approval-card-click.test.ts index 76ef37f0fae8..f4ba9451f784 100644 --- a/extensions/googlechat/src/approval-card-click.test.ts +++ b/extensions/googlechat/src/approval-card-click.test.ts @@ -164,8 +164,8 @@ describe("maybeHandleGoogleChatApprovalCardClick", () => { approvalId: "approval-1", approvalKind: "exec", decision: "allow-once", + channel: "googlechat", senderId: "users/123", - clientDisplayName: "Google Chat approval (users/123)", }); expect(updateGoogleChatMessage).toHaveBeenCalledWith({ account: target.account, diff --git a/extensions/googlechat/src/approval-card-click.ts b/extensions/googlechat/src/approval-card-click.ts index 52324601c820..5e8c43021e7d 100644 --- a/extensions/googlechat/src/approval-card-click.ts +++ b/extensions/googlechat/src/approval-card-click.ts @@ -86,8 +86,8 @@ export async function maybeHandleGoogleChatApprovalCardClick(params: { approvalId: consumed.approvalId, approvalKind: consumed.approvalKind, decision: consumed.decision, + channel: "googlechat", senderId: actor, - clientDisplayName: `Google Chat approval (${actor?.trim() || "unknown"})`, }); await updateGoogleChatMessage({ account: params.target.account, diff --git a/extensions/imessage/src/approval-gateway-types.ts b/extensions/imessage/src/approval-gateway-types.ts new file mode 100644 index 000000000000..6e001307d670 --- /dev/null +++ b/extensions/imessage/src/approval-gateway-types.ts @@ -0,0 +1,5 @@ +import type { resolveApprovalOverGateway } from "openclaw/plugin-sdk/approval-gateway-runtime"; + +export type IMessageApprovalGatewayRuntime = NonNullable< + Parameters[0]["gatewayRuntime"] +>; diff --git a/extensions/imessage/src/approval-polls.test.ts b/extensions/imessage/src/approval-polls.test.ts index 09679c1949ea..caa9f37ccb7f 100644 --- a/extensions/imessage/src/approval-polls.test.ts +++ b/extensions/imessage/src/approval-polls.test.ts @@ -14,8 +14,10 @@ const resolverMocks = vi.hoisted(() => ({ isApprovalNotFoundError: vi.fn(() => false), })); -vi.mock("./approval-resolver.js", () => ({ - resolveIMessageApproval: resolverMocks.resolveIMessageApproval, +vi.mock("openclaw/plugin-sdk/approval-gateway-runtime", () => ({ + resolveApprovalOverGateway: resolverMocks.resolveIMessageApproval, +})); +vi.mock("openclaw/plugin-sdk/error-runtime", () => ({ isApprovalNotFoundError: resolverMocks.isApprovalNotFoundError, })); diff --git a/extensions/imessage/src/approval-polls.ts b/extensions/imessage/src/approval-polls.ts index 33c8b55842dc..c8ebe762dfdc 100644 --- a/extensions/imessage/src/approval-polls.ts +++ b/extensions/imessage/src/approval-polls.ts @@ -9,11 +9,12 @@ import { } from "openclaw/plugin-sdk/approval-reaction-runtime"; import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-reply-runtime"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; +import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime"; import { createLazyRuntimeModule } from "openclaw/plugin-sdk/lazy-runtime"; import { asDateTimestampMs } from "openclaw/plugin-sdk/number-runtime"; import { createPluginStateErrorReporter } from "openclaw/plugin-sdk/plugin-state-runtime"; import { getIMessageApprovalApprovers, imessageApprovalAuth } from "./approval-auth.js"; -import type { IMessageApprovalGatewayRuntime } from "./approval-resolver.js"; +import type { IMessageApprovalGatewayRuntime } from "./approval-gateway-types.js"; import { buildIMessageApprovalConversationKeyForInbound, enumerateApprovalTargetKeys, @@ -52,7 +53,9 @@ type IMessageApprovalPollTarget = { type IMessageApprovalPollTombstone = { approvalId: string }; -const loadApprovalResolver = createLazyRuntimeModule(() => import("./approval-resolver.js")); +const loadApprovalResolver = createLazyRuntimeModule( + () => import("openclaw/plugin-sdk/approval-gateway-runtime"), +); const reportPersistentError = createPluginStateErrorReporter( getOptionalIMessageRuntime, @@ -531,13 +534,14 @@ export async function maybeResolveIMessageApprovalPollVote(params: { return true; } - const { isApprovalNotFoundError, resolveIMessageApproval } = await loadApprovalResolver(); + const { resolveApprovalOverGateway } = await loadApprovalResolver(); try { - const result = await resolveIMessageApproval({ + const result = await resolveApprovalOverGateway({ cfg: params.cfg, approvalId: target.approvalId, approvalKind: target.approvalKind, decision, + channel: "imessage", senderId: event.actorHandle, gatewayUrl: params.gatewayUrl, ...(params.gatewayRuntime ? { gatewayRuntime: params.gatewayRuntime } : {}), diff --git a/extensions/imessage/src/approval-reaction-poller.ts b/extensions/imessage/src/approval-reaction-poller.ts index 2a171e0b92f8..0873c2ae1658 100644 --- a/extensions/imessage/src/approval-reaction-poller.ts +++ b/extensions/imessage/src/approval-reaction-poller.ts @@ -4,6 +4,7 @@ import { asDateTimestampMs, resolveExpiresAtMsFromDurationMs, } from "openclaw/plugin-sdk/number-runtime"; +import type { IMessageApprovalGatewayRuntime } from "./approval-gateway-types.js"; import { extractIMessageApprovalPromptBinding, handleIMessageApprovalReaction, @@ -12,7 +13,6 @@ import { type PendingIMessageApprovalReactionPollTarget, type IMessageApprovalConversationKey, } from "./approval-reactions.js"; -import type { IMessageApprovalGatewayRuntime } from "./approval-resolver.js"; import type { IMessageRpcClient } from "./client.js"; import type { IMessagePayload } from "./monitor/types.js"; diff --git a/extensions/imessage/src/approval-reactions.test.ts b/extensions/imessage/src/approval-reactions.test.ts index 320c05e029fa..461a8541262d 100644 --- a/extensions/imessage/src/approval-reactions.test.ts +++ b/extensions/imessage/src/approval-reactions.test.ts @@ -37,8 +37,10 @@ function registerIMessageApprovalReactionTarget( }); } -vi.mock("./approval-resolver.js", () => ({ - resolveIMessageApproval: resolverMocks.resolveIMessageApproval, +vi.mock("openclaw/plugin-sdk/approval-gateway-runtime", () => ({ + resolveApprovalOverGateway: resolverMocks.resolveIMessageApproval, +})); +vi.mock("openclaw/plugin-sdk/error-runtime", () => ({ isApprovalNotFoundError: resolverMocks.isApprovalNotFoundError, })); @@ -771,6 +773,7 @@ describe("iMessage approval reactions", () => { expect.objectContaining({ approvalId: "exec-self", decision: "allow-once", + channel: "imessage", senderId: "+15551230000", gatewayRuntime, }), @@ -853,6 +856,7 @@ describe("iMessage approval reactions", () => { approvalId: "exec-service-prefix", approvalKind: "exec", decision: "allow-once", + channel: "imessage", senderId: "+15551230000", gatewayUrl: undefined, }); @@ -896,6 +900,7 @@ describe("iMessage approval reactions", () => { approvalId: "exec-prefixed", approvalKind: "exec", decision: "allow-once", + channel: "imessage", senderId: "+15551230000", gatewayUrl: undefined, }); @@ -953,6 +958,7 @@ describe("iMessage approval reactions", () => { approvalId: "exec-dm", approvalKind: "exec", decision: "allow-once", + channel: "imessage", senderId: "+15551230000", gatewayUrl: undefined, }); @@ -1020,6 +1026,7 @@ describe("iMessage approval reactions", () => { approvalId: "plugin:abc", approvalKind: "plugin", decision: "allow-once", + channel: "imessage", senderId: "+15551230000", gatewayUrl: undefined, }); @@ -1059,6 +1066,7 @@ describe("iMessage approval reactions", () => { approvalId: "exec-group", approvalKind: "exec", decision: "deny", + channel: "imessage", senderId: "+15551239999", gatewayUrl: undefined, }); diff --git a/extensions/imessage/src/approval-reactions.ts b/extensions/imessage/src/approval-reactions.ts index 407dde5e7488..f89addc80b82 100644 --- a/extensions/imessage/src/approval-reactions.ts +++ b/extensions/imessage/src/approval-reactions.ts @@ -19,6 +19,7 @@ import { import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-reply-runtime"; import type { OutboundDeliveryResult } from "openclaw/plugin-sdk/channel-send-result"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; +import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime"; import { createLazyRuntimeModule } from "openclaw/plugin-sdk/lazy-runtime"; import { asDateTimestampMs, @@ -28,7 +29,7 @@ import { import { createPluginStateErrorReporter } from "openclaw/plugin-sdk/plugin-state-runtime"; import type { ReplyPayload } from "openclaw/plugin-sdk/reply-runtime"; import { getIMessageApprovalApprovers, imessageApprovalAuth } from "./approval-auth.js"; -import type { IMessageApprovalGatewayRuntime } from "./approval-resolver.js"; +import type { IMessageApprovalGatewayRuntime } from "./approval-gateway-types.js"; import { buildIMessageApprovalConversationKeyForInbound, buildIMessageApprovalConversationKeyForTarget, @@ -78,7 +79,9 @@ export type PendingIMessageApprovalReactionPollTarget = { expiresAtMs: number; }; -const resolverRuntimeLoader = createLazyRuntimeModule(() => import("./approval-resolver.js")); +const resolverRuntimeLoader = createLazyRuntimeModule( + () => import("openclaw/plugin-sdk/approval-gateway-runtime"), +); const pendingReactionPollTargets = new Map(); const loadApprovalResolver = resolverRuntimeLoader; @@ -684,13 +687,14 @@ export async function handleIMessageApprovalReaction(params: { return { handled: true, stopPolling: false }; } - const { isApprovalNotFoundError, resolveIMessageApproval } = await loadApprovalResolver(); + const { resolveApprovalOverGateway } = await loadApprovalResolver(); try { - const result = await resolveIMessageApproval({ + const result = await resolveApprovalOverGateway({ cfg: params.cfg, approvalId: target.approvalId, approvalKind: target.approvalKind, decision: target.decision, + channel: "imessage", senderId: event.actorHandle, gatewayUrl: params.gatewayUrl, ...(params.gatewayRuntime ? { gatewayRuntime: params.gatewayRuntime } : {}), diff --git a/extensions/imessage/src/approval-resolver.test.ts b/extensions/imessage/src/approval-resolver.test.ts deleted file mode 100644 index 32464f9c07a7..000000000000 --- a/extensions/imessage/src/approval-resolver.test.ts +++ /dev/null @@ -1,49 +0,0 @@ -// Imessage tests cover the unified operator approval resolver. -import { beforeEach, describe, expect, it, vi } from "vitest"; - -const approvalGatewayRuntimeHoisted = vi.hoisted(() => ({ - resolveApprovalOverGatewaySpy: vi.fn(), -})); - -vi.mock("openclaw/plugin-sdk/approval-gateway-runtime", () => ({ - resolveApprovalOverGateway: (...args: unknown[]) => - approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy(...args), -})); - -describe("resolveIMessageApproval", () => { - beforeEach(() => { - approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy.mockReset(); - }); - - it("returns canonical first-answer state with explicit ownership", async () => { - const result = { - applied: false, - approval: { status: "denied", decision: "deny", reason: "user" }, - }; - approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy.mockResolvedValue(result); - const { resolveIMessageApproval } = await import("./approval-resolver.js"); - const gatewayRuntime = { request: vi.fn() } as never; - - await expect( - resolveIMessageApproval({ - cfg: {} as never, - approvalId: "plugin:looks-like-a-plugin-but-is-exec", - approvalKind: "exec", - decision: "allow-once", - senderId: "+15551230000", - gatewayRuntime, - }), - ).resolves.toBe(result); - - expect(approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy).toHaveBeenCalledWith({ - cfg: {} as never, - approvalId: "plugin:looks-like-a-plugin-but-is-exec", - approvalKind: "exec", - decision: "allow-once", - senderId: "+15551230000", - gatewayUrl: undefined, - gatewayRuntime, - clientDisplayName: "iMessage approval (+15551230000)", - }); - }); -}); diff --git a/extensions/imessage/src/approval-resolver.ts b/extensions/imessage/src/approval-resolver.ts deleted file mode 100644 index 70e0b7c6d9b6..000000000000 --- a/extensions/imessage/src/approval-resolver.ts +++ /dev/null @@ -1,43 +0,0 @@ -// Imessage plugin module implements approval resolver behavior. -import { - resolveApprovalOverGateway, - type ApprovalResolveResult, -} from "openclaw/plugin-sdk/approval-gateway-runtime"; -import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-reply-runtime"; -import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; -import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime"; - -export { isApprovalNotFoundError }; - -export type IMessageApprovalGatewayRuntime = { - request: ( - method: "approval.resolve", - params: { - id: string; - kind: "exec" | "plugin" | "system-agent"; - decision: ExecApprovalReplyDecision; - }, - options?: { clientDisplayName?: string }, - ) => Promise; -}; - -export async function resolveIMessageApproval(params: { - cfg: OpenClawConfig; - approvalId: string; - approvalKind: "exec" | "plugin"; - decision: ExecApprovalReplyDecision; - senderId?: string | null; - gatewayUrl?: string; - gatewayRuntime?: IMessageApprovalGatewayRuntime; -}): Promise { - return await resolveApprovalOverGateway({ - cfg: params.cfg, - approvalId: params.approvalId, - approvalKind: params.approvalKind, - decision: params.decision, - senderId: params.senderId, - gatewayUrl: params.gatewayUrl, - ...(params.gatewayRuntime ? { gatewayRuntime: params.gatewayRuntime } : {}), - clientDisplayName: `iMessage approval (${params.senderId?.trim() || "unknown"})`, - }); -} diff --git a/extensions/imessage/src/monitor/monitor-provider.ts b/extensions/imessage/src/monitor/monitor-provider.ts index 787d7c655e35..2af4b4f319c7 100644 --- a/extensions/imessage/src/monitor/monitor-provider.ts +++ b/extensions/imessage/src/monitor/monitor-provider.ts @@ -51,10 +51,10 @@ import { sliceUtf16Safe, truncateUtf16Safe } from "openclaw/plugin-sdk/text-util import { waitForTransportReady } from "openclaw/plugin-sdk/transport-ready-runtime"; import { resolveIMessageAccount } from "../accounts.js"; import { iMessageApprovalControlBindings } from "../approval-control-binding-window.js"; +import type { IMessageApprovalGatewayRuntime } from "../approval-gateway-types.js"; import { maybeResolveIMessageApprovalPollVote } from "../approval-polls.js"; import { pollPendingIMessageApprovalReactions } from "../approval-reaction-poller.js"; import { maybeResolveIMessageApprovalReaction } from "../approval-reactions.js"; -import type { IMessageApprovalGatewayRuntime } from "../approval-resolver.js"; import { buildIMessageApprovalConversationKeyForInbound } from "../approval-target-keys.js"; import { markIMessageChatRead, sendIMessageTyping } from "../chat.js"; import { resolveIMessageHomeDir } from "../cli-path.js"; diff --git a/extensions/matrix/src/exec-approval-resolver.test.ts b/extensions/matrix/src/exec-approval-resolver.test.ts deleted file mode 100644 index 910011474f99..000000000000 --- a/extensions/matrix/src/exec-approval-resolver.test.ts +++ /dev/null @@ -1,80 +0,0 @@ -// Matrix tests cover exec approval resolver plugin behavior. -import { beforeEach, describe, expect, it, vi } from "vitest"; - -const approvalRuntimeHoisted = vi.hoisted(() => ({ - resolveApprovalOverGatewaySpy: vi.fn(), -})); - -vi.mock("openclaw/plugin-sdk/approval-gateway-runtime", () => ({ - resolveApprovalOverGateway: (...args: unknown[]) => - approvalRuntimeHoisted.resolveApprovalOverGatewaySpy(...args), -})); - -describe("resolveMatrixApproval", () => { - beforeEach(() => { - approvalRuntimeHoisted.resolveApprovalOverGatewaySpy.mockReset(); - }); - - it("submits exec approval resolutions through the shared gateway resolver", async () => { - const result = { - applied: false, - approval: { status: "denied", decision: "deny" }, - }; - approvalRuntimeHoisted.resolveApprovalOverGatewaySpy.mockResolvedValue(result); - const { resolveMatrixApproval } = await import("./exec-approval-resolver.js"); - - await expect( - resolveMatrixApproval({ - cfg: {} as never, - approvalId: "req-123", - approvalKind: "exec", - decision: "allow-once", - senderId: "@owner:example.org", - }), - ).resolves.toBe(result); - - expect(approvalRuntimeHoisted.resolveApprovalOverGatewaySpy).toHaveBeenCalledWith({ - cfg: {} as never, - approvalId: "req-123", - approvalKind: "exec", - decision: "allow-once", - senderId: "@owner:example.org", - gatewayUrl: undefined, - clientDisplayName: "Matrix approval (@owner:example.org)", - }); - }); - - it("passes plugin approval ids through unchanged", async () => { - const { resolveMatrixApproval } = await import("./exec-approval-resolver.js"); - - await resolveMatrixApproval({ - cfg: {} as never, - approvalId: "plugin:req-123", - approvalKind: "plugin", - decision: "deny", - senderId: "@owner:example.org", - }); - - expect(approvalRuntimeHoisted.resolveApprovalOverGatewaySpy).toHaveBeenCalledWith({ - cfg: {} as never, - approvalId: "plugin:req-123", - approvalKind: "plugin", - decision: "deny", - senderId: "@owner:example.org", - gatewayUrl: undefined, - clientDisplayName: "Matrix approval (@owner:example.org)", - }); - }); - - it("recognizes structured approval-not-found errors", async () => { - const { isApprovalNotFoundError } = await import("./exec-approval-resolver.js"); - const err = new Error("approval not found"); - (err as Error & { gatewayCode?: string; details?: { reason?: string } }).gatewayCode = - "INVALID_REQUEST"; - (err as Error & { gatewayCode?: string; details?: { reason?: string } }).details = { - reason: "APPROVAL_NOT_FOUND", - }; - - expect(isApprovalNotFoundError(err)).toBe(true); - }); -}); diff --git a/extensions/matrix/src/exec-approval-resolver.ts b/extensions/matrix/src/exec-approval-resolver.ts deleted file mode 100644 index 90e7bdd9dafd..000000000000 --- a/extensions/matrix/src/exec-approval-resolver.ts +++ /dev/null @@ -1,29 +0,0 @@ -// Matrix plugin module implements exec approval resolver behavior. -import { - resolveApprovalOverGateway, - type ApprovalResolveResult, -} from "openclaw/plugin-sdk/approval-gateway-runtime"; -import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-runtime"; -import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; -import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime"; - -export { isApprovalNotFoundError }; - -export async function resolveMatrixApproval(params: { - cfg: OpenClawConfig; - approvalId: string; - approvalKind: "exec" | "plugin"; - decision: ExecApprovalReplyDecision; - senderId?: string | null; - gatewayUrl?: string; -}): Promise { - return await resolveApprovalOverGateway({ - cfg: params.cfg, - approvalId: params.approvalId, - approvalKind: params.approvalKind, - decision: params.decision, - senderId: params.senderId, - gatewayUrl: params.gatewayUrl, - clientDisplayName: `Matrix approval (${params.senderId?.trim() || "unknown"})`, - }); -} diff --git a/extensions/matrix/src/matrix/monitor/reaction-events.test.ts b/extensions/matrix/src/matrix/monitor/reaction-events.test.ts index 01da7e865dd1..6c2424d2bd84 100644 --- a/extensions/matrix/src/matrix/monitor/reaction-events.test.ts +++ b/extensions/matrix/src/matrix/monitor/reaction-events.test.ts @@ -43,10 +43,12 @@ type MatrixReactionClient = MatrixReactionParams["client"]; type MatrixReactionCore = MatrixReactionParams["core"]; type MatrixReactionEvent = MatrixReactionParams["event"]; -vi.mock("../../exec-approval-resolver.js", () => ({ +vi.mock("openclaw/plugin-sdk/approval-gateway-runtime", () => ({ + resolveApprovalOverGateway: (...args: unknown[]) => resolveMatrixApproval(...args), +})); +vi.mock("openclaw/plugin-sdk/error-runtime", () => ({ isApprovalNotFoundError: (err: unknown) => err instanceof Error && /unknown or expired approval id/i.test(err.message), - resolveMatrixApproval: (...args: unknown[]) => resolveMatrixApproval(...args), })); vi.mock("../send.js", () => ({ @@ -190,6 +192,7 @@ describe("matrix approval reactions", () => { approvalId: "req-123", approvalKind: "exec", decision: "allow-once", + channel: "matrix", senderId: "@owner:example.org", }); expect(core.system.enqueueSystemEvent).not.toHaveBeenCalled(); @@ -259,6 +262,7 @@ describe("matrix approval reactions", () => { approvalId: "req-123", approvalKind: "exec", decision: "deny", + channel: "matrix", senderId: "@owner:example.org", }); expect(core.system.enqueueSystemEvent).not.toHaveBeenCalled(); @@ -286,6 +290,7 @@ describe("matrix approval reactions", () => { approvalId: "req-123", approvalKind: "exec", decision: "allow-once", + channel: "matrix", senderId: "@owner:example.org", }); expect(core.system.enqueueSystemEvent).not.toHaveBeenCalled(); @@ -321,6 +326,7 @@ describe("matrix approval reactions", () => { approvalId: "plugin:req-123", approvalKind: "plugin", decision: "allow-once", + channel: "matrix", senderId: "@owner:example.org", }); expect(core.system.enqueueSystemEvent).not.toHaveBeenCalled(); diff --git a/extensions/matrix/src/matrix/monitor/reaction-events.ts b/extensions/matrix/src/matrix/monitor/reaction-events.ts index 40bc4b96403f..5e8b9a7963fb 100644 --- a/extensions/matrix/src/matrix/monitor/reaction-events.ts +++ b/extensions/matrix/src/matrix/monitor/reaction-events.ts @@ -1,5 +1,6 @@ // Matrix plugin module implements reaction events behavior. import type { ApprovalResolveResult } from "openclaw/plugin-sdk/approval-gateway-runtime"; +import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime"; import { createLazyRuntimeModule } from "openclaw/plugin-sdk/lazy-runtime"; import { normalizeAccountId } from "openclaw/plugin-sdk/routing"; import { getSessionBindingService } from "openclaw/plugin-sdk/session-binding-runtime"; @@ -21,7 +22,7 @@ const loadApprovalReactionAuth = createLazyRuntimeModule( ); const loadExecApprovalResolver = createLazyRuntimeModule( - () => import("../../exec-approval-resolver.js"), + () => import("openclaw/plugin-sdk/approval-gateway-runtime"), ); const loadMatrixSend = createLazyRuntimeModule(() => import("../send.js")); @@ -120,13 +121,14 @@ async function maybeResolveMatrixApprovalReaction(params: { ) { return false; } - const { isApprovalNotFoundError, resolveMatrixApproval } = await loadExecApprovalResolver(); + const { resolveApprovalOverGateway } = await loadExecApprovalResolver(); try { - const result = await resolveMatrixApproval({ + const result = await resolveApprovalOverGateway({ cfg: params.cfg, approvalId: params.target.approvalId, approvalKind: params.target.approvalKind, decision: params.target.decision, + channel: "matrix", senderId: params.senderId, }); // Retire every delivered anchor; losing surfaces also need the canonical diff --git a/extensions/pixverse/onboard.ts b/extensions/pixverse/onboard.ts index 774d2df299e4..83b904b9039b 100644 --- a/extensions/pixverse/onboard.ts +++ b/extensions/pixverse/onboard.ts @@ -12,7 +12,7 @@ import { normalizeOptionalSecretInput, type OpenClawConfig, type SecretInput, - upsertAuthProfileWithLock, + upsertAuthProfileWithLockOrThrow, validateApiKeyInput, } from "openclaw/plugin-sdk/provider-auth-api-key"; import type { ModelProviderConfig } from "openclaw/plugin-sdk/provider-onboard"; @@ -33,17 +33,6 @@ type PixVerseAuthResult = { notes: string[]; }; -type UpsertAuthProfileParams = Parameters[0]; - -async function upsertAuthProfileWithLockOrThrow(params: UpsertAuthProfileParams): Promise { - const updated = await upsertAuthProfileWithLock(params); - if (!updated) { - throw new Error( - "Failed to update auth profile store; the auth store lock may be busy. Wait a moment and retry.", - ); - } -} - function normalizePixVerseRegion(value: unknown): PixVerseApiRegion | undefined { const region = normalizeOptionalString(value)?.toLowerCase(); switch (region) { diff --git a/extensions/signal/src/approval-reactions.test.ts b/extensions/signal/src/approval-reactions.test.ts index b0f7af7c59d6..1f609fde2e81 100644 --- a/extensions/signal/src/approval-reactions.test.ts +++ b/extensions/signal/src/approval-reactions.test.ts @@ -20,8 +20,10 @@ const resolverMocks = vi.hoisted(() => ({ isApprovalNotFoundError: vi.fn(() => false), })); -vi.mock("./approval-resolver.js", () => ({ - resolveSignalApproval: resolverMocks.resolveSignalApproval, +vi.mock("openclaw/plugin-sdk/approval-gateway-runtime", () => ({ + resolveApprovalOverGateway: resolverMocks.resolveSignalApproval, +})); +vi.mock("openclaw/plugin-sdk/error-runtime", () => ({ isApprovalNotFoundError: resolverMocks.isApprovalNotFoundError, })); @@ -758,6 +760,7 @@ describe("Signal approval reactions", () => { approvalId: "plugin:abc", approvalKind: "plugin", decision: "allow-once", + channel: "signal", senderId: "+15551230000", gatewayUrl: undefined, }); @@ -818,6 +821,7 @@ describe("Signal approval reactions", () => { approvalId: "exec-default-to", approvalKind: "exec", decision: "allow-once", + channel: "signal", senderId: "+15551230000", gatewayUrl: undefined, }); diff --git a/extensions/signal/src/approval-reactions.ts b/extensions/signal/src/approval-reactions.ts index 51b4d9145de3..7b00085b023c 100644 --- a/extensions/signal/src/approval-reactions.ts +++ b/extensions/signal/src/approval-reactions.ts @@ -17,6 +17,7 @@ import { type ExecApprovalReplyDecision, } from "openclaw/plugin-sdk/approval-reply-runtime"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; +import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime"; import { createLazyRuntimeModule } from "openclaw/plugin-sdk/lazy-runtime"; import { createPluginStateErrorReporter } from "openclaw/plugin-sdk/plugin-state-runtime"; import type { ReplyPayload } from "openclaw/plugin-sdk/reply-runtime"; @@ -82,7 +83,9 @@ type SignalApprovalDeliveryResult = { meta?: Record; }; -const resolverRuntimeLoader = createLazyRuntimeModule(() => import("./approval-resolver.js")); +const resolverRuntimeLoader = createLazyRuntimeModule( + () => import("openclaw/plugin-sdk/approval-gateway-runtime"), +); const reportPersistentApprovalReactionError = createPluginStateErrorReporter( getOptionalSignalRuntime, @@ -907,13 +910,14 @@ export async function maybeResolveSignalApprovalReaction(params: { return true; } - const { isApprovalNotFoundError, resolveSignalApproval } = await loadApprovalResolver(); + const { resolveApprovalOverGateway } = await loadApprovalResolver(); try { - const result = await resolveSignalApproval({ + const result = await resolveApprovalOverGateway({ cfg: params.cfg, approvalId: target.approvalId, approvalKind: target.approvalKind, decision: target.decision, + channel: "signal", senderId: actorId, gatewayUrl: params.gatewayUrl, }); diff --git a/extensions/signal/src/approval-resolver.ts b/extensions/signal/src/approval-resolver.ts deleted file mode 100644 index 0c5196ef11e1..000000000000 --- a/extensions/signal/src/approval-resolver.ts +++ /dev/null @@ -1,29 +0,0 @@ -// Signal plugin module implements approval resolver behavior. -import { - resolveApprovalOverGateway, - type ApprovalResolveResult, -} from "openclaw/plugin-sdk/approval-gateway-runtime"; -import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-reply-runtime"; -import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; -import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime"; - -export { isApprovalNotFoundError }; - -export async function resolveSignalApproval(params: { - cfg: OpenClawConfig; - approvalId: string; - approvalKind: "exec" | "plugin"; - decision: ExecApprovalReplyDecision; - senderId?: string | null; - gatewayUrl?: string; -}): Promise { - return await resolveApprovalOverGateway({ - cfg: params.cfg, - approvalId: params.approvalId, - approvalKind: params.approvalKind, - decision: params.decision, - senderId: params.senderId, - gatewayUrl: params.gatewayUrl, - clientDisplayName: `Signal approval (${params.senderId?.trim() || "unknown"})`, - }); -} diff --git a/extensions/slack/src/monitor/events/interactions.block-actions.ts b/extensions/slack/src/monitor/events/interactions.block-actions.ts index 4a6d396b0f74..c3c308644c3d 100644 --- a/extensions/slack/src/monitor/events/interactions.block-actions.ts +++ b/extensions/slack/src/monitor/events/interactions.block-actions.ts @@ -643,8 +643,8 @@ async function handleSlackApprovalInteraction(params: { approvalId: params.approval.approvalId, approvalKind: params.approval.approvalKind, decision: params.approval.decision, + channel: "slack", senderId: params.parsed.userId, - clientDisplayName: `Slack approval (${params.parsed.userId.trim() || "unknown"})`, }); const terminalLabel = resolveSlackApprovalTerminalLabel(result.approval); const prefix = result.applied ? "Resolved" : "Already resolved"; @@ -736,9 +736,9 @@ async function handleSlackLegacyApprovalInteraction(params: { cfg: params.ctx.cfg, approvalId: parsedApproval.approvalId, decision: parsedApproval.decision, + channel: "slack", senderId: params.parsed.userId, resolveMethod, - clientDisplayName: `Slack approval (${params.parsed.userId.trim() || "unknown"})`, }); try { await updateSlackInteractionMessage({ diff --git a/extensions/slack/src/monitor/events/interactions.test.ts b/extensions/slack/src/monitor/events/interactions.test.ts index f8aaccffb0a4..26372d54f158 100644 --- a/extensions/slack/src/monitor/events/interactions.test.ts +++ b/extensions/slack/src/monitor/events/interactions.test.ts @@ -1490,7 +1490,7 @@ describe("registerSlackInteractionEvents", () => { approvalKind: "exec", decision: "allow-once", senderId: "U123", - clientDisplayName: "Slack approval (U123)", + channel: "slack", }); expect(resolvePluginConversationBindingApprovalMock).not.toHaveBeenCalled(); expect(dispatchPluginInteractiveHandlerMock).not.toHaveBeenCalled(); @@ -1618,7 +1618,7 @@ describe("registerSlackInteractionEvents", () => { approvalKind: "exec", decision: "allow-once", senderId: "U123", - clientDisplayName: "Slack approval (U123)", + channel: "slack", }); expectRecordFields(chatUpdateCall(app), { channel: "C1", @@ -1821,7 +1821,7 @@ describe("registerSlackInteractionEvents", () => { approvalKind: "plugin", decision: "allow-always", senderId: "U123OWNER", - clientDisplayName: "Slack approval (U123OWNER)", + channel: "slack", }); expect(resolvePluginConversationBindingApprovalMock).not.toHaveBeenCalled(); expect(dispatchPluginInteractiveHandlerMock).not.toHaveBeenCalled(); @@ -1900,7 +1900,7 @@ describe("registerSlackInteractionEvents", () => { decision: "allow-once", senderId: "U123OWNER", resolveMethod: "plugin", - clientDisplayName: "Slack approval (U123OWNER)", + channel: "slack", }); expect(resolvePluginConversationBindingApprovalMock).not.toHaveBeenCalled(); expect(dispatchPluginInteractiveHandlerMock).not.toHaveBeenCalled(); @@ -1973,7 +1973,7 @@ describe("registerSlackInteractionEvents", () => { approvalId: "req-legacy", decision: "allow-once", senderId: "U123OWNER", - clientDisplayName: "Slack approval (U123OWNER)", + channel: "slack", }; expect(resolveApprovalOverGatewayMock).toHaveBeenNthCalledWith(1, { ...expectedCommon, @@ -2052,7 +2052,7 @@ describe("registerSlackInteractionEvents", () => { decision: "allow-always", senderId: "U999EXEC", resolveMethod: "exec", - clientDisplayName: "Slack approval (U999EXEC)", + channel: "slack", }); expect(resolvePluginConversationBindingApprovalMock).not.toHaveBeenCalled(); expect(dispatchPluginInteractiveHandlerMock).not.toHaveBeenCalled(); diff --git a/extensions/telegram/src/bot-deps.ts b/extensions/telegram/src/bot-deps.ts index e4077d7c2880..e7310143eece 100644 --- a/extensions/telegram/src/bot-deps.ts +++ b/extensions/telegram/src/bot-deps.ts @@ -1,10 +1,16 @@ // Telegram plugin module implements bot deps behavior. +import { + resolveApprovalOverGateway, + type ApprovalResolveResult, +} from "openclaw/plugin-sdk/approval-gateway-runtime"; +import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-reply-runtime"; import { recordChannelActivity } from "openclaw/plugin-sdk/channel-activity-runtime"; import { buildChannelInboundEventContext } from "openclaw/plugin-sdk/channel-inbound"; import { createChannelMessageReplyPipeline, deliverInboundReplyWithMessageSendContext, } from "openclaw/plugin-sdk/channel-outbound"; +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { readChannelAllowFromStore } from "openclaw/plugin-sdk/conversation-runtime"; import { recordInboundSession, @@ -28,14 +34,30 @@ import { loadWebMedia } from "openclaw/plugin-sdk/web-media"; import { syncTelegramMenuCommands } from "./bot-native-command-menu.js"; import { deliverReplies, emitTelegramMessageSentHooks } from "./bot/delivery.js"; import { createTelegramDraftStream } from "./draft-stream.js"; -import { - resolveTelegramApproval, - resolveTelegramLegacyApproval, -} from "./exec-approval-resolver.js"; import { recordOutboundMessageForPromptContext } from "./outbound-message-context.js"; import { editMessageTelegram } from "./send.js"; import { wasSentByBot } from "./sent-message-cache.js"; +type ResolveTelegramApproval = (params: { + cfg: OpenClawConfig; + approvalId: string; + approvalKind: "exec" | "plugin"; + decision: ExecApprovalReplyDecision; + channel: "telegram"; + senderId?: string | null; + gatewayUrl?: string; +}) => Promise; + +type ResolveTelegramLegacyApproval = (params: { + cfg: OpenClawConfig; + approvalId: string; + decision: ExecApprovalReplyDecision; + channel: "telegram"; + senderId?: string | null; + gatewayUrl?: string; + resolveMethod: "exec" | "plugin"; +}) => Promise; + export type TelegramBotDeps = { getRuntimeConfig: typeof getRuntimeConfig; resolveStorePath: typeof resolveStorePath; @@ -57,8 +79,8 @@ export type TelegramBotDeps = { listSkillCommandsForAgents: typeof listSkillCommandsForAgents; syncTelegramMenuCommands?: typeof syncTelegramMenuCommands; wasSentByBot: typeof wasSentByBot; - resolveApproval?: typeof resolveTelegramApproval; - resolveLegacyApproval?: typeof resolveTelegramLegacyApproval; + resolveApproval?: ResolveTelegramApproval; + resolveLegacyApproval?: ResolveTelegramLegacyApproval; createTelegramDraftStream?: typeof createTelegramDraftStream; deliverReplies?: typeof deliverReplies; deliverInboundReplyWithMessageSendContext?: typeof deliverInboundReplyWithMessageSendContext; @@ -130,10 +152,10 @@ export const defaultTelegramBotDeps: TelegramBotDeps = { return wasSentByBot; }, get resolveApproval() { - return resolveTelegramApproval; + return resolveApprovalOverGateway; }, get resolveLegacyApproval() { - return resolveTelegramLegacyApproval; + return resolveApprovalOverGateway; }, get createTelegramDraftStream() { return createTelegramDraftStream; diff --git a/extensions/telegram/src/bot-handlers.callback-approvals.runtime.ts b/extensions/telegram/src/bot-handlers.callback-approvals.runtime.ts index 465f8b393eec..b9c018ae709b 100644 --- a/extensions/telegram/src/bot-handlers.callback-approvals.runtime.ts +++ b/extensions/telegram/src/bot-handlers.callback-approvals.runtime.ts @@ -1,3 +1,4 @@ +import { resolveApprovalOverGateway } from "openclaw/plugin-sdk/approval-gateway-runtime"; import type { parseExecApprovalCommandText } from "openclaw/plugin-sdk/approval-reply-runtime"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime"; @@ -14,10 +15,6 @@ import { TelegramRetryableCallbackError, } from "./bot-handlers.callback-errors.runtime.js"; import type { RegisterTelegramHandlerParams } from "./bot-native-commands.js"; -import { - resolveTelegramApproval, - resolveTelegramLegacyApproval, -} from "./exec-approval-resolver.js"; import { isTelegramExecApprovalApprover, isTelegramExecApprovalAuthorizedSender, @@ -90,11 +87,12 @@ export function createTelegramCallbackApprovalRuntime(params: { }; const resolveCanonicalApproval = async (approvalCallback: TelegramApprovalCallback) => - await (telegramDeps.resolveApproval ?? resolveTelegramApproval)({ + await (telegramDeps.resolveApproval ?? resolveApprovalOverGateway)({ cfg: runtimeCfg, approvalId: approvalCallback.approvalId, approvalKind: approvalCallback.approvalKind, decision: approvalCallback.decision, + channel: "telegram", senderId, }); @@ -178,7 +176,7 @@ export function createTelegramCallbackApprovalRuntime(params: { return; } - const resolveLegacy = telegramDeps.resolveLegacyApproval ?? resolveTelegramLegacyApproval; + const resolveLegacy = telegramDeps.resolveLegacyApproval ?? resolveApprovalOverGateway; for (const approvalKind of approvalKinds) { const canonicalCallback: TelegramApprovalCallback = { type: "approval", @@ -191,9 +189,10 @@ export function createTelegramCallbackApprovalRuntime(params: { await resolveLegacy({ cfg: runtimeCfg, approvalId: approvalCallback.approvalId, - approvalKind, decision: approvalCallback.decision, + channel: "telegram", senderId, + resolveMethod: approvalKind, }); await terminalizeApprovalMessage( buildTelegramLegacyApprovalTerminalText({ diff --git a/extensions/telegram/src/bot.create-telegram-bot.test-harness.ts b/extensions/telegram/src/bot.create-telegram-bot.test-harness.ts index d91c9c2dfd5f..74bdef91246a 100644 --- a/extensions/telegram/src/bot.create-telegram-bot.test-harness.ts +++ b/extensions/telegram/src/bot.create-telegram-bot.test-harness.ts @@ -22,7 +22,11 @@ type ReadSessionUpdatedAtFn = type SessionEntry = import("openclaw/plugin-sdk/session-store-runtime").SessionEntry; type SessionStore = Record; type LoadSessionStoreFn = (storePath?: string, opts?: unknown) => SessionStore; -type ResolveTelegramApprovalForTest = NonNullable; +type ResolveTelegramApprovalForTest = ( + params: + | Parameters>[0] + | Parameters>[0], +) => ReturnType>; type DispatchReplyWithBufferedBlockDispatcherFn = typeof import("openclaw/plugin-sdk/reply-dispatch-runtime").dispatchReplyWithBufferedBlockDispatcher; type DispatchReplyWithBufferedBlockDispatcherResult = Awaited< diff --git a/extensions/telegram/src/bot.test.ts b/extensions/telegram/src/bot.test.ts index 7e64646197a8..285a2b7f1dd4 100644 --- a/extensions/telegram/src/bot.test.ts +++ b/extensions/telegram/src/bot.test.ts @@ -1803,7 +1803,7 @@ describe("createTelegramBot", () => { expect(execApprovals.approvers).toEqual(["9"]); expect(execApprovals.target).toBe("dm"); expect(approvalCall.approvalId).toBe("138e9b8c"); - expect(approvalCall.approvalKind).toBe("exec"); + expect(approvalCall.resolveMethod).toBe("exec"); expect(approvalCall.decision).toBe("allow-once"); expect(approvalCall.senderId).toBe("9"); expect(replySpy).not.toHaveBeenCalled(); @@ -2025,7 +2025,7 @@ describe("createTelegramBot", () => { assertDistinctResult: () => { expect(execApprovalCall(0)).toMatchObject({ approvalId: "stale-legacy-id", - approvalKind: "exec", + resolveMethod: "exec", }); expect(execApprovalCall(1)).toMatchObject({ approvalId: "stale-legacy-id", @@ -2149,12 +2149,12 @@ describe("createTelegramBot", () => { expect(execApprovals.approvers).toEqual(["9"]); expect(execApprovals.target).toBe("dm"); expect(approvalCall.approvalId).toBe("opaque-plugin-approval-id"); - expect(approvalCall.approvalKind).toBe("exec"); + expect(approvalCall.resolveMethod).toBe("exec"); expect(approvalCall.decision).toBe("allow-once"); expect(approvalCall.senderId).toBe("9"); expect(execApprovalCall(1)).toMatchObject({ approvalId: "opaque-plugin-approval-id", - approvalKind: "plugin", + resolveMethod: "plugin", decision: "allow-once", senderId: "9", }); @@ -2268,7 +2268,7 @@ describe("createTelegramBot", () => { expect(execApprovals.enabled).toBe(true); expect(execApprovals.mode).toBe("targets"); expect(approvalCall.approvalId).toBe("plugin:misleading-exec-id"); - expect(approvalCall.approvalKind).toBe("exec"); + expect(approvalCall.resolveMethod).toBe("exec"); expect(approvalCall.decision).toBe("allow-once"); expect(approvalCall.senderId).toBe("9"); expect(resolveExecApprovalSpy).toHaveBeenCalledTimes(1); @@ -2313,7 +2313,7 @@ describe("createTelegramBot", () => { expect(execApprovals.enabled).toBe(true); expect(execApprovals.mode).toBe("targets"); expect(approvalCall.approvalId).toBe("138e9b8c"); - expect(approvalCall.approvalKind).toBe("exec"); + expect(approvalCall.resolveMethod).toBe("exec"); expect(approvalCall.decision).toBe("allow-once"); expect(approvalCall.senderId).toBe("9"); expect(resolveExecApprovalSpy).toHaveBeenCalledTimes(1); @@ -2343,11 +2343,11 @@ describe("createTelegramBot", () => { const approvalCall = execApprovalCall(); expect(approvalCall.approvalId).toBe("138e9b8c"); - expect(approvalCall.approvalKind).toBe("exec"); + expect(approvalCall.resolveMethod).toBe("exec"); expect(approvalCall.decision).toBe("allow-once"); expect(approvalCall.senderId).toBe("9"); expect(resolveExecApprovalSpy).toHaveBeenCalledTimes(2); - expect(execApprovalCall(1).approvalKind).toBe("plugin"); + expect(execApprovalCall(1).resolveMethod).toBe("plugin"); expect(editMessageTextSpy).toHaveBeenCalledWith( 1234, 26, @@ -2391,7 +2391,7 @@ describe("createTelegramBot", () => { expect(execApprovalCall()).toMatchObject({ approvalId: "plugin:138e9b8c", - approvalKind: "exec", + resolveMethod: "exec", decision: "allow-once", senderId: "9", }); diff --git a/extensions/telegram/src/exec-approval-resolver.test.ts b/extensions/telegram/src/exec-approval-resolver.test.ts deleted file mode 100644 index fbcc5edb9f5e..000000000000 --- a/extensions/telegram/src/exec-approval-resolver.test.ts +++ /dev/null @@ -1,77 +0,0 @@ -// Telegram tests cover the unified operator approval resolver. -import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-reply-runtime"; -import { beforeEach, describe, expect, it, vi } from "vitest"; - -const approvalGatewayRuntimeHoisted = vi.hoisted(() => ({ - resolveApprovalOverGatewaySpy: vi.fn(), -})); - -vi.mock("openclaw/plugin-sdk/approval-gateway-runtime", () => ({ - resolveApprovalOverGateway: (...args: unknown[]) => - approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy(...args), -})); - -describe("resolveTelegramApproval", () => { - beforeEach(() => { - approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy.mockReset(); - }); - - it.each([ - ["exec", "plugin:id-that-still-belongs-to-exec", "allow-once"], - ["plugin", "plain-plugin-id", "allow-always"], - ] as const)( - "passes explicit %s ownership without inferring it from %s", - async (approvalKind, approvalId, decision) => { - const result = { - applied: false, - approval: { status: "denied", decision: "deny" }, - }; - approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy.mockResolvedValue(result); - const { resolveTelegramApproval } = await import("./exec-approval-resolver.js"); - - await expect( - resolveTelegramApproval({ - cfg: {} as never, - gatewayUrl: undefined, - approvalId, - approvalKind, - decision: decision as ExecApprovalReplyDecision, - senderId: "9", - }), - ).resolves.toBe(result); - - expect(approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy).toHaveBeenCalledWith({ - cfg: {} as never, - approvalId, - approvalKind, - decision, - senderId: "9", - gatewayUrl: undefined, - clientDisplayName: "Telegram approval (9)", - }); - }, - ); - - it("keeps command/value compatibility on an explicit legacy adapter", async () => { - approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy.mockResolvedValue(undefined); - const { resolveTelegramLegacyApproval } = await import("./exec-approval-resolver.js"); - - await resolveTelegramLegacyApproval({ - cfg: {} as never, - approvalId: "legacy-plugin-id", - approvalKind: "plugin", - decision: "deny", - senderId: "9", - }); - - expect(approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy).toHaveBeenCalledWith({ - cfg: {} as never, - approvalId: "legacy-plugin-id", - decision: "deny", - senderId: "9", - gatewayUrl: undefined, - resolveMethod: "plugin", - clientDisplayName: "Telegram approval (9)", - }); - }); -}); diff --git a/extensions/telegram/src/exec-approval-resolver.ts b/extensions/telegram/src/exec-approval-resolver.ts deleted file mode 100644 index f1bb4c9af5a2..000000000000 --- a/extensions/telegram/src/exec-approval-resolver.ts +++ /dev/null @@ -1,49 +0,0 @@ -// Telegram plugin module resolves typed operator approvals through the Gateway. -import { - resolveApprovalOverGateway, - type ApprovalResolveResult, -} from "openclaw/plugin-sdk/approval-gateway-runtime"; -import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-reply-runtime"; -import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; - -type ResolveTelegramApprovalParams = { - cfg: OpenClawConfig; - approvalId: string; - approvalKind: "exec" | "plugin"; - decision: ExecApprovalReplyDecision; - senderId?: string | null; - gatewayUrl?: string; -}; - -type ResolveTelegramLegacyApprovalParams = Omit & { - approvalKind: "exec" | "plugin"; -}; - -export async function resolveTelegramApproval( - params: ResolveTelegramApprovalParams, -): Promise { - return await resolveApprovalOverGateway({ - cfg: params.cfg, - approvalId: params.approvalId, - approvalKind: params.approvalKind, - decision: params.decision, - senderId: params.senderId, - gatewayUrl: params.gatewayUrl, - clientDisplayName: `Telegram approval (${params.senderId?.trim() || "unknown"})`, - }); -} - -/** Compatibility resolver for command/value buttons that predate typed approval actions. */ -export async function resolveTelegramLegacyApproval( - params: ResolveTelegramLegacyApprovalParams, -): Promise { - await resolveApprovalOverGateway({ - cfg: params.cfg, - approvalId: params.approvalId, - decision: params.decision, - senderId: params.senderId, - gatewayUrl: params.gatewayUrl, - resolveMethod: params.approvalKind, - clientDisplayName: `Telegram approval (${params.senderId?.trim() || "unknown"})`, - }); -} diff --git a/extensions/whatsapp/src/approval-reactions.test.ts b/extensions/whatsapp/src/approval-reactions.test.ts index 33d36ae793ba..b149364ae562 100644 --- a/extensions/whatsapp/src/approval-reactions.test.ts +++ b/extensions/whatsapp/src/approval-reactions.test.ts @@ -17,8 +17,10 @@ const resolverMocks = vi.hoisted(() => ({ isApprovalNotFoundError: vi.fn(() => false), })); -vi.mock("./approval-resolver.js", () => ({ - resolveWhatsAppApproval: resolverMocks.resolveWhatsAppApproval, +vi.mock("openclaw/plugin-sdk/approval-gateway-runtime", () => ({ + resolveApprovalOverGateway: resolverMocks.resolveWhatsAppApproval, +})); +vi.mock("openclaw/plugin-sdk/error-runtime", () => ({ isApprovalNotFoundError: resolverMocks.isApprovalNotFoundError, })); @@ -175,6 +177,7 @@ describe("WhatsApp approval reactions", () => { approvalId: "plugin:abc", approvalKind: "plugin", decision: "allow-once", + channel: "whatsapp", senderId: "+15551230000", gatewayUrl: undefined, }); @@ -251,6 +254,7 @@ describe("WhatsApp approval reactions", () => { approvalId: "exec-self", approvalKind: "exec", decision: "allow-once", + channel: "whatsapp", senderId: "+15551230001", gatewayUrl: undefined, }); @@ -315,6 +319,7 @@ describe("WhatsApp approval reactions", () => { approvalId: "exec-direct", approvalKind: "exec", decision: "allow-once", + channel: "whatsapp", senderId: testCase.actorId, gatewayUrl: undefined, }); diff --git a/extensions/whatsapp/src/approval-reactions.ts b/extensions/whatsapp/src/approval-reactions.ts index 729dad35f404..98b498de8135 100644 --- a/extensions/whatsapp/src/approval-reactions.ts +++ b/extensions/whatsapp/src/approval-reactions.ts @@ -15,6 +15,7 @@ import { import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-reply-runtime"; import type { OutboundDeliveryResult } from "openclaw/plugin-sdk/channel-send-result"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; +import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime"; import type { MessagePresentation } from "openclaw/plugin-sdk/interactive-runtime"; import { createLazyRuntimeModule } from "openclaw/plugin-sdk/lazy-runtime"; import { createPluginStateErrorReporter } from "openclaw/plugin-sdk/plugin-state-runtime"; @@ -55,7 +56,9 @@ type ResolvedWhatsAppApprovalReactionTarget = WhatsAppApprovalReactionResolution remoteJid: string; }; -const resolverRuntimeLoader = createLazyRuntimeModule(() => import("./approval-resolver.js")); +const resolverRuntimeLoader = createLazyRuntimeModule( + () => import("openclaw/plugin-sdk/approval-gateway-runtime"), +); const reportPersistentApprovalReactionError = createPluginStateErrorReporter( getOptionalWhatsAppRuntime, @@ -508,13 +511,14 @@ export async function maybeResolveWhatsAppApprovalReaction(params: { return true; } - const { isApprovalNotFoundError, resolveWhatsAppApproval } = await loadApprovalResolver(); + const { resolveApprovalOverGateway } = await loadApprovalResolver(); try { - const result = await resolveWhatsAppApproval({ + const result = await resolveApprovalOverGateway({ cfg: params.cfg, approvalId: target.approvalId, approvalKind: target.approvalKind, decision: target.decision, + channel: "whatsapp", senderId: actorId, gatewayUrl: params.gatewayUrl, }); diff --git a/extensions/whatsapp/src/approval-resolver.test.ts b/extensions/whatsapp/src/approval-resolver.test.ts deleted file mode 100644 index 5b709c3f0964..000000000000 --- a/extensions/whatsapp/src/approval-resolver.test.ts +++ /dev/null @@ -1,46 +0,0 @@ -// WhatsApp tests cover canonical multi-surface approval resolver outcomes. -import { beforeEach, describe, expect, it, vi } from "vitest"; - -const approvalGatewayRuntimeHoisted = vi.hoisted(() => ({ - resolveApprovalOverGatewaySpy: vi.fn(), -})); - -vi.mock("openclaw/plugin-sdk/approval-gateway-runtime", () => ({ - resolveApprovalOverGateway: (...args: unknown[]) => - approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy(...args), -})); - -describe("resolveWhatsAppApproval", () => { - beforeEach(() => { - approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy.mockReset(); - }); - - it("returns the canonical first-answer result without inferring ownership from the id", async () => { - const result = { - applied: false, - approval: { status: "denied", decision: "deny" }, - }; - approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy.mockResolvedValue(result); - const { resolveWhatsAppApproval } = await import("./approval-resolver.js"); - - await expect( - resolveWhatsAppApproval({ - cfg: {} as never, - approvalId: "plugin:looks-plugin-but-is-exec", - approvalKind: "exec", - decision: "allow-once", - senderId: "+15551230000", - }), - ).resolves.toBe(result); - - expect(approvalGatewayRuntimeHoisted.resolveApprovalOverGatewaySpy).toHaveBeenCalledWith({ - cfg: {}, - approvalId: "plugin:looks-plugin-but-is-exec", - approvalKind: "exec", - decision: "allow-once", - senderId: "+15551230000", - gatewayUrl: undefined, - clientDisplayName: "WhatsApp approval (+15551230000)", - }); - }); -}); diff --git a/extensions/whatsapp/src/approval-resolver.ts b/extensions/whatsapp/src/approval-resolver.ts deleted file mode 100644 index 7fa5c6ad03f9..000000000000 --- a/extensions/whatsapp/src/approval-resolver.ts +++ /dev/null @@ -1,29 +0,0 @@ -// Whatsapp plugin module implements approval resolver behavior. -import { - resolveApprovalOverGateway, - type ApprovalResolveResult, -} from "openclaw/plugin-sdk/approval-gateway-runtime"; -import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-reply-runtime"; -import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; -import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime"; - -export { isApprovalNotFoundError }; - -export async function resolveWhatsAppApproval(params: { - cfg: OpenClawConfig; - approvalId: string; - approvalKind: "exec" | "plugin"; - decision: ExecApprovalReplyDecision; - senderId?: string | null; - gatewayUrl?: string; -}): Promise { - return await resolveApprovalOverGateway({ - cfg: params.cfg, - approvalId: params.approvalId, - approvalKind: params.approvalKind, - decision: params.decision, - senderId: params.senderId, - gatewayUrl: params.gatewayUrl, - clientDisplayName: `WhatsApp approval (${params.senderId?.trim() || "unknown"})`, - }); -} diff --git a/extensions/xiaomi/index.ts b/extensions/xiaomi/index.ts index 6b222d7fc384..bca0b662254a 100644 --- a/extensions/xiaomi/index.ts +++ b/extensions/xiaomi/index.ts @@ -16,7 +16,7 @@ import { normalizeApiKeyInput, normalizeOptionalSecretInput, type SecretInput, - upsertAuthProfileWithLock, + upsertAuthProfileWithLockOrThrow, validateApiKeyInput, } from "openclaw/plugin-sdk/provider-auth-api-key"; import { buildOpenAICompatibleLiveModelProviderConfig } from "openclaw/plugin-sdk/provider-catalog-live-runtime"; @@ -42,8 +42,6 @@ import { buildXiaomiSpeechProvider } from "./speech-provider.js"; import { createMiMoThinkingWrapper } from "./stream.js"; import { resolveMiMoThinkingProfile } from "./thinking.js"; -type UpsertAuthProfileParams = Parameters[0]; - const PAYG_FLAG_NAME = "--xiaomi-api-key"; const PAYG_OPTION_KEY = "xiaomiApiKey"; const PAYG_ENV_VAR = "XIAOMI_API_KEY"; @@ -120,15 +118,6 @@ async function resolveXiaomiCatalog(params: { }; } -async function upsertAuthProfileWithLockOrThrow(params: UpsertAuthProfileParams): Promise { - const updated = await upsertAuthProfileWithLock(params); - if (!updated) { - throw new Error( - "Failed to update auth profile store; the auth store lock may be busy. Wait a moment and retry.", - ); - } -} - function buildXiaomiKeyMismatchMessage(params: { actualKey: string; expectedKind: "payg" | "token-plan"; diff --git a/extensions/zai/index.ts b/extensions/zai/index.ts index 3100574fe355..233f12a8294c 100644 --- a/extensions/zai/index.ts +++ b/extensions/zai/index.ts @@ -16,7 +16,7 @@ import { normalizeApiKeyInput, normalizeOptionalSecretInput, type SecretInput, - upsertAuthProfileWithLock, + upsertAuthProfileWithLockOrThrow, validateApiKeyInput, } from "openclaw/plugin-sdk/provider-auth-api-key"; import { defineSingleProviderPluginEntry } from "openclaw/plugin-sdk/provider-entry"; @@ -41,8 +41,6 @@ import { isGlm52ModelId, resolveThinkingProfile } from "./provider-policy-api.js const PROVIDER_ID = "zai"; const GLM5_TEMPLATE_MODEL_ID = "glm-4.7"; const PROFILE_ID = "zai:default"; -type UpsertAuthProfileParams = Parameters[0]; - function resolveDeprecatedPiAgentAuthPath(env: NodeJS.ProcessEnv): string { const home = env.HOME?.trim() || env.USERPROFILE?.trim() || os.homedir(); return path.join(home, ".pi", "agent", "auth.json"); @@ -71,15 +69,6 @@ function resolveDeprecatedPiAgentAccessToken( return undefined; } -async function upsertAuthProfileWithLockOrThrow(params: UpsertAuthProfileParams): Promise { - const updated = await upsertAuthProfileWithLock(params); - if (!updated) { - throw new Error( - "Failed to update auth profile store; the auth store lock may be busy. Wait a moment and retry.", - ); - } -} - function resolveGlm5ForwardCompatModel(ctx: ProviderResolveDynamicModelContext) { return resolveFamilyForwardCompatModel({ providerId: PROVIDER_ID, diff --git a/src/agents/auth-profiles.ts b/src/agents/auth-profiles.ts index a2e9ca44338c..1275f364eb2a 100644 --- a/src/agents/auth-profiles.ts +++ b/src/agents/auth-profiles.ts @@ -43,6 +43,7 @@ export { setAuthProfileOrder, upsertAuthProfile, upsertAuthProfileWithLock, + upsertAuthProfileWithLockOrThrow, } from "./auth-profiles/profiles.js"; export { repairOAuthProfileIdMismatch, diff --git a/src/agents/auth-profiles/profiles.ts b/src/agents/auth-profiles/profiles.ts index efefc7ab9cdc..64781726c915 100644 --- a/src/agents/auth-profiles/profiles.ts +++ b/src/agents/auth-profiles/profiles.ts @@ -24,7 +24,7 @@ export { listProfilesForProvider, resolveSubscriptionAuthModeForProfiles, } from "./profile-list.js"; -export { upsertAuthProfileWithLock } from "./upsert-with-lock.js"; +export { upsertAuthProfileWithLock, upsertAuthProfileWithLockOrThrow } from "./upsert-with-lock.js"; const authProfileProfilesLog = createSubsystemLogger("agent/embedded"); diff --git a/src/agents/auth-profiles/upsert-with-lock.test.ts b/src/agents/auth-profiles/upsert-with-lock.test.ts new file mode 100644 index 000000000000..4aadd38132f7 --- /dev/null +++ b/src/agents/auth-profiles/upsert-with-lock.test.ts @@ -0,0 +1,44 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const hoisted = vi.hoisted(() => ({ + updateAuthProfileStoreWithLock: vi.fn(), +})); + +vi.mock("./credential-normalize.js", () => ({ + normalizeAuthProfileCredential: (credential: unknown) => credential, +})); +vi.mock("./store.js", () => ({ + updateAuthProfileStoreWithLock: hoisted.updateAuthProfileStoreWithLock, +})); + +import { upsertAuthProfileWithLockOrThrow } from "./upsert-with-lock.js"; + +describe("upsertAuthProfileWithLockOrThrow", () => { + beforeEach(() => { + hoisted.updateAuthProfileStoreWithLock.mockReset(); + }); + + it("resolves after the locked store update succeeds", async () => { + hoisted.updateAuthProfileStoreWithLock.mockResolvedValue({ version: 1, profiles: {} }); + + await expect( + upsertAuthProfileWithLockOrThrow({ + profileId: "test:default", + credential: { type: "token", provider: "test", token: "secret" }, + }), + ).resolves.toBeUndefined(); + }); + + it("fails with the canonical retry guidance when the locked update fails", async () => { + hoisted.updateAuthProfileStoreWithLock.mockResolvedValue(null); + + await expect( + upsertAuthProfileWithLockOrThrow({ + profileId: "test:default", + credential: { type: "token", provider: "test", token: "secret" }, + }), + ).rejects.toThrow( + "Failed to update auth profile store; the auth store lock may be busy. Wait a moment and retry.", + ); + }); +}); diff --git a/src/agents/auth-profiles/upsert-with-lock.ts b/src/agents/auth-profiles/upsert-with-lock.ts index dec4cc28562d..a6652212dcf6 100644 --- a/src/agents/auth-profiles/upsert-with-lock.ts +++ b/src/agents/auth-profiles/upsert-with-lock.ts @@ -28,3 +28,15 @@ export async function upsertAuthProfileWithLock(params: { }, }); } + +/** Upserts an auth profile under the store lock, failing when the store cannot be written. */ +export async function upsertAuthProfileWithLockOrThrow( + params: Parameters[0], +): Promise { + const updated = await upsertAuthProfileWithLock(params); + if (!updated) { + throw new Error( + "Failed to update auth profile store; the auth store lock may be busy. Wait a moment and retry.", + ); + } +} diff --git a/src/commands/auth-choice.apply.plugin-provider.test.ts b/src/commands/auth-choice.apply.plugin-provider.test.ts index 148e4f7a0262..c1e7d0dbe438 100644 --- a/src/commands/auth-choice.apply.plugin-provider.test.ts +++ b/src/commands/auth-choice.apply.plugin-provider.test.ts @@ -47,6 +47,7 @@ const upsertAuthProfile = vi.hoisted(() => vi.fn(() => ({ version: 1, profiles: vi.mock("../agents/auth-profiles.js", () => ({ upsertAuthProfile, upsertAuthProfileWithLock: upsertAuthProfile, + upsertAuthProfileWithLockOrThrow: upsertAuthProfile, })); const resolveDefaultAgentId = vi.hoisted(() => vi.fn(() => "default")); diff --git a/src/commands/auth-choice.test.ts b/src/commands/auth-choice.test.ts index a6a53f8b9014..68445f107ad0 100644 --- a/src/commands/auth-choice.test.ts +++ b/src/commands/auth-choice.test.ts @@ -201,6 +201,13 @@ vi.mock("../agents/auth-profiles.js", () => ({ seedTestAuthProfile(params); return { version: 1, profiles: readTestAuthProfileStore(params.agentDir).profiles }; }, + upsertAuthProfileWithLockOrThrow: async (params: { + profileId: string; + credential: StoredAuthProfile; + agentDir?: string; + }) => { + seedTestAuthProfile(params); + }, })); function normalizeText(value: unknown): string { diff --git a/src/commands/models/auth.test.ts b/src/commands/models/auth.test.ts index 8a92e403c42d..8319cc07c604 100644 --- a/src/commands/models/auth.test.ts +++ b/src/commands/models/auth.test.ts @@ -75,6 +75,7 @@ vi.mock("../../agents/auth-profiles/profiles.js", () => ({ removeProviderAuthProfilesWithLock: mocks.removeProviderAuthProfilesWithLock, upsertAuthProfile: mocks.upsertAuthProfile, upsertAuthProfileWithLock: mocks.upsertAuthProfileWithLock, + upsertAuthProfileWithLockOrThrow: mocks.upsertAuthProfileWithLock, })); vi.mock("../../agents/auth-profiles/store.js", () => ({ diff --git a/src/commands/models/auth.ts b/src/commands/models/auth.ts index 7d4b097c8214..6529d73ecf06 100644 --- a/src/commands/models/auth.ts +++ b/src/commands/models/auth.ts @@ -28,7 +28,7 @@ import { import { listProfilesForProvider, promoteAuthProfileInOrder, - upsertAuthProfileWithLock, + upsertAuthProfileWithLockOrThrow, } from "../../agents/auth-profiles/profiles.js"; import { loadAuthProfileStoreForRuntime } from "../../agents/auth-profiles/store.js"; import type { AuthProfileCredential } from "../../agents/auth-profiles/types.js"; @@ -71,8 +71,6 @@ import { repairCopilotRuntimePluginInstallForModelSelection } from "../copilot-r import { refreshRunningGatewayAuthState } from "./auth-refresh.js"; import { loadValidConfigOrThrow, resolveKnownAgentId, updateConfig } from "./shared.js"; -type UpsertAuthProfileParams = Parameters[0]; - function resolveManualTokenExpiryMs(expiresIn: string | undefined): number | undefined { const normalizedExpiresIn = normalizeStringifiedOptionalString(expiresIn); if (!normalizedExpiresIn) { @@ -779,15 +777,6 @@ export async function modelsAuthPasteApiKeyCommand( runtime.log(`Auth profile: ${profileId} (${provider}/api_key)`); } -async function upsertAuthProfileWithLockOrThrow(params: UpsertAuthProfileParams): Promise { - const updated = await upsertAuthProfileWithLock(params); - if (!updated) { - throw new Error( - "Failed to update auth profile store; the auth store lock may be busy. Wait a moment and retry.", - ); - } -} - /** Interactive helper for adding token auth profiles, with provider/method prompts. */ export async function modelsAuthAddCommand(opts: { agent?: string }, runtime: RuntimeEnv) { const { config, agentDir, workspaceDir, providers } = await resolveModelsAuthContext({ diff --git a/src/commands/onboard-auth.test.ts b/src/commands/onboard-auth.test.ts index 97ccebdc6304..a3cc581f1811 100644 --- a/src/commands/onboard-auth.test.ts +++ b/src/commands/onboard-auth.test.ts @@ -76,6 +76,13 @@ vi.mock("../agents/auth-profiles/profiles.js", async () => { upsert(params); return { version: 1, profiles: {} }; }, + upsertAuthProfileWithLockOrThrow: async (params: { + profileId: string; + credential: unknown; + agentDir?: string; + }) => { + upsert(params); + }, }; }); diff --git a/src/infra/approval-gateway-resolver.test.ts b/src/infra/approval-gateway-resolver.test.ts index 734c019d4d86..84f7dee684ba 100644 --- a/src/infra/approval-gateway-resolver.test.ts +++ b/src/infra/approval-gateway-resolver.test.ts @@ -78,6 +78,37 @@ describe("resolveApprovalOverGateway", () => { expect(result).toEqual({ applied: true, approval: recordedApproval }); }); + it.each([ + ["signal", "Signal"], + ["whatsapp", "WhatsApp"], + ["matrix", "Matrix"], + ["imessage", "iMessage"], + ["telegram", "Telegram"], + ["discord", "Discord"], + ["googlechat", "Google Chat"], + ["slack", "Slack"], + ] as const)( + "derives the %s approval client label from channel metadata", + async (channel, label) => { + await resolveApprovalOverGateway({ + cfg: {} as never, + approvalId: "approval-1", + approvalKind: "exec", + decision: "deny", + channel, + senderId: "owner", + }); + + const [gatewayClientOptions] = requireFirstMockCall( + hoisted.withOperatorApprovalsGatewayClient, + "gateway client", + ); + expect(gatewayClientOptions).toMatchObject({ + clientDisplayName: `${label} approval (owner)`, + }); + }, + ); + it("uses explicit plugin kind without inspecting the approval id", async () => { await resolveApprovalOverGateway({ cfg: {} as never, diff --git a/src/infra/approval-gateway-resolver.ts b/src/infra/approval-gateway-resolver.ts index 906540694ee5..54133c28d8fd 100644 --- a/src/infra/approval-gateway-resolver.ts +++ b/src/infra/approval-gateway-resolver.ts @@ -6,6 +6,7 @@ import type { ApprovalResolveResult, } from "../../packages/gateway-protocol/src/index.js"; import { isWellFormedApprovalId } from "../../packages/gateway-protocol/src/schema/approvals.js"; +import { findChatChannelMeta } from "../channels/chat-meta.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { withOperatorApprovalsGatewayClient } from "../gateway/operator-approvals-client.js"; import { isApprovalNotFoundError } from "./approval-errors.js"; @@ -16,21 +17,24 @@ type ResolveApprovalOverGatewayBaseParams = { cfg: OpenClawConfig; approvalId: string; decision: ApprovalDecision; + channel?: string; senderId?: string | null; gatewayUrl?: string; clientDisplayName?: string; }; +type ApprovalGatewayRuntime = { + request: ( + method: "approval.resolve", + params: ApprovalResolveParams, + options?: { clientDisplayName?: string }, + ) => Promise; +}; + type CanonicalResolveApprovalOverGatewayParams = ResolveApprovalOverGatewayBaseParams & { /** Explicit owner required by the canonical approval resolver. */ approvalKind: ApprovalKind; - gatewayRuntime?: { - request: ( - method: "approval.resolve", - params: ApprovalResolveParams, - options?: { clientDisplayName?: string }, - ) => Promise; - }; + gatewayRuntime?: ApprovalGatewayRuntime; allowPluginFallback?: never; resolveMethod?: never; }; @@ -102,8 +106,14 @@ export async function resolveApprovalOverGateway( if (typeof approvalId !== "string" || !isWellFormedApprovalId(approvalId)) { throw new Error("approval resolution requires an approval id"); } + const senderId = params.senderId?.trim() || "unknown"; + const channel = params.channel?.trim(); + // Channel manifests own operator-facing labels; using their generated metadata + // keeps approval clients aligned without importing plugin runtime or hardcoding ids. + const channelLabel = channel ? (findChatChannelMeta(channel)?.label ?? channel) : undefined; const clientDisplayName = - params.clientDisplayName ?? `Approval (${params.senderId?.trim() || "unknown"})`; + params.clientDisplayName ?? + (channelLabel ? `${channelLabel} approval (${senderId})` : `Approval (${senderId})`); const canonicalGatewayRuntime = (params as CanonicalResolveApprovalOverGatewayParams) .gatewayRuntime; diff --git a/src/plugin-sdk/provider-auth-api-key.ts b/src/plugin-sdk/provider-auth-api-key.ts index ed1a14312168..8107007ff0bf 100644 --- a/src/plugin-sdk/provider-auth-api-key.ts +++ b/src/plugin-sdk/provider-auth-api-key.ts @@ -4,7 +4,11 @@ export type { OpenClawConfig } from "../config/config.js"; export type { SecretInput } from "../config/types.secrets.js"; -export { upsertAuthProfile, upsertAuthProfileWithLock } from "../agents/auth-profiles/profiles.js"; +export { + upsertAuthProfile, + upsertAuthProfileWithLock, + upsertAuthProfileWithLockOrThrow, +} from "../agents/auth-profiles/profiles.js"; export { formatApiKeyPreview, normalizeApiKeyInput, diff --git a/src/plugins/provider-api-key-auth.ts b/src/plugins/provider-api-key-auth.ts index f9bb3c2832c0..6c9701c4d6bd 100644 --- a/src/plugins/provider-api-key-auth.ts +++ b/src/plugins/provider-api-key-auth.ts @@ -1,7 +1,7 @@ /** Builds API-key provider auth methods that write profiles and config updates. */ import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import { normalizeUniqueStringEntries } from "@openclaw/normalization-core/string-normalization"; -import { upsertAuthProfileWithLock } from "../agents/auth-profiles/profiles.js"; +import { upsertAuthProfileWithLockOrThrow } from "../agents/auth-profiles/profiles.js"; import { resolveAgentModelPrimaryValue } from "../config/model-input.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { SecretInput } from "../config/types.secrets.js"; @@ -39,8 +39,6 @@ type ProviderApiKeyAuthMethodOptions = { applyConfig?: (cfg: OpenClawConfig) => OpenClawConfig; }; -type UpsertAuthProfileParams = Parameters[0]; - const loadProviderApiKeyAuthRuntime = createLazyRuntimeSurface( () => import("./provider-api-key-auth.runtime.js"), ({ providerApiKeyAuthRuntime }) => providerApiKeyAuthRuntime, @@ -66,15 +64,6 @@ function resolveProfileIds(params: { return [resolveProfileId(params)]; } -async function upsertAuthProfileWithLockOrThrow(params: UpsertAuthProfileParams): Promise { - const updated = await upsertAuthProfileWithLock(params); - if (!updated) { - throw new Error( - "Failed to update auth profile store; the auth store lock may be busy. Wait a moment and retry.", - ); - } -} - async function applyApiKeyConfig(params: { ctx: ProviderAuthMethodNonInteractiveContext; providerId: string; diff --git a/src/plugins/provider-auth-choice.ts b/src/plugins/provider-auth-choice.ts index bed68d2f4c8c..9cdd5385684b 100644 --- a/src/plugins/provider-auth-choice.ts +++ b/src/plugins/provider-auth-choice.ts @@ -5,7 +5,7 @@ import { resolveAgentDir, resolveAgentWorkspaceDir, } from "../agents/agent-scope.js"; -import { upsertAuthProfileWithLock } from "../agents/auth-profiles.js"; +import { upsertAuthProfileWithLockOrThrow } from "../agents/auth-profiles.js"; import { formatLiteralProviderPrefixedModelRef } from "../agents/model-ref-shared.js"; import { resolveDefaultAgentWorkspaceDir } from "../agents/workspace.js"; import { normalizeAgentModelRefForConfig } from "../config/model-input.js"; @@ -31,8 +31,6 @@ import type { ProviderPlugin, } from "./types.js"; -type UpsertAuthProfileParams = Parameters[0]; - type ApplyProviderAuthChoiceParams = { authChoice: string; config: OpenClawConfig; @@ -643,11 +641,3 @@ export async function applyAuthChoiceLoadedPluginProvider( ...(prepared.retrySelection ? { retrySelection: true } : {}), }; } -async function upsertAuthProfileWithLockOrThrow(params: UpsertAuthProfileParams): Promise { - const updated = await upsertAuthProfileWithLock(params); - if (!updated) { - throw new Error( - "Failed to update auth profile store; the auth store lock may be busy. Wait a moment and retry.", - ); - } -} diff --git a/src/plugins/provider-auth-helpers.ts b/src/plugins/provider-auth-helpers.ts index e59cb18c39fe..e589bf837831 100644 --- a/src/plugins/provider-auth-helpers.ts +++ b/src/plugins/provider-auth-helpers.ts @@ -4,7 +4,11 @@ import path from "node:path"; import { uniqueStrings } from "@openclaw/normalization-core/string-normalization"; import { resolveDefaultAgentDir } from "../agents/agent-scope-config.js"; import { buildAuthProfileId } from "../agents/auth-profiles/identity.js"; -import { upsertAuthProfile, upsertAuthProfileWithLock } from "../agents/auth-profiles/profiles.js"; +import { + upsertAuthProfile, + upsertAuthProfileWithLock, + upsertAuthProfileWithLockOrThrow, +} from "../agents/auth-profiles/profiles.js"; import { resolveProviderIdForAuth } from "../agents/provider-auth-aliases.js"; import { resolveStateDir } from "../config/paths.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; @@ -22,8 +26,6 @@ import { isValidSecretRef } from "../secrets/ref-contract.js"; import { normalizeSecretInput } from "../utils/normalize-secret-input.js"; import type { SecretInputMode } from "./provider-auth-types.js"; -type UpsertAuthProfileParams = Parameters[0]; - const resolveAuthAgentDir = (agentDir?: string, config?: OpenClawConfig) => agentDir ?? resolveDefaultAgentDir(config ?? {}); @@ -140,15 +142,6 @@ export function upsertApiKeyProfile(params: { return profileId; } -async function upsertAuthProfileWithLockOrThrow(params: UpsertAuthProfileParams): Promise { - const updated = await upsertAuthProfileWithLock(params); - if (!updated) { - throw new Error( - "Failed to update auth profile store; the auth store lock may be busy. Wait a moment and retry.", - ); - } -} - export function applyAuthProfileConfig( cfg: OpenClawConfig, params: {