From ddb474d9cddb295d7140bb4eb0906de525643f93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 4 Jul 2026 11:03:31 +0200 Subject: [PATCH 1/2] refactor: harden exec failure wrapping end to end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #1072, closing the structural gaps that let the missing-processExitError bug class exist: - requireExecSuccess(result, message, extra?) in utils/exec.ts: guards an allowFailure result and throws the curated COMMAND_FAILED (flag set via execFailureDetails) itself. Result-side by design — tool providers and executor overrides return results without throwing, so an ExecOptions knob interpreted at spawn time would silently not fire on those paths. ~30 pure guard-and-throw sites across apple/android/web converted; sites with tolerance branches, cleanup, or exit-0 reachability keep their explicit shape. - Source-scan guard (src/__tests__/exec-wrap-guard.test.ts): fails when an AppError('COMMAND_FAILED', ...) details literal rebuilds the stdout/stderr/exitCode trio inline without the helper; intentional holdouts opt out with a documented exec-guard-allow comment. The guard immediately found three wrap sites the July audit missed (runtime-hints run-as probe, device-ready not_ready branch, perf export table) — now converted. - coerceExecResult at the provider boundaries (executor overrides, apple tool provider scope, android adb provider scope): SDK-supplied callbacks cross an unchecked boundary; coercing once there replaces the per-site String(result.stdout ?? '') defensiveness, which is removed. - normalizeError stderr excerpts now strip noise prefixes (adb:/xcrun:/ simctl:/error:) before rendering; skip/strip lists stay in the kernel deliberately (platforms cannot hook normalizeError) with a comment marking the registry escape hatch if they grow. - AppErrorDetails exported and documented in kernel/errors.ts: the magic keys (hint, processExitError, retriable, reason, diagnosticId, logPath, stdout/stderr/exitCode) now carry types and doc comments. - runIosDevicectl gains tolerateOutput; the devicectl uninstall path in app-install.ts reuses it instead of duplicating the wrap + hint logic (its failure hint now falls back to the devicectl default hint). --- src/__tests__/exec-wrap-guard.test.ts | 104 ++++++++++++++++++ src/daemon/device-ready.ts | 25 +++-- .../session-appstate-input-perf.test.ts | 3 +- src/daemon/runtime-hints.ts | 8 +- src/daemon/target-shutdown.ts | 4 +- src/kernel/errors.ts | 44 ++++++-- src/platforms/android/adb-executor.ts | 19 +++- src/platforms/android/app-control.ts | 2 + src/platforms/android/devices.ts | 27 +++-- src/platforms/android/perf.ts | 33 +++--- .../android/snapshot-helper-capture.ts | 2 + src/platforms/apple/core/app-device-io.ts | 33 ++---- src/platforms/apple/core/app-install.ts | 41 ++----- src/platforms/apple/core/app-settings.ts | 37 +++---- .../apple/core/debug-symbols/dsym.ts | 21 ++-- .../apple/core/debug-symbols/symbolication.ts | 27 ++--- src/platforms/apple/core/devicectl.ts | 16 ++- src/platforms/apple/core/perf-xctrace.ts | 53 +++++---- src/platforms/apple/core/perf.ts | 60 +++++----- src/platforms/apple/core/plist.ts | 2 +- .../apple/core/runner/runner-artifact-env.ts | 19 +--- .../apple/core/runner/runner-contract.ts | 3 + .../apple/core/runner/runner-icon.ts | 41 +++---- .../apple/core/runner/runner-transport.ts | 33 +++--- .../apple/core/screenshot-status-bar.ts | 18 ++- src/platforms/apple/core/simulator.ts | 28 ++--- src/platforms/apple/core/tool-provider.ts | 29 ++++- src/platforms/apple/os/macos/helper.ts | 3 + src/platforms/apple/os/macos/host-provider.ts | 56 ++++------ src/platforms/web/agent-browser-provider.ts | 2 + src/utils/__tests__/errors.test.ts | 30 +++++ src/utils/__tests__/exec.test.ts | 42 +++++++ src/utils/exec.ts | 49 ++++++++- 33 files changed, 555 insertions(+), 359 deletions(-) create mode 100644 src/__tests__/exec-wrap-guard.test.ts diff --git a/src/__tests__/exec-wrap-guard.test.ts b/src/__tests__/exec-wrap-guard.test.ts new file mode 100644 index 000000000..2f752fd21 --- /dev/null +++ b/src/__tests__/exec-wrap-guard.test.ts @@ -0,0 +1,104 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test } from 'vitest'; + +// --------------------------------------------------------------------------- +// Source guard: hand-rolled exec failure wraps must not rebuild the +// stdout/stderr/exitCode details trio inline. +// +// normalizeError only surfaces the stderr excerpt for COMMAND_FAILED errors +// whose details carry `processExitError: true`. Before PR #1072 ~49 call sites +// spread the trio by hand and silently missed the flag, so users saw +// " exited with code N" with no cause. The fix is centralized in +// `execFailureDetails` / `requireExecSuccess` (src/utils/exec.ts); this guard +// keeps new call sites from drifting back to inline spreads. +// +// A site that intentionally rebuilds the trio WITHOUT the flag (reachable at +// exit 0, timeout errors where the message beats the excerpt, nested tool +// payloads) opts out with a `// exec-guard-allow: ` comment directly +// above the `new AppError(` — the reason doubles as documentation. +// --------------------------------------------------------------------------- + +const SRC_ROOT = path.join(path.dirname(fileURLToPath(import.meta.url)), '..'); +const ALLOW_MARKER = 'exec-guard-allow:'; +// Longhand `stdout: x.stdout` and shorthand `stdout,` property forms alike. +const TRIO_KEYS = [/\bstdout\s*[:,}]/, /\bstderr\s*[:,}]/, /\bexitCode\s*[:,}]/] as const; +const MAX_CALL_WINDOW = 1_600; + +function listSourceFiles(dir: string, files: string[] = []): string[] { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + if (entry.name === '__tests__' || entry.name === 'node_modules') continue; + const fullPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + listSourceFiles(fullPath, files); + continue; + } + if (!entry.name.endsWith('.ts')) continue; + if (entry.name.endsWith('.test.ts') || entry.name.endsWith('.d.ts')) continue; + files.push(fullPath); + } + return files; +} + +// The argument window of a `new AppError(...)` call: from its opening paren to +// the balancing close, capped defensively in case a string literal unbalances +// the parens. +function appErrorCallWindow(source: string, callStart: number): string { + const openParen = source.indexOf('(', callStart); + if (openParen === -1) return ''; + let depth = 0; + const end = Math.min(source.length, openParen + MAX_CALL_WINDOW); + for (let i = openParen; i < end; i += 1) { + const char = source[i]; + if (char === '(') depth += 1; + if (char === ')') { + depth -= 1; + if (depth === 0) return source.slice(openParen, i + 1); + } + } + return source.slice(openParen, end); +} + +function hasAllowMarker(source: string, callStart: number): boolean { + const precedingLines = source.slice(Math.max(0, callStart - 300), callStart); + return precedingLines.includes(ALLOW_MARKER); +} + +test('AppError details never rebuild the exec stdout/stderr/exitCode trio inline', () => { + const violations: string[] = []; + + for (const filePath of listSourceFiles(SRC_ROOT)) { + const source = fs.readFileSync(filePath, 'utf8'); + let searchFrom = 0; + for (;;) { + const callStart = source.indexOf('new AppError(', searchFrom); + if (callStart === -1) break; + searchFrom = callStart + 1; + + const window = appErrorCallWindow(source, callStart); + // Stderr enrichment only fires for COMMAND_FAILED; the trio is plain + // diagnostic context under other codes (and under dynamic code + // expressions, which are runner/daemon pass-throughs, not exec wraps). + if (!/^\(\s*'COMMAND_FAILED'/.test(window)) continue; + if (!TRIO_KEYS.every((key) => key.test(window))) continue; + if (window.includes('execFailureDetails') || window.includes('requireExecSuccess')) continue; + if (hasAllowMarker(source, callStart)) continue; + + const line = source.slice(0, callStart).split('\n').length; + violations.push(`${path.relative(SRC_ROOT, filePath)}:${line}`); + } + } + + assert.deepEqual( + violations, + [], + `Inline exec stdout/stderr/exitCode spreads in AppError details miss the processExitError flag, ` + + `so normalizeError cannot surface the stderr excerpt. Build the details with ` + + `execFailureDetails()/requireExecSuccess() from src/utils/exec.ts, or — when the throw is ` + + `reachable at exit 0 or the excerpt would degrade the message — opt out with a ` + + `"// ${ALLOW_MARKER} " comment above the new AppError(...) call. Violations:\n` + + violations.join('\n'), + ); +}); diff --git a/src/daemon/device-ready.ts b/src/daemon/device-ready.ts index bd7fe71ee..11110e94a 100644 --- a/src/daemon/device-ready.ts +++ b/src/daemon/device-ready.ts @@ -9,6 +9,7 @@ import { } from '../platforms/apple/core/devicectl.ts'; import { runXcrun } from '../platforms/apple/core/tool-provider.ts'; import { isActiveProviderDevice } from '../provider-device-runtime.ts'; +import { execFailureDetails } from '../utils/exec.ts'; import { createTtlMemo } from '../utils/ttl-memo.ts'; const IOS_DEVICE_READY_TIMEOUT_MS = 15_000; @@ -102,11 +103,12 @@ async function ensureIosDeviceReady(deviceId: string): Promise { timeoutMs: IOS_DEVICE_READY_TIMEOUT_MS + IOS_DEVICE_READY_COMMAND_TIMEOUT_BUFFER_MS, }, ); - const stdout = String(result.stdout ?? ''); - const stderr = String(result.stderr ?? ''); + const { stdout, stderr } = result; const parsed = await readIosReadyPayload(jsonPath); if (result.exitCode === 0) { if (!parsed.parsed) { + // exec-guard-allow: thrown at exit 0 (probe succeeded but the readiness + // JSON was missing/invalid) — not a process-exit wrap. throw new AppError('COMMAND_FAILED', 'iOS device readiness probe failed', { kind: 'probe_inconclusive', deviceId, @@ -126,15 +128,16 @@ async function ensureIosDeviceReady(deviceId: string): Promise { } return; } - throw new AppError('COMMAND_FAILED', 'iOS device is not ready for automation', { - kind: 'not_ready', - deviceId, - stdout, - stderr, - exitCode: result.exitCode, - tunnelState: parsed?.tunnelState, - hint: resolveIosReadyHint(stdout, stderr), - }); + throw new AppError( + 'COMMAND_FAILED', + 'iOS device is not ready for automation', + execFailureDetails(result, { + kind: 'not_ready', + deviceId, + tunnelState: parsed?.tunnelState, + hint: resolveIosReadyHint(stdout, stderr), + }), + ); } catch (error) { if (error instanceof AppError && error.code === 'COMMAND_FAILED') { const kind = typeof error.details?.kind === 'string' ? error.details.kind : ''; diff --git a/src/daemon/handlers/__tests__/session-appstate-input-perf.test.ts b/src/daemon/handlers/__tests__/session-appstate-input-perf.test.ts index fce873874..ea6e2f352 100644 --- a/src/daemon/handlers/__tests__/session-appstate-input-perf.test.ts +++ b/src/daemon/handlers/__tests__/session-appstate-input-perf.test.ts @@ -441,7 +441,8 @@ test('perf preserves successful metrics and normalizes per-metric Android sampli const cpu = (response.data?.metrics as any)?.cpu; expect(startup?.available).toBe(false); expect(memory?.available).toBe(false); - expect(memory?.reason).toBe('error: device offline'); + // The bare `error:` severity prefix is stripped by the stderr-excerpt noise filter. + expect(memory?.reason).toBe('device offline'); expect(memory?.error?.code).toBe('COMMAND_FAILED'); expect(memory?.error?.hint).toMatch(/adb reconnect/i); expect(memory?.error?.details?.metric).toBe('memory'); diff --git a/src/daemon/runtime-hints.ts b/src/daemon/runtime-hints.ts index 0521cdd59..e8721dbf3 100644 --- a/src/daemon/runtime-hints.ts +++ b/src/daemon/runtime-hints.ts @@ -1,5 +1,6 @@ import { isIosFamily, type DeviceInfo } from '../kernel/device.ts'; import { AppError, asAppError } from '../kernel/errors.ts'; +import { execFailureDetails } from '../utils/exec.ts'; import type { SessionRuntimeHints } from './types.ts'; import { resolveRuntimeTransportHints, @@ -125,15 +126,12 @@ async function writeAndroidDevPrefs( runAsDenied ? `Failed to access Android app sandbox for ${packageName}` : `Failed to probe Android app sandbox for ${packageName}`, - { + execFailureDetails(probeResult, { package: packageName, cmd: 'adb', args: probeArgs, - stdout: probeResult.stdout, - stderr: probeResult.stderr, - exitCode: probeResult.exitCode, hint: runAsDenied ? ANDROID_RUN_AS_HINT : ANDROID_PROBE_HINT, - }, + }), ); } diff --git a/src/daemon/target-shutdown.ts b/src/daemon/target-shutdown.ts index aa95c4316..3e2978b20 100644 --- a/src/daemon/target-shutdown.ts +++ b/src/daemon/target-shutdown.ts @@ -71,7 +71,7 @@ async function shutdownAndroidEmulator(device: DeviceInfo): Promise & { +/** + * Details bag for AppError. Free-form context is allowed, but these keys carry + * meaning at normalize/render time and must keep their types: + * - `hint` — overrides `defaultHintForCode`; re-wraps preserve an existing hint. + * - `diagnosticId` / `logPath` — lifted onto the normalized error, stripped from details. + * - `processExitError` + `stdout`/`stderr`/`exitCode` — marks a wrap of a real + * process exit so normalizeError can surface the first meaningful stderr line; + * build these via `execFailureDetails`/`requireExecSuccess` in src/utils/exec.ts + * rather than by hand. + * - `retriable` — typed retry signal hoisted to the wire error shape. + * - `reason` — machine-dispatchable sub-classification within a code. + */ +export type AppErrorDetails = Record & { hint?: string; diagnosticId?: string; logPath?: string; retriable?: boolean; supportedOn?: string; + processExitError?: boolean; + stdout?: string; + stderr?: string; + // null mirrors the raw child_process exit event: killed by signal, no code. + exitCode?: number | null; + reason?: string; }; export type NormalizedError = { @@ -133,18 +151,26 @@ function maybeEnrichCommandFailedMessage( return `${message}: ${excerpt}`; } -function firstStderrLine(stderr: string): string | null { - const skipPatterns = [ - /^an error was encountered processing the command/i, - /^underlying error\b/i, - /^simulator device failed to complete the requested operation/i, - ]; +// Boilerplate preamble lines and tool-name/severity prefixes carry nothing the +// code/message don't already convey. These lists live in the kernel deliberately: +// normalizeError renders wire-level errors after platform code has run, so +// platforms cannot contribute patterns — revisit as a registry only if the +// lists outgrow a handful of entries. +const STDERR_SKIP_PATTERNS = [ + /^an error was encountered processing the command/i, + /^underlying error\b/i, + /^simulator device failed to complete the requested operation/i, +]; +const STDERR_NOISE_PREFIX = /^(?:(?:adb|xcrun|simctl):\s*)?(?:error:\s*)?/i; +function firstStderrLine(stderr: string): string | null { for (const rawLine of stderr.split('\n')) { const line = rawLine.trim(); if (!line) continue; - if (skipPatterns.some((pattern) => pattern.test(line))) continue; - return line.length > 200 ? `${line.slice(0, 200)}...` : line; + if (STDERR_SKIP_PATTERNS.some((pattern) => pattern.test(line))) continue; + const excerpt = line.replace(STDERR_NOISE_PREFIX, '').trim(); + if (!excerpt) continue; + return excerpt.length > 200 ? `${excerpt.slice(0, 200)}...` : excerpt; } return null; } diff --git a/src/platforms/android/adb-executor.ts b/src/platforms/android/adb-executor.ts index fc6985d06..52f651c5a 100644 --- a/src/platforms/android/adb-executor.ts +++ b/src/platforms/android/adb-executor.ts @@ -2,6 +2,7 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import type { Readable, Writable } from 'node:stream'; import type { DeviceInfo } from '../../kernel/device.ts'; import { + coerceExecResult, execFailureDetails, runCmd, runCmdBackground, @@ -382,9 +383,11 @@ function withAdbFailureHintProvider(provider: AndroidAdbProvider): AndroidAdbPro if (adbFailureHintProviders.has(provider)) return provider; const enriched: AndroidAdbProvider = { ...provider, - exec: withAdbFailureHints(provider.exec), - ...(provider.pull ? { pull: withAdbFailureHints(provider.pull) } : {}), - ...(provider.install ? { install: withAdbFailureHints(provider.install) } : {}), + exec: withAdbFailureHints(coerceAdbResults(provider.exec)), + ...(provider.pull ? { pull: withAdbFailureHints(coerceAdbResults(provider.pull)) } : {}), + ...(provider.install + ? { install: withAdbFailureHints(coerceAdbResults(provider.install)) } + : {}), ...(provider.installBundle ? { installBundle: withAdbFailureHints(provider.installBundle) } : {}), @@ -393,6 +396,16 @@ function withAdbFailureHintProvider(provider: AndroidAdbProvider): AndroidAdbPro return enriched; } +// Providers are SDK-supplied callbacks whose results cross an unchecked +// boundary; coerce them once here (see coerceExecResult) so downstream code +// can trust the ExecResult types. Wrapped inside the same enrichment pass so +// the WeakSet memo above also prevents coercer stacking. +function coerceAdbResults( + call: (...args: Args) => Promise, +): (...args: Args) => Promise { + return async (...args) => coerceExecResult(await call(...args)); +} + export function createDeviceAdbExecutor(device: DeviceInfo): AndroidAdbExecutor { return createSerialAdbExecutor(device.id); } diff --git a/src/platforms/android/app-control.ts b/src/platforms/android/app-control.ts index 3d5cfccbc..e5e303481 100644 --- a/src/platforms/android/app-control.ts +++ b/src/platforms/android/app-control.ts @@ -92,6 +92,8 @@ export async function openAndroidAppWithAdb( const component = await resolveAndroidLaunchComponentWithAdb(adb, packageName, [category]); if (!component) { + // exec-guard-allow: reachable at exit 0 (am start "succeeds" with an error + // in its output); the trio is the primary attempt's context, not an exit wrap. throw new AppError( 'COMMAND_FAILED', `Failed to resolve Android launch component for ${packageName}`, diff --git a/src/platforms/android/devices.ts b/src/platforms/android/devices.ts index f20889f3e..cadc6ad56 100644 --- a/src/platforms/android/devices.ts +++ b/src/platforms/android/devices.ts @@ -1,4 +1,4 @@ -import { execFailureDetails, runCmd, runCmdDetached, whichCmd } from '../../utils/exec.ts'; +import { requireExecSuccess, runCmd, runCmdDetached, whichCmd } from '../../utils/exec.ts'; import type { ExecResult } from '../../utils/exec.ts'; import { sleep } from '../../utils/timeouts.ts'; import { AppError, asAppError } from '../../kernel/errors.ts'; @@ -311,19 +311,16 @@ export function resolveAndroidAvdName( } async function listAndroidAvdNames(): Promise { - const result = await runCmd('emulator', ['-list-avds'], { - allowFailure: true, - timeoutMs: ANDROID_BOOT_PROP_TIMEOUT_MS, - }); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to list Android emulator AVDs', - execFailureDetails(result, { - hint: 'Verify Android emulator tooling is installed and available in PATH.', - }), - ); - } + const result = requireExecSuccess( + await runCmd('emulator', ['-list-avds'], { + allowFailure: true, + timeoutMs: ANDROID_BOOT_PROP_TIMEOUT_MS, + }), + 'Failed to list Android emulator AVDs', + { + hint: 'Verify Android emulator tooling is installed and available in PATH.', + }, + ); return parseAndroidAvdList(result.stdout); } @@ -523,6 +520,8 @@ export async function waitForAndroidBoot(serial: string, timeoutMs = 60000): Pro ); lastBootResult = result; if (result.stdout.trim() === '1') return; + // exec-guard-allow: getprop exits 0 while the device is still booting; + // the throw is a retry-loop poll miss, not a process-exit wrap. throw new AppError('COMMAND_FAILED', 'Android device is still booting', { serial, stdout: result.stdout, diff --git a/src/platforms/android/perf.ts b/src/platforms/android/perf.ts index 45a40d785..a90991216 100644 --- a/src/platforms/android/perf.ts +++ b/src/platforms/android/perf.ts @@ -2,7 +2,7 @@ import { promises as fs } from 'node:fs'; import path from 'node:path'; import type { DeviceInfo } from '../../kernel/device.ts'; import { AppError } from '../../kernel/errors.ts'; -import { execFailureDetails } from '../../utils/exec.ts'; +import { execFailureDetails, requireExecSuccess } from '../../utils/exec.ts'; import { splitNonEmptyTrimmedLines } from '../../utils/parsing.ts'; import { androidAdbResultError, @@ -130,23 +130,20 @@ export async function captureAndroidHeapSnapshot( await fs.mkdir(path.dirname(outPath), { recursive: true }); const hadLocalArtifact = await fileExists(outPath); try { - const dumpResult = await adb(['shell', 'am', 'dumpheap', packageName, remotePath], { - allowFailure: true, - timeoutMs: ANDROID_HEAP_DUMP_TIMEOUT_MS, - }); - if (dumpResult.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - `Failed to capture Android heap dump for ${packageName}`, - execFailureDetails(dumpResult, { - kind: 'android-hprof', - package: packageName, - pid, - remotePath, - hint: resolveAndroidHeapDumpHint(dumpResult.stdout, dumpResult.stderr), - }), - ); - } + requireExecSuccess( + await adb(['shell', 'am', 'dumpheap', packageName, remotePath], { + allowFailure: true, + timeoutMs: ANDROID_HEAP_DUMP_TIMEOUT_MS, + }), + `Failed to capture Android heap dump for ${packageName}`, + (dumpResult) => ({ + kind: 'android-hprof', + package: packageName, + pid, + remotePath, + hint: resolveAndroidHeapDumpHint(dumpResult.stdout, dumpResult.stderr), + }), + ); const pullResult = await adb(['pull', remotePath, outPath], { allowFailure: true, diff --git a/src/platforms/android/snapshot-helper-capture.ts b/src/platforms/android/snapshot-helper-capture.ts index 5ea6ec9d2..15cf43600 100644 --- a/src/platforms/android/snapshot-helper-capture.ts +++ b/src/platforms/android/snapshot-helper-capture.ts @@ -156,6 +156,8 @@ async function readFallbackHelperOutputOrThrow( if (error instanceof AppError && result.exitCode !== 0 && error.details?.helper) throw error; const fileOutput = await readFallbackHelperOutputFile(options, resolved, result); if (fileOutput) return { output: fileOutput, cleanupDone: true }; + // exec-guard-allow: reachable at exit 0 (helper output unparseable); the + // message already branches on the exit code. throw new AppError( 'COMMAND_FAILED', result.exitCode === 0 diff --git a/src/platforms/apple/core/app-device-io.ts b/src/platforms/apple/core/app-device-io.ts index 4c47c2028..8c3e008ee 100644 --- a/src/platforms/apple/core/app-device-io.ts +++ b/src/platforms/apple/core/app-device-io.ts @@ -2,8 +2,7 @@ import { promises as fs } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { isMacOs, type DeviceInfo } from '../../../kernel/device.ts'; -import { AppError } from '../../../kernel/errors.ts'; -import { execFailureDetails } from '../../../utils/exec.ts'; +import { requireExecSuccess } from '../../../utils/exec.ts'; import { ensureBootedSimulator, requireSimulatorDevice } from './simulator.ts'; import { readMacOsClipboardText, writeMacOsClipboardText } from '../os/macos/apps.ts'; import { runSimctl } from './apps-simctl.ts'; @@ -14,14 +13,10 @@ export async function readIosClipboardText(device: DeviceInfo): Promise } requireSimulatorDevice(device, 'clipboard'); await ensureBootedSimulator(device); - const result = await runSimctl(device, ['pbpaste', device.id], { allowFailure: true }); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to read iOS simulator clipboard', - execFailureDetails(result), - ); - } + const result = requireExecSuccess( + await runSimctl(device, ['pbpaste', device.id], { allowFailure: true }), + 'Failed to read iOS simulator clipboard', + ); return result.stdout.replace(/\r\n/g, '\n').replace(/\n$/, ''); } @@ -32,17 +27,13 @@ export async function writeIosClipboardText(device: DeviceInfo, text: string): P } requireSimulatorDevice(device, 'clipboard'); await ensureBootedSimulator(device); - const result = await runSimctl(device, ['pbcopy', device.id], { - allowFailure: true, - stdin: text, - }); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to write iOS simulator clipboard', - execFailureDetails(result), - ); - } + requireExecSuccess( + await runSimctl(device, ['pbcopy', device.id], { + allowFailure: true, + stdin: text, + }), + 'Failed to write iOS simulator clipboard', + ); } export async function pushIosNotification( diff --git a/src/platforms/apple/core/app-install.ts b/src/platforms/apple/core/app-install.ts index 701c70339..7c9d923f6 100644 --- a/src/platforms/apple/core/app-install.ts +++ b/src/platforms/apple/core/app-install.ts @@ -1,16 +1,11 @@ import type { DeviceInfo } from '../../../kernel/device.ts'; import { AppError } from '../../../kernel/errors.ts'; import { execFailureDetails } from '../../../utils/exec.ts'; -import { IOS_DEVICE_INSTALL_TIMEOUT_MS, IOS_DEVICECTL_TIMEOUT_MS } from './config.ts'; +import { IOS_DEVICE_INSTALL_TIMEOUT_MS } from './config.ts'; import { runIosDevicectl } from './devicectl.ts'; import { prepareIosInstallArtifact } from './install-artifact.ts'; import { ensureBootedSimulator } from './simulator.ts'; -import { runXcrun } from './tool-provider.ts'; -import { - invalidateIosAppResolutionCache, - maybeResolveIosDevicectlHint, - resolveIosApp, -} from './app-resolution.ts'; +import { invalidateIosAppResolutionCache, resolveIosApp } from './app-resolution.ts'; import { isMissingAppErrorOutput, runSimctl } from './apps-simctl.ts'; type InstallIosAppOptions = { @@ -21,30 +16,14 @@ async function uninstallIosApp(device: DeviceInfo, app: string): Promise<{ bundl return await invalidateIosAppResolutionCache(device, async () => { const bundleId = await resolveIosApp(device, app); if (device.kind !== 'simulator') { - const args = ['devicectl', 'device', 'uninstall', 'app', '--device', device.id, bundleId]; - const result = await runXcrun(args, { - allowFailure: true, - timeoutMs: IOS_DEVICECTL_TIMEOUT_MS, - }); - if (result.exitCode !== 0) { - const stdout = String(result.stdout ?? ''); - const stderr = String(result.stderr ?? ''); - const output = `${stdout}\n${stderr}`.toLowerCase(); - if (!isMissingAppErrorOutput(output)) { - throw new AppError( - 'COMMAND_FAILED', - `Failed to uninstall iOS app ${bundleId}`, - execFailureDetails(result, { - cmd: 'xcrun', - args, - stdout, - stderr, - deviceId: device.id, - hint: maybeResolveIosDevicectlHint(stdout, stderr), - }), - ); - } - } + await runIosDevicectl( + ['device', 'uninstall', 'app', '--device', device.id, bundleId], + { action: `uninstall iOS app ${bundleId}`, deviceId: device.id }, + { + tolerateOutput: (stdout, stderr) => + isMissingAppErrorOutput(`${stdout}\n${stderr}`.toLowerCase()), + }, + ); return { bundleId }; } diff --git a/src/platforms/apple/core/app-settings.ts b/src/platforms/apple/core/app-settings.ts index e7c563185..01a7bd170 100644 --- a/src/platforms/apple/core/app-settings.ts +++ b/src/platforms/apple/core/app-settings.ts @@ -3,7 +3,7 @@ import path from 'node:path'; import { isIosFamily, isMacOs, type DeviceInfo } from '../../../kernel/device.ts'; import { AppError } from '../../../kernel/errors.ts'; import { requireLocationCoordinates } from '../../../utils/location-coordinates.ts'; -import { execFailureDetails } from '../../../utils/exec.ts'; +import { requireExecSuccess } from '../../../utils/exec.ts'; import { resolveIosSimulatorDeviceSetPath } from '../../../utils/device-isolation.ts'; import { getUnsupportedMacOsSettingMessage } from '../../../core/settings-contract.ts'; import { @@ -168,16 +168,12 @@ async function clearIosSimulatorAppState( await ensureBootedSimulator(device); await closeIosApp(device, bundleId); - const result = await runSimctl(device, ['get_app_container', device.id, bundleId, 'data'], { - allowFailure: true, - }); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - `simctl get_app_container failed for ${bundleId}`, - execFailureDetails(result), - ); - } + const result = requireExecSuccess( + await runSimctl(device, ['get_app_container', device.id, bundleId, 'data'], { + allowFailure: true, + }), + `simctl get_app_container failed for ${bundleId}`, + ); const containerPath = result.stdout.trim(); if (!containerPath) { @@ -222,16 +218,12 @@ async function resolveIosAppearanceTarget( const action = parseAppearanceAction(state); if (action !== 'toggle') return action; - const currentResult = await runSimctl(device, ['ui', device.id, 'appearance'], { - allowFailure: true, - }); - if (currentResult.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to read current iOS appearance', - execFailureDetails(currentResult), - ); - } + const currentResult = requireExecSuccess( + await runSimctl(device, ['ui', device.id, 'appearance'], { + allowFailure: true, + }), + 'Failed to read current iOS appearance', + ); const current = parseIosAppearance(currentResult.stdout, currentResult.stderr); if (!current) { throw new AppError('COMMAND_FAILED', 'Unable to determine current iOS appearance for toggle', { @@ -353,6 +345,9 @@ async function getSimctlPrivacyServices(device: DeviceInfo): Promise const result = await runSimctl(device, ['privacy', 'help'], { allowFailure: true }); const services = parseSimctlPrivacyServices(`${result.stdout}\n${result.stderr}`); if (services.size === 0) { + // exec-guard-allow: `simctl privacy help` prints usage to stderr and can + // exit non-zero while still listing services — the guard is on parse + // output, not the exit code. throw new AppError('COMMAND_FAILED', 'Unable to determine supported simctl privacy services', { stdout: result.stdout, stderr: result.stderr, diff --git a/src/platforms/apple/core/debug-symbols/dsym.ts b/src/platforms/apple/core/debug-symbols/dsym.ts index d70d560fa..5c5d71b6a 100644 --- a/src/platforms/apple/core/debug-symbols/dsym.ts +++ b/src/platforms/apple/core/debug-symbols/dsym.ts @@ -2,7 +2,7 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import type { AppleImage, DsymMatch, DsymSlice } from './types.ts'; import { normalizeUuid, unique } from './utils.ts'; -import { execFailureDetails, runCmd } from '../../../../utils/exec.ts'; +import { requireExecSuccess, runCmd } from '../../../../utils/exec.ts'; import { AppError } from '../../../../kernel/errors.ts'; const MAX_SEARCH_ENTRIES = 10_000; @@ -92,17 +92,14 @@ export async function readDsymSlices(dsymPaths: string[], dwarfdump: string): Pr async function readDsymBundleSlices(dsymPath: string, dwarfdump: string): Promise { await assertDsymBundlePath(dsymPath); - const result = await runCmd(dwarfdump, ['--uuid', dsymPath], { - timeoutMs: 15_000, - allowFailure: true, - }); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - `Failed to inspect dSYM UUIDs: ${dsymPath}`, - execFailureDetails(result, { hint: 'Verify the dSYM bundle is valid and readable.' }), - ); - } + const result = requireExecSuccess( + await runCmd(dwarfdump, ['--uuid', dsymPath], { + timeoutMs: 15_000, + allowFailure: true, + }), + `Failed to inspect dSYM UUIDs: ${dsymPath}`, + { hint: 'Verify the dSYM bundle is valid and readable.' }, + ); return parseDwarfdumpUuidOutput(dsymPath, result.stdout); } diff --git a/src/platforms/apple/core/debug-symbols/symbolication.ts b/src/platforms/apple/core/debug-symbols/symbolication.ts index ce82a0115..fc889f9a3 100644 --- a/src/platforms/apple/core/debug-symbols/symbolication.ts +++ b/src/platforms/apple/core/debug-symbols/symbolication.ts @@ -1,6 +1,6 @@ import type { DsymMatch, SymbolicatedAddress, SymbolicationGroup } from './types.ts'; import { addressKey, hex, unique } from './utils.ts'; -import { execFailureDetails, runCmd, type ExecResult } from '../../../../utils/exec.ts'; +import { requireExecSuccess, runCmd } from '../../../../utils/exec.ts'; import { AppError } from '../../../../kernel/errors.ts'; export async function symbolicateAddresses( @@ -38,11 +38,16 @@ async function runAtosForGroup( group: SymbolicationGroup, ): Promise { const addresses = unique(group.addresses.map(hex)); - const result = await runCmd(atos, atosArgs(group, addresses), { - timeoutMs: 30_000, - allowFailure: true, - }); - if (result.exitCode !== 0) throwAtosFailure(result); + const result = requireExecSuccess( + await runCmd(atos, atosArgs(group, addresses), { + timeoutMs: 30_000, + allowFailure: true, + }), + 'atos failed while symbolicating crash frames.', + { + hint: 'Verify the crash artifact and dSYM were produced from the same build and architecture.', + }, + ); return mapAtosOutputToAddresses(group.image, addresses, result.stdout); } @@ -88,16 +93,6 @@ function isSymbolicatedAtosOutput(text: string | undefined, rawAddress: string): return !normalized.startsWith('0x'); } -function throwAtosFailure(result: ExecResult): never { - throw new AppError( - 'COMMAND_FAILED', - 'atos failed while symbolicating crash frames.', - execFailureDetails(result, { - hint: 'Verify the crash artifact and dSYM were produced from the same build and architecture.', - }), - ); -} - export async function resolveAppleTools(): Promise<{ dwarfdump: string; atos: string }> { return { dwarfdump: await resolveAppleTool('dwarfdump'), diff --git a/src/platforms/apple/core/devicectl.ts b/src/platforms/apple/core/devicectl.ts index 20611427e..efce5179a 100644 --- a/src/platforms/apple/core/devicectl.ts +++ b/src/platforms/apple/core/devicectl.ts @@ -38,7 +38,14 @@ type IosDeviceProcessesPayload = { export async function runIosDevicectl( args: string[], context: { action: string; deviceId: string }, - options: { timeoutMs?: number } = {}, + options: { + timeoutMs?: number; + /** + * Treat a non-zero exit as success when its output matches — e.g. an + * uninstall of an app that is already gone. + */ + tolerateOutput?: (stdout: string, stderr: string) => boolean; + } = {}, ): Promise { const fullArgs = ['devicectl', ...args]; const result = await runXcrun(fullArgs, { @@ -46,8 +53,8 @@ export async function runIosDevicectl( timeoutMs: options.timeoutMs ?? IOS_DEVICECTL_TIMEOUT_MS, }); if (result.exitCode === 0) return; - const stdout = String(result.stdout ?? ''); - const stderr = String(result.stderr ?? ''); + const { stdout, stderr } = result; + if (options.tolerateOutput?.(stdout, stderr)) return; throw new AppError( 'COMMAND_FAILED', `Failed to ${context.action}`, @@ -107,8 +114,7 @@ async function runIosDevicectlJsonCommand( try { if (result.exitCode !== 0) { - const stdout = String(result.stdout ?? ''); - const stderr = String(result.stderr ?? ''); + const { stdout, stderr } = result; throw new AppError( 'COMMAND_FAILED', options.failureMessage, diff --git a/src/platforms/apple/core/perf-xctrace.ts b/src/platforms/apple/core/perf-xctrace.ts index d231508ad..aa2dc9c73 100644 --- a/src/platforms/apple/core/perf-xctrace.ts +++ b/src/platforms/apple/core/perf-xctrace.ts @@ -12,6 +12,7 @@ import { import { AppError } from '../../../kernel/errors.ts'; import { execFailureDetails, + requireExecSuccess, runCmdBackground, type ExecBackgroundResult, type ExecResult, @@ -127,18 +128,15 @@ export async function stopAppleXctracePerfCapture( if (outPath !== capture.outPath) { await fs.mkdir(path.dirname(outPath), { recursive: true }); } - const result = await stopAppleXctraceProcess(capture, { failOnForcedKill: true }); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - `Failed to stop Apple xctrace ${capture.mode} capture`, - execFailureDetails(result, { - tracePath: capture.outPath, - captureCleanedUp: true, - hint: resolveIosDevicePerfHint(result.stdout, result.stderr), - }), - ); - } + const result = requireExecSuccess( + await stopAppleXctraceProcess(capture, { failOnForcedKill: true }), + `Failed to stop Apple xctrace ${capture.mode} capture`, + (result) => ({ + tracePath: capture.outPath, + captureCleanedUp: true, + hint: resolveIosDevicePerfHint(result.stdout, result.stderr), + }), + ); if (outPath !== capture.outPath) { await fs.rename(capture.outPath, outPath).catch(async () => { await fs.cp(capture.outPath, outPath, { recursive: true }); @@ -192,22 +190,19 @@ export async function writeAppleXctracePerfReport(params: { '--output', tocPath, ]; - const exportResult = await runXcrun(exportArgs, { - allowFailure: true, - timeoutMs: IOS_DEVICE_PERF_EXPORT_TIMEOUT_MS, - }); - if (exportResult.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to export Apple xctrace report metadata', - execFailureDetails(exportResult, { - cmd: 'xcrun', - args: exportArgs, - tracePath: params.tracePath, - hint: resolveIosDevicePerfHint(exportResult.stdout, exportResult.stderr), - }), - ); - } + requireExecSuccess( + await runXcrun(exportArgs, { + allowFailure: true, + timeoutMs: IOS_DEVICE_PERF_EXPORT_TIMEOUT_MS, + }), + 'Failed to export Apple xctrace report metadata', + (exportResult) => ({ + cmd: 'xcrun', + args: exportArgs, + tracePath: params.tracePath, + hint: resolveIosDevicePerfHint(exportResult.stdout, exportResult.stderr), + }), + ); const report = buildAppleXctracePerfReport({ ...params, tocXml: await fs.readFile(tocPath, 'utf8'), @@ -329,6 +324,8 @@ async function stopAppleXctraceProcess( const forced = await waitForAppleXctraceExit(capture.wait, APPLE_XCTRACE_STOP_FORCE_TIMEOUT_MS); if (forced && !options.failOnForcedKill) return forced; if (forced) { + // exec-guard-allow: force-kill timeout — the timeout message beats + // whatever partial stderr the killed xctrace left behind. throw new AppError('COMMAND_FAILED', 'Timed out waiting for Apple xctrace capture to stop', { exitCode: forced.exitCode, stdout: forced.stdout, diff --git a/src/platforms/apple/core/perf.ts b/src/platforms/apple/core/perf.ts index 0ab8ff157..dd483cd60 100644 --- a/src/platforms/apple/core/perf.ts +++ b/src/platforms/apple/core/perf.ts @@ -10,7 +10,7 @@ import { type PublicPlatform, } from '../../../kernel/device.ts'; import { AppError } from '../../../kernel/errors.ts'; -import { execFailureDetails, type ExecResult } from '../../../utils/exec.ts'; +import { execFailureDetails, requireExecSuccess, type ExecResult } from '../../../utils/exec.ts'; import { splitNonEmptyTrimmedLines } from '../../../utils/parsing.ts'; import { roundPercent } from '../../perf-utils.ts'; import { uniqueStrings } from '../../../daemon/action-utils.ts'; @@ -640,15 +640,13 @@ async function exportIosDevicePerfTable( '--output', outputPath, ]; - const exportResult = await runXcrun(exportArgs, { - allowFailure: true, - timeoutMs: IOS_DEVICE_PERF_EXPORT_TIMEOUT_MS, - }); - if (exportResult.exitCode === 0) return; - throw new AppError( - 'COMMAND_FAILED', + requireExecSuccess( + await runXcrun(exportArgs, { + allowFailure: true, + timeoutMs: IOS_DEVICE_PERF_EXPORT_TIMEOUT_MS, + }), `Failed to export iOS device ${schema} data`, - execFailureDetails(exportResult, { + (exportResult) => ({ cmd: 'xcrun', args: exportArgs, appBundleId, @@ -976,17 +974,14 @@ function summarizeIosDevicePerfSnapshot( async function resolveMacOsBundlePath(appBundleId: string): Promise { const query = `kMDItemCFBundleIdentifier == "${appBundleId.replaceAll('"', '\\"')}"`; - const result = await runAppleToolCommand('mdfind', [query], { - allowFailure: true, - timeoutMs: APPLE_PERF_TIMEOUT_MS, - }); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - `Failed to resolve macOS app bundle for ${appBundleId}`, - execFailureDetails(result, { appBundleId }), - ); - } + const result = requireExecSuccess( + await runAppleToolCommand('mdfind', [query], { + allowFailure: true, + timeoutMs: APPLE_PERF_TIMEOUT_MS, + }), + `Failed to resolve macOS app bundle for ${appBundleId}`, + { appBundleId }, + ); const bundlePath = result.stdout .split('\n') @@ -1010,20 +1005,17 @@ async function resolveIosSimulatorAppContainer( appBundleId, 'app', ]); - const result = await runXcrun(args, { - allowFailure: true, - timeoutMs: APPLE_PERF_TIMEOUT_MS, - }); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - `Failed to resolve iOS simulator app container for ${appBundleId}`, - execFailureDetails(result, { - appBundleId, - hint: 'Ensure the iOS simulator app is installed and booted, then retry perf.', - }), - ); - } + const result = requireExecSuccess( + await runXcrun(args, { + allowFailure: true, + timeoutMs: APPLE_PERF_TIMEOUT_MS, + }), + `Failed to resolve iOS simulator app container for ${appBundleId}`, + { + appBundleId, + hint: 'Ensure the iOS simulator app is installed and booted, then retry perf.', + }, + ); const appPath = result.stdout.trim(); if (appPath.length === 0) { throw new AppError( diff --git a/src/platforms/apple/core/plist.ts b/src/platforms/apple/core/plist.ts index 386174bb4..490436de9 100644 --- a/src/platforms/apple/core/plist.ts +++ b/src/platforms/apple/core/plist.ts @@ -25,7 +25,7 @@ export async function readInfoPlistString( }, ); if (result.exitCode === 0) { - const value = String(result.stdout ?? '').trim(); + const value = result.stdout.trim(); if (value.length > 0) { return value; } diff --git a/src/platforms/apple/core/runner/runner-artifact-env.ts b/src/platforms/apple/core/runner/runner-artifact-env.ts index 9667c8dab..7b276e8d2 100644 --- a/src/platforms/apple/core/runner/runner-artifact-env.ts +++ b/src/platforms/apple/core/runner/runner-artifact-env.ts @@ -2,7 +2,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { AppError } from '../../../../kernel/errors.ts'; import type { DefinedEnvMap as EnvMap } from '../../../../utils/env-map.ts'; -import { execFailureDetails } from '../../../../utils/exec.ts'; +import { requireExecSuccess } from '../../../../utils/exec.ts'; import { runAppleToolCommand } from '../tool-provider.ts'; const RUNNER_XCTESTRUN_CAPTURE_OPTIONS = { @@ -92,20 +92,13 @@ async function writeXctestrunPlist( tmpXctestrunPath: string, ): Promise { fs.writeFileSync(tmpJsonPath, JSON.stringify(parsed, null, 2)); - const plistResult = await runAppleToolCommand( - 'plutil', - ['-convert', 'xml1', '-o', tmpXctestrunPath, tmpJsonPath], - { + requireExecSuccess( + await runAppleToolCommand('plutil', ['-convert', 'xml1', '-o', tmpXctestrunPath, tmpJsonPath], { allowFailure: true, - }, + }), + 'Failed to write xctestrun plist', + { tmpXctestrunPath }, ); - if (plistResult.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to write xctestrun plist', - execFailureDetails(plistResult, { tmpXctestrunPath }), - ); - } } function mergeEnvIntoXctestrunTarget( diff --git a/src/platforms/apple/core/runner/runner-contract.ts b/src/platforms/apple/core/runner/runner-contract.ts index 7506bb02c..046ab18a1 100644 --- a/src/platforms/apple/core/runner/runner-contract.ts +++ b/src/platforms/apple/core/runner/runner-contract.ts @@ -184,6 +184,9 @@ export async function buildRunnerEarlyExitError(params: { stderr: result.stderr, context: { platform: 'ios', phase: 'connect' }, }); + // exec-guard-allow: xcodebuild can exit 0 and still count as an early exit; + // the trio is nested tool context under `xcodebuild`, classified into + // `reason`/`hint` above — not a process-exit wrap. return new AppError('COMMAND_FAILED', message, { port, logPath, diff --git a/src/platforms/apple/core/runner/runner-icon.ts b/src/platforms/apple/core/runner/runner-icon.ts index 5a25d238c..7f2dc40f0 100644 --- a/src/platforms/apple/core/runner/runner-icon.ts +++ b/src/platforms/apple/core/runner/runner-icon.ts @@ -1,7 +1,6 @@ import fs from 'node:fs'; import path from 'node:path'; -import { AppError } from '../../../../kernel/errors.ts'; -import { execFailureDetails } from '../../../../utils/exec.ts'; +import { requireExecSuccess } from '../../../../utils/exec.ts'; import { readApplePlistJson, runAppleToolCommand } from '../tool-provider.ts'; const ICON_PLIST_KEYS = ['CFBundleIcons', 'CFBundleIcons~ipad'] as const; @@ -101,33 +100,27 @@ async function writeIconPlistValue( value: unknown, shouldInsert: boolean, ): Promise { - const result = await runAppleToolCommand( - 'plutil', - [shouldInsert ? '-insert' : '-replace', key, '-json', JSON.stringify(value), plistPath], - { allowFailure: true }, + requireExecSuccess( + await runAppleToolCommand( + 'plutil', + [shouldInsert ? '-insert' : '-replace', key, '-json', JSON.stringify(value), plistPath], + { allowFailure: true }, + ), + 'Failed to update XCTest runner icon plist', + { key, plistPath }, ); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to update XCTest runner icon plist', - execFailureDetails(result, { key, plistPath }), - ); - } } async function codesignRunnerApp(runnerAppPath: string): Promise { - const result = await runAppleToolCommand( - 'codesign', - ['--force', '--sign', '-', '--timestamp=none', '--generate-entitlement-der', runnerAppPath], - { allowFailure: true }, + requireExecSuccess( + await runAppleToolCommand( + 'codesign', + ['--force', '--sign', '-', '--timestamp=none', '--generate-entitlement-der', runnerAppPath], + { allowFailure: true }, + ), + 'Failed to sign XCTest runner app after icon update', + { runnerAppPath }, ); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to sign XCTest runner app after icon update', - execFailureDetails(result, { runnerAppPath }), - ); - } } function copyFileIfChanged(sourcePath: string, destinationPath: string): boolean { diff --git a/src/platforms/apple/core/runner/runner-transport.ts b/src/platforms/apple/core/runner/runner-transport.ts index cf0f98a2e..ffd60d573 100644 --- a/src/platforms/apple/core/runner/runner-transport.ts +++ b/src/platforms/apple/core/runner/runner-transport.ts @@ -7,7 +7,7 @@ import { isRequestCanceledError, } from '../../../../daemon/request-cancel.ts'; import { AppError } from '../../../../kernel/errors.ts'; -import { execFailureDetails } from '../../../../utils/exec.ts'; +import { requireExecSuccess } from '../../../../utils/exec.ts'; import { Deadline, retryWithPolicy } from '../../../../utils/retry.ts'; import type { DeviceInfo } from '../../../../kernel/device.ts'; import { classifyBootFailure, bootFailureHint } from '../../../boot-diagnostics.ts'; @@ -534,25 +534,24 @@ async function postCommandViaSimulator( payload, `http://127.0.0.1:${port}/command`, ]); - const result = await runXcrun(args, { allowFailure: true, timeoutMs, signal }); - const body = result.stdout as string; - if (result.exitCode !== 0) { - const reason = classifyBootFailure({ - message: 'Runner did not accept connection (simctl spawn)', - stdout: result.stdout, - stderr: result.stderr, - context: { platform: 'ios', phase: 'connect' }, - }); - throw new AppError( - 'COMMAND_FAILED', - 'Runner did not accept connection (simctl spawn)', - execFailureDetails(result, { + const result = requireExecSuccess( + await runXcrun(args, { allowFailure: true, timeoutMs, signal }), + 'Runner did not accept connection (simctl spawn)', + (result) => { + const reason = classifyBootFailure({ + message: 'Runner did not accept connection (simctl spawn)', + stdout: result.stdout, + stderr: result.stderr, + context: { platform: 'ios', phase: 'connect' }, + }); + return { port, reason, hint: bootFailureHint(reason), - }), - ); - } + }; + }, + ); + const body = result.stdout as string; return { status: 200, body }; } diff --git a/src/platforms/apple/core/screenshot-status-bar.ts b/src/platforms/apple/core/screenshot-status-bar.ts index 83288ff9e..67cc3e13a 100644 --- a/src/platforms/apple/core/screenshot-status-bar.ts +++ b/src/platforms/apple/core/screenshot-status-bar.ts @@ -1,7 +1,7 @@ import type { DeviceInfo } from '../../../kernel/device.ts'; import { emitDiagnostic } from '../../../utils/diagnostics.ts'; import { AppError } from '../../../kernel/errors.ts'; -import { execFailureDetails, type ExecOptions } from '../../../utils/exec.ts'; +import { requireExecSuccess, type ExecOptions } from '../../../utils/exec.ts'; import { runSimctlForDevice } from './simctl.ts'; import { extractAppleToolErrorMeta } from './tool-diagnostics.ts'; @@ -128,16 +128,12 @@ function statusBarOverrideCacheKey(device: DeviceInfo): string { async function readSimulatorStatusBarOverrides( device: DeviceInfo, ): Promise { - const result = await runSimctl(device, ['status_bar', device.id, 'list'], { - allowFailure: true, - }); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to read simulator status bar overrides', - execFailureDetails(result), - ); - } + const result = requireExecSuccess( + await runSimctl(device, ['status_bar', device.id, 'list'], { + allowFailure: true, + }), + 'Failed to read simulator status bar overrides', + ); return parseSimulatorStatusBarOverrides(result.stdout); } diff --git a/src/platforms/apple/core/simulator.ts b/src/platforms/apple/core/simulator.ts index a41ff6ded..2403588de 100644 --- a/src/platforms/apple/core/simulator.ts +++ b/src/platforms/apple/core/simulator.ts @@ -1,6 +1,6 @@ import type { DeviceInfo } from '../../../kernel/device.ts'; import { AppError } from '../../../kernel/errors.ts'; -import { execFailureDetails } from '../../../utils/exec.ts'; +import { execFailureDetails, requireExecSuccess } from '../../../utils/exec.ts'; import { Deadline, retryWithPolicy } from '../../../utils/retry.ts'; import { createTtlMemo } from '../../../utils/ttl-memo.ts'; import { bootFailureHint, classifyBootFailure } from '../../boot-diagnostics.ts'; @@ -124,11 +124,7 @@ export async function ensureBootedSimulator( allowFailure: true, timeoutMs: remainingMs, }); - bootResult = { - stdout: String(boot.stdout ?? ''), - stderr: String(boot.stderr ?? ''), - exitCode: boot.exitCode, - }; + bootResult = boot; const bootOutput = `${bootResult.stdout}\n${bootResult.stderr}`.toLowerCase(); const bootAlreadyDone = @@ -149,19 +145,9 @@ export async function ensureBootedSimulator( timeoutMs: remainingMs, }, ); - bootStatusResult = { - stdout: String(bootStatus.stdout ?? ''), - stderr: String(bootStatus.stderr ?? ''), - exitCode: bootStatus.exitCode, - }; + bootStatusResult = bootStatus; - if (bootStatusResult.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'simctl bootstatus failed', - execFailureDetails(bootStatusResult), - ); - } + requireExecSuccess(bootStatusResult, 'simctl bootstatus failed'); const nextState = await getSimulatorState(device); if (nextState !== 'Booted') { @@ -231,8 +217,8 @@ export async function shutdownSimulator(device: DeviceInfo): Promise<{ return { success: result.exitCode === 0, exitCode: result.exitCode, - stdout: String(result.stdout ?? ''), - stderr: String(result.stderr ?? ''), + stdout: result.stdout, + stderr: result.stderr, }; } @@ -249,7 +235,7 @@ export async function getSimulatorState(deviceOrUdid: DeviceInfo | string): Prom if (result.exitCode !== 0) return null; try { - const payload = JSON.parse(String(result.stdout ?? '')) as { + const payload = JSON.parse(result.stdout) as { devices: Record; }; diff --git a/src/platforms/apple/core/tool-provider.ts b/src/platforms/apple/core/tool-provider.ts index 01856afab..3bef59011 100644 --- a/src/platforms/apple/core/tool-provider.ts +++ b/src/platforms/apple/core/tool-provider.ts @@ -1,4 +1,10 @@ -import { runCmd, whichCmd, type ExecOptions, type ExecResult } from '../../../utils/exec.ts'; +import { + coerceExecResult, + runCmd, + whichCmd, + type ExecOptions, + type ExecResult, +} from '../../../utils/exec.ts'; import { createScopedProvider } from '../../../utils/scoped-provider.ts'; import { createLocalAppleMacOsHostProvider } from '../os/macos/host-provider.ts'; import type { @@ -7,6 +13,7 @@ import type { ApplePlistProvider, AppleToolAvailabilityChecker, AppleToolCommandExecutor, + AppleToolSubcommandExecutor, AppleXcrunToolProvider, } from './tool-provider-types.ts'; @@ -130,9 +137,25 @@ export async function readApplePlistJson( function normalizeAppleToolProvider(provider: AppleToolProviderInput): AppleToolProvider { if (typeof provider === 'function') { - return createLocalAppleToolProvider({ runCommand: provider }); + return createLocalAppleToolProvider({ runCommand: coerceRunCommand(provider) }); } - return createLocalAppleToolProvider(provider); + return createLocalAppleToolProvider({ + ...provider, + runCommand: coerceRunCommand(provider.runCommand), + ...(provider.simctl ? { simctl: { run: coerceRun(provider.simctl.run) } } : {}), + ...(provider.devicectl ? { devicectl: { run: coerceRun(provider.devicectl.run) } } : {}), + ...(provider.macosHelper ? { macosHelper: { run: coerceRun(provider.macosHelper.run) } } : {}), + }); +} + +// Scoped providers are SDK-supplied callbacks; coerce their results once at +// the boundary (see coerceExecResult) so platform code can trust the types. +function coerceRunCommand(run: AppleToolCommandExecutor): AppleToolCommandExecutor { + return async (cmd, args, options) => coerceExecResult(await run(cmd, args, options)); +} + +function coerceRun(run: AppleToolSubcommandExecutor): AppleToolSubcommandExecutor { + return async (args, options) => coerceExecResult(await run(args, options)); } async function readPlistJsonWithCommand( diff --git a/src/platforms/apple/os/macos/helper.ts b/src/platforms/apple/os/macos/helper.ts index 7bfea31e0..04e4c91fb 100644 --- a/src/platforms/apple/os/macos/helper.ts +++ b/src/platforms/apple/os/macos/helper.ts @@ -282,6 +282,9 @@ async function runMacOsHelper>(args: string[]) parsed && !parsed.ok ? (parsed.error?.message ?? `macOS helper exited with code ${result.exitCode}`) : stdout || result.stderr.trim() || `macOS helper exited with code ${result.exitCode}`; + // exec-guard-allow: the message is already built from the helper's JSON error + // envelope (or its output); a stderr excerpt would only degrade it, and the + // throw is reachable at exit 0 when the envelope reports ok=false. throw new AppError('COMMAND_FAILED', message, { helperPath, args, diff --git a/src/platforms/apple/os/macos/host-provider.ts b/src/platforms/apple/os/macos/host-provider.ts index 82727b9eb..860ac6464 100644 --- a/src/platforms/apple/os/macos/host-provider.ts +++ b/src/platforms/apple/os/macos/host-provider.ts @@ -3,7 +3,7 @@ import os from 'node:os'; import path from 'node:path'; import type { AppsFilter } from '../../../../contracts/app-inventory.ts'; import { AppError } from '../../../../kernel/errors.ts'; -import { execFailureDetails } from '../../../../utils/exec.ts'; +import { requireExecSuccess } from '../../../../utils/exec.ts'; import { filterAppleAppsByBundlePrefix } from '../../core/app-filter.ts'; import type { IosAppInfo } from '../../core/app-info.ts'; import type { @@ -25,40 +25,28 @@ export function createLocalAppleMacOsHostProvider( await runCommand('open', [target]); }, readClipboard: async () => { - const result = await runCommand('pbpaste', [], { allowFailure: true }); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to read macOS clipboard', - execFailureDetails(result), - ); - } + const result = requireExecSuccess( + await runCommand('pbpaste', [], { allowFailure: true }), + 'Failed to read macOS clipboard', + ); return result.stdout.replace(/\r\n/g, '\n').replace(/\n$/, ''); }, writeClipboard: async (text) => { - const result = await runCommand('pbcopy', [], { - allowFailure: true, - stdin: text, - }); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to write macOS clipboard', - execFailureDetails(result), - ); - } + requireExecSuccess( + await runCommand('pbcopy', [], { + allowFailure: true, + stdin: text, + }), + 'Failed to write macOS clipboard', + ); }, readDarkMode: async () => { const script = 'tell application "System Events" to tell appearance preferences to get dark mode'; - const result = await runCommand('osascript', ['-e', script], { allowFailure: true }); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to read macOS appearance', - execFailureDetails(result), - ); - } + const result = requireExecSuccess( + await runCommand('osascript', ['-e', script], { allowFailure: true }), + 'Failed to read macOS appearance', + ); const normalized = result.stdout.trim().toLowerCase(); if (normalized === 'true') return true; if (normalized === 'false') return false; @@ -69,14 +57,10 @@ export function createLocalAppleMacOsHostProvider( }, setDarkMode: async (enabled) => { const script = `tell application "System Events" to tell appearance preferences to set dark mode to ${enabled ? 'true' : 'false'}`; - const result = await runCommand('osascript', ['-e', script], { allowFailure: true }); - if (result.exitCode !== 0) { - throw new AppError( - 'COMMAND_FAILED', - 'Failed to set macOS appearance', - execFailureDetails(result), - ); - } + requireExecSuccess( + await runCommand('osascript', ['-e', script], { allowFailure: true }), + 'Failed to set macOS appearance', + ); }, listApps: async (filter) => await listLocalMacApps(runCommand, readPlistJson, filter), }; diff --git a/src/platforms/web/agent-browser-provider.ts b/src/platforms/web/agent-browser-provider.ts index b8c85a20d..704c729a7 100644 --- a/src/platforms/web/agent-browser-provider.ts +++ b/src/platforms/web/agent-browser-provider.ts @@ -279,6 +279,8 @@ function parseAgentBrowserJson( return JSON.parse(stdout); } catch (error) { const commandFailed = exitCode !== 0; + // exec-guard-allow: reachable at exit 0 (invalid JSON on stdout); the + // message already branches on the exit code and output is truncated. throw new AppError( 'COMMAND_FAILED', commandFailed ? 'agent-browser command failed' : 'agent-browser returned invalid JSON', diff --git a/src/utils/__tests__/errors.test.ts b/src/utils/__tests__/errors.test.ts index 65f949b13..3531a03d7 100644 --- a/src/utils/__tests__/errors.test.ts +++ b/src/utils/__tests__/errors.test.ts @@ -68,6 +68,36 @@ test('normalizeError skips simctl boilerplate wrappers in stderr', () => { assert.equal(normalized.message, 'Operation not permitted'); }); +test('normalizeError strips adb and severity prefixes from the stderr excerpt', () => { + const err = new AppError('COMMAND_FAILED', 'adb exited with code 1', { + exitCode: 1, + processExitError: true, + stderr: 'adb: error: failed to get feature set: device offline\n', + }); + const normalized = normalizeError(err); + assert.equal(normalized.message, 'failed to get feature set: device offline'); +}); + +test('normalizeError strips a bare error prefix when appending to a curated message', () => { + const err = new AppError('COMMAND_FAILED', 'simctl boot failed', { + exitCode: 1, + processExitError: true, + stderr: 'error: device is locked\n', + }); + const normalized = normalizeError(err); + assert.equal(normalized.message, 'simctl boot failed: device is locked'); +}); + +test('normalizeError skips a stderr line that is only a noise prefix', () => { + const err = new AppError('COMMAND_FAILED', 'xcrun exited with code 1', { + exitCode: 1, + processExitError: true, + stderr: 'xcrun: error:\nunable to find utility simctl\n', + }); + const normalized = normalizeError(err); + assert.equal(normalized.message, 'unable to find utility simctl'); +}); + test('normalizeError provides app discovery guidance for app-not-installed errors', () => { const normalized = normalizeError( new AppError('APP_NOT_INSTALLED', 'No package found matching "chat"'), diff --git a/src/utils/__tests__/exec.test.ts b/src/utils/__tests__/exec.test.ts index 31a042076..5db5c6e23 100644 --- a/src/utils/__tests__/exec.test.ts +++ b/src/utils/__tests__/exec.test.ts @@ -5,13 +5,17 @@ import os from 'node:os'; import path from 'node:path'; import { flushDiagnosticsToSessionFile, withDiagnosticsScope } from '../diagnostics.ts'; import { + coerceExecResult, + requireExecSuccess, runCmd, runCmdBackground, runCmdDetached, runCmdStreaming, runCmdSync, whichCmd, + type ExecResult, } from '../exec.ts'; +import { AppError } from '../../kernel/errors.ts'; test('runCmd enforces timeoutMs and rejects with COMMAND_FAILED', async () => { await assert.rejects( @@ -385,3 +389,41 @@ function summarizeExecEvent(event: { omittedArgCount: event.data?.omittedArgCount, }; } + +test('requireExecSuccess passes a zero-exit result through untouched', () => { + const result: ExecResult = { stdout: 'ok', stderr: '', exitCode: 0 }; + assert.equal( + requireExecSuccess(result, 'should not throw', () => { + throw new Error('extra must not be evaluated on success'); + }), + result, + ); +}); + +test('requireExecSuccess throws a flagged COMMAND_FAILED with lazy extras on failure', () => { + const result: ExecResult = { stdout: 'out', stderr: 'boom\n', exitCode: 3 }; + try { + requireExecSuccess(result, 'tool failed', (failed) => ({ hint: `saw ${failed.exitCode}` })); + assert.fail('expected requireExecSuccess to throw'); + } catch (error) { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.message, 'tool failed'); + assert.equal(error.details?.processExitError, true); + assert.equal(error.details?.stderr, 'boom\n'); + assert.equal(error.details?.exitCode, 3); + assert.equal(error.details?.hint, 'saw 3'); + } +}); + +test('coerceExecResult repairs loosely-typed provider results and keeps typed ones identical', () => { + const typed: ExecResult = { stdout: 'a', stderr: 'b', exitCode: 0 }; + assert.equal(coerceExecResult(typed), typed); + + const loose = coerceExecResult({ + stdout: undefined, + stderr: 42, + exitCode: undefined, + } as unknown as ExecResult); + assert.deepEqual(loose, { stdout: '', stderr: '42', exitCode: 1 }); +}); diff --git a/src/utils/exec.ts b/src/utils/exec.ts index 68e443d0b..9b40b324e 100644 --- a/src/utils/exec.ts +++ b/src/utils/exec.ts @@ -95,7 +95,7 @@ export async function runCmd( options: ExecOptions = {}, ): Promise { const overrideResult = commandExecutorOverrideScope.getStore()?.(cmd, args, options); - if (overrideResult) return await overrideResult; + if (overrideResult) return coerceExecResult(await overrideResult); return await runSpawnedCommand(cmd, args, options); } @@ -105,10 +105,30 @@ export async function runCmdStreaming( options: ExecStreamOptions = {}, ): Promise { const overrideResult = commandExecutorOverrideScope.getStore()?.(cmd, args, options); - if (overrideResult) return await overrideResult; + if (overrideResult) return coerceExecResult(await overrideResult); return await runSpawnedCommand(cmd, args, options); } +/** + * Normalize an exec result produced outside this module. Tool providers, + * executor overrides, and SDK-supplied adb executors are plain callbacks whose + * results cross an unchecked boundary; coercing once here lets downstream code + * trust the ExecResult types instead of re-wrapping fields defensively at every + * use. A non-number exitCode coerces to 1 — the same failure branch such a + * result already landed in at every `exitCode !== 0` guard. + */ +export function coerceExecResult>( + result: T, +): T { + const stdout = typeof result.stdout === 'string' ? result.stdout : String(result.stdout ?? ''); + const stderr = typeof result.stderr === 'string' ? result.stderr : String(result.stderr ?? ''); + const exitCode = typeof result.exitCode === 'number' ? result.exitCode : 1; + if (stdout === result.stdout && stderr === result.stderr && exitCode === result.exitCode) { + return result; + } + return { ...result, stdout, stderr, exitCode }; +} + function runSpawnedCommand( cmd: string, args: string[], @@ -564,6 +584,8 @@ function createTimeoutError( stdout: string, stderr: string, ): AppError { + // exec-guard-allow: deliberately no processExitError — "timed out after Nms" + // beats whatever partial stderr the killed process left behind. return new AppError('COMMAND_FAILED', `${executable} timed out after ${timeoutMs}ms`, { cmd, args, @@ -589,6 +611,29 @@ function createExitError( ); } +/** + * Guard an exec result that was obtained with `allowFailure: true`: throw a + * curated COMMAND_FAILED (with `processExitError` set, so normalizeError + * appends the stderr excerpt) on non-zero exit, and pass the result through + * otherwise. This is the standard shape for "run tool, fail with a specific + * message" call sites — it works under every executor (local spawn, tool + * providers, command overrides) because it checks the returned result rather + * than relying on the spawn layer to throw. `extra` accepts a function so + * failure-only work (hint classification) is not paid on the success path. + */ +export function requireExecSuccess( + result: ExecResult, + message: string, + extra?: Record | ((result: ExecResult) => Record), +): ExecResult { + if (result.exitCode === 0) return result; + throw new AppError( + 'COMMAND_FAILED', + message, + execFailureDetails(result, typeof extra === 'function' ? extra(result) : extra), + ); +} + /** * COMMAND_FAILED details for a non-zero exec result. `processExitError: true` * lets normalizeError surface the first meaningful stderr line as the user-facing From 5b66fed05717230e97adfbed20dd78adc2796410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 4 Jul 2026 12:43:52 +0200 Subject: [PATCH 2/2] fix: reconcile exec hardening with the adb failure classifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Main landed the central adb failure classifier (androidAdbResultError + withAdbFailureHintProvider) while this branch was in flight. Resolution: - Android call sites keep main's androidAdbResultError form — it composes execFailureDetails with the classified adb hint, which is strictly richer than a plain requireExecSuccess conversion for adb invocations. requireExecSuccess remains the shape for non-adb tools and apple/web. - Provider result coercion folds into withAdbFailureHintProvider's enrichment pass (exec/pull/install), so the existing WeakSet memo also prevents coercer stacking. - The reverse-remove wrap in adb-executor now uses androidAdbResultError instead of hand-rolling the trio (flagged by the exec-wrap guard). - Guard-test scan split into helpers (fallow cyclomatic threshold) and the perf artifact-tail clone carries fallow-ignore markers on both platforms, matching the pre-existing marker on the apple side. - Two expectations updated for the stderr noise-prefix strip: classifier compose test and perf sampling reason ('device offline', no 'error:'). --- src/__tests__/exec-wrap-guard.test.ts | 47 ++++++++++--------- .../android/__tests__/adb-executor.test.ts | 8 ++-- src/platforms/android/adb-executor.ts | 11 ++--- src/platforms/android/multitouch-helper.ts | 2 + src/platforms/android/perf.ts | 4 +- src/platforms/android/settings.ts | 2 + 6 files changed, 38 insertions(+), 36 deletions(-) diff --git a/src/__tests__/exec-wrap-guard.test.ts b/src/__tests__/exec-wrap-guard.test.ts index 2f752fd21..3410af03e 100644 --- a/src/__tests__/exec-wrap-guard.test.ts +++ b/src/__tests__/exec-wrap-guard.test.ts @@ -66,30 +66,33 @@ function hasAllowMarker(source: string, callStart: number): boolean { return precedingLines.includes(ALLOW_MARKER); } -test('AppError details never rebuild the exec stdout/stderr/exitCode trio inline', () => { - const violations: string[] = []; - - for (const filePath of listSourceFiles(SRC_ROOT)) { - const source = fs.readFileSync(filePath, 'utf8'); - let searchFrom = 0; - for (;;) { - const callStart = source.indexOf('new AppError(', searchFrom); - if (callStart === -1) break; - searchFrom = callStart + 1; - - const window = appErrorCallWindow(source, callStart); - // Stderr enrichment only fires for COMMAND_FAILED; the trio is plain - // diagnostic context under other codes (and under dynamic code - // expressions, which are runner/daemon pass-throughs, not exec wraps). - if (!/^\(\s*'COMMAND_FAILED'/.test(window)) continue; - if (!TRIO_KEYS.every((key) => key.test(window))) continue; - if (window.includes('execFailureDetails') || window.includes('requireExecSuccess')) continue; - if (hasAllowMarker(source, callStart)) continue; +function isInlineTrioViolation(source: string, callStart: number): boolean { + const window = appErrorCallWindow(source, callStart); + // Stderr enrichment only fires for COMMAND_FAILED; the trio is plain + // diagnostic context under other codes (and under dynamic code + // expressions, which are runner/daemon pass-throughs, not exec wraps). + if (!/^\(\s*'COMMAND_FAILED'/.test(window)) return false; + if (!TRIO_KEYS.every((key) => key.test(window))) return false; + if (window.includes('execFailureDetails') || window.includes('requireExecSuccess')) return false; + return !hasAllowMarker(source, callStart); +} - const line = source.slice(0, callStart).split('\n').length; - violations.push(`${path.relative(SRC_ROOT, filePath)}:${line}`); - } +function collectFileViolations(filePath: string): string[] { + const source = fs.readFileSync(filePath, 'utf8'); + const violations: string[] = []; + let searchFrom = 0; + for (;;) { + const callStart = source.indexOf('new AppError(', searchFrom); + if (callStart === -1) return violations; + searchFrom = callStart + 1; + if (!isInlineTrioViolation(source, callStart)) continue; + const line = source.slice(0, callStart).split('\n').length; + violations.push(`${path.relative(SRC_ROOT, filePath)}:${line}`); } +} + +test('AppError details never rebuild the exec stdout/stderr/exitCode trio inline', () => { + const violations = listSourceFiles(SRC_ROOT).flatMap(collectFileViolations); assert.deepEqual( violations, diff --git a/src/platforms/android/__tests__/adb-executor.test.ts b/src/platforms/android/__tests__/adb-executor.test.ts index 108a9bf0c..8a54725ea 100644 --- a/src/platforms/android/__tests__/adb-executor.test.ts +++ b/src/platforms/android/__tests__/adb-executor.test.ts @@ -492,13 +492,11 @@ test('androidAdbResultError composes the classified hint with the stderr excerpt }); // execFailureDetails flags processExitError, so normalizeError suffixes the - // curated message with the stderr excerpt while the classified hint rides along. + // curated message with the stderr excerpt (severity prefix stripped) while + // the classified hint rides along. assert.equal(error.details?.processExitError, true); const normalized = normalizeError(error); - assert.equal( - normalized.message, - 'adb uninstall failed for com.example.app: error: device offline', - ); + assert.equal(normalized.message, 'adb uninstall failed for com.example.app: device offline'); assert.match(String(normalized.hint), /adb reconnect/i); assert.equal(normalized.retriable, true); }); diff --git a/src/platforms/android/adb-executor.ts b/src/platforms/android/adb-executor.ts index 52f651c5a..0191155ef 100644 --- a/src/platforms/android/adb-executor.ts +++ b/src/platforms/android/adb-executor.ts @@ -662,14 +662,9 @@ function createExecAndroidPortReverseProvider(adb: AndroidAdbExecutor): AndroidP timeoutMs: options?.timeoutMs, }); if (result.exitCode !== 0 && !isMissingReverseMapping(result.stdout, result.stderr)) { - throw attachAdbFailureHint( - new AppError('COMMAND_FAILED', `Failed to remove Android port reverse ${local}`, { - local, - stdout: result.stdout, - stderr: result.stderr, - exitCode: result.exitCode, - }), - ); + throw androidAdbResultError(`Failed to remove Android port reverse ${local}`, result, { + local, + }); } for (const locals of owned.values()) { locals.delete(local); diff --git a/src/platforms/android/multitouch-helper.ts b/src/platforms/android/multitouch-helper.ts index cbf657d05..3031d0e6d 100644 --- a/src/platforms/android/multitouch-helper.ts +++ b/src/platforms/android/multitouch-helper.ts @@ -384,6 +384,8 @@ export async function runAndroidMultiTouchHelperGesture(options: { throw error; } } + // exec-guard-allow: reachable at exit 0 (helper output unparseable); the + // message already branches on the exit code. throw new AppError( 'COMMAND_FAILED', result.exitCode === 0 diff --git a/src/platforms/android/perf.ts b/src/platforms/android/perf.ts index a90991216..aef594249 100644 --- a/src/platforms/android/perf.ts +++ b/src/platforms/android/perf.ts @@ -2,7 +2,7 @@ import { promises as fs } from 'node:fs'; import path from 'node:path'; import type { DeviceInfo } from '../../kernel/device.ts'; import { AppError } from '../../kernel/errors.ts'; -import { execFailureDetails, requireExecSuccess } from '../../utils/exec.ts'; +import { requireExecSuccess } from '../../utils/exec.ts'; import { splitNonEmptyTrimmedLines } from '../../utils/parsing.ts'; import { androidAdbResultError, @@ -153,6 +153,7 @@ export async function captureAndroidHeapSnapshot( await cleanupLocalArtifact(outPath, hadLocalArtifact); // The site hint wins over the classified one (attachAdbFailureHint never // overwrites), but the classifier still tags adbFailure/retriable. + // fallow-ignore-next-line code-duplication throw androidAdbResultError( `Failed to pull Android heap dump for ${packageName}`, pullResult, @@ -167,6 +168,7 @@ export async function captureAndroidHeapSnapshot( ); } + // fallow-ignore-next-line code-duplication const stat = await fs.stat(outPath).catch(() => null); if (!stat?.isFile() || stat.size <= 0) { await cleanupLocalArtifact(outPath, hadLocalArtifact); diff --git a/src/platforms/android/settings.ts b/src/platforms/android/settings.ts index f99a28161..76371b7bb 100644 --- a/src/platforms/android/settings.ts +++ b/src/platforms/android/settings.ts @@ -118,6 +118,8 @@ export async function setAndroidSetting( allowFailure: true, }); if (result.exitCode !== 0 || !/\bSuccess\b/i.test(result.stdout)) { + // exec-guard-allow: pm clear can exit 0 without printing Success; the + // guard also covers that non-exit failure mode. throw new AppError( 'COMMAND_FAILED', `Failed to clear Android app data for ${resolved.value}`,