diff --git a/src/adb/AdbConfiguration.ts b/src/adb/AdbConfiguration.ts index 3378d546f5..0c432ce6b6 100644 --- a/src/adb/AdbConfiguration.ts +++ b/src/adb/AdbConfiguration.ts @@ -163,10 +163,15 @@ export const parseAdbVersionOutput = (stdout: string): AdbClientIdentity => { const build = /^Version\s+(\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?)\s*$/mu.exec( stdout, ); - const installed = /^Installed as\s+(.+)$/mu.exec(stdout); + // The literal delimiter and line ending belong to the protocol; path + // whitespace belongs to the reported executable identity. + const installed = /^Installed as ([^\n]*)$/mu + .exec(stdout)?.[1] + ?.replace(/\r$/u, ""); return { version: build === null ? null : build[1]!.trim(), - installedPath: installed === null ? null : installed[1]!.trim(), + installedPath: + installed === undefined || installed === "" ? null : installed, }; }; diff --git a/src/adb/AdbProvider.test.ts b/src/adb/AdbProvider.test.ts index e5712e77f9..a2e712bf2e 100644 --- a/src/adb/AdbProvider.test.ts +++ b/src/adb/AdbProvider.test.ts @@ -10,13 +10,36 @@ import { } from "node:fs/promises"; import { join } from "node:path"; import { expect, onTestFinished, test, vi } from "vitest"; +import { primaryAnalysisError } from "../domain/analysisErrorBase.js"; const adbTestHooks = vi.hoisted(() => ({ afterScreencap: undefined as (() => void) | undefined, + afterBugreportCleanup: undefined as (() => void) | undefined, + bugreportCleanupFailure: undefined as Error | undefined, versionProbeFailure: undefined as Error | undefined, digestCheckFailure: undefined as Error | undefined, })); +vi.mock("node:fs/promises", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + rm: async (...arguments_: Parameters) => { + const path = arguments_[0]; + if (typeof path !== "string" || !path.includes("/.rea-bugreport-")) + return actual.rm(...arguments_); + const failure = adbTestHooks.bugreportCleanupFailure; + if (failure !== undefined) { + adbTestHooks.bugreportCleanupFailure = undefined; + adbTestHooks.afterBugreportCleanup?.(); + throw failure; + } + await actual.rm(...arguments_); + adbTestHooks.afterBugreportCleanup?.(); + }, + }; +}); + vi.mock("../process/ExecFileOutput.js", async (importOriginal) => { const actual = await importOriginal(); @@ -1532,6 +1555,68 @@ exit 1`, expect(closeAttempts).toBe(2); }); +it.each([false, true])( + "retains failed push and late cancellation when runtime cleanup fails: %s", + async (cleanupFails) => { + 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 printf 'error: device offline\\n' >&2; exit 1; fi +exit 1`, + ); + const localFile = join(root, "input.bin"); + await writeFile(localFile, "payload"); + const controller = new AbortController(); + let closeAttempts = 0; + const provider = new AdbProvider({ + environment: { REA_ADB_PATH: binary }, + createRuntimeRoot: async (options) => { + const runtime = await PrivateRuntimeRoot.create(options); + return { + path: runtime.path, + close: async () => { + closeAttempts += 1; + controller.abort("late runtime cancellation"); + if (cleanupFails && closeAttempts === 1) + throw new Error("controlled runtime cleanup refusal"); + await runtime.close(); + }, + }; + }, + }); + onTestFinished(async () => provider.close()); + const result = await provider.execute( + { + operation: "push_adb_file", + input: { + serial: "emulator-5554", + local_path: localFile, + device_path: "/data/local/tmp/input.bin", + overwrite: false, + }, + }, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("Expected the original transport refusal"); + const primary = primaryAnalysisError(result.error); + expect(projectAnalysisError(primary)).toMatchObject({ + code: "unsupported_target", + details: { reason: "error: device offline" }, + }); + expect(primary.secondaryFailures).toEqual([ + expect.objectContaining({ + _tag: "AnalysisCancelledError", + cause: "late runtime cancellation", + }), + ]); + expect(result.error.cleanupIncomplete).toBe(cleanupFails); + if (cleanupFails) { + await provider.close(); + expect(result.error.cleanupIncomplete).toBe(false); + } + }, +); + it("verifies the device-side digest for a long device path", async () => { const { binary, root } = await writeAdbStub( () => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi @@ -2333,28 +2418,141 @@ ${shellStub({ "am:start": "Starting: Intent { act=android.intent.action.VIEW }\n }, ); -it("collects a device bugreport archive with its digest", async () => { - const { binary, root } = await writeAdbStub( - () => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi +it.each([ + { cancel: false, cleanupFails: false }, + { cancel: true, cleanupFails: false }, + { cancel: false, cleanupFails: true }, + { cancel: true, cleanupFails: true }, +])( + "finalizes a bugreport with cancel=$cancel and cleanupFails=$cleanupFails", + async ({ cancel, cleanupFails }) => { + const { binary, root } = await writeAdbStub( + () => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi if [ "$1" = "-s" ] && [ "$3" = "bugreport" ]; then printf 'bugreport-bytes' > "$4/bugreport test.zip" printf 'Bug report copied to %s/bugreport test.zip\\n' "$4" exit 0 fi exit 1`, - ); - const result = await evidenceResult(providerOver(binary), { - operation: "collect_adb_bugreport", - input: { serial: "emulator-5554", output_directory: root }, - }); - const digest = createHash("sha256").update("bugreport-bytes").digest("hex"); - expect(result).toMatchObject({ - local_path: join(root, "bugreport test.zip"), - bytes: 15, - sha256: digest, - adb_reported_path: expect.stringContaining(".rea-bugreport-"), - }); -}); + ); + const controller = new AbortController(); + const provider = providerOver(binary); + adbTestHooks.afterBugreportCleanup = () => { + if (cancel) controller.abort("cancelled during final bugreport cleanup"); + }; + adbTestHooks.bugreportCleanupFailure = cleanupFails + ? Object.assign(new Error("EACCES: controlled staging cleanup denial"), { + code: "EACCES", + }) + : undefined; + onTestFinished(async () => { + adbTestHooks.afterBugreportCleanup = undefined; + adbTestHooks.bugreportCleanupFailure = undefined; + await provider.close(); + }); + const result = await provider.execute( + { + operation: "collect_adb_bugreport", + input: { serial: "emulator-5554", output_directory: root }, + }, + { signal: controller.signal }, + ); + const digest = createHash("sha256").update("bugreport-bytes").digest("hex"); + const receipt = { + local_path: join(root, "bugreport test.zip"), + bytes: 15, + sha256: digest, + adb_reported_path: expect.stringContaining(".rea-bugreport-"), + }; + expect(controller.signal.aborted).toBe(cancel); + if (cancel || cleanupFails) { + if (result.ok) + throw new Error("Expected finalization failure with retained receipt"); + const projected = projectAnalysisError(result.error); + expect(projected).toMatchObject({ + code: cleanupFails ? "cleanup_incomplete" : "cancelled", + details: { + partial_observation: { + provider_id: "adb", + operation: "collect_adb_bugreport", + result: receipt, + }, + }, + }); + expect(result.error.cleanupIncomplete).toBe(cleanupFails); + if (cleanupFails) { + expect(projected).toMatchObject({ + details: { + execution_failure: cancel ? "cancelled" : "unreadable_output", + }, + }); + expect(JSON.stringify(projectAnalysisError(result.error))).toContain( + "EACCES", + ); + expect(result.error.cleanupResources).toEqual([ + expect.stringContaining(".rea-bugreport-"), + ]); + await provider.close(); + expect(result.error.cleanupIncomplete).toBe(false); + } + } else { + if (!result.ok) throw result.error; + expect(result.value.result).toMatchObject(receipt); + } + expect(await readFile(join(root, "bugreport test.zip"), "utf8")).toBe( + "bugreport-bytes", + ); + }, +); + +it.each([false, true])( + "preserves a bugreport transport refusal and late cancellation when cleanup fails: %s", + async (cleanupFails) => { + const { binary, root } = await writeAdbStub( + () => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi +printf 'error: device offline\\n' >&2 +exit 1`, + ); + const controller = new AbortController(); + const provider = providerOver(binary); + adbTestHooks.afterBugreportCleanup = () => + controller.abort("late cancellation"); + adbTestHooks.bugreportCleanupFailure = cleanupFails + ? new Error("controlled cleanup refusal") + : undefined; + onTestFinished(async () => { + adbTestHooks.afterBugreportCleanup = undefined; + adbTestHooks.bugreportCleanupFailure = undefined; + await provider.close(); + }); + const result = await provider.execute( + { + operation: "collect_adb_bugreport", + input: { serial: "emulator-5554", output_directory: root }, + }, + { signal: controller.signal }, + ); + expect(controller.signal.aborted).toBe(true); + if (result.ok) throw new Error("Expected the original transport refusal"); + const primary = primaryAnalysisError(result.error); + expect(projectAnalysisError(primary)).toMatchObject({ + code: "unsupported_target", + details: { reason: "error: device offline" }, + }); + expect(primary.secondaryFailures).toEqual([ + expect.objectContaining({ + _tag: "AnalysisCancelledError", + cause: "late cancellation", + }), + ]); + expect(result.error.cleanupIncomplete).toBe(cleanupFails); + expect(result.error.partialObservation).toBeUndefined(); + if (cleanupFails) { + await provider.close(); + expect(result.error.cleanupIncomplete).toBe(false); + } + }, +); it("quotes caller text before adb joins the remote shell command", async () => { const { binary, root } = await writeAdbStub( diff --git a/src/adb/AdbProvider.ts b/src/adb/AdbProvider.ts index b31a1dfc88..50677b3e1a 100644 --- a/src/adb/AdbProvider.ts +++ b/src/adb/AdbProvider.ts @@ -67,6 +67,7 @@ import { } from "../domain/analysisErrorCore.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; +import { analysisErrorWithCancellation } from "../domain/analysisErrorCancellation.js"; import { readPngHeaderDimensions } from "../domain/pngHeader.js"; import { err, ok, type Result } from "../domain/result.js"; import type { JsonValue } from "../domain/jsonValue.js"; @@ -1092,6 +1093,16 @@ export class AdbProvider { resource: root.path, lease: rootLease, }); + if (signal?.aborted === true) { + const failure = + primary ?? + (result !== undefined && !result.ok ? result.error : undefined); + if (failure !== undefined) + primary = analysisErrorWithCancellation( + failure, + new AnalysisCancelledError(operation, { cause: signal.reason }), + ); + } if (primary === undefined && result?.ok && signal?.aborted === true) { const completed = result.value; primary = new AnalysisCancelledError(operation, { @@ -3328,21 +3339,35 @@ export class AdbProvider { ); } })(); + const retainCancellation = (failure: AnalysisError | undefined) => + signal?.aborted === true + ? analysisErrorWithCancellation( + failure, + new AnalysisCancelledError(operation, { + cause: signal.reason, + partialObservation: retainedObservation, + }), + ) + : failure; // A completed acquisition becomes caller evidence even if publication fails. // Its staging directory must not enter automatic cleanup or cleanup retries. if (retainedObservation !== undefined && publishedPath === undefined) - return collectedResult; + return collectedResult.ok + ? collectedResult + : err( + retainCancellation(collectedResult.error) ?? collectedResult.error, + ); try { await removeOwnedOutputDirectory(stagingDirectory, stagingIdentity); } catch (cause) { - const primary = collectedResult.ok - ? signal?.aborted === true - ? new AnalysisCancelledError(operation, { cause: signal.reason }) - : new AnalysisOutputError( - operation, - "The archive was published, but its staging directory could not be removed", - ) - : collectedResult.error; + const primary = + retainCancellation( + collectedResult.ok ? undefined : collectedResult.error, + ) ?? + new AnalysisOutputError( + operation, + "The archive was published, but its staging directory could not be removed", + ); if (retainedObservation !== undefined) primary.retainPartialObservation(retainedObservation); return err( @@ -3363,6 +3388,9 @@ export class AdbProvider { ), ); } - return collectedResult; + const failure = retainCancellation( + collectedResult.ok ? undefined : collectedResult.error, + ); + return failure === undefined ? collectedResult : err(failure); } } diff --git a/src/adb/AdbVersionProbe.test.ts b/src/adb/AdbVersionProbe.test.ts index c8f9285267..d45b3bafca 100644 --- a/src/adb/AdbVersionProbe.test.ts +++ b/src/adb/AdbVersionProbe.test.ts @@ -1,4 +1,7 @@ -import { expect, it, vi } from "vitest"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { expect, it, onTestFinished, vi } from "vitest"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { AnalysisTimeoutError } from "../domain/analysisErrorCore.js"; @@ -9,10 +12,11 @@ import { inspectAdbClient, } from "./AdbConfiguration.js"; -vi.mock("../process/ExecFileOutput.js", async (importOriginal) => ({ - ...(await importOriginal()), - execFileOutput: vi.fn(), -})); +vi.mock("../process/ExecFileOutput.js", async (importOriginal) => { + const actual = + await importOriginal(); + return { ...actual, execFileOutput: vi.fn(actual.execFileOutput) }; +}); const selection = { binary: "/host/adb", @@ -169,3 +173,53 @@ it("does not blame the selected path for exhausted host spawn capacity", async ( remediation: failure.remediation, }); }); + +it.skipIf(process.platform === "win32").each([ + { + name: "an actual executable path with trailing space", + fileName: "adb ", + installedLine: (binary: string) => `Installed as ${binary}\n`, + expectedPath: (binary: string) => binary, + }, + { + name: "a CRLF installed-path record", + fileName: "adb", + installedLine: (binary: string) => `Installed as ${binary}\r\n`, + expectedPath: (binary: string) => binary, + }, + { + name: "an omitted installed-path record", + fileName: "adb", + installedLine: () => "", + expectedPath: () => null, + }, + { + name: "an empty installed-path record before another field", + fileName: "adb", + installedLine: () => "Installed as \n", + expectedPath: () => null, + }, +])("preserves the source identity for $name", async (testCase) => { + const root = await mkdtemp(join(tmpdir(), "rea-adb-version-")); + onTestFinished(() => rm(root, { recursive: true, force: true })); + const binary = join(root, testCase.fileName); + const output = + "Android Debug Bridge version 1.0.41\nVersion 35.0.2-stub\n" + + testCase.installedLine(binary) + + "Running on Linux\n"; + await writeFile( + binary, + `#!${process.execPath}\nif (process.argv[2] !== "version") throw Error("No device command allowed");\nprocess.stdout.write(${JSON.stringify(output)});\n`, + { mode: 0o755 }, + ); + expect( + await inspectAdbClient({ + binary, + pathSource: "environment", + environment: {}, + }), + ).toEqual({ + version: "35.0.2-stub", + installedPath: testCase.expectedPath(binary), + }); +});