refactor(settings): one appearance and state parser for every owner

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<PermissionTarget>();
   });
 });
+
+// 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<AppearanceAction>().toEqualTypeOf<
+      Extract<SettingsUpdateOptions, { setting: 'appearance' }>['state']
+    >();
+  });
+});

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 <setting> <state>` 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 <setting>` 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 = '<wifi|airplane|location> <on|off>';
 const SETTINGS_LOCATION_SET_USAGE = 'location set <lat> <lon>';
 const SETTINGS_ANIMATIONS_USAGE = 'animations <on|off>';
-const SETTINGS_APPEARANCE_USAGE = 'appearance <light|dark|toggle>';
+const SETTINGS_APPEARANCE_USAGE = `appearance <${APPEARANCE_ACTIONS.join('|')}>`;
 const SETTINGS_FACEID_USAGE = 'faceid <match|nonmatch|enroll|unenroll>';
 const SETTINGS_TOUCHID_USAGE = 'touchid <match|nonmatch|enroll|unenroll>';
 const SETTINGS_FINGERPRINT_USAGE = 'fingerprint <match|nonmatch>';
@@ -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 <setting> <state>` 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}`);
+}

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'],
+  );
+});

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) {

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';