diff --git a/packages/contracts/src/settings.test.ts b/packages/contracts/src/settings.test.ts index 65e75511ee..13b8b5f7da 100644 --- a/packages/contracts/src/settings.test.ts +++ b/packages/contracts/src/settings.test.ts @@ -1,14 +1,18 @@ import { describe, expect, expectTypeOf, test } from 'vitest'; import { READABLE_SETTINGS } from './platform-runtime-operations.ts'; +import type { SettingsUpdateOptions } from './client-settings.ts'; import { + APPEARANCE_ACTIONS, describeSettingRead, describeSettingWrite, getUnsupportedMacOsSettingMessage, isMacOsSettingSupported, MACOS_PERMISSION_TARGETS, MOBILE_PERMISSION_TARGETS, + parseAppearanceAction, parsePermissionAction, parsePermissionTarget, + parseSettingState, parseTextSizeCategory, PERMISSION_ACTIONS, PERMISSION_MODES, @@ -18,6 +22,7 @@ import { SETTINGS_USAGE_OVERRIDE, TEXT_SIZE_CATEGORIES, textSizeSettingPayload, + type AppearanceAction, type PermissionAction, type PermissionTarget, type ReadableSetting, @@ -266,3 +271,63 @@ describe('permission vocabulary types', () => { expectTypeOf<'input-monitoring'>().not.toMatchTypeOf(); }); }); + +// Fixed expected data on purpose: these are the acceptance sets the Apple and Android settings +// owners each enforced in their own copy of the same parser before the copies were deleted. The +// two parsers agreed byte for byte, including the refusal wording, so one declaration now carries +// both — and a widened or narrowed set here fails here rather than diverging one platform at a +// time, which is the only way the copies could have drifted unnoticed. +const APPEARANCE_SET = ['light', 'dark', 'toggle'] as const; + +describe('parseAppearanceAction', () => { + test('holds the appearances every settings surface already accepted, in help order', () => { + expect([...APPEARANCE_ACTIONS]).toEqual([...APPEARANCE_SET]); + }); + + test('accepts each appearance under each normalization', () => { + for (const action of APPEARANCE_SET) { + for (const normalize of NORMALIZATIONS) { + expect(parseAppearanceAction(normalize(action))).toBe(action); + } + } + }); + + test('refuses an appearance outside the vocabulary with the accepted list', () => { + for (const state of ['bright', '', ' ', 'light-dark']) { + expectInvalidArgs( + () => parseAppearanceAction(state), + `Invalid appearance state: ${state}. Use ${[...APPEARANCE_SET].join('|')}.`, + ); + } + }); +}); + +describe('parseSettingState', () => { + test('maps every on and off spelling in any casing', () => { + for (const state of ['on', 'ON', 'true', 'TRUE', '1']) { + expect(parseSettingState(state)).toBe(true); + } + for (const state of ['off', 'OFF', 'false', 'FALSE', '0']) { + expect(parseSettingState(state)).toBe(false); + } + }); + + test('refuses a state that spells neither, and keeps padding unaccepted', () => { + // Padding is not one of these spellings: unlike an appearance name, a state reaches `settings + // put` as an argument, where a stray space is a caller bug rather than a casing variant. + for (const state of ['maybe', '2', '', ' on ']) { + expectInvalidArgs(() => parseSettingState(state), `Invalid setting state: ${state}`); + } + }); +}); + +describe('appearance vocabulary types', () => { + test('the vocabulary stays the appearance state the published client type carries', () => { + // The CLI's membership set, both platform parsers, and the published client union are all built + // from this one declaration now. The union itself stays spelled out in `client-settings.ts` so + // the shipped .d.ts keeps its literal shape; this pin is what stops the two drifting. + expectTypeOf().toEqualTypeOf< + Extract['state'] + >(); + }); +}); diff --git a/packages/contracts/src/settings.ts b/packages/contracts/src/settings.ts index 7d80a89745..15414a5a42 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -70,6 +70,23 @@ export const TEXT_SIZE_CATEGORIES = [ export type TextSizeCategory = (typeof TEXT_SIZE_CATEGORIES)[number]; +/** + * The appearances `settings appearance` accepts, in the order `settings` help lists them. Apple + * applies them natively; Android maps them onto `ui_night_mode`. Acceptance is not support. + */ +export const APPEARANCE_ACTIONS = ['light', 'dark', 'toggle'] as const; + +export type AppearanceAction = (typeof APPEARANCE_ACTIONS)[number]; + +/** + * The state spellings `settings ` accepts. Unlike the appearance and permission + * vocabularies these are aliases onto a boolean rather than a name the device echoes back, so + * membership — not {@link findVocabularyName} — decides, and padding stays rejected: an + * `adb shell settings put` argument with a stray space in it is a caller bug, not a spelling. + */ +const SETTING_STATE_ON = ['on', 'true', '1']; +const SETTING_STATE_OFF = ['off', 'false', '0']; + /** * The settings that answer a bare `settings ` with the value the target holds. This is the * vocabulary every settings type and every settings owner reads it from; the matching value is @@ -137,7 +154,7 @@ export type SettingOptions = { const SETTINGS_WIFI_USAGE = ' '; const SETTINGS_LOCATION_SET_USAGE = 'location set '; const SETTINGS_ANIMATIONS_USAGE = 'animations '; -const SETTINGS_APPEARANCE_USAGE = 'appearance '; +const SETTINGS_APPEARANCE_USAGE = `appearance <${APPEARANCE_ACTIONS.join('|')}>`; const SETTINGS_FACEID_USAGE = 'faceid '; const SETTINGS_TOUCHID_USAGE = 'touchid '; const SETTINGS_FINGERPRINT_USAGE = 'fingerprint '; @@ -244,3 +261,25 @@ export function parsePermissionTarget(value: string | undefined): PermissionTarg `permission setting requires a target: ${MOBILE_PERMISSION_TARGETS.join('|')}`, ); } + +/** + * The appearance a caller asked for. Every surface that sets one parses here first: `simctl` and + * `adb` both answer a bad token with a tool-specific error or a silent no-op, so the refusal has to + * name the accepted set before a device is touched. + */ +export function parseAppearanceAction(state: string): AppearanceAction { + const parsed = findVocabularyName(APPEARANCE_ACTIONS, state); + if (parsed !== undefined) return parsed; + throw new AppError( + 'INVALID_ARGS', + `Invalid appearance state: ${state}. Use ${APPEARANCE_ACTIONS.join('|')}.`, + ); +} + +/** The boolean a `settings ` positional spells, in any casing. */ +export function parseSettingState(state: string): boolean { + const normalized = state.toLowerCase(); + if (SETTING_STATE_ON.includes(normalized)) return true; + if (SETTING_STATE_OFF.includes(normalized)) return false; + throw new AppError('INVALID_ARGS', `Invalid setting state: ${state}`); +} diff --git a/packages/kernel/src/errors.test.ts b/packages/kernel/src/errors.test.ts index c4e884de6f..329bf46504 100644 --- a/packages/kernel/src/errors.test.ts +++ b/packages/kernel/src/errors.test.ts @@ -1,6 +1,10 @@ import assert from 'node:assert/strict'; import { test } from 'vitest'; -import { readElementMatchCandidateRefs, readErrorCandidateViews } from './errors.ts'; +import { + readElementMatchCandidateRefs, + readErrorCandidateViews, + summarizeCommandAttemptFailures, +} from './errors.ts'; test('readElementMatchCandidateRefs extracts refs from candidate lines', () => { assert.deepEqual( @@ -43,3 +47,27 @@ test('readErrorCandidateViews projects devices and rejects object candidates', ( [{ kind: 'device', devices: [{ id: 'SIM-001', name: 'iPhone 17 Pro' }] }], ); }); + +test("summarizeCommandAttemptFailures joins argv and caps each attempt's stderr", () => { + const [summary] = summarizeCommandAttemptFailures([ + { args: ['shell', 'cmd', 'fingerprint'], stdout: 'out', stderr: 'x'.repeat(500), exitCode: 2 }, + ]); + + assert.deepEqual(summary, { + args: 'shell cmd fingerprint', + exitCode: 2, + // 400 is the per-attempt budget the settings retry loops have always shipped evidence under; + // it is pinned as a literal so a changed budget cannot move both the producer and this oracle. + stderr: 'x'.repeat(400), + }); +}); + +test('summarizeCommandAttemptFailures keeps every attempt in the order it ran', () => { + assert.deepEqual( + summarizeCommandAttemptFailures([ + { args: ['first'], stdout: '', stderr: 'a', exitCode: 1 }, + { args: ['second'], stdout: '', stderr: 'b', exitCode: 9 }, + ]).map(({ args, exitCode }) => `${args}:${exitCode}`), + ['first:1', 'second:9'], + ); +}); diff --git a/packages/kernel/src/errors.ts b/packages/kernel/src/errors.ts index 457abefa93..b8fe2b1aa9 100644 --- a/packages/kernel/src/errors.ts +++ b/packages/kernel/src/errors.ts @@ -285,6 +285,32 @@ export function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } +/** One command a retry loop ran and could not make succeed, kept in the tool's own terms. */ +export type CommandAttemptFailure = { + args: readonly string[]; + stdout: string; + stderr: string; + exitCode: number; +}; + +/** + * The per-attempt view a retry loop attaches to its refusal, so the caller can tell which spelling + * the device rejected rather than only that all of them failed. Stderr is truncated per attempt, not + * in total: every attempt keeps a head of its own, because the loop usually fails for a reason that + * appears in only one of them. + */ +const COMMAND_ATTEMPT_STDERR_BUDGET = 400; + +export function summarizeCommandAttemptFailures( + failures: CommandAttemptFailure[], +): Array<{ args: string; exitCode: number; stderr: string }> { + return failures.map((failure) => ({ + args: failure.args.join(' '), + exitCode: failure.exitCode, + stderr: failure.stderr.slice(0, COMMAND_ATTEMPT_STDERR_BUDGET), + })); +} + export function asAppError(err: unknown, fallbackCode: AppErrorCode = 'UNKNOWN'): AppError { if (err instanceof AppError) return err; if (err instanceof Error) { diff --git a/packages/platform-android/src/__tests__/settings-parsing.test.ts b/packages/platform-android/src/__tests__/settings-parsing.test.ts deleted file mode 100644 index 55c7c2014c..0000000000 --- a/packages/platform-android/src/__tests__/settings-parsing.test.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { test } from 'vitest'; -import assert from 'node:assert/strict'; -import { - parseAppearanceAction, - parseSettingState, - summarizeCommandAttemptFailures, -} from '../settings-parsing.ts'; -import { assertThrowsAppError } from './test-utils/app-error.ts'; - -test('parseAppearanceAction accepts the three actions in any casing or padding', () => { - assert.equal(parseAppearanceAction('light'), 'light'); - assert.equal(parseAppearanceAction('DARK'), 'dark'); - assert.equal(parseAppearanceAction(' Toggle '), 'toggle'); -}); - -test('parseAppearanceAction rejects anything else and names the accepted set', () => { - assertThrowsAppError(() => parseAppearanceAction('bright'), { code: 'INVALID_ARGS' }); - assertThrowsAppError(() => parseAppearanceAction(''), { code: 'INVALID_ARGS' }); -}); - -test('parseSettingState reads every on and off spelling', () => { - for (const on of ['on', 'ON', 'true', '1']) assert.equal(parseSettingState(on), true); - for (const off of ['off', 'OFF', 'false', '0']) assert.equal(parseSettingState(off), false); -}); - -test('parseSettingState rejects a state that is neither', () => { - assertThrowsAppError(() => parseSettingState('maybe'), { code: 'INVALID_ARGS' }); - assertThrowsAppError(() => parseSettingState('2'), { code: 'INVALID_ARGS' }); -}); - -test('summarizeCommandAttemptFailures joins args and truncates stderr to its budget', () => { - const [summary] = summarizeCommandAttemptFailures([ - { args: ['shell', 'cmd', 'fingerprint'], stdout: 'out', stderr: 'x'.repeat(500), exitCode: 2 }, - ]); - - assert.equal(summary?.args, 'shell cmd fingerprint'); - assert.equal(summary?.exitCode, 2); - assert.equal(summary?.stderr.length, 400); -}); - -test('summarizeCommandAttemptFailures keeps every attempt in order', () => { - const summaries = summarizeCommandAttemptFailures([ - { args: ['first'], stdout: '', stderr: 'a', exitCode: 1 }, - { args: ['second'], stdout: '', stderr: 'b', exitCode: 9 }, - ]); - - assert.deepEqual( - summaries.map(({ args, exitCode }) => `${args}:${exitCode}`), - ['first:1', 'second:9'], - ); -}); diff --git a/packages/platform-android/src/settings-parsing.ts b/packages/platform-android/src/settings-parsing.ts deleted file mode 100644 index 76f0c4f355..0000000000 --- a/packages/platform-android/src/settings-parsing.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { AppError } from '@agent-device/kernel/errors'; - -export type AppearanceAction = 'light' | 'dark' | 'toggle'; - -export function parseAppearanceAction(state: string): AppearanceAction { - const normalized = state.trim().toLowerCase(); - if (normalized === 'light') return 'light'; - if (normalized === 'dark') return 'dark'; - if (normalized === 'toggle') return 'toggle'; - throw new AppError('INVALID_ARGS', `Invalid appearance state: ${state}. Use light|dark|toggle.`); -} - -export function parseSettingState(state: string): boolean { - const normalized = state.toLowerCase(); - if (normalized === 'on' || normalized === 'true' || normalized === '1') return true; - if (normalized === 'off' || normalized === 'false' || normalized === '0') return false; - throw new AppError('INVALID_ARGS', `Invalid setting state: ${state}`); -} - -export type CommandAttemptFailure = { - args: readonly string[]; - stdout: string; - stderr: string; - exitCode: number; -}; - -export function summarizeCommandAttemptFailures( - failures: CommandAttemptFailure[], -): Array<{ args: string; exitCode: number; stderr: string }> { - return failures.map((failure) => ({ - args: failure.args.join(' '), - exitCode: failure.exitCode, - stderr: failure.stderr.slice(0, 400), - })); -} diff --git a/packages/platform-android/src/settings.ts b/packages/platform-android/src/settings.ts index 7789d8bc4c..1ca822f2d7 100644 --- a/packages/platform-android/src/settings.ts +++ b/packages/platform-android/src/settings.ts @@ -1,8 +1,14 @@ -import { AppError } from '@agent-device/kernel/errors'; +import { + AppError, + summarizeCommandAttemptFailures, + type CommandAttemptFailure, +} from '@agent-device/kernel/errors'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { deviceShellArgv } from '@agent-device/kernel/device-shell'; import { requireLocationCoordinates } from '@agent-device/kernel/location-coordinates'; import { + parseAppearanceAction, + parseSettingState, parseTextSizeCategory, textSizeSettingPayload, TEXT_SIZE_CATEGORIES, @@ -11,12 +17,6 @@ import { type SettingOptions, type TextSizeSettingPayload, } from '@agent-device/contracts/settings'; -import { - parseAppearanceAction, - parseSettingState, - summarizeCommandAttemptFailures, - type CommandAttemptFailure, -} from './settings-parsing.ts'; import { runAndroidAdb, runAndroidShell } from './adb.ts'; import { setAndroidAirplaneMode } from './settings-airplane.ts'; import { androidAdbResultError } from './adb-executor.ts'; diff --git a/packages/platform-apple/src/core/__tests__/settings-parsing.test.ts b/packages/platform-apple/src/core/__tests__/settings-parsing.test.ts deleted file mode 100644 index a76b2bd2dc..0000000000 --- a/packages/platform-apple/src/core/__tests__/settings-parsing.test.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { test } from 'vitest'; -import assert from 'node:assert/strict'; -import { - parseAppearanceAction, - parseSettingState, - summarizeCommandAttemptFailures, -} from '../settings-parsing.ts'; -import { assertThrowsAppError } from '../../__tests__/app-error.ts'; - -test('parseAppearanceAction accepts the three actions in any casing or padding', () => { - assert.equal(parseAppearanceAction('light'), 'light'); - assert.equal(parseAppearanceAction('DARK'), 'dark'); - assert.equal(parseAppearanceAction(' Toggle '), 'toggle'); -}); - -test('parseAppearanceAction rejects anything else and names the accepted set', () => { - assertThrowsAppError(() => parseAppearanceAction('bright'), { code: 'INVALID_ARGS' }); - assertThrowsAppError(() => parseAppearanceAction(''), { code: 'INVALID_ARGS' }); -}); - -test('parseSettingState reads every on and off spelling', () => { - for (const on of ['on', 'ON', 'true', '1']) assert.equal(parseSettingState(on), true); - for (const off of ['off', 'OFF', 'false', '0']) assert.equal(parseSettingState(off), false); -}); - -test('parseSettingState rejects a state that is neither', () => { - assertThrowsAppError(() => parseSettingState('maybe'), { code: 'INVALID_ARGS' }); - assertThrowsAppError(() => parseSettingState('2'), { code: 'INVALID_ARGS' }); -}); - -test('summarizeCommandAttemptFailures joins args and truncates stderr to its budget', () => { - const [summary] = summarizeCommandAttemptFailures([ - { args: ['shell', 'cmd', 'fingerprint'], stdout: 'out', stderr: 'x'.repeat(500), exitCode: 2 }, - ]); - - assert.equal(summary?.args, 'shell cmd fingerprint'); - assert.equal(summary?.exitCode, 2); - assert.equal(summary?.stderr.length, 400); -}); - -test('summarizeCommandAttemptFailures keeps every attempt in order', () => { - const summaries = summarizeCommandAttemptFailures([ - { args: ['first'], stdout: '', stderr: 'a', exitCode: 1 }, - { args: ['second'], stdout: '', stderr: 'b', exitCode: 9 }, - ]); - - assert.deepEqual( - summaries.map(({ args, exitCode }) => `${args}:${exitCode}`), - ['first:1', 'second:9'], - ); -}); diff --git a/packages/platform-apple/src/core/app-settings.ts b/packages/platform-apple/src/core/app-settings.ts index b2d1d25896..b06247e23b 100644 --- a/packages/platform-apple/src/core/app-settings.ts +++ b/packages/platform-apple/src/core/app-settings.ts @@ -1,24 +1,24 @@ import { getUnsupportedMacOsSettingMessage, type MobilePermissionTarget, + parseAppearanceAction, parsePermissionAction, parsePermissionTarget, + parseSettingState, type ReadableSetting, type ReadSettingResult, type SettingOptions, } from '@agent-device/contracts/settings'; import { isIosFamily, isMacOs, type DeviceInfo } from '@agent-device/kernel/device'; -import { AppError } from '@agent-device/kernel/errors'; +import { + AppError, + summarizeCommandAttemptFailures, + type CommandAttemptFailure, +} from '@agent-device/kernel/errors'; import { readHostDirectory, removeHostPath } from '@agent-device/host-kit/host-file'; import path from 'node:path'; import { requireExecSuccess } from '@agent-device/host-kit/command'; import { requireLocationCoordinates } from '@agent-device/kernel/location-coordinates'; -import { - parseAppearanceAction, - parseSettingState, - summarizeCommandAttemptFailures, - type CommandAttemptFailure, -} from './settings-parsing.ts'; import { setMacOsAppearance } from '../os/macos/apps.ts'; import { runMacOsPermissionAction, type MacOsPermissionTarget } from '../os/macos/helper.ts'; import { closeIosApp } from './app-launch.ts'; diff --git a/packages/platform-apple/src/core/settings-parsing.ts b/packages/platform-apple/src/core/settings-parsing.ts deleted file mode 100644 index fe98413908..0000000000 --- a/packages/platform-apple/src/core/settings-parsing.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { AppError } from '@agent-device/kernel/errors'; - -export type AppearanceAction = 'light' | 'dark' | 'toggle'; - -// fallow-ignore-next-line code-duplication -export function parseAppearanceAction(state: string): AppearanceAction { - const normalized = state.trim().toLowerCase(); - if (normalized === 'light') return 'light'; - if (normalized === 'dark') return 'dark'; - if (normalized === 'toggle') return 'toggle'; - throw new AppError('INVALID_ARGS', `Invalid appearance state: ${state}. Use light|dark|toggle.`); -} - -// fallow-ignore-next-line code-duplication -export function parseSettingState(state: string): boolean { - const normalized = state.toLowerCase(); - if (normalized === 'on' || normalized === 'true' || normalized === '1') return true; - if (normalized === 'off' || normalized === 'false' || normalized === '0') return false; - throw new AppError('INVALID_ARGS', `Invalid setting state: ${state}`); -} - -// fallow-ignore-next-line code-duplication -export type CommandAttemptFailure = { - args: readonly string[]; - stdout: string; - stderr: string; - exitCode: number; -}; - -// fallow-ignore-next-line code-duplication -export function summarizeCommandAttemptFailures( - failures: CommandAttemptFailure[], -): Array<{ args: string; exitCode: number; stderr: string }> { - return failures.map((failure) => ({ - args: failure.args.join(' '), - exitCode: failure.exitCode, - stderr: failure.stderr.slice(0, 400), - })); -} diff --git a/packages/platform-apple/src/os/macos/apps.ts b/packages/platform-apple/src/os/macos/apps.ts index d40b38de07..ec44a23c1f 100644 --- a/packages/platform-apple/src/os/macos/apps.ts +++ b/packages/platform-apple/src/os/macos/apps.ts @@ -1,9 +1,9 @@ import type { AppsFilter } from '@agent-device/contracts/device'; import { isDeepLinkTarget } from '@agent-device/contracts/command'; +import { parseAppearanceAction } from '@agent-device/contracts/settings'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { AppError } from '@agent-device/kernel/errors'; import { setTimeout as sleep } from 'node:timers/promises'; -import { parseAppearanceAction } from '../../core/settings-parsing.ts'; import { createAppResolutionCache, type AppResolutionCacheScope, diff --git a/src/commands/capture/settings.ts b/src/commands/capture/settings.ts index c4c098f17f..1fe2997d3b 100644 --- a/src/commands/capture/settings.ts +++ b/src/commands/capture/settings.ts @@ -1,6 +1,7 @@ import { PUBLIC_COMMANDS } from '@agent-device/command-registry/catalog'; import type { SettingsUpdateOptions } from '@agent-device/contracts/client'; import { + APPEARANCE_ACTIONS, MACOS_PERMISSION_TARGETS, MOBILE_PERMISSION_TARGETS, parseTextSizeCategory, @@ -172,7 +173,7 @@ type AppearanceState = Extract const ON_OFF_SETTINGS = setOf('wifi', 'airplane', 'location', 'animations'); const ON_OFF_STATES = setOf('on', 'off'); -const APPEARANCE_STATES = setOf('light', 'dark', 'toggle'); +const APPEARANCE_STATES = setOf(...APPEARANCE_ACTIONS); const BIOMETRIC_SETTINGS = setOf('faceid', 'touchid'); const BIOMETRIC_STATES = setOf('match', 'nonmatch', 'enroll', 'unenroll'); const FINGERPRINT_STATES = setOf('match', 'nonmatch');