Skip to content

Commit ef98bd1

Browse files
haruaiclone-droidharuaiclone-droid
committed
fix: keep queued approvals visible
1 parent fa3c9de commit ef98bd1

6 files changed

Lines changed: 124 additions & 14 deletions

File tree

‎ui/src/ui/app-gateway.node.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,42 @@ describe("connectGateway", () => {
371371
expect(host.chatComposerProvisionalRestore).toBeNull();
372372
});
373373

374+
it("loads pending plugin approvals from approval lists on hello", async () => {
375+
const { host, client } = connectHostGateway();
376+
client.request.mockImplementation(async (method: string) => {
377+
if (method === "exec.approval.list") {
378+
return [];
379+
}
380+
if (method === "plugin.approval.list") {
381+
return [
382+
{
383+
id: "plugin-approval-from-list",
384+
createdAtMs: Date.now(),
385+
expiresAtMs: Date.now() + 120_000,
386+
request: {
387+
title: "Review plugin action",
388+
description: "The plugin wants to run a sensitive action.",
389+
severity: "high",
390+
pluginId: "sage",
391+
agentId: "agent-1",
392+
sessionKey: "main",
393+
},
394+
},
395+
];
396+
}
397+
return {};
398+
});
399+
400+
client.emitHello();
401+
402+
await vi.waitFor(() => {
403+
expect(client.request).toHaveBeenCalledWith("exec.approval.list", {});
404+
expect(client.request).toHaveBeenCalledWith("plugin.approval.list", {});
405+
expect(host.execApprovalQueue[0]?.id).toBe("plugin-approval-from-list");
406+
});
407+
expect(host.execApprovalQueue[0]?.kind).toBe("plugin");
408+
});
409+
374410
it("ignores stale client onEvent callbacks after reconnect", () => {
375411
const host = createHost();
376412

‎ui/src/ui/app-gateway.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ import {
6363
parseExecApprovalResolved,
6464
parsePluginApprovalRequested,
6565
pruneExecApprovalQueue,
66+
refreshPendingApprovalQueue,
6667
} from "./controllers/exec-approval.ts";
6768
import { loadHealthState, type HealthState } from "./controllers/health.ts";
6869
import {
@@ -889,6 +890,7 @@ export function connectGateway(host: GatewayHost, options?: ConnectGatewayOption
889890
host as unknown as SessionsState & { sessionKey: string },
890891
{ force: true },
891892
);
893+
void refreshPendingApprovalQueue(host);
892894
void loadAgentsThenRefreshActiveTabForClient(host, client);
893895
scheduleDeferredStartupWork(() => {
894896
if (host.client !== client) {

‎ui/src/ui/app.exec-approval.test.ts‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
/* @vitest-environment jsdom */
22

3-
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
3+
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
44
import { createStorageMock } from "../test-helpers/storage.ts";
55
import type { ExecApprovalRequest } from "./controllers/exec-approval.ts";
66

77
type RequestFn = (method: string, params?: unknown) => Promise<unknown>;
8+
let OpenClawApp: typeof import("./app.ts").OpenClawApp;
89

910
function createExecApproval(overrides: Partial<ExecApprovalRequest> = {}): ExecApprovalRequest {
1011
return {
@@ -34,7 +35,6 @@ async function createApp(
3435
request: RequestFn,
3536
queue: ExecApprovalRequest[] = [createExecApproval()],
3637
) {
37-
const { OpenClawApp } = await import("./app.ts");
3838
const app = Object.create(OpenClawApp.prototype) as InstanceType<typeof OpenClawApp>;
3939
Object.defineProperties(app, {
4040
client: { value: { request }, writable: true },
@@ -46,6 +46,10 @@ async function createApp(
4646
}
4747

4848
describe("OpenClawApp exec approval decisions", () => {
49+
beforeAll(async () => {
50+
({ OpenClawApp } = await import("./app.ts"));
51+
}, 20_000);
52+
4953
beforeEach(() => {
5054
vi.stubGlobal("localStorage", createStorageMock());
5155
});
@@ -70,6 +74,46 @@ describe("OpenClawApp exec approval decisions", () => {
7074
expect(app.execApprovalBusy).toBe(false);
7175
});
7276

77+
it("keeps the approval prompt on the next queued item after resolving the active item", async () => {
78+
const request = vi.fn<RequestFn>(async () => ({ ok: true }));
79+
const active = createExecApproval({ id: "approval-active", createdAtMs: 2000 });
80+
const queued = createExecApproval({
81+
id: "approval-queued",
82+
request: { command: "pnpm test:changed" },
83+
createdAtMs: 1000,
84+
});
85+
const app = await createApp(request, [active, queued]);
86+
87+
await app.handleExecApprovalDecision("allow-once");
88+
89+
expect(request).toHaveBeenCalledWith("exec.approval.resolve", {
90+
id: "approval-active",
91+
decision: "allow-once",
92+
});
93+
expect(app.execApprovalQueue).toEqual([queued]);
94+
expect(app.execApprovalError).toBeNull();
95+
expect(app.execApprovalBusy).toBe(false);
96+
});
97+
98+
it("resolves plugin approvals through the plugin approval method", async () => {
99+
const request = vi.fn<RequestFn>(async () => ({ ok: true }));
100+
const pluginApproval = createExecApproval({
101+
id: "plugin-approval-1",
102+
kind: "plugin",
103+
pluginTitle: "Plugin approval",
104+
request: { command: "Plugin approval" },
105+
});
106+
const app = await createApp(request, [pluginApproval]);
107+
108+
await app.handleExecApprovalDecision("allow-once");
109+
110+
expect(request).toHaveBeenCalledWith("plugin.approval.resolve", {
111+
id: "plugin-approval-1",
112+
decision: "allow-once",
113+
});
114+
expect(app.execApprovalQueue).toEqual([]);
115+
});
116+
73117
it("dismisses and refreshes when the backend reports an already resolved approval", async () => {
74118
const request = vi.fn<RequestFn>(async (method) => {
75119
if (method === "exec.approval.resolve") {

‎ui/src/ui/components/modal-dialog.test.ts‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,24 @@ import "./modal-dialog.ts";
1313
let container: HTMLDivElement;
1414
let restoreDialogPolyfill: () => void;
1515

16-
async function renderModal() {
16+
async function renderModal(
17+
options: {
18+
label?: string;
19+
description?: string;
20+
firstAction?: string;
21+
lastAction?: string;
22+
} = {},
23+
) {
24+
const label = options.label ?? "Confirm action";
25+
const description = options.description ?? "Review the operation before continuing.";
1726
render(
1827
html`
19-
<openclaw-modal-dialog
20-
label="Confirm action"
21-
description="Review the operation before continuing."
22-
>
28+
<openclaw-modal-dialog label=${label} description=${description}>
2329
<section>
24-
<h2 id="modal-title">Confirm action</h2>
25-
<p id="modal-description">Review the operation before continuing.</p>
26-
<button id="first-action">First</button>
27-
<button id="last-action">Last</button>
30+
<h2 id="modal-title">${label}</h2>
31+
<p id="modal-description">${description}</p>
32+
<button id="first-action">${options.firstAction ?? "First"}</button>
33+
<button id="last-action">${options.lastAction ?? "Last"}</button>
2834
</section>
2935
</openclaw-modal-dialog>
3036
`,
@@ -134,6 +140,23 @@ describe("openclaw-modal-dialog", () => {
134140
expect(onCancel).toHaveBeenCalledTimes(1);
135141
});
136142

143+
it("reopens the same dialog instance after content changes while closed", async () => {
144+
const { modal, dialog } = await renderModal();
145+
dialog.close();
146+
expect(dialog.open).toBe(false);
147+
148+
await renderModal({
149+
label: "Next approval",
150+
description: "Another item is pending.",
151+
firstAction: "Next",
152+
});
153+
154+
const { modal: updatedModal, dialog: updatedDialog } = await getRenderedModalDialog(container);
155+
expect(updatedModal).toBe(modal);
156+
expect(updatedDialog).toBe(dialog);
157+
expect(dialog.open).toBe(true);
158+
});
159+
137160
it("restores focus when closed and removed", async () => {
138161
const returnTarget = document.createElement("button");
139162
returnTarget.textContent = "Return";

‎ui/src/ui/components/modal-dialog.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ export class OpenClawModalDialog extends LitElement {
9191
this.openDialog();
9292
}
9393

94+
override updated() {
95+
this.openDialog();
96+
}
97+
9498
override disconnectedCallback() {
9599
this.closeDialog();
96100
this.restoreFocus();
@@ -122,13 +126,13 @@ export class OpenClawModalDialog extends LitElement {
122126
}
123127

124128
private openDialog() {
125-
if (this.opened) {
126-
return;
127-
}
128129
const dialog = this.dialogElement;
129130
if (!dialog) {
130131
return;
131132
}
133+
if (this.opened && dialog.open) {
134+
return;
135+
}
132136
this.opened = true;
133137
if (typeof dialog.showModal === "function") {
134138
try {

‎ui/src/ui/views/exec-approval.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ export function renderExecApprovalPrompt(state: AppViewState) {
205205
${decisions.map(
206206
(decision) => html`
207207
<button
208+
type="button"
208209
class=${approvalDecisionClass(decision)}
209210
?disabled=${state.execApprovalBusy}
210211
@click=${() => state.handleExecApprovalDecision(decision)}

0 commit comments

Comments
 (0)