From 4b8f9433b86260230bbc7cd2fc5fa9c8f08cc896 Mon Sep 17 00:00:00 2001 From: Shakker Date: Wed, 6 May 2026 02:20:00 +0100 Subject: [PATCH] fix: reuse codex native approvals (cherry picked from commit f1bf2a80a616642497cc6e085fa706ba612c5fbc) --- CHANGELOG.md | 1 + docs/plugins/codex-harness.md | 5 + docs/tools/exec-approvals-advanced.md | 2 + src/agents/harness/native-hook-relay.test.ts | 105 ++++++++++++++++++ src/agents/harness/native-hook-relay.ts | 81 +++++++++++++- .../protocol/schema/plugin-approvals.ts | 6 + .../server-methods/plugin-approval.test.ts | 63 +++++++++++ src/gateway/server-methods/plugin-approval.ts | 21 +++- src/infra/approval-view-model.ts | 6 +- src/infra/exec-approval-forwarder.ts | 2 + src/infra/plugin-approval-forwarder.test.ts | 40 +++++++ src/infra/plugin-approvals.ts | 29 ++++- src/plugin-sdk/approval-renderers.test.ts | 48 ++++++++ src/plugin-sdk/approval-renderers.ts | 5 +- 14 files changed, 405 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d7faf16b08..0e3be979529 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Docs: https://docs.openclaw.ai - Discord/voice: make voice capture less choppy by extending the default post-speech silence grace to 2.5s, add `voice.captureSilenceGraceMs` for noisy Discord sessions, and tighten the spoken-output prompt around live STT fragments. Thanks @vincentkoc. - WhatsApp: route proactive phone-number sends through Baileys LID forward mappings when available, so LID-addressed contacts receive agent messages instead of creating sender-only ghost chats. Fixes #67378. (#74925) Thanks @edenfunf. - WhatsApp: send captioned `MEDIA:` directive auto-replies once instead of emitting an empty media message before the captioned media reply. (#78770) Thanks @ai-hpc. +- Codex/approvals: remember `allow-always` decisions for identical Codex native `PermissionRequest` payloads within the active session window, and make plugin approval requests validate/render their actual allowed decisions so Telegram and other native approval UIs cannot offer stale actions. Thanks @shakkernerd. - Model providers: normalize APNG sniffed PNG uploads, preserve Gemini 3 tool-call thought-signature replay with fallback signatures, accept legacy `__env__:VAR` custom-provider keys, and repair snake_case tool-call transcript sanitization. Fixes #51881, #48915, #77566, and #42858. - Telegram/models: parse provider ids containing dots in `/models` callback buttons so `hf.co` model lists render as inline keyboard buttons. Fixes #38745. diff --git a/docs/plugins/codex-harness.md b/docs/plugins/codex-harness.md index 5c4d229be79..dffc6bcbea0 100644 --- a/docs/plugins/codex-harness.md +++ b/docs/plugins/codex-harness.md @@ -1015,6 +1015,11 @@ it. For `PermissionRequest`, OpenClaw only returns explicit allow or deny decisions when policy decides. A no-decision result is not an allow. Codex treats it as no hook decision and falls through to its own guardian or user approval path. +When an operator chooses `allow-always` for a Codex native permission request, +OpenClaw remembers that exact provider/session/tool input/cwd fingerprint for a +bounded session window. The remembered decision is intentionally exact-match +only: a changed command, arguments, tool payload, or cwd creates a fresh +approval. Codex MCP tool approval elicitations are routed through OpenClaw's plugin approval flow when Codex marks `_meta.codex_approval_kind` as diff --git a/docs/tools/exec-approvals-advanced.md b/docs/tools/exec-approvals-advanced.md index 4ede6fdf3d0..95c9f98903e 100644 --- a/docs/tools/exec-approvals-advanced.md +++ b/docs/tools/exec-approvals-advanced.md @@ -233,6 +233,8 @@ The config shape is identical to `approvals.exec`: `enabled`, `mode`, `agentFilt Channels that support shared interactive replies render the same approval buttons for both exec and plugin approvals. Channels without shared interactive UI fall back to plain text with `/approve` instructions. +Plugin approval requests may restrict the available decisions. Approval surfaces use the request's +declared decision set, and the Gateway rejects attempts to submit a decision that was not offered. ### Same-chat approvals on any channel diff --git a/src/agents/harness/native-hook-relay.test.ts b/src/agents/harness/native-hook-relay.test.ts index df00a3e5ea4..c0f749e85ed 100644 --- a/src/agents/harness/native-hook-relay.test.ts +++ b/src/agents/harness/native-hook-relay.test.ts @@ -1178,6 +1178,111 @@ describe("native hook relay registry", () => { ); }); + it("reuses allow-always PermissionRequest approvals for identical relay content", async () => { + const relay = registerNativeHookRelay({ + provider: "codex", + relayId: "codex-stable-permission-cache", + sessionId: "session-1", + runId: "run-1", + }); + const approvalRequester = vi.fn(async () => "allow-always" as const); + __testing.setNativeHookRelayPermissionApprovalRequesterForTests(approvalRequester); + + const first = await invokeNativeHookRelay({ + provider: "codex", + relayId: relay.relayId, + event: "permission_request", + rawPayload: { + hook_event_name: "PermissionRequest", + cwd: "/repo", + tool_name: "Bash", + tool_use_id: "native-call-1", + tool_input: { command: "browserforce tabs" }, + }, + }); + relay.unregister(); + registerNativeHookRelay({ + provider: "codex", + relayId: "codex-stable-permission-cache", + sessionId: "session-1", + runId: "run-2", + }); + const second = await invokeNativeHookRelay({ + provider: "codex", + relayId: relay.relayId, + event: "permission_request", + rawPayload: { + hook_event_name: "PermissionRequest", + cwd: "/repo", + tool_name: "Bash", + tool_use_id: "native-call-2", + tool_input: { command: "browserforce tabs" }, + }, + }); + + expect(approvalRequester).toHaveBeenCalledTimes(1); + expect([first, second].map((response) => JSON.parse(response.stdout))).toEqual([ + { + hookSpecificOutput: { + hookEventName: "PermissionRequest", + decision: { behavior: "allow" }, + }, + }, + { + hookSpecificOutput: { + hookEventName: "PermissionRequest", + decision: { behavior: "allow" }, + }, + }, + ]); + }); + + it("keeps allow-always PermissionRequest reuse scoped to matching cwd and input", async () => { + const relay = registerNativeHookRelay({ + provider: "codex", + sessionId: "session-1", + runId: "run-1", + }); + const approvalRequester = vi.fn(async () => "allow-always" as const); + __testing.setNativeHookRelayPermissionApprovalRequesterForTests(approvalRequester); + + await invokeNativeHookRelay({ + provider: "codex", + relayId: relay.relayId, + event: "permission_request", + rawPayload: { + hook_event_name: "PermissionRequest", + cwd: "/repo-a", + tool_name: "Bash", + tool_input: { command: "npm test" }, + }, + }); + await invokeNativeHookRelay({ + provider: "codex", + relayId: relay.relayId, + event: "permission_request", + rawPayload: { + hook_event_name: "PermissionRequest", + cwd: "/repo-b", + tool_name: "Bash", + tool_input: { command: "npm test" }, + }, + }); + await invokeNativeHookRelay({ + provider: "codex", + relayId: relay.relayId, + event: "permission_request", + rawPayload: { + hook_event_name: "PermissionRequest", + cwd: "/repo-a", + tool_name: "Bash", + tool_input: { command: "npm test -- --changed" }, + }, + }); + + expect(approvalRequester).toHaveBeenCalledTimes(3); + }); + it("defers PermissionRequest when OpenClaw approval does not decide", async () => { __testing.setNativeHookRelayPermissionApprovalRequesterForTests( vi.fn(async () => "defer" as const), diff --git a/src/agents/harness/native-hook-relay.ts b/src/agents/harness/native-hook-relay.ts index dd78198f9d8..668d3e2d5eb 100644 --- a/src/agents/harness/native-hook-relay.ts +++ b/src/agents/harness/native-hook-relay.ts @@ -159,6 +159,7 @@ type NativeHookRelayProviderAdapter = { const DEFAULT_RELAY_TTL_MS = 30 * 60 * 1000; const DEFAULT_RELAY_TIMEOUT_MS = 5_000; const DEFAULT_PERMISSION_TIMEOUT_MS = 120_000; +const PERMISSION_ALLOW_ALWAYS_TTL_MS = 30 * 60 * 1000; const MAX_NATIVE_HOOK_RELAY_INVOCATIONS = 200; const MAX_NATIVE_HOOK_RELAY_JSON_DEPTH = 64; const MAX_NATIVE_HOOK_RELAY_JSON_NODES = 20_000; @@ -175,6 +176,7 @@ const MAX_APPROVAL_TITLE_LENGTH = 80; const MAX_APPROVAL_DESCRIPTION_LENGTH = 700; const MAX_PERMISSION_APPROVALS_PER_WINDOW = 12; const PERMISSION_APPROVAL_WINDOW_MS = 60_000; +const MAX_PERMISSION_ALLOW_ALWAYS_ENTRIES = 512; const MAX_NATIVE_HOOK_BRIDGE_BODY_BYTES = 5_000_000; const MAX_NATIVE_HOOK_BRIDGE_RESPONSE_BYTES = 5_000_000; const NATIVE_HOOK_BRIDGE_RETRY_INTERVAL_MS = 25; @@ -187,11 +189,15 @@ const pendingPermissionApprovals = new Map< Promise >(); const permissionApprovalWindows = new Map(); +const permissionAllowAlwaysApprovals = new Map(); const log = createSubsystemLogger("agents/harness/native-hook-relay"); type NativeHookRelayPermissionDecision = "allow" | "deny"; -type NativeHookRelayPermissionApprovalResult = NativeHookRelayPermissionDecision | "defer"; +type NativeHookRelayPermissionApprovalResult = + | NativeHookRelayPermissionDecision + | "allow-always" + | "defer"; type NativeHookRelayPermissionApprovalRequest = { provider: NativeHookRelayProvider; @@ -293,6 +299,7 @@ export function registerNativeHookRelay( params: RegisterNativeHookRelayParams, ): NativeHookRelayRegistrationHandle { pruneExpiredNativeHookRelays(); + pruneNativeHookRelayPermissionAllowAlways(); const relayId = normalizeRelayId(params.relayId) ?? randomUUID(); const allowedEvents = normalizeAllowedEvents(params.allowedEvents); unregisterNativeHookRelay(relayId); @@ -937,6 +944,13 @@ async function runNativeHookRelayPermissionRequest(params: { registration: params.registration, request, }); + const allowAlwaysKey = nativeHookRelayPermissionAllowAlwaysKey({ + registration: params.registration, + request, + }); + if (hasNativeHookRelayPermissionAllowAlways(allowAlwaysKey)) { + return params.adapter.renderPermissionDecisionResponse("allow"); + } const pendingApproval = pendingPermissionApprovals.get(approvalKey); try { const decision = await (pendingApproval ?? @@ -948,6 +962,10 @@ async function runNativeHookRelayPermissionRequest(params: { if (decision === "allow") { return params.adapter.renderPermissionDecisionResponse("allow"); } + if (decision === "allow-always") { + rememberNativeHookRelayPermissionAllowAlways(allowAlwaysKey); + return params.adapter.renderPermissionDecisionResponse("allow"); + } if (decision === "deny") { return params.adapter.renderPermissionDecisionResponse("deny", "Denied by user"); } @@ -1033,6 +1051,17 @@ function nativeHookRelayPermissionApprovalKey(params: { ].join(":"); } +function nativeHookRelayPermissionAllowAlwaysKey(params: { + registration: NativeHookRelayRegistration; + request: NativeHookRelayPermissionApprovalRequest; +}): string { + return [ + params.registration.relayId, + "allow-always", + permissionRequestContentFingerprint(params.request), + ].join(":"); +} + function permissionRequestFallbackKey(request: NativeHookRelayPermissionApprovalRequest): string { const command = readOptionalString(request.toolInput.command); if (command) { @@ -1065,6 +1094,8 @@ function permissionRequestContentFingerprint( const hash = createHash("sha256"); hash.update(request.toolName); hash.update("\0"); + hash.update(request.cwd ?? ""); + hash.update("\0"); updateJsonHash(hash, request.toolInput); return hash.digest("hex"); } @@ -1156,6 +1187,40 @@ function consumeNativeHookRelayPermissionBudget(relayId: string, now = Date.now( return true; } +function hasNativeHookRelayPermissionAllowAlways(key: string, now = Date.now()): boolean { + const entry = permissionAllowAlwaysApprovals.get(key); + if (!entry) { + return false; + } + if (entry.expiresAtMs <= now) { + permissionAllowAlwaysApprovals.delete(key); + return false; + } + return true; +} + +function rememberNativeHookRelayPermissionAllowAlways(key: string, now = Date.now()): void { + pruneNativeHookRelayPermissionAllowAlways(now); + permissionAllowAlwaysApprovals.set(key, { + expiresAtMs: now + PERMISSION_ALLOW_ALWAYS_TTL_MS, + }); + while (permissionAllowAlwaysApprovals.size > MAX_PERMISSION_ALLOW_ALWAYS_ENTRIES) { + const oldestKey = permissionAllowAlwaysApprovals.keys().next().value; + if (typeof oldestKey !== "string") { + break; + } + permissionAllowAlwaysApprovals.delete(oldestKey); + } +} + +function pruneNativeHookRelayPermissionAllowAlways(now = Date.now()): void { + for (const [key, entry] of permissionAllowAlwaysApprovals) { + if (entry.expiresAtMs <= now) { + permissionAllowAlwaysApprovals.delete(key); + } + } +} + function removeNativeHookRelayPermissionState(relayId: string): void { permissionApprovalWindows.delete(relayId); for (const key of pendingPermissionApprovals.keys()) { @@ -1332,6 +1397,11 @@ async function requestNativeHookRelayPermissionApproval( severity: "warning", toolName: request.toolName, toolCallId: request.toolCallId, + allowedDecisions: [ + PluginApprovalResolutions.ALLOW_ONCE, + PluginApprovalResolutions.ALLOW_ALWAYS, + PluginApprovalResolutions.DENY, + ], agentId: request.agentId, sessionKey: request.sessionKey, timeoutMs, @@ -1354,12 +1424,12 @@ async function requestNativeHookRelayPermissionApproval( }); decision = waitResult?.decision; } - if ( - decision === PluginApprovalResolutions.ALLOW_ONCE || - decision === PluginApprovalResolutions.ALLOW_ALWAYS - ) { + if (decision === PluginApprovalResolutions.ALLOW_ONCE) { return "allow"; } + if (decision === PluginApprovalResolutions.ALLOW_ALWAYS) { + return "allow-always"; + } if (decision === PluginApprovalResolutions.DENY) { return "deny"; } @@ -1645,6 +1715,7 @@ export const __testing = { invocations.length = 0; pendingPermissionApprovals.clear(); permissionApprovalWindows.clear(); + permissionAllowAlwaysApprovals.clear(); nativeHookRelayPermissionApprovalRequester = requestNativeHookRelayPermissionApproval; }, getNativeHookRelayInvocationsForTests(): NativeHookRelayInvocation[] { diff --git a/src/gateway/protocol/schema/plugin-approvals.ts b/src/gateway/protocol/schema/plugin-approvals.ts index 4aaa8e12a04..eb33e26843b 100644 --- a/src/gateway/protocol/schema/plugin-approvals.ts +++ b/src/gateway/protocol/schema/plugin-approvals.ts @@ -14,6 +14,12 @@ export const PluginApprovalRequestParamsSchema = Type.Object( severity: Type.Optional(Type.String({ enum: ["info", "warning", "critical"] })), toolName: Type.Optional(Type.String()), toolCallId: Type.Optional(Type.String()), + allowedDecisions: Type.Optional( + Type.Array(Type.String({ enum: ["allow-once", "allow-always", "deny"] }), { + minItems: 1, + maxItems: 3, + }), + ), agentId: Type.Optional(Type.String()), sessionKey: Type.Optional(Type.String()), turnSourceChannel: Type.Optional(Type.String()), diff --git a/src/gateway/server-methods/plugin-approval.test.ts b/src/gateway/server-methods/plugin-approval.test.ts index 0a8e6c95c25..6543b816b6f 100644 --- a/src/gateway/server-methods/plugin-approval.test.ts +++ b/src/gateway/server-methods/plugin-approval.test.ts @@ -322,6 +322,40 @@ describe("createPluginApprovalHandlers", () => { expect.objectContaining({ message: expect.stringContaining("unexpected property") }), ); }); + + it("stores scoped allowed decisions on plugin approval requests", async () => { + const handlers = createPluginApprovalHandlers(manager); + const respond = vi.fn(); + const opts = createMockOptions( + "plugin.approval.request", + { + title: "T", + description: "D", + allowedDecisions: ["allow-once", "deny", "allow-once"], + twoPhase: true, + }, + { respond }, + ); + + const handlerPromise = handlers["plugin.approval.request"](opts); + await vi.waitFor(() => { + expect(respond).toHaveBeenCalledWith( + true, + expect.objectContaining({ status: "accepted", id: expect.any(String) }), + undefined, + ); + }); + + const acceptedCall = respond.mock.calls.find( + (call) => (call[1] as Record)?.status === "accepted", + ); + const approvalId = (acceptedCall?.[1] as Record)?.id as string; + expect(manager.getSnapshot(approvalId)?.request).toMatchObject({ + allowedDecisions: ["allow-once", "deny"], + }); + manager.resolve(approvalId, "deny"); + await handlerPromise; + }); }); describe("plugin.approval.list", () => { @@ -463,6 +497,35 @@ describe("createPluginApprovalHandlers", () => { ); }); + it("rejects decisions outside plugin approval allowed decisions", async () => { + const handlers = createPluginApprovalHandlers(manager); + const record = manager.create( + { + title: "T", + description: "D", + allowedDecisions: ["allow-once", "deny"], + }, + 60_000, + ); + void manager.register(record, 60_000); + + const opts = createMockOptions("plugin.approval.resolve", { + id: record.id, + decision: "allow-always", + }); + await handlers["plugin.approval.resolve"](opts); + expect(opts.respond).toHaveBeenCalledWith( + false, + undefined, + expect.objectContaining({ + code: "INVALID_REQUEST", + message: "allow-always is unavailable for this plugin approval", + details: { allowedDecisions: ["allow-once", "deny"] }, + }), + ); + expect(manager.getSnapshot(record.id)?.decision).toBeUndefined(); + }); + it("rejects unknown approval id", async () => { const handlers = createPluginApprovalHandlers(manager); const opts = createMockOptions("plugin.approval.resolve", { diff --git a/src/gateway/server-methods/plugin-approval.ts b/src/gateway/server-methods/plugin-approval.ts index 314bfe3d4ba..52023a6fbb9 100644 --- a/src/gateway/server-methods/plugin-approval.ts +++ b/src/gateway/server-methods/plugin-approval.ts @@ -5,6 +5,7 @@ import type { PluginApprovalRequestPayload } from "../../infra/plugin-approvals. import { DEFAULT_PLUGIN_APPROVAL_TIMEOUT_MS, MAX_PLUGIN_APPROVAL_TIMEOUT_MS, + resolvePluginApprovalRequestAllowedDecisions, } from "../../infra/plugin-approvals.js"; import { normalizeOptionalString } from "../../shared/string-coerce.js"; import type { ExecApprovalManager } from "../exec-approval-manager.js"; @@ -61,6 +62,7 @@ export function createPluginApprovalHandlers( severity?: string | null; toolName?: string | null; toolCallId?: string | null; + allowedDecisions?: string[] | null; agentId?: string | null; sessionKey?: string | null; turnSourceChannel?: string | null; @@ -86,6 +88,13 @@ export function createPluginApprovalHandlers( severity: (p.severity as PluginApprovalRequestPayload["severity"]) ?? null, toolName: p.toolName ?? null, toolCallId: p.toolCallId ?? null, + ...(Array.isArray(p.allowedDecisions) + ? { + allowedDecisions: resolvePluginApprovalRequestAllowedDecisions({ + allowedDecisions: p.allowedDecisions, + }), + } + : {}), agentId: p.agentId ?? null, sessionKey: p.sessionKey ?? null, turnSourceChannel: normalizeTrimmedString(p.turnSourceChannel), @@ -166,14 +175,24 @@ export function createPluginApprovalHandlers( respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "invalid decision")); return; } + const decision = p.decision; await handleApprovalResolve({ manager, inputId: p.id, - decision: p.decision, + decision, respond, context, client, exposeAmbiguousPrefixError: false, + validateDecision: (snapshot) => + resolvePluginApprovalRequestAllowedDecisions(snapshot.request).includes(decision) + ? null + : { + message: `${decision} is unavailable for this plugin approval`, + details: { + allowedDecisions: resolvePluginApprovalRequestAllowedDecisions(snapshot.request), + }, + }, resolvedEventName: "plugin.approval.resolved", buildResolvedEvent: ({ approvalId, decision, resolvedBy, snapshot, nowMs }) => ({ id: approvalId, diff --git a/src/infra/approval-view-model.ts b/src/infra/approval-view-model.ts index 4121e729ff6..cd17fd26838 100644 --- a/src/infra/approval-view-model.ts +++ b/src/infra/approval-view-model.ts @@ -14,7 +14,10 @@ import { resolveExecApprovalRequestAllowedDecisions, type ExecApprovalRequest, } from "./exec-approvals.js"; -import type { PluginApprovalRequest } from "./plugin-approvals.js"; +import { + resolvePluginApprovalRequestAllowedDecisions, + type PluginApprovalRequest, +} from "./plugin-approvals.js"; type ApprovalPhase = "pending" | "resolved" | "expired"; @@ -105,6 +108,7 @@ export function buildPendingApprovalView(request: ApprovalRequest): PendingAppro ...buildPluginViewBase(pluginRequest, "pending"), actions: buildExecApprovalActionDescriptors({ approvalCommandId: pluginRequest.id, + allowedDecisions: resolvePluginApprovalRequestAllowedDecisions(pluginRequest.request), }), expiresAtMs: pluginRequest.expiresAtMs, }; diff --git a/src/infra/exec-approval-forwarder.ts b/src/infra/exec-approval-forwarder.ts index f5bd68252f1..bad4a857560 100644 --- a/src/infra/exec-approval-forwarder.ts +++ b/src/infra/exec-approval-forwarder.ts @@ -38,6 +38,7 @@ import { approvalDecisionLabel, buildPluginApprovalExpiredMessage, buildPluginApprovalRequestMessage, + resolvePluginApprovalRequestAllowedDecisions, type PluginApprovalRequest, type PluginApprovalResolved, } from "./plugin-approvals.js"; @@ -461,6 +462,7 @@ function buildPluginPendingPayload(params: { request: params.request, nowMs: params.nowMs, text: buildPluginApprovalRequestMessage(params.request, params.nowMs), + allowedDecisions: resolvePluginApprovalRequestAllowedDecisions(params.request.request), }), }); } diff --git a/src/infra/plugin-approval-forwarder.test.ts b/src/infra/plugin-approval-forwarder.test.ts index 3498522ed00..184516dbcd1 100644 --- a/src/infra/plugin-approval-forwarder.test.ts +++ b/src/infra/plugin-approval-forwarder.test.ts @@ -154,6 +154,46 @@ describe("plugin approval forwarding", () => { }); }); + it("renders only request-scoped plugin approval decisions", async () => { + const deliver = vi.fn().mockResolvedValue([]); + const { forwarder } = createForwarder({ cfg: PLUGIN_TARGETS_CFG, deliver }); + const result = await forwarder.handlePluginApprovalRequested!( + makePluginRequest({ + request: { + ...makePluginRequest().request, + allowedDecisions: ["allow-once", "deny"], + }, + }), + ); + expect(result).toBe(true); + await flushPendingDelivery(); + const deliveryArgs = deliver.mock.calls[0]?.[0] as + | { payloads?: Array<{ text?: string; interactive?: unknown }> } + | undefined; + const payload = deliveryArgs?.payloads?.[0]; + expect(payload?.text).toContain("Reply with: /approve allow-once|deny"); + expect(payload?.text).not.toContain("allow-always"); + expect(payload?.interactive).toEqual({ + blocks: [ + { + type: "buttons", + buttons: [ + { + label: "Allow Once", + value: "/approve plugin-req-1 allow-once", + style: "success", + }, + { + label: "Deny", + value: "/approve plugin-req-1 deny", + style: "danger", + }, + ], + }, + ], + }); + }); + it("includes severity icon for critical", async () => { const deliver = vi.fn().mockResolvedValue([]); const { forwarder } = createForwarder({ cfg: PLUGIN_TARGETS_CFG, deliver }); diff --git a/src/infra/plugin-approvals.ts b/src/infra/plugin-approvals.ts index c701dcf3da8..2d721ba42cb 100644 --- a/src/infra/plugin-approvals.ts +++ b/src/infra/plugin-approvals.ts @@ -7,6 +7,7 @@ export type PluginApprovalRequestPayload = { severity?: "info" | "warning" | "critical" | null; toolName?: string | null; toolCallId?: string | null; + allowedDecisions?: readonly ExecApprovalDecision[] | null; agentId?: string | null; sessionKey?: string | null; turnSourceChannel?: string | null; @@ -34,6 +35,11 @@ export const DEFAULT_PLUGIN_APPROVAL_TIMEOUT_MS = 120_000; export const MAX_PLUGIN_APPROVAL_TIMEOUT_MS = 600_000; export const PLUGIN_APPROVAL_TITLE_MAX_LENGTH = 80; export const PLUGIN_APPROVAL_DESCRIPTION_MAX_LENGTH = 256; +export const DEFAULT_PLUGIN_APPROVAL_DECISIONS = [ + "allow-once", + "allow-always", + "deny", +] as const satisfies readonly ExecApprovalDecision[]; export function approvalDecisionLabel(decision: ExecApprovalDecision): string { if (decision === "allow-once") { @@ -45,6 +51,23 @@ export function approvalDecisionLabel(decision: ExecApprovalDecision): string { return "denied"; } +export function resolvePluginApprovalRequestAllowedDecisions(params?: { + allowedDecisions?: readonly ExecApprovalDecision[] | readonly string[] | null; +}): readonly ExecApprovalDecision[] { + const explicit: ExecApprovalDecision[] = []; + if (Array.isArray(params?.allowedDecisions)) { + for (const decision of params.allowedDecisions) { + if ( + (decision === "allow-once" || decision === "allow-always" || decision === "deny") && + !explicit.includes(decision) + ) { + explicit.push(decision); + } + } + } + return explicit.length > 0 ? explicit : DEFAULT_PLUGIN_APPROVAL_DECISIONS; +} + export function buildPluginApprovalRequestMessage( request: PluginApprovalRequest, nowMsValue: number, @@ -67,7 +90,11 @@ export function buildPluginApprovalRequestMessage( lines.push(`ID: ${request.id}`); const expiresIn = Math.max(0, Math.round((request.expiresAtMs - nowMsValue) / 1000)); lines.push(`Expires in: ${expiresIn}s`); - lines.push("Reply with: /approve allow-once|allow-always|deny"); + lines.push( + `Reply with: /approve ${resolvePluginApprovalRequestAllowedDecisions(request.request).join( + "|", + )}`, + ); return lines.join("\n"); } diff --git a/src/plugin-sdk/approval-renderers.test.ts b/src/plugin-sdk/approval-renderers.test.ts index 00c9994880b..59b70583f6b 100644 --- a/src/plugin-sdk/approval-renderers.test.ts +++ b/src/plugin-sdk/approval-renderers.test.ts @@ -102,6 +102,54 @@ describe("plugin-sdk/approval-renderers", () => { }, }, }, + { + name: "builds plugin pending payloads with request-scoped decisions", + payload: buildPluginApprovalPendingReplyPayload({ + request: { + id: "plugin-approval-123", + request: { + title: "Sensitive action", + description: "Needs approval", + allowedDecisions: ["allow-once", "deny"], + }, + createdAtMs: 1_000, + expiresAtMs: 61_000, + }, + nowMs: 1_000, + }), + textExpected: (text: string) => + expect(text).toContain("Reply with: /approve allow-once|deny"), + interactiveExpected: { + blocks: [ + { + type: "buttons", + buttons: [ + { + label: "Allow Once", + value: "/approve plugin-approval-123 allow-once", + style: "success", + }, + { + label: "Deny", + value: "/approve plugin-approval-123 deny", + style: "danger", + }, + ], + }, + ], + }, + channelDataExpected: { + execApproval: { + agentId: undefined, + approvalId: "plugin-approval-123", + approvalKind: "plugin", + approvalSlug: "plugin-a", + allowedDecisions: ["allow-once", "deny"], + sessionKey: undefined, + state: "pending", + }, + }, + }, { name: "builds generic resolved payloads with approval metadata", payload: buildApprovalResolvedReplyPayload({ diff --git a/src/plugin-sdk/approval-renderers.ts b/src/plugin-sdk/approval-renderers.ts index e693e66e6e6..a44b513003c 100644 --- a/src/plugin-sdk/approval-renderers.ts +++ b/src/plugin-sdk/approval-renderers.ts @@ -5,6 +5,7 @@ import { import { buildPluginApprovalRequestMessage, buildPluginApprovalResolvedMessage, + resolvePluginApprovalRequestAllowedDecisions, type PluginApprovalRequest, type PluginApprovalResolved, } from "../infra/plugin-approvals.js"; @@ -77,7 +78,9 @@ export function buildPluginApprovalPendingReplyPayload(params: { approvalId: params.request.id, approvalSlug: params.approvalSlug ?? params.request.id.slice(0, 8), text: params.text ?? buildPluginApprovalRequestMessage(params.request, params.nowMs), - allowedDecisions: params.allowedDecisions, + allowedDecisions: + params.allowedDecisions ?? + resolvePluginApprovalRequestAllowedDecisions(params.request.request), channelData: params.channelData, }); }