mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-06 05:51:15 +08:00
Fix stale approval prompts in Control UI (#86270)
* fix(ui): clear stale approval prompts * fix(ui): keep approval prompt state current * test: update approval controller mocks * fix(ui): keep escape denying approvals * refactor(ui): keep approval decisions in app
This commit is contained in:
@@ -90,6 +90,8 @@ vi.mock("./controllers/devices.ts", () => ({
|
||||
|
||||
vi.mock("./controllers/exec-approval.ts", () => ({
|
||||
addExecApproval: vi.fn((queue, entry) => [...queue, entry]),
|
||||
clearResolvedExecApprovalPrompt: vi.fn(),
|
||||
enqueueExecApprovalPrompt: vi.fn(),
|
||||
parseExecApprovalRequested: vi.fn(() => null),
|
||||
parseExecApprovalResolved: vi.fn(() => null),
|
||||
parsePluginApprovalRequested: vi.fn(() => null),
|
||||
|
||||
@@ -183,6 +183,7 @@ function createHost(): TestGatewayHost {
|
||||
refreshSessionsAfterChat: new Set<string>(),
|
||||
chatSideResultTerminalRuns: new Set<string>(),
|
||||
execApprovalQueue: [],
|
||||
execApprovalBusy: false,
|
||||
execApprovalError: null,
|
||||
updateAvailable: null,
|
||||
updateComplete: new Promise(() => undefined),
|
||||
@@ -540,6 +541,77 @@ describe("connectGateway", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("clears stale approval modal errors without interrupting an in-flight local resolve", () => {
|
||||
const { host, client } = connectHostGateway();
|
||||
|
||||
client.emitEvent({
|
||||
event: "exec.approval.requested",
|
||||
payload: {
|
||||
id: "approval-external-1",
|
||||
request: {
|
||||
command: "pnpm test",
|
||||
host: "gateway",
|
||||
},
|
||||
createdAtMs: Date.now(),
|
||||
expiresAtMs: Date.now() + 60_000,
|
||||
},
|
||||
});
|
||||
host.execApprovalError = "Approval failed: approval already resolved";
|
||||
host.execApprovalBusy = true;
|
||||
|
||||
client.emitEvent({
|
||||
event: "exec.approval.resolved",
|
||||
payload: { id: "approval-external-1", decision: "allow-once" },
|
||||
});
|
||||
|
||||
expect(host.execApprovalQueue).toHaveLength(0);
|
||||
expect(host.execApprovalError).toBeNull();
|
||||
expect(host.execApprovalBusy).toBe(true);
|
||||
});
|
||||
|
||||
it("clears active approval errors when event-enqueued approvals expire", () => {
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date("2026-05-25T00:00:00.000Z"));
|
||||
try {
|
||||
const { host, client } = connectHostGateway();
|
||||
const queuedExpiresAtMs = Date.now() + 60_000;
|
||||
const activeExpiresAtMs = Date.now() + 1_000;
|
||||
|
||||
client.emitEvent({
|
||||
event: "exec.approval.requested",
|
||||
payload: {
|
||||
id: "approval-queued",
|
||||
request: {
|
||||
command: "pnpm test",
|
||||
host: "gateway",
|
||||
},
|
||||
createdAtMs: Date.now(),
|
||||
expiresAtMs: queuedExpiresAtMs,
|
||||
},
|
||||
});
|
||||
client.emitEvent({
|
||||
event: "exec.approval.requested",
|
||||
payload: {
|
||||
id: "approval-active-expiring",
|
||||
request: {
|
||||
command: "pnpm check:changed",
|
||||
host: "gateway",
|
||||
},
|
||||
createdAtMs: Date.now() + 1,
|
||||
expiresAtMs: activeExpiresAtMs,
|
||||
},
|
||||
});
|
||||
host.execApprovalError = "Approval failed: Error: gateway unavailable";
|
||||
|
||||
vi.advanceTimersByTime(1_500);
|
||||
|
||||
expect(host.execApprovalQueue.map((entry) => entry.id)).toEqual(["approval-queued"]);
|
||||
expect(host.execApprovalError).toBeNull();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("clears pending session reload timers when the active client closes", () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
|
||||
@@ -44,8 +44,11 @@ vi.mock("./controllers/devices.ts", () => ({
|
||||
}));
|
||||
vi.mock("./controllers/exec-approval.ts", () => ({
|
||||
addExecApproval: vi.fn(),
|
||||
clearResolvedExecApprovalPrompt: vi.fn(),
|
||||
enqueueExecApprovalPrompt: vi.fn(),
|
||||
parseExecApprovalRequested: vi.fn(() => null),
|
||||
parseExecApprovalResolved: vi.fn(() => null),
|
||||
parsePluginApprovalRequested: vi.fn(() => null),
|
||||
pruneExecApprovalQueue: vi.fn((queue) => queue),
|
||||
removeExecApproval: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -44,12 +44,12 @@ import { loadControlUiBootstrapConfig } from "./controllers/control-ui-bootstrap
|
||||
import { loadDevices, type DevicesState } from "./controllers/devices.ts";
|
||||
import type { ExecApprovalRequest } from "./controllers/exec-approval.ts";
|
||||
import {
|
||||
addExecApproval,
|
||||
clearResolvedExecApprovalPrompt,
|
||||
enqueueExecApprovalPrompt,
|
||||
parseExecApprovalRequested,
|
||||
parseExecApprovalResolved,
|
||||
parsePluginApprovalRequested,
|
||||
pruneExecApprovalQueue,
|
||||
removeExecApproval,
|
||||
} from "./controllers/exec-approval.ts";
|
||||
import { loadHealthState, type HealthState } from "./controllers/health.ts";
|
||||
import {
|
||||
@@ -121,6 +121,7 @@ type GatewayHost = {
|
||||
refreshSessionsAfterChat: Set<string>;
|
||||
sessionsLoading?: boolean;
|
||||
execApprovalQueue: ExecApprovalRequest[];
|
||||
execApprovalBusy: boolean;
|
||||
execApprovalError: string | null;
|
||||
updateAvailable: UpdateAvailable | null;
|
||||
reconcileWebPushState?: () => Promise<void> | void;
|
||||
@@ -156,18 +157,13 @@ function enqueueApprovalRequest(host: GatewayHost, entry: ExecApprovalRequest |
|
||||
if (!entry) {
|
||||
return;
|
||||
}
|
||||
host.execApprovalQueue = addExecApproval(host.execApprovalQueue, entry);
|
||||
host.execApprovalError = null;
|
||||
const delay = Math.max(0, entry.expiresAtMs - Date.now() + 500);
|
||||
window.setTimeout(() => {
|
||||
host.execApprovalQueue = removeExecApproval(host.execApprovalQueue, entry.id);
|
||||
}, delay);
|
||||
enqueueExecApprovalPrompt(host, entry);
|
||||
}
|
||||
|
||||
function removeResolvedApprovalRequest(host: GatewayHost, payload: unknown) {
|
||||
const resolved = parseExecApprovalResolved(payload);
|
||||
if (resolved) {
|
||||
host.execApprovalQueue = removeExecApproval(host.execApprovalQueue, resolved.id);
|
||||
clearResolvedExecApprovalPrompt(host, resolved.id);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
113
ui/src/ui/app.exec-approval.test.ts
Normal file
113
ui/src/ui/app.exec-approval.test.ts
Normal file
@@ -0,0 +1,113 @@
|
||||
/* @vitest-environment jsdom */
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { createStorageMock } from "../test-helpers/storage.ts";
|
||||
import type { ExecApprovalRequest } from "./controllers/exec-approval.ts";
|
||||
|
||||
type RequestFn = (method: string, params?: unknown) => Promise<unknown>;
|
||||
|
||||
function createExecApproval(overrides: Partial<ExecApprovalRequest> = {}): ExecApprovalRequest {
|
||||
return {
|
||||
id: "approval-1",
|
||||
kind: "exec",
|
||||
request: { command: "echo hello" },
|
||||
createdAtMs: 1000,
|
||||
expiresAtMs: Date.now() + 60_000,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function createGatewayError(message: string, details?: unknown): Error {
|
||||
const err = new Error(message);
|
||||
Object.defineProperty(err, "gatewayCode", {
|
||||
value: "INVALID_REQUEST",
|
||||
enumerable: true,
|
||||
});
|
||||
Object.defineProperty(err, "details", {
|
||||
value: details,
|
||||
enumerable: true,
|
||||
});
|
||||
return err;
|
||||
}
|
||||
|
||||
async function createApp(
|
||||
request: RequestFn,
|
||||
queue: ExecApprovalRequest[] = [createExecApproval()],
|
||||
) {
|
||||
const { OpenClawApp } = await import("./app.ts");
|
||||
const app = new OpenClawApp();
|
||||
Object.defineProperty(app, "client", {
|
||||
value: { request },
|
||||
writable: true,
|
||||
});
|
||||
app.execApprovalQueue = queue;
|
||||
app.execApprovalBusy = false;
|
||||
app.execApprovalError = null;
|
||||
return app;
|
||||
}
|
||||
|
||||
describe("OpenClawApp exec approval decisions", () => {
|
||||
beforeEach(() => {
|
||||
vi.stubGlobal("localStorage", createStorageMock());
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("dismisses the active approval after same-decision idempotent success", async () => {
|
||||
const request = vi.fn<RequestFn>(async () => ({ ok: true }));
|
||||
const app = await createApp(request);
|
||||
|
||||
await app.handleExecApprovalDecision("allow-once");
|
||||
|
||||
expect(request).toHaveBeenCalledWith("exec.approval.resolve", {
|
||||
id: "approval-1",
|
||||
decision: "allow-once",
|
||||
});
|
||||
expect(app.execApprovalQueue).toEqual([]);
|
||||
expect(app.execApprovalError).toBeNull();
|
||||
expect(app.execApprovalBusy).toBe(false);
|
||||
});
|
||||
|
||||
it("dismisses and refreshes when the backend reports an already resolved approval", async () => {
|
||||
const request = vi.fn<RequestFn>(async (method) => {
|
||||
if (method === "exec.approval.resolve") {
|
||||
throw createGatewayError("approval already resolved", {
|
||||
reason: "APPROVAL_ALREADY_RESOLVED",
|
||||
});
|
||||
}
|
||||
if (method === "exec.approval.list") {
|
||||
return [];
|
||||
}
|
||||
if (method === "plugin.approval.list") {
|
||||
return [];
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const app = await createApp(request);
|
||||
|
||||
await app.handleExecApprovalDecision("deny");
|
||||
|
||||
expect(app.execApprovalQueue).toEqual([]);
|
||||
expect(app.execApprovalError).toBeNull();
|
||||
expect(app.execApprovalBusy).toBe(false);
|
||||
expect(request).toHaveBeenCalledWith("exec.approval.list", {});
|
||||
expect(request).toHaveBeenCalledWith("plugin.approval.list", {});
|
||||
});
|
||||
|
||||
it("keeps the active approval open for unrelated errors", async () => {
|
||||
const request = vi.fn<RequestFn>(async () => {
|
||||
throw createGatewayError("gateway unavailable");
|
||||
});
|
||||
const active = createExecApproval();
|
||||
const app = await createApp(request, [active]);
|
||||
|
||||
await app.handleExecApprovalDecision("deny");
|
||||
|
||||
expect(app.execApprovalQueue).toEqual([active]);
|
||||
expect(app.execApprovalError).toBe("Approval failed: Error: gateway unavailable");
|
||||
expect(app.execApprovalBusy).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -94,7 +94,12 @@ import type {
|
||||
WikiImportInsights,
|
||||
WikiMemoryPalace,
|
||||
} from "./controllers/dreaming.ts";
|
||||
import type { ExecApprovalRequest } from "./controllers/exec-approval.ts";
|
||||
import {
|
||||
dismissExecApprovalPrompt,
|
||||
isStaleApprovalResolutionError,
|
||||
refreshPendingApprovalQueue,
|
||||
type ExecApprovalRequest,
|
||||
} from "./controllers/exec-approval.ts";
|
||||
import type { ExecApprovalsFile, ExecApprovalsSnapshot } from "./controllers/exec-approvals.ts";
|
||||
import type {
|
||||
ClawHubSearchResult,
|
||||
@@ -1226,8 +1231,16 @@ export class OpenClawApp extends LitElement {
|
||||
id: active.id,
|
||||
decision,
|
||||
});
|
||||
this.execApprovalQueue = this.execApprovalQueue.filter((entry) => entry.id !== active.id);
|
||||
dismissExecApprovalPrompt(this, active.id);
|
||||
} catch (err) {
|
||||
if (isStaleApprovalResolutionError(err)) {
|
||||
dismissExecApprovalPrompt(this, active.id);
|
||||
await refreshPendingApprovalQueue(this);
|
||||
return;
|
||||
}
|
||||
if (!this.execApprovalQueue.some((entry) => entry.id === active.id)) {
|
||||
return;
|
||||
}
|
||||
this.execApprovalError = `Approval failed: ${String(err)}`;
|
||||
} finally {
|
||||
this.execApprovalBusy = false;
|
||||
|
||||
@@ -1,5 +1,52 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { parseExecApprovalRequested, parsePluginApprovalRequested } from "./exec-approval.ts";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
addExecApproval,
|
||||
isStaleApprovalResolutionError,
|
||||
parseExecApprovalRequested,
|
||||
parsePluginApprovalRequested,
|
||||
clearResolvedExecApprovalPrompt,
|
||||
refreshPendingApprovalQueue,
|
||||
type ExecApprovalPromptState,
|
||||
type ExecApprovalRequest,
|
||||
} from "./exec-approval.ts";
|
||||
|
||||
type RequestFn = (method: string, params?: unknown) => Promise<unknown>;
|
||||
|
||||
function createExecApproval(overrides: Partial<ExecApprovalRequest> = {}): ExecApprovalRequest {
|
||||
return {
|
||||
id: "approval-1",
|
||||
kind: "exec",
|
||||
request: { command: "echo hello" },
|
||||
createdAtMs: 1000,
|
||||
expiresAtMs: Date.now() + 60_000,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function createPromptState(
|
||||
request: RequestFn,
|
||||
queue: ExecApprovalRequest[] = [createExecApproval()],
|
||||
): ExecApprovalPromptState {
|
||||
return {
|
||||
client: { request },
|
||||
execApprovalQueue: queue,
|
||||
execApprovalBusy: false,
|
||||
execApprovalError: null,
|
||||
};
|
||||
}
|
||||
|
||||
function createGatewayError(message: string, details?: unknown): Error {
|
||||
const err = new Error(message);
|
||||
Object.defineProperty(err, "gatewayCode", {
|
||||
value: "INVALID_REQUEST",
|
||||
enumerable: true,
|
||||
});
|
||||
Object.defineProperty(err, "details", {
|
||||
value: details,
|
||||
enumerable: true,
|
||||
});
|
||||
return err;
|
||||
}
|
||||
|
||||
describe("parseExecApprovalRequested", () => {
|
||||
it("returns entries with kind 'exec'", () => {
|
||||
@@ -142,3 +189,255 @@ describe("parseExecApprovalRequested command spans", () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isStaleApprovalResolutionError", () => {
|
||||
it("detects already-resolved approval errors", () => {
|
||||
expect(
|
||||
isStaleApprovalResolutionError(
|
||||
createGatewayError("approval already resolved", {
|
||||
reason: "APPROVAL_ALREADY_RESOLVED",
|
||||
}),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("detects unknown or expired approval errors", () => {
|
||||
expect(
|
||||
isStaleApprovalResolutionError(createGatewayError("unknown or expired approval id")),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("detects missing approval errors", () => {
|
||||
expect(
|
||||
isStaleApprovalResolutionError(
|
||||
createGatewayError("approval not found", {
|
||||
reason: "APPROVAL_NOT_FOUND",
|
||||
}),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("ignores unrelated approval resolve errors", () => {
|
||||
expect(isStaleApprovalResolutionError(createGatewayError("gateway unavailable"))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("clearResolvedExecApprovalPrompt", () => {
|
||||
it("does not clear the active prompt error when another approval resolves", () => {
|
||||
const active = createExecApproval({ id: "approval-active", createdAtMs: 2 });
|
||||
const queued = createExecApproval({ id: "approval-queued", createdAtMs: 1 });
|
||||
const state = createPromptState(
|
||||
vi.fn<RequestFn>(async () => ({})),
|
||||
[active, queued],
|
||||
);
|
||||
state.execApprovalError = "Approval failed: Error: gateway unavailable";
|
||||
|
||||
clearResolvedExecApprovalPrompt(state, "approval-queued");
|
||||
|
||||
expect(state.execApprovalQueue.map((entry) => entry.id)).toEqual(["approval-active"]);
|
||||
expect(state.execApprovalError).toBe("Approval failed: Error: gateway unavailable");
|
||||
});
|
||||
|
||||
it("clears the active prompt error when the active approval resolves", () => {
|
||||
const state = createPromptState(vi.fn<RequestFn>(async () => ({})));
|
||||
state.execApprovalError = "Approval failed: Error: gateway unavailable";
|
||||
|
||||
clearResolvedExecApprovalPrompt(state, "approval-1");
|
||||
|
||||
expect(state.execApprovalQueue).toEqual([]);
|
||||
expect(state.execApprovalError).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("refreshPendingApprovalQueue", () => {
|
||||
it("keeps approvals received while a refresh is in flight", async () => {
|
||||
let resolveExecList: (value: unknown[]) => void = () => {};
|
||||
const execApprovalList = new Promise<unknown[]>((resolve) => {
|
||||
resolveExecList = resolve;
|
||||
});
|
||||
const request = vi.fn<RequestFn>(async (method) => {
|
||||
if (method === "exec.approval.list") {
|
||||
return execApprovalList;
|
||||
}
|
||||
if (method === "plugin.approval.list") {
|
||||
return [];
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const state = createPromptState(request, []);
|
||||
|
||||
const refreshPromise = refreshPendingApprovalQueue(state);
|
||||
state.execApprovalQueue = addExecApproval(
|
||||
state.execApprovalQueue,
|
||||
createExecApproval({ id: "approval-arrived-during-refresh", createdAtMs: 2000 }),
|
||||
);
|
||||
resolveExecList([]);
|
||||
await refreshPromise;
|
||||
|
||||
expect(state.execApprovalQueue.map((entry) => entry.id)).toEqual([
|
||||
"approval-arrived-during-refresh",
|
||||
]);
|
||||
});
|
||||
|
||||
it("does not requeue approvals resolved while a refresh is in flight", async () => {
|
||||
let resolveExecList: (value: unknown[]) => void = () => {};
|
||||
const execApprovalList = new Promise<unknown[]>((resolve) => {
|
||||
resolveExecList = resolve;
|
||||
});
|
||||
const request = vi.fn<RequestFn>(async (method) => {
|
||||
if (method === "exec.approval.list") {
|
||||
return execApprovalList;
|
||||
}
|
||||
if (method === "plugin.approval.list") {
|
||||
return [];
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const resolvingApproval = createExecApproval({ id: "approval-resolving" });
|
||||
const state = createPromptState(request, [resolvingApproval]);
|
||||
|
||||
const refreshPromise = refreshPendingApprovalQueue(state);
|
||||
clearResolvedExecApprovalPrompt(state, "approval-resolving");
|
||||
resolveExecList([resolvingApproval]);
|
||||
await refreshPromise;
|
||||
|
||||
expect(state.execApprovalQueue).toEqual([]);
|
||||
});
|
||||
|
||||
it("does not requeue new approvals resolved before refresh completes", async () => {
|
||||
let resolveExecList: (value: unknown[]) => void = () => {};
|
||||
let resolvePluginList: (value: unknown[]) => void = () => {};
|
||||
const execApprovalList = new Promise<unknown[]>((resolve) => {
|
||||
resolveExecList = resolve;
|
||||
});
|
||||
const pluginApprovalList = new Promise<unknown[]>((resolve) => {
|
||||
resolvePluginList = resolve;
|
||||
});
|
||||
const request = vi.fn<RequestFn>(async (method) => {
|
||||
if (method === "exec.approval.list") {
|
||||
return execApprovalList;
|
||||
}
|
||||
if (method === "plugin.approval.list") {
|
||||
return pluginApprovalList;
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const state = createPromptState(request, []);
|
||||
const transientApproval = createExecApproval({ id: "approval-transient" });
|
||||
|
||||
const refreshPromise = refreshPendingApprovalQueue(state);
|
||||
state.execApprovalQueue = addExecApproval(state.execApprovalQueue, transientApproval);
|
||||
resolveExecList([transientApproval]);
|
||||
clearResolvedExecApprovalPrompt(state, "approval-transient");
|
||||
resolvePluginList([]);
|
||||
await refreshPromise;
|
||||
|
||||
expect(state.execApprovalQueue).toEqual([]);
|
||||
});
|
||||
|
||||
it("removes refreshed approvals after their expiry", async () => {
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date("2026-05-25T00:00:00.000Z"));
|
||||
try {
|
||||
const expiresAtMs = Date.now() + 1_000;
|
||||
const request = vi.fn<RequestFn>(async (method) => {
|
||||
if (method === "exec.approval.list") {
|
||||
return [
|
||||
{
|
||||
id: "approval-refreshed-1",
|
||||
request: { command: "pnpm check:changed" },
|
||||
createdAtMs: Date.now(),
|
||||
expiresAtMs,
|
||||
},
|
||||
];
|
||||
}
|
||||
if (method === "plugin.approval.list") {
|
||||
return [];
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const state = createPromptState(request, []);
|
||||
|
||||
await refreshPendingApprovalQueue(state);
|
||||
expect(state.execApprovalQueue.map((entry) => entry.id)).toEqual(["approval-refreshed-1"]);
|
||||
|
||||
vi.advanceTimersByTime(1_500);
|
||||
|
||||
expect(state.execApprovalQueue).toEqual([]);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("clears active prompt errors when expiry advances the queue", async () => {
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date("2026-05-25T00:00:00.000Z"));
|
||||
try {
|
||||
const activeExpiresAtMs = Date.now() + 1_000;
|
||||
const queuedExpiresAtMs = Date.now() + 60_000;
|
||||
const request = vi.fn<RequestFn>(async (method) => {
|
||||
if (method === "exec.approval.list") {
|
||||
return [
|
||||
{
|
||||
id: "approval-active-expiring",
|
||||
request: { command: "pnpm check:changed" },
|
||||
createdAtMs: Date.now() + 1,
|
||||
expiresAtMs: activeExpiresAtMs,
|
||||
},
|
||||
{
|
||||
id: "approval-queued",
|
||||
request: { command: "pnpm test" },
|
||||
createdAtMs: Date.now(),
|
||||
expiresAtMs: queuedExpiresAtMs,
|
||||
},
|
||||
];
|
||||
}
|
||||
if (method === "plugin.approval.list") {
|
||||
return [];
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const state = createPromptState(request, []);
|
||||
|
||||
await refreshPendingApprovalQueue(state);
|
||||
state.execApprovalError = "Approval failed: Error: gateway unavailable";
|
||||
|
||||
vi.advanceTimersByTime(1_500);
|
||||
|
||||
expect(state.execApprovalQueue.map((entry) => entry.id)).toEqual(["approval-queued"]);
|
||||
expect(state.execApprovalError).toBeNull();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("does not requeue expired approvals returned by refresh lists", async () => {
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date("2026-05-25T00:00:00.000Z"));
|
||||
try {
|
||||
const request = vi.fn<RequestFn>(async (method) => {
|
||||
if (method === "exec.approval.list") {
|
||||
return [
|
||||
{
|
||||
id: "approval-expired-1",
|
||||
request: { command: "pnpm check:changed" },
|
||||
createdAtMs: Date.now() - 2_000,
|
||||
expiresAtMs: Date.now() - 1_000,
|
||||
},
|
||||
];
|
||||
}
|
||||
if (method === "plugin.approval.list") {
|
||||
return [];
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const state = createPromptState(request, []);
|
||||
|
||||
await refreshPendingApprovalQueue(state);
|
||||
|
||||
expect(state.execApprovalQueue).toEqual([]);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -34,6 +34,19 @@ export type ExecApprovalResolved = {
|
||||
ts?: number | null;
|
||||
};
|
||||
|
||||
export type ExecApprovalPromptState = {
|
||||
client: {
|
||||
request(method: string, params?: unknown): Promise<unknown>;
|
||||
} | null;
|
||||
execApprovalQueue: ExecApprovalRequest[];
|
||||
execApprovalBusy: boolean;
|
||||
execApprovalError: string | null;
|
||||
execApprovalRefreshRemovedIds?: Set<string> | null;
|
||||
};
|
||||
|
||||
const APPROVAL_ALREADY_RESOLVED = "APPROVAL_ALREADY_RESOLVED";
|
||||
const APPROVAL_NOT_FOUND = "APPROVAL_NOT_FOUND";
|
||||
|
||||
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||
return typeof value === "object" && value !== null;
|
||||
}
|
||||
@@ -188,3 +201,160 @@ export function removeExecApproval(
|
||||
): ExecApprovalRequest[] {
|
||||
return pruneExecApprovalQueue(queue).filter((entry) => entry.id !== id);
|
||||
}
|
||||
|
||||
function readGatewayErrorCode(err: unknown): string | null {
|
||||
if (!isRecord(err)) {
|
||||
return null;
|
||||
}
|
||||
return normalizeOptionalString(err.gatewayCode) ?? null;
|
||||
}
|
||||
|
||||
function readGatewayErrorReason(err: unknown): string | null {
|
||||
if (!isRecord(err)) {
|
||||
return null;
|
||||
}
|
||||
const { details } = err;
|
||||
if (!isRecord(details)) {
|
||||
return null;
|
||||
}
|
||||
return normalizeOptionalString(details.reason) ?? null;
|
||||
}
|
||||
|
||||
export function isStaleApprovalResolutionError(err: unknown): boolean {
|
||||
if (!(err instanceof Error)) {
|
||||
return false;
|
||||
}
|
||||
const gatewayCode = readGatewayErrorCode(err);
|
||||
const reason = readGatewayErrorReason(err);
|
||||
if (reason === APPROVAL_ALREADY_RESOLVED || reason === APPROVAL_NOT_FOUND) {
|
||||
return true;
|
||||
}
|
||||
if (gatewayCode === APPROVAL_NOT_FOUND) {
|
||||
return true;
|
||||
}
|
||||
return /unknown or expired approval id/i.test(err.message);
|
||||
}
|
||||
|
||||
function parseApprovalList(
|
||||
payload: unknown,
|
||||
parseEntry: (entry: unknown) => ExecApprovalRequest | null,
|
||||
): ExecApprovalRequest[] | null {
|
||||
if (!Array.isArray(payload)) {
|
||||
return null;
|
||||
}
|
||||
return payload.flatMap((entry) => {
|
||||
const parsed = parseEntry(entry);
|
||||
return parsed ? [parsed] : [];
|
||||
});
|
||||
}
|
||||
|
||||
function sortApprovalsNewestFirst(queue: ExecApprovalRequest[]): ExecApprovalRequest[] {
|
||||
return queue.toSorted((a, b) => b.createdAtMs - a.createdAtMs);
|
||||
}
|
||||
|
||||
function currentApprovalsForKind(
|
||||
queue: ExecApprovalRequest[],
|
||||
kind: ExecApprovalRequest["kind"],
|
||||
): ExecApprovalRequest[] {
|
||||
return pruneExecApprovalQueue(queue).filter((entry) => entry.kind === kind);
|
||||
}
|
||||
|
||||
function mergeRefreshedApprovalQueue(
|
||||
refreshed: ExecApprovalRequest[],
|
||||
refreshStartedWith: ExecApprovalRequest[],
|
||||
currentQueue: ExecApprovalRequest[],
|
||||
removedDuringRefresh: ReadonlySet<string>,
|
||||
): ExecApprovalRequest[] {
|
||||
const refreshStartIds = new Set(refreshStartedWith.map((entry) => entry.id));
|
||||
const prunedCurrentQueue = pruneExecApprovalQueue(currentQueue);
|
||||
const currentQueueIds = new Set(prunedCurrentQueue.map((entry) => entry.id));
|
||||
const currentRefreshed = pruneExecApprovalQueue(refreshed).filter(
|
||||
(entry) =>
|
||||
!removedDuringRefresh.has(entry.id) &&
|
||||
(!refreshStartIds.has(entry.id) || currentQueueIds.has(entry.id)),
|
||||
);
|
||||
const refreshedIds = new Set(currentRefreshed.map((entry) => entry.id));
|
||||
const arrivedDuringRefresh = prunedCurrentQueue.filter(
|
||||
(entry) => !refreshStartIds.has(entry.id) && !refreshedIds.has(entry.id),
|
||||
);
|
||||
return sortApprovalsNewestFirst([...currentRefreshed, ...arrivedDuringRefresh]);
|
||||
}
|
||||
|
||||
function scheduleApprovalExpiryPrune(
|
||||
state: ExecApprovalPromptState,
|
||||
entry: ExecApprovalRequest,
|
||||
): void {
|
||||
const delay = Math.max(0, entry.expiresAtMs - Date.now() + 500);
|
||||
globalThis.setTimeout(() => {
|
||||
removeExecApprovalFromState(state, entry.id);
|
||||
}, delay);
|
||||
}
|
||||
|
||||
function removeExecApprovalFromState(state: ExecApprovalPromptState, id: string): void {
|
||||
const activeId = state.execApprovalQueue[0]?.id ?? null;
|
||||
state.execApprovalQueue = removeExecApproval(state.execApprovalQueue, id);
|
||||
if (activeId !== (state.execApprovalQueue[0]?.id ?? null)) {
|
||||
state.execApprovalError = null;
|
||||
}
|
||||
}
|
||||
|
||||
export function enqueueExecApprovalPrompt(
|
||||
state: ExecApprovalPromptState,
|
||||
entry: ExecApprovalRequest,
|
||||
): void {
|
||||
state.execApprovalQueue = addExecApproval(state.execApprovalQueue, entry);
|
||||
state.execApprovalError = null;
|
||||
scheduleApprovalExpiryPrune(state, entry);
|
||||
}
|
||||
|
||||
export async function refreshPendingApprovalQueue(state: ExecApprovalPromptState): Promise<void> {
|
||||
const client = state.client;
|
||||
if (!client) {
|
||||
return;
|
||||
}
|
||||
const removedDuringRefresh = state.execApprovalRefreshRemovedIds ?? new Set<string>();
|
||||
const ownsRemovedSet = !state.execApprovalRefreshRemovedIds;
|
||||
if (ownsRemovedSet) {
|
||||
state.execApprovalRefreshRemovedIds = removedDuringRefresh;
|
||||
}
|
||||
const refreshStartedWith = pruneExecApprovalQueue(state.execApprovalQueue);
|
||||
try {
|
||||
const [execResult, pluginResult] = await Promise.allSettled([
|
||||
client.request("exec.approval.list", {}),
|
||||
client.request("plugin.approval.list", {}),
|
||||
]);
|
||||
const execApprovals =
|
||||
execResult.status === "fulfilled"
|
||||
? (parseApprovalList(execResult.value, parseExecApprovalRequested) ?? [])
|
||||
: currentApprovalsForKind(state.execApprovalQueue, "exec");
|
||||
const pluginApprovals =
|
||||
pluginResult.status === "fulfilled"
|
||||
? (parseApprovalList(pluginResult.value, parsePluginApprovalRequested) ?? [])
|
||||
: currentApprovalsForKind(state.execApprovalQueue, "plugin");
|
||||
const refreshed = mergeRefreshedApprovalQueue(
|
||||
sortApprovalsNewestFirst([...execApprovals, ...pluginApprovals]),
|
||||
refreshStartedWith,
|
||||
state.execApprovalQueue,
|
||||
removedDuringRefresh,
|
||||
);
|
||||
state.execApprovalQueue = refreshed;
|
||||
for (const entry of refreshed) {
|
||||
scheduleApprovalExpiryPrune(state, entry);
|
||||
}
|
||||
} finally {
|
||||
if (ownsRemovedSet) {
|
||||
state.execApprovalRefreshRemovedIds = null;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function dismissExecApprovalPrompt(state: ExecApprovalPromptState, id: string): void {
|
||||
removeExecApprovalFromState(state, id);
|
||||
state.execApprovalRefreshRemovedIds?.add(id);
|
||||
state.execApprovalError = null;
|
||||
}
|
||||
|
||||
export function clearResolvedExecApprovalPrompt(state: ExecApprovalPromptState, id: string): void {
|
||||
removeExecApprovalFromState(state, id);
|
||||
state.execApprovalRefreshRemovedIds?.add(id);
|
||||
}
|
||||
|
||||
@@ -3,10 +3,7 @@
|
||||
import { nothing, render } from "lit";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { i18n } from "../../i18n/index.ts";
|
||||
import {
|
||||
getRenderedModalDialog,
|
||||
installDialogPolyfill,
|
||||
} from "../../test-helpers/modal-dialog.ts";
|
||||
import { getRenderedModalDialog, installDialogPolyfill } from "../../test-helpers/modal-dialog.ts";
|
||||
import { createStorageMock } from "../../test-helpers/storage.ts";
|
||||
import type { AppViewState } from "../app-view-state.ts";
|
||||
import type { ExecApprovalRequest } from "../controllers/exec-approval.ts";
|
||||
@@ -130,32 +127,29 @@ describe("approval and confirmation modals", () => {
|
||||
expect(spans).toEqual(["ls", "python -c"]);
|
||||
});
|
||||
|
||||
it("maps Escape to exec denial when approval is idle", async () => {
|
||||
it("does not render a visible neutral dismiss action", async () => {
|
||||
render(renderExecApprovalPrompt(createExecState()), container);
|
||||
|
||||
await getRenderedDialog();
|
||||
|
||||
expect(
|
||||
Array.from(container.querySelectorAll(".exec-approval-actions button")).map((button) =>
|
||||
button.textContent?.trim(),
|
||||
),
|
||||
).toEqual(["Allow once", "Always allow", "Deny"]);
|
||||
});
|
||||
|
||||
it("denies exec approval on Escape", async () => {
|
||||
const handleExecApprovalDecision = vi.fn(async () => undefined);
|
||||
render(renderExecApprovalPrompt(createExecState({ handleExecApprovalDecision })), container);
|
||||
|
||||
const { dialog } = await getRenderedDialog();
|
||||
|
||||
dispatchEscape(dialog);
|
||||
|
||||
expect(handleExecApprovalDecision).toHaveBeenCalledTimes(1);
|
||||
expect(handleExecApprovalDecision).toHaveBeenCalledWith("deny");
|
||||
});
|
||||
|
||||
it("does not dispatch an extra exec decision from Escape while busy", async () => {
|
||||
const handleExecApprovalDecision = vi.fn(async () => undefined);
|
||||
render(
|
||||
renderExecApprovalPrompt(
|
||||
createExecState({ execApprovalBusy: true, handleExecApprovalDecision }),
|
||||
),
|
||||
container,
|
||||
);
|
||||
|
||||
const { dialog } = await getRenderedDialog();
|
||||
dispatchEscape(dialog);
|
||||
|
||||
expect(handleExecApprovalDecision).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("renders exec approval chrome from the active locale", async () => {
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date("2026-04-29T00:00:00.000Z"));
|
||||
|
||||
Reference in New Issue
Block a user