diff --git a/CHANGELOG.md b/CHANGELOG.md index d2bf8bd3189..e49f3dd1455 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Gateway: clear speculative node wake state when APNs registration is missing, preventing unregistered or mistyped node IDs from retaining wake throttle entries. Fixes #68847. (#68848) Thanks @Feelw00. - Feishu: make manual App ID/App Secret setup the default channel-binding path while keeping QR scan-to-create as an optional best-effort flow, and document the manual fallback for domestic Feishu mobile clients that do not react to the QR code. Fixes #80591. Thanks @wei-wei-zhao. - Telegram: show resolved thinking defaults in native `/status` and `/think` menus while preserving explicit session overrides. (#80341) Thanks @VACInc. - Channels: cache selected channel registry lookups against the active fallback snapshot so pinned-empty registries refresh native command and alias routing after active registry swaps. (#80333) Thanks @samzong. diff --git a/src/gateway/server-methods/nodes-wake-state.ts b/src/gateway/server-methods/nodes-wake-state.ts index dc33018da00..9c75d07f6fb 100644 --- a/src/gateway/server-methods/nodes-wake-state.ts +++ b/src/gateway/server-methods/nodes-wake-state.ts @@ -23,3 +23,20 @@ export function clearNodeWakeState(nodeId: string): void { nodeWakeById.delete(nodeId); nodeWakeNudgeById.delete(nodeId); } + +// Narrow read-only seam for tests that assert nodeWakeById is cleaned up on +// early-return paths. Mirrors the pattern used in agent-wait-dedupe.ts:223 +// and agents.ts:78 — keep production surface untouched and do not expose the +// underlying Map reference. +export const __testing = { + getNodeWakeByIdSize(): number { + return nodeWakeById.size; + }, + hasNodeWakeEntry(nodeId: string): boolean { + return nodeWakeById.has(nodeId); + }, + resetWakeState(): void { + nodeWakeById.clear(); + nodeWakeNudgeById.clear(); + }, +}; diff --git a/src/gateway/server-methods/nodes.ts b/src/gateway/server-methods/nodes.ts index 1b0a3f0ce07..bda3e379987 100644 --- a/src/gateway/server-methods/nodes.ts +++ b/src/gateway/server-methods/nodes.ts @@ -445,6 +445,12 @@ export async function maybeWakeNodeWithApns( try { const registration = await loadApnsRegistration(nodeId); if (!registration) { + // Avoid leaking the state entry we speculatively set at the top of + // maybeWakeNodeWithApns: this nodeId has no APNs registration, so the + // throttle bookkeeping we just created will never be touched by the + // WS-close cleanup path (clearNodeWakeState is only called for + // registered nodes in ws-connection.ts). + nodeWakeById.delete(nodeId); return withDuration({ available: false, throttled: false, path: "no-registration" }); } diff --git a/src/gateway/server-methods/nodes.wake-leak.test.ts b/src/gateway/server-methods/nodes.wake-leak.test.ts new file mode 100644 index 00000000000..87ee643b9b8 --- /dev/null +++ b/src/gateway/server-methods/nodes.wake-leak.test.ts @@ -0,0 +1,78 @@ +// Regression: maybeWakeNodeWithApns (nodes.ts:308-416) speculatively sets +// nodeWakeById at the top for in-flight coalescing, but on the no-registration +// early-return path (loadApnsRegistration returns null) the entry was never +// removed. The sole cleanup path (clearNodeWakeState, wired from +// ws-connection.ts:327 on WS close) only fires for registered nodes, so any +// operator-driven RPC against an unregistered/re-paired/typo nodeId leaked a +// permanent { lastWakeAtMs: 0 } entry. +// +// Fix: delete the nodeWakeById entry before returning no-registration. +// +// PR #63709 (merged 2026-04-09) introduced clearNodeWakeState for WS close — +// this change is a different leak path (unregistered early-return) and +// complements that PR. +// +// CAL-003 compliance: the null-registration branch is already exercised by +// existing nodes.invoke-wake.test.ts cases. The test just observes that the +// Map size returns to 0, using a minimal read-only __testing seam mirrored on +// agent-wait-dedupe.ts:223 and agents.ts:78. + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + loadApnsRegistration: vi.fn(), + resolveApnsAuthConfigFromEnv: vi.fn(), + resolveApnsRelayConfigFromEnv: vi.fn(), + sendApnsBackgroundWake: vi.fn(), + sendApnsAlert: vi.fn(), + clearApnsRegistrationIfCurrent: vi.fn(), + shouldClearStoredApnsRegistration: vi.fn(() => false), +})); + +vi.mock("../../infra/push-apns.js", () => ({ + clearApnsRegistrationIfCurrent: mocks.clearApnsRegistrationIfCurrent, + loadApnsRegistration: mocks.loadApnsRegistration, + resolveApnsAuthConfigFromEnv: mocks.resolveApnsAuthConfigFromEnv, + resolveApnsRelayConfigFromEnv: mocks.resolveApnsRelayConfigFromEnv, + sendApnsBackgroundWake: mocks.sendApnsBackgroundWake, + sendApnsAlert: mocks.sendApnsAlert, + shouldClearStoredApnsRegistration: mocks.shouldClearStoredApnsRegistration, +})); + +import { maybeWakeNodeWithApns } from "./nodes.js"; +import { __testing as wakeTesting } from "./nodes-wake-state.js"; + +describe("maybeWakeNodeWithApns — no-registration leak guard", () => { + beforeEach(() => { + wakeTesting.resetWakeState(); + vi.clearAllMocks(); + mocks.loadApnsRegistration.mockResolvedValue(null); + }); + + afterEach(() => { + wakeTesting.resetWakeState(); + }); + + it("does not retain nodeWakeById entries for unregistered nodeIds", async () => { + expect(wakeTesting.getNodeWakeByIdSize()).toBe(0); + + for (let i = 0; i < 50; i++) { + const result = await maybeWakeNodeWithApns(`unregistered-node-${i}`); + expect(result).toMatchObject({ + available: false, + throttled: false, + path: "no-registration", + }); + } + + expect(wakeTesting.getNodeWakeByIdSize()).toBe(0); + expect(wakeTesting.hasNodeWakeEntry("unregistered-node-0")).toBe(false); + expect(wakeTesting.hasNodeWakeEntry("unregistered-node-49")).toBe(false); + }); + + it("clears the entry when a single call returns no-registration", async () => { + await maybeWakeNodeWithApns("stale-nodeId"); + expect(wakeTesting.getNodeWakeByIdSize()).toBe(0); + expect(wakeTesting.hasNodeWakeEntry("stale-nodeId")).toBe(false); + }); +});