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/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 () => {};