fix: use non-native dependency and defer machine ID resolution

Author: gagik

src/deferred-promise.ts

@@ -0,0 +1,56 @@
+type DeferredPromiseOptions = {
+    timeout?: number;
+};
+
+/** Creates a promise and exposes its resolve and reject methods, with an optional timeout. */
+export class DeferredPromise<T> extends Promise<T> {
+    resolve!: (value: T) => void;
+    reject!: (reason: unknown) => void;
+    private timeoutId?: NodeJS.Timeout;
+
+    constructor(resolver: (resolve: (value: T) => void, reject: (reason: Error) => void) => void, timeout?: number) {
+        let resolveFn: (value: T) => void;
+        let rejectFn: (reason?: unknown) => void;
+
+        super((resolve, reject) => {
+            resolveFn = resolve;
+            rejectFn = reject;
+        });
+
+        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        this.resolve = resolveFn!;
+        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        this.reject = rejectFn!;
+
+        if (timeout !== undefined) {
+            this.timeoutId = setTimeout(() => {
+                this.reject(new Error("Promise timed out"));
+            }, timeout);
+        }
+
+        if (resolver) {
+            resolver(
+                (value: T) => {
+                    if (this.timeoutId) clearTimeout(this.timeoutId);
+                    this.resolve(value);
+                },
+                (reason: Error) => {
+                    if (this.timeoutId) clearTimeout(this.timeoutId);
+                    this.reject(reason);
+                }
+            );
+        }
+    }
+
+    static fromPromise<T>(promise: Promise<T>, options: DeferredPromiseOptions = {}): DeferredPromise<T> {
+        return new DeferredPromise<T>((resolve, reject) => {
+            promise
+                .then((value) => {
+                    resolve(value);
+                })
+                .catch((reason) => {
+                    reject(reason as Error);
+                });
+        }, options.timeout);
+    }
+}

src/telemetry/telemetry.ts

@@ -7,15 +7,19 @@ import { MACHINE_METADATA } from "./constants.js";
 import { EventCache } from "./eventCache.js";
 import { createHmac } from "crypto";
 import { machineId } from "node-machine-id";
+import { DeferredPromise } from "../deferred-promise.js";
 
 type EventResult = {
     success: boolean;
     error?: Error;
 };
 
