vultr/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-05-06 21:30:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: trim before agent hook PR scope

Browse Source
This commit is contained in:
Jesse Merhi
2026-05-05 11:55:25 +10:00
committed by clawsweeper
parent 5e64394239
commit 1ea050c543
23 changed files with 88 additions and 482 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
apps/ios/Sources/Gateway/ExecApprovalPromptDialog.swift
View File

@@ -104,16 +104,14 @@ private struct ExecApprovalPromptCard: View {
}
VStack(spacing: 10) {
if self.prompt.allowsAllowOnce {
Button {
self.onAllowOnce()
} label: {
Text("Allow Once")
.frame(maxWidth: .infinity)
}
.buttonStyle(.borderedProminent)
.disabled(self.isResolving)
Button {
self.onAllowOnce()
} label: {
Text("Allow Once")
.frame(maxWidth: .infinity)
}
.buttonStyle(.borderedProminent)
.disabled(self.isResolving)
if self.prompt.allowsAllowAlways {
Button {
@@ -127,16 +125,14 @@ private struct ExecApprovalPromptCard: View {
}
HStack(spacing: 10) {
if self.prompt.allowsDeny {
Button(role: .destructive) {
self.onDeny()
} label: {
Text("Deny")
.frame(maxWidth: .infinity)
}
.buttonStyle(.bordered)
.disabled(self.isResolving)
Button(role: .destructive) {
self.onDeny()
} label: {
Text("Deny")
.frame(maxWidth: .infinity)
}
.buttonStyle(.bordered)
.disabled(self.isResolving)
Button(role: .cancel) {
self.onCancel()

8
apps/ios/Sources/Model/NodeAppModel.swift
View File

@@ -71,17 +71,9 @@ final class NodeAppModel {
let agentId: String?
let expiresAtMs: Int?
var allowsAllowOnce: Bool {
self.allowedDecisions.contains("allow-once")
}
var allowsAllowAlways: Bool {
self.allowedDecisions.contains("allow-always")
}
var allowsDeny: Bool {
self.allowedDecisions.contains("deny")
}
}
private enum ExecApprovalResolutionOutcome {

4
apps/ios/Tests/NodeAppModelInvokeTests.swift
View File

@@ -212,9 +212,7 @@ private final class MockBootstrapNotificationCenter: NotificationCentering, @unc
let firstPrompt = try #require(appModel._test_pendingExecApprovalPrompt())
#expect(firstPrompt.id == "approval-1")
#expect(firstPrompt.commandText == "echo first")
#expect(firstPrompt.allowsAllowOnce)
#expect(firstPrompt.allowsAllowAlways == false)
#expect(firstPrompt.allowsDeny)
appModel._test_presentExecApprovalPrompt(
try #require(
@@ -230,9 +228,7 @@ private final class MockBootstrapNotificationCenter: NotificationCentering, @unc
let secondPrompt = try #require(appModel._test_pendingExecApprovalPrompt())
#expect(secondPrompt.id == "approval-2")
#expect(secondPrompt.commandText == "echo second")
#expect(secondPrompt.allowsAllowOnce)
#expect(secondPrompt.allowsAllowAlways)
#expect(secondPrompt.allowsDeny)
appModel._test_dismissPendingExecApprovalPrompt()
#expect(appModel._test_pendingExecApprovalPrompt() == nil)

50
apps/macos/Sources/OpenClaw/ExecApprovalsSocket.swift
View File

@@ -14,7 +14,6 @@ struct ExecApprovalPromptRequest: Codable {
var agentId: String?
var resolvedPath: String?
var sessionKey: String?
var allowedDecisions: [ExecApprovalDecision]?
}
private struct ExecApprovalSocketRequest: Codable {
@@ -236,44 +235,21 @@ enum ExecApprovalsPromptPresenter {
alert.informativeText = "Review the command details before allowing."
alert.accessoryView = self.buildAccessoryView(request)
let decisions = self.renderedDecisions(request)
if decisions.isEmpty {
alert.addButton(withTitle: "Allow Once")
alert.addButton(withTitle: "Always Allow")
alert.addButton(withTitle: "Don't Allow")
if #available(macOS 11.0, *), alert.buttons.indices.contains(2) {
alert.buttons[2].hasDestructiveAction = true
}
switch alert.runModal() {
case .alertFirstButtonReturn:
return .allowOnce
case .alertSecondButtonReturn:
return .allowAlways
default:
return .deny
}
for decision in decisions {
alert.addButton(withTitle: self.buttonTitle(decision))
}
if #available(macOS 11.0, *) {
for (index, decision) in decisions.enumerated()
where decision == .deny && alert.buttons.indices.contains(index)
{
alert.buttons[index].hasDestructiveAction = true
}
}
let response = alert.runModal()
let selectedIndex = response.rawValue - NSApplication.ModalResponse.alertFirstButtonReturn.rawValue
if decisions.indices.contains(selectedIndex) {
return decisions[selectedIndex]
}
return .deny
}
private static func renderedDecisions(_ request: ExecApprovalPromptRequest) -> [ExecApprovalDecision] {
let defaults: [ExecApprovalDecision] = [.allowOnce, .allowAlways, .deny]
let allowed = request.allowedDecisions ?? defaults
return defaults.filter { allowed.contains($0) }
}
private static func buttonTitle(_ decision: ExecApprovalDecision) -> String {
switch decision {
case .allowOnce:
"Allow Once"
case .allowAlways:
"Always Allow"
case .deny:
"Don't Allow"
}
}
@MainActor

4
apps/macos/Sources/OpenClawProtocol/GatewayModels.swift
View File

@@ -5233,7 +5233,6 @@ public struct PluginApprovalRequestParams: Codable, Sendable {
public let turnsourceto: String?
public let turnsourceaccountid: String?
public let turnsourcethreadid: AnyCodable?
public let alloweddecisions: [String]?
public let timeoutms: Int?
public let twophase: Bool?
@@ -5250,7 +5249,6 @@ public struct PluginApprovalRequestParams: Codable, Sendable {
turnsourceto: String?,
turnsourceaccountid: String?,
turnsourcethreadid: AnyCodable?,
alloweddecisions: [String]? = nil,
timeoutms: Int?,
twophase: Bool?)
{
@@ -5266,7 +5264,6 @@ public struct PluginApprovalRequestParams: Codable, Sendable {
self.turnsourceto = turnsourceto
self.turnsourceaccountid = turnsourceaccountid
self.turnsourcethreadid = turnsourcethreadid
self.alloweddecisions = alloweddecisions
self.timeoutms = timeoutms
self.twophase = twophase
}
@@ -5284,7 +5281,6 @@ public struct PluginApprovalRequestParams: Codable, Sendable {
case turnsourceto = "turnSourceTo"
case turnsourceaccountid = "turnSourceAccountId"
case turnsourcethreadid = "turnSourceThreadId"
case alloweddecisions = "allowedDecisions"
case timeoutms = "timeoutMs"
case twophase = "twoPhase"
}

4
apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift
View File

@@ -5233,7 +5233,6 @@ public struct PluginApprovalRequestParams: Codable, Sendable {
public let turnsourceto: String?
public let turnsourceaccountid: String?
public let turnsourcethreadid: AnyCodable?
public let alloweddecisions: [String]?
public let timeoutms: Int?
public let twophase: Bool?
@@ -5250,7 +5249,6 @@ public struct PluginApprovalRequestParams: Codable, Sendable {
turnsourceto: String?,
turnsourceaccountid: String?,
turnsourcethreadid: AnyCodable?,
alloweddecisions: [String]? = nil,
timeoutms: Int?,
twophase: Bool?)
{
@@ -5266,7 +5264,6 @@ public struct PluginApprovalRequestParams: Codable, Sendable {
self.turnsourceto = turnsourceto
self.turnsourceaccountid = turnsourceaccountid
self.turnsourcethreadid = turnsourcethreadid
self.alloweddecisions = alloweddecisions
self.timeoutms = timeoutms
self.twophase = twophase
}
@@ -5284,7 +5281,6 @@ public struct PluginApprovalRequestParams: Codable, Sendable {
case turnsourceto = "turnSourceTo"
case turnsourceaccountid = "turnSourceAccountId"
case turnsourcethreadid = "turnSourceThreadId"
case alloweddecisions = "allowedDecisions"
case timeoutms = "timeoutMs"
case twophase = "twoPhase"
}

1
scripts/protocol-gen-swift.ts
View File

@@ -35,7 +35,6 @@ const header = `// Generated by scripts/protocol-gen-swift.ts — do not edit by
const OPTIONAL_INIT_DEFAULTS = new Map<string, Set<string>>([
["ChatHistoryParams", new Set(["includeblockedoriginalcontent"])],
["PluginApprovalRequestParams", new Set(["alloweddecisions"])],
]);
const reserved = new Set([

3
src/agents/tools/gateway.ts
View File

@@ -7,7 +7,6 @@ import {
type OperatorScope,
} from "../../gateway/method-scopes.js";
import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../../gateway/protocol/client-info.js";
import type { DeviceIdentity } from "../../infra/device-identity.js";
import { formatErrorMessage } from "../../infra/errors.js";
import {
normalizeLowercaseStringOrEmpty,
@@ -21,7 +20,6 @@ export type GatewayCallOptions = {
gatewayUrl?: string;
gatewayToken?: string;
timeoutMs?: number;
deviceIdentity?: DeviceIdentity | null;
};
type GatewayOverrideTarget = "local" | "remote";
@@ -167,7 +165,6 @@ export async function callGatewayTool<T = Record<string, unknown>>(
clientName: GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT,
clientDisplayName: "agent",
mode: GATEWAY_CLIENT_MODES.BACKEND,
deviceIdentity: opts.deviceIdentity,
scopes,
});
}

57
src/config/sessions/transcript-append.ts
View File

@@ -26,10 +26,6 @@ type TranscriptLeafInfo = {
nonSessionEntryCount: number;
};
export type TranscriptRawAppendParentLink = {
parentId?: string | null;
};
async function yieldTranscriptAppendScan(): Promise<void> {
await new Promise<void>((resolve) => setImmediate(resolve));
}
@@ -233,32 +229,6 @@ async function withTranscriptAppendQueue<T>(
}
}
export async function resolveTranscriptRawAppendParentLink(params: {
transcriptPath: string;
useRawWhenLinear?: boolean;
}): Promise<TranscriptRawAppendParentLink> {
const stat = await fs.stat(params.transcriptPath).catch(() => null);
let leafInfo: TranscriptLeafInfo = await readTranscriptLeafInfo(params.transcriptPath).catch(
() => ({
hasParentLinkedEntries: false,
nonSessionEntryCount: 0,
}),
);
const hasLinearEntries = !leafInfo.hasParentLinkedEntries && leafInfo.nonSessionEntryCount > 0;
const allowRawWhenLinear = params.useRawWhenLinear !== false;
const shouldRawAppend =
allowRawWhenLinear && hasLinearEntries && (stat?.size ?? 0) > SESSION_MANAGER_APPEND_MAX_BYTES;
if (hasLinearEntries && !shouldRawAppend) {
const migrated = await migrateLinearTranscriptToParentLinked(params.transcriptPath);
leafInfo = {
...(migrated.leafId ? { leafId: migrated.leafId } : {}),
hasParentLinkedEntries: Boolean(migrated.leafId),
nonSessionEntryCount: leafInfo.nonSessionEntryCount,
};
}
return shouldRawAppend ? {} : { parentId: leafInfo.leafId ?? null };
}
export async function appendSessionTranscriptMessage(params: {
transcriptPath: string;
message: unknown;
@@ -294,14 +264,31 @@ async function appendSessionTranscriptMessageLocked(params: {
...(params.sessionId ? { sessionId: params.sessionId } : {}),
...(params.cwd ? { cwd: params.cwd } : {}),
});
const parentLink = await resolveTranscriptRawAppendParentLink({
transcriptPath: params.transcriptPath,
useRawWhenLinear: params.useRawWhenLinear,
});
const stat = await fs.stat(params.transcriptPath).catch(() => null);
let leafInfo: TranscriptLeafInfo = await readTranscriptLeafInfo(params.transcriptPath).catch(
() => ({
hasParentLinkedEntries: false,
nonSessionEntryCount: 0,
}),
);
const hasLinearEntries = !leafInfo.hasParentLinkedEntries && leafInfo.nonSessionEntryCount > 0;
const allowRawWhenLinear = params.useRawWhenLinear !== false;
const shouldRawAppend =
allowRawWhenLinear &&
hasLinearEntries &&
(stat?.size ?? 0) > SESSION_MANAGER_APPEND_MAX_BYTES;
if (hasLinearEntries && !shouldRawAppend) {
const migrated = await migrateLinearTranscriptToParentLinked(params.transcriptPath);
leafInfo = {
...(migrated.leafId ? { leafId: migrated.leafId } : {}),
hasParentLinkedEntries: Boolean(migrated.leafId),
nonSessionEntryCount: leafInfo.nonSessionEntryCount,
};
}
const entry = {
type: "message",
id: messageId,
...parentLink,
...(shouldRawAppend ? {} : { parentId: leafInfo.leafId ?? null }),
timestamp: new Date(now).toISOString(),
message: params.message,
};

2
src/config/sessions/transcript.ts
View File

@@ -1,7 +1,7 @@
import fs from "node:fs";
import path from "node:path";
import type { SessionManager } from "@mariozechner/pi-coding-agent";
import { type SessionWriteLockAcquireTimeoutConfig } from "../../agents/session-write-lock.js";
import type { SessionWriteLockAcquireTimeoutConfig } from "../../agents/session-write-lock.js";
import { formatErrorMessage } from "../../infra/errors.js";
import { emitSessionTranscriptUpdate } from "../../sessions/transcript-events.js";
import { extractAssistantVisibleText } from "../../shared/chat-message-content.js";

3
src/gateway/protocol/schema/plugin-approvals.ts
View File

@@ -20,9 +20,6 @@ export const PluginApprovalRequestParamsSchema = Type.Object(
turnSourceTo: Type.Optional(Type.String()),
turnSourceAccountId: Type.Optional(Type.String()),
turnSourceThreadId: Type.Optional(Type.Union([Type.String(), Type.Number()])),
allowedDecisions: Type.Optional(
Type.Array(Type.String({ enum: ["allow-once", "allow-always", "deny"] })),
),
timeoutMs: Type.Optional(Type.Integer({ minimum: 1, maximum: MAX_PLUGIN_APPROVAL_TIMEOUT_MS })),
twoPhase: Type.Optional(Type.Boolean()),
},

194
src/gateway/server-methods/plugin-approval.test.ts
View File

@@ -125,145 +125,6 @@ describe("createPluginApprovalHandlers", () => {
);
});
it("preserves explicit allowed decisions on plugin approval requests", async () => {
const handlers = createPluginApprovalHandlers(manager);
const respond = vi.fn();
const opts = createMockOptions(
"plugin.approval.request",
{
title: "Sensitive action",
description: "This tool modifies production data",
severity: "warning",
allowedDecisions: ["allow-once", "deny"],
twoPhase: true,
},
{ respond },
);
const handlerPromise = handlers["plugin.approval.request"](opts);
await vi.waitFor(() => {
expect(opts.context.broadcast).toHaveBeenCalledWith(
"plugin.approval.requested",
expect.objectContaining({
request: expect.objectContaining({
allowedDecisions: ["allow-once", "deny"],
}),
}),
{ dropIfSlow: true },
);
});
const acceptedCall = respond.mock.calls.find(
(c) => (c[1] as Record<string, unknown>)?.status === "accepted",
);
const approvalId = (acceptedCall?.[1] as Record<string, unknown>)?.id as string;
manager.resolve(approvalId, "allow-once");
await handlerPromise;
});
it("keeps deny available on restricted plugin approval requests", async () => {
const handlers = createPluginApprovalHandlers(manager);
const respond = vi.fn();
const opts = createMockOptions(
"plugin.approval.request",
{
title: "Sensitive action",
description: "This tool modifies production data",
severity: "warning",
allowedDecisions: ["allow-once"],
twoPhase: true,
},
{ respond },
);
const handlerPromise = handlers["plugin.approval.request"](opts);
await vi.waitFor(() => {
expect(opts.context.broadcast).toHaveBeenCalledWith(
"plugin.approval.requested",
expect.objectContaining({
request: expect.objectContaining({
allowedDecisions: ["allow-once", "deny"],
}),
}),
{ dropIfSlow: true },
);
});
const acceptedCall = respond.mock.calls.find(
(c) => (c[1] as Record<string, unknown>)?.status === "accepted",
);
const approvalId = (acceptedCall?.[1] as Record<string, unknown>)?.id as string;
manager.resolve(approvalId, "deny");
await handlerPromise;
expect(respond).toHaveBeenLastCalledWith(
true,
expect.objectContaining({ id: approvalId, decision: "deny" }),
undefined,
);
});
it("rejects explicit empty allowed decisions on plugin approval requests", async () => {
const handlers = createPluginApprovalHandlers(manager);
const respond = vi.fn();
const opts = createMockOptions(
"plugin.approval.request",
{
title: "Sensitive action",
description: "This tool modifies production data",
severity: "warning",
allowedDecisions: [],
twoPhase: true,
},
{ respond },
);
await handlers["plugin.approval.request"](opts);
expect(respond).toHaveBeenCalledWith(
false,
undefined,
expect.objectContaining({
code: "INVALID_REQUEST",
message: "allowedDecisions must include at least one supported decision",
}),
);
expect(opts.context.broadcast).not.toHaveBeenCalledWith(
"plugin.approval.requested",
expect.anything(),
expect.anything(),
);
});
it("rejects invalid-only allowed decisions on plugin approval requests", async () => {
const handlers = createPluginApprovalHandlers(manager);
const respond = vi.fn();
const opts = createMockOptions(
"plugin.approval.request",
{
title: "Sensitive action",
description: "This tool modifies production data",
severity: "warning",
allowedDecisions: ["forever"],
},
{ respond },
);
await handlers["plugin.approval.request"](opts);
expect(respond).toHaveBeenCalledWith(
false,
undefined,
expect.objectContaining({
code: "INVALID_REQUEST",
message: expect.stringContaining("invalid plugin.approval.request params"),
}),
);
});
it("expires immediately when no approval route", async () => {
const handlers = createPluginApprovalHandlers(manager);
const opts = createMockOptions(
@@ -602,61 +463,6 @@ describe("createPluginApprovalHandlers", () => {
);
});
it("rejects decisions excluded by plugin approval allowedDecisions", 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: "decision is not allowed for this plugin approval request",
}),
);
expect(opts.context.broadcast).not.toHaveBeenCalledWith(
"plugin.approval.resolved",
expect.anything(),
expect.anything(),
);
});
it("rejects decisions when plugin approval allowedDecisions is explicit empty", async () => {
const handlers = createPluginApprovalHandlers(manager);
const record = manager.create({ title: "T", description: "D", allowedDecisions: [] }, 60_000);
void manager.register(record, 60_000);
const opts = createMockOptions("plugin.approval.resolve", {
id: record.id,
decision: "allow-once",
});
await handlers["plugin.approval.resolve"](opts);
expect(opts.respond).toHaveBeenCalledWith(
false,
undefined,
expect.objectContaining({
code: "INVALID_REQUEST",
message: "decision is not allowed for this plugin approval request",
}),
);
expect(opts.context.broadcast).not.toHaveBeenCalledWith(
"plugin.approval.resolved",
expect.anything(),
expect.anything(),
);
});
it("rejects unknown approval id", async () => {
const handlers = createPluginApprovalHandlers(manager);
const opts = createMockOptions("plugin.approval.resolve", {

34
src/gateway/server-methods/plugin-approval.ts
View File

@@ -1,9 +1,6 @@
import { randomUUID } from "node:crypto";
import type { ExecApprovalForwarder } from "../../infra/exec-approval-forwarder.js";
import {
resolveExecApprovalRequestAllowedDecisions,
type ExecApprovalDecision,
} from "../../infra/exec-approvals.js";
import type { ExecApprovalDecision } from "../../infra/exec-approvals.js";
import type { PluginApprovalRequestPayload } from "../../infra/plugin-approvals.js";
import {
DEFAULT_PLUGIN_APPROVAL_TIMEOUT_MS,
@@ -70,7 +67,6 @@ export function createPluginApprovalHandlers(
turnSourceTo?: string | null;
turnSourceAccountId?: string | null;
turnSourceThreadId?: string | number | null;
allowedDecisions?: string[];
timeoutMs?: number;
twoPhase?: boolean;
};
@@ -82,26 +78,6 @@ export function createPluginApprovalHandlers(
const normalizeTrimmedString = (value?: string | null): string | null =>
normalizeOptionalString(value) || null;
const rawAllowedDecisions = p.allowedDecisions;
const hasExplicitAllowedDecisions = Array.isArray(rawAllowedDecisions);
const allowedDecisions = hasExplicitAllowedDecisions
? rawAllowedDecisions.filter(isApprovalDecision)
: [];
if (hasExplicitAllowedDecisions && allowedDecisions.length === 0) {
respond(
false,
undefined,
errorShape(
ErrorCodes.INVALID_REQUEST,
"allowedDecisions must include at least one supported decision",
),
);
return;
}
const effectiveAllowedDecisions =
hasExplicitAllowedDecisions && !allowedDecisions.includes("deny")
? [...allowedDecisions, "deny" as const]
: allowedDecisions;
const request: PluginApprovalRequestPayload = {
pluginId: p.pluginId ?? null,
@@ -116,7 +92,6 @@ export function createPluginApprovalHandlers(
turnSourceTo: normalizeTrimmedString(p.turnSourceTo),
turnSourceAccountId: normalizeTrimmedString(p.turnSourceAccountId),
turnSourceThreadId: p.turnSourceThreadId ?? null,
...(hasExplicitAllowedDecisions ? { allowedDecisions: effectiveAllowedDecisions } : {}),
};
// Always server-generate the ID — never accept plugin-provided IDs.
@@ -191,19 +166,14 @@ export function createPluginApprovalHandlers(
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "invalid decision"));
return;
}
const decision: ExecApprovalDecision = p.decision;
await handleApprovalResolve({
manager,
inputId: p.id,
decision,
decision: p.decision,
respond,
context,
client,
exposeAmbiguousPrefixError: false,
validateDecision: (snapshot) =>
resolveExecApprovalRequestAllowedDecisions(snapshot.request).includes(decision)
? null
: { message: "decision is not allowed for this plugin approval request" },
resolvedEventName: "plugin.approval.resolved",
buildResolvedEvent: ({ approvalId, decision, resolvedBy, snapshot, nowMs }) => ({
id: approvalId,

13
src/infra/approval-native-route-notice.test.ts
View File

@@ -1,7 +1,6 @@
import { describe, expect, it } from "vitest";
import {
describeApprovalDeliveryDestination,
resolveApprovalDeliveryFailedNoticeText,
resolveApprovalRoutedElsewhereNoticeText,
} from "./approval-native-route-notice.js";
@@ -50,15 +49,3 @@ describe("resolveApprovalRoutedElsewhereNoticeText", () => {
expect(resolveApprovalRoutedElsewhereNoticeText([])).toBeNull();
});
});
describe("resolveApprovalDeliveryFailedNoticeText", () => {
it("does not invent fallback decisions for explicit empty restrictions", () => {
expect(
resolveApprovalDeliveryFailedNoticeText({
approvalId: "approval-1",
approvalKind: "plugin",
allowedDecisions: [],
}),
).toContain("No reply decisions are currently available");
});
});

17
src/infra/approval-native-route-notice.ts
View File

@@ -34,18 +34,11 @@ export function resolveApprovalDeliveryFailedNoticeText(params: {
params.approvalKind === "exec" && params.approvalId.length > 8
? params.approvalId.slice(0, 8)
: params.approvalId;
const allowedDecisions = params.allowedDecisions;
const hasExplicitAllowedDecisions = allowedDecisions !== undefined;
const decisions = hasExplicitAllowedDecisions
? allowedDecisions.join("|")
: ["allow-once", "allow-always", "deny"].join("|");
if (!decisions) {
return [
"Approval required. I could not deliver the native approval request.",
"No reply decisions are currently available for this approval.",
"Try again from Control UI or cancel the run.",
].join("\n");
}
const decisions = (
params.allowedDecisions?.length
? params.allowedDecisions
: ["allow-once", "allow-always", "deny"]
).join("|");
return [
"Approval required. I could not deliver the native approval request.",
`Reply with: /approve ${commandId} ${decisions}`,

1
src/infra/approval-view-model.ts
View File

@@ -105,7 +105,6 @@ export function buildPendingApprovalView(request: ApprovalRequest): PendingAppro
...buildPluginViewBase(pluginRequest, "pending"),
actions: buildExecApprovalActionDescriptors({
approvalCommandId: pluginRequest.id,
allowedDecisions: resolveExecApprovalRequestAllowedDecisions(pluginRequest.request),
}),
expiresAtMs: pluginRequest.expiresAtMs,
};

1
src/infra/exec-approval-forwarder.ts
View File

@@ -461,7 +461,6 @@ function buildPluginPendingPayload(params: {
request: params.request,
nowMs: params.nowMs,
text: buildPluginApprovalRequestMessage(params.request, params.nowMs),
allowedDecisions: resolveExecApprovalRequestAllowedDecisions(params.request.request),
}),
});
}

14
src/infra/exec-approvals.ts
View File

@@ -1246,13 +1246,13 @@ export function resolveExecApprovalRequestAllowedDecisions(params?: {
ask?: string | null;
allowedDecisions?: readonly ExecApprovalDecision[] | readonly string[] | null;
}): readonly ExecApprovalDecision[] {
if (Array.isArray(params?.allowedDecisions)) {
return params.allowedDecisions.filter(
(decision): decision is ExecApprovalDecision =>
decision === "allow-once" || decision === "allow-always" || decision === "deny",
);
}
return resolveExecApprovalAllowedDecisions({ ask: params?.ask });
const explicit = Array.isArray(params?.allowedDecisions)
? params.allowedDecisions.filter(
(decision): decision is ExecApprovalDecision =>
decision === "allow-once" || decision === "allow-always" || decision === "deny",
)
: [];
return explicit.length > 0 ? explicit : resolveExecApprovalAllowedDecisions({ ask: params?.ask });
}
export function isExecApprovalDecisionAllowed(params: {

9
src/infra/plugin-approvals.ts
View File

@@ -1,7 +1,4 @@
import {
resolveExecApprovalRequestAllowedDecisions,
type ExecApprovalDecision,
} from "./exec-approvals.js";
import type { ExecApprovalDecision } from "./exec-approvals.js";
export type PluginApprovalRequestPayload = {
pluginId?: string | null;
@@ -16,7 +13,6 @@ export type PluginApprovalRequestPayload = {
turnSourceTo?: string | null;
turnSourceAccountId?: string | null;
turnSourceThreadId?: string | number | null;
allowedDecisions?: readonly ExecApprovalDecision[];
};
export type PluginApprovalRequest = {
@@ -71,8 +67,7 @@ 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`);
const decisions = resolveExecApprovalRequestAllowedDecisions(request.request);
lines.push(`Reply with: /approve <id> ${decisions.join("|")}`);
lines.push("Reply with: /approve <id> allow-once|allow-always|deny");
return lines.join("\n");
}

22
ui/src/ui/controllers/exec-approval.test.ts
View File

@@ -29,7 +29,6 @@ describe("parsePluginApprovalRequested", () => {
pluginId: "sage",
agentId: "agent-1",
sessionKey: "sess-1",
allowedDecisions: ["allow-once", "deny"],
},
};
@@ -42,33 +41,12 @@ describe("parsePluginApprovalRequested", () => {
expect(result!.pluginSeverity).toBe("high");
expect(result!.pluginId).toBe("sage");
expect(result!.request.command).toBe("Dangerous command detected");
expect(result!.request.allowedDecisions).toEqual(["allow-once", "deny"]);
expect(result!.request.agentId).toBe("agent-1");
expect(result!.request.sessionKey).toBe("sess-1");
expect(result!.createdAtMs).toBe(1000);
expect(result!.expiresAtMs).toBe(120_000);
});
it("preserves an explicitly empty allowedDecisions list", () => {
const result = parsePluginApprovalRequested({
...validPayload,
request: { ...validPayload.request, allowedDecisions: [] },
});
expect(result).not.toBeNull();
expect(result!.request.allowedDecisions).toEqual([]);
});
it("drops invalid allowedDecisions without falling back to all actions", () => {
const result = parsePluginApprovalRequested({
...validPayload,
request: { ...validPayload.request, allowedDecisions: ["bad-decision"] },
});
expect(result).not.toBeNull();
expect(result!.request.allowedDecisions).toEqual([]);
});
it("returns null when title is missing from request", () => {
const {
request: { title: _, ...restRequest },

16
ui/src/ui/controllers/exec-approval.ts
View File

@@ -1,14 +1,11 @@
import { normalizeOptionalString } from "../string-coerce.ts";
export type ExecApprovalDecision = "allow-once" | "allow-always" | "deny";
export type ExecApprovalRequestPayload = {
command: string;
cwd?: string | null;
host?: string | null;
security?: string | null;
ask?: string | null;
allowedDecisions?: readonly ExecApprovalDecision[];
agentId?: string | null;
resolvedPath?: string | null;
sessionKey?: string | null;
@@ -37,17 +34,6 @@ function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === "object" && value !== null;
}
function parseAllowedDecisions(value: unknown): ExecApprovalDecision[] | undefined {
if (!Array.isArray(value)) {
return undefined;
}
const decisions = value.filter(
(decision): decision is ExecApprovalDecision =>
decision === "allow-once" || decision === "allow-always" || decision === "deny",
);
return decisions;
}
export function parseExecApprovalRequested(payload: unknown): ExecApprovalRequest | null {
if (!isRecord(payload)) {
return null;
@@ -75,7 +61,6 @@ export function parseExecApprovalRequested(payload: unknown): ExecApprovalReques
host: typeof request.host === "string" ? request.host : null,
security: typeof request.security === "string" ? request.security : null,
ask: typeof request.ask === "string" ? request.ask : null,
allowedDecisions: parseAllowedDecisions(request.allowedDecisions),
agentId: typeof request.agentId === "string" ? request.agentId : null,
resolvedPath: typeof request.resolvedPath === "string" ? request.resolvedPath : null,
sessionKey: typeof request.sessionKey === "string" ? request.sessionKey : null,
@@ -129,7 +114,6 @@ export function parsePluginApprovalRequested(payload: unknown): ExecApprovalRequ
kind: "plugin",
request: {
command: title,
allowedDecisions: parseAllowedDecisions(request.allowedDecisions),
agentId: typeof request.agentId === "string" ? request.agentId : null,
sessionKey: typeof request.sessionKey === "string" ? request.sessionKey : null,
},

26
ui/src/ui/views/exec-approval.test.ts
View File

@@ -152,32 +152,6 @@ describe("approval and confirmation modals", () => {
expect(handleExecApprovalDecision).toHaveBeenCalledWith("deny");
});
it("does not map Escape to denial when deny is not allowed", async () => {
const handleExecApprovalDecision = vi.fn(async () => undefined);
render(
renderExecApprovalPrompt(
createExecState({
execApprovalQueue: [
{
...createExecRequest(),
request: {
...createExecRequest().request,
allowedDecisions: ["allow-once"],
},
},
],
handleExecApprovalDecision,
}),
),
container,
);
const { dialog } = await getRenderedDialog();
dispatchEscape(dialog);
expect(handleExecApprovalDecision).not.toHaveBeenCalled();
});
it("does not dispatch an extra exec decision from Escape while busy", async () => {
const handleExecApprovalDecision = vi.fn(async () => undefined);
render(

55
ui/src/ui/views/exec-approval.ts
View File

@@ -83,13 +83,8 @@ export function renderExecApprovalPrompt(state: AppViewState) {
: t("execApproval.execApprovalNeeded");
const titleId = "exec-approval-title";
const descriptionId = "exec-approval-description";
const allowedDecisions = active.request.allowedDecisions ?? [
"allow-once",
"allow-always",
"deny",
];
const handleCancel = () => {
if (!state.execApprovalBusy && allowedDecisions.includes("deny")) {
if (!state.execApprovalBusy) {
void state.handleExecApprovalDecision("deny");
}
};
@@ -112,33 +107,27 @@ export function renderExecApprovalPrompt(state: AppViewState) {
? html`<div class="exec-approval-error">${state.execApprovalError}</div>`
: nothing}
<div class="exec-approval-actions">
${allowedDecisions.includes("allow-once")
? html`<button
class="btn primary"
?disabled=${state.execApprovalBusy}
@click=${() => state.handleExecApprovalDecision("allow-once")}
>
${t("execApproval.allowOnce")}
</button>`
: nothing}
${allowedDecisions.includes("allow-always")
? html`<button
class="btn"
?disabled=${state.execApprovalBusy}
@click=${() => state.handleExecApprovalDecision("allow-always")}
>
${t("execApproval.alwaysAllow")}
</button>`
: nothing}
${allowedDecisions.includes("deny")
? html`<button
class="btn danger"
?disabled=${state.execApprovalBusy}
@click=${() => state.handleExecApprovalDecision("deny")}
>
${t("execApproval.deny")}
</button>`
: nothing}
<button
class="btn primary"
?disabled=${state.execApprovalBusy}
@click=${() => state.handleExecApprovalDecision("allow-once")}
>
${t("execApproval.allowOnce")}
</button>
<button
class="btn"
?disabled=${state.execApprovalBusy}
@click=${() => state.handleExecApprovalDecision("allow-always")}
>
${t("execApproval.alwaysAllow")}
</button>
<button
class="btn danger"
?disabled=${state.execApprovalBusy}
@click=${() => state.handleExecApprovalDecision("deny")}
>
${t("execApproval.deny")}
</button>
</div>
</div>
</openclaw-modal-dialog>