diff --git a/.github/workflows/qa-live-transports-convex.yml b/.github/workflows/qa-live-transports-convex.yml index e57933866fee..ed95de2ccde2 100644 --- a/.github/workflows/qa-live-transports-convex.yml +++ b/.github/workflows/qa-live-transports-convex.yml @@ -818,6 +818,7 @@ jobs: OPENCLAW_QA_CONVEX_SECRET_CI: ${{ secrets.OPENCLAW_QA_CONVEX_SECRET_CI }} OPENCLAW_QA_REDACT_PUBLIC_METADATA: "1" OPENCLAW_QA_SLACK_CAPTURE_CONTENT: "1" + OPENCLAW_QA_TRANSPORT_READY_TIMEOUT_MS: "180000" INPUT_SCENARIO: ${{ github.event_name == 'workflow_dispatch' && inputs.slack_scenario || '' }} run: | set -euo pipefail diff --git a/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.test.ts b/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.test.ts index fbdee62fee0e..3fa0a42d2a26 100644 --- a/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.test.ts +++ b/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.test.ts @@ -256,6 +256,20 @@ describe("Slack live QA runtime helpers", () => { ).toBe(3_500); }); + it("resolves Slack readiness timeout from the shared transport env", () => { + expect(testing.resolveSlackQaReadyTimeoutMs({})).toBe(45_000); + expect( + testing.resolveSlackQaReadyTimeoutMs({ + OPENCLAW_QA_TRANSPORT_READY_TIMEOUT_MS: "180000", + }), + ).toBe(180_000); + expect( + testing.resolveSlackQaReadyTimeoutMs({ + OPENCLAW_QA_TRANSPORT_READY_TIMEOUT_MS: "bad", + }), + ).toBe(45_000); + }); + it("allows live approval resolve RPCs to take longer than the generic gateway probe timeout", async () => { const call = vi.fn(async () => ({ decision: "allow-once" })); diff --git a/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.ts b/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.ts index e0b0fea13e2d..3cdcd5a1004a 100644 --- a/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.ts +++ b/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.ts @@ -49,7 +49,7 @@ type SlackChannelStatus = { type SlackChannelReadinessMode = "connected" | "started"; -const SLACK_QA_READY_TIMEOUT_MS = 45_000; +const SLACK_QA_DEFAULT_READY_TIMEOUT_MS = 45_000; const SLACK_QA_READY_STABILITY_MS = 3_000; const SLACK_QA_GATEWAY_STOP_SETTLE_MS = 3_000; const SLACK_QA_RETRYABLE_SCENARIO_ATTEMPTS = 2; @@ -1540,8 +1540,9 @@ async function waitForSlackChannelRunning( mode: SlackChannelReadinessMode, ): Promise { const startedAt = Date.now(); + const timeoutMs = resolveSlackQaReadyTimeoutMs(); let lastStatus: SlackChannelStatus | undefined; - while (Date.now() - startedAt < SLACK_QA_READY_TIMEOUT_MS) { + while (Date.now() - startedAt < timeoutMs) { try { const payload = (await gateway.call( "channels.status", @@ -1598,8 +1599,9 @@ async function waitForSlackChannelStable( mode: SlackChannelReadinessMode, ) { const startedAt = Date.now(); + const timeoutMs = resolveSlackQaReadyTimeoutMs(); let readySince: number | undefined; - while (Date.now() - startedAt < SLACK_QA_READY_TIMEOUT_MS) { + while (Date.now() - startedAt < timeoutMs) { const status = await waitForSlackChannelRunning(gateway, accountId, mode); const observedAt = Date.now(); readySince = resolveSlackChannelReadySince({ @@ -1646,6 +1648,14 @@ function resolveSlackChannelReadySince(params: { return params.previousReadySince ?? params.observedAt; } +function resolveSlackQaReadyTimeoutMs(env: NodeJS.ProcessEnv = process.env) { + const raw = env.OPENCLAW_QA_TRANSPORT_READY_TIMEOUT_MS; + if (!raw) { + return SLACK_QA_DEFAULT_READY_TIMEOUT_MS; + } + return parseStrictPositiveInteger(raw) ?? SLACK_QA_DEFAULT_READY_TIMEOUT_MS; +} + function isRetryableSlackQaScenarioError(error: unknown) { return /timed out after \d+ms waiting for Slack message/iu.test(formatErrorMessage(error)); } @@ -2167,6 +2177,7 @@ export const testing = { parseSlackQaCredentialPayload, preserveSlackGatewayDebugArtifacts, resolveSlackChannelReadySince, + resolveSlackQaReadyTimeoutMs, resolveSlackApprovalCheckpointConfig, resolveApprovalDecision, resolveSlackQaRuntimeEnv, diff --git a/extensions/slack/src/account-configured.ts b/extensions/slack/src/account-configured.ts new file mode 100644 index 000000000000..da7dd0e17c97 --- /dev/null +++ b/extensions/slack/src/account-configured.ts @@ -0,0 +1,14 @@ +import { hasConfiguredAccountValue } from "openclaw/plugin-sdk/account-resolution"; +import type { ResolvedSlackAccount } from "./accounts.js"; + +export function isSlackPluginAccountConfigured(account: ResolvedSlackAccount): boolean { + const mode = account.config.mode ?? "socket"; + const hasBotToken = Boolean(account.botToken?.trim()); + if (!hasBotToken) { + return false; + } + if (mode === "http") { + return hasConfiguredAccountValue(account.config.signingSecret); + } + return Boolean(account.appToken?.trim()); +} diff --git a/extensions/slack/src/approval-auth.test.ts b/extensions/slack/src/approval-auth.test.ts index 758f888b2dc3..4fcb4b67e56a 100644 --- a/extensions/slack/src/approval-auth.test.ts +++ b/extensions/slack/src/approval-auth.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { slackApprovalAuth } from "./approval-auth.js"; +import { isSlackApprovalAuthorizedSender, slackApprovalAuth } from "./approval-auth.js"; describe("slackApprovalAuth", () => { it("authorizes general Slack approvers from allowFrom and defaultTo", () => { @@ -96,4 +96,29 @@ describe("slackApprovalAuth", () => { ).toEqual({ authorized: true }); } }); + + it("allows same-chat plugin approval when no concrete Slack approvers are configured", () => { + const cfg = { + channels: { + slack: { + allowFrom: ["*"], + }, + }, + }; + + expect( + slackApprovalAuth.authorizeActorAction({ + cfg, + senderId: "U123OWNER", + action: "approve", + approvalKind: "plugin", + }), + ).toEqual({ authorized: true }); + expect( + isSlackApprovalAuthorizedSender({ + cfg, + senderId: "U123OWNER", + }), + ).toBe(true); + }); }); diff --git a/extensions/slack/src/approval-auth.ts b/extensions/slack/src/approval-auth.ts index 1c7a0b224021..e171813371d7 100644 --- a/extensions/slack/src/approval-auth.ts +++ b/extensions/slack/src/approval-auth.ts @@ -28,7 +28,11 @@ export function isSlackApprovalAuthorizedSender(params: { if (!senderId) { return false; } - return getSlackApprovalApprovers(params).includes(senderId); + const approvers = getSlackApprovalApprovers(params); + if (approvers.length > 0) { + return approvers.includes(senderId); + } + return (resolveSlackAccountAllowFrom(params) ?? []).some((entry) => entry.trim() === "*"); } export const slackApprovalAuth = createResolvedApproverActionAuthAdapter({ diff --git a/extensions/slack/src/approval-native-gates.ts b/extensions/slack/src/approval-native-gates.ts index dfec9ef52f37..e4c3cb46532c 100644 --- a/extensions/slack/src/approval-native-gates.ts +++ b/extensions/slack/src/approval-native-gates.ts @@ -3,6 +3,7 @@ import { matchesApprovalRequestFilters, } from "openclaw/plugin-sdk/approval-client-runtime"; import { + createNativeApprovalChannelRouteGates, doesApprovalRequestMatchChannelAccount, resolveApprovalRequestSessionConversation, } from "openclaw/plugin-sdk/approval-native-runtime"; @@ -10,23 +11,61 @@ import type { ExecApprovalRequest, PluginApprovalRequest, } from "openclaw/plugin-sdk/approval-runtime"; +import type { ChannelApprovalCapability } from "openclaw/plugin-sdk/channel-contract"; +import { + channelRouteTargetsMatchExact, + stringifyRouteThreadId, +} from "openclaw/plugin-sdk/channel-route"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { normalizeMessageChannel } from "openclaw/plugin-sdk/routing"; -import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; -import { resolveSlackAccount } from "./accounts.js"; +import { + normalizeLowercaseStringOrEmpty, + normalizeOptionalString, +} from "openclaw/plugin-sdk/string-coerce-runtime"; +import { isSlackPluginAccountConfigured } from "./account-configured.js"; +import { + listSlackAccountIds, + resolveDefaultSlackAccountId, + resolveSlackAccount, +} from "./accounts.js"; import { getSlackApprovalApprovers } from "./approval-auth.js"; import { getSlackExecApprovalApprovers, isSlackExecApprovalClientEnabled, } from "./exec-approvals.js"; +import { parseSlackTarget } from "./targets.js"; export type SlackApprovalKind = "exec" | "plugin"; export type SlackNativeApprovalRequest = ExecApprovalRequest | PluginApprovalRequest; +export type SlackOriginTarget = { + to: string; + accountId?: string | null; + threadId?: string | number | null; +}; + +type ApprovalForwardingConfig = NonNullable["plugin"]>; +type ApprovalForwardingMode = NonNullable; +type SlackForwardTarget = Parameters< + NonNullable< + NonNullable["shouldSuppressForwardingFallback"] + > +>[0]["target"]; + +const DEFAULT_APPROVAL_FORWARDING_MODE: ApprovalForwardingMode = "session"; +const SLACK_DM_CHANNEL_ID_RE = /^D[A-Z0-9]{8,}$/i; +const SLACK_USER_ID_RE = /^[UW][A-Z0-9]{8,}$/i; export function resolveSlackApprovalKind(request: SlackNativeApprovalRequest): SlackApprovalKind { return request.id.startsWith("plugin:") ? "plugin" : "exec"; } +function isSlackApprovalTransportEnabled(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): boolean { + return isSlackPluginAccountConfigured(resolveSlackAccount(params)); +} + function resolveSlackNativeApprovalConfig(params: { cfg: OpenClawConfig; accountId?: string | null; @@ -38,141 +77,212 @@ function resolvePluginApprovalForwardingConfig(cfg: OpenClawConfig) { return cfg.approvals?.plugin; } -function getSlackNativeApprovalApprovers(params: { - cfg: OpenClawConfig; - accountId?: string | null; - approvalKind: SlackApprovalKind; -}): string[] { - return params.approvalKind === "plugin" - ? getSlackApprovalApprovers(params) - : getSlackExecApprovalApprovers(params); +function normalizeSlackThreadMatchKey(threadId?: string | number | null): string { + return threadId == null ? "" : String(threadId).trim(); } -function normalizeAccountId(value?: string | null): string | undefined { - return normalizeOptionalString(value)?.toLowerCase(); +function normalizeComparableTarget(value: string): string { + return normalizeLowercaseStringOrEmpty(value); } -function matchesSlackAccount(params: { - expectedAccountId?: string | null; - actualAccountId?: string | null; -}): boolean { - const expected = normalizeAccountId(params.expectedAccountId); - const actual = normalizeAccountId(params.actualAccountId); - return !expected || !actual || expected === actual; +function extractSlackSessionKind( + sessionKey?: string | null, +): "direct" | "channel" | "group" | null { + if (!sessionKey) { + return null; + } + const match = sessionKey.match(/slack:(direct|channel|group):/i); + const kind = normalizeLowercaseStringOrEmpty(match?.[1]); + return kind ? (kind as "direct" | "channel" | "group") : null; } -function modeIncludesSession(mode: "session" | "targets" | "both" | undefined): boolean { - return mode === undefined || mode === "session" || mode === "both"; +function resolveSlackTurnSourceDefaultKind(params: { + turnSourceTo: string; + sessionKind: "direct" | "channel" | "group" | null; +}): "user" | "channel" { + // Slack app conversations arrive as the concrete D-channel plus the app + // thread root, so keep that live target instead of rewriting it to a user id. + if (SLACK_DM_CHANNEL_ID_RE.test(params.turnSourceTo)) { + return "channel"; + } + return params.sessionKind === "direct" ? "user" : "channel"; } -function modeIncludesTargets(mode: "session" | "targets" | "both" | undefined): boolean { - return mode === "targets" || mode === "both"; -} - -function hasSlackPluginForwardingTarget(params: { - cfg: OpenClawConfig; - accountId?: string | null; -}): boolean { - const targets = resolvePluginApprovalForwardingConfig(params.cfg)?.targets ?? []; - return targets.some((target) => { - const channel = normalizeMessageChannel(target.channel) ?? target.channel; - return ( - channel === "slack" && - matchesSlackAccount({ - expectedAccountId: params.accountId, - actualAccountId: target.accountId, - }) - ); +export function resolveTurnSourceSlackOriginTarget( + request: SlackNativeApprovalRequest, +): SlackOriginTarget | null { + const turnSourceChannel = normalizeLowercaseStringOrEmpty(request.request.turnSourceChannel); + const turnSourceTo = normalizeOptionalString(request.request.turnSourceTo) ?? ""; + if (turnSourceChannel !== "slack" || !turnSourceTo) { + return null; + } + const sessionKind = extractSlackSessionKind(request.request.sessionKey ?? undefined); + const parsed = parseSlackTarget(turnSourceTo, { + defaultKind: resolveSlackTurnSourceDefaultKind({ turnSourceTo, sessionKind }), }); + if (!parsed) { + return null; + } + return { + to: `${parsed.kind}:${parsed.id}`, + threadId: stringifyRouteThreadId(request.request.turnSourceThreadId), + }; } -function requestHasSlackOriginOrSession(params: { - cfg: OpenClawConfig; - request: SlackNativeApprovalRequest; - accountId?: string | null; -}): boolean { - const request = params.request.request; - const turnSourceChannel = normalizeMessageChannel(request.turnSourceChannel); - if (turnSourceChannel) { - return ( - turnSourceChannel === "slack" && - matchesSlackAccount({ - expectedAccountId: params.accountId, - actualAccountId: request.turnSourceAccountId, - }) - ); +export function resolveSessionSlackOriginTarget(sessionTarget: { + to: string; + threadId?: string | number | null; +}): SlackOriginTarget { + return { + to: sessionTarget.to, + threadId: stringifyRouteThreadId(sessionTarget.threadId), + }; +} + +export function resolveSlackFallbackOriginTarget( + request: SlackNativeApprovalRequest, +): SlackOriginTarget | null { + const sessionTarget = resolveApprovalRequestSessionConversation({ + request, + channel: "slack", + bundledFallback: false, + }); + if (!sessionTarget) { + return null; + } + const parsed = parseSlackTarget(sessionTarget.id.toUpperCase(), { + defaultKind: "channel", + }); + if (!parsed) { + return null; + } + return { + to: `${parsed.kind}:${parsed.id}`, + threadId: sessionTarget.threadId, + }; +} + +export function normalizeSlackOriginTarget(target: SlackOriginTarget): SlackOriginTarget { + return { + ...target, + to: normalizeComparableTarget(target.to), + }; +} + +function parseComparableSlackTarget(target: SlackOriginTarget) { + return parseSlackTarget(target.to, { defaultKind: "channel" }); +} + +function isSlackDmChannelToUserRoutePair(a: SlackOriginTarget, b: SlackOriginTarget): boolean { + const left = parseComparableSlackTarget(a); + const right = parseComparableSlackTarget(b); + if (!left || !right) { + return false; } return ( - resolveApprovalRequestSessionConversation({ - request: params.request, - channel: "slack", - bundledFallback: false, - }) !== null && - doesApprovalRequestMatchChannelAccount({ - cfg: params.cfg, - request: params.request, - channel: "slack", - accountId: params.accountId, - }) + (left.kind === "channel" && SLACK_DM_CHANNEL_ID_RE.test(left.id) && right.kind === "user") || + (right.kind === "channel" && SLACK_DM_CHANNEL_ID_RE.test(right.id) && left.kind === "user") ); } -function isPluginForwardingEnabledForRequest(params: { - cfg: OpenClawConfig; - request: SlackNativeApprovalRequest; -}): boolean { - const config = resolvePluginApprovalForwardingConfig(params.cfg); - if (!config?.enabled) { +export function slackTargetsMatch(a: SlackOriginTarget, b: SlackOriginTarget): boolean { + const threadKey = normalizeSlackThreadMatchKey(a.threadId); + if (threadKey !== normalizeSlackThreadMatchKey(b.threadId)) { return false; } - return matchesApprovalRequestFilters({ - request: params.request.request, - agentFilter: config.agentFilter, - sessionFilter: config.sessionFilter, - }); -} - -function canPluginForwardingRouteToSlack(params: { - cfg: OpenClawConfig; - accountId?: string | null; - request: SlackNativeApprovalRequest; -}): boolean { - const config = resolvePluginApprovalForwardingConfig(params.cfg); - const mode = config?.mode; if ( - modeIncludesSession(mode) && - requestHasSlackOriginOrSession({ - cfg: params.cfg, - request: params.request, - accountId: params.accountId, + channelRouteTargetsMatchExact({ + left: { + channel: "slack", + to: a.to, + }, + right: { + channel: "slack", + to: b.to, + }, }) ) { return true; } - return modeIncludesTargets(mode) && hasSlackPluginForwardingTarget(params); + return Boolean(threadKey && isSlackDmChannelToUserRoutePair(a, b)); +} + +export function normalizeSlackForwardTarget( + target: Pick, +): SlackOriginTarget | null { + const channel = normalizeMessageChannel(target.channel) ?? target.channel; + if (channel !== "slack") { + return null; + } + const to = normalizeOptionalString(target.to); + if (!to) { + return null; + } + const parsed = parseSlackTarget(to, { + defaultKind: SLACK_USER_ID_RE.test(to) ? "user" : "channel", + }); + if (!parsed) { + return null; + } + return { + to: `${parsed.kind}:${parsed.id}`, + accountId: normalizeOptionalString(target.accountId), + threadId: stringifyRouteThreadId(target.threadId), + }; +} + +const slackApprovalRouteGates = createNativeApprovalChannelRouteGates({ + channel: "slack", + defaultForwardingMode: DEFAULT_APPROVAL_FORWARDING_MODE, + isTransportEnabled: isSlackApprovalTransportEnabled, + listAccountIds: listSlackAccountIds, + resolveDefaultAccountId: resolveDefaultSlackAccountId, + normalizeForwardTarget: normalizeSlackForwardTarget, + resolveTurnSourceTarget: resolveTurnSourceSlackOriginTarget, + targetsMatch: slackTargetsMatch, +}); + +const { + canApprovalPotentiallyRouteToChannel: canApprovalPotentiallyRouteToSlack, + isSessionApprovalEligible: isForwardedSlackSessionApprovalEligible, + isExplicitTargetEligible: isForwardedSlackExplicitTargetEligible, +} = slackApprovalRouteGates; + +export function hasSlackPluginApprovers(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): boolean { + return getSlackApprovalApprovers(params).length > 0; +} + +function isSlackPluginNativeApprovalClientConfigEnabled(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): boolean { + const slackNativeConfig = resolveSlackNativeApprovalConfig(params); + return isChannelExecApprovalClientEnabledFromConfig({ + enabled: slackNativeConfig?.enabled, + approverCount: getSlackApprovalApprovers(params).length, + }); +} + +function isSlackPluginForwardingRoutePotentiallyEnabled(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): boolean { + return canApprovalPotentiallyRouteToSlack({ + ...params, + approvalKind: "plugin", + }); } function isSlackPluginNativeApprovalClientEnabled(params: { cfg: OpenClawConfig; accountId?: string | null; }): boolean { - const slackNativeConfig = resolveSlackNativeApprovalConfig(params); - if ( - isChannelExecApprovalClientEnabledFromConfig({ - enabled: slackNativeConfig?.enabled, - approverCount: getSlackApprovalApprovers(params).length, - }) - ) { - return true; - } - const config = resolvePluginApprovalForwardingConfig(params.cfg); - if (!config?.enabled || getSlackApprovalApprovers(params).length <= 0) { - return false; - } - const mode = config.mode; return ( - modeIncludesSession(mode) || - (modeIncludesTargets(mode) && hasSlackPluginForwardingTarget(params)) + isSlackPluginNativeApprovalClientConfigEnabled(params) || + isSlackPluginForwardingRoutePotentiallyEnabled(params) ); } @@ -200,28 +310,62 @@ function shouldHandleSlackPluginViaNativeClientConfig(params: { ) { return false; } - return matchesApprovalRequestFilters({ - request: params.request.request, + return matchesSlackNativeApprovalFilters({ + request: params.request, agentFilter: config?.agentFilter, sessionFilter: config?.sessionFilter, }); } -function shouldHandleSlackPluginNativeApprovalRequest(params: { +function matchesSlackNativeApprovalFilters(params: { + request: SlackNativeApprovalRequest; + agentFilter?: string[]; + sessionFilter?: string[]; +}): boolean { + return matchesApprovalRequestFilters({ + request: params.request.request, + agentFilter: params.agentFilter, + sessionFilter: params.sessionFilter, + }); +} + +function isAnyForwardedSlackExplicitTargetEligible(params: { cfg: OpenClawConfig; accountId?: string | null; request: SlackNativeApprovalRequest; }): boolean { - if (getSlackApprovalApprovers(params).length <= 0) { - return false; - } - if (shouldHandleSlackPluginViaNativeClientConfig(params)) { - return true; - } - if (!isPluginForwardingEnabledForRequest(params)) { - return false; - } - return canPluginForwardingRouteToSlack(params); + const targets = resolvePluginApprovalForwardingConfig(params.cfg)?.targets ?? []; + return targets.some((target) => + isForwardedSlackExplicitTargetEligible({ + ...params, + approvalKind: "plugin", + target, + }), + ); +} + +function shouldHandleSlackPluginViaForwarding(params: { + cfg: OpenClawConfig; + accountId?: string | null; + request: SlackNativeApprovalRequest; +}): boolean { + return ( + isForwardedSlackSessionApprovalEligible({ + ...params, + approvalKind: "plugin", + }) || isAnyForwardedSlackExplicitTargetEligible(params) + ); +} + +export function shouldHandleSlackPluginViaForwardingSession(params: { + cfg: OpenClawConfig; + accountId?: string | null; + request: SlackNativeApprovalRequest; +}): boolean { + return isForwardedSlackSessionApprovalEligible({ + ...params, + approvalKind: "plugin", + }); } export function isSlackNativeApprovalClientEnabled(params: { @@ -259,11 +403,10 @@ export function shouldHandleSlackNativeApprovalRequest(params: { }): boolean { const approvalKind = params.approvalKind ?? resolveSlackApprovalKind(params.request); if (approvalKind === "plugin") { - return shouldHandleSlackPluginNativeApprovalRequest({ - cfg: params.cfg, - accountId: params.accountId, - request: params.request, - }); + return ( + shouldHandleSlackPluginViaNativeClientConfig(params) || + shouldHandleSlackPluginViaForwarding(params) + ); } if ( !doesApprovalRequestMatchChannelAccount({ @@ -279,16 +422,13 @@ export function shouldHandleSlackNativeApprovalRequest(params: { if ( !isChannelExecApprovalClientEnabledFromConfig({ enabled: config?.enabled, - approverCount: getSlackNativeApprovalApprovers({ - ...params, - approvalKind, - }).length, + approverCount: getSlackExecApprovalApprovers(params).length, }) ) { return false; } - return matchesApprovalRequestFilters({ - request: params.request.request, + return matchesSlackNativeApprovalFilters({ + request: params.request, agentFilter: config?.agentFilter, sessionFilter: config?.sessionFilter, }); diff --git a/extensions/slack/src/approval-native.test.ts b/extensions/slack/src/approval-native.test.ts index 6540f55f7d02..c31066788adb 100644 --- a/extensions/slack/src/approval-native.test.ts +++ b/extensions/slack/src/approval-native.test.ts @@ -323,7 +323,7 @@ describe("slack native approval adapter", () => { targets: [{ channel: "slack", to: "U123OWNER" }], }, }, - } as OpenClawConfig; + } as unknown as OpenClawConfig; const request = { id: "plugin:req-1", request: { @@ -375,6 +375,251 @@ describe("slack native approval adapter", () => { ).toBe(true); }); + it("delivers plugin forwarding session approvals to the Slack origin without concrete approvers", async () => { + const cfg = { + ...buildConfig({ + allowFrom: ["*"], + execApprovals: { + enabled: false, + approvers: ["U999EXEC"], + target: "dm", + }, + }), + approvals: { + plugin: { + enabled: true, + mode: "session", + sessionFilter: ["slack:"], + }, + }, + } as unknown as OpenClawConfig; + const request = { + id: "plugin:req-open-session", + request: { + title: "Plugin approval", + description: "Allow access", + sessionKey: "slack:D123APPROVALS:test-run", + turnSourceChannel: "slack", + turnSourceTo: "channel:D123APPROVALS", + turnSourceAccountId: "default", + }, + createdAtMs: 0, + expiresAtMs: 1_000, + }; + + expect( + slackApprovalCapability.nativeRuntime?.availability.isConfigured({ + cfg, + accountId: "default", + }), + ).toBe(true); + expect( + slackApprovalCapability.nativeRuntime?.availability.shouldHandle({ + cfg, + accountId: "default", + request, + }), + ).toBe(true); + expect( + slackNativeApprovalAdapter.native?.describeDeliveryCapabilities({ + cfg, + accountId: "default", + approvalKind: "plugin", + request, + }), + ).toEqual({ + enabled: true, + preferredSurface: "origin", + supportsOriginSurface: true, + supportsApproverDmSurface: false, + notifyOriginWhenDmOnly: true, + }); + expect( + await slackNativeApprovalAdapter.native?.resolveOriginTarget?.({ + cfg, + accountId: "default", + approvalKind: "plugin", + request, + }), + ).toEqual({ + to: "channel:D123APPROVALS", + threadId: undefined, + }); + }); + + it("requires Slack socket transport readiness before plugin forwarding enables native delivery", async () => { + const cfg = { + channels: { + slack: { + defaultAccount: "work", + accounts: { + work: { + botToken: "xoxb-work", + allowFrom: ["U123OWNER"], + execApprovals: { + enabled: false, + target: "both", + }, + }, + }, + }, + }, + approvals: { + plugin: { + enabled: true, + mode: "targets", + targets: [{ channel: "slack", accountId: "work", to: "user:U123OWNER" }], + }, + }, + } as unknown as OpenClawConfig; + const request = { + id: "plugin:req-transport", + request: { + title: "Plugin approval", + description: "Allow access", + }, + createdAtMs: 0, + expiresAtMs: 1000, + }; + + expect( + slackApprovalCapability.nativeRuntime?.availability.isConfigured({ + cfg, + accountId: "work", + }), + ).toBe(false); + expect( + slackApprovalCapability.nativeRuntime?.availability.shouldHandle({ + cfg, + accountId: "work", + request, + }), + ).toBe(false); + expect( + slackNativeApprovalAdapter.native?.describeDeliveryCapabilities({ + cfg, + accountId: "work", + approvalKind: "plugin", + request, + }).enabled, + ).toBe(false); + }); + + it("treats HTTP signing secret configuration as Slack transport readiness", async () => { + const cfg = { + channels: { + slack: { + defaultAccount: "work", + accounts: { + work: { + mode: "http", + botToken: "xoxb-work", + signingSecret: "signing-secret", + allowFrom: ["U123OWNER"], + execApprovals: { + enabled: false, + target: "both", + }, + }, + }, + }, + }, + approvals: { + plugin: { + enabled: true, + mode: "targets", + targets: [{ channel: "slack", accountId: "work", to: "user:U123OWNER" }], + }, + }, + } as OpenClawConfig; + const request = { + id: "plugin:req-http", + request: { + title: "Plugin approval", + description: "Allow access", + }, + createdAtMs: 0, + expiresAtMs: 1000, + }; + + expect( + slackApprovalCapability.nativeRuntime?.availability.isConfigured({ + cfg, + accountId: "work", + }), + ).toBe(true); + expect( + slackApprovalCapability.nativeRuntime?.availability.shouldHandle({ + cfg, + accountId: "work", + request, + }), + ).toBe(true); + expect( + slackNativeApprovalAdapter.native?.describeDeliveryCapabilities({ + cfg, + accountId: "work", + approvalKind: "plugin", + request, + }).enabled, + ).toBe(true); + }); + + it("treats HTTP signing secret SecretRefs as Slack transport readiness", async () => { + const cfg = { + channels: { + slack: { + defaultAccount: "work", + accounts: { + work: { + mode: "http", + botToken: "xoxb-work", + signingSecret: { + source: "env", + id: "SLACK_SIGNING_SECRET", + }, + allowFrom: ["U123OWNER"], + execApprovals: { + enabled: false, + target: "both", + }, + }, + }, + }, + }, + approvals: { + plugin: { + enabled: true, + mode: "targets", + targets: [{ channel: "slack", accountId: "work", to: "user:U123OWNER" }], + }, + }, + } as unknown as OpenClawConfig; + const request = { + id: "plugin:req-http-secret-ref", + request: { + title: "Plugin approval", + description: "Allow access", + }, + createdAtMs: 0, + expiresAtMs: 1000, + }; + + expect( + slackApprovalCapability.nativeRuntime?.availability.isConfigured({ + cfg, + accountId: "work", + }), + ).toBe(true); + expect( + slackApprovalCapability.nativeRuntime?.availability.shouldHandle({ + cfg, + accountId: "work", + request, + }), + ).toBe(true); + }); + it("does not route plugin session fallback across Slack accounts", async () => { writeStore({ "agent:main:slack:channel:c999": { diff --git a/extensions/slack/src/approval-native.ts b/extensions/slack/src/approval-native.ts index b8d2799429b0..c1a0164bc7e0 100644 --- a/extensions/slack/src/approval-native.ts +++ b/extensions/slack/src/approval-native.ts @@ -6,26 +6,28 @@ import { createLazyChannelApprovalNativeRuntimeAdapter } from "openclaw/plugin-s import type { ChannelApprovalNativeRuntimeAdapter } from "openclaw/plugin-sdk/approval-handler-runtime"; import { createChannelNativeOriginTargetResolver, - resolveApprovalRequestSessionConversation, + createNativeApprovalForwardingFallbackSuppressor, } from "openclaw/plugin-sdk/approval-native-runtime"; import type { ChannelApprovalCapability } from "openclaw/plugin-sdk/channel-contract"; -import { - channelRouteTargetsMatchExact, - stringifyRouteThreadId, -} from "openclaw/plugin-sdk/channel-route"; import { normalizeMessageChannel } from "openclaw/plugin-sdk/routing"; -import { - normalizeLowercaseStringOrEmpty, - normalizeOptionalString, -} from "openclaw/plugin-sdk/string-coerce-runtime"; +import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; import { listSlackAccountIds } from "./accounts.js"; import { getSlackApprovalApprovers, isSlackApprovalAuthorizedSender } from "./approval-auth.js"; import { + hasSlackPluginApprovers, isSlackAnyNativeApprovalClientEnabled, + normalizeSlackForwardTarget, + normalizeSlackOriginTarget, + resolveSessionSlackOriginTarget, resolveSlackApprovalKind, + resolveSlackFallbackOriginTarget, + resolveTurnSourceSlackOriginTarget, shouldHandleSlackNativeApprovalRequest, + shouldHandleSlackPluginViaForwardingSession, + slackTargetsMatch, type SlackApprovalKind, type SlackNativeApprovalRequest, + type SlackOriginTarget, } from "./approval-native-gates.js"; import { getSlackExecApprovalApprovers, @@ -33,11 +35,9 @@ import { isSlackExecApprovalClientEnabled, resolveSlackExecApprovalTarget, } from "./exec-approvals.js"; -import { parseSlackTarget } from "./targets.js"; type ApprovalRequest = SlackNativeApprovalRequest; type ApprovalKind = SlackApprovalKind; -type SlackOriginTarget = { to: string; threadId?: string }; type SlackSuppressionAccountInput = { target: { channel: string; accountId?: string | null }; request: { @@ -47,142 +47,6 @@ type SlackSuppressionAccountInput = { }; }; }; -type SlackForwardingSuppressionInput = Parameters< - NonNullable< - NonNullable["shouldSuppressForwardingFallback"] - > ->[0]; - -const SLACK_DM_CHANNEL_ID_RE = /^D[A-Z0-9]{8,}$/i; -const SLACK_USER_ID_RE = /^[UW][A-Z0-9]{8,}$/i; - -function extractSlackSessionKind( - sessionKey?: string | null, -): "direct" | "channel" | "group" | null { - if (!sessionKey) { - return null; - } - const match = sessionKey.match(/slack:(direct|channel|group):/i); - const kind = normalizeLowercaseStringOrEmpty(match?.[1]); - return kind ? (kind as "direct" | "channel" | "group") : null; -} - -function normalizeComparableTarget(value: string): string { - return normalizeLowercaseStringOrEmpty(value); -} - -function normalizeSlackThreadMatchKey(threadId?: string): string { - return threadId?.trim() ?? ""; -} - -function resolveSlackTurnSourceDefaultKind(params: { - turnSourceTo: string; - sessionKind: "direct" | "channel" | "group" | null; -}): "user" | "channel" { - // Slack app conversations arrive at Codex as the concrete D-channel plus the - // app thread root. That live channel target must not be reinterpreted as a - // user id just because the backing session is direct-message shaped. - if (SLACK_DM_CHANNEL_ID_RE.test(params.turnSourceTo)) { - return "channel"; - } - return params.sessionKind === "direct" ? "user" : "channel"; -} - -function resolveTurnSourceSlackOriginTarget(request: ApprovalRequest): SlackOriginTarget | null { - const turnSourceChannel = normalizeLowercaseStringOrEmpty(request.request.turnSourceChannel); - const turnSourceTo = normalizeOptionalString(request.request.turnSourceTo) ?? ""; - if (turnSourceChannel !== "slack" || !turnSourceTo) { - return null; - } - const sessionKind = extractSlackSessionKind(request.request.sessionKey ?? undefined); - const parsed = parseSlackTarget(turnSourceTo, { - defaultKind: resolveSlackTurnSourceDefaultKind({ turnSourceTo, sessionKind }), - }); - if (!parsed) { - return null; - } - const threadId = stringifyRouteThreadId(request.request.turnSourceThreadId); - return { - to: `${parsed.kind}:${parsed.id}`, - threadId, - }; -} - -function resolveSessionSlackOriginTarget(sessionTarget: { - to: string; - threadId?: string | number | null; -}): SlackOriginTarget { - return { - to: sessionTarget.to, - threadId: stringifyRouteThreadId(sessionTarget.threadId), - }; -} - -function resolveSlackFallbackOriginTarget(request: ApprovalRequest): SlackOriginTarget | null { - const sessionTarget = resolveApprovalRequestSessionConversation({ - request, - channel: "slack", - bundledFallback: false, - }); - if (!sessionTarget) { - return null; - } - const parsed = parseSlackTarget(sessionTarget.id.toUpperCase(), { - defaultKind: "channel", - }); - if (!parsed) { - return null; - } - return { - to: `${parsed.kind}:${parsed.id}`, - threadId: sessionTarget.threadId, - }; -} - -function normalizeSlackOriginTarget(target: SlackOriginTarget): SlackOriginTarget { - return { - ...target, - to: normalizeComparableTarget(target.to), - }; -} - -function parseComparableSlackTarget(target: SlackOriginTarget) { - return parseSlackTarget(target.to, { defaultKind: "channel" }); -} - -function isSlackDmChannelToUserRoutePair(a: SlackOriginTarget, b: SlackOriginTarget): boolean { - const left = parseComparableSlackTarget(a); - const right = parseComparableSlackTarget(b); - if (!left || !right) { - return false; - } - return ( - (left.kind === "channel" && SLACK_DM_CHANNEL_ID_RE.test(left.id) && right.kind === "user") || - (right.kind === "channel" && SLACK_DM_CHANNEL_ID_RE.test(right.id) && left.kind === "user") - ); -} - -function slackTargetsMatch(a: SlackOriginTarget, b: SlackOriginTarget): boolean { - const threadKey = normalizeSlackThreadMatchKey(a.threadId); - if (threadKey !== normalizeSlackThreadMatchKey(b.threadId)) { - return false; - } - if ( - channelRouteTargetsMatchExact({ - left: { - channel: "slack", - to: a.to, - }, - right: { - channel: "slack", - to: b.to, - }, - }) - ) { - return true; - } - return Boolean(threadKey && isSlackDmChannelToUserRoutePair(a, b)); -} function resolveSlackNativeSuppressionAccountId({ target, @@ -208,50 +72,6 @@ function shouldConsiderSlackNativeForwardingSuppression( return turnSourceChannel === "slack"; } -function resolveForwardingFallbackSlackTarget( - target: SlackForwardingSuppressionInput["target"], -): SlackOriginTarget | null { - const to = normalizeOptionalString(target.to); - if (!to) { - return null; - } - const parsed = parseSlackTarget(to, { - defaultKind: SLACK_USER_ID_RE.test(to) ? "user" : "channel", - }); - if (!parsed) { - return null; - } - return { - to: `${parsed.kind}:${parsed.id}`, - threadId: stringifyRouteThreadId(target.threadId), - }; -} - -function isSlackPluginForwardingFallbackHandledNatively( - input: SlackForwardingSuppressionInput, -): boolean { - const forwardingTarget = resolveForwardingFallbackSlackTarget(input.target); - if (!forwardingTarget) { - return false; - } - const request = input.request; - const originTarget = resolveSlackOriginTarget({ - cfg: input.cfg, - accountId: resolveSlackNativeSuppressionAccountId(input), - approvalKind: input.approvalKind, - request, - }); - if (originTarget && slackTargetsMatch(forwardingTarget, originTarget)) { - return true; - } - return resolveSlackApproverDmTargets({ - cfg: input.cfg, - accountId: resolveSlackNativeSuppressionAccountId(input), - approvalKind: input.approvalKind, - request, - }).some((target) => slackTargetsMatch(forwardingTarget, target)); -} - const resolveSlackOriginTarget = createChannelNativeOriginTargetResolver({ channel: "slack", shouldHandleRequest: ({ cfg, accountId, request }) => @@ -290,6 +110,19 @@ function resolveSlackApproverDmTargets(params: { return approvers.map((approver) => ({ to: `user:${approver}` })); } +const shouldSuppressSlackForwardingFallback = + createNativeApprovalForwardingFallbackSuppressor({ + channel: "slack", + normalizeForwardTarget: normalizeSlackForwardTarget, + resolveAccountId: ({ target, request }) => + resolveSlackNativeSuppressionAccountId({ target, request }), + isSessionRouteEligible: shouldHandleSlackNativeApprovalRequest, + isExplicitTargetEligible: shouldHandleSlackNativeApprovalRequest, + resolveOriginTarget: resolveSlackOriginTarget, + resolveApproverDmTargets: resolveSlackApproverDmTargets, + targetsMatch: slackTargetsMatch, + }); + const baseSlackApprovalCapability = createApproverRestrictedNativeApprovalCapability({ channel: "slack", channelLabel: "Slack", @@ -357,7 +190,7 @@ export const slackApprovalCapability: ChannelApprovalCapability = { if (!canHandleNative || input.approvalKind !== "plugin") { return canHandleNative; } - return isSlackPluginForwardingFallbackHandledNatively(input); + return shouldSuppressSlackForwardingFallback(input); }, }, native: baseSlackNativeAdapter @@ -365,14 +198,30 @@ export const slackApprovalCapability: ChannelApprovalCapability = { ...baseSlackNativeAdapter, describeDeliveryCapabilities: (params) => { const capabilities = baseSlackNativeAdapter.describeDeliveryCapabilities(params); + const request = params.request as ApprovalRequest; + const approvalKind = params.approvalKind; return { ...capabilities, enabled: shouldHandleSlackNativeApprovalRequest({ cfg: params.cfg, accountId: params.accountId, - approvalKind: params.approvalKind, - request: params.request as ApprovalRequest, + approvalKind, + request, }), + ...(approvalKind === "plugin" && + shouldHandleSlackPluginViaForwardingSession({ + cfg: params.cfg, + accountId: params.accountId, + request, + }) + ? { + preferredSurface: "origin" as const, + supportsApproverDmSurface: hasSlackPluginApprovers({ + cfg: params.cfg, + accountId: params.accountId, + }), + } + : {}), }; }, } diff --git a/extensions/slack/src/monitor/provider.allowlist.test.ts b/extensions/slack/src/monitor/provider.allowlist.test.ts index 34375aba3ec4..837b7bfe9620 100644 --- a/extensions/slack/src/monitor/provider.allowlist.test.ts +++ b/extensions/slack/src/monitor/provider.allowlist.test.ts @@ -73,6 +73,8 @@ describe("slack startup user allowlist resolution", () => { channels: { slack: { enabled: true, + botToken: "xoxb-test", + appToken: "xapp-test", allowFrom: ["U123OWNER"], execApprovals: { enabled: false, diff --git a/extensions/slack/src/shared.ts b/extensions/slack/src/shared.ts index 8d955427dd70..8ca9f42791bc 100644 --- a/extensions/slack/src/shared.ts +++ b/extensions/slack/src/shared.ts @@ -4,6 +4,7 @@ import { adaptScopedAccountAccessor, createScopedChannelConfigAdapter, } from "openclaw/plugin-sdk/channel-config-helpers"; +import { isSlackPluginAccountConfigured } from "./account-configured.js"; import { inspectSlackAccount } from "./account-inspect.js"; import { listSlackAccountIds, @@ -23,17 +24,7 @@ import { SLACK_CHANNEL } from "./setup-shared.js"; export { setSlackChannelAllowlist, SLACK_CHANNEL } from "./setup-shared.js"; -export function isSlackPluginAccountConfigured(account: ResolvedSlackAccount): boolean { - const mode = account.config.mode ?? "socket"; - const hasBotToken = Boolean(account.botToken?.trim()); - if (!hasBotToken) { - return false; - } - if (mode === "http") { - return Boolean(account.config.signingSecret?.trim()); - } - return Boolean(account.appToken?.trim()); -} +export { isSlackPluginAccountConfigured }; export const slackConfigAdapter = createScopedChannelConfigAdapter< ResolvedSlackAccount,