From e627f53d24a957b80ced740a2acfdfdfbcbe6b05 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Sat, 4 Apr 2026 13:23:58 -0400 Subject: [PATCH] core: dedupe approval not-found handling (#60932) Merged via squash. Prepared head SHA: 108221fdfe0e84c486df21effc9d81318df85674 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras --- CHANGELOG.md | 1 + .../.generated/plugin-sdk-api-baseline.sha256 | 4 +-- docs/plugins/sdk-migration.md | 2 +- docs/plugins/sdk-overview.md | 2 +- .../telegram/src/exec-approval-resolver.ts | 33 +---------------- src/auto-reply/reply/commands-approve.ts | 35 +------------------ src/infra/approval-errors.test.ts | 29 +++++++++++++++ src/infra/approval-errors.ts | 29 +++++++++++++++ src/plugin-sdk/error-runtime.ts | 1 + .../contracts/plugin-sdk-subpaths.test.ts | 1 + 10 files changed, 67 insertions(+), 70 deletions(-) create mode 100644 src/infra/approval-errors.test.ts create mode 100644 src/infra/approval-errors.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 5595aeafe6f..2e930493348 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -121,6 +121,7 @@ Docs: https://docs.openclaw.ai - Agents/compaction: keep assistant tool calls and displaced tool results in the same compaction chunk so strict summarization providers stop rejecting orphaned tool pairs. (#58849) Thanks @openperf. - Cron: suppress exact `NO_REPLY` sentinel direct-delivery payloads, keep silent direct replies from falling back into duplicate main-summary sends, and treat structured `deleteAfterRun` silent replies the same as text silent replies. (#45737) Thanks @openperf. - Cron: keep exact silent-token detection case-insensitive again so mixed-case `NO_REPLY` outputs still stay silent in text and direct delivery paths. Thanks @obviyus. +- Core/approvals: share approval-not-found fallback classification through the narrow `plugin-sdk/error-runtime` seam so core `/approve` and Telegram stay aligned without widening `plugin-sdk/infra-runtime`. (#60932) Thanks @gumadeiras. ## 2026.4.2 diff --git a/docs/.generated/plugin-sdk-api-baseline.sha256 b/docs/.generated/plugin-sdk-api-baseline.sha256 index a06fd98c806..f54866b70db 100644 --- a/docs/.generated/plugin-sdk-api-baseline.sha256 +++ b/docs/.generated/plugin-sdk-api-baseline.sha256 @@ -1,2 +1,2 @@ -8c38d3e2d0a61c02db70070e0d032b54b1de474000e1c1221332efc495e8681d plugin-sdk-api-baseline.json -d057310712f83b27f64b53dbe45ef2e92795407e56503a255de0f29d915c1ee4 plugin-sdk-api-baseline.jsonl +0cd9a43c490bb5511890171543a3029754d44c9f1fe1ebf6f5c845fb49f44452 plugin-sdk-api-baseline.json +66e1a9dff2b6c170dd1caceef1f15ad63c18f89c897d98f502cac1f2f46d26c2 plugin-sdk-api-baseline.jsonl diff --git a/docs/plugins/sdk-migration.md b/docs/plugins/sdk-migration.md index 39df55f8683..70f2ae3d9ee 100644 --- a/docs/plugins/sdk-migration.md +++ b/docs/plugins/sdk-migration.md @@ -207,7 +207,7 @@ Current bundled provider examples: | `plugin-sdk/ssrf-runtime` | SSRF runtime helpers | Pinned-dispatcher, guarded fetch, SSRF policy helpers | | `plugin-sdk/collection-runtime` | Bounded cache helpers | `pruneMapToMaxSize` | | `plugin-sdk/diagnostic-runtime` | Diagnostic gating helpers | `isDiagnosticFlagEnabled`, `isDiagnosticsEnabled` | - | `plugin-sdk/error-runtime` | Error formatting helpers | `formatUncaughtError`, error graph helpers | + | `plugin-sdk/error-runtime` | Error formatting helpers | `formatUncaughtError`, `isApprovalNotFoundError`, error graph helpers | | `plugin-sdk/fetch-runtime` | Wrapped fetch/proxy helpers | `resolveFetch`, proxy helpers | | `plugin-sdk/host-runtime` | Host normalization helpers | `normalizeHostname`, `normalizeScpRemoteHost` | | `plugin-sdk/retry-runtime` | Retry helpers | `RetryConfig`, `retryAsync`, policy runners | diff --git a/docs/plugins/sdk-overview.md b/docs/plugins/sdk-overview.md index 5d79cec1670..09d382e451a 100644 --- a/docs/plugins/sdk-overview.md +++ b/docs/plugins/sdk-overview.md @@ -210,7 +210,7 @@ explicitly promotes one as public. | `plugin-sdk/infra-runtime` | System event/heartbeat helpers | | `plugin-sdk/collection-runtime` | Small bounded cache helpers | | `plugin-sdk/diagnostic-runtime` | Diagnostic flag and event helpers | - | `plugin-sdk/error-runtime` | Error graph and formatting helpers | + | `plugin-sdk/error-runtime` | Error graph, formatting, and shared error classification helpers | | `plugin-sdk/fetch-runtime` | Wrapped fetch, proxy, and pinned lookup helpers | | `plugin-sdk/host-runtime` | Hostname and SCP host normalization helpers | | `plugin-sdk/retry-runtime` | Retry config and retry runner helpers | diff --git a/extensions/telegram/src/exec-approval-resolver.ts b/extensions/telegram/src/exec-approval-resolver.ts index 8e2164b8582..dee19b6586a 100644 --- a/extensions/telegram/src/exec-approval-resolver.ts +++ b/extensions/telegram/src/exec-approval-resolver.ts @@ -1,4 +1,5 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; +import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime"; import { createOperatorApprovalsGatewayClient } from "openclaw/plugin-sdk/gateway-runtime"; import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/infra-runtime"; @@ -11,38 +12,6 @@ export type ResolveTelegramExecApprovalParams = { gatewayUrl?: string; }; -const INVALID_REQUEST = "INVALID_REQUEST"; -const APPROVAL_NOT_FOUND = "APPROVAL_NOT_FOUND"; - -function readErrorCode(value: unknown): string | null { - return typeof value === "string" && value.trim() ? value : null; -} - -function readApprovalNotFoundDetailsReason(value: unknown): string | null { - if (!value || typeof value !== "object" || Array.isArray(value)) { - return null; - } - const reason = (value as { reason?: unknown }).reason; - return typeof reason === "string" && reason.trim() ? reason : null; -} - -function isApprovalNotFoundError(err: unknown): boolean { - if (!(err instanceof Error)) { - return false; - } - const gatewayCode = readErrorCode((err as { gatewayCode?: unknown }).gatewayCode); - if (gatewayCode === APPROVAL_NOT_FOUND) { - return true; - } - - const detailsReason = readApprovalNotFoundDetailsReason((err as { details?: unknown }).details); - if (gatewayCode === INVALID_REQUEST && detailsReason === APPROVAL_NOT_FOUND) { - return true; - } - - return /unknown or expired approval id/i.test(err.message); -} - export async function resolveTelegramExecApproval( params: ResolveTelegramExecApprovalParams, ): Promise { diff --git a/src/auto-reply/reply/commands-approve.ts b/src/auto-reply/reply/commands-approve.ts index 8f15c1b1678..0942539b089 100644 --- a/src/auto-reply/reply/commands-approve.ts +++ b/src/auto-reply/reply/commands-approve.ts @@ -3,8 +3,8 @@ import { resolveChannelApprovalCapability, } from "../../channels/plugins/index.js"; import { callGateway } from "../../gateway/call.js"; -import { ErrorCodes } from "../../gateway/protocol/index.js"; import { logVerbose } from "../../globals.js"; +import { isApprovalNotFoundError } from "../../infra/approval-errors.js"; import { resolveApprovalCommandAuthorization } from "../../infra/channel-approval-auth.js"; import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../../utils/message-channel.js"; import { resolveChannelAccountId } from "./channel-context.js"; @@ -78,39 +78,6 @@ function buildResolvedByLabel(params: Parameters[0]): string { return `${channel}:${sender}`; } -function readErrorCode(value: unknown): string | null { - return typeof value === "string" && value.trim() ? value : null; -} - -function readApprovalNotFoundDetailsReason(value: unknown): string | null { - if (!value || typeof value !== "object" || Array.isArray(value)) { - return null; - } - const reason = (value as { reason?: unknown }).reason; - return typeof reason === "string" && reason.trim() ? reason : null; -} - -function isApprovalNotFoundError(err: unknown): boolean { - if (!(err instanceof Error)) { - return false; - } - const gatewayCode = readErrorCode((err as { gatewayCode?: unknown }).gatewayCode); - if (gatewayCode === ErrorCodes.APPROVAL_NOT_FOUND) { - return true; - } - - const detailsReason = readApprovalNotFoundDetailsReason((err as { details?: unknown }).details); - if ( - gatewayCode === ErrorCodes.INVALID_REQUEST && - detailsReason === ErrorCodes.APPROVAL_NOT_FOUND - ) { - return true; - } - - // Legacy server/client combinations may only include the message text. - return /unknown or expired approval id/i.test(err.message); -} - function formatApprovalSubmitError(error: unknown): string { return error instanceof Error ? error.message : String(error); } diff --git a/src/infra/approval-errors.test.ts b/src/infra/approval-errors.test.ts new file mode 100644 index 00000000000..621409b5ba8 --- /dev/null +++ b/src/infra/approval-errors.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { isApprovalNotFoundError } from "./approval-errors.js"; + +describe("isApprovalNotFoundError", () => { + it("matches direct approval-not-found gateway codes", () => { + const err = new Error("approval not found") as Error & { gatewayCode?: string }; + err.gatewayCode = "APPROVAL_NOT_FOUND"; + expect(isApprovalNotFoundError(err)).toBe(true); + }); + + it("matches structured invalid-request approval-not-found details", () => { + const err = new Error("approval not found") as Error & { + gatewayCode?: string; + details?: { reason?: string }; + }; + err.gatewayCode = "INVALID_REQUEST"; + err.details = { reason: "APPROVAL_NOT_FOUND" }; + expect(isApprovalNotFoundError(err)).toBe(true); + }); + + it("matches legacy message-only not-found errors", () => { + expect(isApprovalNotFoundError(new Error("unknown or expired approval id"))).toBe(true); + }); + + it("ignores unrelated errors", () => { + expect(isApprovalNotFoundError(new Error("network timeout"))).toBe(false); + expect(isApprovalNotFoundError("unknown or expired approval id")).toBe(false); + }); +}); diff --git a/src/infra/approval-errors.ts b/src/infra/approval-errors.ts new file mode 100644 index 00000000000..285d3809036 --- /dev/null +++ b/src/infra/approval-errors.ts @@ -0,0 +1,29 @@ +const INVALID_REQUEST = "INVALID_REQUEST"; +const APPROVAL_NOT_FOUND = "APPROVAL_NOT_FOUND"; + +function readErrorCode(value: unknown): string | null { + return typeof value === "string" && value.trim() ? value : null; +} + +function readApprovalNotFoundDetailsReason(value: unknown): string | null { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return null; + } + const reason = (value as { reason?: unknown }).reason; + return typeof reason === "string" && reason.trim() ? reason : null; +} + +export function isApprovalNotFoundError(err: unknown): boolean { + if (!(err instanceof Error)) { + return false; + } + const gatewayCode = readErrorCode((err as { gatewayCode?: unknown }).gatewayCode); + if (gatewayCode === APPROVAL_NOT_FOUND) { + return true; + } + const detailsReason = readApprovalNotFoundDetailsReason((err as { details?: unknown }).details); + if (gatewayCode === INVALID_REQUEST && detailsReason === APPROVAL_NOT_FOUND) { + return true; + } + return /unknown or expired approval id/i.test(err.message); +} diff --git a/src/plugin-sdk/error-runtime.ts b/src/plugin-sdk/error-runtime.ts index a83ec55fa43..2fe70e21eb3 100644 --- a/src/plugin-sdk/error-runtime.ts +++ b/src/plugin-sdk/error-runtime.ts @@ -7,3 +7,4 @@ export { formatUncaughtError, readErrorName, } from "../infra/errors.js"; +export { isApprovalNotFoundError } from "../infra/approval-errors.ts"; diff --git a/src/plugins/contracts/plugin-sdk-subpaths.test.ts b/src/plugins/contracts/plugin-sdk-subpaths.test.ts index 3e6b3116233..f21146e69b4 100644 --- a/src/plugins/contracts/plugin-sdk-subpaths.test.ts +++ b/src/plugins/contracts/plugin-sdk-subpaths.test.ts @@ -841,6 +841,7 @@ describe("plugin-sdk subpath exports", () => { expectSourceMentions("infra-runtime", ["createRuntimeOutboundDelegates"]); expectSourceContains("infra-runtime", "../infra/outbound/send-deps.js"); + expectSourceMentions("error-runtime", ["formatUncaughtError", "isApprovalNotFoundError"]); expect(typeof channelLifecycleSdk.createDraftStreamLoop).toBe("function"); expect(typeof channelLifecycleSdk.createFinalizableDraftLifecycle).toBe("function");