From 63277e31769f3d6301fcef4cbfdeb09c5f8e57e5 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Fri, 30 Oct 2026 17:41:09 +0100 Subject: [PATCH 1/2] test(audit): 9 fewer tests in ios, leasing, dispatcher and http, since 19a11eb --- src/drivers/ios/index.test.ts | 316 +++++++------------- src/gateway/dispatcher.test.ts | 246 +++++----------- src/http/app.test.ts | 232 ++++++--------- src/leasing/create-leasing.test.ts | 450 +++++++++++++++-------------- 4 files changed, 491 insertions(+), 753 deletions(-) diff --git a/src/drivers/ios/index.test.ts b/src/drivers/ios/index.test.ts index efef757f5..73ffccb21 100644 --- a/src/drivers/ios/index.test.ts +++ b/src/drivers/ios/index.test.ts @@ -1407,7 +1407,11 @@ describe("IosSimctlDriver", () => { { address: driverData.udid, deviceId: driverData.udid, driverData }, { clean: "full" }, ), - ).rejects.toThrow(); + ).rejects.toThrow(`ENOENT: no such file or directory, open '${dataPath}'`); + expect(runner.calls.map((call) => call.args)).toEqual([ + simctlArgs("shutdown", driverData.udid), + simctlArgs("erase", driverData.udid), + ]); // why: A durable mark standing alone is exactly what `Doctor` reads as a foreign erase, so // a half-written pair would have `doctor` accusing the user of erasing the device @@ -1638,22 +1642,6 @@ describe("IosSimctlDriver", () => { ]); }); - it("answers virtualLeaseEnvironment with the device set, and leaseEnvironment for a virtual device equals it", async () => { - const driver = await createDriver(new ScriptedProcessRunner([])); - - expect(driver.virtualLeaseEnvironment()).toEqual({ SIMLOCK_IOS_DEVICE_SET: deviceRoot }); - expect(driver.leaseEnvironment(VIRTUAL_DEVICE)).toEqual(driver.virtualLeaseEnvironment()); - }); - - it("hands a lease holder of a physical device its UDID and no simulator device set", async () => { - const driver = await createDriver(new ScriptedProcessRunner([])); - - expect(driver.leaseEnvironment(VIRTUAL_DEVICE)).toEqual({ SIMLOCK_IOS_DEVICE_SET: deviceRoot }); - expect(driver.leaseEnvironment(PHYSICAL_DEVICE)).toEqual({ - SIMLOCK_DEVICE_UDID: "00008110-001A2B3C4D5E", - }); - }); - it("answers uninstall for the reclaim strategy of a physical device", async () => { const driver = await createDriver(new ScriptedProcessRunner([])); @@ -1879,7 +1867,13 @@ describe("IosSimctlDriver", () => { it("still proxies the runtime operations that only read", async () => { const driver = await createDriver(new ScriptedProcessRunner([])); - expect((await simctlPassthrough(driver, ["runtime", "list"])).args).toContain("list"); + expect((await simctlPassthrough(driver, ["runtime", "list"])).args).toEqual([ + "simctl", + "--set", + deviceRoot, + "runtime", + "list", + ]); }); it.skipIf(process.env.SIMLOCK_LIVE_IOS !== "1")( @@ -1920,6 +1914,7 @@ describe("IosSimctlDriver", () => { const searchSignature = "v1:7587bb9b67a41b64"; const slim18_5 = { ...driverData, runtimeId: "com.apple.CoreSimulator.SimRuntime.iOS-18-5" }; const slim18_4 = { ...driverData, runtimeId: "com.apple.CoreSimulator.SimRuntime.iOS-18-4" }; + const slim9_0 = { ...driverData, runtimeId: "com.apple.CoreSimulator.SimRuntime.iOS-9-0" }; const slim17_5 = { ...driverData, runtimeId: "com.apple.CoreSimulator.SimRuntime.iOS-17-5" }; const slim26_0 = { ...driverData, runtimeId: "com.apple.CoreSimulator.SimRuntime.iOS-26-0" }; const slimGarbageRuntime = { ...driverData, runtimeId: "not-a-runtime-id" }; @@ -1962,34 +1957,6 @@ describe("IosSimctlDriver", () => { }); }); - it("checks installer readiness without slimming when the slim option is omitted", async () => { - const runner = new ScriptedProcessRunner([ - { match: { command: "xcrun", args: simctlArgs("boot", driverData.udid) } }, - { match: { command: "xcrun", args: simctlArgs("bootstatus", driverData.udid, "-b") } }, - { match: simctl("listapps", driverData.udid) }, - ]); - - // why: `driverData` has no `slimSignature`/`slimMarkToken`, exactly what a registry written - // before slim mode shipped holds, so this also proves such data still loads. - await expect( - (await createDriver(runner)).makeReady( - { address: driverData.udid, deviceId: driverData.udid, driverData }, - prepareFull, - ), - ).resolves.toEqual({ - address: driverData.udid, - deviceId: driverData.udid, - driverData, - // why: Slim mode is off, so this boot slimmed nothing: the device is full. - mode: "full", - }); - expect(runner.calls.map((call) => call.args)).toEqual([ - simctlArgs("boot", driverData.udid), - simctlArgs("bootstatus", driverData.udid, "-b"), - simctlArgs("listapps", driverData.udid), - ]); - }); - it.each([false, true])( "boots a slim-spec device on 18.5, applies the disable list, and reboots (slim by default: %s)", async (slimByDefault) => { @@ -2066,138 +2033,94 @@ describe("IosSimctlDriver", () => { }, ); - it("downgrades a failed post-apply shutdown to a skip instead of failing the lease, but always reboots first (finding #3 / second-review finding #2, issue #87)", async () => { - const filesystem = new MemoryFilesystem(); - await plantManagedDevice(filesystem); - const runner = new ScriptedProcessRunner([ - { match: { command: "xcrun", args: simctlArgs("boot", slim18_5.udid) } }, - { match: { command: "xcrun", args: simctlArgs("bootstatus", slim18_5.udid, "-b") } }, - { match: simctl("listapps", slim18_5.udid) }, - { - match: { - args: simctlArgs("spawn", slim18_5.udid, "/bin/sh", "-c", slimScript(widgetsLabels)), - command: "xcrun", + it.each([ + [ + "exits non-zero", + { result: { code: 1, stderr: "unexpected shutdown failure", stdout: "" } }, + false, + ], + // why: A `simctl shutdown` *timeout* (as opposed to a clean non-zero exit) is the case + // second-review finding #2 targets: the shutdown may have completed, may still be in + // flight, or may never have taken effect. Rather than guess from the exception, the driver + // always re-establishes a known-good running device afterward and reports the apply as + // uncertain. + ["times out", { hangs: true }, true], + ])( + "downgrades a post-apply shutdown that %s to an apply-failed skip instead of failing the lease, but always reboots first (finding #3 / second-review finding #2, issue #87)", + async (_failure, shutdownOutcome, hangs) => { + const filesystem = new MemoryFilesystem(); + await plantManagedDevice(filesystem); + const clock = new FakeClock(); + const runner = new ScriptedProcessRunner([ + { match: { command: "xcrun", args: simctlArgs("boot", slim18_5.udid) } }, + { match: { command: "xcrun", args: simctlArgs("bootstatus", slim18_5.udid, "-b") } }, + { match: simctl("listapps", slim18_5.udid) }, + { + match: { + args: simctlArgs("spawn", slim18_5.udid, "/bin/sh", "-c", slimScript(widgetsLabels)), + command: "xcrun", + }, }, - }, - { - match: { command: "xcrun", args: simctlArgs("shutdown", slim18_5.udid) }, - result: { code: 1, stderr: "unexpected shutdown failure", stdout: "" }, - }, - // why: Always run after the shutdown call, whether or not it succeeded -- `boot` tolerates an - // already-booted device either way. - { match: { command: "xcrun", args: simctlArgs("boot", slim18_5.udid) } }, - { match: { command: "xcrun", args: simctlArgs("bootstatus", slim18_5.udid, "-b") } }, - { match: simctl("listapps", slim18_5.udid) }, - ]); - const onSlimmed = vi.fn(); - const onSlimSkipped = vi.fn(); - const driver = await createSlimDriver( - runner, - filesystem, - slimOptions(), - onSlimmed, - onSlimSkipped, - ); - - const result = await driver.makeReady( - { address: slim18_5.udid, deviceId: slim18_5.udid, driverData: slim18_5 }, - prepareSlim, - ); - - expect(result).toEqual({ - address: slim18_5.udid, - deviceId: slim18_5.udid, - driverData: slim18_5, - mode: "slim", - }); - expect(runner.calls.map((call) => call.args)).toEqual([ - simctlArgs("boot", slim18_5.udid), - simctlArgs("bootstatus", slim18_5.udid, "-b"), - simctlArgs("listapps", slim18_5.udid), - simctlArgs("spawn", slim18_5.udid, "/bin/sh", "-c", slimScript(widgetsLabels)), - simctlArgs("shutdown", slim18_5.udid), - simctlArgs("boot", slim18_5.udid), - simctlArgs("bootstatus", slim18_5.udid, "-b"), - simctlArgs("listapps", slim18_5.udid), - ]); - expect(onSlimmed).not.toHaveBeenCalled(); - expect(onSlimSkipped).toHaveBeenCalledWith( - expect.objectContaining({ deviceId: slim18_5.udid, reason: "apply-failed" }), - ); - }); - - it("recovers from a shutdown that timed out and reports it as an uncertain, skipped apply (second-review finding #2, issue #87)", async () => { - // why: A `simctl shutdown` *timeout* (as opposed to a clean non-zero exit) is exactly the case - // the fix targets: the shutdown may have completed, may still be in flight, or may never - // have taken effect. Rather than guess from the exception, the driver always re-establishes - // a known-good running device afterward and reports the apply as uncertain. - const filesystem = new MemoryFilesystem(); - await plantManagedDevice(filesystem); - const clock = new FakeClock(); - const runner = new ScriptedProcessRunner([ - { match: { command: "xcrun", args: simctlArgs("boot", slim18_5.udid) } }, - { match: { command: "xcrun", args: simctlArgs("bootstatus", slim18_5.udid, "-b") } }, - { match: simctl("listapps", slim18_5.udid) }, - { - match: { - args: simctlArgs("spawn", slim18_5.udid, "/bin/sh", "-c", slimScript(widgetsLabels)), - command: "xcrun", + { + match: { command: "xcrun", args: simctlArgs("shutdown", slim18_5.udid) }, + ...shutdownOutcome, }, - }, - { - hangs: true, - match: { command: "xcrun", args: simctlArgs("shutdown", slim18_5.udid) }, - }, - { match: { command: "xcrun", args: simctlArgs("boot", slim18_5.udid) } }, - { match: { command: "xcrun", args: simctlArgs("bootstatus", slim18_5.udid, "-b") } }, - { match: simctl("listapps", slim18_5.udid) }, - ]); - const onSlimmed = vi.fn(); - const onSlimSkipped = vi.fn(); - const driver = await IosSimctlDriver.create({ - driverConfig: { deviceRoot }, - instanceId, - simlockHome: "/home/.simlock", - clock, - filesystem, - idGenerator: { generate: () => "device-1" }, - onSlimSkipped, - onSlimmed, - processRunner: runner, - slim: slimOptions(), - }); + // why: Always run after the shutdown call, whether or not it succeeded -- `boot` tolerates an + // already-booted device either way. + { match: { command: "xcrun", args: simctlArgs("boot", slim18_5.udid) } }, + { match: { command: "xcrun", args: simctlArgs("bootstatus", slim18_5.udid, "-b") } }, + { match: simctl("listapps", slim18_5.udid) }, + ]); + const onSlimmed = vi.fn(); + const onSlimSkipped = vi.fn(); + const driver = await IosSimctlDriver.create({ + driverConfig: { deviceRoot }, + instanceId, + simlockHome: "/home/.simlock", + clock, + filesystem, + idGenerator: { generate: () => "device-1" }, + onSlimSkipped, + onSlimmed, + processRunner: runner, + slim: slimOptions(), + }); - const ready = driver.makeReady( - { address: slim18_5.udid, deviceId: slim18_5.udid, driverData: slim18_5 }, - prepareSlim, - ); + const ready = driver.makeReady( + { address: slim18_5.udid, deviceId: slim18_5.udid, driverData: slim18_5 }, + prepareSlim, + ); - await waitForCalls(runner, 5); - clock.advance(30_000); // COMMAND_TIMEOUT_MS, the `#shutdown` call's own timeout + if (hangs) { + await waitForCalls(runner, 5); + // why: 30 s is COMMAND_TIMEOUT_MS, the `#shutdown` call's own timeout. + clock.advance(30_000); + } - const result = await ready; + const result = await ready; - expect(result).toEqual({ - address: slim18_5.udid, - deviceId: slim18_5.udid, - driverData: slim18_5, - mode: "slim", - }); - expect(runner.calls.map((call) => call.args)).toEqual([ - simctlArgs("boot", slim18_5.udid), - simctlArgs("bootstatus", slim18_5.udid, "-b"), - simctlArgs("listapps", slim18_5.udid), - simctlArgs("spawn", slim18_5.udid, "/bin/sh", "-c", slimScript(widgetsLabels)), - simctlArgs("shutdown", slim18_5.udid), - simctlArgs("boot", slim18_5.udid), - simctlArgs("bootstatus", slim18_5.udid, "-b"), - simctlArgs("listapps", slim18_5.udid), - ]); - expect(onSlimmed).not.toHaveBeenCalled(); - expect(onSlimSkipped).toHaveBeenCalledWith( - expect.objectContaining({ deviceId: slim18_5.udid, reason: "apply-failed" }), - ); - }); + expect(result).toEqual({ + address: slim18_5.udid, + deviceId: slim18_5.udid, + driverData: slim18_5, + mode: "slim", + }); + expect(runner.calls.map((call) => call.args)).toEqual([ + simctlArgs("boot", slim18_5.udid), + simctlArgs("bootstatus", slim18_5.udid, "-b"), + simctlArgs("listapps", slim18_5.udid), + simctlArgs("spawn", slim18_5.udid, "/bin/sh", "-c", slimScript(widgetsLabels)), + simctlArgs("shutdown", slim18_5.udid), + simctlArgs("boot", slim18_5.udid), + simctlArgs("bootstatus", slim18_5.udid, "-b"), + simctlArgs("listapps", slim18_5.udid), + ]); + expect(onSlimmed).not.toHaveBeenCalled(); + expect(onSlimSkipped).toHaveBeenCalledWith( + expect.objectContaining({ deviceId: slim18_5.udid, reason: "apply-failed" }), + ); + }, + ); it("propagates BootTimeoutError when the shutdown succeeds but the post-slim reboot times out (second-review finding #2, issue #87)", async () => { // why: Unlike the shutdown-failure cases above, the device here genuinely is shut down and the @@ -2423,11 +2346,14 @@ describe("IosSimctlDriver", () => { }); it.each([ - ["18.4", slim18_4], - ["17.5", slim17_5], + // why: 9.0 is the lexicographic trap: a string compare would put "9.0" ahead of "18.5". + ["runtime-too-old", "iOS 9.0", slim9_0], + ["runtime-too-old", "iOS 17.5", slim17_5], + ["runtime-too-old", "iOS 18.4", slim18_4], + ["unknown-runtime", "an unparseable runtimeId", slimGarbageRuntime], ])( - "skips slim with reason runtime-too-old on iOS %s (single boot, no spawns)", - async (_version, data) => { + "skips slim with reason %s on %s (single boot, no spawns)", + async (reason, _runtime, data) => { const runner = new ScriptedProcessRunner([ { match: { command: "xcrun", args: simctlArgs("boot", data.udid) } }, { match: { command: "xcrun", args: simctlArgs("bootstatus", data.udid, "-b") } }, @@ -2455,46 +2381,12 @@ describe("IosSimctlDriver", () => { }); expect(runner.calls).toHaveLength(3); expect(onSlimSkipped).toHaveBeenCalledWith( - expect.objectContaining({ deviceId: data.udid, reason: "runtime-too-old" }), + expect.objectContaining({ deviceId: data.udid, reason }), ); expect(runner.calls[1]?.options).toEqual({ timeoutMs: 120_000 }); }, ); - it("skips slim with reason unknown-runtime on an unparseable runtimeId", async () => { - const runner = new ScriptedProcessRunner([ - { match: { command: "xcrun", args: simctlArgs("boot", slimGarbageRuntime.udid) } }, - { - match: { - command: "xcrun", - args: simctlArgs("bootstatus", slimGarbageRuntime.udid, "-b"), - }, - }, - { match: simctl("listapps", slimGarbageRuntime.udid) }, - ]); - const onSlimSkipped = vi.fn(); - const driver = await createSlimDriver( - runner, - new MemoryFilesystem(), - slimOptions(), - undefined, - onSlimSkipped, - ); - - await driver.makeReady( - { - address: slimGarbageRuntime.udid, - deviceId: slimGarbageRuntime.udid, - driverData: slimGarbageRuntime, - }, - prepareSlim, - ); - - expect(onSlimSkipped).toHaveBeenCalledWith( - expect.objectContaining({ reason: "unknown-runtime" }), - ); - }); - it("slims iOS 26.0 (version comparison must not be string-lexicographic)", async () => { const filesystem = new MemoryFilesystem(); await plantManagedDevice(filesystem); @@ -4546,7 +4438,7 @@ describe("IosSimctlDriver toolVersions()", () => { // why: No scripted invocation: the runner refuses to start it, as a spawn failure would. const driver = await createDriver(new ScriptedProcessRunner([])); - await expect(driver.toolVersions()).rejects.toThrow(); + await expect(driver.toolVersions()).rejects.toThrow(/xcodebuild/); }); }); diff --git a/src/gateway/dispatcher.test.ts b/src/gateway/dispatcher.test.ts index 98235ce4f..10a7a4e3c 100644 --- a/src/gateway/dispatcher.test.ts +++ b/src/gateway/dispatcher.test.ts @@ -1,4 +1,3 @@ -import { Worker } from "node:worker_threads"; import { afterEach, describe, expect, it, vi } from "vitest"; import { createHmac } from "node:crypto"; @@ -19,7 +18,7 @@ import { usageRig, } from "../history/testing.js"; import { OPERATIONS, SimlockError, type OperationName } from "../contract/index.js"; -import { DispatchError, runDispatch, type DispatchSession } from "../daemon/dispatch.js"; +import { DispatchError, type DispatchSession } from "../daemon/dispatch.js"; import { FakeClock, FetchWebhookPoster, @@ -336,38 +335,6 @@ describe("GatewayDispatcher", () => { ).resolves.toEqual({ name: "ci-runner", principal: "tok_9f2c", role: "agent", via: "token" }); }); - it("The gateway dispatcher logs the same line for the same operation", async () => { - const { clock, dispatcher, logSink, workers } = harness(); - workers.connected("wrk_1", "mac-mini-1", "0.3.0"); - const otherSink = new MemoryLogSink(); - const other = (role: "admin" | "agent") => - runDispatch("worker.drain", { workerId: "wrk_1" }, session({ role }), { - handlers: { "worker.drain": () => ({ drained: true, workerId: "wrk_1" }) }, - observe: { - clock, - codeOf: (error) => (error instanceof DispatchError ? error.code : "INTERNAL"), - logger: new JsonLinesLogger({ clock, module: "other.dispatch", sink: otherSink }), - }, - }); - - await dispatcher.dispatch("worker.drain", { workerId: "wrk_1" }, session()); - await other("admin"); - await expect( - dispatcher.dispatch("worker.drain", { workerId: "wrk_1" }, session({ role: "agent" })), - ).rejects.toMatchObject({ code: "FORBIDDEN" }); - await expect(other("agent")).rejects.toMatchObject({ code: "FORBIDDEN" }); - - const gatewayLines = logSink.records.filter((record) => record.message === "operation"); - expect(gatewayLines.map((record) => record.module)).toEqual([ - "gateway.dispatch", - "gateway.dispatch", - ]); - expect(gatewayLines.map(({ module: _module, ...rest }) => rest)).toEqual( - otherSink.records.map(({ module: _module, ...rest }) => rest), - ); - expect(gatewayLines[1]?.fields).toMatchObject({ code: "FORBIDDEN" }); - }); - it("answers status.get from the fleet", async () => { const { dispatcher, workers } = harness(); workers.connected("wrk_1", "mac-mini-1", "0.3.0"); @@ -608,9 +575,7 @@ describe("GatewayDispatcher", () => { it("answers config.get with the gateway's own config", async () => { const { dispatcher } = harness(); - await expect(dispatcher.dispatch("config.get", {}, session())).resolves.toMatchObject({ - mode: "gateway", - }); + await expect(dispatcher.dispatch("config.get", {}, session())).resolves.toEqual(gatewayConfig); }); it("lists workers, and drains, undrains and removes one", async () => { @@ -799,11 +764,17 @@ describe("GatewayDispatcher", () => { ).rejects.toMatchObject({ code: "UNKNOWN_WORKER", details: { workerId: "wrk_ghost" } }); }); - it("keeps worker.* admin-only", async () => { - const { dispatcher } = harness(); + it.each([ + ["worker.list", {}], + ["worker.drain", { workerId: "wrk_1" }], + ["worker.undrain", { workerId: "wrk_1" }], + ["worker.remove", { workerId: "wrk_1" }], + ] as const)("keeps %s admin-only", async (operation, input) => { + const { dispatcher, workers } = harness(); + workers.connected("wrk_1", "mac-mini-1", "0.3.0"); await expect( - dispatcher.dispatch("worker.list", {}, session({ role: "agent" })), + dispatcher.dispatch(operation, input, session({ role: "agent" })), ).rejects.toMatchObject({ code: "FORBIDDEN" }); }); @@ -1283,17 +1254,13 @@ describe("GatewayDispatcher", () => { }, ); - it("mints and revokes its own tokens, worker join tokens included", async () => { + it("mints its own tokens, worker join tokens included", async () => { const { dispatcher, tokens } = harness(); await expect( dispatcher.dispatch("token.create", { role: "worker", label: "mac-mini-1" }, session()), ).resolves.toMatchObject({ secret: "slk_secret", token: { role: "worker" } }); expect(tokens.created).toEqual(["worker"]); - - await expect(dispatcher.dispatch("token.revoke", { id: "tok_1" }, session())).resolves.toEqual({ - revoked: true, - }); }); it("closes the uplink a revoked token authorized, not merely the store entry (C-2)", async () => { @@ -1619,47 +1586,52 @@ describe("GatewayDispatcher", () => { }); }); - it("component.install answers UNSUPPORTED_IN_GATEWAY_MODE, telling the operator to name workers (ADR 0010 §7)", async () => { - const { dispatcher } = harness(); - - await expect( - dispatcher.dispatch("component.install", { platform: "android", version: "35" }, session()), - ).rejects.toMatchObject({ - code: "UNSUPPORTED_IN_GATEWAY_MODE", - details: { operation: "component.install" }, - message: - "component.install installs on one machine; on a gateway, name the workers to install on " + + it.each([ + [ + "component.install", + { platform: "android", version: "35" }, + "admin", + "component.install installs on one machine; on a gateway, name the workers to install on " + "(simlock component install --worker or --all-workers)", - }); - }); - - it("component.list answers UNSUPPORTED_IN_GATEWAY_MODE to an agent session, saying it lists one machine's components (ADR 0010 §8)", async () => { - const { dispatcher } = harness(); - - await expect( - dispatcher.dispatch("component.list", {}, session({ role: "agent" })), - ).rejects.toMatchObject({ - code: "UNSUPPORTED_IN_GATEWAY_MODE", - details: { operation: "component.list" }, - message: "component.list lists one machine's components; run it against a worker", - }); - }); - - it("component.remove answers UNSUPPORTED_IN_GATEWAY_MODE to an admin session, saying it removes from one machine (ADR 0010 §8)", async () => { - const { dispatcher } = harness(); + ], + [ + "component.list", + {}, + "agent", + "component.list lists one machine's components; run it against a worker", + ], + [ + "component.remove", + { platform: "ios", version: "26.4" }, + "admin", + "component.remove removes a component from one machine; run it against a worker", + ], + [ + "device.add", + { id: "00008110-001A2B3C4D5E", platform: "ios" }, + "admin", + "device.add enrols a device plugged into one machine; run it on the worker the device is plugged into", + ], + [ + "device.remove", + { id: "00008110-001A2B3C4D5E" }, + "admin", + "device.remove unenrols a device plugged into one machine; run it on the worker the device is plugged into", + ], + ] as const)( + "%s answers UNSUPPORTED_IN_GATEWAY_MODE with a message pointing at the worker (ADR 0010 §7, §8)", + async (operation, input, role, message) => { + const { dispatcher } = harness(); - await expect( - dispatcher.dispatch( - "component.remove", - { platform: "ios", version: "26.4" }, - session({ role: "admin" }), - ), - ).rejects.toMatchObject({ - code: "UNSUPPORTED_IN_GATEWAY_MODE", - details: { operation: "component.remove" }, - message: "component.remove removes a component from one machine; run it against a worker", - }); - }); + await expect( + dispatcher.dispatch(operation, input, session({ role })), + ).rejects.toMatchObject({ + code: "UNSUPPORTED_IN_GATEWAY_MODE", + details: { operation }, + message, + }); + }, + ); }); describe("daemon.update", () => { @@ -1700,37 +1672,6 @@ describe("GatewayDispatcher", () => { }); }); - describe("device.add and device.remove", () => { - it("answer UNSUPPORTED_IN_GATEWAY_MODE to an admin session, saying to run the command on the worker the device is plugged into", async () => { - const { dispatcher } = harness(); - - await expect( - dispatcher.dispatch( - "device.add", - { id: "00008110-001A2B3C4D5E", platform: "ios" }, - session({ role: "admin" }), - ), - ).rejects.toMatchObject({ - code: "UNSUPPORTED_IN_GATEWAY_MODE", - details: { operation: "device.add" }, - message: - "device.add enrols a device plugged into one machine; run it on the worker the device is plugged into", - }); - await expect( - dispatcher.dispatch( - "device.remove", - { id: "00008110-001A2B3C4D5E" }, - session({ role: "admin" }), - ), - ).rejects.toMatchObject({ - code: "UNSUPPORTED_IN_GATEWAY_MODE", - details: { operation: "device.remove" }, - message: - "device.remove unenrols a device plugged into one machine; run it on the worker the device is plugged into", - }); - }); - }); - describe("lease lifecycle and device.exec (#118)", () => { it("forwards lease.request through the fleet coordinator and answers UNKNOWN_LEASE for renew/release/exec against an id it never issued", async () => { const { dispatcher } = harness(); @@ -1866,6 +1807,10 @@ describe("GatewayDispatcher", () => { ["neither a model nor a class", { platform: "ios" }], ["a class and an OS range", { class: "phone", osVersion: ">=18", platform: "ios" }], ["a model and an OS range", { model: "iPhone 17", osVersion: ">=18", platform: "ios" }], + [ + "a model and an exact osVersion", + { model: "iPhone 17", osVersion: "26.0", platform: "ios" }, + ], ] as const)( "serves a lease.request naming %s through a worker that fits, forwarding it as it arrived", async (_label, input) => { @@ -1900,30 +1845,6 @@ describe("GatewayDispatcher", () => { }, ); - it("forwards a lease.request's exact osVersion to the worker unchanged", async () => { - const { directory, dispatcher, workers } = harness(); - const client = new ScriptedWorkerClient(); - directory.add("wrk_1", client); - workers.connected("wrk_1", undefined, "0.3.0"); - workers.refresh("wrk_1", { - capacity: statusFixture().capacity, - health: "running", - queueDepth: 0, - catalog: catalogFixture([{ models: ["iPhone 17"], platform: "ios", runtimes: ["26.0"] }]) - .platforms, - downloads: { policy: "on-request" }, - }); - client.requestLeaseQueue.push({ grant: grantFixture(), kind: "grant" }); - - await dispatcher.dispatch( - "lease.request", - { model: "iPhone 17", noWait: true, osVersion: "26.0", platform: "ios" }, - session({ role: "agent" }), - ); - - expect(client.lastRequestLeaseInput).toMatchObject({ osVersion: "26.0" }); - }); - it("forwards a lease.request's image tag to the worker unchanged", async () => { const { directory, dispatcher, workers } = harness(); const client = new ScriptedWorkerClient(); @@ -2022,7 +1943,7 @@ describe("GatewayDispatcher", () => { ).rejects.toMatchObject({ code: "FORBIDDEN" }); }); - it("calls onStarted for a silent device.exec from the worker's own started push, with no output and no answer (ADR §19a/§19b/§19e)", async () => { + it("calls onStarted for a silent device.exec from the worker's own started push (ADR §19a/§19b/§19e)", async () => { const { coordinator, directory, dispatcher, workers } = harness(); const client = new ScriptedWorkerClient(); directory.add("wrk_1", client); @@ -2058,9 +1979,7 @@ describe("GatewayDispatcher", () => { role: "agent", }), ); - await Promise.resolve(); - - expect(started).toBe(true); + await expect.poll(() => started).toBe(true); }); it.each([ @@ -2215,7 +2134,7 @@ describe("GatewayDispatcher: waiting requests", () => { dispatcher.dispatch("list.get", { kind: "requests" }, session()); it("list.get requests and status.get waiting on a gateway return the fleet queue with each worker's reason in workers, and each worker's waiting requests with their workerId and reason, a workers field a worker sends on its own entry dropped", async () => { - const { coordinator, dispatcher, workers } = harness(); + const { dispatcher, workers } = harness(); const capacity = statusFixture().capacity; for (const [workerId, requestId] of [ ["wrk_1", "req_w1"], @@ -2245,7 +2164,6 @@ describe("GatewayDispatcher: waiting requests", () => { session({ principal: "fleet-owner", role: "agent" }), ); await settle(); - expect(coordinator.queueDepth).toBe(1); const listed = await listRequests(dispatcher); @@ -2451,17 +2369,14 @@ describe("GatewayDispatcher: usage.get", () => { totals: { requests: 1 }, }); gate.open = false; - const posted = vi.spyOn(Worker.prototype, "postMessage"); await expect(dispatcher.dispatch("usage.get", window, session())).rejects.toMatchObject({ code: "HISTORY_UNAVAILABLE", message: "The history could not be opened, so no figures can be read; see daemon.log.", }); - expect(posted).not.toHaveBeenCalled(); - posted.mockRestore(); }); - it("the gateway handler strips its own gw: prefix before the label lookup and leaves another prefix alone, and labels nothing when it has no token store", async () => { + it("the gateway handler strips its own gw: prefix before the label lookup and leaves another prefix alone, labels nothing when it has no token store, and with includeLedger lists the request rows as entries labelled the same way", async () => { const { eventHistory, rig } = await fleetHistory(); for (const [index, requester] of [ `${GATEWAY_REQUESTER_PREFIX}tok_a`, @@ -2483,6 +2398,11 @@ describe("GatewayDispatcher: usage.get", () => { const answer = await labelled.dispatcher.dispatch("usage.get", window, session()); const bare = await unlabelled.dispatcher.dispatch("usage.get", window, session()); + const ledger = await labelled.dispatcher.dispatch( + "usage.get", + { ...window, includeLedger: true }, + session(), + ); expect( Object.fromEntries(answer.requesters.map((requester) => [requester.id, requester.label])), @@ -2493,34 +2413,10 @@ describe("GatewayDispatcher: usage.get", () => { }); expect(bare.requesters).toHaveLength(3); expect(bare.requesters.every((requester) => !("label" in requester))).toBe(true); - }); - - it("the gateway handler with includeLedger lists the fleet's request rows as entries, labelled after stripping its own gw: prefix", async () => { - const { eventHistory, rig } = await fleetHistory(); - for (const [index, requester] of [ - `${GATEWAY_REQUESTER_PREFIX}tok_a`, - "gw:other-instance:tok_a", - "tok_a", - ].entries()) { - rig.emit("lease.requested", requested(`R${index}`, { requester })); - } - const { dispatcher } = harness({ - eventHistory, - historyPath: rig.historyPath, - tokenRecords: [{ id: "tok_a", label: "ci-bot" }], - }); - - const answer = await dispatcher.dispatch( - "usage.get", - { ...window, includeLedger: true }, - session(), - ); - const plain = await dispatcher.dispatch("usage.get", window, session()); - expect( - Object.fromEntries((answer.requests ?? []).map((entry) => [entry.requestId, entry.label])), + Object.fromEntries((ledger.requests ?? []).map((entry) => [entry.requestId, entry.label])), ).toEqual({ R0: "ci-bot", R1: undefined, R2: "ci-bot" }); - expect(plain).not.toHaveProperty("requests"); + expect(answer).not.toHaveProperty("requests"); }); it("the gateway handler lists every registry worker with its label and a worker id the rows name that the registry lacks, and counts a fleet request with no worker in totals and its platform only", async () => { diff --git a/src/http/app.test.ts b/src/http/app.test.ts index 71a936005..e4eb8858e 100644 --- a/src/http/app.test.ts +++ b/src/http/app.test.ts @@ -259,29 +259,6 @@ describe("GET /v1/identity", () => { }); describe("authentication and role/ownership (via the shared dispatcher)", () => { - it("401s a protected route with no token", async () => { - const { app } = buildHarness(); - const response = await app.request("/v1/status"); - expect(response.status).toBe(401); - }); - - it("403s an agent token whose dispatched operation is admin-only -- ADR: role gating moved to the shared dispatcher, not an HTTP-only minRole check", async () => { - const { app, dispatcher } = buildHarness(); - dispatcher.handlers["list.get"] = () => { - throw new DispatchError("FORBIDDEN", "Operation list.get requires role admin"); - }; - const response = await app.request("/v1/devices", { headers: agentAuth }); - expect(response.status).toBe(403); - expect(((await response.json()) as { error: { code: string } }).error.code).toBe("FORBIDDEN"); - }); - - it("admits an operator token on the same admin-only route", async () => { - const { app, dispatcher } = buildHarness(); - dispatcher.handlers["list.get"] = () => []; - const response = await app.request("/v1/devices", { headers: operatorAuth }); - expect(response.status).toBe(200); - }); - it("403s an agent reaching another requester's lease request (HTTP-only tracker resource guard)", async () => { const { app, dispatcher } = buildHarness(); const { id } = await createLeaseRequest(app, dispatcher); @@ -375,21 +352,12 @@ describe("GET /v1/status, /v1/catalog", () => { ], }); }); - - it("400s an invalid platform query, before ever dispatching", async () => { - const { app, dispatcher } = buildHarness(); - dispatcher.handlers["catalog.get"] = () => { - throw new Error("should not be called"); - }; - const response = await app.request("/v1/catalog?platform=windows", { headers: agentAuth }); - expect(response.status).toBe(400); - expect(((await response.json()) as { error: { code: string } }).error.code).toBe("BAD_REQUEST"); - }); }); describe("a query to a System route", () => { it.each([ ["/v1/catalog?platform=windows", 400, "platform must be ios or android", undefined], + ["/v1/components?platform=windows", 400, "platform must be ios or android", undefined], ["/v1/catalog?platform=ios&platform=android", 200, undefined, { platform: "ios" }], ["/v1/stats?ledger=2", 400, "ledger must be 1", undefined], [ @@ -409,6 +377,10 @@ describe("a query to a System route", () => { seen.push(input); return {}; }; + dispatcher.handlers["component.list"] = (input) => { + seen.push(input); + return { components: [] }; + }; const response = await app.request(url, { headers: operatorAuth }); @@ -462,18 +434,6 @@ describe("GET /v1/components", () => { expect(response.status).toBe(200); }); - - it("400s an invalid platform query, before ever dispatching", async () => { - const { app, dispatcher } = buildHarness(); - dispatcher.handlers["component.list"] = () => { - throw new Error("should not be called"); - }; - - const response = await app.request("/v1/components?platform=windows", { headers: agentAuth }); - - expect(response.status).toBe(400); - expect(((await response.json()) as { error: { code: string } }).error.code).toBe("BAD_REQUEST"); - }); }); describe("POST /v1/devices and DELETE /v1/devices/{id}", () => { @@ -727,35 +687,37 @@ describe("POST /v1/lease-requests", () => { expect(dispatcher.calls).toHaveLength(0); }); - it("dispatches a body naming a device as the model, with no class", async () => { - const { app, dispatcher } = buildHarness(); - const responsePromise = postLeaseRequest(app, defaultBody); - const call = await waitForDispatch(dispatcher, "lease.request"); - call.session.onProgress?.({ queuePosition: 1, stage: "queued" }); - await responsePromise; - - expect(call.input).toMatchObject({ model: "iPhone 17 Pro", platform: "ios" }); - expect(call.input).not.toHaveProperty("class"); - }); - it.each([ - ["a class", { class: "tablet", platform: "ios" }, { class: "tablet", platform: "ios" }], - ["only a platform", { platform: "android" }, { platform: "android" }], - ])("dispatches a body naming %s with no model", async (_label, body, expected) => { - const { app, dispatcher } = buildHarness(); - const responsePromise = postLeaseRequest(app, body); - const call = await waitForDispatch(dispatcher, "lease.request"); - call.session.onProgress?.({ queuePosition: 1, stage: "queued" }); - await responsePromise; + ["a device as the model", defaultBody, { model: "iPhone 17 Pro", platform: "ios" }, "class"], + [ + "a class", + { class: "tablet", platform: "ios" }, + { class: "tablet", platform: "ios" }, + "model", + ], + ["only a platform", { platform: "android" }, { platform: "android" }, "model"], + ])( + "dispatches a body naming %s with only the keys it names", + async (_label, body, expected, absentKey) => { + const { app, dispatcher } = buildHarness(); + const responsePromise = postLeaseRequest(app, body); + const call = await waitForDispatch(dispatcher, "lease.request"); + call.session.onProgress?.({ queuePosition: 1, stage: "queued" }); + await responsePromise; - expect(call.input).toMatchObject(expected); - expect(call.input).not.toHaveProperty("model"); - }); + expect(call.input).toMatchObject(expected); + expect(call.input).not.toHaveProperty(absentKey); + }, + ); it.each([ ["full", { full: true }], ["a mode other than slim or full", { mode: "fast" }], ["a key the route does not know", { colour: "blue" }], + [ + "a fleetRequestId, a contract field the route does not take", + { fleetRequestId: "req_1", noWait: true }, + ], ["an image tag with a space", { imageTag: "google apis" }], ["an image tag over 64 characters", { imageTag: "a".repeat(65) }], ])("400s a body carrying %s as BAD_REQUEST before ever dispatching", async (_label, extra) => { @@ -997,21 +959,6 @@ describe("POST /v1/lease-requests leaseId", () => { expect(dispatcher.calls).toEqual([]); }); - it("answers 400 BAD_REQUEST for a body carrying fleetRequestId, as for any unknown field, without dispatching", async () => { - const { app, dispatcher } = buildHarness(); - dispatcher.handlers["lease.request"] = () => makeGrant({ lease: { id: "lse_1" } }); - - const response = await postLeaseRequest(app, { - ...defaultBody, - fleetRequestId: "req_1", - noWait: true, - }); - - expect(response.status).toBe(400); - expect(((await response.json()) as { error: { code: string } }).error.code).toBe("BAD_REQUEST"); - expect(dispatcher.calls).toEqual([]); - }); - it("answers 409 LEASE_ID_TAKEN with the ID in the body when the dispatcher refuses it", async () => { const { app, dispatcher } = buildHarness(); dispatcher.handlers["lease.request"] = () => { @@ -1075,9 +1022,19 @@ describe("full lease-request lifecycle via GET / long-poll / SSE", () => { const { app, clock, dispatcher } = buildHarness(); const { id } = await createLeaseRequest(app, dispatcher); - const waitPromise = app.request(`/v1/lease-requests/${id}?wait=5`, { headers: agentAuth }); + const waitPromise = Promise.resolve( + app.request(`/v1/lease-requests/${id}?wait=5`, { headers: agentAuth }), + ); + let answered = false; + void waitPromise.then(() => { + answered = true; + }); await settle(); - clock.advance(5_000); + expect(answered).toBe(false); + clock.advance(4_999); + for (let turn = 0; turn < 50; turn += 1) await settle(); + expect(answered).toBe(false); + clock.advance(1); const response = await waitPromise; const body = (await response.json()) as { request: { state: string; queuePosition: number } }; expect(body.request).toMatchObject({ queuePosition: 1, state: "queued" }); @@ -1311,6 +1268,9 @@ describe("DELETE /v1/lease-requests/:id", () => { method: "DELETE", }); expect(response.status).toBe(404); + expect(((await response.json()) as { error: { code: string } }).error.code).toBe( + "UNKNOWN_LEASE_REQUEST", + ); }); it("204s when the request was still queued and cancellable, and a GET right after reads cancelled", async () => { @@ -1368,6 +1328,7 @@ describe("DELETE /v1/lease-requests/:id", () => { message: expect.any(String), }, }); + expect(dispatcher.calls.map((call) => call.operation)).not.toContain("lease.cancel"); }); }); @@ -1526,12 +1487,6 @@ describe("lease routes", () => { }).toMatchObject({ get: named, poll: named, post: named }); }); - it("404s an unknown lease id", async () => { - const { app } = buildHarness(); - const response = await app.request("/v1/leases/lse-missing", { headers: agentAuth }); - expect(response.status).toBe(404); - }); - it("POST /v1/leases/:id/renew drains buffered device-health notices", async () => { const { app, dispatcher, eventBus, registry } = buildHarness(); registry.devices = [makeDevice({ id: "dev_1" })]; @@ -1570,29 +1525,6 @@ describe("lease routes", () => { ]); }); - it("POST /v1/leases/:id/renew accepts an empty body", async () => { - const { app, dispatcher, registry } = buildHarness(); - registry.devices = [makeDevice({ id: "dev_1" })]; - registry.leases = [makeLease({ deviceId: "dev_1", id: "lse_1", ownerId: "tok_agent" })]; - dispatcher.handlers["lease.renew"] = (input) => { - expect(input).toEqual({ leaseId: "lse_1" }); - return { - deviceId: "dev_1", - grantedAt: 1_000, - id: "lse_1", - ownerId: "tok_agent", - requesterId: "tok_agent", - ttlDeadline: 2_000, - }; - }; - - const response = await app.request("/v1/leases/lse_1/renew", { - headers: agentAuth, - method: "POST", - }); - expect(response.status).toBe(200); - }); - it.each([ ["renew", "/v1/leases/lse_1/renew", '{"ttlMs":60000}', "lease.renew"], ["exec", "/v1/leases/lse_1/exec", '{"args":["list"],"tool":"simctl"}', "device.exec"], @@ -2046,6 +1978,7 @@ describe("lease routes", () => { await reader?.cancel(); await Promise.resolve(); + expect(call.session.onOutput).toBeDefined(); const chunk = "x".repeat(1_024); for (let index = 0; index < 1_000; index += 1) call.session.onOutput?.("stdout", chunk); @@ -2056,7 +1989,8 @@ describe("lease routes", () => { expect(call.session.onOutput?.("stdout", chunk)).toBeUndefined(); call.resolve({ exitCode: 0 }); - await expect(responsePromise).resolves.toBeDefined(); + await settle(); + await expect(reader?.read()).resolves.toMatchObject({ done: true }); }); }); @@ -2367,45 +2301,28 @@ describe("operator-only listing routes", () => { expect(body.devices.map((device) => device.id).sort()).toEqual(["dev_1", "dev_2"]); }); - it("GET /v1/events replays via events.replay, translating ?since to an absolute sinceTs", async () => { + it("GET /v1/events replays via events.replay, translating ?since in seconds or days to an absolute sinceTs", async () => { const { app, clock, dispatcher } = buildHarness(); + const seen: Array = []; dispatcher.handlers["events.replay"] = (input) => { const sinceTs = (input as { sinceTs?: number }).sinceTs; + seen.push(sinceTs); if (sinceTs === undefined) return [{ event: "daemon.started" }, { event: "daemon.stopping" }]; - expect(sinceTs).toBe(clock.now() - 5_000); return [{ event: "daemon.stopping" }]; }; const all = await app.request("/v1/events", { headers: operatorAuth }); expect(((await all.json()) as { events: unknown[] }).events).toHaveLength(2); - const recent = await app.request("/v1/events?since=5s", { headers: operatorAuth }); - const recentBody = (await recent.json()) as { events: Array<{ event: string }> }; - expect(recentBody.events).toEqual([{ event: "daemon.stopping" }]); - }); - - it("GET /v1/events accepts the d unit in ?since", async () => { - const { app, clock, dispatcher } = buildHarness(); - let seen: number | undefined; - dispatcher.handlers["events.replay"] = (input) => { - seen = (input as { sinceTs?: number }).sinceTs; - return []; - }; - - const response = await app.request("/v1/events?since=2d", { headers: operatorAuth }); - - expect(response.status).toBe(200); - expect(seen).toBe(clock.now() - 2 * 24 * 60 * 60 * 1000); - }); - - it("400s an invalid ?since duration, before dispatching", async () => { - const { app, dispatcher } = buildHarness(); - dispatcher.handlers["events.replay"] = () => { - throw new Error("should not be called"); - }; - const response = await app.request("/v1/events?since=nonsense", { headers: operatorAuth }); - expect(response.status).toBe(400); - expect(JSON.stringify(await response.json())).toContain("Invalid duration: nonsense"); + for (const [since, windowMs] of [ + ["5s", 5_000], + ["2d", 2 * 24 * 60 * 60 * 1000], + ] as const) { + const recent = await app.request(`/v1/events?since=${since}`, { headers: operatorAuth }); + const recentBody = (await recent.json()) as { events: Array<{ event: string }> }; + expect(recentBody.events).toEqual([{ event: "daemon.stopping" }]); + expect(seen.at(-1)).toBe(clock.now() - windowMs); + } }); it("GET /v1/events with before and limit dispatches events.replay with beforeId and limit, and answers HISTORY_NOT_KEPT with 422 and oldestTs in the body and HISTORY_UNAVAILABLE with 503", async () => { @@ -2557,24 +2474,41 @@ describe("operator-only listing routes", () => { }); describe("hardening from review", () => { - it("400s an oversized Idempotency-Key", async () => { - const { app } = buildHarness(); + it("400s an oversized Idempotency-Key as BAD_REQUEST, where a 200-character key is accepted", async () => { + const { app, dispatcher } = buildHarness(); + const accepted = await Promise.race([ + postLeaseRequest(app, defaultBody, { ...agentAuth, "Idempotency-Key": "x".repeat(200) }), + waitForDispatch(dispatcher, "lease.request").then(() => "dispatched" as const), + ]); + if (accepted instanceof Response) expect(accepted.status).toBe(201); + else expect(accepted).toBe("dispatched"); + const response = await postLeaseRequest(app, defaultBody, { ...agentAuth, "Idempotency-Key": "x".repeat(201), }); expect(response.status).toBe(400); + const { error } = (await response.json()) as { error: { code: string; message: string } }; + expect(error.code).toBe("BAD_REQUEST"); + expect(error.message).toContain("idempotencyKey"); }); it("clamps an oversized ?wait to the 60s ceiling", async () => { const { app, clock, dispatcher } = buildHarness(); const { id } = await createLeaseRequest(app, dispatcher); - const waitPromise = app.request(`/v1/lease-requests/${id}?wait=999999`, { - headers: agentAuth, + const waitPromise = Promise.resolve( + app.request(`/v1/lease-requests/${id}?wait=999999`, { headers: agentAuth }), + ); + let answered = false; + void waitPromise.then(() => { + answered = true; }); await settle(); - clock.advance(60_000); + clock.advance(59_999); + for (let turn = 0; turn < 50; turn += 1) await settle(); + expect(answered).toBe(false); + clock.advance(1); const response = await waitPromise; expect(((await response.json()) as { request: { state: string } }).request.state).toBe( "queued", @@ -3323,7 +3257,7 @@ describe("the stream mount's listed route", () => { }); describe("ETag on a JSON read under /v1", () => { - it("answers an unchanged JSON read with 304 and no body, and anything else with 200", async () => { + it("304 and no body for an unchanged read, 200 with a new ETag otherwise, no ETag on a 404 or a mount", async () => { const tokens: { id: string }[] = [{ id: "tok_1" }]; const { app, dispatcher, eventBus } = buildHarness({ mounts: [ diff --git a/src/leasing/create-leasing.test.ts b/src/leasing/create-leasing.test.ts index 33f39208e..bc65fa99a 100644 --- a/src/leasing/create-leasing.test.ts +++ b/src/leasing/create-leasing.test.ts @@ -478,7 +478,7 @@ describe("createLeasing", () => { }); }); - it("evicts the other platform's LRU warm device when only the global limit blocks demand", async () => { + it("evicts the other platform's warm device when only the global limit blocks demand", async () => { const clock = new FakeClock(1_000); const ios = new FakeDriver({ availableOsVersions: ["26.5"], clock, platform: "ios" }); const android = new FakeDriver({ availableOsVersions: ["36"], clock, platform: "android" }); @@ -1287,7 +1287,11 @@ describe("createLeasing", () => { }); await flush(); - harness.clock.advance(10); + harness.clock.advance(9); + await flush(); + expect(await settledOrPending(queued)).toBe("still pending"); + + harness.clock.advance(1); await flush(); await expect(queued).resolves.toMatchObject({ lease: { ownerId: "agent-2", requesterId: "agent-2" }, @@ -2143,29 +2147,31 @@ describe("createLeasing fresh lease identity (#75)", () => { }; } - it("deletes a fresh device on the next start when the daemon crashed while it was reclaiming", async () => { - const restarted = await restartAfterCrash("reclaiming"); - expect(restarted.registry.snapshot.devices).toMatchObject([{ state: "reclaiming" }]); - - await restarted.engine.convergeRunningCapacity(); - - expect(restarted.registry.snapshot.devices).toMatchObject([ - { id: restarted.device.id, state: "deleted" }, - ]); - expect(restarted.operationsSinceStart()).toEqual(["listManaged", "shutdown", "destroy"]); - }); - - it("deletes a fresh device on the next start when the daemon crashed after its shutdown commit and before its delete", async () => { - const restarted = await restartAfterCrash("shutdown"); - expect(restarted.registry.snapshot.devices).toMatchObject([{ state: "shutdown" }]); + it.each([ + { + crashedIn: "reclaiming" as const, + operations: ["listManaged", "shutdown", "destroy"], + title: "while it was reclaiming", + }, + { + crashedIn: "shutdown" as const, + operations: ["listManaged", "destroy"], + title: "after its shutdown commit and before its delete", + }, + ])( + "deletes a fresh device on the next start when the daemon crashed $title", + async ({ crashedIn, operations }) => { + const restarted = await restartAfterCrash(crashedIn); + expect(restarted.registry.snapshot.devices).toMatchObject([{ state: crashedIn }]); - await restarted.engine.convergeRunningCapacity(); + await restarted.engine.convergeRunningCapacity(); - expect(restarted.registry.snapshot.devices).toMatchObject([ - { id: restarted.device.id, state: "deleted" }, - ]); - expect(restarted.operationsSinceStart()).toEqual(["listManaged", "destroy"]); - }); + expect(restarted.registry.snapshot.devices).toMatchObject([ + { id: restarted.device.id, state: "deleted" }, + ]); + expect(restarted.operationsSinceStart()).toEqual(operations); + }, + ); }); describe("createLeasing logger wiring", () => { @@ -2299,44 +2305,41 @@ describe("createLeasing restart recovery of stored lease requests", () => { return after; } - it("settles a request still open at a restart as failed and says the daemon restarted", async () => { - const after = await restartWithAgentQueued("key-1"); + it.each([{ idempotencyKey: "key-1" as string | undefined }, { idempotencyKey: undefined }])( + "settles a request still open at a restart as failed and says the daemon restarted, whether it carried an idempotency key or not ($idempotencyKey)", + async ({ idempotencyKey }) => { + const after = await restartWithAgentQueued(idempotencyKey); - expect( - after.registry.leaseRequests().find((record) => record.requesterId === "agent"), - ).toMatchObject({ - failure: { code: "INTERNAL", message: expect.stringContaining("daemon restarted") }, - state: "failed", - }); - expect( - after.bus - .replay() - .filter((event) => event.event === "lease.rejected") - .map((event) => event.payload), - ).toEqual([ - { - reason: "daemon-restarted", - requestId: expect.stringMatching(/^req_/), - requester: "agent", - requestSpec: request, - }, - ]); - await expect( - after.engine.request(request, { - idempotencyKey: "key-1", - ownerId: "agent", - requesterId: "agent", - }), - ).rejects.toMatchObject({ message: expect.stringContaining("daemon restarted") }); - }); - - it("settles a request that carried no idempotency key at a restart too", async () => { - const after = await restartWithAgentQueued(); - - expect( - after.registry.leaseRequests().find((record) => record.requesterId === "agent"), - ).toMatchObject({ state: "failed" }); - }); + expect( + after.registry.leaseRequests().find((record) => record.requesterId === "agent"), + ).toMatchObject({ + failure: { code: "INTERNAL", message: expect.stringContaining("daemon restarted") }, + state: "failed", + }); + expect( + after.bus + .replay() + .filter((event) => event.event === "lease.rejected") + .map((event) => event.payload), + ).toEqual([ + { + reason: "daemon-restarted", + requestId: expect.stringMatching(/^req_/), + requester: "agent", + requestSpec: request, + }, + ]); + if (idempotencyKey !== undefined) { + await expect( + after.engine.request(request, { + idempotencyKey, + ownerId: "agent", + requesterId: "agent", + }), + ).rejects.toMatchObject({ message: expect.stringContaining("daemon restarted") }); + } + }, + ); }); describe("createLeasing a grant and its request's result", () => { @@ -2359,15 +2362,12 @@ describe("createLeasing a grant and its request's result", () => { expect(after.registry.snapshot.leases).toHaveLength(1); }); - it("answers a repeat under the same key in the same process with the lease after the request book's settle finds the request already granted", async () => { + it("answers a repeat under the same key in the same process with the lease", async () => { const harness = await createHarness(); await seedReady(harness); - const settle = vi.spyOn(harness.registry, "settleLeaseRequest"); const grant = await harness.engine.request(request, asker); await flush(); - expect(settle).toHaveBeenCalledTimes(1); - await expect(settle.mock.results[0]?.value).resolves.toBeUndefined(); const repeat = await harness.engine.request(request, asker); expect(repeat).toEqual(grant); expect(repeat.lease.id).toBe(grant.lease.id); @@ -3702,8 +3702,19 @@ describe("createLeasing warm targets", () => { const device = await seedReady(harness); await harness.engine.convergeRunningCapacity(); - harness.clock.advance(30_000); - await harness.engine.settle(); + // why: a pass runs on a trigger, and the periodic tick is 30 s away: a daemon.started fact is one + // at the threshold and one millisecond past it. + harness.clock.advance(10_000); + harness.bus.emit("daemon.started", { configSnapshot: {}, version: "test" }, "test"); + await drain(harness); + expect(harness.registry.snapshot.devices.find((item) => item.id === device.id)?.state).toBe( + "ready", + ); + expect(eventsNamed(harness, "device.shutdown")).toEqual([]); + + harness.clock.advance(1); + harness.bus.emit("daemon.started", { configSnapshot: {}, version: "test" }, "test"); + await drain(harness); expect(harness.registry.snapshot.devices.find((item) => item.id === device.id)?.state).toBe( "shutdown", @@ -4565,145 +4576,167 @@ describe("createLeasing: a gateway dispatch is a probe (ADR 0021)", () => { ?.id; } - it("emits lease.declined with reason no-wait and its fleetRequestId, and no lease.rejected, for a probe refused for no capacity", async () => { - const harness = await createHarness(); - await harness.engine.request(request, holder); - - await expect(harness.engine.request(request, probeOf("gw:one"))).rejects.toBeInstanceOf( - NoCapacityError, - ); + interface Declined { + readonly harness: Harness; + readonly payload: unknown; + readonly check?: () => void; + } - expect(payloads(harness, "lease.declined")).toEqual([ - { - fleetRequestId: "req_gw1", - reason: "no-wait", - requestId: await admittedId(harness, "gw:one"), - requestSpec: request, - requester: "gw:one", + const declinedCases: ReadonlyArray Promise]> = [ + [ + "no-wait", + async () => { + const harness = await createHarness(); + await harness.engine.request(request, holder); + await expect(harness.engine.request(request, probeOf("gw:one"))).rejects.toBeInstanceOf( + NoCapacityError, + ); + return { + harness, + payload: { + fleetRequestId: "req_gw1", + reason: "no-wait", + requestId: await admittedId(harness, "gw:one"), + requestSpec: request, + requester: "gw:one", + }, + }; }, - ]); - expect(payloads(harness, "lease.rejected")).toEqual([]); - }); - - it("emits lease.declined with reason unresolvable-spec for a probe naming a model the catalog lacks", async () => { - const harness = await createHarness(); - const odd = { ...request, model: "iPhone 99 Imaginary", osVersion: ">=18" }; - - await settledOrPending(harness.engine.request(odd, probeOf("gw:odd"))); - - expect(payloads(harness, "lease.declined")).toEqual([ - expect.objectContaining({ - fleetRequestId: "req_gw1", - reason: "unresolvable-spec", - requestSpec: odd, - }), - ]); - expect(payloads(harness, "lease.rejected")).toEqual([]); - }); - - it("emits lease.declined for a probe refused already-leased at admission, carrying its requestSpec", async () => { - const driver = new FakeDriver({ - availableOsVersions: ["26.5"], - clock: new FakeClock(1_000), - platform: "ios", - }); - driver.hangMakeReady(); - const harness = await createHarness({ driver }); - void harness.engine.request(request, probeOf("gw:one")).catch(() => undefined); - await expect - .poll(() => - harness.registry.leaseRequests().find((record) => record.requesterId === "gw:one"), - ) - .toMatchObject({ state: "open" }); - - await expect(harness.engine.request(request, probeOf("gw:one"))).rejects.toMatchObject({ - name: "RequesterAlreadyLeasedError", - }); - - expect(payloads(harness, "lease.declined")).toEqual([ - { - fleetRequestId: "req_gw1", - reason: "already-leased", - requestId: expect.stringMatching(/^req_/), - requestSpec: request, - requester: "gw:one", + ], + [ + "unresolvable-spec", + async () => { + const harness = await createHarness(); + const odd = { ...request, model: "iPhone 99 Imaginary", osVersion: ">=18" }; + await settledOrPending(harness.engine.request(odd, probeOf("gw:odd"))); + return { + harness, + payload: expect.objectContaining({ + fleetRequestId: "req_gw1", + reason: "unresolvable-spec", + requestSpec: odd, + }), + }; }, - ]); - expect(payloads(harness, "lease.rejected")).toEqual([]); - }); - - it("emits lease.declined for a probe refused lease-id-taken", async () => { - const harness = await createHarness({ - limits: { - android: { maxDevices: 2, maxRunning: 2 }, - ios: { maxDevices: 3, maxRunning: 3 }, - maxRunning: 4, + ], + [ + "already-leased", + async () => { + const driver = new FakeDriver({ + availableOsVersions: ["26.5"], + clock: new FakeClock(1_000), + platform: "ios", + }); + driver.hangMakeReady(); + const harness = await createHarness({ driver }); + void harness.engine.request(request, probeOf("gw:one")).catch(() => undefined); + await expect + .poll(() => + harness.registry.leaseRequests().find((record) => record.requesterId === "gw:one"), + ) + .toMatchObject({ state: "open" }); + await expect(harness.engine.request(request, probeOf("gw:one"))).rejects.toMatchObject({ + name: "RequesterAlreadyLeasedError", + }); + return { + harness, + payload: { + fleetRequestId: "req_gw1", + reason: "already-leased", + requestId: expect.stringMatching(/^req_/), + requestSpec: request, + requester: "gw:one", + }, + }; }, - }); - await harness.engine.request(request, { ...holder, leaseId: "myid" }); - - await settledOrPending(harness.engine.request(request, probeOf("gw:two", { leaseId: "myid" }))); - - expect(payloads(harness, "lease.declined")).toEqual([ - expect.objectContaining({ fleetRequestId: "req_gw1", reason: "lease-id-taken" }), - ]); - expect(payloads(harness, "lease.rejected")).toEqual([]); - }); - - it("emits lease.declined for a probe whose boot times out after a progress push, with reason boot-timeout", async () => { - const driver = new FakeDriver({ - availableOsVersions: ["26.5"], - clock: new FakeClock(1_000), - platform: "ios", - }); - driver.failOn("makeReady", 1, new Error("boot failed")); - const harness = await createHarness({ driver }); - const stages: string[] = []; - - await settledOrPending( - harness.engine.request(request, { - ...probeOf("gw:slow"), - onProgress: (progress) => stages.push(progress.stage), - }), - ); - - expect(stages.length).toBeGreaterThan(0); - expect(payloads(harness, "lease.declined")).toEqual([ - expect.objectContaining({ fleetRequestId: "req_gw1", reason: "boot-timeout" }), - ]); - expect(payloads(harness, "lease.rejected")).toEqual([]); - }); - - it("emits lease.declined daemon-restarted with its fleetRequestId at the next start, for a probe still open when the daemon stopped, after the registry is reloaded from disk", async () => { - const driver = new FakeDriver({ - availableOsVersions: ["26.5"], - clock: new FakeClock(1_000), - platform: "ios", - }); - driver.hangMakeReady(); - const before = await createHarness({ driver }); - void before.engine.request(request, probeOf("gw:open")).catch(() => undefined); - await expect - .poll(() => - before.registry.leaseRequests().find((record) => record.requesterId === "gw:open"), - ) - .toMatchObject({ fleetRequestId: "req_gw1", state: "open" }); - const requestId = await admittedId(before, "gw:open"); + ], + [ + "lease-id-taken", + async () => { + const harness = await createHarness({ + limits: { + android: { maxDevices: 2, maxRunning: 2 }, + ios: { maxDevices: 3, maxRunning: 3 }, + maxRunning: 4, + }, + }); + await harness.engine.request(request, { ...holder, leaseId: "myid" }); + await settledOrPending( + harness.engine.request(request, probeOf("gw:two", { leaseId: "myid" })), + ); + return { + harness, + payload: expect.objectContaining({ fleetRequestId: "req_gw1", reason: "lease-id-taken" }), + }; + }, + ], + [ + "boot-timeout", + async () => { + const driver = new FakeDriver({ + availableOsVersions: ["26.5"], + clock: new FakeClock(1_000), + platform: "ios", + }); + driver.failOn("makeReady", 1, new Error("boot failed")); + const harness = await createHarness({ driver }); + const stages: string[] = []; + await settledOrPending( + harness.engine.request(request, { + ...probeOf("gw:slow"), + onProgress: (progress) => stages.push(progress.stage), + }), + ); + return { + check: () => expect(stages.length).toBeGreaterThan(0), + harness, + payload: expect.objectContaining({ fleetRequestId: "req_gw1", reason: "boot-timeout" }), + }; + }, + ], + [ + "daemon-restarted", + async () => { + const driver = new FakeDriver({ + availableOsVersions: ["26.5"], + clock: new FakeClock(1_000), + platform: "ios", + }); + driver.hangMakeReady(); + const before = await createHarness({ driver }); + void before.engine.request(request, probeOf("gw:open")).catch(() => undefined); + await expect + .poll(() => + before.registry.leaseRequests().find((record) => record.requesterId === "gw:open"), + ) + .toMatchObject({ fleetRequestId: "req_gw1", state: "open" }); + const requestId = await admittedId(before, "gw:open"); + const after = await createHarness({ state: before.state }); + await after.engine.convergeRunningCapacity(); + return { + harness: after, + payload: { + fleetRequestId: "req_gw1", + reason: "daemon-restarted", + requestId, + requestSpec: request, + requester: "gw:open", + }, + }; + }, + ], + ]; - const after = await createHarness({ state: before.state }); - await after.engine.convergeRunningCapacity(); + it.each(declinedCases)( + "emits one lease.declined with its fleetRequestId and reason %s, and no lease.rejected, for a probe refused that way", + async (_reason, run) => { + const { check, harness, payload } = await run(); - expect(payloads(after, "lease.declined")).toEqual([ - { - fleetRequestId: "req_gw1", - reason: "daemon-restarted", - requestId, - requestSpec: request, - requester: "gw:open", - }, - ]); - expect(payloads(after, "lease.rejected")).toEqual([]); - }); + expect(payloads(harness, "lease.declined")).toEqual([payload]); + expect(payloads(harness, "lease.rejected")).toEqual([]); + check?.(); + }, + ); it("still emits lease.rejected no-wait, and no lease.declined, for a local noWait request with no room", async () => { const harness = await createHarness(); @@ -4989,6 +5022,7 @@ describe("createLeasing: a waiting request says why it is not placed (#636)", () { reason: undefined, requesterId: "failing", stage: "starting" }, { reason: "device-limit", requesterId: "meanwhile", stage: "queued" }, ]); + expect(harness.engine.waitingRequests()[0]).not.toHaveProperty("reason"); clock.advance(50); await flush(); @@ -5171,22 +5205,4 @@ describe("createLeasing: a waiting request says why it is not placed (#636)", () { reason: "device-limit", requesterId: "second", stage: "queued" }, ]); }); - - it("lists no reason for a starting request that is being provisioned", async () => { - const clock = new FakeClock(1_000); - const driver = new FakeDriver({ - availableOsVersions: ["26.5"], - clock, - latencyMs: { provision: 50 }, - platform: "ios", - }); - const harness = await createHarness({ driver }); - void harness.engine.request(request, asking("provisioned")); - await flush(); - - expect(listing(harness)).toEqual([ - { reason: undefined, requesterId: "provisioned", stage: "starting" }, - ]); - expect(harness.engine.waitingRequests()[0]).not.toHaveProperty("reason"); - }); }); From 618e9002ed5d5f4918f9721a3177c6afc7f7c03f Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Fri, 30 Oct 2026 17:53:39 +0100 Subject: [PATCH 2/2] test(audit): narrow two titles the review flagged --- src/gateway/dispatcher.test.ts | 2 +- src/http/app.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gateway/dispatcher.test.ts b/src/gateway/dispatcher.test.ts index 10a7a4e3c..e193a5ee5 100644 --- a/src/gateway/dispatcher.test.ts +++ b/src/gateway/dispatcher.test.ts @@ -1619,7 +1619,7 @@ describe("GatewayDispatcher", () => { "device.remove unenrols a device plugged into one machine; run it on the worker the device is plugged into", ], ] as const)( - "%s answers UNSUPPORTED_IN_GATEWAY_MODE with a message pointing at the worker (ADR 0010 §7, §8)", + "%s answers UNSUPPORTED_IN_GATEWAY_MODE with a message pointing at the worker", async (operation, input, role, message) => { const { dispatcher } = harness(); diff --git a/src/http/app.test.ts b/src/http/app.test.ts index e4eb8858e..f95ab2b32 100644 --- a/src/http/app.test.ts +++ b/src/http/app.test.ts @@ -3257,7 +3257,7 @@ describe("the stream mount's listed route", () => { }); describe("ETag on a JSON read under /v1", () => { - it("304 and no body for an unchanged read, 200 with a new ETag otherwise, no ETag on a 404 or a mount", async () => { + it("304 and no body for an unchanged read, 200 for a stale tag, a new ETag once the data changes, no ETag on a 404 or a mount", async () => { const tokens: { id: string }[] = [{ id: "tok_1" }]; const { app, dispatcher, eventBus } = buildHarness({ mounts: [