+export const DEVICE_ID_TIMEOUT = 3000;
+
 export class Telemetry {
     private isBufferingEvents: boolean = true;
-    private resolveDeviceId: (deviceId: string) => void = () => {};
+    /** Resolves when the device ID is retrieved or timeout occurs */
+    public deviceIdPromise: DeferredPromise<string> | undefined;
 
     private constructor(
         private readonly session: Session,
@@ -25,7 +29,7 @@ export class Telemetry {
 
     static create(
         session: Session,
-        commonProperties: CommonProperties = MACHINE_METADATA,
+        commonProperties: CommonProperties = { ...MACHINE_METADATA },
         eventCache: EventCache = EventCache.getInstance()
     ): Telemetry {
         const instance = new Telemetry(session, commonProperties, eventCache);
@@ -35,55 +39,42 @@ export class Telemetry {
     }
 
     private async start(): Promise<void> {
-        this.commonProperties.device_id = await this.getDeviceId();
+        this.deviceIdPromise = DeferredPromise.fromPromise(this.getDeviceId());
+        this.commonProperties.device_id = await this.deviceIdPromise;
 
         this.isBufferingEvents = false;
-        await this.emitEvents(this.eventCache.getEvents());
     }
 
     public async close(): Promise<void> {
-        this.resolveDeviceId("unknown");
+        this.deviceIdPromise?.resolve("unknown");
         this.isBufferingEvents = false;
         await this.emitEvents(this.eventCache.getEvents());
     }
 
-    private async machineIdWithTimeout(): Promise<string> {
-        try {
-            return Promise.race<string>([
-                machineId(true),
-                new Promise<string>((resolve, reject) => {
-                    this.resolveDeviceId = resolve;
-                    setTimeout(() => {
-                        reject(new Error("Timeout getting machine ID"));
-                    }, 3000);
-                }),
-            ]);
-        } catch (error) {
-            logger.debug(LogId.telemetryMachineIdFailure, "telemetry", `Error getting machine ID: ${String(error)}`);
-            return "unknown";
-        }
-    }
-
     /**
      * @returns A hashed, unique identifier for the running device or `undefined` if not known.
      */
     private async getDeviceId(): Promise<string> {
-        if (this.commonProperties.device_id) {
-            return this.commonProperties.device_id;
-        }
-
-        // Create a hashed format from the all uppercase version of the machine ID
-        // to match it exactly with the denisbrodbeck/machineid library that Atlas CLI uses.
+        try {
+            if (this.commonProperties.device_id) {
+                return this.commonProperties.device_id;
+            }
 
-        const originalId = (await this.machineIdWithTimeout()).toUpperCase();
+            const originalId = await DeferredPromise.fromPromise(machineId(true), { timeout: DEVICE_ID_TIMEOUT });
 
-        const hmac = createHmac("sha256", originalId);
+            // Create a hashed format from the all uppercase version of the machine ID
+            // to match it exactly with the denisbrodbeck/machineid library that Atlas CLI uses.
+            const hmac = createHmac("sha256", originalId.toUpperCase());
 
-        /** This matches the message used to create the hashes in Atlas CLI */
-        const DEVICE_ID_HASH_MESSAGE = "atlascli";
+            /** This matches the message used to create the hashes in Atlas CLI */
+            const DEVICE_ID_HASH_MESSAGE = "atlascli";
 
-        hmac.update(DEVICE_ID_HASH_MESSAGE);
-        return hmac.digest("hex");
+            hmac.update(DEVICE_ID_HASH_MESSAGE);
+            return hmac.digest("hex");
+        } catch (error) {
+            logger.debug(LogId.telemetryMachineIdFailure, "telemetry", String(error));
+            return "unknown";
+        }
     }
 
     /**
@@ -134,6 +125,7 @@ export class Telemetry {
     public getCommonProperties(): CommonProperties {
         return {
             ...this.commonProperties,
+            device_id: this.commonProperties.device_id,
             mcp_client_version: this.session.agentRunner?.version,
             mcp_client_name: this.session.agentRunner?.name,
             session_id: this.session.sessionId,

tests/integration/telemetry.test.ts

@@ -0,0 +1,27 @@
+import { createHmac } from "crypto";
+import { machineId } from "node-machine-id";
+import { Telemetry } from "../../src/telemetry/telemetry.js";
+import { Session } from "../../src/session.js";
+
+describe("Telemetry", () => {
+    it("should resolve the actual machine ID", async () => {
+        const actualId = await machineId(true);
+        // Should be a UUID
+        expect(actualId).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i);
+        const actualHashedId = createHmac("sha256", actualId.toUpperCase()).update("atlascli").digest("hex");
+
+        const telemetry = Telemetry.create(
+            new Session({
+                apiBaseUrl: "",
+            })
+        );
+
+        expect(telemetry.getCommonProperties().device_id).toBe(undefined);
+        expect(telemetry["isBufferingEvents"]).toBe(true);
+
+        await telemetry.deviceIdPromise;
+
+        expect(telemetry.getCommonProperties().device_id).toBe(actualHashedId);
+        expect(telemetry["isBufferingEvents"]).toBe(false);
+    });
+});

tests/unit/deferred-promise.test.ts

@@ -0,0 +1,61 @@
+import { DeferredPromise } from "../../src/deferred-promise.js";
+
+describe("DeferredPromise", () => {
+    it("should resolve with the correct value", async () => {
+        const deferred = new DeferredPromise<string>((resolve) => {
+            resolve("resolved value");
+        });
+
+        await expect(deferred).resolves.toEqual("resolved value");
+    });
+
+    it("should reject with the correct error", async () => {
+        const deferred = new DeferredPromise<string>((_, reject) => {
+            reject(new Error("rejected error"));
+        });
+
+        await expect(deferred).rejects.toThrow("rejected error");
+    });
+
+    it("should timeout if not resolved or rejected within the specified time", async () => {
+        const deferred = new DeferredPromise<string>(() => {
+            // Do not resolve or reject
+        }, 10);
+
+        await expect(deferred).rejects.toThrow("Promise timed out");
+    });
+
+    it("should clear the timeout when resolved", async () => {
+        jest.useFakeTimers();
+
+        const deferred = new DeferredPromise<string>((resolve) => {
+            setTimeout(() => resolve("resolved value"), 100);
+        }, 200);
+
+        const promise = deferred.then((value) => {
+            expect(value).toBe("resolved value");
+        });
+
+        jest.advanceTimersByTime(100);
+        await promise;
+
+        jest.useRealTimers();
+    });
+
+    it("should clear the timeout when rejected", async () => {
+        jest.useFakeTimers();
+
+        const deferred = new DeferredPromise<string>((_, reject) => {
+            setTimeout(() => reject(new Error("rejected error")), 100);
+        }, 200);
+
+        const promise = deferred.catch((error) => {
+            expect(error).toEqual(new Error("rejected error"));
+        });
+
+        jest.advanceTimersByTime(100);
+        await promise;
+
+        jest.useRealTimers();
+    });
+});