From 12a84ccc0a5485fb626932be29223acc1d6a2e2d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 8 Jun 2026 17:00:20 +0000 Subject: [PATCH] fix(android): surface adb shell exit code in runAdbShell adb.shell only resolves with stdout, and on devices using the legacy adb shell protocol the remote command's exit code is not propagated to the adb process. A failing command therefore resolved as if it succeeded and callers could not perceive the failure. Add AndroidDevice.runShellCommandWithExitCode, which appends an exit-code marker to the command, parses it back from stdout, and throws (including stdout/stderr) when the command exits non-zero. Route both the action call and the timeout path of agent.runAdbShell through it so failures are observable across shell protocol versions. https://claude.ai/code/session_01L925LrXF9ungqFc1mXKyPi --- packages/android/src/agent.ts | 5 +- packages/android/src/device.ts | 59 ++++++++++++++++- .../android/tests/unit-test/agent.test.ts | 13 ++-- packages/android/tests/unit-test/page.test.ts | 63 +++++++++++++++++++ 4 files changed, 132 insertions(+), 8 deletions(-) diff --git a/packages/android/src/agent.ts b/packages/android/src/agent.ts index a8348639e3..3903fd3126 100644 --- a/packages/android/src/agent.ts +++ b/packages/android/src/agent.ts @@ -117,8 +117,9 @@ export class AndroidAgent extends PageAgent { */ async runAdbShell(command: string, opt?: RunAdbShellOpt): Promise { if (opt?.timeout !== undefined) { - const adb = await this.interface.getAdb(); - return await adb.shell(command, { timeout: opt.timeout }); + return await this.interface.runShellCommandWithExitCode(command, { + timeout: opt.timeout, + }); } const action = diff --git a/packages/android/src/device.ts b/packages/android/src/device.ts index b8986fabd1..7f23497200 100644 --- a/packages/android/src/device.ts +++ b/packages/android/src/device.ts @@ -57,6 +57,12 @@ const defaultNormalScrollDuration = 1000; const IME_STRATEGY_ALWAYS_YADB = 'always-yadb' as const; const IME_STRATEGY_YADB_FOR_NON_ASCII = 'yadb-for-non-ascii' as const; + +// Marker used to capture the exit code of a command executed via `adb shell`. +// `adb.shell` only resolves with stdout, and on devices using the legacy adb +// shell protocol the remote exit code is not propagated to the adb process, +// so we echo it explicitly and parse it back from stdout. +const ADB_SHELL_EXIT_CODE_MARKER = '__MIDSCENE_ADB_EXIT_CODE__'; type ScrollDirection = 'up' | 'down' | 'left' | 'right'; const debugDevice = getDebug('android:device'); @@ -441,6 +447,56 @@ ${Object.keys(size) }); } + /** + * Execute an `adb shell` command and surface the remote command's exit code. + * + * `adb.shell` only resolves with stdout, and on devices that use the legacy + * adb shell protocol the remote exit code is not propagated to the adb + * process. That means a failing command would resolve as if it succeeded and + * the caller could not tell it failed. To make failures observable across + * shell protocol versions, append an exit-code marker to the command, parse + * it back from stdout, and throw when the command exited non-zero. + * + * @returns stdout of the command (without the exit-code marker) on success. + * @throws if the command exits with a non-zero code. + */ + public async runShellCommandWithExitCode( + command: string, + opts: { timeout?: number } = {}, + ): Promise { + const adb = await this.getAdb(); + const wrappedCommand = `${command}\necho "${ADB_SHELL_EXIT_CODE_MARKER}$?"`; + const { stdout, stderr } = (await adb.shell(wrappedCommand, { + ...opts, + outputFormat: adb.EXEC_OUTPUT_FORMAT.FULL, + })) as { stdout: string; stderr: string }; + + const markerMatch = stdout.match( + new RegExp(`${ADB_SHELL_EXIT_CODE_MARKER}(-?\\d+)\\s*$`), + ); + if (!markerMatch || markerMatch.index === undefined) { + // The marker is missing (unexpected). Return the raw stdout untouched so + // we don't mask the command output, but log it for debugging. + debugDevice( + `runShellCommandWithExitCode: exit code marker not found for command: ${command}`, + ); + return stdout; + } + + const exitCode = Number(markerMatch[1]); + const commandStdout = stdout.slice(0, markerMatch.index).replace(/\n$/, ''); + + if (exitCode !== 0) { + throw new Error( + `ADB shell command exited with code ${exitCode}: ${command}\n` + + `stdout: ${commandStdout || ''}\n` + + `stderr: ${stderr?.trim() || ''}`, + ); + } + + return commandStdout; + } + /** * Get or create the scrcpy adapter (lazy initialization) */ @@ -2049,8 +2105,7 @@ const createPlatformActions = ( if (!param.command || param.command.trim() === '') { throw new Error('RunAdbShell requires a non-empty command parameter'); } - const adb = await device.getAdb(); - return await adb.shell(param.command); + return await device.runShellCommandWithExitCode(param.command); }, }), Launch: defineAction({ diff --git a/packages/android/tests/unit-test/agent.test.ts b/packages/android/tests/unit-test/agent.test.ts index 284a1aa384..a17aae133a 100644 --- a/packages/android/tests/unit-test/agent.test.ts +++ b/packages/android/tests/unit-test/agent.test.ts @@ -156,10 +156,13 @@ describe('AndroidAgent', () => { }); describe('runAdbShell', () => { - it('should pass timeout options to adb.shell without changing action schema', async () => { + it('should pass timeout options to the device shell helper without changing action schema', async () => { const mockPage = new AndroidDevice('test-device'); - const shell = vi.fn().mockResolvedValue('adb-result'); - (mockPage as any).getAdb = vi.fn().mockResolvedValue({ shell }); + const runShellCommandWithExitCode = vi + .fn() + .mockResolvedValue('adb-result'); + (mockPage as any).runShellCommandWithExitCode = + runShellCommandWithExitCode; const agent = new AndroidAgent(mockPage, { modelConfig: mockedModelConfig, @@ -168,7 +171,9 @@ describe('AndroidAgent', () => { await expect( agent.runAdbShell('sleep 2', { timeout: 2_000 }), ).resolves.toBe('adb-result'); - expect(shell).toHaveBeenCalledWith('sleep 2', { timeout: 2_000 }); + expect(runShellCommandWithExitCode).toHaveBeenCalledWith('sleep 2', { + timeout: 2_000, + }); }); }); diff --git a/packages/android/tests/unit-test/page.test.ts b/packages/android/tests/unit-test/page.test.ts index 8496dab0e5..5c8d1b543f 100644 --- a/packages/android/tests/unit-test/page.test.ts +++ b/packages/android/tests/unit-test/page.test.ts @@ -29,6 +29,7 @@ const createMockAdb = () => ({ hideKeyboard: vi.fn(), push: vi.fn(), isSoftKeyboardPresent: vi.fn().mockResolvedValue(false), + EXEC_OUTPUT_FORMAT: { STDOUT: 'stdout', FULL: 'full' }, }); let mockAdbInstance: ReturnType; @@ -2487,4 +2488,66 @@ describe('AndroidDevice', () => { ); }); }); + + describe('runShellCommandWithExitCode', () => { + it('should append an exit-code marker and request full output', async () => { + mockAdb.shell.mockResolvedValueOnce({ + stdout: 'hello\n__MIDSCENE_ADB_EXIT_CODE__0', + stderr: '', + } as any); + + const result = await device.runShellCommandWithExitCode('echo hello'); + + expect(result).toBe('hello'); + expect(mockAdb.shell).toHaveBeenCalledWith( + 'echo hello\necho "__MIDSCENE_ADB_EXIT_CODE__$?"', + { outputFormat: 'full' }, + ); + }); + + it('should forward the timeout option to adb.shell', async () => { + mockAdb.shell.mockResolvedValueOnce({ + stdout: '__MIDSCENE_ADB_EXIT_CODE__0', + stderr: '', + } as any); + + await device.runShellCommandWithExitCode('whoami', { timeout: 2_000 }); + + expect(mockAdb.shell).toHaveBeenCalledWith( + 'whoami\necho "__MIDSCENE_ADB_EXIT_CODE__$?"', + { timeout: 2_000, outputFormat: 'full' }, + ); + }); + + it('should throw with exit code, stdout and stderr when command fails', async () => { + mockAdb.shell.mockResolvedValueOnce({ + stdout: 'partial output\n__MIDSCENE_ADB_EXIT_CODE__1', + stderr: 'No such file or directory', + } as any); + + await expect( + device.runShellCommandWithExitCode('cat /missing'), + ).rejects.toThrow(/exited with code 1/); + + mockAdb.shell.mockResolvedValueOnce({ + stdout: '__MIDSCENE_ADB_EXIT_CODE__2', + stderr: 'boom', + } as any); + + await expect(device.runShellCommandWithExitCode('false')).rejects.toThrow( + /stderr: boom/, + ); + }); + + it('should return raw stdout when the marker is missing', async () => { + mockAdb.shell.mockResolvedValueOnce({ + stdout: 'unexpected output without marker', + stderr: '', + } as any); + + const result = await device.runShellCommandWithExitCode('weird'); + + expect(result).toBe('unexpected output without marker'); + }); + }); });