mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-18 23:34:44 +00:00
fix(gateway): clear node wake state without registration (#68848)
Summary:
- Clear speculative gateway node wake state when APNs registration is missing.
- Add regression coverage for unregistered node IDs.
- Add changelog credit for @Feelw00.
Verification:
- git diff --check
- pnpm test src/gateway/server-methods/nodes.wake-leak.test.ts src/gateway/server-methods/nodes.invoke-wake.test.ts
- GitHub exact-head checks green on 29db03ff4e
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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();
|
||||
},
|
||||
};
|
||||
|
||||
@@ -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" });
|
||||
}
|
||||
|
||||
|
||||
78
src/gateway/server-methods/nodes.wake-leak.test.ts
Normal file
78
src/gateway/server-methods/nodes.wake-leak.test.ts
Normal file
@@ -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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user