Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/CLI.md
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,11 @@ for tests. Two instances on one machine also need distinct
`SIMLOCK_HOME` cannot isolate it. When the CLI or MCP server auto-starts the daemon, the daemon
process inherits the variable like the rest of the environment.

Keep it short: `daemon.sock` lives directly under it, and the kernel caps a Unix
socket path at 104 bytes on macOS (108 on Linux). A deeper `SIMLOCK_HOME` is
refused up front by every frontend with a `USAGE` error naming the limit, rather
than failing on connect with a bare `EINVAL`.

### `SIMLOCK_ADMIN_TOKEN`

The second source in [admin credential resolution](#admin-credential-resolution)
Expand Down
44 changes: 44 additions & 0 deletions src/cli/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
MemoryIpcTransport,
NodeFilesystem,
NodeIpcTransport,
SocketPathTooLongError,
type Filesystem,
type IdGenerator,
} from "../ports/index.js";
Expand Down Expand Up @@ -1481,6 +1482,49 @@ describe("CLI smoke test (ADR 0003 §12: one per frontend)", () => {
});
});

/**
* `docs/CLI.md` promises exactly one structured error line per failure, and binds `USAGE` to
* exit 2. A `SIMLOCK_HOME` whose socket path exceeds the platform limit had neither: the path
* was resolved while *building* the environment, and `runCli` takes its environment as a
* default parameter -- whose initializer runs before the function body, and therefore before
* the try/catch. The error escaped as an uncaught rejection with a raw stack trace, and took
* `simlock --help` down with it.
*/
describe("CLI: a SIMLOCK_HOME the kernel could not bind", () => {
const tooDeep = `/tmp/${"d".repeat(120)}`;
const ports = (): CliEnvironmentPorts => ({
...realCliEnvironmentPorts(),
dataDirectory: tooDeep,
});

it("still prints help, which needs no socket at all", async () => {
const output = outputCapture(ports());

await expect(runCli(["--help"], output.environmentWith())).resolves.toBe(0);

expect(output.stdout).toContain("Usage: simlock");
expect(output.stderr).toBe("");
});

it("reports a command that does need one as a single USAGE line, exit 2", async () => {
const output = outputCapture(ports());

await expect(runCli(["status"], output.environmentWith())).resolves.toBe(2);

const lines = output.stderr.trimEnd().split("\n");
expect(lines).toHaveLength(1);
const reported = JSON.parse(lines[0] ?? "") as {
readonly error: { readonly code: string; readonly message: string };
};
expect(reported.error.code).toBe("USAGE");
expect(reported.error.message).toContain("SIMLOCK_HOME");
});

it("maps the error to the documented usage exit code", () => {
expect(errorExitCode(new SocketPathTooLongError(`${tooDeep}/daemon.sock`, 103))).toBe(2);
});
});

