From 0e47ce58bc1b001c61eec4155db5132eae5b17bb Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 30 Mar 2026 07:36:44 +0900 Subject: [PATCH] fix(approvals): restore queue targeting and plugin id prefixes --- CHANGELOG.md | 2 ++ .../server-methods/plugin-approval.test.ts | 13 ++------- src/gateway/server-methods/plugin-approval.ts | 13 ++++++++- ui/src/ui/app-gateway.sessions.node.test.ts | 28 +++++++++++++++++++ ui/src/ui/controllers/exec-approval.ts | 2 +- 5 files changed, 46 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b15988d0db..8d634dd47bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -106,6 +106,8 @@ Docs: https://docs.openclaw.ai - Telegram/splitting: replace proportional text estimate with verified HTML-length search so long messages split at word boundaries instead of mid-word; gracefully degrade when tag overhead exceeds the limit. (#56595) - Telegram/delivery: skip whitespace-only and hook-blanked text replies in bot delivery to prevent GrammyError 400 empty-text crashes. (#56620) - Telegram/send: validate `replyToMessageId` at all four API sinks with a shared normalizer that rejects non-numeric, NaN, and mixed-content strings. (#56587) +- Approvals/UI: keep the newest pending approval at the front of the Control UI queue so approving one request does not accidentally target an older expired id. Thanks @vincentkoc. +- Plugin approvals: accept unique short approval-id prefixes on `plugin.approval.resolve`, matching exec approvals and restoring `/approve` fallback flows on chat approval surfaces. Thanks @vincentkoc. - Mistral: normalize OpenAI-compatible request flags so official Mistral API runs no longer fail with remaining `422 status code (no body)` chat errors. - Control UI/config: keep sensitive raw config hidden by default, replace the blank blocked editor with an explicit reveal-to-edit state, and restore raw JSON editing without auto-exposing secrets. Fixes #55322. - CLI/zsh: defer `compdef` registration until `compinit` is available so zsh completion loads cleanly with plugin managers and manual setups. (#56555) diff --git a/src/gateway/server-methods/plugin-approval.test.ts b/src/gateway/server-methods/plugin-approval.test.ts index ab955cce78c..5c68b872806 100644 --- a/src/gateway/server-methods/plugin-approval.test.ts +++ b/src/gateway/server-methods/plugin-approval.test.ts @@ -431,7 +431,7 @@ describe("createPluginApprovalHandlers", () => { ); }); - it("requires exact id and rejects prefixes", async () => { + it("accepts unique short id prefixes", async () => { const handlers = createPluginApprovalHandlers(manager); const record = manager.create({ title: "T", description: "D" }, 60_000, "abcdef-1234"); void manager.register(record, 60_000); @@ -441,15 +441,8 @@ describe("createPluginApprovalHandlers", () => { decision: "allow-always", }); await handlers["plugin.approval.resolve"](opts); - expect(opts.respond).toHaveBeenCalledWith( - false, - undefined, - expect.objectContaining({ - code: "INVALID_REQUEST", - message: expect.stringContaining("unknown or expired"), - details: expect.objectContaining({ reason: "APPROVAL_NOT_FOUND" }), - }), - ); + expect(opts.respond).toHaveBeenCalledWith(true, { ok: true }, undefined); + expect(manager.getSnapshot(record.id)?.decision).toBe("allow-always"); }); it("does not leak candidate ids when prefixes are ambiguous", async () => { diff --git a/src/gateway/server-methods/plugin-approval.ts b/src/gateway/server-methods/plugin-approval.ts index 2646563740e..d3437e4f437 100644 --- a/src/gateway/server-methods/plugin-approval.ts +++ b/src/gateway/server-methods/plugin-approval.ts @@ -217,7 +217,18 @@ export function createPluginApprovalHandlers( respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "invalid decision")); return; } - const approvalId = p.id.trim(); + const resolvedId = manager.lookupPendingId(p.id); + if (resolvedId.kind === "none" || resolvedId.kind === "ambiguous") { + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, "unknown or expired approval id", { + details: APPROVAL_NOT_FOUND_DETAILS, + }), + ); + return; + } + const approvalId = resolvedId.id; const snapshot = manager.getSnapshot(approvalId); if (!snapshot || snapshot.resolvedAtMs !== undefined) { respond( diff --git a/ui/src/ui/app-gateway.sessions.node.test.ts b/ui/src/ui/app-gateway.sessions.node.test.ts index 80c79218666..86b507205c2 100644 --- a/ui/src/ui/app-gateway.sessions.node.test.ts +++ b/ui/src/ui/app-gateway.sessions.node.test.ts @@ -49,6 +49,9 @@ vi.mock("./gateway.ts", () => ({ })); const { handleGatewayEvent } = await import("./app-gateway.ts"); +const { addExecApproval } = await vi.importActual( + "./controllers/exec-approval.ts", +); function createHost() { return { @@ -120,3 +123,28 @@ describe("handleGatewayEvent sessions.changed", () => { expect(loadSessionsMock).toHaveBeenCalledWith(host); }); }); + +describe("addExecApproval", () => { + it("keeps the newest approval at the front of the queue", () => { + const queue = addExecApproval( + [ + { + id: "approval-old", + kind: "exec", + request: { command: "echo old" }, + createdAtMs: 1, + expiresAtMs: Date.now() + 120_000, + }, + ], + { + id: "approval-new", + kind: "exec", + request: { command: "echo new" }, + createdAtMs: 2, + expiresAtMs: Date.now() + 120_000, + }, + ); + + expect(queue.map((entry) => entry.id)).toEqual(["approval-new", "approval-old"]); + }); +}); diff --git a/ui/src/ui/controllers/exec-approval.ts b/ui/src/ui/controllers/exec-approval.ts index e1f1bf37e0f..787eccd4f07 100644 --- a/ui/src/ui/controllers/exec-approval.ts +++ b/ui/src/ui/controllers/exec-approval.ts @@ -134,7 +134,7 @@ export function addExecApproval( entry: ExecApprovalRequest, ): ExecApprovalRequest[] { const next = pruneExecApprovalQueue(queue).filter((item) => item.id !== entry.id); - next.push(entry); + next.unshift(entry); return next; }