From 7a89049d1d45b1e4cb7ef36012c185a9586b8aa0 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 19 Feb 2026 13:54:35 +0000 Subject: [PATCH] refactor: dedupe pending pairing request flow and add reuse tests --- src/infra/device-pairing.test.ts | 22 ++++++++++++++ src/infra/device-pairing.ts | 48 ++++++++++++++--------------- src/infra/node-pairing.test.ts | 22 ++++++++++++++ src/infra/node-pairing.ts | 52 +++++++++++++++----------------- src/infra/pairing-files.ts | 24 +++++++++++++++ 5 files changed, 117 insertions(+), 51 deletions(-) diff --git a/src/infra/device-pairing.test.ts b/src/infra/device-pairing.test.ts index ab0864b9f4c..cb9bd0d2dd5 100644 --- a/src/infra/device-pairing.test.ts +++ b/src/infra/device-pairing.test.ts @@ -33,6 +33,28 @@ function requireToken(token: string | undefined): string { } describe("device pairing tokens", () => { + test("reuses existing pending requests for the same device", async () => { + const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); + const first = await requestDevicePairing( + { + deviceId: "device-1", + publicKey: "public-key-1", + }, + baseDir, + ); + const second = await requestDevicePairing( + { + deviceId: "device-1", + publicKey: "public-key-1", + }, + baseDir, + ); + + expect(first.created).toBe(true); + expect(second.created).toBe(false); + expect(second.request.requestId).toBe(first.request.requestId); + }); + test("generates base64url device tokens with 256-bit entropy output length", async () => { const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); await setupPairedOperatorDevice(baseDir, ["operator.admin"]); diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts index 884f2e9dd31..122463f6e63 100644 --- a/src/infra/device-pairing.ts +++ b/src/infra/device-pairing.ts @@ -5,6 +5,7 @@ import { pruneExpiredPending, readJsonFile, resolvePairingPaths, + upsertPendingPairingRequest, writeJsonAtomic, } from "./pairing-files.js"; import { generatePairingToken, verifyPairingToken } from "./pairing-token.js"; @@ -226,30 +227,29 @@ export async function requestDevicePairing( if (!deviceId) { throw new Error("deviceId required"); } - const existing = Object.values(state.pendingById).find((p) => p.deviceId === deviceId); - if (existing) { - return { status: "pending", request: existing, created: false }; - } - const isRepair = Boolean(state.pairedByDeviceId[deviceId]); - const request: DevicePairingPendingRequest = { - requestId: randomUUID(), - deviceId, - publicKey: req.publicKey, - displayName: req.displayName, - platform: req.platform, - clientId: req.clientId, - clientMode: req.clientMode, - role: req.role, - roles: req.role ? [req.role] : undefined, - scopes: req.scopes, - remoteIp: req.remoteIp, - silent: req.silent, - isRepair, - ts: Date.now(), - }; - state.pendingById[request.requestId] = request; - await persistState(state, baseDir); - return { status: "pending", request, created: true }; + + return await upsertPendingPairingRequest({ + pendingById: state.pendingById, + isExisting: (pending) => pending.deviceId === deviceId, + isRepair: Boolean(state.pairedByDeviceId[deviceId]), + createRequest: (isRepair) => ({ + requestId: randomUUID(), + deviceId, + publicKey: req.publicKey, + displayName: req.displayName, + platform: req.platform, + clientId: req.clientId, + clientMode: req.clientMode, + role: req.role, + roles: req.role ? [req.role] : undefined, + scopes: req.scopes, + remoteIp: req.remoteIp, + silent: req.silent, + isRepair, + ts: Date.now(), + }), + persist: async () => await persistState(state, baseDir), + }); }); } diff --git a/src/infra/node-pairing.test.ts b/src/infra/node-pairing.test.ts index 17c83c03500..8700f3d034f 100644 --- a/src/infra/node-pairing.test.ts +++ b/src/infra/node-pairing.test.ts @@ -28,6 +28,28 @@ async function setupPairedNode(baseDir: string): Promise { } describe("node pairing tokens", () => { + test("reuses existing pending requests for the same node", async () => { + const baseDir = await mkdtemp(join(tmpdir(), "openclaw-node-pairing-")); + const first = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + }, + baseDir, + ); + const second = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + }, + baseDir, + ); + + expect(first.created).toBe(true); + expect(second.created).toBe(false); + expect(second.request.requestId).toBe(first.request.requestId); + }); + test("generates base64url node tokens with 256-bit entropy output length", async () => { const baseDir = await mkdtemp(join(tmpdir(), "openclaw-node-pairing-")); const token = await setupPairedNode(baseDir); diff --git a/src/infra/node-pairing.ts b/src/infra/node-pairing.ts index 88c428df13d..d8a55b57621 100644 --- a/src/infra/node-pairing.ts +++ b/src/infra/node-pairing.ts @@ -4,6 +4,7 @@ import { pruneExpiredPending, readJsonFile, resolvePairingPaths, + upsertPendingPairingRequest, writeJsonAtomic, } from "./pairing-files.js"; import { generatePairingToken, verifyPairingToken } from "./pairing-token.js"; @@ -123,33 +124,30 @@ export async function requestNodePairing( throw new Error("nodeId required"); } - const existing = Object.values(state.pendingById).find((p) => p.nodeId === nodeId); - if (existing) { - return { status: "pending", request: existing, created: false }; - } - - const isRepair = Boolean(state.pairedByNodeId[nodeId]); - const request: NodePairingPendingRequest = { - requestId: randomUUID(), - nodeId, - displayName: req.displayName, - platform: req.platform, - version: req.version, - coreVersion: req.coreVersion, - uiVersion: req.uiVersion, - deviceFamily: req.deviceFamily, - modelIdentifier: req.modelIdentifier, - caps: req.caps, - commands: req.commands, - permissions: req.permissions, - remoteIp: req.remoteIp, - silent: req.silent, - isRepair, - ts: Date.now(), - }; - state.pendingById[request.requestId] = request; - await persistState(state, baseDir); - return { status: "pending", request, created: true }; + return await upsertPendingPairingRequest({ + pendingById: state.pendingById, + isExisting: (pending) => pending.nodeId === nodeId, + isRepair: Boolean(state.pairedByNodeId[nodeId]), + createRequest: (isRepair) => ({ + requestId: randomUUID(), + nodeId, + displayName: req.displayName, + platform: req.platform, + version: req.version, + coreVersion: req.coreVersion, + uiVersion: req.uiVersion, + deviceFamily: req.deviceFamily, + modelIdentifier: req.modelIdentifier, + caps: req.caps, + commands: req.commands, + permissions: req.permissions, + remoteIp: req.remoteIp, + silent: req.silent, + isRepair, + ts: Date.now(), + }), + persist: async () => await persistState(state, baseDir), + }); }); } diff --git a/src/infra/pairing-files.ts b/src/infra/pairing-files.ts index f2578facdfb..6c5bf0ee738 100644 --- a/src/infra/pairing-files.ts +++ b/src/infra/pairing-files.ts @@ -24,3 +24,27 @@ export function pruneExpiredPending( } } } + +export type PendingPairingRequestResult = { + status: "pending"; + request: TPending; + created: boolean; +}; + +export async function upsertPendingPairingRequest(params: { + pendingById: Record; + isExisting: (pending: TPending) => boolean; + createRequest: (isRepair: boolean) => TPending; + isRepair: boolean; + persist: () => Promise; +}): Promise> { + const existing = Object.values(params.pendingById).find(params.isExisting); + if (existing) { + return { status: "pending", request: existing, created: false }; + } + + const request = params.createRequest(params.isRepair); + params.pendingById[request.requestId] = request; + await params.persist(); + return { status: "pending", request, created: true }; +}