From 15664db855f2163de26e01fab5880e437b65ba8b Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 21 Aug 2026 07:56:38 -0700 Subject: [PATCH] refactor(approvals): unify native client setup (#127189) --- src/infra/exec-approval-surface.test.ts | 54 +++++++++++++++++++++++++ src/infra/exec-approval-surface.ts | 51 +++++++++++------------ 2 files changed, 77 insertions(+), 28 deletions(-) diff --git a/src/infra/exec-approval-surface.test.ts b/src/infra/exec-approval-surface.test.ts index a0f78af97a94..10f37e3f855c 100644 --- a/src/infra/exec-approval-surface.test.ts +++ b/src/infra/exec-approval-surface.test.ts @@ -38,10 +38,14 @@ type ExecApprovalSurfaceModule = typeof import("./exec-approval-surface.js"); let resolveExecApprovalInitiatingSurfaceState: ExecApprovalSurfaceModule["resolveExecApprovalInitiatingSurfaceState"]; let resolveApprovalInitiatingSurfaceState: ExecApprovalSurfaceModule["resolveApprovalInitiatingSurfaceState"]; let supportsNativeExecApprovalClient: ExecApprovalSurfaceModule["supportsNativeExecApprovalClient"]; +let describeNativeExecApprovalClientSetup: ExecApprovalSurfaceModule["describeNativeExecApprovalClientSetup"]; +let describeNativePluginApprovalClientSetup: ExecApprovalSurfaceModule["describeNativePluginApprovalClientSetup"]; describe("resolveExecApprovalInitiatingSurfaceState", () => { beforeAll(async () => { ({ + describeNativeExecApprovalClientSetup, + describeNativePluginApprovalClientSetup, resolveApprovalInitiatingSurfaceState, resolveExecApprovalInitiatingSurfaceState, supportsNativeExecApprovalClient, @@ -318,4 +322,54 @@ describe("resolveExecApprovalInitiatingSurfaceState", () => { expect(supportsNativeExecApprovalClient("matrix")).toBe(true); }); + + it("routes normalized setup parameters to the selected approval hook", () => { + const describeExecApprovalSetup = vi.fn(() => "exec setup"); + const describePluginApprovalSetup = vi.fn(() => "plugin setup"); + getChannelPluginMock.mockReturnValue({ + meta: { label: "Telegram" }, + approvalCapability: { describeExecApprovalSetup, describePluginApprovalSetup }, + }); + + expect( + describeNativeExecApprovalClientSetup({ + channel: " Telegram ", + channelLabel: " Telegram Custom ", + accountId: " primary ", + }), + ).toBe("exec setup"); + expect(describeExecApprovalSetup).toHaveBeenCalledWith({ + channel: "telegram", + channelLabel: "Telegram Custom", + accountId: "primary", + }); + + expect(describeNativePluginApprovalClientSetup({ channel: " TELEGRAM " })).toBe("plugin setup"); + expect(describePluginApprovalSetup).toHaveBeenCalledWith({ + channel: "telegram", + channelLabel: "Telegram", + accountId: undefined, + }); + }); + + it.each([undefined, "web", "tui"])("suppresses setup guidance for %s", (channel) => { + expect(describeNativeExecApprovalClientSetup({ channel })).toBeNull(); + expect(describeNativePluginApprovalClientSetup({ channel })).toBeNull(); + expect(getChannelPluginMock).not.toHaveBeenCalled(); + }); + + it("does not fall back to the sibling approval hook", () => { + const describeExecApprovalSetup = vi.fn(() => "exec setup"); + const describePluginApprovalSetup = vi.fn(() => "plugin setup"); + getChannelPluginMock.mockImplementation((channel: string) => ({ + meta: { label: channel }, + approvalCapability: + channel === "telegram" ? { describePluginApprovalSetup } : { describeExecApprovalSetup }, + })); + + expect(describeNativeExecApprovalClientSetup({ channel: "telegram" })).toBeNull(); + expect(describePluginApprovalSetup).not.toHaveBeenCalled(); + expect(describeNativePluginApprovalClientSetup({ channel: "matrix" })).toBeNull(); + expect(describeExecApprovalSetup).not.toHaveBeenCalled(); + }); }); diff --git a/src/infra/exec-approval-surface.ts b/src/infra/exec-approval-surface.ts index 76a19260aa20..211e55eee819 100644 --- a/src/infra/exec-approval-surface.ts +++ b/src/infra/exec-approval-surface.ts @@ -112,44 +112,39 @@ export function listNativeExecApprovalClientLabels(params?: { .toSorted((a, b) => a.localeCompare(b)); } -/** Returns channel-specific setup guidance for native exec approvals, when available. */ -export function describeNativeExecApprovalClientSetup(params: { +type NativeApprovalClientSetupParams = { channel?: string | null; channelLabel?: string | null; accountId?: string | null; -}): string | null { +}; + +function describeNativeApprovalClientSetup( + params: NativeApprovalClientSetupParams, + approvalKind: ChannelApprovalKind, +): string | null { const channel = normalizeMessageChannel(params.channel); if (!channel || channel === INTERNAL_MESSAGE_CHANNEL || channel === "tui") { return null; } const channelLabel = normalizeOptionalString(params.channelLabel) ?? labelForChannel(channel); const accountId = normalizeOptionalString(params.accountId); - return ( - resolveChannelApprovalCapability(getChannelPlugin(channel))?.describeExecApprovalSetup?.({ - channel, - channelLabel, - accountId, - }) ?? null - ); + const capability = resolveChannelApprovalCapability(getChannelPlugin(channel)); + const setupParams = { channel, channelLabel, accountId }; + return approvalKind === "exec" + ? (capability?.describeExecApprovalSetup?.(setupParams) ?? null) + : (capability?.describePluginApprovalSetup?.(setupParams) ?? null); +} + +/** Returns channel-specific setup guidance for native exec approvals, when available. */ +export function describeNativeExecApprovalClientSetup( + params: NativeApprovalClientSetupParams, +): string | null { + return describeNativeApprovalClientSetup(params, "exec"); } /** Returns channel-specific setup guidance for native plugin approvals, when available. */ -export function describeNativePluginApprovalClientSetup(params: { - channel?: string | null; - channelLabel?: string | null; - accountId?: string | null; -}): string | null { - const channel = normalizeMessageChannel(params.channel); - if (!channel || channel === INTERNAL_MESSAGE_CHANNEL || channel === "tui") { - return null; - } - const channelLabel = normalizeOptionalString(params.channelLabel) ?? labelForChannel(channel); - const accountId = normalizeOptionalString(params.accountId); - return ( - resolveChannelApprovalCapability(getChannelPlugin(channel))?.describePluginApprovalSetup?.({ - channel, - channelLabel, - accountId, - }) ?? null - ); +export function describeNativePluginApprovalClientSetup( + params: NativeApprovalClientSetupParams, +): string | null { + return describeNativeApprovalClientSetup(params, "plugin"); }