Skip to content

Commit d1b9914

Browse files
authored
refactor(commands): retire the navigation-only type projection (#2294)
* refactor(commands): retire the navigation-only type projection `commands/system/navigation-projection.ts` built the five navigation client methods out of a phantom-typed registry: a `unique symbol` brand carrying Options/Result/required-ness, two conditional types to read them back, and a mapped type keyed on `clientMethod`. Nothing else ever used the concept, so the machinery existed to derive five signatures that fit in five lines. Those five now say what they mean. `BackCommandOptions`, `HomeCommandOptions`, `OrientationCommandOptions`, `AppSwitcherCommandOptions` and `TvRemoteCommandOptions` join their siblings in `packages/contracts/src/client-system.ts`, and `AgentDeviceCommandClient` declares all 14 methods in one object type. `back` keeps the `--settle` triple (#1638), and `orientation`/`tv-remote` keep their required options parameter. The five MCP output schemas move to `mcp/command-output-schemas.ts` beside the other handwritten ones, byte-identical. With the projection gone, `defineExecutableCommand`'s third overload, `ExecutableCommandProjection`, `AnyCommandDefinition.projection`, `ProjectedCommandOutputSchemas`/`projectCommandOutputSchemas` and the family's `clientCommandMethods` table have no users either. Removing the table also removes the `as unknown as` cast the client used to build eight system methods from it; the client now writes all eight out, typed. That closes the `commands/system` -> `client` inversion the client-types header called the one remaining one. Public API: the five method signatures are unchanged (structural comparison of the built `dist/src/index.d.ts` before and after: empty diff). `HomeCommandOptions` is a new published name for the shape `home` already took. Tests seen red before green: - `src/__tests__/client-system-commands.test.ts` (new): wired `home` to the `app-switcher` daemon command, saw it fail, restored. - `src/mcp/__tests__/command-tools.test.ts`: dropped `durationMs` from the inlined `tv-remote` schema, saw the dispatch-shape assertion fail, restored. - `src/commands/system/index.test.ts`: made `home`'s options parameter required, saw `expectTypeOf` fail under `pnpm typecheck`, restored. * test(mcp): pin the closed top-level shape of the navigation output schemas Retiring the projection replaced an identity assert (`schema === projection.outputSchema`) with a deep-equal over properties/required, which no longer rejected an extra top-level key such as a stray `description` or `additionalProperties`. The loop now also asserts the key set is exactly type/properties/required, so the closed shape is pinned by a test again rather than by object identity. Seen red once by giving the `app-switcher` schema a description argument, which adds a top-level `description` key: the new assert failed with `+ "description"`. Green after removing it. The `deriveSettleObservationSchemas` docstring cited that deleted identity assert as the reason for copying. The press/click shared-object half is the real reason and is all that remains. * chore(gates): drop the retired projection from the R6 inversion rationale The R6 baseline numbers are unchanged (5 inversions, commands -> client still 3): retiring the projection removed a client -> commands edge, which the ratchet does not count. What changed is the ARGUMENT next to those numbers. The commands/mcp -> client bullet justified itself with a zone-level cycle (client-types.ts imported ProjectedNavigationCommandClient back out of commands/system/); that cycle no longer exists, so the bullet now rests only on the port argument that was always the second half of it. docs/dependency-graph-findings.md §0/§0b/§1 carried the same claim and the same 'move the navigation-projection types out of commands/' follow-up, now recorded as answered by deletion. The blocked-shapes table in §1 now reads eight-at-the-time / three-still-blocked, matching the struck navigation row directly under it. * test(mcp): split the navigation schema tests out of command-tools.test.ts
1 parent 54df6f3 commit d1b9914

16 files changed

Lines changed: 300 additions & 384 deletions

docs/dependency-graph-findings.md

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,12 @@ narrow name replaced both.
114114
**The remaining 5 are positions, not debt** — each for a mechanical reason, not an appeal to an ADR:
115115

116116
- **4 × `AgentDeviceClient`** (`commands/command-contract.ts`, `commands/command-surface.ts`,
117-
`commands/family/types.ts`, `mcp/command-tools.ts`). The facade cannot move below `commands/`
118-
because it is *built from* the command surface: `client/client-types.ts` imports
119-
`ProjectedNavigationCommandClient` from `commands/system/navigation-projection.ts`. That is a real
120-
zone-level type cycle, and breaking it means deciding where the projection registry belongsa
121-
design call, not a file move. A narrower port does not exist either: 4 files *name* the facade,
122-
but 26 call sites use methods across 13 of its namespaces, so any port would re-declare it.
117+
`commands/family/types.ts`, `mcp/command-tools.ts`). The zone-level type cycle this bullet used to
118+
cite is gone: retiring the navigation projection left `client/client-types.ts` with no import from
119+
`commands/` at all, and the facade now declares its 14 command methods directly. What keeps the
120+
facade above `commands/` is the remaining argument: a narrower port does not exist4 files
121+
*name* the facade, but 26 call sites use methods across 13 of its namespaces, so any port would
122+
re-declare it.
123123
- **1 × `DaemonCommandRoute`** (`commands/command-explain.ts`). The union lives in core so
124124
descriptors can name a route without importing the daemon, and the handler table covers it with
125125
`satisfies Record<DaemonCommandRoute, …>`. `command-explain.ts` still type-imports the re-export
@@ -183,9 +183,10 @@ duplicate the public API shape — a second source of truth for it — or derive
183183
carry the same dependency.
184184

185185
Those four files are therefore the minimum number of naming sites, not an accident: they are the
186-
choke point. Accepted as a position, argued at `TYPE_INVERSION_BASELINE`. The remaining option is
187-
the one that was always the real question — whether `NAVIGATION_COMMAND_PROJECTIONS` belongs in
188-
`commands/` — and that is a design decision about the command surface, not a dependency cleanup.
186+
choke point. Accepted as a position, argued at `TYPE_INVERSION_BASELINE`. The option this section
187+
used to hold open — moving `NAVIGATION_COMMAND_PROJECTIONS` out of `commands/` — was answered by
188+
deleting it: five direct signatures replaced the registry, so there is no longer a projection
189+
registry whose home is in question.
189190

190191
## 1. The two remaining type-inversion clusters
191192

@@ -201,13 +202,13 @@ changed). `index.d.ts` in fact got *smaller* — 1,726 → 1,682 lines — becau
201202
`main` duplicated into it (the Metro option/result shapes, `ScrollInputDirection`) now resolve
202203
through a shared chunk once the vocabulary sits below both its consumers.
203204

