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
40 changes: 21 additions & 19 deletions docs/dependency-graph-findings.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ narrow name replaced both.
**The remaining 5 are positions, not debt** — each for a mechanical reason, not an appeal to an ADR:

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

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

## 1. The two remaining type-inversion clusters

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

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

| Shape(s) | Blocked by |
|---|---|
| `ScrollOptions` | `ScrollInputDirection` (`commands/interaction/runtime/gestures.ts`) |
| `BackCommandOptions`, `OrientationCommandOptions`, `AppSwitcherCommandOptions`, `TvRemoteCommandOptions`, `AgentDeviceCommandClient` | `NavigationCommandOptions` / `ProjectedNavigationCommandClient` (`commands/system/navigation-projection.ts`) |
| ~~`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 |
| `MetroPrepareResult`, `MetroReloadResult` | `PrepareMetroRuntimeResult` / `ReloadMetroResult` (`metro/client-metro.ts`) |

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

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

**5 + 1 edges → `daemon/daemon-command-registry.ts` and `daemon/types.ts`.** `core`'s descriptor
registry composes the ADR 0003 daemon facet, whose shape the daemon declares. ADR 0003's
Expand Down Expand Up @@ -460,8 +461,9 @@ implementation-pattern checks so reintroduction fails closed.
completed by #1435. Eliminate the four remaining external production importers with
caller-specific public contracts or daemon-owned adapters; keep `DaemonRequest` private.
2. ~~**Split `client/client-types.ts`** (§1).~~ Done — 42 → 18 total inversions. The follow-up is
the upstream moves that unblock the last 5 (§1): `ScrollInputDirection` and the
navigation-projection types out of `commands/`, Metro result payloads out of `metro/`.
the upstream moves that unblock the last 5 (§1): `ScrollInputDirection` out of `commands/`,
Metro result payloads out of `metro/`. The navigation-projection move is done — the projection
was retired rather than relocated.
3. **Retire platform branches into plugin facets** (§5b), highest-count files first.
4. **Share the remaining duplicated validators** (§6), following the `checkIsArgs` shape.
5. Optional: give `daemon/handlers/` the directory structure its filenames already imply (§5).
22 changes: 22 additions & 0 deletions packages/contracts/src/client-system.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
// The public API vocabulary for the system and diagnostic commands (wait, alert, keyboard, clipboard, doctor…).

import type { AlertAction } from './alert-contract.ts';
import type { BackMode } from './back-mode.ts';
import type { SelectorSnapshotCommandOptions } from './client-capture.ts';
import type { DeviceCommandBaseOptions } from './client-connection.ts';
import type { SettleCommandOptions } from './client-gesture.ts';
import type { DeviceRotation } from './device-rotation.ts';
import type { TvRemoteButton } from './tv-remote.ts';

export type WaitCommandTarget =
| {
Expand Down Expand Up @@ -75,6 +79,24 @@ export type AlertCommandOptions = DeviceCommandBaseOptions & {

export type AppStateCommandOptions = DeviceCommandBaseOptions;

/** #1638: `back` carries the shared `--settle` triple, and its result may carry the settled diff. */
export type BackCommandOptions = DeviceCommandBaseOptions & {
mode?: BackMode;
} & SettleCommandOptions;

export type HomeCommandOptions = DeviceCommandBaseOptions;

export type OrientationCommandOptions = DeviceCommandBaseOptions & {
orientation: DeviceRotation;
};

export type AppSwitcherCommandOptions = DeviceCommandBaseOptions;

export type TvRemoteCommandOptions = DeviceCommandBaseOptions & {
button: TvRemoteButton;
durationMs?: number;
};

export type KeyboardCommandOptions = DeviceCommandBaseOptions & {
action?: 'status' | 'dismiss' | 'enter' | 'return';
};
Expand Down
5 changes: 5 additions & 0 deletions packages/contracts/src/facades/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,16 @@ export type { PermissionTarget, SettingsUpdateOptions } from '../client-settings
export type {
AlertCommandOptions,
AppStateCommandOptions,
AppSwitcherCommandOptions,
BackCommandOptions,
ClipboardCommandOptions,
DoctorCommandOptions,
HomeCommandOptions,
KeyboardCommandOptions,
OrientationCommandOptions,
PrepareCommandOptions,
ReactNativeCommandOptions,
TvRemoteCommandOptions,
ViewportCommandOptions,
WaitCommandOptions,
WaitCommandTarget,
Expand Down
13 changes: 6 additions & 7 deletions scripts/layering/check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,13 +253,12 @@ function checkBackEdges(edges: readonly ResolvedImportEdge[]): LayeringViolation
// architectural position rather than a misplaced declaration:
//
// commands/mcp -> client (4) `AgentDeviceClient`, used as an opaque handle ("the client this
// command runs against"). It cannot move below `commands/` because
// the facade is BUILT from the command surface's own projection
// registry: AgentDeviceClient -> AgentDeviceCommandClient ->
// ProjectedNavigationCommandClient -> NAVIGATION_COMMAND_PROJECTIONS
// in commands/system/. That is a genuine zone-level cycle, and
// breaking it means deciding where the projection registry belongs —
// a design call, not a file move. R5 is zero here: nothing imports
// command runs against"). The facade no longer reaches back into
// commands/ — the navigation projection it was once built from is
// retired — so this is no longer a zone-level cycle, just a port
// that would have to cover the whole facade: 4 files NAME it, but 26
// call sites use methods across 13 of its namespaces, so any port
// would re-declare the public API. R5 is zero here: nothing imports
// the client at runtime, only its type.
//
// commands -> daemon-server (1) `DaemonCommandRoute` is declared in core so descriptors can
Expand Down
82 changes: 82 additions & 0 deletions src/__tests__/client-system-commands.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import assert from 'node:assert/strict';
import { test } from 'vitest';
import { createAgentDeviceClient } from '../agent-device-client.ts';
import { createTransport } from './client-transport-fixture.ts';

// Every system command the Node client exposes as its own method, with the
// daemon command name and positionals that method must produce. The client
// declares these one by one, so this table is what catches a method wired to
// the wrong command or losing an argument on the way to the daemon.
const SYSTEM_COMMAND_CALLS: readonly {
method: string;
invoke: (client: ReturnType<typeof createAgentDeviceClient>) => Promise<unknown>;
command: string;
positionals: string[];
flags?: Record<string, unknown>;
}[] = [
{
method: 'appState',
invoke: async (client) => await client.command.appState(),
command: 'appstate',
positionals: [],
},
{
method: 'back',
invoke: async (client) => await client.command.back({ mode: 'system' }),
command: 'back',
positionals: [],
flags: { backMode: 'system' },
},
{
method: 'home',
invoke: async (client) => await client.command.home(),
command: 'home',
positionals: [],
},
{
method: 'orientation',
invoke: async (client) => await client.command.orientation({ orientation: 'landscape-left' }),
command: 'orientation',
positionals: ['landscape-left'],
},
{
method: 'appSwitcher',
invoke: async (client) => await client.command.appSwitcher(),
command: 'app-switcher',
positionals: [],
},
{
method: 'keyboard',
invoke: async (client) => await client.command.keyboard({ action: 'dismiss' }),
command: 'keyboard',
positionals: ['dismiss'],
},
{
method: 'clipboard',
invoke: async (client) => await client.command.clipboard({ action: 'write', text: 'hi' }),
command: 'clipboard',
positionals: ['write', 'hi'],
},
{
method: 'tvRemote',
invoke: async (client) => await client.command.tvRemote({ button: 'select' }),
command: 'tv-remote',
positionals: ['select'],
},
];

for (const call of SYSTEM_COMMAND_CALLS) {
test(`client.command.${call.method} sends the ${call.command} daemon command`, async () => {
const setup = createTransport(async () => ({ ok: true, data: {} }));
const client = createAgentDeviceClient(setup.config, { transport: setup.transport });

await call.invoke(client);

assert.equal(setup.calls.length, 1);
assert.equal(setup.calls[0]?.command, call.command);
assert.deepEqual(setup.calls[0]?.positionals, call.positionals);
for (const [flag, value] of Object.entries(call.flags ?? {})) {
assert.deepEqual(setup.calls[0]?.flags?.[flag], value);
}
});
}
38 changes: 15 additions & 23 deletions src/agent-device-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,13 @@ import {
readSnapshotNodes,
resolveSessionName,
} from './client/client-normalizers.ts';
import type {
AgentDeviceClient,
AgentDeviceCommandClient,
MetroPrepareResult,
} from './client/client-types.ts';
import type { AgentDeviceClient, MetroPrepareResult } from './client/client-types.ts';
import { INTERNAL_COMMANDS } from './command-catalog.ts';
import { buildRequestFlags } from './commands/command-flags.ts';
import {
prepareDaemonCommandRequest,
type DaemonCommandName,
} from './commands/command-projection.ts';
import { systemCommandFamily } from './commands/system/index.ts';
import type { ProjectedNavigationCommandClient } from './commands/system/navigation-projection.ts';
import type { CommandResult } from './core/command-descriptor/command-result.ts';
import { sendToDaemon } from './daemon/client/daemon-client.ts';
import { resolveDaemonPaths } from './daemon/config.ts';
Expand All @@ -81,9 +75,6 @@ import { isRecord } from '@agent-device/kernel/record';
import { createLeaseClient } from './client/lease-client.ts';
import { normalizeScreenshotCaptureResult } from './client/screenshot-result.ts';

type ProjectedSystemCommandClient = ProjectedNavigationCommandClient<InternalRequestOptions> &
Pick<AgentDeviceCommandClient, 'appState' | 'keyboard' | 'clipboard'>;

export function createAgentDeviceClient(
config: AgentDeviceClientConfig = {},
deps: { transport?: AgentDeviceDaemonTransport } = {},
Expand Down Expand Up @@ -142,13 +133,25 @@ export function createAgentDeviceClient(

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

return {
command: {
wait: async (options) => await executeCommand<CommandResult<'wait'>>('wait', options),
alert: async (options = {}) => await executeCommand('alert', options),
...projectedSystemCommands,
appState: async (options = {}) =>
await executeCommand<CommandResult<'appstate'>>('appstate', options),
back: async (options = {}) => await executeCommand<CommandResult<'back'>>('back', options),
home: async (options = {}) => await executeCommand<CommandResult<'home'>>('home', options),
orientation: async (options) =>
await executeCommand<CommandResult<'orientation'>>('orientation', options),
appSwitcher: async (options = {}) =>
await executeCommand<CommandResult<'app-switcher'>>('app-switcher', options),
keyboard: async (options = {}) =>
await executeCommand<CommandResult<'keyboard'>>('keyboard', options),
clipboard: async (options) =>
await executeCommand<CommandResult<'clipboard'>>('clipboard', options),
tvRemote: async (options) =>
await executeCommand<CommandResult<'tv-remote'>>('tv-remote', options),
reactNative: async (options) => await executeCommand('react-native', options),
doctor: async (options = {}) =>
await executeCommand<CommandResult<'doctor'>>('doctor', options),
Expand Down Expand Up @@ -545,17 +548,6 @@ function optionalSnapshotResponseFields(
};
}

function buildProjectedSystemCommandClient(
executeCommand: <T>(command: DaemonCommandName, options?: InternalRequestOptions) => Promise<T>,
): ProjectedSystemCommandClient {
const methods: Record<string, (options?: InternalRequestOptions) => Promise<unknown>> = {};
for (const [method, command] of Object.entries(systemCommandFamily.clientCommandMethods ?? {})) {
methods[method] = async (options = {}) =>
await executeCommand<CommandResult<typeof command>>(command as DaemonCommandName, options);
}
return methods as unknown as ProjectedSystemCommandClient;
}

function readObject(value: unknown): Record<string, unknown> | undefined {
return isRecord(value) ? value : undefined;
}
Expand Down
Loading
Loading