diff --git a/src/adb/AdbCommand.test.ts b/src/adb/AdbCommand.test.ts index a4fbe0c70..52882c811 100644 --- a/src/adb/AdbCommand.test.ts +++ b/src/adb/AdbCommand.test.ts @@ -23,6 +23,37 @@ const probe = (signal?: AbortSignal) => signal, ); +it("classifies the complete diagnostic before shortening its displayed reason", async () => { + const localPath = `/host/${"directory/".repeat(60)}payload.bin`; + const stderr = `adb: error: cannot create '${localPath}': Permission denied`; + vi.mocked(execFileBytes).mockRejectedValueOnce( + Object.assign(new Error("local destination refused"), { + code: 1, + stdout: "", + stderr, + signal: null, + killed: false, + }), + ); + await expect( + runAdbCapture( + "pull_adb_file", + { device: "device", path: "/remote/file", localPath }, + "/host/adb", + [], + { timeoutMs: 100, maxOutputBytes: 2048 }, + undefined, + ), + ).resolves.toMatchObject({ + ok: false, + error: { + _tag: "AnalysisUnsupportedTargetError", + path: localPath, + reason: stderr.slice(0, 512), + }, + }); +}); + it("recognizes only the completed empty false result as absence", async () => { vi.mocked(execFileBytes).mockRejectedValueOnce( Object.assign(new Error("test false"), { @@ -161,7 +192,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: "device" }, "/host/adb", [], { timeoutMs: 100, maxOutputBytes: 1024 }, diff --git a/src/adb/AdbCommand.ts b/src/adb/AdbCommand.ts index 6c4b255aa..c494fd71f 100644 --- a/src/adb/AdbCommand.ts +++ b/src/adb/AdbCommand.ts @@ -17,8 +17,26 @@ import { type ExecFileOutputFailure, } from "../process/ExecFileOutput.js"; import { packageChangeFailureReason } from "./AdbShellOutput.js"; +import { isSuccessfulDaemonStartNotice } from "./AdbDeviceOutput.js"; -const STDERR_EXCERPT_BYTES = 512; +const STDERR_EXCERPT_CHARACTERS = 512; + +// Match the known host operand in adb's local-file diagnostics. The suffix is +// part of the format: opening/reading identify local failures explicitly. +const LOCAL_FILE_DIAGNOSTICS = [ + ["cannot create", ":"], + ["cannot create file/directory", ":"], + ["cannot stat", ":"], + ["cannot open", ":"], + ["cannot write", ":"], + ["failed to stat local file", ":"], + ["failed to read all of", ":"], + ["failed to access", ":"], + ["failed to stat", ":"], + ["failed to open", ":"], + ["opening", " locally failed:"], + ["reading", " locally failed:"], +] as const; /** Project parsed command-owner facts without reinterpreting the rejection. */ export const adbCommandCapturedOutput = ( @@ -70,10 +88,19 @@ export interface AdbRunLimits { readonly maxOutputBytes: number; } +/** Keep the selected device separate from the file addressed by the command. */ +export interface AdbCommandTarget { + readonly device: string; + /** Caller-addressed file, such as a remote path or the original APK input. */ + readonly path?: string; + /** Exact host operand adb consumes, including an owned input snapshot. */ + readonly localPath?: string; +} + /** Run one bounded adb invocation and keep its exact failure shape. */ export const runAdbCapture = async ( operation: string, - targetPath: string, + target: AdbCommandTarget, binary: string, arguments_: readonly string[], limits: AdbRunLimits, @@ -115,33 +142,61 @@ export const runAdbCapture = async ( { cause }, ), ); - const excerpt = failure.stderr.trim().slice(0, STDERR_EXCERPT_BYTES); - if (/unauthorized/iu.test(excerpt)) - return err( - new AnalysisUnsupportedTargetError(operation, targetPath, excerpt, { - cause, - }), + const stderr = failure.stderr.trim(); + const excerpt = stderr.slice(0, STDERR_EXCERPT_CHARACTERS); + // Device-state words inside a reported file name are not device verdicts. + // Only leading notices need line parsing. Do not expand a capture-sized + // stderr into an array whose entries can exceed the byte capture budget. + let diagnosticStart = 0; + while (diagnosticStart < stderr.length) { + const newline = stderr.indexOf("\n", diagnosticStart); + 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, + ) + ) + break; + diagnosticStart = newline === -1 ? stderr.length : newline + 1; + } + const diagnostic = stderr + .slice(diagnosticStart) + .replace( + /^(?:adb:\s*)?(?:error:\s*)?(?:(?:failed to get feature set|connect failed):\s*)?/iu, + "", ); if ( - /device[^\n]*not found|device offline|still authorizing|more than one device/iu.test( - excerpt, + /^(?:unauthorized\b|device(?: '[^'\n]*')? (?:unauthorized\b|offline\b|not found\b|still authorizing\b)|still authorizing\b|more than one device\b)/iu.test( + diagnostic, ) ) return err( - new AnalysisUnsupportedTargetError(operation, targetPath, excerpt, { + new AnalysisUnsupportedTargetError(operation, target.device, excerpt, { cause, }), ); if ( /No such file or directory|Permission denied|read: Permission denied/iu.test( - excerpt, + diagnostic, ) - ) + ) { + const localPath = target.localPath; + const path = + localPath !== undefined && + LOCAL_FILE_DIAGNOSTICS.some(([verb, suffix]) => + [localPath, `'${localPath}'`, `"${localPath}"`].some((operand) => + diagnostic.startsWith(`${verb} ${operand}${suffix}`), + ), + ) + ? localPath + : (target.path ?? target.device); return err( - new AnalysisUnsupportedTargetError(operation, targetPath, excerpt, { - cause, - }), + new AnalysisUnsupportedTargetError(operation, path, excerpt, { cause }), ); + } if (failure.killed) return err( new AnalysisTimeoutError(operation, limits.timeoutMs, { cause }), @@ -210,7 +265,7 @@ export const probeAdbPathExistence = async ( ): Promise> => { const probed = await runAdbCapture( operation, - path, + { device: serial, path }, binary, adbArguments(serial, ["shell", "test", "-e", path]), limits, diff --git a/src/adb/AdbDeviceOutput.ts b/src/adb/AdbDeviceOutput.ts index fd6a28e76..2691b57d1 100644 --- a/src/adb/AdbDeviceOutput.ts +++ b/src/adb/AdbDeviceOutput.ts @@ -32,7 +32,7 @@ const DEVICE_LINE = /^(\S+)\s+(.*)$/u; const normalizeAdbOutputLine = (rawLine: string): string => rawLine.trimEnd(); -const isSuccessfulDaemonStartNotice = (line: string): boolean => +export const isSuccessfulDaemonStartNotice = (line: string): boolean => line === "* daemon started successfully" || line === "* daemon started successfully *"; diff --git a/src/adb/AdbProvider.test.ts b/src/adb/AdbProvider.test.ts index c58bc2830..8509bf3a8 100644 --- a/src/adb/AdbProvider.test.ts +++ b/src/adb/AdbProvider.test.ts @@ -568,7 +568,7 @@ it.each([ async (_, stderr, excerpt) => { const { binary } = await writeAdbStub( () => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi -if [ "$1" = "-s" ]; then printf '%s\\n' "${stderr.replace(/'/gu, "'\\''")}" >&2; exit 1; fi`, +if [ "$1" = "-s" ]; then printf '%s\\n' "${stderr}" >&2; exit 1; fi`, ); const projected = await refusedBy(providerOver(binary), { operation: "inspect_adb_device", @@ -873,6 +873,137 @@ exit 1`, }); }); +it.each([ + [ + "pull_adb_file", + "adb: error: failed to stat remote object /sdcard/unauthorized.bin: No such file or directory", + "/sdcard/unauthorized.bin", + ], + [ + "pull_adb_file", + "adb: error: failed to open /sdcard/unauthorized.bin: Permission denied", + "/sdcard/unauthorized.bin", + ], + [ + "pull_adb_file", + "* daemon not running; starting now at tcp:5037\n* daemon started successfully\nadb: error: device offline", + "emulator-5554", + ], + ["pull_adb_file", "error: device unauthorized", "emulator-5554"], + [ + "pull_adb_file", + "* daemon not running. starting it now on port 5037 *\r\n* daemon started successfully *\r\nerror: device offline", + "emulator-5554", + ], + [ + "pull_adb_file", + "* daemon started successfully *\r\nerror: device 'emulator-5554' not found", + "emulator-5554", + ], + [ + "push_adb_file", + "adb: error: connect failed: device offline", + "emulator-5554", + ], + ["push_adb_file", "error: device unauthorized", "emulator-5554"], + ["push_adb_file", "Permission denied", "/sdcard/unauthorized.bin"], +] as const)( + "attributes %s refusal %s to %s", + async (operation, reason, path) => { + const { binary, root } = await writeAdbStub( + () => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi +if [ "$1" != "-s" ] || [ "$2" != "emulator-5554" ]; then exit 91; fi +if [ "$3" = "shell" ] && [ "$4" = "test" ] && [ "$6" = "/sdcard/unauthorized.bin" ]; then exit 1; fi +if [ "$3:$4" = "pull:/sdcard/unauthorized.bin" ] || [ "$3:$5" = "push:/sdcard/unauthorized.bin" ]; then + printf '%s\\n' '${reason.replaceAll("'", "'\\''")}' >&2 + exit 1 +fi +exit 91`, + ); + const localPath = join(root, "payload.bin"); + await writeFile(localPath, "payload"); + const input = { + serial: "emulator-5554", + device_path: "/sdcard/unauthorized.bin", + }; + const projected = await refusedBy( + providerOver(binary), + operation === "pull_adb_file" + ? { + operation, + input: { ...input, output_directory: join(root, "pulls") }, + } + : { + operation, + input: { ...input, local_path: localPath, overwrite: false }, + }, + ); + expect(projected).toMatchObject({ + code: "unsupported_target", + details: { operation, path, reason }, + message: `Unsupported target for ${operation} at ${path}: ${reason}`, + }); + }, +); + +it.each([ + ["pull_adb_file", "cannot create '%s': Permission denied"], + ["pull_adb_file", "cannot create file/directory '%s': Permission denied"], + ["pull_adb_file", "cannot write '%s': Permission denied"], + ["push_adb_file", "cannot stat '%s': No such file or directory"], + ["push_adb_file", "opening '%s' locally failed: Permission denied"], + ["push_adb_file", "reading '%s' locally failed: Permission denied"], + ["push_adb_file", "failed to read all of '%s': Permission denied"], +] as const)( + "keeps the actual host file identity for %s refusal %s", + async (operation, diagnostic) => { + const { binary, root } = await writeAdbStub( + (stubRoot) => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi +if [ "$1" != "-s" ] || [ "$2" != "emulator-5554" ]; then exit 91; fi +if [ "$3" = "shell" ] && [ "$4" = "test" ]; then exit 1; fi +if [ "$3" = "pull" ]; then + printf '%s' "$5" > '${stubRoot}/host-path' + printf "adb: error: ${diagnostic}\\n" "$5" >&2 + exit 1 +fi +if [ "$3" = "push" ]; then + printf '%s' "$4" > '${stubRoot}/host-path' + printf "adb: error: ${diagnostic}\\n" "$4" >&2 + exit 1 +fi +exit 91`, + ); + const localPath = join(root, "payload.bin"); + await writeFile(localPath, "payload"); + const input = { + serial: "emulator-5554", + device_path: "/sdcard/sample.bin", + }; + const projected = await refusedBy( + providerOver(binary), + operation === "pull_adb_file" + ? { + operation, + input: { ...input, output_directory: join(root, "pulls") }, + } + : { + operation, + input: { ...input, local_path: localPath, overwrite: false }, + }, + ); + const hostPath = await readFile(join(root, "host-path"), "utf8"); + expect(projected).toMatchObject({ + code: "unsupported_target", + details: { + operation, + path: hostPath, + reason: expect.stringContaining(hostPath), + }, + }); + expect(projected.message).toContain(`at ${hostPath}:`); + }, +); + it.each([ ["/data/local/tmp/..", "dot_segment"], ["/data/local/tmp/", "empty_file_name"], @@ -1113,7 +1244,10 @@ if [ "$1" = "-s" ]; then printf '%s\\n' "$3" >> "${stubRoot}/calls"; printf '%s\ }, }); // Existence is unknown, so nothing is claimed about the path and no push runs. - expect(projected).toMatchObject({ code: "unsupported_target" }); + expect(projected).toMatchObject({ + code: "unsupported_target", + details: { path: "emulator-5554", reason: stderr }, + }); expect(projected.message).toContain(state); expect(projected.message).not.toContain("already exists"); expect(await readFile(join(root, "calls"), "utf8")).toBe("shell\n"); diff --git a/src/adb/AdbProvider.ts b/src/adb/AdbProvider.ts index dfcbce7d7..e1430804a 100644 --- a/src/adb/AdbProvider.ts +++ b/src/adb/AdbProvider.ts @@ -1318,7 +1318,9 @@ export class AdbProvider { const shell = (arguments_: readonly string[], limits: AdbRunLimits) => runAdb( operation, - "serial" in request.input ? request.input.serial : operation, + { + device: "serial" in request.input ? request.input.serial : operation, + }, selection.binary, adbArguments( "serial" in request.input ? request.input.serial : undefined, @@ -1338,7 +1340,7 @@ export class AdbProvider { case "list_adb_devices": { const ran = await runAdb( operation, - "adb-devices", + { device: "adb-devices" }, selection.binary, ["devices", "-l"], { @@ -1374,7 +1376,7 @@ export class AdbProvider { case "inspect_adb_device": { const ran = await runAdb( operation, - request.input.serial, + { device: request.input.serial }, selection.binary, adbArguments(request.input.serial, ["shell", "getprop"]), { @@ -1615,7 +1617,7 @@ export class AdbProvider { case "list_adb_directory": { const ran = await runAdbCapture( operation, - request.input.serial, + { device: request.input.serial, path: request.input.device_path }, selection.binary, adbArguments(request.input.serial, [ "shell", @@ -1909,7 +1911,7 @@ export class AdbProvider { const { serial, package: packageName } = input; const paths = await runAdb( operation, - serial, + { device: serial }, selection.binary, adbArguments(serial, ["shell", "pm", "path", packageName]), { @@ -2129,7 +2131,11 @@ export class AdbProvider { const localPath = join(packageDirectory, entry.file_name); const pulled = await runAdb( operation, - serial, + { + device: serial, + path: entry.device_path, + localPath, + }, selection.binary, adbArguments(serial, ["pull", entry.device_path, localPath]), { timeoutMs: PULL_TIMEOUT_MS, maxOutputBytes: PULL_MAX_OUTPUT_BYTES }, @@ -2302,7 +2308,11 @@ export class AdbProvider { if (existing !== undefined) return err(existing); const pulled = await runAdb( operation, - input.serial, + { + device: input.serial, + path: input.device_path, + localPath, + }, selection.binary, adbArguments(input.serial, ["pull", input.device_path, localPath]), { timeoutMs: PULL_TIMEOUT_MS, maxOutputBytes: PULL_MAX_OUTPUT_BYTES }, @@ -2423,7 +2433,11 @@ export class AdbProvider { ); const pushed = await runAdb( operation, - input.device_path, + { + device: input.serial, + path: input.device_path, + localPath: digest.path, + }, selection.binary, adbArguments(input.serial, ["push", digest.path, input.device_path]), { timeoutMs: PULL_TIMEOUT_MS, maxOutputBytes: PULL_MAX_OUTPUT_BYTES }, @@ -2457,7 +2471,7 @@ export class AdbProvider { ); const deviceDigest = await runAdb( operation, - input.device_path, + { device: input.serial, path: input.device_path }, selection.binary, adbArguments(input.serial, ["shell", "sha256sum", input.device_path]), { timeoutMs: SHELL_TIMEOUT_MS, maxOutputBytes: 1024 }, @@ -2563,7 +2577,7 @@ export class AdbProvider { ); const installed = await runAdb( operation, - input.serial, + { device: input.serial, path: apkPath, localPath: digest.path }, selection.binary, adbArguments(input.serial, [ "install", @@ -2621,7 +2635,7 @@ export class AdbProvider { ): Promise> { const uninstalled = await runAdb( operation, - input.serial, + { device: input.serial }, selection.binary, adbArguments(input.serial, [ "shell", @@ -2678,7 +2692,7 @@ export class AdbProvider { ): Promise> { const resolved = await runAdb( operation, - input.serial, + { device: input.serial }, selection.binary, adbArguments(input.serial, [ "shell", @@ -2816,7 +2830,7 @@ export class AdbProvider { ): Promise> { const started = await runAdb( operation, - serial, + { device: serial }, selection.binary, adbArguments(serial, ["shell", "am", "start", "-W", ...arguments_]), { timeoutMs: SHELL_TIMEOUT_MS, maxOutputBytes: PM_PATH_MAX_OUTPUT_BYTES }, @@ -2885,7 +2899,7 @@ export class AdbProvider { ): Promise> { const stopped = await runAdb( operation, - input.serial, + { device: input.serial }, selection.binary, adbArguments(input.serial, ["shell", "am", "force-stop", input.package]), { timeoutMs: SHELL_TIMEOUT_MS, maxOutputBytes: PM_PATH_MAX_OUTPUT_BYTES }, @@ -2927,7 +2941,7 @@ export class AdbProvider { if (existing !== undefined) return err(existing); const captured = await runAdbCapture( operation, - input.serial, + { device: input.serial }, selection.binary, adbArguments(input.serial, ["exec-out", "screencap", "-p"]), { timeoutMs: SCREEN_TIMEOUT_MS, maxOutputBytes: SCREEN_MAX_OUTPUT_BYTES }, @@ -3077,7 +3091,7 @@ export class AdbProvider { }; const collected = await runAdb( operation, - input.serial, + { device: input.serial }, selection.binary, adbArguments(input.serial, ["bugreport", stagingDirectory]), { diff --git a/tests/acceptance/applications/adbErrorIdentity.test.ts b/tests/acceptance/applications/adbErrorIdentity.test.ts new file mode 100644 index 000000000..d71b5c281 --- /dev/null +++ b/tests/acceptance/applications/adbErrorIdentity.test.ts @@ -0,0 +1,96 @@ +import { readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect } from "vitest"; + +import { connectLocalToolsMcp } from "../../fixtures/localToolsMcp.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +cliTest.skipIf(process.platform === "win32")( + "keeps device, remote file and host file refusals distinct through CLI and MCP", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-adb-error-identity-"); + const binary = join(root, "adb"); + const source = join(root, "source.bin"); + await writeFile(source, "caller bytes"); + await writeFile( + binary, + `#!${process.execPath} +const args = process.argv.slice(2); +if (args[0] === 'version') { + console.log('Android Debug Bridge version 1.0.41\\nVersion 34.0.5-fixture\\nInstalled as ' + process.argv[1]); + process.exit(0); +} +if (args[0] !== '-s' || args[1] !== 'device') process.exit(91); +if (args[2] === 'shell') console.error('adb: error: connect failed: device offline'); +else if (args[2] === 'pull' && args[3] === '/sdcard/host.bin') + console.error("adb: error: cannot write '" + args[4] + "': Permission denied"); +else if (args[2] === 'pull') + console.error("adb: error: failed to stat remote object '" + args[3] + "': No such file or directory"); +else process.exit(91); +process.exit(1); +`, + { mode: 0o700 }, + ); + const environment = { + HOME: root, + USERPROFILE: root, + REA_ADB_PATH: binary, + REA_LOG_LEVEL: "silent", + }; + const { client, call } = await connectLocalToolsMcp(environment); + for (const scenario of [ + { + file: "missing.bin", + operation: "pull_adb_file", + path: "/sdcard/missing.bin", + }, + { + file: `${"directory/".repeat(60)}long.bin`, + operation: "pull_adb_file", + path: `/sdcard/${"directory/".repeat(60)}long.bin`, + }, + { + file: "host.bin", + operation: "pull_adb_file", + path: join(root, "host.bin"), + }, + { file: "offline.bin", operation: "push_adb_file", path: "device" }, + ] as const) { + const devicePath = `/sdcard/${scenario.file}`; + const transfer = + scenario.operation === "pull_adb_file" + ? { + arguments: [devicePath, "--output-directory", root], + input: { output_directory: root }, + } + : { arguments: [source, devicePath], input: { local_path: source } }; + const output = await cli.run({ + arguments: [ + scenario.operation.replaceAll("_", "-"), + "device", + ...transfer.arguments, + "--json", + ], + environment, + }); + expect(output.exitCode, output.stderr).toBe(1); + expect(output.json).toMatchObject({ + code: "unsupported_target", + details: { operation: scenario.operation, path: scenario.path }, + }); + const reply = await call(scenario.operation, { + serial: "device", + device_path: devicePath, + ...transfer.input, + }); + expect(reply.isError).toBe(true); + const content = reply.content.find((item) => item.type === "text"); + if (content?.type !== "text") throw new Error("Missing error response"); + expect(JSON.parse(content.text)).toEqual({ error: output.json }); + await client.ping(); + } + expect(await readFile(source, "utf8")).toBe("caller bytes"); + }, +); diff --git a/tests/boundary/providers/adb/adbCommandFinalization.test.ts b/tests/boundary/providers/adb/adbCommandFinalization.test.ts index 0c2832d80..5d9a4b242 100644 --- a/tests/boundary/providers/adb/adbCommandFinalization.test.ts +++ b/tests/boundary/providers/adb/adbCommandFinalization.test.ts @@ -63,7 +63,7 @@ const capture = ( ) => runAdbCapture( "inspect_adb_device", - "device", + { device: "device" }, process.execPath, ["-e", script], limits,