204-
The mutual coupling this section already warned about is what set the floor. Eight shapes could NOT
205-
move down, because each is stated in terms of a HIGHER-ranked zone:
205+
The mutual coupling this section already warned about is what set the floor. Eight shapes could not
206+
move down at the time; three still cannot, because each is stated in terms of a HIGHER-ranked zone:
206207

207208
| Shape(s) | Blocked by |
208209
|---|---|
209210
| `ScrollOptions` | `ScrollInputDirection` (`commands/interaction/runtime/gestures.ts`) |
210-
| `BackCommandOptions`, `OrientationCommandOptions`, `AppSwitcherCommandOptions`, `TvRemoteCommandOptions`, `AgentDeviceCommandClient` | `NavigationCommandOptions` / `ProjectedNavigationCommandClient` (`commands/system/navigation-projection.ts`) |
211+
| ~~`BackCommandOptions`, `OrientationCommandOptions`, `AppSwitcherCommandOptions`, `TvRemoteCommandOptions`, `AgentDeviceCommandClient`~~ | ~~`NavigationCommandOptions` / `ProjectedNavigationCommandClient` (`commands/system/navigation-projection.ts`)~~ — unblocked: the projection was retired, the four Options types (plus a new `HomeCommandOptions`) now live in `contracts/client-system.ts`, and the facade declares its methods directly |
211212
| `MetroPrepareResult`, `MetroReloadResult` | `PrepareMetroRuntimeResult` / `ReloadMetroResult` (`metro/client-metro.ts`) |
212213

213214
Declaring those in `contracts/` would have traded 28 `commands -> client` inversions for
@@ -226,11 +227,11 @@ Two keystone moves made the other 84 shapes movable, and both are worth noting a
226227
`SessionRuntimeHints` — the same type, three zones lower.
227228

