From 8d0802621fa395bf0d4288fb2e58cb78b4f68138 Mon Sep 17 00:00:00 2001 From: Rohit <40559587+Rohit3523@users.noreply.github.com> Date: Sun, 6 Sep 2026 20:39:39 +0530 Subject: [PATCH 1/6] feat: support Maestro setPermissions --- .../__tests__/program-ir-parser.test.ts | 92 +++++++++++ .../__tests__/runtime-port-fixtures.ts | 1 + .../internal/__tests__/runtime-port.test.ts | 34 +++++ .../src/internal/conformance-normalize.ts | 144 +++++++++++++----- .../src/internal/program-ir-command-parser.ts | 97 +++++++++++- .../maestro/src/internal/program-ir-values.ts | 2 +- packages/maestro/src/internal/program-ir.ts | 10 ++ .../src/internal/runtime-port-commands.ts | 42 ++++- .../src/internal/runtime-port-types.ts | 5 + .../maestro/src/internal/support-matrix.ts | 2 +- .../test/conformance/expected-divergence.ts | 5 - ...aemon-runtime-port-set-permissions.test.ts | 97 ++++++++++++ .../daemon-runtime-public-operation.test.ts | 25 +++ .../__tests__/set-permissions-mapping.test.ts | 89 +++++++++++ .../adapters/maestro/daemon-runtime-port.ts | 26 ++++ .../daemon-runtime-public-operation.ts | 25 ++- .../maestro/set-permissions-mapping.ts | 123 +++++++++++++++ src/daemon/daemon-request.ts | 7 + src/daemon/handlers/snapshot-settings.ts | 3 +- website/docs/docs/replay-e2e.md | 2 +- 20 files changed, 783 insertions(+), 48 deletions(-) create mode 100644 src/daemon/adapters/maestro/__tests__/daemon-runtime-port-set-permissions.test.ts create mode 100644 src/daemon/adapters/maestro/__tests__/set-permissions-mapping.test.ts create mode 100644 src/daemon/adapters/maestro/set-permissions-mapping.ts diff --git a/packages/maestro/src/internal/__tests__/program-ir-parser.test.ts b/packages/maestro/src/internal/__tests__/program-ir-parser.test.ts index 9596e2792f..1c8d04e777 100644 --- a/packages/maestro/src/internal/__tests__/program-ir-parser.test.ts +++ b/packages/maestro/src/internal/__tests__/program-ir-parser.test.ts @@ -409,6 +409,98 @@ describe('parseMaestroProgram', () => { }); }); + test('parses setPermissions maps, variables, and optional/label', () => { + const program = parseMaestroProgram(`appId: example.app +--- +- setPermissions: + permissions: + all: deny + notifications: unset +- setPermissions: + appId: child.app + permissions: + camera: \${CAMERA_STATE} + location: always + optional: true + label: Prepare scan +`); + + assert.deepEqual(program.commands[0], { + kind: 'setPermissions', + source: { line: 3 }, + permissions: { all: 'deny', notifications: 'unset' }, + }); + assert.deepEqual(program.commands[1], { + kind: 'setPermissions', + source: { line: 7 }, + appId: 'child.app', + permissions: { camera: '${CAMERA_STATE}', location: 'always' }, + optional: true, + label: 'Prepare scan', + }); + assert.throws( + () => + parseMaestroProgram(`--- +- setPermissions: + appId: example.app +`), + /requires permissions.*line 2/i, + ); + assert.throws( + () => + parseMaestroProgram(`--- +- setPermissions: + permissions: + camera: sometimes +`), + /allow\|deny\|unset.*line 4/i, + ); + assert.throws( + () => + parseMaestroProgram(`--- +- setPermissions: + permissions: + camera: \${ALLOW + 1} +`), + /not supported.*line 4/i, + ); + }); + + test('parses launchApp permissions maps', () => { + const program = parseMaestroProgram(`appId: example.app +--- +- launchApp: + clearState: true + permissions: + all: deny + camera: \${CAMERA_STATE} +`); + + assert.deepEqual(program.commands[0], { + kind: 'launchApp', + source: { line: 3 }, + clearState: true, + permissions: { all: 'deny', camera: '${CAMERA_STATE}' }, + }); + assert.throws( + () => + parseMaestroProgram(`--- +- launchApp: + permissions: {} +`), + /launchApp\.permissions requires at least one permission.*line 2/i, + ); + assert.throws( + () => + parseMaestroProgram(`--- +- launchApp: + permissions: + camera: sometimes +`), + /allow\|deny\|unset.*line 4/i, + ); + }); + test('reports source lines for unsupported and invalid command shapes', () => { assert.throws( () => diff --git a/packages/maestro/src/internal/__tests__/runtime-port-fixtures.ts b/packages/maestro/src/internal/__tests__/runtime-port-fixtures.ts index 8223e75dbc..7c0a4d7374 100644 --- a/packages/maestro/src/internal/__tests__/runtime-port-fixtures.ts +++ b/packages/maestro/src/internal/__tests__/runtime-port-fixtures.ts @@ -59,6 +59,7 @@ export function makeOperations( resolveGestureViewport: async () => ({ x: 0, y: 0, width: 402, height: 874 }), launchApp: noOp, stopApp: noOp, + setPermissions: noOp, openLink: noOp, tapOn: noOp, doubleTapOn: noOp, diff --git a/packages/maestro/src/internal/__tests__/runtime-port.test.ts b/packages/maestro/src/internal/__tests__/runtime-port.test.ts index b3bd802d82..514d515208 100644 --- a/packages/maestro/src/internal/__tests__/runtime-port.test.ts +++ b/packages/maestro/src/internal/__tests__/runtime-port.test.ts @@ -10,6 +10,40 @@ import { } from './runtime-port-fixtures.ts'; describe('MaestroRuntimePort', () => { + test('dispatches setPermissions with the flow appId and resolved values', async () => { + const calls: RecordedCall[] = []; + const operations = makeOperations({ + setPermissions: vi.fn(async (input, context) => + record(calls, 'setPermissions', input, context), + ), + }); + const program = parseMaestroProgram( + [ + 'appId: com.example.checkout', + 'env:', + ' CAMERA_STATE: allow', + '---', + '- setPermissions:', + ' permissions:', + ' all: deny', + ' camera: ${CAMERA_STATE}', + ].join('\n'), + ); + + const result = await executeMaestroProgram(program, createMaestroRuntimePort(operations)); + + expect(result).toMatchObject({ executed: 1, skipped: 0 }); + expect(calls).toHaveLength(1); + expect(calls[0]).toMatchObject({ + kind: 'setPermissions', + input: { + appId: 'com.example.checkout', + permissions: { all: 'deny', camera: 'allow' }, + }, + appId: 'com.example.checkout', + }); + }); + test('delegates typed lifecycle, input, keyboard, screenshot, and script operations', async () => { const calls: RecordedCall[] = []; const operations = makeOperations({ diff --git a/packages/maestro/src/internal/conformance-normalize.ts b/packages/maestro/src/internal/conformance-normalize.ts index a7d127bf20..3a524e544a 100644 --- a/packages/maestro/src/internal/conformance-normalize.ts +++ b/packages/maestro/src/internal/conformance-normalize.ts @@ -37,7 +37,13 @@ export type CanonicalGesture = | { mode: 'element'; from: CanonicalSelector; direction?: string; duration?: number | string }; export type CanonicalCommand = - | { kind: 'launchApp'; appId?: string; clearState?: boolean; stopApp?: boolean } + | { + kind: 'launchApp'; + appId?: string; + clearState?: boolean; + stopApp?: boolean; + permissions?: Record; + } // Upstream models `doubleTapOn` as a tap with repeat.repeat == 2, so the repeat // COUNT is the canonical field on both sides rather than a `double` variant on // one — that keeps our distinct tapOn/doubleTapOn kinds comparable to upstream @@ -77,6 +83,7 @@ export type CanonicalCommand = | { kind: 'takeScreenshot' } | { kind: 'waitForAnimationToEnd'; timeout?: number | string } | { kind: 'stopApp' } + | { kind: 'setPermissions'; appId?: string; permissions?: Record } | { kind: 'repeat'; times: string | number } | { kind: 'retry'; maxRetries?: string | number } | { kind: 'runFlow'; label?: string; source: 'file' | 'commands' } @@ -98,7 +105,19 @@ export function canonicalizeUpstreamFlow(commands: UpstreamCommand[]): Canonical .map(canonicalizeUpstreamCommand); } +/** Upstream commands that canonicalize to a bare kind with no fields. */ +const BARE_UPSTREAM_CANONICAL: Record = { + ScrollCommand: { kind: 'scroll' }, + BackPressCommand: { kind: 'back' }, + HideKeyboardCommand: { kind: 'hideKeyboard' }, + TakeScreenshotCommand: { kind: 'takeScreenshot' }, + StopAppCommand: { kind: 'stopApp' }, + RunScriptCommand: { kind: 'runScript' }, +}; + function canonicalizeUpstreamCommand(command: UpstreamCommand): CanonicalCommand { + const bare = BARE_UPSTREAM_CANONICAL[command.type]; + if (bare) return bare; const f = command.fields; switch (command.type) { case 'LaunchAppCommand': @@ -107,6 +126,7 @@ function canonicalizeUpstreamCommand(command: UpstreamCommand): CanonicalCommand appId: str(f.appId), clearState: bool(f.clearState), stopApp: bool(f.stopApp), + permissions: permissionsRecord(f.permissions), }); case 'TapOnElementCommand': { const repeat = asRecord(f.repeat); @@ -164,8 +184,6 @@ function canonicalizeUpstreamCommand(command: UpstreamCommand): CanonicalCommand } case 'SwipeCommand': return dropUndefined({ kind: 'swipe', label: str(f.label), gesture: upstreamGesture(f) }); - case 'ScrollCommand': - return { kind: 'scroll' }; case 'ScrollUntilVisibleCommand': return dropUndefined({ kind: 'scrollUntilVisible', @@ -185,19 +203,17 @@ function canonicalizeUpstreamCommand(command: UpstreamCommand): CanonicalCommand return dropUndefined({ kind: 'openLink', link: str(f.link) }); case 'PressKeyCommand': return { kind: 'pressKey', key: lower(str(f.code)) ?? '' }; - case 'BackPressCommand': - return { kind: 'back' }; - case 'HideKeyboardCommand': - return { kind: 'hideKeyboard' }; - case 'TakeScreenshotCommand': - return { kind: 'takeScreenshot' }; case 'WaitForAnimationToEndCommand': return dropUndefined({ kind: 'waitForAnimationToEnd', timeout: numLike(f.timeout) ?? str(f.timeout), }); - case 'StopAppCommand': - return { kind: 'stopApp' }; + case 'SetPermissionsCommand': + return dropUndefined({ + kind: 'setPermissions', + appId: str(f.appId), + permissions: permissionsRecord(f.permissions), + }); case 'RepeatCommand': return { kind: 'repeat', times: numLike(f.times) ?? str(f.times) ?? '' }; case 'RetryCommand': @@ -211,8 +227,6 @@ function canonicalizeUpstreamCommand(command: UpstreamCommand): CanonicalCommand label: str(f.label), source: f.sourceDescription != null ? 'file' : 'commands', }); - case 'RunScriptCommand': - return { kind: 'runScript' }; default: return { kind: 'unsupported', command: unsupportedName(command.type) }; } @@ -286,6 +300,18 @@ function lower(value: string | undefined): string | undefined { return value?.toLowerCase(); } +function permissionsRecord(value: unknown): Record | undefined { + const record = asRecord(value); + if (!record) return undefined; + const permissions: Record = {}; + for (const [key, entry] of Object.entries(record)) { + const coerced = str(entry)?.toLowerCase(); + if (coerced === undefined) return undefined; + permissions[key] = coerced; + } + return permissions; +} + // --------------------------------------------------------------------------- // agent-device engine IR → canonical // --------------------------------------------------------------------------- @@ -302,18 +328,32 @@ export function canonicalizeAgentCommands( return program.commands.map((command) => canonicalizeAgentCommand(command, program.config)); } -function canonicalizeAgentCommand( - command: MaestroCommand, - config: MaestroProgram['config'], -): CanonicalCommand { +/** Agent commands that canonicalize to a bare kind with no fields. */ +const BARE_AGENT_CANONICAL = { + scroll: { kind: 'scroll' }, + back: { kind: 'back' }, + hideKeyboard: { kind: 'hideKeyboard' }, + takeScreenshot: { kind: 'takeScreenshot' }, + stopApp: { kind: 'stopApp' }, + runScript: { kind: 'runScript' }, +} satisfies Record; + +type BareAgentCommand = Extract; + +function isBareAgentCommand(command: MaestroCommand): command is BareAgentCommand { + return command.kind in BARE_AGENT_CANONICAL; +} + +type AgentTapCommand = Extract; + +function isAgentTapCommand(command: MaestroCommand): command is AgentTapCommand { + return ( + command.kind === 'tapOn' || command.kind === 'doubleTapOn' || command.kind === 'longPressOn' + ); +} + +function canonicalizeAgentTapCommand(command: AgentTapCommand): CanonicalCommand { switch (command.kind) { - case 'launchApp': - return dropUndefined({ - kind: 'launchApp', - appId: command.appId ?? config.appId, - clearState: command.clearState, - stopApp: command.stopApp, - }); case 'tapOn': { const repeat = numLike(command.repeat) ?? 1; const repeatIsNumber = typeof repeat === 'number'; @@ -343,6 +383,25 @@ function canonicalizeAgentCommand( label: command.label, target: canonicalizeAgentTarget(command.target), }); + } +} + +type AgentAssertCommand = Extract< + MaestroCommand, + { kind: 'assertVisible' | 'assertNotVisible' | 'assertTrue' | 'extendedWaitUntil' } +>; + +function isAgentAssertCommand(command: MaestroCommand): command is AgentAssertCommand { + return ( + command.kind === 'assertVisible' || + command.kind === 'assertNotVisible' || + command.kind === 'assertTrue' || + command.kind === 'extendedWaitUntil' + ); +} + +function canonicalizeAgentAssertCommand(command: AgentAssertCommand): CanonicalCommand { + switch (command.kind) { case 'assertVisible': return dropUndefined({ kind: 'assert', @@ -376,6 +435,25 @@ function canonicalizeAgentCommand( label: command.label, selector: canonicalizeAgentSelector(command.notVisible ?? command.visible), }); + } +} + +function canonicalizeAgentCommand( + command: MaestroCommand, + config: MaestroProgram['config'], +): CanonicalCommand { + if (isBareAgentCommand(command)) return BARE_AGENT_CANONICAL[command.kind]; + if (isAgentTapCommand(command)) return canonicalizeAgentTapCommand(command); + if (isAgentAssertCommand(command)) return canonicalizeAgentAssertCommand(command); + switch (command.kind) { + case 'launchApp': + return dropUndefined({ + kind: 'launchApp', + appId: command.appId ?? config.appId, + clearState: command.clearState, + stopApp: command.stopApp, + permissions: command.permissions, + }); case 'swipe': return { kind: 'swipe', label: command.label, gesture: agentGesture(command.gesture) }; case 'inputText': @@ -384,8 +462,6 @@ function canonicalizeAgentCommand( return dropUndefined({ kind: 'eraseText', count: numLike(command.charactersToErase) }); case 'openLink': return dropUndefined({ kind: 'openLink', link: command.link }); - case 'scroll': - return { kind: 'scroll' }; case 'scrollUntilVisible': // Upstream materializes the DOWN default onto the command at parse time; // our engine defers it to execution (runtime-port-commands.ts). Materialize @@ -400,16 +476,14 @@ function canonicalizeAgentCommand( }); case 'pressKey': return { kind: 'pressKey', key: command.key.toLowerCase() }; - case 'back': - return { kind: 'back' }; - case 'hideKeyboard': - return { kind: 'hideKeyboard' }; - case 'takeScreenshot': - return { kind: 'takeScreenshot' }; case 'waitForAnimationToEnd': return dropUndefined({ kind: 'waitForAnimationToEnd', timeout: numLike(command.timeout) }); - case 'stopApp': - return { kind: 'stopApp' }; + case 'setPermissions': + return dropUndefined({ + kind: 'setPermissions', + appId: command.appId ?? config.appId, + permissions: command.permissions, + }); case 'repeat': return { kind: 'repeat', times: numLike(command.times) ?? str(command.times) ?? '' }; case 'retry': @@ -423,8 +497,6 @@ function canonicalizeAgentCommand( label: command.label, source: command.include.kind === 'file' ? 'file' : 'commands', }); - case 'runScript': - return { kind: 'runScript' }; default: { const exhaustive: never = command; throw new Error(`Unhandled agent command: ${JSON.stringify(exhaustive)}`); diff --git a/packages/maestro/src/internal/program-ir-command-parser.ts b/packages/maestro/src/internal/program-ir-command-parser.ts index 9dced618f8..966d53eed0 100644 --- a/packages/maestro/src/internal/program-ir-command-parser.ts +++ b/packages/maestro/src/internal/program-ir-command-parser.ts @@ -14,6 +14,7 @@ import type { MaestroPressKeyCommand, MaestroScrollCommand, MaestroScrollUntilVisibleCommand, + MaestroSetPermissionsCommand, MaestroStopAppCommand, MaestroTakeScreenshotCommand, MaestroWaitForAnimationToEndCommand, @@ -58,6 +59,7 @@ import { readSequenceItems, sourceAt, type MaestroProgramParseContext, + VARIABLE_PATTERN, } from './program-ir-values.ts'; export function parseMaestroCommandList( @@ -122,6 +124,7 @@ const COMMAND_VALUE_PARSERS: Readonly> = { back: parseBack, waitForAnimationToEnd: parseWaitForAnimationToEnd, stopApp: parseStopApp, + setPermissions: parseSetPermissions, runScript: parseMaestroRunScriptCommand, runFlow: (value, node, context) => parseMaestroRunFlowCommand(value, node, context, parseMaestroCommandList), @@ -165,7 +168,7 @@ function parseLaunchApp( assertOnlyKeys( entries, 'launchApp', - ['appId', 'stopApp', 'clearState', 'arguments', 'launchArguments'], + ['appId', 'stopApp', 'clearState', 'permissions', 'arguments', 'launchArguments'], context, ); const appId = readOptionalEntry(entries, 'appId', (entry) => @@ -177,6 +180,15 @@ function parseLaunchApp( const clearState = readOptionalEntry(entries, 'clearState', (entry) => readOptionalBoolean(entry, 'launchApp.clearState', context), ); + const permissions = readOptionalEntry(entries, 'permissions', (entry) => + readSetPermissionsMap(entry, context, 'launchApp'), + ); + if (permissions && Object.keys(permissions).length === 0) + invalidAt( + 'Maestro launchApp.permissions requires at least one permission.', + commandNode, + context, + ); const args = readOptionalEntry(entries, 'arguments', (entry) => parseLaunchArguments(entry, 'launchApp.arguments', context), ); @@ -189,6 +201,7 @@ function parseLaunchApp( appId, stopApp, clearState, + permissions, arguments: args, launchArguments, }); @@ -447,6 +460,88 @@ function parseStopApp( return { kind: 'stopApp', source, appId: readRequiredString(value, 'stopApp', context) }; } +const MAESTRO_PERMISSION_VALUES = new Set([ + 'allow', + 'deny', + 'unset', + 'always', + 'inuse', + 'never', + 'limited', +]); + +function parseSetPermissions( + value: Node | null, + commandNode: Node, + context: MaestroProgramParseContext, +): MaestroSetPermissionsCommand { + const source = sourceAt(commandNode, context); + const entries = readMapEntries(value, 'setPermissions', context); + assertOnlyKeys(entries, 'setPermissions', ['appId', 'permissions', 'optional', 'label'], context); + if (!hasEntry(entries, 'permissions')) + invalidAt('Maestro setPermissions requires permissions.', commandNode, context); + const appId = readOptionalEntry(entries, 'appId', (entry) => + readOptionalString(entry, 'setPermissions.appId', context), + ); + const permissions = readSetPermissionsMap(entryValue(entries, 'permissions'), context); + if (Object.keys(permissions).length === 0) + invalidAt('Maestro setPermissions requires at least one permission.', commandNode, context); + const options = readOptionalCommandOption(entries, 'setPermissions', context); + const label = readMaestroCommandLabel(entries, 'setPermissions', context); + return stripUndefined({ + kind: 'setPermissions' as const, + source, + appId, + permissions, + ...options, + label, + }); +} + +function readSetPermissionsMap( + node: Node | null | undefined, + context: MaestroProgramParseContext, + owner = 'setPermissions', +): Record { + const entries = readMapEntries(node, `${owner}.permissions`, context); + const permissions: Record = {}; + for (const entry of entries) { + if (entry.key in permissions) + invalidAt( + `Maestro ${owner}.permissions contains duplicate permission "${entry.key}".`, + entry.keyNode, + context, + ); + permissions[entry.key] = readPermissionValue(entry, context, owner); + } + return permissions; +} + +function readPermissionValue( + entry: { key: string; value: Node | null }, + context: MaestroProgramParseContext, + owner = 'setPermissions', +): string { + const name = `${owner}.permissions.${entry.key}`; + const value = readScalarValue(entry.value, name, context); + if (typeof value !== 'string') + invalidAt(`Maestro ${name} expects a string.`, entry.value, context); + const normalized = value.toLowerCase(); + if (MAESTRO_PERMISSION_VALUES.has(normalized)) return normalized; + if (VARIABLE_PATTERN.test(value)) return value; + if (value.includes('${')) + invalidAt( + `Maestro ${name} only supports allow|deny|unset (plus always|inuse|never|limited for location/photos) or a bare \${VAR} lookup; JavaScript expressions are not supported.`, + entry.value, + context, + ); + invalidAt( + `Maestro ${name} expects allow|deny|unset (plus always|inuse|never|limited for location/photos) or a bare \${VAR} lookup.`, + entry.value, + context, + ); +} + function parseLaunchArguments( node: Node | null | undefined, name: string, diff --git a/packages/maestro/src/internal/program-ir-values.ts b/packages/maestro/src/internal/program-ir-values.ts index 4883e79d2e..789056b431 100644 --- a/packages/maestro/src/internal/program-ir-values.ts +++ b/packages/maestro/src/internal/program-ir-values.ts @@ -199,7 +199,7 @@ export function readOptionalBoolean( return value; } -const VARIABLE_PATTERN = /^\$\{[A-Za-z_][A-Za-z0-9_.]*\}$/; +export const VARIABLE_PATTERN = /^\$\{[A-Za-z_][A-Za-z0-9_.]*\}$/; const NUMERIC_STRING_PATTERN = /^-?\d+(\.\d+)?$/; const INTEGER_STRING_PATTERN = /^-?\d+$/; diff --git a/packages/maestro/src/internal/program-ir.ts b/packages/maestro/src/internal/program-ir.ts index d32e7844d5..947815f65f 100644 --- a/packages/maestro/src/internal/program-ir.ts +++ b/packages/maestro/src/internal/program-ir.ts @@ -55,6 +55,7 @@ export type MaestroLaunchAppCommand = { appId?: string; stopApp?: boolean; clearState?: boolean; + permissions?: Record; arguments?: MaestroLaunchArguments; launchArguments?: MaestroLaunchArguments; }; @@ -207,6 +208,14 @@ export type MaestroStopAppCommand = { appId?: string; }; +export type MaestroSetPermissionsCommand = MaestroOptionalCommand & { + kind: 'setPermissions'; + source: MaestroSourceLocation; + appId?: string; + permissions: Record; + label?: string; +}; + export type MaestroRunScriptCommand = { kind: 'runScript'; source: MaestroSourceLocation; @@ -265,6 +274,7 @@ export type MaestroCommand = | MaestroBackCommand | MaestroWaitForAnimationToEndCommand | MaestroStopAppCommand + | MaestroSetPermissionsCommand | MaestroRunScriptCommand | MaestroRunFlowCommand | MaestroRepeatCommand diff --git a/packages/maestro/src/internal/runtime-port-commands.ts b/packages/maestro/src/internal/runtime-port-commands.ts index 577ec13846..6f3556aeb4 100644 --- a/packages/maestro/src/internal/runtime-port-commands.ts +++ b/packages/maestro/src/internal/runtime-port-commands.ts @@ -29,7 +29,9 @@ type MaestroCommandOf = Extract< { kind: K } >; -type MaestroLifecycleCommand = MaestroCommandOf<'launchApp' | 'stopApp' | 'openLink'>; +type MaestroLifecycleCommand = MaestroCommandOf< + 'launchApp' | 'stopApp' | 'setPermissions' | 'openLink' +>; type MaestroTargetCommand = MaestroCommandOf<'tapOn' | 'doubleTapOn' | 'longPressOn'>; type MaestroTextCommand = MaestroCommandOf<'inputText' | 'eraseText'>; type MaestroNavigationCommand = MaestroCommandOf< @@ -53,6 +55,7 @@ type MaestroRuntimeCommandHandlers = { const MAESTRO_RUNTIME_COMMAND_HANDLERS = { launchApp: executeLifecycleCommand, stopApp: executeLifecycleCommand, + setPermissions: executeLifecycleCommand, openLink: executeLifecycleCommand, tapOn: executeTargetCommand, doubleTapOn: executeTargetCommand, @@ -77,6 +80,7 @@ const MAESTRO_RUNTIME_COMMAND_HANDLERS = { const MAESTRO_COMMAND_REQUIRES_SETTLED_PREDECESSOR = { launchApp: true, stopApp: true, + setPermissions: true, openLink: true, tapOn: true, doubleTapOn: true, @@ -142,6 +146,16 @@ async function executeLifecycleCommand( context, 'invalidate', ); + case 'setPermissions': + return await invokeOperation( + operations.setPermissions, + { + appId: command.appId ?? request.appId, + permissions: resolveSetPermissions(command.permissions), + }, + context, + 'invalidate', + ); case 'openLink': return await invokeOperation( operations.openLink, @@ -157,11 +171,37 @@ function launchAppInput(command: MaestroCommandOf<'launchApp'>, request: Maestro appId: command.appId ?? request.appId, stopApp: command.stopApp, clearState: command.clearState, + permissions: command.permissions ? resolveSetPermissions(command.permissions) : undefined, arguments: command.arguments, launchArguments: command.launchArguments, }); } +const RESOLVED_PERMISSION_VALUES = new Set([ + 'allow', + 'deny', + 'unset', + 'always', + 'inuse', + 'never', + 'limited', +]); + +function resolveSetPermissions(permissions: Readonly>) { + const resolved: Record = {}; + for (const [name, value] of Object.entries(permissions)) { + const normalized = value.toLowerCase(); + if (!RESOLVED_PERMISSION_VALUES.has(normalized)) { + throw new AppError( + 'INVALID_ARGS', + `Maestro setPermissions.permissions.${name} expects allow|deny|unset (plus always|inuse|never|limited for location/photos); received "${value}".`, + ); + } + resolved[name] = normalized; + } + return resolved; +} + async function executeTargetCommand( command: MaestroTargetCommand, request: MaestroRuntimeRequest, diff --git a/packages/maestro/src/internal/runtime-port-types.ts b/packages/maestro/src/internal/runtime-port-types.ts index 47427b404d..8a4ba96960 100644 --- a/packages/maestro/src/internal/runtime-port-types.ts +++ b/packages/maestro/src/internal/runtime-port-types.ts @@ -119,10 +119,15 @@ export type MaestroRuntimeOperations = { readonly appId?: string; readonly stopApp?: boolean; readonly clearState?: boolean; + readonly permissions?: Readonly>; readonly arguments?: MaestroLaunchArguments; readonly launchArguments?: MaestroLaunchArguments; }>; readonly stopApp: MaestroRuntimeOperation<{ readonly appId?: string }>; + readonly setPermissions: MaestroRuntimeOperation<{ + readonly appId?: string; + readonly permissions: Readonly>; + }>; readonly openLink: MaestroRuntimeOperation<{ readonly link: string }>; readonly tapOn: MaestroRuntimeOperation<{ diff --git a/packages/maestro/src/internal/support-matrix.ts b/packages/maestro/src/internal/support-matrix.ts index 160519b510..6e34b83cd2 100644 --- a/packages/maestro/src/internal/support-matrix.ts +++ b/packages/maestro/src/internal/support-matrix.ts @@ -1,5 +1,5 @@ export const MAESTRO_COMPAT_SUPPORTED_CAPABILITIES = [ - 'Flows: launchApp; runFlow file/inline with platform, visibility, and limited boolean conditions; onFlowStart/onFlowComplete; repeat.times and retry.', + 'Flows: launchApp (with clearState, permissions, and Apple-only launch arguments); setPermissions (mid-flow permission grants/denials/resets, expanded per platform); runFlow file/inline with platform, visibility, and limited boolean conditions; onFlowStart/onFlowComplete; repeat.times and retry.', 'Interactions: tapOn, doubleTapOn, longPressOn, inputText on the focused element, eraseText, openLink, hideKeyboard, basic pressKey, and back; selector targets poll until available and support recursive index, childOf, above, below, leftOf, rightOf, containsChild, containsDescendants, points, and optional; outer command labels are metadata, not target selectors.', 'Assertions and navigation: assertVisible, assertNotVisible, assertTrue (literal values and ${VAR} lookups only; "", "false", "0", "null", and "undefined" are falsy, everything else is truthy), extendedWaitUntil, scroll, scrollUntilVisible, absolute/percentage/target swipe, takeScreenshot, waitForAnimationToEnd, and stopApp.', 'Scripts: ordered runScript file/env scripts with http.post, json, and output variables.', diff --git a/packages/maestro/test/conformance/expected-divergence.ts b/packages/maestro/test/conformance/expected-divergence.ts index f17e413054..b838032ae8 100644 --- a/packages/maestro/test/conformance/expected-divergence.ts +++ b/packages/maestro/test/conformance/expected-divergence.ts @@ -53,11 +53,6 @@ export const FLOW_DIVERGENCES: Record = { reason: 'Standalone killApp is outside the supported subset.', unsupported: ['killApp'], }, - 'upstream/131_setPermissions': { - classification: 'we-reject', - reason: 'Standalone setPermissions is outside the supported subset.', - unsupported: ['setPermissions'], - }, 'upstream/053_repeat_times': { classification: 'we-reject', reason: diff --git a/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-set-permissions.test.ts b/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-set-permissions.test.ts new file mode 100644 index 0000000000..4c368691fa --- /dev/null +++ b/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-set-permissions.test.ts @@ -0,0 +1,97 @@ +import { expect, test } from 'vitest'; +import type { DaemonRequest } from '../../../daemon-request.ts'; +import { createDaemonMaestroRuntimePort } from '../daemon-runtime-port.ts'; +import { makeBaseRequest, makeDependencies } from './daemon-runtime-port-fixtures.ts'; + +function makePort(requests: DaemonRequest[], platform: 'ios' | 'android') { + return createDaemonMaestroRuntimePort({ + baseReq: makeBaseRequest({ flags: { platform, replayBackend: 'maestro' } }), + invoke: async (request) => { + requests.push(request); + return { ok: true, data: {} }; + }, + dependencies: makeDependencies(), + platform, + }); +} + +test('setPermissions fans out to one settings call per permission', async () => { + const requests: DaemonRequest[] = []; + const port = makePort(requests, 'android'); + + await port.execute({ + command: { + kind: 'setPermissions', + source: { line: 3 }, + permissions: { all: 'deny', notifications: 'unset' }, + }, + appId: 'com.example.app', + generation: 0, + env: {}, + invalidateObservation() {}, + }); + + expect(requests.map(({ command }) => command)).toEqual([ + 'settings', + 'settings', + 'settings', + 'settings', + 'settings', + 'settings', + ]); + expect(requests.map(({ positionals }) => positionals)).toEqual([ + ['permission', 'deny', 'camera'], + ['permission', 'deny', 'contacts'], + ['permission', 'deny', 'microphone'], + ['permission', 'deny', 'notifications'], + ['permission', 'deny', 'photos'], + ['permission', 'reset', 'notifications'], + ]); + expect( + requests.every(({ internal }) => internal?.settingsAppBundleId === 'com.example.app'), + ).toBe(true); +}); + +test('launchApp applies permissions after the state-clearing launch', async () => { + const requests: DaemonRequest[] = []; + const port = makePort(requests, 'android'); + + await port.execute({ + command: { + kind: 'launchApp', + source: { line: 3 }, + appId: 'com.example.app', + clearState: true, + permissions: { camera: 'allow' }, + }, + appId: 'com.example.app', + generation: 0, + env: {}, + invalidateObservation() {}, + }); + + expect(requests.map(({ command }) => command)).toEqual(['open', 'settings']); + expect(requests[1]?.positionals).toEqual(['permission', 'grant', 'camera']); + expect(requests[1]?.internal?.settingsAppBundleId).toBe('com.example.app'); +}); + +test('setPermissions without an appId leaves targeting to the session app', async () => { + const requests: DaemonRequest[] = []; + const port = makePort(requests, 'ios'); + + await port.execute({ + command: { + kind: 'setPermissions', + source: { line: 2 }, + permissions: { location: 'always' }, + }, + generation: 0, + env: {}, + invalidateObservation() {}, + }); + + expect(requests.map(({ positionals }) => positionals)).toEqual([ + ['permission', 'grant', 'location-always'], + ]); + expect(requests[0]).not.toHaveProperty('internal'); +}); diff --git a/src/daemon/adapters/maestro/__tests__/daemon-runtime-public-operation.test.ts b/src/daemon/adapters/maestro/__tests__/daemon-runtime-public-operation.test.ts index 0a2d5fbcb2..2039f50747 100644 --- a/src/daemon/adapters/maestro/__tests__/daemon-runtime-public-operation.test.ts +++ b/src/daemon/adapters/maestro/__tests__/daemon-runtime-public-operation.test.ts @@ -175,6 +175,31 @@ describe('Maestro public operation projection', () => { flags: { noRecord: true }, }, }, + { + operation: { + kind: 'settingsPermission', + appId: 'com.example', + state: 'grant', + permission: 'camera', + }, + expected: { + command: 'settings', + positionals: ['permission', 'grant', 'camera'], + internal: { settingsAppBundleId: 'com.example' }, + }, + }, + { + operation: { + kind: 'settingsPermission', + state: 'grant', + permission: 'photos', + mode: 'limited', + }, + expected: { + command: 'settings', + positionals: ['permission', 'grant', 'photos', 'limited'], + }, + }, ])('projects $operation.kind', ({ operation, expected }) => { expect(projectMaestroPublicOperation(operation)).toEqual(expected); }); diff --git a/src/daemon/adapters/maestro/__tests__/set-permissions-mapping.test.ts b/src/daemon/adapters/maestro/__tests__/set-permissions-mapping.test.ts new file mode 100644 index 0000000000..a6b9f500fd --- /dev/null +++ b/src/daemon/adapters/maestro/__tests__/set-permissions-mapping.test.ts @@ -0,0 +1,89 @@ +import assert from 'node:assert/strict'; +import { describe, test } from 'vitest'; +import { mapMaestroSetPermissions } from '../set-permissions-mapping.ts'; + +describe('mapMaestroSetPermissions', () => { + test('maps single permissions to grant/deny/reset', () => { + assert.deepEqual( + mapMaestroSetPermissions({ camera: 'allow', notifications: 'deny' }, 'android'), + [ + { state: 'grant', permission: 'camera' }, + { state: 'deny', permission: 'notifications' }, + ], + ); + assert.deepEqual(mapMaestroSetPermissions({ notifications: 'unset' }, 'android'), [ + { state: 'reset', permission: 'notifications' }, + ]); + }); + + test('expands all to the platform servable set with specifics overriding', () => { + assert.deepEqual(mapMaestroSetPermissions({ all: 'deny', notifications: 'unset' }, 'android'), [ + { state: 'deny', permission: 'camera' }, + { state: 'deny', permission: 'contacts' }, + { state: 'deny', permission: 'microphone' }, + { state: 'deny', permission: 'notifications' }, + { state: 'deny', permission: 'photos' }, + { state: 'reset', permission: 'notifications' }, + ]); + const ios = mapMaestroSetPermissions({ all: 'allow' }, 'ios'); + assert.deepEqual( + ios.map((mutation) => mutation.permission), + [ + 'calendar', + 'camera', + 'contacts', + 'location', + 'media-library', + 'microphone', + 'motion', + 'notifications', + 'photos', + 'reminders', + 'siri', + ], + ); + assert.ok(ios.every((mutation) => mutation.state === 'grant')); + }); + + test('maps iOS granular values and the medialibrary alias', () => { + assert.deepEqual(mapMaestroSetPermissions({ location: 'always' }, 'ios'), [ + { state: 'grant', permission: 'location-always' }, + ]); + assert.deepEqual(mapMaestroSetPermissions({ location: 'inuse' }, 'ios'), [ + { state: 'grant', permission: 'location' }, + ]); + assert.deepEqual(mapMaestroSetPermissions({ location: 'never' }, 'ios'), [ + { state: 'reset', permission: 'location' }, + ]); + assert.deepEqual(mapMaestroSetPermissions({ photos: 'limited' }, 'ios'), [ + { state: 'grant', permission: 'photos', mode: 'limited' }, + ]); + assert.deepEqual(mapMaestroSetPermissions({ medialibrary: 'allow' }, 'ios'), [ + { state: 'grant', permission: 'media-library' }, + ]); + }); + + test('rejects unservable names, empty maps, and nonsense value combos', () => { + assert.throws( + () => mapMaestroSetPermissions({ bluetooth: 'allow' }, 'android'), + /bluetooth.*not supported on android/i, + ); + assert.throws( + () => mapMaestroSetPermissions({ speech: 'allow' }, 'ios'), + /speech.*not supported on ios/i, + ); + assert.throws( + () => + mapMaestroSetPermissions( + { 'android.permission.MANAGE_EXTERNAL_STORAGE': 'deny' }, + 'android', + ), + /not supported on android/i, + ); + assert.throws(() => mapMaestroSetPermissions({}, 'ios'), /at least one permission/i); + assert.throws( + () => mapMaestroSetPermissions({ camera: 'always' }, 'ios'), + /camera.*does not accept.*always/i, + ); + }); +}); diff --git a/src/daemon/adapters/maestro/daemon-runtime-port.ts b/src/daemon/adapters/maestro/daemon-runtime-port.ts index 70e7c8b066..e33242799c 100644 --- a/src/daemon/adapters/maestro/daemon-runtime-port.ts +++ b/src/daemon/adapters/maestro/daemon-runtime-port.ts @@ -11,6 +11,7 @@ import { import { registerDiagnosticSensitiveValue } from '@agent-device/host-kit/diagnostics'; import { stripUndefined } from '@agent-device/kernel/record'; import { executeRunScriptFile } from './run-script-execution.ts'; +import { mapMaestroSetPermissions } from './set-permissions-mapping.ts'; import { waitForMaestroAnimationToEnd } from './wait-for-animation-to-end.ts'; import { observeTypedMaestroCondition, @@ -77,6 +78,27 @@ function createDaemonMaestroRuntimeParts(options: CreateDaemonMaestroRuntimeOper context: MaestroRuntimeOperationContext, stability: 'none' | 'deferred' = 'none', ) => await withMutation(() => invoke(operation), context, stability); + // launchApp.permissions applies after the (possibly state-clearing) launch, + // so grants land on the fresh install rather than being wiped by it. + const applyPermissions = async ( + appId: string | undefined, + permissions: Readonly>, + context: MaestroRuntimeOperationContext, + ): Promise => { + const mutations = mapMaestroSetPermissions(permissions, platform); + for (const mutation of mutations) { + await invokeMutation( + { + kind: 'settingsPermission', + ...(appId ? { appId } : {}), + state: mutation.state, + permission: mutation.permission, + ...(mutation.mode ? { mode: mutation.mode } : {}), + }, + context, + ); + } + }; const typeTextAndSettle = async ( text: string, context: MaestroRuntimeOperationContext, @@ -127,11 +149,15 @@ function createDaemonMaestroRuntimeParts(options: CreateDaemonMaestroRuntimeOper context, 'deferred', ); + if (input.permissions) await applyPermissions(appId, input.permissions, context); }, stopApp: async (input, context) => { const appId = input.appId ?? context.appId; await invokeMutation({ kind: 'stopApp', ...(appId ? { appId } : {}) }, context); }, + setPermissions: async (input, context) => { + await applyPermissions(input.appId ?? context.appId, input.permissions, context); + }, openLink: async (input, context) => { await invokeMutation( { diff --git a/src/daemon/adapters/maestro/daemon-runtime-public-operation.ts b/src/daemon/adapters/maestro/daemon-runtime-public-operation.ts index 72a0bb3016..3cb762b267 100644 --- a/src/daemon/adapters/maestro/daemon-runtime-public-operation.ts +++ b/src/daemon/adapters/maestro/daemon-runtime-public-operation.ts @@ -20,6 +20,13 @@ export type MaestroPublicOperation = launchArgs: string[]; } | { kind: 'stopApp'; appId?: string } + | { + kind: 'settingsPermission'; + appId?: string; + state: 'grant' | 'deny' | 'reset'; + permission: string; + mode?: 'full' | 'limited'; + } | { kind: 'openLink'; appId?: string; link: string; prewarmRunner: boolean } | { kind: 'typeText'; text: string } | { @@ -47,6 +54,7 @@ export function projectMaestroPublicOperation( ): ProjectedMaestroPublicOperation { if (isAppOperation(operation)) return projectAppOperation(operation); if (isCaptureOperation(operation)) return projectCaptureOperation(operation); + if (operation.kind === 'settingsPermission') return projectSettingsPermission(operation); return projectInputOperation(operation); } @@ -106,9 +114,24 @@ function projectOpenLink( }; } +function projectSettingsPermission( + operation: Extract, +): ProjectedMaestroPublicOperation { + return { + command: 'settings', + positionals: [ + 'permission', + operation.state, + operation.permission, + ...(operation.mode ? [operation.mode] : []), + ], + ...(operation.appId ? { internal: { settingsAppBundleId: operation.appId } } : {}), + }; +} + type MaestroInputOperation = Exclude< MaestroPublicOperation, - MaestroAppOperation | MaestroCaptureOperation + MaestroAppOperation | MaestroCaptureOperation | { kind: 'settingsPermission' } >; function projectInputOperation(operation: MaestroInputOperation): ProjectedMaestroPublicOperation { diff --git a/src/daemon/adapters/maestro/set-permissions-mapping.ts b/src/daemon/adapters/maestro/set-permissions-mapping.ts new file mode 100644 index 0000000000..971f64c9ea --- /dev/null +++ b/src/daemon/adapters/maestro/set-permissions-mapping.ts @@ -0,0 +1,123 @@ +import { AppError } from '@agent-device/kernel/errors'; + +export type MaestroPermissionMutation = { + readonly state: 'grant' | 'deny' | 'reset'; + readonly permission: string; + readonly mode?: 'full' | 'limited'; +}; + +/** + * Canonical Maestro names each `settings permission` backend can serve. Names + * outside these lists (bluetooth/phone/sms/storage/location/calendar on + * Android; speech/usertracking/homekit on iOS; health everywhere; custom + * Android IDs) fail loudly below instead of being silently skipped — + * extending the platform backends is a separate, device-verified change. + */ +const EXPANDABLE_PERMISSIONS = { + android: ['camera', 'contacts', 'microphone', 'notifications', 'photos'], + ios: [ + 'calendar', + 'camera', + 'contacts', + 'location', + 'media-library', + 'microphone', + 'motion', + 'notifications', + 'photos', + 'reminders', + 'siri', + ], +} as const; + +/** Per-platform hint for names the backends cannot serve yet. */ +const UNSUPPORTED_HINTS = { + android: + 'Supported: camera, contacts, microphone, notifications, photos (via all or individually). Other names need platform-backend support first.', + ios: 'Supported: calendar, camera, contacts, location, media-library, microphone, motion, notifications, photos, reminders, siri (via all or individually). Granular iOS values: location always|inuse|never, photos limited.', +} as const; + +/** Non-canonical spellings accepted alongside the lists above. */ +const PERMISSION_ALIASES: Readonly> = { + medialibrary: 'media-library', +}; + +function canonicalName(name: string): string { + const normalized = name.toLowerCase(); + return PERMISSION_ALIASES[normalized] ?? normalized; +} + +/** Plain values map 1:1 onto settings states; granular iOS values map per permission. */ +const PLAIN_VALUE_STATES = { allow: 'grant', deny: 'deny', unset: 'reset' } as const; + +const GRANULAR_MUTATIONS: Record> = { + location: { + always: { state: 'grant', permission: 'location-always' }, + inuse: { state: 'grant', permission: 'location' }, + never: { state: 'reset', permission: 'location' }, + }, + photos: { + limited: { state: 'grant', permission: 'photos', mode: 'limited' }, + }, +}; + +const GRANULAR_HINTS: Record = { + location: 'Use allow|deny|unset, or the iOS granular always|inuse|never.', + photos: 'Use allow|deny|unset, or the iOS granular limited.', +}; + +/** + * Expand a Maestro `setPermissions` map into ordered `settings permission` + * mutations. `all` expands to the platform's servable set first so specific + * entries always override it regardless of authored order. Values arrive + * lowercased from the Maestro runtime layer; anything else is refused. + */ +export function mapMaestroSetPermissions( + permissions: Readonly>, + platform: 'ios' | 'android', +): MaestroPermissionMutation[] { + const entries = Object.entries(permissions); + if (entries.length === 0) { + throw new AppError('INVALID_ARGS', 'Maestro setPermissions requires at least one permission.'); + } + const expandable = new Set(EXPANDABLE_PERMISSIONS[platform]); + const specific = new Map(); + let allValue: string | undefined; + for (const [name, value] of entries) { + if (name.toLowerCase() === 'all') { + allValue = value; + } else { + specific.set(canonicalName(name), value); + } + } + const merged: Array<[string, string]> = + allValue === undefined + ? [...specific] + : [ + ...EXPANDABLE_PERMISSIONS[platform].map((name): [string, string] => [name, allValue]), + ...specific, + ]; + return merged.map(([name, value]) => mapMaestroPermission(name, value, platform, expandable)); +} + +function mapMaestroPermission( + name: string, + value: string, + platform: 'ios' | 'android', + expandable: ReadonlySet, +): MaestroPermissionMutation { + if (!expandable.has(name)) { + throw new AppError( + 'UNSUPPORTED_OPERATION', + `Maestro permission "${name}" is not supported on ${platform} yet.`, + { hint: UNSUPPORTED_HINTS[platform] }, + ); + } + const granular = GRANULAR_MUTATIONS[name]?.[value]; + if (granular) return granular; + const state = PLAIN_VALUE_STATES[value as keyof typeof PLAIN_VALUE_STATES]; + if (state) return { state, permission: name }; + throw new AppError('INVALID_ARGS', `Maestro permission "${name}" does not accept "${value}".`, { + hint: GRANULAR_HINTS[name] ?? 'Use allow|deny|unset.', + }); +} diff --git a/src/daemon/daemon-request.ts b/src/daemon/daemon-request.ts index a1cebef2af..ea8403b6dd 100644 --- a/src/daemon/daemon-request.ts +++ b/src/daemon/daemon-request.ts @@ -111,6 +111,13 @@ type DaemonRequestInternal = { * spoof authored provenance. Same channel as `replayTargetGuard` above. */ replayPlanStep?: boolean; + /** + * Maestro `setPermissions` app targeting. The `settings permission` + * positionals carry no app slot, so the Maestro adapter threads an explicit + * appId here; the settings handler prefers it over the session app. + * Daemon-only like the other keys above — never accepted off the wire. + */ + settingsAppBundleId?: string; }; /** diff --git a/src/daemon/handlers/snapshot-settings.ts b/src/daemon/handlers/snapshot-settings.ts index 107e7a9d7b..e4cb0f921c 100644 --- a/src/daemon/handlers/snapshot-settings.ts +++ b/src/daemon/handlers/snapshot-settings.ts @@ -172,7 +172,8 @@ export async function handleSettingsCommand( return errorResponse('INVALID_ARGS', getUnsupportedMacOsSettingMessage(setting)); } - const appBundleId = parsed.appBundleId ?? session?.appBundleId; + const appBundleId = + parsed.appBundleId ?? req.internal?.settingsAppBundleId ?? session?.appBundleId; if (setting === 'clear-app-state' && !appBundleId) { return errorResponse( 'INVALID_ARGS', diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index 5940fc7205..d5082308f8 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -70,7 +70,7 @@ agent-device test ./maestro-flows --maestro --platform android --artifacts-dir . Supported subset: -- Flows: `launchApp`; `runFlow` file/inline with platform, visibility, and limited boolean conditions; `onFlowStart`/`onFlowComplete`; `repeat.times` and retry. +- Flows: `launchApp` (with `clearState`, `permissions`, and Apple-only launch arguments); `setPermissions` (mid-flow permission grants/denials/resets, expanded per platform); `runFlow` file/inline with platform, visibility, and limited boolean conditions; `onFlowStart`/`onFlowComplete`; `repeat.times` and retry. - Interactions: `tapOn`, `doubleTapOn`, `longPressOn`, `inputText` on the focused element, `eraseText`, `openLink`, `hideKeyboard`, basic `pressKey`, and `back`; selector targets poll until available and support recursive `index`, `childOf`, `above`, `below`, `leftOf`, `rightOf`, `containsChild`, `containsDescendants`, points, and `optional`; outer command labels are metadata, not target selectors. - Assertions and navigation: `assertVisible`, `assertNotVisible`, `assertTrue` (literal values and `${VAR}` lookups only; `""`, `"false"`, `"0"`, `"null"`, and `"undefined"` are falsy, everything else is truthy), `extendedWaitUntil`, `scroll`, `scrollUntilVisible`, absolute/percentage/target `swipe`, `takeScreenshot`, `waitForAnimationToEnd`, and `stopApp`. - Scripts: ordered `runScript` file/env scripts with `http.post`, `json`, and `output` variables. From 4de150aa37539d042f6305e9349b7d362eb22784 Mon Sep 17 00:00:00 2001 From: Rohit <40559587+Rohit3523@users.noreply.github.com> Date: Sun, 6 Sep 2026 23:13:22 +0530 Subject: [PATCH 2/6] fix(maestro): apply launchApp permissions before launch, deny on location never - launchApp.permissions now runs after state clearing but before open, so startup code observes the requested state; the map is validated before any mutation via a new clearAppState public operation. - location never maps to deny (unset keeps the reset prompt state). - ios all expansion skips the probe-unsupported camera/notifications so the sequential mutations cannot stop partway through. --- ...aemon-runtime-port-set-permissions.test.ts | 51 ++++++++++++++++++- .../daemon-runtime-public-operation.test.ts | 8 +++ .../__tests__/set-permissions-mapping.test.ts | 21 +++++++- .../adapters/maestro/daemon-runtime-port.ts | 43 +++++++++++++--- .../daemon-runtime-public-operation.ts | 16 +++++- .../maestro/set-permissions-mapping.ts | 32 +++++++++--- 6 files changed, 153 insertions(+), 18 deletions(-) diff --git a/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-set-permissions.test.ts b/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-set-permissions.test.ts index 4c368691fa..e149a65d89 100644 --- a/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-set-permissions.test.ts +++ b/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-set-permissions.test.ts @@ -52,7 +52,7 @@ test('setPermissions fans out to one settings call per permission', async () => ).toBe(true); }); -test('launchApp applies permissions after the state-clearing launch', async () => { +test('launchApp applies permissions after clearing but before launch', async () => { const requests: DaemonRequest[] = []; const port = makePort(requests, 'android'); @@ -70,9 +70,56 @@ test('launchApp applies permissions after the state-clearing launch', async () = invalidateObservation() {}, }); - expect(requests.map(({ command }) => command)).toEqual(['open', 'settings']); + expect(requests.map(({ command }) => command)).toEqual(['settings', 'settings', 'open']); + expect(requests[0]?.positionals).toEqual(['clear-app-state', 'com.example.app']); expect(requests[1]?.positionals).toEqual(['permission', 'grant', 'camera']); expect(requests[1]?.internal?.settingsAppBundleId).toBe('com.example.app'); + expect(requests[2]?.command).toBe('open'); + expect(requests[2]?.flags).not.toMatchObject({ clearAppState: true }); +}); + +test('launchApp without clearState applies permissions before launch', async () => { + const requests: DaemonRequest[] = []; + const port = makePort(requests, 'android'); + + await port.execute({ + command: { + kind: 'launchApp', + source: { line: 3 }, + appId: 'com.example.app', + permissions: { camera: 'allow' }, + }, + appId: 'com.example.app', + generation: 0, + env: {}, + invalidateObservation() {}, + }); + + expect(requests.map(({ command }) => command)).toEqual(['settings', 'open']); + expect(requests[0]?.positionals).toEqual(['permission', 'grant', 'camera']); + expect(requests[1]?.command).toBe('open'); +}); + +test('launchApp with rejected permissions launches nothing', async () => { + const requests: DaemonRequest[] = []; + const port = makePort(requests, 'android'); + + await expect( + port.execute({ + command: { + kind: 'launchApp', + source: { line: 3 }, + appId: 'com.example.app', + clearState: true, + permissions: { bluetooth: 'allow' }, + }, + appId: 'com.example.app', + generation: 0, + env: {}, + invalidateObservation() {}, + }), + ).rejects.toThrow(/bluetooth.*not supported on android/i); + expect(requests).toEqual([]); }); test('setPermissions without an appId leaves targeting to the session app', async () => { diff --git a/src/daemon/adapters/maestro/__tests__/daemon-runtime-public-operation.test.ts b/src/daemon/adapters/maestro/__tests__/daemon-runtime-public-operation.test.ts index 2039f50747..8f19d88ff4 100644 --- a/src/daemon/adapters/maestro/__tests__/daemon-runtime-public-operation.test.ts +++ b/src/daemon/adapters/maestro/__tests__/daemon-runtime-public-operation.test.ts @@ -48,6 +48,14 @@ describe('Maestro public operation projection', () => { operation: { kind: 'stopApp' }, expected: { command: 'close', positionals: [], internal: { closeAppOnly: true } }, }, + { + operation: { kind: 'clearAppState', appId: 'com.example' }, + expected: { command: 'settings', positionals: ['clear-app-state', 'com.example'] }, + }, + { + operation: { kind: 'clearAppState' }, + expected: { command: 'settings', positionals: ['clear-app-state'] }, + }, { operation: { kind: 'openLink', diff --git a/src/daemon/adapters/maestro/__tests__/set-permissions-mapping.test.ts b/src/daemon/adapters/maestro/__tests__/set-permissions-mapping.test.ts index a6b9f500fd..7b0b7fbd96 100644 --- a/src/daemon/adapters/maestro/__tests__/set-permissions-mapping.test.ts +++ b/src/daemon/adapters/maestro/__tests__/set-permissions-mapping.test.ts @@ -30,13 +30,11 @@ describe('mapMaestroSetPermissions', () => { ios.map((mutation) => mutation.permission), [ 'calendar', - 'camera', 'contacts', 'location', 'media-library', 'microphone', 'motion', - 'notifications', 'photos', 'reminders', 'siri', @@ -45,6 +43,21 @@ describe('mapMaestroSetPermissions', () => { assert.ok(ios.every((mutation) => mutation.state === 'grant')); }); + test('ios all skips the probe-unsupported camera and notifications', () => { + // This host's `simctl privacy help` (the source the iOS backend probe + // parses) lists neither service, so `all` excludes them rather than + // stopping the sequential mutations partway through. Explicit entries + // still reach the backend for its loud verdict. + const permissions = mapMaestroSetPermissions({ all: 'deny' }, 'ios').map( + (mutation) => mutation.permission, + ); + assert.ok(!permissions.includes('camera')); + assert.ok(!permissions.includes('notifications')); + assert.deepEqual(mapMaestroSetPermissions({ camera: 'allow' }, 'ios'), [ + { state: 'grant', permission: 'camera' }, + ]); + }); + test('maps iOS granular values and the medialibrary alias', () => { assert.deepEqual(mapMaestroSetPermissions({ location: 'always' }, 'ios'), [ { state: 'grant', permission: 'location-always' }, @@ -53,6 +66,10 @@ describe('mapMaestroSetPermissions', () => { { state: 'grant', permission: 'location' }, ]); assert.deepEqual(mapMaestroSetPermissions({ location: 'never' }, 'ios'), [ + { state: 'deny', permission: 'location' }, + ]); + // never denies access while unset restores the prompt state. + assert.deepEqual(mapMaestroSetPermissions({ location: 'unset' }, 'ios'), [ { state: 'reset', permission: 'location' }, ]); assert.deepEqual(mapMaestroSetPermissions({ photos: 'limited' }, 'ios'), [ diff --git a/src/daemon/adapters/maestro/daemon-runtime-port.ts b/src/daemon/adapters/maestro/daemon-runtime-port.ts index e33242799c..f71929786d 100644 --- a/src/daemon/adapters/maestro/daemon-runtime-port.ts +++ b/src/daemon/adapters/maestro/daemon-runtime-port.ts @@ -11,7 +11,10 @@ import { import { registerDiagnosticSensitiveValue } from '@agent-device/host-kit/diagnostics'; import { stripUndefined } from '@agent-device/kernel/record'; import { executeRunScriptFile } from './run-script-execution.ts'; -import { mapMaestroSetPermissions } from './set-permissions-mapping.ts'; +import { + mapMaestroSetPermissions, + type MaestroPermissionMutation, +} from './set-permissions-mapping.ts'; import { waitForMaestroAnimationToEnd } from './wait-for-animation-to-end.ts'; import { observeTypedMaestroCondition, @@ -78,14 +81,15 @@ function createDaemonMaestroRuntimeParts(options: CreateDaemonMaestroRuntimeOper context: MaestroRuntimeOperationContext, stability: 'none' | 'deferred' = 'none', ) => await withMutation(() => invoke(operation), context, stability); - // launchApp.permissions applies after the (possibly state-clearing) launch, - // so grants land on the fresh install rather than being wiped by it. - const applyPermissions = async ( + // launchApp.permissions applies after state clearing but before launch, so + // startup code observes the requested state, and the map is validated before + // any mutation — a rejected map launches nothing. Splitting clear from open + // matches what open --clearAppState does (clear-app-state, then open). + const applyPermissionMutations = async ( appId: string | undefined, - permissions: Readonly>, + mutations: ReadonlyArray, context: MaestroRuntimeOperationContext, ): Promise => { - const mutations = mapMaestroSetPermissions(permissions, platform); for (const mutation of mutations) { await invokeMutation( { @@ -99,6 +103,13 @@ function createDaemonMaestroRuntimeParts(options: CreateDaemonMaestroRuntimeOper ); } }; + const applyPermissions = async ( + appId: string | undefined, + permissions: Readonly>, + context: MaestroRuntimeOperationContext, + ): Promise => { + await applyPermissionMutations(appId, mapMaestroSetPermissions(permissions, platform), context); + }; const typeTextAndSettle = async ( text: string, context: MaestroRuntimeOperationContext, @@ -138,6 +149,25 @@ function createDaemonMaestroRuntimeParts(options: CreateDaemonMaestroRuntimeOper ]; const clearState = input.clearState === true; const relaunch = !clearState && input.stopApp !== false; + if (input.permissions) { + const mutations = mapMaestroSetPermissions(input.permissions, platform); + if (clearState) { + await invokeMutation({ kind: 'clearAppState', ...(appId ? { appId } : {}) }, context); + } + await applyPermissionMutations(appId, mutations, context); + await invokeMutation( + { + kind: 'launchApp', + ...(appId ? { appId } : {}), + relaunch, + clearState: false, + launchArgs, + }, + context, + 'deferred', + ); + return; + } await invokeMutation( { kind: 'launchApp', @@ -149,7 +179,6 @@ function createDaemonMaestroRuntimeParts(options: CreateDaemonMaestroRuntimeOper context, 'deferred', ); - if (input.permissions) await applyPermissions(appId, input.permissions, context); }, stopApp: async (input, context) => { const appId = input.appId ?? context.appId; diff --git a/src/daemon/adapters/maestro/daemon-runtime-public-operation.ts b/src/daemon/adapters/maestro/daemon-runtime-public-operation.ts index 3cb762b267..709f9135e6 100644 --- a/src/daemon/adapters/maestro/daemon-runtime-public-operation.ts +++ b/src/daemon/adapters/maestro/daemon-runtime-public-operation.ts @@ -20,6 +20,7 @@ export type MaestroPublicOperation = launchArgs: string[]; } | { kind: 'stopApp'; appId?: string } + | { kind: 'clearAppState'; appId?: string } | { kind: 'settingsPermission'; appId?: string; @@ -55,6 +56,7 @@ export function projectMaestroPublicOperation( if (isAppOperation(operation)) return projectAppOperation(operation); if (isCaptureOperation(operation)) return projectCaptureOperation(operation); if (operation.kind === 'settingsPermission') return projectSettingsPermission(operation); + if (operation.kind === 'clearAppState') return projectClearAppState(operation); return projectInputOperation(operation); } @@ -104,6 +106,15 @@ function projectStopApp( }; } +function projectClearAppState( + operation: Extract, +): ProjectedMaestroPublicOperation { + return { + command: 'settings', + positionals: operation.appId ? ['clear-app-state', operation.appId] : ['clear-app-state'], + }; +} + function projectOpenLink( operation: Extract, ): ProjectedMaestroPublicOperation { @@ -131,7 +142,10 @@ function projectSettingsPermission( type MaestroInputOperation = Exclude< MaestroPublicOperation, - MaestroAppOperation | MaestroCaptureOperation | { kind: 'settingsPermission' } + | MaestroAppOperation + | MaestroCaptureOperation + | { kind: 'settingsPermission' } + | { kind: 'clearAppState' } >; function projectInputOperation(operation: MaestroInputOperation): ProjectedMaestroPublicOperation { diff --git a/src/daemon/adapters/maestro/set-permissions-mapping.ts b/src/daemon/adapters/maestro/set-permissions-mapping.ts index 971f64c9ea..bf2cbfe3fe 100644 --- a/src/daemon/adapters/maestro/set-permissions-mapping.ts +++ b/src/daemon/adapters/maestro/set-permissions-mapping.ts @@ -12,6 +12,10 @@ export type MaestroPermissionMutation = { * Android; speech/usertracking/homekit on iOS; health everywhere; custom * Android IDs) fail loudly below instead of being silently skipped — * extending the platform backends is a separate, device-verified change. + * + * Explicit entries keep the full servable set so the backend stays the owner + * of the verdict: a name the runtime cannot serve (e.g. iOS camera on runtimes + * whose `simctl privacy help` lists no camera service) fails loudly there. */ const EXPANDABLE_PERMISSIONS = { android: ['camera', 'contacts', 'microphone', 'notifications', 'photos'], @@ -30,6 +34,20 @@ const EXPANDABLE_PERMISSIONS = { ], } as const; +/** + * Names excluded from `all` expansion. `all` must succeed on the runtimes we + * ship, so it covers only the probe-supported subset: this host's + * `simctl privacy help` (the same source `getSimctlPrivacyServices` parses in + * the iOS backend) lists neither camera nor notifications, and the iOS backend + * rejects grant/deny for notifications with UNSUPPORTED_OPERATION — keeping + * either in `all` would stop the sequential mutations partway through. + * Explicit entries for those names still reach the backend above. + */ +const ALL_EXCLUDED_PERMISSIONS: Readonly> = { + android: [], + ios: ['camera', 'notifications'], +}; + /** Per-platform hint for names the backends cannot serve yet. */ const UNSUPPORTED_HINTS = { android: @@ -54,7 +72,8 @@ const GRANULAR_MUTATIONS: Record = { /** * Expand a Maestro `setPermissions` map into ordered `settings permission` - * mutations. `all` expands to the platform's servable set first so specific + * mutations. `all` expands to the platform's servable subset first so specific * entries always override it regardless of authored order. Values arrive * lowercased from the Maestro runtime layer; anything else is refused. + * The expansion is fully validated here, so callers must map before issuing + * any mutation — a rejected map changes nothing. */ export function mapMaestroSetPermissions( permissions: Readonly>, @@ -81,6 +102,8 @@ export function mapMaestroSetPermissions( throw new AppError('INVALID_ARGS', 'Maestro setPermissions requires at least one permission.'); } const expandable = new Set(EXPANDABLE_PERMISSIONS[platform]); + const excluded = new Set(ALL_EXCLUDED_PERMISSIONS[platform]); + const allExpansion = EXPANDABLE_PERMISSIONS[platform].filter((name) => !excluded.has(name)); const specific = new Map(); let allValue: string | undefined; for (const [name, value] of entries) { @@ -93,10 +116,7 @@ export function mapMaestroSetPermissions( const merged: Array<[string, string]> = allValue === undefined ? [...specific] - : [ - ...EXPANDABLE_PERMISSIONS[platform].map((name): [string, string] => [name, allValue]), - ...specific, - ]; + : [...allExpansion.map((name): [string, string] => [name, allValue]), ...specific]; return merged.map(([name, value]) => mapMaestroPermission(name, value, platform, expandable)); } From 7285bb189ab6170843fbd1ab3a2ba33e609facf5 Mon Sep 17 00:00:00 2001 From: Rohit <40559587+Rohit3523@users.noreply.github.com> Date: Mon, 7 Sep 2026 00:05:24 +0530 Subject: [PATCH 3/6] fix(maestro): reset notifications via reset-all fallback, declare permission divergences - iOS reset notifications bypasses the simctl probe gate into the existing reset-all fallback (verified live on iOS 26.3 where help omits the service); grant/deny stay loud rejections. - Support matrix and replay docs now declare the intentional gaps vs upstream: no silent all-allow launch default, backend-servable all expansion, loud rejections, true-reset unset, never denies. --- .../maestro/src/internal/support-matrix.ts | 2 +- .../src/core/__tests__/app-settings.test.ts | 38 +++++++++++++ .../platform-apple/src/core/app-settings.ts | 55 ++++++++++++------- website/docs/docs/replay-e2e.md | 2 +- 4 files changed, 75 insertions(+), 22 deletions(-) diff --git a/packages/maestro/src/internal/support-matrix.ts b/packages/maestro/src/internal/support-matrix.ts index 6e34b83cd2..e316f0ee48 100644 --- a/packages/maestro/src/internal/support-matrix.ts +++ b/packages/maestro/src/internal/support-matrix.ts @@ -1,5 +1,5 @@ export const MAESTRO_COMPAT_SUPPORTED_CAPABILITIES = [ - 'Flows: launchApp (with clearState, permissions, and Apple-only launch arguments); setPermissions (mid-flow permission grants/denials/resets, expanded per platform); runFlow file/inline with platform, visibility, and limited boolean conditions; onFlowStart/onFlowComplete; repeat.times and retry.', + 'Flows: launchApp (with clearState, permissions, and Apple-only launch arguments; permissions apply after state clearing but before launch, and a launchApp without permissions touches nothing — there is no silent all: allow default); setPermissions (mid-flow permission grants/denials/resets; all expands to the backend-servable set — Android: camera/contacts/microphone/notifications/photos, iOS: the simctl privacy help subset excluding camera/notifications; anything else fails loudly instead of being skipped; unset fully resets and location: never denies); runFlow file/inline with platform, visibility, and limited boolean conditions; onFlowStart/onFlowComplete; repeat.times and retry.', 'Interactions: tapOn, doubleTapOn, longPressOn, inputText on the focused element, eraseText, openLink, hideKeyboard, basic pressKey, and back; selector targets poll until available and support recursive index, childOf, above, below, leftOf, rightOf, containsChild, containsDescendants, points, and optional; outer command labels are metadata, not target selectors.', 'Assertions and navigation: assertVisible, assertNotVisible, assertTrue (literal values and ${VAR} lookups only; "", "false", "0", "null", and "undefined" are falsy, everything else is truthy), extendedWaitUntil, scroll, scrollUntilVisible, absolute/percentage/target swipe, takeScreenshot, waitForAnimationToEnd, and stopApp.', 'Scripts: ordered runScript file/env scripts with http.post, json, and output variables.', diff --git a/packages/platform-apple/src/core/__tests__/app-settings.test.ts b/packages/platform-apple/src/core/__tests__/app-settings.test.ts index 6f2d9011e6..5331b0995e 100644 --- a/packages/platform-apple/src/core/__tests__/app-settings.test.ts +++ b/packages/platform-apple/src/core/__tests__/app-settings.test.ts @@ -465,6 +465,44 @@ test('setIosSetting permission reset notifications falls back to reset all when ); }); +test('setIosSetting permission reset notifications falls back to reset all when unlisted in privacy help', async () => { + // Runtimes like iOS 26.3 omit notifications from `simctl privacy help`, yet + // direct reset fails only with "operation not permitted" while `reset all` + // succeeds — so reset bypasses the probe gate into the existing fallback. + const device: DeviceInfo = { + ...IOS_TEST_SIMULATOR, + simulatorSetPath: '/fake/privacy-help-no-notifications', + }; + const HELP_WITHOUT_NOTIFICATIONS = `Usage: simctl privacy [] + + service + The service: + microphone - Allow access to audio input.`; + await withFakeAppleTool( + (args) => { + if (isSimctlListDevices(args)) return BOOTED_SIM_LIST_JSON; + if (args.includes('help')) return HELP_WITHOUT_NOTIFICATIONS; + const flat = args.join(' '); + if (flat.includes('reset notifications com.example.app')) { + return { stderr: 'Failed to reset access\nOperation not permitted', exitCode: 1 }; + } + if (flat.includes('reset all com.example.app')) return ''; + return unexpectedArgs(args); + }, + async ({ calls }) => { + await setIosSetting(device, 'permission', 'reset', 'com.example.app', { + permissionTarget: 'notifications', + }); + const flat = calls.map((args) => args.join(' ')); + assert.equal( + flat.some((line) => line.includes('reset all com.example.app')), + true, + flat.join('; '), + ); + }, + ); +}); + test('setIosSetting permission deny notifications returns unsupported on runtimes that block it', async () => { await withFakeAppleTool( (args) => { diff --git a/packages/platform-apple/src/core/app-settings.ts b/packages/platform-apple/src/core/app-settings.ts index 7ff4666b41..54e7d46f9f 100644 --- a/packages/platform-apple/src/core/app-settings.ts +++ b/packages/platform-apple/src/core/app-settings.ts @@ -272,7 +272,11 @@ async function runIosPrivacyCommand( appBundleId: string, ): Promise { const supportedServices = await getSimctlPrivacyServices(device); - if (!supportedServices.has(target)) { + // reset notifications falls back to `reset all` below (direct reset fails + // with "operation not permitted" on runtimes whose help omits the service), + // so it passes the probe gate even when the service is unlisted. Grant/deny + // for notifications stay loud rejections. + if (!supportedServices.has(target) && !(action === 'reset' && target === 'notifications')) { throw new AppError( 'UNSUPPORTED_OPERATION', `iOS simctl privacy does not support service "${target}" on this runtime.`, @@ -285,29 +289,40 @@ async function runIosPrivacyCommand( } const args = ['privacy', device.id, action, target, appBundleId]; - const isNotificationsTarget = target === 'notifications'; - if (!(action === 'reset' && isNotificationsTarget)) { - try { - await runSimctl(device, args); - return; - } catch (error) { - if (!(isNotificationsTarget && isNotificationsOperationNotPermitted(error))) { - throw error; - } - throw new AppError( - 'UNSUPPORTED_OPERATION', - 'iOS simulator does not support setting notifications permission via simctl privacy on this runtime.', - { - deviceId: device.id, - appBundleId, - hint: 'Use reset notifications for reprompt behavior, or toggle notifications manually in Settings.', - }, - ); + if (action === 'reset' && target === 'notifications') { + await resetIosNotificationsPermission(device, appBundleId); + return; + } + try { + await runSimctl(device, args); + return; + } catch (error) { + if (!(target === 'notifications' && isNotificationsOperationNotPermitted(error))) { + throw error; } + throw new AppError( + 'UNSUPPORTED_OPERATION', + 'iOS simulator does not support setting notifications permission via simctl privacy on this runtime.', + { + deviceId: device.id, + appBundleId, + hint: 'Use reset notifications for reprompt behavior, or toggle notifications manually in Settings.', + }, + ); } +} +/** + * Direct `reset notifications` fails with "operation not permitted" on + * runtimes whose help omits the service, while `reset all` succeeds — so + * reset goes through the fallback instead of failing loudly like grant/deny. + */ +async function resetIosNotificationsPermission( + device: DeviceInfo, + appBundleId: string, +): Promise { try { - await runSimctl(device, args); + await runSimctl(device, ['privacy', device.id, 'reset', 'notifications', appBundleId]); return; } catch (error) { if (!isNotificationsOperationNotPermitted(error)) { diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index d5082308f8..14bb0800d6 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -70,7 +70,7 @@ agent-device test ./maestro-flows --maestro --platform android --artifacts-dir . Supported subset: -- Flows: `launchApp` (with `clearState`, `permissions`, and Apple-only launch arguments); `setPermissions` (mid-flow permission grants/denials/resets, expanded per platform); `runFlow` file/inline with platform, visibility, and limited boolean conditions; `onFlowStart`/`onFlowComplete`; `repeat.times` and retry. +- Flows: `launchApp` (with `clearState`, `permissions`, and Apple-only launch arguments; `permissions` apply after state clearing but before launch, and a `launchApp` without `permissions` touches nothing — there is no silent `all: allow` default); `setPermissions` (mid-flow permission grants/denials/resets; `all` expands to the backend-servable set — Android: camera/contacts/microphone/notifications/photos, iOS: the `simctl privacy help` subset excluding `camera`/`notifications`; anything else fails loudly instead of being skipped; `unset` fully resets and `location: never` denies); `runFlow` file/inline with platform, visibility, and limited boolean conditions; `onFlowStart`/`onFlowComplete`; `repeat.times` and retry. - Interactions: `tapOn`, `doubleTapOn`, `longPressOn`, `inputText` on the focused element, `eraseText`, `openLink`, `hideKeyboard`, basic `pressKey`, and `back`; selector targets poll until available and support recursive `index`, `childOf`, `above`, `below`, `leftOf`, `rightOf`, `containsChild`, `containsDescendants`, points, and `optional`; outer command labels are metadata, not target selectors. - Assertions and navigation: `assertVisible`, `assertNotVisible`, `assertTrue` (literal values and `${VAR}` lookups only; `""`, `"false"`, `"0"`, `"null"`, and `"undefined"` are falsy, everything else is truthy), `extendedWaitUntil`, `scroll`, `scrollUntilVisible`, absolute/percentage/target `swipe`, `takeScreenshot`, `waitForAnimationToEnd`, and `stopApp`. - Scripts: ordered `runScript` file/env scripts with `http.post`, `json`, and `output` variables. From cf1f91da491872cf742ddf2f37c160ff2fcbe696 Mon Sep 17 00:00:00 2001 From: Rohit <40559587+Rohit3523@users.noreply.github.com> Date: Mon, 7 Sep 2026 00:42:05 +0530 Subject: [PATCH 4/6] fix(maestro): resolve all in the backends, harden fan-out, deduplicate layers - settings permission all is now a backend target: iOS runs one simctl privacy call, Android intersects the package's declared permissions from dumpsys before mutating, skipping non-changeable ids with reasons instead of stopping partway. The adapter no longer keeps a fixed expansion list. - Android serves the full upstream name table (bluetooth, calendar, location, media-library, phone, sms, storage) through pm. - Fan-out failures report applied and failed mutations; launchApp collapses to one invoke; permission values share one maestro constant; duplicate-key and empty-map checks consolidated; TAP/ASSERT kind lists unified; settings app precedence noted. --- packages/contracts/src/client-settings.ts | 1 + packages/contracts/src/settings.ts | 5 +- packages/maestro/src/index.ts | 2 + .../__tests__/program-ir-parser.test.ts | 14 +- .../src/internal/conformance-normalize.ts | 27 +- .../src/internal/program-ir-command-parser.ts | 29 +- .../maestro/src/internal/program-ir-values.ts | 16 + .../src/internal/runtime-port-commands.ts | 13 +- .../maestro/src/internal/support-matrix.ts | 2 +- .../__tests__/permission-grant-state.test.ts | 46 ++- .../src/__tests__/settings-permission.test.ts | 135 ++++++- .../src/permission-grant-state.ts | 63 +++- .../src/settings-permission.ts | 339 ++++++++++++++++-- .../src/core/__tests__/app-settings.test.ts | 20 ++ .../platform-apple/src/core/app-settings.ts | 3 +- src/commands/capture/settings.ts | 1 + ...aemon-runtime-port-set-permissions.test.ts | 75 +++- .../__tests__/set-permissions-mapping.test.ts | 62 ++-- .../adapters/maestro/daemon-runtime-port.ts | 71 ++-- .../maestro/set-permissions-mapping.ts | 86 ++--- src/daemon/handlers/snapshot-settings.ts | 2 + website/docs/docs/replay-e2e.md | 2 +- 22 files changed, 794 insertions(+), 220 deletions(-) diff --git a/packages/contracts/src/client-settings.ts b/packages/contracts/src/client-settings.ts index 5e182d1bb8..6d5c6293c5 100644 --- a/packages/contracts/src/client-settings.ts +++ b/packages/contracts/src/client-settings.ts @@ -3,6 +3,7 @@ import type { DeviceCommandBaseOptions } from './client-connection.ts'; export type PermissionTarget = + | 'all' | 'camera' | 'microphone' | 'photos' diff --git a/packages/contracts/src/settings.ts b/packages/contracts/src/settings.ts index 6f67738e23..69aac2269b 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -2,6 +2,7 @@ import { AppError } from '@agent-device/kernel/errors'; export type PermissionAction = 'grant' | 'deny' | 'reset'; export type PermissionTarget = + | 'all' | 'camera' | 'microphone' | 'photos' @@ -22,6 +23,7 @@ export type SettingOptions = { longitude?: number; }; const PERMISSION_TARGETS: readonly PermissionTarget[] = [ + 'all', 'camera', 'microphone', 'photos', @@ -47,7 +49,7 @@ const SETTINGS_FINGERPRINT_USAGE = 'fingerprint '; const SETTINGS_CLEAR_APP_STATE_USAGE = 'clear-app-state [app-id]'; const SETTINGS_RESET_KEYCHAIN_USAGE = 'reset-keychain clear'; const SETTINGS_PERMISSION_USAGE = - 'permission [full|limited]'; + 'permission [full|limited]'; const SETTINGS_MACOS_PERMISSION_USAGE = 'permission '; const SETTINGS_MACOS_SUPPORTED_MESSAGE = `macOS supports only settings ${SETTINGS_APPEARANCE_USAGE} and settings ${SETTINGS_MACOS_PERMISSION_USAGE}. wifi|airplane|location|animations remain unsupported on macOS.`; @@ -88,6 +90,7 @@ export function parsePermissionAction(action: string): PermissionAction { export function parsePermissionTarget(value: string | undefined): PermissionTarget { const normalized = value?.trim().toLowerCase(); if ( + normalized === 'all' || normalized === 'camera' || normalized === 'microphone' || normalized === 'photos' || diff --git a/packages/maestro/src/index.ts b/packages/maestro/src/index.ts index cddc432a77..a598d0972c 100644 --- a/packages/maestro/src/index.ts +++ b/packages/maestro/src/index.ts @@ -35,6 +35,8 @@ export { MAESTRO_COMPAT_SUPPORTED_CAPABILITIES, } from './internal/facade-support.ts'; +export { MAESTRO_PERMISSION_VALUES } from './internal/program-ir-values.ts'; + export { createMaestroRuntimePort, literalFromMaestroRegex, diff --git a/packages/maestro/src/internal/__tests__/program-ir-parser.test.ts b/packages/maestro/src/internal/__tests__/program-ir-parser.test.ts index 1c8d04e777..d96fc1f4cb 100644 --- a/packages/maestro/src/internal/__tests__/program-ir-parser.test.ts +++ b/packages/maestro/src/internal/__tests__/program-ir-parser.test.ts @@ -438,6 +438,18 @@ describe('parseMaestroProgram', () => { optional: true, label: 'Prepare scan', }); + // Prototype names are not duplicates: the YAML layer already rejects real + // duplicate keys, so parsing accepts them and the backend verdict applies. + const prototype = parseMaestroProgram(`--- +- setPermissions: + permissions: + constructor: allow +`); + assert.deepEqual(prototype.commands[0], { + kind: 'setPermissions', + source: { line: 2 }, + permissions: { constructor: 'allow' }, + }); assert.throws( () => parseMaestroProgram(`--- @@ -488,7 +500,7 @@ describe('parseMaestroProgram', () => { - launchApp: permissions: {} `), - /launchApp\.permissions requires at least one permission.*line 2/i, + /launchApp\.permissions requires at least one permission.*line 3/i, ); assert.throws( () => diff --git a/packages/maestro/src/internal/conformance-normalize.ts b/packages/maestro/src/internal/conformance-normalize.ts index 3a524e544a..56b90c074a 100644 --- a/packages/maestro/src/internal/conformance-normalize.ts +++ b/packages/maestro/src/internal/conformance-normalize.ts @@ -344,12 +344,12 @@ function isBareAgentCommand(command: MaestroCommand): command is BareAgentComman return command.kind in BARE_AGENT_CANONICAL; } -type AgentTapCommand = Extract; +type AgentTapCommand = Extract; + +const AGENT_TAP_KINDS = ['tapOn', 'doubleTapOn', 'longPressOn'] as const; function isAgentTapCommand(command: MaestroCommand): command is AgentTapCommand { - return ( - command.kind === 'tapOn' || command.kind === 'doubleTapOn' || command.kind === 'longPressOn' - ); + return (AGENT_TAP_KINDS as readonly string[]).includes(command.kind); } function canonicalizeAgentTapCommand(command: AgentTapCommand): CanonicalCommand { @@ -386,18 +386,17 @@ function canonicalizeAgentTapCommand(command: AgentTapCommand): CanonicalCommand } } -type AgentAssertCommand = Extract< - MaestroCommand, - { kind: 'assertVisible' | 'assertNotVisible' | 'assertTrue' | 'extendedWaitUntil' } ->; +type AgentAssertCommand = Extract; + +const AGENT_ASSERT_KINDS = [ + 'assertVisible', + 'assertNotVisible', + 'assertTrue', + 'extendedWaitUntil', +] as const; function isAgentAssertCommand(command: MaestroCommand): command is AgentAssertCommand { - return ( - command.kind === 'assertVisible' || - command.kind === 'assertNotVisible' || - command.kind === 'assertTrue' || - command.kind === 'extendedWaitUntil' - ); + return (AGENT_ASSERT_KINDS as readonly string[]).includes(command.kind); } function canonicalizeAgentAssertCommand(command: AgentAssertCommand): CanonicalCommand { diff --git a/packages/maestro/src/internal/program-ir-command-parser.ts b/packages/maestro/src/internal/program-ir-command-parser.ts index 966d53eed0..735b734763 100644 --- a/packages/maestro/src/internal/program-ir-command-parser.ts +++ b/packages/maestro/src/internal/program-ir-command-parser.ts @@ -59,6 +59,7 @@ import { readSequenceItems, sourceAt, type MaestroProgramParseContext, + MAESTRO_PERMISSION_VALUES, VARIABLE_PATTERN, } from './program-ir-values.ts'; @@ -183,12 +184,6 @@ function parseLaunchApp( const permissions = readOptionalEntry(entries, 'permissions', (entry) => readSetPermissionsMap(entry, context, 'launchApp'), ); - if (permissions && Object.keys(permissions).length === 0) - invalidAt( - 'Maestro launchApp.permissions requires at least one permission.', - commandNode, - context, - ); const args = readOptionalEntry(entries, 'arguments', (entry) => parseLaunchArguments(entry, 'launchApp.arguments', context), ); @@ -460,16 +455,6 @@ function parseStopApp( return { kind: 'stopApp', source, appId: readRequiredString(value, 'stopApp', context) }; } -const MAESTRO_PERMISSION_VALUES = new Set([ - 'allow', - 'deny', - 'unset', - 'always', - 'inuse', - 'never', - 'limited', -]); - function parseSetPermissions( value: Node | null, commandNode: Node, @@ -484,8 +469,6 @@ function parseSetPermissions( readOptionalString(entry, 'setPermissions.appId', context), ); const permissions = readSetPermissionsMap(entryValue(entries, 'permissions'), context); - if (Object.keys(permissions).length === 0) - invalidAt('Maestro setPermissions requires at least one permission.', commandNode, context); const options = readOptionalCommandOption(entries, 'setPermissions', context); const label = readMaestroCommandLabel(entries, 'setPermissions', context); return stripUndefined({ @@ -504,14 +487,12 @@ function readSetPermissionsMap( owner = 'setPermissions', ): Record { const entries = readMapEntries(node, `${owner}.permissions`, context); + if (entries.length === 0) + invalidAt(`Maestro ${owner}.permissions requires at least one permission.`, node, context); const permissions: Record = {}; for (const entry of entries) { - if (entry.key in permissions) - invalidAt( - `Maestro ${owner}.permissions contains duplicate permission "${entry.key}".`, - entry.keyNode, - context, - ); + // No duplicate-key check: the YAML layer already rejects duplicate mapping + // keys, and `in`-style checks misfire on prototype names like `constructor`. permissions[entry.key] = readPermissionValue(entry, context, owner); } return permissions; diff --git a/packages/maestro/src/internal/program-ir-values.ts b/packages/maestro/src/internal/program-ir-values.ts index 789056b431..0b71c13ebb 100644 --- a/packages/maestro/src/internal/program-ir-values.ts +++ b/packages/maestro/src/internal/program-ir-values.ts @@ -200,6 +200,22 @@ export function readOptionalBoolean( } export const VARIABLE_PATTERN = /^\$\{[A-Za-z_][A-Za-z0-9_.]*\}$/; + +/** + * The `setPermissions`/`launchApp.permissions` value vocabulary, shared by the + * parser, the runtime port, and the daemon adapter: the plain states plus the + * iOS granular `location`/`photos` values. Per-permission validity (which + * granular value belongs where) is enforced by the execution layers. + */ +export const MAESTRO_PERMISSION_VALUES: ReadonlySet = new Set([ + 'allow', + 'deny', + 'unset', + 'always', + 'inuse', + 'never', + 'limited', +]); const NUMERIC_STRING_PATTERN = /^-?\d+(\.\d+)?$/; const INTEGER_STRING_PATTERN = /^-?\d+$/; diff --git a/packages/maestro/src/internal/runtime-port-commands.ts b/packages/maestro/src/internal/runtime-port-commands.ts index 6f3556aeb4..34b85acfa5 100644 --- a/packages/maestro/src/internal/runtime-port-commands.ts +++ b/packages/maestro/src/internal/runtime-port-commands.ts @@ -1,5 +1,6 @@ import { AppError } from '@agent-device/kernel/errors'; import { pointInsideRect, stripUndefined } from './shared.ts'; +import { MAESTRO_PERMISSION_VALUES } from './program-ir-values.ts'; import { maestroScrollDurationFromSpeed, MAESTRO_COMPATIBILITY_PRESETS, @@ -177,21 +178,11 @@ function launchAppInput(command: MaestroCommandOf<'launchApp'>, request: Maestro }); } -const RESOLVED_PERMISSION_VALUES = new Set([ - 'allow', - 'deny', - 'unset', - 'always', - 'inuse', - 'never', - 'limited', -]); - function resolveSetPermissions(permissions: Readonly>) { const resolved: Record = {}; for (const [name, value] of Object.entries(permissions)) { const normalized = value.toLowerCase(); - if (!RESOLVED_PERMISSION_VALUES.has(normalized)) { + if (!MAESTRO_PERMISSION_VALUES.has(normalized)) { throw new AppError( 'INVALID_ARGS', `Maestro setPermissions.permissions.${name} expects allow|deny|unset (plus always|inuse|never|limited for location/photos); received "${value}".`, diff --git a/packages/maestro/src/internal/support-matrix.ts b/packages/maestro/src/internal/support-matrix.ts index e316f0ee48..49f013f6da 100644 --- a/packages/maestro/src/internal/support-matrix.ts +++ b/packages/maestro/src/internal/support-matrix.ts @@ -1,5 +1,5 @@ export const MAESTRO_COMPAT_SUPPORTED_CAPABILITIES = [ - 'Flows: launchApp (with clearState, permissions, and Apple-only launch arguments; permissions apply after state clearing but before launch, and a launchApp without permissions touches nothing — there is no silent all: allow default); setPermissions (mid-flow permission grants/denials/resets; all expands to the backend-servable set — Android: camera/contacts/microphone/notifications/photos, iOS: the simctl privacy help subset excluding camera/notifications; anything else fails loudly instead of being skipped; unset fully resets and location: never denies); runFlow file/inline with platform, visibility, and limited boolean conditions; onFlowStart/onFlowComplete; repeat.times and retry.', + 'Flows: launchApp (with clearState, permissions, and Apple-only launch arguments; permissions apply after state clearing but before launch, and a launchApp without permissions touches nothing — there is no silent all: allow default); setPermissions (mid-flow permission grants/denials/resets; all resolves in the backend — one simctl call on iOS, the declared permissions on Android — with specifics overriding after it; unservable names fail loudly instead of being skipped; unset fully resets and location: never denies); runFlow file/inline with platform, visibility, and limited boolean conditions; onFlowStart/onFlowComplete; repeat.times and retry.', 'Interactions: tapOn, doubleTapOn, longPressOn, inputText on the focused element, eraseText, openLink, hideKeyboard, basic pressKey, and back; selector targets poll until available and support recursive index, childOf, above, below, leftOf, rightOf, containsChild, containsDescendants, points, and optional; outer command labels are metadata, not target selectors.', 'Assertions and navigation: assertVisible, assertNotVisible, assertTrue (literal values and ${VAR} lookups only; "", "false", "0", "null", and "undefined" are falsy, everything else is truthy), extendedWaitUntil, scroll, scrollUntilVisible, absolute/percentage/target swipe, takeScreenshot, waitForAnimationToEnd, and stopApp.', 'Scripts: ordered runScript file/env scripts with http.post, json, and output variables.', diff --git a/packages/platform-android/src/__tests__/permission-grant-state.test.ts b/packages/platform-android/src/__tests__/permission-grant-state.test.ts index 41fcb9c1cf..f7f62dcbf2 100644 --- a/packages/platform-android/src/__tests__/permission-grant-state.test.ts +++ b/packages/platform-android/src/__tests__/permission-grant-state.test.ts @@ -1,6 +1,9 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; -import { parseAndroidRuntimePermissionGrants } from '../permission-grant-state.ts'; +import { + parseAndroidRequestedPermissions, + parseAndroidRuntimePermissionGrants, +} from '../permission-grant-state.ts'; // Captured from `adb shell dumpsys package com.callstack.agentdevicelab` on a Pixel 7 / API 36 // emulator, trimmed to the sections that decide the answer. The indentation is load-bearing: @@ -91,3 +94,44 @@ test('sections after Packages: cannot reopen the scan', () => { assert.equal(grants?.get('android.permission.RECORD_AUDIO'), 'not_granted'); assert.equal(grants?.get('android.permission.CAMERA'), 'granted'); }); + +// Requested ids live in their own section: bare names, no grant flags, ending where the +// install section begins. Later top-level sections cannot contribute ids either. +const REQUESTED_DUMP = [ + 'Packages:', + ' Package [com.example.app] (5f3a1c2):', + ' requested permissions:', + ' android.permission.INTERNET', + ' android.permission.RECORD_AUDIO: restricted=false', + ' com.example.app.CUSTOM_PERMISSION', + ' install permissions:', + ' android.permission.INTERNET: granted=true', + ' User 0: ceDataInode=0 installed=true', + 'Queries:', + ' com.example.app.OTHER: granted=true', +].join('\n'); + +test('requested permissions read bare ids up to the install section', () => { + assert.deepEqual(parseAndroidRequestedPermissions(REQUESTED_DUMP), [ + 'android.permission.INTERNET', + 'android.permission.RECORD_AUDIO', + 'com.example.app.CUSTOM_PERMISSION', + ]); +}); + +test.each([ + ['empty output', ''], + ['no Packages section', 'Activity Resolver Table:'], + ['a package without the section', 'Packages:\n Package [com.example.app] (abc):'], +] as const)('requested permissions reads %s as unknown', (_label, output) => { + assert.equal(parseAndroidRequestedPermissions(output), undefined); +}); + +test('an empty requested block declares nothing', () => { + assert.deepEqual( + parseAndroidRequestedPermissions( + 'Packages:\n Package [com.example.app] (abc):\n requested permissions:\n User 0: installed=true', + ), + [], + ); +}); diff --git a/packages/platform-android/src/__tests__/settings-permission.test.ts b/packages/platform-android/src/__tests__/settings-permission.test.ts index cce31f7711..610b625680 100644 --- a/packages/platform-android/src/__tests__/settings-permission.test.ts +++ b/packages/platform-android/src/__tests__/settings-permission.test.ts @@ -302,7 +302,7 @@ test.each([ ], [ 'an iOS-only target', - { permissionTarget: 'calendar' }, + { permissionTarget: 'location-always' }, /Unsupported permission target on Android/i, ], ] as const)('setAndroidSetting permission rejects %s', async (_label, options, message) => { @@ -321,3 +321,136 @@ test('setAndroidSetting permission requires an app in session', async () => { { code: 'INVALID_ARGS', message: /requires an active app in session/ }, ); }); + +// Explicit multi-id names fan out to one pm call per id, in table order. +test('setAndroidSetting permission grant contacts grants both contact ids', async () => { + await withFakeAdb( + fakeAdb((flat) => (flat === CURRENT_USER ? '0' : undefined)), + async ({ calls, device }) => { + await setAndroidSetting(device, 'permission', 'grant', 'com.example.app', { + permissionTarget: 'contacts', + }); + const flat = calls.map((args) => args.join(' ')); + assert.ok( + flat.includes('shell pm grant --user 0 com.example.app android.permission.READ_CONTACTS'), + flat.join('; '), + ); + assert.ok( + flat.includes('shell pm grant --user 0 com.example.app android.permission.WRITE_CONTACTS'), + flat.join('; '), + ); + }, + ); +}); + +/** A dump shaped like the lab app's: install, custom, and runtime permissions side by side. */ +function dumpsysWithRequested(): string { + return [ + 'Packages:', + ' Package [com.example.app] (abc):', + ' requested permissions:', + ' android.permission.INTERNET', + ' android.permission.RECORD_AUDIO', + ' com.example.app.CUSTOM_PERMISSION', + ' install permissions:', + ' android.permission.INTERNET: granted=true', + ' User 0: ceDataInode=0 installed=true', + ' runtime permissions:', + ' android.permission.RECORD_AUDIO: granted=true, flags=[ USER_SET]', + 'Queries:', + ].join('\n'); +} + +// `all` intersects the declared set before issuing anything: INTERNET is declared +// but not changeable, so it is skipped with a reason while RECORD_AUDIO lands. +test('setAndroidSetting permission grant all applies the declared changeable ids', async () => { + await withFakeAdb( + fakeAdb((flat) => { + if (flat === CURRENT_USER) return '0'; + if (flat === DUMPSYS) return dumpsysWithRequested(); + if (flat === 'shell pm grant --user 0 com.example.app android.permission.INTERNET') { + return { + stderr: + "Exception occurred while executing 'grant':\njava.lang.SecurityException: INTERNET is not a changeable permission type", + exitCode: 1, + }; + } + if (flat === 'shell pm grant --user 0 com.example.app com.example.app.CUSTOM_PERMISSION') { + return { + stderr: + 'SecurityException: Package com.example.app has not requested permission com.example.app.CUSTOM_PERMISSION', + exitCode: 1, + }; + } + return undefined; + }), + async ({ calls, device }) => { + const result = await setAndroidSetting(device, 'permission', 'grant', 'com.example.app', { + permissionTarget: 'all', + }); + const flat = calls.map((args) => args.join(' ')); + assert.ok( + flat.includes('shell pm grant --user 0 com.example.app android.permission.RECORD_AUDIO'), + flat.join('; '), + ); + assert.deepEqual(result, { + permission: 'all', + applied: ['android.permission.RECORD_AUDIO'], + warnings: [ + "Skipped android.permission.INTERNET for com.example.app: Exception occurred while executing 'grant': java.lang.SecurityException: INTERNET is not a changeable permission type", + 'Skipped com.example.app.CUSTOM_PERMISSION for com.example.app: SecurityException: Package com.example.app has not requested permission com.example.app.CUSTOM_PERMISSION', + ], + }); + }, + ); +}); + +// Revoke under `all` warns per held permission, like the single path. +test('setAndroidSetting permission revoke all warns for the held runtime id', async () => { + await withFakeAdb( + fakeAdb((flat) => { + if (flat === CURRENT_USER) return '0'; + if (flat === DUMPSYS) return dumpsysWithRequested(); + return undefined; + }), + async ({ device }) => { + const result = (await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { + permissionTarget: 'all', + })) as Record; + assert.deepEqual(result.permission, 'all'); + assert.ok( + (result.applied as string[]).includes('android.permission.RECORD_AUDIO'), + JSON.stringify(result), + ); + const warnings = (result.warnings as string[]).join('\n'); + assert.match(warnings, /RECORD_AUDIO was granted before this revoke/); + }, + ); +}); + +// Validation happens before mutation: an unreadable dump issues no pm call. +test.each([ + ['dumpsys fails', { stderr: 'error', exitCode: 1 }], + ['no requested section', dumpsys([{ id: 0, runtime: [[MICROPHONE, false]] }])], +] as const)('setAndroidSetting permission all refuses when %s', async (_label, reply) => { + await withFakeAdb( + fakeAdb((flat) => { + if (flat === CURRENT_USER) return '0'; + if (flat === DUMPSYS) return reply as string; + return { stderr: `unexpected args: ${flat}`, exitCode: 1 }; + }), + async ({ calls, device }) => { + await assertRejectsAppError( + () => + setAndroidSetting(device, 'permission', 'grant', 'com.example.app', { + permissionTarget: 'all', + }), + { code: 'COMMAND_FAILED', message: /declared permissions|requested permissions/i }, + ); + assert.ok( + calls.every((args) => !args.includes('pm')), + calls.map((args) => args.join(' ')).join('; '), + ); + }, + ); +}); diff --git a/packages/platform-android/src/permission-grant-state.ts b/packages/platform-android/src/permission-grant-state.ts index 5e0bfd18c9..d4ce607615 100644 --- a/packages/platform-android/src/permission-grant-state.ts +++ b/packages/platform-android/src/permission-grant-state.ts @@ -62,10 +62,21 @@ export async function readAndroidCurrentUserId(device: DeviceInfo): Promise text.trim().length > 0) + .map((text) => ({ text, indent: text.length - text.trimStart().length })); +} + /** * Runtime permission grants for `userId` only, or `undefined` when that user has no * runtime-permission block in the dump. @@ -80,10 +91,7 @@ export function parseAndroidRuntimePermissionGrants( dumpsysOutput: string, userId: number, ): AndroidRuntimePermissionGrants | undefined { - const lines = dumpsysOutput - .split('\n') - .filter((text) => text.trim().length > 0) - .map((text) => ({ text, indent: text.length - text.trimStart().length })); + const lines = dumpLines(dumpsysOutput); const packages = nestedBlock( lines, (line) => line.indent === 0 && line.text.trim() === 'Packages:', @@ -117,3 +125,50 @@ function nestedBlock( const end = rest.findIndex((line) => line.indent <= lines[start]!.indent); return end < 0 ? rest : rest.slice(0, end); } + +/** + * Both halves of one `dumpsys package` read: the declared ids and the acting + * user's runtime grants. Each half keeps its own absent-vs-empty semantics — + * see the two parsers — so callers can refuse on a missing section while + * still answering `unknown` for missing grants. + */ +export function parseAndroidPackagePermissions( + dumpsysOutput: string, + userId: number, +): { + requested: string[] | undefined; + grants: AndroidRuntimePermissionGrants | undefined; +} { + return { + requested: parseAndroidRequestedPermissions(dumpsysOutput), + grants: parseAndroidRuntimePermissionGrants(dumpsysOutput, userId), + }; +} + +/** + * The permission ids the package declares, in dump order, or `undefined` when + * the dump carries no `requested permissions:` block for a package. An empty + * block is still an answer — the app declares nothing — while a missing one + * means the device did not tell us, and `all` must refuse rather than guess. + * + * Entries are bare ids (`android.permission.CAMERA`); any trailing attribute + * (`: restricted=false`) is not part of the id. Section scoping reuses the + * same `Packages:` → `Package […]` nesting as the grants read, so the later + * top-level sections cannot leak ids in. + */ +export function parseAndroidRequestedPermissions(dumpsysOutput: string): string[] | undefined { + const lines = dumpLines(dumpsysOutput); + const packages = nestedBlock( + lines, + (line) => line.indent === 0 && line.text.trim() === 'Packages:', + ); + const pkg = nestedBlock(packages, (line) => PACKAGE_BLOCK.test(line.text)); + const requested = nestedBlock(pkg, (line) => REQUESTED_PERMISSIONS_BLOCK.test(line.text)); + if (!requested) return undefined; + const ids: string[] = []; + for (const { text } of requested) { + const id = PERMISSION_ID.exec(text)?.[1]; + if (id && id.includes('.') && !ids.includes(id)) ids.push(id); + } + return ids; +} diff --git a/packages/platform-android/src/settings-permission.ts b/packages/platform-android/src/settings-permission.ts index 3c174d1671..ebe7bfd8a4 100644 --- a/packages/platform-android/src/settings-permission.ts +++ b/packages/platform-android/src/settings-permission.ts @@ -4,9 +4,11 @@ import { parsePermissionAction, parsePermissionTarget } from '@agent-device/cont import type { SettingOptions } from '@agent-device/contracts/settings'; import { runAndroidAdb } from './adb.ts'; import { + parseAndroidPackagePermissions, readAndroidCurrentUserId, readAndroidRuntimePermissionGrants, type AndroidPriorGrantState, + type AndroidRuntimePermissionGrants, } from './permission-grant-state.ts'; /** @@ -35,6 +37,43 @@ export function androidRevokedPermissionWarning( type AndroidPermissionTarget = ReturnType; +/** + * Canonical Maestro/Android names to the `pm` permission ids they fan out to. + * Mirrors upstream Maestro's `translatePermissionName`; every id is applied + * with the same `pm grant|revoke` mechanism, so the table needs no per-entry + * device verification — only the mechanism does, and it is covered on both + * paths below. `photos` (SDK-dependent probing) and `notifications` (appops) + * keep their dedicated kinds; `all` resolves against the package instead. + */ +const ANDROID_PERMISSION_TABLE: Readonly> = { + bluetooth: ['android.permission.BLUETOOTH_CONNECT', 'android.permission.BLUETOOTH_SCAN'], + calendar: ['android.permission.WRITE_CALENDAR', 'android.permission.READ_CALENDAR'], + camera: ['android.permission.CAMERA'], + contacts: ['android.permission.READ_CONTACTS', 'android.permission.WRITE_CONTACTS'], + location: [ + 'android.permission.ACCESS_FINE_LOCATION', + 'android.permission.ACCESS_COARSE_LOCATION', + ], + 'media-library': [ + 'android.permission.WRITE_EXTERNAL_STORAGE', + 'android.permission.READ_EXTERNAL_STORAGE', + 'android.permission.READ_MEDIA_AUDIO', + 'android.permission.READ_MEDIA_IMAGES', + 'android.permission.READ_MEDIA_VIDEO', + ], + microphone: ['android.permission.RECORD_AUDIO'], + phone: ['android.permission.CALL_PHONE', 'android.permission.ANSWER_PHONE_CALLS'], + sms: [ + 'android.permission.READ_SMS', + 'android.permission.RECEIVE_SMS', + 'android.permission.SEND_SMS', + ], + storage: [ + 'android.permission.WRITE_EXTERNAL_STORAGE', + 'android.permission.READ_EXTERNAL_STORAGE', + ], +}; + /** * `--user ` for every permission mutation, resolved once so the state read and the mutation * cannot address different users. Never empty: a permission mutation that cannot name its user @@ -74,6 +113,9 @@ export async function setAndroidPermission( const target = parseAndroidPermissionTarget(options?.permissionTarget, options?.permissionMode); const userId = await requireAndroidPermissionUser(device); const userArgs: AndroidUserArgs = ['--user', String(userId)]; + if (target.kind === 'all') { + return await setAllAndroidPermissions(device, appPackage, action, userId, userArgs); + } if (action === 'grant') { await grantAndroidPermission(device, appPackage, target, userArgs); return; @@ -81,16 +123,224 @@ export async function setAndroidPermission( // Read before the revoke — afterwards every permission reads as not granted — but resolved // after it, because `photos` only learns which permission it revoked by probing the device. const grants = await readAndroidRuntimePermissionGrants(device, appPackage, userId); - const permission = await revokeAndroidPermission(device, appPackage, action, target, userArgs); - const priorGrantState: AndroidPriorGrantState = grants?.get(permission) ?? 'unknown'; - const warning = androidRevokedPermissionWarning(appPackage, permission, priorGrantState); + const revoked = await revokeAndroidPermission(device, appPackage, action, target, userArgs); + const states = revoked.map((permission) => grants?.get(permission) ?? 'unknown'); + const priorGrantState: AndroidPriorGrantState = states.includes('granted') + ? 'granted' + : states.includes('unknown') + ? 'unknown' + : 'not_granted'; + const warnings = revoked.flatMap((permission, index) => { + const warning = androidRevokedPermissionWarning(appPackage, permission, states[index]!); + return warning ? [warning] : []; + }); return { - permission, + permission: revoked.join(','), priorGrantState, - ...(warning ? { warnings: [warning] } : {}), + ...(warnings.length > 0 ? { warnings } : {}), }; } +/** + * `all`: every permission the package declares, resolved from one `dumpsys + * package` read before anything is mutated. Declared-but-not-changeable ids + * (install permissions like INTERNET, special ids like MANAGE_EXTERNAL_STORAGE, + * custom ids the runtime rejects) are skipped with a reason instead of + * stopping the sequence — while an explicit target for the same id still + * fails loudly. Anything the dump does not list is never attempted, which is + * what keeps `pm` from throwing "has not requested permission" partway. + */ +async function setAllAndroidPermissions( + device: DeviceInfo, + appPackage: string, + action: 'grant' | 'deny' | 'reset', + userId: number, + userArgs: AndroidUserArgs, +): Promise> { + const dump = await runAndroidAdb(device, ['shell', 'dumpsys', 'package', appPackage], { + allowFailure: true, + }); + if (dump.exitCode !== 0) { + throw new AppError( + 'COMMAND_FAILED', + `Could not read declared permissions for ${appPackage}, so no permission was changed.`, + { appPackage, stdout: dump.stdout, stderr: dump.stderr, exitCode: dump.exitCode }, + ); + } + const { requested, grants: revokedGrants } = parseAndroidPackagePermissions(dump.stdout, userId); + if (requested === undefined) { + throw new AppError( + 'COMMAND_FAILED', + `Could not find declared permissions for ${appPackage}, so no permission was changed.`, + { appPackage }, + ); + } + const grants = action === 'grant' ? undefined : revokedGrants; + const applied: string[] = []; + const warnings: string[] = []; + for (const unit of allPermissionUnits(requested)) { + await applyAllPermissionUnit( + { device, appPackage, action, userArgs, grants, applied, warnings }, + unit, + ); + } + return { + permission: 'all', + applied, + ...(warnings.length > 0 ? { warnings } : {}), + }; +} + +type AllUnitContext = { + device: DeviceInfo; + appPackage: string; + action: 'grant' | 'deny' | 'reset'; + userArgs: AndroidUserArgs; + grants: AndroidRuntimePermissionGrants | undefined; + applied: string[]; + warnings: string[]; +}; + +/** One declared-permission unit: strict appops for notifications, best-effort pm otherwise. */ +async function applyAllPermissionUnit(ctx: AllUnitContext, unit: AllPermissionUnit): Promise { + if (unit.kind === 'notification') return await applyAllNotificationsUnit(ctx); + if (unit.kind === 'photos') return await applyAllPhotosUnit(ctx); + return await applyAllPmUnit(ctx, unit.value); +} + +async function applyAllNotificationsUnit(ctx: AllUnitContext): Promise { + const { device, appPackage, action, userArgs, grants, applied, warnings } = ctx; + await setAndroidNotificationPermission( + device, + appPackage, + action, + { appOps: 'POST_NOTIFICATION', permission: 'android.permission.POST_NOTIFICATIONS' }, + userArgs, + ); + applied.push('android.permission.POST_NOTIFICATIONS'); + warnIfRevoked(warnings, grants, appPackage, 'android.permission.POST_NOTIFICATIONS'); +} + +async function applyAllPhotosUnit(ctx: AllUnitContext): Promise { + const { device, appPackage, action, userArgs, warnings } = ctx; + const resolved = await tryPhotosUnit( + device, + appPackage, + action === 'grant' ? 'grant' : 'revoke', + userArgs, + ); + if (resolved === undefined) { + warnings.push( + `Skipped Android photos permission for ${appPackage}: device refused both media candidates.`, + ); + return; + } + await finishAllUnit(ctx, resolved); +} + +async function applyAllPmUnit(ctx: AllUnitContext, permission: string): Promise { + const { device, appPackage, action, userArgs, warnings } = ctx; + const attempt = await tryPmUnit( + device, + action === 'grant' ? 'grant' : 'revoke', + userArgs, + appPackage, + permission, + ); + if (!attempt.ok) { + warnings.push(`Skipped ${permission} for ${appPackage}: ${attempt.reason}`); + return; + } + await finishAllUnit(ctx, permission); +} + +/** Record a landed mutation: reset its flags when asked, then warn if it may have killed the app. */ +async function finishAllUnit(ctx: AllUnitContext, permission: string): Promise { + const { device, appPackage, action, userArgs, grants, applied, warnings } = ctx; + applied.push(permission); + if (action === 'reset') + await clearAndroidPermissionFlags(device, appPackage, permission, userArgs); + if (action !== 'grant') warnIfRevoked(warnings, grants, appPackage, permission); +} + +type AllPermissionUnit = + | { kind: 'photos' } + | { kind: 'notification' } + | { kind: 'pm'; value: string }; + +/** Collapse declared ids into mutation units: one photos probe, one appops path, direct pm otherwise. */ +function allPermissionUnits(requested: readonly string[]): AllPermissionUnit[] { + const units: AllPermissionUnit[] = []; + let photosQueued = false; + for (const id of requested) { + if (id === 'android.permission.POST_NOTIFICATIONS') units.push({ kind: 'notification' }); + else if ( + id === 'android.permission.READ_MEDIA_IMAGES' || + id === 'android.permission.READ_EXTERNAL_STORAGE' + ) { + if (!photosQueued) { + photosQueued = true; + units.push({ kind: 'photos' }); + } + } else units.push({ kind: 'pm', value: id }); + } + return units; +} + +function warnIfRevoked( + warnings: string[], + grants: AndroidRuntimePermissionGrants | undefined, + appPackage: string, + permission: string, +): void { + const warning = androidRevokedPermissionWarning( + appPackage, + permission, + grants?.get(permission) ?? 'unknown', + ); + if (warning) warnings.push(warning); +} + +async function tryPmUnit( + device: DeviceInfo, + pmAction: 'grant' | 'revoke', + userArgs: AndroidUserArgs, + appPackage: string, + permission: string, +): Promise<{ ok: true } | { ok: false; reason: string }> { + const result = await runAndroidAdb( + device, + ['shell', 'pm', pmAction, ...userArgs, appPackage, permission], + { allowFailure: true }, + ); + if (result.exitCode === 0) return { ok: true }; + return { ok: false, reason: firstStderrLine(result.stderr) }; +} + +async function tryPhotosUnit( + device: DeviceInfo, + appPackage: string, + pmAction: 'grant' | 'revoke', + userArgs: AndroidUserArgs, +): Promise { + try { + return await setAndroidPhotoPermission(device, appPackage, pmAction, userArgs); + } catch { + return undefined; + } +} + +function firstStderrLine(stderr: string): string { + const lines = stderr + .split('\n') + .map((part) => part.trim()) + .filter((part) => part.length > 0); + const first = lines[0] ?? 'unknown device error'; + // adb wraps the cause onto the next line ("Exception occurred ...:\njava.lang..."). + const reason = first.endsWith(':') && lines[1] ? `${first} ${lines[1]}` : first; + return reason.slice(0, 200); +} + async function grantAndroidPermission( device: DeviceInfo, appPackage: string, @@ -99,62 +349,82 @@ async function grantAndroidPermission( ): Promise { if (target.kind === 'notifications') { await setAndroidNotificationPermission(device, appPackage, 'grant', target, userArgs); - } else if (target.type === 'photos') { + } else if (target.kind === 'photos') { await setAndroidPhotoPermission(device, appPackage, 'grant', userArgs); + } else if (target.kind === 'pm') { + for (const value of target.values) { + await runAndroidAdb(device, ['shell', 'pm', 'grant', ...userArgs, appPackage, value]); + } + } else if (target.kind === 'all') { + throw new Error('Unhandled Android permission target: all is resolved by the caller.'); } else { - await runAndroidAdb(device, ['shell', 'pm', 'grant', ...userArgs, appPackage, target.value]); + const exhaustive: never = target; + throw new Error(`Unhandled Android permission target: ${JSON.stringify(exhaustive)}`); } } -/** Revokes (and for `reset`, clears the flags of) the target; returns the permission revoked. */ +/** Revokes (and for `reset`, clears the flags of) the target; returns the permissions revoked. */ async function revokeAndroidPermission( device: DeviceInfo, appPackage: string, action: 'deny' | 'reset', target: AndroidPermissionTarget, userArgs: AndroidUserArgs, -): Promise { +): Promise { if (target.kind === 'notifications') { await setAndroidNotificationPermission(device, appPackage, action, target, userArgs); - return target.permission; + return [target.permission]; } - let permission: string; - if (target.type === 'photos') { - permission = await setAndroidPhotoPermission(device, appPackage, 'revoke', userArgs); - } else { - permission = target.value; - await runAndroidAdb(device, ['shell', 'pm', 'revoke', ...userArgs, appPackage, permission]); + if (target.kind === 'photos') { + const resolved = await setAndroidPhotoPermission(device, appPackage, 'revoke', userArgs); + if (action === 'reset') { + await clearAndroidPermissionFlags(device, appPackage, resolved, userArgs); + } + return [resolved]; } - if (action === 'reset') { - await clearAndroidPermissionFlags(device, appPackage, permission, userArgs); + if (target.kind === 'pm') { + for (const value of target.values) { + await runAndroidAdb(device, ['shell', 'pm', 'revoke', ...userArgs, appPackage, value]); + } + if (action === 'reset') { + for (const value of target.values) { + await clearAndroidPermissionFlags(device, appPackage, value, userArgs); + } + } + return [...target.values]; + } + if (target.kind === 'all') { + throw new Error('Unhandled Android permission target: all is resolved by the caller.'); } - return permission; + const exhaustive: never = target; + throw new Error(`Unhandled Android permission target: ${JSON.stringify(exhaustive)}`); } function parseAndroidPermissionTarget( permissionTarget: string | undefined, permissionMode: string | undefined, ): - | { kind: 'pm'; value: string; type: 'camera' | 'microphone' | 'photos' | 'contacts' } - | { kind: 'notifications'; appOps: string; permission: string } { + | { kind: 'pm'; values: readonly string[] } + | { kind: 'photos' } + | { kind: 'notifications'; appOps: string; permission: string } + | { kind: 'all' } { const normalized = parsePermissionTarget(permissionTarget); + if (normalized === 'all') { + if (permissionMode?.trim()) { + throw new AppError( + 'INVALID_ARGS', + `Permission mode is only supported for photos. Received: ${permissionMode}.`, + ); + } + return { kind: 'all' }; + } if (permissionMode?.trim()) { throw new AppError( 'INVALID_ARGS', `Permission mode is only supported for photos. Received: ${permissionMode}.`, ); } - if (normalized === 'camera') - return { kind: 'pm', value: 'android.permission.CAMERA', type: 'camera' }; - if (normalized === 'microphone') { - return { kind: 'pm', value: 'android.permission.RECORD_AUDIO', type: 'microphone' }; - } - if (normalized === 'photos') { - return { kind: 'pm', value: 'android.permission.READ_MEDIA_IMAGES', type: 'photos' }; - } - if (normalized === 'contacts') { - return { kind: 'pm', value: 'android.permission.READ_CONTACTS', type: 'contacts' }; - } + if (normalized === 'photos') return { kind: 'photos' }; if (normalized === 'notifications') { return { kind: 'notifications', @@ -162,9 +432,12 @@ function parseAndroidPermissionTarget( permission: 'android.permission.POST_NOTIFICATIONS', }; } + const values = ANDROID_PERMISSION_TABLE[normalized]; + if (values) return { kind: 'pm', values }; throw new AppError( 'INVALID_ARGS', - `Unsupported permission target on Android: ${permissionTarget}. Use camera|microphone|photos|contacts|notifications.`, + `Unsupported permission target on Android: ${permissionTarget}. Use all|bluetooth|calendar|camera|contacts|location|media-library|microphone|notifications|phone|photos|sms|storage.`, + { hint: 'Android custom permission ids are attempted through all, not individually.' }, ); } diff --git a/packages/platform-apple/src/core/__tests__/app-settings.test.ts b/packages/platform-apple/src/core/__tests__/app-settings.test.ts index 5331b0995e..2c7c7f0dc6 100644 --- a/packages/platform-apple/src/core/__tests__/app-settings.test.ts +++ b/packages/platform-apple/src/core/__tests__/app-settings.test.ts @@ -320,6 +320,26 @@ test('setIosSetting permission grant calendar uses simctl privacy calendar targe ); }); +test('setIosSetting permission grant all passes all through as one simctl call', async () => { + await withFakeAppleTool( + (args) => { + if (isSimctlListDevices(args)) return BOOTED_SIM_LIST_JSON; + if (args.join(' ') === 'simctl privacy sim-1 grant all com.example.app') return ''; + return unexpectedArgs(args); + }, + async ({ calls }) => { + await setIosSetting(IOS_TEST_SIMULATOR, 'permission', 'grant', 'com.example.app', { + permissionTarget: 'all', + }); + const flat = calls.map((args) => args.join(' ')); + assert.deepEqual( + flat.filter((line) => line.includes('privacy sim-1')), + ['simctl privacy sim-1 grant all com.example.app'], + ); + }, + ); +}); + test('setIosSetting clear-app-state wipes iOS simulator app data container', async () => { const containerPath = await mkdtempForTest('agent-device-ios-clear-app-state-container-'); await fs.mkdir(path.join(containerPath, 'Documents'), { recursive: true }); diff --git a/packages/platform-apple/src/core/app-settings.ts b/packages/platform-apple/src/core/app-settings.ts index 54e7d46f9f..b69aec85f1 100644 --- a/packages/platform-apple/src/core/app-settings.ts +++ b/packages/platform-apple/src/core/app-settings.ts @@ -414,6 +414,7 @@ function parseIosPermissionTarget( `Permission mode is only supported for photos. Received: ${permissionMode}.`, ); } + if (normalized === 'all') return 'all'; if (normalized === 'camera') return 'camera'; if (normalized === 'microphone') return 'microphone'; if (normalized === 'contacts') return 'contacts'; @@ -434,7 +435,7 @@ function parseIosPermissionTarget( } throw new AppError( 'INVALID_ARGS', - `Unsupported permission target: ${permissionTarget}. Use camera|microphone|photos|contacts|contacts-limited|notifications|calendar|location|location-always|media-library|motion|reminders|siri.`, + `Unsupported permission target: ${permissionTarget}. Use all|camera|microphone|photos|contacts|contacts-limited|notifications|calendar|location|location-always|media-library|motion|reminders|siri.`, ); } diff --git a/src/commands/capture/settings.ts b/src/commands/capture/settings.ts index d6fa688409..8ac68246a5 100644 --- a/src/commands/capture/settings.ts +++ b/src/commands/capture/settings.ts @@ -156,6 +156,7 @@ const BIOMETRIC_STATES = setOf('match', 'nonmatch', 'enroll', 'u const FINGERPRINT_STATES = setOf('match', 'nonmatch'); const PERMISSION_STATES = setOf('grant', 'deny', 'reset'); const PERMISSION_TARGETS = setOf( + 'all', 'camera', 'microphone', 'photos', diff --git a/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-set-permissions.test.ts b/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-set-permissions.test.ts index e149a65d89..2639a796b0 100644 --- a/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-set-permissions.test.ts +++ b/src/daemon/adapters/maestro/__tests__/daemon-runtime-port-set-permissions.test.ts @@ -1,3 +1,4 @@ +import assert from 'node:assert/strict'; import { expect, test } from 'vitest'; import type { DaemonRequest } from '../../../daemon-request.ts'; import { createDaemonMaestroRuntimePort } from '../daemon-runtime-port.ts'; @@ -15,7 +16,7 @@ function makePort(requests: DaemonRequest[], platform: 'ios' | 'android') { }); } -test('setPermissions fans out to one settings call per permission', async () => { +test('setPermissions sends all as one backend call with specifics after it', async () => { const requests: DaemonRequest[] = []; const port = makePort(requests, 'android'); @@ -31,20 +32,9 @@ test('setPermissions fans out to one settings call per permission', async () => invalidateObservation() {}, }); - expect(requests.map(({ command }) => command)).toEqual([ - 'settings', - 'settings', - 'settings', - 'settings', - 'settings', - 'settings', - ]); + expect(requests.map(({ command }) => command)).toEqual(['settings', 'settings']); expect(requests.map(({ positionals }) => positionals)).toEqual([ - ['permission', 'deny', 'camera'], - ['permission', 'deny', 'contacts'], - ['permission', 'deny', 'microphone'], - ['permission', 'deny', 'notifications'], - ['permission', 'deny', 'photos'], + ['permission', 'deny', 'all'], ['permission', 'reset', 'notifications'], ]); expect( @@ -52,6 +42,59 @@ test('setPermissions fans out to one settings call per permission', async () => ).toBe(true); }); +test('a mid-sequence backend rejection names what already landed', async () => { + const requests: DaemonRequest[] = []; + let calls = 0; + const port = createDaemonMaestroRuntimePort({ + baseReq: makeBaseRequest({ flags: { platform: 'android', replayBackend: 'maestro' } }), + invoke: async (request) => { + requests.push(request); + calls += 1; + if (calls === 2) { + return { + ok: false, + error: { code: 'UNSUPPORTED_OPERATION', message: 'No such service on this runtime.' }, + }; + } + return { ok: true, data: {} }; + }, + dependencies: makeDependencies(), + platform: 'android', + }); + + const failure = await port + .execute({ + command: { + kind: 'setPermissions', + source: { line: 3 }, + permissions: { all: 'deny', notifications: 'unset' }, + }, + appId: 'com.example.app', + generation: 0, + env: {}, + invalidateObservation() {}, + }) + .then( + () => { + throw new Error('expected setPermissions to fail'); + }, + (error: unknown) => error, + ); + expect(requests.map(({ positionals }) => positionals)).toEqual([ + ['permission', 'deny', 'all'], + ['permission', 'reset', 'notifications'], + ]); + assert.match(String((failure as Error).message), /No such service on this runtime/); + assert.deepEqual( + (failure as { details?: Record }).details?.appliedPermissionMutations, + ['deny all'], + ); + assert.equal( + (failure as { details?: Record }).details?.failedPermissionMutation, + 'reset notifications', + ); +}); + test('launchApp applies permissions after clearing but before launch', async () => { const requests: DaemonRequest[] = []; const port = makePort(requests, 'android'); @@ -111,14 +154,14 @@ test('launchApp with rejected permissions launches nothing', async () => { source: { line: 3 }, appId: 'com.example.app', clearState: true, - permissions: { bluetooth: 'allow' }, + permissions: { health: 'allow' }, }, appId: 'com.example.app', generation: 0, env: {}, invalidateObservation() {}, }), - ).rejects.toThrow(/bluetooth.*not supported on android/i); + ).rejects.toThrow(/health.*not supported on android/i); expect(requests).toEqual([]); }); diff --git a/src/daemon/adapters/maestro/__tests__/set-permissions-mapping.test.ts b/src/daemon/adapters/maestro/__tests__/set-permissions-mapping.test.ts index 7b0b7fbd96..316e79ff92 100644 --- a/src/daemon/adapters/maestro/__tests__/set-permissions-mapping.test.ts +++ b/src/daemon/adapters/maestro/__tests__/set-permissions-mapping.test.ts @@ -16,46 +16,22 @@ describe('mapMaestroSetPermissions', () => { ]); }); - test('expands all to the platform servable set with specifics overriding', () => { + test('all travels as one backend call with specifics overriding after it', () => { assert.deepEqual(mapMaestroSetPermissions({ all: 'deny', notifications: 'unset' }, 'android'), [ - { state: 'deny', permission: 'camera' }, - { state: 'deny', permission: 'contacts' }, - { state: 'deny', permission: 'microphone' }, - { state: 'deny', permission: 'notifications' }, - { state: 'deny', permission: 'photos' }, + { state: 'deny', permission: 'all' }, { state: 'reset', permission: 'notifications' }, ]); - const ios = mapMaestroSetPermissions({ all: 'allow' }, 'ios'); - assert.deepEqual( - ios.map((mutation) => mutation.permission), - [ - 'calendar', - 'contacts', - 'location', - 'media-library', - 'microphone', - 'motion', - 'photos', - 'reminders', - 'siri', - ], + assert.deepEqual(mapMaestroSetPermissions({ all: 'allow' }, 'ios'), [ + { state: 'grant', permission: 'all' }, + ]); + assert.throws( + () => mapMaestroSetPermissions({ all: 'never' }, 'ios'), + /'allow', 'deny' or 'unset'/i, ); - assert.ok(ios.every((mutation) => mutation.state === 'grant')); - }); - - test('ios all skips the probe-unsupported camera and notifications', () => { - // This host's `simctl privacy help` (the source the iOS backend probe - // parses) lists neither service, so `all` excludes them rather than - // stopping the sequential mutations partway through. Explicit entries - // still reach the backend for its loud verdict. - const permissions = mapMaestroSetPermissions({ all: 'deny' }, 'ios').map( - (mutation) => mutation.permission, + assert.throws( + () => mapMaestroSetPermissions({ all: 'limited' }, 'ios'), + /'allow', 'deny' or 'unset'/i, ); - assert.ok(!permissions.includes('camera')); - assert.ok(!permissions.includes('notifications')); - assert.deepEqual(mapMaestroSetPermissions({ camera: 'allow' }, 'ios'), [ - { state: 'grant', permission: 'camera' }, - ]); }); test('maps iOS granular values and the medialibrary alias', () => { @@ -80,10 +56,22 @@ describe('mapMaestroSetPermissions', () => { ]); }); + test('maps the extended Android names to backend targets', () => { + assert.deepEqual(mapMaestroSetPermissions({ bluetooth: 'allow' }, 'android'), [ + { state: 'grant', permission: 'bluetooth' }, + ]); + assert.deepEqual(mapMaestroSetPermissions({ location: 'deny' }, 'android'), [ + { state: 'deny', permission: 'location' }, + ]); + assert.deepEqual(mapMaestroSetPermissions({ sms: 'unset' }, 'android'), [ + { state: 'reset', permission: 'sms' }, + ]); + }); + test('rejects unservable names, empty maps, and nonsense value combos', () => { assert.throws( - () => mapMaestroSetPermissions({ bluetooth: 'allow' }, 'android'), - /bluetooth.*not supported on android/i, + () => mapMaestroSetPermissions({ health: 'allow' }, 'android'), + /health.*not supported on android/i, ); assert.throws( () => mapMaestroSetPermissions({ speech: 'allow' }, 'ios'), diff --git a/src/daemon/adapters/maestro/daemon-runtime-port.ts b/src/daemon/adapters/maestro/daemon-runtime-port.ts index f71929786d..57bcbfe9e7 100644 --- a/src/daemon/adapters/maestro/daemon-runtime-port.ts +++ b/src/daemon/adapters/maestro/daemon-runtime-port.ts @@ -9,6 +9,7 @@ import { type MaestroRuntimePort, } from '@agent-device/maestro'; import { registerDiagnosticSensitiveValue } from '@agent-device/host-kit/diagnostics'; +import { AppError } from '@agent-device/kernel/errors'; import { stripUndefined } from '@agent-device/kernel/record'; import { executeRunScriptFile } from './run-script-execution.ts'; import { @@ -42,6 +43,10 @@ import { export type { CreateDaemonMaestroRuntimeOperationsOptions } from './daemon-runtime-port-support.ts'; +function describePermissionMutation(mutation: MaestroPermissionMutation): string { + return `${mutation.state} ${mutation.permission}${mutation.mode ? ` ${mutation.mode}` : ''}`; +} + function createDaemonMaestroRuntimeParts(options: CreateDaemonMaestroRuntimeOperationsOptions): { operations: MaestroRuntimeOperations; snapshots: MaestroSnapshotSource; @@ -83,33 +88,41 @@ function createDaemonMaestroRuntimeParts(options: CreateDaemonMaestroRuntimeOper ) => await withMutation(() => invoke(operation), context, stability); // launchApp.permissions applies after state clearing but before launch, so // startup code observes the requested state, and the map is validated before - // any mutation — a rejected map launches nothing. Splitting clear from open - // matches what open --clearAppState does (clear-app-state, then open). + // any mutation — a rejected map launches nothing. The split mirrors open + // --clearAppState (clear-app-state, then open without it); one nuance does + // not carry over: that flag also folds a runtime launch URL into the open on + // iOS, which Maestro flows never set, so the split is equivalent here. const applyPermissionMutations = async ( appId: string | undefined, mutations: ReadonlyArray, context: MaestroRuntimeOperationContext, ): Promise => { + const applied: string[] = []; for (const mutation of mutations) { - await invokeMutation( - { - kind: 'settingsPermission', - ...(appId ? { appId } : {}), - state: mutation.state, - permission: mutation.permission, - ...(mutation.mode ? { mode: mutation.mode } : {}), - }, - context, - ); + try { + await invokeMutation( + { + kind: 'settingsPermission', + ...(appId ? { appId } : {}), + state: mutation.state, + permission: mutation.permission, + ...(mutation.mode ? { mode: mutation.mode } : {}), + }, + context, + ); + } catch (error) { + if (error instanceof AppError) { + throw new AppError(error.code, error.message, { + ...error.details, + appliedPermissionMutations: applied, + failedPermissionMutation: describePermissionMutation(mutation), + }); + } + throw error; + } + applied.push(describePermissionMutation(mutation)); } }; - const applyPermissions = async ( - appId: string | undefined, - permissions: Readonly>, - context: MaestroRuntimeOperationContext, - ): Promise => { - await applyPermissionMutations(appId, mapMaestroSetPermissions(permissions, platform), context); - }; const typeTextAndSettle = async ( text: string, context: MaestroRuntimeOperationContext, @@ -155,25 +168,13 @@ function createDaemonMaestroRuntimeParts(options: CreateDaemonMaestroRuntimeOper await invokeMutation({ kind: 'clearAppState', ...(appId ? { appId } : {}) }, context); } await applyPermissionMutations(appId, mutations, context); - await invokeMutation( - { - kind: 'launchApp', - ...(appId ? { appId } : {}), - relaunch, - clearState: false, - launchArgs, - }, - context, - 'deferred', - ); - return; } await invokeMutation( { kind: 'launchApp', ...(appId ? { appId } : {}), relaunch, - clearState, + clearState: clearState && !input.permissions, launchArgs, }, context, @@ -185,7 +186,11 @@ function createDaemonMaestroRuntimeParts(options: CreateDaemonMaestroRuntimeOper await invokeMutation({ kind: 'stopApp', ...(appId ? { appId } : {}) }, context); }, setPermissions: async (input, context) => { - await applyPermissions(input.appId ?? context.appId, input.permissions, context); + await applyPermissionMutations( + input.appId ?? context.appId, + mapMaestroSetPermissions(input.permissions, platform), + context, + ); }, openLink: async (input, context) => { await invokeMutation( diff --git a/src/daemon/adapters/maestro/set-permissions-mapping.ts b/src/daemon/adapters/maestro/set-permissions-mapping.ts index bf2cbfe3fe..3082f18d43 100644 --- a/src/daemon/adapters/maestro/set-permissions-mapping.ts +++ b/src/daemon/adapters/maestro/set-permissions-mapping.ts @@ -1,4 +1,5 @@ import { AppError } from '@agent-device/kernel/errors'; +import { MAESTRO_PERMISSION_VALUES } from '@agent-device/maestro'; export type MaestroPermissionMutation = { readonly state: 'grant' | 'deny' | 'reset'; @@ -7,18 +8,28 @@ export type MaestroPermissionMutation = { }; /** - * Canonical Maestro names each `settings permission` backend can serve. Names - * outside these lists (bluetooth/phone/sms/storage/location/calendar on - * Android; speech/usertracking/homekit on iOS; health everywhere; custom - * Android IDs) fail loudly below instead of being silently skipped — - * extending the platform backends is a separate, device-verified change. - * - * Explicit entries keep the full servable set so the backend stays the owner - * of the verdict: a name the runtime cannot serve (e.g. iOS camera on runtimes - * whose `simctl privacy help` lists no camera service) fails loudly there. + * Canonical Maestro names each `settings permission` backend serves + * individually. `all` is not listed: it travels as one `settings permission` + * call and each backend resolves it (iOS `simctl privacy … all`, Android's + * declared-permission intersection). Names outside these lists (iOS + * speech/usertracking/homekit/health; Android custom ids) fail loudly below + * instead of being silently skipped. */ const EXPANDABLE_PERMISSIONS = { - android: ['camera', 'contacts', 'microphone', 'notifications', 'photos'], + android: [ + 'bluetooth', + 'calendar', + 'camera', + 'contacts', + 'location', + 'media-library', + 'microphone', + 'notifications', + 'phone', + 'photos', + 'sms', + 'storage', + ], ios: [ 'calendar', 'camera', @@ -34,25 +45,11 @@ const EXPANDABLE_PERMISSIONS = { ], } as const; -/** - * Names excluded from `all` expansion. `all` must succeed on the runtimes we - * ship, so it covers only the probe-supported subset: this host's - * `simctl privacy help` (the same source `getSimctlPrivacyServices` parses in - * the iOS backend) lists neither camera nor notifications, and the iOS backend - * rejects grant/deny for notifications with UNSUPPORTED_OPERATION — keeping - * either in `all` would stop the sequential mutations partway through. - * Explicit entries for those names still reach the backend above. - */ -const ALL_EXCLUDED_PERMISSIONS: Readonly> = { - android: [], - ios: ['camera', 'notifications'], -}; - /** Per-platform hint for names the backends cannot serve yet. */ const UNSUPPORTED_HINTS = { android: - 'Supported: camera, contacts, microphone, notifications, photos (via all or individually). Other names need platform-backend support first.', - ios: 'Supported: calendar, camera, contacts, location, media-library, microphone, motion, notifications, photos, reminders, siri (via all or individually). Granular iOS values: location always|inuse|never, photos limited.', + 'Supported: all, bluetooth, calendar, camera, contacts, location, media-library, microphone, notifications, phone, photos, sms, storage. Android custom permission ids are attempted through all, not individually.', + ios: 'Supported: all, calendar, camera, contacts, location, media-library, microphone, motion, notifications, photos, reminders, siri. Granular iOS values: location always|inuse|never, photos limited.', } as const; /** Non-canonical spellings accepted alongside the lists above. */ @@ -87,9 +84,9 @@ const GRANULAR_HINTS: Record = { /** * Expand a Maestro `setPermissions` map into ordered `settings permission` - * mutations. `all` expands to the platform's servable subset first so specific - * entries always override it regardless of authored order. Values arrive - * lowercased from the Maestro runtime layer; anything else is refused. + * mutations. `all` travels as one backend call first so specific entries + * always override it regardless of authored order. Values arrive lowercased + * from the Maestro runtime layer; anything else is refused. * The expansion is fully validated here, so callers must map before issuing * any mutation — a rejected map changes nothing. */ @@ -101,32 +98,39 @@ export function mapMaestroSetPermissions( if (entries.length === 0) { throw new AppError('INVALID_ARGS', 'Maestro setPermissions requires at least one permission.'); } - const expandable = new Set(EXPANDABLE_PERMISSIONS[platform]); - const excluded = new Set(ALL_EXCLUDED_PERMISSIONS[platform]); - const allExpansion = EXPANDABLE_PERMISSIONS[platform].filter((name) => !excluded.has(name)); + const mutations: MaestroPermissionMutation[] = []; const specific = new Map(); - let allValue: string | undefined; for (const [name, value] of entries) { if (name.toLowerCase() === 'all') { - allValue = value; + mutations.push(mapMaestroAll(value)); } else { specific.set(canonicalName(name), value); } } - const merged: Array<[string, string]> = - allValue === undefined - ? [...specific] - : [...allExpansion.map((name): [string, string] => [name, allValue]), ...specific]; - return merged.map(([name, value]) => mapMaestroPermission(name, value, platform, expandable)); + for (const [name, value] of specific) { + mutations.push(mapMaestroPermission(name, value, platform)); + } + return mutations; +} + +/** `all` accepts only the plain values; granular ones name no single backend state. */ +function mapMaestroAll(value: string): MaestroPermissionMutation { + const state = PLAIN_VALUE_STATES[value as keyof typeof PLAIN_VALUE_STATES]; + if (!MAESTRO_PERMISSION_VALUES.has(value) || !state) { + throw new AppError( + 'INVALID_ARGS', + `Permission 'all' can be set to 'allow', 'deny' or 'unset', not '${value}'.`, + ); + } + return { state, permission: 'all' }; } function mapMaestroPermission( name: string, value: string, platform: 'ios' | 'android', - expandable: ReadonlySet, ): MaestroPermissionMutation { - if (!expandable.has(name)) { + if (!new Set(EXPANDABLE_PERMISSIONS[platform]).has(name)) { throw new AppError( 'UNSUPPORTED_OPERATION', `Maestro permission "${name}" is not supported on ${platform} yet.`, diff --git a/src/daemon/handlers/snapshot-settings.ts b/src/daemon/handlers/snapshot-settings.ts index e4cb0f921c..f711db6e9e 100644 --- a/src/daemon/handlers/snapshot-settings.ts +++ b/src/daemon/handlers/snapshot-settings.ts @@ -172,6 +172,8 @@ export async function handleSettingsCommand( return errorResponse('INVALID_ARGS', getUnsupportedMacOsSettingMessage(setting)); } + // Explicit positional wins; the Maestro adapter's daemon-internal + // settingsAppBundleId overrides the session app for cross-app targeting. const appBundleId = parsed.appBundleId ?? req.internal?.settingsAppBundleId ?? session?.appBundleId; if (setting === 'clear-app-state' && !appBundleId) { diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index 14bb0800d6..e83d46add6 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -70,7 +70,7 @@ agent-device test ./maestro-flows --maestro --platform android --artifacts-dir . Supported subset: -- Flows: `launchApp` (with `clearState`, `permissions`, and Apple-only launch arguments; `permissions` apply after state clearing but before launch, and a `launchApp` without `permissions` touches nothing — there is no silent `all: allow` default); `setPermissions` (mid-flow permission grants/denials/resets; `all` expands to the backend-servable set — Android: camera/contacts/microphone/notifications/photos, iOS: the `simctl privacy help` subset excluding `camera`/`notifications`; anything else fails loudly instead of being skipped; `unset` fully resets and `location: never` denies); `runFlow` file/inline with platform, visibility, and limited boolean conditions; `onFlowStart`/`onFlowComplete`; `repeat.times` and retry. +- Flows: `launchApp` (with `clearState`, `permissions`, and Apple-only launch arguments; `permissions` apply after state clearing but before launch, and a `launchApp` without `permissions` touches nothing — there is no silent `all: allow` default); `setPermissions` (mid-flow permission grants/denials/resets; `all` resolves in the backend — one simctl call on iOS, the declared permissions on Android — with specifics overriding after it; unservable names fail loudly instead of being skipped; `unset` fully resets and `location: never` denies); `runFlow` file/inline with platform, visibility, and limited boolean conditions; `onFlowStart`/`onFlowComplete`; `repeat.times` and retry. - Interactions: `tapOn`, `doubleTapOn`, `longPressOn`, `inputText` on the focused element, `eraseText`, `openLink`, `hideKeyboard`, basic `pressKey`, and `back`; selector targets poll until available and support recursive `index`, `childOf`, `above`, `below`, `leftOf`, `rightOf`, `containsChild`, `containsDescendants`, points, and `optional`; outer command labels are metadata, not target selectors. - Assertions and navigation: `assertVisible`, `assertNotVisible`, `assertTrue` (literal values and `${VAR}` lookups only; `""`, `"false"`, `"0"`, `"null"`, and `"undefined"` are falsy, everything else is truthy), `extendedWaitUntil`, `scroll`, `scrollUntilVisible`, absolute/percentage/target `swipe`, `takeScreenshot`, `waitForAnimationToEnd`, and `stopApp`. - Scripts: ordered `runScript` file/env scripts with `http.post`, `json`, and `output` variables. From 682d43df783764a771db540025adb39473bf6ed4 Mon Sep 17 00:00:00 2001 From: Rohit <40559587+Rohit3523@users.noreply.github.com> Date: Mon, 7 Sep 2026 00:47:45 +0530 Subject: [PATCH 5/6] fix(maestro): fail explicit notifications reset instead of reset-all fallback A notifications-only unset must not clear microphone, location and other permissions through the reset-all sledgehammer. The probe gate rejects unlisted notifications again; the reset-all fallback stays for runtimes that list the service but block the direct reset. Regression proves a microphone grant survives the failed reset. --- .../src/core/__tests__/app-settings.test.ts | 29 ++++++++++++------- .../platform-apple/src/core/app-settings.ts | 14 ++++----- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/packages/platform-apple/src/core/__tests__/app-settings.test.ts b/packages/platform-apple/src/core/__tests__/app-settings.test.ts index 2c7c7f0dc6..0e02a0bbd3 100644 --- a/packages/platform-apple/src/core/__tests__/app-settings.test.ts +++ b/packages/platform-apple/src/core/__tests__/app-settings.test.ts @@ -485,10 +485,10 @@ test('setIosSetting permission reset notifications falls back to reset all when ); }); -test('setIosSetting permission reset notifications falls back to reset all when unlisted in privacy help', async () => { - // Runtimes like iOS 26.3 omit notifications from `simctl privacy help`, yet - // direct reset fails only with "operation not permitted" while `reset all` - // succeeds — so reset bypasses the probe gate into the existing fallback. +test('setIosSetting permission reset notifications fails explicitly without touching other services', async () => { + // Runtimes like iOS 26.3 omit notifications from `simctl privacy help`, where + // no targeted reset exists: the probe gate rejects before any privacy call, + // so an earlier microphone grant survives the failed reset. const device: DeviceInfo = { ...IOS_TEST_SIMULATOR, simulatorSetPath: '/fake/privacy-help-no-notifications', @@ -503,19 +503,28 @@ test('setIosSetting permission reset notifications falls back to reset all when if (isSimctlListDevices(args)) return BOOTED_SIM_LIST_JSON; if (args.includes('help')) return HELP_WITHOUT_NOTIFICATIONS; const flat = args.join(' '); - if (flat.includes('reset notifications com.example.app')) { - return { stderr: 'Failed to reset access\nOperation not permitted', exitCode: 1 }; - } - if (flat.includes('reset all com.example.app')) return ''; + if (flat.includes('grant microphone com.example.app')) return ''; return unexpectedArgs(args); }, async ({ calls }) => { - await setIosSetting(device, 'permission', 'reset', 'com.example.app', { - permissionTarget: 'notifications', + await setIosSetting(device, 'permission', 'grant', 'com.example.app', { + permissionTarget: 'microphone', }); + await assertRejectsAppError( + () => + setIosSetting(device, 'permission', 'reset', 'com.example.app', { + permissionTarget: 'notifications', + }), + { code: 'UNSUPPORTED_OPERATION', message: /does not support service "notifications"/i }, + ); const flat = calls.map((args) => args.join(' ')); assert.equal( flat.some((line) => line.includes('reset all com.example.app')), + false, + flat.join('; '), + ); + assert.equal( + flat.some((line) => line.includes('grant microphone com.example.app')), true, flat.join('; '), ); diff --git a/packages/platform-apple/src/core/app-settings.ts b/packages/platform-apple/src/core/app-settings.ts index b69aec85f1..ff1ae3e0b7 100644 --- a/packages/platform-apple/src/core/app-settings.ts +++ b/packages/platform-apple/src/core/app-settings.ts @@ -272,11 +272,7 @@ async function runIosPrivacyCommand( appBundleId: string, ): Promise { const supportedServices = await getSimctlPrivacyServices(device); - // reset notifications falls back to `reset all` below (direct reset fails - // with "operation not permitted" on runtimes whose help omits the service), - // so it passes the probe gate even when the service is unlisted. Grant/deny - // for notifications stay loud rejections. - if (!supportedServices.has(target) && !(action === 'reset' && target === 'notifications')) { + if (!supportedServices.has(target)) { throw new AppError( 'UNSUPPORTED_OPERATION', `iOS simctl privacy does not support service "${target}" on this runtime.`, @@ -313,9 +309,11 @@ async function runIosPrivacyCommand( } /** - * Direct `reset notifications` fails with "operation not permitted" on - * runtimes whose help omits the service, while `reset all` succeeds — so - * reset goes through the fallback instead of failing loudly like grant/deny. + * Direct `reset notifications` can fail with "operation not permitted" even on + * runtimes that list the service, while `reset all` succeeds — so reset goes + * through the fallback instead of failing loudly like grant/deny. When the + * probe omits notifications entirely there is no targeted reset available, and + * the gate above fails explicitly rather than clearing unrelated state. */ async function resetIosNotificationsPermission( device: DeviceInfo, From 752b88d583a9cfb83ae777860fcef2539cadc0dd Mon Sep 17 00:00:00 2001 From: Rohit <40559587+Rohit3523@users.noreply.github.com> Date: Mon, 7 Sep 2026 20:33:39 +0530 Subject: [PATCH 6/6] fix(maestro): fail targeted notifications reset, propagate operational Android failures, cover setPermissions in fuzz --- .../src/__tests__/settings-permission.test.ts | 66 +++++++++++++++++++ .../src/settings-permission.ts | 46 ++++++++++++- .../src/core/__tests__/app-settings.test.ts | 29 ++++++-- .../platform-apple/src/core/app-settings.ts | 51 ++++---------- .../fuzz/validation-arbitraries-maestro.ts | 8 +++ 5 files changed, 152 insertions(+), 48 deletions(-) diff --git a/packages/platform-android/src/__tests__/settings-permission.test.ts b/packages/platform-android/src/__tests__/settings-permission.test.ts index 610b625680..3f1fe1c7ca 100644 --- a/packages/platform-android/src/__tests__/settings-permission.test.ts +++ b/packages/platform-android/src/__tests__/settings-permission.test.ts @@ -454,3 +454,69 @@ test.each([ }, ); }); + +// An operational pm failure mid-`all` aborts instead of being skipped: an +// offline device must not let launchApp continue with half-applied permissions. +test('setAndroidSetting permission grant all propagates an operational pm failure', async () => { + await withFakeAdb( + fakeAdb((flat) => { + if (flat === CURRENT_USER) return '0'; + if (flat === DUMPSYS) return dumpsysWithRequested(); + if (flat === 'shell pm grant --user 0 com.example.app android.permission.RECORD_AUDIO') { + return { stderr: 'device offline', exitCode: 1 }; + } + return undefined; + }), + async ({ device }) => { + await assertRejectsAppError( + () => + setAndroidSetting(device, 'permission', 'grant', 'com.example.app', { + permissionTarget: 'all', + }), + { code: 'COMMAND_FAILED', message: /Failed to grant Android permission.*RECORD_AUDIO/ }, + ); + }, + ); +}); + +// A photos probe that fails operationally (not as non-changeable) aborts `all` +// rather than collapsing into a skip warning. +test('setAndroidSetting permission grant all propagates an operational photos failure', async () => { + const requested = [ + 'Packages:', + ' Package [com.example.app] (abc):', + ' requested permissions:', + ' android.permission.READ_MEDIA_IMAGES', + ' User 0: ceDataInode=0 installed=true', + ' runtime permissions:', + ' android.permission.READ_MEDIA_IMAGES: granted=false', + 'Queries:', + ].join('\n'); + await withFakeAdb( + fakeAdb((flat) => { + if (flat === 'shell getprop ro.build.version.sdk') return '36'; + if (flat === CURRENT_USER) return '0'; + if (flat === DUMPSYS) return requested; + if ( + flat.startsWith('shell pm grant --user 0 com.example.app android.permission.READ_MEDIA') + ) { + return { stderr: 'device offline', exitCode: 1 }; + } + if ( + flat.startsWith('shell pm grant --user 0 com.example.app android.permission.READ_EXTERNAL') + ) { + return { stderr: 'device offline', exitCode: 1 }; + } + return undefined; + }), + async ({ device }) => { + await assertRejectsAppError( + () => + setAndroidSetting(device, 'permission', 'grant', 'com.example.app', { + permissionTarget: 'all', + }), + { code: 'COMMAND_FAILED', message: /Failed to grant Android photos permission/ }, + ); + }, + ); +}); diff --git a/packages/platform-android/src/settings-permission.ts b/packages/platform-android/src/settings-permission.ts index ebe7bfd8a4..0c09ddef72 100644 --- a/packages/platform-android/src/settings-permission.ts +++ b/packages/platform-android/src/settings-permission.ts @@ -3,6 +3,7 @@ import type { DeviceInfo } from '@agent-device/kernel/device'; import { parsePermissionAction, parsePermissionTarget } from '@agent-device/contracts/settings'; import type { SettingOptions } from '@agent-device/contracts/settings'; import { runAndroidAdb } from './adb.ts'; +import { androidAdbResultError } from './adb-failure.ts'; import { parseAndroidPackagePermissions, readAndroidCurrentUserId, @@ -149,6 +150,8 @@ export async function setAndroidPermission( * stopping the sequence — while an explicit target for the same id still * fails loudly. Anything the dump does not list is never attempted, which is * what keeps `pm` from throwing "has not requested permission" partway. + * Operational failures (offline device, dropped transport) abort the fan-out + * instead of becoming skips, so launchApp cannot continue half-applied. */ async function setAllAndroidPermissions( device: DeviceInfo, @@ -314,7 +317,31 @@ async function tryPmUnit( { allowFailure: true }, ); if (result.exitCode === 0) return { ok: true }; - return { ok: false, reason: firstStderrLine(result.stderr) }; + if (isSkippablePmStderr(result.stderr)) { + return { ok: false, reason: firstStderrLine(result.stderr) }; + } + throw androidAdbResultError( + `Failed to ${pmAction} Android permission ${permission} for ${appPackage}`, + result, + { appPackage, permission }, + ); +} + +/** + * Only established non-changeable signals are skipped under `all`: an install + * permission `pm` cannot touch, an id the package never requested, or a name + * the runtime does not know as a runtime permission. Anything else (offline + * device, dropped transport, denied op) is operational and must abort the + * fan-out rather than let launchApp continue with half-applied permissions. + */ +function isSkippablePmStderr(stderr: string): boolean { + const text = stderr.toLowerCase(); + return ( + text.includes('not a changeable permission') || + text.includes('has not requested permission') || + text.includes('is not a runtime permission') || + text.includes('unknown permission') + ); } async function tryPhotosUnit( @@ -325,11 +352,24 @@ async function tryPhotosUnit( ): Promise { try { return await setAndroidPhotoPermission(device, appPackage, pmAction, userArgs); - } catch { - return undefined; + } catch (error) { + if (isSkippablePhotosError(error)) return undefined; + throw error; } } +/** A photos probe failure is skippable only when every candidate was refused as non-changeable. */ +function isSkippablePhotosError(error: unknown): boolean { + if (!(error instanceof AppError) || error.code !== 'COMMAND_FAILED') return false; + const attempts = error.details?.attempts; + if (!Array.isArray(attempts) || attempts.length === 0) return false; + return attempts.every( + (attempt) => + typeof (attempt as { stderr?: unknown }).stderr === 'string' && + isSkippablePmStderr((attempt as { stderr: string }).stderr), + ); +} + function firstStderrLine(stderr: string): string { const lines = stderr .split('\n') diff --git a/packages/platform-apple/src/core/__tests__/app-settings.test.ts b/packages/platform-apple/src/core/__tests__/app-settings.test.ts index 0e02a0bbd3..7e4d084f1f 100644 --- a/packages/platform-apple/src/core/__tests__/app-settings.test.ts +++ b/packages/platform-apple/src/core/__tests__/app-settings.test.ts @@ -455,21 +455,35 @@ test('setIosSetting permission rejects mode for non-photos target', async () => ); }); -test('setIosSetting permission reset notifications falls back to reset all when direct reset is blocked', async () => { +test('setIosSetting permission reset notifications fails targeted when direct reset is blocked', async () => { + // A listed-but-blocked notifications service must not fall back to `reset + // all`: a notifications-only reset would clear microphone, location, and + // other grants. The targeted reset fails instead, leaving the earlier grant + // in place. await withFakeAppleTool( (args) => { if (isSimctlListDevices(args)) return BOOTED_SIM_LIST_JSON; if (args[0] === 'simctl' && args[1] === 'privacy' && args[2] === 'help') return undefined; + if (args.join(' ') === 'simctl privacy sim-1 grant microphone com.example.app') return ''; if (args.join(' ') === 'simctl privacy sim-1 reset notifications com.example.app') { return { stderr: 'Failed to reset access\nOperation not permitted', exitCode: 1 }; } - if (args.join(' ') === 'simctl privacy sim-1 reset all com.example.app') return ''; return unexpectedArgs(args); }, async ({ calls }) => { - await setIosSetting(IOS_TEST_SIMULATOR, 'permission', 'reset', 'com.example.app', { - permissionTarget: 'notifications', + await setIosSetting(IOS_TEST_SIMULATOR, 'permission', 'grant', 'com.example.app', { + permissionTarget: 'microphone', }); + await assertRejectsAppError( + () => + setIosSetting(IOS_TEST_SIMULATOR, 'permission', 'reset', 'com.example.app', { + permissionTarget: 'notifications', + }), + { + code: 'UNSUPPORTED_OPERATION', + message: /does not support resetting notifications permission/i, + }, + ); const flat = calls.map((args) => args.join(' ')); assert.equal( flat.includes('simctl privacy sim-1 reset notifications com.example.app'), @@ -477,7 +491,12 @@ test('setIosSetting permission reset notifications falls back to reset all when flat.join('; '), ); assert.equal( - flat.includes('simctl privacy sim-1 reset all com.example.app'), + flat.some((line) => line.includes('reset all com.example.app')), + false, + flat.join('; '), + ); + assert.equal( + flat.includes('simctl privacy sim-1 grant microphone com.example.app'), true, flat.join('; '), ); diff --git a/packages/platform-apple/src/core/app-settings.ts b/packages/platform-apple/src/core/app-settings.ts index ff1ae3e0b7..407706ef69 100644 --- a/packages/platform-apple/src/core/app-settings.ts +++ b/packages/platform-apple/src/core/app-settings.ts @@ -285,10 +285,6 @@ async function runIosPrivacyCommand( } const args = ['privacy', device.id, action, target, appBundleId]; - if (action === 'reset' && target === 'notifications') { - await resetIosNotificationsPermission(device, appBundleId); - return; - } try { await runSimctl(device, args); return; @@ -296,6 +292,17 @@ async function runIosPrivacyCommand( if (!(target === 'notifications' && isNotificationsOperationNotPermitted(error))) { throw error; } + if (action === 'reset') { + throw new AppError( + 'UNSUPPORTED_OPERATION', + 'iOS simulator does not support resetting notifications permission via simctl privacy on this runtime.', + { + deviceId: device.id, + appBundleId, + hint: 'Use reinstall to force a fresh notifications prompt, or reset simulator content and settings.', + }, + ); + } throw new AppError( 'UNSUPPORTED_OPERATION', 'iOS simulator does not support setting notifications permission via simctl privacy on this runtime.', @@ -308,42 +315,6 @@ async function runIosPrivacyCommand( } } -/** - * Direct `reset notifications` can fail with "operation not permitted" even on - * runtimes that list the service, while `reset all` succeeds — so reset goes - * through the fallback instead of failing loudly like grant/deny. When the - * probe omits notifications entirely there is no targeted reset available, and - * the gate above fails explicitly rather than clearing unrelated state. - */ -async function resetIosNotificationsPermission( - device: DeviceInfo, - appBundleId: string, -): Promise { - try { - await runSimctl(device, ['privacy', device.id, 'reset', 'notifications', appBundleId]); - return; - } catch (error) { - if (!isNotificationsOperationNotPermitted(error)) { - throw error; - } - } - - try { - await runSimctl(device, ['privacy', device.id, 'reset', 'all', appBundleId]); - } catch (error) { - throw new AppError( - 'COMMAND_FAILED', - 'iOS simulator blocked direct notifications reset. Fallback reset-all also failed.', - { - deviceId: device.id, - appBundleId, - hint: 'Use reinstall to force a fresh notifications prompt, or reset simulator content and settings.', - }, - error instanceof Error ? error : undefined, - ); - } -} - function isNotificationsOperationNotPermitted(error: unknown): boolean { if (!(error instanceof AppError) || error.code !== 'COMMAND_FAILED') return false; const stderr = String(error.details?.stderr ?? '').toLowerCase(); diff --git a/scripts/fuzz/validation-arbitraries-maestro.ts b/scripts/fuzz/validation-arbitraries-maestro.ts index a332a504f4..513a53d3fd 100644 --- a/scripts/fuzz/validation-arbitraries-maestro.ts +++ b/scripts/fuzz/validation-arbitraries-maestro.ts @@ -45,6 +45,9 @@ function validMaestroCommand(pick: number, salt: number): string[] { () => ['- scrollUntilVisible:', ' element:', ` text: ${text}`], () => ['- repeat:', ' times: 2', ' commands:', ' - back'], () => ['- runFlow: other.yaml'], + () => ['- setPermissions:', ' permissions:', ' camera: allow'], + () => ['- setPermissions:', ' permissions:', ' all: deny'], + () => ['- launchApp:', ' appId: com.example.app', ' permissions:', ' camera: allow'], ]; return options[pick % options.length]!(); } @@ -92,6 +95,11 @@ const MAESTRO_MUTATIONS: readonly MaestroMutation[] = [ }, { name: 'bad-press-key', code: 'INVALID_ARGS', lines: () => ['- pressKey: sleep'] }, { name: 'scroll-options', code: 'INVALID_ARGS', lines: () => ['- scroll:', ' direction: UP'] }, + { + name: 'bad-permission-value', + code: 'INVALID_ARGS', + lines: () => ['- setPermissions:', ' permissions:', ' camera: maybe'], + }, ]; /** Declared classes plus the config-level variant `unsupported-field` renders for a salt slice. */