From b7e76a57cd29853e796b05a770cb14a0c435f35c Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 26 Oct 2026 23:06:11 +0800 Subject: [PATCH] fix(adb): preserve selected environment through provider execution --- src/adb/AdbCommand.test.ts | 10 +-- src/adb/AdbCommand.ts | 18 ++--- src/adb/AdbConfiguration.test.ts | 3 + src/adb/AdbConfiguration.ts | 18 ++++- src/adb/AdbProvider.ts | 39 +++++------ src/adb/AdbVersionProbe.test.ts | 1 + .../adb/adbCommandFinalization.test.ts | 13 ++-- .../adb/adbSelectedLocalPaths.test.ts | 65 ++++++++++++++++++- 8 files changed, 126 insertions(+), 41 deletions(-) diff --git a/src/adb/AdbCommand.test.ts b/src/adb/AdbCommand.test.ts index 3791142e9f..5fff97243e 100644 --- a/src/adb/AdbCommand.test.ts +++ b/src/adb/AdbCommand.test.ts @@ -16,7 +16,7 @@ const probe = (signal?: AbortSignal) => "push_adb_file", "device", "/device/path", - "/host/adb", + { binary: "/host/adb", pathSource: "environment", environment: {} }, { timeoutMs: 100, maxOutputBytes: 1024, @@ -40,7 +40,7 @@ it("classifies the complete diagnostic before shortening its displayed reason", runAdbCapture( "pull_adb_file", { device: "device", path: "/remote/file", localPath }, - "/host/adb", + { binary: "/host/adb", pathSource: "environment", environment: {} }, [], { timeoutMs: 100, maxOutputBytes: 2048 }, undefined, @@ -200,7 +200,7 @@ it.each([true, false])( const result = await runAdbCapture( volatileOutput ? "read_adb_logcat" : "install_adb_package", { device: "device" }, - "/host/adb", + { binary: "/host/adb", pathSource: "environment", environment: {} }, volatileOutput ? ["logcat", "-d"] : ["install", "app.apk"], { timeoutMs: 100, maxOutputBytes: 1024, volatileOutput }, undefined, @@ -230,7 +230,7 @@ it("reports the effective fixed adb deadline", async () => { const result = await runAdbCapture( "pull_adb_file", { device: "device" }, - "/host/adb", + { binary: "/host/adb", pathSource: "environment", environment: {} }, ["pull", "/remote"], { timeoutMs: 600_000, maxOutputBytes: 1024 }, undefined, @@ -255,7 +255,7 @@ it("keeps a physical failure even when the later abort reason is that same error const result = await runAdbCapture( "inspect_adb_device", { device: "device" }, - "/host/adb", + { binary: "/host/adb", pathSource: "environment", environment: {} }, [], { timeoutMs: 100, maxOutputBytes: 1024 }, controller.signal, diff --git a/src/adb/AdbCommand.ts b/src/adb/AdbCommand.ts index 223f769003..af67e9f932 100644 --- a/src/adb/AdbCommand.ts +++ b/src/adb/AdbCommand.ts @@ -17,6 +17,7 @@ import { } from "../process/ExecFileOutput.js"; import { packageChangeFailureReason } from "./AdbShellOutput.js"; import { isSuccessfulDaemonStartNotice } from "./AdbDeviceOutput.js"; +import type { AdbBinarySelection } from "./AdbConfiguration.js"; const STDERR_EXCERPT_CHARACTERS = 512; @@ -97,7 +98,7 @@ export interface AdbCommandTarget { export const runAdbCapture = async ( operation: string, target: AdbCommandTarget, - binary: string, + selection: AdbBinarySelection, arguments_: readonly string[], limits: AdbRunLimits, signal: AbortSignal | undefined, @@ -106,7 +107,8 @@ export const runAdbCapture = async ( return err(new AnalysisCancelledError(operation, { cause: signal.reason })); try { return ok( - await execFileBytes(binary, arguments_, { + await execFileBytes(selection.binary, arguments_, { + env: selection.environment, timeout: limits.timeoutMs, maxBuffer: limits.maxOutputBytes, stopSignal: "SIGTERM", @@ -268,11 +270,11 @@ export const probeAdbPathExistence = async ( operation: string, serial: string, path: string, - binary: string, + selection: AdbBinarySelection, limits: AdbRunLimits, signal: AbortSignal | undefined, ): Promise> => { - return probeAdbPath(operation, serial, path, binary, limits, signal, "-e"); + return probeAdbPath(operation, serial, path, selection, limits, signal, "-e"); }; /** Only the clean true/false result of test -d establishes directory status. */ @@ -280,17 +282,17 @@ export const probeAdbPathDirectory = async ( operation: string, serial: string, path: string, - binary: string, + selection: AdbBinarySelection, limits: AdbRunLimits, signal: AbortSignal | undefined, ): Promise> => - probeAdbPath(operation, serial, path, binary, limits, signal, "-d"); + probeAdbPath(operation, serial, path, selection, limits, signal, "-d"); const probeAdbPath = async ( operation: string, serial: string, path: string, - binary: string, + selection: AdbBinarySelection, limits: AdbRunLimits, signal: AbortSignal | undefined, predicate: "-d" | "-e", @@ -300,7 +302,7 @@ const probeAdbPath = async ( const probed = await runAdbCapture( operation, { device: serial, path }, - binary, + selection, adbArguments(serial, ["shell", "test", predicate, path]), { ...limits, volatileOutput: true }, signal, diff --git a/src/adb/AdbConfiguration.test.ts b/src/adb/AdbConfiguration.test.ts index bc786db710..35ad7e2f00 100644 --- a/src/adb/AdbConfiguration.test.ts +++ b/src/adb/AdbConfiguration.test.ts @@ -11,6 +11,7 @@ describe("resolveAdbBinary", () => { expect(resolveAdbBinary({ [ADB_BINARY_ENV]: "/opt/tools/adb" })).toEqual({ binary: "/opt/tools/adb", pathSource: "environment", + environment: { [ADB_BINARY_ENV]: "/opt/tools/adb" }, }); }); @@ -18,10 +19,12 @@ describe("resolveAdbBinary", () => { expect(resolveAdbBinary({})).toEqual({ binary: "adb", pathSource: "path", + environment: {}, }); expect(resolveAdbBinary({ [ADB_BINARY_ENV]: " " })).toEqual({ binary: "adb", pathSource: "path", + environment: { [ADB_BINARY_ENV]: " " }, }); }); diff --git a/src/adb/AdbConfiguration.ts b/src/adb/AdbConfiguration.ts index 778af73f2a..2661dfde27 100644 --- a/src/adb/AdbConfiguration.ts +++ b/src/adb/AdbConfiguration.ts @@ -4,6 +4,7 @@ import { AnalysisTimeoutError, } from "../domain/analysisErrorCore.js"; import { isAbsolute } from "node:path"; +import { snapshotEnvironment } from "../process/snapshotEnvironment.js"; import { execFileOutput, @@ -26,6 +27,7 @@ const VERSION_MAX_OUTPUT_BYTES = 64 * 1024; export interface AdbBinarySelection { readonly binary: string; readonly pathSource: "environment" | "path"; + readonly environment: Readonly>; } /** @@ -36,16 +38,25 @@ export interface AdbBinarySelection { export const resolveAdbBinary = ( environment: Readonly>, ): AdbBinarySelection => { - const configured = environment[ADB_BINARY_ENV]; + const selectedEnvironment = snapshotEnvironment(environment); + const configured = selectedEnvironment[ADB_BINARY_ENV]; if (configured === undefined || configured.trim() === "") - return { binary: "adb", pathSource: "path" }; + return { + binary: "adb", + pathSource: "path", + environment: selectedEnvironment, + }; if (!isAbsolute(configured)) throw new AdbConfigurationFailure( "not_absolute", `${ADB_BINARY_ENV} must be an absolute path to the adb binary`, { configured_path: configured }, ); - return { binary: configured, pathSource: "environment" }; + return { + binary: configured, + pathSource: "environment", + environment: selectedEnvironment, + }; }; export type AdbConfigurationFailureCode = @@ -98,6 +109,7 @@ export const inspectAdbClient = async ( selection.binary, ["version"], { + env: selection.environment, timeout: VERSION_TIMEOUT_MS, stopSignal: "SIGTERM", maxBuffer: VERSION_MAX_OUTPUT_BYTES, diff --git a/src/adb/AdbProvider.ts b/src/adb/AdbProvider.ts index 7912e586ab..01f5793acb 100644 --- a/src/adb/AdbProvider.ts +++ b/src/adb/AdbProvider.ts @@ -104,6 +104,7 @@ import { type AdbBinarySelection, type AdbClientIdentity, } from "./AdbConfiguration.js"; +import { snapshotEnvironment } from "../process/snapshotEnvironment.js"; import { displayDevicePath, parseAdbDevicesOutput, @@ -883,7 +884,7 @@ export class AdbProvider { #outputCleanupPromise: Promise | undefined; constructor(options: AdbProviderOptions = {}) { - this.environment = options.environment ?? process.env; + this.environment = snapshotEnvironment(options.environment ?? process.env); this.#createRuntimeRoot = options.createRuntimeRoot ?? ((rootOptions) => PrivateRuntimeRoot.create(rootOptions)); @@ -1370,7 +1371,7 @@ export class AdbProvider { { device: "serial" in request.input ? request.input.serial : operation, }, - selection.binary, + selection, adbArguments( "serial" in request.input ? request.input.serial : undefined, ["shell", ...arguments_], @@ -1390,7 +1391,7 @@ export class AdbProvider { const ran = await runAdb( operation, { device: "adb-devices" }, - selection.binary, + selection, ["devices", "-l"], { timeoutMs: DEVICES_TIMEOUT_MS, @@ -1427,7 +1428,7 @@ export class AdbProvider { const ran = await runAdb( operation, { device: request.input.serial }, - selection.binary, + selection, adbArguments(request.input.serial, ["shell", "getprop"]), { timeoutMs: SHELL_TIMEOUT_MS, @@ -1662,7 +1663,7 @@ export class AdbProvider { const ran = await runAdbCapture( operation, { device: request.input.serial, path: request.input.device_path }, - selection.binary, + selection, adbArguments(request.input.serial, [ "shell", "sh", @@ -1972,7 +1973,7 @@ export class AdbProvider { const paths = await runAdb( operation, { device: serial }, - selection.binary, + selection, adbArguments(serial, ["shell", "pm", "path", packageName]), { timeoutMs: PM_PATH_TIMEOUT_MS, @@ -2197,7 +2198,7 @@ export class AdbProvider { path: entry.device_path, localPath, }, - selection.binary, + selection, adbArguments(serial, ["pull", entry.device_path, localPath]), { timeoutMs: PULL_TIMEOUT_MS, maxOutputBytes: PULL_MAX_OUTPUT_BYTES }, signal, @@ -2391,7 +2392,7 @@ export class AdbProvider { path: input.device_path, localPath, }, - selection.binary, + selection, adbArguments(input.serial, ["pull", input.device_path, localPath]), { timeoutMs: PULL_TIMEOUT_MS, maxOutputBytes: PULL_MAX_OUTPUT_BYTES }, signal, @@ -2496,7 +2497,7 @@ export class AdbProvider { operation, input.serial, input.device_path, - selection.binary, + selection, { timeoutMs: SETTINGS_TIMEOUT_MS, maxOutputBytes: 1024 }, signal, ); @@ -2508,7 +2509,7 @@ export class AdbProvider { operation, input.serial, devicePath, - selection.binary, + selection, { timeoutMs: SETTINGS_TIMEOUT_MS, maxOutputBytes: 1024 }, signal, ); @@ -2528,7 +2529,7 @@ export class AdbProvider { path: input.device_path, localPath: digest.path, }, - selection.binary, + selection, adbArguments(input.serial, ["push", digest.path, input.device_path]), { timeoutMs: PULL_TIMEOUT_MS, maxOutputBytes: PULL_MAX_OUTPUT_BYTES }, signal, @@ -2569,7 +2570,7 @@ export class AdbProvider { const deviceDigest = await runAdb( operation, { device: input.serial, path: devicePath }, - selection.binary, + selection, adbArguments(input.serial, ["shell", "sha256sum", devicePath]), { timeoutMs: SHELL_TIMEOUT_MS, maxOutputBytes: 1024 }, signal, @@ -2700,7 +2701,7 @@ export class AdbProvider { const installed = await runAdb( operation, { device: input.serial, path: apkPath, localPath: digest.path }, - selection.binary, + selection, adbArguments(input.serial, [ "install", ...(input.replace ? ["-r"] : []), @@ -2772,7 +2773,7 @@ export class AdbProvider { const uninstalled = await runAdb( operation, { device: input.serial }, - selection.binary, + selection, adbArguments(input.serial, [ "shell", "pm", @@ -2839,7 +2840,7 @@ export class AdbProvider { const resolved = await runAdb( operation, { device: input.serial }, - selection.binary, + selection, adbArguments(input.serial, [ "shell", "cmd", @@ -2959,7 +2960,7 @@ export class AdbProvider { const started = await runAdb( operation, { device: serial }, - selection.binary, + selection, adbArguments(serial, ["shell", "am", "start", "-W", ...arguments_]), { timeoutMs: SHELL_TIMEOUT_MS, maxOutputBytes: PM_PATH_MAX_OUTPUT_BYTES }, signal, @@ -3047,7 +3048,7 @@ export class AdbProvider { const stopped = await runAdb( operation, { device: input.serial }, - selection.binary, + selection, adbArguments(input.serial, ["shell", "am", "force-stop", input.package]), { timeoutMs: SHELL_TIMEOUT_MS, maxOutputBytes: PM_PATH_MAX_OUTPUT_BYTES }, signal, @@ -3089,7 +3090,7 @@ export class AdbProvider { const captured = await runAdbCapture( operation, { device: input.serial }, - selection.binary, + selection, adbArguments(input.serial, ["exec-out", "screencap", "-p"]), { timeoutMs: SCREEN_TIMEOUT_MS, @@ -3243,7 +3244,7 @@ export class AdbProvider { const collected = await runAdb( operation, { device: input.serial }, - selection.binary, + selection, adbArguments(input.serial, ["bugreport", stagingDirectory]), { timeoutMs: BUGREPORT_TIMEOUT_MS, diff --git a/src/adb/AdbVersionProbe.test.ts b/src/adb/AdbVersionProbe.test.ts index 0c637b9a3d..55be8b94f6 100644 --- a/src/adb/AdbVersionProbe.test.ts +++ b/src/adb/AdbVersionProbe.test.ts @@ -13,6 +13,7 @@ vi.mock("../process/ExecFileOutput.js", async (importOriginal) => ({ const selection = { binary: "/host/adb", pathSource: "environment", + environment: {}, } as const; it("reports the fixed adb version-probe deadline", async () => { diff --git a/tests/boundary/providers/adb/adbCommandFinalization.test.ts b/tests/boundary/providers/adb/adbCommandFinalization.test.ts index ce2ff943fc..126d534e64 100644 --- a/tests/boundary/providers/adb/adbCommandFinalization.test.ts +++ b/tests/boundary/providers/adb/adbCommandFinalization.test.ts @@ -3,7 +3,10 @@ import { dirname, join } from "node:path"; import { beforeEach, expect, onTestFinished, test, vi } from "vitest"; import { runAdbCapture } from "../../../../src/adb/AdbCommand.js"; -import { inspectAdbClient } from "../../../../src/adb/AdbConfiguration.js"; +import { + inspectAdbClient, + resolveAdbBinary, +} from "../../../../src/adb/AdbConfiguration.js"; import { AdbDeviceAnalysisService } from "../../../../src/application/adb/AdbDeviceAnalysisService.js"; import { execFileOutput, @@ -65,7 +68,7 @@ const capture = ( runAdbCapture( "inspect_adb_device", { device: "device" }, - process.execPath, + resolveAdbBinary({ ...process.env, REA_ADB_PATH: process.execPath }), ["-e", script], limits, signal, @@ -152,7 +155,7 @@ const clientScript = async (source: string) => { await writeFile(binary, `#!${process.execPath}\n${source}\n`, { mode: 0o755, }); - return { binary, pathSource: "environment" as const }; + return resolveAdbBinary({ ...process.env, REA_ADB_PATH: binary }); }; const nativeClientTest = test.skipIf(process.platform === "win32"); @@ -162,7 +165,7 @@ test.each([null, "caller stopped", new Error("caller stopped")] as const)( const controller = new AbortController(); controller.abort(reason); const error: unknown = await inspectAdbClient( - { binary: "/must-not-start", pathSource: "environment" }, + resolveAdbBinary({ REA_ADB_PATH: "/must-not-start" }), controller.signal, ).catch((cause: unknown) => cause); expect(error).toMatchObject({ @@ -207,7 +210,7 @@ nativeClientTest( const controller = new AbortController(); hooks.afterCommand = () => controller.abort(null); const error: unknown = await inspectAdbClient( - { binary: join(root, "missing"), pathSource: "environment" }, + resolveAdbBinary({ REA_ADB_PATH: join(root, "missing") }), controller.signal, ).catch((cause: unknown) => cause); expect(error).toMatchObject({ diff --git a/tests/boundary/providers/adb/adbSelectedLocalPaths.test.ts b/tests/boundary/providers/adb/adbSelectedLocalPaths.test.ts index 138c5bfa79..4aa51dfad9 100644 --- a/tests/boundary/providers/adb/adbSelectedLocalPaths.test.ts +++ b/tests/boundary/providers/adb/adbSelectedLocalPaths.test.ts @@ -8,7 +8,7 @@ import { writeFile, } from "node:fs/promises"; import { dirname, join } from "node:path"; -import { expect, onTestFinished, test } from "vitest"; +import { expect, onTestFinished, test, vi } from "vitest"; import { AdbProvider } from "../../../../src/adb/AdbProvider.js"; import { AdbDeviceAnalysisService } from "../../../../src/application/adb/AdbDeviceAnalysisService.js"; @@ -26,6 +26,69 @@ const png = Buffer.from( const digest = (bytes: string | Buffer) => createHash("sha256").update(bytes).digest("hex"); +it.each(["path", "explicit"] as const)( + "uses the captured %s launcher environment for both ADB probing and device queries", + async (source) => { + const root = await createTestTempDirectory("rea-adb-selected-environment-"); + const selectedDirectory = join(root, "selected"); + const ambientDirectory = join(root, "ambient"); + const sourceCode = `#!/bin/sh +printf '%s|%s|%s|%s\\n' "$1" "$ADB_SERVER_SOCKET" "$CALLER_EMPTY" "\${CALLER_ABSENT-unset}" >> "$0.calls" +if [ "$1" = version ]; then + printf 'Android Debug Bridge version 1.0.41\\nVersion 35.0.1-fixture\\nInstalled as %s\\n' "$0" +elif [ "$1" = devices ]; then + printf 'List of devices attached\\n%s device model:fixture transport_id:1\\n' "$ANDROID_SERIAL" +else exit 1; fi +`; + for (const directory of [selectedDirectory, ambientDirectory]) { + await mkdir(directory); + await writeFile(join(directory, "adb"), sourceCode, { mode: 0o755 }); + } + const selected: NodeJS.ProcessEnv = { + ...process.env, + PATH: selectedDirectory, + ADB_SERVER_SOCKET: "tcp:localhost:5041", + ANDROID_SERIAL: "selected-device", + CALLER_EMPTY: "", + ...(source === "explicit" + ? { REA_ADB_PATH: join(selectedDirectory, "adb") } + : {}), + }; + delete selected.CALLER_ABSENT; + if (source === "path") delete selected.REA_ADB_PATH; + const provider = new AdbProvider({ environment: selected }); + onTestFinished(async () => { + try { + await provider.close(); + } finally { + vi.unstubAllEnvs(); + } + }); + selected.ADB_SERVER_SOCKET = "tcp:localhost:5042"; + selected.ANDROID_SERIAL = "later-selected-device"; + vi.stubEnv("PATH", ambientDirectory); + vi.stubEnv("ADB_SERVER_SOCKET", "tcp:localhost:5043"); + vi.stubEnv("ANDROID_SERIAL", "ambient-device"); + vi.stubEnv("CALLER_EMPTY", "ambient-value"); + vi.stubEnv("CALLER_ABSENT", "ambient-only"); + const service = new AdbDeviceAnalysisService(provider); + const result = await service.execute("list_adb_devices", {}); + if (!result.ok) throw result.error; + expect(result.value.normalized_result).toMatchObject({ + devices: [{ serial: "selected-device" }], + }); + expect(JSON.stringify(result.value)).not.toContain("tcp:localhost:"); + expect(await readFile(join(selectedDirectory, "adb.calls"), "utf8")).toBe( + "version|tcp:localhost:5041||unset\ndevices|tcp:localhost:5041||unset\n", + ); + await expect( + access(join(ambientDirectory, "adb.calls")), + ).rejects.toMatchObject({ + code: "ENOENT", + }); + }, +); + // All ADB commands run a selected local script. No device, server or real adb // executable participates; file snapshots, publication and Evidence are real. const fixture = async () => {