228229
**Remaining `commands -> client` (5) needs the upstream declarations to come down first**: move
229-
`ScrollInputDirection` and the navigation-projection types out of `commands/`, and the Metro
230-
prepare/reload result payloads out of `metro/`. Each is small; the sequencing is the point. The
231-
`mcp -> client` edge is different in kind — it is the `AgentDeviceClient` facade itself, i.e. the
232-
question of whether a command surface should know the client type. That is a design decision, not a
233-
misplaced declaration.
230+
`ScrollInputDirection` out of `commands/`, and the Metro prepare/reload result payloads out of
231+
`metro/`. The navigation-projection leg of this list is done. Each is small; the sequencing is the
232+
point. The `mcp -> client` edge is different in kind — it is the `AgentDeviceClient` facade itself,
233+
i.e. the question of whether a command surface should know the client type. That is a design
234+
decision, not a misplaced declaration.
234235

235236
**5 + 1 edges → `daemon/daemon-command-registry.ts` and `daemon/types.ts`.** `core`'s descriptor
236237
registry composes the ADR 0003 daemon facet, whose shape the daemon declares. ADR 0003's
@@ -460,8 +461,9 @@ implementation-pattern checks so reintroduction fails closed.
460461
completed by #1435. Eliminate the four remaining external production importers with
461462
caller-specific public contracts or daemon-owned adapters; keep `DaemonRequest` private.
462463
2. ~~**Split `client/client-types.ts`** (§1).~~ Done — 42 → 18 total inversions. The follow-up is
463-
the upstream moves that unblock the last 5 (§1): `ScrollInputDirection` and the
464-
navigation-projection types out of `commands/`, Metro result payloads out of `metro/`.
464+
the upstream moves that unblock the last 5 (§1): `ScrollInputDirection` out of `commands/`,
465+
Metro result payloads out of `metro/`. The navigation-projection move is done — the projection
466+
was retired rather than relocated.
465467
3. **Retire platform branches into plugin facets** (§5b), highest-count files first.
466468
4. **Share the remaining duplicated validators** (§6), following the `checkIsArgs` shape.
467469
5. Optional: give `daemon/handlers/` the directory structure its filenames already imply (§5).

packages/contracts/src/client-system.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
// The public API vocabulary for the system and diagnostic commands (wait, alert, keyboard, clipboard, doctor…).
22

33
import type { AlertAction } from './alert-contract.ts';
4+
import type { BackMode } from './back-mode.ts';
45
import type { SelectorSnapshotCommandOptions } from './client-capture.ts';
56
import type { DeviceCommandBaseOptions } from './client-connection.ts';
7+
import type { SettleCommandOptions } from './client-gesture.ts';
8+
import type { DeviceRotation } from './device-rotation.ts';
9+
import type { TvRemoteButton } from './tv-remote.ts';
610

