From 554ea7611ea7eb94d73c80b57be168527de17d79 Mon Sep 17 00:00:00 2001 From: Jin Date: Tue, 13 Oct 2026 11:49:20 -0400 Subject: [PATCH] fix(adb): keep the device's refusal when a package change exits nonzero (#2344) Squash merge of https://github.com/morluto/rea/pull/2344. Co-authored-by: Hui Co-authored-by: Claude Opus 5.5 --- src/adb/AdbProvider.test.ts | 62 +++++++++++++++++++++++++++++++++++++ src/adb/AdbProvider.ts | 38 +++++++++++++++++++++-- src/adb/AdbShellOutput.ts | 9 ++++-- 3 files changed, 105 insertions(+), 4 deletions(-) diff --git a/src/adb/AdbProvider.test.ts b/src/adb/AdbProvider.test.ts index 23645e456..3d15325fd 100644 --- a/src/adb/AdbProvider.test.ts +++ b/src/adb/AdbProvider.test.ts @@ -1127,6 +1127,68 @@ exit 1`, expect(projected.message).toContain("not installed for 0"); }); +// adb exits 1 on a refused package change: `adb install` after printing the +// status line to stderr, `cmd package uninstall` after printing it to stdout. +it("preserves the device's install refusal when adb exits nonzero", async () => { + const { binary, root } = await writeAdbStub( + () => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi +if [ "$1" = "-s" ] && [ "$3" = "install" ]; then + printf 'Performing Streamed Install\\n' + printf 'adb: failed to install app.apk: Failure [INSTALL_FAILED_ALREADY_EXISTS: Attempt to re-install com.example without first uninstalling.]\\n' >&2 + exit 1 +fi +exit 1`, + ); + const apk = join(root, "app.apk"); + await writeFile(apk, "apk"); + const projected = await refusedBy(providerOver(binary), { + operation: "install_adb_package", + input: { serial: "emulator-5554", apk_path: apk, replace: false }, + }); + expect(projected).toMatchObject({ + code: "unsupported_target", + details: { + reason: + "INSTALL_FAILED_ALREADY_EXISTS: Attempt to re-install com.example without first uninstalling.", + }, + }); + expect(projected.message).toContain( + "INSTALL_FAILED_ALREADY_EXISTS: Attempt to re-install com.example without first uninstalling.", + ); +}); + +it("preserves the device's uninstall refusal when adb exits nonzero", async () => { + const { binary } = await writeAdbStub( + () => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi +if [ "$1" = "-s" ] && [ "$3" = "uninstall" ]; then printf 'Failure [not installed for 0]\\n'; exit 1; fi +exit 1`, + ); + const projected = await refusedBy(providerOver(binary), { + operation: "uninstall_adb_package", + input: { serial: "emulator-5554", package: "com.absent" }, + }); + expect(projected).toMatchObject({ + code: "unsupported_target", + details: { reason: "not installed for 0" }, + }); + expect(projected.message).toContain("not installed for 0"); +}); + +it("keeps a nonzero install without a device verdict as an unreadable result", async () => { + const { binary, root } = await writeAdbStub( + () => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi +if [ "$1" = "-s" ] && [ "$3" = "install" ]; then printf 'adb: failed to install app.apk: closed\\n' >&2; exit 1; fi +exit 1`, + ); + const apk = join(root, "app.apk"); + await writeFile(apk, "apk"); + const projected = await refusedBy(providerOver(binary), { + operation: "install_adb_package", + input: { serial: "emulator-5554", apk_path: apk, replace: false }, + }); + expect(projected).toMatchObject({ code: "unreadable_output" }); +}); + it("starts an app through resolved launcher component and force-stops it", async () => { const { binary } = await writeAdbStub( shellStub({ diff --git a/src/adb/AdbProvider.ts b/src/adb/AdbProvider.ts index d26345eba..bce989aca 100644 --- a/src/adb/AdbProvider.ts +++ b/src/adb/AdbProvider.ts @@ -71,6 +71,7 @@ import { parseBugreportCopiedPath, parseDumpsysPackageOutput, parseFeatureListOutput, + packageChangeFailureReason, parsePackageChangeOutput, parseProcessListOutput, parsePngDimensions, @@ -248,6 +249,31 @@ const runAdb = async ( }); }; +/** + * adb exits nonzero when the device refuses a package change, printing the + * `Failure [...]` verdict to stderr (install) or stdout (uninstall). Read the + * refusal from that failed run so it is not reported as unreadable output. + */ +const packageChangeRefusal = ( + operation: string, + serial: string, + error: AnalysisError, +): Result | undefined => { + if (!(error instanceof AnalysisProtocolError)) return undefined; + const failure = execFileOutputFailure(error.cause); + if (failure === undefined) return undefined; + const reason = packageChangeFailureReason( + `${failure.stdout}\n${failure.stderr}`, + ); + return reason === null + ? undefined + : err( + new AnalysisUnsupportedTargetError(operation, serial, reason, { + cause: error, + }), + ); +}; + const clientIdentityResult = ( selection: AdbBinarySelection, identity: AdbClientIdentity, @@ -1754,7 +1780,11 @@ export class AdbProvider { { timeoutMs: PULL_TIMEOUT_MS, maxOutputBytes: PULL_MAX_OUTPUT_BYTES }, signal, ); - if (!installed.ok) return installed; + if (!installed.ok) + return ( + packageChangeRefusal(operation, input.serial, installed.error) ?? + installed + ); const verdict = parsePackageChangeOutput(installed.value.stdout); if (verdict.status === "failure") return err( @@ -1803,7 +1833,11 @@ export class AdbProvider { }, signal, ); - if (!uninstalled.ok) return uninstalled; + if (!uninstalled.ok) + return ( + packageChangeRefusal(operation, input.serial, uninstalled.error) ?? + uninstalled + ); const verdict = parsePackageChangeOutput(uninstalled.value.stdout); if (verdict.status === "failure") return err( diff --git a/src/adb/AdbShellOutput.ts b/src/adb/AdbShellOutput.ts index 726e6164d..6dac4cc0c 100644 --- a/src/adb/AdbShellOutput.ts +++ b/src/adb/AdbShellOutput.ts @@ -293,6 +293,10 @@ export const parseBugreportCopiedPath = (stdout: string): string | null => { return match === null ? null : match[1]!; }; +/** Read the device's `Failure [REASON]` verdict from package-change output. */ +export const packageChangeFailureReason = (output: string): string | null => + /Failure \[([^\]]+)\]/u.exec(output)?.[1] ?? null; + /** * Parse `adb install`/`adb uninstall` output. adb can print `Failure [...]` * with a zero exit code, so the text decides, not the exit status. @@ -305,10 +309,11 @@ export const parsePackageChangeOutput = ( } => { const text = stdout.trim(); if (/^Success$/mu.test(text)) return { status: "success", reason: null }; - const failure = /Failure \[([^\]]+)\]/u.exec(text); return { status: "failure", - reason: failure?.[1] ?? (text === "" ? null : text.slice(0, 512)), + reason: + packageChangeFailureReason(text) ?? + (text === "" ? null : text.slice(0, 512)), }; };