diff --git a/apps/ios/Sources/Gateway/ExecApprovalPromptDialog.swift b/apps/ios/Sources/Gateway/ExecApprovalPromptDialog.swift index 8c4d747017c..d970b7068a3 100644 --- a/apps/ios/Sources/Gateway/ExecApprovalPromptDialog.swift +++ b/apps/ios/Sources/Gateway/ExecApprovalPromptDialog.swift @@ -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() diff --git a/apps/ios/Sources/Model/NodeAppModel.swift b/apps/ios/Sources/Model/NodeAppModel.swift index 9cba430d32e..60748dacc33 100644 --- a/apps/ios/Sources/Model/NodeAppModel.swift +++ b/apps/ios/Sources/Model/NodeAppModel.swift @@ -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 { diff --git a/apps/ios/Tests/NodeAppModelInvokeTests.swift b/apps/ios/Tests/NodeAppModelInvokeTests.swift index 5b709c725af..5a10a8a43d7 100644 --- a/apps/ios/Tests/NodeAppModelInvokeTests.swift +++ b/apps/ios/Tests/NodeAppModelInvokeTests.swift @@ -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) diff --git a/apps/macos/Sources/OpenClaw/ExecApprovalsSocket.swift b/apps/macos/Sources/OpenClaw/ExecApprovalsSocket.swift index f7461304e0f..98fa9e55d29 100644 --- a/apps/macos/Sources/OpenClaw/ExecApprovalsSocket.swift +++ b/apps/macos/Sources/OpenClaw/ExecApprovalsSocket.swift @@ -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 diff --git a/apps/macos/Sources/OpenClawProtocol/GatewayModels.swift b/apps/macos/Sources/OpenClawProtocol/GatewayModels.swift index b134bc1c58c..31c2535d7eb 100644 --- a/apps/macos/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/macos/Sources/OpenClawProtocol/GatewayModels.swift @@ -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" } diff --git a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift index b134bc1c58c..31c2535d7eb 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift @@ -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" } diff --git a/scripts/protocol-gen-swift.ts b/scripts/protocol-gen-swift.ts index f1e021d90b9..68e9fd64018 100644 --- a/scripts/protocol-gen-swift.ts +++ b/scripts/protocol-gen-swift.ts @@ -35,7 +35,6 @@ const header = `// Generated by scripts/protocol-gen-swift.ts — do not edit by const OPTIONAL_INIT_DEFAULTS = new Map>([ ["ChatHistoryParams", new Set(["includeblockedoriginalcontent"])], - ["PluginApprovalRequestParams", new Set(["alloweddecisions"])], ]); const reserved = new Set([ diff --git a/src/agents/tools/gateway.ts b/src/agents/tools/gateway.ts index 227266b4994..9141db26bbe 100644 --- a/src/agents/tools/gateway.ts +++ b/src/agents/tools/gateway.ts @@ -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>( clientName: GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT, clientDisplayName: "agent", mode: GATEWAY_CLIENT_MODES.BACKEND, - deviceIdentity: opts.deviceIdentity, scopes, }); } diff --git a/src/config/sessions/transcript-append.ts b/src/config/sessions/transcript-append.ts index b597dae5f65..9f88b786427 100644 --- a/src/config/sessions/transcript-append.ts +++ b/src/config/sessions/transcript-append.ts @@ -26,10 +26,6 @@ type TranscriptLeafInfo = { nonSessionEntryCount: number; }; -export type TranscriptRawAppendParentLink = { - parentId?: string | null; -}; - async function yieldTranscriptAppendScan(): Promise { await new Promise((resolve) => setImmediate(resolve)); } @@ -233,32 +229,6 @@ async function withTranscriptAppendQueue( } } -export async function resolveTranscriptRawAppendParentLink(params: { - transcriptPath: string; - useRawWhenLinear?: boolean; -}): Promise { - 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, }; diff --git a/src/config/sessions/transcript.ts b/src/config/sessions/transcript.ts index 4e1c77fe3a2..7ebd91e490e 100644 --- a/src/config/sessions/transcript.ts +++ b/src/config/sessions/transcript.ts @@ -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"; diff --git a/src/gateway/protocol/schema/plugin-approvals.ts b/src/gateway/protocol/schema/plugin-approvals.ts index 04c031c5fa6..4aaa8e12a04 100644 --- a/src/gateway/protocol/schema/plugin-approvals.ts +++ b/src/gateway/protocol/schema/plugin-approvals.ts @@ -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()), }, diff --git a/src/gateway/server-methods/plugin-approval.test.ts b/src/gateway/server-methods/plugin-approval.test.ts index 7f6214c9e0a..0a8e6c95c25 100644 --- a/src/gateway/server-methods/plugin-approval.test.ts +++ b/src/gateway/server-methods/plugin-approval.test.ts @@ -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)?.status === "accepted", - ); - const approvalId = (acceptedCall?.[1] as Record)?.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)?.status === "accepted", - ); - const approvalId = (acceptedCall?.[1] as Record)?.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", { diff --git a/src/gateway/server-methods/plugin-approval.ts b/src/gateway/server-methods/plugin-approval.ts index 824c5e051b7..314bfe3d4ba 100644 --- a/src/gateway/server-methods/plugin-approval.ts +++ b/src/gateway/server-methods/plugin-approval.ts @@ -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, diff --git a/src/infra/approval-native-route-notice.test.ts b/src/infra/approval-native-route-notice.test.ts index 3410596179a..dac2f715fbf 100644 --- a/src/infra/approval-native-route-notice.test.ts +++ b/src/infra/approval-native-route-notice.test.ts @@ -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"); - }); -}); diff --git a/src/infra/approval-native-route-notice.ts b/src/infra/approval-native-route-notice.ts index f166dd0ecc1..ec26e563c4b 100644 --- a/src/infra/approval-native-route-notice.ts +++ b/src/infra/approval-native-route-notice.ts @@ -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}`, diff --git a/src/infra/approval-view-model.ts b/src/infra/approval-view-model.ts index 3d31620867e..4121e729ff6 100644 --- a/src/infra/approval-view-model.ts +++ b/src/infra/approval-view-model.ts @@ -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, }; diff --git a/src/infra/exec-approval-forwarder.ts b/src/infra/exec-approval-forwarder.ts index ebb53d72472..f5bd68252f1 100644 --- a/src/infra/exec-approval-forwarder.ts +++ b/src/infra/exec-approval-forwarder.ts @@ -461,7 +461,6 @@ function buildPluginPendingPayload(params: { request: params.request, nowMs: params.nowMs, text: buildPluginApprovalRequestMessage(params.request, params.nowMs), - allowedDecisions: resolveExecApprovalRequestAllowedDecisions(params.request.request), }), }); } diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index 2f853b74c0e..ebefb855a05 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -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: { diff --git a/src/infra/plugin-approvals.ts b/src/infra/plugin-approvals.ts index 95e3c96cdf1..c701dcf3da8 100644 --- a/src/infra/plugin-approvals.ts +++ b/src/infra/plugin-approvals.ts @@ -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 ${decisions.join("|")}`); + lines.push("Reply with: /approve allow-once|allow-always|deny"); return lines.join("\n"); } diff --git a/ui/src/ui/controllers/exec-approval.test.ts b/ui/src/ui/controllers/exec-approval.test.ts index 12ede771a30..5e913185e81 100644 --- a/ui/src/ui/controllers/exec-approval.test.ts +++ b/ui/src/ui/controllers/exec-approval.test.ts @@ -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 }, diff --git a/ui/src/ui/controllers/exec-approval.ts b/ui/src/ui/controllers/exec-approval.ts index 43cea2f4998..bd44c726939 100644 --- a/ui/src/ui/controllers/exec-approval.ts +++ b/ui/src/ui/controllers/exec-approval.ts @@ -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 { 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, }, diff --git a/ui/src/ui/views/exec-approval.test.ts b/ui/src/ui/views/exec-approval.test.ts index 3a4e2e629ad..c25542490db 100644 --- a/ui/src/ui/views/exec-approval.test.ts +++ b/ui/src/ui/views/exec-approval.test.ts @@ -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( diff --git a/ui/src/ui/views/exec-approval.ts b/ui/src/ui/views/exec-approval.ts index 44a5c68eb7f..ff5b2fed8ab 100644 --- a/ui/src/ui/views/exec-approval.ts +++ b/ui/src/ui/views/exec-approval.ts @@ -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`
${state.execApprovalError}
` : nothing}
- ${allowedDecisions.includes("allow-once") - ? html`` - : nothing} - ${allowedDecisions.includes("allow-always") - ? html`` - : nothing} - ${allowedDecisions.includes("deny") - ? html`` - : nothing} + + +