From ae7f365fbc50a2c12540b02587b46289ec164359 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 28 Apr 2026 01:09:57 +0100 Subject: [PATCH] fix: stop native approval auth retry loops --- CHANGELOG.md | 1 + src/gateway/operator-approvals-client.ts | 8 +- src/infra/approval-handler-bootstrap.test.ts | 40 +++++++++ src/infra/approval-handler-bootstrap.ts | 5 ++ .../exec-approval-channel-runtime.test.ts | 82 ++++++++++++++++++- src/infra/exec-approval-channel-runtime.ts | 44 +++++++++- 6 files changed, 176 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30993dbb23c..8b83f9dabd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ Docs: https://docs.openclaw.ai - Plugins/runtime deps: declare retained staged bundled plugin dependencies in the npm staging manifest while installing only newly missing packages, so Gateway restarts avoid reinstalling the full retained dependency set when one runtime dependency is absent. Fixes #73055. Thanks @GCorp2026. - CLI/status: keep default `openclaw status` off the heavyweight security audit, plugin compatibility, and memory-vector probes while still showing configured Telegram channels through setup metadata, so routine health checks stay fast and no longer render an empty Channels table. Fixes #72993. Thanks @comick1. - Channels/Telegram: send a best-effort native typing cue immediately after an inbound message is accepted, so slow pre-dispatch turns show Telegram liveness before queueing, compaction, model, or tool work starts. Fixes #63759. Thanks @alessandropcostabr. +- Channels/Telegram: stop native approval startup auth failures from retrying every second, while still waiting through retryable Gateway auth handoffs, so Telegram approval setup problems no longer create a reconnect/log loop during channel startup. Refs #72846 and #72867. Thanks @kiranvk-2011 and @porly1985. - Channels/Microsoft Teams: unwrap staged CommonJS JWT runtime dependencies before Bot Connector token validation so inbound Teams messages no longer 401 after the bundled runtime-deps move. Fixes #73026. Thanks @kbrown10000. - Gateway/auth: allow local direct callers in trusted-proxy mode to use the configured gateway password as an internal fallback while keeping token fallback rejected. Fixes #17761. Thanks @dashed, @vincentkoc, and @jetd1. - Channels/sessions: prevent guarded inbound session recording from creating route-only phantom sessions while still allowing last-route updates for sessions that already exist. Carries forward #73009. Thanks @jzakirov. diff --git a/src/gateway/operator-approvals-client.ts b/src/gateway/operator-approvals-client.ts index 87842d75b2c..4428cef76f0 100644 --- a/src/gateway/operator-approvals-client.ts +++ b/src/gateway/operator-approvals-client.ts @@ -6,7 +6,12 @@ import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "./protocol/client-in export async function createOperatorApprovalsGatewayClient( params: Pick< GatewayClientOptions, - "clientDisplayName" | "onClose" | "onConnectError" | "onEvent" | "onHelloOk" + | "clientDisplayName" + | "onClose" + | "onConnectError" + | "onEvent" + | "onHelloOk" + | "onReconnectPaused" > & { config: OpenClawConfig; gatewayUrl?: string; @@ -29,6 +34,7 @@ export async function createOperatorApprovalsGatewayClient( onEvent: params.onEvent, onHelloOk: params.onHelloOk, onConnectError: params.onConnectError, + onReconnectPaused: params.onReconnectPaused, onClose: params.onClose, }); } diff --git a/src/infra/approval-handler-bootstrap.test.ts b/src/infra/approval-handler-bootstrap.test.ts index ee5f555133b..4b4aa42d2c0 100644 --- a/src/infra/approval-handler-bootstrap.test.ts +++ b/src/infra/approval-handler-bootstrap.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { createRuntimeChannel } from "../plugins/runtime/runtime-channel.js"; import { startChannelApprovalHandlerBootstrap } from "./approval-handler-bootstrap.js"; import { createApprovalNativeRuntimeAdapterStubs } from "./approval-handler.test-helpers.js"; +import { ExecApprovalChannelRuntimeTerminalStartError } from "./exec-approval-channel-runtime.js"; const { createChannelApprovalHandlerFromCapability } = vi.hoisted(() => ({ createChannelApprovalHandlerFromCapability: vi.fn(), @@ -231,6 +232,45 @@ describe("startChannelApprovalHandlerBootstrap", () => { await cleanup(); }); + it("does not retry terminal native approval startup failures", async () => { + vi.useFakeTimers(); + const channelRuntime = createRuntimeChannel(); + const terminalError = new ExecApprovalChannelRuntimeTerminalStartError({ + code: 1008, + reason: "pairing required", + detailCode: "PAIRING_REQUIRED", + }); + const start = vi.fn().mockRejectedValue(terminalError); + const stop = vi.fn().mockResolvedValue(undefined); + const logger = { + error: vi.fn(), + warn: vi.fn(), + info: vi.fn(), + debug: vi.fn(), + child: vi.fn(), + isEnabled: vi.fn().mockReturnValue(true), + isVerboseEnabled: vi.fn().mockReturnValue(false), + verbose: vi.fn(), + }; + createChannelApprovalHandlerFromCapability.mockResolvedValue({ start, stop }); + + const cleanup = await startTestBootstrap({ channelRuntime, logger }); + + registerApprovalContext(channelRuntime); + await flushTransitions(); + await vi.advanceTimersByTimeAsync(3_000); + await flushTransitions(); + + expect(createChannelApprovalHandlerFromCapability).toHaveBeenCalledTimes(1); + expect(start).toHaveBeenCalledTimes(1); + expect(stop).toHaveBeenCalledTimes(1); + expect(logger.error).toHaveBeenCalledWith( + `native approval handler disabled: ${String(terminalError)}`, + ); + + await cleanup(); + }); + it("does not let a stale retry stop a newer active handler", async () => { vi.useFakeTimers(); const channelRuntime = createRuntimeChannel(); diff --git a/src/infra/approval-handler-bootstrap.ts b/src/infra/approval-handler-bootstrap.ts index c9fca820f96..a1ccce4424a 100644 --- a/src/infra/approval-handler-bootstrap.ts +++ b/src/infra/approval-handler-bootstrap.ts @@ -12,6 +12,7 @@ import { getChannelRuntimeContext, watchChannelRuntimeContexts, } from "./channel-runtime-context.js"; +import { isExecApprovalChannelRuntimeTerminalStartError } from "./exec-approval-channel-runtime.js"; type ApprovalBootstrapHandler = ChannelApprovalHandler; const APPROVAL_HANDLER_BOOTSTRAP_RETRY_MS = 1_000; @@ -117,6 +118,10 @@ export async function startChannelApprovalHandlerBootstrap(params: { await startHandlerForContext(context, generation); } catch (error) { if (generation === activeGeneration) { + if (isExecApprovalChannelRuntimeTerminalStartError(error)) { + logger.error(`native approval handler disabled: ${String(error)}`); + return; + } logger.error(`failed to start native approval handler: ${String(error)}`); scheduleRetryForContext(context, generation); } diff --git a/src/infra/exec-approval-channel-runtime.test.ts b/src/infra/exec-approval-channel-runtime.test.ts index 16925693304..f7de18d32f3 100644 --- a/src/infra/exec-approval-channel-runtime.test.ts +++ b/src/infra/exec-approval-channel-runtime.test.ts @@ -25,6 +25,7 @@ vi.mock("../logging/subsystem.js", () => ({ })); let createExecApprovalChannelRuntime: typeof import("./exec-approval-channel-runtime.js").createExecApprovalChannelRuntime; +let ExecApprovalChannelRuntimeTerminalStartError: typeof import("./exec-approval-channel-runtime.js").ExecApprovalChannelRuntimeTerminalStartError; function createDeferred() { let resolve!: (value: T | PromiseLike) => void; @@ -116,7 +117,8 @@ afterEach(() => { }); beforeAll(async () => { - ({ createExecApprovalChannelRuntime } = await import("./exec-approval-channel-runtime.js")); + ({ createExecApprovalChannelRuntime, ExecApprovalChannelRuntimeTerminalStartError } = + await import("./exec-approval-channel-runtime.js")); }); describe("createExecApprovalChannelRuntime", () => { @@ -287,6 +289,84 @@ describe("createExecApprovalChannelRuntime", () => { expect(mockGatewayClientStarts).toHaveBeenCalledTimes(1); }); + it("waits through retryable connect auth errors until hello succeeds", async () => { + const authError = Object.assign(new Error("gateway token mismatch"), { + details: { + code: "AUTH_TOKEN_MISMATCH", + canRetryWithDeviceToken: true, + }, + }); + mockCreateOperatorApprovalsGatewayClient.mockImplementationOnce(async (params) => ({ + start: () => { + mockGatewayClientStarts(); + params.onConnectError?.(authError); + queueMicrotask(() => { + params.onHelloOk?.({ type: "hello-ok" } as never); + }); + }, + stop: mockGatewayClientStops, + request: mockGatewayClientRequests, + })); + const runtime = createExecApprovalChannelRuntime({ + label: "test/exec-approvals", + clientDisplayName: "Test Exec Approvals", + cfg: {} as never, + isConfigured: () => true, + shouldHandle: () => true, + deliverRequested: async () => [], + finalizeResolved: async () => undefined, + }); + + await expect(runtime.start()).resolves.toBeUndefined(); + + expect(mockGatewayClientStarts).toHaveBeenCalledTimes(1); + expect(loggerMocks.error).toHaveBeenCalledWith("connect error: gateway token mismatch"); + }); + + it("surfaces reconnect pauses as terminal startup errors", async () => { + const authError = Object.assign(new Error("pairing required"), { + details: { + code: "PAIRING_REQUIRED", + }, + }); + mockCreateOperatorApprovalsGatewayClient.mockImplementationOnce(async (params) => ({ + start: () => { + mockGatewayClientStarts(); + params.onConnectError?.(authError); + params.onReconnectPaused?.({ + code: 1008, + reason: "pairing required", + detailCode: "PAIRING_REQUIRED", + }); + params.onClose?.(1008, "pairing required"); + }, + stop: mockGatewayClientStops, + request: mockGatewayClientRequests, + })); + const runtime = createExecApprovalChannelRuntime({ + label: "test/exec-approvals", + clientDisplayName: "Test Exec Approvals", + cfg: {} as never, + isConfigured: () => true, + shouldHandle: () => true, + deliverRequested: async () => [], + finalizeResolved: async () => undefined, + }); + + let caught: unknown; + await runtime.start().catch((error) => { + caught = error; + }); + + expect(caught).toBeInstanceOf(ExecApprovalChannelRuntimeTerminalStartError); + expect(caught).toMatchObject({ + detailCode: "PAIRING_REQUIRED", + }); + + expect(mockGatewayClientStarts).toHaveBeenCalledTimes(1); + expect(mockGatewayClientStops).toHaveBeenCalledTimes(1); + }); + it("does not leave a gateway client running when stop wins the startup race", async () => { const pendingClient = createDeferred(); mockCreateOperatorApprovalsGatewayClient.mockReturnValueOnce(pendingClient.promise); diff --git a/src/infra/exec-approval-channel-runtime.ts b/src/infra/exec-approval-channel-runtime.ts index 938d2332843..c86c8118f46 100644 --- a/src/infra/exec-approval-channel-runtime.ts +++ b/src/infra/exec-approval-channel-runtime.ts @@ -1,5 +1,6 @@ -import type { GatewayClient } from "../gateway/client.js"; +import type { GatewayClient, GatewayReconnectPausedInfo } from "../gateway/client.js"; import { createOperatorApprovalsGatewayClient } from "../gateway/operator-approvals-client.js"; +import { readConnectErrorDetailCode } from "../gateway/protocol/connect-error-details.js"; import type { EventFrame } from "../gateway/protocol/index.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { formatErrorMessage } from "./errors.js"; @@ -19,6 +20,26 @@ export type { type ApprovalRequestEvent = ExecApprovalRequest | PluginApprovalRequest; type ApprovalResolvedEvent = ExecApprovalResolved | PluginApprovalResolved; +export class ExecApprovalChannelRuntimeTerminalStartError extends Error { + readonly detailCode: string | null; + + constructor(info: GatewayReconnectPausedInfo, cause?: unknown) { + super( + `native approval gateway client paused reconnect after startup auth failure` + + ` (${info.detailCode ?? "unknown"}): gateway closed (${info.code}): ${info.reason}`, + cause === undefined ? undefined : { cause }, + ); + this.name = "ExecApprovalChannelRuntimeTerminalStartError"; + this.detailCode = info.detailCode; + } +} + +export function isExecApprovalChannelRuntimeTerminalStartError( + error: unknown, +): error is ExecApprovalChannelRuntimeTerminalStartError { + return error instanceof ExecApprovalChannelRuntimeTerminalStartError; +} + type PendingApprovalEntry< TPending, TRequest extends ApprovalRequestEvent, @@ -44,6 +65,13 @@ function resolveApprovalReplayMethods( return methods; } +function readGatewayConnectErrorDetailCode(error: unknown): string | null { + if (!error || typeof error !== "object") { + return null; + } + return readConnectErrorDetailCode((error as { details?: unknown }).details); +} + export function createExecApprovalChannelRuntime< TPending, TRequest extends ApprovalRequestEvent = ExecApprovalRequest, @@ -284,6 +312,7 @@ export function createExecApprovalChannelRuntime< resolveReady = resolve; rejectReady = reject; }); + let lastConnectError: unknown = null; const settleReady = (fn: () => void) => { if (readySettled) { return; @@ -303,11 +332,22 @@ export function createExecApprovalChannelRuntime< }, onConnectError: (err) => { log.error(`connect error: ${err.message}`); + lastConnectError = err; + if (readGatewayConnectErrorDetailCode(err)) { + return; + } settleReady(() => rejectReady(err)); }, + onReconnectPaused: (info) => { + settleReady(() => + rejectReady(new ExecApprovalChannelRuntimeTerminalStartError(info, lastConnectError)), + ); + }, onClose: (code, reason) => { log.debug(`gateway closed: ${code} ${reason}`); - settleReady(() => rejectReady(new Error(`gateway closed: ${code} ${reason}`))); + settleReady(() => + rejectReady(lastConnectError ?? new Error(`gateway closed: ${code} ${reason}`)), + ); }, });