refactor(mcp): remove preview tool surface (ADA-115)

ADA-115.plan.md

@@ -0,0 +1,202 @@
+# ADA-115 MCP tool-surface redesign plan
+
+## Fixed rulings and target
+
+- Ship four independently green PRs, in order: preview deletion; spawn/thread trim;
+  concept fixes; AX/schema polish.
+- The current surface is 26 tools: 13 `preview_*` tools and 13 `t3_*` tools.
+  The final surface is exactly 11 tools:
+  `t3_thread_start`, `t3_spawn_subagent`, `t3_steer_subagent`, `t3_subagents`,
+  `t3_schedule_create`, `t3_schedule_list`, `t3_schedule_update`,
+  `t3_schedule_delete`, `t3_notify`, `t3_get_usage`, and `t3_list_backends`.
+- `t3_spawn_subagent` and `t3_thread_start` expose exactly required `prompt`,
+  `model`, `title`, plus optional `directory`, `branch`, and `reasoningEffort`.
+  The handler always constructs the internal `ModelSelection`; omitted
+  `reasoningEffort` becomes `xhigh` only for models whose Codex
+  `reasoningEffort` option specifically advertises `xhigh`. A Codex model that
+  advertises reasoning effort but not `xhigh` fails correctively on omission,
+  naming the model and listing its supported efforts so the caller can use the
+  optional override without silently weakening the mandated default. Models
+  without a `reasoningEffort` option remain valid and do not receive a
+  fabricated provider option; explicitly supplying the field for such a model
+  fails correctively with that model and its valid option IDs.
+- Internal orchestration services and peer protocols remain intact unless code is
+  dead after the MCP surface changes. In particular, wake-on-completion,
+  `ChildThreadCoordinator`, remote-child polling/steering, and authenticated
+  peer parent linkage are product contracts, not tool APIs to delete.
+
+## Registration and caller map
+
+The single registration root is `apps/server/src/mcp/McpHttpServer.ts`.
+`ToolkitRegistrationLive` merges preview, thread, subagent, notify, usage, and
+visibility toolkits into `/mcp`. The official-client boundary is exercised by
+`apps/server/src/mcp/McpOfficialClientConformance.test.ts`; schema-only checks
+also live in `apps/server/src/mcp/toolSchemas.test.ts`.
+
+### Preview agent surface (PR 1)
+
+- Definitions/handlers: `apps/server/src/mcp/toolkits/preview/{tools,handlers}.ts`.
+- Bespoke snapshot registration and all preview toolkit registration:
+  `apps/server/src/mcp/McpHttpServer.ts`.
+- Preview-tool-exclusive transport: `apps/server/src/mcp/PreviewAutomationBroker.ts`,
+  its `server.ts` layer, preview automation RPC authorization/handlers in
+  `apps/server/src/ws.ts`, RPC contracts in `packages/contracts/src/rpc.ts`, and
+  RPC client methods in `packages/client-runtime/src/rpc/client.ts`.
+- Preview-tool-exclusive web host: `apps/web/src/AppRoot.tsx`,
+  `PreviewAutomationHosts.tsx`, and the adjacent
+  `previewAutomation{ClientId,Errors,OpenReadiness,RequestConsumer,Target}.ts`
+  helpers. Remove `WS_METHODS.previewAutomationConnect` from
+  `apps/web/src/rpc/requestLatencyState.ts`'s production untracked-ack set as
+  well as updating its test.
+- Shared preview contracts are not one deletion unit. Preserve the desktop/IPC
+  action, status, snapshot, resize, keyboard, and recording schemas from
+  `packages/contracts/src/previewAutomation.ts`; they are consumed by
+  `apps/desktop/src/preview/Manager.ts`, desktop preview IPC, and
+  `packages/shared/src/previewViewport.ts`. Remove/split only the broker-host,
+  response, stream-event, and focus contracts that become unreferenced with the
+  server/WebSocket automation transport.
+- Agent instructions: remove the preview tool mandate from
+  `apps/server/src/provider/CodexDeveloperInstructions.ts`.
+- Tests migrate/delete alongside their owned surface: MCP server/conformance,
+  broker, web automation helper/host/AppRoot, request latency, and server layer
+  tests.
+- Preserve the collaborative browser itself: desktop preview manager/IPC,
+  preview viewport utilities, preview tab UI, and their contracts/tests stay.
+  `packages/client-runtime/src/state/preview.ts` is mixed: remove its
+  `previewAutomationConnect`/`Respond`/`FocusHost` subscription and command atoms
+  plus matching Web RPC interface fields in `packages/contracts/src/ipc.ts`,
+  while preserving its tab/session/event collaborative-browser state. Preserve
+  desktop-local automation IPC methods used by the preview manager; they do not
+  reconnect to the deleted server broker.
+
+Guardrail first: change official `tools/list` expectations to the exact 13
+non-preview tools and explicitly assert every `preview_*` name is absent. Run on
+old code and record RED before deleting implementation.
+
+### Spawn/thread surface (PR 2)
+
+- Schemas: `apps/server/src/mcp/toolkits/thread/tools.ts` owns the shared start
+  input; `apps/server/src/mcp/toolkits/subagent/tools.ts` composes spawn plus
+  check/wait/list and schedule inputs.
+- Thread handler: `apps/server/src/mcp/toolkits/thread/handlers.ts` resolves
+  directory/worktree/project context, model routing, inherited runtime mode, and
+  project setup. It will receive only the slim public input and apply fixed
+  internal defaults (`new_worktree`, project-config setup behavior, inherited
+  modes, and an internal `ModelSelection`).
+- Spawn handler: `apps/server/src/mcp/toolkits/subagent/handlers.ts` wraps thread
+  start, registers local/remote child metadata, steers, checks/lists/waits, and
+  schedules. Remove MCP-only foreground wait and wait handler paths while
+  preserving coordinator/wake delivery.
+- External MCP callers/docs/tests: `McpOfficialClientConformance.test.ts`, both
+  toolkit handler test files, `apps/server/src/subagents/McpPeerClient.test.ts`,
+  `apps/server/e2e/drive.sh`, `e2e/README.md`, and provider instruction text.
+- Internal callers/services that must keep working:
+  `ChildThreadCoordinator`, `PendingDispatches`, remote-child polling, and peer
+  client internals. Rename only user-facing tool vocabulary where it appears in
+  generated/injected guidance.
+- The existing peer receiver passes `remoteParentThreadId` and
+  `remoteParentEnvironmentId` through the same public tool name. Keep a private
+  version-tolerant `PeerSpawnSubagentInput` for authenticated peer calls. It
+  accepts the legacy optional title/model/modelSelection, detached and parent
+  link fields from old parents, normalizes absent title/model through the
+  existing inherited selection/title behavior, and normalizes the remaining
+  legacy start fields to the new internal defaults. Register spawn with a public
+  six-field JSON schema plus invocation-aware decoding: provider-session calls
+  decode only the public slim input; peer-token calls decode this private legacy
+  union. Thus
+  `tools/list` never advertises peer bookkeeping, while `McpPeerClient` retains a
+  typed internal contract and remote children retain authenticated parentage.
+  The public `target` selector is removed per the exact-argument ruling; this is
+  not replaced with a new public routing syntax.
+- Replace `t3_check_subagent` + `t3_list_subagents` with `t3_subagents` whose
+  optional `childThreadId` selects detail; omission lists children for the
+  invocation's parent thread. Detail includes latest assistant text only after
+  provider invocations assert that the child belongs to the invoking thread and
+  peer invocations pass their existing child entitlement; add a guardrail that a
+  different parent's child ID is rejected without text.
+- Preserve mixed-version peer polling without retaining old names in the public
+  provider tool list: new peer clients try `t3_subagents` and fall back to
+  `t3_check_subagent` on an old peer. The stock Effect toolkit cannot filter its
+  process-wide registry by authenticated credential, so PR 2 owns an explicit
+  boundary in `McpHttpServer.ts`: before the generic toolkit decoder, an
+  authenticated peer-scope `tools/call` compatibility dispatcher recognizes
+  legacy `t3_check_subagent` and legacy-shaped `t3_spawn_subagent`, decodes the
+  private schemas, and invokes the new shared handlers. Those compatibility
+  entries are never registered with `McpServer`, so provider-session (and
+  ordinary peer) `tools/list` remains the exact public surface; provider-scope
+  calls to the old name still return unknown-tool. Cover raw JSON-RPC peer-token
+  calls plus new-parent/old-child and old-parent/new-child directions. Remove
+  the dispatcher only in a later protocol migration after persisted remote
+  children can no longer straddle versions.
+
+Guardrails first: exact public JSON-schema property sets for both start tools;
+removed tool names absent; `t3_subagents` list/detail behavior; and a bare spawn
+with no `reasoningEffort` producing a Codex session/model selection with
+`options.reasoningEffort === "xhigh"`. Run the default-effort assertion on old
+code and record RED.
+
+### Backend/notify/schedule concepts (PR 3)
+
+- Current `t3_list_backends` schema/handler in
+  `toolkits/visibility/{tools,handlers}.ts` reads `ProviderRegistry`, so its
+  top-level `backends` are actually provider instances. Switch its topology to
+  `SubagentPeerRegistry`; each public row is one local/peer backend and nests
+  providers/models.
+- Define the provider inventory without extending the unauthenticated
+  `/.well-known/t3/environment` descriptor: a peer-token invocation of
+  `t3_list_backends` returns only that server's local backend and local
+  `ProviderRegistry` inventory. A provider-session invocation returns its local
+  row, then queries registered peers through `McpPeerClient` and relabels each
+  peer's single local row with the registry alias. This is authenticated and
+  non-recursive by credential kind. Probe/timeout failures still yield a row
+  with the peer alias, descriptor/last-seen OS when available, offline/error
+  status, and no providers rather than dropping the configured backend.
+- Add the peer registry/HTTP dependencies at the visibility toolkit layer and
+  update visibility tests plus official-client conformance fixtures.
+- `t3_notify` aliases `ServerNotifyInput` from `packages/contracts/src/server.ts`.
+  Do not change notification delivery contracts used by desktop/PWA; define the
+  MCP input without `requireInteraction` and let the handler use the delivery
+  service default.
+- Remove `busyPolicy` only from schedule create/update MCP inputs and handlers.
+  Keep persistence/reactor/web schedule contracts and the persisted default
+  `skip`; create writes `skip`, update preserves the existing value.
+
+### AX and OpenAI schema compatibility (PR 4)
+
+- Pin the exact 11-name list and every per-tool input property set in the
+  official `@modelcontextprotocol/sdk` conformance test.
+- Recursively reject nullable optional input encoding (`anyOf` containing
+  `{type:"null"}`) while retaining intentional required-nullable output/data
+  contracts. Every tool input/output root remains `type: "object"`.
+- Use non-nullable optional fields for tool inputs. Where an old nullable input
+  represented an explicit reset (schedule model), use the smallest compatible
+  interpretation: omission means unchanged and a plain optional model selects a
+  model; no new reset sentinel is introduced.
+- Document all omitted-field defaults in tool descriptions, including start
+  worktree/setup/mode inheritance, `reasoningEffort=xhigh`, subagent list mode,
+  schedule timezone/model/thread/default skip, notify deep link, and usage scope.
+- Audit validation paths in thread/subagent/visibility/schedule handlers. Errors
+  include the invalid value and enumerate currently valid values (models,
+  backends, modes/options) when such a finite set exists.
+
+## Verification and landing per PR
+
+1. Add/run the PR-specific guardrail against the old code where required and
+   capture RED.
+2. Implement the smallest owned slice; run affected tests, `vp check`, and
+   `vp run typecheck`.
+3. Rebuild and verify the MCP boundary on a disposable T3 server or the official
+   HTTP-client conformance harness; never touch the live server.
+4. Commit only through the prepared factory gate, open a draft PR whose body
+   contains `Fixes ADA-115` as explicitly required for every PR, mark ready
+   exactly once as required by this task's
+   anti-recycle ruling, wait through `codex-review-wait`, resolve findings when
+   the current-HEAD gate can do so without another ready transition, and land
+   with `wizzo-approve --apply`. A finding that requires a fix commit and hence a
+   forbidden second ready transition is a named workflow blocker for Adam, not
+   permission to recycle the PR. If Linear's closing-keyword automation marks
+   ADA-115 Done after PRs 1–3, immediately restore In Progress; close it only
+   after PR 4 lands.
+5. Update `origin/main`, create the next branch, and repeat. Append each PR's
+   evidence and any smallest-interpretation note to the dispatch memo and
+   stabilization log.