Files
Ignacy Łątka 2be58a5cf1 fix(boot-device): five correctness fixes from post-merge Linux review (#296)
## Summary

Five issues found in the post-merge code review of #249 (Linux support).
All confirmed present on main; none fixed by #194.

### H1 — `awaitFirstRealFrame` timeout leaves emulator running

`awaitFirstRealFrame` is called after the cold-boot `try/catch` closes.
On a SurfaceFlinger timeout the emulator process stayed alive:
`inFlightBoots.finally` cleared the map entry, so the next `boot-device`
call found the stale serial via the `alreadyRunning` fast path, had to
run `assertScreencapAlive`, killed it there, and then fell through to
another full cold boot — doubling latency and surfacing a confusing
"wedged framebuffer" error instead of the original timeout.

**Fix:** wrap the `awaitFirstRealFrame` call in a try/catch that calls
`killEmulatorQuietly` before re-throwing.

### H2 — `awaitFirstRealFrame` budget not clamped against
`overallDeadline`

Every `attemptBoot` stage clamps via `Math.min(params.attemptDeadline,
…)`. `awaitFirstRealFrame` received the fixed
`STAGE_BUDGET.firstRealFrame` (90 s) with no knowledge of
`overallDeadline`, allowing total wall time to silently exceed
`bootTimeoutMs` by up to 90 s — violating the documented contract and
the Zod schema's `max(900_000)`.

**Fix:** pass `Math.min(STAGE_BUDGET.firstRealFrame, Math.max(0,
overallDeadline - Date.now()))`.

### H3 — `ARGENT_SIMULATOR_SERVER_DIR` semantic changed without
migration hint

The env var previously pointed directly at the directory containing
`simulator-server`; it now points at the root of the per-platform tree.
A user with the old flat layout gets a bare "binary not found" with no
explanation of what changed.

**Fix:** detect the old flat layout and include a migration hint in the
error message; update the env var's doc comment.

### M4 — `ARGENT_EMULATOR_NO_WINDOW=false` silently enables headless
mode

`selectExtraEmulatorArgs` used a deny-list (`!== "0"`) so `"false"` and
`"no"` — natural shell idioms for "disabled" — both mapped to
`-no-window`. Test suite covered `""`, `" "`, `"0"` but not `"false"`.

**Fix:** switch to an allowlist `["1", "true", "yes"]`; add `"false"`
and `"no"` to the parameterized off-test.

### M5 — `bootstrapDylibPathTcp` / `nativeDevtoolsDylibPathTcp` missing
`requireDarwin`

All six non-TCP dylib/binary exports gained explicit `requireDarwin()`
guards in #249. The two TCP dylib exports did not. A Linux caller gets
"dylib not found" instead of "requires macOS host".

**Fix:** add `requireDarwin()` as the first statement in both TCP dylib
functions; add a test.

### S7 (bonus) — `selectGpuMode` validation fires mid-function after
slow I/O

`selectGpuMode()` and `selectExtraEmulatorArgs()` were called inside
deep nested code after several seconds of I/O. A typo in
`ARGENT_EMULATOR_GPU_MODE` surfaced late with a spurious "emulator has
been terminated" suffix appended.

**Fix:** call both once at the top of `bootAndroidImpl`, just after
`ensureDep`, and reuse the captured values — eliminates the late error
and the redundant double-call.

## Test plan

- `npm run test -w @argent/tool-server` — **847 passed** (was 845; +2
for `false`/`no` NO_WINDOW cases)
- `npm run test -w @argent/native-devtools-ios` — **10 passed** (was 8;
+2 for TCP dylib darwin gate and migration hint)
2026-06-03 17:32:28 +02:00

184 lines
8.5 KiB
TypeScript

import * as fs from "node:fs";
import * as os from "node:os";
import * as path from "node:path";
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
// Static import primes vite's module graph so the dynamic re-import below
// resolves to the same node; we re-import via the SAME specifier after
// vi.resetModules() to reset captured env per-test.
import type * as ResolverModule from "../src/index";
// Unit tests for the resolver in this package. The resolver is what gates
// iOS-only binaries from being looked up on Linux callers, and what joins
// `process.platform` into the simulator-server bin path. Both behaviors
// only execute on resolver invocation, so we test the exported functions
// directly. We override ARGENT_NATIVE_DEVTOOLS_DIR /
// ARGENT_SIMULATOR_SERVER_DIR per-test so the resolver looks at a tmpdir we
// control rather than the real packages/native-devtools-ios layout.
let tmpRoot = "";
const originalPlatform = process.platform;
const originalDevtoolsDir = process.env.ARGENT_NATIVE_DEVTOOLS_DIR;
const originalSimulatorDir = process.env.ARGENT_SIMULATOR_SERVER_DIR;
function setPlatform(value: NodeJS.Platform) {
Object.defineProperty(process, "platform", { value, configurable: true });
}
beforeAll(() => {
tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), "argent-resolver-"));
});
afterAll(() => {
if (tmpRoot) fs.rmSync(tmpRoot, { recursive: true, force: true });
setPlatform(originalPlatform);
if (originalDevtoolsDir === undefined) delete process.env.ARGENT_NATIVE_DEVTOOLS_DIR;
else process.env.ARGENT_NATIVE_DEVTOOLS_DIR = originalDevtoolsDir;
if (originalSimulatorDir === undefined) delete process.env.ARGENT_SIMULATOR_SERVER_DIR;
else process.env.ARGENT_SIMULATOR_SERVER_DIR = originalSimulatorDir;
});
afterEach(() => {
setPlatform(originalPlatform);
});
/**
* Re-import the resolver after env / platform mutations. The module captures
* ARGENT_NATIVE_DEVTOOLS_DIR / ARGENT_SIMULATOR_SERVER_DIR at top-level on
* first load; vi.resetModules() clears vite's cache so the next import()
* re-executes the module body and picks up the current env. The dynamic
* specifier must match the static import above byte-for-byte so vite's
* module graph resolves to the same node.
*/
async function loadResolver(): Promise<typeof ResolverModule> {
vi.resetModules();
return await import("../src/index");
}
describe("requireDarwin gating", () => {
it("throws with a root-cause message on linux for iOS-only dylibs", async () => {
// A Linux caller asking for an iOS Simulator dylib should get the root
// cause ("requires a macOS host") not a misleading "file not found",
// which is what happened before this PR and led to confused bug reports.
setPlatform("linux");
const r = await loadResolver();
expect(() => r.bootstrapDylibPath()).toThrow(/requires a macOS host/);
expect(() => r.nativeDevtoolsDylibPath()).toThrow(/requires a macOS host/);
expect(() => r.keyboardPatchDylibPath()).toThrow(/requires a macOS host/);
});
it("throws with a root-cause message on linux for TCP iOS-only dylibs", async () => {
// TCP variants are equally iOS-Simulator-only; the guard must fire before
// the file-existence check so Linux callers get the platform error, not
// a misleading "dylib not found" for the tcp/ subdirectory.
setPlatform("linux");
const r = await loadResolver();
expect(() => r.bootstrapDylibPathTcp()).toThrow(/requires a macOS host/);
expect(() => r.nativeDevtoolsDylibPathTcp()).toThrow(/requires a macOS host/);
});
it("throws with a root-cause message on linux for ax-service", async () => {
// ax-service is darwin-only (runs INSIDE the iOS Simulator), so the
// resolver must throw the platform error BEFORE the file-existence check.
setPlatform("linux");
process.env.ARGENT_SIMULATOR_SERVER_DIR = tmpRoot;
const r = await loadResolver();
expect(() => r.axServiceBinaryPath()).toThrow(/requires a macOS host/);
});
it("throws with a root-cause message on linux for ax-service (tcp)", async () => {
setPlatform("linux");
process.env.ARGENT_SIMULATOR_SERVER_DIR = tmpRoot;
const r = await loadResolver();
expect(() => r.axServiceBinaryPathTcp()).toThrow(/requires a macOS host/);
});
});
describe("simulator-server path resolution", () => {
it("joins process.platform into the bin path", async () => {
// The whole point of the per-platform bin layout. We're verifying that
// the resolver constructs <BIN_ROOT>/<platform>/simulator-server, not
// <BIN_ROOT>/simulator-server.
const dir = fs.mkdtempSync(path.join(tmpRoot, "platform-join-"));
const platDir = path.join(dir, process.platform);
fs.mkdirSync(platDir, { recursive: true });
const binPath = path.join(platDir, "simulator-server");
fs.writeFileSync(binPath, "", { mode: 0o755 });
process.env.ARGENT_SIMULATOR_SERVER_DIR = dir;
const r = await loadResolver();
expect(r.simulatorServerBinaryPath()).toBe(binPath);
expect(r.simulatorServerBinaryDir()).toBe(platDir);
});
it("throws when the per-platform binary is missing", async () => {
// Empty per-platform dir: the resolver should fail loudly with both the
// platform name and the expected path in the message so the user can
// act on it (re-run the download script, switch hosts, etc).
const dir = fs.mkdtempSync(path.join(tmpRoot, "missing-"));
fs.mkdirSync(path.join(dir, process.platform), { recursive: true });
process.env.ARGENT_SIMULATOR_SERVER_DIR = dir;
const r = await loadResolver();
expect(() => r.simulatorServerBinaryPath()).toThrow(/simulator-server binary not found/);
expect(() => r.simulatorServerBinaryPath()).toThrow(new RegExp(process.platform));
});
it("includes a migration hint when ARGENT_SIMULATOR_SERVER_DIR points at the old flat layout", async () => {
// Prior to the Linux per-platform layout, ARGENT_SIMULATOR_SERVER_DIR
// was the directory that contained simulator-server directly. A user
// with that setup now gets a "not found" error; the migration hint in
// the message tells them exactly what changed and how to fix it.
const dir = fs.mkdtempSync(path.join(tmpRoot, "flat-layout-"));
fs.mkdirSync(path.join(dir, process.platform), { recursive: true });
// Place the binary at the flat (old) path.
const flatBin = path.join(dir, "simulator-server");
fs.writeFileSync(flatBin, "", { mode: 0o755 });
process.env.ARGENT_SIMULATOR_SERVER_DIR = dir;
const r = await loadResolver();
expect(() => r.simulatorServerBinaryPath()).toThrow(/old flat path/);
expect(() => r.simulatorServerBinaryPath()).toThrow(new RegExp(process.platform));
});
});
describe("ax-service path resolution", () => {
it("joins process.platform into the ax-service bin path", async () => {
if (originalPlatform !== "darwin") return;
const dir = fs.mkdtempSync(path.join(tmpRoot, "ax-platform-"));
const platDir = path.join(dir, "darwin");
fs.mkdirSync(platDir, { recursive: true });
const binPath = path.join(platDir, "ax-service");
fs.writeFileSync(binPath, "", { mode: 0o755 });
process.env.ARGENT_SIMULATOR_SERVER_DIR = dir;
const r = await loadResolver();
expect(r.axServiceBinaryPath()).toBe(binPath);
});
it("joins process.platform/tcp into the ax-service TCP bin path", async () => {
if (originalPlatform !== "darwin") return;
const dir = fs.mkdtempSync(path.join(tmpRoot, "ax-tcp-"));
const tcpDir = path.join(dir, "darwin", "tcp");
fs.mkdirSync(tcpDir, { recursive: true });
const binPath = path.join(tcpDir, "ax-service");
fs.writeFileSync(binPath, "", { mode: 0o755 });
process.env.ARGENT_SIMULATOR_SERVER_DIR = dir;
const r = await loadResolver();
expect(r.axServiceBinaryPathTcp()).toBe(binPath);
});
});
describe("ARGENT_NATIVE_DEVTOOLS_DIR override", () => {
it("uses the env-var dir for dylib resolution on darwin", async () => {
// The launcher script swaps the dylib dir at runtime via this env var
// (matches the ARGENT_SIMULATOR_SERVER_DIR pattern). Linux callers can't
// exercise the happy path because requireDarwin throws first, so this
// test only meaningfully runs on darwin — skip elsewhere rather than
// pretend to test it.
if (originalPlatform !== "darwin") return;
const dir = fs.mkdtempSync(path.join(tmpRoot, "dylib-override-"));
const dylib = path.join(dir, "libNativeDevtoolsIos.dylib");
fs.writeFileSync(dylib, "");
process.env.ARGENT_NATIVE_DEVTOOLS_DIR = dir;
setPlatform("darwin");
const r = await loadResolver();
expect(r.nativeDevtoolsDylibPath()).toBe(dylib);
});
});