Validate every message the relay receives, and make the inbound frame a discriminated union

.changeset/relay-inbound-validated.md

@@ -0,0 +1,45 @@
+---
+'@tapflowio/protocol': minor
+'@tapflowio/relay': minor
+---
+
+Validate every message the relay receives, and make the inbound frame a discriminated union
+
+The outbound direction has been compile-checked since #419 — `sendTo` refuses a message outside its
+union. Nothing checked the inbound direction: the relay's `RelayMessage` was a flat interface where
+`type` was the only required member, so every field it read was optional by construction and every
+field it needed came with a `!`. That is how the two type systems could disagree about the same wire
+field — `format?` in the relay against a required `format` in the protocol — with nothing to report it.
+
+`@tapflowio/protocol/validate` is a second entry point, imported only by the relay, that parses an
+inbound frame into a discriminated union at the door. It is a parse rather than a cast on purpose:
+narrowing the union with `as` would have turned the relay's one visible `msg.payload as ChromePayload`
+into an invisible `msg.payload`, with the compiler vouching for JSON that arrived over a socket.
+
+What a user can observe:
+
+- **A malformed command is refused where it used to be forwarded.** A `device:boot` with no payload, a
+  `session:start` whose `sessionId` is the empty string, an `app:install` whose `buildId` is an object
+  — these reached an agent before, or produced a reply whose own required field was missing. The frame
+  is now dropped and the log names the field that failed, instead of the command silently doing
+  nothing. `app:install` and `app:launch` are the exception and still answer `Build not found`, because
+  that answer already existed and is worth more than the refusal. No client shipped here can produce
+  any of these; a third-party one can.
+- **A key appended to a browser message no longer reaches a device.** Browser-origin frames are
+  forwarded as the parse product, so anything the contract does not declare is gone before an agent
+  sees it. Agent-origin frames are forwarded unchanged, so a field a newer agent adds still survives a
+  relay that does not know it.
+- **Nothing else changes.** Every well-formed frame routes exactly as before.
+
+`@tapflowio/protocol` gains a `./validate` subpath and, with it, a runtime dependency on `zod` — its
+first dependency of any kind. The main entry is unchanged: still types only, still fully erased by
+`import type`, and it does not reach `zod`. A consumer that imports only `@tapflowio/protocol` gains
+nothing in its bundle and one package in its install.
+
+Agent payloads are deliberately not validated, and that is a decision with a reason rather than a gap:
+`AgentRegister.platform` is `string` — open, so a third-party platform can register through
+`AgentRegistry.register()` — while `ChromePayload` is a closed two-member union. A platform this
+project promises to support has no valid `session:chrome` variant to send, and refusing one would cost
+it bezel and buttons for the life of the session. The six messages the relay consumes are validated,
+each with a default for every field the relay previously read through a `??`, so an agent older than a
+field keeps working exactly as it did.

CHANGELOG.md

@@ -34,9 +34,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   produced it — the set is deliberately smaller than any one agent's internals. (`not-session-owner`
   is the eighth member and is the relay's alone: it refuses such a frame at its door, before any agent
   sees it.) Prose stays welcome in `message` and may now be omitted.
+- **`@tapflowio/relay` no longer exports `RelayMessage` or `MessageType`.** They were the relay's own
+  copy of the wire contract — a flat interface where `type` was the only required member, and a
+  hand-maintained list of 63 literals beside it — and they disagreed with `@tapflowio/protocol` about
+  the same fields, which is the drift this release closes. Nothing in tapflow imported them; this
+  affects code outside it that did.
+  `Migrate:` import the message types from `@tapflowio/protocol` instead, which declares one interface
+  per message and unions them by direction — `BrowserToRelay`, `AgentToRelay`, `BrowserInbound` and so
+  on. A `RelayMessage` used as "any frame on this socket" becomes the union for that socket's
+  direction, and narrowing on `type` gives the individual message.
 
 ### Changed
 
