From 04310da48b522089ccb45e4655c9aab36a672b93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 28 Oct 2026 12:47:55 +0100 Subject: [PATCH 01/18] fix(android): record the instrumentation result when the snapshot helper exits without output Refs #4005 Co-Authored-By: Claude Sonnet 5.5 --- .../snapshot-helper-exit-result.test.ts | 240 ++++++++++++++++++ .../src/instrumentation-helper.ts | 79 +++++- .../src/snapshot-helper-capture.ts | 2 + .../src/snapshot-helper-session-lifecycle.ts | 1 + .../src/snapshot-helper-session-protocol.ts | 7 + packages/platform-android/src/snapshot.ts | 5 +- 6 files changed, 332 insertions(+), 2 deletions(-) create mode 100644 packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts diff --git a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts new file mode 100644 index 0000000000..68c9bcb4c5 --- /dev/null +++ b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts @@ -0,0 +1,240 @@ +import assert from 'node:assert/strict'; +import { promises as fs } from 'node:fs'; +import path from 'node:path'; +import { afterEach, beforeEach, test } from 'vitest'; +import { withDiagnosticsScope } from '@agent-device/host-kit/diagnostics'; +import { captureAndroidSnapshotWithHelper } from '../snapshot-helper-capture.ts'; +import { captureAndroidSnapshotWithHelperSession } from '../snapshot-helper-session.ts'; +import { resetAndroidSnapshotHelperSessions } from '../snapshot-helper-session-lifecycle.ts'; +import { resetAndroidSnapshotHelperRetirements } from '../snapshot-helper-retirement.ts'; +import { snapshotAndroid } from '../snapshot.ts'; +import type { AndroidAdbExecutor } from '../snapshot-helper-types.ts'; +import type { AndroidAdbProvider } from '../adb-executor.ts'; +import { + androidHelperRuntimeProbeResult, + createSessionProvider, + FakeAndroidProcess, + isAndroidHelperRuntimeForceStop, + isAndroidHelperRuntimeProbe, +} from './snapshot-helper-session.fixtures.ts'; +import { mkdtempForTest } from './test-utils/tmp-dir.ts'; +import { ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT } from './test-utils/android-snapshot-helper.ts'; + +beforeEach(async () => { + resetAndroidSnapshotHelperRetirements(); + await resetAndroidSnapshotHelperSessions(); +}); + +afterEach(async () => { + await resetAndroidSnapshotHelperSessions(); +}); + +// Instrumentation outputs as `am instrument -w` prints them when the helper never reports. +const ABORTED_RUN = [ + 'INSTRUMENTATION_STATUS: numtests=1', + 'INSTRUMENTATION_STATUS_CODE: 1', + 'INSTRUMENTATION_ABORTED: System has crashed.', + '', +].join('\n'); + +const CRASHED_RUN = [ + 'INSTRUMENTATION_RESULT: shortMsg=java.lang.IllegalStateException', + 'INSTRUMENTATION_RESULT: longMsg=java.lang.IllegalStateException: UiAutomation not connected', + 'INSTRUMENTATION_CODE: 0', + '', +].join('\n'); + +const TRUNCATED_XML_RUN = [ + 'INSTRUMENTATION_STATUS: agentDeviceProtocol=android-snapshot-helper-v1', + 'INSTRUMENTATION_STATUS: outputFormat=uiautomator-xml', + 'INSTRUMENTATION_STATUS: chunkIndex=0', + 'INSTRUMENTATION_STATUS: chunkCount=1', + `INSTRUMENTATION_STATUS: payloadBase64=${Buffer.from(']+/gi; -const REDACTED_STRING_MAX_LENGTH = 400; +/** The longest ordinary string a redacted diagnostic value keeps. */ +export const REDACTED_STRING_MAX_LENGTH = 400; const REDACTED_STDERR_MAX_LENGTH = 8192; const TRUNCATION_SUFFIX = '...'; @@ -53,9 +54,21 @@ function redactString(value: string, keyHint?: string): string { const trimmed = value.trim(); if (!trimmed) return boundRedactedString(value, keyHint); if (keyHint && SENSITIVE_KEY_RE.test(keyHint)) return '[REDACTED]'; + return boundRedactedString(redactSensitiveText(trimmed), keyHint); +} + +/** + * Redacts secrets and URL credentials in free text without the length bound + * `redactDiagnosticData` applies, for a caller that must choose which end of the text survives. + */ +export function redactDiagnosticText(value: string): string { + return redactSensitiveText(value.trim()); +} + +function redactSensitiveText(trimmed: string): string { let output = redactUrls(trimmed); output = output.replace(SECRET_TOKEN_RE, '[REDACTED]'); - output = output.replace( + return output.replace( SENSITIVE_ASSIGNMENT_RE, (match, key: string, separator: string, rawValue: string, offset: number, input: string) => { if (isSafeSetupUrlAssignment({ key, separator, rawValue, offset, input })) return match; @@ -63,7 +76,6 @@ function redactString(value: string, keyHint?: string): string { return `${key}${separator}[REDACTED]`; }, ); - return boundRedactedString(output, keyHint); } function boundRedactedString(value: string, keyHint?: string): string { diff --git a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts index 59dca73133..02f0be4152 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts @@ -8,6 +8,8 @@ import { captureAndroidSnapshotWithHelperSession } from '../snapshot-helper-sess import { resetAndroidSnapshotHelperSessions } from '../snapshot-helper-session-lifecycle.ts'; import { resetAndroidSnapshotHelperRetirements } from '../snapshot-helper-retirement.ts'; import { snapshotAndroid } from '../snapshot.ts'; +import { AppError, normalizeError } from '@agent-device/kernel/errors'; +import type { DeviceInfo } from '@agent-device/kernel/device'; import type { AndroidAdbExecutor } from '../snapshot-helper-types.ts'; import type { AndroidAdbProvider } from '../adb-executor.ts'; import { @@ -54,71 +56,236 @@ const TRUNCATED_XML_RUN = [ '', ].join('\n'); -function captureWith(result: { exitCode: number; stdout: string; stderr?: string }) { - return captureAndroidSnapshotWithHelper({ - adb: async () => ({ stderr: '', ...result }), - }); -} +type FailedRun = { exitCode: number; stdout: string; stderr?: string; signal?: string }; + +type ExitResultFields = { + exitCode: number | null; + signal?: string; + resultCode?: number; + aborted?: string; + failed?: string; + shortMsg?: string; + longMsg?: string; + outputTail: string; + outputTruncated: boolean; +}; -async function captureDetails(result: { - exitCode: number; - stdout: string; - stderr?: string; -}): Promise> { +async function captureError(run: FailedRun): Promise { try { - await captureWith(result); + await captureAndroidSnapshotWithHelper({ adb: async () => ({ stderr: '', ...run }) }); } catch (error) { - return (error as { details: Record }).details; + return error as AppError; } throw new Error('expected the capture to fail'); } -test('an aborted instrumentation run reports the abort and the exit status', async () => { - const details = await captureDetails({ exitCode: 1, stdout: ABORTED_RUN }); +async function captureExitResult(run: FailedRun): Promise { + const error = await captureError(run); + return error.details?.instrumentation as ExitResultFields; +} + +function encodedPayload(xml: string): string { + return `INSTRUMENTATION_STATUS: payloadBase64=${Buffer.from(xml).toString('base64')}`; +} + +const SECRET = 'hunter2-account-number'; +const SECRET_XML = ``; - assert.deepEqual(details.instrumentation, { +test('an aborted instrumentation run reports the abort and the exit status', async () => { + assert.deepEqual(await captureExitResult({ exitCode: 1, stdout: ABORTED_RUN }), { exitCode: 1, aborted: 'System has crashed.', - outputTail: ABORTED_RUN, + outputTail: ABORTED_RUN.trimEnd(), outputTruncated: false, }); }); test('a crashed instrumentation run reports shortMsg, longMsg and the result code', async () => { - const details = await captureDetails({ exitCode: 0, stdout: CRASHED_RUN }); - - assert.deepEqual(details.instrumentation, { + assert.deepEqual(await captureExitResult({ exitCode: 0, stdout: CRASHED_RUN }), { exitCode: 0, resultCode: 0, shortMsg: 'java.lang.IllegalStateException', longMsg: 'java.lang.IllegalStateException: UiAutomation not connected', - outputTail: CRASHED_RUN, + outputTail: CRASHED_RUN.trimEnd(), outputTruncated: false, }); }); -test('a truncated XML exit keeps its exit status and only a bounded tail of the snapshot', async () => { - const bulk = `${'x'.repeat(20_000)}\n`; - const details = await captureDetails({ +test('the host signal that ended the run reaches the details', async () => { + const result = await captureExitResult({ exitCode: 1, stdout: '', signal: 'SIGKILL' }); + + assert.equal(result.signal, 'SIGKILL'); +}); + +test('an empty INSTRUMENTATION_CODE is not recorded as a zero result', async () => { + const result = await captureExitResult({ exitCode: 1, - stdout: bulk + TRUNCATED_XML_RUN, - stderr: 'am: Killed', + stdout: 'INSTRUMENTATION_RESULT: shortMsg=Process crashed.\nINSTRUMENTATION_CODE: \n', }); - const instrumentation = details.instrumentation as { - exitCode: number; - outputTail: string; - outputTruncated: boolean; - }; + assert.equal(result.resultCode, undefined); + assert.equal(result.shortMsg, 'Process crashed.'); +}); + +test('multiline result values keep their continuation lines', async () => { + const crlf = await captureExitResult({ + exitCode: 0, + stdout: [ + 'INSTRUMENTATION_RESULT: shortMsg=Process crashed.', + 'INSTRUMENTATION_RESULT: longMsg=UiAutomation failed', + 'Caused by: binder has died', + 'INSTRUMENTATION_CODE: 0', + '', + ].join('\r\n'), + }); + assert.equal(crlf.shortMsg, 'Process crashed.'); + assert.equal(crlf.longMsg, 'UiAutomation failed\nCaused by: binder has died'); + assert.equal(crlf.resultCode, 0); + + const stack = await captureExitResult({ + exitCode: 0, + stdout: [ + 'INSTRUMENTATION_RESULT: longMsg=java.lang.IllegalStateException: no root', + '\tat android.app.UiAutomation.connect(UiAutomation.java:12)', + '\tat com.callstack.agentdevice.snapshothelper.Main.(Main.java:7)', + 'INSTRUMENTATION_CODE: 0', + ].join('\n'), + }); + assert.equal( + stack.longMsg, + [ + 'java.lang.IllegalStateException: no root', + '\tat android.app.UiAutomation.connect(UiAutomation.java:12)', + '\tat com.callstack.agentdevice.snapshothelper.Main.(Main.java:7)', + ].join('\n'), + ); + + const interleaved = await captureExitResult({ + exitCode: 0, + stdout: [ + 'INSTRUMENTATION_RESULT: longMsg=first line', + 'second line', + encodedPayload(SECRET_XML), + 'INSTRUMENTATION_STATUS_CODE: 1', + 'not part of the message', + 'INSTRUMENTATION_CODE: 0', + ].join('\n'), + }); + assert.equal(interleaved.longMsg, 'first line\nsecond line'); +}); + +test('startup failures are recognized in every envelope AOSP prints', async () => { + const forms = [ + ['INSTRUMENTATION_FAILED: com.example/.Missing', 'com.example/.Missing'], + [ + 'android.util.AndroidException: INSTRUMENTATION_FAILED: com.example/.Missing', + 'com.example/.Missing', + ], + [ + 'onError: commandError=true message=INSTRUMENTATION_FAILED: com.example/.Missing', + 'com.example/.Missing', + ], + ['INSTRUMENTATION_FAILED:', ''], + ] as const; + for (const [line, expected] of forms) { + const result = await captureExitResult({ exitCode: 1, stdout: `${line}\n` }); + assert.equal(result.failed, expected, line); + } +}); + +test('a truncated XML exit keeps its exit status and drops the snapshot payload', async () => { + const details = ( + await captureError({ + exitCode: 1, + stdout: `${'x'.repeat(20_000)}\n${TRUNCATED_XML_RUN}`, + stderr: 'am: Killed', + }) + ).details as { instrumentation: ExitResultFields }; + + const instrumentation = details.instrumentation; assert.equal(instrumentation.exitCode, 1); assert.equal(instrumentation.outputTruncated, true); - assert.ok(instrumentation.outputTail.length <= 4_096); + assert.ok(instrumentation.outputTail.length <= 400); assert.ok(instrumentation.outputTail.endsWith('am: Killed')); - assert.ok(!instrumentation.outputTail.includes('x'.repeat(5_000))); + assert.ok(instrumentation.outputTail.includes('payloadBase64=')); +}); + +test('encoded and raw snapshot content never reaches the error details', async () => { + const runs = [ + { + exitCode: 1, + stdout: [ + 'INSTRUMENTATION_STATUS: agentDeviceProtocol=android-snapshot-helper-v1', + encodedPayload(SECRET_XML), + 'INSTRUMENTATION_STATUS_CODE: 1', + ].join('\n'), + stderr: 'am: Killed', + }, + { + exitCode: 1, + stdout: `INSTRUMENTATION_STATUS: stream=${SECRET_XML}\nINSTRUMENTATION_STATUS_CODE: 1`, + stderr: '', + }, + // An incomplete raw fragment, cut inside an attribute. + { exitCode: 1, stdout: ` { + const kill = ['-e', "process.kill(process.pid, 'SIGKILL')"]; + const options = { allowFailure: true }; - const exited = await runCmd(process.execPath, ['-e', 'process.exit(3)'], { allowFailure: true }); - assert.equal(exited.exitCode, 3); - assert.equal('signal' in exited, false); -}); + assert.equal((await runCmd(process.execPath, kill, options)).signal, 'SIGKILL'); + assert.equal((await runCmdStreaming(process.execPath, kill, options)).signal, 'SIGKILL'); + assert.equal(runCmdSync(process.execPath, kill, options).signal, 'SIGKILL'); + assert.equal((await runCmdBackground(process.execPath, kill, options).wait).signal, 'SIGKILL'); + + const exited = await runCmd(process.execPath, ['-e', 'process.exit(3)'], options); + assert.equal(exited.exitCode, 3); + assert.equal('signal' in exited, false); + }, +); diff --git a/packages/host-kit/src/internal/exec.ts b/packages/host-kit/src/internal/exec.ts index 5dda5d2dc0..b552493e5b 100644 --- a/packages/host-kit/src/internal/exec.ts +++ b/packages/host-kit/src/internal/exec.ts @@ -452,7 +452,13 @@ export function runCmdSync( throw createExitError(executable, cmd, args, exitCode, stdout, stderr); } - return { stdout, stderr, exitCode, stdoutBuffer }; + return { + stdout, + stderr, + exitCode, + stdoutBuffer, + ...(result.signal ? { signal: result.signal } : {}), + }; } export function runCmdDetached( @@ -563,7 +569,12 @@ export function runCmdBackground( reject(failure); return; } - resolve({ stdout, stderr, exitCode: finalExitCode }); + resolve({ + stdout, + stderr, + exitCode: finalExitCode, + ...(child.signalCode ? { signal: child.signalCode } : {}), + }); } const settlement = createCommandKillSettlement({ killProcessTree: () => killProcessTree(child, options), diff --git a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts index 02f0be4152..fb39ed5741 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts @@ -239,6 +239,103 @@ test('encoded and raw snapshot content never reaches the error details', async ( } }); +test('a large one-line XML report never leaves a sensitive value in the details or the request log', async () => { + const nodes = Array.from( + { length: 400 }, + (_, index) => ``, + ).join(''); + const lateSecret = ``; + const stdouts = [ + `${nodes}${lateSecret}`, + // Cut so that the kept end of the report has no opening tag left. + `${'a'.repeat(30_000)} text="${SECRET}" content-desc="account" bounds="[0,0][1,1]" />`, + `INSTRUMENTATION_STATUS: stream=${nodes}${lateSecret}`, + ]; + for (const stdout of stdouts) { + const error = await captureError({ exitCode: 1, stdout, stderr: 'am: Killed' }); + const wire = JSON.stringify(normalizeError(error)); + assert.ok(!wire.includes(SECRET), wire.slice(-600)); + assert.ok(wire.includes('am: Killed')); + + const tmpDir = await mkdtempForTest('agent-device-android-capture-xml-diag-'); + const logPath = path.join(tmpDir, 'diag.ndjson'); + try { + await withDiagnosticsScope( + { debug: true, logPath, session: 'android-test', requestId: 'req-7', command: 'snapshot' }, + async () => { + await assert.rejects( + snapshotAndroid(emulator, { + helperAdb: helperFailingWith({ exitCode: 1, stdout, stderr: 'am: Killed' }), + helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, + }), + ); + }, + ); + const log = await fs.readFile(logPath, 'utf8'); + assert.ok(log.includes('android_snapshot_helper_failed')); + assert.ok(!log.includes(SECRET), log.slice(-600)); + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } + } +}); + +test('a single-line shortMsg or longMsg is held to the message cap', async () => { + const result = await captureExitResult({ + exitCode: 0, + stdout: `INSTRUMENTATION_RESULT: shortMsg=${'s'.repeat(10_000)}\nINSTRUMENTATION_RESULT: longMsg=${'l'.repeat(10_000)}\nINSTRUMENTATION_CODE: 0`, + }); + + assert.equal(result.shortMsg?.length, 2_048); + assert.equal(result.longMsg?.length, 2_048); +}); + +test('an INSTRUMENTATION_CODE beyond the safe integer range is not recorded', async () => { + const result = await captureExitResult({ + exitCode: 1, + stdout: `INSTRUMENTATION_CODE: ${'9'.repeat(400)}\n`, + }); + + assert.equal(result.resultCode, undefined); +}); + +test('a session exit result includes output that arrives after the exit event', async () => { + const tmpDir = await mkdtempForTest('agent-device-android-session-drain-'); + const logPath = path.join(tmpDir, 'diag.ndjson'); + try { + const processes: FakeAndroidProcess[] = []; + const provider = createSessionProvider({ calls: [], processes }); + const capture = () => + captureAndroidSnapshotWithHelperSession({ + adb: provider.exec, + adbProvider: provider, + deviceKey: 'android:emulator-5556', + }); + await withDiagnosticsScope( + { debug: true, logPath, session: 'android-test', requestId: 'req-8', command: 'snapshot' }, + async () => { + assert.ok(await capture()); + const process = processes[0]!; + process.exitCode = 1; + process.emit('exit', 1, null); + const second = capture(); + await new Promise((resolve) => setTimeout(resolve, 20)); + process.stdout.write(CRASHED_RUN); + await new Promise((resolve) => setTimeout(resolve, 20)); + process.emit('close', 1, null); + await second; + }, + ); + + const event = await readDiagnostic(logPath, 'android_snapshot_helper_session_exited'); + const exit = readExitResult(event.data ?? {}); + assert.equal(exit.exitCode, 1); + assert.equal(exit.shortMsg, 'java.lang.IllegalStateException'); + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } +}); + test('the real cause past the redaction cap survives in the error and the request log', async () => { const filler = Array.from({ length: 40 }, (_, index) => `line ${index} of helper output`).join( '\n', diff --git a/packages/platform-android/src/instrumentation-exit-result.ts b/packages/platform-android/src/instrumentation-exit-result.ts index f8968517a2..da27a904e4 100644 --- a/packages/platform-android/src/instrumentation-exit-result.ts +++ b/packages/platform-android/src/instrumentation-exit-result.ts @@ -33,7 +33,10 @@ const PAYLOAD_OMITTED = 'payloadBase64='; const XML_OMITTED = ''; const PAYLOAD_BASE64_RE = /payloadBase64=\S*/g; const XML_START_RE = /<(?:\?xml|hierarchy|node)\b.*/g; -const XML_ELEMENT_LINE_RE = /^\s*<\/?(?:hierarchy|node)\b/; +const XML_MARKER_RE = /<\/?(?:\?xml|hierarchy|node)\b/; +// An attribute such as `text="…"` or `content-desc="…"` is snapshot content even when the +// element's opening tag was cut off or never printed. +const XML_ATTRIBUTE_RE = /\s[\w:.-]+="/; // `android.util.AndroidException: INSTRUMENTATION_FAILED: ` and // `onError: ... message=INSTRUMENTATION_FAILED: ` carry the marker mid-line. const FAILED_RE = /INSTRUMENTATION_FAILED:[ \t]?(.*)$/; @@ -43,16 +46,20 @@ const CODE_PREFIX = 'INSTRUMENTATION_CODE: '; const PROTOCOL_RECORD_RE = /^INSTRUMENTATION_(?:STATUS|STATUS_CODE|RESULT|CODE|ABORTED|FAILED):/; /** - * Removes the app content a helper run may have printed: encoded snapshot payloads and raw XML, - * including a fragment cut off mid-element. Instrumentation records and every other line stay. + * Removes the app content a helper run may have printed: encoded snapshot payloads and XML in any + * shape, including a fragment whose opening tag is missing. Instrumentation records and every other + * line stay. This runs on the whole output, before any tail is cut from it. */ export function withoutSnapshotContent(output: string): string { return output .split(/\r?\n/) - .filter((line) => !XML_ELEMENT_LINE_RE.test(line)) - .map((line) => - line.replace(PAYLOAD_BASE64_RE, PAYLOAD_OMITTED).replace(XML_START_RE, XML_OMITTED), - ) + .map((line) => { + if (PROTOCOL_RECORD_RE.test(line)) { + return line.replace(PAYLOAD_BASE64_RE, PAYLOAD_OMITTED).replace(XML_START_RE, XML_OMITTED); + } + if (XML_MARKER_RE.test(line) || XML_ATTRIBUTE_RE.test(line)) return XML_OMITTED; + return line.replace(PAYLOAD_BASE64_RE, PAYLOAD_OMITTED); + }) .join('\n'); } @@ -123,12 +130,14 @@ function readProtocolRecord(line: string, fields: InstrumentationFields): Messag const separator = record.indexOf('='); const key = record.slice(0, separator); if ((key !== 'shortMsg' && key !== 'longMsg') || fields[key] !== undefined) return undefined; - fields[key] = record.slice(separator + 1); + fields[key] = record.slice(separator + 1, separator + 1 + MESSAGE_FIELD_MAX_CHARS); return key; } if (line.startsWith(CODE_PREFIX)) { const code = line.slice(CODE_PREFIX.length).trim(); - if (/^-?\d+$/.test(code)) fields.resultCode ??= Number(code); + if (/^-?\d+$/.test(code) && Number.isSafeInteger(Number(code))) { + fields.resultCode ??= Number(code); + } return undefined; } const aborted = ABORTED_RE.exec(line); @@ -143,28 +152,54 @@ function appendLine(value: string | undefined, line: string): string | undefined /** The exit of a spawned helper process and what it printed, kept to a bounded recent window. */ export type AndroidInstrumentationProcessRecord = { - /** The process's exit result, or `undefined` while it is running. */ + /** Settles when the process's output streams have closed, the point its output is complete. */ + closed: Promise; + /** The process's exit result once its streams have closed, or `undefined` before that. */ exitResult(): AndroidInstrumentationExitResult | undefined; }; export function recordInstrumentationProcess( - process: AndroidAdbProcess, + child: AndroidAdbProcess, ): AndroidInstrumentationProcessRecord { - let output = ''; + // Snapshot content is removed per line as it completes, so the bounded window never holds a + // line whose opening tag was cut away. A line too long to hold is dropped as snapshot content. + let kept = ''; + let pending = ''; + let pendingDropped = false; + const keep = (line: string) => { + kept = `${kept}${withoutSnapshotContent(line)}\n`.slice(-OUTPUT_SCAN_MAX_CHARS); + }; const onData = (chunk: Buffer | string) => { - output = (output + chunk.toString()).slice(-OUTPUT_SCAN_MAX_CHARS); + const lines = (pending + chunk.toString()).split('\n'); + pending = lines.pop() ?? ''; + for (const line of lines) { + keep(pendingDropped ? XML_OMITTED : line); + pendingDropped = false; + } + if (pending.length > OUTPUT_SCAN_MAX_CHARS) { + pending = ''; + pendingDropped = true; + } }; - process.stdout?.on('data', onData); - process.stderr?.on('data', onData); + child.stdout?.on('data', onData); + child.stderr?.on('data', onData); + let isClosed = false; + const closed = new Promise((resolve) => { + child.once('close', () => { + isClosed = true; + resolve(); + }); + }); return { + closed, exitResult: () => - process.exitCode == null && process.signalCode == null - ? undefined - : readInstrumentationExitResult({ - stdout: output, + isClosed + ? readInstrumentationExitResult({ + stdout: pendingDropped ? `${kept}${XML_OMITTED}` : `${kept}${pending}`, stderr: '', - exitCode: process.exitCode, - signal: process.signalCode, - }), + exitCode: child.exitCode, + signal: child.signalCode, + }) + : undefined, }; } diff --git a/packages/platform-android/src/snapshot-helper-session-lifecycle.ts b/packages/platform-android/src/snapshot-helper-session-lifecycle.ts index 66c0b3f64f..1770552b80 100644 --- a/packages/platform-android/src/snapshot-helper-session-lifecycle.ts +++ b/packages/platform-android/src/snapshot-helper-session-lifecycle.ts @@ -471,7 +471,14 @@ export async function stopAndroidSnapshotHelperSession( const session = sessions().get(deviceKey); if (!session) return false; sessions().delete(deviceKey); - // Read before teardown: a process this stop ends has no cause of its own to report. + // Read before teardown, once the streams have drained: a process this stop ends has no cause of + // its own to report, and one that already exited may still be flushing its last output. + if (hasAndroidSnapshotHelperProcessEnded(session.process)) { + await waitForAndroidSnapshotHelperProcessExit( + session.processRecord.closed, + ANDROID_SNAPSHOT_HELPER_HOST_PROCESS_EXIT_GRACE_MS, + ); + } const exitedOnItsOwn = session.processRecord.exitResult(); if (exitedOnItsOwn) { emitDiagnostic({ From 30f2f0d0643b32701251144c44aa9b70953382e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 28 Oct 2026 14:19:53 +0100 Subject: [PATCH 05/18] fix(android): match only UiAutomator attributes as snapshot content and keep long diagnostic lines Fold the exit-result code into instrumentation-helper.ts so the eager closure does not grow. Refs #4005 Co-Authored-By: Claude Sonnet 5.5 --- .../snapshot-helper-exit-result.test.ts | 115 +++++++++ .../src/instrumentation-exit-result.ts | 205 --------------- .../src/instrumentation-helper.ts | 242 ++++++++++++++++++ .../src/snapshot-helper-capture.ts | 4 +- .../src/snapshot-helper-session-lifecycle.ts | 2 +- .../src/snapshot-helper-session-protocol.ts | 2 - 6 files changed, 359 insertions(+), 211 deletions(-) delete mode 100644 packages/platform-android/src/instrumentation-exit-result.ts diff --git a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts index fb39ed5741..3ba7f19209 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts @@ -280,6 +280,121 @@ test('a large one-line XML report never leaves a sensitive value in the details } }); +test('ordinary quoted assignments are kept while snapshot attributes are dropped', async () => { + const error = await captureError({ + exitCode: 1, + stdout: [ + 'E AndroidRuntime: java.lang.IllegalStateException message="binder died" code="7"', + `ret-value" text="${SECRET}" bounds="[0,0][1,1]" />`, + ].join('\n'), + stderr: 'am: Killed', + }); + + const result = error.details?.instrumentation as ExitResultFields & { + snapshotContentRemoved?: true; + }; + assert.ok(result.outputTail.includes('message="binder died" code="7"'), result.outputTail); + assert.ok(!result.outputTail.includes(SECRET)); + assert.equal(result.snapshotContentRemoved, true); + assert.equal(result.outputTruncated, true); +}); + +test('a long diagnostic line keeps its end and a long snapshot line is dropped', async () => { + const diagnostic = `${'frame '.repeat(5_000)}Caused by: binder has died`; + const kept = (await captureError({ exitCode: 1, stdout: diagnostic })).details + ?.instrumentation as ExitResultFields & { snapshotContentRemoved?: true }; + assert.ok(kept.outputTail.endsWith('Caused by: binder has died'), kept.outputTail); + assert.equal(kept.outputTruncated, true); + assert.equal(kept.snapshotContentRemoved, undefined); + + const snapshot = `${``.repeat(2_000)}`; + const dropped = (await captureError({ exitCode: 1, stdout: snapshot, stderr: 'am: Killed' })) + .details?.instrumentation as ExitResultFields & { snapshotContentRemoved?: true }; + assert.ok(!dropped.outputTail.includes(SECRET)); + assert.ok(dropped.outputTail.endsWith('am: Killed')); + assert.equal(dropped.snapshotContentRemoved, true); + assert.equal(dropped.outputTruncated, true); +}); + +test('a session exit result keeps a long diagnostic line and drops a long snapshot line', async () => { + const tmpDir = await mkdtempForTest('agent-device-android-session-long-line-'); + const logPath = path.join(tmpDir, 'diag.ndjson'); + try { + const processes: FakeAndroidProcess[] = []; + const provider = createSessionProvider({ calls: [], processes }); + const capture = () => + captureAndroidSnapshotWithHelperSession({ + adb: provider.exec, + adbProvider: provider, + deviceKey: 'android:emulator-5556', + }); + await withDiagnosticsScope( + { debug: true, logPath, session: 'android-test', requestId: 'req-9', command: 'snapshot' }, + async () => { + assert.ok(await capture()); + // The line arrives in pieces, so it outgrows the window before it completes. + processes[0]!.stdout.write('frame '.repeat(4_000)); + await new Promise((resolve) => setTimeout(resolve, 5)); + processes[0]!.stdout.write('frame '.repeat(1_000) + 'Caused by: binder has died\n'); + processes[0]!.stdout.write( + `${``.repeat(2_000)}\n`, + ); + processes[0]!.stdout.write('INSTRUMENTATION_ABORTED: System has crashed.\n'); + await new Promise((resolve) => setTimeout(resolve, 20)); + processes[0]!.emitExit(1, null); + await capture(); + }, + ); + + const log = await fs.readFile(logPath, 'utf8'); + assert.ok(!log.includes(SECRET)); + const exit = readExitResult( + (await readDiagnostic(logPath, 'android_snapshot_helper_session_exited')).data ?? {}, + ) as Partial & { snapshotContentRemoved?: true }; + assert.equal(exit.aborted, 'System has crashed.'); + assert.equal(exit.snapshotContentRemoved, true); + assert.equal(exit.outputTruncated, true); + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } +}); + +test('a session exit result keeps the end of a long diagnostic line', async () => { + const tmpDir = await mkdtempForTest('agent-device-android-session-long-diag-'); + const logPath = path.join(tmpDir, 'diag.ndjson'); + try { + const processes: FakeAndroidProcess[] = []; + const provider = createSessionProvider({ calls: [], processes }); + const capture = () => + captureAndroidSnapshotWithHelperSession({ + adb: provider.exec, + adbProvider: provider, + deviceKey: 'android:emulator-5556', + }); + await withDiagnosticsScope( + { debug: true, logPath, session: 'android-test', requestId: 'req-10', command: 'snapshot' }, + async () => { + assert.ok(await capture()); + // The line arrives in pieces, so it outgrows the window before it completes. + processes[0]!.stdout.write('frame '.repeat(4_000)); + await new Promise((resolve) => setTimeout(resolve, 5)); + processes[0]!.stdout.write('frame '.repeat(1_000) + 'Caused by: binder has died\n'); + await new Promise((resolve) => setTimeout(resolve, 20)); + processes[0]!.emitExit(1, null); + await capture(); + }, + ); + + const exit = readExitResult( + (await readDiagnostic(logPath, 'android_snapshot_helper_session_exited')).data ?? {}, + ); + assert.ok(exit.outputTail?.endsWith('Caused by: binder has died'), exit.outputTail); + assert.equal(exit.outputTruncated, true); + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } +}); + test('a single-line shortMsg or longMsg is held to the message cap', async () => { const result = await captureExitResult({ exitCode: 0, diff --git a/packages/platform-android/src/instrumentation-exit-result.ts b/packages/platform-android/src/instrumentation-exit-result.ts deleted file mode 100644 index da27a904e4..0000000000 --- a/packages/platform-android/src/instrumentation-exit-result.ts +++ /dev/null @@ -1,205 +0,0 @@ -import { REDACTED_STRING_MAX_LENGTH, redactDiagnosticText } from '@agent-device/kernel/redaction'; -import type { AndroidAdbProcess } from './adb-executor.ts'; - -/** - * What an `am instrument` run that returned no usable report says about its own end. Every field is - * a value the instrumentation or the host process printed or exited with; an absent field was not - * printed. - */ -export type AndroidInstrumentationExitResult = { - /** Host process exit status; `null` when a signal ended it. */ - exitCode: number | null; - signal?: string; - /** `INSTRUMENTATION_CODE`. */ - resultCode?: number; - /** `INSTRUMENTATION_ABORTED`: the system ended the run before it reported. */ - aborted?: string; - /** `INSTRUMENTATION_FAILED`: the run never started. */ - failed?: string; - shortMsg?: string; - longMsg?: string; - /** - * The end of the output after snapshot content is removed and secrets are redacted, held to the - * length a redacted diagnostic string keeps so the cause at the end is what survives the - * boundary. - */ - outputTail: string; - outputTruncated: boolean; -}; - -const OUTPUT_SCAN_MAX_CHARS = 16_384; -const MESSAGE_FIELD_MAX_CHARS = 2_048; -const PAYLOAD_OMITTED = 'payloadBase64='; -const XML_OMITTED = ''; -const PAYLOAD_BASE64_RE = /payloadBase64=\S*/g; -const XML_START_RE = /<(?:\?xml|hierarchy|node)\b.*/g; -const XML_MARKER_RE = /<\/?(?:\?xml|hierarchy|node)\b/; -// An attribute such as `text="…"` or `content-desc="…"` is snapshot content even when the -// element's opening tag was cut off or never printed. -const XML_ATTRIBUTE_RE = /\s[\w:.-]+="/; -// `android.util.AndroidException: INSTRUMENTATION_FAILED: ` and -// `onError: ... message=INSTRUMENTATION_FAILED: ` carry the marker mid-line. -const FAILED_RE = /INSTRUMENTATION_FAILED:[ \t]?(.*)$/; -const ABORTED_RE = /^INSTRUMENTATION_ABORTED:[ \t]?(.*)$/; -const RESULT_PREFIX = 'INSTRUMENTATION_RESULT: '; -const CODE_PREFIX = 'INSTRUMENTATION_CODE: '; -const PROTOCOL_RECORD_RE = /^INSTRUMENTATION_(?:STATUS|STATUS_CODE|RESULT|CODE|ABORTED|FAILED):/; - -/** - * Removes the app content a helper run may have printed: encoded snapshot payloads and XML in any - * shape, including a fragment whose opening tag is missing. Instrumentation records and every other - * line stay. This runs on the whole output, before any tail is cut from it. - */ -export function withoutSnapshotContent(output: string): string { - return output - .split(/\r?\n/) - .map((line) => { - if (PROTOCOL_RECORD_RE.test(line)) { - return line.replace(PAYLOAD_BASE64_RE, PAYLOAD_OMITTED).replace(XML_START_RE, XML_OMITTED); - } - if (XML_MARKER_RE.test(line) || XML_ATTRIBUTE_RE.test(line)) return XML_OMITTED; - return line.replace(PAYLOAD_BASE64_RE, PAYLOAD_OMITTED); - }) - .join('\n'); -} - -export function readInstrumentationExitResult(run: { - stdout: string; - stderr: string; - exitCode: number | null | undefined; - signal?: string | null; -}): AndroidInstrumentationExitResult { - const streams = [run.stdout, run.stderr].filter(Boolean).map(withoutSnapshotContent); - const fields = streams.reduce( - (found, stream) => scanInstrumentationFields(stream, found), - {}, - ); - const output = streams.join('\n'); - const text = redactDiagnosticText(output.slice(-OUTPUT_SCAN_MAX_CHARS)); - return { - exitCode: run.exitCode ?? null, - ...(run.signal ? { signal: run.signal } : {}), - ...fields, - outputTail: text.slice(-REDACTED_STRING_MAX_LENGTH), - outputTruncated: - text.length > REDACTED_STRING_MAX_LENGTH || output.length > OUTPUT_SCAN_MAX_CHARS, - }; -} - -type InstrumentationFields = Partial< - Pick< - AndroidInstrumentationExitResult, - 'resultCode' | 'aborted' | 'failed' | 'shortMsg' | 'longMsg' - > ->; - -/** - * Reads the run's own end from one stream. AOSP prints a bundle value as it is, so a value with - * newlines spans lines: the lines after `shortMsg=` or `longMsg=` belong to it until the next - * protocol record. Fields already found in an earlier stream win. - */ -function scanInstrumentationFields( - output: string, - found: InstrumentationFields, -): InstrumentationFields { - const fields: InstrumentationFields = { ...found }; - let open: MessageField | undefined; - for (const line of output.split(/\r?\n/)) { - const failed = FAILED_RE.exec(line); - if (failed) { - fields.failed ??= (failed[1] ?? '').trim(); - open = undefined; - } else if (PROTOCOL_RECORD_RE.test(line)) { - open = readProtocolRecord(line, fields); - } else if (open) { - fields[open] = appendLine(fields[open], line); - } - } - for (const key of ['shortMsg', 'longMsg'] as const) { - if (fields[key] !== undefined) fields[key] = fields[key].trimEnd(); - } - return fields; -} - -type MessageField = 'shortMsg' | 'longMsg'; - -/** Records what a protocol line states; answers the message field whose value it opens. */ -function readProtocolRecord(line: string, fields: InstrumentationFields): MessageField | undefined { - if (line.startsWith(RESULT_PREFIX)) { - const record = line.slice(RESULT_PREFIX.length); - const separator = record.indexOf('='); - const key = record.slice(0, separator); - if ((key !== 'shortMsg' && key !== 'longMsg') || fields[key] !== undefined) return undefined; - fields[key] = record.slice(separator + 1, separator + 1 + MESSAGE_FIELD_MAX_CHARS); - return key; - } - if (line.startsWith(CODE_PREFIX)) { - const code = line.slice(CODE_PREFIX.length).trim(); - if (/^-?\d+$/.test(code) && Number.isSafeInteger(Number(code))) { - fields.resultCode ??= Number(code); - } - return undefined; - } - const aborted = ABORTED_RE.exec(line); - if (aborted) fields.aborted ??= (aborted[1] ?? '').trim(); - return undefined; -} - -function appendLine(value: string | undefined, line: string): string | undefined { - const next = `${value ?? ''}\n${line}`; - return next.length > MESSAGE_FIELD_MAX_CHARS ? value : next; -} - -/** The exit of a spawned helper process and what it printed, kept to a bounded recent window. */ -export type AndroidInstrumentationProcessRecord = { - /** Settles when the process's output streams have closed, the point its output is complete. */ - closed: Promise; - /** The process's exit result once its streams have closed, or `undefined` before that. */ - exitResult(): AndroidInstrumentationExitResult | undefined; -}; - -export function recordInstrumentationProcess( - child: AndroidAdbProcess, -): AndroidInstrumentationProcessRecord { - // Snapshot content is removed per line as it completes, so the bounded window never holds a - // line whose opening tag was cut away. A line too long to hold is dropped as snapshot content. - let kept = ''; - let pending = ''; - let pendingDropped = false; - const keep = (line: string) => { - kept = `${kept}${withoutSnapshotContent(line)}\n`.slice(-OUTPUT_SCAN_MAX_CHARS); - }; - const onData = (chunk: Buffer | string) => { - const lines = (pending + chunk.toString()).split('\n'); - pending = lines.pop() ?? ''; - for (const line of lines) { - keep(pendingDropped ? XML_OMITTED : line); - pendingDropped = false; - } - if (pending.length > OUTPUT_SCAN_MAX_CHARS) { - pending = ''; - pendingDropped = true; - } - }; - child.stdout?.on('data', onData); - child.stderr?.on('data', onData); - let isClosed = false; - const closed = new Promise((resolve) => { - child.once('close', () => { - isClosed = true; - resolve(); - }); - }); - return { - closed, - exitResult: () => - isClosed - ? readInstrumentationExitResult({ - stdout: pendingDropped ? `${kept}${XML_OMITTED}` : `${kept}${pending}`, - stderr: '', - exitCode: child.exitCode, - signal: child.signalCode, - }) - : undefined, - }; -} diff --git a/packages/platform-android/src/instrumentation-helper.ts b/packages/platform-android/src/instrumentation-helper.ts index daaf07a5df..566b9be1e6 100644 --- a/packages/platform-android/src/instrumentation-helper.ts +++ b/packages/platform-android/src/instrumentation-helper.ts @@ -1,3 +1,5 @@ +import { REDACTED_STRING_MAX_LENGTH, redactDiagnosticText } from '@agent-device/kernel/redaction'; +import type { AndroidAdbProcess } from './adb-executor.ts'; // Shared primitives for the Android instrumentation helpers (snapshot + multi-touch). // Both helpers drive `am instrument -w` and parse the resulting // INSTRUMENTATION_STATUS / INSTRUMENTATION_RESULT key/value records, and both @@ -115,6 +117,246 @@ export function readInstrumentationResultBoolean(value: string | undefined): boo return undefined; } +/** + * What an `am instrument` run that returned no usable report says about its own end. Every field is + * a value the instrumentation or the host process printed or exited with; an absent field was not + * printed. + */ +export type AndroidInstrumentationExitResult = { + /** Host process exit status; `null` when a signal ended it. */ + exitCode: number | null; + signal?: string; + /** `INSTRUMENTATION_CODE`. */ + resultCode?: number; + /** `INSTRUMENTATION_ABORTED`: the system ended the run before it reported. */ + aborted?: string; + /** `INSTRUMENTATION_FAILED`: the run never started. */ + failed?: string; + shortMsg?: string; + longMsg?: string; + /** + * The end of the output after snapshot content is removed and secrets are redacted, held to the + * length a redacted diagnostic string keeps so the cause at the end is what survives the + * boundary. + */ + outputTail: string; + /** Output was cut, or snapshot content was removed from it. */ + outputTruncated: boolean; + /** Lines holding snapshot content were removed, so the tail is a filtered view of the output. */ + snapshotContentRemoved?: true; +}; + +const OUTPUT_SCAN_MAX_CHARS = 16_384; +const MESSAGE_FIELD_MAX_CHARS = 2_048; +const PAYLOAD_OMITTED = 'payloadBase64='; +const XML_OMITTED = ''; +const PAYLOAD_BASE64_RE = /payloadBase64=\S*/g; +const XML_START_RE = /<(?:\?xml|hierarchy|node)\b.*/g; +// A UiAutomator hierarchy dump prints ``; +const SECRET_BASE64 = Buffer.from(SECRET_XML).toString('base64'); + +const emulator: DeviceInfo = { + platform: 'android', + id: 'emulator-5556', + name: 'Pixel', + kind: 'emulator', + booted: true, }; async function captureError(run: FailedRun): Promise { @@ -80,23 +86,105 @@ async function captureError(run: FailedRun): Promise { } async function captureExitResult(run: FailedRun): Promise { - const error = await captureError(run); - return error.details?.instrumentation as ExitResultFields; + return (await captureError(run)).details?.instrumentation as ExitResultFields; } function encodedPayload(xml: string): string { return `INSTRUMENTATION_STATUS: payloadBase64=${Buffer.from(xml).toString('base64')}`; } -const SECRET = 'hunter2-account-number'; -const SECRET_XML = ``; +function helperFailingWith(run: FailedRun): AndroidAdbExecutor { + return async (args) => { + if (args.includes('--show-versioncode')) { + return { + exitCode: 0, + stdout: 'package:com.callstack.agentdevice.snapshothelper versionCode:13004', + stderr: '', + }; + } + if (isAndroidHelperRuntimeProbe(args)) return androidHelperRuntimeProbeResult(); + if (isAndroidHelperRuntimeForceStop(args)) return { exitCode: 0, stdout: '', stderr: '' }; + return { stderr: '', ...run }; + }; +} + +async function withLog( + run: (logPath: string) => Promise, +): Promise<{ result: T; log: string }> { + const tmpDir = await mkdtempForTest('agent-device-android-exit-result-'); + const logPath = path.join(tmpDir, 'diag.ndjson'); + try { + const result = await run(logPath); + return { result, log: await fs.readFile(logPath, 'utf8') }; + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } +} + +function inScope(logPath: string, body: () => Promise): Promise { + return withDiagnosticsScope( + { debug: true, logPath, session: 'android-test', requestId: 'req-1', command: 'snapshot' }, + body, + ); +} + +function diagnosticData(log: string, phase: string): Record { + const event = log + .split('\n') + .filter(Boolean) + .map((line) => JSON.parse(line) as { phase?: string; data?: Record }) + .find((entry) => entry.phase === phase); + assert.ok(event, `expected a ${phase} diagnostic`); + return event.data ?? {}; +} -test('an aborted instrumentation run reports the abort and the exit status', async () => { +function exitResultOf(fields: Record): ExitResultFields { + return fields.instrumentation as ExitResultFields; +} + +/** Runs a ready session whose helper process prints through `print` and then exits with 1. */ +async function sessionExit( + print: (process: FakeAndroidProcess) => Promise, +): Promise<{ exit: ExitResultFields; log: string }> { + const { result, log } = await withLog(async (logPath) => { + const processes: FakeAndroidProcess[] = []; + const provider = createSessionProvider({ calls: [], processes }); + const capture = () => + captureAndroidSnapshotWithHelperSession({ + adb: provider.exec, + adbProvider: provider, + deviceKey: 'android:emulator-5556', + }); + await inScope(logPath, async () => { + assert.ok(await capture()); + await print(processes[0]!); + await pause(); + processes[0]!.emitExit(1, null); + await capture(); + }); + return undefined; + }); + void result; + return { + exit: exitResultOf(diagnosticData(log, 'android_snapshot_helper_session_exited')), + log, + }; +} + +const pause = () => new Promise((resolve) => setTimeout(resolve, 10)); + +function assertPrivate(text: string): void { + assert.ok(!text.includes(SECRET), text.slice(-800)); + assert.ok(!text.includes(SECRET_BASE64.slice(0, 24)), text.slice(-800)); +} + +test('an aborted instrumentation run reports the abort, the status code and the exit status', async () => { assert.deepEqual(await captureExitResult({ exitCode: 1, stdout: ABORTED_RUN }), { exitCode: 1, + statusCode: 1, aborted: 'System has crashed.', - outputTail: ABORTED_RUN.trimEnd(), - outputTruncated: false, + stderrTail: '', + stderrTruncated: false, }); }); @@ -106,28 +194,34 @@ test('a crashed instrumentation run reports shortMsg, longMsg and the result cod resultCode: 0, shortMsg: 'java.lang.IllegalStateException', longMsg: 'java.lang.IllegalStateException: UiAutomation not connected', - outputTail: CRASHED_RUN.trimEnd(), - outputTruncated: false, + stderrTail: '', + stderrTruncated: false, }); }); test('the host signal that ended the run reaches the details', async () => { - const result = await captureExitResult({ exitCode: 1, stdout: '', signal: 'SIGKILL' }); - - assert.equal(result.signal, 'SIGKILL'); + assert.equal( + (await captureExitResult({ exitCode: 1, stdout: '', signal: 'SIGKILL' })).signal, + 'SIGKILL', + ); }); -test('an empty INSTRUMENTATION_CODE is not recorded as a zero result', async () => { - const result = await captureExitResult({ +test('an empty or out-of-range INSTRUMENTATION_CODE is not recorded as a result code', async () => { + const empty = await captureExitResult({ exitCode: 1, stdout: 'INSTRUMENTATION_RESULT: shortMsg=Process crashed.\nINSTRUMENTATION_CODE: \n', }); + assert.equal(empty.resultCode, undefined); + assert.equal(empty.shortMsg, 'Process crashed.'); - assert.equal(result.resultCode, undefined); - assert.equal(result.shortMsg, 'Process crashed.'); + const huge = await captureExitResult({ + exitCode: 1, + stdout: `INSTRUMENTATION_CODE: ${'9'.repeat(400)}\n`, + }); + assert.equal(huge.resultCode, undefined); }); -test('multiline result values keep their continuation lines', async () => { +test('multiline protocol values keep their continuation lines until the next record', async () => { const crlf = await captureExitResult({ exitCode: 0, stdout: [ @@ -145,20 +239,23 @@ test('multiline result values keep their continuation lines', async () => { const stack = await captureExitResult({ exitCode: 0, stdout: [ - 'INSTRUMENTATION_RESULT: longMsg=java.lang.IllegalStateException: no root', + 'INSTRUMENTATION_STATUS: Error=java.lang.IllegalStateException: no root', + 'INSTRUMENTATION_STATUS: stack=java.lang.IllegalStateException: no root', '\tat android.app.UiAutomation.connect(UiAutomation.java:12)', '\tat com.callstack.agentdevice.snapshothelper.Main.(Main.java:7)', - 'INSTRUMENTATION_CODE: 0', + 'INSTRUMENTATION_STATUS_CODE: -2', ].join('\n'), }); + assert.equal(stack.error, 'java.lang.IllegalStateException: no root'); assert.equal( - stack.longMsg, + stack.stack, [ 'java.lang.IllegalStateException: no root', '\tat android.app.UiAutomation.connect(UiAutomation.java:12)', '\tat com.callstack.agentdevice.snapshothelper.Main.(Main.java:7)', ].join('\n'), ); + assert.equal(stack.statusCode, -2); const interleaved = await captureExitResult({ exitCode: 0, @@ -166,14 +263,22 @@ test('multiline result values keep their continuation lines', async () => { 'INSTRUMENTATION_RESULT: longMsg=first line', 'second line', encodedPayload(SECRET_XML), - 'INSTRUMENTATION_STATUS_CODE: 1', 'not part of the message', - 'INSTRUMENTATION_CODE: 0', ].join('\n'), }); assert.equal(interleaved.longMsg, 'first line\nsecond line'); }); +test('single-line and multiline messages are held to the message cap', async () => { + const result = await captureExitResult({ + exitCode: 0, + stdout: `INSTRUMENTATION_RESULT: shortMsg=${'s'.repeat(10_000)}\nINSTRUMENTATION_RESULT: longMsg=${'l'.repeat(2_000)}\n${'m'.repeat(2_000)}\nINSTRUMENTATION_CODE: 0`, + }); + + assert.equal(result.shortMsg?.length, 2_048); + assert.ok((result.longMsg?.length ?? 0) <= 2_048); +}); + test('startup failures are recognized in every envelope AOSP prints', async () => { const forms = [ ['INSTRUMENTATION_FAILED: com.example/.Missing', 'com.example/.Missing'], @@ -188,312 +293,186 @@ test('startup failures are recognized in every envelope AOSP prints', async () = ['INSTRUMENTATION_FAILED:', ''], ] as const; for (const [line, expected] of forms) { - const result = await captureExitResult({ exitCode: 1, stdout: `${line}\n` }); - assert.equal(result.failed, expected, line); + for (const stream of ['stdout', 'stderr'] as const) { + const run = + stream === 'stdout' ? { stdout: `${line}\n` } : { stdout: '', stderr: `${line}\n` }; + const result = await captureExitResult({ exitCode: 1, ...run }); + assert.equal(result.failed, expected, `${stream}: ${line}`); + } } }); -test('a truncated XML exit keeps its exit status and drops the snapshot payload', async () => { - const details = ( - await captureError({ - exitCode: 1, - stdout: `${'x'.repeat(20_000)}\n${TRUNCATED_XML_RUN}`, - stderr: 'am: Killed', - }) - ).details as { instrumentation: ExitResultFields }; - - const instrumentation = details.instrumentation; - assert.equal(instrumentation.exitCode, 1); - assert.equal(instrumentation.outputTruncated, true); - assert.ok(instrumentation.outputTail.length <= 400); - assert.ok(instrumentation.outputTail.endsWith('am: Killed')); - assert.ok(instrumentation.outputTail.includes('payloadBase64=')); +test('stdout is never copied: only parsed protocol fields reach the details', async () => { + const error = await captureError({ + exitCode: 1, + stdout: `${CRASHED_RUN}some unrelated stdout line\n`, + }); + + assert.equal('stdout' in (error.details ?? {}), false); + assert.ok(!JSON.stringify(error.details).includes('some unrelated stdout line')); }); -test('encoded and raw snapshot content never reaches the error details', async () => { - const runs = [ +test('a sensitive value never reaches the details wherever the helper printed it', async () => { + const runs: FailedRun[] = [ + // A complete encoded payload. + { exitCode: 1, stdout: `${encodedPayload(SECRET_XML)}\nINSTRUMENTATION_STATUS_CODE: 1` }, + // An unknown status key carrying raw XML. + { exitCode: 1, stdout: `INSTRUMENTATION_STATUS: stream=${SECRET_XML}\n` }, + // A protocol record holding an attribute with no opening tag. { exitCode: 1, - stdout: [ - 'INSTRUMENTATION_STATUS: agentDeviceProtocol=android-snapshot-helper-v1', - encodedPayload(SECRET_XML), - 'INSTRUMENTATION_STATUS_CODE: 1', - ].join('\n'), - stderr: 'am: Killed', + stdout: `INSTRUMENTATION_STATUS: detail=ret" text="${SECRET}" bounds="[0,0][1,1]" />\n`, }, + // A tagless fragment as a free line, and as a message continuation. + { exitCode: 1, stdout: `ret" text="${SECRET}" bounds="[0,0][1,1]" />\n` }, { exitCode: 1, - stdout: `INSTRUMENTATION_STATUS: stream=${SECRET_XML}\nINSTRUMENTATION_STATUS_CODE: 1`, - stderr: '', + stdout: `INSTRUMENTATION_RESULT: longMsg=boom\nret" text="${SECRET}" bounds="[0,0][1,1]" />\n`, }, - // An incomplete raw fragment, cut inside an attribute. - { exitCode: 1, stdout: `\n`, + }, + // A fragment on stderr, raw and as a one-line report with a late secret. + { exitCode: 1, stdout: '', stderr: `ret" text="${SECRET}" bounds="[0,0][1,1]" />` }, + { exitCode: 1, stdout: '', stderr: `${'a'.repeat(30_000)}${SECRET_XML}` }, ]; for (const run of runs) { - const error = await captureError(run); + const error = await captureError({ ...run, stderr: `${run.stderr ?? ''}\nam: Killed` }); const wire = JSON.stringify(normalizeError(error)); - assert.ok(!wire.includes(SECRET), wire); - assert.ok(!wire.includes(Buffer.from(SECRET).toString('base64')), wire); - assert.ok(!wire.includes(Buffer.from(SECRET_XML).toString('base64').slice(0, 24)), wire); - assert.ok(wire.includes('am: Killed') || run.stderr === '', wire); - } -}); + assertPrivate(wire); + assert.ok(wire.includes('am: Killed'), wire.slice(-400)); + assert.equal(exitResultOf(normalizeError(error).details ?? {}).snapshotContentRemoved, true); -test('a large one-line XML report never leaves a sensitive value in the details or the request log', async () => { - const nodes = Array.from( - { length: 400 }, - (_, index) => ``, - ).join(''); - const lateSecret = ``; - const stdouts = [ - `${nodes}${lateSecret}`, - // Cut so that the kept end of the report has no opening tag left. - `${'a'.repeat(30_000)} text="${SECRET}" content-desc="account" bounds="[0,0][1,1]" />`, - `INSTRUMENTATION_STATUS: stream=${nodes}${lateSecret}`, - ]; - for (const stdout of stdouts) { - const error = await captureError({ exitCode: 1, stdout, stderr: 'am: Killed' }); - const wire = JSON.stringify(normalizeError(error)); - assert.ok(!wire.includes(SECRET), wire.slice(-600)); - assert.ok(wire.includes('am: Killed')); - - const tmpDir = await mkdtempForTest('agent-device-android-capture-xml-diag-'); - const logPath = path.join(tmpDir, 'diag.ndjson'); - try { - await withDiagnosticsScope( - { debug: true, logPath, session: 'android-test', requestId: 'req-7', command: 'snapshot' }, - async () => { - await assert.rejects( - snapshotAndroid(emulator, { - helperAdb: helperFailingWith({ exitCode: 1, stdout, stderr: 'am: Killed' }), - helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, - }), - ); - }, - ); - const log = await fs.readFile(logPath, 'utf8'); - assert.ok(log.includes('android_snapshot_helper_failed')); - assert.ok(!log.includes(SECRET), log.slice(-600)); - } finally { - await fs.rm(tmpDir, { recursive: true, force: true }); - } - } -}); - -test('ordinary quoted assignments are kept while snapshot attributes are dropped', async () => { - const error = await captureError({ - exitCode: 1, - stdout: [ - 'E AndroidRuntime: java.lang.IllegalStateException message="binder died" code="7"', - `ret-value" text="${SECRET}" bounds="[0,0][1,1]" />`, - ].join('\n'), - stderr: 'am: Killed', - }); - - const result = error.details?.instrumentation as ExitResultFields & { - snapshotContentRemoved?: true; - }; - assert.ok(result.outputTail.includes('message="binder died" code="7"'), result.outputTail); - assert.ok(!result.outputTail.includes(SECRET)); - assert.equal(result.snapshotContentRemoved, true); - assert.equal(result.outputTruncated, true); -}); - -test('a long diagnostic line keeps its end and a long snapshot line is dropped', async () => { - const diagnostic = `${'frame '.repeat(5_000)}Caused by: binder has died`; - const kept = (await captureError({ exitCode: 1, stdout: diagnostic })).details - ?.instrumentation as ExitResultFields & { snapshotContentRemoved?: true }; - assert.ok(kept.outputTail.endsWith('Caused by: binder has died'), kept.outputTail); - assert.equal(kept.outputTruncated, true); - assert.equal(kept.snapshotContentRemoved, undefined); - - const snapshot = `${``.repeat(2_000)}`; - const dropped = (await captureError({ exitCode: 1, stdout: snapshot, stderr: 'am: Killed' })) - .details?.instrumentation as ExitResultFields & { snapshotContentRemoved?: true }; - assert.ok(!dropped.outputTail.includes(SECRET)); - assert.ok(dropped.outputTail.endsWith('am: Killed')); - assert.equal(dropped.snapshotContentRemoved, true); - assert.equal(dropped.outputTruncated, true); -}); - -test('a session exit result keeps a long diagnostic line and drops a long snapshot line', async () => { - const tmpDir = await mkdtempForTest('agent-device-android-session-long-line-'); - const logPath = path.join(tmpDir, 'diag.ndjson'); - try { - const processes: FakeAndroidProcess[] = []; - const provider = createSessionProvider({ calls: [], processes }); - const capture = () => - captureAndroidSnapshotWithHelperSession({ - adb: provider.exec, - adbProvider: provider, - deviceKey: 'android:emulator-5556', - }); - await withDiagnosticsScope( - { debug: true, logPath, session: 'android-test', requestId: 'req-9', command: 'snapshot' }, - async () => { - assert.ok(await capture()); - // The line arrives in pieces, so it outgrows the window before it completes. - processes[0]!.stdout.write('frame '.repeat(4_000)); - await new Promise((resolve) => setTimeout(resolve, 5)); - processes[0]!.stdout.write('frame '.repeat(1_000) + 'Caused by: binder has died\n'); - processes[0]!.stdout.write( - `${``.repeat(2_000)}\n`, + const { log } = await withLog(async (logPath) => { + await inScope(logPath, async () => { + await assert.rejects( + snapshotAndroid(emulator, { + helperAdb: helperFailingWith({ ...run, stderr: `${run.stderr ?? ''}\nam: Killed` }), + helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, + }), ); - processes[0]!.stdout.write('INSTRUMENTATION_ABORTED: System has crashed.\n'); - await new Promise((resolve) => setTimeout(resolve, 20)); - processes[0]!.emitExit(1, null); - await capture(); - }, - ); - - const log = await fs.readFile(logPath, 'utf8'); - assert.ok(!log.includes(SECRET)); - const exit = readExitResult( - (await readDiagnostic(logPath, 'android_snapshot_helper_session_exited')).data ?? {}, - ) as Partial & { snapshotContentRemoved?: true }; - assert.equal(exit.aborted, 'System has crashed.'); - assert.equal(exit.snapshotContentRemoved, true); - assert.equal(exit.outputTruncated, true); - } finally { - await fs.rm(tmpDir, { recursive: true, force: true }); + }); + }); + assert.ok(log.includes('android_snapshot_helper_failed')); + assertPrivate(log); } }); -test('a session exit result keeps the end of a long diagnostic line', async () => { - const tmpDir = await mkdtempForTest('agent-device-android-session-long-diag-'); - const logPath = path.join(tmpDir, 'diag.ndjson'); - try { - const processes: FakeAndroidProcess[] = []; - const provider = createSessionProvider({ calls: [], processes }); - const capture = () => - captureAndroidSnapshotWithHelperSession({ - adb: provider.exec, - adbProvider: provider, - deviceKey: 'android:emulator-5556', - }); - await withDiagnosticsScope( - { debug: true, logPath, session: 'android-test', requestId: 'req-10', command: 'snapshot' }, - async () => { - assert.ok(await capture()); - // The line arrives in pieces, so it outgrows the window before it completes. - processes[0]!.stdout.write('frame '.repeat(4_000)); - await new Promise((resolve) => setTimeout(resolve, 5)); - processes[0]!.stdout.write('frame '.repeat(1_000) + 'Caused by: binder has died\n'); - await new Promise((resolve) => setTimeout(resolve, 20)); - processes[0]!.emitExit(1, null); - await capture(); - }, - ); +test('a session exit never leaks a sensitive value split across chunks', async () => { + const { exit, log } = await sessionExit(async (process) => { + // The payload and the sensitive key arrive in pieces, with stderr interleaved. + process.stdout.write(`INSTRUMENTATION_STATUS: payloadBase64=${SECRET_BASE64.slice(0, 20)}`); + process.stderr.write('pass'); + await pause(); + process.stdout.write(`${SECRET_BASE64.slice(20)}\nINSTRUMENTATION_STATUS_CODE: 1\n`); + process.stderr.write(`word=${SECRET}\n`); + process.stderr.write(`ret" text="${SECRET}"`); + process.stderr.write(' bounds="[0,0][1,1]" />\nam: Killed\n'); + process.stdout.write('INSTRUMENTATION_ABORTED: System has crashed.\n'); + }); - const exit = readExitResult( - (await readDiagnostic(logPath, 'android_snapshot_helper_session_exited')).data ?? {}, - ); - assert.ok(exit.outputTail?.endsWith('Caused by: binder has died'), exit.outputTail); - assert.equal(exit.outputTruncated, true); - } finally { - await fs.rm(tmpDir, { recursive: true, force: true }); - } + assertPrivate(log); + assert.equal(exit.aborted, 'System has crashed.'); + assert.equal(exit.snapshotContentRemoved, true); + assert.ok(exit.stderrTail.endsWith('am: Killed'), exit.stderrTail); }); -test('a single-line shortMsg or longMsg is held to the message cap', async () => { - const result = await captureExitResult({ - exitCode: 0, - stdout: `INSTRUMENTATION_RESULT: shortMsg=${'s'.repeat(10_000)}\nINSTRUMENTATION_RESULT: longMsg=${'l'.repeat(10_000)}\nINSTRUMENTATION_CODE: 0`, +test('stderr chunks do not disturb a partial stdout record', async () => { + const { exit } = await sessionExit(async (process) => { + process.stdout.write('INSTRUMENTATION_RESULT: longMsg=UiAutomation fai'); + process.stderr.write('am: warning\n'); + await pause(); + process.stdout.write('led\nCaused by: binder has died\nINSTRUMENTATION_CODE: 0\n'); }); - assert.equal(result.shortMsg?.length, 2_048); - assert.equal(result.longMsg?.length, 2_048); + assert.equal(exit.longMsg, 'UiAutomation failed\nCaused by: binder has died'); + assert.equal(exit.resultCode, 0); + assert.equal(exit.stderrTail, 'am: warning'); }); -test('an INSTRUMENTATION_CODE beyond the safe integer range is not recorded', async () => { +test('ordinary quoted assignments on stderr are kept while snapshot attributes are dropped', async () => { const result = await captureExitResult({ exitCode: 1, - stdout: `INSTRUMENTATION_CODE: ${'9'.repeat(400)}\n`, + stdout: '', + stderr: [ + 'E AndroidRuntime: java.lang.IllegalStateException message="binder died" code="7"', + `ret" text="${SECRET}" bounds="[0,0][1,1]" />`, + ].join('\n'), }); - assert.equal(result.resultCode, undefined); -}); - -test('a session exit result includes output that arrives after the exit event', async () => { - const tmpDir = await mkdtempForTest('agent-device-android-session-drain-'); - const logPath = path.join(tmpDir, 'diag.ndjson'); - try { - const processes: FakeAndroidProcess[] = []; - const provider = createSessionProvider({ calls: [], processes }); - const capture = () => - captureAndroidSnapshotWithHelperSession({ - adb: provider.exec, - adbProvider: provider, - deviceKey: 'android:emulator-5556', - }); - await withDiagnosticsScope( - { debug: true, logPath, session: 'android-test', requestId: 'req-8', command: 'snapshot' }, - async () => { - assert.ok(await capture()); - const process = processes[0]!; - process.exitCode = 1; - process.emit('exit', 1, null); - const second = capture(); - await new Promise((resolve) => setTimeout(resolve, 20)); - process.stdout.write(CRASHED_RUN); - await new Promise((resolve) => setTimeout(resolve, 20)); - process.emit('close', 1, null); - await second; - }, - ); - - const event = await readDiagnostic(logPath, 'android_snapshot_helper_session_exited'); - const exit = readExitResult(event.data ?? {}); - assert.equal(exit.exitCode, 1); - assert.equal(exit.shortMsg, 'java.lang.IllegalStateException'); - } finally { - await fs.rm(tmpDir, { recursive: true, force: true }); - } + assert.ok(result.stderrTail.includes('message="binder died" code="7"'), result.stderrTail); + assert.ok(!result.stderrTail.includes(SECRET)); + assert.equal(result.snapshotContentRemoved, true); + assert.equal(result.stderrTruncated, true); }); test('the real cause past the redaction cap survives in the error and the request log', async () => { const filler = Array.from({ length: 40 }, (_, index) => `line ${index} of helper output`).join( '\n', ); - const stdout = `${filler}\nam: Killed by the low memory killer`; - const error = await captureError({ exitCode: 1, stdout }); + const stderr = `${filler}\nam: Killed by the low memory killer`; + const error = await captureError({ exitCode: 1, stdout: '', stderr }); + + const wire = exitResultOf(normalizeError(error).details ?? {}); + assert.ok(wire.stderrTail.endsWith('am: Killed by the low memory killer'), wire.stderrTail); + assert.ok(wire.stderrTail.length <= 400); + assert.equal(wire.stderrTruncated, true); + + const short = await captureError({ exitCode: 1, stdout: '', stderr: `${'y'.repeat(500)} done` }); + const shortWire = exitResultOf(normalizeError(short).details ?? {}); + assert.equal(shortWire.stderrTruncated, true); + assert.ok(shortWire.stderrTail.endsWith('done')); + + const { log } = await withLog(async (logPath) => { + await inScope(logPath, async () => { + await assert.rejects( + snapshotAndroid(emulator, { + helperAdb: helperFailingWith({ exitCode: 1, stdout: '', stderr }), + helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, + }), + ); + }); + }); + const logged = exitResultOf(diagnosticData(log, 'android_snapshot_helper_failed')); + assert.ok(logged.stderrTail.endsWith('am: Killed by the low memory killer'), logged.stderrTail); + assert.equal(logged.stderrTruncated, true); +}); - const wire = normalizeError(error).details?.instrumentation as ExitResultFields; - assert.ok(wire.outputTail.endsWith('am: Killed by the low memory killer'), wire.outputTail); - assert.ok(wire.outputTail.length <= 400); - assert.equal(wire.outputTruncated, true); +test('a long stderr diagnostic line keeps its end and a long snapshot line is dropped', async () => { + const diagnostic = `${'frame '.repeat(5_000)}Caused by: binder has died`; + const kept = await captureExitResult({ exitCode: 1, stdout: '', stderr: diagnostic }); + assert.ok(kept.stderrTail.endsWith('Caused by: binder has died'), kept.stderrTail); + assert.equal(kept.stderrTruncated, true); + assert.equal(kept.snapshotContentRemoved, undefined); - const short = await captureError({ exitCode: 1, stdout: `${'y'.repeat(500)} done` }); - const shortWire = normalizeError(short).details?.instrumentation as ExitResultFields; - assert.equal(shortWire.outputTruncated, true); - assert.ok(shortWire.outputTail.endsWith('done')); + const snapshot = `${``.repeat(2_000)}`; + const dropped = await captureExitResult({ + exitCode: 1, + stdout: '', + stderr: `${snapshot}\nam: Killed`, + }); + assert.ok(!dropped.stderrTail.includes(SECRET)); + assert.ok(dropped.stderrTail.endsWith('am: Killed')); + assert.equal(dropped.snapshotContentRemoved, true); + assert.equal(dropped.stderrTruncated, true); +}); - const tmpDir = await mkdtempForTest('agent-device-android-exit-tail-'); - const logPath = path.join(tmpDir, 'diag.ndjson'); - try { - await withDiagnosticsScope( - { debug: true, logPath, session: 'android-test', requestId: 'req-3', command: 'snapshot' }, - async () => { - const helperAdb = helperFailingWith({ exitCode: 1, stdout, stderr: '' }); - await assert.rejects( - snapshotAndroid(emulator, { - helperAdb, - helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, - }), - ); - }, - ); - const logged = readExitResult( - (await readDiagnostic(logPath, 'android_snapshot_helper_failed')).data ?? {}, - ); - assert.ok( - logged.outputTail?.endsWith('am: Killed by the low memory killer'), - logged.outputTail, - ); - assert.equal(logged.outputTruncated, true); - } finally { - await fs.rm(tmpDir, { recursive: true, force: true }); - } +test('a session exit keeps a long diagnostic line and flags a chunked snapshot line it dropped', async () => { + const { exit, log } = await sessionExit(async (process) => { + // Lines arrive in pieces and outgrow the window before they complete. + process.stderr.write('frame '.repeat(4_000)); + await pause(); + process.stderr.write(`${'frame '.repeat(1_000)}Caused by: binder has died\n`); + process.stderr.write(``); + await pause(); + process.stderr.write(`${``.repeat(1_000)}\n`); + }); + + assertPrivate(log); + assert.ok(exit.stderrTail.includes('Caused by: binder has died'), exit.stderrTail); + assert.equal(exit.snapshotContentRemoved, true); + assert.equal(exit.stderrTruncated, true); }); test('a helper failure that reports its own result keeps that report without a second record', async () => { @@ -510,10 +489,8 @@ test('a helper failure that reports its own result keeps that report without a s assert.equal(error.details?.instrumentation, undefined); }); -test('a session that exits before it is ready records its exit status and output tail', async () => { - const tmpDir = await mkdtempForTest('agent-device-android-session-exit-diag-'); - const logPath = path.join(tmpDir, 'diag.ndjson'); - try { +test('a session that exits before it is ready records its exit status and the output it printed', async () => { + const { log } = await withLog(async (logPath) => { const base = createSessionProvider({ calls: [], processes: [] }); const provider: AndroidAdbProvider = { ...base, @@ -521,39 +498,45 @@ test('a session that exits before it is ready records its exit status and output const process = new FakeAndroidProcess(); setImmediate(() => { process.stdout.write(ABORTED_RUN); + process.stderr.write('am: Killed\n'); process.emitExit(1, null); }); return process; }, }; - await withDiagnosticsScope( - { debug: true, logPath, session: 'android-test', requestId: 'req-1', command: 'snapshot' }, - async () => { - const output = await captureAndroidSnapshotWithHelperSession({ - adb: provider.exec, - adbProvider: provider, - deviceKey: 'android:emulator-5556', - }); - assert.equal(output, undefined); - }, - ); + await inScope(logPath, async () => { + const output = await captureAndroidSnapshotWithHelperSession({ + adb: provider.exec, + adbProvider: provider, + deviceKey: 'android:emulator-5556', + }); + assert.equal(output, undefined); + }); + }); - const event = await readDiagnostic(logPath, 'android_snapshot_helper_session_start_failed'); - assert.deepEqual(event.data?.instrumentation, { + assert.deepEqual( + exitResultOf(diagnosticData(log, 'android_snapshot_helper_session_start_failed')), + { exitCode: 1, + statusCode: 1, aborted: 'System has crashed.', - outputTail: ABORTED_RUN.trimEnd(), - outputTruncated: false, - }); - } finally { - await fs.rm(tmpDir, { recursive: true, force: true }); - } + stderrTail: 'am: Killed', + stderrTruncated: false, + }, + ); }); -test('a session that exits after it was ready records its exit result and the output it printed', async () => { - const tmpDir = await mkdtempForTest('agent-device-android-session-late-exit-'); - const logPath = path.join(tmpDir, 'diag.ndjson'); - try { +test('a session that exits after it was ready records its exit result', async () => { + const { exit } = await sessionExit(async (process) => { + process.stdout.write(CRASHED_RUN); + }); + + assert.equal(exit.exitCode, 1); + assert.equal(exit.shortMsg, 'java.lang.IllegalStateException'); +}); + +test('a session exit result includes output that arrives after the exit event', async () => { + const { log } = await withLog(async (logPath) => { const processes: FakeAndroidProcess[] = []; const provider = createSessionProvider({ calls: [], processes }); const capture = () => @@ -562,145 +545,57 @@ test('a session that exits after it was ready records its exit result and the ou adbProvider: provider, deviceKey: 'android:emulator-5556', }); - await withDiagnosticsScope( - { debug: true, logPath, session: 'android-test', requestId: 'req-4', command: 'snapshot' }, - async () => { - assert.ok(await capture()); - processes[0]!.stdout.write(CRASHED_RUN); - processes[0]!.emitExit(1, null); - await capture(); - }, - ); + await inScope(logPath, async () => { + assert.ok(await capture()); + const process = processes[0]!; + process.exitCode = 1; + process.emit('exit', 1, null); + const second = capture(); + await pause(); + process.stdout.write(CRASHED_RUN); + await pause(); + process.emit('close', 1, null); + await second; + }); + }); - const event = await readDiagnostic(logPath, 'android_snapshot_helper_session_exited'); - const exit = readExitResult(event.data ?? {}); - assert.equal(exit.exitCode, 1); - assert.equal(exit.shortMsg, 'java.lang.IllegalStateException'); - } finally { - await fs.rm(tmpDir, { recursive: true, force: true }); - } + const exit = exitResultOf(diagnosticData(log, 'android_snapshot_helper_session_exited')); + assert.equal(exit.exitCode, 1); + assert.equal(exit.shortMsg, 'java.lang.IllegalStateException'); }); test('a session that is stopped while it is still running records no invented cause', async () => { - const tmpDir = await mkdtempForTest('agent-device-android-session-live-stop-'); - const logPath = path.join(tmpDir, 'diag.ndjson'); - try { + const { log } = await withLog(async (logPath) => { const provider = createSessionProvider({ calls: [], processes: [] }); - await withDiagnosticsScope( - { debug: true, logPath, session: 'android-test', requestId: 'req-5', command: 'snapshot' }, - async () => { - await captureAndroidSnapshotWithHelperSession({ - adb: provider.exec, - adbProvider: provider, - deviceKey: 'android:emulator-5556', - }); - await resetAndroidSnapshotHelperSessions(); - }, - ); + await inScope(logPath, async () => { + await captureAndroidSnapshotWithHelperSession({ + adb: provider.exec, + adbProvider: provider, + deviceKey: 'android:emulator-5556', + }); + await resetAndroidSnapshotHelperSessions(); + }); + }); - const log = await fs.readFile(logPath, 'utf8'); - assert.ok(!log.includes('android_snapshot_helper_session_exited')); - } finally { - await fs.rm(tmpDir, { recursive: true, force: true }); - } + assert.ok(!log.includes('android_snapshot_helper_session_exited')); }); -const emulator: DeviceInfo = { - platform: 'android', - id: 'emulator-5556', - name: 'Pixel', - kind: 'emulator', - booted: true, -}; - -function helperFailingWith(run: FailedRun): AndroidAdbExecutor { - return async (args) => { - if (args.includes('--show-versioncode')) { - return { - exitCode: 0, - stdout: 'package:com.callstack.agentdevice.snapshothelper versionCode:13004', - stderr: '', - }; - } - if (isAndroidHelperRuntimeProbe(args)) return androidHelperRuntimeProbeResult(); - if (isAndroidHelperRuntimeForceStop(args)) return { exitCode: 0, stdout: '', stderr: '' }; - return { stderr: '', ...run }; - }; -} - test('the request log entry for a failed capture carries the instrumentation result', async () => { - const tmpDir = await mkdtempForTest('agent-device-android-capture-exit-diag-'); - const logPath = path.join(tmpDir, 'diag.ndjson'); - try { - const helperAdb = helperFailingWith({ exitCode: 1, stdout: ABORTED_RUN }); - await withDiagnosticsScope( - { debug: true, logPath, session: 'android-test', requestId: 'req-2', command: 'snapshot' }, - async () => { - await assert.rejects( - snapshotAndroid(emulator, { - helperAdb, - helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, - }), - (error) => { - const details = (error as { details: Record }).details; - assert.equal(readExitResult(details).aborted, 'System has crashed.'); - return true; - }, - ); - }, - ); - - const event = await readDiagnostic(logPath, 'android_snapshot_helper_failed'); - const logged = readExitResult(event.data ?? {}); - assert.equal(logged.exitCode, 1); - assert.equal(logged.aborted, 'System has crashed.'); - } finally { - await fs.rm(tmpDir, { recursive: true, force: true }); - } -}); - -test('encoded snapshot content never reaches the request log', async () => { - const tmpDir = await mkdtempForTest('agent-device-android-capture-payload-diag-'); - const logPath = path.join(tmpDir, 'diag.ndjson'); - try { - const helperAdb = helperFailingWith({ - exitCode: 1, - stdout: `${encodedPayload(SECRET_XML)}\nINSTRUMENTATION_STATUS_CODE: 1`, + const { result: caught, log } = await withLog(async (logPath) => { + return await inScope(logPath, async () => { + let thrown: unknown; + await snapshotAndroid(emulator, { + helperAdb: helperFailingWith({ exitCode: 1, stdout: ABORTED_RUN }), + helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, + }).catch((error: unknown) => { + thrown = error; + }); + return thrown as AppError; }); - await withDiagnosticsScope( - { debug: true, logPath, session: 'android-test', requestId: 'req-6', command: 'snapshot' }, - async () => { - await assert.rejects( - snapshotAndroid(emulator, { - helperAdb, - helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, - }), - ); - }, - ); - const log = await fs.readFile(logPath, 'utf8'); - assert.ok(log.includes('android_snapshot_helper_failed')); - assert.ok(!log.includes(Buffer.from(SECRET_XML).toString('base64').slice(0, 24)), log); - assert.ok(!log.includes(SECRET)); - } finally { - await fs.rm(tmpDir, { recursive: true, force: true }); - } -}); - -function readExitResult(fields: Record): Partial { - return (fields.instrumentation ?? {}) as Partial; -} + }); -async function readDiagnostic( - logPath: string, - phase: string, -): Promise<{ data?: Record }> { - const log = await fs.readFile(logPath, 'utf8'); - const event = log - .split('\n') - .filter(Boolean) - .map((line) => JSON.parse(line) as { phase?: string; data?: Record }) - .find((entry) => entry.phase === phase); - assert.ok(event, `expected a ${phase} diagnostic`); - return event; -} + assert.equal(exitResultOf(caught.details ?? {}).aborted, 'System has crashed.'); + const logged = exitResultOf(diagnosticData(log, 'android_snapshot_helper_failed')); + assert.equal(logged.exitCode, 1); + assert.equal(logged.aborted, 'System has crashed.'); +}); diff --git a/packages/platform-android/src/instrumentation-helper.ts b/packages/platform-android/src/instrumentation-helper.ts index 491e05c998..e1d171126d 100644 --- a/packages/platform-android/src/instrumentation-helper.ts +++ b/packages/platform-android/src/instrumentation-helper.ts @@ -118,9 +118,11 @@ export function readInstrumentationResultBoolean(value: string | undefined): boo } /** - * What an `am instrument` run that returned no usable report says about its own end. Every field is - * a value the instrumentation or the host process printed or exited with; an absent field was not - * printed. + * What an `am instrument` run that returned no usable report says about its own end. + * + * Private by construction: stdout contributes only the protocol fields parsed below and never a line + * of its own, so a snapshot payload or raw XML the helper printed has no way in. The only free text + * is a bounded stderr tail, from which lines holding snapshot content are removed. */ export type AndroidInstrumentationExitResult = { /** Host process exit status; `null` when a signal ended it. */ @@ -128,166 +130,200 @@ export type AndroidInstrumentationExitResult = { signal?: string; /** `INSTRUMENTATION_CODE`. */ resultCode?: number; + /** The last `INSTRUMENTATION_STATUS_CODE`. */ + statusCode?: number; /** `INSTRUMENTATION_ABORTED`: the system ended the run before it reported. */ aborted?: string; /** `INSTRUMENTATION_FAILED`: the run never started. */ failed?: string; shortMsg?: string; longMsg?: string; + error?: string; + stack?: string; /** - * The end of the output after snapshot content is removed and secrets are redacted, held to the - * length a redacted diagnostic string keeps so the cause at the end is what survives the - * boundary. + * The end of stderr after secrets are redacted, held to the length a redacted diagnostic string + * keeps so the cause at the end is what survives the boundary. */ - outputTail: string; - /** Output was cut, or snapshot content was removed from it. */ - outputTruncated: boolean; - /** Lines holding snapshot content were removed, so the tail is a filtered view of the output. */ + stderrTail: string; + /** Stderr was cut, or a line of it was removed as snapshot content. */ + stderrTruncated: boolean; + /** A line of either stream held snapshot content and was dropped. */ snapshotContentRemoved?: true; }; -const OUTPUT_SCAN_MAX_CHARS = 16_384; +const STDERR_WINDOW_MAX_CHARS = 16_384; +const STDOUT_LINE_MAX_CHARS = 4_096; +// A line is held to a few times the tail that survives the diagnostic boundary. +const STDERR_LINE_MAX_CHARS = 1_024; const MESSAGE_FIELD_MAX_CHARS = 2_048; -const PAYLOAD_OMITTED = 'payloadBase64='; -const XML_OMITTED = ''; -const PAYLOAD_BASE64_RE = /payloadBase64=\S*/g; -const XML_START_RE = /<(?:\?xml|hierarchy|node)\b.*/g; +const SNAPSHOT_OMITTED = ''; // A UiAutomator hierarchy dump prints `` and // `onError: ... message=INSTRUMENTATION_FAILED: ` carry the marker mid-line. const FAILED_RE = /INSTRUMENTATION_FAILED:[ \t]?(.*)$/; const ABORTED_RE = /^INSTRUMENTATION_ABORTED:[ \t]?(.*)$/; -const RESULT_PREFIX = 'INSTRUMENTATION_RESULT: '; -const CODE_PREFIX = 'INSTRUMENTATION_CODE: '; const PROTOCOL_RECORD_RE = /^INSTRUMENTATION_(?:STATUS|STATUS_CODE|RESULT|CODE|ABORTED|FAILED):/; +const VALUE_RECORD_RE = /^INSTRUMENTATION_(?:STATUS|RESULT): ([^=]*)=(.*)$/; +const CODE_RE = /^INSTRUMENTATION_(STATUS_CODE|CODE): (-?\d+)\s*$/; +// The protocol keys whose values are text, by the field they fill. +const MESSAGE_FIELDS = { shortMsg: 'shortMsg', longMsg: 'longMsg', Error: 'error', stack: 'stack' }; +type MessageField = 'shortMsg' | 'longMsg' | 'error' | 'stack'; -function holdsSnapshotContent(line: string): boolean { - return ( - XML_ELEMENT_RE.test(line) || XML_ATTRIBUTE_RE.test(line) || line.includes('payloadBase64=') - ); -} +type LineAssembler = { write(chunk: string): void; end(): void }; /** - * Removes the app content a helper run may have printed: encoded snapshot payloads and XML in any - * shape, including a fragment whose opening tag is missing. Instrumentation records and every other - * line stay. This runs on the whole output, before any tail is cut from it. + * Splits one stream into lines. A line longer than `maxChars` keeps its head or its tail, and is + * reported as snapshot content if any part of it that was held at some point looked like it. */ -function filterSnapshotContent(output: string): { text: string; removed: boolean } { - let removed = false; - const text = output - .split(/\r?\n/) - .map((line) => { - if (!holdsSnapshotContent(line)) return line; - removed = true; - if (PROTOCOL_RECORD_RE.test(line)) { - return line.replace(PAYLOAD_BASE64_RE, PAYLOAD_OMITTED).replace(XML_START_RE, XML_OMITTED); - } - return XML_ELEMENT_RE.test(line) || XML_ATTRIBUTE_RE.test(line) - ? XML_OMITTED - : line.replace(PAYLOAD_BASE64_RE, PAYLOAD_OMITTED); - }) - .join('\n'); - return { text, removed }; -} - -export function withoutSnapshotContent(output: string): string { - return filterSnapshotContent(output).text; -} - -export function readInstrumentationExitResult(run: { - stdout: string; - stderr: string; - exitCode: number | null | undefined; - signal?: string | null; - /** What a caller that already filtered or cut the output before handing it over has done to it. */ - prior?: { snapshotContentRemoved: boolean; cut: boolean }; -}): AndroidInstrumentationExitResult { - const filtered = [run.stdout, run.stderr].filter(Boolean).map(filterSnapshotContent); - const streams = filtered.map((stream) => stream.text); - const snapshotContentRemoved = - filtered.some((stream) => stream.removed) || run.prior?.snapshotContentRemoved === true; - const fields = streams.reduce( - (found, stream) => scanInstrumentationFields(stream, found), - {}, - ); - const output = streams.join('\n'); - const text = redactDiagnosticText(output.slice(-OUTPUT_SCAN_MAX_CHARS)); +function createLineAssembler( + keep: 'head' | 'tail', + maxChars: number, + onLine: (line: string, snapshot: boolean, cut: boolean) => void, +): LineAssembler { + let pending = ''; + let snapshot = false; + let cut = false; + const append = (text: string) => { + pending += text; + if (pending.length <= maxChars) return; + snapshot ||= SNAPSHOT_CONTENT_RE.test(pending); + cut = true; + pending = keep === 'head' ? pending.slice(0, maxChars) : startAtToken(pending.slice(-maxChars)); + }; + const flush = () => { + const line = pending.replace(/\r$/, ''); + onLine(line, snapshot || SNAPSHOT_CONTENT_RE.test(line), cut); + pending = ''; + snapshot = false; + cut = false; + }; return { - exitCode: run.exitCode ?? null, - ...(run.signal ? { signal: run.signal } : {}), - ...fields, - outputTail: text.slice(-REDACTED_STRING_MAX_LENGTH), - outputTruncated: - text.length > REDACTED_STRING_MAX_LENGTH || - output.length > OUTPUT_SCAN_MAX_CHARS || - snapshotContentRemoved || - run.prior?.cut === true, - ...(snapshotContentRemoved ? { snapshotContentRemoved: true as const } : {}), + write(chunk) { + let rest = chunk; + for (let newline = rest.indexOf('\n'); newline >= 0; newline = rest.indexOf('\n')) { + append(rest.slice(0, newline)); + flush(); + rest = rest.slice(newline + 1); + } + append(rest); + }, + end() { + if (pending) flush(); + }, }; } -type InstrumentationFields = Partial< - Pick< - AndroidInstrumentationExitResult, - 'resultCode' | 'aborted' | 'failed' | 'shortMsg' | 'longMsg' - > ->; +/** Drops the token a tail cut landed in, so a redaction key is never left without its name. */ +function startAtToken(text: string): string { + const boundary = /\s/.exec(text); + return boundary ? text.slice(boundary.index + 1) : ''; +} -/** - * Reads the run's own end from one stream. AOSP prints a bundle value as it is, so a value with - * newlines spans lines: the lines after `shortMsg=` or `longMsg=` belong to it until the next - * protocol record. Fields already found in an earlier stream win. - */ -function scanInstrumentationFields( - output: string, - found: InstrumentationFields, -): InstrumentationFields { - const fields: InstrumentationFields = { ...found }; +/** One run's output, read as two streams with a parser state each. */ +function createInstrumentationOutput() { + const fields: Partial< + Record & + Record<'resultCode' | 'statusCode', number> + > = {}; let open: MessageField | undefined; - for (const line of output.split(/\r?\n/)) { + let snapshotContentRemoved = false; + let stderrKept = ''; + let stderrCut = false; + const accept = (text: string, snapshot: boolean): string | undefined => { + snapshotContentRemoved ||= snapshot; + return snapshot ? undefined : text; + }; + const readFailed = (line: string) => { const failed = FAILED_RE.exec(line); - if (failed) { - fields.failed ??= (failed[1] ?? '').trim(); + if (failed) fields.failed ??= accept((failed[1] ?? '').trim(), SNAPSHOT_CONTENT_RE.test(line)); + return failed !== null; + }; + const readStdoutLine = (line: string, snapshot: boolean) => { + if (readFailed(line)) { open = undefined; - } else if (PROTOCOL_RECORD_RE.test(line)) { - open = readProtocolRecord(line, fields); - } else if (open) { - fields[open] = appendLine(fields[open], line); + return; } - } - for (const key of ['shortMsg', 'longMsg'] as const) { - if (fields[key] !== undefined) fields[key] = fields[key].trimEnd(); - } - return fields; -} - -type MessageField = 'shortMsg' | 'longMsg'; - -/** Records what a protocol line states; answers the message field whose value it opens. */ -function readProtocolRecord(line: string, fields: InstrumentationFields): MessageField | undefined { - if (line.startsWith(RESULT_PREFIX)) { - const record = line.slice(RESULT_PREFIX.length); - const separator = record.indexOf('='); - const key = record.slice(0, separator); - if ((key !== 'shortMsg' && key !== 'longMsg') || fields[key] !== undefined) return undefined; - fields[key] = record.slice(separator + 1, separator + 1 + MESSAGE_FIELD_MAX_CHARS); - return key; - } - if (line.startsWith(CODE_PREFIX)) { - const code = line.slice(CODE_PREFIX.length).trim(); - if (/^-?\d+$/.test(code) && Number.isSafeInteger(Number(code))) { - fields.resultCode ??= Number(code); + if (!PROTOCOL_RECORD_RE.test(line)) { + if (!open) snapshotContentRemoved ||= snapshot; + else if (accept(line, snapshot) !== undefined) { + fields[open] = appendLine(fields[open], line); + } + return; } - return undefined; - } - const aborted = ABORTED_RE.exec(line); - if (aborted) fields.aborted ??= (aborted[1] ?? '').trim(); - return undefined; + open = undefined; + const code = CODE_RE.exec(line); + if (code) { + const value = Number(code[2]); + if (!Number.isSafeInteger(value)) return; + if (code[1] === 'CODE') fields.resultCode ??= value; + else fields.statusCode = value; + return; + } + const aborted = ABORTED_RE.exec(line); + if (aborted) { + fields.aborted ??= accept((aborted[1] ?? '').trim(), snapshot); + return; + } + const record = VALUE_RECORD_RE.exec(line); + const field = + record && Object.hasOwn(MESSAGE_FIELDS, record[1]!) + ? (MESSAGE_FIELDS[record[1] as keyof typeof MESSAGE_FIELDS] as MessageField) + : undefined; + if (!field || fields[field] !== undefined) { + snapshotContentRemoved ||= snapshot; + return; + } + const value = accept((record![2] ?? '').slice(0, MESSAGE_FIELD_MAX_CHARS), snapshot); + if (value === undefined) return; + fields[field] = value; + open = field; + }; + const readStderrLine = (line: string, snapshot: boolean, cut: boolean) => { + readFailed(line); + stderrCut ||= cut || snapshot; + snapshotContentRemoved ||= snapshot; + stderrKept += `${snapshot ? SNAPSHOT_OMITTED : line}\n`; + if (stderrKept.length > STDERR_WINDOW_MAX_CHARS) { + stderrCut = true; + stderrKept = stderrKept.slice( + stderrKept.indexOf('\n', stderrKept.length - STDERR_WINDOW_MAX_CHARS) + 1, + ); + } + }; + const stdout = createLineAssembler('head', STDOUT_LINE_MAX_CHARS, readStdoutLine); + const stderr = createLineAssembler('tail', STDERR_LINE_MAX_CHARS, readStderrLine); + return { + writeStdout: stdout.write, + writeStderr: stderr.write, + result( + exitCode: number | null | undefined, + signal?: string | null, + ): AndroidInstrumentationExitResult { + stdout.end(); + stderr.end(); + const text = redactDiagnosticText(stderrKept); + const { aborted, failed, shortMsg, longMsg, error, stack, resultCode, statusCode } = fields; + return { + exitCode: exitCode ?? null, + ...(signal ? { signal } : {}), + ...(resultCode !== undefined ? { resultCode } : {}), + ...(statusCode !== undefined ? { statusCode } : {}), + ...(aborted !== undefined ? { aborted } : {}), + ...(failed !== undefined ? { failed } : {}), + ...(shortMsg !== undefined ? { shortMsg: shortMsg.trimEnd() } : {}), + ...(longMsg !== undefined ? { longMsg: longMsg.trimEnd() } : {}), + ...(error !== undefined ? { error: error.trimEnd() } : {}), + ...(stack !== undefined ? { stack: stack.trimEnd() } : {}), + stderrTail: text.slice(-REDACTED_STRING_MAX_LENGTH), + stderrTruncated: stderrCut || text.length > REDACTED_STRING_MAX_LENGTH, + ...(snapshotContentRemoved ? { snapshotContentRemoved: true as const } : {}), + }; + }, + }; } function appendLine(value: string | undefined, line: string): string | undefined { @@ -295,6 +331,18 @@ function appendLine(value: string | undefined, line: string): string | undefined return next.length > MESSAGE_FIELD_MAX_CHARS ? value : next; } +export function readInstrumentationExitResult(run: { + stdout: string; + stderr: string; + exitCode: number | null | undefined; + signal?: string | null; +}): AndroidInstrumentationExitResult { + const output = createInstrumentationOutput(); + output.writeStdout(run.stdout); + output.writeStderr(run.stderr); + return output.result(run.exitCode, run.signal); +} + /** The exit of a spawned helper process and what it printed, kept to a bounded recent window. */ export type AndroidInstrumentationProcessRecord = { /** Settles when the process's output streams have closed, the point its output is complete. */ @@ -306,35 +354,15 @@ export type AndroidInstrumentationProcessRecord = { export function recordInstrumentationProcess( child: AndroidAdbProcess, ): AndroidInstrumentationProcessRecord { - // Each line is filtered as it completes, so the bounded window never holds a line whose opening - // tag was cut away. A line that outgrows the window is classified on the part held: snapshot - // content is dropped, anything else keeps its end. - let kept = ''; - let pending = ''; - let pendingDropped = false; - const prior = { snapshotContentRemoved: false, cut: false }; - const keep = (line: string) => { - const filtered = filterSnapshotContent(line); - prior.snapshotContentRemoved ||= filtered.removed; - const next = `${kept}${filtered.text}\n`; - prior.cut ||= next.length > OUTPUT_SCAN_MAX_CHARS; - kept = next.slice(-OUTPUT_SCAN_MAX_CHARS); - }; - const onData = (chunk: Buffer | string) => { - const lines = (pending + chunk.toString()).split('\n'); - pending = lines.pop() ?? ''; - for (const line of lines) { - keep(pendingDropped ? XML_OMITTED : line); - pendingDropped = false; - } - if (pending.length > OUTPUT_SCAN_MAX_CHARS) { - pendingDropped ||= holdsSnapshotContent(pending); - pending = pendingDropped ? '' : pending.slice(-OUTPUT_SCAN_MAX_CHARS); - prior.cut = true; - } + const output = createInstrumentationOutput(); + const listen = (stream: AndroidAdbProcess['stdout'], write: (chunk: string) => void) => { + const decoder = new TextDecoder(); + stream?.on('data', (chunk: Buffer | string) => { + write(typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true })); + }); }; - child.stdout?.on('data', onData); - child.stderr?.on('data', onData); + listen(child.stdout, output.writeStdout); + listen(child.stderr, output.writeStderr); let isClosed = false; const closed = new Promise((resolve) => { child.once('close', () => { @@ -344,16 +372,7 @@ export function recordInstrumentationProcess( }); return { closed, - exitResult: () => - isClosed - ? readInstrumentationExitResult({ - stdout: pendingDropped ? `${kept}${XML_OMITTED}` : `${kept}${pending}`, - stderr: '', - exitCode: child.exitCode, - signal: child.signalCode, - prior, - }) - : undefined, + exitResult: () => (isClosed ? output.result(child.exitCode, child.signalCode) : undefined), }; } diff --git a/packages/platform-android/src/snapshot-helper-capture.ts b/packages/platform-android/src/snapshot-helper-capture.ts index 75c12f603a..ca9cb5cfa0 100644 --- a/packages/platform-android/src/snapshot-helper-capture.ts +++ b/packages/platform-android/src/snapshot-helper-capture.ts @@ -12,7 +12,6 @@ import { readInstrumentationResultIntegerList, readInstrumentationResultNumber, readInstrumentationExitResult, - withoutSnapshotContent, } from './instrumentation-helper.ts'; import { ANDROID_SNAPSHOT_HELPER_CAPTURE_TIMEOUT_MS, @@ -224,16 +223,18 @@ function readAndroidSnapshotHelperOutput( if (helperFailure) throw helperFailure; // exec-guard-allow: reachable at exit 0 (helper output unparseable); the // message already branches on the exit code. + // The raw output is not copied into the details: it can hold the snapshot the helper was + // printing, so only the parsed instrumentation result and its filtered stderr tail are kept. + const instrumentation = readInstrumentationExitResult(result); throw new AppError( 'COMMAND_FAILED', result.exitCode === 0 ? 'Android snapshot helper output could not be parsed' : 'Android snapshot helper failed before returning parseable output', { - stdout: withoutSnapshotContent(result.stdout), - stderr: withoutSnapshotContent(result.stderr), + stderr: instrumentation.stderrTail, exitCode: result.exitCode, - instrumentation: readInstrumentationExitResult(result), + instrumentation, ...androidCaptureFailureReasonDetail( androidCaptureFailureReasonFromExitCode(result.exitCode), ), diff --git a/packages/platform-android/src/snapshot-helper-session-lifecycle.ts b/packages/platform-android/src/snapshot-helper-session-lifecycle.ts index bfbbf56c2f..707c87c1a9 100644 --- a/packages/platform-android/src/snapshot-helper-session-lifecycle.ts +++ b/packages/platform-android/src/snapshot-helper-session-lifecycle.ts @@ -310,6 +310,7 @@ async function startAndroidSnapshotHelperSession(params: { allowFailure: true, captureOutput: false, }); + const processRecord = recordInstrumentationProcess(childProcess); const session: AndroidSnapshotHelperSession = { identity: params.identity, deviceKey: params.deviceKey, @@ -323,7 +324,7 @@ async function startAndroidSnapshotHelperSession(params: { port, adb: params.options.adb, process: childProcess, - processRecord: recordInstrumentationProcess(childProcess), + processRecord, startedAtMs: Date.now(), capturedCount: 0, commands: new Set(), @@ -337,6 +338,7 @@ async function startAndroidSnapshotHelperSession(params: { childProcess, params.startBudgetMs, params.options.signal, + processRecord, ); sessions().set(params.deviceKey, session); failedSessions().delete(params.identity); diff --git a/packages/platform-android/src/snapshot-helper-session-protocol.ts b/packages/platform-android/src/snapshot-helper-session-protocol.ts index 746230f5d1..058fe3544b 100644 --- a/packages/platform-android/src/snapshot-helper-session-protocol.ts +++ b/packages/platform-android/src/snapshot-helper-session-protocol.ts @@ -5,8 +5,7 @@ import { androidCaptureFailureReasonFromHelperResult, } from './snapshot-capture-failure-reason.ts'; import { - readInstrumentationExitResult, - withoutSnapshotContent, + type AndroidInstrumentationProcessRecord, readInstrumentationResultBoolean, readInstrumentationResultIntegerList, readInstrumentationResultNumber, @@ -116,7 +115,8 @@ const SESSION_EXITED_BEFORE_READY_REASON = 'android_snapshot_helper_session_exit export function waitForAndroidSnapshotHelperSessionReady( childProcess: AndroidAdbProcess, timeoutMs: number, - signal?: AbortSignal, + signal: AbortSignal | undefined, + record: AndroidInstrumentationProcessRecord, ): Promise> { return new Promise((resolve, reject) => { let output = ''; @@ -124,7 +124,7 @@ export function waitForAndroidSnapshotHelperSessionReady( signal?.removeEventListener('abort', onAbort); reject( new AppError('COMMAND_FAILED', 'Android snapshot helper session did not become ready', { - output: withoutSnapshotContent(output), + output, timeoutMs, reason: SESSION_READY_TIMEOUT_REASON, }), @@ -157,19 +157,17 @@ export function waitForAndroidSnapshotHelperSessionReady( childProcess.once('exit', (code, exitSignal) => { clearTimeout(timer); signal?.removeEventListener('abort', onAbort); - reject( - new AppError('COMMAND_FAILED', 'Android snapshot helper session exited before ready', { - output: withoutSnapshotContent(output), - exitCode: code, - signal: exitSignal, - instrumentation: readInstrumentationExitResult({ - stdout: output, - stderr: '', + // The exit result is read once the streams have drained, so it carries the last output. + void record.closed.then(() => + reject( + new AppError('COMMAND_FAILED', 'Android snapshot helper session exited before ready', { + output, exitCode: code, signal: exitSignal, + instrumentation: record.exitResult(), + reason: SESSION_EXITED_BEFORE_READY_REASON, }), - reason: SESSION_EXITED_BEFORE_READY_REASON, - }), + ), ); }); childProcess.on('error', (error) => { From a61ecc1005f326f12e260dd04fcc958c5b8453f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 28 Oct 2026 14:41:28 +0100 Subject: [PATCH 08/18] test(android): cover stderr beyond the retained window Co-Authored-By: Claude Sonnet 5.5 --- .../__tests__/snapshot-helper-exit-result.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts index d42506ffea..0b992ee681 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts @@ -439,6 +439,18 @@ test('the real cause past the redaction cap survives in the error and the reques assert.equal(logged.stderrTruncated, true); }); +test('stderr beyond the retained window keeps the newest lines', async () => { + const lines = Array.from({ length: 100 }, (_, index) => `${index} ${'w'.repeat(400)}`); + const result = await captureExitResult({ + exitCode: 1, + stdout: '', + stderr: `${lines.join('\n')}\nam: Killed`, + }); + + assert.ok(result.stderrTail.endsWith('am: Killed')); + assert.equal(result.stderrTruncated, true); +}); + test('a long stderr diagnostic line keeps its end and a long snapshot line is dropped', async () => { const diagnostic = `${'frame '.repeat(5_000)}Caused by: binder has died`; const kept = await captureExitResult({ exitCode: 1, stdout: '', stderr: diagnostic }); From 8bcc0aec735afaa9ab78da7953bc1302f2a4c9d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 28 Oct 2026 14:45:28 +0100 Subject: [PATCH 09/18] refactor(android): split protocol record reading out of the stdout line reader Co-Authored-By: Claude Sonnet 5.5 --- .../src/instrumentation-helper.ts | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/packages/platform-android/src/instrumentation-helper.ts b/packages/platform-android/src/instrumentation-helper.ts index e1d171126d..b3e68a5079 100644 --- a/packages/platform-android/src/instrumentation-helper.ts +++ b/packages/platform-android/src/instrumentation-helper.ts @@ -245,42 +245,39 @@ function createInstrumentationOutput() { const readStdoutLine = (line: string, snapshot: boolean) => { if (readFailed(line)) { open = undefined; - return; - } - if (!PROTOCOL_RECORD_RE.test(line)) { - if (!open) snapshotContentRemoved ||= snapshot; - else if (accept(line, snapshot) !== undefined) { - fields[open] = appendLine(fields[open], line); - } - return; + } else if (PROTOCOL_RECORD_RE.test(line)) { + open = readProtocolRecord(line, snapshot); + } else if (!open) { + snapshotContentRemoved ||= snapshot; + } else if (accept(line, snapshot) !== undefined) { + fields[open] = appendLine(fields[open], line); } - open = undefined; + }; + /** Reads one protocol record; answers the message field its value opens, if any. */ + const readProtocolRecord = (line: string, snapshot: boolean): MessageField | undefined => { const code = CODE_RE.exec(line); if (code) { const value = Number(code[2]); - if (!Number.isSafeInteger(value)) return; + if (!Number.isSafeInteger(value)) return undefined; if (code[1] === 'CODE') fields.resultCode ??= value; else fields.statusCode = value; - return; + return undefined; } const aborted = ABORTED_RE.exec(line); if (aborted) { fields.aborted ??= accept((aborted[1] ?? '').trim(), snapshot); - return; + return undefined; } const record = VALUE_RECORD_RE.exec(line); - const field = - record && Object.hasOwn(MESSAGE_FIELDS, record[1]!) - ? (MESSAGE_FIELDS[record[1] as keyof typeof MESSAGE_FIELDS] as MessageField) - : undefined; + const field = record ? messageFieldOf(record[1]!) : undefined; if (!field || fields[field] !== undefined) { snapshotContentRemoved ||= snapshot; - return; + return undefined; } const value = accept((record![2] ?? '').slice(0, MESSAGE_FIELD_MAX_CHARS), snapshot); - if (value === undefined) return; + if (value === undefined) return undefined; fields[field] = value; - open = field; + return field; }; const readStderrLine = (line: string, snapshot: boolean, cut: boolean) => { readFailed(line); @@ -326,6 +323,12 @@ function createInstrumentationOutput() { }; } +function messageFieldOf(key: string): MessageField | undefined { + return Object.hasOwn(MESSAGE_FIELDS, key) + ? (MESSAGE_FIELDS[key as keyof typeof MESSAGE_FIELDS] as MessageField) + : undefined; +} + function appendLine(value: string | undefined, line: string): string | undefined { const next = `${value ?? ''}\n${line}`; return next.length > MESSAGE_FIELD_MAX_CHARS ? value : next; From 3db82759dc965f2f8eb2d07aa89b50fdef0dfc97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 28 Oct 2026 14:56:35 +0100 Subject: [PATCH 10/18] fix(android): attach no raw helper stream on any failure path and flag a cut unbroken stderr line Co-Authored-By: Claude Sonnet 5.5 --- .../snapshot-helper-exit-result.test.ts | 125 +++++++++++++++++- .../src/instrumentation-helper.ts | 5 +- .../src/snapshot-helper-capture.ts | 8 +- .../src/snapshot-helper-session-protocol.ts | 7 +- 4 files changed, 137 insertions(+), 8 deletions(-) diff --git a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts index 0b992ee681..10882dafec 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts @@ -3,8 +3,13 @@ import { promises as fs } from 'node:fs'; import path from 'node:path'; import { afterEach, beforeEach, test } from 'vitest'; import { withDiagnosticsScope } from '@agent-device/host-kit/diagnostics'; -import { captureAndroidSnapshotWithHelper } from '../snapshot-helper-capture.ts'; +import { + captureAndroidSnapshotWithHelper, + parseAndroidSnapshotHelperOutput, +} from '../snapshot-helper-capture.ts'; import { captureAndroidSnapshotWithHelperSession } from '../snapshot-helper-session.ts'; +import { waitForAndroidSnapshotHelperSessionReady } from '../snapshot-helper-session-protocol.ts'; +import { recordInstrumentationProcess } from '../instrumentation-helper.ts'; import { resetAndroidSnapshotHelperSessions } from '../snapshot-helper-session-lifecycle.ts'; import { resetAndroidSnapshotHelperRetirements } from '../snapshot-helper-retirement.ts'; import { snapshotAndroid } from '../snapshot.ts'; @@ -378,6 +383,104 @@ test('a session exit never leaks a sensitive value split across chunks', async ( assert.ok(exit.stderrTail.endsWith('am: Killed'), exit.stderrTail); }); +test('a startup failure attaches no raw helper stream to its details or the log', async () => { + const printed = (process: FakeAndroidProcess) => { + process.stdout.write(`${encodedPayload(SECRET_XML)}\n`); + process.stdout.write(`INSTRUMENTATION_STATUS: stream=${SECRET_XML}\n`); + process.stdout.write(`ret" text="${SECRET}" bounds="[0,0][1,1]" />\n`); + process.stderr.write(`ret" content-desc="${SECRET}" bounds="[0,0][1,1]" />\nam: Killed\n`); + }; + const waitFor = (process: FakeAndroidProcess, timeoutMs: number) => + waitForAndroidSnapshotHelperSessionReady( + process, + timeoutMs, + undefined, + recordInstrumentationProcess(process), + ); + + const exited = new FakeAndroidProcess(); + const exitedFailure = waitFor(exited, 5_000).catch((error: AppError) => error); + printed(exited); + await pause(); + exited.emitExit(1, null); + const exitedError = await exitedFailure; + assertPrivate(JSON.stringify(normalizeError(exitedError))); + assert.equal('output' in (exitedError.details ?? {}), false); + assert.equal(exitResultOf(exitedError.details ?? {}).exitCode, 1); + assert.ok(exitResultOf(exitedError.details ?? {}).stderrTail.endsWith('am: Killed')); + + const stalled = new FakeAndroidProcess(); + const stalledFailure = waitFor(stalled, 50).catch((error: AppError) => error); + printed(stalled); + const stalledError = await stalledFailure; + assertPrivate(JSON.stringify(normalizeError(stalledError))); + assert.equal('output' in (stalledError.details ?? {}), false); + assert.equal(exitResultOf(stalledError.details ?? {}).exitCode, null); + + const { log } = await withLog(async (logPath) => { + const base = createSessionProvider({ calls: [], processes: [] }); + const provider: AndroidAdbProvider = { + ...base, + spawn: () => { + const process = new FakeAndroidProcess(); + setImmediate(() => { + printed(process); + process.emitExit(1, null); + }); + return process; + }, + }; + await inScope(logPath, async () => { + await captureAndroidSnapshotWithHelperSession({ + adb: provider.exec, + adbProvider: provider, + deviceKey: 'android:emulator-5556', + }); + }); + }); + assertPrivate(log); + assert.equal( + exitResultOf(diagnosticData(log, 'android_snapshot_helper_session_start_failed')) + .snapshotContentRemoved, + true, + ); +}); + +test('a nonzero exit after parseable helper output keeps no raw stream in its details', async () => { + const xml = ``; + const stdout = [ + 'INSTRUMENTATION_STATUS: agentDeviceProtocol=android-snapshot-helper-v1', + 'INSTRUMENTATION_STATUS: outputFormat=uiautomator-xml', + 'INSTRUMENTATION_STATUS: chunkIndex=0', + 'INSTRUMENTATION_STATUS: chunkCount=1', + `INSTRUMENTATION_STATUS: payloadBase64=${Buffer.from(xml).toString('base64')}`, + 'INSTRUMENTATION_STATUS_CODE: 1', + 'INSTRUMENTATION_RESULT: agentDeviceProtocol=android-snapshot-helper-v1', + 'INSTRUMENTATION_RESULT: ok=true', + 'INSTRUMENTATION_CODE: -1', + ].join('\n'); + const error = await captureError({ exitCode: 1, stdout, stderr: 'am: Killed' }); + + assertPrivate(JSON.stringify(normalizeError(error))); + assert.equal(error.details?.stdout, ''); + assert.equal(error.details?.stderr, 'am: Killed'); + assert.equal(error.details?.exitCode, 1); +}); + +test('an unbroken stderr line that is cut is flagged as truncated', async () => { + const unbroken = await captureExitResult({ exitCode: 1, stdout: '', stderr: 'x'.repeat(5_000) }); + assert.equal(unbroken.stderrTruncated, true); + + const snapshot = await captureExitResult({ + exitCode: 1, + stdout: '', + stderr: ` { const { exit } = await sessionExit(async (process) => { process.stdout.write('INSTRUMENTATION_RESULT: longMsg=UiAutomation fai'); @@ -611,3 +714,23 @@ test('the request log entry for a failed capture carries the instrumentation res assert.equal(logged.exitCode, 1); assert.equal(logged.aborted, 'System has crashed.'); }); + +test('a malformed helper report names its size, never the snapshot text it held', () => { + const stdout = [ + 'INSTRUMENTATION_STATUS: agentDeviceProtocol=android-snapshot-helper-v1', + 'INSTRUMENTATION_STATUS: outputFormat=uiautomator-xml', + 'INSTRUMENTATION_STATUS: chunkIndex=0', + 'INSTRUMENTATION_STATUS: chunkCount=1', + `INSTRUMENTATION_STATUS: payloadBase64=${Buffer.from(``).toString('base64')}`, + 'INSTRUMENTATION_RESULT: agentDeviceProtocol=android-snapshot-helper-v1', + 'INSTRUMENTATION_RESULT: ok=true', + ].join('\n'); + + assert.throws( + () => parseAndroidSnapshotHelperOutput(stdout), + (error: unknown) => { + assertPrivate(JSON.stringify(normalizeError(error))); + return error instanceof AppError && typeof error.details?.xmlLength === 'number'; + }, + ); +}); diff --git a/packages/platform-android/src/instrumentation-helper.ts b/packages/platform-android/src/instrumentation-helper.ts index b3e68a5079..83e4e2f69d 100644 --- a/packages/platform-android/src/instrumentation-helper.ts +++ b/packages/platform-android/src/instrumentation-helper.ts @@ -212,7 +212,7 @@ function createLineAssembler( append(rest); }, end() { - if (pending) flush(); + if (pending || cut || snapshot) flush(); }, }; } @@ -352,6 +352,8 @@ export type AndroidInstrumentationProcessRecord = { closed: Promise; /** The process's exit result once its streams have closed, or `undefined` before that. */ exitResult(): AndroidInstrumentationExitResult | undefined; + /** What the process has printed so far, for a process that is still running. */ + current(): AndroidInstrumentationExitResult; }; export function recordInstrumentationProcess( @@ -376,6 +378,7 @@ export function recordInstrumentationProcess( return { closed, exitResult: () => (isClosed ? output.result(child.exitCode, child.signalCode) : undefined), + current: () => output.result(child.exitCode, child.signalCode), }; } diff --git a/packages/platform-android/src/snapshot-helper-capture.ts b/packages/platform-android/src/snapshot-helper-capture.ts index ca9cb5cfa0..4b17a32519 100644 --- a/packages/platform-android/src/snapshot-helper-capture.ts +++ b/packages/platform-android/src/snapshot-helper-capture.ts @@ -108,10 +108,14 @@ export async function captureAndroidSnapshotWithHelper( } const output = readAndroidSnapshotHelperOutput(result); if (result.exitCode !== 0) { + const instrumentation = readInstrumentationExitResult(result); throw new AppError( 'COMMAND_FAILED', 'Android snapshot helper failed', - execFailureDetails(result, { helper: output.metadata }), + execFailureDetails( + { stdout: '', stderr: instrumentation.stderrTail, exitCode: result.exitCode }, + { helper: output.metadata, instrumentation }, + ), ); } return output; @@ -315,7 +319,7 @@ function decodeHelperXml( ).toString('utf8'); if (!xml.includes('')) { throw new AppError('COMMAND_FAILED', 'Android snapshot helper output did not contain XML', { - xml, + xmlLength: xml.length, }); } return xml; diff --git a/packages/platform-android/src/snapshot-helper-session-protocol.ts b/packages/platform-android/src/snapshot-helper-session-protocol.ts index 058fe3544b..e51a180868 100644 --- a/packages/platform-android/src/snapshot-helper-session-protocol.ts +++ b/packages/platform-android/src/snapshot-helper-session-protocol.ts @@ -124,7 +124,7 @@ export function waitForAndroidSnapshotHelperSessionReady( signal?.removeEventListener('abort', onAbort); reject( new AppError('COMMAND_FAILED', 'Android snapshot helper session did not become ready', { - output, + instrumentation: record.current(), timeoutMs, reason: SESSION_READY_TIMEOUT_REASON, }), @@ -161,7 +161,6 @@ export function waitForAndroidSnapshotHelperSessionReady( void record.closed.then(() => reject( new AppError('COMMAND_FAILED', 'Android snapshot helper session exited before ready', { - output, exitCode: code, signal: exitSignal, instrumentation: record.exitResult(), @@ -229,7 +228,7 @@ export function parseAndroidSnapshotHelperSessionSnapshotResponse( throw new AppError( 'COMMAND_FAILED', 'Android snapshot helper session returned malformed output', - { response }, + { responseLength: response.length }, ); } const headers = parseHeaders(response.slice(0, separator)); @@ -283,7 +282,7 @@ function validateSnapshotXml(headers: Record, xml: string): void if (!xml.includes('')) { throw new AppError('COMMAND_FAILED', 'Android snapshot helper session did not return XML', { headers, - xml, + xmlLength: xml.length, }); } } From 28678f42790de2d8298fa83d796110e46f07cfc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 28 Oct 2026 15:01:58 +0100 Subject: [PATCH 11/18] test(android): narrow the rejected startup error before reading its details Co-Authored-By: Claude Sonnet 5.5 --- .../__tests__/snapshot-helper-exit-result.test.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts index 10882dafec..80aa5d0efa 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts @@ -176,6 +176,17 @@ async function sessionExit( }; } +/** The AppError a promise rejects with; fails the test when it resolves. */ +async function rejection(promise: Promise): Promise { + try { + await promise; + } catch (error) { + assert.ok(error instanceof AppError, String(error)); + return error; + } + throw new Error('expected the promise to reject'); +} + const pause = () => new Promise((resolve) => setTimeout(resolve, 10)); function assertPrivate(text: string): void { @@ -399,7 +410,7 @@ test('a startup failure attaches no raw helper stream to its details or the log' ); const exited = new FakeAndroidProcess(); - const exitedFailure = waitFor(exited, 5_000).catch((error: AppError) => error); + const exitedFailure = rejection(waitFor(exited, 5_000)); printed(exited); await pause(); exited.emitExit(1, null); @@ -410,7 +421,7 @@ test('a startup failure attaches no raw helper stream to its details or the log' assert.ok(exitResultOf(exitedError.details ?? {}).stderrTail.endsWith('am: Killed')); const stalled = new FakeAndroidProcess(); - const stalledFailure = waitFor(stalled, 50).catch((error: AppError) => error); + const stalledFailure = rejection(waitFor(stalled, 50)); printed(stalled); const stalledError = await stalledFailure; assertPrivate(JSON.stringify(normalizeError(stalledError))); From c35d1869112fd462d7d2f39149be2a9cceb9c8bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 28 Oct 2026 15:07:14 +0100 Subject: [PATCH 12/18] fix(android): read instrumentation results for every parse failure and snapshot without finalizing Co-Authored-By: Claude Sonnet 5.5 --- .../snapshot-helper-exit-result.test.ts | 35 +++++++++++++++++++ .../src/instrumentation-helper.ts | 21 +++++++---- .../src/snapshot-helper-capture.ts | 12 +++++-- 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts index 80aa5d0efa..e30f4cc375 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts @@ -745,3 +745,38 @@ test('a malformed helper report names its size, never the snapshot text it held' }, ); }); + +test('a helper report with ok=true but no XML chunks still carries the instrumentation result', async () => { + const error = await captureError({ + exitCode: 1, + stdout: [ + 'INSTRUMENTATION_RESULT: agentDeviceProtocol=android-snapshot-helper-v1', + 'INSTRUMENTATION_RESULT: ok=true', + 'INSTRUMENTATION_CODE: -1', + ].join('\n'), + stderr: 'am: Killed', + }); + + const result = exitResultOf(error.details ?? {}); + assert.equal(result.exitCode, 1); + assert.equal(result.resultCode, -1); + assert.equal(result.stderrTail, 'am: Killed'); +}); + +test('a ready timeout in the middle of a record leaves the final fields intact', async () => { + const child = new FakeAndroidProcess(); + const record = recordInstrumentationProcess(child); + const failure = rejection(waitForAndroidSnapshotHelperSessionReady(child, 30, undefined, record)); + child.stdout.write('INSTRUMENTATION_RESULT: longMsg=UiAutomation fai'); + const timedOut = await failure; + + assert.equal(exitResultOf(timedOut.details ?? {}).longMsg, undefined); + + child.stdout.write('led\nCaused by: binder has died\nINSTRUMENTATION_CODE: 0\n'); + await pause(); + child.emitExit(1, null); + await record.closed; + const final = record.exitResult(); + assert.equal(final?.longMsg, 'UiAutomation failed\nCaused by: binder has died'); + assert.equal(final?.resultCode, 0); +}); diff --git a/packages/platform-android/src/instrumentation-helper.ts b/packages/platform-android/src/instrumentation-helper.ts index 83e4e2f69d..775461c53e 100644 --- a/packages/platform-android/src/instrumentation-helper.ts +++ b/packages/platform-android/src/instrumentation-helper.ts @@ -298,10 +298,13 @@ function createInstrumentationOutput() { writeStderr: stderr.write, result( exitCode: number | null | undefined, - signal?: string | null, + signal: string | null | undefined, + final: boolean, ): AndroidInstrumentationExitResult { - stdout.end(); - stderr.end(); + if (final) { + stdout.end(); + stderr.end(); + } const text = redactDiagnosticText(stderrKept); const { aborted, failed, shortMsg, longMsg, error, stack, resultCode, statusCode } = fields; return { @@ -343,7 +346,7 @@ export function readInstrumentationExitResult(run: { const output = createInstrumentationOutput(); output.writeStdout(run.stdout); output.writeStderr(run.stderr); - return output.result(run.exitCode, run.signal); + return output.result(run.exitCode, run.signal, true); } /** The exit of a spawned helper process and what it printed, kept to a bounded recent window. */ @@ -352,7 +355,10 @@ export type AndroidInstrumentationProcessRecord = { closed: Promise; /** The process's exit result once its streams have closed, or `undefined` before that. */ exitResult(): AndroidInstrumentationExitResult | undefined; - /** What the process has printed so far, for a process that is still running. */ + /** + * What the process has printed so far, for a process that is still running. A line still being + * written is left out, and the record keeps reading it. + */ current(): AndroidInstrumentationExitResult; }; @@ -377,8 +383,9 @@ export function recordInstrumentationProcess( }); return { closed, - exitResult: () => (isClosed ? output.result(child.exitCode, child.signalCode) : undefined), - current: () => output.result(child.exitCode, child.signalCode), + exitResult: () => + isClosed ? output.result(child.exitCode, child.signalCode, true) : undefined, + current: () => output.result(child.exitCode, child.signalCode, false), }; } diff --git a/packages/platform-android/src/snapshot-helper-capture.ts b/packages/platform-android/src/snapshot-helper-capture.ts index 4b17a32519..cd2ebaf900 100644 --- a/packages/platform-android/src/snapshot-helper-capture.ts +++ b/packages/platform-android/src/snapshot-helper-capture.ts @@ -221,10 +221,10 @@ function readAndroidSnapshotHelperOutput( // The helper can report structured ok=false details even when am exits non-zero. return parseAndroidSnapshotHelperOutput(`${result.stdout}\n${result.stderr}`); } catch (error) { - const helperFailure = error instanceof AppError && error.details?.helper ? error : undefined; // `am instrument` exits 0 after a helper that reported its own failure, so that report is the - // answer. - if (helperFailure) throw helperFailure; + // answer. Any other parse failure, including a report of success with no XML, has no report of + // its own and falls through to the instrumentation result below. + if (error instanceof AppError && isExplicitHelperFailure(error.details?.helper)) throw error; // exec-guard-allow: reachable at exit 0 (helper output unparseable); the // message already branches on the exit code. // The raw output is not copied into the details: it can hold the snapshot the helper was @@ -248,6 +248,12 @@ function readAndroidSnapshotHelperOutput( } } +function isExplicitHelperFailure(helper: unknown): boolean { + return ( + typeof helper === 'object' && helper !== null && (helper as { ok?: unknown }).ok === 'false' + ); +} + export function parseAndroidSnapshotHelperOutput(output: string): AndroidSnapshotHelperOutput { const records = parseInstrumentationRecords(output); const finalResult = readFinalHelperResult(records.results); From 77a77653c214862b04f73764b54a4bb9df4dc6d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 28 Oct 2026 15:30:52 +0100 Subject: [PATCH 13/18] refactor(android): record parsed instrumentation fields only, drop the stderr tail Read startup failures from strict whole-line envelopes, sanitize executor timeouts at the capture boundary, bound the exit drain, and stop delaying live-helper retirement. Refs #4005 Co-Authored-By: Claude Sonnet 5.5 --- packages/kernel/src/redaction.ts | 18 +- .../snapshot-helper-exit-result.test.ts | 246 ++++++++---------- .../src/__tests__/snapshot-helper.test.ts | 18 +- .../src/__tests__/snapshot.test.ts | 4 +- .../src/instrumentation-helper.ts | 138 +++------- .../src/snapshot-helper-capture.ts | 34 ++- .../src/snapshot-helper-session-lifecycle.ts | 4 +- .../src/snapshot-helper-session-protocol.ts | 26 +- .../src/snapshot-helper-session.ts | 17 +- packages/platform-android/src/snapshot.ts | 6 + 10 files changed, 226 insertions(+), 285 deletions(-) diff --git a/packages/kernel/src/redaction.ts b/packages/kernel/src/redaction.ts index 4a1022163a..7889a19f04 100644 --- a/packages/kernel/src/redaction.ts +++ b/packages/kernel/src/redaction.ts @@ -5,8 +5,7 @@ const SECRET_TOKEN_RE = const SENSITIVE_ASSIGNMENT_RE = /\b([a-z0-9_-]*(?:api[_-]?key|token|secret|password|user[_-]?code|device[_-]?code|refresh[_-]?credential)[a-z0-9_-]*)(\s*[=:]\s*)("[^"]*"|'[^']*'|\S+)/gi; const URL_RE = /https?:\/\/[^\s"'<>]+/gi; -/** The longest ordinary string a redacted diagnostic value keeps. */ -export const REDACTED_STRING_MAX_LENGTH = 400; +const REDACTED_STRING_MAX_LENGTH = 400; const REDACTED_STDERR_MAX_LENGTH = 8192; const TRUNCATION_SUFFIX = '...'; @@ -54,21 +53,9 @@ function redactString(value: string, keyHint?: string): string { const trimmed = value.trim(); if (!trimmed) return boundRedactedString(value, keyHint); if (keyHint && SENSITIVE_KEY_RE.test(keyHint)) return '[REDACTED]'; - return boundRedactedString(redactSensitiveText(trimmed), keyHint); -} - -/** - * Redacts secrets and URL credentials in free text without the length bound - * `redactDiagnosticData` applies, for a caller that must choose which end of the text survives. - */ -export function redactDiagnosticText(value: string): string { - return redactSensitiveText(value.trim()); -} - -function redactSensitiveText(trimmed: string): string { let output = redactUrls(trimmed); output = output.replace(SECRET_TOKEN_RE, '[REDACTED]'); - return output.replace( + output = output.replace( SENSITIVE_ASSIGNMENT_RE, (match, key: string, separator: string, rawValue: string, offset: number, input: string) => { if (isSafeSetupUrlAssignment({ key, separator, rawValue, offset, input })) return match; @@ -76,6 +63,7 @@ function redactSensitiveText(trimmed: string): string { return `${key}${separator}[REDACTED]`; }, ); + return boundRedactedString(output, keyHint); } function boundRedactedString(value: string, keyHint?: string): string { diff --git a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts index e30f4cc375..dacbaf5d3e 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts @@ -7,6 +7,7 @@ import { captureAndroidSnapshotWithHelper, parseAndroidSnapshotHelperOutput, } from '../snapshot-helper-capture.ts'; +import { runCmd } from '@agent-device/host-kit/command'; import { captureAndroidSnapshotWithHelperSession } from '../snapshot-helper-session.ts'; import { waitForAndroidSnapshotHelperSessionReady } from '../snapshot-helper-session-protocol.ts'; import { recordInstrumentationProcess } from '../instrumentation-helper.ts'; @@ -64,9 +65,6 @@ type ExitResultFields = { longMsg?: string; error?: string; stack?: string; - stderrTail: string; - stderrTruncated: boolean; - snapshotContentRemoved?: true; }; const SECRET = 'hunter2-account-number'; @@ -199,8 +197,6 @@ test('an aborted instrumentation run reports the abort, the status code and the exitCode: 1, statusCode: 1, aborted: 'System has crashed.', - stderrTail: '', - stderrTruncated: false, }); }); @@ -210,8 +206,6 @@ test('a crashed instrumentation run reports shortMsg, longMsg and the result cod resultCode: 0, shortMsg: 'java.lang.IllegalStateException', longMsg: 'java.lang.IllegalStateException: UiAutomation not connected', - stderrTail: '', - stderrTruncated: false, }); }); @@ -328,7 +322,8 @@ test('stdout is never copied: only parsed protocol fields reach the details', as assert.ok(!JSON.stringify(error.details).includes('some unrelated stdout line')); }); -test('a sensitive value never reaches the details wherever the helper printed it', async () => { +test('a sensitive value never reaches the details or the log wherever the helper printed it', async () => { + const quoted = `${'p'.repeat(1_015)}password="${SECRET}" trailing`; const runs: FailedRun[] = [ // A complete encoded payload. { exitCode: 1, stdout: `${encodedPayload(SECRET_XML)}\nINSTRUMENTATION_STATUS_CODE: 1` }, @@ -339,32 +334,38 @@ test('a sensitive value never reaches the details wherever the helper printed it exitCode: 1, stdout: `INSTRUMENTATION_STATUS: detail=ret" text="${SECRET}" bounds="[0,0][1,1]" />\n`, }, - // A tagless fragment as a free line, and as a message continuation. + // A tagless fragment as a free line. { exitCode: 1, stdout: `ret" text="${SECRET}" bounds="[0,0][1,1]" />\n` }, + // Fragments on stderr, including a role-description attribute and a quoted value whose key a + // line cut would have removed. + { exitCode: 1, stdout: '', stderr: `ret" text="${SECRET}" bounds="[0,0][1,1]" />` }, + { exitCode: 1, stdout: '', stderr: `ret" role-description="${SECRET}" />` }, + { exitCode: 1, stdout: '', stderr: quoted }, + { exitCode: 1, stdout: '', stderr: `${'a'.repeat(30_000)}${SECRET_XML}` }, + // A hierarchy cut off in front of a startup-failure marker. { exitCode: 1, - stdout: `INSTRUMENTATION_RESULT: longMsg=boom\nret" text="${SECRET}" bounds="[0,0][1,1]" />\n`, + stdout: '', + stderr: `\n`, + stdout: '', + stderr: `android.util.AndroidException: INSTRUMENTATION_FAILED: ` }, - { exitCode: 1, stdout: '', stderr: `${'a'.repeat(30_000)}${SECRET_XML}` }, ]; for (const run of runs) { - const error = await captureError({ ...run, stderr: `${run.stderr ?? ''}\nam: Killed` }); + const withKill = { ...run, stderr: `${run.stderr ?? ''}\nam: Killed` }; + const error = await captureError(withKill); const wire = JSON.stringify(normalizeError(error)); assertPrivate(wire); - assert.ok(wire.includes('am: Killed'), wire.slice(-400)); - assert.equal(exitResultOf(normalizeError(error).details ?? {}).snapshotContentRemoved, true); + assert.equal(exitResultOf(normalizeError(error).details ?? {}).failed, undefined); const { log } = await withLog(async (logPath) => { await inScope(logPath, async () => { await assert.rejects( snapshotAndroid(emulator, { - helperAdb: helperFailingWith({ ...run, stderr: `${run.stderr ?? ''}\nam: Killed` }), + helperAdb: helperFailingWith(withKill), helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, }), ); @@ -390,8 +391,6 @@ test('a session exit never leaks a sensitive value split across chunks', async ( assertPrivate(log); assert.equal(exit.aborted, 'System has crashed.'); - assert.equal(exit.snapshotContentRemoved, true); - assert.ok(exit.stderrTail.endsWith('am: Killed'), exit.stderrTail); }); test('a startup failure attaches no raw helper stream to its details or the log', async () => { @@ -418,7 +417,6 @@ test('a startup failure attaches no raw helper stream to its details or the log' assertPrivate(JSON.stringify(normalizeError(exitedError))); assert.equal('output' in (exitedError.details ?? {}), false); assert.equal(exitResultOf(exitedError.details ?? {}).exitCode, 1); - assert.ok(exitResultOf(exitedError.details ?? {}).stderrTail.endsWith('am: Killed')); const stalled = new FakeAndroidProcess(); const stalledFailure = rejection(waitFor(stalled, 50)); @@ -451,9 +449,8 @@ test('a startup failure attaches no raw helper stream to its details or the log' }); assertPrivate(log); assert.equal( - exitResultOf(diagnosticData(log, 'android_snapshot_helper_session_start_failed')) - .snapshotContentRemoved, - true, + exitResultOf(diagnosticData(log, 'android_snapshot_helper_session_start_failed')).exitCode, + 1, ); }); @@ -474,24 +471,10 @@ test('a nonzero exit after parseable helper output keeps no raw stream in its de assertPrivate(JSON.stringify(normalizeError(error))); assert.equal(error.details?.stdout, ''); - assert.equal(error.details?.stderr, 'am: Killed'); + assert.equal(error.details?.stderr, ''); assert.equal(error.details?.exitCode, 1); }); -test('an unbroken stderr line that is cut is flagged as truncated', async () => { - const unbroken = await captureExitResult({ exitCode: 1, stdout: '', stderr: 'x'.repeat(5_000) }); - assert.equal(unbroken.stderrTruncated, true); - - const snapshot = await captureExitResult({ - exitCode: 1, - stdout: '', - stderr: ` { const { exit } = await sessionExit(async (process) => { process.stdout.write('INSTRUMENTATION_RESULT: longMsg=UiAutomation fai'); @@ -502,103 +485,6 @@ test('stderr chunks do not disturb a partial stdout record', async () => { assert.equal(exit.longMsg, 'UiAutomation failed\nCaused by: binder has died'); assert.equal(exit.resultCode, 0); - assert.equal(exit.stderrTail, 'am: warning'); -}); - -test('ordinary quoted assignments on stderr are kept while snapshot attributes are dropped', async () => { - const result = await captureExitResult({ - exitCode: 1, - stdout: '', - stderr: [ - 'E AndroidRuntime: java.lang.IllegalStateException message="binder died" code="7"', - `ret" text="${SECRET}" bounds="[0,0][1,1]" />`, - ].join('\n'), - }); - - assert.ok(result.stderrTail.includes('message="binder died" code="7"'), result.stderrTail); - assert.ok(!result.stderrTail.includes(SECRET)); - assert.equal(result.snapshotContentRemoved, true); - assert.equal(result.stderrTruncated, true); -}); - -test('the real cause past the redaction cap survives in the error and the request log', async () => { - const filler = Array.from({ length: 40 }, (_, index) => `line ${index} of helper output`).join( - '\n', - ); - const stderr = `${filler}\nam: Killed by the low memory killer`; - const error = await captureError({ exitCode: 1, stdout: '', stderr }); - - const wire = exitResultOf(normalizeError(error).details ?? {}); - assert.ok(wire.stderrTail.endsWith('am: Killed by the low memory killer'), wire.stderrTail); - assert.ok(wire.stderrTail.length <= 400); - assert.equal(wire.stderrTruncated, true); - - const short = await captureError({ exitCode: 1, stdout: '', stderr: `${'y'.repeat(500)} done` }); - const shortWire = exitResultOf(normalizeError(short).details ?? {}); - assert.equal(shortWire.stderrTruncated, true); - assert.ok(shortWire.stderrTail.endsWith('done')); - - const { log } = await withLog(async (logPath) => { - await inScope(logPath, async () => { - await assert.rejects( - snapshotAndroid(emulator, { - helperAdb: helperFailingWith({ exitCode: 1, stdout: '', stderr }), - helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, - }), - ); - }); - }); - const logged = exitResultOf(diagnosticData(log, 'android_snapshot_helper_failed')); - assert.ok(logged.stderrTail.endsWith('am: Killed by the low memory killer'), logged.stderrTail); - assert.equal(logged.stderrTruncated, true); -}); - -test('stderr beyond the retained window keeps the newest lines', async () => { - const lines = Array.from({ length: 100 }, (_, index) => `${index} ${'w'.repeat(400)}`); - const result = await captureExitResult({ - exitCode: 1, - stdout: '', - stderr: `${lines.join('\n')}\nam: Killed`, - }); - - assert.ok(result.stderrTail.endsWith('am: Killed')); - assert.equal(result.stderrTruncated, true); -}); - -test('a long stderr diagnostic line keeps its end and a long snapshot line is dropped', async () => { - const diagnostic = `${'frame '.repeat(5_000)}Caused by: binder has died`; - const kept = await captureExitResult({ exitCode: 1, stdout: '', stderr: diagnostic }); - assert.ok(kept.stderrTail.endsWith('Caused by: binder has died'), kept.stderrTail); - assert.equal(kept.stderrTruncated, true); - assert.equal(kept.snapshotContentRemoved, undefined); - - const snapshot = `${``.repeat(2_000)}`; - const dropped = await captureExitResult({ - exitCode: 1, - stdout: '', - stderr: `${snapshot}\nam: Killed`, - }); - assert.ok(!dropped.stderrTail.includes(SECRET)); - assert.ok(dropped.stderrTail.endsWith('am: Killed')); - assert.equal(dropped.snapshotContentRemoved, true); - assert.equal(dropped.stderrTruncated, true); -}); - -test('a session exit keeps a long diagnostic line and flags a chunked snapshot line it dropped', async () => { - const { exit, log } = await sessionExit(async (process) => { - // Lines arrive in pieces and outgrow the window before they complete. - process.stderr.write('frame '.repeat(4_000)); - await pause(); - process.stderr.write(`${'frame '.repeat(1_000)}Caused by: binder has died\n`); - process.stderr.write(``); - await pause(); - process.stderr.write(`${``.repeat(1_000)}\n`); - }); - - assertPrivate(log); - assert.ok(exit.stderrTail.includes('Caused by: binder has died'), exit.stderrTail); - assert.equal(exit.snapshotContentRemoved, true); - assert.equal(exit.stderrTruncated, true); }); test('a helper failure that reports its own result keeps that report without a second record', async () => { @@ -646,8 +532,6 @@ test('a session that exits before it is ready records its exit status and the ou exitCode: 1, statusCode: 1, aborted: 'System has crashed.', - stderrTail: 'am: Killed', - stderrTruncated: false, }, ); }); @@ -760,7 +644,6 @@ test('a helper report with ok=true but no XML chunks still carries the instrumen const result = exitResultOf(error.details ?? {}); assert.equal(result.exitCode, 1); assert.equal(result.resultCode, -1); - assert.equal(result.stderrTail, 'am: Killed'); }); test('a ready timeout in the middle of a record leaves the final fields intact', async () => { @@ -780,3 +663,88 @@ test('a ready timeout in the middle of a record leaves the final fields intact', assert.equal(final?.longMsg, 'UiAutomation failed\nCaused by: binder has died'); assert.equal(final?.resultCode, 0); }); + +test('startup failures are read from stderr only through a strict whole-line envelope', async () => { + const component = 'com.example/.Missing$Inner'; + for (const line of [ + `android.util.AndroidException: INSTRUMENTATION_FAILED: ${component}`, + `onError: commandError=true message=INSTRUMENTATION_FAILED: ${component}`, + ]) { + const result = await captureExitResult({ exitCode: 1, stdout: '', stderr: `${line}\n` }); + assert.equal(result.failed, component, line); + } + for (const line of [ + `android.util.AndroidException: INSTRUMENTATION_FAILED: `, + ` { + const script = + 'process.stdout.write(process.env.PRINTED_STDOUT); process.stderr.write(process.env.PRINTED_STDERR); setTimeout(() => {}, 10_000);'; + const env = { + ...process.env, + PRINTED_STDOUT: `${encodedPayload(SECRET_XML)}\nINSTRUMENTATION_STATUS_CODE: 1\n`, + PRINTED_STDERR: `ret" text="${SECRET}" />\n`, + }; + let caught: AppError | undefined; + try { + await captureAndroidSnapshotWithHelper({ + adb: async () => await runCmd(process.execPath, ['-e', script], { timeoutMs: 600, env }), + }); + } catch (error) { + caught = error as AppError; + } + + assert.ok(caught instanceof AppError); + const wire = normalizeError(caught); + assertPrivate(JSON.stringify(wire)); + assert.equal('stdout' in (caught.details ?? {}), false); + assert.equal('stderr' in (caught.details ?? {}), false); + assert.equal(caught.details?.timeoutMs, 600); + assert.equal(exitResultOf(caught.details ?? {}).statusCode, 1); +}); + +test('a session that exits while its streams stay open still fails startup promptly', async () => { + const child = new FakeAndroidProcess(); + const controller = new AbortController(); + const failure = rejection( + waitForAndroidSnapshotHelperSessionReady( + child, + 5_000, + controller.signal, + recordInstrumentationProcess(child), + ), + ); + child.stdout.write('INSTRUMENTATION_ABORTED: System has crashed.\n'); + await pause(); + child.exitCode = 1; + child.emit('exit', 1, null); + + const startedAt = Date.now(); + const error = await failure; + + assert.ok(Date.now() - startedAt < 2_000); + assert.equal(exitResultOf(error.details ?? {}).exitCode, 1); + assert.equal(exitResultOf(error.details ?? {}).aborted, 'System has crashed.'); +}); + +test('a failed live session is retired without waiting for an exit', async () => { + const processes: FakeAndroidProcess[] = []; + const provider = createSessionProvider({ calls: [], processes, responseMode: 'malformed' }); + + const startedAt = Date.now(); + const output = await captureAndroidSnapshotWithHelperSession({ + adb: provider.exec, + adbProvider: provider, + deviceKey: 'android:emulator-5556', + }); + + assert.equal(output, undefined); + assert.ok(Date.now() - startedAt < 200, `retirement took ${Date.now() - startedAt}ms`); +}); diff --git a/packages/platform-android/src/__tests__/snapshot-helper.test.ts b/packages/platform-android/src/__tests__/snapshot-helper.test.ts index cc6e8ff820..a78ff8d0c9 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper.test.ts @@ -701,14 +701,14 @@ test('captureAndroidSnapshotWithHelper gives adb command overhead beyond helper assert.equal(commandTimeoutMs, 13000); }); -test('captureAndroidSnapshotWithHelper wraps unparseable failed output with adb details', async () => { +test('captureAndroidSnapshotWithHelper wraps unparseable failed output with the instrumentation result', async () => { await assert.rejects( () => captureAndroidSnapshotWithHelper({ adb: async () => ({ exitCode: 1, - stdout: '', - stderr: 'instrumentation failed', + stdout: 'INSTRUMENTATION_ABORTED: instrumentation failed', + stderr: 'raw stderr text', }), }), (error) => { @@ -716,11 +716,13 @@ test('captureAndroidSnapshotWithHelper wraps unparseable failed output with adb (error as Error).message, 'Android snapshot helper failed before returning parseable output', ); - assert.equal((error as { details?: Record }).details?.exitCode, 1); - assert.equal( - (error as { details?: Record }).details?.stderr, - 'instrumentation failed', - ); + const details = (error as { details?: Record }).details; + assert.equal(details?.exitCode, 1); + assert.deepEqual(details?.instrumentation, { + exitCode: 1, + aborted: 'instrumentation failed', + }); + assert.equal('stderr' in (details ?? {}), false); return true; }, ); diff --git a/packages/platform-android/src/__tests__/snapshot.test.ts b/packages/platform-android/src/__tests__/snapshot.test.ts index 15d063a768..431b25cf56 100644 --- a/packages/platform-android/src/__tests__/snapshot.test.ts +++ b/packages/platform-android/src/__tests__/snapshot.test.ts @@ -907,7 +907,7 @@ test('snapshotAndroid emits helper failure diagnostics', async () => { } if (args[0] === 'shell' && args[1] === 'am' && args[2] === 'force-stop') return { exitCode: 0, stdout: '', stderr: '' }; - return { exitCode: 1, stdout: '', stderr: 'helper unavailable' }; + return { exitCode: 1, stdout: 'INSTRUMENTATION_ABORTED: helper unavailable', stderr: '' }; }; const diagnostics = await captureDiagnostics( @@ -1134,7 +1134,7 @@ test('snapshotAndroid re-probes helper install after helper capture failure', as (error: unknown) => { assert(error instanceof AppError); assert.equal(error.code, 'COMMAND_FAILED'); - assert.match(error.message, /instrumentation failed/); + assert.match(error.message, /failed before returning parseable output/); return true; }, ); diff --git a/packages/platform-android/src/instrumentation-helper.ts b/packages/platform-android/src/instrumentation-helper.ts index 775461c53e..14540b41ce 100644 --- a/packages/platform-android/src/instrumentation-helper.ts +++ b/packages/platform-android/src/instrumentation-helper.ts @@ -1,4 +1,3 @@ -import { REDACTED_STRING_MAX_LENGTH, redactDiagnosticText } from '@agent-device/kernel/redaction'; import type { AndroidAdbProcess } from './adb-executor.ts'; // Shared primitives for the Android instrumentation helpers (snapshot + multi-touch). // Both helpers drive `am instrument -w` and parse the resulting @@ -120,9 +119,9 @@ export function readInstrumentationResultBoolean(value: string | undefined): boo /** * What an `am instrument` run that returned no usable report says about its own end. * - * Private by construction: stdout contributes only the protocol fields parsed below and never a line - * of its own, so a snapshot payload or raw XML the helper printed has no way in. The only free text - * is a bounded stderr tail, from which lines holding snapshot content are removed. + * Private by construction: no line of either stream is copied. The fields below are parsed from + * stdout protocol records, plus the startup-failure component from a strict whole-line envelope on + * either stream. A snapshot payload or raw XML the helper printed has no way in. */ export type AndroidInstrumentationExitResult = { /** Host process exit status; `null` when a signal ended it. */ @@ -134,37 +133,24 @@ export type AndroidInstrumentationExitResult = { statusCode?: number; /** `INSTRUMENTATION_ABORTED`: the system ended the run before it reported. */ aborted?: string; - /** `INSTRUMENTATION_FAILED`: the run never started. */ + /** The component of `INSTRUMENTATION_FAILED`: the run never started. */ failed?: string; shortMsg?: string; longMsg?: string; error?: string; stack?: string; - /** - * The end of stderr after secrets are redacted, held to the length a redacted diagnostic string - * keeps so the cause at the end is what survives the boundary. - */ - stderrTail: string; - /** Stderr was cut, or a line of it was removed as snapshot content. */ - stderrTruncated: boolean; - /** A line of either stream held snapshot content and was dropped. */ - snapshotContentRemoved?: true; }; -const STDERR_WINDOW_MAX_CHARS = 16_384; -const STDOUT_LINE_MAX_CHARS = 4_096; -// A line is held to a few times the tail that survives the diagnostic boundary. -const STDERR_LINE_MAX_CHARS = 1_024; +const LINE_MAX_CHARS = 4_096; const MESSAGE_FIELD_MAX_CHARS = 2_048; -const SNAPSHOT_OMITTED = ''; -// A UiAutomator hierarchy dump prints `; - wait: Promise; + /** A background run reports no `signal`: its caller owns the child and reads its exit directly. */ + wait: Promise>; }; type ExecDetachedOptions = ExecOptions & { @@ -270,13 +271,14 @@ function runSpawnedCommand( reject(failure); return; } - resolve({ - stdout, - stderr, - exitCode: finalExitCode, - stdoutBuffer: stdoutChunks ? Buffer.concat(stdoutChunks) : undefined, - ...endedBySignal(child), - }); + resolve( + execResult(child, { + stdout, + stderr, + exitCode: finalExitCode, + stdoutBuffer: stdoutChunks ? Buffer.concat(stdoutChunks) : undefined, + }), + ); } // A deadline that fires after the child exited on its own still has to settle: the // group kill that would have ended the pipe holder can no longer run through a child @@ -339,9 +341,9 @@ function runSpawnedCommand( }); } -/** Returns the `signal` field when a signal terminated the child process. */ -function endedBySignal(child: { signalCode: NodeJS.Signals | null }): Pick { - return child.signalCode ? { signal: child.signalCode } : {}; +/** Returns the result with `signal` added when a signal terminated the process. */ +function execResult(ended: { signalCode: string | null }, result: ExecResult): ExecResult { + return ended.signalCode ? { ...result, signal: ended.signalCode } : result; } export async function whichCmd(cmd: string, env: EnvMap): Promise { @@ -457,13 +459,7 @@ export function runCmdSync( throw createExitError(executable, cmd, args, exitCode, stdout, stderr); } - return { - stdout, - stderr, - exitCode, - stdoutBuffer, - ...(result.signal ? { signal: result.signal } : {}), - }; + return execResult({ signalCode: result.signal }, { stdout, stderr, exitCode, stdoutBuffer }); } export function runCmdDetached( @@ -538,7 +534,7 @@ export function runCmdBackground( }); } - const wait = new Promise((resolve, reject) => { + const wait = new Promise>((resolve, reject) => { let settled = false; function finish(event: 'error' | 'exit'): boolean { if (settled) return false; @@ -574,12 +570,7 @@ export function runCmdBackground( reject(failure); return; } - resolve({ - stdout, - stderr, - exitCode: finalExitCode, - ...endedBySignal(child), - }); + resolve({ stdout, stderr, exitCode: finalExitCode }); } const settlement = createCommandKillSettlement({ killProcessTree: () => killProcessTree(child, options), diff --git a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts index dacbaf5d3e..249157f6cd 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import { promises as fs } from 'node:fs'; import path from 'node:path'; -import { afterEach, beforeEach, test } from 'vitest'; +import { afterEach, beforeEach, test, vi } from 'vitest'; import { withDiagnosticsScope } from '@agent-device/host-kit/diagnostics'; import { captureAndroidSnapshotWithHelper, @@ -322,9 +322,9 @@ test('stdout is never copied: only parsed protocol fields reach the details', as assert.ok(!JSON.stringify(error.details).includes('some unrelated stdout line')); }); -test('a sensitive value never reaches the details or the log wherever the helper printed it', async () => { +test('a sensitive value never reaches the instrumentation detail or the log wherever the helper printed it', async () => { const quoted = `${'p'.repeat(1_015)}password="${SECRET}" trailing`; - const runs: FailedRun[] = [ + const onStdout: FailedRun[] = [ // A complete encoded payload. { exitCode: 1, stdout: `${encodedPayload(SECRET_XML)}\nINSTRUMENTATION_STATUS_CODE: 1` }, // An unknown status key carrying raw XML. @@ -336,36 +336,15 @@ test('a sensitive value never reaches the details or the log wherever the helper }, // A tagless fragment as a free line. { exitCode: 1, stdout: `ret" text="${SECRET}" bounds="[0,0][1,1]" />\n` }, - // Fragments on stderr, including a role-description attribute and a quoted value whose key a - // line cut would have removed. - { exitCode: 1, stdout: '', stderr: `ret" text="${SECRET}" bounds="[0,0][1,1]" />` }, - { exitCode: 1, stdout: '', stderr: `ret" role-description="${SECRET}" />` }, - { exitCode: 1, stdout: '', stderr: quoted }, - { exitCode: 1, stdout: '', stderr: `${'a'.repeat(30_000)}${SECRET_XML}` }, - // A hierarchy cut off in front of a startup-failure marker. - { - exitCode: 1, - stdout: '', - stderr: ` { await inScope(logPath, async () => { await assert.rejects( snapshotAndroid(emulator, { - helperAdb: helperFailingWith(withKill), + helperAdb: helperFailingWith({ ...run, stderr: 'am: Killed' }), helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, }), ); @@ -374,6 +353,38 @@ test('a sensitive value never reaches the details or the log wherever the helper assert.ok(log.includes('android_snapshot_helper_failed')); assertPrivate(log); } + + // The failure message and `stderr` detail are unchanged from before this diagnostic existed, so + // for stderr only the new `instrumentation` detail and its request-log entry are asserted. + const onStderr = [ + `ret" text="${SECRET}" bounds="[0,0][1,1]" />`, + `ret" role-description="${SECRET}" />`, + quoted, + `${'a'.repeat(30_000)}${SECRET_XML}`, + ` { + await inScope(logPath, async () => { + await assert.rejects( + snapshotAndroid(emulator, { + helperAdb: helperFailingWith(run), + helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, + }), + ); + }); + }); + assertPrivate( + JSON.stringify(diagnosticData(log, 'android_snapshot_helper_failed').instrumentation), + ); + } }); test('a session exit never leaks a sensitive value split across chunks', async () => { @@ -471,7 +482,7 @@ test('a nonzero exit after parseable helper output keeps no raw stream in its de assertPrivate(JSON.stringify(normalizeError(error))); assert.equal(error.details?.stdout, ''); - assert.equal(error.details?.stderr, ''); + assert.equal(error.details?.stderr, 'am: Killed'); assert.equal(error.details?.exitCode, 1); }); @@ -703,9 +714,9 @@ test('a helper that prints chunks and then times out in the executor leaks no st assert.ok(caught instanceof AppError); const wire = normalizeError(caught); - assertPrivate(JSON.stringify(wire)); + assertPrivate(JSON.stringify({ ...wire.details, stderr: undefined })); assert.equal('stdout' in (caught.details ?? {}), false); - assert.equal('stderr' in (caught.details ?? {}), false); + assert.ok(typeof caught.details?.stderr === 'string'); assert.equal(caught.details?.timeoutMs, 600); assert.equal(exitResultOf(caught.details ?? {}).statusCode, 1); }); @@ -734,17 +745,21 @@ test('a session that exits while its streams stay open still fails startup promp assert.equal(exitResultOf(error.details ?? {}).aborted, 'System has crashed.'); }); -test('a failed live session is retired without waiting for an exit', async () => { +test('a failed live session is retired without waiting on a timer for an exit', async () => { const processes: FakeAndroidProcess[] = []; const provider = createSessionProvider({ calls: [], processes, responseMode: 'malformed' }); - const startedAt = Date.now(); - const output = await captureAndroidSnapshotWithHelperSession({ - adb: provider.exec, - adbProvider: provider, - deviceKey: 'android:emulator-5556', - }); - - assert.equal(output, undefined); - assert.ok(Date.now() - startedAt < 200, `retirement took ${Date.now() - startedAt}ms`); + // With timers frozen, any retirement step that waits on a timer for exit evidence never ends. + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + try { + const output = await captureAndroidSnapshotWithHelperSession({ + adb: provider.exec, + adbProvider: provider, + deviceKey: 'android:emulator-5556', + }); + assert.equal(output, undefined); + } finally { + vi.useRealTimers(); + } + assert.equal(processes[0]?.killed, true); }); diff --git a/packages/platform-android/src/__tests__/snapshot-helper.test.ts b/packages/platform-android/src/__tests__/snapshot-helper.test.ts index a78ff8d0c9..cc6e8ff820 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper.test.ts @@ -701,14 +701,14 @@ test('captureAndroidSnapshotWithHelper gives adb command overhead beyond helper assert.equal(commandTimeoutMs, 13000); }); -test('captureAndroidSnapshotWithHelper wraps unparseable failed output with the instrumentation result', async () => { +test('captureAndroidSnapshotWithHelper wraps unparseable failed output with adb details', async () => { await assert.rejects( () => captureAndroidSnapshotWithHelper({ adb: async () => ({ exitCode: 1, - stdout: 'INSTRUMENTATION_ABORTED: instrumentation failed', - stderr: 'raw stderr text', + stdout: '', + stderr: 'instrumentation failed', }), }), (error) => { @@ -716,13 +716,11 @@ test('captureAndroidSnapshotWithHelper wraps unparseable failed output with the (error as Error).message, 'Android snapshot helper failed before returning parseable output', ); - const details = (error as { details?: Record }).details; - assert.equal(details?.exitCode, 1); - assert.deepEqual(details?.instrumentation, { - exitCode: 1, - aborted: 'instrumentation failed', - }); - assert.equal('stderr' in (details ?? {}), false); + assert.equal((error as { details?: Record }).details?.exitCode, 1); + assert.equal( + (error as { details?: Record }).details?.stderr, + 'instrumentation failed', + ); return true; }, ); diff --git a/packages/platform-android/src/__tests__/snapshot.test.ts b/packages/platform-android/src/__tests__/snapshot.test.ts index 431b25cf56..15d063a768 100644 --- a/packages/platform-android/src/__tests__/snapshot.test.ts +++ b/packages/platform-android/src/__tests__/snapshot.test.ts @@ -907,7 +907,7 @@ test('snapshotAndroid emits helper failure diagnostics', async () => { } if (args[0] === 'shell' && args[1] === 'am' && args[2] === 'force-stop') return { exitCode: 0, stdout: '', stderr: '' }; - return { exitCode: 1, stdout: 'INSTRUMENTATION_ABORTED: helper unavailable', stderr: '' }; + return { exitCode: 1, stdout: '', stderr: 'helper unavailable' }; }; const diagnostics = await captureDiagnostics( @@ -1134,7 +1134,7 @@ test('snapshotAndroid re-probes helper install after helper capture failure', as (error: unknown) => { assert(error instanceof AppError); assert.equal(error.code, 'COMMAND_FAILED'); - assert.match(error.message, /failed before returning parseable output/); + assert.match(error.message, /instrumentation failed/); return true; }, ); diff --git a/packages/platform-android/src/snapshot-helper-capture.ts b/packages/platform-android/src/snapshot-helper-capture.ts index 916ff22b1a..1c55463ba3 100644 --- a/packages/platform-android/src/snapshot-helper-capture.ts +++ b/packages/platform-android/src/snapshot-helper-capture.ts @@ -113,7 +113,7 @@ export async function captureAndroidSnapshotWithHelper( 'COMMAND_FAILED', 'Android snapshot helper failed', execFailureDetails( - { stdout: '', stderr: '', exitCode: result.exitCode }, + { stdout: '', stderr: result.stderr, exitCode: result.exitCode }, { helper: output.metadata, instrumentation }, ), ); @@ -122,15 +122,13 @@ export async function captureAndroidSnapshotWithHelper( } /** - * An executor failure (a deadline, a spawn error) carries the helper's raw streams, which can hold - * the snapshot it was printing. They are replaced by the parsed instrumentation result; the typed - * failure metadata stays. + * An executor failure (a deadline, a spawn error) carries the helper's raw stdout, which can hold the + * snapshot it was printing. It is replaced by the parsed instrumentation result; `stderr` and the + * typed failure metadata stay. */ function withoutRawStreams(error: unknown): unknown { - if (!(error instanceof AppError) || !error.details) return error; - const { stdout, stderr, ...details } = error.details; - if (stdout === undefined && stderr === undefined) return error; - const exitCode = typeof details.exitCode === 'number' ? details.exitCode : null; + if (!(error instanceof AppError) || error.details?.stdout === undefined) return error; + const { stdout, ...details } = error.details; return new AppError( error.code, error.message, @@ -138,8 +136,8 @@ function withoutRawStreams(error: unknown): unknown { ...details, instrumentation: readInstrumentationExitResult({ stdout: typeof stdout === 'string' ? stdout : '', - stderr: typeof stderr === 'string' ? stderr : '', - exitCode, + stderr: typeof details.stderr === 'string' ? details.stderr : '', + exitCode: typeof details.exitCode === 'number' ? details.exitCode : null, }), }, error.cause, @@ -252,8 +250,8 @@ function readAndroidSnapshotHelperOutput( if (error instanceof AppError && isExplicitHelperFailure(error.details?.helper)) throw error; // exec-guard-allow: reachable at exit 0 (helper output unparseable); the // message already branches on the exit code. - // The raw output is not copied into the details: it can hold the snapshot the helper was - // printing, so only the parsed instrumentation result is kept. + // Stdout is not copied into the details: it can hold the snapshot the helper was printing. + // The new `instrumentation` detail is parsed fields only; `stderr` stays as it always was. const instrumentation = readInstrumentationExitResult(result); throw new AppError( 'COMMAND_FAILED', @@ -261,6 +259,7 @@ function readAndroidSnapshotHelperOutput( ? 'Android snapshot helper output could not be parsed' : 'Android snapshot helper failed before returning parseable output', { + stderr: result.stderr, exitCode: result.exitCode, instrumentation, ...androidCaptureFailureReasonDetail( diff --git a/packages/platform-android/src/snapshot-helper-session-lifecycle.ts b/packages/platform-android/src/snapshot-helper-session-lifecycle.ts index 699a2a91d4..d1eb481cec 100644 --- a/packages/platform-android/src/snapshot-helper-session-lifecycle.ts +++ b/packages/platform-android/src/snapshot-helper-session-lifecycle.ts @@ -473,24 +473,7 @@ export async function stopAndroidSnapshotHelperSession( const session = sessions().get(deviceKey); if (!session) return false; sessions().delete(deviceKey); - // Read before teardown, once the streams have drained: a process this stop ends has no cause of - // its own to report, and one that already exited may still be flushing its last output. - if (hasAndroidSnapshotHelperProcessEnded(session.process)) { - await waitForAndroidSnapshotHelperProcessExit( - session.processRecord.closed, - ANDROID_SNAPSHOT_HELPER_HOST_PROCESS_EXIT_GRACE_MS, - ); - } - const exitedOnItsOwn = hasAndroidSnapshotHelperProcessEnded(session.process) - ? (session.processRecord.exitResult() ?? session.processRecord.current()) - : undefined; - if (exitedOnItsOwn) { - emitDiagnostic({ - level: 'warn', - phase: 'android_snapshot_helper_session_exited', - data: { deviceKey, capturedCount: session.capturedCount, instrumentation: exitedOnItsOwn }, - }); - } + await reportSessionExitedOnItsOwn(deviceKey, session); const processExit = observeAndroidSnapshotHelperProcessExit(session.process); const force = options.force === true || options.signal?.aborted === true; const graceful = await requestGracefulSessionExit(session, processExit, force, options.signal); @@ -561,6 +544,32 @@ export async function stopAndroidSnapshotHelperSession( return true; } +/** + * Records the exit result of a session whose helper process ended before this stop. Read before + * teardown, so a process this stop ends has no cause of its own to report; a process that exited may + * still be flushing its last output, so its streams get a short drain. A running process costs + * nothing here. + */ +async function reportSessionExitedOnItsOwn( + deviceKey: string, + session: AndroidSnapshotHelperSession, +): Promise { + if (!hasAndroidSnapshotHelperProcessEnded(session.process)) return; + await waitForAndroidSnapshotHelperProcessExit( + session.processRecord.closed, + ANDROID_SNAPSHOT_HELPER_HOST_PROCESS_EXIT_GRACE_MS, + ); + emitDiagnostic({ + level: 'warn', + phase: 'android_snapshot_helper_session_exited', + data: { + deviceKey, + capturedCount: session.capturedCount, + instrumentation: session.processRecord.exitResult() ?? session.processRecord.current(), + }, + }); +} + async function requestGracefulSessionExit( session: AndroidSnapshotHelperSession, processExit: AndroidSnapshotHelperProcessExit, diff --git a/packages/platform-android/src/snapshot.ts b/packages/platform-android/src/snapshot.ts index 9f23f27bc5..baa411529e 100644 --- a/packages/platform-android/src/snapshot.ts +++ b/packages/platform-android/src/snapshot.ts @@ -7,7 +7,6 @@ import { import path from 'node:path'; import { isPositiveFiniteRect, unionRects } from '@agent-device/kernel/rect'; import { emitDiagnostic, withDiagnosticTimer } from '@agent-device/host-kit/diagnostics'; -import type { AndroidInstrumentationExitResult } from './instrumentation-helper.ts'; import type { SnapshotOptions as InteractorSnapshotOptions } from '@agent-device/contracts/interactor-types'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { @@ -711,11 +710,6 @@ function formatAndroidSnapshotHelperFailureReason(error: unknown): string { return `${normalized.message}: ${helperMessage}`; } if (helperMessage) return helperMessage; - const instrumentation = normalized.details?.instrumentation as - | AndroidInstrumentationExitResult - | undefined; - const cause = instrumentation?.aborted || instrumentation?.shortMsg || instrumentation?.failed; - if (cause) return `${normalized.message}: ${cause}`; const stderr = typeof normalized.details?.stderr === 'string' ? normalized.details.stderr.trim() : ''; const firstLine = stderr.split(/\r?\n/).find((line) => line.trim()); From 63ee8b59c2ef3216cad787949a99ed6d0478a8a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 28 Oct 2026 15:58:13 +0100 Subject: [PATCH 15/18] fix(android): withhold message fields that hold snapshot markers and keep helper-bearing errors as on main Co-Authored-By: Claude Sonnet 5.5 --- .../snapshot-helper-exit-result.test.ts | 108 +++++++++++++++--- .../src/instrumentation-helper.ts | 61 +++++++++- .../src/snapshot-helper-capture.ts | 18 +-- 3 files changed, 156 insertions(+), 31 deletions(-) diff --git a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts index 249157f6cd..353193c9d8 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts @@ -15,6 +15,7 @@ import { resetAndroidSnapshotHelperSessions } from '../snapshot-helper-session-l import { resetAndroidSnapshotHelperRetirements } from '../snapshot-helper-retirement.ts'; import { snapshotAndroid } from '../snapshot.ts'; import { AppError, normalizeError } from '@agent-device/kernel/errors'; +import { readAndroidCaptureFailureReason } from '@agent-device/contracts/android-snapshot-quality'; import type { DeviceInfo } from '@agent-device/kernel/device'; import type { AndroidAdbExecutor } from '../snapshot-helper-types.ts'; import type { AndroidAdbProvider } from '../adb-executor.ts'; @@ -55,6 +56,7 @@ const CRASHED_RUN = [ type FailedRun = { exitCode: number; stdout: string; stderr?: string; signal?: string }; type ExitResultFields = { + withheldFields?: string[]; exitCode: number | null; signal?: string; resultCode?: number; @@ -141,6 +143,10 @@ function diagnosticData(log: string, phase: string): Record { return event.data ?? {}; } +function helperOf(error: AppError): Record { + return (error.details?.helper ?? {}) as Record; +} + function exitResultOf(fields: Record): ExitResultFields { return fields.instrumentation as ExitResultFields; } @@ -336,10 +342,21 @@ test('a sensitive value never reaches the instrumentation detail or the log wher }, // A tagless fragment as a free line. { exitCode: 1, stdout: `ret" text="${SECRET}" bounds="[0,0][1,1]" />\n` }, + // A fragment as a continuation of an open message, and inline in each retained value. + { + exitCode: 0, + stdout: `INSTRUMENTATION_RESULT: longMsg=UiAutomation failed\nret" role-description="${SECRET}" />\nINSTRUMENTATION_CODE: 0\n`, + }, + { exitCode: 1, stdout: `INSTRUMENTATION_RESULT: shortMsg=\n` }, + { exitCode: 1, stdout: `INSTRUMENTATION_STATUS: Error=\n` }, + { exitCode: 1, stdout: `INSTRUMENTATION_STATUS: stack=boom\n\tat x hint="${SECRET}"\n` }, + { exitCode: 1, stdout: `INSTRUMENTATION_ABORTED: \n` }, + { exitCode: 1, stdout: `INSTRUMENTATION_ABORTED: ret" content-desc="${SECRET}"\n` }, ]; for (const run of onStdout) { const error = await captureError({ ...run, stderr: 'am: Killed' }); assertPrivate(JSON.stringify(normalizeError(error))); + // Stderr exposure stays as on main (#4026); this PR asserts privacy on everything it adds. const { log } = await withLog(async (logPath) => { await inScope(logPath, async () => { await assert.rejects( @@ -498,7 +515,77 @@ test('stderr chunks do not disturb a partial stdout record', async () => { assert.equal(exit.resultCode, 0); }); -test('a helper failure that reports its own result keeps that report without a second record', async () => { +test('a message holding snapshot markers is withheld whole and named', async () => { + const result = await captureExitResult({ + exitCode: 0, + stdout: [ + 'INSTRUMENTATION_RESULT: shortMsg=Process crashed.', + 'INSTRUMENTATION_RESULT: longMsg=UiAutomation failed', + `ret" role-description="${SECRET}" />`, + 'INSTRUMENTATION_STATUS: stack=java.lang.IllegalStateException', + '\tat com.callstack.Main.(Main.java:7)', + 'INSTRUMENTATION_ABORTED: ', + 'INSTRUMENTATION_CODE: 0', + ].join('\n'), + }); + + assert.equal(result.shortMsg, 'Process crashed.'); + assert.equal(result.longMsg, undefined); + assert.equal(result.aborted, undefined); + assert.equal( + result.stack, + 'java.lang.IllegalStateException\n\tat com.callstack.Main.(Main.java:7)', + ); + assert.deepEqual(result.withheldFields, ['aborted', 'longMsg']); +}); + +test('a streamed session withholds a message whose fragment arrives in chunks', async () => { + const { exit, log } = await sessionExit(async (process) => { + process.stdout.write('INSTRUMENTATION_RESULT: longMsg=UiAutomation failed\nret" role-descr'); + await pause(); + process.stdout.write(`iption="${SECRET}" />\nINSTRUMENTATION_CODE: 0\n`); + }); + + assertPrivate(log); + assert.equal(exit.longMsg, undefined); + assert.deepEqual(exit.withheldFields, ['longMsg']); + assert.equal(exit.resultCode, 0); +}); + +test('a helper report without ok keeps the message, metadata and typed reason it had on main', async () => { + const error = await captureError({ + exitCode: 0, + stdout: [ + 'INSTRUMENTATION_RESULT: agentDeviceProtocol=android-snapshot-helper-v1', + 'INSTRUMENTATION_RESULT: errorType=java.util.concurrent.TimeoutException', + 'INSTRUMENTATION_RESULT: message=uiautomator unavailable', + ].join('\n'), + }); + + assert.equal(error.message, 'uiautomator unavailable'); + assert.equal(readAndroidCaptureFailureReason(error), 'accessibility-timeout'); + assert.equal(helperOf(error).message, 'uiautomator unavailable'); + assert.equal('stderr' in (error.details ?? {}), false); + assert.equal(exitResultOf(error.details ?? {}).exitCode, 0); +}); + +test('an ok=true report with no XML chunks keeps main message and gains the instrumentation detail', async () => { + const error = await captureError({ + exitCode: 1, + stdout: [ + 'INSTRUMENTATION_RESULT: agentDeviceProtocol=android-snapshot-helper-v1', + 'INSTRUMENTATION_RESULT: ok=true', + 'INSTRUMENTATION_CODE: -1', + ].join('\n'), + stderr: 'am: Killed', + }); + + assert.equal(error.message, 'Android snapshot helper did not return XML chunks'); + assert.equal('stderr' in (error.details ?? {}), false); + assert.equal(exitResultOf(error.details ?? {}).resultCode, -1); +}); + +test('a helper failure that reports its own result keeps that report and gains the instrumentation detail', async () => { const error = await captureError({ exitCode: 0, stdout: [ @@ -509,7 +596,8 @@ test('a helper failure that reports its own result keeps that report without a s ].join('\n'), }); - assert.equal(error.details?.instrumentation, undefined); + assert.equal(helperOf(error).ok, 'false'); + assert.equal(exitResultOf(error.details ?? {}).resultCode, -1); }); test('a session that exits before it is ready records its exit status and the output it printed', async () => { @@ -641,22 +729,6 @@ test('a malformed helper report names its size, never the snapshot text it held' ); }); -test('a helper report with ok=true but no XML chunks still carries the instrumentation result', async () => { - const error = await captureError({ - exitCode: 1, - stdout: [ - 'INSTRUMENTATION_RESULT: agentDeviceProtocol=android-snapshot-helper-v1', - 'INSTRUMENTATION_RESULT: ok=true', - 'INSTRUMENTATION_CODE: -1', - ].join('\n'), - stderr: 'am: Killed', - }); - - const result = exitResultOf(error.details ?? {}); - assert.equal(result.exitCode, 1); - assert.equal(result.resultCode, -1); -}); - test('a ready timeout in the middle of a record leaves the final fields intact', async () => { const child = new FakeAndroidProcess(); const record = recordInstrumentationProcess(child); diff --git a/packages/platform-android/src/instrumentation-helper.ts b/packages/platform-android/src/instrumentation-helper.ts index 14540b41ce..6d7dd4b38a 100644 --- a/packages/platform-android/src/instrumentation-helper.ts +++ b/packages/platform-android/src/instrumentation-helper.ts @@ -139,6 +139,8 @@ export type AndroidInstrumentationExitResult = { longMsg?: string; error?: string; stack?: string; + /** Fields dropped whole because their value held a snapshot marker. */ + withheldFields?: Array; }; const LINE_MAX_CHARS = 4_096; @@ -151,6 +153,49 @@ const FAILED_ENVELOPES = [ new RegExp(`^android\\.util\\.AndroidException: INSTRUMENTATION_FAILED: ${COMPONENT}$`), new RegExp(`^onError: commandError=(?:true|false) message=INSTRUMENTATION_FAILED: ${COMPONENT}$`), ]; +// Attributes `AccessibilityTreeXml` writes. A value that holds one in `name="` form, a tag +// bracket, or a payload record is withheld whole rather than filtered. +const SNAPSHOT_ATTRIBUTES = [ + 'bounds', + 'checked', + 'class', + 'clickable', + 'content-desc', + 'drawing-order', + 'editable', + 'enabled', + 'focusable', + 'focused', + 'heading', + 'hint', + 'hint-showing', + 'index', + 'package', + 'password', + 'resource-id', + 'role-description', + 'scrollable', + 'selected', + 'selection-end', + 'selection-start', + 'text', + 'visible-to-user', + 'window-active', + 'window-focused', + 'window-index', + 'window-layer', + 'window-region-rect', + 'window-type', +]; +const SNAPSHOT_MARKER_RE = new RegExp( + `<|/>|payloadBase64|(?', '').replaceAll('', '')); +} + const ABORTED_RE = /^INSTRUMENTATION_ABORTED:[ \t]?(.*)$/; const PROTOCOL_RECORD_RE = /^INSTRUMENTATION_(?:STATUS|STATUS_CODE|RESULT|CODE|ABORTED|FAILED):/; const VALUE_RECORD_RE = /^INSTRUMENTATION_(?:STATUS|RESULT): ([^=]*)=(.*)$/; @@ -199,6 +244,11 @@ function createInstrumentationOutput() { Record<'resultCode' | 'statusCode', number> > = {}; let open: MessageField | undefined; + const withheld = new Set(); + const checked = (field: MessageField | 'aborted', text: string) => { + if (holdsSnapshotMarker(text)) withheld.add(field); + return text; + }; const readFailed = (line: string, cut: boolean): boolean => { if (cut) return false; for (const envelope of FAILED_ENVELOPES) { @@ -216,7 +266,7 @@ function createInstrumentationOutput() { } else if (PROTOCOL_RECORD_RE.test(line)) { open = readProtocolRecord(line); } else if (open) { - fields[open] = appendLine(fields[open], line); + fields[open] = appendLine(fields[open], checked(open, line)); } }; /** Reads one protocol record; answers the message field whose value it opens, if any. */ @@ -231,13 +281,13 @@ function createInstrumentationOutput() { } const aborted = ABORTED_RE.exec(line); if (aborted) { - fields.aborted ??= (aborted[1] ?? '').trim(); + fields.aborted ??= checked('aborted', (aborted[1] ?? '').trim()); return undefined; } const record = VALUE_RECORD_RE.exec(line); const field = record ? messageFieldOf(record[1]!) : undefined; if (!field || fields[field] !== undefined) return undefined; - fields[field] = (record![2] ?? '').slice(0, MESSAGE_FIELD_MAX_CHARS); + fields[field] = checked(field, record![2] ?? '').slice(0, MESSAGE_FIELD_MAX_CHARS); return field; }; const stdout = createLineAssembler(readStdoutLine); @@ -255,7 +305,9 @@ function createInstrumentationOutput() { stdout.end(); stderr.end(); } - const { aborted, failed, shortMsg, longMsg, error, stack, resultCode, statusCode } = fields; + const shown = { ...fields }; + for (const field of withheld) delete shown[field]; + const { aborted, failed, shortMsg, longMsg, error, stack, resultCode, statusCode } = shown; return { exitCode: exitCode ?? null, ...(signal ? { signal } : {}), @@ -267,6 +319,7 @@ function createInstrumentationOutput() { ...(longMsg !== undefined ? { longMsg: longMsg.trimEnd() } : {}), ...(error !== undefined ? { error: error.trimEnd() } : {}), ...(stack !== undefined ? { stack: stack.trimEnd() } : {}), + ...(withheld.size > 0 ? { withheldFields: [...withheld].sort() } : {}), }; }, }; diff --git a/packages/platform-android/src/snapshot-helper-capture.ts b/packages/platform-android/src/snapshot-helper-capture.ts index 1c55463ba3..c1402b0186 100644 --- a/packages/platform-android/src/snapshot-helper-capture.ts +++ b/packages/platform-android/src/snapshot-helper-capture.ts @@ -245,9 +245,15 @@ function readAndroidSnapshotHelperOutput( return parseAndroidSnapshotHelperOutput(`${result.stdout}\n${result.stderr}`); } catch (error) { // `am instrument` exits 0 after a helper that reported its own failure, so that report is the - // answer. Any other parse failure, including a report of success with no XML, has no report of - // its own and falls through to the instrumentation result below. - if (error instanceof AppError && isExplicitHelperFailure(error.details?.helper)) throw error; + // answer: its message, metadata and typed reason stay, and the instrumentation result is added. + if (error instanceof AppError && error.details?.helper) { + throw new AppError( + error.code, + error.message, + { ...error.details, instrumentation: readInstrumentationExitResult(result) }, + error.cause, + ); + } // exec-guard-allow: reachable at exit 0 (helper output unparseable); the // message already branches on the exit code. // Stdout is not copied into the details: it can hold the snapshot the helper was printing. @@ -271,12 +277,6 @@ function readAndroidSnapshotHelperOutput( } } -function isExplicitHelperFailure(helper: unknown): boolean { - return ( - typeof helper === 'object' && helper !== null && (helper as { ok?: unknown }).ok === 'false' - ); -} - export function parseAndroidSnapshotHelperOutput(output: string): AndroidSnapshotHelperOutput { const records = parseInstrumentationRecords(output); const finalResult = readFinalHelperResult(records.results); From 2e62e97b1728dad4f8d51e48591764f64868ff83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 28 Oct 2026 16:19:26 +0100 Subject: [PATCH 16/18] fix(android): withhold message fields with a generic fail-closed rule Drop the attribute list: a message value, or a continuation line, that holds an attribute-like assignment, markup, an entity, a payload record or a cut line is withheld whole and named in withheldFields. The abort message is checked like the others. Refs #4005 Co-Authored-By: Claude Sonnet 5.5 --- .../snapshot-helper-exit-result.test.ts | 59 ++++++++++++- .../src/instrumentation-helper.ts | 82 +++++++------------ 2 files changed, 86 insertions(+), 55 deletions(-) diff --git a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts index 353193c9d8..c6b0a018de 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts @@ -288,7 +288,7 @@ test('multiline protocol values keep their continuation lines until the next rec test('single-line and multiline messages are held to the message cap', async () => { const result = await captureExitResult({ exitCode: 0, - stdout: `INSTRUMENTATION_RESULT: shortMsg=${'s'.repeat(10_000)}\nINSTRUMENTATION_RESULT: longMsg=${'l'.repeat(2_000)}\n${'m'.repeat(2_000)}\nINSTRUMENTATION_CODE: 0`, + stdout: `INSTRUMENTATION_RESULT: shortMsg=${'s'.repeat(3_000)}\nINSTRUMENTATION_RESULT: longMsg=${'l'.repeat(2_000)}\n${'m'.repeat(2_000)}\nINSTRUMENTATION_CODE: 0`, }); assert.equal(result.shortMsg?.length, 2_048); @@ -352,6 +352,31 @@ test('a sensitive value never reaches the instrumentation detail or the log wher { exitCode: 1, stdout: `INSTRUMENTATION_STATUS: stack=boom\n\tat x hint="${SECRET}"\n` }, { exitCode: 1, stdout: `INSTRUMENTATION_ABORTED: \n` }, { exitCode: 1, stdout: `INSTRUMENTATION_ABORTED: ret" content-desc="${SECRET}"\n` }, + // Attributes the dump writes that an attribute list could miss, in any case or quoting. + { + exitCode: 1, + stdout: `INSTRUMENTATION_RESULT: longMsg=boom\ncan-scroll-forward="${SECRET}"\n`, + }, + { exitCode: 1, stdout: `INSTRUMENTATION_RESULT: longMsg=boom\nwindow-bounds="${SECRET}"\n` }, + { exitCode: 1, stdout: `INSTRUMENTATION_RESULT: shortMsg=ret" TEXT="${SECRET}"\n` }, + { exitCode: 1, stdout: `INSTRUMENTATION_RESULT: shortMsg=ret" Text="${SECRET}"\n` }, + { exitCode: 1, stdout: `INSTRUMENTATION_RESULT: shortMsg=ret text='${SECRET}'\n` }, + { + exitCode: 1, + stdout: `INSTRUMENTATION_RESULT: shortMsg=<node text="${SECRET}">\n`, + }, + { exitCode: 1, stdout: `INSTRUMENTATION_RESULT: shortMsg=<${SECRET}\n` }, + // The abort message continues on the next line. + { exitCode: 1, stdout: `INSTRUMENTATION_ABORTED: crashed\nret" text="${SECRET}" />\n` }, + // A private prefix in a line the 4,096-character limit cuts before any marker shows. + { + exitCode: 1, + stdout: `INSTRUMENTATION_RESULT: longMsg=${SECRET}${'x'.repeat(4_200)} text="y"\n`, + }, + { + exitCode: 1, + stdout: `INSTRUMENTATION_RESULT: longMsg=boom\n${SECRET}${'x'.repeat(4_200)} text="y"\n`, + }, ]; for (const run of onStdout) { const error = await captureError({ ...run, stderr: 'am: Killed' }); @@ -539,6 +564,38 @@ test('a message holding snapshot markers is withheld whole and named', async () assert.deepEqual(result.withheldFields, ['aborted', 'longMsg']); }); +test('a plain crash message and stack trace are kept', async () => { + const result = await captureExitResult({ + exitCode: 0, + stdout: [ + 'INSTRUMENTATION_RESULT: shortMsg=Process crashed.', + 'INSTRUMENTATION_RESULT: longMsg=java.lang.IllegalStateException: binder died', + 'INSTRUMENTATION_STATUS: stack=java.lang.IllegalStateException: binder died', + '\tat com.foo.Bar.(Bar.java:7)', + '\tat com.foo.Bar.baz(Bar.java:12)', + 'INSTRUMENTATION_ABORTED: System has crashed.', + 'INSTRUMENTATION_CODE: 0', + ].join('\n'), + }); + + assert.equal(result.withheldFields, undefined); + assert.equal(result.aborted, 'System has crashed.'); + assert.equal( + result.stack, + 'java.lang.IllegalStateException: binder died\n\tat com.foo.Bar.(Bar.java:7)\n\tat com.foo.Bar.baz(Bar.java:12)', + ); +}); + +test('a field is withheld when a line of it was cut by the line limit', async () => { + const result = await captureExitResult({ + exitCode: 0, + stdout: `INSTRUMENTATION_RESULT: longMsg=${'a'.repeat(5_000)}\nINSTRUMENTATION_CODE: 0\n`, + }); + + assert.equal(result.longMsg, undefined); + assert.deepEqual(result.withheldFields, ['longMsg']); +}); + test('a streamed session withholds a message whose fragment arrives in chunks', async () => { const { exit, log } = await sessionExit(async (process) => { process.stdout.write('INSTRUMENTATION_RESULT: longMsg=UiAutomation failed\nret" role-descr'); diff --git a/packages/platform-android/src/instrumentation-helper.ts b/packages/platform-android/src/instrumentation-helper.ts index 6d7dd4b38a..05c820f4b9 100644 --- a/packages/platform-android/src/instrumentation-helper.ts +++ b/packages/platform-android/src/instrumentation-helper.ts @@ -140,7 +140,7 @@ export type AndroidInstrumentationExitResult = { error?: string; stack?: string; /** Fields dropped whole because their value held a snapshot marker. */ - withheldFields?: Array; + withheldFields?: Array; }; const LINE_MAX_CHARS = 4_096; @@ -153,47 +153,18 @@ const FAILED_ENVELOPES = [ new RegExp(`^android\\.util\\.AndroidException: INSTRUMENTATION_FAILED: ${COMPONENT}$`), new RegExp(`^onError: commandError=(?:true|false) message=INSTRUMENTATION_FAILED: ${COMPONENT}$`), ]; -// Attributes `AccessibilityTreeXml` writes. A value that holds one in `name="` form, a tag -// bracket, or a payload record is withheld whole rather than filtered. -const SNAPSHOT_ATTRIBUTES = [ - 'bounds', - 'checked', - 'class', - 'clickable', - 'content-desc', - 'drawing-order', - 'editable', - 'enabled', - 'focusable', - 'focused', - 'heading', - 'hint', - 'hint-showing', - 'index', - 'package', - 'password', - 'resource-id', - 'role-description', - 'scrollable', - 'selected', - 'selection-end', - 'selection-start', - 'text', - 'visible-to-user', - 'window-active', - 'window-focused', - 'window-index', - 'window-layer', - 'window-region-rect', - 'window-type', -]; -const SNAPSHOT_MARKER_RE = new RegExp( - `<|/>|payloadBase64|(?]|<|>|"|&#|payloadBase64/i; -/** A JVM constructor name is not an element; every other `<` is a snapshot marker. */ +/** Only the JVM tokens `` and `` are exempt from the markup rule. */ function holdsSnapshotMarker(text: string): boolean { - return SNAPSHOT_MARKER_RE.test(text.replaceAll('', '').replaceAll('', '')); + return ( + ATTRIBUTE_LIKE_RE.test(text) || + MARKUP_RE.test(text.replaceAll('', '').replaceAll('', '')) + ); } const ABORTED_RE = /^INSTRUMENTATION_ABORTED:[ \t]?(.*)$/; @@ -203,6 +174,8 @@ const CODE_RE = /^INSTRUMENTATION_(STATUS_CODE|CODE): (-?\d+)\s*$/; // The protocol keys whose values are text, by the field they fill. const MESSAGE_FIELDS = { shortMsg: 'shortMsg', longMsg: 'longMsg', Error: 'error', stack: 'stack' }; type MessageField = 'shortMsg' | 'longMsg' | 'error' | 'stack'; +/** A field whose value can continue on the lines after its record. */ +type OpenField = MessageField | 'aborted'; type LineAssembler = { write(chunk: string): void; end(): void }; @@ -240,13 +213,13 @@ function createLineAssembler(onLine: (line: string, cut: boolean) => void): Line /** One run's output, read as two streams with a parser state each. */ function createInstrumentationOutput() { const fields: Partial< - Record & - Record<'resultCode' | 'statusCode', number> + Record & Record<'resultCode' | 'statusCode', number> > = {}; - let open: MessageField | undefined; - const withheld = new Set(); - const checked = (field: MessageField | 'aborted', text: string) => { - if (holdsSnapshotMarker(text)) withheld.add(field); + let open: OpenField | undefined; + const withheld = new Set(); + /** A line the limit cut cannot be proven clean, so its field is withheld like a marked one. */ + const checked = (field: OpenField, text: string, cut: boolean) => { + if (cut || holdsSnapshotMarker(text)) withheld.add(field); return text; }; const readFailed = (line: string, cut: boolean): boolean => { @@ -264,13 +237,13 @@ function createInstrumentationOutput() { if (readFailed(line, cut)) { open = undefined; } else if (PROTOCOL_RECORD_RE.test(line)) { - open = readProtocolRecord(line); + open = readProtocolRecord(line, cut); } else if (open) { - fields[open] = appendLine(fields[open], checked(open, line)); + fields[open] = appendLine(fields[open], checked(open, line, cut)); } }; - /** Reads one protocol record; answers the message field whose value it opens, if any. */ - const readProtocolRecord = (line: string): MessageField | undefined => { + /** Reads one protocol record; answers the field whose value it opens, if any. */ + const readProtocolRecord = (line: string, cut: boolean): OpenField | undefined => { const code = CODE_RE.exec(line); if (code) { const value = Number(code[2]); @@ -281,13 +254,14 @@ function createInstrumentationOutput() { } const aborted = ABORTED_RE.exec(line); if (aborted) { - fields.aborted ??= checked('aborted', (aborted[1] ?? '').trim()); - return undefined; + if (fields.aborted !== undefined) return undefined; + fields.aborted = checked('aborted', aborted[1] ?? '', cut).slice(0, MESSAGE_FIELD_MAX_CHARS); + return 'aborted'; } const record = VALUE_RECORD_RE.exec(line); const field = record ? messageFieldOf(record[1]!) : undefined; if (!field || fields[field] !== undefined) return undefined; - fields[field] = checked(field, record![2] ?? '').slice(0, MESSAGE_FIELD_MAX_CHARS); + fields[field] = checked(field, record![2] ?? '', cut).slice(0, MESSAGE_FIELD_MAX_CHARS); return field; }; const stdout = createLineAssembler(readStdoutLine); @@ -313,7 +287,7 @@ function createInstrumentationOutput() { ...(signal ? { signal } : {}), ...(resultCode !== undefined ? { resultCode } : {}), ...(statusCode !== undefined ? { statusCode } : {}), - ...(aborted !== undefined ? { aborted } : {}), + ...(aborted !== undefined ? { aborted: aborted.trim() } : {}), ...(failed !== undefined ? { failed } : {}), ...(shortMsg !== undefined ? { shortMsg: shortMsg.trimEnd() } : {}), ...(longMsg !== undefined ? { longMsg: longMsg.trimEnd() } : {}), From c97885f99d39463bcac251bcf168f8434450214e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 28 Oct 2026 16:33:07 +0100 Subject: [PATCH 17/18] fix(android): match entities generically and keep the abort message to one line Co-Authored-By: Claude Sonnet 5.5 --- .../snapshot-helper-exit-result.test.ts | 25 ++++++++++++++++- .../src/instrumentation-helper.ts | 27 ++++++++++--------- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts index c6b0a018de..4ea23f6649 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-exit-result.test.ts @@ -366,8 +366,12 @@ test('a sensitive value never reaches the instrumentation detail or the log wher stdout: `INSTRUMENTATION_RESULT: shortMsg=<node text="${SECRET}">\n`, }, { exitCode: 1, stdout: `INSTRUMENTATION_RESULT: shortMsg=<${SECRET}\n` }, - // The abort message continues on the next line. + // Text after the one-line abort message is not part of it. { exitCode: 1, stdout: `INSTRUMENTATION_ABORTED: crashed\nret" text="${SECRET}" />\n` }, + // Entities of any kind. + { exitCode: 1, stdout: `INSTRUMENTATION_RESULT: shortMsg=a & ${SECRET}\n` }, + { exitCode: 1, stdout: `INSTRUMENTATION_RESULT: shortMsg=it's ${SECRET}\n` }, + { exitCode: 1, stdout: `INSTRUMENTATION_RESULT: shortMsg='${SECRET}\n` }, // A private prefix in a line the 4,096-character limit cuts before any marker shows. { exitCode: 1, @@ -586,6 +590,25 @@ test('a plain crash message and stack trace are kept', async () => { ); }); +test('the abort message is one line: a plain line after it is not copied anywhere', async () => { + const stdout = 'INSTRUMENTATION_ABORTED: System has crashed.\nPLAIN_FOLLOWING_LINE\n'; + const error = await captureError({ exitCode: 1, stdout }); + const { log } = await withLog(async (logPath) => { + await inScope(logPath, async () => { + await assert.rejects( + snapshotAndroid(emulator, { + helperAdb: helperFailingWith({ exitCode: 1, stdout }), + helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, + }), + ); + }); + }); + + assert.equal(exitResultOf(error.details ?? {}).aborted, 'System has crashed.'); + assert.ok(!JSON.stringify(normalizeError(error)).includes('PLAIN_FOLLOWING_LINE')); + assert.ok(!log.includes('PLAIN_FOLLOWING_LINE')); +}); + test('a field is withheld when a line of it was cut by the line limit', async () => { const result = await captureExitResult({ exitCode: 0, diff --git a/packages/platform-android/src/instrumentation-helper.ts b/packages/platform-android/src/instrumentation-helper.ts index 05c820f4b9..58183df5d2 100644 --- a/packages/platform-android/src/instrumentation-helper.ts +++ b/packages/platform-android/src/instrumentation-helper.ts @@ -139,8 +139,11 @@ export type AndroidInstrumentationExitResult = { longMsg?: string; error?: string; stack?: string; - /** Fields dropped whole because their value held a snapshot marker. */ - withheldFields?: Array; + /** + * Fields dropped whole because their value looked like snapshot content (markup, an + * attribute-like assignment, a payload record) or because a line of it was cut at the line limit. + */ + withheldFields?: Array; }; const LINE_MAX_CHARS = 4_096; @@ -157,7 +160,7 @@ const FAILED_ENVELOPES = [ // attribute-like assignment, any markup or entity, or a payload record. The accepted cost is that a // crash message with an ordinary `key="value"` is withheld too; `withheldFields` still says it existed. const ATTRIBUTE_LIKE_RE = /[A-Za-z][\w:-]*\s*=\s*["']/; -const MARKUP_RE = /[<>]|<|>|"|&#|payloadBase64/i; +const MARKUP_RE = /[<>]|&(?:[A-Za-z][A-Za-z0-9]*|#[0-9]+|#x[0-9A-Fa-f]+);|&#|payloadBase64/i; /** Only the JVM tokens `` and `` are exempt from the markup rule. */ function holdsSnapshotMarker(text: string): boolean { @@ -174,8 +177,6 @@ const CODE_RE = /^INSTRUMENTATION_(STATUS_CODE|CODE): (-?\d+)\s*$/; // The protocol keys whose values are text, by the field they fill. const MESSAGE_FIELDS = { shortMsg: 'shortMsg', longMsg: 'longMsg', Error: 'error', stack: 'stack' }; type MessageField = 'shortMsg' | 'longMsg' | 'error' | 'stack'; -/** A field whose value can continue on the lines after its record. */ -type OpenField = MessageField | 'aborted'; type LineAssembler = { write(chunk: string): void; end(): void }; @@ -213,12 +214,13 @@ function createLineAssembler(onLine: (line: string, cut: boolean) => void): Line /** One run's output, read as two streams with a parser state each. */ function createInstrumentationOutput() { const fields: Partial< - Record & Record<'resultCode' | 'statusCode', number> + Record & + Record<'resultCode' | 'statusCode', number> > = {}; - let open: OpenField | undefined; - const withheld = new Set(); + let open: MessageField | undefined; + const withheld = new Set(); /** A line the limit cut cannot be proven clean, so its field is withheld like a marked one. */ - const checked = (field: OpenField, text: string, cut: boolean) => { + const checked = (field: MessageField | 'aborted', text: string, cut: boolean) => { if (cut || holdsSnapshotMarker(text)) withheld.add(field); return text; }; @@ -242,8 +244,8 @@ function createInstrumentationOutput() { fields[open] = appendLine(fields[open], checked(open, line, cut)); } }; - /** Reads one protocol record; answers the field whose value it opens, if any. */ - const readProtocolRecord = (line: string, cut: boolean): OpenField | undefined => { + /** Reads one protocol record; answers the message field whose value it opens, if any. */ + const readProtocolRecord = (line: string, cut: boolean): MessageField | undefined => { const code = CODE_RE.exec(line); if (code) { const value = Number(code[2]); @@ -255,8 +257,9 @@ function createInstrumentationOutput() { const aborted = ABORTED_RE.exec(line); if (aborted) { if (fields.aborted !== undefined) return undefined; + // The abort message is one line; whatever follows it is not part of it. fields.aborted = checked('aborted', aborted[1] ?? '', cut).slice(0, MESSAGE_FIELD_MAX_CHARS); - return 'aborted'; + return undefined; } const record = VALUE_RECORD_RE.exec(line); const field = record ? messageFieldOf(record[1]!) : undefined; From 5e9942f87c29ce45fa0b88595c08f0a75164a9fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 28 Oct 2026 17:57:19 +0100 Subject: [PATCH 18/18] refactor(android): drop the dead numeric-entity branches from the markup rule Co-Authored-By: Claude Sonnet 5.5 --- packages/platform-android/src/instrumentation-helper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/platform-android/src/instrumentation-helper.ts b/packages/platform-android/src/instrumentation-helper.ts index 58183df5d2..33c59867f2 100644 --- a/packages/platform-android/src/instrumentation-helper.ts +++ b/packages/platform-android/src/instrumentation-helper.ts @@ -160,7 +160,7 @@ const FAILED_ENVELOPES = [ // attribute-like assignment, any markup or entity, or a payload record. The accepted cost is that a // crash message with an ordinary `key="value"` is withheld too; `withheldFields` still says it existed. const ATTRIBUTE_LIKE_RE = /[A-Za-z][\w:-]*\s*=\s*["']/; -const MARKUP_RE = /[<>]|&(?:[A-Za-z][A-Za-z0-9]*|#[0-9]+|#x[0-9A-Fa-f]+);|&#|payloadBase64/i; +const MARKUP_RE = /[<>]|&[A-Za-z][A-Za-z0-9]*;|&#|payloadBase64/i; /** Only the JVM tokens `` and `` are exempt from the markup rule. */ function holdsSnapshotMarker(text: string): boolean {