From 90e03e1fc9e2bc95e4064ad55c55c171fb1d767b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 6 Aug 2026 13:03:19 +0200 Subject: [PATCH 1/5] fix(ios): keep the baseline presentation when a tap corroboration has no request flags matchingCaptureFlags dropped the baseline snapshot's scope/depth/raw whenever the incoming request carried no flags, so the post-action corroboration capture ran at the default presentation and could never match a non-default baseline's presentationKey. The corroboration then silently declined to engage, leaking the raw XCTEST_RECORDED_FAILURE it exists to eliminate. --- .../interaction-ios-tap-outcome.test.ts | 47 +++++++++++++++++++ .../handlers/interaction-ios-tap-outcome.ts | 4 +- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/daemon/handlers/__tests__/interaction-ios-tap-outcome.test.ts b/src/daemon/handlers/__tests__/interaction-ios-tap-outcome.test.ts index 250c418fc..680057415 100644 --- a/src/daemon/handlers/__tests__/interaction-ios-tap-outcome.test.ts +++ b/src/daemon/handlers/__tests__/interaction-ios-tap-outcome.test.ts @@ -3,6 +3,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { AppError } from '@agent-device/kernel/errors'; +import { buildSnapshotPresentationKey } from '@agent-device/kernel/snapshot'; import { handleInteractionCommands } from '../interaction.ts'; import { handleSnapshotCommands } from '../snapshot.ts'; import { dispatchCommand } from '../../../core/dispatch.ts'; @@ -234,6 +235,52 @@ test('a changed capture with a different presentation keeps the tap failure', as expect(sessionStore.get(sessionName)?.actions).toHaveLength(0); }); +test('corroborates a tap when the request carries no flags and the baseline used a non-default scope', async () => { + const sessionName = 'ios-no-flags-tap-corroboration'; + const sessionStore = makeSessionStore(); + const baseline = snapshot(profileNodes); + baseline.presentationKey = buildSnapshotPresentationKey({ depth: 2, raw: false }); + sessionStore.set( + sessionName, + makeIosSession(sessionName, { + appBundleId: 'com.example.app', + snapshot: baseline, + }), + ); + mockDispatch.mockImplementation(async (_device, command) => { + if (command === 'press') { + throw new AppError( + 'XCTEST_RECORDED_FAILURE', + 'XCTest recorded a failure while executing tap; the action may not have been performed.', + ); + } + if (command === 'snapshot') return snapshotPayload(imageViewerNodes); + return {}; + }); + + // Deliberately built without a `flags` key at all (not `flags: {}`) — this + // mirrors the real production paths (batch steps with no flags, JSON-RPC + // requests that omit the key) that hide the bug this test pins. + const response = await handleInteractionCommands({ + req: { + token: 'test', + session: sessionName, + command: 'click', + positionals: ['id="unfollow"'], + }, + sessionName, + sessionStore, + contextFromFlags, + }); + + expect(response?.ok).toBe(true); + if (response?.ok) { + expect(response.data?.warning).toMatch(/post-action accessibility capture changed/); + expect(response.data?.selector).toBe('id="unfollow"'); + } + expect(sessionStore.get(sessionName)?.actions).toHaveLength(1); +}); + test('a changed capture against a stale baseline keeps the tap failure', async () => { const sessionName = 'ios-stale-baseline-tap-corroboration'; const sessionStore = makeSessionStore(); diff --git a/src/daemon/handlers/interaction-ios-tap-outcome.ts b/src/daemon/handlers/interaction-ios-tap-outcome.ts index 526b755c9..9334acd5a 100644 --- a/src/daemon/handlers/interaction-ios-tap-outcome.ts +++ b/src/daemon/handlers/interaction-ios-tap-outcome.ts @@ -223,9 +223,9 @@ function matchingCaptureFlags( flags: CommandFlags | undefined, presentation: SnapshotPresentation | undefined, ): CommandFlags | undefined { - if (!flags) return undefined; + if (!flags && !presentation) return undefined; return { - ...flags, + ...(flags ?? {}), out: undefined, ...(presentation ? { From 6f61b7932cade51b806595db450aa0c8b5d2fe2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 6 Aug 2026 13:13:27 +0200 Subject: [PATCH 2/5] fix(daemon): report the device claim retained by a failed close When close cannot confirm the device was released, it deliberately keeps the advisory claim (handing an unconfirmed device to the next session would be worse) but deletes the session record on the next line regardless, leaving a claim naming a session the daemon no longer tracks with no trace. Emit a warn diagnostic naming the device key and session, mirroring the open path's existing rollbackNewSessionClaim handling. Retention policy is unchanged. --- .../__tests__/session-close-shutdown.test.ts | 158 ++++++++++++++++++ src/daemon/handlers/session-close.ts | 13 +- 2 files changed, 170 insertions(+), 1 deletion(-) diff --git a/src/daemon/handlers/__tests__/session-close-shutdown.test.ts b/src/daemon/handlers/__tests__/session-close-shutdown.test.ts index 7c0a9f91b..7bca26a84 100644 --- a/src/daemon/handlers/__tests__/session-close-shutdown.test.ts +++ b/src/daemon/handlers/__tests__/session-close-shutdown.test.ts @@ -69,6 +69,9 @@ import { stopAndroidSnapshotHelperSessionForDevice } from '../../../platforms/an import { stopIosRunnerSession } from '../../../platforms/apple/core/runner/runner-client.ts'; import { WEB_DESKTOP_DEVICE } from '../../../__tests__/test-utils/index.ts'; import { setActiveProviderDeviceRuntimes } from '../../../provider-device-runtime.ts'; +import { acquireAdvisoryDeviceClaim } from '../../device-claims.ts'; +import { inspectDeviceClaims } from '../../device-claim-inspection.ts'; +import { flushDiagnosticsToSessionFile, withDiagnosticsScope } from '../../../utils/diagnostics.ts'; const mockShutdownSimulator = vi.mocked(shutdownSimulator); const mockRunCmd = vi.mocked(runCmd); @@ -1077,6 +1080,161 @@ test('targeted close preserves the platform-close AppError and still runs later expect(sessionStore.get(sessionName)).toBeUndefined(); }); +// #1478-adjacent (device-claim retention observability): a failed platform close deliberately +// keeps the advisory device claim (handing an unconfirmed device to the next session would be +// worse), but the session record is still deleted on the very next line. Before this pair of +// tests, nothing said so — the claim just quietly named a session `session list` no longer +// reported. These two tests pin both branches of that decision so a future refactor cannot +// silently invert either one. +test('a failed platform close retains the device claim and reports it', async () => { + const claimsRoot = mkdtempForTestSync('agent-device-session-close-claim-retained-'); + const previousClaimsDir = process.env.AGENT_DEVICE_CLAIMS_DIR; + process.env.AGENT_DEVICE_CLAIMS_DIR = claimsRoot; + try { + const sessionStore = makeSessionStore(); + const sessionName = 'targeted-close-claim-retained-session'; + const device = { + platform: 'apple' as const, + id: 'sim-udid-close-claim-retained', + name: 'iPhone 15', + kind: 'simulator' as const, + booted: true, + }; + const acquired = await acquireAdvisoryDeviceClaim({ + device, + session: sessionName, + workspace: process.cwd(), + stateDir: sessionStore.resolveDaemonStateDir(), + }); + if (!acquired.ownership) { + throw new Error('expected the test session to acquire a device claim'); + } + const session = { + ...makeSession(sessionName, device), + // Recording defeats runner retention so this mirrors the platform-close-error test above + // rather than exercising a different code path. + recording: { outPath: '/tmp/recording.mp4' }, + deviceClaim: acquired.ownership, + } as unknown as SessionState; + sessionStore.set(sessionName, session); + + const platformCloseError = new AppError('DEVICE_UNAVAILABLE', 'platform close failed', { + reason: 'device_disconnected', + hint: 'Reconnect the device and retry close.', + }); + mockDispatchCommand.mockRejectedValueOnce(platformCloseError); + + const diagnosticsLogPath = path.join(claimsRoot, 'diagnostics.ndjson'); + const thrown = await withDiagnosticsScope( + { session: sessionName, command: 'close', logPath: diagnosticsLogPath }, + async () => { + let caught: unknown; + try { + await handleSessionCommands({ + req: { + token: 't', + session: sessionName, + command: 'close', + positionals: ['com.example.app'], + flags: {}, + }, + sessionName, + logPath: path.join(os.tmpdir(), 'daemon.log'), + sessionStore, + invoke: noopInvoke, + }); + } catch (error) { + caught = error; + } + flushDiagnosticsToSessionFile({ force: true }); + return caught; + }, + ); + + expect(thrown).toBe(platformCloseError); + // The session is still deleted (deliberate, pre-existing behavior) even though the claim + // could not be confirmed released. + expect(sessionStore.get(sessionName)).toBeUndefined(); + + // The claim itself was NOT cleared: it is still live, still naming the deleted session. + const claimState = inspectDeviceClaims({ serial: device.id })[0]; + expect(claimState?.classification).toBe('live'); + expect(claimState?.claim?.session).toBe(sessionName); + + // A warn diagnostic names the retained claim's device key and owning session so the retention + // is observable instead of silent. + const rows = fs + .readFileSync(diagnosticsLogPath, 'utf8') + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + expect(rows).toContainEqual( + expect.objectContaining({ + level: 'warn', + phase: 'device_claim_close_effects_unconfirmed', + data: { deviceKey: acquired.ownership.deviceKey, session: sessionName }, + }), + ); + } finally { + if (previousClaimsDir === undefined) delete process.env.AGENT_DEVICE_CLAIMS_DIR; + else process.env.AGENT_DEVICE_CLAIMS_DIR = previousClaimsDir; + fs.rmSync(claimsRoot, { recursive: true, force: true }); + } +}); + +test('a successful close clears the device claim', async () => { + const claimsRoot = mkdtempForTestSync('agent-device-session-close-claim-cleared-'); + const previousClaimsDir = process.env.AGENT_DEVICE_CLAIMS_DIR; + process.env.AGENT_DEVICE_CLAIMS_DIR = claimsRoot; + try { + const sessionStore = makeSessionStore(); + const sessionName = 'targeted-close-claim-cleared-session'; + const device = { + platform: 'android' as const, + id: 'emulator-5554', + name: 'Pixel', + kind: 'emulator' as const, + booted: true, + }; + const acquired = await acquireAdvisoryDeviceClaim({ + device, + session: sessionName, + workspace: process.cwd(), + stateDir: sessionStore.resolveDaemonStateDir(), + }); + if (!acquired.ownership) { + throw new Error('expected the test session to acquire a device claim'); + } + const session = { + ...makeSession(sessionName, device), + deviceClaim: acquired.ownership, + }; + sessionStore.set(sessionName, session); + + const response = await handleSessionCommands({ + req: { + token: 't', + session: sessionName, + command: 'close', + positionals: [], + flags: {}, + }, + sessionName, + logPath: path.join(os.tmpdir(), 'daemon.log'), + sessionStore, + invoke: noopInvoke, + }); + + expect(response?.ok).toBe(true); + expect(sessionStore.get(sessionName)).toBeUndefined(); + expect(inspectDeviceClaims({ serial: device.id })).toEqual([]); + } finally { + if (previousClaimsDir === undefined) delete process.env.AGENT_DEVICE_CLAIMS_DIR; + else process.env.AGENT_DEVICE_CLAIMS_DIR = previousClaimsDir; + fs.rmSync(claimsRoot, { recursive: true, force: true }); + } +}); + test('targeted close skips platform dispatch and preserves the error when the required pre-close runner stop fails', async () => { const sessionStore = makeSessionStore(); const sessionName = 'targeted-close-preclose-failure-session'; diff --git a/src/daemon/handlers/session-close.ts b/src/daemon/handlers/session-close.ts index f652fb844..8fe47a71c 100644 --- a/src/daemon/handlers/session-close.ts +++ b/src/daemon/handlers/session-close.ts @@ -406,7 +406,18 @@ async function runCloseTeardownAndRelease(params: { failures: cleanupFailures, }); const deviceClaimBlockingError = platformCloseError ?? cleanupAggregate; - if (!deviceClaimBlockingError) { + if (deviceClaimBlockingError) { + if (session.deviceClaim) { + emitDiagnostic({ + level: 'warn', + phase: 'device_claim_close_effects_unconfirmed', + data: { + deviceKey: session.deviceClaim.deviceKey, + session: sessionName, + }, + }); + } + } else { await clearAdvisoryDeviceClaim(session.deviceClaim); } sessionStore.delete(sessionName); From 2be4b65843ecd9136bee74289dd030f8dc9c27ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 6 Aug 2026 12:50:04 +0200 Subject: [PATCH 3/5] fix(android): shell-quote free-text arguments reaching the device shell Text entry (input text) and clipboard write (cmd clipboard set text) now quote their free-form text argument with the same shellQuoteIfNeeded helper app-lifecycle.ts already uses for deep-link URLs and launch arguments, and app-lifecycle.ts's local duplicate of that helper is retired in favor of the shared one. Multi-word clipboard writes also now arrive at the device as a single argument instead of being re-tokenized into separate ones. Updates the provider-scenario test harness's scripted clipboard-state simulator to unwrap shell quoting the same way a device shell does, so it keeps modelling what the device actually receives. --- .../__tests__/device-input-state.test.ts | 25 +++++++++++ .../android/__tests__/input-actions.test.ts | 41 +++++++++++++++++++ src/platforms/android/app-lifecycle.ts | 12 ++---- src/platforms/android/device-input-state.ts | 3 +- src/platforms/android/input-actions.ts | 8 +++- .../android-lifecycle.test.ts | 2 +- .../provider-scenarios/android-world.ts | 21 +++++++++- 7 files changed, 99 insertions(+), 13 deletions(-) diff --git a/src/platforms/android/__tests__/device-input-state.test.ts b/src/platforms/android/__tests__/device-input-state.test.ts index 01493fe4e..484cba636 100644 --- a/src/platforms/android/__tests__/device-input-state.test.ts +++ b/src/platforms/android/__tests__/device-input-state.test.ts @@ -7,6 +7,7 @@ import { dismissAndroidKeyboard, getAndroidKeyboardState, getAndroidKeyboardStatusWithAdb, + writeAndroidClipboardWithAdb, } from '../device-input-state.ts'; import { flushDiagnosticsToSessionFile, withDiagnosticsScope } from '../../../utils/diagnostics.ts'; import { withScriptedAdb } from '../../../__tests__/test-utils/mocked-binaries.ts'; @@ -269,6 +270,30 @@ test('getAndroidKeyboardState treats stale input view as hidden when the IME win ); }); +test('writeAndroidClipboardWithAdb shell-quotes text containing metacharacters', async () => { + const calls: string[][] = []; + const adb: AndroidAdbExecutor = async (args) => { + calls.push(args); + return { stdout: '', stderr: '', exitCode: 0 }; + }; + + await writeAndroidClipboardWithAdb(adb, 'otp; echo pwned'); + + assert.deepEqual(calls, [['shell', 'cmd', 'clipboard', 'set', 'text', "'otp; echo pwned'"]]); +}); + +test('writeAndroidClipboardWithAdb leaves safe text unquoted', async () => { + const calls: string[][] = []; + const adb: AndroidAdbExecutor = async (args) => { + calls.push(args); + return { stdout: '', stderr: '', exitCode: 0 }; + }; + + await writeAndroidClipboardWithAdb(adb, 'android-otp'); + + assert.deepEqual(calls, [['shell', 'cmd', 'clipboard', 'set', 'text', 'android-otp']]); +}); + test('dismissAndroidKeyboard skips keyevent when keyboard is already hidden', async () => { await withScriptedAdb( 'agent-device-android-keyboard-dismiss-hidden-', diff --git a/src/platforms/android/__tests__/input-actions.test.ts b/src/platforms/android/__tests__/input-actions.test.ts index 083d8fd26..14f7e75fc 100644 --- a/src/platforms/android/__tests__/input-actions.test.ts +++ b/src/platforms/android/__tests__/input-actions.test.ts @@ -208,6 +208,47 @@ test('typeAndroid sends one character at a time when delay is requested', async ); }); +test('typeAndroid shell-quotes text containing shell metacharacters', async () => { + await withScriptedAdb( + 'agent-device-android-type-shell-metacharacters-', + [ + '#!/bin/sh', + 'printf "__CMD__\\n" >> "$AGENT_DEVICE_TEST_ARGS_FILE"', + 'printf "%s\\n" "$@" >> "$AGENT_DEVICE_TEST_ARGS_FILE"', + 'exit 0', + '', + ].join('\n'), + async ({ argsLogPath, device }) => { + await typeAndroid(device, 'otp; echo pwned'); + const logged = await fs.readFile(argsLogPath, 'utf8'); + // The chunk carrying `;` is single-quoted so the device shell cannot + // re-tokenize it into a second command. + assert.match(logged, /shell\ninput\ntext\n'otp;%sech'/); + // The next chunk has no shell-significant characters and stays unquoted. + assert.match(logged, /shell\ninput\ntext\no%spwned\n/); + }, + ); +}); + +test('typeAndroid leaves safe text unquoted', async () => { + await withScriptedAdb( + 'agent-device-android-type-safe-unquoted-', + [ + '#!/bin/sh', + 'printf "__CMD__\\n" >> "$AGENT_DEVICE_TEST_ARGS_FILE"', + 'printf "%s\\n" "$@" >> "$AGENT_DEVICE_TEST_ARGS_FILE"', + 'exit 0', + '', + ].join('\n'), + async ({ argsLogPath, device }) => { + await typeAndroid(device, 'hello'); + const logged = await fs.readFile(argsLogPath, 'utf8'); + assert.match(logged, /shell\ninput\ntext\nhello\n/); + assert.doesNotMatch(logged, /shell\ninput\ntext\n'/); + }, + ); +}); + test('fillAndroid uses chunk-safe shell input and retries when verification still fails', async () => { await withScriptedAdb( 'agent-device-android-fill-fallback-', diff --git a/src/platforms/android/app-lifecycle.ts b/src/platforms/android/app-lifecycle.ts index 1dc4b17ef..97a78a842 100644 --- a/src/platforms/android/app-lifecycle.ts +++ b/src/platforms/android/app-lifecycle.ts @@ -7,6 +7,7 @@ import { sleep } from '../../utils/timeouts.ts'; import type { AppsFilter } from '@agent-device/contracts/device'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { isDeepLinkTarget } from '@agent-device/contracts/command'; +import { shellQuoteIfNeeded } from '../../utils/shell-quote.ts'; import { createAppResolutionCache, type AppResolutionCacheScope } from '../app-resolution-cache.ts'; import { waitForAndroidBoot } from './devices.ts'; import { runAndroidAdb } from './adb.ts'; @@ -306,13 +307,8 @@ export type OpenAndroidAppOptions = { // characters, so they round-trip untouched. URLs and launch arguments are // user-supplied and may contain JSON, spaces, `#`, or `&`; each is single-quoted // unless it consists entirely of safe shell characters. -function quoteAndroidShellArg(arg: string): string { - if (/^[A-Za-z0-9_@%+=:,./-]+$/.test(arg)) return arg; - return `'${arg.replace(/'/g, `'\\''`)}'`; -} - function androidLaunchArgs(options: OpenAndroidAppOptions): string[] { - return (options.launchArgs ?? []).map(quoteAndroidShellArg); + return (options.launchArgs ?? []).map(shellQuoteIfNeeded); } export async function openAndroidApp( @@ -367,7 +363,7 @@ async function openAndroidDeepLink( '-a', 'android.intent.action.VIEW', '-d', - quoteAndroidShellArg(target), + shellQuoteIfNeeded(target), ...androidDeepLinkPackageArgs(options.appBundleId), ...androidLaunchArgs(options), ]); @@ -398,7 +394,7 @@ async function openAndroidAppBoundDeepLink( '-a', 'android.intent.action.VIEW', '-d', - quoteAndroidShellArg(deepLinkUrl), + shellQuoteIfNeeded(deepLinkUrl), '-p', resolved, ...androidLaunchArgs(options), diff --git a/src/platforms/android/device-input-state.ts b/src/platforms/android/device-input-state.ts index 61d25630c..cec37ffd6 100644 --- a/src/platforms/android/device-input-state.ts +++ b/src/platforms/android/device-input-state.ts @@ -1,6 +1,7 @@ import { emitDiagnostic } from '../../utils/diagnostics.ts'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { AppError } from '@agent-device/kernel/errors'; +import { shellQuoteIfNeeded } from '../../utils/shell-quote.ts'; import { isClipboardShellUnsupported, sleep } from './adb.ts'; import { androidAdbResultError, @@ -308,7 +309,7 @@ export async function writeAndroidClipboardWithAdb( ): Promise { await runAndroidClipboardShellCommand( adb, - ['shell', 'cmd', 'clipboard', 'set', 'text', text], + ['shell', 'cmd', 'clipboard', 'set', 'text', shellQuoteIfNeeded(text)], 'write', ); } diff --git a/src/platforms/android/input-actions.ts b/src/platforms/android/input-actions.ts index 067ab48a2..1478ad78b 100644 --- a/src/platforms/android/input-actions.ts +++ b/src/platforms/android/input-actions.ts @@ -10,6 +10,7 @@ import { import type { DeviceInfo } from '@agent-device/kernel/device'; import { AppError } from '@agent-device/kernel/errors'; import { emitDiagnostic } from '../../utils/diagnostics.ts'; +import { shellQuoteIfNeeded } from '../../utils/shell-quote.ts'; import { resolveAndroidAdbExecutor, resolveAndroidTextInjector, @@ -398,7 +399,12 @@ async function typeAndroidShell( async function typeAndroidShellChunk(device: DeviceInfo, text: string): Promise { if (!text) return; try { - await runAndroidAdb(device, ['shell', 'input', 'text', encodeAndroidInputText(text)]); + await runAndroidAdb(device, [ + 'shell', + 'input', + 'text', + shellQuoteIfNeeded(encodeAndroidInputText(text)), + ]); } catch (error) { if (isAndroidInputTextUnsupported(error)) { throw unsupportedAndroidShellTextError(text, error); diff --git a/test/integration/provider-scenarios/android-lifecycle.test.ts b/test/integration/provider-scenarios/android-lifecycle.test.ts index 6af601164..b7257a2b2 100644 --- a/test/integration/provider-scenarios/android-lifecycle.test.ts +++ b/test/integration/provider-scenarios/android-lifecycle.test.ts @@ -1482,7 +1482,7 @@ function assertAndroidPushAndEventContract(world: AndroidSettingsWorld): void { 'com.example.demo', ]); assertCommandCall(adbCalls, ['shell', 'cmd', 'clipboard', 'get', 'text']); - assertCommandCall(adbCalls, ['shell', 'cmd', 'clipboard', 'set', 'text', 'android otp']); + assertCommandCall(adbCalls, ['shell', 'cmd', 'clipboard', 'set', 'text', "'android otp'"]); assertCommandCall(adbCalls, ['shell', 'dumpsys', 'input_method']); } diff --git a/test/integration/provider-scenarios/android-world.ts b/test/integration/provider-scenarios/android-world.ts index 0113dc254..e1a87b68d 100644 --- a/test/integration/provider-scenarios/android-world.ts +++ b/test/integration/provider-scenarios/android-world.ts @@ -263,16 +263,33 @@ function createAndroidProviderShellState(): AndroidProviderShellState { return { searchText: '', clipboardText: 'hello' }; } +const ANDROID_CLIPBOARD_SET_TEXT_PREFIX = ['shell', 'cmd', 'clipboard', 'set', 'text']; + function updateAndroidProviderShellState(args: string[], state: AndroidProviderShellState): void { if (args[0] === 'shell' && args[1] === 'input' && args[2] === 'text') { state.searchText = String(args[3] ?? '').replaceAll('%s', ' '); return; } - if (args.join(' ') === 'shell cmd clipboard set text android otp') { - state.clipboardText = 'android otp'; + if (argsStartWith(args, ANDROID_CLIPBOARD_SET_TEXT_PREFIX)) { + state.clipboardText = unquoteAndroidShellArg( + String(args[ANDROID_CLIPBOARD_SET_TEXT_PREFIX.length] ?? ''), + ); } } +function argsStartWith(args: string[], prefix: string[]): boolean { + return prefix.every((value, index) => args[index] === value); +} + +// The real device shell unwraps a single-quoted argument (and collapses the +// `'\''` escape back to `'`) before `cmd` ever sees it, so this harness has +// to mirror that unwrap to keep modelling what the device actually receives +// — the inverse of the quoting in src/utils/shell-quote.ts. +function unquoteAndroidShellArg(value: string): string { + if (!value.startsWith("'") || !value.endsWith("'") || value.length < 2) return value; + return value.slice(1, -1).replaceAll("'\\''", "'"); +} + function androidDeviceStateAdbResult( key: string, args: string[], From 849c2d473baf3a111cbb482274006c103b9d253e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 6 Aug 2026 12:41:52 +0200 Subject: [PATCH 4/5] perf(cli): route command aliases through the help fast path bin.ts's `--help` fast path resolved aliases through a hand-written two-entry table that had drifted out of sync with the real CLI_COMMAND_ALIASES registry (five entries). `tap`, `launch`, and `relaunch` missed the table and silently fell through to a full runCli() bootstrap just to print static help text (~150-165ms vs ~45-50ms for aliases already in the table). Delegate to the shared normalizeCliCommandAlias registry instead of the stale local table, so every alias the registry knows about gets the fast path automatically. --- src/bin.ts | 10 +-- .../cli-help-alias-fast-path.test.ts | 66 +++++++++++++++++++ 2 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 src/cli/parser/__tests__/cli-help-alias-fast-path.test.ts diff --git a/src/bin.ts b/src/bin.ts index 18b0a2006..4afd45ebb 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -1,3 +1,5 @@ +import { normalizeCliCommandAlias } from './commands/cli-command-aliases.ts'; + const argv = process.argv.slice(2); declare const __AGENT_DEVICE_VERSION__: string; @@ -54,7 +56,7 @@ function runHelpFastPath(argv: string[]): boolean { process.stdout.write(`${buildUsageText()}\n`); return; } - const commandHelp = buildCommandUsageText(normalizeHelpTarget(helpTarget)); + const commandHelp = buildCommandUsageText(normalizeCliCommandAlias(helpTarget)); if (commandHelp) { process.stdout.write(commandHelp); return; @@ -97,12 +99,6 @@ function resolveTrailingHelpTarget( return isHelpFlag(helpArg) ? command : undefined; } -function normalizeHelpTarget(command: string): string { - if (command === 'long-press') return 'longpress'; - if (command === 'metrics') return 'perf'; - return command; -} - function isHelpCommand(command: string | undefined): boolean { return command === 'help'; } diff --git a/src/cli/parser/__tests__/cli-help-alias-fast-path.test.ts b/src/cli/parser/__tests__/cli-help-alias-fast-path.test.ts new file mode 100644 index 000000000..1e164223d --- /dev/null +++ b/src/cli/parser/__tests__/cli-help-alias-fast-path.test.ts @@ -0,0 +1,66 @@ +// Pins the exact composition `bin.ts`'s `--help` fast path relies on: +// `buildCommandUsageText(normalizeCliCommandAlias(helpTarget))`. Before this +// fix, `bin.ts` used its own hand-written two-entry table instead of this +// composition, so `tap`, `launch`, and `relaunch` silently missed the fast +// path and fell through to a full CLI bootstrap just to print static help +// text. `bin.ts` runs unguarded top-level dispatch on import (and is +// deliberately excluded from coverage — see vitest.config.ts), so it cannot +// be imported directly in a test; these tests instead pin the registry +// composition it calls. That makes them a real regression pin for a *future* +// alias missing help text (test 2 is durable for that), but not a substitute +// for the manual proof, run outside this suite, that bin.ts itself calls +// this composition (see the plan's execution report for the red/green +// evidence: with the stale table, `tap --help` loads `src/cli.ts`; with this +// fix, it does not). +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import { buildCommandUsageText } from '../cli-help.ts'; +import { + cliAliasesForCommand, + normalizeCliCommandAlias, +} from '../../../commands/cli-command-aliases.ts'; +import { listCliCommandNames } from '../../../command-catalog.ts'; + +test('alias help output matches its canonical command', () => { + const cases: ReadonlyArray = [ + ['tap', 'press'], + ['launch', 'open'], + ['relaunch', 'open'], + ['long-press', 'longpress'], + ['metrics', 'perf'], + ]; + for (const [alias, canonical] of cases) { + const aliasHelp = buildCommandUsageText(normalizeCliCommandAlias(alias)); + const canonicalHelp = buildCommandUsageText(canonical); + assert.notEqual(aliasHelp, null, `expected help text for alias "${alias}"`); + assert.equal( + aliasHelp, + canonicalHelp, + `expected "${alias} --help" to be byte-identical to "${canonical} --help"`, + ); + } +}); + +test('every CLI alias resolves to a command with help text', () => { + // Derive the alias list from the registry itself (via the canonical + // commands it targets) rather than hard-coding the five current alias + // names — a hard-coded list would silently stop covering a future sixth + // alias, reintroducing exactly the drift this test exists to catch. + const aliases = listCliCommandNames().flatMap((command) => + cliAliasesForCommand(command).map((entry) => entry.alias), + ); + assert.ok(aliases.length > 0, 'expected at least one alias to exercise this test'); + for (const alias of aliases) { + const help = buildCommandUsageText(normalizeCliCommandAlias(alias)); + assert.notEqual(help, null, `expected buildCommandUsageText to resolve alias "${alias}"`); + } +}); + +test('rotate still has no fast-path help', () => { + // `rotate` is not in the alias registry, so it must fall through to the + // slow path (`src/cli/parser/args.ts`'s `normalizeCommandAlias`), which is + // where the "renamed to orientation" migration error is raised. The fast + // path must never special-case `rotate` itself. + const help = buildCommandUsageText(normalizeCliCommandAlias('rotate')); + assert.equal(help, null); +}); From 54d5798636a0d88642387d883a7649c86f2b1deb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 6 Aug 2026 13:06:49 +0200 Subject: [PATCH 5/5] fix(remote): stop persisting the daemon bearer token in connection state ADR 0007 requires generated connection profiles to strip daemon and Metro bearer tokens; only the Metro half was honored. `connect` was writing the daemon bearer token into the 0600 connection-state file, and every later command read it back out. Stop writing `authToken` into `RemoteConnectionState['daemon']` and resolve it at each reader from the existing flag -> environment (AGENT_DEVICE_DAEMON_AUTH_TOKEN) -> remote-config-profile chain instead, matching src/cli/auth-session.ts's precedence. Behavior change: a user who ran `connect --daemon-auth-token ` and relied on later commands picking the token back up from the state file will now get an auth failure. They must export AGENT_DEVICE_DAEMON_AUTH_TOKEN, set daemonAuthToken in their remote config, or pass --daemon-auth-token on each command. website/docs/docs/remote-proxy.md is updated to show the supported env-var workflow. --- src/__tests__/remote-connection.test.ts | 9 +- src/cli/commands/connection-runtime.ts | 8 +- src/cli/commands/connection.ts | 4 +- .../__tests__/remote-connection-state.test.ts | 102 ++++++++++++++++++ src/remote/remote-connection-state.ts | 8 +- .../smoke-provider-cli-disconnect.test.ts | 4 + website/docs/docs/remote-proxy.md | 9 +- 7 files changed, 129 insertions(+), 15 deletions(-) create mode 100644 src/remote/__tests__/remote-connection-state.test.ts diff --git a/src/__tests__/remote-connection.test.ts b/src/__tests__/remote-connection.test.ts index 8ba8895b3..d05a0aa8d 100644 --- a/src/__tests__/remote-connection.test.ts +++ b/src/__tests__/remote-connection.test.ts @@ -233,7 +233,6 @@ test('connect proxy writes normal remote state with generated non-secret profile assert.equal(state.leaseId, undefined); assert.deepEqual(state.daemon, { baseUrl: 'http://proxy.example.test/agent-device', - authToken: 'proxy-secret', transport: 'http', }); assert.match(state.remoteConfigPath, /remote-connections\/generated\/proxy-[a-f0-9]{16}\.json$/); @@ -277,7 +276,6 @@ test('connect daemon-base-url shortcut uses proxy profile for direct proxy URLs' assert.match(state.clientId ?? '', /^[a-f0-9]{16}$/); assert.deepEqual(state.daemon, { baseUrl: 'http://127.0.0.1:4310/agent-device', - authToken: 'proxy-secret', transport: 'http', }); assert.equal(state.leaseId, undefined); @@ -2269,7 +2267,6 @@ test('disconnect releases proxy lease with provider client and device metadata', leaseId: 'abc123abc123abc1', daemon: { baseUrl: 'http://proxy.example.test/agent-device', - authToken: 'proxy-secret', }, leaseBackend: 'ios-instance', leaseProvider: 'proxy', @@ -2290,6 +2287,10 @@ test('disconnect releases proxy lease with provider client and device metadata', version: false, stateDir, shutdown: true, + // Not persisted on connection state (ADR 0007): the caller supplies + // it per-command via flag/env/CLI-session, mirroring how the CLI + // dispatcher resolves it before invoking this handler. + daemonAuthToken: 'test-not-a-real-token', }, client: createTestClient({ release: async (request) => { @@ -2306,7 +2307,7 @@ test('disconnect releases proxy lease with provider client and device metadata', assert.equal(releaseRequest?.leaseId, 'abc123abc123abc1'); assert.equal(releaseRequest?.leaseBackend, 'ios-instance'); assert.equal(releaseRequest?.daemonBaseUrl, 'http://proxy.example.test/agent-device'); - assert.equal(releaseRequest?.daemonAuthToken, 'proxy-secret'); + assert.equal(releaseRequest?.daemonAuthToken, 'test-not-a-real-token'); assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' }), null); fs.rmSync(tempRoot, { recursive: true, force: true }); }); diff --git a/src/cli/commands/connection-runtime.ts b/src/cli/commands/connection-runtime.ts index ad41010ed..15dd78d63 100644 --- a/src/cli/commands/connection-runtime.ts +++ b/src/cli/commands/connection-runtime.ts @@ -483,6 +483,9 @@ export async function stopReactDevtoolsCleanup(options: { export async function releaseRemoteConnectionLease( client: AgentDeviceClient, state: RemoteConnectionState, + // The daemon bearer token is never persisted on `state` (ADR 0007); callers + // pass the token already resolved via the flag/env/CLI-session chain. + daemonAuthToken?: string, ): Promise<{ released: boolean; provider?: CloudProviderSessionResult }> { if (!state.leaseId) return { released: false }; const result = await client.leases.release({ @@ -491,7 +494,7 @@ export async function releaseRemoteConnectionLease( leaseId: state.leaseId, leaseBackend: state.leaseBackend, daemonBaseUrl: state.daemon?.baseUrl, - daemonAuthToken: state.daemon?.authToken, + daemonAuthToken, daemonTransport: state.daemon?.transport, daemonServerMode: state.daemon?.serverMode, leaseProvider: state.leaseProvider, @@ -504,10 +507,11 @@ export async function releaseRemoteConnectionLease( export async function releasePreviousLease( client: AgentDeviceClient, previous: RemoteConnectionState, + daemonAuthToken?: string, ): Promise { if (!previous.leaseId) return; try { - await releaseRemoteConnectionLease(client, previous); + await releaseRemoteConnectionLease(client, previous, daemonAuthToken); } catch { // Reconnect must succeed even if the old lease was already released. } diff --git a/src/cli/commands/connection.ts b/src/cli/commands/connection.ts index 91989a9a7..f3c8e7d10 100644 --- a/src/cli/commands/connection.ts +++ b/src/cli/commands/connection.ts @@ -244,7 +244,7 @@ async function cleanupForcedPreviousConnection( if (!previous || !flags.force) return; await stopMetroCleanup(previous.metro); await stopReactDevtoolsCleanup({ stateDir, state: previous }); - await releasePreviousLease(client, previous); + await releasePreviousLease(client, previous, flags.daemonAuthToken); } function readRemoteConfigConnectionMetadata( @@ -286,7 +286,7 @@ export const disconnectCommand: ClientCommandHandler = async ({ flags, client }) let released = false; if (state.leaseId) { try { - const release = await releaseRemoteConnectionLease(client, state); + const release = await releaseRemoteConnectionLease(client, state, flags.daemonAuthToken); released = release.released; providerData ??= release.provider; } catch { diff --git a/src/remote/__tests__/remote-connection-state.test.ts b/src/remote/__tests__/remote-connection-state.test.ts new file mode 100644 index 000000000..96587bca7 --- /dev/null +++ b/src/remote/__tests__/remote-connection-state.test.ts @@ -0,0 +1,102 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { mkdtempForTest } from '../../__tests__/test-utils/tmp-dir.ts'; +import { + buildRemoteConnectionDaemonState, + hashRemoteConfigFile, + resolveRemoteConnectionDefaults, + writeRemoteConnectionState, + type RemoteConnectionState, +} from '../remote-connection-state.ts'; + +// Regression coverage for ADR 0007: generated connection profiles must strip +// the daemon bearer token. `connect` used to write it straight into the +// persisted connection-state file; these tests guard against that shadow +// coming back. + +const FAKE_DAEMON_TOKEN = 'test-not-a-real-daemon-token'; + +test('buildRemoteConnectionDaemonState does not persist the daemon auth token', () => { + const daemon = buildRemoteConnectionDaemonState({ + daemonBaseUrl: 'https://daemon.example.test', + daemonAuthToken: FAKE_DAEMON_TOKEN, + daemonTransport: 'http', + daemonServerMode: 'http', + }); + + assert.equal(Object.hasOwn(daemon ?? {}, 'authToken'), false); + assert.equal(daemon?.baseUrl, 'https://daemon.example.test'); + assert.equal(daemon?.transport, 'http'); + assert.equal(daemon?.serverMode, 'http'); +}); + +test('written connection state contains no daemon auth token', async () => { + const tempRoot = await mkdtempForTest('agent-device-remote-connection-state-write-'); + const stateDir = path.join(tempRoot, '.state'); + const remoteConfigPath = path.join(tempRoot, 'remote.json'); + fs.writeFileSync(remoteConfigPath, '{}'); + + const daemon = buildRemoteConnectionDaemonState({ + daemonBaseUrl: 'https://daemon.example.test', + daemonAuthToken: FAKE_DAEMON_TOKEN, + daemonTransport: 'http', + daemonServerMode: 'http', + }); + const now = new Date().toISOString(); + const state: RemoteConnectionState = { + version: 1, + session: 'adc-write-test', + remoteConfigPath, + remoteConfigHash: hashRemoteConfigFile(remoteConfigPath), + daemon, + tenant: 'acme', + runId: 'run-1', + connectedAt: now, + updatedAt: now, + }; + + writeRemoteConnectionState({ stateDir, state }); + + const writtenPath = path.join(stateDir, 'remote-connections', 'adc-write-test.json'); + const written = fs.readFileSync(writtenPath, 'utf8'); + assert.equal(written.includes(FAKE_DAEMON_TOKEN), false); + assert.equal(written.includes('authToken'), false); +}); + +test('resolveRemoteConnectionDefaults falls back to the environment token', async () => { + const tempRoot = await mkdtempForTest('agent-device-remote-connection-state-defaults-'); + const stateDir = path.join(tempRoot, '.state'); + const remoteConfigPath = path.join(tempRoot, 'remote.json'); + fs.writeFileSync(remoteConfigPath, '{}'); + + const daemon = buildRemoteConnectionDaemonState({ + daemonBaseUrl: 'https://daemon.example.test', + daemonAuthToken: undefined, + daemonTransport: 'http', + daemonServerMode: 'http', + }); + const now = new Date().toISOString(); + const state: RemoteConnectionState = { + version: 1, + session: 'adc-env-fallback', + remoteConfigPath, + remoteConfigHash: hashRemoteConfigFile(remoteConfigPath), + daemon, + tenant: 'acme', + runId: 'run-1', + connectedAt: now, + updatedAt: now, + }; + writeRemoteConnectionState({ stateDir, state }); + + const defaults = resolveRemoteConnectionDefaults({ + stateDir, + session: 'adc-env-fallback', + cwd: tempRoot, + env: { AGENT_DEVICE_DAEMON_AUTH_TOKEN: FAKE_DAEMON_TOKEN }, + }); + + assert.equal(defaults?.flags.daemonAuthToken, FAKE_DAEMON_TOKEN); +}); diff --git a/src/remote/remote-connection-state.ts b/src/remote/remote-connection-state.ts index 81bae0e3f..89f5dea1b 100644 --- a/src/remote/remote-connection-state.ts +++ b/src/remote/remote-connection-state.ts @@ -19,7 +19,6 @@ export type RemoteConnectionState = { remoteConfigHash: string; daemon?: { baseUrl?: string; - authToken?: string; transport?: CliFlags['daemonTransport']; serverMode?: CliFlags['daemonServerMode']; }; @@ -94,7 +93,6 @@ export function buildRemoteConnectionDaemonState( ): RemoteConnectionState['daemon'] { return { baseUrl: sanitizeDaemonBaseUrl(flags.daemonBaseUrl), - authToken: flags.daemonAuthToken, transport: flags.daemonTransport, serverMode: flags.daemonServerMode, }; @@ -154,7 +152,11 @@ export function resolveRemoteConnectionDefaults(options: { ...profile, remoteConfig: state.remoteConfigPath, daemonBaseUrl: state.daemon?.baseUrl ?? profile.daemonBaseUrl, - daemonAuthToken: state.daemon?.authToken ?? profile.daemonAuthToken, + // Deliberately not sourced from state: the daemon bearer token is never + // persisted to the connection-state file (ADR 0007). It is resolved + // from the profile here, and from the flag/env/CLI-session chain in + // resolveRemoteAuth (src/cli/auth-session.ts) at command dispatch time. + daemonAuthToken: profile.daemonAuthToken, daemonTransport: state.daemon?.transport ?? profile.daemonTransport, daemonServerMode: state.daemon?.serverMode ?? profile.daemonServerMode, ...leaseScopeToCommandFlags(leaseScope), diff --git a/test/integration/smoke-provider-cli-disconnect.test.ts b/test/integration/smoke-provider-cli-disconnect.test.ts index ff739cd15..de8e3daec 100644 --- a/test/integration/smoke-provider-cli-disconnect.test.ts +++ b/test/integration/smoke-provider-cli-disconnect.test.ts @@ -48,6 +48,10 @@ function createProviderEnv(fixture: ProviderDaemonFixture): NodeJS.ProcessEnv { BROWSERSTACK_USERNAME: 'browser-user', BROWSERSTACK_ACCESS_KEY: 'browser-key', AGENT_DEVICE_TEST_RPC_LOG_PATH: fixture.rpcLogPath, + // Generated connection state never persists the daemon bearer token + // (ADR 0007), so commands after `connect` must resolve it from the + // environment/CLI chain, same as real usage. + AGENT_DEVICE_DAEMON_AUTH_TOKEN: 'test-daemon-token', NODE_OPTIONS: [process.env.NODE_OPTIONS, `--import=${fetchFixtureUrl}`] .filter(Boolean) .join(' '), diff --git a/website/docs/docs/remote-proxy.md b/website/docs/docs/remote-proxy.md index 869acfa15..bf97512b6 100644 --- a/website/docs/docs/remote-proxy.md +++ b/website/docs/docs/remote-proxy.md @@ -31,12 +31,11 @@ By default the proxy binds `127.0.0.1`. Use `--host 0.0.0.0` only when you inten ## Remote Client -On the machine running the agent, connect to the public tunnel origin with the `/agent-device` base path and the printed token: +On the machine running the agent, connect to the public tunnel origin with the `/agent-device` base path and the printed token. The generated connection profile never stores the token (only routing metadata), so export it once and every command in the session picks it up: ```bash -agent-device connect proxy \ - --daemon-base-url https://example.trycloudflare.com/agent-device \ - --daemon-auth-token +export AGENT_DEVICE_DAEMON_AUTH_TOKEN= +agent-device connect proxy --daemon-base-url https://example.trycloudflare.com/agent-device agent-device devices --platform ios agent-device open MyApp --platform ios agent-device snapshot --platform ios @@ -44,6 +43,8 @@ agent-device close agent-device disconnect ``` +Passing `--daemon-auth-token ` instead of exporting the environment variable also works, but only authenticates the single command it is passed to; subsequent commands need the token again through the env var, a `daemonAuthToken` entry in your remote config profile, or a repeated `--daemon-auth-token` flag. + `connect proxy` stores the proxy profile and client identity. Device leases are automatic on `open` and expire after five minutes without commands. `close` releases the active session and device lease; `disconnect` clears local connection state. Multiple agents can share one proxy when each uses the normal `connect proxy`, `open`, commands, `close`, and `disconnect` flow. A busy device error means another agent owns the device until it closes or its inactivity lease expires.