From af10be59d81a8af0af51df4a0fff4d98e898583a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 28 Apr 2026 11:53:14 +0100 Subject: [PATCH] fix(approvals): stop stale approval resume loops --- CHANGELOG.md | 2 ++ .../main-session-restart-recovery.test.ts | 34 +++++++++++++++++++ src/agents/main-session-restart-recovery.ts | 26 +++++++++++--- src/gateway/call.test.ts | 26 +++++++++----- src/gateway/call.ts | 7 +++- 5 files changed, 81 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eae328fbc30..042ef485058 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,8 @@ Docs: https://docs.openclaw.ai ### Fixes +- Agents/approvals: fail restart-interrupted sessions whose transcript tail is still `approval-pending` instead of replaying stale exec approval IDs into the new Gateway process after restart. Fixes #65486. Thanks @mjmai20682068-create. +- CLI/Gateway: use method-specific least-privilege scopes for classified CLI Gateway calls while preserving legacy broad scopes for unclassified plugin methods, so read-only commands no longer create admin/write/pairing scope-upgrade prompts. Fixes #68634. Thanks @nightmusher. - Gateway/sessions: align `chat.history` and `sessions.list` thinking defaults with owning-agent and catalog-aware resolution so Control UI session defaults match backend runtime state. (#63418) Thanks @jpreagan. - Devices/pairing: recover array-shaped device and node pairing state files before persisting approvals, so UUID-keyed pending and paired entries no longer disappear after a malformed JSON store write. Fixes #63035. Thanks @sar618. - Gateway/auth: clear reused stale device tokens and stop reconnecting on device-token mismatch in the Control UI and Node gateway clients, avoiding rate-limit loops after scope-upgrade or token-rotation handoffs. Fixes #71609. Thanks @ricksayhi. diff --git a/src/agents/main-session-restart-recovery.test.ts b/src/agents/main-session-restart-recovery.test.ts index 0fbfe5b466c..2de96b27574 100644 --- a/src/agents/main-session-restart-recovery.test.ts +++ b/src/agents/main-session-restart-recovery.test.ts @@ -123,6 +123,40 @@ describe("main-session-restart-recovery", () => { expect(store["agent:main:main"]?.abortedLastRun).toBe(false); }); + it("fails marked sessions with stale approval-pending exec tool results", async () => { + const sessionsDir = await makeSessionsDir(); + await writeStore(sessionsDir, { + "agent:main:main": { + sessionId: "main-session", + updatedAt: Date.now() - 10_000, + status: "running", + abortedLastRun: true, + }, + }); + await writeTranscript(sessionsDir, "main-session", [ + { role: "user", content: "run a command that needs approval" }, + { role: "assistant", content: [{ type: "toolCall", id: "call-1", name: "exec" }] }, + { + role: "toolResult", + content: "Approval required (id stale, full stale-approval-id).", + details: { + status: "approval-pending", + approvalId: "stale-approval-id", + host: "gateway", + command: "echo stale", + }, + }, + ]); + + const result = await recoverRestartAbortedMainSessions({ stateDir: tmpDir }); + + expect(result).toEqual({ recovered: 0, failed: 1, skipped: 0 }); + expect(callGateway).not.toHaveBeenCalled(); + const store = loadSessionStore(path.join(sessionsDir, "sessions.json")); + expect(store["agent:main:main"]?.status).toBe("failed"); + expect(store["agent:main:main"]?.abortedLastRun).toBe(true); + }); + it("does not scan ordinary running sessions without the restart-aborted marker", async () => { const sessionsDir = await makeSessionsDir(); await writeStore(sessionsDir, { diff --git a/src/agents/main-session-restart-recovery.ts b/src/agents/main-session-restart-recovery.ts index a976b110aa6..48635665c8e 100644 --- a/src/agents/main-session-restart-recovery.ts +++ b/src/agents/main-session-restart-recovery.ts @@ -62,9 +62,26 @@ function isResumableTailMessage(message: unknown): boolean { return role === "user" || role === "tool" || role === "toolResult"; } -function isMainSessionResumable(messages: unknown[]): boolean { +function isApprovalPendingToolResult(message: unknown): boolean { + if (!message || typeof message !== "object" || getMessageRole(message) !== "toolResult") { + return false; + } + const details = (message as { details?: unknown }).details; + if (!details || typeof details !== "object") { + return false; + } + return (details as { status?: unknown }).status === "approval-pending"; +} + +function resolveMainSessionResumeBlockReason(messages: unknown[]): string | null { const lastMeaningful = messages.toReversed().find(isMeaningfulTailMessage); - return lastMeaningful ? isResumableTailMessage(lastMeaningful) : false; + if (!lastMeaningful || !isResumableTailMessage(lastMeaningful)) { + return "transcript tail is not resumable"; + } + if (isApprovalPendingToolResult(lastMeaningful)) { + return "transcript tail is a stale approval-pending tool result"; + } + return null; } function buildResumeMessage(): string { @@ -216,11 +233,12 @@ async function recoverStore(params: { continue; } - if (!isMainSessionResumable(messages)) { + const resumeBlockReason = resolveMainSessionResumeBlockReason(messages); + if (resumeBlockReason) { await markSessionFailed({ storePath: params.storePath, sessionKey, - reason: "transcript tail is not resumable", + reason: resumeBlockReason, }); result.failed++; continue; diff --git a/src/gateway/call.test.ts b/src/gateway/call.test.ts index bae1c13e1a9..32361ac2ed9 100644 --- a/src/gateway/call.test.ts +++ b/src/gateway/call.test.ts @@ -482,16 +482,9 @@ describe("callGateway url resolution", () => { expectedScopes: ["operator.read"], }, { - label: "keeps legacy admin scopes for explicit CLI callers", + label: "uses least-privilege scopes by default for explicit CLI callers", call: () => callGatewayCli({ method: "health" }), - expectedScopes: [ - "operator.admin", - "operator.read", - "operator.write", - "operator.approvals", - "operator.pairing", - "operator.talk.secrets", - ], + expectedScopes: ["operator.read"], }, ])("scope selection: $label", async ({ call, expectedScopes }) => { setLocalLoopbackGatewayConfig(); @@ -499,6 +492,21 @@ describe("callGateway url resolution", () => { expect(lastClientOptions?.scopes).toEqual(expectedScopes); }); + it("keeps legacy broad scopes for unclassified explicit CLI methods", async () => { + setLocalLoopbackGatewayConfig(); + + await callGatewayCli({ method: "plugin.custom.unclassified" }); + + expect(lastClientOptions?.scopes).toEqual([ + "operator.admin", + "operator.read", + "operator.write", + "operator.approvals", + "operator.pairing", + "operator.talk.secrets", + ]); + }); + it("passes explicit scopes through, including empty arrays", async () => { setLocalLoopbackGatewayConfig(); diff --git a/src/gateway/call.ts b/src/gateway/call.ts index 4dbc458ff17..2984a74fcef 100644 --- a/src/gateway/call.ts +++ b/src/gateway/call.ts @@ -35,6 +35,7 @@ import { import { canSkipGatewayConfigLoad } from "./explicit-connection-policy.js"; import { CLI_DEFAULT_OPERATOR_SCOPES, + isGatewayMethodClassified, resolveLeastPrivilegeOperatorScopesForMethod, type OperatorScope, } from "./method-scopes.js"; @@ -635,7 +636,11 @@ export async function callGatewayScoped>( export async function callGatewayCli>( opts: CallGatewayCliOptions, ): Promise { - const scopes = Array.isArray(opts.scopes) ? opts.scopes : CLI_DEFAULT_OPERATOR_SCOPES; + const scopes = Array.isArray(opts.scopes) + ? opts.scopes + : isGatewayMethodClassified(opts.method) + ? resolveLeastPrivilegeOperatorScopesForMethod(opts.method) + : CLI_DEFAULT_OPERATOR_SCOPES; return await callGatewayWithScopes(opts, scopes); }