mirror of
https://github.com/morluto/rea.git
synced 2026-11-11 17:28:08 +00:00
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
This commit is contained in:
1 parent
e416ce1983
commit
33e0bf101a
6 files changed
+330
-23
No files matched your search
@@ -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
|
||||
|
||||
+108
-2
@@ -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 } },
|
||||
});
|
||||
});
|
||||
+47
-16
@@ -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<Result<boolean, AnalysisError>> => {
|
||||
): Promise<Result<AdbPathProbe, AnalysisError>> => {
|
||||
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<Result<boolean, AnalysisError>> =>
|
||||
): Promise<Result<AdbPathProbe, AnalysisError>> =>
|
||||
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<Result<boolean, AnalysisError>> => {
|
||||
): Promise<Result<AdbPathProbe, AnalysisError>> => {
|
||||
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,
|
||||
|
||||
@@ -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]!;
|
||||
|
||||
+16
-3
@@ -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"
|
||||
|
||||
@@ -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<typeof import("../process/ExecFileOutput.js")>()),
|
||||
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" });
|
||||
},
|
||||
);
|
||||
Reference in new issue
Block a user