Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions src/commands/cli-grammar/common.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import { describe, expect, test } from 'vitest';
import type { CliFlags } from '@agent-device/contracts/command';
import { commonInputFromFlags, selectionOptionsFromFlags } from './common.ts';

function flags(overrides: Partial<CliFlags> = {}): CliFlags {
return overrides as CliFlags;
}

// Every flag either reader function is documented to read (see the `commonInputFromFlags` and
// `selectionOptionsFromFlags` doc comments in ./common.ts).
const ALL_COMMON_FLAGS: Partial<CliFlags> = {
noRecord: true,
session: 'my-session',
platform: 'ios',
target: 'mobile',
device: 'iPhone 15',
udid: 'ABCD-1234',
serial: 'emulator-5554',
iosSimulatorDeviceSet: '/tmp/device-set',
iosXctestrunFile: '/tmp/run.xctestrun',
iosXctestDerivedDataPath: '/tmp/derived-data',
iosXctestEnvDir: '/tmp/env-dir',
androidDeviceAllowlist: 'emulator-5554,emulator-5556',
};

// Common-input-fields.ts rows that read a real `CliFlags` key but deliberately join neither
// `flagIn` projection today (operator/env-sourced). Setting these too, on top of
// `ALL_COMMON_FLAGS`, means a row that wrongly gained `flagIn: ['input']` would surface a
// concrete value here rather than being silently dropped by `commonInputFromFlags`'s
// `compactRecord` — over-inclusion `ALL_COMMON_FLAGS` alone cannot catch, since an unset flag
// reads as `undefined` and gets compacted away either way.
const NON_PROJECTED_COMMON_FLAGS: Partial<CliFlags> = {
daemonBaseUrl: 'https://daemon.example',
daemonAuthToken: 'token-value',
tenant: 'tenant-value',
runId: 'run-value',
leaseId: 'lease-value',
};

describe('commonInputFromFlags', () => {
test('projects every common flag into the reader-input shape', () => {
expect(
commonInputFromFlags(flags({ ...ALL_COMMON_FLAGS, ...NON_PROJECTED_COMMON_FLAGS })),
).toStrictEqual({
noRecord: true,
session: 'my-session',
platform: 'ios',
deviceTarget: 'mobile',
device: 'iPhone 15',
udid: 'ABCD-1234',
serial: 'emulator-5554',
iosSimulatorDeviceSet: '/tmp/device-set',
iosXctestrunFile: '/tmp/run.xctestrun',
iosXctestDerivedDataPath: '/tmp/derived-data',
iosXctestEnvDir: '/tmp/env-dir',
androidDeviceAllowlist: 'emulator-5554,emulator-5556',
});
});

test('drops every key when no common flag is set', () => {
expect(commonInputFromFlags(flags())).toStrictEqual({});
});
});

describe('selectionOptionsFromFlags', () => {
test('projects the selection-options subset, keeping the raw target spelling', () => {
expect(
selectionOptionsFromFlags(flags({ ...ALL_COMMON_FLAGS, ...NON_PROJECTED_COMMON_FLAGS })),
).toStrictEqual({
noRecord: true,
platform: 'ios',
target: 'mobile',
device: 'iPhone 15',
udid: 'ABCD-1234',
serial: 'emulator-5554',
iosSimulatorDeviceSet: '/tmp/device-set',
androidDeviceAllowlist: 'emulator-5554,emulator-5556',
});
});

test('keeps every key present (undefined, not dropped) when no common flag is set', () => {
expect(selectionOptionsFromFlags(flags())).toStrictEqual({
noRecord: undefined,
platform: undefined,
target: undefined,
device: undefined,
udid: undefined,
serial: undefined,
iosSimulatorDeviceSet: undefined,
androidDeviceAllowlist: undefined,
});
});
});
74 changes: 30 additions & 44 deletions src/commands/cli-grammar/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
SELECTOR_EXPRESSION_REQUIRED_MESSAGE,
splitSelectorFromArgs,
} from '@agent-device/selectors';
import { commonFlagProjection } from '../common-input-fields.ts';
import type { SelectorSnapshotInput } from '../command-input.ts';
import { compactRecord } from '../input-readers.ts';
import type {
Expand Down Expand Up @@ -56,34 +57,28 @@ function readDeviceTarget(value: unknown): InternalRequestOptions['target'] | un
return value === 'mobile' || value === 'tv' || value === 'desktop' ? value : undefined;
}

/**
* The reader-input shape: every common key whose `commands/common-input-fields.ts`
* row lists `input` in `flagIn`, keyed by its own row name (`deviceTarget`, not
* the CLI's `--target` spelling). `--no-record` is one such row
* (`COMMON_COMMAND_SUPPORTED_FLAG_KEYS`) — it is accepted on, and meaningful
* for, every recordable command, and joining the table's `flagIn` is what
* makes it ride this projection and `selectionOptionsFromFlags` below without
* either reader restating it (#1304/#1305 fixed only the reader layer, so the
* flag still never reached the daemon). One row now declares membership for
* both projections instead of two hand-written lists to keep in sync; the
* `flagKey` a row binds to is checked against `CliFlags` at compile time, but
* a *new* common row that omits `flagIn` still joins neither projection
* without a compile error — the #1304/#1305 failure mode is narrowed, not
* made impossible.
*
* `--record` deliberately does NOT join `flagIn`: it is scoped to the
* observation-only commands the repair-segment exclusion can drop (ADR 0012
* decision 6 amendment), so it stays on the narrow
* `observationRecordInputFromFlags` seam below.
*/
export function commonInputFromFlags(flags: CliFlags): Record<string, unknown> {
return compactRecord({
// `--no-record` is a COMMON flag (`COMMON_COMMAND_SUPPORTED_FLAG_KEYS`): it
// is accepted on, and meaningful for, every recordable command. It rides
// the common seam every reader already spreads, so a reader cannot forget
// it and a new reader inherits it for free. The three seams it must survive
// are this one, `readCommonInput`, and `commonToClientOptions`
// (`commands/common-input-fields.ts`) — a drop at any one of them silently
// disables the flag (#1304/#1305 fixed only the reader layer, so the flag
// still never reached the daemon).
//
// `--record` deliberately does NOT ride here: it is scoped to the
// observation-only commands the repair-segment exclusion can drop
// (ADR 0012 decision 6 amendment), so it stays on the narrow
// `observationRecordInputFromFlags` seam below.
noRecord: flags.noRecord,
session: flags.session,
platform: flags.platform,
deviceTarget: flags.target,
device: flags.device,
udid: flags.udid,
serial: flags.serial,
iosSimulatorDeviceSet: flags.iosSimulatorDeviceSet,
iosXctestrunFile: flags.iosXctestrunFile,
iosXctestDerivedDataPath: flags.iosXctestDerivedDataPath,
iosXctestEnvDir: flags.iosXctestEnvDir,
androidDeviceAllowlist: flags.androidDeviceAllowlist,
});
return compactRecord(commonFlagProjection(flags, 'input'));
}

/**
Expand All @@ -101,25 +96,16 @@ export function observationRecordInputFromFlags(flags: CliFlags): Record<string,
}

/**
* The reader layer has TWO parallel common seams, not one: this builds the
* client-options shape (`target`) for readers that construct a typed Options
* object directly (`is`/`find`/`wait`/`settings`), while `commonInputFromFlags`
* above builds the reader-input shape (`deviceTarget`). They are different
* projections, not duplicates — so `--no-record` has to ride BOTH or the
* readers using this one silently drop it (which is what #1304/#1305's
* per-reader helper was papering over).
* The client-options shape for readers that construct a typed Options object
* directly (`is`/`find`/`wait`/`settings`): every common row that lists
* `selection` in `flagIn`, keyed by `clientKey ?? key` (so `deviceTarget`
* comes out as `target`, matching `commonToClientOptions`'s renaming). This is
* a different projection from `commonInputFromFlags` above, not a duplicate —
* unlike that one it is not compacted, so an unset common flag still appears
* with an `undefined` value.
*/
export function selectionOptionsFromFlags(flags: CliFlags): SelectionOptions {
return {
noRecord: flags.noRecord,
platform: flags.platform,
target: flags.target,
device: flags.device,
udid: flags.udid,
serial: flags.serial,
iosSimulatorDeviceSet: flags.iosSimulatorDeviceSet,
androidDeviceAllowlist: flags.androidDeviceAllowlist,
};
return commonFlagProjection(flags, 'selection') as SelectionOptions;
}

export function selectorSnapshotInputFromFlags(flags: CliFlags): Record<string, unknown> {
Expand Down
7 changes: 5 additions & 2 deletions src/commands/cli-grammar/flag-groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,11 @@ export const REPLAY_FLAGS = flagKeys('replayUpdate', 'replayEnv');
// structured command input, while the table's `cwd` and `debug` are not flags
// and its `deviceTarget` row is spelled `target` here. Deriving this list from
// that table would mean 25 rows carrying no schema, reader, or projection, so
// the two stay separate; the table declares only `envFlagKeys`, where a flag key
// names the environment variable an operator-owned input comes from.
// the two stay separate; the table declares `envFlagKeys` (which environment
// variable an operator-owned input comes from) and `flagKey`/`flagIn` (which
// `CliFlags` property a row reads and which reader projections it joins) —
// this list stays the parser-side axis, naming every flag the CLI grammar
// accepts regardless of whether a common-input-fields row reads it at all.
export const COMMON_COMMAND_SUPPORTED_FLAG_KEYS = flagKeys(
'remoteConfig',
'stateDir',
Expand Down
Loading
Loading