Skip to content

Commit 4588eb8

Browse files
author
NianJiuZst
committed
fix: explain one-shot exec approvals
1 parent 1cd6f81 commit 4588eb8

65 files changed

Lines changed: 445 additions & 90 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎packages/gateway-protocol/src/schema/exec-approvals.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,9 @@ export const ExecApprovalRequestParamsSchema = Type.Object(
157157
maxItems: 1,
158158
}),
159159
),
160+
allowAlwaysUnavailableReason: Type.Optional(
161+
Type.String({ enum: ["approval-policy-always", "non-persistable-command"] }),
162+
),
160163
commandSpans: Type.Optional(
161164
Type.Array(
162165
Type.Object(

‎src/agents/bash-tools.exec-approval-request.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
normalizeOptionalString as parseString,
1313
} from "@openclaw/normalization-core/string-coerce";
1414
import type {
15+
ExecApprovalAllowAlwaysUnavailableReason,
1516
ExecApprovalCommandSpan,
1617
ExecApprovalUnavailableDecision,
1718
ExecAsk,
@@ -57,6 +58,7 @@ type RequestExecApprovalDecisionParams = {
5758
warningText?: string;
5859
commandSpans?: ExecApprovalCommandSpan[];
5960
unavailableDecisions?: readonly ExecApprovalUnavailableDecision[];
61+
allowAlwaysUnavailableReason?: ExecApprovalAllowAlwaysUnavailableReason | null;
6062
agentId?: string;
6163
resolvedPath?: string;
6264
sessionKey?: string;
@@ -93,6 +95,9 @@ function buildExecApprovalRequestToolParams(
9395
...(params.unavailableDecisions?.length
9496
? { unavailableDecisions: params.unavailableDecisions }
9597
: {}),
98+
...(params.allowAlwaysUnavailableReason
99+
? { allowAlwaysUnavailableReason: params.allowAlwaysUnavailableReason }
100+
: {}),
96101
agentId: params.agentId,
97102
resolvedPath: params.resolvedPath,
98103
sessionKey: params.sessionKey,
@@ -199,6 +204,7 @@ type HostExecApprovalParams = {
199204
warningText?: string;
200205
commandSpans?: ExecApprovalCommandSpan[];
201206
unavailableDecisions?: readonly ExecApprovalUnavailableDecision[];
207+
allowAlwaysUnavailableReason?: ExecApprovalAllowAlwaysUnavailableReason | null;
202208
commandHighlighting?: boolean;
203209
agentId?: string;
204210
resolvedPath?: string;
@@ -309,6 +315,7 @@ async function buildHostApprovalDecisionParams(
309315
warningText: params.warningText,
310316
commandSpans,
311317
unavailableDecisions: params.unavailableDecisions,
318+
allowAlwaysUnavailableReason: params.allowAlwaysUnavailableReason,
312319
...buildExecApprovalRequesterContext({
313320
agentId: params.agentId,
314321
sessionKey: params.sessionKey,

‎src/agents/bash-tools.exec-host-gateway.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,22 @@ const resolveExecApprovalUnavailableDecisionsMock = vi.hoisted(() =>
102102
: [],
103103
),
104104
);
105+
const resolveExecApprovalAllowAlwaysUnavailableReasonMock = vi.hoisted(() =>
106+
vi.fn(
107+
(params?: {
108+
ask?: string | null;
109+
allowAlwaysPersistence?: { kind: string } | null;
110+
}): "approval-policy-always" | "non-persistable-command" | null => {
111+
if (params?.ask === "always") {
112+
return "approval-policy-always";
113+
}
114+
if (params?.allowAlwaysPersistence?.kind === "one-shot") {
115+
return "non-persistable-command";
116+
}
117+
return null;
118+
},
119+
),
120+
);
105121
const buildEnforcedShellCommandMock = vi.hoisted(() =>
106122
vi.fn((): { ok: boolean; reason?: string; command?: string } => ({
107123
ok: false,
@@ -161,6 +177,8 @@ vi.mock("../infra/exec-approvals.js", async (importOriginal) => ({
161177
resolveApprovalAuditTrustPath: vi.fn(() => null),
162178
resolveAllowAlwaysPatterns: vi.fn(() => []),
163179
resolveExecApprovalAllowedDecisions: resolveExecApprovalAllowedDecisionsMock,
180+
resolveExecApprovalAllowAlwaysUnavailableReason:
181+
resolveExecApprovalAllowAlwaysUnavailableReasonMock,
164182
resolveExecApprovalUnavailableDecisions: resolveExecApprovalUnavailableDecisionsMock,
165183
addAllowlistEntry: vi.fn(),
166184
addDurableCommandApproval: vi.fn(),
@@ -811,6 +829,7 @@ describe("processGatewayAllowlist", () => {
811829
expect(buildExecApprovalPendingToolResultMock).toHaveBeenCalledWith(
812830
expect.objectContaining({
813831
allowedDecisions: ["allow-once", "deny"],
832+
allowAlwaysUnavailableReason: "non-persistable-command",
814833
}),
815834
);
816835
});

‎src/agents/bash-tools.exec-host-gateway.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
recordAllowlistMatchesUse,
2525
resolveApprovalAuditTrustPath,
2626
resolveAllowAlwaysPersistenceDecision,
27+
resolveExecApprovalAllowAlwaysUnavailableReason,
2728
resolveExecApprovalUnavailableDecisions,
2829
requiresExecApproval,
2930
} from "../infra/exec-approvals.js";
@@ -597,9 +598,16 @@ export async function processGatewayAllowlist(
597598
ask: hostAsk,
598599
allowAlwaysPersistence: effectiveAllowAlwaysPersistence,
599600
});
601+
const allowAlwaysUnavailableReason = resolveExecApprovalAllowAlwaysUnavailableReason({
602+
ask: hostAsk,
603+
allowAlwaysPersistence: effectiveAllowAlwaysPersistence,
604+
});
600605
const unavailableDecisionRequestParams =
601606
approvalUnavailableDecisions.length > 0
602-
? { unavailableDecisions: approvalUnavailableDecisions }
607+
? {
608+
unavailableDecisions: approvalUnavailableDecisions,
609+
allowAlwaysUnavailableReason,
610+
}
603611
: {};
604612
if (requiresSecurityAuditSuppressionApproval) {
605613
params.warnings.push(
@@ -1015,6 +1023,7 @@ export async function processGatewayAllowlist(
10151023
sentApproverDms,
10161024
unavailableReason,
10171025
allowedDecisions: approvalAllowedDecisions,
1026+
allowAlwaysUnavailableReason,
10181027
}),
10191028
};
10201029
}

‎src/agents/bash-tools.exec-host-node.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,22 @@ const resolveExecApprovalUnavailableDecisionsMock = vi.hoisted(() =>
164164
: [],
165165
),
166166
);
167+
const resolveExecApprovalAllowAlwaysUnavailableReasonMock = vi.hoisted(() =>
168+
vi.fn(
169+
(params?: {
170+
ask?: string | null;
171+
allowAlwaysPersistence?: { kind: string } | null;
172+
}): "approval-policy-always" | "non-persistable-command" | null => {
173+
if (params?.ask === "always") {
174+
return "approval-policy-always";
175+
}
176+
if (params?.allowAlwaysPersistence?.kind === "one-shot") {
177+
return "non-persistable-command";
178+
}
179+
return null;
180+
},
181+
),
182+
);
167183
const resolveExecHostApprovalContextMock = vi.hoisted(() =>
168184
vi.fn(() => ({
169185
approvals: { allowlist: [] as ExecAllowlistEntry[], file: { version: 1, agents: {} } },
@@ -222,6 +238,8 @@ vi.mock("../infra/exec-approvals.js", () => ({
222238
resolveAllowAlwaysPersistenceDecision: resolveAllowAlwaysPersistenceDecisionMock,
223239
resolveAllowAlwaysPatternCoverage: resolveAllowAlwaysPatternCoverageMock,
224240
resolveExecApprovalAllowedDecisions: resolveExecApprovalAllowedDecisionsMock,
241+
resolveExecApprovalAllowAlwaysUnavailableReason:
242+
resolveExecApprovalAllowAlwaysUnavailableReasonMock,
225243
resolveExecApprovalUnavailableDecisions: resolveExecApprovalUnavailableDecisionsMock,
226244
resolveExecApprovalsFromFile: resolveExecApprovalsFromFileMock,
227245
maxAsk: (a: ExecAsk, b: ExecAsk): ExecAsk => {
@@ -1791,9 +1809,13 @@ describe("executeNodeHostCommand", () => {
17911809
},
17921810
});
17931811
expect(requireRegisteredApprovalRequest().unavailableDecisions).toEqual(["allow-always"]);
1812+
expect(requireRegisteredApprovalRequest().allowAlwaysUnavailableReason).toBe(
1813+
"approval-policy-always",
1814+
);
17941815
expect(buildExecApprovalPendingToolResultMock).toHaveBeenCalledWith(
17951816
expect.objectContaining({
17961817
allowedDecisions: ["allow-once", "deny"],
1818+
allowAlwaysUnavailableReason: "approval-policy-always",
17971819
}),
17981820
);
17991821
});
@@ -2247,9 +2269,13 @@ describe("executeNodeHostCommand", () => {
22472269
allowAlwaysPersistence: { kind: "one-shot", reasons: ["unplanned"] },
22482270
});
22492271
expect(requireRegisteredApprovalRequest().unavailableDecisions).toEqual(["allow-always"]);
2272+
expect(requireRegisteredApprovalRequest().allowAlwaysUnavailableReason).toBe(
2273+
"non-persistable-command",
2274+
);
22502275
expect(buildExecApprovalPendingToolResultMock).toHaveBeenCalledWith(
22512276
expect.objectContaining({
22522277
allowedDecisions: ["allow-once", "deny"],
2278+
allowAlwaysUnavailableReason: "non-persistable-command",
22532279
}),
22542280
);
22552281
});

‎src/agents/bash-tools.exec-host-node.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
maxAsk,
1212
requiresExecApproval,
1313
resolveExecApprovalAllowedDecisions,
14+
resolveExecApprovalAllowAlwaysUnavailableReason,
1415
resolveExecApprovalUnavailableDecisions,
1516
} from "../infra/exec-approvals.js";
1617
import { defaultExecAutoReviewer, type ExecAutoReviewInput } from "../infra/exec-auto-review.js";
@@ -142,8 +143,12 @@ export async function executeNodeHostCommand(
142143
ask: approvalDecisionAsk,
143144
allowAlwaysPersistence,
144145
});
146+
const allowAlwaysUnavailableReason = resolveExecApprovalAllowAlwaysUnavailableReason({
147+
ask: approvalDecisionAsk,
148+
allowAlwaysPersistence,
149+
});
145150
const unavailableDecisionRequestParams =
146-
unavailableDecisions.length > 0 ? { unavailableDecisions } : {};
151+
unavailableDecisions.length > 0 ? { unavailableDecisions, allowAlwaysUnavailableReason } : {};
147152
const requiresAsk =
148153
requiresExecApproval({
149154
ask: hostAsk,
@@ -456,6 +461,7 @@ export async function executeNodeHostCommand(
456461
sentApproverDms,
457462
unavailableReason,
458463
allowedDecisions,
464+
allowAlwaysUnavailableReason,
459465
nodeId: target.nodeId,
460466
});
461467
}

‎src/agents/bash-tools.exec-host-shared.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
resolveExecApprovals,
1919
resolveExecApprovalsTranscriptPath,
2020
type ExecAsk,
21+
type ExecApprovalAllowAlwaysUnavailableReason,
2122
type ExecApprovalDecision,
2223
type ExecSecurity,
2324
} from "../infra/exec-approvals.js";
@@ -503,6 +504,7 @@ export function buildExecApprovalPendingToolResult(params: {
503504
sentApproverDms: boolean;
504505
unavailableReason: ExecApprovalUnavailableReason | null;
505506
allowedDecisions?: readonly ExecApprovalDecision[];
507+
allowAlwaysUnavailableReason?: ExecApprovalAllowAlwaysUnavailableReason | null;
506508
nodeId?: string;
507509
}): AgentToolResult<ExecToolDetails> {
508510
const allowedDecisions = params.allowedDecisions ?? resolveExecApprovalAllowedDecisions();
@@ -525,6 +527,7 @@ export function buildExecApprovalPendingToolResult(params: {
525527
approvalSlug: params.approvalSlug,
526528
approvalId: params.approvalId,
527529
allowedDecisions,
530+
allowAlwaysUnavailableReason: params.allowAlwaysUnavailableReason,
528531
command: params.command,
529532
cwd: params.cwd,
530533
host: params.host,
@@ -553,6 +556,7 @@ export function buildExecApprovalPendingToolResult(params: {
553556
approvalSlug: params.approvalSlug,
554557
expiresAtMs: params.expiresAtMs,
555558
allowedDecisions,
559+
allowAlwaysUnavailableReason: params.allowAlwaysUnavailableReason ?? undefined,
556560
host: params.host,
557561
command: params.command,
558562
cwd: params.cwd,

‎src/agents/bash-tools.exec-runtime.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,12 @@ import {
1313
} from "../infra/event-session-routing.js";
1414
import {
1515
DEFAULT_EXEC_APPROVAL_TIMEOUT_MS,
16+
describeExecApprovalAllowAlwaysUnavailableReason,
17+
describeExecApprovalBackgroundModeUnavailableReason,
1618
resolveExecApprovalAllowedDecisions,
17-
type ExecHost,
19+
type ExecApprovalAllowAlwaysUnavailableReason,
1820
type ExecApprovalDecision,
21+
type ExecHost,
1922
type ExecTarget,
2023
} from "../infra/exec-approvals.js";
2124
import { requestHeartbeat } from "../infra/heartbeat-wake.js";
@@ -368,6 +371,7 @@ export function buildApprovalPendingMessage(params: {
368371
approvalSlug: string;
369372
approvalId: string;
370373
allowedDecisions?: readonly ExecApprovalDecision[];
374+
allowAlwaysUnavailableReason?: ExecApprovalAllowAlwaysUnavailableReason | null;
371375
command: string;
372376
cwd: string | undefined;
373377
host: "gateway" | "node";
@@ -397,12 +401,12 @@ export function buildApprovalPendingMessage(params: {
397401
lines.push(
398402
allowedDecisions.includes("allow-always")
399403
? "Background mode requires pre-approved policy (allow-always or ask=off)."
400-
: "Background mode requires an effective policy that allows pre-approval (for example ask=off).",
404+
: describeExecApprovalBackgroundModeUnavailableReason(params.allowAlwaysUnavailableReason),
401405
);
402406
lines.push(`Reply with: /approve ${params.approvalSlug} ${decisionText}`);
403407
if (!allowedDecisions.includes("allow-always")) {
404408
lines.push(
405-
"The effective approval policy requires approval every time, so Allow Always is unavailable.",
409+
describeExecApprovalAllowAlwaysUnavailableReason(params.allowAlwaysUnavailableReason),
406410
);
407411
}
408412
lines.push("If the short code is ambiguous, use the full id in /approve.");

‎src/agents/bash-tools.exec-types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ export type ExecToolDetails =
135135
approvalSlug: string;
136136
expiresAtMs: number;
137137
allowedDecisions?: readonly ExecApprovalDecision[];
138+
allowAlwaysUnavailableReason?: "approval-policy-always" | "non-persistable-command";
138139
host: ExecHost;
139140
command: string;
140141
cwd?: string;

‎src/agents/bash-tools.exec.approval-id.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ function expectPendingApprovalText(
250250
expect(pendingText).toContain(
251251
(options.allowedDecisions ?? "").includes("allow-always")
252252
? "Background mode requires pre-approved policy"
253-
: "Background mode requires an effective policy that allows pre-approval",
253+
: "Background mode note: non-interactive runs cannot wait for chat approvals",
254254
);
255255
}
256256
return details;

0 commit comments

Comments
 (0)