describe("CLI: pure helpers", () => {
it("fallbackRequesterId prefers SIMLOCK_AGENT_ID over a pid-derived default", () => {
expect(fallbackRequesterId({ SIMLOCK_AGENT_ID: "agent-7" })).toBe("agent-7");
Expand Down
17 changes: 13 additions & 4 deletions src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import {
NodeIpcTransport,
NodeParentWatch,
NodeSystemStats,
resolveDaemonSocketPath,
resolveSimlockHome,
SocketPathTooLongError,
SystemClock,
type Clock,
type DaemonLauncher,
Expand Down Expand Up @@ -351,7 +353,6 @@ export function buildCliEnvironment(
env: NodeJS.ProcessEnv = process.env,
): CliEnvironment {
const { clock, dataDirectory, filesystem, ipc, launcher, systemStats } = ports;
const socketPath = join(dataDirectory, "daemon.sock");
const configPath = join(dataDirectory, "config.json");
const logPath = join(dataDirectory, "daemon.log");
const adminTokenPath = join(dataDirectory, "admin.token");
Expand All @@ -367,7 +368,15 @@ export function buildCliEnvironment(
resolveCredential: () => Promise<string | undefined>,
options?: { readonly heartbeat?: boolean },
): Promise<SimlockAdminClient> => {
const connection = await connector.connect(socketPath);
// Resolved here rather than once during construction, because it can throw: a
// `SIMLOCK_HOME` whose socket path exceeds the platform limit is a `SocketPathTooLongError`.
// `runCli` takes its environment as a *default parameter*, and a default initializer runs
// before the function body -- so resolving eagerly put that throw outside `runCli`'s own
// try/catch, where it escaped as an uncaught rejection with a raw stack trace instead of
// the single structured error line `docs/CLI.md` promises, and took `simlock --help` (which
// needs no socket at all) down with it. Every consumer of this path is behind a command
// that is already inside that try.
const connection = await connector.connect(resolveDaemonSocketPath(dataDirectory));
const credential = await resolveCredential();
return connectSimlockAdmin({
connection,
Expand Down Expand Up @@ -569,7 +578,7 @@ function writeError(environment: CliEnvironment, error: unknown): void {
}

function cliErrorCode(error: unknown): string {
if (error instanceof UsageError) return "USAGE";
if (error instanceof UsageError || error instanceof SocketPathTooLongError) return "USAGE";
if (isSimlockError(error)) return error.code;
return "INTERNAL";
}
Expand Down Expand Up @@ -611,7 +620,7 @@ async function runPassthrough(
* second mappings" -- driven from `ERROR_TABLE`'s `cliExitCode` column rather than a second,
* CLI-maintained map. */
export function errorExitCode(error: unknown): number {
if (error instanceof UsageError) return 2;
if (error instanceof UsageError || error instanceof SocketPathTooLongError) return 2;
if (isSimlockError(error)) return ERROR_TABLE[error.code].cliExitCode;
return 1;
}
Expand Down
54 changes: 54 additions & 0 deletions src/core/domain.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,60 @@ describe("transition", () => {
] as const)("rejects %s -> %s", (from, to) => {
expect(() => transition({ ...baseDevice, state: from }, to)).toThrow(IllegalTransition);
});

it.each([
["ready", "shutdown"],
["reclaiming", "shutdown"],
] as const)("drops the address on %s -> %s, since nothing listens there any more", (from, to) => {
const result = transition({ ...baseDevice, address: "emulator-5586", state: from }, to);

expect(result).toEqual({ ...baseDevice, state: to });
expect(result).not.toHaveProperty("address");
});

/**
* The counterpart, and the reason the drop above stops at `shutdown`. A quarantined device
* is just as unreachable, but its way back to `ready` is a `reclaim`, and `ReclaimResult`
* carries no address for `recoverFromQuarantine` to put back -- so dropping it here is
* permanent, and the device is then grantable with no serial the holder can reach it by.
*/
it.each([
["reclaiming", "quarantined"],
["provisioning", "quarantined"],
] as const)("keeps the address on %s -> %s, since recovery cannot re-supply one", (from, to) => {
const result = transition({ ...baseDevice, address: "emulator-5586", state: from }, to);

expect(result.address).toBe("emulator-5586");
});

it("carries an address through quarantine and back out to ready", () => {
const quarantined = transition(
{ ...baseDevice, address: "emulator-5586", state: "reclaiming" },
"quarantined",
);

// Exactly what `Registry.recoverFromQuarantine` does: no `DeviceTransitionUpdate`, because
// the driver's reclaim result has no address to give it.
expect(transition(quarantined, "ready").address).toBe("emulator-5586");
});

it("keeps the address across transitions between running states", () => {
const leased = transition(
{ ...baseDevice, address: "emulator-5586", state: "ready" },
"leased",
);

expect(leased.address).toBe("emulator-5586");
expect(transition(leased, "reclaiming").address).toBe("emulator-5586");
});

it("takes the address a stop supplies over the one it drops", () => {
const result = transition({ ...baseDevice, address: "old", state: "ready" }, "shutdown", {
address: "new",
});

expect(result.address).toBe("new");
});
});

describe("transitionEnteredAt", () => {
Expand Down
18 changes: 18 additions & 0 deletions src/core/domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,24 @@ export function transition(
throw new IllegalTransition(record.state, to);
}

if (to === "shutdown") {
// Nothing is listening at the old address once the device stops, and the next
// `makeReady` supplies a fresh one on the way back to `ready` -- so dropping it here
// costs nothing and keeps `list --devices` from showing an address no one can reach.
//
// Deliberately *not* extended to `quarantined`, though a quarantined device is equally
// unreachable: `quarantined -> ready` is a `reclaim`, and `ReclaimResult` carries no
// address for `Registry.recoverFromQuarantine` to restore. Dropping it there stranded
// the device permanently -- `AcquisitionPlanner` would then grant it, `grantedDevice`
// makes `address` optional so nothing rejected it, and the holder got a grant with no
// adb serial it could not recover (`driverData`, which holds the port, is not part of a
// grant). The address is also not what a port collision is made of: the console port
// lives in `driverData.port`, and this driver reuses the one already recorded there
// rather than taking a new one (see `ManagedDeviceLifecycle.recoverLeased`).
const { address: _stale, ...stopped } = record;
return { ...stopped, ...update, state: to };
}

return { ...record, ...update, state: to };
}

Expand Down
6 changes: 6 additions & 0 deletions src/core/driver-catalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ export class DriverCatalog {
return driver;
}

/** Whether a driver started for this platform; false for one discovery refused. */
// fallow-ignore-next-line unused-class-member -- reached through StartupDriverAvailability by StartupConverger.
has(platform: Platform): boolean {
return this.#drivers.has(platform);
}

/**
* Routes `simlock <tool> <args>` to whichever driver claims that tool name. Routing is
* all this does: which flag scopes the tool, which verbs it refuses, and what its
Expand Down
1 change: 1 addition & 0 deletions src/core/lease-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ export class LeaseEngine {
claims: this.#claims,
cleanup: this.cleanup,
decisions: this.#decisions,
drivers: this.#drivers,
interruptedReclaimRecovery: {
recoverInterruptedReclaim: async (device) => {
await this.#warmPool.recoverInterrupted(device.id);
Expand Down
24 changes: 24 additions & 0 deletions src/core/startup-converger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ function createHarness(
devices: DeviceRecord[],
leases: LeaseRecord[] = [],
limits = { android: 3, global: 3, ios: 3 },
darkPlatforms: ReadonlySet<Platform> = new Set(),
) {
const order: string[] = [];
const claimed = new Set<string>();
Expand Down Expand Up @@ -100,6 +101,7 @@ function createHarness(
claims: { isClaimed: (deviceId) => claimed.has(deviceId) },
cleanup,
decisions: new SerializedDecision(),
drivers: { has: (platform) => !darkPlatforms.has(platform) },
interruptedReclaimRecovery: recovery,
quarantineRestore,
registry: {
Expand Down Expand Up @@ -231,6 +233,28 @@ describe("StartupConverger", () => {
);
});

it("leaves a platform without a driver untouched instead of failing convergence", async () => {
const interrupted = device("ios-reclaiming", "ios", "reclaiming", 1);
const excess = device("ios-ready", "ios", "ready", 2);
const androidInterrupted = device("android-reclaiming", "android", "reclaiming", 3);
const harness = createHarness(
[interrupted, excess, androidInterrupted],
[],
{ android: 1, global: 1, ios: 0 },
new Set<Platform>(["ios"]),
);

await harness.converger.converge();

expect(harness.recovery.recoverInterruptedReclaim).toHaveBeenCalledOnce();
expect(harness.recovery.recoverInterruptedReclaim).toHaveBeenCalledWith(
expect.objectContaining({ id: androidInterrupted.id }),
);
expect(harness.cleanupCalls).toEqual([]);
expect(harness.devices.find((item) => item.id === interrupted.id)?.state).toBe("reclaiming");
expect(harness.devices.find((item) => item.id === excess.id)?.state).toBe("ready");
});

it("is idempotent after recovery and successful convergence", async () => {
const recovering = device("recovering", "ios", "reclaiming", 1);
const ready = device("ready", "ios", "ready", 2);
Expand Down
26 changes: 25 additions & 1 deletion src/core/startup-converger.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { CleanupActionExecutor } from "./cleanup-executor.js";
import type { DeviceRecord, LeaseRecord } from "./domain.js";
import type { DeviceRecord, LeaseRecord, Platform } from "./domain.js";
import type { CapacityReader } from "./lease-ports.js";
import type { SerializedDecision } from "./serialized-decision.js";
import { compareLeastRecentlyUsed } from "./warm-pool.js";
Expand Down Expand Up @@ -41,11 +41,17 @@ export interface DeviceClaimReader {
isClaimed(deviceId: string): boolean;
}

/** Which platforms have a driver this daemon can drive devices through. */
export interface StartupDriverAvailability {
has(platform: Platform): boolean;
}

export interface StartupConvergerOptions {
readonly capacity: CapacityReader;
readonly claims: DeviceClaimReader;
readonly cleanup: CleanupActionExecutor;
readonly decisions: SerializedDecision;
readonly drivers: StartupDriverAvailability;
readonly interruptedReclaimRecovery: InterruptedReclaimRecovery;
readonly quarantineRestore: QuarantineRestorer;
readonly registry: StartupRegistry;
Expand All @@ -56,6 +62,22 @@ export interface StartupConvergerOptions {
/**
* Directly coordinates the required startup recovery sequence. It emits no
* events itself; recovery and cleanup own their post-commit lifecycle facts.
*
* Convergence never calls a driver that was refused at discovery. Recovering or shutting a
* device down needs a driver call there is no driver for, and a `NoDriverError` out of
* convergence stops the whole daemon -- costing the healthy platform for a root the other one
* rejected, which is the opposite of the per-platform fail-closed behaviour discovery
* promises. `simlock doctor` reports the rejection; the inventory waits for the driver to
* come back.
*
* Two limits on that, both deliberate and neither silent. `#releaseOrphanedHeldLeases` runs
* first and unguarded: a held lease cannot have a live holder across a restart, so it is
* released whatever its platform, which moves the device to `reclaiming` and leaves the
* background reclaim to fail into its own catch. The device is then stuck in `reclaiming`
* until its driver returns -- worse than untouched, better than a phantom lease pinning a
* device nobody holds. And a dark platform's devices still count toward capacity (see
* `capacity/limits.ts`), so a large refused inventory can make the *healthy* platform look
* over budget; excess selection below excludes them from the candidates, not from the count.
*/
export class StartupConverger {
constructor(private readonly options: StartupConvergerOptions) {}
Expand Down Expand Up @@ -110,6 +132,7 @@ export class StartupConverger {
return snapshot.devices.filter(
(device) =>
device.state === "reclaiming" &&
this.options.drivers.has(device.spec.platform) &&
!leasedDeviceIds.has(device.id) &&
!this.options.claims.isClaimed(device.id),
);
Expand All @@ -134,6 +157,7 @@ export class StartupConverger {
.filter(
(device) =>
device.state === "ready" &&
this.options.drivers.has(device.spec.platform) &&
!leasedDeviceIds.has(device.id) &&
!this.options.claims.isClaimed(device.id) &&
!refused.has(device.id) &&
Expand Down
3 changes: 2 additions & 1 deletion src/daemon/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
NodeProcessSupervisor,
NodeSystemStats,
NodeTcpProbe,
resolveDaemonSocketPath,
resolveSimlockHome,
SystemClock,
type SystemStats,
Expand Down Expand Up @@ -93,7 +94,7 @@ export async function startDaemon(options: StartDaemonOptions = {}): Promise<Dae
const tcpProbe = options.tcpProbe ?? new NodeTcpProbe();
const configPath = options.configPath ?? join(dataDirectory, "config.json");
const statePath = options.statePath ?? join(dataDirectory, "state.json");
const socketPath = options.socketPath ?? join(dataDirectory, "daemon.sock");
const socketPath = options.socketPath ?? resolveDaemonSocketPath(dataDirectory);
const config = await loadConfig({
configPath,
filesystem,
Expand Down
Loading