+- **The relay now checks every message it receives against the contract, and refuses the ones that
+  break it.** Until now it checked only what it *sent*. A command with a missing payload, an empty
+  session id, or a build id that was not a number was forwarded to a device anyway — or answered with a
+  reply whose own required field was missing, which every client discards, turning a diagnosis into a
+  caller waiting out its deadline. Where a request has an error reply the caller still gets one; where
+  it has none the frame is dropped and the log names the field that failed, instead of the command
+  silently doing nothing. Well-formed messages are unaffected.
+- **A field appended to a browser message no longer reaches a device.** Anything the contract does not
+  declare is removed before the relay forwards it on. Messages coming *from* an agent are forwarded
+  untouched, so an agent newer than its relay does not lose fields it adds.
+- **`@tapflowio/protocol` has a second entry point, `@tapflowio/protocol/validate`.** It holds the
+  relay's inbound parser, and it brings the package its first runtime dependency (`zod`). The main
+  entry is unchanged — types only, fully erased by `import type`, and it does not reach `zod` — so a
+  consumer that imports only `@tapflowio/protocol` gains nothing in its bundle.
 - Split stable dashboard vendor dependencies into smaller chunks to reduce maximum bundle size and improve cache reuse across releases.
 - **A refused session now says which session it refused and why.** Opening a device someone else already
   has open, or one whose Mac is under load, used to produce a generic failure the dashboard could not

packages/ios-agent/src/IOSAgent.ts

