diff --git a/docs/adb-device-analysis.md b/docs/adb-device-analysis.md index efb8bf2b34..79c74fd2fb 100644 --- a/docs/adb-device-analysis.md +++ b/docs/adb-device-analysis.md @@ -25,7 +25,21 @@ REA never installs the Android SDK, an emulator, or a system image. Readiness is `rea inspect-adb-client`: it runs `adb version` and reports the resolved binary, its version, and where it was selected from. A missing binary reports `capability_unavailable` with the `REA_ADB_PATH` remediation -rather than attempting an installation. +rather than attempting an installation. A path that cannot resolve to a file +(`ENOENT`, `ENOTDIR`, `ELOOP` or `ENAMETOOLONG`) is reported as missing. A +selected path the host refuses to execute, such as a directory or a file +without execute permission (`EACCES`) or one the host does not permit +(`EPERM`), and any other spawn error report `capability_unavailable` naming +that path. Exhausted host capacity (`EAGAIN`, `EMFILE`, `ENFILE` or `ENOMEM`) +is not a configuration failure. `inspect_adb_client`, and any device +operation whose adb cannot start at its version probe or at a later adb +command, report a retryable `capacity` resource constraint rather than a +capability refusal or an adb protocol failure; the `binary_session` inventory +reports the provider as `runtime_unavailable`. Both say the host could not +start adb and ask for a retry after freeing processes or descriptors, without +blaming `REA_ADB_PATH`. When adb never started, the refusal diagnostics record +the spawn error as `filesystem_error` with `path_source`, never as an +`exit_code`. The `binary_session` inventory uses the same bounded client-version probe; it does not query devices, start the adb server, or establish that a requested serial is reachable. Device state is checked by each device operation. @@ -210,9 +224,11 @@ and during shutdown. them in `raw_result.device_path_probes`. Incomplete startup, extra diagnostics, truncation and cancellation do not establish absence. The check is not atomic, so a different process can create the path before `adb push` writes it. When available, the device's - `sha256sum` verifies the transfer. A missing command or output without a - recognizable digest reports `device_digest_source: "unavailable"` with an - unverified-transfer limitation; installs preserve the + `sha256sum` verifies the transfer. A missing command, output without a + recognizable digest, or host capacity that prevents the digest check from + starting reports `device_digest_source: "unavailable"` with an + unverified-transfer limitation rather than a retryable error; installs + preserve the device's `Failure [...]` reason verbatim. - **Producer identity stays attached.** Every result embeds the producing adb client (`binary_path`, `path_source`, `version`, `installed_path`). diff --git a/src/adb/AdbCommand.test.ts b/src/adb/AdbCommand.test.ts index 89cac7286a..056b3734d6 100644 --- a/src/adb/AdbCommand.test.ts +++ b/src/adb/AdbCommand.test.ts @@ -371,3 +371,61 @@ it.each([ error: { capturedOutput: { stderr, exit_code: 1 } }, }); }); + +it("reports an adb that never started without claiming an exit code", async () => { + vi.mocked(execFileBytes).mockRejectedValueOnce( + Object.assign(new Error("spawn ENOTDIR"), { + code: "ENOTDIR", + spawnFailed: true, + stdout: "", + stderr: "", + signal: null, + killed: false, + }), + ); + const result = await runAdbCapture( + "list_adb_devices", + { device: "device" }, + { binary: "/host/adb", pathSource: "environment", environment: {} }, + ["devices"], + { timeoutMs: 100, maxOutputBytes: 1024 }, + undefined, + ); + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisProtocolError", + commandFailure: { code: "ENOTDIR", spawnFailed: true, signal: null }, + }, + }); +}); + +it("reports exhausted host capacity rather than an adb protocol failure", async () => { + vi.mocked(execFileBytes).mockRejectedValueOnce( + Object.assign(new Error("spawn EAGAIN"), { + code: "EAGAIN", + spawnFailed: true, + stdout: "", + stderr: "", + signal: null, + killed: false, + }), + ); + const result = await runAdbCapture( + "list_adb_devices", + { device: "device" }, + { binary: "/host/adb", pathSource: "environment", environment: {} }, + ["devices"], + { timeoutMs: 100, maxOutputBytes: 1024 }, + undefined, + ); + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisResourceConstraintError", + resource: "capacity", + retryable: true, + reportedLimits: { spawn_error: "EAGAIN" }, + }, + }); +}); diff --git a/src/adb/AdbCommand.ts b/src/adb/AdbCommand.ts index 5c627493d6..ebb79b90c4 100644 --- a/src/adb/AdbCommand.ts +++ b/src/adb/AdbCommand.ts @@ -7,6 +7,7 @@ import { } from "../domain/analysisErrorCore.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { fixedAllowanceOptions } from "../domain/fixedAllowanceRemediation.js"; +import type { JsonValue } from "../domain/jsonValue.js"; import { err, ok, type Result } from "../domain/result.js"; import { execFileBytes, @@ -14,6 +15,7 @@ import { ExecFileOutputCancelledError, execFileOutputFailure, type ExecFileOutputFailure, + spawnCapacityExhausted, } from "../process/ExecFileOutput.js"; import { packageChangeFailureReason } from "./AdbShellOutput.js"; import { @@ -21,7 +23,10 @@ import { isDaemonStartingNotice, hasOnlySuccessfulAdbStartupNotices, } from "./AdbDeviceOutput.js"; -import type { AdbBinarySelection } from "./AdbConfiguration.js"; +import { + adbHostCapacityRemediation, + type AdbBinarySelection, +} from "./AdbConfiguration.js"; const STDERR_EXCERPT_CHARACTERS = 512; @@ -98,6 +103,26 @@ export interface AdbCommandTarget { readonly localPath?: string; } +/** The host, not adb, lacked process, memory or descriptor capacity to start it. */ +export const adbSpawnCapacityError = ( + operation: string, + spawnError: string, + cause: unknown, + diagnostics?: Readonly>, +): AnalysisResourceConstraintError => + new AnalysisResourceConstraintError( + operation, + "capacity", + `The host could not start adb (${spawnError}) for ${operation}`, + { spawn_error: spawnError }, + { + cause, + diagnostics, + retryable: true, + remediationAction: adbHostCapacityRemediation(spawnError), + }, + ); + /** Run one bounded adb invocation and keep its exact failure shape. */ export const runAdbCapture = async ( operation: string, @@ -218,11 +243,16 @@ export const runAdbCapture = async ( remediationAction: `This ${String(limits.timeoutMs)} ms adb deadline has no runtime override. Allow execution, cleanup and delivery headroom in the outer MCP client deadline. Confirm outstanding work and cleanup before retrying.`, }), ); + // The host, not adb, lacked capacity to start it. + if (spawnCapacityExhausted(failure)) + return err(adbSpawnCapacityError(operation, String(failure.code), cause)); return err( new AdbProtocolError( - excerpt === "" - ? `adb exited with code ${String(failure.code)} for ${operation}` - : excerpt, + failure.spawnFailed === true + ? `adb could not be started (${String(failure.code)}) for ${operation}` + : excerpt === "" + ? `adb exited with code ${String(failure.code)} for ${operation}` + : excerpt, failure, cause, ), diff --git a/src/adb/AdbConfiguration.ts b/src/adb/AdbConfiguration.ts index a25bbe0010..c32f86db29 100644 --- a/src/adb/AdbConfiguration.ts +++ b/src/adb/AdbConfiguration.ts @@ -13,22 +13,28 @@ import { execFileFailureCapturedOutput, ExecFileOutputCancelledError, execFileOutputFailure, + spawnCapacityExhausted, } from "../process/ExecFileOutput.js"; import type { ProviderAvailability } from "../application/AnalysisProvider.js"; import type { ProviderRejectionCode } from "../domain/providerSelection.js"; import { fixedAllowanceOptions } from "../domain/fixedAllowanceRemediation.js"; import type { JsonValue } from "../domain/jsonValue.js"; +import { isUnresolvedPathErrorCode } from "../filesystem/RegularFile.js"; /** Environment key selecting the caller's adb binary. */ export const ADB_BINARY_ENV = "REA_ADB_PATH"; +/** One remediation for every host-capacity spawn failure; the selected path is not at fault. */ +export const adbHostCapacityRemediation = (code: string): string => + `The host ran out of process, memory or file-descriptor capacity (${code}). Close unused processes or files, or raise the process or open-file limit of the REA process, then retry; ${ADB_BINARY_ENV} does not need to change.`; + const VERSION_TIMEOUT_MS = 10_000; const ADB_VERSION_REMEDIATION = `Set ${ADB_BINARY_ENV} to an Android SDK Platform-Tools adb binary whose \`adb version\` reports its version, or put one on PATH.`; /** Repair when the selected adb binary does not exist. */ export const ADB_MISSING_REMEDIATION = `Set ${ADB_BINARY_ENV} to an absolute path to an existing adb binary.`; -/** Repair when execution of the selected adb binary is denied. */ -export const ADB_NOT_EXECUTABLE_REMEDIATION = `Check file type and execute permission for ${ADB_BINARY_ENV} or select an executable adb binary.`; +/** Repair when execution of the adb binary selected by the environment is denied. */ +export const ADB_NOT_EXECUTABLE_REMEDIATION = `Set ${ADB_BINARY_ENV} to an absolute path to the adb executable file, not a directory, and check execute permission on it and search permission on its parent directories.`; const VERSION_MAX_OUTPUT_BYTES = 64 * 1024; /** How the adb binary was selected for this provider instance. */ @@ -128,6 +134,7 @@ export type AdbConfigurationFailureCode = | "binary_missing" | "binary_unresolved" | "binary_not_executable" + | "host_capacity" | "version_unresolved"; /** Why the selected adb binary is unusable, with the owner's repair instruction. */ @@ -213,7 +220,9 @@ export const inspectAdbClient = async ( const diagnostics = { selected_binary: selection.binary, path_source: selection.pathSource, - ...(failure === undefined ? {} : { exit_code: failure.code }), + ...(failure === undefined || failure.spawnFailed === true + ? {} + : { exit_code: failure.code }), }; const options = { cause, @@ -243,35 +252,58 @@ export const inspectAdbClient = async ( userMessage: `adb version probe reached its ${String(VERSION_TIMEOUT_MS)} ms deadline (deadlineExpired). This release has no runtime override.`, remediationAction: `This ${String(VERSION_TIMEOUT_MS)} ms adb version probe has no runtime override. Allow execution, cleanup and delivery headroom in the outer MCP client deadline. Confirm outstanding work and cleanup before retrying.`, }); - if (failure?.code === "ENOENT") - throw new AdbConfigurationFailure( - "binary_missing", - `The selected adb binary was not found: ${selection.binary}`, - ADB_MISSING_REMEDIATION, - { - selected_binary: selection.binary, - path_source: selection.pathSource, - }, - { cause }, - ); - if (failure?.code === "EACCES" || failure?.code === "EPERM") + // The process never started: `failure.code` is a spawn errno, not an exit + // status, and no version probe ran. + if (failure?.spawnFailed === true) { + const code = String(failure.code); + const fromEnvironment = selection.pathSource === "environment"; + const facts = { + selected_binary: selection.binary, + path_source: selection.pathSource, + filesystem_error: code, + }; + // Nothing executable resolves at the selected path. + if (isUnresolvedPathErrorCode(code)) + throw new AdbConfigurationFailure( + "binary_missing", + `The selected adb binary was not found: ${selection.binary}`, + ADB_MISSING_REMEDIATION, + facts, + { cause }, + ); + // The host lacked process, memory or descriptor capacity; the selected + // path is not the cause. + if (spawnCapacityExhausted(failure)) + throw new AdbConfigurationFailure( + "host_capacity", + `The host could not start the selected adb binary (${code}): ${selection.binary}`, + adbHostCapacityRemediation(code), + facts, + { cause }, + ); throw new AdbConfigurationFailure( "binary_not_executable", - `Execution of the selected adb binary was denied (${failure.code}): ${selection.binary}`, - ADB_NOT_EXECUTABLE_REMEDIATION, - { - selected_binary: selection.binary, - path_source: selection.pathSource, - filesystem_error: failure.code, - }, + code === "EACCES" || code === "EPERM" + ? `Execution of the selected adb binary was denied (${code}): ${selection.binary}` + : `The selected adb binary could not be started (${code}): ${selection.binary}`, + code === "EACCES" + ? fromEnvironment + ? ADB_NOT_EXECUTABLE_REMEDIATION + : `Make sure the adb found on PATH is a file with execute permission, not a directory, and that its parent directories have search permission, or set ${ADB_BINARY_ENV} to an absolute path to an executable adb binary.` + : code === "EPERM" + ? `The host refused to run this adb binary, for example because of a security policy or file flags; allow it to run, or set ${ADB_BINARY_ENV} to an absolute path to an adb binary the host permits.` + : `Select an adb executable this host can start directly: set ${ADB_BINARY_ENV} to its absolute path, or make it the adb found on PATH.`, + facts, { cause }, ); + } throw new AdbConfigurationFailure( "version_unresolved", - "The selected adb binary did not report a usable version", + `The selected adb binary did not report a usable version: ${selection.binary}`, ADB_VERSION_REMEDIATION, { selected_binary: selection.binary, + path_source: selection.pathSource, ...(failure === undefined ? {} : { @@ -294,7 +326,9 @@ const rejectionCodeOf = ( code === "binary_not_executable" || code === "binary_unresolved" ? "not_configured" - : "version_unresolved"; + : code === "host_capacity" + ? "runtime_unavailable" + : "version_unresolved"; export const adbConfigurationAvailability = ( cause: AdbConfigurationFailure, diff --git a/src/adb/AdbProvider.test.ts b/src/adb/AdbProvider.test.ts index 08303731fb..4c4aed2f87 100644 --- a/src/adb/AdbProvider.test.ts +++ b/src/adb/AdbProvider.test.ts @@ -13,6 +13,8 @@ import { expect, onTestFinished, test, vi } from "vitest"; const adbTestHooks = vi.hoisted(() => ({ afterScreencap: undefined as (() => void) | undefined, + versionProbeFailure: undefined as Error | undefined, + digestCheckFailure: undefined as Error | undefined, })); vi.mock("../process/ExecFileOutput.js", async (importOriginal) => { @@ -23,10 +25,21 @@ vi.mock("../process/ExecFileOutput.js", async (importOriginal) => { execFileBytes: async ( ...arguments_: Parameters ) => { + const failure = adbTestHooks.digestCheckFailure; + if (failure !== undefined && arguments_[1].includes("sha256sum")) + throw failure; const result = await actual.execFileBytes(...arguments_); if (arguments_[1].includes("exec-out")) adbTestHooks.afterScreencap?.(); return result; }, + execFileOutput: async ( + ...arguments_: Parameters + ) => { + const failure = adbTestHooks.versionProbeFailure; + if (failure !== undefined && arguments_[1].includes("version")) + throw failure; + return actual.execFileOutput(...arguments_); + }, }; }); @@ -611,6 +624,45 @@ it("reports a missing selected binary as a capability gap with recovery", async }); }); +it("reports exhausted host capacity at the version probe as a retryable constraint", async () => { + const { binary } = await writeAdbStub(() => VERSION_BLOCK); + adbTestHooks.versionProbeFailure = Object.assign(new Error("spawn EMFILE"), { + code: "EMFILE", + syscall: `spawn ${binary}`, + spawnFailed: true, + stdout: "", + stderr: "", + signal: null, + killed: false, + }); + onTestFinished(() => { + adbTestHooks.versionProbeFailure = undefined; + }); + // The readiness operation runs the same probe as the device operations. + for (const operation of ["inspect_adb_client", "list_adb_devices"] as const) { + const projected = await refusedBy(providerOver(binary), { + operation, + input: {}, + }); + expect(projected).toMatchObject({ + code: "resource_constraint", + retryable: true, + remediation: { + action: expect.stringContaining("does not need to change") as unknown, + }, + details: { + diagnostics: { + selected_binary: binary, + path_source: "environment", + filesystem_error: "EMFILE", + }, + }, + }); + // One remediation, carried by the constraint rather than its diagnostics. + expect(projected.details?.diagnostics).not.toHaveProperty("remediation"); + } +}); + it("reports relative adb configuration as an actionable refusal", async () => { const provider = new AdbProvider({ environment: { REA_ADB_PATH: "relative/adb" }, @@ -1205,6 +1257,52 @@ exit 1`, }, ); +it("keeps a completed push as a receipt when host capacity stops its digest check", async () => { + const { binary, root } = await writeAdbStub( + () => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi +if [ "$1" = "-s" ] && [ "$4" = "test" ]; then exit 1; fi +if [ "$1" = "-s" ] && [ "$3" = "push" ]; then exit 0; fi +exit 1`, + ); + adbTestHooks.digestCheckFailure = Object.assign(new Error("spawn EMFILE"), { + code: "EMFILE", + syscall: `spawn ${binary}`, + spawnFailed: true, + stdout: "", + stderr: "", + signal: null, + killed: false, + }); + onTestFinished(() => { + adbTestHooks.digestCheckFailure = undefined; + }); + const localFile = join(root, "payload.bin"); + await writeFile(localFile, "push-payload"); + const result = await providerOver(binary).execute({ + operation: "push_adb_file", + input: { + serial: "emulator-5554", + local_path: localFile, + device_path: "/data/local/tmp/payload.bin", + overwrite: false, + }, + }); + // A retryable error would invite a repeat that overwrite=false then refuses. + if (!result.ok) + throw new Error(JSON.stringify(projectAnalysisError(result.error))); + expect(result.value.result).toMatchObject({ + sha256: PUSHED_DIGEST, + device_sha256: null, + device_digest_source: "unavailable", + }); + expect( + result.value.limitations.some( + (line) => + line.includes("EMFILE") && line.includes("not device-side verified"), + ), + ).toBe(true); +}); + it("pushes the same private bytes that its reported digest identifies", async () => { const { binary, root } = await writeAdbStub( (stubRoot) => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi diff --git a/src/adb/AdbProvider.ts b/src/adb/AdbProvider.ts index ac8f80c610..00cc3ec7ab 100644 --- a/src/adb/AdbProvider.ts +++ b/src/adb/AdbProvider.ts @@ -95,6 +95,7 @@ import { import { adbCommandOutputFailure, adbArguments, + adbSpawnCapacityError, runAdb, runAdbCapture, packageChangeRefusal, @@ -1317,6 +1318,18 @@ export class AdbProvider { { prerequisite_operation: "inspect_adb_client" }, ), ); + if ( + cause instanceof AdbConfigurationFailure && + cause.code === "host_capacity" + ) + return err( + adbSpawnCapacityError( + request.operation, + String(cause.diagnostics.filesystem_error), + cause, + cause.diagnostics, + ), + ); if (cause instanceof AdbConfigurationFailure) return err( providerConfigurationRefusal( @@ -2501,10 +2514,16 @@ export class AdbProvider { if (!deviceDigest.ok) { const cancelled = deviceDigest.error instanceof AnalysisCancelledError; + // The check never started and the push already completed, so a + // retryable error would invite a repeat that overwrite=false refuses. + const hostCapacity = + deviceDigest.error instanceof AnalysisResourceConstraintError && + deviceDigest.error.resource === "capacity"; const verificationFailed = cancelled || - deviceDigest.error instanceof AnalysisResourceConstraintError || - deviceDigest.error instanceof AnalysisTimeoutError; + (!hostCapacity && + (deviceDigest.error instanceof AnalysisResourceConstraintError || + deviceDigest.error instanceof AnalysisTimeoutError)); const completed = receipt({ device_sha256: null, device_digest_source: "unavailable", @@ -2512,7 +2531,9 @@ export class AdbProvider { ? "The device-side digest check was cancelled after push completed" : verificationFailed ? `Device-side digest verification failed after push completed: ${deviceDigest.error.message}` - : "The device did not answer sha256sum, so the transfer is not device-side verified", + : hostCapacity + ? `Push completed, but the transfer is not device-side verified: ${deviceDigest.error.reason}` + : "The device did not answer sha256sum, so the transfer is not device-side verified", }); if (verificationFailed) deviceDigest.error.retainPartialObservation( diff --git a/src/adb/AdbVersionProbe.test.ts b/src/adb/AdbVersionProbe.test.ts index 55be8b94f6..c8f9285267 100644 --- a/src/adb/AdbVersionProbe.test.ts +++ b/src/adb/AdbVersionProbe.test.ts @@ -3,7 +3,11 @@ import { expect, it, vi } from "vitest"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { AnalysisTimeoutError } from "../domain/analysisErrorCore.js"; import { execFileOutput } from "../process/ExecFileOutput.js"; -import { inspectAdbClient } from "./AdbConfiguration.js"; +import { + AdbConfigurationFailure, + adbConfigurationAvailability, + inspectAdbClient, +} from "./AdbConfiguration.js"; vi.mock("../process/ExecFileOutput.js", async (importOriginal) => ({ ...(await importOriginal()), @@ -70,6 +74,7 @@ it("distinguishes execute denial from missing or unreadable version output", asy vi.mocked(execFileOutput).mockRejectedValueOnce( Object.assign(new Error("denied"), { code: "EACCES", + spawnFailed: true, stdout: "", stderr: "", signal: null, @@ -84,3 +89,83 @@ it("distinguishes execute denial from missing or unreadable version output", asy }, }); }); + +it("names an EPERM execute refusal separately from missing permission", async () => { + vi.mocked(execFileOutput).mockRejectedValueOnce( + Object.assign(new Error("not permitted"), { + code: "EPERM", + spawnFailed: true, + stdout: "", + stderr: "", + signal: null, + killed: false, + }), + ); + await expect(inspectAdbClient(selection)).rejects.toMatchObject({ + code: "binary_not_executable", + remediation: expect.stringContaining("REA_ADB_PATH") as unknown, + diagnostics: { + selected_binary: "/host/adb", + path_source: "environment", + filesystem_error: "EPERM", + }, + }); +}); + +it("names any other not-started spawn errno without an exit code", async () => { + vi.mocked(execFileOutput).mockRejectedValueOnce( + Object.assign(new Error("spawn EINVAL"), { + code: "EINVAL", + spawnFailed: true, + stdout: "", + stderr: "", + signal: null, + killed: false, + }), + ); + const failure = await inspectAdbClient(selection).catch( + (cause: unknown) => cause, + ); + expect(failure).toMatchObject({ + code: "binary_not_executable", + remediation: expect.stringContaining("REA_ADB_PATH") as unknown, + diagnostics: { + selected_binary: "/host/adb", + path_source: "environment", + filesystem_error: "EINVAL", + }, + }); + expect(failure).not.toHaveProperty("diagnostics.exit_code"); +}); + +it("does not blame the selected path for exhausted host spawn capacity", async () => { + vi.mocked(execFileOutput).mockRejectedValueOnce( + Object.assign(new Error("spawn EMFILE"), { + code: "EMFILE", + spawnFailed: true, + stdout: "", + stderr: "", + signal: null, + killed: false, + }), + ); + const failure = await inspectAdbClient(selection).catch( + (cause: unknown) => cause, + ); + expect(failure).toMatchObject({ + code: "host_capacity", + remediation: expect.any(String) as unknown, + diagnostics: { + selected_binary: "/host/adb", + path_source: "environment", + filesystem_error: "EMFILE", + }, + }); + expect(failure).not.toHaveProperty("diagnostics.exit_code"); + if (!(failure instanceof AdbConfigurationFailure)) throw failure; + expect(adbConfigurationAvailability(failure)).toMatchObject({ + status: "unavailable", + code: "runtime_unavailable", + remediation: failure.remediation, + }); +}); diff --git a/src/filesystem/RegularFile.ts b/src/filesystem/RegularFile.ts index 1230dd93bf..35e7ff7566 100644 --- a/src/filesystem/RegularFile.ts +++ b/src/filesystem/RegularFile.ts @@ -63,11 +63,20 @@ export class RegularFileChangedError extends Error { } } -const SELECTED_REGULAR_FILE_ERROR_CODES = new Set([ +/** Errnos establishing that no file resolves at the path as written. */ +const UNRESOLVED_PATH_ERROR_CODES: ReadonlySet = new Set([ "ENOENT", "ENOTDIR", "ELOOP", "ENAMETOOLONG", +]); + +/** Whether an errno establishes that nothing resolves at the selected path. */ +export const isUnresolvedPathErrorCode = (code: unknown): boolean => + typeof code === "string" && UNRESOLVED_PATH_ERROR_CODES.has(code); + +const SELECTED_REGULAR_FILE_ERROR_CODES = new Set([ + ...UNRESOLVED_PATH_ERROR_CODES, "EISDIR", "ENOTFILE", "ESTALE", diff --git a/src/process/ExecFileOutput.test.ts b/src/process/ExecFileOutput.test.ts index 4bf135651d..2a126a5789 100644 --- a/src/process/ExecFileOutput.test.ts +++ b/src/process/ExecFileOutput.test.ts @@ -654,6 +654,85 @@ describe("execFileOutput failure metadata", () => { code: 7, }); }); + // Without stopSignal the command takes Node's execFile; adb always passes + // one and takes the owned spawn route. Both throw ENOTDIR synchronously. + it.skipIf(process.platform === "win32").each([ + { route: "execFile", options: {} }, + { route: "spawn", options: { stopSignal: "SIGTERM" as const } }, + ])( + "admits a synchronously thrown spawn error through $route with its code and empty capture", + async ({ options }) => { + const root = await createTestTempDirectory("rea-exec-enotdir-"); + const file = join(root, "regular"); + await writeFile(file, ""); + const failure: unknown = await execFileOutput( + join(file, "adb"), + ["version"], + options, + ).then( + () => undefined, + (cause: unknown) => cause, + ); + + expect(failure).toMatchObject({ code: "ENOTDIR", syscall: "spawn" }); + expect(execFileOutputFailure(failure)).toEqual({ + stdout: "", + stderr: "", + stdoutBytes: 0, + stderrBytes: 0, + code: "ENOTDIR", + signal: null, + killed: false, + outputTruncated: false, + spawnFailed: true, + }); + }, + ); + + it("marks an asynchronous spawn failure but not a started process's exit", async () => { + const root = await createTestTempDirectory("rea-exec-spawn-owner-"); + const missing = join(root, "missing"); + const spawnFailure: unknown = await execFileOutput(missing, [ + "version", + ]).then( + () => undefined, + (cause: unknown) => cause, + ); + expect(spawnFailure).toMatchObject({ + code: "ENOENT", + path: missing, + spawnargs: ["version"], + }); + expect(execFileOutputFailure(spawnFailure)).toMatchObject({ + code: "ENOENT", + spawnFailed: true, + }); + + const exitFailure: unknown = await execFileOutput(process.execPath, [ + "-e", + "process.exitCode = 3", + ]).then( + () => undefined, + (cause: unknown) => cause, + ); + expect(execFileOutputFailure(exitFailure)).toMatchObject({ code: 3 }); + expect(execFileOutputFailure(exitFailure)).not.toHaveProperty( + "spawnFailed", + ); + }); + + it("leaves a synchronous argument validation error unadmitted", async () => { + const failure: unknown = await execFileOutput(process.execPath, [ + "nul\0byte", + ]).then( + () => undefined, + (cause: unknown) => cause, + ); + + expect(failure).toMatchObject({ code: "ERR_INVALID_ARG_VALUE" }); + expect(execFileOutputFailure(failure)).toBeUndefined(); + }); + it("preserves explicit truncation metadata from an owned process failure", () => { const failure = Object.assign(new Error("owned output was truncated"), { stdout: "partial stdout", diff --git a/src/process/ExecFileOutput.ts b/src/process/ExecFileOutput.ts index 19522a6901..a96837152e 100644 --- a/src/process/ExecFileOutput.ts +++ b/src/process/ExecFileOutput.ts @@ -87,6 +87,8 @@ export interface ExecFileOutputFailure { readonly stdoutBytes?: number; readonly stderrBytes?: number; readonly collectionFailure?: boolean; + /** The process never started; `code` is the spawn errno, not an exit. */ + readonly spawnFailed?: true; } /** Read captured output metadata from a failed execFileOutput invocation. */ @@ -103,6 +105,7 @@ export const execFileOutputFailure = ( const stdoutBytes = Reflect.get(cause, "stdoutBytes"); const stderrBytes = Reflect.get(cause, "stderrBytes"); const collectionFailure = Reflect.get(cause, "collectionFailure"); + const spawnFailed = Reflect.get(cause, "spawnFailed"); if (typeof stdout !== "string" || typeof stderr !== "string") return undefined; return { @@ -124,9 +127,30 @@ export const execFileOutputFailure = ( outputTruncated: outputTruncated === true || code === "ERR_CHILD_PROCESS_STDIO_MAXBUFFER", ...(collectionFailure === true ? { collectionFailure: true } : {}), + ...(spawnFailed === true ? { spawnFailed: true } : {}), }; }; +/** Spawn errnos reporting exhausted host capacity rather than an unusable command. */ +const HOST_SPAWN_CAPACITY_CODES: ReadonlySet = new Set([ + "EAGAIN", + "EMFILE", + "ENFILE", + "ENOMEM", +]); + +/** + * Whether the command never started because the host lacked process, memory + * or file-descriptor capacity. The command itself may be usable; a retry after + * capacity is freed can succeed. + */ +export const spawnCapacityExhausted = ( + failure: ExecFileOutputFailure | undefined, +): boolean => + failure?.spawnFailed === true && + typeof failure.code === "string" && + HOST_SPAWN_CAPACITY_CODES.has(failure.code); + /** Project admitted command failure facts without reinterpreting failure policy. */ export const execFileFailureCapturedOutput = ( failure: ExecFileOutputFailure, @@ -144,6 +168,23 @@ export const execFileFailureCapturedOutput = ( truncated: failure.outputTruncated, }); +/** + * Mark an errno failure from starting the process, which Node reports as + * `syscall: "spawn"` when thrown synchronously and `spawn ` when + * emitted. Argument validation errors carry no errno. + */ +const admitSpawnFailure = (error: Error): boolean => { + const syscall = Reflect.get(error, "syscall"); + if ( + typeof Reflect.get(error, "errno") !== "number" || + typeof syscall !== "string" || + !(syscall === "spawn" || syscall.startsWith("spawn ")) + ) + return false; + Reflect.set(error, "spawnFailed", true); + return true; +}; + /** Run a shell-free command with no input while capturing its complete output bytes. */ export const execFileBytes = ( command: string, @@ -172,6 +213,7 @@ export const execFileBytes = ( stderr: Buffer, ): void => { if (error !== null) { + admitSpawnFailure(error); Reflect.set(error, "stdout", stdout.toString("utf8")); Reflect.set(error, "stderr", stderr.toString("utf8")); Reflect.set(error, "stdoutBytes", stdout.byteLength); @@ -181,36 +223,47 @@ export const execFileBytes = ( } resolve({ stdout, stderr }); }; - if (stopSignal === undefined) { - const child = execFile( + try { + if (stopSignal === undefined) { + const child = execFile( + command, + [...arguments_], + { + ...execOptions, + ...(signal === undefined ? {} : { signal }), + ...(timeout === undefined ? {} : { timeout }), + encoding: "buffer", + maxBuffer: options.maxBuffer ?? Number.POSITIVE_INFINITY, + }, + (error, stdout, stderr) => + complete( + error?.name === "AbortError" && error.code === "ABORT_ERR" + ? new ExecFileOutputCancelledError(error.cause) + : error, + stdout, + stderr, + ), + ); + // This promise API exposes no input writer; EOF belongs to the owner. + child.stdin?.end(); + return; + } + startStoppableCommand( command, - [...arguments_], - { - ...execOptions, - ...(signal === undefined ? {} : { signal }), - ...(timeout === undefined ? {} : { timeout }), - encoding: "buffer", - maxBuffer: options.maxBuffer ?? Number.POSITIVE_INFINITY, - }, - (error, stdout, stderr) => - complete( - error?.name === "AbortError" && error.code === "ABORT_ERR" - ? new ExecFileOutputCancelledError(error.cause) - : error, - stdout, - stderr, - ), + arguments_, + { ...options, stopSignal }, + complete, ); - // This promise API exposes no input writer; EOF belongs to the owner. - child.stdin?.end(); - return; + } catch (error) { + // Node throws spawn errors other than EACCES, EAGAIN, EMFILE, ENFILE and + // ENOENT (for example ENOTDIR, EPERM or ENOMEM) synchronously. No + // process started, so a spawn errno completes with the same empty + // capture an asynchronous spawn failure carries; argument validation + // errors stay unadmitted. + if (error instanceof Error && admitSpawnFailure(error)) + complete(error, Buffer.alloc(0), Buffer.alloc(0)); + else reject(error); } - startStoppableCommand( - command, - arguments_, - { ...options, stopSignal }, - complete, - ); }); /** Own stop requests and bounded capture until the root and inherited pipes close. */ @@ -244,8 +297,14 @@ const startStoppableCommand = ( : ["ignore", "pipe", "pipe"], killSignal: stopSignal, }); + child.on("error", (error: Error) => { + processError ??= error; + }); + // After an EMFILE or ENFILE spawn failure Node creates no stdio and reports + // the failure only through the error and close events. + const stdio = child.stdio as typeof child.stdio | undefined; if (lifetimePipe === true) { - const pipe = child.stdio[3]; + const pipe = stdio?.[3]; if (pipe != null && "resume" in pipe) pipe.resume(); } // The first stop request names the failure; a later one is not resent. @@ -259,12 +318,12 @@ const startStoppableCommand = ( stop("abort"); }; const capture = ( - stream: NonNullable, + stream: typeof child.stdout | undefined, name: "stdout" | "stderr", ) => { const chunks: Buffer[] = []; let retained = 0; - stream.on("data", (chunk: Buffer) => { + stream?.on("data", (chunk: Buffer) => { const remaining = Math.max(0, Math.floor(maxBuffer - retained)); if (chunk.byteLength <= remaining) { chunks.push(chunk); @@ -283,11 +342,8 @@ const startStoppableCommand = ( }); return () => Buffer.concat(chunks, retained); }; - const stdout = capture(child.stdout!, "stdout"); - const stderr = capture(child.stderr!, "stderr"); - child.on("error", (error: Error) => { - processError ??= error; - }); + const stdout = capture(stdio?.[1], "stdout"); + const stderr = capture(stdio?.[2], "stderr"); child.on("exit", (code, exitSignal) => { // A failed root has completed its outcome even while descendants retain // its pipes. Finalization cancellation cannot replace that failure. diff --git a/tests/boundary/cli/adbCli.test.ts b/tests/boundary/cli/adbCli.test.ts index 23fccc5593..01b77ff3b8 100644 --- a/tests/boundary/cli/adbCli.test.ts +++ b/tests/boundary/cli/adbCli.test.ts @@ -1,4 +1,11 @@ -import { chmod, lstat, readFile, unlink, writeFile } from "node:fs/promises"; +import { + chmod, + lstat, + readFile, + symlink, + unlink, + writeFile, +} from "node:fs/promises"; import { createHash } from "node:crypto"; import { crc32 } from "node:zlib"; import { basename, dirname, join } from "node:path"; @@ -242,6 +249,106 @@ it.each([ }, ); +it.skipIf(process.platform === "win32")( + "names an existing REA_ADB_PATH the host cannot execute", + async () => { + const workspace = await createTestWorkspace("rea-adb-unusable-cli-"); + onTestFinished(() => removeTestWorkspace(workspace.root)); + const directory = dirname(await workspace.write("adb-dir/keep", "")); + const file = await workspace.write( + "adb", + `#!/bin/sh +printf 'Android Debug Bridge version 1.0.41\\nVersion 34.0.5-cli\\n' +`, + ); + await chmod(file, 0o644); + for (const [argv, binary] of [ + [["inspect-adb-client", "--json"], directory], + [["inspect-adb-client", "--json"], file], + [["list-adb-devices", "--json"], directory], + ] as const) { + const result = await serve(argv, { REA_ADB_PATH: binary }); + expectConfigurationRefusal( + result, + `Execution of the selected adb binary was denied (EACCES): ${binary}. Set REA_ADB_PATH to an absolute path to the adb executable file, not a directory`, + ); + } + // A parent component that is a file, a symlink loop and a missing path + // fail before any process starts, so no exit code is reported. + const loop = workspace.path("adb-loop"); + await symlink(loop, loop); + for (const [binary, filesystemError] of [ + [`${file}/adb`, "ENOTDIR"], + [loop, "ELOOP"], + [workspace.path("missing-adb"), "ENOENT"], + ] as const) { + const refused = await serve(["inspect-adb-client", "--json"], { + REA_ADB_PATH: binary, + }); + expectConfigurationRefusal( + refused, + `The selected adb binary was not found: ${binary}. Set REA_ADB_PATH`, + ); + const { details } = JSON.parse(refused.stdout) as { + details: { diagnostics: Record }; + }; + expect(details.diagnostics).toMatchObject({ + selected_binary: binary, + path_source: "environment", + filesystem_error: filesystemError, + }); + expect(details.diagnostics).not.toHaveProperty("exit_code"); + } + await chmod(file, 0o755); + const result = await serve(["inspect-adb-client", "--json"], { + REA_ADB_PATH: file, + }); + expect(JSON.parse(result.stdout)).toMatchObject({ + normalized_result: { + client: { binary_path: file, path_source: "environment" }, + }, + }); + }, +); + +it.skipIf(process.platform === "win32")( + "names a PATH adb the host cannot execute", + async () => { + const workspace = await createTestWorkspace("rea-adb-unusable-path-"); + onTestFinished(() => removeTestWorkspace(workspace.root)); + const adb = await workspace.write("bin/adb", "#!/bin/sh\nexit 0\n"); + await chmod(adb, 0o644); + const environment: Record = { + ...process.env, + PATH: dirname(adb), + }; + delete environment.REA_ADB_PATH; + const stdout = await execFileOutput( + process.execPath, + ["scripts/rea.mjs", "inspect-adb-client", "--json"], + { + env: environment, + timeout: 20_000, + stopSignal: "SIGTERM", + maxBuffer: 1024 * 1024, + }, + ).then( + () => { + throw new Error("inspect-adb-client accepted a non-executable adb"); + }, + (cause: unknown) => { + const failure = execFileOutputFailure(cause); + if (failure === undefined) throw cause; + return failure.stdout; + }, + ); + expectConfigurationRefusal( + { stdout, exitCode: 1 }, + "Execution of the selected adb binary was denied (EACCES): adb. Make sure the adb found on PATH is a file with execute permission, not a directory", + ); + }, +); + it("validates device serials before contacting any device", async () => { const result = await serve(["inspect-adb-device", "bad serial", "--json"]); expect(result.exitCode).toBe(1); diff --git a/tests/boundary/process/adbDescriptorPressure.test.ts b/tests/boundary/process/adbDescriptorPressure.test.ts new file mode 100644 index 0000000000..c7d962225f --- /dev/null +++ b/tests/boundary/process/adbDescriptorPressure.test.ts @@ -0,0 +1,99 @@ +import { execFile } from "node:child_process"; +import { chmod, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { promisify } from "node:util"; +import { expect, it } from "vitest"; + +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +// Exhausts the open-file limit, then starts the selected adb through the +// version probe and a device command; both take the owned stop-signal route. +const PRESSURE = ` +import { open } from "node:fs/promises"; +import { pathToFileURL } from "node:url"; +const [repo, binary] = process.argv.slice(1); +const load = (path) => import(pathToFileURL(repo + "/dist/" + path)); +const { inspectAdbClient } = await load("adb/AdbConfiguration.js"); +const { runAdbCapture } = await load("adb/AdbCommand.js"); +const selection = { binary, pathSource: "environment", environment: {} }; +const probe = () => + inspectAdbClient(selection).then( + (identity) => ({ version: identity.version }), + (cause) => ({ code: cause.code, diagnostics: cause.diagnostics }), + ); +const command = () => + runAdbCapture("adb devices", { device: "serial" }, selection, ["devices"], + { timeoutMs: 10_000, maxOutputBytes: 1024 }, undefined, + ).then((result) => result.ok ? { ok: true } : { + tag: result.error._tag, + resource: result.error.resource, + retryable: result.error.retryable, + reportedLimits: result.error.reportedLimits, + }); +const handles = []; +for (let index = 0; index < 128; index += 1) { + try { + handles.push(await open("/dev/null", "r")); + } catch (cause) { + if (cause.code !== "EMFILE") throw cause; + break; + } +} +if (handles.length === 128) throw new Error("Run under a low descriptor limit"); +const pressure = { probe: await probe(), command: await command() }; +for (const handle of handles) await handle.close(); +const after = { probe: await probe(), command: await command() }; +process.stdout.write(JSON.stringify({ pressure, after })); +`; + +it.skipIf(process.platform === "win32")( + "reports open-file exhaustion as retryable host capacity without an unhandled spawn error", + async () => { + const directory = await createTestTempDirectory("rea-adb-descriptors-"); + const adb = join(directory, "adb"); + await writeFile( + adb, + `#!/bin/sh +if [ "$1" = version ]; then printf 'Android Debug Bridge version 1.0.41\\nVersion 34.0.5-cli\\n'; fi +exit 0 +`, + ); + await chmod(adb, 0o755); + const { stdout, stderr } = await promisify(execFile)( + "/bin/sh", + [ + "-c", + 'ulimit -n 64; exec "$@"', + "adb-descriptor-pressure", + process.execPath, + "--input-type=module", + "--eval", + PRESSURE, + process.cwd(), + adb, + ], + { timeout: 30_000, maxBuffer: 1024 * 1024 }, + ); + + // An unhandled child error would exit the process before this report. + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ + pressure: { + probe: { + code: "host_capacity", + diagnostics: expect.objectContaining({ + selected_binary: adb, + filesystem_error: "EMFILE", + }), + }, + command: { + tag: "AnalysisResourceConstraintError", + resource: "capacity", + retryable: true, + reportedLimits: { spawn_error: "EMFILE" }, + }, + }, + after: { probe: { version: "34.0.5-cli" }, command: { ok: true } }, + }); + }, +);