From 33e0bf101ab2afddc35da6dba985ab308dc504c9 Mon Sep 17 00:00:00 2001 From: ShuaixinHuang <1836656514@qq.com> Date: Wed, 4 Nov 2026 19:55:13 +0800 Subject: [PATCH] fix(adb): admit completed path probes after daemon startup (#8858) * fix(adb): admit completed path probes after daemon startup * test(adb): preserve startup probe and digest evidence --- docs/adb-device-analysis.md | 8 +- src/adb/AdbCommand.test.ts | 110 +++++++++++++++++++++- src/adb/AdbCommand.ts | 63 +++++++++---- src/adb/AdbDeviceOutput.ts | 22 +++++ src/adb/AdbProvider.ts | 19 +++- src/adb/AdbPushProbeEvidence.test.ts | 131 +++++++++++++++++++++++++++ 6 files changed, 330 insertions(+), 23 deletions(-) create mode 100644 src/adb/AdbPushProbeEvidence.test.ts diff --git a/docs/adb-device-analysis.md b/docs/adb-device-analysis.md index 1dfcb826da..efb8bf2b34 100644 --- a/docs/adb-device-analysis.md +++ b/docs/adb-device-analysis.md @@ -203,8 +203,12 @@ and during shutdown. `uninstall_adb_package` declare `mutatesTarget` and never run implicitly: pushes check for existing device files before transfer and refuse them unless `overwrite` is set. A failed or unexpected existence probe stops - transfer even with `overwrite`; only a clean `test -e` exit of 1 with no - output is interpreted as absence. The check is not atomic, so a different + transfer even with `overwrite`. A completed `test -e` exit of 1 establishes + absence only with empty stdout and either empty stderr or a recognized, + completed adb-server startup notice sequence. Startup notices are host + observations, not remote-path diagnostics; successful push receipts retain + 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 diff --git a/src/adb/AdbCommand.test.ts b/src/adb/AdbCommand.test.ts index 5fff97243e..89cac7286a 100644 --- a/src/adb/AdbCommand.test.ts +++ b/src/adb/AdbCommand.test.ts @@ -65,7 +65,21 @@ it("recognizes only the completed empty false result as absence", async () => { killed: false, }), ); - await expect(probe()).resolves.toEqual({ ok: true, value: false }); + await expect(probe()).resolves.toEqual({ + ok: true, + value: { + matches: false, + capturedOutput: { + exit_code: 1, + stdout: "", + stderr: "", + stdout_bytes: 0, + stderr_bytes: 0, + signal: null, + truncated: false, + }, + }, + }); }); it.each([ @@ -124,7 +138,21 @@ it("recognizes an empty successful probe as existence", async () => { stdout: Buffer.alloc(0), stderr: Buffer.alloc(0), }); - await expect(probe()).resolves.toEqual({ ok: true, value: true }); + await expect(probe()).resolves.toEqual({ + ok: true, + value: { + matches: true, + capturedOutput: { + exit_code: 0, + stdout: "", + stderr: "", + stdout_bytes: 0, + stderr_bytes: 0, + signal: null, + truncated: false, + }, + }, + }); }); it.each(["stdout", "stderr"] as const)( @@ -265,3 +293,81 @@ it("keeps a physical failure even when the later abort reason is that same error error: { _tag: "AnalysisProtocolError", cause: failure }, }); }); + +const STARTUP_NOTICES = [ + "* daemon not running; starting now at tcp:5037\n* daemon started successfully\n", + "* daemon not running. starting it now on port 5037 *\r\n* daemon started successfully *\r\n", +]; +it.each(STARTUP_NOTICES)( + "recognizes completed true and false probes with successful startup: %j", + async (stderr) => { + vi.mocked(execFileBytes).mockResolvedValueOnce({ + stdout: Buffer.alloc(0), + stderr: Buffer.from(stderr), + }); + await expect(probe()).resolves.toMatchObject({ + ok: true, + value: { + matches: true, + capturedOutput: { + stdout: "", + stderr, + exit_code: 0, + stderr_bytes: Buffer.byteLength(stderr), + }, + }, + }); + vi.mocked(execFileBytes).mockRejectedValueOnce( + Object.assign(new Error("false"), { + code: 1, + stdout: "", + stderr, + signal: null, + killed: false, + }), + ); + await expect(probe()).resolves.toMatchObject({ + ok: true, + value: { + matches: false, + capturedOutput: { + stdout: "", + stderr, + exit_code: 1, + stderr_bytes: Buffer.byteLength(stderr), + }, + }, + }); + }, +); + +it.each([ + "* daemon not running; starting now at tcp:5037\n", + "* daemon failed to start\n", + "* daemon started successfully\n* daemon not running; starting now at tcp:5037\n", + "* daemon started successfully\nunexpected\n", + "\n", + " ", +])("refuses an incomplete or unknown startup output: %j", async (stderr) => { + vi.mocked(execFileBytes).mockResolvedValueOnce({ + stdout: Buffer.alloc(0), + stderr: Buffer.from(stderr), + }); + await expect(probe()).resolves.toMatchObject({ + ok: false, + error: { capturedOutput: { stderr, exit_code: 0 } }, + }); + vi.mocked(execFileBytes).mockRejectedValueOnce( + Object.assign(new Error("false"), { + code: 1, + stdout: "", + stderr, + signal: null, + killed: false, + }), + ); + await expect(probe()).resolves.toMatchObject({ + ok: false, + error: { capturedOutput: { stderr, exit_code: 1 } }, + }); +}); diff --git a/src/adb/AdbCommand.ts b/src/adb/AdbCommand.ts index be544ec3e5..5c627493d6 100644 --- a/src/adb/AdbCommand.ts +++ b/src/adb/AdbCommand.ts @@ -16,7 +16,11 @@ import { type ExecFileOutputFailure, } from "../process/ExecFileOutput.js"; import { packageChangeFailureReason } from "./AdbShellOutput.js"; -import { isSuccessfulDaemonStartNotice } from "./AdbDeviceOutput.js"; +import { + isSuccessfulDaemonStartNotice, + isDaemonStartingNotice, + hasOnlySuccessfulAdbStartupNotices, +} from "./AdbDeviceOutput.js"; import type { AdbBinarySelection } from "./AdbConfiguration.js"; const STDERR_EXCERPT_CHARACTERS = 512; @@ -164,12 +168,7 @@ export const runAdbCapture = async ( const line = stderr .slice(diagnosticStart, newline === -1 ? stderr.length : newline) .trimEnd(); - if ( - !isSuccessfulDaemonStartNotice(line) && - !/^\* daemon not running(?:; starting now at [^\r\n]*|\. starting it now on port \d+ \*)$/u.test( - line, - ) - ) + if (!isSuccessfulDaemonStartNotice(line) && !isDaemonStartingNotice(line)) break; diagnosticStart = newline === -1 ? stderr.length : newline + 1; } @@ -272,7 +271,38 @@ export const packageChangeRefusal = ( ); }; -/** Only the clean false result of test -e establishes absence. */ +/** Parsed predicate and the complete output that supports it. */ +export interface AdbPathProbe { + readonly matches: boolean; + readonly capturedOutput: { + readonly stdout: string; + readonly stderr: string; + readonly stdout_bytes: number; + readonly stderr_bytes: number; + readonly exit_code: 0 | 1; + readonly signal: null; + readonly truncated: false; + }; +} +const completedAdbPathProbe = ( + matches: boolean, + stderr: string, +): AdbPathProbe => ({ + matches, + capturedOutput: { + stdout: "", + stderr, + stdout_bytes: 0, + stderr_bytes: Buffer.byteLength(stderr), + exit_code: matches ? 0 : 1, + signal: null, + truncated: false, + }, +}); +const recognizedProbeStderr = (stderr: string): boolean => + stderr === "" || hasOnlySuccessfulAdbStartupNotices(stderr); + +/** Only a complete false predicate, with no device diagnostics, establishes absence. */ export const probeAdbPathExistence = async ( operation: string, serial: string, @@ -280,11 +310,11 @@ export const probeAdbPathExistence = async ( selection: AdbBinarySelection, limits: AdbRunLimits, signal: AbortSignal | undefined, -): Promise> => { +): Promise> => { return probeAdbPath(operation, serial, path, selection, limits, signal, "-e"); }; -/** Only the clean true/false result of test -d establishes directory status. */ +/** Completed predicate status is independent of recognized host startup notices. */ export const probeAdbPathDirectory = async ( operation: string, serial: string, @@ -292,7 +322,7 @@ export const probeAdbPathDirectory = async ( selection: AdbBinarySelection, limits: AdbRunLimits, signal: AbortSignal | undefined, -): Promise> => +): Promise> => probeAdbPath(operation, serial, path, selection, limits, signal, "-d"); const probeAdbPath = async ( @@ -303,7 +333,7 @@ const probeAdbPath = async ( limits: AdbRunLimits, signal: AbortSignal | undefined, predicate: "-d" | "-e", -): Promise> => { +): Promise> => { const fact = predicate === "-d" ? "destination_directory" : "destination_existence"; const probed = await runAdbCapture( @@ -315,8 +345,9 @@ const probeAdbPath = async ( signal, ); if (probed.ok) { - if (probed.value.stdout.length === 0 && probed.value.stderr.length === 0) - return ok(true); + const stderr = probed.value.stderr.toString("utf8"); + if (probed.value.stdout.length === 0 && recognizedProbeStderr(stderr)) + return ok(completedAdbPathProbe(true, stderr)); return err( new AnalysisProtocolError( `The ADB path ${predicate === "-d" ? "directory" : "existence"} probe returned unexpected output; destination status is unknown`, @@ -345,9 +376,9 @@ const probeAdbPath = async ( !failure.outputTruncated && failure.collectionFailure !== true && failure.stdout === "" && - failure.stderr === "" + recognizedProbeStderr(failure.stderr) ) - return ok(false); + return ok(completedAdbPathProbe(false, failure.stderr)); return err( new AnalysisProtocolError(error.message, { cause: error.cause, diff --git a/src/adb/AdbDeviceOutput.ts b/src/adb/AdbDeviceOutput.ts index dc24522b75..4892e2860f 100644 --- a/src/adb/AdbDeviceOutput.ts +++ b/src/adb/AdbDeviceOutput.ts @@ -40,6 +40,28 @@ export const isSuccessfulDaemonStartNotice = (line: string): boolean => line === "* daemon started successfully" || line === "* daemon started successfully *"; +/** Host-side startup line, shared by failure and successful probe parsing. */ +export const isDaemonStartingNotice = (line: string): boolean => + /^\* daemon not running(?:; starting now at [^\r\n]*|\. starting it now on port \d+ \*)$/u.test( + line, + ); + +/** No diagnostics beyond a completed, recognized startup sequence. */ +export const hasOnlySuccessfulAdbStartupNotices = (output: string): boolean => { + let started = false; + for (let offset = 0; offset < output.length;) { + const newline = output.indexOf("\n", offset); + const line = normalizeAdbOutputLine( + output.slice(offset, newline === -1 ? output.length : newline), + ); + if (isSuccessfulDaemonStartNotice(line)) started = true; + else if (isDaemonStartingNotice(line)) started = false; + else return false; + offset = newline === -1 ? output.length : newline + 1; + } + return started; +}; + const metadataValue = (remainder: string, key: string): string | null => { const match = new RegExp(`(?:^|\\s)${key}:(\\S+)`, "u").exec(remainder); return match === null ? null : match[1]!; diff --git a/src/adb/AdbProvider.ts b/src/adb/AdbProvider.ts index 36724a187a..ac8f80c610 100644 --- a/src/adb/AdbProvider.ts +++ b/src/adb/AdbProvider.ts @@ -2407,7 +2407,7 @@ export class AdbProvider { signal, ); if (!directoryProbe.ok) return directoryProbe; - const devicePath = directoryProbe.value + const devicePath = directoryProbe.value.matches ? `${input.device_path}${input.device_path.endsWith("/") ? "" : "/"}${basename(localPath)}` : input.device_path; const existsProbe = await probeAdbPathExistence( @@ -2419,7 +2419,7 @@ export class AdbProvider { signal, ); if (!existsProbe.ok) return existsProbe; - const deviceExists = existsProbe.value; + const deviceExists = existsProbe.value.matches; if (deviceExists && !input.overwrite) return err( new AnalysisOutputError( @@ -2440,6 +2440,17 @@ export class AdbProvider { signal, ); if (!pushed.ok) return pushed; + const observedProbeStartup = + directoryProbe.value.capturedOutput.stderr !== "" || + existsProbe.value.capturedOutput.stderr !== ""; + const probeOutput = observedProbeStartup + ? { + device_path_probes: { + directory: { ...directoryProbe.value.capturedOutput }, + existence: { ...existsProbe.value.capturedOutput }, + }, + } + : undefined; const receipt = ( deviceDigest: AdbDeviceDigest, rawResult?: { @@ -2463,7 +2474,9 @@ export class AdbProvider { }, provider, { - ...(rawResult === undefined ? {} : { rawResult }), + ...(rawResult === undefined && probeOutput === undefined + ? {} + : { rawResult: { ...rawResult, ...probeOutput } }), limitations: [ "Push writes a private snapshot of the caller's file to the device", ...(deviceDigest.device_digest_source === "unavailable" diff --git a/src/adb/AdbPushProbeEvidence.test.ts b/src/adb/AdbPushProbeEvidence.test.ts new file mode 100644 index 0000000000..099a7edf30 --- /dev/null +++ b/src/adb/AdbPushProbeEvidence.test.ts @@ -0,0 +1,131 @@ +import { createHash } from "node:crypto"; +import { mkdtemp, readFile, rm, stat, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { expect, it, onTestFinished, vi } from "vitest"; +import { execFileBytes, execFileOutput } from "../process/ExecFileOutput.js"; +import { AdbProvider } from "./AdbProvider.js"; +vi.mock("../process/ExecFileOutput.js", async (importOriginal) => ({ + ...(await importOriginal()), + execFileBytes: vi.fn(), + execFileOutput: vi.fn(), +})); +const startup = + "* daemon not running; starting now at tcp:5037\n* daemon started successfully\n"; +it.each([false, true])( + "retains startup probe evidence alongside digest observations (recognized=%s)", + async (recognized) => { + vi.clearAllMocks(); + const root = await mkdtemp(join(tmpdir(), "rea-adb-probe-evidence-")); + onTestFinished(() => rm(root, { recursive: true, force: true })); + const source = join(root, "input.bin"); + const bytes = Buffer.from("caller-owned bytes"); + await writeFile(source, bytes); + const digest = createHash("sha256").update(bytes).digest("hex"); + const provider = new AdbProvider({ + environment: { REA_ADB_PATH: process.execPath }, + }); + onTestFinished(() => provider.close()); + vi.mocked(execFileOutput).mockImplementation(async (_binary, args) => { + expect(args).toEqual(["version"]); + const stdout = + "Android Debug Bridge version 1.0.41\nVersion 37.0.0-fixture\n"; + return { + stdout, + stderr: "", + stdoutBytes: Buffer.byteLength(stdout), + stderrBytes: 0, + }; + }); + const digestOutput = recognized + ? `${digest} /sdcard/receiver/input.bin\n` + : "unrecognized sha256sum fixture output\n"; + const commands: string[][] = []; + let staged: string | undefined; + vi.mocked(execFileBytes).mockImplementation(async (_binary, args) => { + const command = [...args]; + commands.push(command); + expect(command.slice(0, 2)).toEqual(["-s", "device"]); + if ( + command[2] === "shell" && + command[3] === "test" && + command[4] === "-d" + ) + return { stdout: Buffer.alloc(0), stderr: Buffer.from(startup) }; + if ( + command[2] === "shell" && + command[3] === "test" && + command[4] === "-e" + ) + throw Object.assign(new Error("absent"), { + code: 1, + signal: null, + killed: false, + stdout: "", + stderr: startup, + }); + if (command[2] === "push") { + staged = command[3]; + expect(staged).not.toBe(source); + expect((await readFile(staged!)).equals(bytes)).toBe(true); + return { + stdout: Buffer.from("1 file pushed\n"), + stderr: Buffer.alloc(0), + }; + } + if (command[2] === "shell" && command[3] === "sha256sum") + return { stdout: Buffer.from(digestOutput), stderr: Buffer.alloc(0) }; + throw new Error("Unexpected adb fixture command"); + }); + const result = await provider.execute({ + operation: "push_adb_file", + input: { + serial: "device", + local_path: source, + device_path: "/sdcard/receiver", + overwrite: false, + }, + }); + if (!result.ok) throw result.error; + expect(result.value.result).toMatchObject({ + sha256: digest, + device_sha256: recognized ? digest : null, + overwritten: false, + device_digest_source: recognized ? "device_sha256sum" : "unavailable", + }); + expect(result.value.rawResult).toEqual({ + device_path_probes: { + directory: { + stdout: "", + stderr: startup, + stdout_bytes: 0, + stderr_bytes: Buffer.byteLength(startup), + exit_code: 0, + signal: null, + truncated: false, + }, + existence: { + stdout: "", + stderr: startup, + stdout_bytes: 0, + stderr_bytes: Buffer.byteLength(startup), + exit_code: 1, + signal: null, + truncated: false, + }, + }, + ...(recognized + ? {} + : { device_sha256sum: { stdout: digestOutput, stderr: "" } }), + }); + expect(commands.map((command) => command.slice(2, 5))).toEqual([ + ["shell", "test", "-d"], + ["shell", "test", "-e"], + ["push", staged, "/sdcard/receiver"], + ["shell", "sha256sum", "/sdcard/receiver/input.bin"], + ]); + expect((await readFile(source)).equals(bytes)).toBe(true); + expect(staged).toBeDefined(); + await expect(stat(staged!)).rejects.toMatchObject({ code: "ENOENT" }); + }, +);