@@ -762,7 +762,8 @@ export class IOSAgent implements DeviceAgent {
    *  hands back a `string` the case can close over.
    *
    *  It is unvalidated JSON, so the check is real work rather than ceremony: the declaration is required and
-   *  every in-repo sender is typed against it, but nothing validates inbound (#444), and `mcp-server`'s tool
+   *  every in-repo sender is typed against it, and #444 made the relay refuse an empty one — but this agent may
+   *  be talking to a relay older than that, and `mcp-server`'s tool
    *  schemas are bare `z.string()` so a model can produce `''`. */
   private correlatorOf(msg: { type: string; requestId?: string }): string | null {
     if (typeof msg.requestId === 'string' && msg.requestId !== '') return msg.requestId

packages/mcp-server/AGENTS.md

@@ -21,7 +21,7 @@ Connects to the relay over WebSocket + REST (`TapflowClient`), registers MCP too
 ## HOW
 
 - Entry: `src/index.ts` — reads `TAPFLOW_RELAY_URL` and `TAPFLOW_TOKEN` env vars, connects `TapflowClient`, calls `registerTools`, starts `StdioServerTransport`.
-- Client: `src/client.ts` — WebSocket connection to relay + REST calls for build/app data. Its `send()` takes `BrowserToRelay` from [`@tapflowio/protocol`](../protocol/AGENTS.md), so a new outbound message goes in that union first. Receiving stays loose (`Record<string, unknown>` + a predicate) — a **deferral, not a settled decision**, tracked in #512. Narrowing it would catch a live defect (`error` is matched on a `sessionId` that message does not have, so one session's failure can be reported against another's join) but it also makes `message: string`, turning this file's `?? 'failed'` fallbacks into unreachable code. Deleting those while nothing validates inbound JSON removes a real defence, so the validators (#444) come first.
+- Client: `src/client.ts` — WebSocket connection to relay + REST calls for build/app data. Its `send()` takes `BrowserToRelay` from [`@tapflowio/protocol`](../protocol/AGENTS.md), so a new outbound message goes in that union first. Receiving stays loose (`Record<string, unknown>` + a predicate) — a **deferral, not a settled decision**, tracked in #512. Narrowing it would catch a live defect (`error` is matched on a `sessionId` that message does not have, so one session's failure can be reported against another's join) but it also makes `message: string`, turning this file's `?? 'failed'` fallbacks into unreachable code. Deleting those used to remove a real defence, because nothing validated inbound JSON; #444 landed that validation at the relay, so the prerequisite this deferral named is met and #512 is now a judgement about this file alone.
 - Tools: `src/tools.ts` — all MCP tool definitions. One `registerTools(server, client)` call registers everything.
 - Screenshots are saved to a temp file and returned as MCP `image` content with base64 encoding.
 - **The screenshot's format is read from its magic bytes, not from the request or the reply** (#508).

packages/protocol/AGENTS.md

@@ -23,7 +23,7 @@ It exists because the relay and the dashboard each kept their own copy and they
 
 `protocol` is deliberately broader than what the package holds today, because the alternatives age worse.
 
-- `protocol-types` would become a lie the moment a runtime validator lands, and one plausibly will — the relay validates nothing on the way in.
+- `protocol-types` became a lie the moment a runtime validator landed, and one did — `src/validate/` is the relay's inbound parser (#444). The broad name is what let it move in without renaming the package.
 - `relay-protocol` reads narrower than the truth: these messages are exchanged by browser ↔ relay ↔ agent, not owned by the relay. It also sits one letter away from `@tapflowio/relay` at every import site.
 - `messages` cannot hold anything that is not a message, which is the same corner `protocol-types` paints into.
 
@@ -32,7 +32,18 @@ The cost of a broad name is ambiguity about what belongs — answered by the two
 ## Scope — what belongs here
 
 - **JSON message types** over the relay WebSocket, grouped by direction. That is all the package holds today, and the main entry point is **runtime-free** — see HOW NOT.
-- **Runtime validators** would belong here *conceptually* — next to the types they validate, since splitting them recreates the drift this package removes. But they cannot go in the main entry: that would break the erasure the dashboard depends on. Adding them means a second entry point (`@tapflowio/protocol/validate`) that only server-side consumers import, which is an explicit scope change, not a drive-by addition. Tracked in #444.
+- **Runtime validation of inbound messages** lives in `src/validate/`, reached as
+  `@tapflowio/protocol/validate` and imported only by the relay. It is here rather than in the relay for
+  the reason the package exists: a schema file is a second copy of the contract, and a second copy
+  drifts. The main entry stays runtime-free, so the erasure the dashboard depends on is unchanged.
+  Two rules make the copy safe, and both are compile errors rather than conventions:
+  - **Two tiers, with different static types.** `Validated` parses to the interface;
+    `Envelope` parses to `EnvelopeOf<I>` — the interface projected onto `type`/`sessionId`/`requestId`
+    — so a payload the door did not check **cannot be read** off the result. A field that was not
+    validated must not appear in the type; anything else moves the lie somewhere quieter.
+  - **`SchemaExact` ties each schema to its interface**, and refuses `z.custom<T>()` and a
+    `const s: z.ZodType<T>` annotation by kind, because both produce `T` with no `any` for `IsAny` to
+    catch and would compare `T` with itself.
 
 ## Scope — what does not
 
@@ -297,7 +308,8 @@ claimed *"the escape hatch for a failure the relay cannot correlate to a session
 be true, and the program plan recorded that they were both in HEAD.
 
 L5c settled it by removing the general role rather than the specific one. A request naming no session is
-dropped at the relay's door (`isAddressed`), because answering it would ship a frame whose own required
+dropped at the relay's door — by the schema in `src/validate/` since #444, and by an `isAddressed`
+predicate before that — because answering it would ship a frame whose own required
 `sessionId` `JSON.stringify` erases — and `error` has no `requestId` either, so a caller could not attribute
 the answer and would wait out the same deadline silence costs. With nothing left needing an unaddressed
 failure, all four producers answer one specific join, and `error` **extends `SessionScoped`**: the shape is the
@@ -341,7 +353,8 @@ type-check that message could take over the session's video path. The stream soc
 own send site in `agent-core/src/utils/stream.ts`.
 
 That mattered because an agent's literal was the one thing no compiler saw — the relay forwards replies with
-`JSON.stringify(msg)`, so nothing typed re-creates them. #489 and #490 are what the gap cost, and
+`JSON.stringify(raw)` — the frame exactly as it arrived — so nothing typed re-creates them. #489 and
+#490 are what the gap cost, and
 `inputErrorReason.test.mjs` exists because a script had to stand in for a compiler.
 
 **The browser side is the same rule and the same check shape.** All three browser-role producers — the dashboard's
@@ -374,8 +387,8 @@ A browser receives 28 message types. They come from two producers, and the diffe
 - **`RelayToBrowser`** — the relay builds these itself, so `sendTo(socket, msg: RelayOutbound)` holds
   them to the union. The compiler is the check.
 - **`AgentToBrowser`** — an agent builds these and the relay forwards them with
-  `JSON.stringify(msg)`. Nothing on the relay's send path references them, so **no compiler sees
-  them.** That is why all twelve forward-only messages were absent from this file until L3, and why
+  `JSON.stringify(raw)`, the frame exactly as it arrived. Nothing on the relay's send path references
+  them, so **no compiler sees them.** That is why all twelve forward-only messages were absent from this file until L3, and why
   `scripts/__tests__/browserInboundRouting.test.mjs` exists: it compares the relay's forward case
   labels against this union in both directions.
 - **`RelayOrAgentToBrowser`** — the ten with *both* producers (the relay replays session state to a
@@ -391,13 +404,17 @@ A browser receives 28 message types. They come from two producers, and the diffe
 
 ### `sessionId` stays required, even where the relay cannot prove it
 
-The relay reaches eleven sessions through `msg.sessionId!` — an assertion the compiler cannot verify —
-and `JSON.stringify` drops a key whose value is `undefined`. **Eight are agent→browser forwards and three
-are request-side paths that deliberately have no address gate**; the seven *reply* sites this paragraph
-used to count went away with L5c's door predicates, and the number outlived them here. The composition
-matters more than the total, because "all forwards" invites the conclusion that the request side is
-settled — and `device:shutdown` is on the request side with no ownership gate either (#527). The fix is
-**not** to widen the declaration:
+The relay used to reach eleven sessions through `msg.sessionId!` — an assertion the compiler cannot
+verify — and `JSON.stringify` drops a key whose value is `undefined`. **`RelayServer.ts` now contains
+none of them.** #444 made the inbound frame the product of a parse, so `sessionId` is a narrowed
+`string` by the time any case reads it, and `.min(1)` refuses the empty string the old predicates were
+written to catch. Two numbers stood in this paragraph before that — seven reply sites removed by L5c's
+door predicates, then eleven reads — and each outlived its own basis, which is why there is no count
+here now.
+
+`device:shutdown` is still on the request side with no **ownership** gate (#527); that is a different
+question from addressing and the parse does not answer it. The declaration was nevertheless right to
+stay required rather than be widened:
 
 - Every in-repo sender does supply one. `BrowserToRelay` declares `sessionId: string` on every member
   but `agents:list`, and since L4c all three senders are typed against that union, so the compiler
@@ -411,8 +428,9 @@ settled — and `device:shutdown` is on the request side with no ownership gate
   that names no session at the door. Both halves of the old advice are gone: there is no unaddressed failure
   left to send, and nothing left that would need one.
 
-Widening would let #444 delete those `!` with no consumer forced to care, and the guarantee would go
-quietly with them. It is also close to irreversible: once optional, every consumer grows a guard.
+Widening would have let #444 delete those `!` with no consumer forced to care, and the guarantee would
+have gone quietly with them — the door now enforces the declaration instead. It is also close to
+irreversible: once optional, every consumer grows a guard.
 
 The same reasoning applies to "no producer sends this yet, so leave it open." `mcp-server` has no
 clipboard tool today; when one is added, `requestId` being **required** is what makes a missing id a
@@ -460,11 +478,24 @@ sends through `sendTo(socket, msg: RelayOutbound)`, so its literal is checked by
 ## HOW NOT
 
 - **No `enum`, no const objects, no runtime values of any kind — in the main entry.** They compile to JavaScript, so the moment a consumer references one as a value it stops being erased by `import type` and lands in the dashboard's browser bundle. String literal unions only. (`src/typeAssertions.ts` is checked by `tsconfig.assertions.json` and excluded from the build for exactly this reason — it declares values, so it must not reach `dist`.)
-- **Do not add a dependency.** This package is a leaf — it must stay importable from the browser bundle, the relay, and mcp-server alike.
+- **Do not add a dependency to the main entry.** It must stay importable from the browser bundle, the
+  relay and mcp-server alike, and erasable under `import type`.
+  `zod` is a dependency of the package (`./validate` needs it at runtime) and is deliberately **not**
+  reachable from `./`. The cost is stated rather than hidden: `agent-core` and `flow-runner` import this
+  package without importing `/validate`, so they carry zod in their install for nothing. It has no
+  transitive dependencies and neither ships to a browser, which is why that was judged cheap — a second
+  runtime dependency is not automatically the same trade.
 - Do not widen a message to `unknown`/`Record<string, unknown>` to make a call site compile. That reopens the hole this package closes; fix the call site or correct the type.
 
 ## Consuming it
 
+**Every `exports` subpath needs its own `source` condition.** `./validate` carries one, and without it
+the relay's tests would validate against whatever was last *built* of this package — a stale parser
+reporting green on a schema you just edited, which is the failure #459 shipped and the direction that
+hides a validation hole rather than exposing one. `scripts/__tests__/testsReadSource.test.mjs` checks
+that a package extends `sourceFirst`; it does **not** look at subpaths, so nothing would report the
+omission.
+
 The relay is composite (TS project references), so it needs both the dependency **and** a `references` entry pointing here — see [`contributing/monorepo-project-references.md`](../../contributing/monorepo-project-references.md). `dashboard` and `mcp-server` are not composite and need only the dependency — but they do **not** read `src` under `tsc`. Neither sets `customConditions` (`dashboard` is `moduleResolution: bundler`, `mcp-server` is `Node16`), so both take the first key in this package's `exports`, which is `./dist/index.d.ts`. The `source` condition is wired into **vitest** only, via `vitest.shared.ts`'s `ssr.resolve.conditions`.
 
 That has a consequence worth knowing before measuring anything: **`pnpm typecheck` lies about these two until this package is rebuilt.** Tightening a field here and running the dashboard's typecheck against a stale `dist` reports 0 errors. And `tsc -b` never sees them at all — neither is in the root `references`, so a change whose fallout is entirely in `dashboard` builds clean.