mirror of
https://github.com/moltbot/moltbot.git
synced 2026-05-09 20:17:27 +00:00
fix: reuse codex native approvals
This commit is contained in:
@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Plugin skills/Windows: publish plugin-provided skill directories as junctions on Windows so standard users without Developer Mode can register plugin skills without symlink EPERM failures. Fixes #77958. (#77971) Thanks @hclsys and @jarro.
|
||||
- MS Teams: surface blocked Bot Framework egress by logging JWKS fetch network failures and adding a Bot Connector send hint for transport-level reply failures. Fixes #77674. (#78081) Thanks @Beandon13.
|
||||
- Gateway/sessions: fast-path already-qualified model refs while building session-list rows so `openclaw sessions` and Control UI session lists avoid heavyweight model resolution on large stores. (#77902) Thanks @ragesaq.
|
||||
- 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.
|
||||
- PR triage: mark external pull requests with `proof: supplied` when Barnacle finds structured real behavior proof, keep stale negative proof labels in sync across CRLF-edited PR bodies, and let ClawSweeper own the stronger `proof: sufficient` judgement.
|
||||
- Sessions CLI: show the selected agent runtime in the `openclaw sessions` table so terminal output matches the runtime visibility already present in JSON/status surfaces. Thanks @vincentkoc.
|
||||
- Talk/voice: unify realtime relay, transcription relay, managed-room handoff, Voice Call, Google Meet, VoiceClaw, and native clients around a shared Talk session controller and add the Gateway-managed `talk.session.*` RPC surface.
|
||||
|
||||
@@ -1016,6 +1016,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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -151,6 +151,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;
|
||||
@@ -167,6 +168,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;
|
||||
@@ -179,11 +181,15 @@ const pendingPermissionApprovals = new Map<
|
||||
Promise<NativeHookRelayPermissionApprovalResult>
|
||||
>();
|
||||
const permissionApprovalWindows = new Map<string, number[]>();
|
||||
const permissionAllowAlwaysApprovals = new Map<string, { expiresAtMs: number }>();
|
||||
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;
|
||||
@@ -285,6 +291,7 @@ export function registerNativeHookRelay(
|
||||
params: RegisterNativeHookRelayParams,
|
||||
): NativeHookRelayRegistrationHandle {
|
||||
pruneExpiredNativeHookRelays();
|
||||
pruneNativeHookRelayPermissionAllowAlways();
|
||||
const relayId = normalizeRelayId(params.relayId) ?? randomUUID();
|
||||
const allowedEvents = normalizeAllowedEvents(params.allowedEvents);
|
||||
unregisterNativeHookRelay(relayId);
|
||||
@@ -921,6 +928,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 ??
|
||||
@@ -932,6 +946,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");
|
||||
}
|
||||
@@ -1017,6 +1035,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) {
|
||||
@@ -1049,6 +1078,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");
|
||||
}
|
||||
@@ -1140,6 +1171,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()) {
|
||||
@@ -1316,6 +1381,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,
|
||||
@@ -1338,12 +1408,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";
|
||||
}
|
||||
@@ -1629,6 +1699,7 @@ export const __testing = {
|
||||
invocations.length = 0;
|
||||
pendingPermissionApprovals.clear();
|
||||
permissionApprovalWindows.clear();
|
||||
permissionAllowAlwaysApprovals.clear();
|
||||
nativeHookRelayPermissionApprovalRequester = requestNativeHookRelayPermissionApproval;
|
||||
},
|
||||
getNativeHookRelayInvocationsForTests(): NativeHookRelayInvocation[] {
|
||||
|
||||
@@ -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()),
|
||||
|
||||
@@ -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<string, unknown>)?.status === "accepted",
|
||||
);
|
||||
const approvalId = (acceptedCall?.[1] as Record<string, unknown>)?.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", {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
|
||||
@@ -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),
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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 <id> 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 });
|
||||
|
||||
@@ -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 <id> allow-once|allow-always|deny");
|
||||
lines.push(
|
||||
`Reply with: /approve <id> ${resolvePluginApprovalRequestAllowedDecisions(request.request).join(
|
||||
"|",
|
||||
)}`,
|
||||
);
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
|
||||
@@ -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 <id> 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({
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user