diff --git a/e2e/console/fixtures.ts b/e2e/console/fixtures.ts index 8b2bf0522..44618a63d 100644 --- a/e2e/console/fixtures.ts +++ b/e2e/console/fixtures.ts @@ -22,16 +22,8 @@ import { freeLoopbackPort } from "../helpers/port.js"; import { REPO_ROOT } from "../helpers/repo-root.js"; import { waitFor } from "../helpers/wait.js"; -/** - * The browser lane's daemons. Each is a real daemon process with HTTP enabled on a free - * loopback port and its own `SIMLOCK_HOME`, so nothing here touches the machine's own Simlock. - * A worker runs the end-to-end suite's fake driver; a gateway runs no drivers at all. - */ - export interface ConsoleDaemon { - /** The console's address, exactly as `simlock status --json` reports it. */ readonly url: string; - /** Runs `simlock` against this daemon, for a test that changes its state (a revoked token). */ readonly cli: (args: readonly string[]) => Promise; readonly tokens: { readonly operator: string; @@ -40,19 +32,13 @@ export interface ConsoleDaemon { }; } -/** A daemon a test drives itself: its CLI, and stopping and starting it in place. */ export interface RunningDaemon extends ConsoleDaemon { readonly port: number; - /** The daemon's unix socket, for a test that drives it with the programmatic client. */ readonly socketPath: string; cliBackground(args: readonly string[]): CliBackgroundHandle; - /** Replaces the fake driver's script. A daemon reads its host facts and catalog at start. */ writeDriverScript(script: FakeDriverScript): Promise; - /** `simlock daemon stop`, keeping the home: `start()` brings the same daemon back. */ stop(): Promise; - /** `simlock daemon start` on the same home, port and tokens. */ start(): Promise; - /** Stops the daemon and deletes its home. */ dispose(options?: { readonly preservePages?: readonly Page[] }): Promise; /** why: stats read completed minutes only, so this moves the usage rows two minutes back. */ prepareHistoricalUsage(): Promise; @@ -60,7 +46,6 @@ export interface RunningDaemon extends ConsoleDaemon { export interface StartOptions { readonly config?: Readonly>; - /** The fake driver's script, written before the daemon starts. */ readonly driverScript?: FakeDriverScript; } @@ -213,7 +198,7 @@ async function closeActivePages(preserved: ReadonlySet): Promise } /** - * Pins a worker's capacity to the flow, not the machine running it, as the end-to-end suite + * why: Pins a worker's capacity to the flow, not the machine running it, as the end-to-end suite * does: fake devices use no real memory, so a nominal per-device budget keeps the RAM gate from * refusing a lease on a small CI runner. */ @@ -417,7 +402,6 @@ export async function startDaemon( } } -/** A gateway and the workers that joined it, each labelled. */ export interface Fleet { readonly gateway: RunningDaemon; readonly workers: readonly RunningDaemon[]; @@ -432,7 +416,7 @@ export async function mintJoinToken(gateway: RunningDaemon): Promise { } /** - * Starts a gateway and one worker per entry of `workers`, each joined to it with its label, and + * why: Starts a gateway and one worker per entry of `workers`, each joined to it with its label, and * waits until the gateway lists every one of them as connected and has read it: a worker that is * still `starting`, or not yet read, has no capacity or catalog in its view, and a test that * leases through the gateway or reads a card's capacity right away would see that gap. @@ -504,15 +488,8 @@ export async function disposeFleet( await fleet.gateway.dispose(options); } -/** The protocol range the stand-in below claims: older than any this gateway speaks. */ export const OLD_PROTOCOL = { max: 3, min: 1 } as const; -/** - * A worker too old for the gateway, without building an old Simlock: it dials the gateway's - * uplink with a real join token, as a worker does, and answers the gateway's `hello` the way a - * daemon speaking only protocol {@link OLD_PROTOCOL} does. The gateway then lists it as - * `incompatible` with both ranges. Resolves once the uplink is open; call the result to hang up. - */ export async function joinOldWorker( gateway: RunningDaemon, identity: { readonly id: string; readonly label: string }, @@ -699,10 +676,6 @@ export async function startProxiedDaemon( }; } -/** - * `daemon` is a single host, shared by every test in a Playwright worker, and the base URL every - * `page.goto` resolves against. `gateway` starts only for a test that asks for it. - */ export const test = base.extend< { proxied: ProxiedDaemon }, { daemon: RunningDaemon; gateway: RunningDaemon; proxiedDaemon: ProxiedDaemon } @@ -755,7 +728,6 @@ export const test = base.extend< export { expect }; -/** Signs in through the sign-in screen, as an operator would. */ export async function signIn(page: Page, token: string): Promise { await page.getByLabel("Operator token").fill(token); await page.getByRole("button", { name: "Sign in" }).click(); diff --git a/e2e/gateway-component-install.test.ts b/e2e/gateway-component-install.test.ts index 795deb1c0..ea4ac3c9f 100644 --- a/e2e/gateway-component-install.test.ts +++ b/e2e/gateway-component-install.test.ts @@ -36,7 +36,6 @@ function idOf(workers: readonly WorkerView[], label: string): string { return worker.id; } -/** Which workers the gateway's catalog lists Android `runtime` for. */ async function androidRuntimeWorkers(gateway: TestEnv, runtime: string): Promise { const catalog = await gateway.cli(["catalog", "--platform", "android", "--json"]); expect(catalog.code, catalog.stderr).toBe(0); @@ -61,7 +60,7 @@ describe("simlock component install through a gateway", () => { availableOsVersions: ["34"], installProgress: [50], knownModels: ["Pixel 9"], - // Held open long enough for `worker list` to see it. + // why: Held open long enough for `worker list` to see it. latencyMs: { installComponent: 3_000 }, }, }; @@ -94,7 +93,6 @@ describe("simlock component install through a gateway", () => { const locked = idOf(workers, "locked"); expect(await androidRuntimeWorkers(gateway, "35")).toBeUndefined(); - // An agent token on the gateway's HTTP API is refused, and no worker starts an install. const agent = await gateway.cli(["token", "create", "--role", "agent"]); expect(agent.code, agent.stderr).toBe(0); const refused = await fetch(`http://127.0.0.1:${port}/v1/components/install`, { @@ -143,7 +141,6 @@ describe("simlock component install through a gateway", () => { outcome: "refused", workerId: locked, }); - // Progress on stderr names the worker it came from. const progress = install.stderr .trim() .split("\n") @@ -151,7 +148,7 @@ describe("simlock component install through a gateway", () => { .filter((line) => line["stage"] !== undefined); expect(progress).toContainEqual({ fraction: 0.5, stage: "downloading", workerId: open }); - // Read straight after the command, with no wait: the relay refreshed the catalog first. + // why: Read straight after the command, with no wait: the relay refreshed the catalog first. expect(await androidRuntimeWorkers(gateway, "35")).toEqual([open]); }); diff --git a/e2e/gateway-event-relay.test.ts b/e2e/gateway-event-relay.test.ts index 24acf102d..c4cae3445 100644 --- a/e2e/gateway-event-relay.test.ts +++ b/e2e/gateway-event-relay.test.ts @@ -10,12 +10,6 @@ import { SimlockWire } from "../src/simlock-client/wire.js"; import { mintJoinToken, waitFor, withDaemon, withHttpDaemon } from "./helpers/index.js"; import type { RecordedEvent } from "./helpers/events.js"; -/** - * ADR 0014 §2 and §5 with real processes: a gateway and one worker, the CLI an operator uses. - * A worker's event keeps its `id` and `timestamp` on the gateway, which is what lets the two - * event files be joined; `workerId` and the gateway's own `seq` are the only marks of the relay. - */ - async function gatewayWithWorker() { const { env: gateway, port } = await withHttpDaemon((port) => ({ configOverrides: { http: { host: "127.0.0.1", port } }, @@ -49,7 +43,6 @@ function leaseArgs(agentId: string): string[] { ]; } -/** The lines of a stream: a replayed event is printed as itself, a pushed one inside its push. */ function parse(stdout: string): RecordedEvent[] { return stdout .split("\n") @@ -281,11 +274,8 @@ describe("gateway event relay", () => { const follower = gateway.cliBackground(["events", "--follow", "--since", "1h"], { env: { SIMLOCK_ADMIN_TOKEN: adminToken, SIMLOCK_HOME: gate.home }, }); - // The follower has subscribed and asked for the replay; the gate holds that request. await waitFor(() => gate.replayHeld(), { label: "the replay request reached the gate" }); - // A lease is granted while the replay is pending: its push reaches the follower, and the - // replay, released only afterwards, is taken after the grant and holds it too. const holder = worker.cliBackground(leaseArgs("relay-follow")); const grant = JSON.parse(await holder.firstStdoutLine()) as { lease: { id: string } }; const isGrant = (event: RecordedEvent) => @@ -364,12 +354,6 @@ async function gatewayWithTwoWorkers() { const idsOf = (events: readonly RecordedEvent[]) => events.map((event) => event.id); -/** - * A unix-socket proxy in front of the gateway's daemon socket that holds the follower's - * `events.replay` request until `release()`. Everything else, pushes included, passes through, so - * an event emitted while the request is held reaches the follower as a push and is then also in - * the replay taken after the release. - */ async function gateReplay(upstreamSocket: string) { const home = await mkdtemp(join(tmpdir(), "sl-gate-")); // why: the follower runs with `--gateway`, which looks for a gateway folder under this root. @@ -424,7 +408,7 @@ async function gateReplay(upstreamSocket: string) { held?.(); }, async close() { - // `server.close` waits for every open connection; destroy them so a failed assertion in + // why: `server.close` waits for every open connection; destroy them so a failed assertion in // the test body is reported, not replaced by a timeout here. for (const socket of open) socket.destroy(); await new Promise((resolve) => server.close(() => resolve())); diff --git a/e2e/gateway-fast-fail.test.ts b/e2e/gateway-fast-fail.test.ts index 789150814..5a66ceaeb 100644 --- a/e2e/gateway-fast-fail.test.ts +++ b/e2e/gateway-fast-fail.test.ts @@ -4,16 +4,6 @@ import { mintJoinToken, waitFor, withDaemon, withHttpDaemon } from "./helpers/in import type { CliResult, TestEnv } from "./helpers/index.js"; import type { FakeDriverScript } from "./fake-driver/types.js"; -/** - * ADR 0009 §4 end to end, with real processes: a gateway daemon, worker daemons that dial it - * over a real WebSocket, and the CLI an agent would use. A request no worker can serve fails at - * once with the right code instead of waiting. - * - * Every request below has no `--no-wait` and no `--timeout`, the case that used to wait for ever. - * The CLI call carries a deadline of its own, so a request that waits fails here by name instead - * of hanging the suite. - */ - const FAST = { timeout: 20_000 }; const EXIT_NO_CAPACITY = 11; const EXIT_UNSERVABLE = 12; @@ -26,7 +16,6 @@ interface WorkerView { } async function startGateway() { - // `driver: "none"`: a gateway starts no drivers (ADR 0005 §2). const { env: gateway, port } = await withHttpDaemon((port) => ({ configOverrides: { http: { enabled: true, host: "127.0.0.1", port } }, role: "gateway", @@ -40,7 +29,7 @@ async function startGateway() { script: FakeDriverScript, extraConfig: Record = {}, ) => - // Seeded before the daemon starts: a worker reports its catalog as its uplink opens. + // why: Seeded before the daemon starts: a worker reports its catalog as its uplink opens. withDaemon({ configOverrides: { gateway: { label, token: await mintJoinToken(gateway), url: `ws://127.0.0.1:${port}` }, @@ -58,7 +47,6 @@ async function workersOf(gateway: TestEnv): Promise { return (listed.json as { workers: WorkerView[] }).workers; } -/** Waits until `labels` are all connected with a catalog read, and returns their ids by label. */ async function waitForWorkers( gateway: TestEnv, labels: readonly string[], @@ -118,7 +106,6 @@ function expectFailure(result: CliResult, code: string, exit: number): void { describe("a gateway fails a request no worker can serve at once", () => { it("fails a request for a model, a runtime, or a platform no worker has, and lets the same requester ask again", async () => { const { gateway, join } = await startGateway(); - // 27.0 is installed, but iPhone 17 pairs with 26.0 only. await join("worker-a", { ios: { ...IPHONE_17, @@ -128,20 +115,17 @@ describe("a gateway fails a request no worker can serve at once", () => { }); await waitForWorkers(gateway, ["worker-a"]); - // A model no worker lists. expectFailure( await lease(gateway, "agent-1", "iPhone 99", ["--detach"]), "UNKNOWN_MODEL", EXIT_UNSERVABLE, ); - // The same requester asks again at once: another answer, not REQUESTER_ALREADY_LEASED. expectFailure( await lease(gateway, "agent-1", "iPhone 99", ["--detach"]), "UNKNOWN_MODEL", EXIT_UNSERVABLE, ); - // A runtime no worker has, and, with --allow-download, still none: only installed runtimes count. const runtime = [ "--platform", "ios", @@ -157,7 +141,6 @@ describe("a gateway fails a request no worker can serve at once", () => { const download = await gateway.cli(["lease", ...runtime, "--detach", "--allow-download"], FAST); expectFailure(download, "RUNTIME_MISSING", EXIT_UNSERVABLE); - // A runtime a worker has, that no worker pairs with the model asked for. const unpaired = await gateway.cli( [ "lease", @@ -175,7 +158,6 @@ describe("a gateway fails a request no worker can serve at once", () => { ); expectFailure(unpaired, "RUNTIME_MISSING", EXIT_UNSERVABLE); - // A platform no worker has a driver for. const android = await gateway.cli( [ "lease", @@ -191,7 +173,6 @@ describe("a gateway fails a request no worker can serve at once", () => { ); expectFailure(android, "NO_DRIVER", EXIT_UNSERVABLE); - // The fleet still serves what it has, to the requester that was refused five times. const granted = await lease(gateway, "agent-1", "iPhone 17", ["--detach"]); expect(granted.code, granted.stderr).toBe(0); const { lease: held } = granted.json as { lease: { id: string } }; @@ -215,11 +196,9 @@ describe("a gateway fails a request no worker can serve at once", () => { FAST, ); - // No worker lists a model of the class. const watch = await ask("--class", "watch"); expectFailure(watch, "UNKNOWN_MODEL", EXIT_UNSERVABLE); expect(watch.stderr).toContain("watch"); - // Every model of the class pairs with a runtime outside the range, with or without a download. expectFailure( await ask("--class", "phone", "--os", "<=17"), "RUNTIME_MISSING", @@ -230,7 +209,6 @@ describe("a gateway fails a request no worker can serve at once", () => { "RUNTIME_MISSING", EXIT_UNSERVABLE, ); - // A request that names neither model nor class means phone, and the fleet has one. const granted = await ask(); expect(granted.code, granted.stderr).toBe(0); const { lease: held } = granted.json as { lease: { id: string } }; @@ -268,7 +246,6 @@ describe("a gateway fails a request no worker can serve at once", () => { const away = await join("worker-16", { ios: IPHONE_16 }); const ids = await waitForWorkers(gateway, ["worker-17", "worker-16"]); - // worker-17 drained: the only worker that lists iPhone 17 is not taking requests. expect((await gateway.cli(["worker", "drain", ids["worker-17"] ?? ""])).code).toBe(0); expectFailure( await lease(gateway, "agent-1", "iPhone 17", ["--detach"]), @@ -277,8 +254,6 @@ describe("a gateway fails a request no worker can serve at once", () => { ); expect((await gateway.cli(["worker", "undrain", ids["worker-17"] ?? ""])).code).toBe(0); - // worker-16 gone: a request only it lists fails with NO_CAPACITY, not UNKNOWN_MODEL, while - // worker-17 still takes requests. await away.killDaemon(); await waitFor( async () => @@ -322,7 +297,6 @@ describe("a gateway fails a request no worker can serve at once", () => { const held = await lease(gateway, "agent-holder", "iPhone 17", ["--detach"]); expect(held.code, held.stderr).toBe(0); - // A second requester waits: the worker can serve it and is busy. const waiting = gateway.cliBackground([ "lease", "--platform", diff --git a/e2e/gateway-fleet.test.ts b/e2e/gateway-fleet.test.ts index 4d58c4bc4..da305f2bd 100644 --- a/e2e/gateway-fleet.test.ts +++ b/e2e/gateway-fleet.test.ts @@ -9,16 +9,6 @@ import { } from "./helpers/index.js"; import type { TestEnv } from "./helpers/env.js"; -/** - * ADR 0005 #117 end to end, with real processes: a gateway daemon, two worker daemons that dial - * it over a real WebSocket, and the CLI an operator would actually use. - * - * What this flow proves that the unit suites cannot: the uplink is a real network connection on - * the gateway's own HTTP port, the join token is a real credential minted on the gateway and - * read from a worker's config file, and `simlock status` against the gateway aggregates two - * separate machines' registries. - */ - interface WorkerView { readonly id: string; readonly label?: string; @@ -63,7 +53,6 @@ async function listWorkers(gateway: TestEnv): Promise { return (result.json as { workers: WorkerView[] }).workers; } -/** Polls the *gateway*, never a worker: the gateway's own view is what this flow is about. */ async function waitForWorkers( gateway: TestEnv, predicate: (workers: WorkerView[]) => boolean, @@ -80,13 +69,11 @@ async function waitForWorkers( return workers; } -/** A worker's per-platform limit, or 0 if the view has no capacity yet. Out here so the test - * body below stays a sequence of assertions rather than a chain of fallbacks. */ function iosLimit(worker: WorkerView | undefined): number { return worker?.capacity?.ios.limit ?? 0; } -/** The id of the worker an operator would recognize by `label`. Throws rather than degrading +/** why: The id of the worker an operator would recognize by `label`. Throws rather than degrading * to `""`, which would silently turn "no such worker" into a request for one. */ function idOf(workers: readonly WorkerView[], label: string): string { const worker = workers.find((candidate) => candidate.label === label); @@ -103,8 +90,6 @@ function iosModels(json: unknown): readonly string[] { return platforms.find((entry) => entry.platform === "ios")?.models ?? []; } -/** A worker's iOS catalog entry, by the worker's label. */ -/** The label of the worker with the smallest id: the one whose catalog wins a disagreement. */ function firstWorkerLabel(workers: readonly WorkerView[]): string { const first = [...workers].sort((left, right) => (left.id < right.id ? -1 : 1))[0]; return first?.label ?? ""; @@ -116,14 +101,12 @@ function iosEntryOf(views: readonly WorkerView[], label: string) { ?.catalog.find((entry) => entry.platform === "ios"); } -/** The Xcode entry in a worker's host facts, by the worker's label. */ function xcodeOf(views: readonly WorkerView[], label: string) { return views .find((view) => view.label === label) ?.host?.tools.find((tool) => tool.name === "xcode"); } -/** The worker named `label` has the same host facts and catalog in both lists. */ function expectSameView( actual: readonly WorkerView[], expected: readonly WorkerView[], @@ -138,7 +121,7 @@ function expectSameView( describe("gateway fleet", () => { it("two workers join, status aggregates them, a kill disconnects one, drain flags the other", async () => { - // `driver: "none"`: a gateway starts no drivers and warns about every worker-only key in + // why: `driver: "none"`: a gateway starts no drivers and warns about every worker-only key in // its config (ADR 0005 §2), so it gets no fake-driver wiring and no capacity seeding. const { env: gateway, port } = await withHttpDaemon((port) => ({ configOverrides: { http: { host: "127.0.0.1", port } }, @@ -146,8 +129,6 @@ describe("gateway fleet", () => { driver: "none", })); - // 1. The gateway mints a join token. `--role worker` is what makes it a join token and - // nothing else: it opens an uplink and no `/v1` route. const minted = await gateway.cli(["token", "create", "--role", "worker"]); expect(minted.code).toBe(0); const { secret, token } = minted.json as { secret: string; token: { role: string } }; @@ -158,7 +139,7 @@ describe("gateway fleet", () => { const url = `ws://127.0.0.1:${port}`; const workerA = await withDaemon({ configOverrides: { gateway: { label: "worker-a", token: secret, url } }, - // Seeded before the daemon starts: the worker reports its catalog as its uplink opens, and + // why: Seeded before the daemon starts: the worker reports its catalog as its uplink opens, and // a script written afterwards would only reach the gateway on its 30s periodic refresh. driverScript: { ios: { knownModels: ["iPhone 16"], availableOsVersions: ["18.4"] } }, }); @@ -171,17 +152,14 @@ describe("gateway fleet", () => { gateway, (workers) => workers.length === 2 && - // The gateway marks a worker connected before its first refresh builds the view, so + // why: The gateway marks a worker connected before its first refresh builds the view, so // "connected" alone can still be a view with no capacity: wait for the capacity too. workers.every((worker) => worker.connection === "connected" && iosLimit(worker) > 0), "both workers connected to the gateway, each with its capacity", ); expect(joined.map((worker) => worker.label).sort()).toEqual(["worker-a", "worker-b"]); - // Two machines, two identities: the ids are the workers' own `instance.json`, never a name - // the gateway invented or a label an operator chose. expect(new Set(joined.map((worker) => worker.id)).size).toBe(2); - // 3. `simlock status` against the gateway is the same shape one machine returns, summed. const status = await gateway.cli(["status", "--json"]); expect(status.code).toBe(0); const statusView = status.json as StatusView; @@ -190,7 +168,6 @@ describe("gateway fleet", () => { expect(iosLimit(joined[0])).toBeGreaterThan(0); expect(statusView.capacity.ios.limit).toBe(iosLimit(joined[0]) + iosLimit(joined[1])); - // 4. The catalog is the union, and says which worker each model came from. await waitFor( async () => { const models = iosModels((await gateway.cli(["catalog", "--json"])).json); @@ -206,7 +183,7 @@ describe("gateway fleet", () => { ).platforms.find((entry) => entry.platform === "ios"); expect(iosCatalog?.modelWorkers?.["iPhone 16"]).toEqual([idOf(joined, "worker-a")]); - // 5. Killing a worker flips its view without anyone polling it: the uplink closing *is* the + // why: 5. Killing a worker flips its view without anyone polling it: the uplink closing *is* the // signal (ADR 0005 §6). The gateway's own backstop tick is 30s, and this must not need // it -- so a short deadline here is the assertion. await workerA.killDaemon(); @@ -215,12 +192,9 @@ describe("gateway fleet", () => { (workers) => connectionOf(workers, "worker-a") === "disconnected", "the killed worker's view flipped to disconnected", ); - // The other worker is untouched, and the dead one's view is kept rather than dropped. expect(surviving).toHaveLength(2); expect(connectionOf(surviving, "worker-b")).toBe("connected"); - // 6. Draining flags the surviving worker (what #118's dispatch will honour), and the flag - // is on the view an operator reads back. const workerBId = idOf(surviving, "worker-b"); const drain = await gateway.cli(["worker", "drain", workerBId]); expect(drain.code).toBe(0); @@ -229,7 +203,6 @@ describe("gateway fleet", () => { true, ); - // 7. Removing a *connected* worker is refused; the disconnected one is forgotten. const refused = await gateway.cli(["worker", "remove", workerBId]); expect(refused.error?.code).toBe("WORKER_CONNECTED"); const workerAId = idOf(surviving, "worker-a"); @@ -237,17 +210,6 @@ describe("gateway fleet", () => { expect(removed.code).toBe(0); expect(await listWorkers(gateway)).toHaveLength(1); - // 8. A worker's own *business* events -- not just the gateway's own connect/disconnect - // facts, which carry `workerId` too and would pass this check even with republishing - // (`WorkerLink#onWorkerEvent`) deleted outright -- reach the gateway's buffer with - // `workerId` attached, which is what makes `simlock events` against a gateway a - // fleet-wide view (ADR 0005 §22). Driven deterministically: a real `lease.granted` on - // worker B's own daemon, requested directly against its socket (routing across the - // fleet is #118's, not this PR's), well after its `events.subscribe` was established at - // connect. `module` is asserted as `lease-lifecycle` -- the emitting engine's own name, - // unchanged by republishing -- specifically because it is not one of the six subjects - // `docs/internal/EVENTS.md` calls "the gateway's own" (`worker.*`), so this cannot be satisfied by - // a gateway-authored fact that merely happens to name worker B. const events = await gateway.events(); const connectedFacts = events.filter((event) => event.event === "worker.connected"); expect(connectedFacts).toHaveLength(2); @@ -292,9 +254,6 @@ describe("gateway fleet", () => { driver: "none", })); const url = `ws://127.0.0.1:${port}`; - // Both workers have the same model and the same two runtimes installed, and pair the model - // with a different one of them. Each also accepts its own other name for it and reports its - // own image of one shared runtime. const installed = ["18.4", "26.0"]; await withDaemon({ configOverrides: { gateway: { label: "worker-a", token: await mintJoinToken(gateway), url } }, @@ -352,8 +311,6 @@ describe("gateway fleet", () => { modelRuntimes: { "iPhone 16": ["26.0"] }, }); - // Each worker's host facts sit beside its catalog, tools included once the worker has read - // them; `GET /v1/workers` serves the same views. const withTools = await waitForWorkers( gateway, (views) => @@ -387,8 +344,6 @@ describe("gateway fleet", () => { ); expect(ios?.modelRuntimes).toEqual({ "iPhone 16": ["18.4", "26.0"] }); expect(ios?.modelAliases).toEqual({ "iPhone 16": ["iphone-16-a", "iphone-16-b"] }); - // The two workers class the model differently; the first worker in id order wins, so the - // fleet's class is the one the worker with the smaller id reported. const classOfLabel: Record = { "worker-a": "phone", "worker-b": "tablet", @@ -407,7 +362,6 @@ describe("gateway fleet", () => { driver: "none", })); const url = `ws://127.0.0.1:${port}`; - // Both workers list My Tablet; only worker-a has it as a custom profile. await withDaemon({ configOverrides: { gateway: { label: "worker-a", token: await mintJoinToken(gateway), url } }, driverScript: { @@ -460,8 +414,6 @@ describe("gateway fleet", () => { driver: "none", })); const url = `ws://127.0.0.1:${port}`; - // One model per worker, so the model alone decides where a request lands. Both workers can - // slim the one runtime they have. await withDaemon({ configOverrides: { gateway: { label: "default-full", token: await mintJoinToken(gateway), url }, @@ -536,8 +488,6 @@ describe("gateway fleet", () => { })); const minted = await gateway.cli(["token", "create", "--role", "worker"]); const { secret } = minted.json as { secret: string }; - // The fake driver resolves model names exactly, so a grant proves the gateway sent the - // worker its own name for the model. await withDaemon({ configOverrides: { gateway: { label: "worker", token: secret, url: `ws://127.0.0.1:${port}` }, @@ -608,7 +558,6 @@ describe("gateway fleet", () => { "both workers connected with their catalogs", ); - // Warm one device on the first worker through its own socket, and let it go idle. const warmed = await warmWorker.cli([ "lease", "--platform", @@ -624,7 +573,7 @@ describe("gateway fleet", () => { }; await warmWorker.cli(["release", warmLease.id]); await waitForDeviceState(warmWorker, warmDevice.driverDeviceId, "ready"); - // Until the gateway's view shows that device ready, a request could miss it. + // why: Until the gateway's view shows that device ready, a request could miss it. await waitFor( async () => { const status = await gateway.cli(["status", "--json"]); @@ -674,7 +623,6 @@ describe("gateway fleet", () => { }, fakeDriverPlatforms: ["ios"], }); - // Sorted either way by id, only the range can send the request to the newer worker. await join("worker-old", "17.5"); await join("worker-new", "26.0"); await waitForWorkers( @@ -705,9 +653,6 @@ describe("gateway fleet", () => { role: "gateway", driver: "none", })); - // An operator token is a real credential that opens no uplink (ADR 0005 §25). The worker - // keeps retrying at its backoff cap rather than giving up, so what this asserts is the - // gateway's side: it never becomes a worker. const operator = await gateway.cli(["token", "create", "--role", "operator"]); const { secret } = operator.json as { secret: string }; await withDaemon({ @@ -725,9 +670,6 @@ describe("gateway fleet", () => { }, { label: "the gateway reported the refused uplink", timeout: 20_000 }, ); - // M3: `workerId` is whatever the dial claimed in its headers over the real WebSocket - // adapter, not just the in-memory one the unit suite scripts -- an operator otherwise had - // no way to tell *which* machine's misconfigured worker keeps knocking. expect(rejected?.workerId).toBeDefined(); expect(await listWorkers(gateway)).toEqual([]); diff --git a/e2e/gateway-smoke.test.ts b/e2e/gateway-smoke.test.ts index 7a3aae25f..b46eb1631 100644 --- a/e2e/gateway-smoke.test.ts +++ b/e2e/gateway-smoke.test.ts @@ -8,25 +8,6 @@ import { withHttpDaemon, } from "./helpers/index.js"; -/** - * ADR 0005's happy path, over real transports: a real gateway process, real worker processes - * dialling it over a real WebSocket, and a client reaching the fleet the way a user actually - * does -- the CLI on the gateway's own machine, and HTTP from anywhere else. - * - * Why this file exists at all. `src/daemon/gateway-fleet.e2e.test.ts` already proves lease - * routing and a real `device.exec` across two worker processes, but it drives them through the - * daemon's own `dispatch()` in-process. Everything between that dispatcher and a user was - * unproven for gateway mode: the CLI's own gateway/worker branch, the HTTP lease and exec - * routes against a gateway, SSE streaming of proxied output, and exit-code propagation. The one - * out-of-process lease that existed (`e2e/gateway-fleet.test.ts`) deliberately went straight to - * a worker's own socket, because fleet routing belonged to a later PR than the one that wrote - * it -- that PR has since landed, and nothing came back to close the gap. - * - * These are smoke tests. They prove the layers are wired to each other, not that the routing - * policy, the queue's ordering rules or the refusal lists are correct -- `src/gateway`'s own - * suites own that, and duplicating it here would buy nothing for a great deal of wall clock. - */ - interface WorkerView { readonly id: string; readonly label?: string; @@ -45,7 +26,6 @@ interface Grant { interface WorkerSpec { readonly label: string; readonly models: readonly string[]; - /** Seeded straight into the worker's config; used to pin a one-device worker for queueing. */ readonly limits?: Record; } @@ -56,7 +36,7 @@ interface Fleet { } /** - * A gateway plus its workers, all as separate daemons with their own `SIMLOCK_HOME` (and so + * why: A gateway plus its workers, all as separate daemons with their own `SIMLOCK_HOME` (and so * their own instance identity), joined over a real uplink. * * Returns only once the gateway can actually *serve* each worker's models, not merely once the @@ -65,7 +45,7 @@ interface Fleet { * these tests are about. */ async function startFleet(specs: readonly WorkerSpec[], agentId: string): Promise { - // `driver: "none"`: a gateway starts no drivers and warns about every worker-only key in its + // why: `driver: "none"`: a gateway starts no drivers and warns about every worker-only key in its // config (ADR 0005 §2), so it gets no fake-driver wiring and no capacity seeding. const { env: gateway, port } = await withHttpDaemon((port) => ({ agentId, @@ -86,7 +66,7 @@ async function startFleet(specs: readonly WorkerSpec[], agentId: string): Promis }, ...(spec.limits === undefined ? {} : { limits: spec.limits }), }, - // Before the daemon starts, not after: the worker reports its catalog as its uplink + // why: Before the daemon starts, not after: the worker reports its catalog as its uplink // opens, and the gateway would otherwise only see these models on its 30s refresh. driverScript: { android: { knownModels: [...spec.models], availableOsVersions: ["35"] } }, }); @@ -123,8 +103,6 @@ interface SseFrame { readonly data: unknown; } -/** Reads a finite SSE body, skipping keepalive comments -- the exec route ends the stream on - * its terminal `exit`/`error` event. Same shape as `device-exec.test.ts`'s own reader. */ async function readSse(response: Response): Promise { const frames: SseFrame[] = []; for (const raw of (await response.text()).split("\n\n")) { @@ -149,7 +127,6 @@ function chunksOf(frames: readonly SseFrame[], stream: "stdout" | "stderr"): str .join(""); } -/** Mints an agent token on the gateway and returns the headers an HTTP client would send. */ async function agentAuth(gateway: TestEnv): Promise> { const created = await gateway.cli(["token", "create", "--role", "agent"]); expect(created.code).toBe(0); @@ -159,8 +136,6 @@ async function agentAuth(gateway: TestEnv): Promise> { }; } -/** A `queued` progress push at a given position, as `contention.test.ts` reads them on one - * machine -- ADR §10 requires a gateway to report the same states a worker does. */ function isQueuedAt(event: unknown, position: number): boolean { return ( typeof event === "object" && @@ -186,16 +161,10 @@ describe("gateway smoke", () => { expect(leased.code).toBe(0); const grant = leased.json as Grant; expect(grant.lease.worker?.label).toBe("worker-a"); - // ADR §16: the gateway lease id names its worker. Asserted as a prefix rather than parsed, - // which is exactly what the docs tell a *client* not to do -- this is the fleet's own - // invariant, not a client reading it. expect(grant.lease.id.startsWith(`${grant.lease.worker?.id ?? ""}.`)).toBe(true); - // 2. §27a: the worker sees the requester namespaced by the gateway, so a local `smoke-agent` - // on that machine and this one cannot collide on its one-lease rule. This is the seam the - // in-process test cannot see, because it never asks the worker for its own view. const worker = workers[0]; - // Thrown rather than optional-chained: a fleet with no worker means the harness is broken, + // why: Thrown rather than optional-chained: a fleet with no worker means the harness is broken, // and reading through `undefined` would surface that as a TypeError three lines later // instead of as this sentence. if (worker === undefined) throw new Error("expected the fleet to have a worker"); @@ -228,8 +197,6 @@ describe("gateway smoke", () => { ); expect(exec.code).toBe(3); expect(exec.stderr).toContain("a warning"); - // The *worker's* driver resolved the command -- its own device root is prepended, on a - // machine the CLI never talked to directly. expect(JSON.parse(exec.stdout)).toEqual({ argv: ["/fake/android", "devices", "--fake-exec-stderr=a warning", "--fake-exec-exit=3"], platform: "android", @@ -257,7 +224,6 @@ describe("gateway smoke", () => { { label: "the gateway's HTTP port accepting connections" }, ); - // 1. The same request resource a worker serves, answered by a gateway that owns no devices. const created = await fetch(`${baseUrl}/v1/lease-requests`, { body: JSON.stringify({ device: "Pixel 8", platform: "android" }), headers: auth, @@ -282,8 +248,6 @@ describe("gateway smoke", () => { ); expect(leaseId).not.toBe(""); - // 2. A command that really runs: the gateway proxies it to the worker and relays the - // worker's own chunks and exit code back down this SSE stream. const streamed = await fetch(`${baseUrl}/v1/leases/${leaseId}/exec`, { body: JSON.stringify({ args: ["devices", "--fake-exec-exit=5"], tool: "adb" }), headers: auth, @@ -295,18 +259,6 @@ describe("gateway smoke", () => { expect(frames.at(-1)).toEqual({ data: { exitCode: 5 }, event: "exit" }); expect(JSON.parse(chunksOf(frames, "stdout"))).toMatchObject({ platform: "android" }); - // 3. A **silent, slow** command: the one shape that isolates ADR §19a's `started` push end - // to end. `http/app.ts` races the command settling against `started`, and a command that - // merely succeeds reaches the same committed `200` either way -- just not until it - // exits. So what `started` actually buys is timing, and the assertion has to be about - // timing: the response head must arrive while the command is still running. Relaying it - // is the gateway's own job (it holds no driver refusal list and cannot know a process - // exists any other way), so this is the assertion that fails if that relay is lost. - // - // Verified by mutation: neutering `FleetLeaseCoordinator#exec`'s `onStarted` relay - // leaves every other assertion in this file green and fails only the deadline below. - // Both weaker versions of this test -- a short silent command, and the refusal in 4 -- - // passed against that mutation, which is why neither of them is the one that guards it. const slowStartedAt = Date.now(); const silent = await fetch(`${baseUrl}/v1/leases/${leaseId}/exec`, { body: JSON.stringify({ @@ -319,18 +271,12 @@ describe("gateway smoke", () => { const headHeadersMs = Date.now() - slowStartedAt; expect(silent.status).toBe(200); expect(silent.headers.get("content-type")).toContain("text/event-stream"); - // Generously below the command's own 4s: the point is "well before it exits", not a precise - // latency budget, so this stays honest on a loaded machine while still failing outright if - // the route waits for the process to finish before answering. expect(headHeadersMs).toBeLessThan(2_000); const silentFrames = await readSse(silent); expect(chunksOf(silentFrames, "stdout")).toBe(""); expect(silentFrames.at(-1)).toEqual({ data: { exitCode: 0 }, event: "exit" }); - // 4. A refusal that happens *before* any process exists keeps its own status instead: the - // route has not committed to a stream, so §19a′'s `PASSTHROUGH_REFUSED` arrives as a - // real `422` rather than buried inside an already-committed `200`. const refused = await fetch(`${baseUrl}/v1/leases/${leaseId}/exec`, { body: JSON.stringify({ args: ["kill-server"], tool: "adb" }), headers: auth, @@ -485,7 +431,7 @@ describe("gateway smoke", () => { const first = await submit(); const firstRead = await readGranted(first.request.id); await release(firstRead); - // The release answers before the worker has reclaimed the device. The second request is + // why: The release answers before the worker has reclaimed the device. The second request is // granted inside its own `201` only once the gateway's view shows that device ready again. await waitFor( async () => { @@ -523,10 +469,6 @@ describe("gateway smoke", () => { }); it("queues a fleet-wide FIFO behind a full worker, refuses --no-wait, and serves the waiter on release", async () => { - // One worker that can run exactly one device, so the fleet's whole capacity is one lease. - // ADR §10: the gateway keeps a single fleet-wide queue and reports the same codes and - // progress states a worker's own queue does -- which is what this asserts, over the real - // CLI, against a daemon that owns no devices at all. const { gateway } = await startFleet( [ { @@ -543,13 +485,11 @@ describe("gateway smoke", () => { expect(first.code).toBe(0); const firstGrant = first.json as Grant; - // §14: one lease per requester is fleet-wide, not per machine. const repeat = await gateway.cli([...leaseArgs, "--agent-id", "agent-a", "--detach"]); expect(repeat.code).toBe(13); expect(repeat.error).toMatchObject({ code: "REQUESTER_ALREADY_LEASED" }); expect(repeat.error?.message).toContain(firstGrant.lease.id); - // A second requester with nowhere to go is refused rather than queued. const noWait = await gateway.cli([ ...leaseArgs, "--agent-id", @@ -560,13 +500,11 @@ describe("gateway smoke", () => { expect(noWait.code).toBe(11); expect(noWait.error).toMatchObject({ code: "NO_CAPACITY" }); - // A waiter queues, and says so with the same progress push a worker sends. const waiter = gateway.cliBackground([...leaseArgs, "--agent-id", "agent-c"]); await waitFor(() => waiter.progressEvents().some((event) => isQueuedAt(event, 1)), { label: "agent-c queued at position 1 in the gateway's fleet queue", }); - // Releasing the only device in the fleet is what dispatches it. expect((await gateway.cli(["release", firstGrant.lease.id])).code).toBe(0); const served = JSON.parse(await waiter.firstStdoutLine(30_000)) as Grant; expect(served.lease.worker?.label).toBe("worker-a"); @@ -577,8 +515,6 @@ describe("gateway smoke", () => { }); it("renews and releases a gateway-issued lease by the id the gateway handed out, and caps its own TTL", async () => { - // The gateway's own cap applies before anything is dispatched (§15), so it is the - // gateway's value that decides -- not the worker's, which is the 4h default here. const { gateway } = await startFleet( [{ label: "worker-a", models: ["Pixel 8"] }], "smoke-agent", @@ -610,9 +546,6 @@ describe("gateway smoke", () => { expect(leased.code).toBe(0); const grant = leased.json as Grant; - // The id an operator reads back has to be the id every other verb accepts. Round 6 found a - // projection that broke exactly this for `status.get`, caught only by a unit test -- here it - // is over the socket, forwarded to the worker and back. const listed = await gateway.cli(["status", "--json"]); expect(listed.code).toBe(0); const fleetLeases = (listed.json as { leases: { id: string }[] }).leases; @@ -625,7 +558,6 @@ describe("gateway smoke", () => { expect(renewedView.ttlDeadline).toBeGreaterThanOrEqual(grant.lease.ttlDeadline); expect((await gateway.cli(["release", grant.lease.id])).code).toBe(0); - // Released on the worker too, not merely forgotten by the gateway. await waitFor( async () => { const after = await gateway.cli(["status", "--json"]); diff --git a/e2e/helpers/index.ts b/e2e/helpers/index.ts index 2e8c73228..d73664460 100644 --- a/e2e/helpers/index.ts +++ b/e2e/helpers/index.ts @@ -1,7 +1,3 @@ -// Public surface for flow test files. Kept to exactly what a flow currently imports -// through this barrel -- add an export here (from its owning helper module) the -// moment a flow needs it, rather than pre-declaring the full internal surface, so an -// unused re-export is always a real signal. export { withDaemon, withSharedDaemon } from "./env.js"; export { assertPinnedAgentDevice, @@ -16,8 +12,6 @@ export { freeLoopbackPort } from "./port.js"; export { deviceStreamUrl } from "./device-stream.js"; export { withHttpDaemon } from "./http-daemon.js"; export { mintJoinToken } from "./join-token.js"; -// A flow that factors its own setup out of a test body needs to name what `withDaemon` hands -// back; every other flow keeps it inferred. export type { SharedTestEnv, TestEnv } from "./env.js"; export { DEVICE_READY_MS, waitFor } from "./wait.js"; export { events } from "./events.js"; diff --git a/scripts/slow-e2e.sh b/scripts/slow-e2e.sh index b4b1819ff..b020e4b21 100755 --- a/scripts/slow-e2e.sh +++ b/scripts/slow-e2e.sh @@ -222,7 +222,7 @@ log="${TMPDIR:-/tmp}/simlock-slow-e2e-$(date +%Y%m%d-%H%M%S).log" if ! mkdir "$lock" 2>/dev/null; then holder=$(cat "$lock/pid" 2>/dev/null || echo "") - # No pid yet means another caller is still starting its runner, unless that was long ago. + # why: No pid yet means another caller is still starting its runner, unless that was long ago. if { [ -n "$holder" ] && kill -0 "$holder" 2>/dev/null; } \ || { [ -z "$holder" ] && [ -n "$(find "$lock" -maxdepth 0 -mmin -1)" ]; }; then echo "busy: slow lane running as pid $holder from $(cat "$lock/root" 2>/dev/null), log $(cat "$lock/log" 2>/dev/null)" >&2 @@ -240,19 +240,17 @@ echo "$log" > "$lock/log" echo "$log" > "$lock.last" mkdir -p "$lane_tmp" -# The runner owns the lock from here: it records its pid, runs the lane, writes the exit code, -# and removes the lock however the lane ends. TMPDIR="$lane_tmp/" python3 - "$lock" "$log" "$root" "$script" "$@" <<'EOF' import os, subprocess, sys lock, log, root, script, args = sys.argv[1], sys.argv[2], sys.argv[3], sys.argv[4], sys.argv[5:] child = os.fork() if child != 0: - # Written before this returns, so a second caller never sees a lock without a live pid. + # why: Written before this returns, so a second caller never sees a lock without a live pid. with open(os.path.join(lock, "pid"), "w") as f: f.write(str(child)) sys.exit(0) os.setsid() -# Let go of the caller's stdout and stderr, or `$(scripts/slow-e2e.sh)` waits for the whole run. +# why: Let go of the caller's stdout and stderr, or `$(scripts/slow-e2e.sh)` waits for the whole run. null = os.open(os.devnull, os.O_RDWR) for fd in (0, 1, 2): os.dup2(null, fd) diff --git a/src/bus/index.ts b/src/bus/index.ts index ecec9799c..f5959acdd 100644 --- a/src/bus/index.ts +++ b/src/bus/index.ts @@ -13,31 +13,19 @@ export type WorkerRejectedReason = "unauthenticated" | "forbidden" | "worker-mis export interface EventMap { "lease.requested": { - /** The stored request's id, so an observer can correlate this event with its record. */ readonly requestId: string; - /** The request as it arrived (ADR 0015 §9): `class` when it named one, and `model` only when - * it did. A request naming neither carries neither; the class it means is `phone`. */ readonly requestSpec: unknown; readonly requester: string; readonly waitPolicy: string; - /** ADR 0021: the gateway's request id, on a worker's event for a gateway dispatch (a - * probe). Absent on a local request. */ readonly fleetRequestId?: string; }; "lease.queued": { readonly requestId: string; readonly queuePosition: number }; - /** ADR 0004's Consequences: `mode` left this payload, a deliberate one-off exception to - * events rule 6 (additive changes only) taken while the package is 0.x -- there is one kind - * of lease now, so there is nothing to report. `EVENTS.md` records the exception. */ "lease.granted": { readonly leaseId: string; readonly deviceId: string; readonly requester: string; - /** The stored request this lease served: the id its `lease.requested` carried. */ readonly requestId: string; - /** How the device came to be ready: handed over `warm`, `booted` from shut down, or - * `provisioned` for this request. */ readonly source: "warm" | "booted" | "provisioned"; - /** ADR 0021: the gateway's request id, when this grant served a gateway dispatch. */ readonly fleetRequestId?: string; // why: events stored before this field existed lack it and a rebuild replays them, so the projector reads it as optional. readonly platform: "ios" | "android"; @@ -47,7 +35,7 @@ export interface EventMap { readonly leaseId: string; readonly deviceId: string; readonly reason: "explicit" | "killed" | "device-lost"; - /** ADR 0003 §8: the released lease's owner, so a `lease-lost` push can be routed to every + /** why: ADR 0003 §8: the released lease's owner, so a `lease-lost` push can be routed to every * live connection whose principal owns it -- the registry no longer has the lease to look * this up from by the time this event fires (`beginRelease` already removed it), so it * travels on the event instead. */ @@ -59,40 +47,29 @@ export interface EventMap { readonly ownerId: string; }; "lease.rejected": { - /** The request's id. A request refused at admission (`killed`, `already-leased`, `lease-id-taken`) was never - * stored and has no `lease.requested`, but it carries the id it would have been stored - * under. A gateway's `lease-id-taken` can also follow a grant (a worker's lease whose ID the - * gateway already routes elsewhere): that request was stored and has its `lease.requested`. */ readonly requestId: string; readonly requester: string; - /** The request as it arrived, as on `lease.requested`: `class` when it named one, `model` - * only when it did (ADR 0015 §9). */ readonly requestSpec: unknown; readonly reason: | "timeout" | "no-wait" | "unresolvable-spec" - /** A gateway's request that no worker taking requests can serve (ADR 0009 §4). */ | "no-worker" | "already-leased" - /** The request named a lease ID an active lease or a waiting request holds (ADR 0020). */ | "lease-id-taken" | "boot-timeout" | "killed" | "updating" | "cancelled" - /** A request still open when the daemon stopped, settled as failed at the next start. */ | "daemon-restarted" - /** A gateway's request that failed on the worker it went to (ADR 0021 §4). */ | "worker-failed"; - /** On `worker-failed`: the error code the caller got. */ readonly code?: string; - /** On `worker-failed`: the worker the request failed on. Named `worker`, never `workerId`, + /** why: On `worker-failed`: the worker the request failed on. Named `worker`, never `workerId`, * because `payload.workerId` marks a relayed event (ADR 0014 §6). */ readonly worker?: string; }; /** - * ADR 0021 §2: a worker refused or failed a gateway dispatch (a probe). The gateway owns that + * why: ADR 0021 §2: a worker refused or failed a gateway dispatch (a probe). The gateway owns that * request's outcome, so the worker never rejects it. `reason` takes `lease.rejected`'s values. */ "lease.declined": { @@ -235,9 +212,7 @@ export interface EventMap { "component.installed": { readonly platform: string; readonly componentId: string; - /** The exact version now installed. */ readonly version: string; - /** True when the installer ran and the component was already there; nothing was recorded. */ readonly alreadyPresent: boolean; readonly durationMs: number; readonly requesterId?: string; @@ -251,15 +226,10 @@ export interface EventMap { }; "component.removed": { readonly platform: string; - /** The component string the removal was asked for: the version, as the catalog lists it. */ readonly componentId: string; - /** The exact version removed: the version of Simlock's record of installing it. */ readonly version: string; - /** The component's size as listed just before it was removed, when the driver could read it. */ readonly sizeBytes?: number; - /** What stayed on disk after the removal, and how to reclaim it, when something did. */ readonly residue?: string; - /** The principal of the admin session that asked. */ readonly requesterId: string; }; "daemon.started": { readonly version: string; readonly configSnapshot: unknown }; @@ -301,15 +271,9 @@ export interface EventMap { readonly reason: string; }; "doctor.reconciled": { readonly driftFindings: unknown }; - /** - * A warm pool target that was met, or not yet checked, ends a pass short for a reason: the pass - * could do nothing for it. Emitted once per kind of device on that edge (targets that resolve to one spec emit one event - * between them), not on each short pass after. - */ "warm-pool.target-missed": { readonly platform: string; readonly model: string; - /** Absent for a target that names none and did not resolve. */ readonly osVersion?: string; readonly mode: "slim" | "full"; readonly origin: "configured"; @@ -318,15 +282,6 @@ export interface EventMap { /** why: one reason, not all: the first of the reasons the target is short that applies. */ readonly reason: string; }; - // ---- gateway facts (ADR 0005 §22) --------------------------------------------------------- - // - // Emitted only by a daemon in gateway mode, about the workers connected to it. A worker's own - // events are *republished* on the gateway's bus under their original names with `workerId` - // added to the payload, so `simlock events` against a gateway shows the fleet; these six are - // the facts only the gateway can know. - /** A worker's uplink opened and its `hello` completed: the gateway can now drive it. An - * uplink that authenticated but negotiated no protocol emits nothing at all (ADR 0005 §31): - * it is in the registry as `incompatible`, which is where an operator finds it. */ "worker.connected": { readonly workerId: string; readonly label?: string; @@ -363,59 +318,31 @@ export interface EventMap { readonly label?: string; readonly count?: number; }; - /** A worker's uplink closed. `leaseCount` is what the view still shows it holding at that - * moment -- the number an operator needs to know how much is stranded, and the reason the - * view is not dropped immediately (§6). */ "worker.disconnected": { readonly workerId: string; readonly label?: string; readonly leaseCount: number; }; - /** A disconnected worker's view was forgotten: by an operator (`worker.remove`) or because - * `gateway.disconnectedRetentionMs` elapsed. */ "worker.removed": { readonly workerId: string; readonly label?: string; readonly reason: "operator" | "retention"; }; - /** ADR 0005 §9: the worker keeps its leases and receives no new dispatches from here on. */ "worker.drain-started": { readonly workerId: string; readonly label?: string }; "worker.drain-ended": { readonly workerId: string; readonly label?: string }; - /** - * ADR 0005 §11/§22 (#118): the gateway's own dispatch sent a queued request to a worker and - * the worker took it (a grant, or the first `progress` push, whichever came first -- device - * work having started means the request is that worker's now). A `NO_CAPACITY` refusal is a - * stale view, not a dispatch, and emits nothing -- the request stays queued, or a `noWait` - * request gets one more walk without that worker (ADR 0009 §5). - * - * C2 (round 2 review): widened back to what `docs/internal/EVENTS.md` always specified for this row -- - * `requesterId`/`platform`/`model` so the fact is self-contained (events rule 6) rather than - * naming only the gateway-internal `requestId`, `reason` (the routing policy's own warm-hit / - * free-capacity distinction, already computed by `RoutingPolicy#select` and previously - * discarded rather than plumbed through), and `queuedMs` (time spent queued before this - * dispatch). `reason` is spelled out as a literal union here rather than importing - * `RoutingReason` from `src/gateway/routing.ts`: this module stays platform- and - * gateway-agnostic (`architecture.md`), so a payload shape is duplicated rather than an import - * reaching up into a module layered above it. - */ "request.dispatched": { readonly requestId: string; readonly workerId: string; readonly requesterId: string; readonly platform: string; - /** The exact model the request named; absent for a class request (ADR 0015 §9). */ readonly model?: string; - /** The class the request named; absent for an exact one or one naming neither. */ readonly class?: string; - /** The mode the request named (ADR 0009 §8); absent when it named none. */ readonly mode?: "slim" | "full"; readonly physical?: true; readonly reason: "warm-hit" | "free-capacity"; - /** The routing stage that decided the pick (ADR 0009 §8). */ readonly stage: string; readonly queuedMs: number; }; - /** Payloads owned by the driver module that refused the root; see `DriverRejection`. */ "driver.root-rejected": { readonly platform: string; readonly root: string; @@ -428,8 +355,6 @@ export type EventName = keyof EventMap; export type EventEnvelope = Event extends EventName ? { - /** Names this event for good: minted once where it happened, it survives a restart and is - * what every reader tells events apart by (ADR 0014). `seq` only orders within one run. */ readonly id: string; readonly seq: number; readonly timestamp: number; @@ -439,7 +364,6 @@ export type EventEnvelope = Event extends E } : never; -/** An envelope before a bus gives it a `seq`: everything the place the fact happened stamped. */ export interface StampedEnvelope { readonly id: string; readonly timestamp: number; @@ -498,12 +422,6 @@ export class EventBus { }); } - /** - * Publishes an envelope whose fact happened elsewhere (ADR 0014 §2): it keeps the `id` and - * `timestamp` it was stamped with and gets only this bus's next `seq`. Reaches the ring and - * every subscriber exactly as `emit` does. Not a second way to state a fact -- the gateway's - * worker link is its one caller, relaying what a worker already stated. - */ republish(envelope: StampedEnvelope): EventEnvelope { return this.#publish(envelope); } @@ -585,7 +503,7 @@ export class EventBus { try { this.logger.error("Event handler failed", { error, envelope }); } catch { - // Logging must not break event delivery. + // why: Logging must not break event delivery. } } } diff --git a/src/contract/operations.test.ts b/src/contract/operations.test.ts index 17267d3fc..8cc811d19 100644 --- a/src/contract/operations.test.ts +++ b/src/contract/operations.test.ts @@ -12,11 +12,6 @@ import { PUSH_SCHEMAS } from "./pushes.js"; import type { Role } from "./roles.js"; import { platformCatalogSchema } from "./schemas.js"; -/** - * The ADR §3 operation matrix, name -> role. A table-driven test against this makes - * "forgetting a role" (or getting one wrong) a test failure, not just a type-level one -- - * `doctor.run` gets two rows since its role is a function of `fix`. - */ const ROLE_MATRIX: ReadonlyArray<{ readonly name: OperationName; readonly input: unknown; @@ -61,16 +56,13 @@ const ROLE_MATRIX: ReadonlyArray<{ { name: "worker.undrain", input: { workerId: "wrk_1" }, role: "admin" }, { name: "worker.remove", input: { workerId: "wrk_1" }, role: "admin" }, { name: "worker.update", input: { workerId: "wrk_1" }, role: "admin" }, - // ADR 0010 §6: an operator's consent to a download. { name: "component.install", input: { platform: "ios", version: "26.4" }, role: "admin" }, { name: "component.list", input: {}, role: "agent" }, - // ADR 0010 §7: the same consent, given for a set of workers through their gateway. { name: "worker.install-component", input: { platform: "ios", version: "26.4", workers: "all" }, role: "admin", }, - // ADR 0010 §8: removal is destructive, so admin-only. { name: "component.remove", input: { platform: "ios", version: "26.4" }, role: "admin" }, { name: "device.add", input: { id: "00008110-001A2B3C4D5E", platform: "ios" }, role: "admin" }, { name: "device.remove", input: { id: "00008110-001A2B3C4D5E" }, role: "admin" }, @@ -454,7 +446,7 @@ describe("operation input/output round trips", () => { }); it("lease.request/lease.renew: a non-positive ttlMs is a schema-level BAD_REQUEST", () => { - // The upper bound (`lease.maxTtlMs`) is the dispatcher's to enforce -- it is a daemon + // why: The upper bound (`lease.maxTtlMs`) is the dispatcher's to enforce -- it is a daemon // config value this module cannot see -- but "a TTL is a positive number" is shape. expect(() => OPERATIONS["lease.request"].input.parse({ @@ -1047,7 +1039,6 @@ describe("operation input/output round trips", () => { Object.fromEntries(names(count).map((name) => [name, ["a"]])); const classedModels = (count: number) => Object.fromEntries(names(count).map((name) => [name, "phone"])); - // Each pair is the largest value accepted and the smallest one refused. const bounds = [ [ { modelAliases: { ["x".repeat(256)]: ["a"] } }, @@ -1092,12 +1083,12 @@ describe("operation input/output round trips", () => { }); it("worker.remove reports whether there was a view to forget; drain never lies", () => { - // `remove` is idempotent: forgetting a worker the gateway has already forgotten is done, + // why: `remove` is idempotent: forgetting a worker the gateway has already forgotten is done, // not an error -- so `false` is a legitimate outcome, the same shape `token.revoke` uses. expect(OPERATIONS["worker.remove"].output.parse({ workerId: "wrk_1", removed: false })).toEqual( { workerId: "wrk_1", removed: false }, ); - // Drain and undrain each report the state they establish, and only that state: an + // why: Drain and undrain each report the state they establish, and only that state: an // `undrain` answering `drained: true` would be a contradiction rather than an outcome. expect(OPERATIONS["worker.drain"].output.parse({ workerId: "wrk_1", drained: true })).toEqual({ workerId: "wrk_1", @@ -1119,7 +1110,7 @@ describe("operation input/output round trips", () => { }); it("device.exec: parses a command, keeps stdin optional, and refuses anything else", () => { - // ADR 0005 §19a. `.strict()` is doing real work here: a client that sent `input` or `env` + // why: ADR 0005 §19a. `.strict()` is doing real work here: a client that sent `input` or `env` // hoping either would reach the child must be told no, not have it silently dropped. expect( OPERATIONS["device.exec"].input.parse({ @@ -1134,7 +1125,7 @@ describe("operation input/output round trips", () => { OPERATIONS["device.exec"].input.parse({ args: [], leaseId: "lse_1", tool: "simctl" }), ).toEqual({ args: [], leaseId: "lse_1", tool: "simctl" }); - // `tool` is an open string, exactly as `driver.passthrough`'s is: which wrappers exist is + // why: `tool` is an open string, exactly as `driver.passthrough`'s is: which wrappers exist is // the drivers' answer, so a name none of them wraps parses fine here and comes back as // `UNKNOWN_PASSTHROUGH_TOOL` from the driver catalog rather than as a malformed body. expect( @@ -1157,8 +1148,6 @@ describe("operation input/output round trips", () => { }); it("the output push carries a frame id, a stream, and a chunk", () => { - // The `output` family is request-scoped exactly like `progress` (ADR 0005 §19a), which is - // the property that lets one connection run more than one command at a time. expect(PUSH_SCHEMAS.output.parse({ chunk: "hello", requestId: 7, stream: "stdout" })).toEqual({ chunk: "hello", requestId: 7, @@ -1183,7 +1172,6 @@ describe("operation input/output round trips", () => { } }); - // Each refused push beside the nearest one that is accepted, so the refusal is the bound's. it.each([ ["a percent above 100", { percent: 101 }, { percent: 100 }], ["a percent below 0", { percent: -1 }, { percent: 0 }], diff --git a/src/contract/operations.ts b/src/contract/operations.ts index 4f0c381aa..3a30de277 100644 --- a/src/contract/operations.ts +++ b/src/contract/operations.ts @@ -1,12 +1,3 @@ -/** - * Every daemon operation, declared once (ADR 0003 §1). Name, role, input schema, output - * schema, optional `authorize` hook. Public TypeScript types are inferred from the zod - * schemas, never hand-written in parallel -- see each operation's `Input`/`Output` export. - * - * `defineOperation` returning a typed record whose `role` may be a plain `Role` or a function - * of the (already-validated) input is what lets `doctor.run` model an input-dependent role - * honestly (see its declaration below) without a second gating mechanism next to `authorize`. - */ import { z } from "zod"; import { CONSOLE_URL_MAX_LENGTH } from "./console-url.js"; @@ -68,15 +59,9 @@ export interface OperationDefinition< > { readonly name: Name; readonly role: Role | ((input: z.infer) => Role); - /** Whether a call changes state. The daemon log records every `write` at its default level - * and a `read` only at `debug`. Never sent on the wire. Like `role`, it may depend on the - * already-validated input. */ readonly effect: Effect | ((input: z.infer) => Effect); readonly input: InputSchema; readonly output: OutputSchema; - /** Ownership/ownership-adjacent gating a session-aware dispatcher would run after the role - * check (ADR §1's `ownsLease` example). Declared for typing purposes only in this PR -- - * nothing sets it, and nothing calls it; PR 2 adds both. */ readonly authorize?: (input: z.infer, context: AuthorizeContext) => boolean; } @@ -90,8 +75,6 @@ function defineOperation< return definition; } -// ---- catalog.get ------------------------------------------------------------------------------ - // fallow-ignore-next-line unused-export -- consumed only through the OPERATIONS registry, not by name; still public contract surface. export const catalogGet = defineOperation({ name: "catalog.get", @@ -101,31 +84,23 @@ export const catalogGet = defineOperation({ output: z.object({ platforms: z.array(platformCatalogSchema) }), }); -// ---- status.get --------------------------------------------------------------------------- - export const statusGet = defineOperation({ name: "status.get", role: "agent", effect: "read", input: z.object({}), output: z.object({ - // `devices`, `leases`, `capacity` and `queueDepth` are absent while `daemon.health` is + // why: `devices`, `leases`, `capacity` and `queueDepth` are absent while `daemon.health` is // `starting`: the registry has not been checked yet, so a reader sees a daemon that says // it is starting and what host it runs on, and nothing it could mistake for an empty fleet. devices: z.array(statusDeviceSchema).optional(), leases: z.array(statusLeaseSchema).optional(), capacity: statusCapacitySchema.optional(), - /** - * The warm pool as its last pass left it: whether it is on, the reserve, and each kind of device - * the targets name with how many are ready and booting and why it is short. Absent only while `daemon.health` - * is `starting`, like the fields above: a started worker and a gateway always send it. - */ warmPool: statusWarmPoolSchema.optional(), capabilities: capabilityListSchema.optional(), daemon: z.object({ health: daemonHealthSchema, mode: daemonModeSchema, - // Bounded: a gateway parses every worker's answer (safety rule 10). consoleUrl: z.string().max(CONSOLE_URL_MAX_LENGTH).optional(), ...daemonUpdateFields, }), @@ -136,7 +111,7 @@ export const statusGet = defineOperation({ physicalQueueDepth: z.number().optional(), installs: statusInstallsSchema.optional(), waiting: z.array(waitingRequestSchema).optional(), - /** Absent, not empty, from a worker: an empty array would read as a gateway with no workers. */ + /** why: Absent, not empty, from a worker: an empty array would read as a gateway with no workers. */ workers: z.array(workerViewSchema).optional(), }), }); @@ -172,7 +147,7 @@ const leaseRequestBaseSchema = z model: z.string().min(1).optional(), class: deviceClassSchema.optional(), platform: platformSchema, - // A plain string, checked by `refuseBadOsVersion`: a refined field would be checked by the MCP SDK + // why: A plain string, checked by `refuseBadOsVersion`: a refined field would be checked by the MCP SDK // before the handler, and its error would stand in for `BAD_REQUEST`. osVersion: z.string().optional(), mode: z.enum(["slim", "full"]).optional(), @@ -182,35 +157,11 @@ const leaseRequestBaseSchema = z allowDownload: z.boolean().optional(), noWait: z.boolean().optional(), timeoutMs: z.number().finite().positive().optional(), - /** Capped by the dispatcher's handler, not here: the cap is daemon config this module cannot see. */ + /** why: Capped by the dispatcher's handler, not here: the cap is daemon config this module cannot see. */ ttlMs: z.number().finite().positive().optional(), owner: z.string().optional(), - /** - * Makes this request repeatable: the daemon stores it under `(requesterId, idempotencyKey)`, - * and the same request sent again returns the stored result instead of a second lease -- - * the way a client that lost its answer, to a disconnect or a daemon restart, gets it back. - * The same key naming a different device, or a different `leaseId` (a missing one on either - * side counts as different), is `IDEMPOTENCY_CONFLICT`; a repeat from another - * principal is `FORBIDDEN`. Optional: a request without one is still stored, it just cannot - * be repeated. Bounded because the daemon stores it. - */ idempotencyKey: z.string().min(1).max(200).optional(), - /** - * ADR 0020: the ID the granted lease gets, instead of one simlock generates. The requester - * guarantees it is unique for all time; simlock refuses one an active lease or a waiting - * request holds (`LEASE_ID_TAKEN`) and keeps no record of IDs already used. A request - * without one works as before. Like `idempotencyKey` it is an option of the request, not - * part of the device it names. Anything outside the pattern is `BAD_REQUEST`. - */ leaseId: leaseIdSchema.optional(), - /** - * ADR 0021: the gateway's own request id, sent on every dispatch to a worker. A request that - * carries it is a probe: the worker declines it (`lease.declined`) rather than rejecting it, - * never queues it, and names the id on the request's lease events. Only the gateway's uplink - * session may send it, and only with `noWait: true`; the dispatcher refuses it otherwise - * (`FORBIDDEN`, then `BAD_REQUEST`). Not part of the MCP lease tool or the HTTP lease body. - * Bounded as `idempotencyKey` is, because the worker stores it. - */ fleetRequestId: z.string().min(1).max(200).optional(), requires: z .array(capabilityNameSchema) @@ -237,7 +188,7 @@ export function refuseModelWithClass( }; } -/** `allowDownload: false` passes because the CLI always sends it. */ +/** why: `allowDownload: false` passes because the CLI always sends it. */ function refusePhysicalWithImageFields(): < T extends { readonly physical?: boolean | undefined; @@ -327,7 +278,7 @@ export const leaseRequest = defineOperation({ effect: "write", input: leaseRequestInputSchema, output: leaseGrantSchema, - // No `authorize` hook on purpose: any agent session may request under an arbitrary + // why: No `authorize` hook on purpose: any agent session may request under an arbitrary // `requesterId`, and `owner` is gated by role in the handler (the resource does not exist yet). }); @@ -367,7 +318,7 @@ export const leaseCancel = defineOperation({ input: z.object({ requesterId: z.string().optional(), platform: platformSchema.optional() }), output: z.object({ result: z.enum(["cancelled", "not-found", "not-cancellable"]) }), /** - * ADR §9: "cancels this principal's pending request by requester id". Gated on the pending + * why: ADR §9: "cancels this principal's pending request by requester id". Gated on the pending * request's *recorded owner* (`pendingRequestOwner`, ADR §4's `ownerId` -- always the * creating session's principal, never the caller-suppliable `requesterId`), not on comparing * `requesterId` to the principal directly -- that comparison would forbid exactly the case @@ -391,8 +342,6 @@ export const leaseCancel = defineOperation({ }, }); -// ---- lease.renew ---------------------------------------------------------------------------- - /** * ADR 0004 §1: the only thing that keeps a lease alive, on every transport. An omitted `ttlMs` * re-applies the lease's own stored width -- never `lease.defaultTtlMs` -- so a lease granted @@ -410,8 +359,6 @@ export const leaseRenew = defineOperation({ authorize: ownsLease((input) => input.leaseId), }); -// ---- lease.release -------------------------------------------------------------------------- - export const leaseRelease = defineOperation({ name: "lease.release", role: "agent", @@ -421,8 +368,6 @@ export const leaseRelease = defineOperation({ authorize: ownsLease((input) => input.leaseId), }); -// ---- driver.passthrough (ADR 0001, decision 7) ----------------------------------------------- - /** * Builds the scoped command behind `simlock simctl` / `simlock adb`. It lives in the daemon * rather than the CLI because only the driver that proved a device root knows which flag points @@ -444,8 +389,6 @@ export const driverPassthrough = defineOperation({ output: passthroughCommandSchema, }); -// ---- device.exec (ADR 0005 §19a) ------------------------------------------------------------- - /** * Runs `simctl`, `adb` or `devicectl` **on the machine that owns the device** and streams the output back * (ADR 0005 §19a). The difference from `driver.passthrough` is where the process runs, not @@ -494,7 +437,7 @@ export const deviceExec = defineOperation({ args: z.array(z.string()), stdin: z.string().optional(), /** - * Who this command is being run *for* -- the requester the lease is attributed to (ADR + * why: Who this command is being run *for* -- the requester the lease is attributed to (ADR * 0003 §4), not the connection's principal. Read **only on an admin session**, where it * is the gateway case: a gateway holds one admin session over its uplink and proxies * many agents through it, each under its own namespaced requester id (ADR 0005 @@ -508,14 +451,14 @@ export const deviceExec = defineOperation({ }) .strict(), /** - * Only the exit code: every byte the command wrote already left as `output` pushes while it + * why: Only the exit code: every byte the command wrote already left as `output` pushes while it * ran (see `pushes.ts`), so an answer that also carried the output would mean the daemon had * buffered all of it -- which ADR §19e forbids, and which is why there is no size cap here * to speak of either. */ output: z.object({ exitCode: z.number() }), /** - * `ownsLease` for an agent, and something stricter for an admin. + * why: `ownsLease` for an agent, and something stricter for an admin. * * An **agent** session is gated exactly like `lease.renew`: its principal against the * lease's `ownerId`. It may not supply `requesterId` at all -- round 4, F4: this used to be @@ -552,8 +495,6 @@ export const deviceExec = defineOperation({ }, }); -// ---- lease.list (new, ADR §9) ----------------------------------------------------------------- - /** * ADR 0005 §20: the element carries an optional `workerId` -- absent from a worker's own * answer, set by a gateway, which lists the leases its worker views report. Additive, so a @@ -578,12 +519,10 @@ export const leaseGet = defineOperation({ output: z.object({ lease: statusLeaseSchema }), }); -// ---- doctor.run ------------------------------------------------------------------------------- - const doctorRunInputSchema = z.object({ fix: z.boolean().optional(), /** - * Destroys every orphan the run finds (ADR 0001, decision 5) -- the single opt-in exception + * why: Destroys every orphan the run finds (ADR 0001, decision 5) -- the single opt-in exception * to registry-only destruction (safety rule 1). Its own flag rather than part of `fix`, so a * `doctor --fix` already running unattended in CI does not acquire a destructive behaviour on * upgrade. @@ -595,7 +534,7 @@ const doctorRunInputSchema = z.object({ export const doctorRun = defineOperation({ name: "doctor.run", /** - * The role genuinely depends on the input: `fix: false` (read-only, shells out per device) + * why: The role genuinely depends on the input: `fix: false` (read-only, shells out per device) * is agent-visible; `fix: true` (applies safe fixes to registry-owned devices) is admin * only -- see ADR §3's matrix, which lists `doctor.run` twice for exactly this reason. A * function-of-input role, rather than an `authorize` hook, is the right tool here: `authorize` @@ -614,8 +553,6 @@ export const doctorRun = defineOperation({ output: doctorReportSchema, }); -// ---- lease.release-all ------------------------------------------------------------------------ - // fallow-ignore-next-line unused-export -- consumed only through the OPERATIONS registry, not by name; still public contract surface. export const leaseReleaseAll = defineOperation({ name: "lease.release-all", @@ -641,8 +578,6 @@ export const listGet = defineOperation({ ]), }); -// ---- cleanup.run -------------------------------------------------------------------------- - // fallow-ignore-next-line unused-export -- consumed only through the OPERATIONS registry, not by name; still public contract surface. export const cleanupRun = defineOperation({ name: "cleanup.run", @@ -652,8 +587,6 @@ export const cleanupRun = defineOperation({ output: z.array(proposalSchema), }); -// ---- nuke.run ----------------------------------------------------------------------------- - // fallow-ignore-next-line unused-export -- consumed only through the OPERATIONS registry, not by name; still public contract surface. export const nukeRun = defineOperation({ name: "nuke.run", @@ -663,8 +596,6 @@ export const nukeRun = defineOperation({ output: nukeReportSchema, }); -// ---- config.get --------------------------------------------------------------------------- - // fallow-ignore-next-line unused-export -- consumed only through the OPERATIONS registry, not by name; still public contract surface. export const configGet = defineOperation({ name: "config.get", @@ -674,13 +605,6 @@ export const configGet = defineOperation({ output: configSchema, }); -// ---- daemon.stop -------------------------------------------------------------------------- -// -// Part of the closed OPERATIONS registry (ADR §3); `daemon.stop` is handled as a -// frozen-exception special case in `DaemonServer#dispatchLine` rather than through the normal -// dispatch switch, so nothing imports this declaration by name yet. Declared here because the -// contract must still describe its input/output/role. - // fallow-ignore-next-line unused-export -- see the comment above. export const daemonStop = defineOperation({ name: "daemon.stop", @@ -720,8 +644,6 @@ export const daemonUpdate = defineOperation({ }), }); -// ---- events.replay ------------------------------------------------------------------------ - // fallow-ignore-next-line unused-export -- consumed only through the OPERATIONS registry, not by name; still public contract surface. export const eventsReplay = defineOperation({ name: "events.replay", @@ -747,8 +669,6 @@ export const usageGet = defineOperation({ output: usageOutputSchema, }); -// ---- events.subscribe --------------------------------------------------------------------- - // fallow-ignore-next-line unused-export -- consumed only through the OPERATIONS registry, not by name; still public contract surface. export const eventsSubscribe = defineOperation({ name: "events.subscribe", @@ -758,8 +678,6 @@ export const eventsSubscribe = defineOperation({ output: z.object({ subscribed: z.literal(true), subscriptionId: z.string() }), }); -// ---- events.unsubscribe ------------------------------------------------------------------- - // fallow-ignore-next-line unused-export -- consumed only through the OPERATIONS registry, not by name; still public contract surface. export const eventsUnsubscribe = defineOperation({ name: "events.unsubscribe", @@ -769,8 +687,6 @@ export const eventsUnsubscribe = defineOperation({ output: z.object({ subscribed: z.literal(false) }), }); -// ---- token.create / token.list / token.revoke -------------------------------------------- - // why: ADR §11 makes the daemon the only owner of the tokens, and that move has landed: both // dispatchers handle these three, the CLI reaches them as admin round-trips from // `src/cli/commands/token.ts`, and no CLI code reads the token store itself. The declarations live @@ -803,17 +719,6 @@ export const tokenRevoke = defineOperation({ output: z.object({ revoked: z.boolean() }), }); -// ---- worker.list / worker.drain / worker.undrain / worker.remove (ADR 0005 §8, §23) --------- -// -// They act on the gateway's worker registry. A worker answers them as a fleet of one (ADR -// 0012): `worker.list` returns one view, the worker itself, and `worker.drain`, -// `worker.undrain` and `worker.remove` answer `UNSUPPORTED_IN_WORKER_MODE`, since a single host -// has no gateway to drain it or forget it. -// -// `admin` throughout: these are operator tools (drain a machine for maintenance, forget a -// machine that is gone), and ADR 0003 §3's matrix puts every daemon-wide administrative -// operation at that role. - // fallow-ignore-next-line unused-export -- consumed only through the OPERATIONS registry, not by name; still public contract surface. export const workerList = defineOperation({ name: "worker.list", @@ -885,8 +790,6 @@ export const workerUpdate = defineOperation({ output: daemonUpdate.output.extend({ workerId: z.string() }), }); -// ---- component.install (ADR 0010 §6) -------------------------------------------------------- - /** * Installs one iOS simulator runtime or Android system image without leasing a device (ADR 0010 * §6). Admin-only: the command is an operator's consent to a multi-GB download, which @@ -908,8 +811,6 @@ export const componentInstall = defineOperation({ }), }); -// ---- component.list (ADR 0010 §8) ----------------------------------------------------------- - /** * Every installed iOS simulator runtime and Android system image on this machine, ordered by * platform, then version, then variant. A read for any session. `installedBySimlock` is true only @@ -941,9 +842,6 @@ export const componentList = defineOperation({ }), }); -// ---- worker.install-component (ADR 0010 §7) ------------------------------------------------- - -/** The longest worker id a gateway accepts at the uplink, and so the longest one it can name. */ const WORKER_ID_MAX_LENGTH = 128; /** The longest error message a relayed result carries. A worker's message is a claim, cut to @@ -959,14 +857,6 @@ const workerInstallTargetsSchema = z.union([ .refine((ids) => new Set(ids).size === ids.length, "worker ids must be distinct"), ]); -/** - * One worker's answer to a relayed install. `installed` and `already-installed` are the worker's - * own outcome, with the version it installed or found. `refused` is the worker's - * `DOWNLOADS_DISABLED`; `failed` is any other error the worker answered; `skipped` is a worker - * the gateway did not ask; `unknown` is a worker whose uplink closed, or that did not answer - * within its own `downloads.timeoutMs` plus a minute. Every outcome but the first two carries - * `error`. - */ const workerInstallResultSchema = z.object({ workerId: z.string(), label: z.string().optional(), diff --git a/src/contract/protocol.ts b/src/contract/protocol.ts index 54aa285c7..0095990df 100644 --- a/src/contract/protocol.ts +++ b/src/contract/protocol.ts @@ -48,7 +48,7 @@ export function mapLegacyProtocolMismatch( export const helloRequestSchema = z .object({ clientVersion: z.string(), - /** Legacy exact version, sent alongside `protocolRange` so an old protocol-2 daemon (which + /** why: Legacy exact version, sent alongside `protocolRange` so an old protocol-2 daemon (which * only ever compares this field) still answers intelligibly. */ protocolVersion: z.number().int().optional(), protocolRange: protocolRangeSchema.optional(), diff --git a/src/contract/schemas.ts b/src/contract/schemas.ts index 54c7a1b5c..0f2431a2d 100644 --- a/src/contract/schemas.ts +++ b/src/contract/schemas.ts @@ -1,22 +1,3 @@ -/** - * Zod schemas re-declaring today's wire shapes for the contract module (ADR 0003 §1). - * - * These schemas deliberately do NOT import their "real" counterparts from `src/core` -- - * `DeviceRecord`, `LeaseRecord`, `Config`, `DoctorFinding`, `Proposal`, `EventEnvelope`, and so - * on are core-private types. Re-declaring their fields here, by hand, is exactly what keeps - * private domain records off the public package surface (the daemon maps its own records onto - * these shapes in exactly one place -- `DaemonDispatcher#dispatch`'s `#parseOutput` call in - * `src/daemon/dispatcher.ts`, which parses every handler's result through the operation's - * output schema before it reaches a transport, stripping any field (such as `DeviceRecord`'s - * quarantine/foreign/recovery bookkeeping) that a narrower output schema like - * `grantedDeviceSchema` does not declare). If a core type's shape changes - * without a matching edit here, that is a compile-time or (for shapes structurally compatible - * but semantically different) a runtime output-validation failure at the daemon boundary -- - * see `DaemonServer`'s output parsing -- not a silent drift. - * - * This module must never import from src/core, src/daemon, src/drivers, src/http, src/cli, - * src/mcp, or src/ports -- enforced by `pnpm lint` (`.oxlintrc.json`). - */ import { z } from "zod"; export const platformSchema = z.enum(["ios", "android"]); @@ -95,7 +76,6 @@ function checkDeviceType( } } -/** The longest image tag a lease request may name. */ const IMAGE_TAG_MAX = 64; /** @@ -109,11 +89,7 @@ export const imageTagSchema = z .max(IMAGE_TAG_MAX) .regex(/^[A-Za-z0-9_.-]+$/, "imageTag may contain only letters, digits, '_', '.' and '-'"); -/** A device's spec as responses show it. The spec's planned mode is not shown (ADR 0007 §9): the - * device's `mode` is the one mode a response reports. `imageTag` is shown on a device whose - * request named one. */ const deviceSpecSchema = z.object({ - /** `true` for an enrolled physical device, `false` for a simulator or emulator. */ physical: z.boolean(), platform: platformSchema, model: z.string(), @@ -220,7 +196,6 @@ export const deviceRecordSchema = z address: z.string().optional(), absentReason: absentReasonSchema.optional(), physical: z.boolean(), - /** Present exactly when `physical` is `false`. */ mode: deviceModeSchema.optional(), class: physicalClassSchema.optional(), leaseIdentity: leaseIdentitySchema.optional(), @@ -258,7 +233,7 @@ export const statusDeviceSchema = z workerId: z.string().optional(), }) .superRefine((device, context) => checkDeviceType(device, context, true)) - // `list.get` records carry a virtual device's driver address, and a worker view parses them + // why: `list.get` records carry a virtual device's driver address, and a worker view parses them // through this schema: it drops the address of a device whose canonical ID is not shown. .transform((device): typeof device => { if (device.physical || device.driverDeviceId === undefined) return device; @@ -279,11 +254,8 @@ export const grantedDeviceSchema = z * nothing here ever guesses at a value it wasn't told. */ address: z.string().optional(), - /** Always present: see `checkDeviceType`. */ physical: z.boolean(), - /** The mode the device actually has (see `DeviceRecord.mode`); absent exactly when `physical`. */ mode: deviceModeSchema.optional(), - /** Carried by a physical device, but a grant does not require it (see `checkDeviceType`). */ class: physicalClassSchema.optional(), }) .superRefine((device, context) => checkDeviceType(device, context, false)); @@ -311,20 +283,7 @@ export const leaseRecordSchema = z.object({ ttlMs: z.number(), ttlDeadline: z.number(), lastRenewedAt: z.number(), - /** - * ADR 0020: whether the requester chose `id` (`lease.request`'s `leaseId`) rather than simlock - * generating it. A gateway rebuilding its routing table reads it to name such a lease bare. - */ idChosenByRequester: z.boolean(), - /** - * ADR 0005 §18: "the lease object gains `worker: { id, label }` (additive) so a client and - * the console can tell where the device lives. A worker's network address is never exposed." - * Additive and gateway-only -- a worker granting its own local lease has no second machine to - * name and never sets this; a gateway stamps it on every lease it forwards a grant/renew for, - * using the same worker id `id` already encodes (§16: a gateway lease id is - * `.`) plus the worker's display `label`, when it has one, for a - * console to render without a second lookup. - */ worker: z.object({ id: z.string(), label: z.string().optional() }).optional(), }); @@ -335,19 +294,12 @@ const leaseTimingSchema = z.object({ estimatedReadyMs: z.number(), }); -/** - * Scoping variables a lease holder needs to reach the device it was just granted -- the - * device-set path on iOS, the adb server port on Android (ADR 0001, decision 7). Built by the - * driver that owns the device root and forwarded verbatim; no key here means anything to the - * core or to this contract, which is what lets a third driver contribute its own scoping - * without an edit here (architecture rule 2). - */ const leaseEnvironmentSchema = z.record(z.string(), z.string()); export const leaseGrantSchema = z.object({ device: grantedDeviceSchema, /** - * Always present, `{}` at the least: containment cuts both ways, and a grant that did not + * why: Always present, `{}` at the least: containment cuts both ways, and a grant that did not * carry it would hand back a `driverDeviceId` no documented workflow can address. */ environment: leaseEnvironmentSchema, @@ -479,8 +431,6 @@ export const passthroughCommandSchema = z.object({ export const leaseProgressSchema = z.discriminatedUnion("stage", [ z.object({ stage: z.literal("queued"), queuePosition: z.number() }), - // ADR 0010 §3: the request waits on a component download. Bounded here so a gateway checks a - // worker's push before relaying it (safety rule 10). z.object({ stage: z.literal("downloading"), component: z.string().max(64), @@ -499,11 +449,6 @@ const runningCapacityEntrySchema = z.object({ overLimit: z.boolean(), }); -/** - * One platform's entry. `atRamBudget`: creating one more full device of the platform would be - * refused for RAM -- the check the planner makes, so a gateway can pass such a worker over - * (ADR 0009 §7). Always `false` under a strategy that keeps no RAM budget. - */ const platformCapacityEntrySchema = runningCapacityEntrySchema.extend({ limit: z.number(), warm: z.number(), @@ -533,7 +478,6 @@ export const WARM_POOL_NAME_MAX = 256; /** why: the configured targets plus the learned kinds, 32 per platform, the pool keeps. */ const WARM_POOL_ENTRIES_MAX = WARM_POOL_TARGETS_MAX + 64; -/** Why a warm target is short, in the order the first that applies is the one reported. */ const warmPoolShortSchema = z.enum([ "disabled", "no-driver", @@ -610,17 +554,11 @@ export const daemonUpdateFields = { latestVersion: updateVersionSchema.nullable(), }; -/** - * Limits on the catalog's other names and images. A worker's catalog is a claim the gateway - * parses off the wire, so every string and list in these fields has a bound well above any - * real host (an SDK ships some 70 device profiles and a few dozen images). - */ const CATALOG_NAME_MAX = 256; const CATALOG_ALIASES_PER_MODEL_MAX = 32; const CATALOG_ALIASED_MODELS_MAX = 4096; const CATALOG_IMAGE_FIELD_MAX = 128; const CATALOG_IMAGES_MAX = 1024; -/** `customModels` is a subset of `models`; it takes the bound already on a per-model list. */ const CATALOG_CUSTOM_MODELS_MAX = CATALOG_ALIASED_MODELS_MAX; /** @@ -640,22 +578,14 @@ const catalogImageSchema = z.object({ abi: z.string().max(CATALOG_IMAGE_FIELD_MAX), }); -/** - * ADR 0008 §5: what machine a daemon runs on. Every string and the `tools` list are bounded, - * because a gateway stores what each worker reports in that worker's view (safety rule 10). - */ const MAX_HOST_STRING_LENGTH = 128; const MAX_HOST_TOOLS = 32; const hostStringSchema = z.string().min(1).max(MAX_HOST_STRING_LENGTH); export const hostFactsSchema = z.object({ - /** Operating system product name (`macOS`), or the kernel name where there is none. */ os: hostStringSchema, osVersion: hostStringSchema, - /** CPU architecture as Node names it (`arm64`, `x64`). */ arch: hostStringSchema, - /** The platform tools each driver uses, with the version installed. Empty on a gateway, which - * runs no drivers. */ tools: z .array( z.object({ @@ -668,7 +598,6 @@ export const hostFactsSchema = z.object({ .max(MAX_HOST_TOOLS), }); -/** The shape `fitHostFacts` reads: the core's own host facts type and this schema's both fit. */ interface HostFactsShape { readonly arch: string; readonly os: string; @@ -754,18 +683,7 @@ export const platformCatalogSchema = z.object({ models: z.array(z.string()), runtimes: z.array(z.string()), defaultRuntime: z.string().optional(), - /** - * ADR 0008 §1: for each name in `models`, the installed runtimes it pairs with -- every pair - * listed here can be leased. An empty list means no installed runtime pairs with that model. - * On a gateway this is the union of each connected worker's own list for the model, never - * the cross product of fleet models and fleet runtimes. - */ modelRuntimes: z.record(z.string(), z.array(z.string())), - /** - * ADR 0008 §1: for a name in `models`, the other names a lease request may use for it, in any - * letter case -- a device id a driver accepts beside a display name. Only models that have another - * name appear. On a gateway it is the union per model of what each worker lists. - */ modelAliases: z .record( z.string().max(CATALOG_NAME_MAX), @@ -774,38 +692,14 @@ export const platformCatalogSchema = z.object({ .refine((aliases) => Object.keys(aliases).length <= CATALOG_ALIASED_MODELS_MAX, { message: `modelAliases lists more than ${CATALOG_ALIASED_MODELS_MAX} models`, }), - /** - * ADR 0015 §3: for a name in `models`, its class, when the platform's tools report one. A model - * the tools say nothing usable about has no entry. On a gateway it is the union over the - * connected workers; when two workers class a model differently, the first worker in id order - * wins. - */ modelClasses: z .record(z.string().max(CATALOG_NAME_MAX), deviceClassSchema) .refine((classes) => Object.keys(classes).length <= CATALOG_ALIASED_MODELS_MAX), - /** - * ADR 0015 §4: for a class, the model Simlock would create for it on this host -- of the - * names on the class's preference list that `models` lists and that are of the class, the - * first that pairs with an installed runtime, else the first of them. A class in which no - * listed model counts has no entry. On a gateway a class's entry is kept only when every - * connected worker reports the same model for it, and that worker lists it itself. - */ classDefaults: z.partialRecord(deviceClassSchema, z.string().max(CATALOG_NAME_MAX)), - /** - * ADR 0008 §1: every installed image, present only for a platform whose driver has images - * (Android). `runtime` is a value from `runtimes`; an image of an ABI the host cannot run - * natively is listed too. On a gateway it is the union by runtime, tag, and ABI. - */ images: z.array(catalogImageSchema).max(CATALOG_IMAGES_MAX).optional(), - /** - * Names from `models` that exist because of something on this machine rather than the - * platform's tools -- on Android, a custom profile from Android Studio's `devices.xml`. - * Absent when there are none. On a gateway a model is listed when any worker that lists it - * marks it custom. - */ customModels: z.array(z.string().max(CATALOG_NAME_MAX)).max(CATALOG_CUSTOM_MODELS_MAX).optional(), /** - * ADR 0005 §21: on a gateway, `models`/`runtimes` are the *union* over the fleet, and these + * why: ADR 0005 §21: on a gateway, `models`/`runtimes` are the *union* over the fleet, and these * two maps say which workers each entry came from (`{"iPhone 16": ["wrk_a", "wrk_b"]}`). * Additive and gateway-only: a worker's own catalog needs no attribution, and a renderer * that predates the fleet still reads `models`/`runtimes` unchanged. Kept as annotations @@ -862,7 +756,7 @@ export const proposalSchema = z.object({ export const cleanupRuleSummarySchema = z.object({ name: z.string() }); -/** Mirrors `DriverDevice` (src/core/driver.ts) -- `driverData` stays opaque `unknown` on +/** why: Mirrors `DriverDevice` (src/core/driver.ts) -- `driverData` stays opaque `unknown` on * purpose, exactly as the core treats it. */ const driverDeviceSchema = z.object({ deviceId: z.string(), @@ -962,16 +856,6 @@ export const nukeReportSchema = z.object({ releasedLeaseIds: z.array(z.string()), }); -/** - * Mirrors `EventEnvelope` (src/bus/index.ts) at the envelope level only. `event` stays a plain - * string and `payload` stays `z.unknown()` rather than redeclaring all ~30 `EventMap` payload - * shapes here -- see docs/internal/EVENTS.md for the authoritative catalog. This is a deliberate, - * documented simplification: re-declaring every business event's payload shape a second time, - * next to `events.md`'s existing documentation requirement, is a lot of near-duplicate surface - * for a channel this PR does not change the routing of. Tightening this (e.g. a discriminated - * union keyed on `event`) is left as follow-up work; flagged in the PR description. - */ -/** The largest magnitude, in ms, a JavaScript `Date` holds. */ const MAX_DATE_MS = 8.64e15; export const historyStatusSchema = z.object({ @@ -985,18 +869,14 @@ export const historyStatusSchema = z.object({ export const EVENT_ID_PATTERN = /^evt_[A-Za-z0-9_-]{1,64}$/; export const eventEnvelopeSchema = z.object({ - /** Wire input is a claim (safety.md): a pushed `id` is bounded, not trusted. */ id: z.string().regex(EVENT_ID_PATTERN), seq: z.number(), - /** A claim, bounded to the range a JavaScript date holds so the console can render it. */ timestamp: z.number().min(-MAX_DATE_MS).max(MAX_DATE_MS), event: z.string(), payload: z.unknown().optional(), module: z.string(), }); -// ---- config.get ----------------------------------------------------------------------------- - const platformCapacityOptionsSchema = z.object({ maxDevices: z.number().optional(), maxRunning: z.number().optional(), @@ -1030,19 +910,11 @@ const resourceCapacityConfigSchema = z.object({ }), }); -/** - * Mirrors `CapacityConfig` (src/core/capacity/strategies/index.ts), discriminated on - * `strategy` exactly as the core type is. Adding a third strategy needs a matching edit here -- - * there is no way around that with a hand-declared contract; it is the same trade-off the ADR - * accepts for every other core-private shape. - */ const capacityConfigSchema = z.discriminatedUnion("strategy", [ fixedCapacityConfigSchema, resourceCapacityConfigSchema, ]); -/** Mirrors `Config` (src/core/config.ts) field for field. */ -/** ADR 0015 §4: `ios.defaultModels` and `android.defaultModels`, a list of model names per class. */ const defaultModelsSchema = z.partialRecord(deviceClassSchema, z.array(z.string().min(1)).min(1)); const proxyAuthSchema = z.object({ @@ -1054,9 +926,6 @@ const proxyAuthSchema = z.object({ }); export const configSchema = z.object({ - /** ADR 0005 §1. `config.get` is how a caller learns which mode the daemon it is talking to - * runs in without inferring it from behaviour; `status.get`'s `daemon.mode` is the - * agent-role answer to the same question. */ mode: z.enum(["worker", "gateway"]), capabilities: z.record( capabilityNameSchema, @@ -1115,7 +984,7 @@ export const configSchema = z.object({ level: z.enum(["debug", "info", "warn", "error"]), rotateBytes: z.number(), }), - // Optional: a newer gateway parses this from an older worker's `config.get`. + // why: Optional: a newer gateway parses this from an older worker's `config.get`. eventLog: z .object({ retention: z.number().optional(), @@ -1154,29 +1023,13 @@ export const configSchema = z.object({ bootAnimation: z.boolean(), }), }), - /** ADR 0005 §3/§6. `url`/`token`/`label` are the worker's half, `disconnectedRetentionMs` - * the gateway's; a daemon carries the whole block whichever mode it runs in, and simply - * reads the half that applies (see `Config["gateway"]`). */ gateway: z.object({ url: z.string().optional(), token: z.string().optional(), label: z.string().optional(), disconnectedRetentionMs: z.number(), execTimeoutMs: z.number(), - /** ADR 0005 §29/P2: the forwarded `lease.request`'s own bound. Round 6 review: this was - * declared on `Config`, validated by `loadConfig`, and documented in CONFIGURATION.md, but - * missing here -- and since this schema is `config.get`'s declared output, zod stripped it, - * so an operator inspecting the effective config saw every other gateway key and concluded - * this one did not exist. `config.test.ts` now parses a loaded config through this schema - * and compares key sets, so the next such omission fails rather than disappears. */ leaseRequestTimeoutMs: z.number(), - /** ADR 0005 §13/Decision 7: which routing policy dispatch uses. Gateway-side, read once at - * construction like `capacity.strategy` -- never a per-request choice. The set of names is - * re-declared here rather than imported from `src/gateway/routing.ts`'s own registry, the - * same way `capacityConfigSchema` above re-declares "fixed"/"resource" instead of importing - * `src/core/capacity`'s: this module describes wire shapes and imports no engine (its own - * header comment), gateway or worker alike. `"warm-then-free"` is the only policy in v1; - * adding a second means adding it to both registries, exactly as a capacity strategy does. */ routing: z.enum(["warm-then-free"]), /** * why: `format` and `events` are re-declared, not imported from core; a test checks `format` against core's list. `secret` is optional so a `slack` destination parses. @@ -1199,8 +1052,6 @@ export const configSchema = z.object({ }), }); -// ---- tokens ----------------------------------------------------------------------------------- - /** * `TokenRole` (agent/operator/worker token store roles, src/auth/tokens.ts) is a * deliberately different vocabulary from the daemon session `Role` (agent/admin, see @@ -1226,8 +1077,6 @@ export const tokenRecordSchema = z.object({ workerId: z.string().max(128).optional(), }); -// ---- modes and worker views (ADR 0005) -------------------------------------------------------- - /** * Which mode a daemon runs in (ADR 0005 §1). A worker owns devices -- drivers, registry, * capacity, leases -- and is what every simlock daemon has been until now, so it is the @@ -1236,49 +1085,31 @@ export const tokenRecordSchema = z.object({ */ export const daemonModeSchema = z.enum(["worker", "gateway"]); -/** Mirrors `ProtocolRange` (protocol.ts). Re-declared rather than imported so this file stays - * the one place a wire *shape* is written down; `protocol.ts` owns the negotiation logic. */ const protocolRangeShapeSchema = z.object({ min: z.number().int(), max: z.number().int() }); -/** - * ADR 0005 §6: the uplink is the reachability signal. `connected` means the worker's uplink is - * open and its session negotiated a protocol version; `disconnected` means it closed and the - * view is the last-known one (kept until `gateway.disconnectedRetentionMs` elapses or an - * operator removes it); `incompatible` means the uplink is open but `hello` found no - * overlapping protocol version (§31) -- the view then carries both ranges and nothing else the - * gateway would have had to ask for over a protocol it cannot speak. - */ const workerConnectionStateSchema = z.enum(["connected", "disconnected", "incompatible"]); -// ---- installs in progress (ADR 0010 §3) --------------------------------------------------- - const MAX_LISTED_INSTALLS = 16; const MAX_INSTALL_COMPONENT_LENGTH = 64; /** - * One component install a daemon has waiting or running. A worker's list reaches the gateway as + * why: One component install a daemon has waiting or running. A worker's list reaches the gateway as * a claim (safety rule 10), so the bounds cut rather than refuse: an over-long list keeps its * first (oldest) 16 entries and an over-long `component` its first 64 characters, and the view * stays usable instead of failing its whole refresh over one field. */ const installInProgressSchema = z.object({ platform: platformSchema, - /** As the daemon's driver named it: a version, or a word such as "newest". */ component: z.string().transform((value) => value.slice(0, MAX_INSTALL_COMPONENT_LENGTH)), - /** `waiting` behind another install on its platform, or `downloading`. */ state: z.enum(["waiting", "downloading"]), - /** When the first call for this install arrived. */ since: z.number(), - /** How many calls are joined to it. */ waiters: z.number().int().nonnegative(), }); -/** A list of at most `MAX_LISTED_INSTALLS` entries; a longer one is cut to its first ones. */ function boundedInstalls(entry: Entry) { return z.array(entry).transform((installs) => installs.slice(0, MAX_LISTED_INSTALLS)); } -/** A worker view's installs: the worker's own `status.get` list, copied (ADR 0010 §7). */ const workerInstallsSchema = boundedInstalls(installInProgressSchema); /** `status.get`'s installs. On a gateway each entry names the worker it runs on. */ @@ -1347,50 +1178,33 @@ export function capabilityReason(text: string): string { export const workerViewSchema = z.object({ capabilities: capabilityListSchema.optional(), load: workerLoadSchema.optional(), - /** The worker's own instance identity (`instance.json`), ADR 0005 §3a: stable across - * restarts, unique by construction, opaque to clients. Never a host name or an address. */ id: z.string(), - /** `gateway.label` from the worker's config -- display only, need not be unique, and never - * used to route (§13: "label is display-only"). */ label: z.string().optional(), connection: workerConnectionStateSchema, - /** ADR 0005 §9: a drained worker keeps its leases and receives no new dispatches. */ drained: z.boolean(), - /** When the gateway last had this worker's uplink open, or last refreshed its view. */ lastSeenAt: z.number(), health: daemonHealthSchema.optional(), - /** The worker daemon's own version string, as reported by its `hello` reply. */ version: z.string().optional(), - /** Set only for an `incompatible` worker (§31), naming both ranges so an operator can see - * which side to upgrade. */ protocol: z .object({ gateway: protocolRangeShapeSchema, worker: protocolRangeShapeSchema }) .optional(), capacity: statusCapacitySchema.optional(), - /** The worker's warm pool as its last `status.get` read it; absent until one was read. */ warmPool: statusWarmPoolSchema.optional(), - /** - * The worker's effective `downloads.policy`, read once with `config.get` when the uplink - * connects. Display only: routing counts installed runtimes and never reads it, and the - * gateway forwards every request with `allowDownload: false` (ADR 0009 §3). Absent for a - * worker whose `config.get` the gateway could not read (an incompatible one, or a call that - * failed). - */ downloads: z .object({ policy: z.enum(["never", "on-request", "always"]), - /** Optional so a gateway on an older build still parses a view. */ + /** why: Optional so a gateway on an older build still parses a view. */ timeoutMs: z.number().optional(), }) .optional(), - /** A worker whose own cap is lower still refuses a request the gateway accepted; this lets the gateway notice. */ + /** why: A worker whose own cap is lower still refuses a request the gateway accepted; this lets the gateway notice. */ lease: z.object({ maxTtlMs: z.number() }).optional(), queueDepth: z.number().optional(), physicalQueueDepth: z.number().optional(), leases: z.array(leaseRecordSchema).optional(), devices: z.array(statusDeviceSchema).optional(), catalog: z.array(platformCatalogSchema).optional(), - /** Cleared on connect, so a just-connected worker has none: it tells a catalog read and found empty from one not yet arrived. */ + /** why: Cleared on connect, so a just-connected worker has none: it tells a catalog read and found empty from one not yet arrived. */ catalogReadAt: z.number().optional(), host: hostFactsSchema.optional(), installs: workerInstallsSchema.optional(), @@ -1401,7 +1215,6 @@ export const workerViewSchema = z.object({ }); export const statusLeaseSchema = leaseRecordSchema.extend({ - /** Which worker holds this lease; absent on a worker's own answer, set by a gateway. */ workerId: z.string().optional(), // why: the part of a gateway lease id after `.`, so a relayed event naming the worker's lease can be matched; set only by a gateway's `lease.list`, `lease.get` and `list.get kind=leases`. workerLeaseId: z.string().optional(), diff --git a/src/core/device-root.ts b/src/core/device-root.ts index 78650fc77..5ae9de56d 100644 --- a/src/core/device-root.ts +++ b/src/core/device-root.ts @@ -5,14 +5,8 @@ import type { Platform } from "./domain.js"; export const OWNED_ROOT_MARKER_FILE = ".simlock-owned.json"; -/** Owner-only. Group or other bits on a device root are grounds for refusing it. */ const ROOT_MODE = 0o700; -/** - * Why a root was refused. These strings are wire-visible: they travel in the - * `driver.root-rejected` event payload and are listed in `docs/internal/EVENTS.md`, so the - * vocabulary is fixed and a new one cannot be invented here alone. - */ export type RootRejectionReason = | "not-absolute" | "missing-marker" @@ -47,34 +41,20 @@ export class OwnedRootError extends Error { export interface EnsureOwnedRootOptions { readonly filesystem: Filesystem; - /** Names the staging directory a root is assembled in, so two racing daemons never share one. */ readonly idGenerator: IdGenerator; readonly instanceId: string; readonly path: string; /** why: staging directories carry the writer's pid so startup can tell a dead writer's leftover from a live one's. */ readonly pid?: number; readonly platform: Platform; - /** `process.getuid?.()`; `undefined` skips the ownership check on platforms without uids. */ readonly uid?: number; } type RootContext = Omit & { readonly root: string }; -/** - * Establishes that `path` is a device root this Simlock instance owns, creating it when - * nothing is there, and returns its resolved path. - * - * This is the whole of Simlock's structural ownership proof, and it is deliberately - * platform-agnostic: a driver supplies a path and nothing else. It fails closed in every - * ambiguous case -- there is no fallback to a default device location and no adoption of - * a directory Simlock did not create empty itself, because "empty right now" is not proof - * of "empty when marked" (ADR 0001, decision 2). Every refusal is an `OwnedRootError`, - * including one that comes out of an unexpected filesystem failure, because the caller - * skips a single platform on this error and stops the whole daemon on any other. - */ export async function ensureOwnedRoot(options: EnsureOwnedRootOptions): Promise { if (!isAbsolute(options.path)) { - // `resolve` would make one up: a relative path lands wherever the process that + // why: `resolve` would make one up: a relative path lands wherever the process that // launched the daemon happened to be standing, which for an auto-launched daemon is // some agent's working directory rather than a place anyone chose to put tens of // gigabytes of device data -- and in CP5 it is what `--purge-orphans` is aimed at. @@ -90,7 +70,7 @@ export async function ensureOwnedRoot(options: EnsureOwnedRootOptions): Promise< const context: RootContext = { ...options, root: resolve(options.path) }; if (await createRoot(context)) { - // A root is never trusted on the strength of "we think we just made it": the checks + // why: A root is never trusted on the strength of "we think we just made it": the checks // cost one `lstat`, and they close the window in which what was published is not what // is about to be used. await validateRootDirectory(context); @@ -102,10 +82,7 @@ export async function ensureOwnedRoot(options: EnsureOwnedRootOptions): Promise< } /** - * Creates the root with its marker, or reports `false` when something already occupies the - * path and validation has to decide about it instead. - * - * The root is assembled under a sibling staging name and published with a single `rename`, + * why: The root is assembled under a sibling staging name and published with a single `rename`, * so neither another daemon nor a crash can observe it half-made. Creating it in place * would publish an unmarked directory first: every later start would refuse that as * `non-empty-unowned-root` -- the temporary file `writeFileAtomic` leaves beside the marker @@ -132,7 +109,7 @@ async function createRoot(context: RootContext): Promise { const staging = stagingPath(root, context.idGenerator, context.pid); try { await filesystem.mkdir(staging, { mode: ROOT_MODE }); - // `mkdir`'s mode is a request the process umask can only subtract from: under a + // why: `mkdir`'s mode is a request the process umask can only subtract from: under a // restrictive one (`0o277`, say) the root comes back read-only to its own owner and // every device written into it afterwards fails. `chmod` is the only way to end at the // permissions validation demands of this root on the next start, and it cannot @@ -140,8 +117,6 @@ async function createRoot(context: RootContext): Promise { await filesystem.chmod(staging, ROOT_MODE); await filesystem.writeFileAtomic(markerPath(staging), serializeMarker(context)); } catch (error: unknown) { - // Nothing partial may survive anywhere near the real path, so the staging directory - // goes before the failure is reported. await discard(filesystem, staging); throw unexpected(context, "it could not be assembled", error); } @@ -151,9 +126,6 @@ async function createRoot(context: RootContext): Promise { } catch (error: unknown) { await discard(filesystem, staging); - // Something occupies the path now: another daemon published its root there first, or a - // directory of the user's appeared. Either way it is an existing root like any other, - // and validation -- which never creates -- is what decides about it. if (isOccupiedPathError(error)) { return false; } @@ -171,14 +143,14 @@ async function validateExistingRoot(context: RootContext): Promise { async function validateRootDirectory(context: RootContext): Promise { const { root, uid } = context; - // Only the root itself is read without following links. An *ancestor* symlink is + // why: Only the root itself is read without following links. An *ancestor* symlink is // ordinary -- `/tmp` is a link to `/private/tmp` on macOS, and every test home created // with `mkdtemp` sits under it -- so comparing `realpath(root)` against `root` would // fail closed on perfectly healthy machines. Do not "harden" this into a realpath check. const details = await pathDetails(context, root, "it"); if (details === undefined) { - // The root existed a moment ago (that is why creation stood down, or the rename that + // why: The root existed a moment ago (that is why creation stood down, or the rename that // published it succeeded) and is gone now. Creating it here would be the second // attempt this function promises never to make. throw rejected(context, "missing-marker", "it disappeared while it was being validated"); @@ -189,7 +161,7 @@ async function validateRootDirectory(context: RootContext): Promise { } if (details.kind !== "directory") { - // The reason vocabulary is fixed by the docs and has no separate "not a directory" + // why: The reason vocabulary is fixed by the docs and has no separate "not a directory" // member. This is the closest true statement it can make: something Simlock did not // create is sitting where the root belongs. throw rejected(context, "non-empty-unowned-root", "it is not a directory"); @@ -235,11 +207,6 @@ async function validateMarker(context: RootContext): Promise { } } -/** - * An unmarked root is refused either way; the two reasons differ only in what they tell - * the user. An empty one is most likely a path they created for Simlock by hand, and the - * fix is to remove it and let Simlock create it. A populated one holds someone's data. - */ async function missingMarkerRejection(context: RootContext): Promise { const entries = await attempt(context, "its contents could not be listed", () => context.filesystem.readdir(context.root), @@ -258,12 +225,11 @@ async function missingMarkerRejection(context: RootContext): Promise { - // A marker that cannot be read and one that cannot be parsed are the same answer: this + // why: A marker that cannot be read and one that cannot be parsed are the same answer: this // is not a root whose ownership Simlock can vouch for. This catch stays narrow on // purpose -- widening it to `unreadable` would demote a deliberate refusal to a // filesystem accident. @@ -278,7 +244,7 @@ async function readMarker( } /** - * The platform is part of the identity check, not just of the payload: swapping the iOS + * why: The platform is part of the identity check, not just of the payload: swapping the iOS * and Android roots in config would otherwise hand each driver the other's devices. */ function isOwnedRootMarker(value: unknown, platform: Platform): value is OwnedRootMarker { @@ -313,7 +279,7 @@ function markerPath(root: string): string { } /** - * Hidden, and unique to this attempt: two daemons racing to create one root must not land + * why: Hidden, and unique to this attempt: two daemons racing to create one root must not land * on the same staging directory, or the loser's cleanup would delete the winner's work * mid-flight. The uniqueness comes from the injected generator rather than an ambient * `randomUUID`, because this is core code and rule 9 admits no exception for entropy. @@ -323,7 +289,7 @@ function stagingPath(root: string, idGenerator: IdGenerator, pid: number | undef return join(dirname(root), `.${basename(root)}.${writer}${idGenerator.generate()}.staging`); } -/** Best effort: the failure that brought us here is the one worth reporting. */ +/** why: Best effort: the failure that brought us here is the one worth reporting. */ async function discard(filesystem: Filesystem, path: string): Promise { try { await filesystem.rm(path); @@ -334,7 +300,6 @@ async function discard(filesystem: Filesystem, path: string): Promise { } } -/** `undefined` for a path that is absent; anything else that goes wrong is `unreadable`. */ async function pathDetails( context: RootContext, path: string, @@ -364,7 +329,7 @@ async function attempt( } /** - * A filesystem failure the validator has no answer for is still a refused root, never a + * why: A filesystem failure the validator has no answer for is still a refused root, never a * raw errno escaping to the caller: the daemon skips one platform on an `OwnedRootError` * and dies on anything else, so a mistyped `deviceRoot` whose parent is a file (`ENOTDIR`) * would otherwise take down every driver instead of one (safety rule 9). @@ -395,7 +360,6 @@ function refusal( ); } -/** The errno matters as much as the message: it is what tells a user what to go and fix. */ function describeError(error: unknown): string { const message = error instanceof Error ? error.message : String(error); const code = errorCode(error); @@ -412,7 +376,7 @@ function isMissingPathError(error: unknown): boolean { } /** - * `rename` reports an occupied destination three ways depending on the kernel and on what + * why: `rename` reports an occupied destination three ways depending on the kernel and on what * is sitting there: a non-empty directory, a directory replaced by a file, or a plain * refusal to clobber. */ diff --git a/src/core/doctor.test.ts b/src/core/doctor.test.ts index ace7dbdbc..bb8993735 100644 --- a/src/core/doctor.test.ts +++ b/src/core/doctor.test.ts @@ -201,7 +201,7 @@ describe("Doctor", () => { mark: { durable: undefined, erasable: "tok", erasableReadable: true }, }, { detail: undefined, mark: { durable: "tok", erasable: "tok", erasableReadable: true } }, - // Unreadable is not absent: an Android mark is only reachable over adb while + // why: Unreadable is not absent: an Android mark is only reachable over adb while // the emulator runs, so a shut-down device must never read as erased. { detail: undefined, mark: { durable: "tok", erasable: undefined, erasableReadable: false } }, ] as const; @@ -264,9 +264,7 @@ describe("Doctor", () => { eventBus.replay().some((entry) => entry.event === "device.foreign-provenance-detected"), ).toBe(true); } - // A mark finding says nothing of the run state, which matches the registry's. expect(registry.snapshot.devices[0]?.foreignStateDetectedAt).toBeUndefined(); - // Report-only: --fix must never touch a device over a mark finding. expect(registry.snapshot.devices[0]?.state).toBe("ready"); } }); @@ -639,7 +637,7 @@ describe("Doctor", () => { eventBus, idGenerator: sequence(), }); - // A lease-path boot from `shutdown` runs `makeReady` while the committed record still + // why: A lease-path boot from `shutdown` runs `makeReady` while the committed record still // says `shutdown` -- observed reality goes `running` before that boot's own transition // commits. Claiming the device is what tells doctor this is in-flight work, not drift. const iosDevice = await shutdownDevice(registry, "simlock-ios-1", "ios"); @@ -844,7 +842,6 @@ describe("Doctor", () => { spec: { physical: false, model: "Phone", osVersion: "1", platform: "ios" }, }); - // No driver for the platform: nothing to measure against, so no stall. expect( isStalledTransition({ claims: undefined, @@ -857,9 +854,6 @@ describe("Doctor", () => { }); it("the doctor's stalled-transition finding is unchanged", async () => { - // The finding a stalled provisioning device and a stalled reclaiming device produced - // before the stall test became `isStalledTransition`, field for field, and none a moment - // before each threshold. const clock = new FakeClock(10_000); const eventBus = new EventBus(clock); const registry = await loadRegistry(clock, eventBus); @@ -926,7 +920,7 @@ describe("Doctor", () => { }); it("does not report a stall for a device this daemon holds an operation claim on", async () => { - // A backgrounded reclaim keeps its device in `reclaiming` for a full erase, which can + // why: A backgrounded reclaim keeps its device in `reclaiming` for a full erase, which can // outlast the threshold. The claim, not the clock, says work is in progress; without it a // healthy reclaim is reported as a stall and `--fix` quarantines it. const clock = new FakeClock(10_000); @@ -965,8 +959,6 @@ describe("Doctor", () => { [], ); - // Releasing the claim is what a crash cannot do: the same device, same age, is a - // stall once no live operation accounts for it. claim?.release(); const unclaimed = await new Doctor({ decisions: new SerializedDecision(), @@ -1001,7 +993,7 @@ describe("Doctor", () => { ttlDeadline: 999_999, }); await registry.beginRelease(lease.id); - // `kill -9` on the emulator leaves it absent from reality altogether, not `stopped`. + // why: `kill -9` on the emulator leaves it absent from reality altogether, not `stopped`. driver.setManagedReality({ devices: [], processes: [] }); clock.advance(6_001); @@ -1031,7 +1023,7 @@ describe("Doctor", () => { eventBus, idGenerator: sequence(), }); - // A record in `reclaiming` does not say which clean level started it, so the threshold + // why: A record in `reclaiming` does not say which clean level started it, so the threshold // has to clear the slower of the two: pricing it at `standard` would call a healthy // `full` reclaim a stall. const driver = new FakeDriver({ @@ -1136,8 +1128,6 @@ describe("Doctor", () => { provisionDuration: 0, spec: { physical: false, model: "Phone", osVersion: "1", platform: "ios" }, }); - // Reality still shows the device (never actually booted) so the fix pass isn't - // also chasing an unrelated registry-device-missing finding on the same device. driver.setManagedReality({ devices: [ { @@ -1556,7 +1546,7 @@ describe("Doctor", () => { }), ]); - // Post-commit: the registry mutations that resolved the drift are already committed + // why: Post-commit: the registry mutations that resolved the drift are already committed // by the time the fact is published, and doctor.reconciled is the final event. const commitEvents = events.filter( (event) => event.event === "device.ready" || event.event === "device.shutdown", @@ -1655,7 +1645,7 @@ describe("Doctor", () => { ); expect(report.findings.map((finding) => finding.kind)).toEqual(["driver-unavailable"]); expect(report.findings[0]).toMatchObject({ - // Discovery happens once, at startup, so the finding has to say what actually + // why: Discovery happens once, at startup, so the finding has to say what actually // retries the platform -- otherwise repairing the root and re-running `doctor` // reports the identical line and reads as a repair that did not work. detail: expect.stringContaining( @@ -1853,7 +1843,6 @@ describe("Doctor", () => { }; const RESTART = /restart the daemon/; - /** A check that reports `missing` and counts its runs. */ function prerequisiteCheck( platform: "ios" | "android", missing: readonly (typeof EMULATOR)[] = [], @@ -2108,7 +2097,7 @@ describe("Doctor with a platform it cannot observe", () => { registry, }).reconcile({ fix: true }); - // "I could not look" is not "the device is gone". Marking these `deleted` would strand + // why: "I could not look" is not "the device is gone". Marking these `deleted` would strand // every simulator in the root behind a registry with no record of it -- the permanent // multi-gigabyte leak ADR 0001 exists to prevent, reachable with a `chmod`. expect(report.findings.map((finding) => finding.kind)).toEqual(["driver-unavailable"]); @@ -2127,8 +2116,6 @@ describe("Doctor with a platform it cannot observe", () => { registry, }).reconcile({ fix: true }); - // Same reasoning with no rejection to report: a missing Android SDK or a non-Mac host - // is just as unobservable as a refused root. expect(report.findings).toEqual([]); expect(registry.snapshot.devices[0]?.state).toBe("ready"); }); @@ -2160,8 +2147,6 @@ describe("Doctor with a platform it cannot observe", () => { const purged = await doctor.reconcile({ purgeOrphans: true }); - // The process is the device's: destroying the device covers it, so reporting the - // process afterwards would name something that no longer exists. expect(purged.findings).toEqual([]); expect(eventBus.replay()).toContainEqual( expect.objectContaining({ @@ -2218,7 +2203,7 @@ describe("Doctor with a platform it cannot observe", () => { registry, }).reconcile({ purgeOrphans: true }); - // A daemon that cannot prove one of its roots is not one to keep destroying on, and + // why: A daemon that cannot prove one of its roots is not one to keep destroying on, and // the roots are proven before anything is destroyed -- so this costs a re-run, never a // half-purge. expect( @@ -2343,7 +2328,7 @@ describe("Doctor with a platform it cannot observe", () => { registry, }).reconcile({ fix: true }); - // "I could not look outside the root" is not "it is out there": the conservative + // why: "I could not look outside the root" is not "it is out there": the conservative // finding is the one whose fix only writes to the registry. expect(report.findings.map((finding) => finding.kind)).toEqual(["registry-device-missing"]); expect(driver.calls.filter((call) => call.operation === "destroyLegacy")).toEqual([]); @@ -2573,7 +2558,6 @@ function observed(deviceId: string, runState: "running" | "stopped") { return { ...driverDevice(deviceId), runState }; } -/** One `ready` iOS device in an otherwise empty registry. */ async function readyIosDevice() { const clock = new FakeClock(10_000); const eventBus = new EventBus(clock); diff --git a/src/core/doctor.ts b/src/core/doctor.ts index e9a2a1dc6..6dd0cd734 100644 --- a/src/core/doctor.ts +++ b/src/core/doctor.ts @@ -54,17 +54,12 @@ export type DoctorFinding = readonly detail: ProvenanceDrift; } | { - /** A platform Simlock is running without, because its driver refused to start. */ readonly kind: "driver-unavailable"; readonly platform: Platform; readonly reason: DriverRejectionReason; readonly detail: string; } | { - /** - * A registry device that outlived the move to owned roots: absent from this - * driver's root, still present in the location the platform used before it. - */ readonly kind: "legacy-device"; readonly deviceId: string; readonly platform: Platform; @@ -81,27 +76,12 @@ export type DoctorFinding = readonly thresholdMs: number; } | { - /** - * Configuration-level information from a single driver's `advisories()`, not drift: - * nothing here describes a divergence between the registry and observed reality, so - * `--fix` never acts on it (see `#applySafeFixes`) and it is excluded from the - * `doctor.reconciled` event's `driftFindings` payload (see the comment at that emit - * call) -- it stays in `DoctorReport.findings` (what `doctor.run` returns to a caller) - * only. - */ readonly kind: "driver-advisory"; readonly platform: Platform; readonly code: string; readonly message: string; } | { - /** - * A tool the platform needs and Simlock does not install, from that platform's - * `PrerequisiteCheck`. `prerequisite`, `message` and `remedy` are the driver module's own - * text, carried unread -- except `daemon-restart`, the one this file adds itself. Like - * `driver-advisory` it is not drift: `--fix` never acts on it and `doctor.reconciled` - * does not carry it. - */ readonly kind: "prerequisite-missing"; readonly platform: Platform; readonly prerequisite: string; @@ -162,7 +142,7 @@ export interface DoctorOptions { readonly clock: Clock; readonly config: Config; /** - * The decision section every registry write of this daemon is made in. A write made outside it + * why: The decision section every registry write of this daemon is made in. A write made outside it * can land on top of a concurrent one that read the registry before it, and undo it. */ readonly decisions: Pick; @@ -186,24 +166,12 @@ export interface DoctorOptions { export interface DoctorReconcileOptions { readonly fix?: boolean; - /** - * The startup read to reconcile against instead of listing every driver again. A platform it - * leaves out has no reality to compare, so its devices get no missing-device, orphan or other - * drift-against-reality finding. A `stalled-transition` finding is the exception: it reads the - * registry and the platform's driver, not the listing, so a platform that has a driver but - * whose listing failed still gets it. - */ readonly read?: StartupRead; /** - * Destroys the orphans this run finds. Safety rule 1's single opt-in exception, and - * deliberately not part of `fix`: someone already running `doctor --fix` unattended in + * why: deliberately not part of `fix`: someone already running `doctor --fix` unattended in * CI must not acquire a destructive behaviour by upgrading (ADR 0001, decision 6). */ readonly purgeOrphans?: boolean; - /** - * Runs every `PrerequisiteCheck`. Off by default so startup convergence starts no tool; - * `doctor.run` turns it on, so the machine is looked at again on every run. - */ readonly prerequisites?: boolean; } @@ -222,7 +190,7 @@ export class Doctor { }: DoctorReconcileOptions = {}): Promise { const snapshot = this.options.registry.snapshot; const realities = await this.#realities(read); - // A platform with no driver has no observable reality: its driver refused to start, + // why: A platform with no driver has no observable reality: its driver refused to start, // its SDK is missing, or this host has none. The same holds for a platform the startup // read could not list. "I could not look" is not "the device is // gone", and reading it as such is destructive -- every registry device of that @@ -269,7 +237,7 @@ export class Doctor { if (fix) { await this.#applySafeFixes(findings, driversByPlatform); } - // Prerequisites last: a check may run for tens of seconds, and nothing `--fix` or a purge + // why: Prerequisites last: a check may run for tens of seconds, and nothing `--fix` or a purge // acts on should wait behind it and go stale. They are never acted on themselves. const remaining = [ ...(purgeOrphans ? await this.#purgeOrphans(findings, driversByPlatform) : findings), @@ -293,7 +261,6 @@ export class Doctor { return report; } - /** Each driver's reality: the startup read's, or a fresh `listManaged` when there is none. */ async #realities( read: StartupRead | undefined, ): Promise { @@ -312,10 +279,7 @@ export class Doctor { } /** - * Destroys every orphan this run found, and reports back the findings that outlived the - * attempt. - * - * This is the one place Simlock destroys something the registry has never heard of, and + * why: This is the one place Simlock destroys something the registry has never heard of, and * the residual risk is accepted rather than mitigated: an orphan has no registry record, * so no central safety filter and no `DeviceOperationClaims` entry can protect it -- * including a device this very daemon is between `driver.provision` and @@ -365,7 +329,7 @@ export class Doctor { ); } - // An `orphan-process` for a device that is now gone is gone with it: destroying the + // why: An `orphan-process` for a device that is now gone is gone with it: destroying the // device covers the process it was running, so reporting both would ask the operator // to deal with something that no longer exists. return findings.filter( @@ -378,9 +342,7 @@ export class Doctor { } /** - * Re-proves each root the purge is about to reach into, before its first destroy. - * - * The proof a purge acts on is the one taken at startup, and the thing it authorises is + * why: The proof a purge acts on is the one taken at startup, and the thing it authorises is * recomputed on every reconcile: a root replaced by a symlink, or a `mv` that leaves the * user's own device set at the configured path, turns `listManaged` into a claim over * their simulators and this command into the thing that deletes them. Reporting can live @@ -416,9 +378,9 @@ export class Doctor { } /** - * Registry versus root, per device: existence, run state and provenance. Drift compares the + * why: Registry versus root, per device: existence, run state and provenance. Drift compares the * registry with what a root holds, so it reads virtual devices only: a physical record is in - * no root and would read as missing (ADR 0022 §4). + * no root and would read as missing. */ async #driftFindings( devices: readonly VirtualDevice[], @@ -474,12 +436,7 @@ export class Doctor { } /** - * Rewrites a `registry-device-missing` finding as `legacy-device` when the driver still - * finds the device where the platform kept it before roots existed. Absent from the root - * is what both findings have in common; where the device actually is decides which one it - * is, and only the driver can look there. - * - * A driver that cannot answer -- no legacy location, or a lookup that failed -- leaves + * why: A driver that cannot answer -- no legacy location, or a lookup that failed -- leaves * the original finding alone. `registry-device-missing` is the conservative of the two: * its fix only writes to the registry, while the legacy fix destroys. */ @@ -519,13 +476,6 @@ export class Doctor { ); } - /** - * `advisories()` is optional, read-only, best-effort diagnostic information a single driver - * reports about its own configuration -- unlike `listManaged` (whose failure today aborts the - * whole reconcile via the unhandled rejection in the `Promise.all` above; widening that - * tolerance is out of scope here), a driver with no `advisories()` or one that rejects simply - * contributes nothing rather than failing every other driver's findings along with it. - */ async #collectAdvisories(): Promise { const perDriver = await Promise.all( this.options.drivers.map(async (driver): Promise => { @@ -549,11 +499,7 @@ export class Doctor { } /** - * Every platform's prerequisite check, run now: a package installed since the last run is - * no longer reported, with no restart. A check that rejects could not tell, so it is logged - * and contributes nothing, as a rejecting `advisories()` does. - * - * Driver discovery, though, ran once at startup. So a platform that is not running gets the + * why: Driver discovery, though, ran once at startup. So a platform that is not running gets the * restart sentence on each finding, and one whose prerequisites are all present gets a * finding of its own that says so -- unless its driver was refused, which `driver-unavailable` * already explains. @@ -687,7 +633,7 @@ export class Doctor { ); } } - // The machine's reserve cannot hold more slots than the machine has: a figure past the limit + // why: The machine's reserve cannot hold more slots than the machine has: a figure past the limit // would print a room below zero, and with no target there is nothing to report. const reserved = Math.min(reserves.ios + reserves.android, limits.global.maxRunning); const room = limits.global.maxRunning - reserved; @@ -771,7 +717,6 @@ export class Doctor { break; case "orphan-device": case "orphan-process": - // Registry-only destruction: unregistered reality is report-only. break; case "expired-live-lease": if (this.options.leaseExpirer !== undefined) { @@ -782,10 +727,10 @@ export class Doctor { await this.options.decisions.run(() => this.#fixForeignStateChange(finding)); break; case "foreign-provenance-change": - // Report-only: re-marking destroys the evidence, and the device may be leased. + // why: Report-only: re-marking destroys the evidence, and the device may be leased. break; case "driver-unavailable": - // Nothing here can repair a refused root or an occupied port without doing the + // why: Nothing here can repair a refused root or an occupied port without doing the // one thing failing closed exists to prevent: adopting what it refused. break; case "legacy-device": @@ -795,10 +740,8 @@ export class Doctor { await this.#fixStalledTransition(finding); break; case "driver-advisory": - // Information, not drift: `--fix` never acts on a driver advisory. break; case "prerequisite-missing": - // Simlock installs no prerequisite (safety rule 4), `--fix` included. break; case "service-broken": // why: installing again is the operator's call: `--fix` never rewrites the service. @@ -815,7 +758,7 @@ export class Doctor { } /** - * The destroy reaches outside the driver's root, which every other path in Simlock is + * why: The destroy reaches outside the driver's root, which every other path in Simlock is * forbidden to do -- and is permitted here for one reason: the device is in the registry. * Registry-only destruction (safety rule 1) is a rule about *what may be destroyed*, and * a registry record satisfies it exactly as a root membership would; the record is also @@ -860,7 +803,7 @@ export class Doctor { if (device === undefined) { return; } - // Only `ready` and `shutdown` have a legal transition to the opposite run state. A + // why: Only `ready` and `shutdown` have a legal transition to the opposite run state. A // device recorded as `leased` with no lease record reaches here past the lease guard // above; correcting it would throw IllegalTransition and abort the whole reconcile, // so leave it to the report rather than crashing the reaper tick. @@ -872,7 +815,7 @@ export class Doctor { } else if ( device.state === "shutdown" && finding.observed === "running" && - // A spent fresh device stays `shutdown`, running or not: every path that deletes it + // why: A spent fresh device stays `shutdown`, running or not: every path that deletes it // selects `shutdown`, and the delete stops a running simulator first. Moving it to // `ready` would strand it as ungrantable warm inventory that nothing deletes. mayBeGranted(device) @@ -890,7 +833,7 @@ export class Doctor { async #fixStalledTransition( finding: Extract, ): Promise { - // A device this finding targets is never `leased` -- that's a separate state + // why: A device this finding targets is never `leased` -- that's a separate state // entirely -- but the guard is kept anyway, matching every other fix here, so a // future change to what produces this finding can never reach a leased device. const snapshot = this.options.registry.snapshot; @@ -950,12 +893,6 @@ function missingDeviceFinding( return { deviceId: device.id, kind: "registry-device-missing", platform: device.spec.platform }; } -/** - * Run-state and provenance drift for a device whose registry state expects a definite run - * state and that has an observed-reality match. Split out of `registryDriftFindings` so the - * mid-transition/claim guard there is one flat condition rather than threading through this - * logic's own branches. - */ function liveDriftFindings( device: DeviceRecord, expected: "running" | "stopped", @@ -987,8 +924,7 @@ function liveDriftFindings( } /** - * The reclaim estimate for the slowest strategy the driver would pick, whatever the clean - * level. A `reclaiming` record does not say which level started it -- every production path + * why: A `reclaiming` record does not say which level started it -- every production path * asks for `standard`, but that is the caller's choice, not an invariant this finding can * depend on -- and the two errors are not symmetric: an estimate that is too tight turns a * healthy reclaim into a false stall finding, while one that is too loose only delays a real @@ -1002,7 +938,7 @@ function slowestReclaimEstimateMs(driver: Pick, spec: Device } /** - * A `provisioning` / `reclaiming` device is normally in-flight work Simlock itself is + * why: A `provisioning` / `reclaiming` device is normally in-flight work Simlock itself is * driving (see `expectedRunState`), not drift -- but only up to a point. Past a * driver-derived threshold it stops being "still working" and becomes a stall: the * driver call that was supposed to resolve it never did. This is a documented failure @@ -1030,10 +966,6 @@ function slowestReclaimEstimateMs(driver: Pick, spec: Device * contended disk speed would still be guessing; the claim answers it exactly. A reclaim * orphaned by a crash has no claim in the new process, so the case this test exists to * catch is untouched. - * - * The one place the stall rule lives (architecture rule 10): `doctor`'s - * `stalled-transition` finding and the `stalled` flag `status.get` and `list.get` put on a - * device both ask it. */ export function isStalledTransition(input: StallInput): boolean { const { claims, config, device, driver, now } = input; @@ -1050,8 +982,6 @@ export function isStalledTransition(input: StallInput): boolean { return now - enteredAt > stallThresholdMs(device, driver, config); } -/** What `isStalledTransition` reads: one device, the driver for its platform (absent when that - * platform has none), the `stalledTransition` config, the operation claims and the time. */ export interface StallInput { readonly device: DeviceRecord; readonly driver: Pick | undefined; @@ -1060,7 +990,6 @@ export interface StallInput { readonly now: number; } -/** How long `device` may stay in its transition before it is a stall. */ function stallThresholdMs( device: DeviceRecord, driver: Pick, @@ -1074,7 +1003,6 @@ function stallThresholdMs( return Math.max(estimateMs * config.thresholdMultiplier, config.minimumThresholdMs); } -/** The `stalled-transition` finding for a device `isStalledTransition` holds for. */ function stalledTransitionFinding( device: DeviceRecord, driversByPlatform: ReadonlyMap, @@ -1086,8 +1014,6 @@ function stalledTransitionFinding( if (!isStalledTransition({ claims, config, device, driver, now })) { return undefined; } - // Narrowing only: `isStalledTransition` holds just for a device in one of these two states, - // with an entry time and a driver. const enteredAt = transitionEnteredAt(device); if ( driver === undefined || @@ -1108,7 +1034,7 @@ function stalledTransitionFinding( } /** - * Report-only by design. Re-marking would overwrite the only evidence that the + * why: Report-only by design. Re-marking would overwrite the only evidence that the * device was tampered with, and a fresh mark cannot be written to a leased * device without disturbing its holder -- so the core never repairs a mark. */ @@ -1126,7 +1052,7 @@ function provenanceDrift(mark: ObservedMark): ProvenanceDrift | undefined { } /** - * Discovery runs once, at daemon start, so a refusal is a frozen snapshot: repairing the + * why: Discovery runs once, at daemon start, so a refusal is a frozen snapshot: repairing the * root and running `doctor` again reports the identical finding, because nothing has * looked at the root since. The finding therefore has to name the step that actually * retries the platform, or it reads as a repair that did not work. @@ -1146,7 +1072,6 @@ function driverUnavailableFindings(rejections: readonly DriverRejection[]): Doct const PLATFORM_NAMES: Readonly> = { android: "Android", ios: "iOS" }; -/** Whether an OS constraint is a range rather than one version. */ function isRange(osVersion: string): boolean { const parsed = parseOsConstraint(osVersion); return parsed.ok && parsed.constraint.kind === "range"; @@ -1161,7 +1086,6 @@ function unreachableTarget( const named = [model, osVersion, target.mode].filter((part) => part !== undefined).join(" / "); const name = PLATFORM_NAMES[platform]; if (short === "runtime-missing") { - // A range names no version to install: `component install` takes one catalog version. if (osVersion !== undefined && isRange(osVersion)) { return [ { @@ -1221,14 +1145,12 @@ function unreachableTarget( return []; } -/** Targets whose counts add up to more than the running limit leaves for them. */ function overLimit(input: { readonly target: string; readonly sum: number; readonly limit: number; readonly reserve: number; readonly room: number; - /** `ios ` or `android `, or empty for the machine. */ readonly scope: string; readonly platform?: Platform; }): DoctorFinding { @@ -1244,7 +1166,6 @@ function overLimit(input: { }; } -/** Whether a finding belongs in `doctor.reconciled`'s `driftFindings` -- see the emit call. */ function isDrift(finding: DoctorFinding): boolean { return ( finding.kind !== "driver-advisory" && @@ -1255,7 +1176,6 @@ function isDrift(finding: DoctorFinding): boolean { ); } -/** One platform's check result, as findings -- see `Doctor#collectPrerequisites`. */ function prerequisiteFindings( platform: Platform, missing: readonly MissingPrerequisite[], diff --git a/src/core/driver.ts b/src/core/driver.ts index aee6e0abb..022b6198e 100644 --- a/src/core/driver.ts +++ b/src/core/driver.ts @@ -90,7 +90,7 @@ export function virtualDriverDevice(device: DriverDevice): VirtualDriverDevice { return { ...device, physical: false }; } -/** Always writes `mode`, defaulting `undefined` to `"full"` here and nowhere else, so a stale `"slim"` cannot outlive a re-boot that did not slim. */ +/** why: Always writes `mode`, defaulting `undefined` to `"full"` here and nowhere else, so a stale `"slim"` cannot outlive a re-boot that did not slim. */ export function readyTransitionUpdate(readyDevice: DriverDevice): DeviceTransitionUpdate { return { address: readyDevice.address, @@ -100,7 +100,7 @@ export function readyTransitionUpdate(readyDevice: DriverDevice): DeviceTransiti } /** - * Observed boot state of a managed device. `transitioning` covers states a + * why: Observed boot state of a managed device. `transitioning` covers states a * driver cannot settle into `running` or `stopped` yet -- simulators report * `Booting` / `Shutting Down`, emulators appear `offline` in `adb devices` * before they answer `getprop` -- and must never produce a drift finding. @@ -108,7 +108,7 @@ export function readyTransitionUpdate(readyDevice: DriverDevice): DeviceTransiti export type ObservedRunState = "running" | "stopped" | "transitioning"; /** - * Provenance-mark readings for one managed device. Simlock writes the same + * why: Provenance-mark readings for one managed device. Simlock writes the same * token into two regions of a device it owns: one that survives a fresh-state * erase and one the erase destroys. Comparing the pair is what makes a foreign * erase visible -- an erased device still exists and still boots, so run-state @@ -117,11 +117,9 @@ export type ObservedRunState = "running" | "stopped" | "transitioning"; * Where each region lives is the driver's business; the core only compares. */ export interface ObservedMark { - /** Token in the region that survives an erase, or undefined when absent. */ readonly durable: string | undefined; - /** Token in the region an erase destroys, or undefined when absent. */ readonly erasable: string | undefined; - /** Unreadable is not absent and must never be reported as an erase. */ + /** why: Unreadable is not absent and must never be reported as an erase. */ readonly erasableReadable: boolean; } @@ -144,7 +142,7 @@ export interface ReclaimResult { export type ReclaimStrategy = ReclaimResult["strategy"]; /** - * `reclaim` carries the clean level because the strategies differ by an order of magnitude + * why: `reclaim` carries the clean level because the strategies differ by an order of magnitude * (iOS `erase` tens of seconds, Android `snapshot` restore a few); one blended number is wrong for both. */ export type DriverEstimate = @@ -168,44 +166,25 @@ export interface DriverCatalogEntry { readonly customModels?: readonly string[]; } -/** - * One installed image. `runtime` is a value from the entry's `runtimes`; `tag` and `abi` are - * the driver's own words for the image's variant and instruction set, carried unread. - */ export interface DriverCatalogImage { readonly runtime: string; readonly tag: string; readonly abi: string; } -/** - * A configuration-level problem only the owning driver can see -- reported by `doctor` - * alongside its own drift findings (`src/core/doctor.ts`'s `driver-advisory` finding kind). - * Unlike drift, this is never something `--fix` acts on: it describes a standing condition of - * the driver's own configuration (e.g. a feature silently doing nothing given the installed - * runtimes), not a divergence between the registry and reality. - */ export interface DriverAdvisory { - /** Short kebab-case identifier the driver owns; the core never interprets it. */ readonly code: string; readonly message: string; } -/** - * A tool Simlock needs and does not install, found missing by a `PrerequisiteCheck`. Every - * field is the driver module's own text; the core carries all three unread. - */ export interface MissingPrerequisite { - /** Short kebab-case identifier the driver module owns. */ readonly prerequisite: string; - /** What is missing. */ readonly message: string; - /** The command or step that installs it. */ readonly remedy: string; } /** - * Looks for one platform's prerequisites on this machine. Deliberately not a `Driver` method: + * why: Looks for one platform's prerequisites on this machine. Deliberately not a `Driver` method: * it has to answer exactly when the driver could not be built, because a prerequisite is * missing. Read-only -- it never installs, downloads or writes -- and every process it starts * is bounded. It rejects when it cannot tell, rather than reporting something missing. @@ -250,18 +229,9 @@ export type PhysicalInspection = export interface Driver { readonly platform: Platform; readonly defaultModels: Readonly>>; - /** - * Absolute path of the root this driver owns and scopes every platform command to. - * Membership in it is what proves a device is Simlock's; the core carries the string - * around without interpreting it, the same way it carries `address`. - */ readonly deviceRoot: string; /** - * Re-runs this driver's own root validation, resolving only while the root still proves - * ownership and rejecting with the driver's own refusal (an `OwnedRootError`) when it - * does not. - * - * Ownership is proven once, at startup, and then trusted for the life of the process -- + * why: Ownership is proven once, at startup, and then trusted for the life of the process -- * tolerable for reporting, not for destroying. A daemon that has been up for days is one * `mv` or one symlink away from `deviceRoot` naming the user's own device set, at which * point `listManaged` answers with every simulator on the machine and @@ -269,30 +239,8 @@ export interface Driver { * because only it knows how its root was built (architecture rule 2). */ revalidateRoot(): Promise; - /** - * Resolves a request into the spec a device is planned on. `request.mode` is always set here, - * resolved by the core. A driver returns a spec with `mode: "slim"` only for a slim request it - * will actually slim; for any other request, and for a slim one it cannot slim, it returns a - * full spec (ADR 0007 §4). A driver that does not slim at all never sets the spec's mode. - */ - /* - * Never downloads (ADR 0010 §2). When a download could satisfy the request it throws - * `RuntimeMissingError` with `downloadable: true` and `component`, the string to hand to - * `installComponent` -- the core never decides whether to download from inside a driver. - */ resolveSpec(request: ExactDeviceRequest): Promise; - /** - * A read: the installed component that satisfies `component`, with its exact version and its - * receipt, or `undefined`. Never downloads. A string that is not a version, such as the - * driver's word for "newest", answers `undefined`: only an installer run can tell. - */ findComponent(component: string): Promise; - /** - * Runs the platform installer for `component` and verifies the result against a fresh read. - * `installed` when the receipt it ends with was not there before the run, `already-installed` - * when it was. Ends the installer when `signal` fires. Joins no callers, checks no disk, emits - * no events and reads no policy: `ComponentInstaller` is its only caller and does all four. - */ installComponent( component: string, options: { @@ -300,34 +248,19 @@ export interface Driver { readonly signal: AbortSignal; }, ): Promise; - /** - * The disk one install needs, as a fixed estimate, and the path it lands on. Neither platform - * installer reports a size before it downloads, so this is what `ComponentInstaller` reserves. - */ readonly componentFootprint: { readonly path: string; readonly bytes: number }; /** - * Every component installed on the machine now, one entry each (ADR 0010 §8). Read-only, never - * downloads, and every process it starts is bounded: the same contract as `listCatalog`. - * Counting `foreignDevices` is the only place a driver looks at devices outside its own root; + * why: Counting `foreignDevices` is the only place a driver looks at devices outside its own root; * it reads them and never writes there (safety rule 1). */ listComponents(): Promise; - /** - * Removes the installed component whose receipt equals `receipt` (ADR 0010 §8), through - * `removeListedComponent`: it proves the receipt against a fresh listing - * (`ComponentNotOwnedError` when nothing matches), refuses when a foreign device uses it - * (`ComponentInUseError`), runs the platform's own removal bounded by - * `COMPONENT_REMOVAL_TIMEOUT_MS` and ended by `signal`, and lists again to verify it is gone. - * `ComponentInstaller` is its only caller, and calls it only for a component it has a record - * of and no Simlock device uses. - */ removeComponent( receipt: ComponentReceipt, options: { readonly signal: AbortSignal }, ): Promise; provision(spec: VirtualSpec): Promise; /** - * Boots the device and returns it with a freshly read `address`. Never trust the address a + * why: Boots the device and returns it with a freshly read `address`. Never trust the address a * caller passed in or one captured at `provision` -- an Android console port is assigned per * boot, so a device coming back from `shutdown` (or a driver restart) can land on a different * one. `deviceId` and the registry-relevant parts of `driverData` do not change. @@ -335,9 +268,7 @@ export interface Driver { makeReady( device: VirtualDriverDevice, options: { - /** `"recover"` reboots a still-`leased` device: boot only, never change its configuration. */ readonly purpose: "prepare" | "recover"; - /** The registry is the only record of the mode (ADR 0007 §7); a driver keeps no copy. */ readonly mode: DeviceMode; }, ): Promise; @@ -354,7 +285,6 @@ export interface Driver { listManaged(): Promise; inspectPhysical(id: string): Promise; listPresentPhysical(enrolledIds: readonly string[]): Promise; - /** Read-only: must never trigger a runtime / system-image download. */ listCatalog(): Promise; estimate( estimate: DriverEstimate & { readonly operation: "boot"; readonly device: DriverDevice }, @@ -365,9 +295,9 @@ export interface Driver { device: VirtualDriverDevice | PhysicalDriverDevice, ): Readonly>; virtualLeaseEnvironment(): Readonly>; - /** Android must reap the adb server it supervises by pid; nothing else can. */ + /** why: Android must reap the adb server it supervises by pid; nothing else can. */ dispose?(): Promise; - /** Spelled `| undefined` so a driver that decides at construction can still say it has none. */ + /** why: Spelled `| undefined` so a driver that decides at construction can still say it has none. */ readonly passthroughTools?: readonly string[] | undefined; // why: optional because a platform whose screen cannot be taken leaves it out, and the stream // module learns that from its absence rather than from a platform name. @@ -379,47 +309,14 @@ export interface Driver { args: readonly string[], context?: PassthroughContext, ): Promise; - /** - * Looks for a registry device in the location this platform used before Simlock owned a - * root, and reports it without touching it. Optional: a driver with no pre-root history - * has nothing to find. The core only ever asks about a device the root itself no longer - * holds, so a "yes" here is what separates "stranded by the migration" from "gone". - */ findLegacy?(driverDeviceId: string): Promise; - /** - * Destroys such a device through the old, unscoped path it actually lives on. This is - * the only Simlock call that reaches outside an owned root, and it is permitted because - * the device is in the registry: registry-only destruction (safety rule 1) is satisfied - * by the record, not by the root (ADR 0001, Migration). - */ destroyLegacy?(device: VirtualDriverDevice): Promise; - /** - * Configuration-level problems only this driver can see -- reported by `doctor` alongside its - * drift findings. Read-only and side-effect free (same contract as `listCatalog`): it must - * never trigger a download, boot, or mutate anything. Optional: a driver with nothing to - * advise omits it. - */ advisories?(): Promise; - /** - * The versions of the platform tools this driver uses, for `status.get`'s host facts. - * Read-only and never downloads, like `advisories`. A tool that is not installed is left out. - * A read that fails -- a tool that is there but errors, times out, or answers in a shape the - * driver does not know -- rejects, and the core keeps what this driver reported last - * (`HostFactsReader`). Every process it starts is bounded. Optional: a driver with no tools to - * report omits it. - */ toolVersions?(): Promise; } -/** - * A device this driver created before Simlock owned a root, still sitting where the - * platform put it. Neither CoreSimulator nor the Android SDK can relocate a device, so - * these are reported and destroyed rather than migrated, and users re-provision. - */ export interface LegacyDevice { - /** Addressed through the driver's unscoped path, never through the root's. */ readonly device: DriverDevice; - /** Where the driver found it, for the report. Absent when the tool does not say. */ readonly path?: string; } @@ -445,7 +342,7 @@ export interface PassthroughContext { } /** - * A passthrough verb a driver refuses to proxy. The wrappers exist to inject scoping + * why: The wrappers exist to inject scoping * flags, and injecting them into `simctl delete` would hand back exactly the capability * the containment root exists to remove -- so the refusal, and the message naming what to * run instead, live with the driver that knows which verbs those are. @@ -460,31 +357,19 @@ export class PassthroughRefusedError extends Error { } } -/** The events a driver may refuse to start with; each pairs with its own payload below. */ type DriverRejectionEvent = "driver.root-rejected" | "driver.adb-server-rejected"; -/** - * Why Simlock's own adb server could not be established. Wire-visible, like the root - * reasons: these travel in the `driver.adb-server-rejected` payload and are listed in - * `docs/internal/EVENTS.md`, so the vocabulary is fixed and closed. - * - * It sits beside the event names rather than in `drivers/android` because the two are one - * contract: the core publishes neither without the other, and a driver module cannot be - * the place a core type is defined. Nothing here interprets a term (architecture rule 2). - */ export type AdbServerRejectionReason = "occupied" | "start-failed" | "invalid-port"; /** - * Every term a refusal may report. Typed rather than a bare `string` so a reason that no + * why: Typed rather than a bare `string` so a reason that no * documented vocabulary contains cannot reach `doctor` output or the bus: the whole value * of publishing these words is that a user who reads one can look it up. */ export type DriverRejectionReason = RootRejectionReason | AdbServerRejectionReason; /** - * One refusal, with the payload the event it names is published with. - * - * The pairing is the point. The core forwards `payload` to the bus without reading it, so + * why: The pairing is the point. The core forwards `payload` to the bus without reading it, so * this is the only place the wire contract in `docs/internal/EVENTS.md` can still be checked -- a * wider `Record` would type-check an adb rejection carrying a * string port, or a root rejection with no root at all, and it would reach @@ -495,7 +380,7 @@ interface DriverRefusal { readonly event: Event; readonly payload: EventMap[Event]; /** - * The refusal's vocabulary term (`missing-marker`, `occupied`, ...), stated separately + * why: The refusal's vocabulary term (`missing-marker`, `occupied`, ...), stated separately * rather than read back out of `payload`, which is a wire contract the core does not * open. `doctor` reports it as the failing reason. */ @@ -506,12 +391,11 @@ interface DriverRefusal { * only compares it to the requested tool name; the name itself is the driver's. */ readonly passthroughTools?: readonly string[]; - /** One line, for `doctor` output and the startup log. */ readonly summary: string; } /** - * Why a driver could not start. A driver that refuses to start fails closed and takes only + * why: A driver that refuses to start fails closed and takes only * its own platform with it (safety rule 9), so the daemon has to be able to report the * refusal without understanding it: the driver module names the event and builds the * payload, and the core carries both to the bus and to `doctor` unread. @@ -520,14 +404,8 @@ export type DriverRejection = | DriverRefusal<"driver.root-rejected"> | DriverRefusal<"driver.adb-server-rejected">; -/** - * What names an installed component itself, opaque to the core: it compares receipts for - * equality and reads nothing in them (ADR 0010 §5). It differs for every install, so a component - * the user deleted and installed again does not match a record of Simlock's earlier install. - */ export type ComponentReceipt = Readonly>; -/** Whether two receipts name the same installed thing: the same keys with the same values. */ export function sameReceipt(left: ComponentReceipt, right: ComponentReceipt): boolean { const keys = Object.keys(left); return ( @@ -536,63 +414,28 @@ export function sameReceipt(left: ComponentReceipt, right: ComponentReceipt): bo ); } -/** An installed component: the exact version the catalog lists, and its receipt. */ export interface InstalledComponent { readonly version: string; readonly receipt: ComponentReceipt; } -/** - * One component `listComponents` found installed. `receipt` is built by the same function - * `findComponent` and `installComponent` use, so a listed component and the record of its install - * compare equal with `sameReceipt`. `variant` is the driver's own words for what tells two - * components of one version apart, carried unread. `sizeBytes` is absent when the driver could - * not read it. `foreignDevices` is the number of devices outside Simlock's root that count as - * users of it; the driver decides what counts as a user, and a removal is refused while it is - * above zero. - */ export interface DriverComponent extends InstalledComponent { readonly variant?: string; readonly sizeBytes?: number; readonly foreignDevices: number; } -/** - * What one `installComponent` run ended with. `installed` when the receipt was not there before - * the run; `already-installed` when it was, so Simlock never records a component it did not put - * there. - */ export interface ComponentInstallResult extends InstalledComponent { readonly outcome: "installed" | "already-installed"; } -/** - * What one `removeComponent` freed and left. `sizeBytes` is the component's size as listed just - * before it was removed, absent when the driver could not read it. `residue` is one sentence or - * more for the operator when something stays on disk after the removal, and how to reclaim it. - */ export interface ComponentRemoval { readonly sizeBytes?: number; readonly residue?: string; } -/** - * How long one platform removal (`simctl runtime delete`, `sdkmanager --uninstall`) may run, - * measured from its start. A driver that waits after the tool answers spends the same budget. - */ export const COMPONENT_REMOVAL_TIMEOUT_MS = 5 * 60_000; -/** - * The one shape of `Driver.removeComponent`, for every driver alike (ADR 0010 §8): - * - * 1. find the listed component whose receipt equals `receipt`; none is `ComponentNotOwnedError`; - * 2. a foreign device that uses it is `ComponentInUseError`; - * 3. `remove` it, the driver's own platform step; - * 4. list again, and fail when a component with that receipt is still there. - * - * `list` is the driver's listing, the same one `listComponents` answers from, so the "in use" - * answer here and in `component.list` cannot differ. A listing that rejects rejects the removal. - */ export async function removeListedComponent(options: { readonly platform: Platform; readonly receipt: ComponentReceipt; @@ -626,10 +469,6 @@ export async function removeListedComponent(opti }; } -/** - * A removal Simlock refuses because the component is not Simlock's (ADR 0010 §5): there is no - * record of installing it, or what is installed now is not what the record names. - */ export class ComponentNotOwnedError extends Error { constructor( readonly platform: Platform, @@ -640,10 +479,6 @@ export class ComponentNotOwnedError extends Error { } } -/** - * A removal Simlock refuses because a device uses the component (ADR 0010 §8): `devices` of - * Simlock's own, in any state but deleted, and `foreignDevices` outside Simlock. - */ export class ComponentInUseError extends Error { readonly devices: number; readonly foreignDevices: number; @@ -665,7 +500,6 @@ export class ComponentInUseError extends Error { } } -/** A removal refused because an install, or another removal, is running or waiting on the platform. */ export class ComponentBusyError extends Error { constructor( readonly platform: Platform, @@ -679,16 +513,11 @@ export class ComponentBusyError extends Error { } } -/** A driver's progress report while it installs: the percentage its installer printed. */ export interface ComponentInstallProgress { readonly stage: "downloading"; readonly percent: number; } -/** - * A component install call ran out of its one budget, `downloads.timeoutMs`, measured from the - * moment the call arrived (ADR 0010 §3). Waiting for the platform's turn spends it too. - */ export class ComponentInstallTimeoutError extends Error { constructor( readonly platform: Platform, @@ -704,19 +533,7 @@ export class ComponentInstallTimeoutError extends Error { } export class RuntimeMissingError extends Error { - /** - * Whether a download could plausibly fix this: `true` exactly when the thrower names the - * `component` to install -- a plain "runtime not installed" is what `--allow-download` exists - * for. Without one (out of the model's pairing range, an installed runtime that does not pair - * with the model, a version below a download floor) it is `false`, so callers (see the - * daemon's download-policy suffix) don't point someone at a flag that cannot help. - */ readonly downloadable: boolean; - /** - * The string to hand to `Driver.installComponent` (ADR 0010 §2), set whenever `downloadable` - * is. It may be broader than a version -- the driver's word for "newest" -- and the core hands - * it back unread. - */ readonly component?: string; constructor( @@ -735,11 +552,6 @@ export class RuntimeMissingError extends Error { } } -/** - * A request named an option this driver does not have, such as an image tag on a platform whose - * runtimes come in one type. The driver decides that itself, so the core keeps no list of which - * platforms take which option. - */ export class UnsupportedRequestOptionError extends Error { constructor( readonly platform: Platform, @@ -751,14 +563,9 @@ export class UnsupportedRequestOptionError extends Error { } export class UnknownModelError extends Error { - /** The class a request named or meant when no model of it is listed on this host. */ readonly class: DeviceClass | undefined; readonly model: string | undefined; - /** - * An exact model the host does not list, or, with `deviceClass` and no model, a class with no - * model on this host: the message then names the config key to set. - */ constructor( readonly platform: Platform, model: string | undefined, @@ -808,13 +615,6 @@ function formatGibibytes(bytes: number): string { return `${(bytes / 1024 ** 3).toFixed(1)} GiB`; } -/** - * A component install was refused because a required license/EULA is not accepted -- - * platform-agnostic the same way `RuntimeMissingError` is, so the daemon can map it to a - * stable error code without importing a driver module. `AndroidLicenseNotAcceptedError` (the - * only concrete case today) extends this with its own message; a future platform with the same - * shape of gate would do the same rather than the daemon special-casing Android. - */ export class LicenseNotAcceptedError extends Error { constructor( readonly platform: Platform, @@ -826,10 +626,7 @@ export class LicenseNotAcceptedError extends Error { } /** - * Serializes disk-space preflight across concurrent component installs sharing a volume. - * Checked before any multi-GB component download/install, so a full disk fails fast with a - * clear message instead of filling up mid-download (see safety rule 4's spirit -- downloads - * must never surprise the machine they run on). A check of free space alone only ever sees the + * why: A check of free space alone only ever sees the * disk at the instant it runs: two installs racing the same preflight (an iOS runtime download * and an Android system-image install, or two of either) can each observe enough free space and * both proceed, jointly overfilling the volume neither alone would have. A single shared @@ -847,13 +644,6 @@ export class LicenseNotAcceptedError extends Error { export class DiskSpaceGuard { readonly #outstandingBytesByPath = new Map(); - /** - * Reserves `requiredBytes` against `path`'s free space, minus whatever this guard already has - * outstanding there. Throws `InsufficientDiskSpaceError` when the reservation would not fit. - * On success, returns a release function the caller must invoke exactly once (typically in a - * `finally`) once the install this reservation was made for has settled, freeing the bytes for - * the next reservation. - */ async reserve( filesystem: Pick, platform: Platform, diff --git a/src/daemon/connection-host.ts b/src/daemon/connection-host.ts index 3cb515ddb..e0c93e3e8 100644 --- a/src/daemon/connection-host.ts +++ b/src/daemon/connection-host.ts @@ -64,7 +64,6 @@ export interface DaemonEndpointHostOptions { readonly logger?: Logger; } -/** Owns endpoint claiming, stale-entry recovery, and listener lifecycle. */ export class DaemonEndpointHost implements ConnectionHost { readonly endpoint: string; readonly #logger: Logger; diff --git a/src/daemon/dispatcher.ts b/src/daemon/dispatcher.ts index 2187d61ce..d6919d3d9 100644 --- a/src/daemon/dispatcher.ts +++ b/src/daemon/dispatcher.ts @@ -85,21 +85,15 @@ import type { UpdateCoordinator, UpdateProgress } from "./update-coordinator.js" export { DispatchError, type ContractDispatcher, type DispatchSession } from "./dispatch.js"; -/* - * `DispatchSession`, `DispatchError`, and `ContractDispatcher` moved to `./dispatch.js` when - * ADR 0005 gave the contract a second dispatcher implementation (`src/gateway/`), which needs - * them without importing this file's `src/core` dependencies. They are re-exported above, so - * every existing `from "./dispatcher.js"` import keeps working. - */ /** - * How long a timed-out `device.exec` child gets between SIGTERM and SIGKILL (ADR 0005 §19e). + * why: How long a timed-out `device.exec` child gets between SIGTERM and SIGKILL (ADR 0005 §19e). * Fixed rather than derived from `exec.timeoutMs`: it is a termination-cleanup budget, not a * fraction of the command's own allowance -- the same reasoning, and the same ten seconds, as * `NodeProcessRunner`'s `SIGTERM_TO_SIGKILL_GRACE_MS`. */ const EXEC_SIGKILL_GRACE_MS = 10_000; -/** The `Promise.race` marker for "the timeout won". A unique object rather than a string, so +/** why: The `Promise.race` marker for "the timeout won". A unique object rather than a string, so * it can never collide with something a `StreamingProcessHandle` could resolve with. */ const EXEC_EXPIRED = Symbol("exec-expired"); @@ -130,15 +124,9 @@ export interface DispatcherOptions { readonly historyPath?: string; readonly capabilitySources?: readonly CapabilitySource[]; readonly capacity: CapacityReader; - /** The warm pool as its last pass left it, for `status.get`. */ readonly warmPool: WarmPoolReader; readonly catalog: CatalogReader; readonly clock: Clock; - /** - * The one installer (ADR 0010 §3), shared with the lease path so `component.install` and a - * lease request's download join one install, and read for `status.get`'s installs in progress - * and for `component.list`. `component.remove` goes through it too (ADR 0010 §8). - */ readonly components: Pick; readonly config: Config; readonly doctor?: Doctor; @@ -146,21 +134,15 @@ export interface DispatcherOptions { readonly deviceModes: DeviceModeReader; readonly leases: LeaseCommands; readonly logger?: Logger; - /** Classifies a thrown error for the `operation` log line (`classifyError` in production). + /** why: Classifies a thrown error for the `operation` log line (`classifyError` in production). * Injected because `error-code.ts` imports this module. Unset or `undefined`: `INTERNAL`. */ readonly errorCode?: (error: unknown) => string | undefined; readonly nuke?: Nuke; readonly enrollment?: Pick; - /** Optional so the many tests that never issue a passthrough need not fabricate a resolver. */ readonly passthrough?: PassthroughResolver; - /** - * Runs the command `device.exec` resolves (ADR 0005 §19a). Optional for the same reason as - * `passthrough`: the many tests that never exec should not have to fabricate a runner -- - * `#deviceExec` answers `INTERNAL` when it is missing rather than crashing. - */ readonly processRunner?: ProcessRunner; /** - * The environment a `device.exec` child starts from, before the driver's own scoping keys are + * why: The environment a `device.exec` child starts from, before the driver's own scoping keys are * layered on top. Injected rather than read from `process.env` here (architecture rule 9), * and layered rather than replaced because `ProcessRunner` replaces a child's environment * wholesale: a child given only `--set`/`-P` scoping would lose `PATH` and never find the @@ -171,13 +153,6 @@ export interface DispatcherOptions { readonly presence?: (driverDeviceId: string) => LatestPresence | undefined; readonly reaper: CleanupReaper; readonly registry: Registry; - /** - * What the `stalled` flag `status.get` and `list.get` put on a device is worked out from - * (`isStalledTransition`, with `config.stalledTransition` and the clock): the drivers this - * daemon runs, and its device operation claims, so a live operation is never read as a - * stall. Optional so the tests that never stall a device need not fabricate either; absent, - * no device is flagged. - */ readonly stalls?: { readonly drivers: readonly Pick[]; readonly claims: NonNullable; @@ -189,28 +164,18 @@ export interface DispatcherOptions { * clear `DispatchError` when it is missing rather than crashing on `undefined.create(...)`. */ readonly tokens?: TokenOperations; - /** Reports current daemon health for `status.get`. */ readonly health: () => "starting" | "running" | "failed"; - /** `status.get`'s host block, from memory: answering never waits for a read (ADR 0008 §7). */ readonly hostFacts: () => HostFacts; readonly load?: () => StatusLoad | undefined; readonly awaitReady: () => Promise; - /** This daemon's instance id (`instance.json`), the id it presents to a gateway. `worker.list` - * answers with it as the one worker's id (ADR 0012 §1). */ readonly instanceId: string; - /** This daemon's own version, the one its `hello` reply carries; `worker.list`'s `version`. */ readonly version: string; - /** - * Where `worker.list` hears `WORKER_VIEW_CATALOG_EVENTS`, so its kept catalog is read again at - * once rather than on its next interval. Optional for the tests that never call `worker.list`; - * without it the kept catalog is only re-read on the interval. - */ readonly eventBus?: Pick; readonly update?: UpdateCoordinator; } /** - * A handler's declared *input* type is tied to its operation's contract schema (real + * why: A handler's declared *input* type is tied to its operation's contract schema (real * type-checking value: a handler that reads `input.wrongField` fails to compile). Its return * type is deliberately left as `unknown` rather than `z.infer<...Output>`: core's domain types * (`LeaseRecord`, `DoctorReport`, ...) are hand-mirrored by the contract's schemas, not reused @@ -225,27 +190,13 @@ type Handler = ( session: DispatchSession, ) => Promise | unknown; -/** Every operation this (worker-mode) dispatcher implements: the contract's set minus - * `daemon.stop` (intercepted by `DaemonServer` itself, ADR 0003 §6). */ type WorkerOperationName = Exclude; -/** - * The transport-independent dispatcher (ADR 0003 §2). One `dispatch()` call does, in order: - * parse input, role check, `authorize` hook, park on startup readiness, call handler, parse - * output. Handlers never see a raw payload or run their own role/ownership check -- both - * already happened by the time a handler's function body runs. - * - * Deliberately excludes `hello` (protocol-level, answered before a session exists) and - * `daemon.stop` (ADR §6's frozen exception -- scoped to the protocol-version gate only, so it - * stays reachable across a version mismatch; still requires a completed handshake and the - * `admin` role, checked in `DaemonServer#dispatchLine` itself) -- both stay in `DaemonServer`, - * same as before this PR. - */ export class Dispatcher { readonly #logger: Logger; readonly #dispatchLogger: Logger; /** - * Total over every operation but `daemon.stop`. Deliberately *not* a partial map: a declared + * why: Total over every operation but `daemon.stop`. Deliberately *not* a partial map: a declared * operation whose handler was never written is otherwise invisible to the compiler and only * shows up as `UNKNOWN_REQUEST` at runtime -- which is exactly how `driver.passthrough` came * to be declared, dispatched, and unimplemented at once. @@ -265,7 +216,6 @@ export class Dispatcher { * share it; a read that fails is dropped, so the next call reads again. */ #viewCatalog: { readonly readAt: number; readonly catalog: Promise } | undefined; - /** Ends the bus subscriptions that drop `#viewCatalog`; see `dispose`. */ readonly #unsubscribe: (() => void)[] = []; readonly #usage: UsageReader; @@ -290,7 +240,6 @@ export class Dispatcher { }, ], }); - // Observers only (architecture rule 5): dropping a kept read decides nothing. for (const event of WORKER_VIEW_CATALOG_EVENTS) { const unsubscribe = options.eventBus?.subscribe(event, () => { this.#viewCatalog = undefined; @@ -384,8 +333,7 @@ export class Dispatcher { }); } - // ---- handlers --------------------------------------------------------------------------- - // Arrow-function class fields (not methods): each closes over `this` so it can be stored in + // why: Arrow-function class fields (not methods): each closes over `this` so it can be stored in // `#handlers` and invoked as a plain function, the same way any other transport-independent // callback would be. None of these run a role or ownership check -- `dispatch()` above // already did, per the ADR's ordering. @@ -509,7 +457,7 @@ export class Dispatcher { const requesterId = input.requesterId ?? session.principal; const requestedAllowDownload = input.allowDownload ?? false; const downloadsPolicy = this.options.config.downloads.policy; - // `owner` is read only from the gateway's uplink, never from role alone: HTTP's `operator` token + // why: `owner` is read only from the gateway's uplink, never from role alone: HTTP's `operator` token // maps to `admin`, so a role check would let any operator name someone else's owner. Refused, not ignored. if (input.owner !== undefined && session.isGatewayUplink !== true) { throw new DispatchError( @@ -517,7 +465,7 @@ export class Dispatcher { "Only the gateway's own uplink session may set lease.request's `owner` field", ); } - // Uplink only, refused before anything is stored: a probe is never queued on a worker, so it must say `noWait`. + // why: Uplink only, refused before anything is stored: a probe is never queued on a worker, so it must say `noWait`. if (input.fleetRequestId !== undefined) { if (session.isGatewayUplink !== true) { throw new DispatchError( @@ -565,7 +513,7 @@ export class Dispatcher { throw new DispatchError("NO_CAPACITY", error.message, { capacityReason }); } } - // The lease path sees only the clamped-to-false permission, so this is the one place that can + // why: The lease path sees only the clamped-to-false permission, so this is the one place that can // tell the caller that config, not missing consent, blocked the download. if ( downloadsPolicy === "never" && @@ -584,24 +532,11 @@ export class Dispatcher { return { result }; }; - /** - * ADR 0004 §1: the one keep-alive, on every transport. An omitted `ttlMs` re-applies the - * lease's own stored width (`LeaseLifecycle#renew`), never `lease.defaultTtlMs`. - */ #leaseRenew: Handler<"lease.renew"> = async (input) => { this.#requireTtlWithinCap(input.ttlMs); return this.options.leases.renew(input.leaseId, input.ttlMs); }; - /** - * ADR 0004 §4's cap, applied identically to a request and a renew. It lives here rather than - * in the contract schema because `lease.maxTtlMs` is a daemon config value, and the contract - * module cannot see one; it lives in the dispatcher rather than in each transport because - * every transport reaches leases through this one shared object (ADR 0003 §2), so HTTP gets - * the same `400 BAD_REQUEST` the socket gets from the same line of code. Rejecting rather - * than clamping is the point: a caller silently given less time than it asked for would go - * on believing it had the time it named. - */ #requireTtlWithinCap(ttlMs: number | undefined): void { const maxTtlMs = this.options.config.lease.maxTtlMs; if (ttlMs !== undefined && ttlMs > maxTtlMs) { @@ -617,13 +552,6 @@ export class Dispatcher { return { leaseId: input.leaseId }; }; - /** - * `killed`, not `explicit`: `docs/internal/EVENTS.md` splits the two by who ended the lease and - * whether its holder asked. An operator's `simlock release --all` (and `nuke`, which already - * reports `killed` through `NukeService`) takes leases away from holders that never asked -- - * which is exactly what a `lease-lost` reader needs to tell apart from the holder's own - * `lease.release` on its way out. - */ #leaseReleaseAll: Handler<"lease.release-all"> = async () => { const leaseIds = await this.options.leases.releaseAll("killed"); return { leaseIds: [...leaseIds] }; @@ -660,7 +588,7 @@ export class Dispatcher { } /** - * Resolution only: the daemon builds the command and never runs it. Spawning it here would + * why: Resolution only: the daemon builds the command and never runs it. Spawning it here would * attach a user's interactive `adb shell` to the daemon's stdio, and the CLI is the process * that actually has a terminal (ADR 0001, decision 7). * @@ -758,25 +686,17 @@ export class Dispatcher { } const handle = this.options.processRunner.spawnStreaming(command.command, command.args, { env: execEnv, - // Returned, not fired and forgotten: whatever the transport hands back is what pauses + // why: Returned, not fired and forgotten: whatever the transport hands back is what pauses // the child until the chunk has actually gone somewhere (ADR 0005 §19e). onChunk: (stream, chunk) => session.onOutput?.(stream, chunk), ...(input.stdin === undefined ? {} : { input: input.stdin }), }); - // Announced before the first chunk can arrive, because "it started" is what a transport + // why: Announced before the first chunk can arrive, because "it started" is what a transport // needs to decide its response shape on -- see `DispatchSession.onStarted`. session.onStarted?.(); return { exitCode: await this.#awaitExec(handle, input.tool, input.args) }; }; - /** - * Waits for an exec'd command, killing it if it outruns `exec.timeoutMs` (ADR 0005 §19e). - * SIGTERM first, SIGKILL after a grace window, because a tool that ignores the first must - * not be able to hold this operation -- and the caller's connection -- open forever; that - * escalation mirrors `NodeProcessRunner#run`'s own. The timeout is reported as - * `EXEC_TIMEOUT` rather than as the exit code the kill produced: "we stopped it" and "it - * failed" are different facts, and only the first tells a caller to raise the limit. - */ async #awaitExec( handle: StreamingProcessHandle, tool: string, @@ -790,28 +710,28 @@ export class Dispatcher { }); let killTimer: TimerHandle | undefined; try { - // Raced rather than decided by a flag the timer sets: a command that exits in the same + // why: Raced rather than decided by a flag the timer sets: a command that exits in the same // turn the timer fires has *finished*, and reporting that as a timeout would blame the // limit for a command that met it. `Promise.race` settles on whichever actually // happened first, which is exactly the question. const outcome = await Promise.race([waited, expired]); if (outcome !== EXEC_EXPIRED) return exitCodeOf(outcome); - // SIGTERM first, SIGKILL after a grace window: a tool that ignores the first must not be + // why: SIGTERM first, SIGKILL after a grace window: a tool that ignores the first must not be // able to hold this operation -- and the caller's connection -- open forever. The same // escalation `NodeProcessRunner#run` makes for its own timeout. killQuietly(handle, "SIGTERM"); killTimer = this.options.clock.setTimer(EXEC_SIGKILL_GRACE_MS, () => { killQuietly(handle, "SIGKILL"); }); - // Once the kill is issued, the timeout is the authoritative fact (ADR 0005 §19e) -- + // why: Once the kill is issued, the timeout is the authoritative fact (ADR 0005 §19e) -- // `waited` rejecting with `ExecOutputDeliveryStalledError` (a consumer that stopped // reading is *why* the command outran its timeout in the first place: the pause that // stops it finishing is the same pause that stalls delivery) is a consequence of the // timeout, not a competing answer to it. Swallowed here rather than left to reject // this `await` and skip the throw below. await waited.catch(() => undefined); - // `EXEC_TIMEOUT` rather than the exit code the kill produced: "we stopped it" and "it + // why: `EXEC_TIMEOUT` rather than the exit code the kill produced: "we stopped it" and "it // failed" are different facts, and only the first tells a caller to raise the limit. throw new DispatchError( "EXEC_TIMEOUT", @@ -825,13 +745,11 @@ export class Dispatcher { #doctorRun: Handler<"doctor.run"> = async (input) => { if (this.options.doctor === undefined) throw new DoctorUnavailableError(); - // `purgeOrphans` travels as its own flag all the way down, never folded into `fix`: + // why: `purgeOrphans` travels as its own flag all the way down, never folded into `fix`: // someone already running `doctor --fix` unattended in CI must not acquire a // destructive behaviour by upgrading (ADR 0001, decision 6). return this.options.doctor.reconcile({ fix: input.fix ?? false, - // Only an operator's `doctor` looks at the machine's prerequisites; startup convergence - // runs no tool. prerequisites: true, purgeOrphans: input.purgeOrphans ?? false, }); @@ -897,16 +815,6 @@ export class Dispatcher { #configGet: Handler<"config.get"> = () => this.options.config; - /** - * ADR 0012 §1: this host as its own fleet of one. The fields a gateway reads over the uplink - * come from the same four reads, made here against this dispatcher's own handlers; the - * contract's one builder turns them into a view. The parses only turn the handlers' core - * records into the wire types the builder takes: `worker.list`'s own output schema, applied - * by `dispatch()`, is what bounds and narrows the view on the wire. The catalog is the kept - * one (see `#viewCatalog`); status and devices are read on every call, as a gateway reads - * them on every worker event. `drained` is always `false`: a gateway that drained this worker - * holds that flag, not this host. - */ #workerList: Handler<"worker.list"> = async (_input, session) => { // why: taken as status resolves, not after the slower catalog read, so the load is not made to look fresher. let receivedAt = this.options.clock.now(); @@ -940,12 +848,6 @@ export class Dispatcher { }; }; - /** - * The kept catalog for `worker.list`, read again once it is older than the interval. The age - * is wall-clock time, so one that is negative (the clock was set back) counts as stale rather - * than keeping the catalog for as long as the clock went back. A failed read drops whatever is - * kept, at worst one read newer than it, which costs one more read. - */ #keptViewCatalog(session: DispatchSession): Promise { const now = this.options.clock.now(); const kept = this.#viewCatalog; @@ -966,13 +868,6 @@ export class Dispatcher { return catalog; } - /** - * ADR 0010 §4 and §6: an operator's explicit install. The command itself is the consent, so the - * one consent function is asked with `true`; only `downloads.policy: "never"` refuses it, and it - * does so before the installer is reached. Everything else -- joining an install already - * running, waiting a platform's turn, the disk reservation, the budget, the events -- is the - * installer's, exactly as for a lease request's download. - */ #componentInstall: Handler<"component.install"> = async (input, session) => { if (!effectiveAllowDownload(this.options.config.downloads.policy, true)) { throw new DispatchError( @@ -992,7 +887,7 @@ export class Dispatcher { : { onProgress: (progress) => onComponentProgress(toWireProgress(progress)) }), ...(onStarted === undefined ? {} : { onAdmitted: onStarted }), }); - // No `stillNeeded` is passed, so `not-needed` cannot come back; a version is always present + // why: No `stillNeeded` is passed, so `not-needed` cannot come back; a version is always present // for the two outcomes that can. if (result.outcome === "not-needed" || result.version === undefined) { throw new Error(`Internal: component install ended as ${result.outcome} with no version`); @@ -1005,16 +900,10 @@ export class Dispatcher { }; }; - /** ADR 0010 §8: a read for any session; the installer merges the drivers and the registry. */ #componentList: Handler<"component.list"> = async (input) => ({ components: await this.options.components.list(input.platform), }); - /** - * ADR 0010 §8: an operator's removal of a component Simlock installed. Every check and the - * removal itself are the installer's; this names the admin principal for `component.removed` - * (safety rule 6) and puts the in-use counts on the wire as `COMPONENT_IN_USE`'s details. - */ #componentRemove: Handler<"component.remove"> = async (input, session) => { try { const result = await this.options.components.remove({ @@ -1149,7 +1038,7 @@ export class Dispatcher { } /** - * The installer's progress in the contract's words: its percentage, already within 0..100, + * why: The installer's progress in the contract's words: its percentage, already within 0..100, * becomes a fraction from 0 to 1, rounded to three decimals so that 1.1 percent is 0.011 and not * the 0.011000000000000001 a plain division gives. Only 100 percent becomes 1: 99.96 would round * up to it, and 1 is what an install says once it is complete. With none (the install has just @@ -1162,7 +1051,7 @@ function toWireProgress(progress: ComponentInstallerProgress): ComponentProgress return { fraction: progress.percent < 100 ? Math.min(fraction, 0.999) : 1, stage: "downloading" }; } -/** A child that exited between the timer firing and the signal landing is not an error worth +/** why: A child that exited between the timer firing and the signal landing is not an error worth * failing the operation over -- the wait below is about to report how it ended anyway. */ function killQuietly(handle: StreamingProcessHandle, signal: NodeJS.Signals): void { try { diff --git a/src/daemon/main.test.ts b/src/daemon/main.test.ts index f6c560251..bb17845ec 100644 --- a/src/daemon/main.test.ts +++ b/src/daemon/main.test.ts @@ -94,8 +94,6 @@ const STANDALONE_OFF = { http: { enabled: false }, } as const; -/** `drivers: undefined` is spelled out by a caller that wants real (module-substituted) - * discovery rather than the fake driver below. */ async function start( overrides: Partial> & { readonly deviceDataTables?: StartDaemonOptions["deviceDataTables"] | undefined; @@ -409,7 +407,6 @@ describe("the event history", () => { it("judges retention by the injected clock, so an event younger than the retention on that clock survives the start-up prune", async () => { const directory = await mkdtemp(join(tmpdir(), "simlock-main-")); temporaryDirectories.push(directory); - // The fake clock reads 1_000; a real clock would put the 7-day cutoff far past 500. await seedHistory(directory, [500]); await start({ dataDirectory: directory }); @@ -446,7 +443,7 @@ async function queryState( } } -/** A gateway URL on a port just released. Nothing listens there, so a worker's uplink dial +/** why: A gateway URL on a port just released. Nothing listens there, so a worker's uplink dial * fails fast, and the fake clock never fires its retry. */ async function unreachableGatewayUrl(): Promise { const probe = createServer(); @@ -491,7 +488,6 @@ async function writeModule(contents: string): Promise { return modulePath; } -/** A driver whose disposal is observable, which `FakeDriver` deliberately is not. */ class DisposableFakeDriver extends FakeDriver { constructor( private readonly disposed: string[], @@ -517,7 +513,6 @@ async function agentStatus(daemon: DaemonServer) { )) as { readonly host: { readonly tools: readonly unknown[] } }; } -/** A fake driver that reports tool versions, which `FakeDriver` does not. */ class ToolsFakeDriver extends FakeDriver { constructor( options: ConstructorParameters[0], @@ -532,7 +527,7 @@ class ToolsFakeDriver extends FakeDriver { } /** - * Binds an ephemeral loopback port and releases it. A fixed port fails whenever two runs of + * why: Binds an ephemeral loopback port and releases it. A fixed port fails whenever two runs of * this file overlap -- two worktrees running the suite at once, or Stryker's parallel runners * -- and another run's daemon can even answer on it. */ @@ -565,7 +560,7 @@ describe("startDaemon", () => { readonly gateway: { readonly token?: string }; }; - // The fixture is real: the daemon did log its start and emit `daemon.started` to both + // why: The fixture is real: the daemon did log its start and emit `daemon.started` to both // the ring and the file, so "absent" below means redacted, not never written. expect(sink.records.some((record) => record.message === "Daemon started")).toBe(true); expect(eventFile).toContain('"daemon.started"'); @@ -634,13 +629,12 @@ describe("startDaemon", () => { role: "admin", } as const; - // doctor.run parks until startup convergence has finished, so by the time it answers, + // why: doctor.run parks until startup convergence has finished, so by the time it answers, // convergence's own reconcile has run -- and must not have counted. const first = (await daemon.dispatch("doctor.run", {}, admin)) as { readonly findings: readonly { readonly kind: string; readonly prerequisite?: string }[]; }; expect(runs).toEqual({ android: 1, ios: 1 }); - // Android has no driver in this daemon, so the finding is the platform's, not running. expect(first.findings).toContainEqual( expect.objectContaining({ kind: "prerequisite-missing", platform: "android" }), ); @@ -801,8 +795,6 @@ describe("startDaemon", () => { .catch(() => undefined); } - // With the default cap both failures stay stored; with an unclassified failure the code - // would be INTERNAL. await expect .poll(() => queryState(directory, "SELECT requester_id, failure_code FROM lease_requests")) .toEqual([{ failure_code: "RUNTIME_MISSING", requester_id: "second" }]); @@ -821,14 +813,14 @@ describe("startDaemon", () => { await first.daemon.stop("test"); await start({ dataDirectory: directory, filesystem }); - // Never regenerated: a second id would strand every device already sitting in a root + // why: Never regenerated: a second id would strand every device already sitting in a root // marked with the first one (ADR 0001, decision 2). expect(written.instanceId).not.toBe(""); expect(JSON.parse(await filesystem.readFile(identityPath))).toEqual(written); }); it("disposes every driver on shutdown, and reports the one that failed without stopping", async () => { - // The wiring, not the halves: `DaemonServer` awaiting its `dispose` option is tested in + // why: The wiring, not the halves: `DaemonServer` awaiting its `dispose` option is tested in // `server.test.ts` and a driver reaping its adb server in the Android driver's own, and // between them sat the composition that connects the two. Losing it silently abandons // Simlock's adb server on every shutdown, and only `Driver.dispose` can stop one. @@ -1024,7 +1016,7 @@ describe("startDaemon wires core and leasing together", () => { readonly device: { readonly id: string }; }; - /** Lets the work a mutated daemon would still be doing finish, so "nothing happened" is a + /** why: Lets the work a mutated daemon would still be doing finish, so "nothing happened" is a * statement about the daemon and not about how early the test looked. Only sound when that * work is all in memory: a timer fires after it however loaded the machine is, but real I/O * (a process spawn, a real file) can outlast it. */ @@ -1178,7 +1170,6 @@ describe("startDaemon wires core and leasing together", () => { const clock = new FakeClock(1_000); const directory = await mkdtemp(join(tmpdir(), "simlock-main-")); temporaryDirectories.push(directory); - // The previous process's registry: one request still open when it stopped. const state = await openStateDatabase({ dataDirectory: directory, filesystem: new MemoryFilesystem(), @@ -1320,7 +1311,6 @@ describe("startDaemon wires core and leasing together", () => { }); await daemon.dispatch("lease.release", { leaseId: grant.lease.id }, agent); - // Well past the stalled-transition threshold, but the erase holds its claim on the device. clock.advance(300_000); const devices = (await daemon.dispatch("list.get", { kind: "devices" }, admin)) as readonly { readonly id: string; @@ -1367,7 +1357,6 @@ describe("startDaemon wires core and leasing together", () => { const grant = (await started.daemon.dispatch("lease.request", ios, agent)) as Grant; await started.daemon.dispatch("lease.release", { leaseId: grant.lease.id }, agent); - // Shutdown waits for the failed purge to settle into quarantine, which arms the retry. await started.daemon.stop("test"); clock.advance(30_000); await settleRealTime(); @@ -1457,11 +1446,6 @@ describe("startDaemon wires core and leasing together", () => { }); describe("startDaemon startup readiness", () => { - // Reproduces the issue #41 symptom end-to-end through the real startDaemon wiring: - // doctor.reconcile() shells out to driver.listManaged() per driver, which is where - // real convergence time is lost. Here it's held open on the FakeClock so the test - // can prove the socket answers hello/status.get ("starting"), parks lease.request, - // and only then converges -- without waiting on a real clock. it("claims the socket and answers hello/status.get while doctor.reconcile is still in flight", async () => { const directory = await mkdtemp(join(tmpdir(), "simlock-main-slow-")); temporaryDirectories.push(directory); @@ -1506,7 +1490,7 @@ describe("startDaemon startup readiness", () => { void parkedLease.then(() => { leaseSettled = true; }); - // Round-tripping another status.get proves the parked request was already + // why: Round-tripping another status.get proves the parked request was already // dispatched (and is awaiting the readiness gate) without relying on a timer. await client.request("status.get", {}); expect(leaseSettled).toBe(false); @@ -1559,7 +1543,6 @@ describe("startDaemon stopped while starting", () => { protocolVersion: DAEMON_PROTOCOL_VERSION, }); expect((await client.request("daemon.stop", {})).ok).toBe(true); - // `daemon.stop` answers first and tears down after, so give the teardown real time to begin. await new Promise((resolve) => setTimeout(resolve, 25)); } finally { client.socket.end(); @@ -1573,11 +1556,6 @@ describe("startDaemon stopped while starting", () => { }); describe("startDaemon HTTP gateway startup readiness", () => { - // ADR 0003 §2: "an HTTP request during startup now waits like a socket request instead of - // being refused." Mirrors the socket-side test above (same FakeDriver latency trick to hold - // convergence open on the FakeClock), but drives a real HTTP request instead of a socket - // frame -- proving the gateway is actually listening (and its request actually parks) before - // convergence finishes, not just that `dispatch()` would park it in principle. it("parks an HTTP request until convergence completes, instead of refusing the connection", async () => { const directory = await mkdtemp(join(tmpdir(), "simlock-main-http-slow-")); temporaryDirectories.push(directory); @@ -1607,7 +1585,7 @@ describe("startDaemon HTTP gateway startup readiness", () => { return daemon; }); - // Polls rather than a fixed delay: the gateway binds asynchronously (`onSocketClaimed` + // why: Polls rather than a fixed delay: the gateway binds asynchronously (`onSocketClaimed` // fires once the socket claim resolves, then `gateway.start()` itself awaits a real // `listen()`), so there is no single microtask boundary to await here the way the // socket-side test above can just retry-connect on the unix socket. @@ -1621,8 +1599,6 @@ describe("startDaemon HTTP gateway startup readiness", () => { void parkedStatusPromise.then(() => { settled = true; }); - // Give the parked request's own microtasks a chance to run before asserting it hasn't -- - // it must still be awaiting `dispatch()`'s startup-readiness gate, not merely slow. await new Promise((resolve) => setTimeout(resolve, 20)); expect(settled).toBe(false); @@ -1900,7 +1876,7 @@ describe("startDaemon stop with a warm target", () => { .poll(() => driver.calls.filter((call) => call.operation === "makeReady").length) .toBe(1); - // The creation ends while the HTTP gateway is still being stopped: a pool not closed by then + // why: The creation ends while the HTTP gateway is still being stopped: a pool not closed by then // would start the second one, and the drain would wait for its whole boot. const stopping = daemon.stop("test-stop-target"); driver.releaseMakeReady(); @@ -1911,24 +1887,6 @@ describe("startDaemon stop with a warm target", () => { }); describe("startDaemon HTTP gateway stop-during-start race (review finding S5)", () => { - // Before this fix, `stopAuxiliary` (this file's `stopAuxiliary` closure passed to - // `DaemonServer`) returned immediately whenever the concurrently-started HTTP gateway - // (`onSocketClaimed` -> `startHttpGateway()`) hadn't finished its own `listen()` yet -- - // `stopHttpGateway` isn't assigned until it does. `#stop()` (server.ts) awaits - // `stopAuxiliary()` first, so it would go on to release leases, settle, and dispose while the - // gateway was still binding, then start accepting HTTP requests -- against a torn-down engine - // -- once its own `listen()` finally resolved. The fix makes `stopAuxiliary` await - // `gatewayStarted` (settled only once the gateway's own bind attempt finishes, one way or the - // other) before doing anything else. - // - // The window this reproduces (`onSocketClaimed` firing to the gateway's `listen()` actually - // resolving) is normally a handful of real milliseconds -- reachable, per the review, via - // `simlock daemon stop` during startup, but not something a test can land inside reliably by - // just racing real I/O against real I/O. So this test holds the window open deterministically - // instead: it patches `net.Server.prototype.listen` (restored in `finally`) to delay only a - // TCP bind (the gateway's) by 200ms, leaving the daemon's own Unix-socket `listen()` (a string - // first argument, not a port) untouched -- so the socket claims and answers `daemon.stop` - // long before the patched gateway bind is even allowed to start. it("never lets the HTTP gateway outlive full teardown when daemon.stop lands while the gateway is still mid-bind", async () => { const directory = await mkdtemp(join(tmpdir(), "simlock-main-http-race-")); temporaryDirectories.push(directory); @@ -1952,14 +1910,8 @@ describe("startDaemon HTTP gateway stop-during-start race (review finding S5)", socketPath, version: "1.2.3", } as StartDaemonOptions); - // `stop()` is idempotent, so it is safe for `afterEach` to also stop whatever this - // resolves to (it will, once the delayed gateway bind above finishes settling). void startPromise.then((daemon) => runningDaemons.push(daemon)).catch(() => undefined); - // Connects and authenticates as admin, then sends `daemon.stop` -- comfortably inside the - // 200ms window the patch above holds the gateway's own bind open for. The admin secret is - // written (`AdminSecretManager#persist`) before `onSocketClaimed` fires, so it is already - // there by the time the socket itself is reachable. const client = await connectRetrying(socketPath); const secret = (await readFileRetrying(filesystem, join(directory, "admin.token"))).trim(); await client.request("hello", { @@ -1971,8 +1923,6 @@ describe("startDaemon HTTP gateway stop-during-start race (review finding S5)", expect(stopReply.ok).toBe(true); client.socket.end(); - // Lets the delayed gateway bind actually fire and settle (one way or the other) before - // this test reads the log for it. await new Promise((resolve) => setTimeout(resolve, 400)); const stoppingIndex = sink.records.findIndex( @@ -1985,20 +1935,11 @@ describe("startDaemon HTTP gateway stop-during-start race (review finding S5)", const gatewayListeningIndex = sink.records.findIndex( (record) => record.message === "HTTP gateway listening", ); - // The gateway actually got to bind (it does here -- the port is free and 200ms is ample - // real time for a loopback `listen()`), so this asserts the meaningful case directly - // rather than treating it as merely possible: it bound, and it was stopped, and both - // happened strictly before "Daemon stopping" (logged only once `stopAuxiliary` resolves -- - // see `server.ts#stop`). Before the fix, `stoppingIndex` would be *smaller* than both of - // these -- `stopAuxiliary` returned immediately, so "Daemon stopping" logged right away, - // well before the (patch-delayed) gateway bind even started. expect(gatewayListeningIndex).toBeGreaterThanOrEqual(0); expect(gatewayListeningIndex).toBeLessThan(stoppingIndex); expect(gatewayStoppedIndex).toBeGreaterThanOrEqual(0); expect(gatewayStoppedIndex).toBeLessThan(stoppingIndex); - // And the port must never be reachable once the daemon has finished stopping -- the - // gateway did not outlive teardown just because it bound after `stop()` was requested. await expect( fetch(`http://127.0.0.1:${port}/v1/status`, { headers: { authorization: "Bearer irrelevant" }, @@ -2012,12 +1953,6 @@ describe("startDaemon HTTP gateway stop-during-start race (review finding S5)", }); describe("startDaemon socket race with HTTP enabled", () => { - // Review finding V1: `gatewayStarted` is settled only from inside `onSocketClaimed`'s - // handler, and `start()` fires that callback only after the socket claim succeeds. A daemon - // that loses the start race therefore rejected without the callback ever running, so nothing - // settled `gatewayStarted` and `Promise.allSettled` waited on it forever -- `startDaemon()` - // hung instead of reporting the lost race, with no rejection and no non-zero exit code. - // Reachable by racing `simlock daemon start`, or by the CLI's own auto-launch. it("rejects rather than hanging when the socket is already claimed", async () => { const directory = await mkdtemp(join(tmpdir(), "simlock-main-socket-race-")); temporaryDirectories.push(directory); @@ -2046,7 +1981,6 @@ describe("startDaemon socket race with HTTP enabled", () => { const first = await startDaemon(options(await freeLoopbackPort())); try { - // A distinct port, so the only thing that can fail is the socket claim itself. const second = startDaemon(options(await freeLoopbackPort())); const outcome = await Promise.race([ second.then( @@ -2063,7 +1997,7 @@ describe("startDaemon socket race with HTTP enabled", () => { }); describe("startDaemon gateway uplink when the socket claim fails", () => { - // #395: the uplink is dialled from `onSocketClaimed`, so a daemon that loses the claim never + // why: #395: the uplink is dialled from `onSocketClaimed`, so a daemon that loses the claim never // dials its gateway -- it would otherwise join as a worker it is about to stop being. it("never dials the gateway when the worker's socket is already claimed", async () => { const directory = await mkdtemp(join(tmpdir(), "simlock-main-uplink-claim-")); @@ -2107,7 +2041,6 @@ describe("startDaemon gateway uplink when the socket claim fails", () => { () => "resolved" as const, () => "rejected" as const, ); - // The dial, were it made, reaches a loopback listener within milliseconds. await new Promise((resolve) => setTimeout(resolve, 500)); expect({ dials, outcome }).toEqual({ dials: [], outcome: "rejected" }); } finally { @@ -2118,8 +2051,6 @@ describe("startDaemon gateway uplink when the socket claim fails", () => { }); describe("startDaemon with HTTP off", () => { - // #395: the socket-claim callback now runs with HTTP off too (it dials the uplink), so it - // must still leave the HTTP frontend unstarted. it("starts no HTTP gateway when http.enabled is false", async () => { const port = await freeLoopbackPort(); await start({ configOverrides: { http: { enabled: false, host: "127.0.0.1", port } } }); @@ -2137,12 +2068,6 @@ describe("startDaemon with HTTP off", () => { }); describe("startDaemon HTTP gateway bind failure", () => { - // Review finding B6: before this fix, an HTTP bind failure (occupied port) logged and - // stopped the daemon from inside `onSocketClaimed`'s handler without `startDaemon()` itself - // ever seeing it -- the daemon's own `start()` would go on to resolve successfully once - // convergence finished, so the CLI reported success (exit code 0) with no socket, no HTTP, - // and no daemon actually running. This proves `startDaemon()` now rejects instead, and that - // the daemon never reports having started after it already reported stopping. it("rejects startDaemon() when the configured HTTP port is already in use, without emitting a started record after a stopping one", async () => { const directory = await mkdtemp(join(tmpdir(), "simlock-main-http-bind-")); temporaryDirectories.push(directory); @@ -2174,10 +2099,6 @@ describe("startDaemon HTTP gateway bind failure", () => { const startedIndex = sink.records.findIndex((record) => record.message === "Daemon started"); const stoppingIndex = sink.records.findIndex((record) => record.message === "Daemon stopping"); expect(stoppingIndex).toBeGreaterThanOrEqual(0); - // Either "Daemon started" never appears (the common case: the bind failure is discovered - // and the whole thing torn down without convergence ever seeing readiness), or -- if - // convergence happened to finish first -- it appears strictly before "Daemon stopping", - // never after (ADR events rule 3: a fact must be true when emitted). if (startedIndex >= 0) { expect(startedIndex).toBeLessThan(stoppingIndex); } @@ -2343,7 +2264,7 @@ describe("discoverDrivers", () => { const { drivers, rejections } = await discoverIos({ hostPlatform: "linux" }); expect(drivers.some((driver) => driver.platform === "ios")).toBe(false); - // Nothing refused anything here: a host without Xcode is not a fail-closed refusal + // why: Nothing refused anything here: a host without Xcode is not a fail-closed refusal // and must not be reported as one. expect(rejections).toEqual([]); }); @@ -2355,7 +2276,7 @@ describe("discoverDrivers", () => { }); it("builds both prerequisite checks on macOS, whether or not a driver started", async () => { - // The refused root costs the iOS driver; it must not cost the check that explains it. + // why: The refused root costs the iOS driver; it must not cost the check that explains it. const { drivers, prerequisiteChecks } = await discoverIos({ filesystem: await foreignOwnedRoot(), }); @@ -2390,7 +2311,7 @@ describe("discoverDrivers", () => { const { drivers, rejections } = await discoverIos({ filesystem }); - // Safety rule 9: the platform goes, the daemon stays -- a daemon that will not start + // why: Safety rule 9: the platform goes, the daemon stays -- a daemon that will not start // is a daemon that cannot tell anyone why. expect(drivers.some((driver) => driver.platform === "ios")).toBe(false); expect(rejections.map((rejection) => rejection.platform)).toEqual(["ios"]); @@ -2402,7 +2323,7 @@ describe("discoverDrivers", () => { const { rejections } = await discoverIos({ filesystem }); - // Built from a real `OwnedRootError`, key by key: the payload is a wire contract + // why: Built from a real `OwnedRootError`, key by key: the payload is a wire contract // `simlock events --json` publishes, so renaming `root` has to fail here. expect(rejections[0]).toEqual({ event: "driver.root-rejected", @@ -2418,7 +2339,7 @@ describe("discoverDrivers", () => { }); it("still fails startup when the iOS driver fails for a reason that is not a refusal", async () => { - // Anything that is not an `OwnedRootError` is a bug rather than a fail-closed + // why: Anything that is not an `OwnedRootError` is a bug rather than a fail-closed // decision, and swallowing it would hide it behind a silently missing platform. await expect( discoverIos({ @@ -2449,7 +2370,7 @@ describe("discoverDrivers on a host with an Android SDK", () => { const { drivers, rejections } = await discoverAndroid(filesystem, new FakeTcpProbe([5038])); expect(drivers.some((driver) => driver.platform === "android")).toBe(false); - // The payload is a wire contract `simlock events --json` publishes for + // why: The payload is a wire contract `simlock events --json` publishes for // `driver.adb-server-rejected`, so a renamed key has to fail here. expect(rejections[0]).toMatchObject({ event: "driver.adb-server-rejected", @@ -2464,7 +2385,6 @@ describe("discoverDrivers on a host with an Android SDK", () => { const filesystem = await androidSdk(); const probe = new FakeTcpProbe([5038]); await filesystem.mkdirp(SIMLOCK_HOME); - // The server a previous daemon left behind, with the pid that proves it is Simlock's. await filesystem.writeFileAtomic( join(SIMLOCK_HOME, "adb-server.json"), JSON.stringify({ pid: 4242, port: 5038, startedAt: 1 }), @@ -2503,8 +2423,6 @@ describe("discoverDrivers on a host with an Android SDK", () => { join(SIMLOCK_HOME, "adb-server.json"), JSON.stringify({ pid: 4242, port: 5038, startedAt: 1 }), ); - // Only the console-port lookup a boot starts with is scripted: the emulator launch after it - // is recorded and then refused, so the boot stops right after showing its argv. const processRunner = new ScriptedProcessRunner([ { match: { args: ["devices"], command: "/android-sdk/platform-tools/adb" }, @@ -2644,7 +2562,6 @@ describe("discoverDrivers on a host with an Android SDK", () => { ]); }); - /** The minimum layout `discoverSdk` accepts, in memory. */ async function androidSdk(): Promise { const filesystem = new MemoryFilesystem(); process.env.ANDROID_HOME = "/android-sdk"; @@ -2697,7 +2614,6 @@ const SIMLOCK_HOME = "/home/.simlock"; const IOS_ROOT = join(SIMLOCK_HOME, "devices", "ios"); const INSTANCE_ID = "instance-1"; -/** A root marked by a different Simlock instance -- the cheapest real `OwnedRootError`. */ async function foreignOwnedRoot(): Promise { const filesystem = new MemoryFilesystem(); await filesystem.mkdirp(IOS_ROOT); @@ -2713,7 +2629,6 @@ async function foreignOwnedRoot(): Promise { return filesystem; } -/** The very `OwnedRootError` the driver raises for that root, never a hand-written copy. */ async function refusedRoot(filesystem: MemoryFilesystem): Promise { try { await IosSimctlDriver.create({ @@ -2731,7 +2646,6 @@ async function refusedRoot(filesystem: MemoryFilesystem): Promise { it("writes no daemon log line for component.installed and device.slimmed", async () => { const directory = await mkdtemp(join(tmpdir(), "simlock-main-event-file-")); temporaryDirectories.push(directory); - // The daemon's own bus is reachable only through what it hands discovery. process.env.SIMLOCK_DRIVERS_MODULE = await writeModule( `export function createDrivers({ eventBus }) { eventBus.emit("component.installed", { componentId: "18.6", durationMs: 1, platform: "ios" }, "test"); @@ -3098,7 +3011,6 @@ describe("discoverDrivers with SIMLOCK_DRIVERS_MODULE", () => { }); it("a throwing event subscriber writes one JSON line through the logger and nothing through console.error", async () => { - // The drivers module is the one seam that reaches the daemon's own bus from outside. process.env.SIMLOCK_DRIVERS_MODULE = await writeModule( `export function createDrivers(context) { context.eventBus.subscribe("daemon.started", () => { @@ -3135,7 +3047,6 @@ describe("discoverDrivers with SIMLOCK_DRIVERS_MODULE", () => { `export function createDrivers() { return []; }`, ); - // No `prerequisiteChecks` export: `doctor` runs no check rather than the real ones. await expect(discover(new MemoryLogSink())).resolves.toEqual({ drivers: [], prerequisiteChecks: [], @@ -3168,7 +3079,6 @@ interface MinimalClient { ): Promise<{ readonly ok: boolean; readonly payload?: unknown }>; } -/** Minimal newline-delimited-JSON client, mirroring the real daemon protocol framing. */ function connectClient(socketPath: string): Promise { return new Promise((resolve, reject) => { const socket = connect(socketPath); @@ -3218,7 +3128,7 @@ async function connectRetrying(socketPath: string, timeoutMs = 2_000): Promise { throw Object.assign(new Error("statfs failed"), { code: "EIO" }); @@ -3321,7 +3227,7 @@ describe("startDaemon logger wiring", () => { it("startDaemon hands its logger to the cleanup reaper: a scheduled cleanup run that fails is logged", async () => { const { sink } = await start({ filesystem: new UnmeasurableDiskFilesystem() }); - // `daemon.started` triggers a run nobody awaits; its failure has no caller to reach. + // why: `daemon.started` triggers a run nobody awaits; its failure has no caller to reach. await pollUntil(() => sink.records.some((record) => record.message === "scheduled cleanup run failed"), ); @@ -3337,7 +3243,7 @@ describe("startDaemon logger wiring", () => { }); it("startDaemon hands the gateway dispatcher its error classifier: a typed refusal is logged with its code at info, not as INTERNAL at error", async () => { - // A gateway serves HTTP; on its default port two overlapping runs of this file collide. + // why: A gateway serves HTTP; on its default port two overlapping runs of this file collide. const { daemon, sink } = await start({ configOverrides: { http: { enabled: true, host: "127.0.0.1", port: await freeLoopbackPort() }, @@ -3372,8 +3278,6 @@ describe("startDaemon logger wiring", () => { }); }); -/** Polls until `condition` holds or the time runs out, without failing: the caller's own - * assertion afterwards is what names a miss. */ async function pollUntil(condition: () => boolean, timeoutMs = 2_000): Promise { const deadline = Date.now() + timeoutMs; while (!condition() && Date.now() < deadline) { diff --git a/src/daemon/main.ts b/src/daemon/main.ts index 61fc3738e..3540b583e 100644 --- a/src/daemon/main.ts +++ b/src/daemon/main.ts @@ -238,7 +238,6 @@ function updateOptionsFor( }; } -/** Constructs the daemon's real adapters once; all state remains in the daemon. */ export async function startDaemon(options: StartDaemonOptions = {}): Promise { let opened: StateDatabase | undefined; let check: VersionCheck | undefined; @@ -310,7 +309,7 @@ async function composeDaemon( for (const message of configWarnings) logger.warn(message); if (dataDirectoryWarning !== undefined) logger.warn(dataDirectoryWarning); const processRunner = processRunnerFor(config.log.level, baseProcessRunner, logger, clock); - // ADR 0008 §6: the machine is read once, here. Bounded and never throwing, so a slow or + // why: ADR 0008 §6: the machine is read once, here. Bounded and never throwing, so a slow or // missing `sw_vers` costs a fallback value, not the daemon. const hostSystem = await ( options.hostInfo ?? new NodeHostInfo({ platform: process.platform, processRunner }) @@ -357,10 +356,6 @@ async function composeDaemon( secrets: new CryptoTokenSecrets(), }); const auth = { ...created, adminSecret: reportingAdminSecret(created.adminSecret, options) }; - // ADR 0005 §1/§2: one process, one mode. A gateway starts no drivers, validates no device - // roots, loads no registry, and runs no reaper, health monitor or capacity strategy -- so the - // branch is here, before any of that is built, rather than as a set of conditionals threaded - // through the worker's composition below. const npm = options.npm ?? new NodeNpmCli({ runner: baseProcessRunner, filesystem, dataDirectory }); const versionCheck = startVersionCheck({ @@ -408,7 +403,7 @@ async function composeDaemon( }, state, }); - // Before discovery, because every root a driver validates is checked against it, and it + // why: Before discovery, because every root a driver validates is checked against it, and it // is written exactly once per home and never regenerated (ADR 0001, decision 2). const instanceId = await loadInstanceId({ filesystem, @@ -441,8 +436,6 @@ async function composeDaemon( prerequisiteChecks: options.prerequisiteChecks ?? [], rejections: [], }; - // ADR 0008 §7: read at startup in the background, served from memory, re-read when stale. - // Nothing waits on the first read; `status.get` reports no tools until it lands. const hostFacts = new HostFactsReader({ clock, drivers, @@ -458,10 +451,7 @@ async function composeDaemon( hostLoadProbe === null ? undefined : new HostLoadSampler({ clock, logger: logger.child("host-load"), probe: hostLoadProbe }); - // One gate for every registry write: leasing's, core's and the installer's records. const decisions = new SerializedDecision(); - // The one caller of `Driver.installComponent` (ADR 0010 §3). Its `DiskSpaceGuard` is the only - // one, so an iOS and an Android install see each other's reservations. const components = new ComponentInstaller({ clock, decisions, @@ -528,7 +518,6 @@ async function composeDaemon( tokens: auth.tokens, role: "worker", }); - // Builds leasing on top of core (ADR 0018 §2). const leasing = createLeasing({ clock, components, @@ -543,7 +532,7 @@ async function composeDaemon( modelPreferences, }); cancelPendingRequest = (requesterId) => leasing.cancelPending(requesterId); - // Core declared ports only leasing can implement; this is the one place they are handed over, + // why: Core declared ports only leasing can implement; this is the one place they are handed over, // before any request is admitted (ADR 0018 §2). core.connect(leasing.corePorts); const reaper = new CleanupReaper({ @@ -558,11 +547,6 @@ async function composeDaemon( targetedDevices: () => core.targetedDevices(), }); const nuke = new Nuke({ executor: core.nuke, registry }); - /** - * ADR 0005 §3/§4: two config keys join a fleet. Undefined here means this worker has no - * gateway, which is every worker today -- and the daemon is otherwise completely unchanged - * either way, which is the point of the uplink being outbound. - */ const gatewayUplink = config.gateway.url === undefined || config.gateway.token === undefined ? undefined @@ -576,11 +560,11 @@ async function composeDaemon( workerId: instanceId, ...(config.gateway.label === undefined ? {} : { label: config.gateway.label }), }); - // Reassigned once the HTTP frontend actually starts, but must exist as a stable closure now: + // why: Reassigned once the HTTP frontend actually starts, but must exist as a stable closure now: // `stopAuxiliary` is fixed at `DaemonServer` construction time, before the frontend itself // exists. let stopHttpGateway: (() => Promise) | undefined; - // Settles once the HTTP gateway either finishes starting or fails to -- resolved/rejected from + // why: Settles once the HTTP gateway either finishes starting or fails to -- resolved/rejected from // inside `onSocketClaimed`'s handler below. `startDaemon()` awaits this alongside // `daemon.start()` itself (see the bottom of this function) so a bind failure (occupied port, // invalid host) makes `startDaemon()` reject and the entrypoint report a non-zero exit code, @@ -594,8 +578,6 @@ async function composeDaemon( // immediately. let resolveGatewayStarted: (() => void) | undefined; let rejectGatewayStarted: ((error: unknown) => void) | undefined; - /** Whether `onSocketClaimed` ever fired, i.e. whether anything will ever settle - * `gatewayStarted`. See the `finally` on `daemon.start()` below. */ let socketClaimed = false; const gatewayStarted: Promise = config.http.enabled ? new Promise((resolve, reject) => { @@ -603,7 +585,7 @@ async function composeDaemon( rejectGatewayStarted = reject; }) : Promise.resolve(); - // ADR 0019 §1, in this order: settle the open requests, read each platform once, doctor's + // why: ADR 0019 §1, in this order: settle the open requests, read each platform once, doctor's // startup pass on that read, end every lease whose device is not running and restore the // timers of the rest, then core's device convergence, then the queue's first depth, which every // run begins with. One after another: the reconciler needs the read, and convergence must not @@ -640,7 +622,7 @@ async function composeDaemon( leasing.announceQueueDepth(); }; const daemon = new DaemonServer({ - // Closes the warm pool the moment a stop is asked for, ahead of every await in the stop. + // why: Closes the warm pool the moment a stop is asked for, ahead of every await in the stop. beginStop: () => core.closeWarmPool(), // why: the source's name is the config key's; the core names no module, so the composition supplies it. capabilitySources: [...moduleHost.capabilitySources], @@ -712,12 +694,12 @@ async function composeDaemon( await leasing.settle(); await core.drain(); }, - // Every driver is tried even when another throws: Android's disposal is the only thing that can stop the adb server it started. + // why: Every driver is tried even when another throws: Android's disposal is the only thing that can stop the adb server it started. dispose: async () => { versionCheck.dispose(); core.dispose(); leasing.dispose(); - // Before the drivers: a running install is ended through its signal while its driver + // why: Before the drivers: a running install is ended through its signal while its driver // can still stop the installer process. await components.close(); await moduleHost.beforeDriversDispose(); @@ -733,7 +715,7 @@ async function composeDaemon( await eventHistory.stop(); state.close(); }, - // Review finding S5: waits for the concurrently-started gateway to either finish binding + // why: Review finding S5: waits for the concurrently-started gateway to either finish binding // (so `stopHttpGateway` is assigned and can actually be called) or fail to bind (nothing to // stop) *before* returning -- `#stop()` awaits this call before settling and disposing // (see `server.ts`; the leases themselves are left alone, ADR 0004 §3), so by the time @@ -746,13 +728,13 @@ async function composeDaemon( await moduleHost.beforeDrain(); await gatewayStarted.catch(() => undefined); await stopHttpGateway?.(); - // Stopped with the other auxiliary frontends, and for the same reason: the uplink is a + // why: Stopped with the other auxiliary frontends, and for the same reason: the uplink is a // live connection into this daemon's dispatcher, so it must be shut before lease/queue // teardown begins. Stopping it also stops the reconnect loop, so a shutting-down worker // does not redial the gateway it just left. await gatewayUplink?.stop(); }, - // ADR 0003 §2: "an HTTP request during startup now waits like a socket request instead of + // why: ADR 0003 §2: "an HTTP request during startup now waits like a socket request instead of // being refused". Firing the gateway's own `start()` from here (not after `daemon.start()` // resolves) is what makes that true: the gateway is listening, and every route calls // `daemon.dispatch(...)`, which parks on the same startup-readiness gate a socket request @@ -818,7 +800,7 @@ async function composeDaemon( await gateway.stop(); app.dispose(); }; - // No self-stop check here: `stopAuxiliary` above is the single place that decides whether + // why: No self-stop check here: `stopAuxiliary` above is the single place that decides whether // to stop the gateway, and it does so by awaiting `gatewayStarted` (settled once this // function's caller -- `onSocketClaimed`'s handler below -- resolves or rejects) before // calling `stopHttpGateway`. Stopping here too would double-stop it. @@ -841,7 +823,7 @@ async function composeDaemon( }); const [daemonResult, gatewayResult] = await Promise.allSettled([daemonStarted, gatewayStarted]); if (gatewayResult.status === "rejected" && daemonResult.status === "fulfilled") { - // The daemon itself came all the way up -- convergence succeeded, `daemon.started` was + // why: The daemon itself came all the way up -- convergence succeeded, `daemon.started` was // already emitted -- but its HTTP gateway never bound. Tear the whole thing down now, // *after* that success is a settled fact, so `daemon.stopping` never precedes (or follows // out of order) `daemon.started`; see this function's own comment above `onSocketClaimed` @@ -864,7 +846,6 @@ interface GatewayDaemonOptions { readonly filesystem: Filesystem; readonly gatewayRelay?: (relay: GatewayRelay) => void; readonly gatewayUplinks?: (uplinks: WebSocketUplinkListenerFactory) => void; - /** The gateway's own machine. It runs no drivers, so its host facts carry no tools. */ readonly hostSystem: HostSystem; readonly hostLoadProbe?: HostLoadProbe | null; readonly idGenerator: IdGenerator; @@ -880,17 +861,6 @@ interface GatewayDaemonOptions { readonly webhookPoster?: WebhookPoster; } -/** - * A daemon in gateway mode (ADR 0005 §2, §32): no drivers, no registry, no reaper, no health - * monitor, no capacity strategy. What it has instead is a worker registry fed by uplinks, and - * `GatewayDispatcher` answering the same contract from those views. - * - * Two things are unconditional here where a worker makes them optional, and both follow from - * §2's "a gateway always listens on HTTP (it is the fleet's contact point) and on its unix - * socket": HTTP is started (the config loader refuses `http.enabled: false` in this mode), and - * the uplink listener is attached to that same HTTP server, so the whole fleet has exactly one - * inbound port. - */ // fallow-ignore-next-line complexity -- explicit production composition, exactly like `startDaemon`'s worker half. async function startGatewayDaemon(options: GatewayDaemonOptions): Promise { const { @@ -905,18 +875,14 @@ async function startGatewayDaemon(options: GatewayDaemonOptions): Promise leaseIndex.all().map((entry) => ({ id: entry.gatewayLeaseId, ownerId: entry.ownerId })), logger: logger.child("server"), ownerRoutedFacts: gatewayOwnerRoutedFacts, resolveRole: { resolve: (hello) => auth.resolveHelloRole(hello) }, version: options.version, - // A gateway's "convergence" is starting to accept uplinks and arming the refresh tick. + // why: A gateway's "convergence" is starting to accept uplinks and arming the refresh tick. // Running it here rather than before `start()` keeps one lifecycle: a listener that cannot // start fails the daemon start, and `status.get` answers throughout (a fleet with no views // yet is a true answer, not an unready one). A worker that dials in the window before this @@ -1137,32 +1088,21 @@ async function startGatewayDaemon(options: GatewayDaemonOptions): Promise uplinks.attach(server), port: config.http.port, }); @@ -1192,7 +1132,7 @@ async function startGatewayDaemon(options: GatewayDaemonOptions): Promise undefined); @@ -1220,22 +1160,18 @@ function errorMessage(error: unknown): string { } export interface DriverDiscoveryContext { - /** Threaded into the Android driver's `downloads.acceptAndroidLicenses` gate; defaults to `false` when omitted. */ readonly acceptAndroidLicenses?: boolean; readonly clock: Clock; - /** Whole `drivers` config section: each driver is handed its own block, unread. */ readonly driversConfig: Config["drivers"]; - /** Bridges the iOS driver's `device.slimmed` fact onto the bus -- see `emitSlimDiagnostic`. */ readonly eventBus: Pick; readonly filesystem: Filesystem; /** - * Which platform's tooling this host has, `process.platform` in production and supplied + * why: Which platform's tooling this host has, `process.platform` in production and supplied * by the composition root rather than read here. Discovery's fail-closed branch -- the * one that must cost a platform and never the daemon -- is otherwise reachable only on a * Mac, and so is untestable everywhere Simlock's own CI runs. */ readonly hostPlatform: NodeJS.Platform; - /** The host port's architecture; the Android driver prefers images of the ABI it runs. */ readonly hostArch: string; readonly idGenerator: IdGenerator; readonly instanceId: string; @@ -1244,31 +1180,14 @@ export interface DriverDiscoveryContext { readonly processSupervisor: ProcessSupervisor; readonly simlockHome: string; readonly tcpProbe: TcpProbe; - /** - * How the iOS driver makes a slim device (`ios.slim` in config), whatever the default mode. - * Never threaded into the Android driver -- slim is iOS-only until Android slim lands. Omitted - * means the iOS driver slims nothing. - */ readonly slim?: { readonly categories?: readonly string[]; readonly bootTimeoutMs: number; }; - /** - * Whether `ios.defaultMode` is `slim`, so the iOS driver reports runtimes it cannot slim in - * `doctor` (ADR 0007 §14). The driver is told this boolean, never the default itself. - */ readonly slimByDefault?: boolean; - /** - * How the Android driver launches emulators (`android.emulator` in config). Handed over - * unread; omitted or undefined leaves the driver's own default launch untouched. - */ readonly androidEmulator?: AndroidEmulatorLaunchOptions; } -/** - * Drivers that started, and the platforms that refused to -- both are startup outcomes -- with - * the prerequisite checks for every platform this host could run, started or not. - */ export interface DriverDiscovery { readonly drivers: readonly Driver[]; readonly prerequisiteChecks: readonly PrerequisiteCheck[]; @@ -1314,7 +1233,7 @@ export async function discoverDrivers(options: DriverDiscoveryContext): Promise< const logger = options.logger.child("driver-discovery"); const driversModule = process.env.SIMLOCK_DRIVERS_MODULE; if (driversModule !== undefined) { - // A substituted driver set owns whatever roots it wants, so there is nothing here to + // why: A substituted driver set owns whatever roots it wants, so there is nothing here to // refuse on its behalf. return { ...(await loadDriversModule(driversModule, options, logger)), rejections: [] }; } @@ -1342,11 +1261,6 @@ export async function discoverDrivers(options: DriverDiscoveryContext): Promise< }; } -/** - * The checks `doctor` runs for the platforms this host could run: iOS only on macOS, as - * discovery itself decides, and Android everywhere. Built whether or not the platform's - * driver started, because a driver that did not start is exactly when they are needed. - */ export function platformPrerequisiteChecks(options: { readonly env: Readonly>; readonly filesystem: Filesystem; @@ -1370,7 +1284,7 @@ export function platformPrerequisiteChecks(options: { } /** - * A refused root costs the daemon one platform, never the whole daemon: the other platform + * why: A refused root costs the daemon one platform, never the whole daemon: the other platform * may be perfectly healthy, and a daemon that will not start is a daemon that cannot tell * anyone why (safety rule 9). Every other failure still fails startup -- an unreadable root * is already an `OwnedRootError`, so what is left is a genuine bug. @@ -1406,7 +1320,7 @@ async function discoverIosDriver( simlockHome: options.simlockHome, ...(options.slim === undefined ? {} : { slim: options.slim }), ...(options.slimByDefault === undefined ? {} : { slimByDefault: options.slimByDefault }), - // Ambient like `homedir()` above, and read here rather than in the driver so the + // why: Ambient like `homedir()` above, and read here rather than in the driver so the // composition root stays the only place that touches process state. ...(process.getuid === undefined ? {} : { uid: process.getuid() }), pid: process.pid, @@ -1442,7 +1356,7 @@ export function logAndroidDiagnostic(logger: Logger, diagnostic: AndroidDriverDi } /** - * Android refuses for one more reason than iOS does -- it needs a private adb server as + * why: Android refuses for one more reason than iOS does -- it needs a private adb server as * well as an owned root -- and both refusals cost the platform rather than the daemon. * A missing SDK is not a refusal at all: this host simply has no Android tooling, which is * ordinary and reported by the absence of the platform. @@ -1470,7 +1384,7 @@ async function discoverAndroidDriver( simlockHome: options.simlockHome, tcpProbe: options.tcpProbe, tcpStreamConnector: new NodeTcpStreamConnector(), - // Ambient like `homedir()` above, and read here rather than in the driver so the + // why: Ambient like `homedir()` above, and read here rather than in the driver so the // composition root stays the only place that touches process state. ...(process.getuid === undefined ? {} : { uid: process.getuid() }), pid: process.pid, @@ -1533,16 +1447,6 @@ function rootRejection( }; } -/** - * Testing/advanced hook: substitutes real driver discovery with a module supplied via - * `SIMLOCK_DRIVERS_MODULE`. The daemon always runs as a separately spawned process, so - * the module is resolved as a file path (relative to `process.cwd()`) and dynamically - * imported -- this is how the e2e suite injects a scriptable fake driver without the - * daemon ever knowing it isn't talking to real hardware. A missing module, an import - * error, or a module without a `createDrivers` export fails daemon startup loudly - * rather than silently falling back to real discovery. Its optional `prerequisiteChecks` - * export replaces the real checks too; without one, `doctor` runs none. - */ async function loadDriversModule( modulePath: string, context: DriverDiscoveryContext, @@ -1572,12 +1476,6 @@ async function loadDriversModule( return { drivers, prerequisiteChecks: imported.prerequisiteChecks ?? [] }; } -/** - * What the config says about device modes, in the two shapes the composition root hands out - * (ADR 0007 §2, §14): the worker's default mode per platform for leasing, and whether - * the iOS default is slim for the iOS driver's advisory. Android has no key until Android slim - * lands, so it is left out and falls to the core's own `"full"`. - */ export function deviceModeWiring(config: Pick): { readonly defaultModes: Readonly>>; readonly slimByDefault: boolean; @@ -1588,11 +1486,6 @@ export function deviceModeWiring(config: Pick): { }; } -/** - * The one place a class's preference list is merged (ADR 0015 §4): the operator's names from - * `ios.defaultModels` or `android.defaultModels` first, then the driver's built-in ones, per - * platform that has a driver. The catalog reads the result; nothing else builds the list. - */ export function modelPreferenceWiring( config: Pick, drivers: readonly Pick[], @@ -1674,11 +1567,6 @@ function stopForRefusedHistory(daemon: { stop(reason: string): Promise }): }; } -/** - * The runner every driver and `device.exec` shell out through. At `log.level: debug` each - * command leaves a `process` line; at any other level the runner is used unwrapped, so a - * daemon not asked for that detail pays nothing for it. - */ export function processRunnerFor( level: LogLevel, runner: ProcessRunner, @@ -1689,7 +1577,6 @@ export function processRunnerFor( return new LoggingProcessRunner({ clock, inner: runner, logger: logger.child("process") }); } -/** Writes a failing event subscriber to the daemon log as one JSON line, not to stderr. */ export function eventBusLogger(logger: Logger): EventBusLogger { const bus = logger.child("bus"); return { diff --git a/src/gateway/dispatcher.test.ts b/src/gateway/dispatcher.test.ts index cdf07cbfe..d5195238b 100644 --- a/src/gateway/dispatcher.test.ts +++ b/src/gateway/dispatcher.test.ts @@ -143,21 +143,14 @@ class FakeTokens implements TokenOperations { async revoke(id: string) { this.revoked.push(id); - // Only "tok_1" is ever actually minted by this fake -- anything else is the "already gone, - // or never real" case C-2's test below needs a real `false` for. return id === "tok_1"; } } -/** Matches `service.test.ts`'s `principal: "gw:instance-1"` -- ADR 0005 §14/§27's own-lease - * prefix is that principal plus a trailing `:`, the same shape `worker-registry.test.ts` uses. */ const GATEWAY_REQUESTER_PREFIX = "gw:instance-1:"; -/** The gateway's own machine, as the composition root hands it over: read once, no tools. */ const GATEWAY_HOST = { arch: "x64", os: "Linux", osVersion: "6.8.0", tools: [] }; -/** A `WorkerDirectory` a test populates by hand -- see `fleet-coordinator.test.ts`'s own copy - * for the fuller doc comment; this file only ever needs one worker at a time. */ class FakeDirectory implements WorkerDirectory { readonly clients = new Map(); readonly refreshCalls: string[] = []; @@ -203,9 +196,7 @@ function harness( readonly historyPath?: string; readonly tokenRecords?: readonly { id: string; label?: string }[]; readonly withoutTokens?: boolean; - /** The gateway's `http` block; enabled on 127.0.0.1:4700 by default. */ readonly http?: (typeof gatewayConfig)["http"]; - /** The gateway's own health; `running` by default. */ readonly health?: "starting" | "running" | "failed"; readonly load?: ConstructorParameters[0]["load"]; /** why: real alerts over a fake poster, wired as `testDestination`; none leaves the option out. */ @@ -225,7 +216,6 @@ function harness( retentionMs: 24 * 60 * 60_000, }); const tokens = new FakeTokens(options.tokenRecords); - /** C-2: every id `closeUplinksForToken` was actually called with, in call order. */ const closedUplinkTokens: string[] = []; const directory = new FakeDirectory(); const leaseIndex = new FleetLeaseIndex(GATEWAY_REQUESTER_PREFIX); @@ -284,7 +274,6 @@ function harness( host: GATEWAY_HOST, ...(options.load === undefined ? {} : { load: options.load }), leaseIndex, - // `classifyError`'s answer for the errors this dispatcher throws itself. errorCode: (error) => (error instanceof DispatchError ? error.code : undefined), logger: new JsonLinesLogger({ clock, module: "gateway", sink: logSink }), ...(alerts === undefined ? {} : { testDestination: (name) => alerts.test(name) }), @@ -318,7 +307,6 @@ function session(overrides: Partial = {}): DispatchSession { }; } -/** Every operation the contract declares, minus `daemon.stop` (the transport answers it). */ const EVERY_OPERATION = (Object.keys(OPERATIONS) as OperationName[]).filter( (name) => name !== "daemon.stop", ); @@ -351,8 +339,6 @@ describe("GatewayDispatcher", () => { 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"); - // The line `runDispatch` writes for the same call through any other dispatcher, a worker's - // included: only the module differs. const otherSink = new MemoryLogSink(); const other = (role: "admin" | "agent") => runDispatch("worker.drain", { workerId: "wrk_1" }, session({ role }), { @@ -835,7 +821,7 @@ describe("GatewayDispatcher", () => { await expect(dispatcher.dispatch("list.get", { kind: "devices" }, session())).resolves.toEqual([ expect.objectContaining({ id: "dev_1", workerId: "wrk_1" }), ]); - // Cleanup rules are a machine's own configuration, and a gateway runs no reaper. + // why: Cleanup rules are a machine's own configuration, and a gateway runs no reaper. await expect(dispatcher.dispatch("list.get", { kind: "rules" }, session())).resolves.toEqual( [], ); @@ -1017,7 +1003,7 @@ describe("GatewayDispatcher", () => { ttlMs: id === "lease_1" ? 111 : 222, }); workers.refresh("wrk_1", { leases: [owned("lease_1", "dev_1"), owned("lease_2", "dev_2")] }); - // An entry for a worker the gateway holds no view of, as a race with a removal leaves it. + // why: An entry for a worker the gateway holds no view of, as a race with a removal leaves it. leaseIndex.add({ gatewayLeaseId: "wrk_gone.lease_9", grantedAt: 1, @@ -1039,41 +1025,19 @@ describe("GatewayDispatcher", () => { ]); }); - // P-1 (third review round): the previous version of `#leaseList` compared a namespaced form - // of the session's own principal to each lease's `ownerId` -- a comparison that was false by - // construction (see `#leaseList`'s own comment) and so, in practice, indistinguishable from - // simply returning `[]`. - // - // Round 6 review: this test's title used to claim a non-admin session sees "no fleet leases at - // all", which #118 -- this PR -- made false when it replaced the handler with `FleetLeaseIndex`: - // such a session now does see leases this gateway issued to it, as the third test below proves. - // What this one actually pins is the baseline the other two build on: a worker's *own local* - // lease is not a fleet lease and is never a candidate, whoever asks. The title says only that - // now (testing rule 1). it("shows a non-admin session none of a worker's own local leases", async () => { const { dispatcher, workers } = harness(); workers.connected("wrk_1", undefined, undefined); workers.refresh("wrk_1", { leases: [leaseFixture("lease_1", "dev_1")] }); - // The worker's lease is a worker-local one -- never indexed by this gateway (only - // `rebuildFromWorker`/an actual grant adds an entry) -- so a non-admin session holds none of - // it, exactly as an unrelated fleet client should. await expect( dispatcher.dispatch("lease.list", {}, session({ principal: "someone", role: "agent" })), ).resolves.toEqual({ leases: [] }); }); - // #118 replaced the P2 namespace-comparison this test used to pin with the lease index: a - // non-admin session's `lease.list` now iterates `leaseIndex.all()` rather than scanning every - // worker's raw leases, so a worker's own local lease is never even a candidate -- it cannot - // "match" a session by coincidence of principal spelling, because it is never looked up by - // principal at all. Still worth its own test: this is the case that would tell a correct - // implementation apart from a naive "compare raw principals" one. it("does not let a gateway session's raw principal match a worker's own local lease owner of the same name", async () => { const { dispatcher, workers } = harness(); workers.connected("wrk_1", undefined, undefined); - // `leaseFixture`'s ownerId ("agent-1") is an un-namespaced *local* principal -- exactly what - // a worker's own agent looks like, and never added to this gateway's lease index. workers.refresh("wrk_1", { leases: [leaseFixture("lease_1", "dev_1")] }); await expect( @@ -1081,11 +1045,6 @@ describe("GatewayDispatcher", () => { ).resolves.toEqual({ leases: [] }); }); - // The positive match: a lease this gateway issued, rebuilt purely from a worker's reported - // view (§30's reconnect path -- see `fleet-coordinator.test.ts` for the admission-path - // equivalent), is rewritten to its gateway id and fleet-level requester and is visible to the - // owner that requested it (ADR §27a: `ownerId` is the real principal, round-tripped -- not - // namespaced, unlike `requesterId`). it("sees a gateway-issued lease rebuilt from a worker's view, rewritten to its gateway id", async () => { const { dispatcher, workers } = harness(); workers.connected("wrk_1", "mac-mini-1", undefined); @@ -1094,10 +1053,6 @@ describe("GatewayDispatcher", () => { ownerId: "agent-1", requesterId: `${GATEWAY_REQUESTER_PREFIX}agent-1`, }; - // `workers.refresh` alone is enough: `FleetLeaseCoordinator` is subscribed to this same - // registry's `onViewsChanged` and rebuilds its lease index from every view it sees, exactly - // as it would from a real `WorkerLink`'s refresh -- nothing in this test drives the index by - // hand. workers.refresh("wrk_1", { leases: [lease] }); await expect( @@ -1341,11 +1296,6 @@ describe("GatewayDispatcher", () => { }); }); - // C-2, ADR 0005 §8 ("revoking closes the uplink"): `#tokenRevoke` used to only write the - // store -- the join token was checked once, at upgrade, and nothing ever re-verified a live - // link, so revoking a worker's token had no observable effect until that worker happened to - // reconnect on its own, possibly never. This asserts the actual wiring `main.ts` depends on: - // a successful revoke calls `closeUplinksForToken` with exactly the revoked id. it("closes the uplink a revoked token authorized, not merely the store entry (C-2)", async () => { const { closedUplinkTokens, dispatcher } = harness(); @@ -1356,8 +1306,6 @@ describe("GatewayDispatcher", () => { expect(closedUplinkTokens).toEqual(["tok_1"]); }); - // The other half of the same wiring: revoking an id the store never had is not a live link to - // close either, and must not call the hook with a token id that authorized nothing. it("does not try to close an uplink for a token id that was never real (C-2)", async () => { const { closedUplinkTokens, dispatcher } = harness(); @@ -1591,7 +1539,6 @@ describe("GatewayDispatcher", () => { }); describe("worker.install-component (ADR 0010 §7)", () => { - /** One connected worker whose config has been read, scripted behind the directory. */ function withWorker() { const fixture = harness(); const worker = new ScriptedWorkerClient(); @@ -1833,7 +1780,7 @@ describe("GatewayDispatcher", () => { }); }); - // ADR §27a's whole point: the owner a rebuilt lease authorizes against has to be the real + // why: ADR §27a's whole point: the owner a rebuilt lease authorizes against has to be the real // principal the worker round-tripped back, not this gateway's own uplink principal -- and it // has to work on an index built purely by `rebuildFromWorker` (a reconnect), never through // `coordinator.request` in this process. This is the one place a naive "derive ownerId from @@ -2068,11 +2015,6 @@ describe("GatewayDispatcher", () => { ).rejects.toMatchObject({ code: "FORBIDDEN" }); }); - // C3 (round 3 review): exercises the gateway's *real* dispatcher end to end - // (`GatewayDispatcher` -> `FleetLeaseCoordinator#exec` -> a scripted worker), unlike - // `http/app.test.ts`'s own "opens the stream when the process starts" test, which drives - // `call.session.onStarted?.()` by hand and so cannot notice a dispatcher that stops calling - // it for a genuinely silent, long-running command. 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 () => { const { coordinator, directory, dispatcher, workers } = harness(); const client = new ScriptedWorkerClient(); @@ -2091,7 +2033,7 @@ describe("GatewayDispatcher", () => { { model: "iPhone 17", platform: "ios" }, { allowDownload: false, noWait: true, ownerId: "agent-1", requesterId: "agent-1" }, ); - // §19b's own worked example: `simctl install ` never writes anything while it runs, + // why: §19b's own worked example: `simctl install ` never writes anything while it runs, // and the worker here never answers at all -- but it does say the process exists, which is // the fact a transport needs to commit its 200. No clock is advanced below: this asserts // the relay, not a timer. @@ -2207,15 +2149,11 @@ describe("GatewayDispatcher", () => { }); it("has an answer for every operation the contract declares", async () => { - // Not one operation may fall through to `UNKNOWN_REQUEST` on a gateway: the contract is the - // same one every frontend renders, so "the gateway forgot about this operation" must be a - // failing test rather than a runtime surprise. Inputs here are only shaped well enough to - // reach the handler. const inputs: Partial> = { "driver.passthrough": { args: [], tool: "adb" }, "lease.release": { leaseId: "lease_1" }, "lease.renew": { leaseId: "lease_1" }, - // `noWait: true` matters here: with no worker connected, an ordinary "wait" request would + // why: `noWait: true` matters here: with no worker connected, an ordinary "wait" request would // sit in the fleet queue forever and hang this test rather than answer promptly. "lease.request": { model: "iPhone 17", noWait: true, platform: "ios" }, "token.create": { role: "agent" }, @@ -2242,7 +2180,6 @@ describe("GatewayDispatcher", () => { describe("GatewayDispatcher: waiting requests", () => { const SPEC = { model: "iPhone 17", platform: "ios" as const }; - /** A request a worker lists in its own `status.get`'s `waiting`. */ function onWorker( id: string, requesterId: string, @@ -2263,7 +2200,6 @@ describe("GatewayDispatcher: waiting requests", () => { }; } - /** Lets the coordinator's admission and dispatch pass run. */ async function settle(): Promise { for (let count = 0; count < 20; count += 1) await Promise.resolve(); } @@ -2273,8 +2209,6 @@ describe("GatewayDispatcher: waiting requests", () => { 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(); - // Two workers that can serve the request and have no free slot, whose own queues hold their - // local agents' requests. const capacity = statusFixture().capacity; for (const [workerId, requestId] of [ ["wrk_1", "req_w1"], @@ -2298,7 +2232,6 @@ describe("GatewayDispatcher: waiting requests", () => { ], }); } - // Both are busy, so the fleet request waits in the gateway's own queue. void dispatcher.dispatch( "lease.request", { ...SPEC, idempotencyKey: "key-1", requesterId: "fleet-agent" }, @@ -2327,7 +2260,6 @@ describe("GatewayDispatcher: waiting requests", () => { ]); expect(JSON.stringify(listed)).not.toContain("key-1"); expect(JSON.stringify(listed)).not.toContain("fleet-owner"); - // `status.get` carries the fleet queue only; each worker's own is on its view. const status = await dispatcher.dispatch("status.get", {}, session()); expect(status.waiting).toEqual([listed[0]]); }); @@ -2397,7 +2329,6 @@ describe("GatewayDispatcher: waiting requests", () => { const { dispatcher, directory, workers } = harness(); const client = new ScriptedWorkerClient(); directory.add("wrk_1", client); - // The worker takes the request and never answers, so it stays mid-dispatch. client.requestLeaseQueue.push({ kind: "hang" }); workers.connected("wrk_1", undefined, "0.3.0"); workers.refresh("wrk_1", { @@ -2417,8 +2348,6 @@ describe("GatewayDispatcher: waiting requests", () => { ); await settle(); expect(client.calls.filter((call) => call.startsWith("lease.request"))).toHaveLength(1); - // The worker lists the request it was sent under the gateway's requester id, beside a - // local agent's own. workers.refresh("wrk_1", { waiting: [ onWorker("req_from_gateway", `${GATEWAY_REQUESTER_PREFIX}fleet-agent`), diff --git a/src/gateway/dispatcher.ts b/src/gateway/dispatcher.ts index ba105aa2a..f919df034 100644 --- a/src/gateway/dispatcher.ts +++ b/src/gateway/dispatcher.ts @@ -77,7 +77,7 @@ type DeepReadonly = Value extends (infer Item)[] : Value; /** - * The config `config.get` answers with, as this module sees it: the contract's own shape, made + * why: The config `config.get` answers with, as this module sees it: the contract's own shape, made * deeply readonly so `src/core`'s `Config` (which is) assigns into it. Declared this way rather * than by importing `Config` because `src/gateway` imports nothing from `core` (ADR 0005 §33), * and the gateway has no business with the type's *meaning* anyway -- it stores this value and @@ -107,7 +107,6 @@ export interface GatewayUpdate { export interface GatewayDispatcherOptions { readonly historyPath?: string; - /** The gateway's own config -- what `config.get` returns (ADR 0005 §34). */ readonly config: GatewayConfig; readonly eventHistory: Pick; readonly workers: WorkerRegistry; @@ -116,22 +115,12 @@ export interface GatewayDispatcherOptions { * import. Posts one sample to the named alert destination and says what came of it. */ readonly testDestination?: (destination: string) => Promise; - /** Each worker's live link, for `worker.install-component` (ADR 0010 §7). `GatewayService` - * satisfies it. */ readonly directory: WorkerDirectory; readonly tokens?: TokenOperations; - /** - * C-2, ADR 0005 §8: "revoking closes the uplink". Called after a successful `token.revoke` - * with the revoked token's id, so any uplink that token authenticated is closed immediately - * rather than only at that worker's next reconnect (uplinks are long-lived, so without this a - * revoked worker token has no observable effect until the worker happens to redial). Optional - * so a test exercising `token.revoke` in isolation, with no `GatewayService` behind it, need - * not supply one -- revocation still writes the store either way. - */ readonly closeUplinksForToken?: (tokenId: string) => Promise; readonly clock: Clock; readonly logger?: Logger; - /** Classifies a thrown error for the `operation` log line (`classifyError` in production), + /** why: Classifies a thrown error for the `operation` log line (`classifyError` in production), * injected because `src/gateway` imports nothing from `src/core`. Unset or `undefined`: * `INTERNAL`. */ readonly errorCode?: (error: unknown) => string | undefined; @@ -262,7 +251,6 @@ export class GatewayDispatcher { "cleanup.run": unsupportedByDesign("cleanup.run"), "doctor.run": unsupportedByDesign("doctor.run"), "driver.passthrough": unsupportedByDesign("driver.passthrough"), - // ADR 0010 §7: a gateway owns no components and has no download policy of its own. "component.install": unsupportedByDesign( "component.install", "component.install installs on one machine; on a gateway, name the workers to install on " + @@ -276,7 +264,6 @@ export class GatewayDispatcher { "component.remove", "component.remove removes a component from one machine; run it against a worker", ), - // A device is plugged into one machine, and a gateway asks no worker. "device.add": unsupportedByDesign( "device.add", "device.add enrols a device plugged into one machine; run it on the worker the device is plugged into", @@ -294,7 +281,7 @@ export class GatewayDispatcher { "device.exec": this.#deviceExec, "relay.request": relayRefused, "relay.cancel": relayRefused, - // "daemon.stop" is absent for the same reason it is absent from the worker's table: the + // why: "daemon.stop" is absent for the same reason it is absent from the worker's table: the // transport intercepts it ahead of any dispatch (ADR 0003 §6's frozen exception). }; } @@ -424,9 +411,6 @@ export class GatewayDispatcher { #tokenRevoke: Handler<"token.revoke"> = async (input) => { const revoked = await this.#requireTokens().revoke(input.id); - // C-2: only a token that actually existed can have opened an uplink -- revoking an unknown - // id (already-gone, or never real) has nothing to close, same as it has nothing to forget - // in the store. if (revoked) await this.options.closeUplinksForToken?.(input.id); return { revoked }; }; @@ -531,8 +515,6 @@ export class GatewayDispatcher { workerId: input.workerId, }); - /** ADR 0010 §7: the gateway asks each worker; each worker's own policy decides. The HTTP - * stream opens once the targets are resolved, which is this session's `onStarted`. */ #workerInstallComponent: Handler<"worker.install-component"> = (input, session) => relayComponentInstall( { @@ -545,7 +527,7 @@ export class GatewayDispatcher { ); /** - * ADR 0005 §14/§20: a non-admin session sees only the fleet leases *it* owns -- resolved + * why: ADR 0005 §14/§20: a non-admin session sees only the fleet leases *it* owns -- resolved * through the lease index by `ownerId`, never by scanning every worker's raw leases and * comparing `ownerId` to `session.principal` directly. That comparison would let an agent * token on the gateway that happens to name itself the same as some worker's own local agent @@ -609,11 +591,6 @@ export class GatewayDispatcher { return undefined; } - /** Every lease this gateway issued to `ownerId`, projected to its fleet-facing form. Looks up - * each entry's current record from the owning worker's view rather than caching one on the - * index itself, so a renewed TTL/`lastRenewedAt` is always current. An entry whose worker view - * (or the specific lease on it) is momentarily missing -- a race with a reconnect rebuild, or - * a worker that just disconnected -- is skipped rather than fabricated. */ #ownedFleetLeases(ownerId: string) { const leases = []; for (const entry of this.options.leaseIndex.all()) { @@ -627,7 +604,7 @@ export class GatewayDispatcher { } /** - * The admin inspection of the fleet. `devices` and `leases` are the aggregate with + * why: The admin inspection of the fleet. `devices` and `leases` are the aggregate with * `workerId`; `rules` is empty, because cleanup rules are a machine's own configuration and a * gateway runs no reaper (ADR 0005 §2) -- an empty list says "this endpoint has none", which * is true, where a refusal would suggest the question was wrong. @@ -651,7 +628,7 @@ export class GatewayDispatcher { }; /** - * The fleet queue's requests, then each worker's own queue, each of those with its `workerId`. + * why: The fleet queue's requests, then each worker's own queue, each of those with its `workerId`. * A worker lists a request this gateway sent it under the gateway's own requester id (ADR 0005 * §27); the gateway already lists that request, so the worker's copy is left out. */ @@ -667,11 +644,6 @@ export class GatewayDispatcher { return [...this.options.coordinator.waitingRequests(), ...onWorkers]; } - /** Every lease every worker's view reports, each projected through the lease index -- rewritten - * to its gateway id/requester/`worker` for a lease this gateway issued (test: "lease.list - * rewrites ids only for gateway-issued leases"), passed through with only `workerId` added for - * a worker's own local lease. Admin-only visibility (`list.get`, and `#leaseList` for an admin - * session): a non-admin session never reaches this, see `#ownedFleetLeases` above. */ #fleetLeases() { return this.options.workers .views() @@ -679,7 +651,7 @@ export class GatewayDispatcher { } /** - * ADR 0005 §15: "the gateway's `lease.defaultTtlMs` fills in a request that names no `ttlMs` + * why: ADR 0005 §15: "the gateway's `lease.defaultTtlMs` fills in a request that names no `ttlMs` * and its `lease.maxTtlMs` caps what its own clients may ask for, both before anything is * dispatched". `owner` (§27a, narrowed round 3 review H3) is read only from `session. * isGatewayUplink` -- never merely `role === "admin"`, which HTTP's `operator` token also maps @@ -704,8 +676,6 @@ export class GatewayDispatcher { "Only the gateway's own uplink session may set lease.request's `owner` field", ); } - // ADR 0021 §1: gateways do not chain, so no session may send the field a gateway puts on its - // own dispatch to a worker. Refused whatever `noWait` says, as on a worker. if (input.fleetRequestId !== undefined) { throw new DispatchError( "FORBIDDEN", @@ -713,11 +683,11 @@ export class GatewayDispatcher { ); } return this.options.coordinator.request( - // Forwarded as it arrived (ADR 0007 §2): the gateway has no default mode of its own, and + // why: Forwarded as it arrived (ADR 0007 §2): the gateway has no default mode of its own, and // relays an image tag unread. requestedDevice(input), { - // `input.allowDownload` stays accepted and is not passed on: only installed runtimes + // why: `input.allowDownload` stays accepted and is not passed on: only installed runtimes // count through a gateway (ADR 0009 §3). noWait: input.noWait ?? false, ownerId: input.owner ?? session.principal, @@ -727,11 +697,10 @@ export class GatewayDispatcher { ? {} : { onAdmitted: session.onRequestAdmitted }), ...(input.idempotencyKey === undefined ? {} : { idempotencyKey: input.idempotencyKey }), - // ADR 0020: forwarded to the worker as sent, and the gateway's own lease ID when it grants. ...(input.leaseId === undefined ? {} : { leaseId: input.leaseId }), ...(input.requires === undefined ? {} : { requires: input.requires }), ...(input.timeoutMs === undefined ? {} : { timeoutMs: input.timeoutMs }), - // Always explicit past this point (§15): a request that named none is filled in here, + // why: Always explicit past this point (§15): a request that named none is filled in here, // before dispatch, rather than left for whichever worker happens to grant it to default // on its own -- which could differ fleet to fleet. ttlMs: input.ttlMs ?? this.options.config.lease.defaultTtlMs, @@ -745,8 +714,6 @@ export class GatewayDispatcher { return { result }; }; - /** ADR §15's cap applies here too -- "what its own clients may ask for" is any `ttlMs` a - * fleet client names, renew included, not only the initial request. */ #leaseRenew: Handler<"lease.renew"> = async (input) => { this.#requireTtlWithinCap(input.ttlMs); return this.options.coordinator.renew(input.leaseId, input.ttlMs); @@ -762,10 +729,6 @@ export class GatewayDispatcher { return { leaseIds: [...leaseIds] }; }; - /** ADR §19b: proxied to the worker that owns the device, over the same uplink as everything - * else. Ownership was already checked once, at the gateway (this operation's own `authorize` - * hook, via `leaseRequesterId`/`ownerId` above) -- the worker checks it again, against the - * namespaced requester `FleetLeaseCoordinator#exec` forwards (§19a'). */ #deviceExec: Handler<"device.exec"> = async (input, session) => this.options.coordinator.exec( { @@ -778,7 +741,7 @@ export class GatewayDispatcher { ); /** - * ADR §15/§4's cap, applied identically to a request and a renew. Lives here rather than in + * why: ADR §15/§4's cap, applied identically to a request and a renew. Lives here rather than in * the contract schema for the same reason the worker's own copy does: `lease.maxTtlMs` is a * daemon config value the contract module cannot see. */ @@ -800,7 +763,6 @@ export class GatewayDispatcher { } } -/** ADR 0005 §34: stays per-worker, permanently. */ function unsupportedByDesign( operation: OperationName, message = `${operation} acts on one machine's devices; run it against a worker`, diff --git a/src/gateway/service.ts b/src/gateway/service.ts index b32ce803c..65ba61150 100644 --- a/src/gateway/service.ts +++ b/src/gateway/service.ts @@ -1,11 +1,3 @@ -/** - * The gateway's moving parts, wired together: the uplink listener, one `WorkerLink` per - * connected worker, the worker registry those links write into, and the slow periodic tick that - * backstops event-driven refreshes and sweeps expired views (ADR 0005 §6, §7). - * - * Everything here is lifecycle. What an operation *answers* lives in `GatewayDispatcher`, and - * what a view *is* lives in `WorkerRegistry`; this class only makes sure both are fed. - */ import type { EventBus, WorkerRejectedReason } from "../bus/index.js"; import type { AcceptedUplink, @@ -24,7 +16,7 @@ import { WorkerLink, type WorkerClientFactory } from "./worker-link.js"; import { WorkerRegistry } from "./worker-registry.js"; /** - * How often the gateway refreshes every worker view and sweeps expired ones, absent an + * why: How often the gateway refreshes every worker view and sweeps expired ones, absent an * override. ADR 0005 §7 asks for "a slow periodic tick as a backstop" -- slow because the * uplink's event stream is the real signal and this only catches what it missed (an event lost * to a refresh that failed, a worker whose `events.subscribe` was refused). Thirty seconds is @@ -47,7 +39,7 @@ const DEFAULT_WORKER_REFRESH_INTERVAL_MS = WORKER_VIEW_REFRESH_INTERVAL_MS; export const REJECTION_COALESCE_WINDOW_MS = 1_000; /** - * P-2: bounds `#recentRejections` itself. Coalescing keys on the claimed worker id, which is + * why: P-2: bounds `#recentRejections` itself. Coalescing keys on the claimed worker id, which is * attacker-controlled (P3) -- without a cap, a flood that varies its claimed id every dial would * trade one unbounded structure (the event bus's ring buffer) for another (this map) rather than * actually fixing anything. Evicting the oldest window on overflow means a wide-enough flood @@ -70,18 +62,12 @@ export interface GatewayServiceOptions { credential: string | undefined, workerId: string | undefined, ) => Promise; - /** `gateway.disconnectedRetentionMs` (§6). */ readonly retentionMs: number; - /** This gateway's own `lease.maxTtlMs` (§15) -- threaded straight into the registry, which - * warns when a connected worker's own reported cap is lower. */ readonly leaseMaxTtlMs: number; - /** Where drained worker ids survive a restart (Decision 3). */ readonly drainStore?: DrainStore; - /** The principal the gateway announces to each worker at `hello`. */ readonly principal: string; readonly logger?: Logger; readonly refreshIntervalMs?: number; - /** Injected in tests to script a worker without a real daemon behind the uplink. */ readonly connect?: WorkerClientFactory; } @@ -93,7 +79,7 @@ export class GatewayService { #listener: UplinkListener | undefined; #tick: TimerHandle | undefined; #stopped = false; - /** P-2: open coalescing windows for `worker.rejected`, keyed by `${reason}:${claimed id}`. + /** why: P-2: open coalescing windows for `worker.rejected`, keyed by `${reason}:${claimed id}`. * Insertion order is what makes the overflow eviction in `#recordRejection` an LRU-ish * "drop the oldest key" rather than an arbitrary one -- `Map` preserves it. */ readonly #recentRejections = new Map(); @@ -104,9 +90,6 @@ export class GatewayService { this.#registry = new WorkerRegistry({ clock: options.clock, eventBus: options.eventBus, - // ADR 0005 §14's own-lease prefix, reused for §6's retention hold (M2): `principal` is - // exactly `gw:`, the same shape requirement 27 stamps on - // every requester id this gateway forwards, just without the trailing `:`. gatewayRequesterPrefix: `${options.principal}:`, leaseMaxTtlMs: options.leaseMaxTtlMs, logger: this.#logger, @@ -115,35 +98,21 @@ export class GatewayService { }); } - /** The views every gateway handler reads. */ get workers(): WorkerRegistry { return this.#registry; } - /** `./fleet-ports.ts`'s `WorkerDirectory#target` -- #118's way to resolve a worker id to its - * live link without importing `WorkerLink` or reaching into `#links` itself. */ target(workerId: string): WorkerLink | undefined { return this.#links.get(workerId); } - /** - * C-2, ADR 0005 §8: "revoking closes the uplink". Closes every currently-open link that - * authenticated with join token `tokenId` -- ordinarily at most one, but a token is a - * credential, not a worker id, so nothing stops the same secret from having opened more than - * one uplink before it was revoked. `link.close()` runs the same teardown a worker-initiated - * disconnect does (`registry.disconnected`, `#links` eviction via `onClosed`), so a revoked - * worker is indistinguishable in the fleet's view and its event history from one that simply - * hung up. Links that never got a `tokenId` (every uplink opened before this fix's `main.ts` - * wiring, or a test `authenticate` that returns the bare outcome string) are never matched by - * a real token id, so this is a no-op for them rather than a false positive. - */ async closeLinksForToken(tokenId: string): Promise { const matches = [...this.#links.values()].filter((link) => link.tokenId === tokenId); await Promise.all(matches.map((link) => link.close())); } async start(): Promise { - // Before the listener, not after: a worker that reconnects in the first millisecond must + // why: Before the listener, not after: a worker that reconnects in the first millisecond must // find its drain state already restored, or it would be dispatched to once (#118) before // the flag landed. await this.#registry.load(); @@ -165,15 +134,6 @@ export class GatewayService { this.#scheduleTick(); } - /** - * P-2: emits `worker.rejected` for the first refusal of a given (reason, claimed id) in a - * window and merely counts every one after it, until the window elapses -- rather than one - * event per dial, which an unauthenticated flood against `GET /v1/uplink` could otherwise use - * to pin the event bus's ring buffer full of refusals and evict everything else in it (P3). - * The next dial after the window closes carries the closed window's total as `count`, so an - * operator watching `simlock events` still learns *that* a flood happened and *how big* it - * was -- just not once per attempt. - */ #recordRejection( reason: WorkerRejectedReason, workerId: string | undefined, @@ -186,14 +146,8 @@ export class GatewayService { existing.count += 1; return; } - // A new window: either this key has never been seen, or its last one has aged out. Either - // way, this dial itself is reported immediately -- `count` names how many total refusals - // (including this one) the just-closed window absorbed, so a lone refusal keeps the exact - // pre-P-2 payload shape (no `count` field at all) rather than gaining a redundant `count: 1`. const previousTotal = existing?.count; if (this.#recentRejections.size >= MAX_TRACKED_REJECTION_WINDOWS) { - // Oldest insertion first, per `Map`'s iteration order -- a flood that varies its claimed - // id every dial must not grow this map without bound either (see the constant's doc). const oldestKey = this.#recentRejections.keys().next().value; if (oldestKey !== undefined) this.#recentRejections.delete(oldestKey); } @@ -225,7 +179,7 @@ export class GatewayService { void uplink.connection.close(); return; } - // A worker that reconnects while the gateway still holds its previous link (a half-open + // why: A worker that reconnects while the gateway still holds its previous link (a half-open // socket the OS has not reported dead) replaces it: the id is the worker's identity, and // the newest uplink is the one that can actually be driven. The old link is closed rather // than left to fight the new one over the same view. @@ -239,7 +193,7 @@ export class GatewayService { const link = new WorkerLink({ clock: this.options.clock, eventBus: this.options.eventBus, - // The same "is this still the current link" test as `onClosed` below, reused for D1: a + // why: The same "is this still the current link" test as `onClosed` below, reused for D1: a // stale link's late close (the OS finally reporting a half-open socket dead, well after a // reconnect already replaced it) must not mark the registry's *current* entry // disconnected. One predicate, read from the same map, keeps both guards from drifting @@ -247,7 +201,7 @@ export class GatewayService { isCurrentLink: () => this.#links.get(uplink.workerId) === link, logger: this.#logger, onClosed: (workerId) => { - // Only forget the link if it is still the current one: a replaced link's late close + // why: Only forget the link if it is still the current one: a replaced link's late close // must not evict its successor. if (this.#links.get(workerId) === link) this.#links.delete(workerId); }, @@ -268,7 +222,7 @@ export class GatewayService { } /** - * One timer, rearmed after each run rather than a repeating interval, so a slow round of + * why: One timer, rearmed after each run rather than a repeating interval, so a slow round of * refreshes can never overlap itself. Failures inside a refresh are the link's to log; this * loop only guarantees the next tick happens. */ @@ -281,8 +235,6 @@ export class GatewayService { } async #runTick(): Promise { - // Catalogs are re-read on the tick, not on every lease event: models and runtimes change - // when someone installs an SDK, which is exactly the kind of slow change a backstop is for. await Promise.all( [...this.#links.values()].map((link) => link.refresh({ includeCatalog: true })), ); diff --git a/src/gateway/worker-registry.ts b/src/gateway/worker-registry.ts index 7242530cd..fcdcea447 100644 --- a/src/gateway/worker-registry.ts +++ b/src/gateway/worker-registry.ts @@ -1,14 +1,3 @@ -/** - * The gateway's picture of its fleet: one `WorkerView` per worker it has seen, and the facts it - * emits as those views change (ADR 0005 §6, §8, §9, §22). - * - * Views are rebuilt over the uplink (§30: a gateway restart re-derives everything from the - * workers that reconnect). Exactly one thing on them is persisted -- the drained set, which no - * worker reports and which must survive both a worker reconnect and a gateway restart (Decision - * 3, see `./drain-store.ts`). This class owns *what a view is and when it changes*; `WorkerLink` - * owns the uplink that supplies the facts, and `GatewayDispatcher` owns answering operations - * from them. - */ import type { z } from "zod"; import type { EventBus, WorkerRejectedReason } from "../bus/index.js"; @@ -24,8 +13,6 @@ import type { DrainStore } from "./drain-store.js"; export type WorkerView = z.infer; -/** The fields a refresh over the uplink replaces. Everything else on a view -- identity, drain - * state, connection state -- is this registry's own bookkeeping, never a worker's to report. */ export type WorkerViewSnapshot = Pick< WorkerView, | "capabilities" @@ -48,12 +35,12 @@ export type WorkerViewSnapshot = Pick< | "warmPool" >; -/** A worker's device in the shape a grant carries it: what `GET /v1/leases/{id}` builds a lease +/** why: A worker's device in the shape a grant carries it: what `GET /v1/leases/{id}` builds a lease * payload from. Kept beside a view, never on it, because a view is what `status.get` and * `worker.list` show every agent, and a virtual device's `driverDeviceId` is only the lease holder's to see. */ export type WorkerGrantedDevice = z.infer; -/** What one refresh over the uplink commits: the view's fields, and the same devices in the +/** why: What one refresh over the uplink commits: the view's fields, and the same devices in the * grant shape. Both come from one refresh and land in one call, so a lease the view reports is * never visible before the device it names. */ export type WorkerRefresh = Partial & { @@ -63,15 +50,10 @@ export type WorkerRefresh = Partial & { export interface WorkerRegistryOptions { readonly clock: Clock; readonly eventBus: EventBus; - /** `gateway.disconnectedRetentionMs` (ADR 0005 §6). */ readonly retentionMs: number; - /** - * Where the drained set is kept across restarts (ADR 0005, Decision 3). Optional so a test - * that does not care about persistence need not fabricate one; a gateway always has one. - */ readonly drainStore?: DrainStore; /** - * `gw::` (ADR 0005 §14/§27) -- what marks a lease as *this* + * why: `gw::` (ADR 0005 §14/§27) -- what marks a lease as *this* * gateway's own, rather than another gateway's or a worker's local one. Scopes the retention * hold (§6) to leases this gateway issued: a worker's own local lease is none of this * gateway's business to keep a view alive for, and holding one anyway would mean a machine @@ -81,7 +63,7 @@ export interface WorkerRegistryOptions { */ readonly gatewayRequesterPrefix?: string; /** - * Not optional: a real gateway always has a `lease.maxTtlMs`, and there is no safe placeholder + * why: Not optional: a real gateway always has a `lease.maxTtlMs`, and there is no safe placeholder * to compare a worker's cap against in its absence. */ readonly leaseMaxTtlMs: number; @@ -89,7 +71,7 @@ export interface WorkerRegistryOptions { } /** - * A view without what a `status.get` read of a running worker fills in. A worker that answers + * why: A view without what a `status.get` read of a running worker fills in. A worker that answers * `starting` has had none of it checked, so what an earlier session saw is dropped rather than * shown as current: absent says "not known", where a stale or empty value would say "nothing * is leased". @@ -115,22 +97,21 @@ function withoutReadFields(view: WorkerView): WorkerView { export class WorkerRegistry { readonly #workers = new Map(); readonly #grantedDevices = new Map(); - /** A `starting` refresh leaves `catalog` off the view, but the gateway still knows what that + /** why: A `starting` refresh leaves `catalog` off the view, but the gateway still knows what that * worker listed when it last read it (ADR 0009 §4: a reconnecting worker stays known while its * new catalog is on the way). */ readonly #lastCatalog = new Map>(); // why: a restarting worker stays known for its enrolled devices, as for its catalog (ADR 0009 §4). readonly #lastPhysicalDevices = new Map>(); - /** A `starting` refresh leaves `leases` off the view, which is "not known", not "none". */ + /** why: A `starting` refresh leaves `leases` off the view, which is "not known", not "none". */ readonly #lastLeases = new Map>(); /** - * Drained worker ids, including ids with no view yet. Kept beside the views rather than only + * why: Drained worker ids, including ids with no view yet. Kept beside the views rather than only * on them because drain outlives a view: an operator drains a machine, the machine is turned * off (its view eventually retired), and when it comes back it must still be drained. This * set is what `load()` restores and what `setDrained` writes back. */ readonly #drained = new Set(); - /** `./fleet-ports.ts`'s `FleetViews#onViewsChanged` -- see that method's own doc comment. */ readonly #viewsChangedListeners = new Set<() => void>(); readonly #logger: Logger; @@ -138,7 +119,7 @@ export class WorkerRegistry { this.#logger = options.logger ?? new NoopLogger(); } - /** Restores the persisted drain set. Called once, before the uplink listener starts, so a + /** why: Restores the persisted drain set. Called once, before the uplink listener starts, so a * worker that reconnects in the first second is already drained when its view is built. */ async load(): Promise { for (const workerId of (await this.options.drainStore?.load()) ?? []) { @@ -146,7 +127,6 @@ export class WorkerRegistry { } } - /** Every view, ordered by id so two calls in a row -- and two gateways -- agree. */ views(): readonly WorkerView[] { return [...this.#workers.values()].sort((left, right) => left.id.localeCompare(right.id)); } @@ -162,7 +142,6 @@ export class WorkerRegistry { * Never shown to an operator: the view itself says "not read yet". */ routingViews(): readonly WorkerView[] { - // A view that has a catalog holds the last one read, so only a starting view differs. return this.views().map((view) => { const catalog = this.#lastCatalog.get(view.id); const physical = @@ -176,7 +155,7 @@ export class WorkerRegistry { } /** - * Every worker's devices in the grant shape, each with the `workerId` it came from: what a + * why: Every worker's devices in the grant shape, each with the `workerId` it came from: what a * gateway's `GET /v1/leases/{id}` builds the lease payload from. Ordered by worker id, as * `views()` is. The caller matches a device by worker and id together, never by id alone. */ @@ -190,7 +169,7 @@ export class WorkerRegistry { } /** - * `./fleet-ports.ts`'s `FleetViews#onViewsChanged` -- #118's seam for running dispatch + * why: `./fleet-ports.ts`'s `FleetViews#onViewsChanged` -- #118's seam for running dispatch * whenever the fleet's queue or any worker view changes (ADR 0005 §11), without reaching into * this registry's internals or polling it. Called once per mutation this registry actually * commits (`connected`, `incompatible`, `refresh`, `disconnected`, `setDrained`, `remove`, @@ -223,7 +202,7 @@ export class WorkerRegistry { } /** - * A worker's uplink is open. A view that already exists keeps its drain state -- draining is + * why: A worker's uplink is open. A view that already exists keeps its drain state -- draining is * an operator's decision about a *machine*, and a worker that reconnects (a restart, a * flapping network) has not undone it. Everything the worker itself reports is left for the * first refresh; until then the view says `connected` and carries whatever the last session @@ -239,7 +218,7 @@ export class WorkerRegistry { const view: WorkerView = { ...existing, connection: "connected", - // Read from the persisted set, not from the previous view: a worker connecting for the + // why: Read from the persisted set, not from the previous view: a worker connecting for the // first time after a gateway restart has no previous view, and must still be drained. drained: this.#drained.has(workerId), id: workerId, @@ -247,7 +226,7 @@ export class WorkerRegistry { ...(label === undefined ? {} : { label }), ...(version === undefined ? {} : { version }), }; - // A reconnect after an `incompatible` session must not keep the old ranges around. + // why: A reconnect after an `incompatible` session must not keep the old ranges around. const { protocol: _dropped, ...withoutProtocol } = view; this.#workers.set(workerId, withoutProtocol); this.options.eventBus.emit( @@ -264,7 +243,7 @@ export class WorkerRegistry { } /** - * ADR 0005 §31: the uplink is open but `hello` found no overlapping protocol version. The + * why: ADR 0005 §31: the uplink is open but `hello` found no overlapping protocol version. The * view records both ranges and nothing else -- the gateway asked the worker nothing, because * it cannot. * @@ -292,7 +271,7 @@ export class WorkerRegistry { ...(label === undefined ? {} : { label }), ...(version === undefined ? {} : { version }), }; - // ADR 0008 §8: host facts come from `status.get`, which an incompatible worker is never + // why: ADR 0008 §8: host facts come from `status.get`, which an incompatible worker is never // asked. A view left over from an earlier, compatible session must not keep showing them, // nor the installs or waiting requests that status listed (ADR 0010 §7). const { @@ -364,7 +343,6 @@ export class WorkerRegistry { ...held, ...snapshot, lastSeenAt: now, - // ADR 0009 §4: a refresh that carries a catalog is a read of it. ...(snapshot.catalog === undefined ? {} : { catalogReadAt: now }), }; const next = built.connection === "connected" ? built : withoutLoad(built); @@ -395,7 +373,7 @@ export class WorkerRegistry { } /** - * ADR 0005 §15: the gateway's own `lease.maxTtlMs` decides a fleet lease's width at + * why: ADR 0005 §15: the gateway's own `lease.maxTtlMs` decides a fleet lease's width at * admission, before any worker is chosen -- but what the gateway dispatches is an ordinary * `lease.request`, so a worker whose own cap is lower still refuses a request the gateway * already accepted. That mismatch becomes visible the moment this worker's `config.get` @@ -415,8 +393,6 @@ export class WorkerRegistry { */ #warnOnLowerMaxTtl(previous: WorkerView, next: WorkerView): void { const workerMaxTtlMs = next.lease?.maxTtlMs; - // No cap reported (an incompatible worker, a failed config.get, or one older than this - // field) is not a fact to warn about -- there is nothing to compare. if (workerMaxTtlMs === undefined || workerMaxTtlMs >= this.options.leaseMaxTtlMs) return; if (previous.lease?.maxTtlMs === workerMaxTtlMs) return; this.#logger.warn( @@ -431,7 +407,7 @@ export class WorkerRegistry { } /** - * ADR 0005 §6: the uplink is the reachability signal, and a closed one does not delete + * why: ADR 0005 §6: the uplink is the reachability signal, and a closed one does not delete * anything. The view keeps its last-known state -- capacity, devices, and above all the * leases the worker still holds -- until retention elapses or an operator removes it. A view * that disconnects while `starting` holds no capacity, devices or leases; the leases last read @@ -440,7 +416,7 @@ export class WorkerRegistry { disconnected(workerId: string): void { const existing = this.#workers.get(workerId); if (existing === undefined || existing.connection === "disconnected") return; - // ADR 0010 §7: an install on a worker nobody can reach is not known to be in progress, so + // why: ADR 0010 §7: an install on a worker nobody can reach is not known to be in progress, so // the view stops listing it rather than showing it running for ever. The same goes for the // requests waiting in that worker's own queue. const { @@ -456,7 +432,7 @@ export class WorkerRegistry { this.options.eventBus.emit( "worker.disconnected", { - // A view that went away while `starting` has no leases listed; what the gateway last + // why: A view that went away while `starting` has no leases listed; what the gateway last // read is what the worker still holds, which is what is stranded. leaseCount: (existing.leases ?? this.#lastLeases.get(workerId) ?? []).length, workerId, @@ -468,7 +444,7 @@ export class WorkerRegistry { } /** - * ADR 0005 §9. Idempotent by design: draining an already-drained worker succeeds and emits + * why: ADR 0005 §9. Idempotent by design: draining an already-drained worker succeeds and emits * nothing, because nothing changed and an event is a fact about a change. An operator * re-running a command should not have to care, and neither should a script. * @@ -515,7 +491,7 @@ export class WorkerRegistry { if (this.#drained.delete(workerId)) { await this.options.drainStore?.save([...this.#drained]); } - // Unlike drain, an id with no view is not an error: "forget this worker" is already true + // why: Unlike drain, an id with no view is not an error: "forget this worker" is already true // of one the gateway has never heard of or has already retired, so it is an outcome to // report (`removed: false`) rather than a failure to raise. return false; @@ -532,7 +508,7 @@ export class WorkerRegistry { } /** - * ADR 0005 §6's retention sweep, run from the gateway's periodic tick. Two conditions, both + * why: ADR 0005 §6's retention sweep, run from the gateway's periodic tick. Two conditions, both * required: * * 1. every *gateway-issued* lease the view shows, or the last leases read from it when it @@ -565,7 +541,7 @@ export class WorkerRegistry { (view) => view.connection === "disconnected" && view.lastSeenAt <= cutoff && - // A view with no `leases` field is "not read" (a worker never read, or one that went + // why: A view with no `leases` field is "not read" (a worker never read, or one that went // away while starting): the leases last read stand, and none when none ever were. !(view.leases ?? this.#lastLeases.get(view.id) ?? []).some( (lease) => diff --git a/src/ports/daemon-launcher.test.ts b/src/ports/daemon-launcher.test.ts index ef08c55dc..a33c6ceed 100644 --- a/src/ports/daemon-launcher.test.ts +++ b/src/ports/daemon-launcher.test.ts @@ -29,7 +29,7 @@ describe("NodeDaemonLauncher", () => { it("hands the daemon the home the launching process already resolved", async () => { const directory = await mkdtemp(join(tmpdir(), "simlock-launcher-")); const logPath = join(directory, "daemon.log"); - // A relative SIMLOCK_HOME resolves against whatever directory reads it, so passing the + // why: A relative SIMLOCK_HOME resolves against whatever directory reads it, so passing the // variable through unresolved would let the daemon build its device roots somewhere the // process that launched it never looks. const launcher = new NodeDaemonLauncher({ diff --git a/src/ports/daemon-launcher.ts b/src/ports/daemon-launcher.ts index 69afa5ed5..7bf4c053d 100644 --- a/src/ports/daemon-launcher.ts +++ b/src/ports/daemon-launcher.ts @@ -2,7 +2,6 @@ import { spawn } from "node:child_process"; import { chmod, mkdir, open, readFile, stat } from "node:fs/promises"; import { dirname } from "node:path"; -/** Starts the detached daemon owned by the operating system rather than the CLI. */ export interface DaemonLauncher { launch(): Promise; } @@ -74,7 +73,7 @@ export interface NodeDaemonLauncherOptions { readonly command: string; readonly logPath: string; /** - * The *resolved* `SIMLOCK_HOME` the launching process is using. It is exported into the + * why: The *resolved* `SIMLOCK_HOME` the launching process is using. It is exported into the * daemon's environment rather than left to be inherited, because a relative * `SIMLOCK_HOME` resolves against whatever directory the process reading it happens to * be standing in -- and the two processes agreeing only because one spawned the other @@ -117,7 +116,7 @@ export class NodeDaemonLauncher implements DaemonLauncher { try { const child = spawn(this.options.command, this.options.args, { detached: true, - // Explicit rather than relying on spawn's default: the daemon must inherit + // why: Explicit rather than relying on spawn's default: the daemon must inherit // overrides like SIMLOCK_DRIVERS_MODULE from whichever frontend (CLI/MCP) // auto-launched it, and must be told the home this one already resolved. env: { ...process.env, SIMLOCK_HOME: this.options.simlockHome }, diff --git a/src/ports/filesystem.test.ts b/src/ports/filesystem.test.ts index 603def6ee..2300e3a24 100644 --- a/src/ports/filesystem.test.ts +++ b/src/ports/filesystem.test.ts @@ -4,14 +4,13 @@ import { afterEach, describe, expect, it } from "vitest"; import { type Filesystem, MemoryFilesystem, NodeFilesystem } from "./index.js"; -// Per process: `afterEach` deletes it, so a shared name let two overlapping runs (two suites in +// why: Per process: `afterEach` deletes it, so a shared name let two overlapping runs (two suites in // one checkout, Stryker's runners in one sandbox) delete each other's files mid-test. const temporaryDirectory = `${process.cwd()}/.simlock-ports-test-${process.pid}`; const implementations: Array<{ name: string; create(): Filesystem; - /** Neither implementation creates symlinks through the port, and both have to be asked. */ link(filesystem: Filesystem, path: string, target: string): Promise; }> = [ { @@ -67,7 +66,7 @@ describe.each(implementations)("Filesystem contract: $name", ({ create, link }) await filesystem.mkdirp(temporaryDirectory); await filesystem.writeFileAtomic(file, "é line one\nline two\n"); - // "é" is two bytes: the offset counts bytes, not characters. + // why: "é" is two bytes: the offset counts bytes, not characters. await expect(filesystem.readFileFrom(file, 12)).resolves.toBe("line two\n"); await expect(filesystem.readFileFrom(file, 0)).resolves.toBe("é line one\nline two\n"); await expect(filesystem.readFileFrom(file, 21)).resolves.toBe(""); @@ -226,7 +225,7 @@ describe.each(implementations)("Filesystem contract: $name", ({ create, link }) }); it("refuses to answer for a path that runs through a file", async () => { - // A mistyped device root -- `/notes.txt/ios` -- is a different problem from one + // why: A mistyped device root -- `/notes.txt/ios` -- is a different problem from one // that is simply not there yet, and callers can only tell them apart by the errno. const filesystem = create(); @@ -346,7 +345,7 @@ describe("MemoryFilesystem", () => { }); it("fails at a path a test declared broken, with the errno it declared", async () => { - // The only way to reach the branches that turn a filesystem failure into a typed + // why: The only way to reach the branches that turn a filesystem failure into a typed // rejection: an EACCES on a device root cannot be produced by writing to the double. const filesystem = new MemoryFilesystem(); await filesystem.mkdirp("/devices/ios"); diff --git a/src/ports/filesystem.ts b/src/ports/filesystem.ts index 6cd3ccf32..1c76686f5 100644 --- a/src/ports/filesystem.ts +++ b/src/ports/filesystem.ts @@ -20,18 +20,12 @@ export interface FileStat { readonly kind: "file" | "directory"; readonly size: number; readonly modifiedAtMs: number; - /** POSIX permission bits (e.g. `0o600`), masked to the low 9 bits. Not populated by every - * caller's needs -- only relevant for callers verifying a file's permissions after creation - * (e.g. the daemon's per-start admin secret, ADR 0003 §5). */ readonly mode?: number; - /** Which file this is, stable across a rename and different for a new file at the same path - * (device and inode). Lets a reader that follows a rotating log tell a rotation apart from - * the same file growing. */ readonly identity: string; } export interface WriteFileAtomicOptions { - /** POSIX permission bits the file is created with (e.g. `0o600` for owner-only). Set at + /** why: POSIX permission bits the file is created with (e.g. `0o600` for owner-only). Set at * creation via the temp file's own open flags -- never a separate `chmod` after the fact, * which would leave a window where the file exists world-readable. Omitted keeps Node's * default (governed by the process umask), unchanged from before this option existed. */ @@ -49,26 +43,18 @@ export interface MkdirpOptions { readonly mode?: number; } -/** - * What an un-followed `lstat` reports, narrowed to what ownership validation asks of a - * path: what it really is, who owns it, and how exposed it is. Anything richer would be - * a `fs.Stats` leak into the ports layer. - */ export interface PathDetails { readonly kind: "file" | "directory" | "symlink" | "other"; readonly uid: number; - /** Permission bits only (`mode & 0o777`). */ readonly mode: number; } export interface Filesystem { readFile(path: string): Promise; - /** The file's contents from byte `offset` to its current end; empty past the end. `offset` - * must fall on a character boundary, as a line boundary always does. */ readFileFrom(path: string, offset: number): Promise; writeFileAtomic(path: string, contents: string, options?: WriteFileAtomicOptions): Promise; /** - * Writes a file only when nothing is there, failing `EEXIST` otherwise. The kernel + * why: Writes a file only when nothing is there, failing `EEXIST` otherwise. The kernel * decides who wins, which is the only way two processes racing to write one file both * end up agreeing about what it says. The file appears with its full contents, never * empty, so a loser that reads it straight away reads what the winner wrote. @@ -86,34 +72,22 @@ export interface Filesystem { */ mkdirp(path: string, options?: MkdirpOptions): Promise; copyFile(from: string, to: string): Promise; - /** - * Moves a path in one step, so what it names becomes visible complete or not at all. - * Fails when the destination is occupied by anything but an empty directory. - */ rename(from: string, to: string): Promise; rm(path: string): Promise; stat(path: string): Promise; - /** Metadata read without following a final symlink, so a symlinked root is detectable. */ lstat(path: string): Promise; /** - * Creates exactly one directory and fails when the path already exists (no recursion). + * why: Creates exactly one directory and fails when the path already exists (no recursion). * Failing on an existing path is the point: it is what makes "Simlock created this root * itself, empty" provable rather than assumed. */ mkdir(path: string, options?: { readonly mode?: number }): Promise; chmod(path: string, mode: number): Promise; - /** Fully resolved real path. Used for diagnostics, and to recognise one directory under two - * spellings -- never to reject a path that already matched without it. */ realpath(path: string): Promise; readdir(path: string): Promise; exists(path: string): Promise; diskFree(path: string): Promise; diskSpace(path: string): Promise<{ readonly freeBytes: number; readonly totalBytes: number }>; - /** - * The bytes the files under a directory hold, its subdirectories included. Symlinks are not - * followed, so nothing outside the directory is counted. Rejects when the directory, or - * anything under it, cannot be read. - */ directorySize(path: string): Promise; } @@ -193,7 +167,7 @@ export class NodeFilesystem implements Filesystem { contents: string, options?: WriteFileExclusiveOptions, ): Promise { - // `writeFile` with the `wx` flag creates the file empty and fills it a moment later, so a + // why: `writeFile` with the `wx` flag creates the file empty and fills it a moment later, so a // racer that loses the create can read it in between and find nothing. Write the contents // aside first and hard-link them into place: `link` fails `EEXIST` like `wx` does, but the // file it creates is already complete. @@ -317,9 +291,7 @@ export class NodeFilesystem implements Filesystem { } } -/** Attributes every in-memory entry carries, so callers can read them without a `kind` check. */ interface MemoryAttributes { - /** Stands in for an inode: assigned on creation, carried by `rename`. */ readonly identity: number; modifiedAtMs: number; uid: number; @@ -334,12 +306,11 @@ type MemoryEntryKind = type MemoryEntry = MemoryAttributes & MemoryEntryKind; /** - * Owner-only, matching the permissions Simlock demands of a device root, so a test that + * why: Owner-only, matching the permissions Simlock demands of a device root, so a test that * says nothing about permissions describes a healthy path rather than a rejected one. */ const DEFAULT_MEMORY_MODE = 0o700; -// A symlink cycle is a legitimate thing to model; resolving it forever is not. const MAX_SYMLINK_HOPS = 32; /** why: Node's default for a plain file, kept as the default both `writeFileAtomic` and @@ -437,7 +408,7 @@ export class MemoryFilesystem implements Filesystem { } /** - * Publishes `from` at `to` the way POSIX does, because the create path in + * why: Publishes `from` at `to` the way POSIX does, because the create path in * `ensureOwnedRoot` branches on exactly the failures it reports: an occupied destination * is what tells it another process won the race. */ @@ -560,7 +531,7 @@ export class MemoryFilesystem implements Filesystem { } /** - * Follows symlinks, like the `stat` the Node implementation answers with: a dangling + * why: Follows symlinks, like the `stat` the Node implementation answers with: a dangling * link is absent, and a path whose parent is a file is a failure rather than a "no". * A double that answered from its raw key set would let code that only works against * the double pass for correct. @@ -603,19 +574,17 @@ export class MemoryFilesystem implements Filesystem { } /** - * Test-only: makes every operation on `path` fail with `code`, which is the only way to + * why: Test-only: makes every operation on `path` fail with `code`, which is the only way to * reach the branches that turn an unexpected filesystem failure into a typed rejection. */ defineFailure(path: string, code: string): void { this.#failures.set(path, code); } - /** Test-only: places a symlink at `path`, whether or not `target` exists. */ defineSymlink(path: string, target: string): void { this.#entries.set(path, this.#newEntry({ kind: "symlink", target })); } - /** Test-only: rewrites an existing entry's ownership or permission bits. */ defineAttributes( path: string, attributes: { readonly uid?: number; readonly mode?: number }, @@ -648,16 +617,14 @@ export class MemoryFilesystem implements Filesystem { }; } - /** The entry a following call (`stat`, `readFile`) sees: symlinks resolved. */ #entryAt(path: string): MemoryEntry { return this.#rawEntryAt(this.#resolveLinks(path, 0)); } - /** The entry itself, symlink or not, as `lstat` and `chmod` address it. */ #rawEntryAt(path: string): MemoryEntry { const entry = this.#entries.get(path); if (entry === undefined) { - // Carries `code: "ENOENT"` like Node's real fs errors, so callers that branch on + // why: Carries `code: "ENOENT"` like Node's real fs errors, so callers that branch on // `isMissingPathError` (rather than swallowing every read failure) behave the same // against this fake as they do against `NodeFilesystem`. throw enoent(path); @@ -667,7 +634,7 @@ export class MemoryFilesystem implements Filesystem { } /** - * Resolves symlinks in every component, not just the last one, because that is what + * why: Resolves symlinks in every component, not just the last one, because that is what * makes the "an ancestor symlink is normal, and not grounds for rejection" case * expressible in a test at all. */ @@ -681,7 +648,7 @@ export class MemoryFilesystem implements Filesystem { } const parent = this.#resolveLinks(parentPath(path), hops); - // A component that exists but is not a directory cannot be walked through. Node says + // why: A component that exists but is not a directory cannot be walked through. Node says // ENOTDIR here, and callers tell that apart from ENOENT -- "the path you configured // runs through a file" is a different problem from "it is not there yet". if (this.#entries.get(parent)?.kind === "file") { @@ -729,7 +696,7 @@ function nodeKind(details: { } /** - * Callers branch on `code` -- an existing root is only "somebody won the race" when the + * why: Callers branch on `code` -- an existing root is only "somebody won the race" when the * failure is `EEXIST`, and a path is only absent when the failure is `ENOENT` -- so the * in-memory double has to speak Node's errno vocabulary, not just fail. */ diff --git a/src/ports/index.ts b/src/ports/index.ts index 2acd9795b..241441e50 100644 --- a/src/ports/index.ts +++ b/src/ports/index.ts @@ -53,7 +53,7 @@ export { } from "./service-manager.js"; export { type Clock, FakeClock, SystemClock, type TimerHandle } from "./clock.js"; /** - * ADR 0005 §33's uplink port and its in-memory adapter. The WebSocket adapters are + * why: ADR 0005 §33's uplink port and its in-memory adapter. The WebSocket adapters are * deliberately *not* re-exported here: they live in `./uplink-websocket.js`, the one module * that imports `ws`, and only the daemon entrypoint needs them -- so the CLI and the MCP * server, which import this barrel, never load a WebSocket implementation they never use. diff --git a/src/ports/paths.ts b/src/ports/paths.ts index 58c430c39..d1e984adf 100644 --- a/src/ports/paths.ts +++ b/src/ports/paths.ts @@ -22,7 +22,7 @@ export function resolveSimlockHome(env: NodeJS.ProcessEnv = process.env): string } /** - * The longest AF_UNIX socket path the kernel accepts, in bytes, excluding the trailing + * why: The longest AF_UNIX socket path the kernel accepts, in bytes, excluding the trailing * NUL: `sun_path` is 104 bytes on macOS and the BSDs and 108 on Linux. Windows named * pipes have no such limit. */ @@ -31,7 +31,6 @@ function maxSocketPathLength(platform: NodeJS.Platform = process.platform): numb return (platform === "linux" ? 108 : 104) - 1; } -/** A `SIMLOCK_HOME` so deep that no socket can be bound or connected under it. */ export class SocketPathTooLongError extends Error { constructor( readonly path: string, @@ -46,7 +45,7 @@ export class SocketPathTooLongError extends Error { } /** - * Where the daemon listens under a data directory. Checked here, once, for every process + * why: Where the daemon listens under a data directory. Checked here, once, for every process * that derives it: a path past the kernel's limit otherwise fails on connect as a bare * `EINVAL`, which says nothing about `SIMLOCK_HOME` being the cause. */ diff --git a/src/ports/uplink-websocket.test.ts b/src/ports/uplink-websocket.test.ts index a3e76fc3a..df65afd36 100644 --- a/src/ports/uplink-websocket.test.ts +++ b/src/ports/uplink-websocket.test.ts @@ -1,5 +1,5 @@ /** - * P3: `WebSocketUplinkListenerFactory#handleUpgrade` reads `x-simlock-worker-id` and the + * why: P3: `WebSocketUplinkListenerFactory#handleUpgrade` reads `x-simlock-worker-id` and the * URI-decoded `x-simlock-worker-label` *before* `authenticate` runs -- so anyone who can reach * the gateway's HTTP port, with no credential at all, can make it claim an arbitrary worker id * and a label up to Node's header budget (~16 KB). Both reach `authenticate`'s `claimed` @@ -44,8 +44,6 @@ afterEach(async () => { server = undefined; }); -/** Starts a real gateway-side listener whose `authenticate` always refuses (recording every - * `claimed` it was handed) and returns the base URL to dial it at. */ async function startRefusingGateway( outcome: Exclude = "unauthenticated", ): Promise<{ @@ -72,8 +70,6 @@ async function startRefusingGateway( return { claims, url: `ws://127.0.0.1:${port}` }; } -/** Dials with no credential the gateway will accept, and swallows the resulting refusal -- every - * test here is about what `authenticate` was handed, not about the connection succeeding. */ async function dialAndIgnoreRefusal(url: string, workerId: string, label?: string): Promise { await new WebSocketUplinkConnector() .connect({ token: "irrelevant", url, workerId, ...(label === undefined ? {} : { label }) }) @@ -106,7 +102,7 @@ describe("WebSocketUplinkListenerFactory / P3", () => { it("drops a claimed label containing a control character instead of passing it through", async () => { const { claims, url } = await startRefusingGateway(); - // A worker's own label never contains one; an attacker's header can claim anything. + // why: A worker's own label never contains one; an attacker's header can claim anything. await dialAndIgnoreRefusal(url, "wrk_1", "mac-mini-evil\ntitle"); expect(claims).toHaveLength(1); diff --git a/src/ports/uplink-websocket.ts b/src/ports/uplink-websocket.ts index 5ca015571..643120285 100644 --- a/src/ports/uplink-websocket.ts +++ b/src/ports/uplink-websocket.ts @@ -1,5 +1,5 @@ /** - * The WebSocket adapters behind the uplink port (`./uplink.ts`) -- the one file in the tree + * why: The WebSocket adapters behind the uplink port (`./uplink.ts`) -- the one file in the tree * that imports `ws`, kept out of the ports barrel so processes that never open an uplink (the * CLI, the MCP server) never load it. * @@ -55,7 +55,7 @@ class WebSocketUplinkConnection implements UplinkTransportConnection { readonly #closeListeners = new Set<() => void>(); readonly #errorListeners = new Set<(error: Error) => void>(); /** - * Frames that arrived before anything subscribed. A real socket buffers what is sent to it + * why: Frames that arrived before anything subscribed. A real socket buffers what is sent to it * until someone reads; `ws` does not -- it emits `message` events, and one emitted with no * listener attached is simply gone. * @@ -100,8 +100,6 @@ class WebSocketUplinkConnection implements UplinkTransportConnection { onData(listener: (chunk: string) => void): () => void { const unsubscribe = subscribeListener(this.#dataListeners, listener); - // Whatever arrived before anyone was listening is delivered to the first subscriber, in - // order, before it sees anything new -- see `#pending`. if (this.#pending.length > 0) { const buffered = this.#pending.splice(0); for (const chunk of buffered) listener(chunk); @@ -146,7 +144,7 @@ class WebSocketUplinkConnection implements UplinkTransportConnection { async write(contents: string): Promise { if (this.#closed || this.#socket.readyState !== WebSocket.OPEN) return; await new Promise((resolve) => { - // Resolving on the send callback (rather than fire-and-forget) keeps `write`'s contract + // why: Resolving on the send callback (rather than fire-and-forget) keeps `write`'s contract // the same as the unix-socket adapter's: the caller may await a frame actually leaving. // A send error is not rethrown -- the `error` event above already reports it to every // listener, and a rejected `write` would tear down a caller that has nothing to do @@ -172,7 +170,7 @@ class WebSocketUplinkConnection implements UplinkTransportConnection { } /** - * The gateway's half. `listen()` records the handlers; `attach(server)` wires the HTTP + * why: The gateway's half. `listen()` records the handlers; `attach(server)` wires the HTTP * server's `upgrade` event. The split exists because the HTTP server is created after this * factory (see `src/daemon/main.ts`), and a listener that silently did nothing until some * other call arrived would be a worse contract than one that names the call that wires it. @@ -200,7 +198,6 @@ export class WebSocketUplinkListenerFactory implements UplinkListenerFactory { }; } - /** Wires this factory to a Node HTTP server's `upgrade` event. */ attach(server: Server): void { const onUpgrade = (request: IncomingMessage, socket: Duplex, head: Buffer) => { void this.#handleUpgrade(request, socket, head); @@ -214,14 +211,12 @@ export class WebSocketUplinkListenerFactory implements UplinkListenerFactory { async #handleUpgrade(request: IncomingMessage, socket: Duplex, head: Buffer): Promise { const handlers = this.#handlers; if (handlers === undefined) { - // Nothing is listening for uplinks (a daemon mid-shutdown): refuse rather than leave the + // why: Nothing is listening for uplinks (a daemon mid-shutdown): refuse rather than leave the // socket hanging half-upgraded. respondAndDestroy(socket, 404, "Not Found"); return; } if (!isUplinkPath(request.url)) { - // The gateway upgrades exactly one path. Anything else is a client that guessed, or an - // HTTP route someone tried to upgrade; either way it is not an uplink. respondAndDestroy(socket, 404, "Not Found"); return; } @@ -230,7 +225,7 @@ export class WebSocketUplinkListenerFactory implements UplinkListenerFactory { respondAndDestroy(socket, 400, "Bad Request"); return; } - // P3: truncated before anything else touches it, which bounds the *size* of a flood -- one + // why: P3: truncated before anything else touches it, which bounds the *size* of a flood -- one // claimed id or label, up to Node's header budget (~16 KB), times the ring buffer's capacity // -- not the flood itself. Both fields below reach the gateway's event bus (`worker.rejected` // on the refusal path below, and a real `worker.connected` past authentication) *before* any @@ -241,7 +236,7 @@ export class WebSocketUplinkListenerFactory implements UplinkListenerFactory { // outage they are debugging, since the buffer is bounded by count, not bytes -- is P-2's: // `GatewayService#recordRejection` coalesces repeated refusals into one event per window. const workerId = rawWorkerId.slice(0, MAX_CLAIMED_FIELD_LENGTH); - // Decoded before authentication, not after: a dial that fails the credential check still + // why: Decoded before authentication, not after: a dial that fails the credential check still // claimed this identity in its headers, and ADR 0005 §22's `worker.rejected` reports // whatever a refused peer claimed rather than nothing at all. const label = decodeLabel(headerValue(request, WORKER_LABEL_HEADER)); @@ -250,7 +245,7 @@ export class WebSocketUplinkListenerFactory implements UplinkListenerFactory { try { result = await handlers.authenticate(bearerToken(request), claimed); } catch { - // A token store that cannot be read is not an authenticated peer. + // why: A token store that cannot be read is not an authenticated peer. result = "unauthenticated"; } const outcome = uplinkOutcome(result); @@ -261,8 +256,6 @@ export class WebSocketUplinkListenerFactory implements UplinkListenerFactory { else respondAndDestroy(socket, 403, "Forbidden"); return; } - // C-2: the token record id that authorized this uplink, carried onto `AcceptedUplink` so a - // later `token.revoke` of it can find and close this exact link (ADR 0005 §8). const tokenId = uplinkTokenId(result); this.#server.handleUpgrade(request, socket, head, (client) => { const connection = new WebSocketUplinkConnection(client); @@ -280,7 +273,6 @@ export class WebSocketUplinkListenerFactory implements UplinkListenerFactory { } } -/** The worker's half: dials `gateway.url` with the join token and its instance id. */ export class WebSocketUplinkConnector implements UplinkConnector { readonly #handshakeTimeoutMs: number; @@ -291,8 +283,6 @@ export class WebSocketUplinkConnector implements UplinkConnector { } async connect(options: UplinkDialOptions): Promise { - // `options.url` is `gateway.url`, the gateway's base URL; the endpoint is this worker's to - // derive (ADR 0005 §4), so an operator configures one URL and never an internal path. const socket = new WebSocket(resolveUplinkUrl(options.url), { handshakeTimeout: this.#handshakeTimeoutMs, headers: { @@ -303,7 +293,7 @@ export class WebSocketUplinkConnector implements UplinkConnector { : { [WORKER_LABEL_HEADER]: encodeURIComponent(options.label) }), }, }); - // Wrapped *before* awaiting `open`, not after: the gateway speaks first over an uplink, and + // why: Wrapped *before* awaiting `open`, not after: the gateway speaks first over an uplink, and // its `hello` can be emitted in the same tick the socket opens. The wrapper's `message` // listener has to be attached by then, or that frame is lost and the link deadlocks (see // `WebSocketUplinkConnection#pending`). @@ -348,7 +338,7 @@ function waitForOpen(socket: WebSocket): Promise { cleanup(); reject(toUplinkError(error)); }; - // `ws` emits this instead of `error` when the handshake gets an HTTP response -- which is + // why: `ws` emits this instead of `error` when the handshake gets an HTTP response -- which is // exactly what a refused join token looks like (401). Without it, a revoked token would be // indistinguishable from an unreachable gateway in the worker's log. const onUnexpectedResponse = (_request: unknown, response: IncomingMessage) => { @@ -387,8 +377,6 @@ function toUplinkError(error: unknown): UplinkError { ); } -/** The upgrade request's path, compared without its query string. A request with no `url` at - * all (which Node's typings allow) is not the uplink. */ function isUplinkPath(url: string | undefined): boolean { if (url === undefined) return false; const [path] = url.split("?"); @@ -414,7 +402,7 @@ function bearerToken(request: IncomingMessage): string | undefined { * ~16 KB a raw header can carry. */ const MAX_CLAIMED_FIELD_LENGTH = 128; -/** Control characters (C0 and DEL), the one thing worth rejecting a label outright for rather +/** why: Control characters (C0 and DEL), the one thing worth rejecting a label outright for rather * than just truncating -- a raw header value the gateway's own JSON logging already renders * safely (`writeResult` JSON-encodes; terminal escapes are not the concern here), but a * multi-line or unreadable "machine name" in `simlock worker list` and `simlock events` is not @@ -427,11 +415,9 @@ function decodeLabel(value: string | undefined): string | undefined { try { const decoded = decodeURIComponent(value); if (decoded === "" || NON_PRINTABLE.test(decoded)) return undefined; - // P3: truncated like `workerId` above, before it reaches the bus or a view -- see - // `MAX_CLAIMED_FIELD_LENGTH`'s comment for why. return decoded.slice(0, MAX_CLAIMED_FIELD_LENGTH); } catch { - // A label is cosmetic; a malformed encoding drops it rather than refusing the uplink. + // why: A label is cosmetic; a malformed encoding drops it rather than refusing the uplink. return undefined; } } diff --git a/src/ports/uplink.ts b/src/ports/uplink.ts index 5f683c0f7..a97ef1220 100644 --- a/src/ports/uplink.ts +++ b/src/ports/uplink.ts @@ -1,5 +1,5 @@ /** - * The worker uplink, as a port (architecture rule 9; ADR 0005 §33: "the uplink is a port + * why: The worker uplink, as a port (architecture rule 9; ADR 0005 §33: "the uplink is a port * (`UplinkListenerFactory` on the gateway, `UplinkConnector` on the worker) with a WebSocket * adapter, so tests script it in memory"). * @@ -30,21 +30,15 @@ */ import { subscribeListener, type IpcConnection } from "./ipc.js"; -/** Header carrying the worker's instance identity (`instance.json`) at upgrade (ADR 0005 §3a, +/** why: Header carrying the worker's instance identity (`instance.json`) at upgrade (ADR 0005 §3a, * §4). A header rather than a query parameter so the id stays out of proxy access logs. */ export const WORKER_ID_HEADER = "x-simlock-worker-id"; -/** Header carrying `gateway.label`, URI-encoded (it is free text and may be non-ASCII). */ +/** why: Header carrying `gateway.label`, URI-encoded (it is free text and may be non-ASCII). */ export const WORKER_LABEL_HEADER = "x-simlock-worker-label"; -/** - * The one route a `worker`-role join token may use (ADR 0005 §25), and the only path a gateway - * upgrades an uplink on: `GET /v1/uplink`. A worker's `gateway.url` is the gateway's *base* - * URL and this is derived from it -- see `resolveUplinkUrl` -- so an operator configures one - * URL rather than remembering an endpoint. - */ export const UPLINK_PATH = "/v1/uplink"; /** - * Derives the uplink endpoint from `gateway.url`. Resolved against the base rather than + * why: Derives the uplink endpoint from `gateway.url`. Resolved against the base rather than * concatenated, so a gateway published under a sub-path by a reverse proxy * (`wss://ci.example/simlock/`) lands on `wss://ci.example/simlock/v1/uplink` rather than at * the host root. A base URL that already names the uplink path resolves to itself. @@ -68,20 +62,10 @@ export interface MemoryTransportConnection extends UplinkTransportConnection { fail(error: Error): void; } -/** One accepted uplink: who opened it, and the frames it carries. */ export interface AcceptedUplink { - /** The worker's instance id, from `WORKER_ID_HEADER`. */ readonly workerId: string; - /** `gateway.label`, when the worker sent one. Display only. */ readonly label?: string; readonly connection: UplinkTransportConnection; - /** - * The join token record's own id, when `authenticate` reported one alongside `accept` - * (C-2, ADR 0005 §8: "revoking closes the uplink"). Recorded here so `GatewayService` can - * find every link opened with a given token and close it the moment `token.revoke` runs, - * rather than only at that worker's next reconnect. `undefined` for a caller that returns - * the bare `UplinkAuthOutcome` string -- every test that never exercises revocation. - */ readonly tokenId?: string; } @@ -103,33 +87,18 @@ export interface UplinkListener { */ export type UplinkAuthOutcome = "accept" | "unauthenticated" | "forbidden" | "worker-mismatch"; -/** - * `authenticate`'s actual return type: the bare outcome, or -- only on acceptance -- an object - * naming the token record that authorized it. The object form exists solely to carry `tokenId` - * to `AcceptedUplink` (C-2); a discriminated *addition* to the bare string rather than a new - * required shape, so every `authenticate` that has no token store to consult (every uplink test - * in the tree) keeps returning the plain string it always has. Read with `uplinkOutcome` and - * `uplinkTokenId` rather than switching on `typeof` at each call site. - */ export type UplinkAuthResult = | UplinkAuthOutcome | { readonly outcome: "accept"; readonly tokenId: string }; -/** The outcome out of an `UplinkAuthResult`, whichever form `authenticate` returned. */ export function uplinkOutcome(result: UplinkAuthResult): UplinkAuthOutcome { return typeof result === "string" ? result : result.outcome; } -/** The token id out of an `UplinkAuthResult`, or `undefined` when `authenticate` returned the - * bare outcome string (including every refusal, which never carries one). */ export function uplinkTokenId(result: UplinkAuthResult): string | undefined { return typeof result === "string" ? undefined : result.tokenId; } -/** What the dial *claims* about itself before authentication has run -- the worker id and label - * headers, present or not, regardless of whether the credential checks out. Passed to - * `authenticate` so a rejection can still name the peer it turned away (ADR 0005 §22: - * `worker.rejected`'s `workerId`/`label` are "whatever the connection claimed in its headers"). */ export interface ClaimedUplinkIdentity { readonly workerId?: string; readonly label?: string; @@ -145,7 +114,6 @@ export interface UplinkHandlers { credential: string | undefined, claimed: ClaimedUplinkIdentity, ): Promise; - /** Called once per accepted uplink. */ accept(uplink: AcceptedUplink): void; } @@ -154,12 +122,8 @@ export interface UplinkListenerFactory { } export interface UplinkDialOptions { - /** `gateway.url`: the gateway's **base** URL (`ws://host:port`, or `wss://...`). The adapter - * derives the uplink endpoint from it -- see `resolveUplinkUrl`. */ readonly url: string; - /** The join token, sent as `Authorization: Bearer `. */ readonly token: string; - /** This worker's instance id (`instance.json`). */ readonly workerId: string; readonly label?: string; } @@ -169,7 +133,7 @@ export interface UplinkConnector { } /** - * Why a dial failure is its own error class: the worker's reconnect supervisor backs off on + * why: Why a dial failure is its own error class: the worker's reconnect supervisor backs off on * *any* failure, but an operator reading the log needs to tell "the gateway is not up yet" * (`unreachable`) from "the gateway refused this token" (`rejected` -- a revoked or mistyped * join token, which retrying alone never fixes). ADR 0005 §4/§8: a revoked token closes the @@ -189,12 +153,6 @@ export class UplinkError extends Error { } } -/** - * Deterministic in-process uplink, for tests that script a fleet without a socket (ADR 0005 - * §33/§35). `connect()` runs the same authentication the WebSocket adapter does, hands the - * dialer one end and the listener's `accept` the other, and rejects with the same - * `UplinkError` codes -- so a test that exercises a revoked token exercises the real branch. - */ export class MemoryUplinkTransport implements UplinkListenerFactory, UplinkConnector { #handlers: UplinkHandlers | undefined; @@ -235,12 +193,6 @@ export class MemoryUplinkTransport implements UplinkListenerFactory, UplinkConne } } -/** - * Two `IpcConnection`s wired to each other: what one writes, the other's `onData` listeners - * receive; closing either closes both. Used by `MemoryUplinkTransport`, and exported so a test - * that needs a bare pair (a scripted worker on one end, a real `DaemonServer` on the other) - * does not have to build one. - */ export function createConnectionPair(): [MemoryTransportConnection, MemoryTransportConnection] { const left = new PairedConnection(); const right = new PairedConnection(); @@ -256,7 +208,7 @@ type PairedMessage = class PairedConnection implements MemoryTransportConnection { readonly #closeListeners = new Set<() => void>(); readonly #dataListeners = new Set<(chunk: string) => void>(); - /** Buffered until the first `onData`, exactly as the WebSocket adapter does and as a real + /** why: Buffered until the first `onData`, exactly as the WebSocket adapter does and as a real * socket does: the peer may write before this end has been handed to whoever reads it. */ readonly #pending: string[] = []; readonly #errorListeners = new Set<(error: Error) => void>(); @@ -287,7 +239,7 @@ class PairedConnection implements MemoryTransportConnection { onClose(listener: () => void): () => void { if (this.#closed) { - // A listener attached after the close would otherwise never fire, and consumers attach + // why: A listener attached after the close would otherwise never fire, and consumers attach // `onClose` after construction -- an in-memory pair can close in between. listener(); return () => {}; diff --git a/src/source-hygiene.test.ts b/src/source-hygiene.test.ts index 9ef5705eb..35ccb94c9 100644 --- a/src/source-hygiene.test.ts +++ b/src/source-hygiene.test.ts @@ -86,7 +86,7 @@ const isWebsiteAsset = (name: string): boolean => describe("tracked files", () => { const trackedFiles = lsFiles([]) .filter((name) => !isCommittedBinary(name)) - // A submodule or a path deleted from the working tree is still listed by + // why: A submodule or a path deleted from the working tree is still listed by // `git ls-files`; only regular files can be read. .filter((name) => { try { @@ -97,7 +97,7 @@ describe("tracked files", () => { }); it("were enumerated from the git index", () => { - // Without this, an `execFileSync` that returned nothing would leave `it.each` with an + // why: Without this, an `execFileSync` that returned nothing would leave `it.each` with an // empty list and the suite would pass while checking no file at all. expect(trackedFiles.length).toBeGreaterThan(100); });