Skip to content

refactor(commands): retire the navigation-only type projection - #2294

Merged
thymikee merged 4 commits into
mainfrom
claude/audit-retire-navigation-projection
Sep 5, 2026
Merged

refactor(commands): retire the navigation-only type projection#2294
thymikee merged 4 commits into
mainfrom
claude/audit-retire-navigation-projection

Conversation

@thymikee

@thymikee thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member

Summary

commands/system/navigation-projection.ts derived five client method signatures from a
phantom-typed registry: a unique symbol brand carrying Options/Result/required-ness, conditional
types to read them back, and a mapped type keyed on clientMethod. Nothing else used it.

Before, the published .d.ts resolved command.back through
ProjectedNavigationCommandClient<DeviceCommandBaseOptions> and that brand; after, the facade says
back: (options?: BackCommandOptions) => Promise<CommandResult<'back'>>, with the five Options
types beside their siblings in packages/contracts/src/client-system.ts.

back keeps its --settle triple (#1638); orientation and tv-remote keep their required
options parameter. The five MCP output schemas move next to the other handwritten ones. With the
projection gone, defineExecutableCommand's third overload, ExecutableCommandProjection,
AnyCommandDefinition.projection, projectCommandOutputSchemas and the family's
clientCommandMethods table have no users either; dropping that table also drops the
as unknown as cast the client used to build its eight system methods.

Validation

Tested commit: 335506b — pnpm check:affected --run green locally (format, lint, typecheck, layering, di-seams, fallow, mcp-metadata, build, package, integration-node, macos-coverage, vitest-related, integration-progress, replay-compat, daemon-wire-compat, affected-selector, gate-manifest, gate-manifest-model, depgraph, tmpdir-leaks, tmpdir-leaks-model, coverage-model, wire-compat-model, production-exports, bundle-owner-files, fixture-cache, fixture-fallback, command-docs, agent-guidance, xctest-selection, maestro-conformance, mutation-model)

  • Split the navigation schema tests into command-tools-navigation-schemas.test.ts (test-size ratchet: 1110 → 1029 + 82 lines).

Public-signature evidence: built dist/src/index.d.ts before and after, resolved the five methods
off ReturnType<typeof createAgentDeviceClient>['command'] with the TypeScript checker, and printed
each one's parameter arity/optionality and fully resolved parameter/result property sets. The diff
is empty.

Each changed regression test was seen red first: home wired to app-switcher, durationMs
dropped from the inlined tv-remote schema, a stray top-level key on a navigation schema, and
home's options parameter made required.

`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.
…hemas

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.
…nale

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.
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.61 MB 2.61 MB -772 B
JS gzip 878.8 kB 878.6 kB -199 B
npm bundled raw 2.61 MB 2.61 MB -772 B
npm bundled gzip 878.8 kB 878.6 kB -199 B
npm tarball 1.04 MB 1.04 MB -770 B
npm unpacked 3.52 MB 3.52 MB -2.6 kB
npm clean-installed 3.52 MB 3.52 MB -2.6 kB

npm unpacked components

Component Base Current Diff
JS / dist source 2.77 MB 2.77 MB -2.6 kB
Apple runner source/project 544.4 kB 544.4 kB 0 B
Apple snapshot presentation source 33.8 kB 33.8 kB 0 B
Apple Simulator snapshot bridge source 30.5 kB 30.5 kB 0 B
macOS helper source 54.8 kB 54.8 kB 0 B
Android helper artifacts 43.3 kB 43.3 kB 0 B
Other package files 46.7 kB 46.7 kB 0 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 22.5 ms 21.6 ms -0.9 ms
CLI --help 53.3 ms 52.4 ms -0.8 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/registry.js -1.5 kB -356 B
dist/src/cli-help.js 0 B -1 B

Top changed packed files

Packed file Base Current Diff
dist/src/client-types.d.ts 58.0 kB 56.2 kB -1.8 kB
dist/src/registry.js 171.8 kB 170.3 kB -1.5 kB
dist/src/command-tools.js 22.1 kB 22.6 kB +546 B
dist/src/agent-device-client.js 18.8 kB 18.9 kB +200 B
dist/src/command-schema.js 8.2 kB 8.3 kB +5 B

@thymikee

thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Reviewed exact head d85f1573d9b851f667e6c81e82b4f981fa36b2a6. The projection deletion itself is sound: command descriptors still own public identity and route classification; the smaller direct Node facade and MCP schema projections preserve the existing boundary and are covered by transport/schema tests. One blocker remains: Coverage correctly rejects growing src/mcp/__tests__/command-tools.test.ts from 1,053 to 1,110 lines while it is already over the tripwire. Move the new navigation output-schema coverage into a sibling test mirroring src/mcp/command-output-schemas.ts (or equivalently split the aggregation), then rerun the affected gate. All other checks are green; no device evidence is separately owed for this API/type/schema refactor.

@thymikee

thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed exact head 335506be6c52d090c32b80d41f3bcb7a0b4eb6cf: READY. The prior blocker is resolved cleanly—the complete navigation-schema fixture/assertion moved verbatim into the topology-specific src/mcp/__tests__/command-tools-navigation-schemas.test.ts, while command-tools.test.ts dropped from 1,110 lines on the blocked head to 1,029, leaving real ratchet headroom. All five closed output shapes, required keys, exact properties, and descriptor-derived settle assertions remain covered. The delta contains no production change or assertion loss. CI is still in progress; per review policy this does not block applying readiness to a clean head.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 5, 2026
@thymikee
thymikee merged commit d1b9914 into main Sep 5, 2026
17 of 18 checks passed
@thymikee
thymikee deleted the claude/audit-retire-navigation-projection branch September 5, 2026 17:30
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-05 17:30 UTC

thymikee added a commit that referenced this pull request Sep 5, 2026
* origin/main:
  perf: bundle runtime dependencies and report full install size (#2310)
  ci: avoid unrelated Apple runner cache invalidation (#2303)
  fix(web): preserve the backend ref so snapshot refs match actionable refs (#2283)
  test(daemon): session-open-url-prewarm through the request seam (#2304)
  test(daemon): session-devices-batch-runtime through the request seam (#2305)
  chore(gates): layering baselines ratchet against merge-base (#2299)
  test(daemon): one typed conformance helper for the daemon runtime suites (#2298)
  chore(layering): derive the contracts export inventory from package.json (#2297)
  perf: bundle tar-stream to reduce install footprint (#2286)
  docs: simplify agent context and resolve conflicting guidance (#2287)
  refactor(cli): let help resolve command aliases itself and retire R12 (#2293)
  refactor(commands): retire the navigation-only type projection (#2294)
  feat(runtime): route managed leases through contained transports (#2285)
  refactor(contracts): build unavailable runtime facts once (#2291)
  refactor(cli): derive the common flag readers from the common-field table (#2292)
  feat(daemon): add managed allocation operation journal (#2284)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant