refactor(mcp): project viewport family output schema from its owning module - #3033
Conversation
Size Report
Startup median (7 runs, lower is better):
|
|
Reviewed at 8faeba2. This looks good: the viewport output schema now lives in The failing Smoke Tests job is a live iOS simulator wait timeout ("wait for Agent Device Tester"). This diff does not touch that path, so it looks unrelated. The Coverage failure is the eager-closure budget check that fails the same way on the other PRs in this stack, through |
…module Moves the single hand-authored MCP outputSchema entry for viewport out of command-output-schemas.ts into a frozen VIEWPORT_COMMAND_OUTPUT_SCHEMAS map exported from src/commands/management/viewport.ts, following the #2810 projection seam already used by the replay, system, interaction, device-management, push-management, and recording families on this stack. Registers the family in the PROJECTED_FAMILIES disjointness check and adds command-tools-management-viewport-schemas.test.ts with a reference-equality proof against the module object and an owner-file check, matching the pattern from the other projected families. viewport does not carry the post-action observation trait (#1652), so no settle-graft copy applies here. The moved code is byte-identical to what command-output-schemas.ts held: same objectSchema/numberSchema/stringSchema call and field order. Verified the published outputSchema for viewport is byte-identical to origin/main by dumping COMMAND_OUTPUT_SCHEMAS.viewport as JSON on both refs. Part of #2819.
8faeba2 to
f32c4ca
Compare
|
Rebased onto the updated base (recording/index schemas); new head f32c4ca. Resolved one import conflict in the MCP output schema registry, keeping the viewport family together with the interaction and recording modules. No review findings were open. Coverage failures seen earlier came from the lower links' eager-closure change, which the base now fixes; local checks (pnpm check:affected) pass. |
|
I found no problems in the change at f32c4ca. The earlier note on 8faeba2 still holds, and the viewport output schema now comes from its owning module with the same content. I confirmed that by reading the two hunks, not by dumping COMMAND_OUTPUT_SCHEMAS.viewport on both refs. I did not run the eager-closure budget check locally. I relied on the diff's import edges and the passing test in the Coverage log. Two CI jobs failed, and neither touches this patch. Coverage failed one test, "delayed restart health probe stops at the RPC deadline without retrying" in src/daemon-client/tests/daemon-client-transport.test.ts. It looks like a timing-sensitive loopback test under load, and the daemon-client code comes from the rebased base. Smoke Tests failed at "wait text Automation lab" because the lab app was not running on the iOS simulator. That is the Apple runner and daemon wait route, and this patch only changes MCP output-schema advertisement. I did not re-run either job, so this rests on the routes not overlapping, not on a green rerun. Please rerun Smoke Tests and Coverage before merge. I did not re-review the rebased base, only the delta. |
Summary
Moves the
viewportcommand family's MCP output schema out of the sharedcommand-output-schemas.tsgrab-bag and intosrc/commands/management/viewport.ts, its owning module, matching the pattern already applied to interaction, device-management, push-management, and recording families earlier in this stack.Part of #2819. Touches 4 files. Stacked on
refactor/mcp-output-schemas-2819-recording-index-schemas(the next PR in the #2819 sequence).Validation
Tested commit:
8faeba26d038808be6295c76cd0d1c6fba801321.pnpm check:affected --run: 161 test files / 1352 tests passed, no failures, no retries needed.No device needed — pure MCP schema move. Proved the published
outputSchemais byte-identical before/after: dumpedJSON.stringify(COMMAND_OUTPUT_SCHEMAS.viewport)via a throwaway vitest test onorigin/main(freshpnpm install --frozen-lockfilein a scratch worktree, removed afterward) and on this branch's HEAD. Both produced:No unresolved risk: this is a structural move with an unchanged public schema, confirmed by the diff above (import/re-export churn only).