711
export type WaitCommandTarget =
812
| {
@@ -75,6 +79,24 @@ export type AlertCommandOptions = DeviceCommandBaseOptions & {
7579

7680
export type AppStateCommandOptions = DeviceCommandBaseOptions;
7781

82+
/** #1638: `back` carries the shared `--settle` triple, and its result may carry the settled diff. */
83+
export type BackCommandOptions = DeviceCommandBaseOptions & {
84+
mode?: BackMode;
85+
} & SettleCommandOptions;
86+
87+
export type HomeCommandOptions = DeviceCommandBaseOptions;
88+
89+
export type OrientationCommandOptions = DeviceCommandBaseOptions & {
90+
orientation: DeviceRotation;
91+
};
92+
93+
export type AppSwitcherCommandOptions = DeviceCommandBaseOptions;
94+
95+
export type TvRemoteCommandOptions = DeviceCommandBaseOptions & {
96+
button: TvRemoteButton;
97+
durationMs?: number;
98+
};
99+
78100
export type KeyboardCommandOptions = DeviceCommandBaseOptions & {
79101
action?: 'status' | 'dismiss' | 'enter' | 'return';
80102
};

packages/contracts/src/facades/client.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,16 @@ export type { PermissionTarget, SettingsUpdateOptions } from '../client-settings
112112
export type {
113113
AlertCommandOptions,
114114
AppStateCommandOptions,
115+
AppSwitcherCommandOptions,
116+
BackCommandOptions,
115117
ClipboardCommandOptions,
116118
DoctorCommandOptions,
119+
HomeCommandOptions,
117120
KeyboardCommandOptions,
121+
OrientationCommandOptions,
118122
PrepareCommandOptions,
119123
ReactNativeCommandOptions,
124+
TvRemoteCommandOptions,
120125
ViewportCommandOptions,
121126
WaitCommandOptions,
122127
WaitCommandTarget,

scripts/layering/check.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -253,13 +253,12 @@ function checkBackEdges(edges: readonly ResolvedImportEdge[]): LayeringViolation
253253
// architectural position rather than a misplaced declaration:
254254
//
255255
// commands/mcp -> client (4) `AgentDeviceClient`, used as an opaque handle ("the client this
256-
// command runs against"). It cannot move below `commands/` because
257-
// the facade is BUILT from the command surface's own projection
258-
// registry: AgentDeviceClient -> AgentDeviceCommandClient ->
259-
// ProjectedNavigationCommandClient -> NAVIGATION_COMMAND_PROJECTIONS
260-
// in commands/system/. That is a genuine zone-level cycle, and
261-
// breaking it means deciding where the projection registry belongs —
262-
// a design call, not a file move. R5 is zero here: nothing imports
256+
// command runs against"). The facade no longer reaches back into
257+
// commands/ — the navigation projection it was once built from is
258+
// retired — so this is no longer a zone-level cycle, just a port
259+
// that would have to cover the whole facade: 4 files NAME it, but 26
260+
// call sites use methods across 13 of its namespaces, so any port
261+
// would re-declare the public API. R5 is zero here: nothing imports
263262
// the client at runtime, only its type.
264263
//
265264
// commands -> daemon-server (1) `DaemonCommandRoute` is declared in core so descriptors can
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import assert from 'node:assert/strict';
2+
import { test } from 'vitest';
3+
import { createAgentDeviceClient } from '../agent-device-client.ts';
4+
import { createTransport } from './client-transport-fixture.ts';
5+
6+
// Every system command the Node client exposes as its own method, with the
7+
// daemon command name and positionals that method must produce. The client
8+
// declares these one by one, so this table is what catches a method wired to
9+
// the wrong command or losing an argument on the way to the daemon.
10+
const SYSTEM_COMMAND_CALLS: readonly {
11+
method: string;
12+
invoke: (client: ReturnType<typeof createAgentDeviceClient>) => Promise<unknown>;
13+
command: string;
14+
positionals: string[];
15+
flags?: Record<string, unknown>;
16+
}[] = [
17+
{
18+
method: 'appState',
19+
invoke: async (client) => await client.command.appState(),
20+
command: 'appstate',
21+
positionals: [],
22+
},
23+
{
24+
method: 'back',
25+
invoke: async (client) => await client.command.back({ mode: 'system' }),
26+
command: 'back',
27+
positionals: [],
28+
flags: { backMode: 'system' },
29+
},
30+
{
31+
method: 'home',
32+
invoke: async (client) => await client.command.home(),
33+
command: 'home',
34+
positionals: [],
35+
},
36+
{
37+
method: 'orientation',
38+
invoke: async (client) => await client.command.orientation({ orientation: 'landscape-left' }),
39+
command: 'orientation',
40+
positionals: ['landscape-left'],
41+
},
42+
{
43+
method: 'appSwitcher',
44+
invoke: async (client) => await client.command.appSwitcher(),
45+
command: 'app-switcher',
46+
positionals: [],
47+
},
48+
{
49+
method: 'keyboard',
50+
invoke: async (client) => await client.command.keyboard({ action: 'dismiss' }),
51+
command: 'keyboard',
52+
positionals: ['dismiss'],
53+
},
54+
{
55+
method: 'clipboard',
56+
invoke: async (client) => await client.command.clipboard({ action: 'write', text: 'hi' }),
57+
command: 'clipboard',
58+
positionals: ['write', 'hi'],
59+
},
60+
{
61+
method: 'tvRemote',
62+
invoke: async (client) => await client.command.tvRemote({ button: 'select' }),
63+
command: 'tv-remote',
64+
positionals: ['select'],
65+
},
66+
];
67+
68+
for (const call of SYSTEM_COMMAND_CALLS) {
69+
test(`client.command.${call.method} sends the ${call.command} daemon command`, async () => {
70+
const setup = createTransport(async () => ({ ok: true, data: {} }));
71+
const client = createAgentDeviceClient(setup.config, { transport: setup.transport });
72+
73+
await call.invoke(client);
74+
75+
assert.equal(setup.calls.length, 1);
76+
assert.equal(setup.calls[0]?.command, call.command);
77+
assert.deepEqual(setup.calls[0]?.positionals, call.positionals);
78+
for (const [flag, value] of Object.entries(call.flags ?? {})) {
79+
assert.deepEqual(setup.calls[0]?.flags?.[flag], value);
80+
}
81+
});
82+
}

src/agent-device-client.ts

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -54,19 +54,13 @@ import {
5454
readSnapshotNodes,
5555
resolveSessionName,
5656
} from './client/client-normalizers.ts';
57-
import type {
58-
AgentDeviceClient,
59-
AgentDeviceCommandClient,
60-
MetroPrepareResult,
61-
} from './client/client-types.ts';
57+
import type { AgentDeviceClient, MetroPrepareResult } from './client/client-types.ts';
6258
import { INTERNAL_COMMANDS } from './command-catalog.ts';
6359
import { buildRequestFlags } from './commands/command-flags.ts';
6460
import {
6561
prepareDaemonCommandRequest,
6662
type DaemonCommandName,
6763
} from './commands/command-projection.ts';
68-
import { systemCommandFamily } from './commands/system/index.ts';
69-
import type { ProjectedNavigationCommandClient } from './commands/system/navigation-projection.ts';
7064
import type { CommandResult } from './core/command-descriptor/command-result.ts';
7165
import { sendToDaemon } from './daemon/client/daemon-client.ts';
7266
import { resolveDaemonPaths } from './daemon/config.ts';
@@ -81,9 +75,6 @@ import { isRecord } from '@agent-device/kernel/record';
8175
import { createLeaseClient } from './client/lease-client.ts';
8276
import { normalizeScreenshotCaptureResult } from './client/screenshot-result.ts';
8377

84-
type ProjectedSystemCommandClient = ProjectedNavigationCommandClient<InternalRequestOptions> &
85-
Pick<AgentDeviceCommandClient, 'appState' | 'keyboard' | 'clipboard'>;
86-
8778
export function createAgentDeviceClient(
8879
config: AgentDeviceClientConfig = {},
8980
deps: { transport?: AgentDeviceDaemonTransport } = {},
@@ -142,13 +133,25 @@ export function createAgentDeviceClient(
142133

143134
const resolveRequestSession = (options: InternalRequestOptions = {}) =>
144135
resolveSessionName(mergeClientOptions(config, options).session);
145-
const projectedSystemCommands = buildProjectedSystemCommandClient(executeCommand);
146136

147137
return {
148138
command: {
149139
wait: async (options) => await executeCommand<CommandResult<'wait'>>('wait', options),
150140
alert: async (options = {}) => await executeCommand('alert', options),
151-
...projectedSystemCommands,
141+
appState: async (options = {}) =>
142+
await executeCommand<CommandResult<'appstate'>>('appstate', options),
143+
back: async (options = {}) => await executeCommand<CommandResult<'back'>>('back', options),
144+
home: async (options = {}) => await executeCommand<CommandResult<'home'>>('home', options),
145+
orientation: async (options) =>
146+
await executeCommand<CommandResult<'orientation'>>('orientation', options),
147+
appSwitcher: async (options = {}) =>
148+
await executeCommand<CommandResult<'app-switcher'>>('app-switcher', options),
149+
keyboard: async (options = {}) =>
150+
await executeCommand<CommandResult<'keyboard'>>('keyboard', options),
151+
clipboard: async (options) =>
152+
await executeCommand<CommandResult<'clipboard'>>('clipboard', options),
153+
tvRemote: async (options) =>
154+
await executeCommand<CommandResult<'tv-remote'>>('tv-remote', options),
152155
reactNative: async (options) => await executeCommand('react-native', options),
153156
doctor: async (options = {}) =>
154157
await executeCommand<CommandResult<'doctor'>>('doctor', options),
@@ -545,17 +548,6 @@ function optionalSnapshotResponseFields(
545548
};
546549
}
547550

548-
function buildProjectedSystemCommandClient(
549-
executeCommand: <T>(command: DaemonCommandName, options?: InternalRequestOptions) => Promise<T>,
550-
): ProjectedSystemCommandClient {
551-
const methods: Record<string, (options?: InternalRequestOptions) => Promise<unknown>> = {};
552-
for (const [method, command] of Object.entries(systemCommandFamily.clientCommandMethods ?? {})) {
553-
methods[method] = async (options = {}) =>
554-
await executeCommand<CommandResult<typeof command>>(command as DaemonCommandName, options);
555-
}
556-
return methods as unknown as ProjectedSystemCommandClient;
557-
}
558-
559551
function readObject(value: unknown): Record<string, unknown> | undefined {
560552
return isRecord(value) ? value : undefined;
561553
}

0 commit comments

Comments
 (0)