From f9dbb236b6877ca020f4c7dc9aa3d1621f07c27b Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Wed, 1 Apr 2026 19:50:34 -0400 Subject: [PATCH] Exec approvals: soften effective-policy triage regressions --- src/agents/bash-tools.exec-runtime.ts | 6 ++-- src/cli/exec-approvals-cli.test.ts | 32 +++++++++++++++++++ src/cli/exec-approvals-cli.ts | 12 +++++-- src/gateway/server-methods/exec-approval.ts | 2 +- .../server-methods/server-methods.test.ts | 3 +- src/infra/exec-approval-forwarder.ts | 6 ++-- src/infra/exec-approval-reply.test.ts | 6 ++-- src/infra/exec-approval-reply.ts | 4 ++- 8 files changed, 60 insertions(+), 11 deletions(-) diff --git a/src/agents/bash-tools.exec-runtime.ts b/src/agents/bash-tools.exec-runtime.ts index 5a4a08e3bbb..ab473ed9b7d 100644 --- a/src/agents/bash-tools.exec-runtime.ts +++ b/src/agents/bash-tools.exec-runtime.ts @@ -368,11 +368,13 @@ export function buildApprovalPendingMessage(params: { lines.push( allowedDecisions.includes("allow-always") ? "Background mode requires pre-approved policy (allow-always or ask=off)." - : "Background mode requires host policy that allows pre-approval (for example ask=off).", + : "Background mode requires an effective policy that allows pre-approval (for example ask=off).", ); lines.push(`Reply with: /approve ${params.approvalSlug} ${decisionText}`); if (!allowedDecisions.includes("allow-always")) { - lines.push("Host policy requires approval every time, so Allow Always is unavailable."); + lines.push( + "The effective approval policy requires approval every time, so Allow Always is unavailable.", + ); } lines.push("If the short code is ambiguous, use the full id in /approve."); return lines.join("\n"); diff --git a/src/cli/exec-approvals-cli.test.ts b/src/cli/exec-approvals-cli.test.ts index ce3563fe956..48d3f1a382d 100644 --- a/src/cli/exec-approvals-cli.test.ts +++ b/src/cli/exec-approvals-cli.test.ts @@ -251,6 +251,38 @@ describe("exec approvals CLI", () => { ); }); + it("keeps gateway approvals output when config.get fails", async () => { + callGatewayFromCli.mockImplementation( + async (method: string, _opts: unknown, params?: unknown) => { + if (method === "config.get") { + throw new Error("gateway config unavailable"); + } + if (method === "exec.approvals.get") { + return { + path: "/tmp/exec-approvals.json", + exists: true, + hash: "hash-1", + file: { version: 1, agents: {} }, + }; + } + return { method, params }; + }, + ); + + await runApprovalsCommand(["approvals", "get", "--gateway", "--json"]); + + expect(defaultRuntime.writeJson).toHaveBeenCalledWith( + expect.objectContaining({ + effectivePolicy: { + note: "Config unavailable.", + scopes: [], + }, + }), + 0, + ); + expect(runtimeErrors).toHaveLength(0); + }); + it("reports agent scopes with inherited global requested policy", async () => { localSnapshot.file = { version: 1, diff --git a/src/cli/exec-approvals-cli.ts b/src/cli/exec-approvals-cli.ts index 7a72d6c4045..6c266e9dcf1 100644 --- a/src/cli/exec-approvals-cli.ts +++ b/src/cli/exec-approvals-cli.ts @@ -168,8 +168,16 @@ async function loadConfigForApprovalsTarget(params: { if (params.source === "local") { return await readBestEffortConfig(); } - const snapshot = (await callGatewayFromCli("config.get", params.opts, {})) as ConfigSnapshotLike; - return snapshot.config && typeof snapshot.config === "object" ? snapshot.config : null; + try { + const snapshot = (await callGatewayFromCli( + "config.get", + params.opts, + {}, + )) as ConfigSnapshotLike; + return snapshot.config && typeof snapshot.config === "object" ? snapshot.config : null; + } catch { + return null; + } } function collectExecPolicySnapshots(params: { cfg: OpenClawConfig; approvals: ExecApprovalsFile }) { diff --git a/src/gateway/server-methods/exec-approval.ts b/src/gateway/server-methods/exec-approval.ts index 397895ca18a..3f6ef2e0379 100644 --- a/src/gateway/server-methods/exec-approval.ts +++ b/src/gateway/server-methods/exec-approval.ts @@ -337,7 +337,7 @@ export function createExecApprovalHandlers( undefined, errorShape( ErrorCodes.INVALID_REQUEST, - "allow-always is unavailable because host policy requires approval every time", + "allow-always is unavailable because the effective policy requires approval every time", ), ); return; diff --git a/src/gateway/server-methods/server-methods.test.ts b/src/gateway/server-methods/server-methods.test.ts index 918f8c2f02e..540f04c880b 100644 --- a/src/gateway/server-methods/server-methods.test.ts +++ b/src/gateway/server-methods/server-methods.test.ts @@ -597,7 +597,8 @@ describe("exec approval handlers", () => { false, undefined, expect.objectContaining({ - message: "allow-always is unavailable because host policy requires approval every time", + message: + "allow-always is unavailable because the effective policy requires approval every time", }), ); diff --git a/src/infra/exec-approval-forwarder.ts b/src/infra/exec-approval-forwarder.ts index cf8179bb7b8..106b97c1cca 100644 --- a/src/infra/exec-approval-forwarder.ts +++ b/src/infra/exec-approval-forwarder.ts @@ -238,11 +238,13 @@ function buildRequestMessage(request: ExecApprovalRequest, nowMs: number) { lines.push( allowedDecisions.includes("allow-always") ? "Background mode note: non-interactive runs cannot wait for chat approvals; use pre-approved policy (allow-always or ask=off)." - : "Background mode note: non-interactive runs cannot wait for chat approvals; host policy still requires per-run approval unless ask=off.", + : "Background mode note: non-interactive runs cannot wait for chat approvals; the effective policy still requires per-run approval unless ask=off.", ); lines.push(`Reply with: /approve ${decisionText}`); if (!allowedDecisions.includes("allow-always")) { - lines.push("Allow Always is unavailable because host policy requires approval every time."); + lines.push( + "Allow Always is unavailable because the effective policy requires approval every time.", + ); } return lines.join("\n"); } diff --git a/src/infra/exec-approval-reply.test.ts b/src/infra/exec-approval-reply.test.ts index d24ca0af5f9..bd496488b07 100644 --- a/src/infra/exec-approval-reply.test.ts +++ b/src/infra/exec-approval-reply.test.ts @@ -138,7 +138,7 @@ describe("exec approval reply helpers", () => { expect(payload.text).toContain("Full id: `req-1`"); }); - it("omits allow-always actions when host policy requires approval every time", () => { + it("omits allow-always actions when the effective policy requires approval every time", () => { const payload = buildExecApprovalPendingReplyPayload({ approvalId: "req-ask-always", approvalSlug: "slug-always", @@ -156,7 +156,9 @@ describe("exec approval reply helpers", () => { }); expect(payload.text).toContain("```txt\n/approve slug-always allow-once\n```"); expect(payload.text).not.toContain("allow-always"); - expect(payload.text).toContain("Allow Always is unavailable"); + expect(payload.text).toContain( + "The effective approval policy requires approval every time, so Allow Always is unavailable.", + ); expect(payload.interactive).toEqual({ blocks: [ { diff --git a/src/infra/exec-approval-reply.ts b/src/infra/exec-approval-reply.ts index 4f2734da50d..a5362c3d26e 100644 --- a/src/infra/exec-approval-reply.ts +++ b/src/infra/exec-approval-reply.ts @@ -267,7 +267,9 @@ export function buildExecApprovalPendingReplyPayload( lines.push(secondaryFence); } if (!allowedDecisions.includes("allow-always")) { - lines.push("Host policy requires approval every time, so Allow Always is unavailable."); + lines.push( + "The effective approval policy requires approval every time, so Allow Always is unavailable.", + ); } const info: string[] = []; info.push(`Host: ${params.host}`);