From e8b29b8fabdc9cec386f415d440ad3dee4c93af6 Mon Sep 17 00:00:00 2001 From: Duchan Date: Sat, 15 Aug 2026 17:35:11 +0900 Subject: [PATCH 1/5] feat(relay): parse inbound frames into a discriminated union at the door MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The outbound direction has been compile-checked since #419; nothing checked the inbound one. `RelayMessage` was a flat interface where `type` was the only required member, so every field the relay read was optional by construction and every field it needed came with a `!` — which is how it could declare `format?` against the protocol's required `format` with nothing to report it (#550). `@tapflowio/protocol/validate` is a second entry point, relay-only, that parses an inbound frame into a discriminated union (#444). A parse rather than a cast: narrowing with `as` would have turned the one visible `msg.payload as ChromePayload` into an invisible `msg.payload`, with the compiler vouching for JSON from a socket. Two tiers, with different static types, so a field that was not validated does not appear in the type. Validated parses to the interface; Envelope parses to `EnvelopeOf` and its payload is unreadable. The whole agent direction is Envelope except the six messages the relay consumes: `ChromePayload` is a closed two-member union while `AgentRegister.platform` is open by OCP, so validating `session:chrome` would refuse a third-party platform the repo promises to support — permanently, since it arrives once per boot and a rejection empties the re-join replay too. Forwarding differs by direction and both halves are checked: agent-origin frames go on as they arrived so a newer agent keeps its added fields; browser-origin frames go on as the parse product so a key appended from devtools does not reach a device. Deleted: `RelayMessage`, `MessageType` (62 hand-copied literals), `AGENT_MSG_TYPE_LIST` (29 more) with its assertions, `isAddressed`, `isCorrelated`, and every `msg.sessionId!`. 15 mutations run against the new guards, 15 caught — including a payload added to an Envelope schema as `z.unknown()` / `z.any()` / `z.custom()`, which the first design's weaker assertion passed on all three. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/relay-inbound-validated.md | 38 ++ CHANGELOG.md | 19 + packages/protocol/AGENTS.md | 28 +- packages/protocol/package.json | 16 +- packages/protocol/src/__tests__/tsconfig.json | 33 ++ .../protocol/src/__tests__/validate.test.ts | 226 ++++++++ packages/protocol/src/index.ts | 48 +- packages/protocol/src/validate/assert.ts | 71 +++ packages/protocol/src/validate/index.ts | 493 ++++++++++++++++++ packages/protocol/tsconfig.assertions.json | 2 +- packages/protocol/tsconfig.json | 3 +- packages/relay/AGENTS.md | 11 +- packages/relay/src/RelayServer.ts | 421 ++++++++------- .../__tests__/RelayServer.heartbeat.test.ts | 16 +- .../relay/src/__tests__/RelayServer.test.ts | 446 ++++++++-------- .../src/__tests__/SessionManager.test.ts | 6 +- .../src/__tests__/agentReconnectGrace.test.ts | 24 +- .../src/__tests__/appCommandErrors.test.ts | 11 +- .../relay/src/__tests__/clearState.test.ts | 31 +- .../relay/src/__tests__/clipboard.test.ts | 37 +- .../src/__tests__/deviceReadyReplay.test.ts | 24 +- .../src/__tests__/inputErrorReason.test.ts | 34 +- .../__tests__/lifecycleCorrelation.test.ts | 28 +- .../relay/src/__tests__/screenshot.test.ts | 50 +- .../relay/src/__tests__/sessionRebind.test.ts | 20 +- .../relay/src/__tests__/socketHelpers.test.ts | 2 +- packages/relay/src/__tests__/uiTree.test.ts | 26 +- packages/relay/src/index.ts | 1 - packages/relay/src/types.ts | 141 +---- pnpm-lock.yaml | 7 + .../__tests__/browserInboundRouting.test.mjs | 26 +- .../correlatedRequestsGated.test.mjs | 94 ++-- 32 files changed, 1646 insertions(+), 787 deletions(-) create mode 100644 .changeset/relay-inbound-validated.md create mode 100644 packages/protocol/src/__tests__/tsconfig.json create mode 100644 packages/protocol/src/__tests__/validate.test.ts create mode 100644 packages/protocol/src/validate/assert.ts create mode 100644 packages/protocol/src/validate/index.ts diff --git a/.changeset/relay-inbound-validated.md b/.changeset/relay-inbound-validated.md new file mode 100644 index 00000000..c562918d --- /dev/null +++ b/.changeset/relay-inbound-validated.md @@ -0,0 +1,38 @@ +--- +'@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. Where the + request has an error reply the caller still gets one; where it has none it is dropped and logged with + the field that failed, instead of silently doing nothing. +- **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. + +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. diff --git a/CHANGELOG.md b/CHANGELOG.md index 90b4562b..cfb809d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,9 +34,28 @@ 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 62 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. - 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 diff --git a/packages/protocol/AGENTS.md b/packages/protocol/AGENTS.md index 533242aa..cb02f999 100644 --- a/packages/protocol/AGENTS.md +++ b/packages/protocol/AGENTS.md @@ -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` — 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()` and a + `const s: z.ZodType` annotation by kind, because both produce `T` with no `any` for `IsAny` to + catch and would compare `T` with itself. ## Scope — what does not @@ -460,11 +471,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` 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. diff --git a/packages/protocol/package.json b/packages/protocol/package.json index 468945f6..67d41e1d 100644 --- a/packages/protocol/package.json +++ b/packages/protocol/package.json @@ -28,6 +28,13 @@ "tsx": "./src/index.ts", "import": "./dist/index.js", "default": "./dist/index.js" + }, + "./validate": { + "types": "./dist/validate/index.d.ts", + "source": "./src/validate/index.ts", + "tsx": "./src/validate/index.ts", + "import": "./dist/validate/index.js", + "default": "./dist/validate/index.js" } }, "files": [ @@ -43,10 +50,15 @@ }, "scripts": { "build": "tsc -b", - "typecheck": "tsc -b && tsc -p tsconfig.assertions.json", + "typecheck": "tsc -b && tsc -p tsconfig.assertions.json && tsc -p src/__tests__/tsconfig.json", + "test": "vitest run", "lint": "eslint src" }, + "dependencies": { + "zod": "^4.4.3" + }, "devDependencies": { - "typescript": "^5.0.0" + "typescript": "^5.0.0", + "vitest": "^4.1.10" } } diff --git a/packages/protocol/src/__tests__/tsconfig.json b/packages/protocol/src/__tests__/tsconfig.json new file mode 100644 index 00000000..3574bfb0 --- /dev/null +++ b/packages/protocol/src/__tests__/tsconfig.json @@ -0,0 +1,33 @@ +// Type-checks this package's test tree, which the build tsconfig excludes. +// +// Tests must not reach `dist`, so they cannot simply join the build's `include` — the same shape as +// `protocol/tsconfig.assertions.json`, and for the same reason. Wired into the package's `typecheck` +// script, which runs it after `tsc -b` so the sibling `dist` it resolves against is current. +// +// **Named `tsconfig.json` and placed here on purpose.** typescript-eslint's `projectService` finds a +// file's project the way tsserver does — by walking up for a `tsconfig.json` — so a +// `tsconfig.test.json` at the package root is invisible to it, and every rule then fails as +// `was not found by the project service` rather than reporting. #422 needs both gates, and this is +// the one file that serves both. +// +// `moduleResolution: bundler` rather than the build's Node16: vitest resolves through vite, which +// does not require the `.js` suffix. 166 of the errors this first surfaced were nothing but that +// suffix missing — checking tests under a resolution they never run with would have meant rewriting +// every import to satisfy a compiler no test obeys. +{ + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + "composite": false, + "incremental": false, + "declaration": false, + "declarationMap": false, + "emitDeclarationOnly": false, + "rootDir": "../..", + "module": "ESNext", + "moduleResolution": "bundler" + }, + "include": ["../**/*.ts", "../**/*.tsx"], + "exclude": ["../../dist", "../../node_modules"], + "references": [] +} diff --git a/packages/protocol/src/__tests__/validate.test.ts b/packages/protocol/src/__tests__/validate.test.ts new file mode 100644 index 00000000..f7d01522 --- /dev/null +++ b/packages/protocol/src/__tests__/validate.test.ts @@ -0,0 +1,226 @@ +/** + * `parseInbound` / `directionOf` — the runtime half of the door. + * + * The type-level half lives beside the schemas in `validate/index.ts` and is mutation-tested: eight + * mutations were run against it and eight were caught (a payload added to an Envelope schema as + * `z.unknown()`, `z.any()` and `z.custom()`; an envelope emptied to `z.object({})`; a Validated + * schema swapped for `z.custom`; a member dropped from the map; an envelope losing its correlator; an + * optional field made required). + * + * **Two survived, and they are what this file exists for.** `.min(1)` and `.default([])` are runtime + * behaviour that does not change what `z.output` infers — by design, since that is exactly why they + * cost the tier assertions nothing — so no type-level assertion can hold them. Removing either leaves + * the whole static suite green. `rejects an empty sessionId` and `accepts an agent that predates + * capabilities` are the tests that fail instead; the rest of the file covers the door's own branches. + */ +import { describe, expect, it } from 'vitest' + +import { directionOf, parseInbound } from '../validate/index.js' + +/** The narrowing every test does. Written once because `parseInbound` returns a union and each test + * would otherwise repeat the same four lines to reach `msg`. */ +function ok(raw: unknown) { + const r = parseInbound(raw) + if (!r.ok) throw new Error(`expected a parse, got ${r.reason}`) + return r +} + +function fail(raw: unknown) { + const r = parseInbound(raw) + if (r.ok) throw new Error('expected a rejection, got a parse') + return r +} + +describe('the door rejects what it cannot name', () => { + // `JSON.parse` returns bare `null`, numbers and strings without throwing, and the caller reads + // `.type` off whatever it is handed. + it.each([null, 42, 'a string', true, []])('refuses %p as not-an-object', (raw) => { + expect(fail(raw).reason).toBe('not-an-object') + }) + + it('refuses a frame with no type', () => { + expect(fail({ sessionId: 's' }).reason).toBe('unknown-type') + }) + + it('refuses a type nothing declares', () => { + const r = fail({ type: 'input:teleport', sessionId: 's' }) + expect(r.reason).toBe('unknown-type') + if (r.reason === 'unknown-type') expect(r.type).toBe('input:teleport') + }) + + // **The universe is the inbound map, not every literal the protocol declares.** Eleven types are + // relay-produced and belong to no inbound direction. A browser that sends one is inert today — it + // reaches the switch and matches no case. Classifying it as a *direction* violation instead would + // route it to the 1008 close, disconnecting a dashboard over a frame that does nothing. + it.each(['session:joined', 'error', 'agents:listed', 'session:terminated', 'stream:registered'])( + 'reads the relay-produced %s as unknown rather than as a direction violation', + (type) => { + expect(fail({ type, sessionId: 's' }).reason).toBe('unknown-type') + }, + ) + + it('reports the type it refused on a shape failure, so a log can name it', () => { + const r = fail({ type: 'device:boot', sessionId: 's', requestId: 'r' }) + expect(r.reason).toBe('bad-shape') + if (r.reason === 'bad-shape') { + expect(r.type).toBe('device:boot') + expect(r.detail).toMatch(/payload/i) + } + }) +}) + +describe('an empty correlator is not a correlator', () => { + // The predicates this replaced (`isAddressed` / `isCorrelated`) rejected `''` as well as absence, + // and a bare `z.string()` accepts it. Losing that half is invisible to every type-level assertion: + // `.min(1)` does not change what `z.output` infers. What it costs is concrete — a `device:boot` + // with `requestId: ''` would pass the door, fail at `dispatchTarget`, and be answered with a + // `device:boot-error` carrying `requestId: ''`, a frame whose required correlator is + // present-but-empty and which every correlating consumer discards. + it('rejects an empty sessionId', () => { + expect(fail({ type: 'session:start', sessionId: '' }).reason).toBe('bad-shape') + }) + + it('rejects an empty requestId', () => { + const raw = { type: 'device:boot', sessionId: 's', requestId: '', payload: { deviceId: 'd' } } + expect(fail(raw).reason).toBe('bad-shape') + }) + + it('rejects a non-string sessionId', () => { + expect(fail({ type: 'session:start', sessionId: 7 }).reason).toBe('bad-shape') + }) + + // The mirror. Without it, a schema that rejected *every* sessionId would pass the two above. + it('accepts a real one', () => { + expect(ok({ type: 'session:start', sessionId: 's' }).msg).toEqual({ type: 'session:start', sessionId: 's' }) + }) +}) + +describe('an agent older than a field still registers', () => { + // `AgentRegister` declares `capabilities` and `devices` required, and an agent that predates either + // sends neither — which is how a viewer tells them apart. The relay carried `msg.capabilities ?? []` + // for exactly this. The `.default([])` moves that tolerance into the schema, where it is visible, + // and it is invisible to the type assertions because `z.output` is `string[]` either way. + it('accepts an agent that predates capabilities', () => { + const raw = { type: 'agent:register', platform: 'ios', agentName: 'mac-1', devices: [] } + expect(ok(raw).msg).toMatchObject({ capabilities: [], devices: [] }) + }) + + it('accepts an agent that reports no devices', () => { + const raw = { type: 'agent:register', platform: 'ios', agentName: 'mac-1', capabilities: ['clipboard'] } + expect(ok(raw).msg).toMatchObject({ capabilities: ['clipboard'], devices: [] }) + }) + + // The tolerance is for *absence*, not for a wrong shape — otherwise `.default()` would be + // indistinguishable from not checking the field at all. + it('still refuses capabilities that are not strings', () => { + const raw = { type: 'agent:register', platform: 'ios', agentName: 'm', capabilities: [{}] } + expect(fail(raw).reason).toBe('bad-shape') + }) + + it('defaults a screenshot format the way the relay used to', () => { + const raw = { type: 'screenshot:done', sessionId: 's', requestId: 'r', data: 'AAA' } + expect(ok(raw).msg).toMatchObject({ format: 'png' }) + }) +}) + +describe('the Envelope tier hands back no payload', () => { + // The tier's whole claim, at runtime. The static half is TC15's mutation — adding `payload` to an + // envelope schema — which the assertion catches at compile time; this is what a reader can see. + it('drops a chrome payload from the parse product while keeping it on the raw frame', () => { + const payload = { buttons: [], streamType: 'h264' } + const r = ok({ type: 'session:chrome', sessionId: 's', payload }) + expect(r.msg).toEqual({ type: 'session:chrome', sessionId: 's' }) + expect(r.raw['payload']).toBe(payload) + }) + + // Why the tier exists: `ChromePayload` is a closed two-member union while `AgentRegister.platform` + // is open by OCP, so a third-party platform has no valid variant to send. Validating this message + // would cost that platform its bezel and buttons for the life of the session — the message arrives + // once per boot, and a rejection skips the cache the re-join replay reads. + it('accepts a chrome payload belonging to neither declared variant', () => { + const payload = { kind: 'some-third-platform', frameSvg: '' } + const r = ok({ type: 'session:chrome', sessionId: 's', payload }) + expect(r.raw['payload']).toBe(payload) + }) + + it('accepts an unknown field on a forwarded reply, so a newer agent is not broken by an older relay', () => { + const raw = { type: 'input:done', sessionId: 's', requestId: 'r', hapticsApplied: true } + const r = ok(raw) + expect(r.msg).toEqual({ type: 'input:done', sessionId: 's', requestId: 'r' }) + expect(r.raw['hapticsApplied']).toBe(true) + }) + + // The envelope is still an envelope: the fields it does declare are checked. + it('refuses a forwarded reply whose correlator is missing', () => { + expect(fail({ type: 'input:done', sessionId: 's' }).reason).toBe('bad-shape') + }) +}) + +describe('a browser frame is stripped, because its product is what gets forwarded', () => { + // The browser direction is the attacker-controllable one, and `z.object` strips rather than + // rejects — so what makes an appended key harmless is that the relay forwards `msg` here and not + // `raw`. If that ever flips, this test still passes and the relay's own test is what fails, which + // is why the relay carries one too. + it('removes a key the schema does not declare', () => { + const raw = { + type: 'input:key', sessionId: 's', requestId: 'r', + payload: { code: 'KeyA', modifiers: 0, injected: 'rm -rf /' }, + extra: 'appended', + } + const r = ok(raw) + expect(r.msg).toEqual({ type: 'input:key', sessionId: 's', requestId: 'r', payload: { code: 'KeyA', modifiers: 0 } }) + expect(r.raw['extra']).toBe('appended') + }) + + it('keeps an optional payload absent rather than inventing one', () => { + const r = ok({ type: 'input:touch:end', sessionId: 's', requestId: 'r' }) + expect(r.msg).toEqual({ type: 'input:touch:end', sessionId: 's', requestId: 'r' }) + }) + + // **`buildId` is carried through as `NaN` rather than refused, and that is deliberate.** It is the one + // browser-side shape failure the relay *answers*: the handler checks `Number.isInteger` and replies + // `Build not found`, so a caller learns why instead of waiting out its deadline. Refusing the frame + // here deleted that answer — the door has no socket and no correlator policy, so it cannot answer in + // the handler's place, and six relay tests asserting "answers … without going silent" went silent. + // + // What the schema still buys is that better-sqlite3 never sees the object or array that made it + // *throw* — an exception the message loop swallowed, which is the silence `Number.isInteger` was + // added to remove in the first place. + it('turns an unusable buildId into NaN rather than refusing the frame', () => { + for (const buildId of [{}, [], '3', 1.5, null, undefined]) { + const r = ok({ type: 'app:install', sessionId: 's', requestId: 'r', buildId }) + expect(r.msg).toMatchObject({ type: 'app:install' }) + expect(Number.isInteger((r.msg as { buildId: number }).buildId)).toBe(false) + } + }) + + it('keeps a real buildId', () => { + expect(ok({ type: 'app:install', sessionId: 's', requestId: 'r', buildId: 3 }).msg).toMatchObject({ buildId: 3 }) + }) +}) + +describe('directionOf replaces the hand-written agent list', () => { + it('routes the handshake messages that decide a role', () => { + expect(directionOf('agent:register')).toBe('agent') + expect(directionOf('stream:register')).toBe('stream') + expect(directionOf('session:start')).toBe('browser') + }) + + it.each(['screenshot:done', 'ui:tree:response', 'session:chrome', 'clipboard:data', 'input:error'] as const)( + 'calls %s an agent message, as the 1008 gate did', + (type) => { expect(directionOf(type)).toBe('agent') }, + ) + + it.each(['agents:list', 'device:boot', 'input:touch:move', 'clipboard:read', 'session:leave'] as const)( + 'calls %s a browser message', + (type) => { expect(directionOf(type)).toBe('browser') }, + ) + + // The first frame of an agent connection is the case that broke the first design of this module: + // the role does not exist until this message is parsed, so a parser selecting a schema *by* role + // could never validate it. Parsing first is what makes the answer available at all. + it('answers for a type parsed before any role exists', () => { + const r = ok({ type: 'agent:register', platform: 'ios', agentName: 'm', capabilities: [], devices: [] }) + expect(directionOf(r.msg.type)).toBe('agent') + }) +}) diff --git a/packages/protocol/src/index.ts b/packages/protocol/src/index.ts index 456ec37d..a780d196 100644 --- a/packages/protocol/src/index.ts +++ b/packages/protocol/src/index.ts @@ -414,32 +414,29 @@ export interface DeviceReady { * `sessionId` is what makes a failure findable. An MCP caller waits for the reply that carries its own * sessionId, and without one it waits out the deadline instead (#445). * - * Required here is a **specification the relay does not yet meet**, not a description of the wire. - * Nothing validates inbound messages, so a client that sends `{"type":"input:touch:end"}` with no - * sessionId reaches `sessions.get(undefined)`, misses, and the relay answers through `msg.sessionId!` — - * `JSON.stringify` then drops the key, shipping a frame whose required field this declaration says is there. - * No in-repo client omits a sessionId, so the gap is reachable only from a third-party one — with one - * exception measured since: `sessionId: ''` type-checks and `mcp-server`'s tools take `z.string()`, so an LLM - * can produce it. + * **This is now a description of the wire, and for two releases it was a specification the relay did not + * meet.** Nothing validated inbound messages, so a client sending `{"type":"input:touch:end"}` with no + * sessionId reached `sessions.get(undefined)`, missed, and was answered through `msg.sessionId!` — + * `JSON.stringify` then dropped the key, shipping a frame whose required field this declaration claimed + * was there. `sessionId: ''` was the sharper half: it type-checks, and `mcp-server`'s tool schemas are + * bare `z.string()`, so an LLM could produce one. * - * **The request side is closed and the count below is not the one it used to be.** L5c's door predicates - * (`isCorrelated`, `isAddressed`) narrowed the handlers, which removed the seven reply sites this note used - * to enumerate; the line numbers it carried outlived them and pointed at unrelated code for one release. What - * is left is eleven `sessions.get(msg.sessionId!)` in `RelayServer.ts`, and it is **not** uniformly one kind: - * eight are agent→browser forwards (`session:chrome`, `session:deviceInfo`, `device:booting`, - * `device:boot-error`, `device:shutdown-done`, `device:ready`, `keyboard:toggled`, `clipboard:error`), and - * three are request-side paths that deliberately carry no address gate — `stream:register` on a stream-role - * socket, `device:shutdown` (whose gate a mutation showed nothing could hold, since an unaddressed shutdown - * is dropped by the session miss anyway), and `forwardUnacked`. Count them by reading the sites, not by - * trusting this sentence: a stale number here is what taught the lesson. + * #444 closed it at the door. `@tapflowio/protocol/validate` parses an inbound frame against these + * declarations before the relay routes it, with `.min(1)` on this field so the empty string is refused + * too — and **`RelayServer.ts` now contains no `msg.sessionId!` at all**, where it once held eleven. + * There is no count left to keep current here, which is the point: the assertion is gone rather than + * enumerated, and a stale number in this paragraph is what taught that lesson. * - * Optional would describe that wire accurately and still be the wrong contract: an MCP caller that - * receives an uncorrelatable `input:error` has nothing it can do with it — it waits out the deadline either - * way. The producer is what has to change, and #444 is the validator that makes it. An earlier version of - * this note said the producer should "send `error` instead", pointing at `GenericError` below as an escape - * hatch for a failure with no session. **That escape is gone**: `GenericError` requires `sessionId` since - * L5d, and the door predicates drop such a request rather than answering it, because a reply carrying no - * `requestId` cannot be attributed and costs the caller the same deadline silence would. + * One member still declares `sessionId?` — `DeviceReady`, and its own note says why. That is a reasoned + * deferral about a correlator, not a gap in this one. + * + * Optional would have described the old wire accurately and still been the wrong contract: an MCP caller + * that receives an uncorrelatable `input:error` has nothing it can do with it and waits out the deadline + * either way. An earlier version of this note said the producer should "send `error` instead", pointing at + * `GenericError` below as an escape hatch for a failure with no session. **That escape is gone**: + * `GenericError` requires `sessionId` since L5d, and a request naming no session is refused at the door + * rather than answered, because a reply carrying no `requestId` cannot be attributed and costs the caller + * the same deadline silence would. */ export interface SessionScoped { sessionId: string @@ -1106,7 +1103,8 @@ export interface AppLaunchToRelay { * describes a message nobody sends. * * What required buys is a **compile error for a future sender**, no more. The agents keep that branch and - * should: `bundleId: ''` type-checks, and nothing here validates inbound JSON (#444). So this is the same + * should: `bundleId: ''` type-checks — the door checks the *shape*, and an empty string is a valid one + * everywhere but the two correlation fields, which carry `.min(1)`. So this is the same * argument as `sessionId` below, not a stronger one. * * Unlike `app:install` / `app:launch`, which carry a `buildId` the relay resolves into a bundle id from the diff --git a/packages/protocol/src/validate/assert.ts b/packages/protocol/src/validate/assert.ts new file mode 100644 index 00000000..c853c1cc --- /dev/null +++ b/packages/protocol/src/validate/assert.ts @@ -0,0 +1,71 @@ +/** + * Type-level machinery that keeps the schemas in this directory tied to the interfaces in + * `../index.ts`. + * + * A schema file is a **second copy** of the contract, and a second copy drifts — that is the whole + * reason `@tapflowio/protocol` exists (see AGENTS.md). Every assertion here is a compile error at the + * declaration rather than a test somebody has to remember to run, and all of them cost nothing at + * runtime. + * + * Deliberately a copy of `Assert` / `IsEmpty` rather than an import from `relay/src/types.ts`: + * importing would invert the dependency — the relay depends on protocol, not the other way round. + * `../typeAssertions.ts` established that convention with its own `AssertTrue` / `NoOverlap` pair. + */ +import type * as z from 'zod' + +/** Fails to instantiate when its argument is not `true`. + * + * **`never` passes this** (`never extends true` is true), which is why every conditional below is + * bracketed. An unbracketed conditional distributes over a union and can answer `never` for an empty + * one — and that would be a silently passing assertion, which is worse than none. */ +export type Assert = T + +/** `[T] extends [never]`, not a bare `T extends never` — see the note on `Assert`. */ +export type IsEmpty = [T] extends [never] ? true : false + +/** `0 extends 1 & T` is true only for `any`: intersecting anything else with `1` cannot produce a type + * `0` is assignable to. */ +type IsAny = 0 extends 1 & T ? true : false + +/** + * Mutual assignability, with `any` refused on both sides. + * + * Without the `IsAny` arms this is vacuously `true` whenever either side is `any`, because `any` + * extends and is extended by everything. `z.any()` is the obvious way to hit that. + */ +export type Exact = + [IsAny] extends [true] ? false + : [IsAny] extends [true] ? false + : [A] extends [B] ? ([B] extends [A] ? true : false) : false + +/** + * What the Envelope tier is allowed to type: the interface projected onto the three fields the door + * actually checks. + * + * `Pick` rather than a hand-written shape, so the projection cannot disagree with the interface it + * projects — `DeviceShutdown`'s `requestId?` stays optional here, and `AgentsList`, which declares + * nothing but `type`, projects to `{ type }` and is therefore checked *exactly* rather than passing + * because there was nothing to compare. + */ +export type EnvelopeOf = + Pick> + +/** + * The one assertion both tiers use. Only the target differs: the interface for Validated, its + * `EnvelopeOf` projection for Envelope. + * + * **The `ZodObject` guard is the part `IsAny` cannot do.** A first design paired `Exact` with an + * `IsAny` rejection and named `z.custom()` among the holes it closed — it is not one. In zod 4.4.3 + * `custom(…): ZodCustom` (`v4/classic/schemas.d.ts:741`), so `z.output` is `O` with no `any` + * anywhere in the comparison, and `Exact` is `true` while the schema checks no structure at all. + * The same goes for the `const s: z.ZodType = …` annotation somebody reaches for when a schema is + * awkward. Neither is a `ZodObject`, so both are refused here by kind rather than by output. + * + * `$strip` is pinned, not incidental: it states in the type that unknown keys are removed rather than + * rejected, which is what the browser-direction forward depends on — the relay sends the parse product + * on that side precisely so a key an attacker added is gone by the time an agent sees the frame. + * Switching a schema to `z.strictObject` would refuse the whole frame instead, so it should be a + * decision with a reason, and this makes it a compile error until someone writes one. + */ +export type SchemaExact = + [S] extends [z.ZodObject] ? Exact, Target> : false diff --git a/packages/protocol/src/validate/index.ts b/packages/protocol/src/validate/index.ts new file mode 100644 index 00000000..5f8e6c7e --- /dev/null +++ b/packages/protocol/src/validate/index.ts @@ -0,0 +1,493 @@ +/** + * Runtime validation for everything the relay receives — the half of the contract types cannot hold. + * + * `../index.ts` is types only, and deliberately so: the dashboard consumes it with `import type`, so it + * must erase completely. This entry point is where the runtime lives, reached as + * `@tapflowio/protocol/validate`, and only the relay imports it. `../AGENTS.md` reserved this spot. + * + * ## Why this exists at all + * + * The outbound direction has been compile-checked since #419 — `sendTo` refuses a message outside its + * union. Nothing checked the inbound direction, so the relay reached for `msg.sessionId!` and + * `msg.payload as X` on values that arrived over a network. #550 asked for `RelayMessage` (a flat + * interface where `type` is the only required member) to become a discriminated union, and doing that + * with a bare `as` at the door would have been a **downgrade**: the visible cast at + * `RelayServer.ts:753` would have become an invisible `msg.payload`, with the compiler now vouching + * for an attacker's JSON. So the union has to be the product of a parse, and this is that parse. + * + * ## The rule the two tiers come from + * + * **A field that was not validated must not appear in the type.** Anything else reintroduces the lie + * in a quieter place. + * + * - **Validated** — the full schema; parsing yields the interface. Used where the relay reads or acts + * on the message. + * - **Envelope** — `type` plus whichever of `sessionId` / `requestId` the interface declares; parsing + * yields `EnvelopeOf`, so the relay **cannot** read a payload it did not check. Used where the + * relay only forwards. + * + * ## Why the whole agent direction is Envelope except the six it consumes + * + * Not laziness, and not a deferral: agent-payload conformance is a property the protocol deliberately + * does not have. `AgentRegister.platform` is `string` — open, because a third-party platform registers + * through `AgentRegistry.register()` and the root AGENTS.md's OCP rule says that must work without + * modifying existing code — while `ChromePayload` is a **closed two-member union**. So a platform this + * repo promises to support has no valid `session:chrome` variant to send. Rejecting one would cost + * that platform its bezel and buttons permanently: the message arrives once per boot, a rejection + * skips `setChromeData` and therefore empties the re-join replay too, and there is no + * `session:chrome-error` for anyone to be told through. + * + * The six the relay consumes (`agent:register`, `agent:resources`, the two screenshot replies and the + * two ui-tree replies) forward nowhere, so rejecting them breaks no forward path — and their schemas + * carry a `.default()` for every field the relay currently reads through a `??`, which keeps today's + * tolerance for an older agent exactly as it is. `UIElement` is safe to validate where `ChromePayload` + * is not, for a reason worth stating: it is not a per-platform union but the normalized shape every + * platform maps *into*, so a third-party agent conforms by construction. + * + * ## What is not checked here, and where it is + * + * Role authorisation. `directionOf` answers which socket may send a type; deciding what to do about a + * mismatch (the relay closes a browser socket with 1008) stays in `RelayServer`, because it is a + * policy about a connection rather than a fact about a message. + */ +import * as z from 'zod' + +import type { + AgentRegister, AgentResourceReport, AgentsList, AppClearState, AppClearStateDone, + AppClearStateError, AppInstallDone, AppInstallError, AppInstallToRelay, AppLaunchDone, + AppLaunchError, AppLaunchToRelay, ClipboardData, ClipboardError, ClipboardRead, ClipboardWrite, + ClipboardWriteDone, DeviceBoot, DeviceBootError, DeviceBooting, DeviceReady, DeviceShutdown, + DeviceShutdownDone, InputButton, InputDone, InputError, InputKey, InputKeyboardToggle, + InputPinchEnd, InputPinchMove, InputPinchStart, InputRotate, InputTouchEnd, InputTouchMove, + InputTouchStart, InputType, InputTypeDone, InputTypeError, KeyboardToggled, OpenUrl, OpenUrlDone, + OpenUrlError, ScreenshotDone, ScreenshotError, SessionChrome, SessionDeviceInfo, SessionEnd, + SessionLeave, SessionStart, StreamRegister, UiTreeError, UiTreeResponse, +} from '../index.js' +import type { Assert, EnvelopeOf, IsEmpty, SchemaExact } from './assert.js' + +// ── field helpers ──────────────────────────────────────────────────────────── +// +// `.min(1)`, never a bare `z.string()`, on both correlation fields. The predicates this replaces +// (`isAddressed` / `isCorrelated`) rejected the empty string as well as the absent one, and dropping +// that half would be a silent regression: a `device:boot` carrying `requestId: ''` would pass the door +// and the relay would answer `device:boot-error` with `requestId: ''` — a frame whose required +// correlator is present-but-empty, which every correlating consumer discards. `.min(1)` does not +// change what `z.output` infers (`min(minLength): this` — zod 4.4.3 `v4/classic/schemas.d.ts:95`), so +// it costs the tier assertions nothing. +const sessionId = z.string().min(1) +const requestId = z.string().min(1) + +const point = z.object({ x: z.number(), y: z.number() }) + +// ── envelope-tier builders ─────────────────────────────────────────────────── +// +// Four shapes cover all 22 forward-only messages. Getting one wrong is a compile error at its +// assertion below rather than something that shows up as a dropped frame in production. + +/** `{ type, sessionId }` — no correlator on the interface. */ +const env = (type: T) => z.object({ type: z.literal(type), sessionId }) +/** `{ type, sessionId, requestId }`. */ +const envC = (type: T) => z.object({ type: z.literal(type), sessionId, requestId }) +/** `{ type, sessionId, requestId? }` — the relay originates some of these itself, so the agent's copy + * is not always a reply. */ +const envCo = (type: T) => + z.object({ type: z.literal(type), sessionId, requestId: requestId.optional() }) + +// ── browser → relay ────────────────────────────────────────────────────────── +// +// Fully validated, and the only direction whose parse product is what gets forwarded. This is the +// attacker-controllable side: a viewer can send arbitrary frames from devtools. `z.object` strips +// unknown keys rather than rejecting them, so forwarding the product — not the original — is what +// makes a key an attacker appended disappear before any agent sees it. Nothing in the repo loses a +// field to that: the dashboard, `mcp-server` and `flow-runner` all send through a +// `send(msg: BrowserToRelay)` signature, so their frames are already compile-checked against exactly +// these shapes. + +const BROWSER_INBOUND = { + 'agents:list': z.object({ type: z.literal('agents:list') }), + 'session:start': z.object({ type: z.literal('session:start'), sessionId }), + 'session:end': z.object({ type: z.literal('session:end'), sessionId }), + 'session:leave': z.object({ type: z.literal('session:leave'), sessionId }), + 'device:boot': z.object({ + type: z.literal('device:boot'), + sessionId, + requestId, + payload: z.object({ + deviceId: z.string(), + resetMode: z.enum(['app-only', 'full-erase']).optional(), + acceptH264: z.boolean().optional(), + secureContext: z.boolean().optional(), + }), + }), + 'device:shutdown': z.object({ + type: z.literal('device:shutdown'), + sessionId, + requestId: requestId.optional(), + payload: z.object({ deviceId: z.string() }), + }), + // **`.catch(NaN)` rather than a plain `z.number().int()`, and the reason is a measured regression.** + // + // A bad `buildId` is the one browser-side shape failure the relay *answers* today: the handler checks + // `Number.isInteger` and replies `Build not found`, so a caller learns why instead of waiting out its + // deadline. Rejecting the frame at the door deleted that answer — the parse fails, `route` never runs, + // and six tests that assert "answers … without going silent" went silent. The door has no socket and + // no correlator policy, so it cannot answer in the handler's place. + // + // So the schema carries the value through as `NaN` and the handler keeps answering. `z.output` is + // still `number`, which is what the tier assertion compares, and better-sqlite3 never sees the + // object or array that made it throw — that exception, swallowed by the message-loop catch, is the + // silence `Number.isInteger` was added to remove in the first place. + 'app:install': z.object({ + type: z.literal('app:install'), sessionId, requestId, buildId: z.number().int().catch(Number.NaN), + }), + 'app:launch': z.object({ + type: z.literal('app:launch'), sessionId, requestId, buildId: z.number().int().catch(Number.NaN), + }), + 'app:clear-state': z.object({ + type: z.literal('app:clear-state'), sessionId, requestId, + payload: z.object({ bundleId: z.string() }), + }), + 'open-url': z.object({ + type: z.literal('open-url'), sessionId, requestId, payload: z.object({ url: z.string() }), + }), + 'input:touch:start': z.object({ type: z.literal('input:touch:start'), sessionId, payload: point }), + 'input:touch:move': z.object({ type: z.literal('input:touch:move'), sessionId, payload: point }), + // `payload` optional because that is what the wire carries — the dashboard omits it and the agents + // never read it. See the note on `InputTouchEnd`. + 'input:touch:end': z.object({ + type: z.literal('input:touch:end'), sessionId, requestId, payload: point.optional(), + }), + 'input:pinch:start': z.object({ + type: z.literal('input:pinch:start'), sessionId, payload: z.object({ f0: point, f1: point }), + }), + 'input:pinch:move': z.object({ + type: z.literal('input:pinch:move'), sessionId, payload: z.object({ f0: point, f1: point }), + }), + 'input:pinch:end': z.object({ type: z.literal('input:pinch:end'), sessionId, requestId }), + // `modifiers` is a bitmap, not a list — see the note on `InputKey`. + 'input:key': z.object({ + type: z.literal('input:key'), sessionId, requestId, + payload: z.object({ code: z.string(), modifiers: z.number().optional() }), + }), + 'input:type': z.object({ + type: z.literal('input:type'), sessionId, requestId, payload: z.object({ text: z.string() }), + }), + 'input:button': z.object({ + type: z.literal('input:button'), sessionId, requestId, + payload: z.object({ name: z.string(), phase: z.enum(['down', 'up']).optional() }), + }), + 'input:rotate': z.object({ type: z.literal('input:rotate'), sessionId }), + 'input:keyboard:toggle': z.object({ type: z.literal('input:keyboard:toggle'), sessionId }), + 'clipboard:read': z.object({ + type: z.literal('clipboard:read'), sessionId, requestId, + payload: z.object({ press: z.enum(['copy', 'cut']).optional() }).optional(), + }), + 'clipboard:write': z.object({ + type: z.literal('clipboard:write'), sessionId, requestId, + payload: z.object({ text: z.string(), pasteAfter: z.boolean().optional() }), + }), +} as const + +// ── agent → relay ──────────────────────────────────────────────────────────── + +/** The six the relay consumes. Every `.default()` below mirrors a `??` that is in `RelayServer` today, + * so an agent older than a field keeps working exactly as it does now — the default lands in the + * schema, where it is visible, instead of at the read site, where it read as defensive noise. + * + * This is what makes rejection affordable here: `z.input` stays as loose as the wire has ever been + * while `z.output` matches the interface, which is what the tier assertion compares. */ +const AGENT_CONSUMED = { + 'agent:register': z.object({ + type: z.literal('agent:register'), + platform: z.string(), + // Required on the interface; absent from agents that predate the field, which is how a viewer + // tells them apart. `RelayServer` carried `msg.capabilities ?? []` for exactly this. + capabilities: z.array(z.string()).default([]), + agentId: z.string().optional(), + agentName: z.string(), + // Deduplication by device id stays in the handler — it is a policy about the *set*, not a shape. + devices: z.array(z.object({ + id: z.string(), name: z.string(), platform: z.string(), status: z.string(), + osVersion: z.string().optional(), + })).default([]), + }), + 'agent:resources': z.object({ + type: z.literal('agent:resources'), + resources: z.object({ + cpuPercent: z.number(), memUsedMB: z.number(), memTotalMB: z.number(), + slotsAvailable: z.number(), slotsTotal: z.number(), reportedAt: z.number(), + }), + }), + 'screenshot:done': z.object({ + type: z.literal('screenshot:done'), sessionId, requestId, + // The claim, not the truth — the relay sniffs the bytes and logs a mismatch rather than + // overwriting this, because only the agent knows what it produced (#508). + format: z.enum(['png', 'jpeg']).default('png'), + data: z.string().default(''), + }), + 'screenshot:error': z.object({ + type: z.literal('screenshot:error'), sessionId, requestId, message: z.string().default(''), + }), + 'ui:tree:response': z.object({ + type: z.literal('ui:tree:response'), sessionId, requestId, + // Safe to validate where `ChromePayload` is not: a normalized shape every platform maps into, + // not a union with one member per platform. + elements: z.array(z.object({ + role: z.enum([ + 'button', 'text', 'input', 'image', 'checkbox', 'switch', 'slider', 'list', 'cell', 'tab', 'other', + ]), + label: z.string(), + identifier: z.string().optional(), + frame: z.object({ x: z.number(), y: z.number(), width: z.number(), height: z.number() }), + enabled: z.boolean(), + rawRole: z.string().optional(), + })).default([]), + }), + 'ui:tree:error': z.object({ + type: z.literal('ui:tree:error'), sessionId, requestId, message: z.string().default(''), + }), +} as const + +/** The 22 the relay only forwards. Envelope tier — see the header for why there is no exception. */ +const AGENT_FORWARDED = { + 'session:chrome': env('session:chrome'), + 'session:deviceInfo': env('session:deviceInfo'), + // The only member whose `sessionId` is optional, and it is a documented deferral rather than an + // oversight — stamping it would make a replayed ready satisfy an in-flight boot. See `DeviceReady`. + 'device:ready': z.object({ + type: z.literal('device:ready'), + sessionId: sessionId.optional(), + requestId: requestId.optional(), + }), + 'device:booting': env('device:booting'), + 'device:shutdown-done': envCo('device:shutdown-done'), + 'device:boot-error': envCo('device:boot-error'), + 'app:install-done': envC('app:install-done'), + 'app:install-error': envC('app:install-error'), + 'app:launch-done': envC('app:launch-done'), + 'app:launch-error': envC('app:launch-error'), + 'app:clear-state-done': envC('app:clear-state-done'), + 'app:clear-state-error': envC('app:clear-state-error'), + 'open-url:done': envC('open-url:done'), + 'open-url:error': envC('open-url:error'), + 'input:done': envC('input:done'), + 'input:error': envC('input:error'), + 'input:type-done': envC('input:type-done'), + 'input:type-error': envC('input:type-error'), + 'keyboard:toggled': env('keyboard:toggled'), + 'clipboard:data': envC('clipboard:data'), + 'clipboard:write-done': envC('clipboard:write-done'), + 'clipboard:error': envC('clipboard:error'), +} as const + +const AGENT_INBOUND = { ...AGENT_CONSUMED, ...AGENT_FORWARDED } as const + +// ── stream → relay ─────────────────────────────────────────────────────────── +// +// One message, and its own direction on purpose: the relay assigns the role `'stream'` from it, so +// merging it into the agent direction would let a control socket claim to be a session's stream +// socket. Everything else on that socket is binary and never reaches this parser. +const STREAM_INBOUND = { + 'stream:register': z.object({ type: z.literal('stream:register'), sessionId }), +} as const + +const INBOUND = { ...BROWSER_INBOUND, ...AGENT_INBOUND, ...STREAM_INBOUND } as const + +// ── what the door proved ───────────────────────────────────────────────────── + +/** + * The parse product, derived from the map — **not** a union of the protocol's interfaces. + * + * An earlier draft returned `BrowserToRelay | AgentToRelay | AgentToBrowser | StreamToRelay`, which + * violates this file's own opening rule with its return type: narrowing that by + * `msg.type === 'session:chrome'` hands back `payload: ChromePayload`, fully typed, with nothing + * having checked it — worse than the `as` it replaced, because a cast can at least be grepped. + * Deriving from `INBOUND` means an Envelope member arrives as `{ type, sessionId }` and reading + * `msg.payload` off it is a compile error, which is the claim the tiers make. + */ +export type ParsedInbound = { [K in keyof typeof INBOUND]: z.output<(typeof INBOUND)[K]> }[keyof typeof INBOUND] + +export type InboundType = keyof typeof INBOUND +export type InboundDirection = 'browser' | 'agent' | 'stream' + +export type ParseFailure = + | { ok: false; reason: 'not-an-object' } + | { ok: false; reason: 'unknown-type'; type: string } + | { ok: false; reason: 'bad-shape'; type: InboundType; detail: string } + +export type ParseResult = + | { + ok: true + msg: ParsedInbound + /** The frame as it arrived. + * + * Forwarding differs by direction and this is why both are available. An **agent**-origin + * message is forwarded as this, so a field a newer agent added survives a relay that does not + * know it — `z.object` strips, and stripping here would break upward compatibility in the one + * direction where the sender is the more recently updated side. A **browser**-origin message is + * forwarded as `msg` instead, so a key an attacker appended does not survive. + * + * It is also where the two Envelope payloads the relay stores come from, and it should stay + * visibly separate for that reason: a value read off `raw` is a value the parser did not + * vouch for. */ + raw: Readonly> + } + | ParseFailure + +const DIRECTIONS: ReadonlyMap = new Map([ + ...Object.keys(BROWSER_INBOUND).map((t) => [t, 'browser'] as const), + ...Object.keys(AGENT_INBOUND).map((t) => [t, 'agent'] as const), + ...Object.keys(STREAM_INBOUND).map((t) => [t, 'stream'] as const), +]) + +/** + * Which socket role is allowed to send this type. + * + * This replaces `AGENT_MSG_TYPE_LIST` — 29 literals hand-copied into `RelayServer` and held by two + * type assertions. It is derived from the map above, which the assertions at the bottom of this file + * tie back to the protocol's own direction unions, so the copy is gone rather than moved. + * + * A runtime set cannot come out of a union directly (types erase), which is why it comes out of the + * schema map's keys. That makes the relay's role gate depend on this map existing — a real coupling, + * and the reason the coverage assertions below are not optional. + */ +export function directionOf(type: InboundType): InboundDirection { + // Non-null: `DIRECTIONS` is built from the same keys `InboundType` is derived from, and the + // coverage assertions below make a divergence a compile error rather than a runtime miss. + return DIRECTIONS.get(type)! +} + +/** + * Parse a frame the relay received. + * + * **Takes no role.** A first design did, and it could not be called: `classifyConnection` returns + * `role: 'first-message'` for every local connection and every remote agent-scoped PAT, so the role is + * assigned from the first message's own `type`. `agent:register` is the message that *creates* the + * role, so selecting a schema by role to validate it is circular — and defaulting the unknown role to + * `'browser'`, which is what the relay does for a socket that skips the handshake, would have closed + * every agent's socket on its opening frame. The order is parse, then role, then gate. + */ +export function parseInbound(raw: unknown): ParseResult { + // `JSON.parse` returns bare `null`, numbers and strings without throwing, and a caller that reads + // `.type` off one of those is the reason this is checked before anything else. + if (typeof raw !== 'object' || raw === null || Array.isArray(raw)) return { ok: false, reason: 'not-an-object' } + const frame = raw as Record + const type = frame['type'] + if (typeof type !== 'string' || !Object.hasOwn(INBOUND, type)) { + return { ok: false, reason: 'unknown-type', type: typeof type === 'string' ? type : String(type) } + } + const known = type as InboundType + // **The universe is the inbound map, not `AnyWireMessage`.** Eleven literals belong to no inbound + // direction — `session:joined`, `error`, `agents:listed` and the rest the relay produces. A browser + // that sends one today reaches the switch, matches no case and is ignored. Measuring `unknown-type` + // against every declared literal instead would classify those as a direction violation, and the + // relay closes a direction violation with 1008 — disconnecting a dashboard over a frame that is + // inert today. + const result = INBOUND[known].safeParse(frame) + if (!result.success) { + return { ok: false, reason: 'bad-shape', type: known, detail: z.prettifyError(result.error) } + } + return { ok: true, msg: result.data as ParsedInbound, raw: frame } +} + +// ── the map is tied to the protocol, both ways ─────────────────────────────── +// +// Everything below is type-level and erases. `satisfies` reports at the literal; these report at the +// declaration and survive the map being moved or re-exported, which is the same belt-and-braces shape +// `relay/src/types.ts` uses for its own membership claims. +// +// Note what is deliberately NOT written here: `const INBOUND: Record<…, z.ZodType>`. Annotating the +// map erases each entry's schema type, so `z.output` would answer `unknown` and every tier assertion +// below would pass while checking nothing. + +type BrowserKeys = keyof typeof BROWSER_INBOUND +type AgentKeys = keyof typeof AGENT_INBOUND +type StreamKeys = keyof typeof STREAM_INBOUND + +/** Tier and direction membership, stated rather than conventional: giving `device:boot` an envelope + * schema, or filing an agent reply under the browser direction, is a compile error here instead of + * something noticed when a relay line fails to read a payload. */ +type _BrowserCovers = Assert>> +type _BrowserInventsNothing = Assert>> +type _AgentCovers = Assert< + IsEmpty> +> +type _AgentInventsNothing = Assert< + IsEmpty> +> +type _StreamCovers = Assert>> +type _StreamInventsNothing = Assert>> + +// ── each schema against the interface it mirrors ───────────────────────────── +// +// The Validated tier compares against the interface; the Envelope tier against `EnvelopeOf`, which +// is the interface projected onto the fields the door checks. One assertion kind for both, because the +// projection is derived from the interface and so cannot disagree with it. +// +// An earlier design gave the Envelope tier a weaker assertion — "the interface is assignable to what I +// parsed" — and it was vacuous: adding `payload: z.unknown()` to an envelope schema passed, as did +// `z.any()`, `z.custom()` and an empty `z.object({})`, which is to say every idiomatic way of +// making the mistake it was written to catch. + +// `Assert` is applied at each use below rather than inside these two, because a constraint on a +// generic alias is checked against the *unresolved* parameter — `SchemaExact<…[K], I>` for an +// unknown `K` is not provably `true`, so the alias would fail to declare while saying nothing about +// any actual pair. +type V = SchemaExact<(typeof INBOUND)[K], I> +type E = SchemaExact<(typeof INBOUND)[K], EnvelopeOf> + +type _AgentsList = Assert> +type _SessionStart = Assert> +type _SessionEnd = Assert> +type _SessionLeave = Assert> +type _DeviceBoot = Assert> +type _DeviceShutdown = Assert> +type _AppInstall = Assert> +type _AppLaunch = Assert> +type _AppClearState = Assert> +type _OpenUrl = Assert> +type _InputTouchStart = Assert> +type _InputTouchMove = Assert> +type _InputTouchEnd = Assert> +type _InputPinchStart = Assert> +type _InputPinchMove = Assert> +type _InputPinchEnd = Assert> +type _InputKey = Assert> +type _InputType = Assert> +type _InputButton = Assert> +type _InputRotate = Assert> +type _InputKeyboardToggle = Assert> +type _ClipboardRead = Assert> +type _ClipboardWrite = Assert> + +type _AgentRegister = Assert> +type _AgentResources = Assert> +type _ScreenshotDone = Assert> +type _ScreenshotError = Assert> +type _UiTreeResponse = Assert> +type _UiTreeError = Assert> + +type _StreamRegister = Assert> + +type _SessionChrome = Assert> +type _SessionDeviceInfo = Assert> +type _DeviceReady = Assert> +type _DeviceBooting = Assert> +type _DeviceShutdownDone = Assert> +type _DeviceBootError = Assert> +type _AppInstallDone = Assert> +type _AppInstallError = Assert> +type _AppLaunchDone = Assert> +type _AppLaunchError = Assert> +type _AppClearStateDone = Assert> +type _AppClearStateError = Assert> +type _OpenUrlDone = Assert> +type _OpenUrlError = Assert> +type _InputDone = Assert> +type _InputError = Assert> +type _InputTypeDone = Assert> +type _InputTypeError = Assert> +type _KeyboardToggled = Assert> +type _ClipboardData = Assert> +type _ClipboardWriteDone = Assert> +type _ClipboardError = Assert> diff --git a/packages/protocol/tsconfig.assertions.json b/packages/protocol/tsconfig.assertions.json index 13dc5706..2a2d6e09 100644 --- a/packages/protocol/tsconfig.assertions.json +++ b/packages/protocol/tsconfig.assertions.json @@ -13,5 +13,5 @@ "sourceMap": false }, "include": ["src"], - "exclude": ["dist", "node_modules"] + "exclude": ["dist", "node_modules", "src/__tests__"] } diff --git a/packages/protocol/tsconfig.json b/packages/protocol/tsconfig.json index ae71cd07..31fe5215 100644 --- a/packages/protocol/tsconfig.json +++ b/packages/protocol/tsconfig.json @@ -20,6 +20,7 @@ "exclude": [ "dist", "node_modules", - "src/typeAssertions.ts" + "src/typeAssertions.ts", + "src/__tests__" ] } diff --git a/packages/relay/AGENTS.md b/packages/relay/AGENTS.md index 11ec422e..6cfc0ea0 100644 --- a/packages/relay/AGENTS.md +++ b/packages/relay/AGENTS.md @@ -33,10 +33,17 @@ iOS build format: `.app.zip` **or** `.tar.gz`/`.tgz` (EAS `eas build` simulator ## HOW - The agent connects to the relay via outbound WebSocket first (the key to NAT traversal). -- **Auth boundary**: connections from `localhost` are unauthenticated; every other origin must authenticate — browsers by JWT cookie / PAT, agents by a PAT with the `agent` scope (`Authorization: Bearer`). The role (browser / agent / stream) is decided in `classifyConnection` (`lib/connectionAuth.ts`); a `browser`-role socket that sends an agent-only message (`AGENT_MSG_TYPES`, which includes `stream:register`) is closed with 1008. +- **Auth boundary**: connections from `localhost` are unauthenticated; every other origin must authenticate — browsers by JWT cookie / PAT, agents by a PAT with the `agent` scope (`Authorization: Bearer`). The role (browser / agent / stream) is decided in `classifyConnection` (`lib/connectionAuth.ts`); a `browser`-role socket that sends an agent-only message (`directionOf` from `@tapflowio/protocol/validate`, which counts `stream:register` as its own direction) is closed with 1008. That set used to be `AGENT_MSG_TYPES`, a hand-copied array of 29 literals held against the protocol by two type assertions; it is derived now, and the derived set was verified member for member against the array it replaced. + + **The role and direction are settled before the shape is, and the order is load-bearing.** A malformed frame is dropped, but an agent-only type a browser spoofed *badly* must still close the socket — gating after shape validation lets such a spoofer keep its connection. The direction is a fact about the `type` alone, which is known even when the payload is not. A failed handshake is the one exception: it confers no role, so an agent whose `agent:register` does not parse gets its frame dropped rather than a `Forbidden` close, and its next attempt still introduces it. - JSON messages and binary frames share the same WebSocket connection, branched by the `isBinary` flag. - Control message protocol: `input:touch:*`, `input:pinch:*`, `input:button`, `input:key`, `input:type`, `input:rotate`, `input:keyboard:toggle`, `device:boot`, `device:shutdown`, `session:start`, `session:end`, `clipboard:read`, `clipboard:write`. -- **The message shapes live in [`@tapflowio/protocol`](../protocol/AGENTS.md), not here.** Every message the relay *originates* goes through `sendTo(socket, msg: RelayOutbound)`, so adding one means adding it to that union first — the compiler will not let you do it in the other order. Messages the relay only *forwards* keep their inbound type and are re-serialised unchanged. +- **The message shapes live in [`@tapflowio/protocol`](../protocol/AGENTS.md), not here.** Every message the relay *originates* goes through `sendTo(socket, msg: RelayOutbound)`, so adding one means adding it to that union first — the compiler will not let you do it in the other order. **Inbound frames are parsed into a discriminated union at the door** (`parseInbound`), not cast — see the header of `protocol/src/validate/`. Which frame gets forwarded differs by direction and is checked by `scripts/__tests__/browserInboundRouting.test.mjs`: + + - **agent → browser: the original frame (`raw`).** `z.object` strips undeclared keys, so forwarding the parse product would delete a field a newer agent added — the one direction where the sender is the more recently updated side. + - **browser → agent: the parse product (`msg`).** Here the stripping is the point: a key a viewer appended from devtools is gone before any agent sees it. + + Agent payloads are **deliberately not validated**, and the reason is not a deferral. `AgentRegister.platform` is `string`, open so a third-party platform can register through `AgentRegistry.register()` (OCP), while `ChromePayload` is a closed two-member union — so a platform this repo promises to support has no valid `session:chrome` variant, and refusing one would cost it bezel and buttons for the life of the session. The six messages the relay *consumes* (`agent:register`, `agent:resources`, the screenshot and ui-tree replies) are validated, with a `.default()` for every field the relay used to read through a `??`. - **Clipboard bridge** (`clipboard:*`): browser→agent `clipboard:read` (`payload.press`: `'copy' | 'cut'` presses that chord on the device first) and `clipboard:write` (`payload.text`, `payload.pasteAfter`); agent→browser `clipboard:data` / `clipboard:write-done` / `clipboard:error`, correlated by `requestId`. Unlike the other agent→browser replies these are **bound to the session's own `agentSocket`** — their payload lands on the viewer's host OS clipboard, so a second agent must not be able to address someone else's session. An undeliverable request answers `clipboard:error` immediately rather than letting the caller's deadline expire. Agents advertise `capabilities: ['clipboard']` in `agent:register`; the relay echoes them on `session:joined` so a viewer can tell a capable agent from one that predates the feature instead of inferring it from silence. - **An input the relay cannot dispatch is answered here, with a reason.** The four terminal frames get an `input:error` and `input:type` gets an `input:type-error`, so an MCP or browser caller fails now instead of diff --git a/packages/relay/src/RelayServer.ts b/packages/relay/src/RelayServer.ts index 9c7dd84b..de5488fd 100644 --- a/packages/relay/src/RelayServer.ts +++ b/packages/relay/src/RelayServer.ts @@ -7,10 +7,10 @@ import { randomUUID } from 'crypto' import { WebSocketServer, WebSocket } from 'ws' import { SessionManager } from './SessionManager.js' import type { Session } from './SessionManager.js' -import type { Assert, DeviceDetails, IsEmpty, RelayMessage, UIElement } from './types.js' -import type { - AgentControlOutbound, ChromePayload, InputErrorReason, RelayOutbound, StreamToRelay, -} from '@tapflowio/protocol' +import type { DeviceDetails, UIElement } from './types.js' +import type { ChromePayload, InputErrorReason, RelayOutbound } from '@tapflowio/protocol' +import { directionOf, parseInbound } from '@tapflowio/protocol/validate' +import type { ParsedInbound, ParseFailure, ParseResult } from '@tapflowio/protocol/validate' import { Router, json } from './router.js' import { requireViewAuth, requireAuth, getAuth, verifyPat } from './middleware/auth.js' import { classifyConnection } from './lib/connectionAuth.js' @@ -112,18 +112,6 @@ const RESOURCE_THRESHOLD = Number.isFinite(_parsedThreshold) ? _parsedThreshold // Terminal input messages the MCP client awaits an ack for — if the agent is offline // the relay replies input:error so the client fails truthfully (non-terminal moves/starts // expect no ack and are dropped silently). -/** A request that carries a usable correlator. - * - * A predicate rather than a bare `typeof` at each site, for two measured reasons. A bare check in the - * `switch` narrows the *property* and not the object, so `this.handleBrowserAppInstall(ws, msg)` still - * sees `requestId?: string` and fails to compile. And narrowing does **not** survive into a nested - * function, so a `fail()` closure built after the check sees `string | undefined` again — whose shortest - * fix is `msg.requestId!`, the write removed in `e98abd4` precisely because it puts a frame with an - * absent required field on the wire. This carries the narrowing through the handler signature instead, - * so neither a `const` copy nor an assertion is needed. - * - * Empty string counts as absent: it type-checks against a required `string`, nothing validates inbound - * JSON (#444), and `mcp-server`'s tool schemas are bare `z.string()`. */ /** Why a session is not this socket's to command. One reason, two prose strings — the treatment `#492` * settled for `agent offline` / `Session not found`: telling a caller the session is in use when it is * idle steers it off a device it could have had. Shared so the two never drift. */ @@ -131,98 +119,43 @@ function ownershipRefusal(session: Session): string { return session.browserSocket ? 'session held by another client' : 'session not joined' } -/** A request that names a session, with the same policy as `isCorrelated` and for the same reason. - * - * Nothing validates inbound JSON (#444), so a third-party client — or an LLM driving `mcp-server`, whose - * tool schemas are bare `z.string()` — can send a command with no `sessionId` or an empty one. Every reply - * these doors produce declares `sessionId` **required**, so answering means putting a frame on the wire - * whose required field `JSON.stringify` erases, which every consumer's session gate then discards. +/** + * What the door refused, said once and in a form an operator can act on. * - * **So the request is dropped, not answered.** The base's doc argued the opposite until L5d — "the - * only correct thing for it to send with no sessionId is `{ type: 'error' }`" — and its own premise refutes - * it: `GenericError` has no `requestId`, so a caller that receives one cannot attribute it and waits out the - * same deadline it would have waited out on silence. Answering buys nothing it did not already have; not - * shipping a frame that violates its own declaration is the whole payoff, and dropping achieves that more - * cheaply. L5d then made `error` require `sessionId`, so the escape it named no longer exists at all. + * This is where `isAddressed` and `isCorrelated` ended up. Both were predicates whose whole + * observable output was a `console.warn` — an id-less request resolved no session and was dropped by + * the miss anyway — and the schemas now reject the same frames earlier, including the empty-string + * case a bare `z.string()` would have let through. What is new is that the log names the *field*: + * "requestId: Too small" instead of "dropped, cannot correlate a reply". * - * The general form of that argument — "a caller cannot attribute an unaddressed answer" — is **narrower than - * it sounds**, and worth stating so nobody builds on the wide version. A dashboard viewer holds one session - * per socket, so an unaddressed reply would land somewhere sensible there; `SessionList` attributes a - * shutdown by single-slot convention with no correlator at all. What makes dropping right here is not that - * attribution is impossible for every consumer, but that the frame would violate its own declaration and be - * discarded by each consumer's session gate before any of them looked. + * Worth logging at all for the reason `isCorrelated` gave: the three places a bad frame can be + * dropped are otherwise silent, and an operator who upgrades the relay but not an independently + * installed `mcp-server` would watch commands do nothing with no trace. * - * Widening `SessionStartFailure` to carry an "unaddressed" reason was the alternative, and it is what L5d - * is for: that union's own doc says it has a single producer in `handleSessionStart`, and adding a member - * here would make that false while pre-deciding what `error` is. */ -function isAddressed(msg: RelayMessage): msg is RelayMessage & { sessionId: string } { - if (typeof msg.sessionId === 'string' && msg.sessionId !== '') return true - console.warn(`[tapflow] ${msg.type} without a usable sessionId — dropped, no reply could be addressed`) - return false -} - -function isCorrelated(msg: RelayMessage): msg is RelayMessage & { requestId: string } { - if (typeof msg.requestId === 'string' && msg.requestId !== '') return true - // Logged, because the three places this can be dropped are all silent otherwise: the relay `break`s, the - // agents' own guards never see the frame, and the browser gate `return`s. An operator who upgrades the - // relay but not an independently installed `mcp-server` would watch commands do nothing with no trace. - console.warn(`[tapflow] ${msg.type} without a usable requestId — dropped, cannot correlate a reply`) - return false + * `unknown-type` stays at debug: eleven relay-produced literals land here whenever a client echoes + * one back, and none of them is a defect. + */ +function logInboundRejection(failure: ParseFailure): void { + if (failure.reason === 'not-an-object') return + if (failure.reason === 'unknown-type') { + logger.debug(`[tapflow] inbound frame of unknown type ${failure.type} — dropped`) + return + } + logger.warn(`[tapflow] inbound ${failure.type} does not match the contract — dropped:\n${failure.detail}`) } - -const AGENT_MSG_TYPE_LIST = [ - 'agent:register', 'agent:resources', 'screenshot:done', 'screenshot:error', - 'ui:tree:response', 'ui:tree:error', - 'app:clear-state-done', 'app:clear-state-error', - 'device:booting', 'device:boot-error', 'device:shutdown-done', 'device:ready', - 'session:chrome', 'session:deviceInfo', - 'app:install-done', 'app:install-error', 'app:launch-done', 'app:launch-error', - 'open-url:done', 'open-url:error', 'keyboard:toggled', - 'input:type-done', 'input:type-error', 'input:done', 'input:error', - // clipboard:data carries the simulator's clipboard — agent-authenticated, so a - // browser socket can never inject it into another viewer. - 'clipboard:data', 'clipboard:write-done', 'clipboard:error', - // stream:register binds a session's stream socket — agent-only, or a browser - // (view PAT / cookie) could hijack an existing session's video feed. - 'stream:register', -] as const - -/** Typed `ReadonlySet`, deliberately: the door below tests a `MessageType`, which is wider - * than the literals above, and `Set.has` takes a `T`. The literal union stays reachable through - * the array, which is where the assertions read it from. */ -const AGENT_MSG_TYPES: ReadonlySet = new Set(AGENT_MSG_TYPE_LIST) - -// ── the list above is checked against the protocol, both ways (#532) ───────────────────────────── -// -// It is a hand-maintained second copy of "what an agent produces", and it is the copy with the -// security consequence: the door below closes a `browser`-role socket with 1008 for any member. The -// forwards it guards mostly resolve a session from the message and send to *that session's* browser -// with no check that the sender is that session's agent — `clipboard:*` is the deliberate exception. -// So an agent→browser message added to the protocol and forgotten here makes a viewer drivable by -// anyone who knows a session id, with the type union claiming otherwise. -// -// Measured before this change: dropping `keyboard:toggled` from the set left the static suite and -// the relay suite green. `clipboard:data` was held only because somebody wrote that one test by -// hand. The other 28 entries were held by nothing. -// -// Derivation is not available — types erase, so no runtime array can come out of a union. What is -// available is the compiler checking two lists against each other, which is what these two lines do. -type AgentProduced = (AgentControlOutbound | StreamToRelay)['type'] -type Listed = (typeof AGENT_MSG_TYPE_LIST)[number] -// **`satisfies` first, and it is the security direction's real floor.** `Exclude` -// is `never`, so if the list ever widens past its literals — dropping `as const`, or one non-literal -// element — the covering assertion below passes while checking nothing. Only its sibling would fail, -// and that sibling's own comment calls its direction the one that "gates nothing", so an author -// trusting the comment could delete the wrong one. This line fails first and names the list. -AGENT_MSG_TYPE_LIST satisfies readonly AgentProduced[] -type _AgentSetCoversProtocol = Assert>> -type _AgentSetInventsNothing = Assert>> - -// The invariant the door *enforces* — that nothing a browser may send is something an agent produces -// — is asserted in `protocol/src/typeAssertions.ts` instead. It is a claim about the protocol's own -// directions, not about this file, and naming `BrowserToRelay` here made `clientOutboundTyped` read -// the relay as a browser-role sender. That guard was right to say so. - +/** One member of the parse product, by literal. `route` narrows to these; a handler that takes one + * gets exactly what the door proved for that type and nothing else. */ +type Inbound = Extract + +/** The five inputs an ack answers, and the six it does not. Written as unions of `Inbound<…>` rather + * than as `& { requestId: string }` intersections: the correlator is now declared on the members + * themselves, so an intersection would re-state a fact the union already carries — and would keep + * compiling if one of them lost the field. */ +type Acked = Inbound<'input:touch:end' | 'input:pinch:end' | 'input:key' | 'input:button' | 'input:type'> +type Unacked = Inbound< + 'input:touch:start' | 'input:touch:move' | 'input:pinch:start' | 'input:pinch:move' + | 'input:rotate' | 'input:keyboard:toggle' +> export class RelayServer { private httpServer: http.Server | https.Server @@ -616,18 +549,32 @@ export class RelayServer { } catch { return // genuinely malformed — there is no type to answer on } - // `JSON.parse` returns bare `null`, numbers and strings without throwing, and `route` reads - // `.type` off whatever it is handed. Rejecting non-objects here is what lets the catch below - // name the message safely. - if (typeof parsed !== 'object' || parsed === null) return - const msg = parsed as RelayMessage + // **The frame becomes a union here, and not by a cast.** `#550` asked for `RelayMessage` — a flat + // interface where `type` is the only required member — to become a discriminated union, and doing + // that with an `as` at this line would have been a downgrade: the visible cast at the + // `session:chrome` handler would have turned into an invisible `msg.payload`, with the compiler + // vouching for JSON that arrived over a socket. So the union is the *product* of a parse (#444), + // and what the parse could not prove is not in the type. Non-objects, unknown types and shape + // failures are all refused in there — including the bare `null` / number / string that + // `JSON.parse` returns without throwing. + const inbound = parseInbound(parsed) + // **Role and direction are settled before the shape is, and the order is load-bearing.** A first + // draft rejected a malformed frame and returned, which silently dropped an agent-only type a + // browser socket had spoofed *badly* — the 1008 that closes such a socket never fired, so the + // spoofer kept its connection. The direction is a fact about the `type` alone, and the type is + // known on a shape failure too, so nothing about that check needs the payload to be valid. + if (!this.settleRole(ws, inbound)) return + if (!inbound.ok) { + logInboundRejection(inbound) + return + } try { - this.route(ws, msg) + this.route(ws, inbound.msg, inbound.raw) } catch (e) { // A throw inside a handler used to land in the same catch as a parse failure and vanish. // Anything reaching here is a bug in routing, not a bad message, and the caller is left // waiting either way — so at least say so once instead of dropping it silently. - logger.error(`route failed for ${String(msg.type)}:`, e) + logger.error(`route failed for ${inbound.msg.type}:`, e) } }) @@ -662,25 +609,58 @@ export class RelayServer { }) } - private route(ws: WebSocket, msg: RelayMessage): void { - // Assign role on the first message for local no-auth connections (agents / streams) + /** + * @param msg what the door proved — see `parseInbound`. An Envelope-tier member arrives carrying + * `type` and its correlators and nothing else, so a payload the parser did not check + * cannot be read off it. + * @param raw the frame as it arrived. Agent-origin messages are **forwarded as this**, so a field + * a newer agent added survives a relay that does not know it — `z.object` strips, and + * stripping is wrong in the one direction where the sender is the more recently + * updated side. Browser-origin messages are forwarded as `msg` instead, so a key an attacker + * appended does not survive. It is also where the two stored Envelope payloads come + * from, which is the point: a value read off `raw` is one the parser did not vouch for. + */ + /** + * Assign this socket's role if it does not have one, then refuse a message its role may not send. + * + * @returns `false` when the caller must stop — the socket was closed, or the frame is a failed + * handshake that must not confer a role. + */ + private settleRole(ws: WebSocket, inbound: ParseResult): boolean { + const type = inbound.ok ? inbound.msg.type : inbound.reason === 'bad-shape' ? inbound.type : undefined + if (type === undefined) return false + + // **The role comes from the two handshake literals, deliberately not from `directionOf`.** Reading + // it from the message's own direction is the obvious simplification now that one exists, and it + // inverts the check below: a socket whose first frame is `screenshot:done` would be handed the + // `agent` role and then waved through the gate that exists to refuse exactly that. + const handshake = type === 'agent:register' || type === 'stream:register' if (!this.wsRoles.has(ws)) { - if (msg.type === 'agent:register') { - this.wsRoles.set(ws, 'agent') - } else if (msg.type === 'stream:register') { - this.wsRoles.set(ws, 'stream') - } else { - // Local connection whose first message is not an agent/stream handshake — - // treat it as a browser (e.g. dashboard opened on the same machine). - this.wsRoles.set(ws, 'browser') - } + // A handshake that did not parse confers nothing. Returning here rather than falling through to + // `browser` is what keeps an agent whose register is malformed from being closed with + // `Forbidden` — its frame is dropped and logged, and the next one still gets to introduce it. + if (handshake && !inbound.ok) return false + if (type === 'agent:register') this.wsRoles.set(ws, 'agent') + else if (type === 'stream:register') this.wsRoles.set(ws, 'stream') + // Local connection whose first message is not an agent/stream handshake — treat it as a browser + // (e.g. dashboard opened on the same machine). This fallback is what puts such a socket under + // the gate below. + else this.wsRoles.set(ws, 'browser') } - // Browser sockets must not spoof agent control messages - if (this.wsRoles.get(ws) === 'browser' && AGENT_MSG_TYPES.has(msg.type)) { + // Browser sockets must not spoof agent control messages. + // + // This was a hand-copied array of 29 literals with two type assertions holding it against the + // protocol. `directionOf` derives the same set from the schema map — verified member for member + // against the array it replaces, no literal added and none lost. + if (this.wsRoles.get(ws) === 'browser' && directionOf(type) !== 'browser') { ws.close(1008, 'Forbidden') - return + return false } + return true + } + + private route(ws: WebSocket, msg: ParsedInbound, raw: Readonly>): void { switch (msg.type) { // ── Agent → Relay ───────────────────────────────────────────────────── @@ -696,7 +676,7 @@ export class RelayServer { } // ── Session / Stream lifecycle ───────────────────────────────────────── - case 'session:start': if (isAddressed(msg)) this.handleSessionStart(ws, msg); break + case 'session:start': this.handleSessionStart(ws, msg); break // ── the two session commands, gated but not answered ─────────────────────────────────────── // // Both destroy state a viewer depends on, and until L5c both acted on the strength of the session @@ -714,7 +694,7 @@ export class RelayServer { // `session:leave-error` would grow the wire for a message no consumer reads — and `session:end` has // no in-repo sender at all, so it would be a reply to nobody. case 'session:end': { - if (msg.sessionId && this.ownsSession(ws, this.sessions.get(msg.sessionId))) { + if (this.ownsSession(ws, this.sessions.get(msg.sessionId))) { this.sessions.remove(msg.sessionId) this.dropHandlers.delete(msg.sessionId) this.audioDropHandlers.delete(msg.sessionId) @@ -724,7 +704,7 @@ export class RelayServer { break } case 'session:leave': { - if (msg.sessionId && this.ownsSession(ws, this.sessions.get(msg.sessionId))) { + if (this.ownsSession(ws, this.sessions.get(msg.sessionId))) { this.sessions.clearBrowser(msg.sessionId) this.dropHandlers.delete(msg.sessionId) this.audioDropHandlers.delete(msg.sessionId) @@ -734,7 +714,7 @@ export class RelayServer { break } case 'stream:register': { - const session = this.sessions.get(msg.sessionId!) + const session = this.sessions.get(msg.sessionId) if (session) { this.sessions.setStreamSocket(session.id, ws) this.sendTo(ws, { type: 'stream:registered' }) @@ -744,61 +724,74 @@ export class RelayServer { // ── Agent → Browser ──────────────────────────────────────────────────── case 'session:chrome': { - const session = this.sessions.get(msg.sessionId!) + const session = this.sessions.get(msg.sessionId) if (!session) break - // Cast, not a check: nothing validates inbound messages, so the relay takes the agent's - // word for the shape. It only stores and forwards this, so a wrong shape surfaces in the - // viewer rather than here — but this is where a runtime validator belongs when one exists - // (see packages/protocol/AGENTS.md). - this.sessions.setChromeData(session.id, msg.payload as ChromePayload) + // **Off `raw`, and still a cast — deliberately both.** + // + // `session:chrome` is Envelope tier, so the parser proved `type` and `sessionId` and nothing + // about this value; reading it from `raw` is what makes that visible at the line rather than + // in a header. The old comment here promised a validator would replace the cast. It did not, + // and the reason is worth having instead of the promise: `ChromePayload` is a **closed + // two-member union** while `AgentRegister.platform` is `string`, open so that a third-party + // platform can register through `AgentRegistry.register()` (root AGENTS.md, OCP). Validating + // this would refuse a platform the repo promises to support — and refusing costs more than a + // dropped frame, because the message arrives once per boot and skipping `setChromeData` also + // empties what the re-join replay reads. The relay never looks inside; the viewer does, and it + // is where the variants are already told apart. + this.sessions.setChromeData(session.id, raw['payload'] as ChromePayload) if (session.browserSocket?.readyState === WebSocket.OPEN) { - session.browserSocket.send(JSON.stringify(msg)) + session.browserSocket.send(JSON.stringify(raw)) } break } case 'session:deviceInfo': { - const session = this.sessions.get(msg.sessionId!) + const session = this.sessions.get(msg.sessionId) if (!session) break - this.sessions.setDeviceInfo(session.id, msg.payload as DeviceDetails) + // See `session:chrome` above for why this one is a cast off `raw` too. + this.sessions.setDeviceInfo(session.id, raw['payload'] as DeviceDetails) if (session.browserSocket?.readyState === WebSocket.OPEN) { - session.browserSocket.send(JSON.stringify(msg)) + session.browserSocket.send(JSON.stringify(raw)) } break } case 'device:booting': { // clear cached device data so reconnecting browser doesn't get stale chrome - const session = this.sessions.get(msg.sessionId!) + const session = this.sessions.get(msg.sessionId) if (!session) break this.sessions.clearDeviceCache(session.id) if (session.browserSocket?.readyState === WebSocket.OPEN) { - session.browserSocket.send(JSON.stringify(msg)) + session.browserSocket.send(JSON.stringify(raw)) } break } case 'device:boot-error': { - const session = this.sessions.get(msg.sessionId!) + const session = this.sessions.get(msg.sessionId) if (session?.browserSocket?.readyState === WebSocket.OPEN) { - session.browserSocket.send(JSON.stringify(msg)) + session.browserSocket.send(JSON.stringify(raw)) } break } case 'device:shutdown-done': { - const session = this.sessions.get(msg.sessionId!) + const session = this.sessions.get(msg.sessionId) if (!session) break this.sessions.updateDeviceStatus(session.id, 'shutdown') this.sessions.setReadySent(session.id, false) if (session.browserSocket?.readyState === WebSocket.OPEN) { - session.browserSocket.send(JSON.stringify(msg)) + session.browserSocket.send(JSON.stringify(raw)) } break } case 'device:ready': { - const session = this.sessions.get(msg.sessionId!) + // The one inbound member whose `sessionId` is declared optional, and it is a reasoned deferral + // rather than an oversight — see `DeviceReady`. So this is a real guard, not a dropped `!`: + // an agent that omits it resolves no session here, exactly as before. + if (msg.sessionId === undefined) break + const session = this.sessions.get(msg.sessionId) if (!session) break this.sessions.updateDeviceStatus(session.id, 'booted') this.sessions.setReadySent(session.id, true) if (session.browserSocket?.readyState === WebSocket.OPEN) { - session.browserSocket.send(JSON.stringify(msg)) + session.browserSocket.send(JSON.stringify(raw)) } break } @@ -815,9 +808,9 @@ export class RelayServer { case 'input:done': case 'input:error': case 'keyboard:toggled': { - const session = this.sessions.get(msg.sessionId!) + const session = this.sessions.get(msg.sessionId) if (session?.browserSocket?.readyState === WebSocket.OPEN) { - session.browserSocket.send(JSON.stringify(msg)) + session.browserSocket.send(JSON.stringify(raw)) } break } @@ -827,10 +820,10 @@ export class RelayServer { case 'clipboard:data': case 'clipboard:write-done': case 'clipboard:error': { - const session = this.sessions.get(msg.sessionId!) + const session = this.sessions.get(msg.sessionId) if (session?.agentSocket !== ws) break if (session.browserSocket?.readyState === WebSocket.OPEN) { - session.browserSocket.send(JSON.stringify(msg)) + session.browserSocket.send(JSON.stringify(raw)) } break } @@ -844,11 +837,13 @@ export class RelayServer { // one direction no reply reports. `correlatedRequestsGated` resolves fall-through by sharing the // next non-empty body, so it would have read the gate as covering both and passed. case 'device:boot': { - // At the door, before the session lookup — one policy for the whole request, the same shape as - // `open-url` next door. Both things the relay could do with an id-less boot are downstream of - // here (forward it, or answer it with a `device:boot-error`), and gating only one of them leaves - // the guarantee resting on whichever branch was not gated. - if (!isCorrelated(msg) || !isAddressed(msg)) break + // The door gate that stood here is now the schema: `device:boot` declares `sessionId` and + // `requestId` required, and the parser rejects an absent **or empty** one before this case is + // reached. The policy it enforced is unchanged and its reason still holds — both things the + // relay could do with an id-less boot are downstream of here (forward it, or answer it with a + // `device:boot-error`), so gating only one of them would leave the guarantee resting on + // whichever branch was not gated. + // // A boot the agent never receives leaves the viewer on "Waiting for first frame…" with nothing // said, and the reasons it might not arrive are worth telling apart: `bootDevice` is the first // call an MCP caller makes, so reporting a stale session id as a dead Mac sends the reader after @@ -871,20 +866,25 @@ export class RelayServer { } // Tag the boot with whether the viewer is external (public IP) so the agent can pick the // downscale tier. The browser already reports secureContext in the payload. - if (msg.payload && typeof msg.payload === 'object') { - (msg.payload as Record).external = this.wsExternal.get(ws) ?? false - } + // + // **Mutating the parse product, not the frame that arrived**, which is what makes this safe as + // well as correct: `payload` is a fresh object the parser built, so `raw` is untouched. The + // presence check the old line carried is gone because the schema requires the payload — and + // `external` is deliberately not declared on `DeviceBoot`, since the browser never sends it. + (msg.payload as Record)['external'] = this.wsExternal.get(ws) ?? false + // The parse product, so a key a viewer appended from devtools is gone before the agent sees it. boot.session.agentSocket.send(JSON.stringify(msg)) break } case 'device:shutdown': { // Deliberately **un**correlated: the relay sends this itself from the idle timer, with no browser and - // no id behind it, so a correlator cannot be required and an absent one is not an error. Addressing - // is different, and it gets **no gate either** — for the reason the unacked input clause does not: - // an unaddressed shutdown resolves no session and is dropped by the miss, so a gate would buy only - // its log, and a line no test can hold is a line that will drift. A mutation confirmed there is - // nothing observable to hold. Ownership is #527. - const session = this.sessions.get(msg.sessionId!) + // no id behind it, so a correlator cannot be required and an absent one is not an error. + // + // Addressing used to have no gate either, on the grounds that an unaddressed shutdown resolves no + // session and is dropped by the miss, so a gate would buy only its log. That reasoning is now moot + // rather than wrong: the schema declares `sessionId` required, so the parser refuses the frame and + // the log it would have bought is the one `logInboundRejection` writes. Ownership is #527. + const session = this.sessions.get(msg.sessionId) if (session?.agentSocket.readyState === WebSocket.OPEN) { session.agentSocket.send(JSON.stringify(msg)) } @@ -892,8 +892,8 @@ export class RelayServer { } // Door checks, one policy per request: an uncorrelatable request is not forwarded, not rebuilt and // not answered, because every reply these produce declares `requestId` as required. - case 'app:install': if (isCorrelated(msg) && isAddressed(msg)) this.handleBrowserAppInstall(ws, msg); break - case 'app:launch': if (isCorrelated(msg) && isAddressed(msg)) this.handleBrowserAppLaunch(ws, msg); break + case 'app:install': this.handleBrowserAppInstall(ws, msg); break + case 'app:launch': this.handleBrowserAppLaunch(ws, msg); break case 'open-url': { // **At the door, before either branch.** A correlator is required on this request, and the // relay has two things it could do with one that lacks it — forward it, or answer it — so @@ -903,16 +903,17 @@ export class RelayServer { // an agent that predates this field would execute the request and reply uncorrelated, and // then nothing downstream can attribute the reply. // - // The check is here rather than in a validator because this is one required field on one - // message, and the relay can act on its absence locally. General inbound validation is #444. + // **The check is the validator now**, and the argument for it is unchanged. It read "here rather + // than in a validator because this is one required field on one message"; #444 built the + // validator, and the schema for `open-url` demands the correlator before this case is reached. // - // Dropping is the only honest answer: `open-url:error` requires the correlator too, so + // Dropping is still the only honest answer: `open-url:error` requires the correlator too, so // answering would mean shipping a frame that violates its own declaration — `JSON.stringify` // erases the absent key, every correlating consumer discards the result, and "agent offline" - // becomes a caller waiting out its full deadline. That was the first draft's other half: - // `requestId: msg.requestId!`, which is not the `sessionId!` below it in kind, because that - // one feeds a *read* whose miss still produces a visible error. - if (!isCorrelated(msg) || !isAddressed(msg)) break + // becomes a caller waiting out its full deadline. That was the first draft's other half, + // `requestId: msg.requestId!` — an assertion in an *outbound* frame, unlike the inbound + // `sessionId!` this file used to carry, which fed a read whose miss still produced a visible + // error. Neither exists any more. const target = this.dispatchTarget(ws, msg.sessionId) if (target.ok) { target.session.agentSocket.send(JSON.stringify(msg)) @@ -929,7 +930,6 @@ export class RelayServer { case 'app:clear-state': { // Verbatim forward like `open-url`, so the correlator rides for free; the door check and the echo // are the same two lines. - if (!isCorrelated(msg) || !isAddressed(msg)) break const target = this.dispatchTarget(ws, msg.sessionId) if (target.ok) { target.session.agentSocket.send(JSON.stringify(msg)) @@ -960,11 +960,10 @@ export class RelayServer { // No ack, so no correlator to check and nothing to answer. An unowned frame is dropped rather // than refused for the same reason: there is no waiter to tell. // - // **And no `isAddressed` gate**, unlike the answered five. An unaddressed frame resolves no session - // and is dropped by the miss either way, so the gate would buy only its log — one line per - // `input:touch:move`, which is the ~60/s the ownership warn was removed from this same method for. - // A mutation confirmed it: adding the gate here changes nothing a test can observe, and a line no - // test can hold is a line that will drift. + // These carry no correlator by declaration, so nothing was ever gated on one here — and the + // address is now the schema's business rather than this clause's. What the split still buys is + // unchanged and is the reason it exists: a correlator gate written into a shared body would have + // reached these six as well, dropping every opening and move frame with the answered five. this.forwardUnacked(ws, msg) break } @@ -975,7 +974,7 @@ export class RelayServer { case 'input:type': { // One policy at the door, as for the app commands: an uncorrelatable request is not forwarded and // not answered, because every reply it could produce declares `requestId` required. - if (isCorrelated(msg) && isAddressed(msg)) this.handleAckedInput(ws, msg) + this.handleAckedInput(ws, msg) break } // Kept out of the input:* chain above: these need their own error type, and the caller @@ -990,7 +989,6 @@ export class RelayServer { // `clipboard:error` whose required correlator is missing, which `useClipboardBridge` discards on // `if (!msg.requestId) return` — so "agent offline" became the caller waiting out its budget. // Removed once already in `e98abd4`, for `open-url`, and still here. - if (!isCorrelated(msg) || !isAddressed(msg)) break // Ownership matters most here of all of them: a `clipboard:write` from a socket that does not hold // the session pastes its text into someone else's device, and a `clipboard:read` presses the copy // or cut chord on it — and the reply routes to the session's own browser, so the payload lands on @@ -1009,8 +1007,7 @@ export class RelayServer { } } - private handleAgentResources(ws: WebSocket, msg: RelayMessage): void { - if (!msg.resources) return + private handleAgentResources(ws: WebSocket, msg: Inbound<'agent:resources'>): void { this.sessions.setResources(ws, msg.resources) const agentName = this.sessions.getAllByAgentSocket(ws)[0]?.agentName if (agentName) { @@ -1143,7 +1140,7 @@ export class RelayServer { return true } - private handleAgentRegister(ws: WebSocket, msg: RelayMessage): void { + private handleAgentRegister(ws: WebSocket, msg: Inbound<'agent:register'>): void { // Re-register from the same Mac (machine id + platform): the old socket's close may not have // fired yet after an unclean drop (Wi-Fi loss, sleep) — its TCP teardown lags — which would // leave a duplicate, eventually-"Stale" card. Evict the stale agent's sessions and terminate @@ -1241,7 +1238,7 @@ export class RelayServer { * non-empty `string` by the door (`isAddressed`), so every refusal can name the join it refuses. Before * L5d they carried none, and the clients' join waiters matched `sessionId === undefined || sessionId === * mine` — with no such key the left half was always true, so any refusal resolved any pending join. */ - private handleSessionStart(ws: WebSocket, msg: RelayMessage & { sessionId: string }): void { + private handleSessionStart(ws: WebSocket, msg: Inbound<'session:start'>): void { const session = this.sessions.get(msg.sessionId) if (!session) { this.sendTo(ws, { type: 'error', sessionId: msg.sessionId, message: 'Session not found', reason: 'session-not-found' }) @@ -1367,8 +1364,8 @@ export class RelayServer { * Dropped rather than refused when unowned, and that asymmetry with `handleAckedInput` is the whole * reason these have their own clause: refusing means answering, and there is no waiter here to answer. * `clipboard:data`'s silent `break` is the precedent that fits — a frame nobody is waiting on. */ - private forwardUnacked(ws: WebSocket, msg: RelayMessage): void { - const session = this.sessions.get(msg.sessionId!) + private forwardUnacked(ws: WebSocket, msg: Unacked): void { + const session = this.sessions.get(msg.sessionId) if (session && !this.ownsSession(ws, session)) { // **Deliberately silent.** A first draft logged here, which is one line per `input:touch:move` — the // dashboard sends those per `pointermove`, so ~60/s for as long as a finger is down, unbounded and @@ -1387,9 +1384,8 @@ export class RelayServer { * a 2s deadline must not be dropped silently, because the caller's fallback reports silence from a * session that has never acked as **success** (#457) — so a silent drop here would report an input that * never left the relay as landed, which is worse than the misrouting it replaced. */ - private handleAckedInput(ws: WebSocket, msg: RelayMessage & { requestId: string; sessionId: string }): void { - // No `!`: the door predicate narrowed the parameter, so the assertion here was dead and counted itself - // into #444's remaining body as if it were a real gap. + private handleAckedInput(ws: WebSocket, msg: Acked): void { + // No `!`: the parameter is narrowed by the door, which is the parse now rather than a predicate. const session = this.sessions.get(msg.sessionId) if (session && !this.ownsSession(ws, session)) { @@ -1433,7 +1429,7 @@ export class RelayServer { * and burned its caller's full deadline. */ private refuseInput( ws: WebSocket, - msg: RelayMessage & { requestId: string; sessionId: string }, + msg: Acked, message: string, reason: InputErrorReason, ): void { @@ -1451,7 +1447,7 @@ export class RelayServer { * caller waits for the reply matching its own sessionId, so anything else is indistinguishable * from silence and it waits out the deadline (#445). `Session not found` is app-specific for the * same reason: a generic `error` cannot be correlated by construction. */ - private handleBrowserAppInstall(ws: WebSocket, msg: RelayMessage & { requestId: string; sessionId: string }): void { + private handleBrowserAppInstall(ws: WebSocket, msg: Inbound<'app:install'>): void { const sessionId = msg.sessionId const { requestId } = msg // Closing over the narrowed correlator covers all four failure exits at once. It does **not** cover a @@ -1469,10 +1465,13 @@ export class RelayServer { // someone else was testing, with the reply going to that session's browser rather than to it. if (!this.ownsSession(ws, session)) return fail(ownershipRefusal(session)) - // `JSON.parse` does not honour `RelayMessage`, so buildId is whatever the sender put there. + // The schema already refused a non-integer `buildId`, so this is now belt-and-braces rather than + // the only guard. Kept because it is also the *answer*: the parser drops a bad frame silently and + // this tells the caller `Build not found` instead of leaving it on its deadline. // better-sqlite3 binds a missing value as NULL but *throws* on an object or array — and that // exception is swallowed by the message-loop catch, which is the silence this PR exists to - // remove. Checked here rather than trusted; general inbound validation is #444. + // remove. The schema refuses the object and array outright; this catches the `NaN` it carries + // through for the rest, which is what keeps the caller answered instead of silently dropped. if (!Number.isInteger(msg.buildId)) return fail('Build not found') const build = getDb() @@ -1497,7 +1496,7 @@ export class RelayServer { } /** Relay looks up bundle_id from DB. Same correlation rules as `handleBrowserAppInstall`. */ - private handleBrowserAppLaunch(ws: WebSocket, msg: RelayMessage & { requestId: string; sessionId: string }): void { + private handleBrowserAppLaunch(ws: WebSocket, msg: Inbound<'app:launch'>): void { const sessionId = msg.sessionId const { requestId } = msg // Closing over the narrowed correlator covers all four failure exits at once. It does **not** cover a @@ -1515,8 +1514,7 @@ export class RelayServer { // someone else was testing, with the reply going to that session's browser rather than to it. if (!this.ownsSession(ws, session)) return fail(ownershipRefusal(session)) - // See handleBrowserAppInstall — an object here throws inside the driver and the exception is - // swallowed upstream. + // See `handleBrowserAppInstall` — the schema refuses it first; this is what answers the caller. if (!Number.isInteger(msg.buildId)) return fail('Bundle ID not available for this build') const build = getDb() @@ -1644,26 +1642,27 @@ export class RelayServer { }) } - private handleUITreeResponse(msg: RelayMessage): void { - if (!msg.requestId) return + private handleUITreeResponse(msg: Inbound<'ui:tree:response'>): void { const pending = this.pendingUITrees.get(msg.requestId) if (!pending) return - pending.resolve(msg.elements ?? []) + pending.resolve(msg.elements) } - private handleUITreeError(msg: RelayMessage): void { - if (!msg.requestId) return + private handleUITreeError(msg: Inbound<'ui:tree:error'>): void { const pending = this.pendingUITrees.get(msg.requestId) if (!pending) return - pending.reject(new Error(msg.message ?? 'UI tree query failed')) + // `||`, not `??`: the schema defaults an absent message to `''` rather than leaving it undefined, + // so the fallback has to treat empty as absent or an older agent's error would read as blank. + pending.reject(new Error(msg.message || 'UI tree query failed')) } - private handleScreenshotDone(msg: RelayMessage): void { - if (!msg.requestId) return + private handleScreenshotDone(msg: Inbound<'screenshot:done'>): void { const pending = this.pendingScreenshots.get(msg.requestId) if (!pending) return - const buf = Buffer.from(msg.data ?? '', 'base64') - const claimed = msg.format ?? 'png' + // The `?? ''` / `?? 'png'` these two used to carry are now `.default()`s in the schema, where a + // reader can see that the tolerance is for an older agent rather than for any absent field. + const buf = Buffer.from(msg.data, 'base64') + const claimed = msg.format // Logged, **not** overwritten. The field means what the agent says it produced, and correcting it // here would make the relay the authority on something only the agent can know — a contract // change, where this is a drift detector. It costs four bytes of an already-decoded buffer. @@ -1680,11 +1679,11 @@ export class RelayServer { pending.resolve(buf, claimed) } - private handleScreenshotError(msg: RelayMessage): void { - if (!msg.requestId) return + private handleScreenshotError(msg: Inbound<'screenshot:error'>): void { const pending = this.pendingScreenshots.get(msg.requestId) if (!pending) return - pending.reject(new Error(msg.message ?? 'Screenshot failed')) + // `||` for the reason `handleUITreeError` gives. + pending.reject(new Error(msg.message || 'Screenshot failed')) } private flushResourceBuffers(): void { diff --git a/packages/relay/src/__tests__/RelayServer.heartbeat.test.ts b/packages/relay/src/__tests__/RelayServer.heartbeat.test.ts index 37d3f190..7cf66e68 100644 --- a/packages/relay/src/__tests__/RelayServer.heartbeat.test.ts +++ b/packages/relay/src/__tests__/RelayServer.heartbeat.test.ts @@ -4,9 +4,9 @@ import os from 'os' import path from 'path' import { WebSocket, WebSocketServer } from 'ws' import { RelayServer } from '../RelayServer' -import type { RelayMessage } from '../types' import { initDb, closeDb } from '../db' import { waitForMessage, waitForOpen } from '@tapflowio/test-utils' +import type { AgentsListed } from '@tapflowio/protocol' // Minimal stand-in for a ws socket — only the surface runHeartbeat() touches. @@ -135,8 +135,8 @@ describe('RelayServer — WebSocket heartbeat (#313)', () => { it('terminates a dead agent socket and evicts its sessions (terminate → existing close cleanup)', async () => { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', agentName: 'DeadMac', devices: [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] })) - await waitForMessage(agent) // agent:registered + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'DeadMac', devices: [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] })) + await waitForMessage(agent) // agent:registered const closed = new Promise((resolve) => agent.on('close', () => resolve())) @@ -151,7 +151,7 @@ describe('RelayServer — WebSocket heartbeat (#313)', () => { await waitForOpen(observer) await vi.waitFor(async () => { observer.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForMessage(observer) + const listed = await waitForMessage(observer) expect(listed.sessions).toHaveLength(0) }, { timeout: 2000 }) observer.close() @@ -160,8 +160,8 @@ describe('RelayServer — WebSocket heartbeat (#313)', () => { it('keeps a live agent connected across heartbeats (real auto-pong)', async () => { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', agentName: 'LiveMac', devices: [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] })) - await waitForMessage(agent) + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'LiveMac', devices: [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] })) + await waitForMessage(agent) let closedUnexpectedly = false agent.on('close', () => { closedUnexpectedly = true }) @@ -176,8 +176,8 @@ describe('RelayServer — WebSocket heartbeat (#313)', () => { const observer = new WebSocket(`ws://localhost:${port}`) await waitForOpen(observer) observer.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForMessage(observer) - expect(listed.sessions!.filter((s) => s.agentName === 'LiveMac')).toHaveLength(1) + const listed = await waitForMessage(observer) + expect(listed.sessions.filter((s) => s.agentName === 'LiveMac')).toHaveLength(1) agent.close() observer.close() diff --git a/packages/relay/src/__tests__/RelayServer.test.ts b/packages/relay/src/__tests__/RelayServer.test.ts index 757abe78..756281f8 100644 --- a/packages/relay/src/__tests__/RelayServer.test.ts +++ b/packages/relay/src/__tests__/RelayServer.test.ts @@ -8,9 +8,9 @@ import { WebSocket } from 'ws' import { RelayServer } from '../RelayServer' import { initDb, closeDb, getDb } from '../db' import { hashPat } from '../middleware/auth' -import type { RelayMessage } from '../types' import { writeEnvelopeHeader, HEADER_SIZE, CODEC_AUDIO } from '@tapflowio/agent-core/utils' import { barrier, waitForMessage, waitForOpen, waitForType, waitForTypeOrNull } from '@tapflowio/test-utils' +import type { AgentRegistered, AgentsListed, DeviceReady, DeviceShutdown, InputTouchStart, OpenUrl, OpenUrlError, SessionJoined, SessionTerminated, StreamRequestIdr } from '@tapflowio/protocol' // Sends a raw HTTP request, bypassing client-side URL normalization. const rawHttpGet = (targetPort: number, rawPath: string): Promise => @@ -65,12 +65,12 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const ws = new WebSocket(`ws://localhost:${port}`) await waitForOpen(ws) - ws.send(JSON.stringify({ type: 'agent:register', devices })) - const msg = await waitForMessage(ws) + ws.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-1', devices })) + const msg = await waitForMessage(ws) expect(msg.type).toBe('agent:registered') expect(msg.registeredSessions).toHaveLength(1) - expect(msg.registeredSessions![0].deviceId).toBe('devA') - expect(typeof msg.registeredSessions![0].sessionId).toBe('string') + expect(msg.registeredSessions[0]!.deviceId).toBe('devA') + expect(typeof msg.registeredSessions[0]!.sessionId).toBe('string') ws.close() }) @@ -81,10 +81,10 @@ describe('RelayServer', () => { ] const ws = new WebSocket(`ws://localhost:${port}`) await waitForOpen(ws) - ws.send(JSON.stringify({ type: 'agent:register', devices })) - const msg = await waitForMessage(ws) + ws.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-1', devices })) + const msg = await waitForMessage(ws) expect(msg.registeredSessions).toHaveLength(2) - const ids = msg.registeredSessions!.map((s) => s.sessionId) + const ids = msg.registeredSessions.map((s) => s.sessionId) expect(ids[0]).not.toBe(ids[1]) ws.close() }) @@ -95,14 +95,14 @@ describe('RelayServer', () => { const agent1 = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent1) agent1.send(JSON.stringify({ type: 'agent:register', agentId: 'uuid-1', agentName: 'MyMac', platform: 'ios', devices })) - await waitForType(agent1, 'agent:registered') + await waitForType(agent1, 'agent:registered') const agent1Closed = new Promise((resolve) => agent1.on('close', () => resolve())) // Unclean reconnect: a fresh socket from the same Mac before the old socket's close fires. const agent2 = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent2) agent2.send(JSON.stringify({ type: 'agent:register', agentId: 'uuid-1', agentName: 'MyMac', platform: 'ios', devices })) - await waitForType(agent2, 'agent:registered') + await waitForType(agent2, 'agent:registered') // The relay terminates the stale socket. await agent1Closed @@ -110,8 +110,8 @@ describe('RelayServer', () => { const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForType(browser, 'agents:listed') - expect(listed.sessions!.filter((s) => s.agentName === 'MyMac')).toHaveLength(1) + const listed = await waitForType(browser, 'agents:listed') + expect(listed.sessions.filter((s) => s.agentName === 'MyMac')).toHaveLength(1) agent2.close() browser.close() @@ -121,18 +121,18 @@ describe('RelayServer', () => { const iosAgent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(iosAgent) iosAgent.send(JSON.stringify({ type: 'agent:register', agentId: 'uuid-mac', agentName: 'MyMac', platform: 'ios', devices: [{ id: 'i1', name: 'iPhone', platform: 'ios', status: 'shutdown' }] })) - await waitForType(iosAgent, 'agent:registered') + await waitForType(iosAgent, 'agent:registered') const androidAgent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(androidAgent) androidAgent.send(JSON.stringify({ type: 'agent:register', agentId: 'uuid-mac', agentName: 'MyMac', platform: 'android', devices: [{ id: 'a1', name: 'Pixel', platform: 'android', status: 'shutdown' }] })) - await waitForType(androidAgent, 'agent:registered') + await waitForType(androidAgent, 'agent:registered') const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForType(browser, 'agents:listed') - expect(listed.sessions!.filter((s) => s.agentName === 'MyMac')).toHaveLength(2) + const listed = await waitForType(browser, 'agents:listed') + expect(listed.sessions.filter((s) => s.agentName === 'MyMac')).toHaveLength(2) iosAgent.close() androidAgent.close() @@ -145,18 +145,18 @@ describe('RelayServer', () => { const macA = new WebSocket(`ws://localhost:${port}`) await waitForOpen(macA) macA.send(JSON.stringify({ type: 'agent:register', agentId: 'uuid-A', agentName: 'DupName', platform: 'ios', devices })) - await waitForType(macA, 'agent:registered') + await waitForType(macA, 'agent:registered') const macB = new WebSocket(`ws://localhost:${port}`) await waitForOpen(macB) macB.send(JSON.stringify({ type: 'agent:register', agentId: 'uuid-B', agentName: 'DupName', platform: 'ios', devices })) - await waitForType(macB, 'agent:registered') + await waitForType(macB, 'agent:registered') const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForType(browser, 'agents:listed') - expect(listed.sessions!.filter((s) => s.agentName === 'DupName')).toHaveLength(2) + const listed = await waitForType(browser, 'agents:listed') + expect(listed.sessions.filter((s) => s.agentName === 'DupName')).toHaveLength(2) macA.close() macB.close() @@ -167,14 +167,14 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-2', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - const msg = await waitForMessage(browser) + const msg = await waitForMessage(browser) expect(msg.type).toBe('session:joined') // **The address on the success half, and nothing held it.** L5d added four assertions that each // *refusal* names the right session and none that the reply does — and the reply is the one both @@ -200,9 +200,9 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-3', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const sessions = (server as unknown as { sessions: { join(id: string, ws: WebSocket): void } }).sessions const join = vi.spyOn(sessions, 'join').mockImplementation(() => { throw new Error('unforeseen') }) @@ -210,7 +210,7 @@ describe('RelayServer', () => { const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - const msg = await waitForMessage(browser) + const msg = await waitForMessage(browser) expect(msg.type).toBe('error') expect(msg.sessionId).toBe(sessionId) @@ -227,7 +227,7 @@ describe('RelayServer', () => { const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId: 'bad-id' })) - const msg = await waitForMessage(browser) + const msg = await waitForMessage(browser) expect(msg.type).toBe('error') expect(msg.message).toBe('Session not found') // The refusal names the id it could not find — the session does not exist, but the *request* does, and it @@ -241,19 +241,19 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-4', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser1 = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser1) browser1.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser1) // session:joined + await waitForMessage(browser1) // session:joined const browser2 = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser2) browser2.send(JSON.stringify({ type: 'session:start', sessionId })) - const msg = await waitForMessage(browser2) + const msg = await waitForMessage(browser2) expect(msg.type).toBe('error') expect(msg.message).toBe('Session busy') // L5d. Every refusal names the join it refuses, and each of the four exits needs its own assertion: @@ -276,21 +276,21 @@ describe('RelayServer', () => { ] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionA = registeredSessions!.find((s) => s.deviceId === 'devA')!.sessionId - const sessionB = registeredSessions!.find((s) => s.deviceId === 'devB')!.sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-5', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionA = registeredSessions.find((s) => s.deviceId === 'devA')!.sessionId + const sessionB = registeredSessions.find((s) => s.deviceId === 'devB')!.sessionId const browserA = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browserA) browserA.send(JSON.stringify({ type: 'session:start', sessionId: sessionA })) - const msgA = await waitForMessage(browserA) + const msgA = await waitForMessage(browserA) expect(msgA.type).toBe('session:joined') const browserB = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browserB) browserB.send(JSON.stringify({ type: 'session:start', sessionId: sessionB })) - const msgB = await waitForMessage(browserB) + const msgB = await waitForMessage(browserB) expect(msgB.type).toBe('session:joined') agent.close() @@ -309,18 +309,18 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${shortPort}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-6', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${shortPort}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForType(browser, 'session:joined') + await waitForType(browser, 'session:joined') // Without the notice the browser keeps a live socket addressed to a sessionId the relay has // dropped: everything it sends is ignored and nothing streams back, with no way to tell. - const endedPromise = waitForType(browser, 'session:terminated') + const endedPromise = waitForType(browser, 'session:terminated') agent.close() const ended = await endedPromise @@ -337,8 +337,8 @@ describe('RelayServer', () => { const devices = [{ id: 'devSolo', name: 'iPhone Solo', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - await waitForMessage(agent) + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-7', devices })) + await waitForMessage(agent) // Arm the close listener before closing: `agent.close()` returns immediately and the relay's // eviction runs on its own close handler, so querying straight away can observe the session @@ -354,11 +354,11 @@ describe('RelayServer', () => { const probe = new WebSocket(`ws://localhost:${port}`) await waitForOpen(probe) probe.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForType(probe, 'agents:listed') + const listed = await waitForType(probe, 'agents:listed') // Not `listed.sessions ?? []` — that would also pass if the field went missing entirely, which // is a different bug wearing the same green tick. expect(listed.sessions).toBeDefined() - expect(listed.sessions!.filter((s) => s.devices.some((d) => d.id === 'devSolo'))).toHaveLength(0) + expect(listed.sessions.filter((s) => s.devices.some((d) => d.id === 'devSolo'))).toHaveLength(0) probe.close() }) @@ -366,16 +366,16 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-8', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) + await waitForMessage(browser) - const touchPromise = waitForMessage(agent) + const touchPromise = waitForMessage(agent) browser.send(JSON.stringify({ type: 'input:touch:start', sessionId, payload: { x: 0.5, y: 0.5 } })) const touch = await touchPromise expect(touch.type).toBe('input:touch:start') @@ -392,18 +392,18 @@ describe('RelayServer', () => { ] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionA = registeredSessions!.find((s) => s.deviceId === 'devA')!.sessionId - const sessionB = registeredSessions!.find((s) => s.deviceId === 'devB')!.sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-9', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionA = registeredSessions.find((s) => s.deviceId === 'devA')!.sessionId + const sessionB = registeredSessions.find((s) => s.deviceId === 'devB')!.sessionId const browserA = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browserA) browserA.send(JSON.stringify({ type: 'session:start', sessionId: sessionA })) - await waitForMessage(browserA) + await waitForMessage(browserA) // Agent receives browserA's touch for sessionA - const touchPromise = waitForType(agent, 'input:touch:start') + const touchPromise = waitForType(agent, 'input:touch:start') browserA.send(JSON.stringify({ type: 'input:touch:start', sessionId: sessionA, payload: { x: 0.1, y: 0.2 } })) const touch = await touchPromise expect(touch.sessionId).toBe(sessionA) // carries sessionA, not sessionB @@ -412,9 +412,9 @@ describe('RelayServer', () => { const browserB = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browserB) browserB.send(JSON.stringify({ type: 'session:start', sessionId: sessionB })) - await waitForMessage(browserB) + await waitForMessage(browserB) - const touch2Promise = waitForType(agent, 'input:touch:start') + const touch2Promise = waitForType(agent, 'input:touch:start') browserB.send(JSON.stringify({ type: 'input:touch:start', sessionId: sessionB, payload: { x: 0.9, y: 0.8 } })) const touch2 = await touch2Promise expect(touch2.sessionId).toBe(sessionB) @@ -428,16 +428,16 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-10', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) + await waitForMessage(browser) - const bootPromise = waitForMessage(agent) + const bootPromise = waitForMessage(agent) browser.send(JSON.stringify({ type: 'device:boot', sessionId, requestId: 'rq-route', payload: { deviceId: 'devA' } })) const boot = await bootPromise expect(boot.type).toBe('device:boot') @@ -451,21 +451,21 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-11', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) + await waitForMessage(browser) - const bootingPromise = waitForMessage(browser) + const bootingPromise = waitForMessage(browser) agent.send(JSON.stringify({ type: 'device:booting', sessionId })) const booting = await bootingPromise expect(booting.type).toBe('device:booting') - const readyPromise = waitForMessage(browser) + const readyPromise = waitForMessage(browser) agent.send(JSON.stringify({ type: 'device:ready', sessionId, payload: { deviceId: 'devA' } })) const ready = await readyPromise expect(ready.type).toBe('device:ready') @@ -479,21 +479,21 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-12', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) browser.binaryType = 'nodebuffer' await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) // session:joined + await waitForMessage(browser) // session:joined // Stream WS connects and registers const streamWs = new WebSocket(`ws://localhost:${port}`) await waitForOpen(streamWs) streamWs.send(JSON.stringify({ type: 'stream:register', sessionId })) - const ack = await waitForMessage(streamWs) + const ack = await waitForMessage(streamWs) expect(ack.type).toBe('stream:registered') // Binary frames sent via stream WS are forwarded to browser @@ -514,20 +514,20 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-13', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) browser.binaryType = 'nodebuffer' await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) // session:joined + await waitForMessage(browser) // session:joined const streamWs = new WebSocket(`ws://localhost:${port}`) await waitForOpen(streamWs) streamWs.send(JSON.stringify({ type: 'stream:register', sessionId })) - await waitForMessage(streamWs) // stream:registered + await waitForMessage(streamWs) // stream:registered const capturedAt = Date.now() - 10 const envelopedFrame = writeEnvelopeHeader(Buffer.from([0xFF, 0xD8]), capturedAt) @@ -556,20 +556,20 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-14', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) browser.binaryType = 'nodebuffer' await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) // session:joined + await waitForMessage(browser) // session:joined const streamWs = new WebSocket(`ws://localhost:${port}`) await waitForOpen(streamWs) streamWs.send(JSON.stringify({ type: 'stream:register', sessionId })) - await waitForMessage(streamWs) // stream:registered + await waitForMessage(streamWs) // stream:registered const pcm = Buffer.from([0x11, 0x22, 0x33, 0x44]) const audioFrame = writeEnvelopeHeader(pcm, Date.now() - 5, { codec: CODEC_AUDIO }) @@ -598,20 +598,20 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-15', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) browser.binaryType = 'nodebuffer' await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) // session:joined + await waitForMessage(browser) // session:joined const streamWs = new WebSocket(`ws://localhost:${port}`) await waitForOpen(streamWs) streamWs.send(JSON.stringify({ type: 'stream:register', sessionId })) - await waitForMessage(streamWs) // stream:registered + await waitForMessage(streamWs) // stream:registered const framePromise = new Promise((r) => browser.once('message', (d, isBinary) => { if (isBinary) r(d as Buffer) }) @@ -633,28 +633,28 @@ describe('RelayServer', () => { ] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionA = registeredSessions!.find((s) => s.deviceId === 'devA')!.sessionId - const sessionB = registeredSessions!.find((s) => s.deviceId === 'devB')!.sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-16', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionA = registeredSessions.find((s) => s.deviceId === 'devA')!.sessionId + const sessionB = registeredSessions.find((s) => s.deviceId === 'devB')!.sessionId const browserA = new WebSocket(`ws://localhost:${port}`) browserA.binaryType = 'nodebuffer' await waitForOpen(browserA) browserA.send(JSON.stringify({ type: 'session:start', sessionId: sessionA })) - await waitForMessage(browserA) + await waitForMessage(browserA) const browserB = new WebSocket(`ws://localhost:${port}`) browserB.binaryType = 'nodebuffer' await waitForOpen(browserB) browserB.send(JSON.stringify({ type: 'session:start', sessionId: sessionB })) - await waitForMessage(browserB) + await waitForMessage(browserB) // Stream WS for devA const streamA = new WebSocket(`ws://localhost:${port}`) await waitForOpen(streamA) streamA.send(JSON.stringify({ type: 'stream:register', sessionId: sessionA })) - await waitForMessage(streamA) // stream:registered + await waitForMessage(streamA) // stream:registered // browserB should NOT receive frames from streamA let browserBGotBinary = false @@ -687,20 +687,20 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${strictPort}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-17', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${strictPort}`) browser.binaryType = 'nodebuffer' await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) // session:joined + await waitForMessage(browser) // session:joined const streamWs = new WebSocket(`ws://localhost:${strictPort}`) await waitForOpen(streamWs) streamWs.send(JSON.stringify({ type: 'stream:register', sessionId })) - await waitForMessage(streamWs) // stream:registered + await waitForMessage(streamWs) // stream:registered // Register before send — if drop is broken the frame may arrive before setImmediate fires const dropCheck = new Promise((resolve, reject) => { @@ -727,21 +727,21 @@ describe('RelayServer', () => { ] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', agentName: 'MyMac', devices })) - await waitForMessage(agent) + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'MyMac', devices })) + await waitForMessage(agent) const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'agents:list' })) - const msg = await waitForMessage(browser) + const msg = await waitForMessage(browser) expect(msg.type).toBe('agents:listed') expect(msg.sessions).toHaveLength(1) - expect(msg.sessions![0].agentName).toBe('MyMac') - expect(msg.sessions![0].devices).toHaveLength(2) - expect(msg.sessions![0].devices[0].sessionId).toBeTruthy() - expect(msg.sessions![0].devices[1].sessionId).toBeTruthy() - expect(msg.sessions![0].devices[0].busy).toBe(false) + expect(msg.sessions[0]!.agentName).toBe('MyMac') + expect(msg.sessions[0]!.devices).toHaveLength(2) + expect(msg.sessions[0]!.devices[0]!.sessionId).toBeTruthy() + expect(msg.sessions[0]!.devices[1]!.sessionId).toBeTruthy() + expect(msg.sessions[0]!.devices[0]!.busy).toBe(false) agent.close() browser.close() @@ -751,20 +751,20 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-18', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) // session:joined + await waitForMessage(browser) // session:joined const observer = new WebSocket(`ws://localhost:${port}`) await waitForOpen(observer) observer.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForMessage(observer) - expect(listed.sessions![0].devices[0].busy).toBe(true) + const listed = await waitForMessage(observer) + expect(listed.sessions[0]!.devices[0]!.busy).toBe(true) agent.close() browser.close() @@ -778,15 +778,15 @@ describe('RelayServer', () => { ] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - await waitForMessage(agent) + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-19', devices })) + await waitForMessage(agent) agent.close() const observer = new WebSocket(`ws://localhost:${port}`) await waitForOpen(observer) await vi.waitFor(async () => { observer.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForMessage(observer) + const listed = await waitForMessage(observer) expect(listed.sessions).toHaveLength(0) }, { timeout: 2000 }) observer.close() @@ -795,8 +795,8 @@ describe('RelayServer', () => { it('agent:resources is reflected in agents:listed', async () => { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', agentName: 'Mac1', devices: [{ id: 'd1', name: 'iPhone', platform: 'ios', status: 'shutdown' }] })) - await waitForMessage(agent) + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'Mac1', devices: [{ id: 'd1', name: 'iPhone', platform: 'ios', status: 'shutdown' }] })) + await waitForMessage(agent) agent.send(JSON.stringify({ type: 'agent:resources', @@ -807,9 +807,9 @@ describe('RelayServer', () => { await waitForOpen(observer) await vi.waitFor(async () => { observer.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForMessage(observer) - expect(listed.sessions![0].resources?.cpuPercent).toBe(25) - expect(listed.sessions![0].resources?.slotsTotal).toBe(3) + const listed = await waitForMessage(observer) + expect(listed.sessions[0]!.resources?.cpuPercent).toBe(25) + expect(listed.sessions[0]!.resources?.slotsTotal).toBe(3) }, { timeout: 500 }) agent.close() @@ -819,8 +819,8 @@ describe('RelayServer', () => { it('agent resources are cleared after agent disconnects', async () => { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', agentName: 'Mac1', devices: [{ id: 'd1', name: 'iPhone', platform: 'ios', status: 'shutdown' }] })) - await waitForMessage(agent) + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'Mac1', devices: [{ id: 'd1', name: 'iPhone', platform: 'ios', status: 'shutdown' }] })) + await waitForMessage(agent) agent.send(JSON.stringify({ type: 'agent:resources', resources: { cpuPercent: 50, memUsedMB: 8000, memTotalMB: 16000, slotsAvailable: 3, slotsTotal: 3, reportedAt: 1000 }, @@ -831,7 +831,7 @@ describe('RelayServer', () => { await waitForOpen(observer) await vi.waitFor(async () => { observer.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForMessage(observer) + const listed = await waitForMessage(observer) expect(listed.sessions).toHaveLength(0) }, { timeout: 2000 }) @@ -843,28 +843,28 @@ describe('RelayServer', () => { const agentA = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agentA) agentA.send(JSON.stringify({ - type: 'agent:register', + type: 'agent:register', platform: 'ios', agentName: 'Mac-A', devices: [ { id: 'a1', name: 'iPhone A1', platform: 'ios', status: 'shutdown' }, { id: 'a2', name: 'iPhone A2', platform: 'ios', status: 'shutdown' }, ], })) - await waitForMessage(agentA) + await waitForMessage(agentA) const agentB = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agentB) agentB.send(JSON.stringify({ - type: 'agent:register', + type: 'agent:register', platform: 'ios', agentName: 'Mac-B', devices: [{ id: 'b1', name: 'iPhone B1', platform: 'ios', status: 'shutdown' }], })) - await waitForMessage(agentB) + await waitForMessage(agentB) const observer = new WebSocket(`ws://localhost:${port}`) await waitForOpen(observer) observer.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForMessage(observer) + const listed = await waitForMessage(observer) const sessions = listed.sessions! expect(sessions).toHaveLength(2) @@ -881,21 +881,21 @@ describe('RelayServer', () => { it('각 agent의 resources가 agents:listed에 독립적으로 반영됨', async () => { const agentA = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agentA) - agentA.send(JSON.stringify({ type: 'agent:register', agentName: 'Mac-A', devices: [{ id: 'a1', name: 'iPhone', platform: 'ios', status: 'shutdown' }] })) - await waitForMessage(agentA) + agentA.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'Mac-A', devices: [{ id: 'a1', name: 'iPhone', platform: 'ios', status: 'shutdown' }] })) + await waitForMessage(agentA) agentA.send(JSON.stringify({ type: 'agent:resources', resources: { cpuPercent: 30, memUsedMB: 4000, memTotalMB: 16000, slotsAvailable: 1, slotsTotal: 1, reportedAt: 1000 } })) const agentB = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agentB) - agentB.send(JSON.stringify({ type: 'agent:register', agentName: 'Mac-B', devices: [{ id: 'b1', name: 'iPhone', platform: 'ios', status: 'shutdown' }] })) - await waitForMessage(agentB) + agentB.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'Mac-B', devices: [{ id: 'b1', name: 'iPhone', platform: 'ios', status: 'shutdown' }] })) + await waitForMessage(agentB) agentB.send(JSON.stringify({ type: 'agent:resources', resources: { cpuPercent: 70, memUsedMB: 12000, memTotalMB: 16000, slotsAvailable: 1, slotsTotal: 1, reportedAt: 1000 } })) const observer = new WebSocket(`ws://localhost:${port}`) await waitForOpen(observer) await vi.waitFor(async () => { observer.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForMessage(observer) + const listed = await waitForMessage(observer) const sessions = listed.sessions! const macA = sessions.find((s) => s.agentName === 'Mac-A') const macB = sessions.find((s) => s.agentName === 'Mac-B') @@ -911,13 +911,13 @@ describe('RelayServer', () => { it('한 agent 종료 시 해당 agent 세션만 제거됨', async () => { const agentA = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agentA) - agentA.send(JSON.stringify({ type: 'agent:register', agentName: 'Mac-A', devices: [{ id: 'a1', name: 'iPhone', platform: 'ios', status: 'shutdown' }] })) - await waitForMessage(agentA) + agentA.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'Mac-A', devices: [{ id: 'a1', name: 'iPhone', platform: 'ios', status: 'shutdown' }] })) + await waitForMessage(agentA) const agentB = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agentB) - agentB.send(JSON.stringify({ type: 'agent:register', agentName: 'Mac-B', devices: [{ id: 'b1', name: 'iPhone', platform: 'ios', status: 'shutdown' }] })) - await waitForMessage(agentB) + agentB.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'Mac-B', devices: [{ id: 'b1', name: 'iPhone', platform: 'ios', status: 'shutdown' }] })) + await waitForMessage(agentB) agentA.close() @@ -925,7 +925,7 @@ describe('RelayServer', () => { await waitForOpen(observer) await vi.waitFor(async () => { observer.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForMessage(observer) + const listed = await waitForMessage(observer) const sessions = listed.sessions! expect(sessions).toHaveLength(1) expect(sessions[0].agentName).toBe('Mac-B') @@ -944,16 +944,16 @@ describe('RelayServer', () => { const agent = new WebSocket(`ws://localhost:${shortPort}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices: [{ id: 'devA', name: 'A', platform: 'ios', status: 'shutdown' }] })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-20', devices: [{ id: 'devA', name: 'A', platform: 'ios', status: 'shutdown' }] })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${shortPort}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) // session:joined + await waitForMessage(browser) // session:joined - const shutdownPromise = waitForType(agent, 'device:shutdown') + const shutdownPromise = waitForType(agent, 'device:shutdown') browser.close() const shutdown = await shutdownPromise @@ -974,14 +974,14 @@ describe('RelayServer', () => { const agent = new WebSocket(`ws://localhost:${shortPort}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices: [{ id: 'devA', name: 'A', platform: 'ios', status: 'shutdown' }] })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-21', devices: [{ id: 'devA', name: 'A', platform: 'ios', status: 'shutdown' }] })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${shortPort}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) + await waitForMessage(browser) browser.close() // yield to event loop so relay receives close event and sets idle timer @@ -990,7 +990,7 @@ describe('RelayServer', () => { const browser2 = new WebSocket(`ws://localhost:${shortPort}`) await waitForOpen(browser2) browser2.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser2) // session:joined — relay cancels idle timer + await waitForMessage(browser2) // session:joined — relay cancels idle timer // If shutdown arrives at any point during the wait, fail immediately await new Promise((resolve, reject) => { @@ -1019,14 +1019,14 @@ describe('RelayServer', () => { const agent = new WebSocket(`ws://localhost:${shortPort}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices: [{ id: 'devA', name: 'A', platform: 'ios', status: 'shutdown' }] })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-22', devices: [{ id: 'devA', name: 'A', platform: 'ios', status: 'shutdown' }] })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${shortPort}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) + await waitForMessage(browser) // Agent closes first → all sessions removed. Browser closes → idle timer set on now-removed session. agent.close() @@ -1042,18 +1042,18 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-23', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) // session:joined + await waitForMessage(browser) // session:joined // Agent reports device is ready agent.send(JSON.stringify({ type: 'device:ready', sessionId, payload: { deviceId: 'devA' } })) - await waitForType(browser, 'device:ready') + await waitForType(browser, 'device:ready') browser.close() // Poll until relay has cleared browser from session (busy=false) @@ -1062,16 +1062,16 @@ describe('RelayServer', () => { await waitForOpen(tmpObs) await vi.waitFor(async () => { tmpObs.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForMessage(tmpObs) - expect(listed.sessions![0].devices[0].busy).toBe(false) + const listed = await waitForMessage(tmpObs) + expect(listed.sessions[0]!.devices[0]!.busy).toBe(false) }, { timeout: 2000 }) tmpObs.close() // Browser reconnects — set up both listeners before sending to avoid race const browser2 = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser2) - const joinedPromise = waitForType(browser2, 'session:joined') - const readyPromise = waitForType(browser2, 'device:ready') + const joinedPromise = waitForType(browser2, 'session:joined') + const readyPromise = waitForType(browser2, 'device:ready') browser2.send(JSON.stringify({ type: 'session:start', sessionId })) await joinedPromise await readyPromise @@ -1084,18 +1084,18 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'booted' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-24', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) + await waitForMessage(browser) // session:start on a booted device now also sends the agent a join-IDR (stream:request-idr), // so wait specifically for open-url rather than the next message. - const msgPromise = waitForType(agent, 'open-url') + const msgPromise = waitForType(agent, 'open-url') browser.send(JSON.stringify({ type: 'open-url', sessionId, requestId: 'req-fwd', payload: { url: 'myapp://home' } })) const received = await msgPromise expect(received.type).toBe('open-url') @@ -1115,18 +1115,18 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'booted' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-25', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId agent.send(JSON.stringify({ type: 'device:ready', sessionId, payload: { deviceId: 'devA' } })) // Round-trip on the agent socket rather than a sleep: the browser joins on a different // connection, so nothing orders its session:start against the device:ready above. - const listed = waitForType(agent, 'agents:listed') + const listed = waitForType(agent, 'agents:listed') agent.send(JSON.stringify({ type: 'agents:list' })) await listed - const idrPromise = waitForType(agent, 'stream:request-idr') + const idrPromise = waitForType(agent, 'stream:request-idr') const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) @@ -1141,17 +1141,17 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'booted' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-26', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) + await waitForMessage(browser) - const msgPromise = waitForMessage(browser) - agent.send(JSON.stringify({ type: 'open-url:done', sessionId })) + const msgPromise = waitForMessage(browser) + agent.send(JSON.stringify({ type: 'open-url:done', sessionId, requestId: 'rq-done' })) const received = await msgPromise expect(received.type).toBe('open-url:done') @@ -1163,17 +1163,17 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'booted' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-27', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) + await waitForMessage(browser) - const msgPromise = waitForMessage(browser) - agent.send(JSON.stringify({ type: 'open-url:error', sessionId, message: 'URL handler not found' })) + const msgPromise = waitForMessage(browser) + agent.send(JSON.stringify({ type: 'open-url:error', sessionId, requestId: 'rq-err', message: 'URL handler not found' })) const received = await msgPromise expect(received.type).toBe('open-url:error') expect(received.message).toBe('URL handler not found') @@ -1186,7 +1186,7 @@ describe('RelayServer', () => { const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) - const msgPromise = waitForMessage(browser) + const msgPromise = waitForMessage(browser) browser.send(JSON.stringify({ type: 'open-url', sessionId: 'nonexistent-session', requestId: 'req-nos', payload: { url: 'myapp://home' } })) const received = await msgPromise expect(received.type).toBe('open-url:error') @@ -1207,14 +1207,14 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'booted' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-28', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) + await waitForMessage(browser) browser.send(JSON.stringify({ type: 'open-url', sessionId, payload: { url: 'myapp://home' } })) // Two barriers, on two sockets, and both are load-bearing. Order is preserved **within** a @@ -1230,7 +1230,7 @@ describe('RelayServer', () => { // proves a test *can* fail, not that it always will. await barrier(browser) await barrier(agent) - expect(await waitForTypeOrNull(agent, 'open-url', 0)).toBeNull() + expect(await waitForTypeOrNull(agent, 'open-url', 0)).toBeNull() agent.close(); browser.close() }) @@ -1245,7 +1245,7 @@ describe('RelayServer', () => { browser.send(JSON.stringify({ type: 'open-url', sessionId: 'nonexistent-session', payload: { url: 'myapp://home' } })) await barrier(browser) - expect(await waitForTypeOrNull(browser, 'open-url:error', 0)).toBeNull() + expect(await waitForTypeOrNull(browser, 'open-url:error', 0)).toBeNull() browser.close() }) @@ -1254,14 +1254,14 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'booted' }] const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-29', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForMessage(browser) + await waitForMessage(browser) agent.close() await new Promise((resolve) => setTimeout(resolve, 50)) @@ -1269,7 +1269,7 @@ describe('RelayServer', () => { // By type, not "the next message": losing the agent also produces a `session:terminated`, and // whichever lands first is not this test's subject. browser.send(JSON.stringify({ type: 'open-url', sessionId, requestId: 'req-closed', payload: { url: 'myapp://home' } })) - const received = await waitForType(browser, 'open-url:error') + const received = await waitForType(browser, 'open-url:error') expect(received.message).toBe('agent offline') expect(received.requestId).toBe('req-closed') @@ -1281,9 +1281,9 @@ describe('RelayServer', () => { const devices = [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] const agent = new WebSocket(`ws://localhost:${p}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-30', devices })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId return { agent, sessionId } } @@ -1295,7 +1295,7 @@ describe('RelayServer', () => { const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - const msg = await waitForMessage(browser) + const msg = await waitForMessage(browser) expect(msg.type).toBe('error') expect(msg.message).toBe('Agent resources exhausted') expect(msg.sessionId).toBe(sessionId) @@ -1312,7 +1312,7 @@ describe('RelayServer', () => { const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - const msg = await waitForMessage(browser) + const msg = await waitForMessage(browser) expect(msg.type).toBe('error') expect(msg.message).toBe('Agent resources exhausted') expect(msg.sessionId).toBe(sessionId) @@ -1329,7 +1329,7 @@ describe('RelayServer', () => { const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - const msg = await waitForMessage(browser) + const msg = await waitForMessage(browser) expect(msg.type).toBe('session:joined') agent.close() @@ -1344,7 +1344,7 @@ describe('RelayServer', () => { const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - const msg = await waitForMessage(browser) + const msg = await waitForMessage(browser) expect(msg.type).toBe('session:joined') agent.close() @@ -1357,7 +1357,7 @@ describe('RelayServer', () => { const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - const msg = await waitForMessage(browser) + const msg = await waitForMessage(browser) expect(msg.type).toBe('session:joined') agent.close() @@ -1384,13 +1384,13 @@ describe('RelayServer', () => { const ws = new WebSocket(`ws://localhost:${port}`) await waitForOpen(ws) ws.send(JSON.stringify({ type: 'session:start', sessionId: 'nonexistent' })) - await waitForMessage(ws) // error: Session not found — browser role assigned + await waitForMessage(ws) // error: Session not found — browser role assigned // Agent-only message from a browser-role socket must close the connection const closePromise = new Promise((resolve) => ws.once('close', (code) => resolve(code)) ) - ws.send(JSON.stringify({ type: 'agent:register', devices: [] })) + ws.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-31', devices: [] })) const code = await closePromise expect(code).toBe(1008) }) @@ -1400,15 +1400,15 @@ describe('RelayServer', () => { // 정당한 agent + 세션을 만들어 탈취 대상 sessionId를 확보 const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices: [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-32', devices: [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId // browser 역할을 확정시킨 뒤 stream:register 시도 const ws = new WebSocket(`ws://localhost:${port}`) await waitForOpen(ws) ws.send(JSON.stringify({ type: 'session:start', sessionId: 'nonexistent' })) - await waitForMessage(ws) // error → browser role assigned + await waitForMessage(ws) // error → browser role assigned const closePromise = new Promise((resolve) => ws.once('close', (code) => resolve(code)) @@ -1465,8 +1465,8 @@ describe('RelayServer', () => { const token = insertPat('agent') const ws = agentWs(token) await waitForOpen(ws) - ws.send(JSON.stringify({ type: 'agent:register', devices: [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] })) - const msg = await waitForMessage(ws) + ws.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-49', devices: [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] })) + const msg = await waitForMessage(ws) expect(msg.type).toBe('agent:registered') expect(msg.registeredSessions).toHaveLength(1) ws.close() @@ -1477,7 +1477,7 @@ describe('RelayServer', () => { const token = insertPat('view,builds:write') const ws = agentWs(token) await waitForOpen(ws) - ws.send(JSON.stringify({ type: 'agent:register', devices: [] })) + ws.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-50', devices: [] })) const { code } = await waitForClose(ws) expect(code).toBe(1008) }) @@ -1495,14 +1495,14 @@ describe('RelayServer', () => { const token = insertPat('agent') const agent = agentWs(token) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices: [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] })) - const { registeredSessions } = await waitForMessage(agent) - const sessionId = registeredSessions![0].sessionId + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'RelayServer-51', devices: [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }] })) + const { registeredSessions } = await waitForMessage(agent) + const sessionId = registeredSessions[0]!.sessionId const stream = agentWs(token) await waitForOpen(stream) stream.send(JSON.stringify({ type: 'stream:register', sessionId })) - const msg = await waitForMessage(stream) + const msg = await waitForMessage(stream) expect(msg.type).toBe('stream:registered') agent.close() stream.close() diff --git a/packages/relay/src/__tests__/SessionManager.test.ts b/packages/relay/src/__tests__/SessionManager.test.ts index 6b2ca9c2..5e2149c2 100644 --- a/packages/relay/src/__tests__/SessionManager.test.ts +++ b/packages/relay/src/__tests__/SessionManager.test.ts @@ -358,20 +358,20 @@ describe('SessionManager', () => { const ws = mockSocket() const [idA] = sm.create(ws, [{ id: 'devA', name: 'A', platform: 'ios', status: 'shutdown' }]) const listed = sm.list() - expect(listed[0].devices[0].sessionId).toBe(idA) + expect(listed[0].devices[0]!.sessionId).toBe(idA) }) it('reflects busy=true when browserSocket is set', () => { const sm = new SessionManager() const [id] = sm.create(mockSocket(), [{ id: 'd1', name: 'X', platform: 'ios', status: 'shutdown' }]) sm.join(id, mockSocket()) - expect(sm.list()[0].devices[0].busy).toBe(true) + expect(sm.list()[0].devices[0]!.busy).toBe(true) }) it('reflects busy=false when browserSocket is null', () => { const sm = new SessionManager() sm.create(mockSocket(), [{ id: 'd1', name: 'X', platform: 'ios', status: 'shutdown' }]) - expect(sm.list()[0].devices[0].busy).toBe(false) + expect(sm.list()[0].devices[0]!.busy).toBe(false) }) }) diff --git a/packages/relay/src/__tests__/agentReconnectGrace.test.ts b/packages/relay/src/__tests__/agentReconnectGrace.test.ts index 26575fd7..285dabcc 100644 --- a/packages/relay/src/__tests__/agentReconnectGrace.test.ts +++ b/packages/relay/src/__tests__/agentReconnectGrace.test.ts @@ -6,7 +6,7 @@ import { WebSocket } from 'ws' import { RelayServer } from '../RelayServer' import { initDb, closeDb } from '../db' import { barrier, waitForOpen, waitForType, waitForTypeOrNull } from '@tapflowio/test-utils' -import type { RelayMessage } from '../types' +import type { AgentRegistered, AgentsListed, DeviceBootError, SessionAgentAway, SessionJoined, SessionRebound, SessionTerminated } from '@tapflowio/protocol' // #426 stage 3. Stage 2 taught the relay to re-point a session at a restarted agent's socket, and // it worked — but only while the old socket was still open. On a real restart it never was: the @@ -54,8 +54,8 @@ describe('a session outlives its agent socket long enough to be reclaimed (#426) type: 'agent:register', agentId, agentName: 'the-mac', platform: 'ios', devices, capabilities: ['clipboard'], })) - const reply = await waitForType(agent, 'agent:registered') - const byDevice = new Map(reply.registeredSessions!.map((r) => [r.deviceId, r.sessionId])) + const reply = await waitForType(agent, 'agent:registered') + const byDevice = new Map(reply.registeredSessions.map((r) => [r.deviceId, r.sessionId])) return { agent, byDevice } } @@ -63,7 +63,7 @@ describe('a session outlives its agent socket long enough to be reclaimed (#426) const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - const joined = await waitForType(browser, 'session:joined') + const joined = await waitForType(browser, 'session:joined') return Object.assign(browser, { joined }) } @@ -97,7 +97,7 @@ describe('a session outlives its agent socket long enough to be reclaimed (#426) async function devices(ws: WebSocket) { ws.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForType(ws, 'agents:listed') + const listed = await waitForType(ws, 'agents:listed') return (listed.sessions ?? []).flatMap((s) => s.devices) } @@ -114,7 +114,7 @@ describe('a session outlives its agent socket long enough to be reclaimed (#426) await closeAndHeld(first.agent, browser) const second = await register() - const rebound = await waitForType(browser, 'session:rebound') + const rebound = await waitForType(browser, 'session:rebound') expect(rebound.sessionId).toBe(sessionId) expect(second.byDevice.get('devA')).toBe(sessionId) @@ -127,7 +127,7 @@ describe('a session outlives its agent socket long enough to be reclaimed (#426) await closeAndSettle(first.agent) - const away = await waitForType(browser, 'session:agent-away') + const away = await waitForType(browser, 'session:agent-away') expect(away.sessionId).toBe(first.byDevice.get('devA')) // Nothing has been decided yet — the frozen frame is explained, not resolved. Only // `session:terminated` is worth asserting: no second agent registers here, so `session:rebound` @@ -144,7 +144,7 @@ describe('a session outlives its agent socket long enough to be reclaimed (#426) await closeAndSettle(first.agent) await waitForType(browser, 'session:agent-away') - const ended = await waitForType(browser, 'session:terminated') + const ended = await waitForType(browser, 'session:terminated') expect(ended.reason).toBe('agent-disconnected') browser.close() @@ -183,7 +183,7 @@ describe('a session outlives its agent socket long enough to be reclaimed (#426) const second = await register() - expect((await waitForType(browserB, 'session:rebound')).sessionId).toBe(sessionId) + expect((await waitForType(browserB, 'session:rebound')).sessionId).toBe(sessionId) second.agent.close(); browserB.close() }) @@ -225,7 +225,7 @@ describe('a session outlives its agent socket long enough to be reclaimed (#426) browser.send(JSON.stringify({ type: 'device:boot', sessionId, requestId: 'rq-grace', payload: { deviceId: 'devA' } })) - expect((await waitForType(browser, 'device:boot-error')).message).toBe('Session not found') + expect((await waitForType(browser, 'device:boot-error')).message).toBe('Session not found') browser.close() }) @@ -322,7 +322,7 @@ describe('a session outlives its agent socket long enough to be reclaimed (#426) const second = await register([DEV_A], 'a-different-machine-id') - expect((await waitForType(browser, 'session:terminated')).reason).toBe('agent-disconnected') + expect((await waitForType(browser, 'session:terminated')).reason).toBe('agent-disconnected') expect((await devices(second.agent)).map((d) => d.id)).toEqual(['devA']) second.agent.close(); browser.close() @@ -363,7 +363,7 @@ describe('a session outlives its agent socket long enough to be reclaimed (#426) const agent = new WebSocket(`ws://localhost:${ownPort}`) await waitForOpen(agent) agent.send(JSON.stringify({ - type: 'agent:register', agentId: 'mac-1', platform: 'ios', devices: [DEV_A], + type: 'agent:register', agentName: 'agentReconnectGrace-1', agentId: 'mac-1', platform: 'ios', devices: [DEV_A], })) await waitForType(agent, 'agent:registered') diff --git a/packages/relay/src/__tests__/appCommandErrors.test.ts b/packages/relay/src/__tests__/appCommandErrors.test.ts index 21e277de..d01c1a46 100644 --- a/packages/relay/src/__tests__/appCommandErrors.test.ts +++ b/packages/relay/src/__tests__/appCommandErrors.test.ts @@ -5,8 +5,8 @@ import path from 'path' import { WebSocket } from 'ws' import { RelayServer } from '../RelayServer' import { initDb, closeDb, getDb } from '../db' -import type { RelayMessage } from '../types' import { barrier, waitForOpen, waitForType, waitForTypeOrNull } from '@tapflowio/test-utils' +import type { AgentRegistered } from '@tapflowio/protocol' // #445: every failure of app:install / app:launch has to reach the caller, carrying the sessionId @@ -42,11 +42,11 @@ describe('app command failures reach the caller (#445)', () => { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) agent.send(JSON.stringify({ - type: 'agent:register', + type: 'agent:register', platform: 'ios', agentName: 'appCommandErrors-1', devices: [{ id: 'dev-1', name: 'iPhone', platform: 'ios', status: 'booted' }], })) - const reply = await waitForType(agent, 'agent:registered') - const sessionId = reply.registeredSessions![0]!.sessionId + const reply = await waitForType(agent, 'agent:registered') + const sessionId = reply.registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) @@ -220,7 +220,8 @@ describe('app command failures reach the caller (#445)', () => { agent.close(); browser.close() }) - // `JSON.parse` does not honour the `RelayMessage` type, so buildId arrives as whatever was sent. + // The schema refuses a non-integer `buildId` at the door now, so this asserts the *answer* the + // handler gives rather than the parse — the caller gets `Build not found` instead of silence. // An object or array makes better-sqlite3 throw, and that exception used to be caught by the // message loop alongside genuine parse failures — the caller got nothing at all. This is the // same silence the rest of the file is about, reached through the type system's blind spot. diff --git a/packages/relay/src/__tests__/clearState.test.ts b/packages/relay/src/__tests__/clearState.test.ts index b0a70cdb..7dd2cf0c 100644 --- a/packages/relay/src/__tests__/clearState.test.ts +++ b/packages/relay/src/__tests__/clearState.test.ts @@ -5,9 +5,16 @@ import path from 'path' import { WebSocket } from 'ws' import { RelayServer } from '../RelayServer' import { initDb, closeDb } from '../db' -import type { RelayMessage } from '../types' import { waitForOpen, waitForType } from '@tapflowio/test-utils' +import type { AgentRegistered, BrowserToRelay, RelayToAgent } from '@tapflowio/protocol' + +/** What an agent socket actually receives. `RelayToAgent` is only the half the relay + * originates or rebuilds; browser commands it forwards verbatim (`app:clear-state`, + * `clipboard:*`, `input:*`) arrive unchanged and are declared in `BrowserToRelay`. The + * protocol has no single union for this — #557. */ +type AgentSocketInbound = RelayToAgent | BrowserToRelay + describe('app:clear-state relay routing', () => { let server: RelayServer @@ -38,11 +45,11 @@ describe('app:clear-state relay routing', () => { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) agent.send(JSON.stringify({ - type: 'agent:register', + type: 'agent:register', platform: 'ios', agentName: 'clearState-1', devices: [{ id: 'dev-1', name: 'iPhone', platform: 'ios', status: 'booted' }], })) - const reply = await waitForType(agent, 'agent:registered') - const sessionId = reply.registeredSessions![0].sessionId + const reply = await waitForType(agent, 'agent:registered') + const sessionId = reply.registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) @@ -55,10 +62,10 @@ describe('app:clear-state relay routing', () => { const { agent, browser, sessionId } = await setup() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'app:clear-state') { expect((msg.payload as { bundleId: string }).bundleId).toBe('com.example.app') - agent.send(JSON.stringify({ type: 'app:clear-state-done', sessionId: msg.sessionId })) + agent.send(JSON.stringify({ type: 'app:clear-state-done', sessionId: msg.sessionId, requestId: msg.requestId })) } }) @@ -88,9 +95,9 @@ describe('app:clear-state relay routing', () => { const { agent, browser, sessionId } = await setup() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'app:clear-state') { - agent.send(JSON.stringify({ type: 'app:clear-state-error', sessionId: msg.sessionId, message: 'pm clear failed' })) + agent.send(JSON.stringify({ type: 'app:clear-state-error', sessionId: msg.sessionId, requestId: msg.requestId, message: 'pm clear failed' })) } }) @@ -123,7 +130,7 @@ describe('app:clear-state relay routing', () => { const { agent, browser, sessionId } = await setup() const closed = new Promise((resolve) => browser.on('close', (code) => resolve(code))) - browser.send(JSON.stringify({ type: 'app:clear-state-done', sessionId })) + browser.send(JSON.stringify({ type: 'app:clear-state-done', sessionId, requestId: 'rq-clear' })) expect(await closed).toBe(1008) agent.close() @@ -133,9 +140,9 @@ describe('app:clear-state relay routing', () => { const { agent, browser, sessionId } = await setup() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'input:type') { - agent.send(JSON.stringify({ type: 'input:type-done', sessionId: msg.sessionId })) + agent.send(JSON.stringify({ type: 'input:type-done', sessionId: msg.sessionId, requestId: msg.requestId })) } }) @@ -151,7 +158,7 @@ describe('app:clear-state relay routing', () => { const { agent, browser, sessionId } = await setup() const closed = new Promise((resolve) => browser.on('close', (code) => resolve(code))) - browser.send(JSON.stringify({ type: 'input:type-done', sessionId })) + browser.send(JSON.stringify({ type: 'input:type-done', sessionId, requestId: 'rq-type' })) expect(await closed).toBe(1008) agent.close() diff --git a/packages/relay/src/__tests__/clipboard.test.ts b/packages/relay/src/__tests__/clipboard.test.ts index d23a1367..bcb5fa27 100644 --- a/packages/relay/src/__tests__/clipboard.test.ts +++ b/packages/relay/src/__tests__/clipboard.test.ts @@ -5,9 +5,16 @@ import path from 'path' import { WebSocket } from 'ws' import { RelayServer } from '../RelayServer' import { initDb, closeDb } from '../db' -import type { RelayMessage } from '../types' import { barrier, waitForOpen, waitForType, waitForTypeOrNull } from '@tapflowio/test-utils' +import type { AgentRegistered, BrowserInbound, BrowserToRelay, RelayToAgent } from '@tapflowio/protocol' + +/** What an agent socket actually receives. `RelayToAgent` is only the half the relay + * originates or rebuilds; browser commands it forwards verbatim (`app:clear-state`, + * `clipboard:*`, `input:*`) arrive unchanged and are declared in `BrowserToRelay`. The + * protocol has no single union for this — #557. */ +type AgentSocketInbound = RelayToAgent | BrowserToRelay + describe('clipboard bridge relay routing', () => { let server: RelayServer @@ -38,11 +45,11 @@ describe('clipboard bridge relay routing', () => { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) agent.send(JSON.stringify({ - type: 'agent:register', + type: 'agent:register', platform: 'ios', agentName: 'clipboard-1', devices: [{ id: 'dev-1', name: 'iPhone', platform: 'ios', status: 'booted' }], })) - const reply = await waitForType(agent, 'agent:registered') - const sessionId = reply.registeredSessions![0].sessionId + const reply = await waitForType(agent, 'agent:registered') + const sessionId = reply.registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) @@ -58,12 +65,12 @@ describe('clipboard bridge relay routing', () => { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) agent.send(JSON.stringify({ - type: 'agent:register', + type: 'agent:register', platform: 'ios', agentName: 'clipboard-1', capabilities: ['clipboard'], devices: [{ id: 'dev-1', name: 'iPhone', platform: 'ios', status: 'booted' }], })) - const reply = await waitForType(agent, 'agent:registered') - const sessionId = reply.registeredSessions![0].sessionId + const reply = await waitForType(agent, 'agent:registered') + const sessionId = reply.registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) @@ -93,7 +100,7 @@ describe('clipboard bridge relay routing', () => { const { agent, browser, sessionId } = await setup() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'clipboard:read') { agent.send(JSON.stringify({ type: 'clipboard:data', @@ -119,7 +126,7 @@ describe('clipboard bridge relay routing', () => { // Unicode must survive the JSON round trip untouched — the whole point of the bridge. const text = '한글 テスト 🎉\nline2\ttab' agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'clipboard:write') { expect((msg.payload as { text: string }).text).toBe(text) agent.send(JSON.stringify({ type: 'clipboard:write-done', sessionId: msg.sessionId, requestId: msg.requestId })) @@ -138,7 +145,7 @@ describe('clipboard bridge relay routing', () => { const { agent, browser, sessionId } = await setup() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'clipboard:read') { agent.send(JSON.stringify({ type: 'clipboard:error', sessionId: msg.sessionId, requestId: msg.requestId, message: 'no booted device', @@ -198,8 +205,8 @@ describe('clipboard bridge relay routing', () => { it('a browser socket cannot inject clipboard:data into its own viewer', async () => { const { agent, browser, sessionId } = await setup() - const got: RelayMessage[] = [] - browser.on('message', (d) => got.push(JSON.parse(d.toString()) as RelayMessage)) + const got: BrowserInbound[] = [] + browser.on('message', (d) => got.push(JSON.parse(d.toString()) as BrowserInbound)) const closed = new Promise((r) => browser.on('close', (code) => r(code))) browser.send(JSON.stringify({ @@ -221,13 +228,13 @@ describe('clipboard bridge relay routing', () => { const rogue = new WebSocket(`ws://localhost:${port}`) await waitForOpen(rogue) rogue.send(JSON.stringify({ - type: 'agent:register', + type: 'agent:register', platform: 'ios', agentName: 'clipboard-2', devices: [{ id: 'rogue-1', name: 'Rogue', platform: 'ios', status: 'booted' }], })) await waitForType(rogue, 'agent:registered') - const got: RelayMessage[] = [] - browser.on('message', (d) => got.push(JSON.parse(d.toString()) as RelayMessage)) + const got: BrowserInbound[] = [] + browser.on('message', (d) => got.push(JSON.parse(d.toString()) as BrowserInbound)) rogue.send(JSON.stringify({ type: 'clipboard:data', sessionId, requestId: 'clip-1', payload: { text: 'ATTACKER' }, diff --git a/packages/relay/src/__tests__/deviceReadyReplay.test.ts b/packages/relay/src/__tests__/deviceReadyReplay.test.ts index d56ddb95..ea354eef 100644 --- a/packages/relay/src/__tests__/deviceReadyReplay.test.ts +++ b/packages/relay/src/__tests__/deviceReadyReplay.test.ts @@ -6,7 +6,7 @@ import { WebSocket } from 'ws' import { RelayServer } from '../RelayServer' import { initDb, closeDb } from '../db' import { barrier, waitForOpen, waitForType, waitForTypeOrNull } from '@tapflowio/test-utils' -import type { RelayMessage } from '../types' +import type { AgentRegistered, AgentsListed, DeviceReady, SessionChrome, SessionDeviceInfo, SessionJoined, StreamRegistered, StreamRequestIdr } from '@tapflowio/protocol' // #440: the relay replays `device:ready` so a browser that reconnects mid-stream gets a picture @@ -43,22 +43,22 @@ describe('device:ready replay tracks the session, not the device (#440)', () => const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) agent.send(JSON.stringify({ - type: 'agent:register', + type: 'agent:register', platform: 'ios', agentName: 'deviceReadyReplay-1', devices: [{ id: 'devA', name: 'iPhone A', platform: 'ios', status }], })) - const reply = await waitForType(agent, 'agent:registered') - return { agent, sessionId: reply.registeredSessions![0]!.sessionId } + const reply = await waitForType(agent, 'agent:registered') + return { agent, sessionId: reply.registeredSessions[0]!.sessionId } } async function joinAs(sessionId: string) { const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - await waitForType(browser, 'session:joined') + await waitForType(browser, 'session:joined') // The barrier proves the relay is done with the join, so whatever it was going to send is // already recorded — `waitForTypeOrNull` reads that recording rather than racing it. await barrier(browser) - const ready = await waitForTypeOrNull(browser, 'device:ready', 0) + const ready = await waitForTypeOrNull(browser, 'device:ready', 0) return { browser, ready } } @@ -93,8 +93,8 @@ describe('device:ready replay tracks the session, not the device (#440)', () => // recording `waitForType` reads from is attached by its first call, so a reply that arrives before // that call is not queued anywhere and the wait hangs. `handleSessionStart` sends the join and both // replays in one turn, so awaiting them one after the other flakes — measured, once. - const chrome = waitForType(browser, 'session:chrome') - const info = waitForType(browser, 'session:deviceInfo') + const chrome = waitForType(browser, 'session:chrome') + const info = waitForType(browser, 'session:deviceInfo') browser.send(JSON.stringify({ type: 'session:start', sessionId })) expect((await chrome).sessionId).toBe(sessionId) @@ -114,7 +114,7 @@ describe('device:ready replay tracks the session, not the device (#440)', () => // The IDR request rides the same branch, and it goes out during the join — so the listener has // to exist before it. Asserting it here keeps it covered: moving it out of the replay block // would otherwise be caught by nothing. - const idrPromise = waitForType(agent, 'stream:request-idr') + const idrPromise = waitForType(agent, 'stream:request-idr') const { browser, ready } = await joinAs(sessionId) expect(ready).not.toBeNull() @@ -166,7 +166,7 @@ describe('device:ready replay tracks the session, not the device (#440)', () => const streamWs = new WebSocket(`ws://localhost:${port}`) await waitForOpen(streamWs) streamWs.send(JSON.stringify({ type: 'stream:register', sessionId })) - await waitForType(streamWs, 'stream:registered') + await waitForType(streamWs, 'stream:registered') const closed = new Promise((r) => streamWs.on('close', () => r())) streamWs.close() await closed @@ -187,9 +187,9 @@ describe('device:ready replay tracks the session, not the device (#440)', () => const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForType(browser, 'agents:listed') + const listed = await waitForType(browser, 'agents:listed') - const device = listed.sessions![0]!.devices.find((d) => d.sessionId === sessionId) + const device = listed.sessions[0]!.devices.find((d) => d.sessionId === sessionId) expect(device?.status).toBe('booted') agent.close(); browser.close() diff --git a/packages/relay/src/__tests__/inputErrorReason.test.ts b/packages/relay/src/__tests__/inputErrorReason.test.ts index f48e3284..95fe87c2 100644 --- a/packages/relay/src/__tests__/inputErrorReason.test.ts +++ b/packages/relay/src/__tests__/inputErrorReason.test.ts @@ -5,8 +5,8 @@ import path from 'path' import { WebSocket } from 'ws' import { RelayServer } from '../RelayServer' import { initDb, closeDb } from '../db' -import type { RelayMessage } from '../types' import { barrier, waitForOpen, waitForType, waitForTypeOrNull } from '@tapflowio/test-utils' +import type { AgentRegistered, AppInstallError, GenericError, InputError, InputTouchEnd, InputTypeError } from '@tapflowio/protocol' // #492. The relay answers a terminal input it cannot dispatch, and it was the last producer of // `input:error` sending no `reason` — while being the one that knows the answer with the most @@ -44,11 +44,11 @@ describe('input:error from the relay carries a reason (#492)', () => { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) agent.send(JSON.stringify({ - type: 'agent:register', + type: 'agent:register', platform: 'ios', agentName: 'inputErrorReason-1', devices: [{ id: 'dev-1', name: 'iPhone', platform: 'ios', status: 'booted' }], })) - const reply = await waitForType(agent, 'agent:registered') - const sessionId = reply.registeredSessions![0].sessionId + const reply = await waitForType(agent, 'agent:registered') + const sessionId = reply.registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) @@ -150,11 +150,11 @@ describe('input:error from the relay carries a reason (#492)', () => { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) agent.send(JSON.stringify({ - type: 'agent:register', + type: 'agent:register', platform: 'ios', agentName: 'inputErrorReason-1', devices: [{ id: 'dev-1', name: 'iPhone', platform: 'ios', status: 'booted' }], })) - const reg = await waitForType(agent, 'agent:registered') - const sessionId = reg.registeredSessions![0]!.sessionId + const reg = await waitForType(agent, 'agent:registered') + const sessionId = reg.registeredSessions[0]!.sessionId const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) @@ -188,7 +188,7 @@ describe('input:error from the relay carries a reason (#492)', () => { // The control for the two below: the ownership check must not refuse the normal path. const { agent, browser, sessionId } = await live() - const fwd = waitForType(agent, 'input:touch:end') + const fwd = waitForType(agent, 'input:touch:end') browser.send(JSON.stringify({ type: 'input:touch:end', sessionId, requestId: 'rq-own', payload: { x: 0.5, y: 0.5 }, })) @@ -208,7 +208,7 @@ describe('input:error from the relay carries a reason (#492)', () => { other.send(JSON.stringify({ type: 'input:touch:end', sessionId, requestId: 'rq-inject', payload: { x: 0.5, y: 0.5 }, })) - const err = await waitForType(other, 'input:error') + const err = await waitForType(other, 'input:error') expect(err.reason).toBe('not-session-owner') expect(err.requestId).toBe('rq-inject') @@ -239,7 +239,7 @@ describe('input:error from the relay carries a reason (#492)', () => { other.send(JSON.stringify({ type: 'input:touch:end', sessionId, requestId: 'rq-unheld', payload: { x: 0.5, y: 0.5 }, })) - const err = await waitForType(other, 'input:error') + const err = await waitForType(other, 'input:error') expect(err.reason).toBe('not-session-owner') expect(err.message).toBe('session not joined') @@ -256,7 +256,7 @@ describe('input:error from the relay carries a reason (#492)', () => { other.send(JSON.stringify({ type: 'input:key', sessionId, requestId: 'rq-held-prose', payload: { code: 'KeyA' }, })) - const err = await waitForType(other, 'input:error') + const err = await waitForType(other, 'input:error') expect(err.reason).toBe('not-session-owner') expect(err.message).toBe('session held by another client') @@ -269,7 +269,7 @@ describe('input:error from the relay carries a reason (#492)', () => { const other = await outsider() other.send(JSON.stringify({ type: 'input:type', sessionId, requestId: 'rq-t', payload: { text: 'hi' } })) - const err = await waitForType(other, 'input:type-error') + const err = await waitForType(other, 'input:type-error') expect(err.requestId).toBe('rq-t') // The reason rides this shape too. Without it, `not-session-owner` was unreachable for one of the five @@ -374,7 +374,7 @@ describe('input:error from the relay carries a reason (#492)', () => { const other = await outsider() other.send(JSON.stringify({ type, sessionId, requestId: `rq-${type}`, ...extra })) - const err = await waitForType(other, errType) + const err = await waitForType(other, errType) expect(err.message).toBe('session held by another client') // The correlator rides the refusal, or the caller cannot attribute it and waits out its deadline — @@ -397,7 +397,7 @@ describe('input:error from the relay carries a reason (#492)', () => { const other = await outsider() other.send(JSON.stringify({ type: 'app:install', sessionId, requestId: 'rq-inst', buildId: 999999 })) - const err = await waitForType(other, 'app:install-error') + const err = await waitForType(other, 'app:install-error') // Not `Build not found`, which is what an owner would get for this buildId. expect(err.message).toBe('session held by another client') @@ -419,8 +419,8 @@ describe('input:error from the relay carries a reason (#492)', () => { await barrier(other) // Nothing answered, and the session still works for the socket that holds it. - expect(await waitForTypeOrNull(other, 'error', 0)).toBeNull() - const fwd = waitForType(agent, 'input:touch:end') + expect(await waitForTypeOrNull(other, 'error', 0)).toBeNull() + const fwd = waitForType(agent, 'input:touch:end') browser.send(JSON.stringify({ type: 'input:touch:end', sessionId, requestId: 'rq-survived', payload: { x: 0.5, y: 0.5 }, })) @@ -440,7 +440,7 @@ describe('input:error from the relay carries a reason (#492)', () => { browser.send(JSON.stringify({ type: 'input:touch:end', sessionId, requestId: 'rq-after-leave', payload: { x: 0.5, y: 0.5 }, })) - const err = await waitForType(browser, 'input:error') + const err = await waitForType(browser, 'input:error') expect(err.message).toBe('session not joined') agent.close(); browser.close() diff --git a/packages/relay/src/__tests__/lifecycleCorrelation.test.ts b/packages/relay/src/__tests__/lifecycleCorrelation.test.ts index 248f7219..f49920af 100644 --- a/packages/relay/src/__tests__/lifecycleCorrelation.test.ts +++ b/packages/relay/src/__tests__/lifecycleCorrelation.test.ts @@ -6,7 +6,7 @@ import { WebSocket } from 'ws' import { RelayServer } from '../RelayServer' import { initDb, closeDb } from '../db' import { barrier, waitForOpen, waitForType, waitForTypeOrNull } from '@tapflowio/test-utils' -import type { RelayMessage } from '../types' +import type { AgentRegistered, DeviceBoot, DeviceBootError, DeviceReady, DeviceShutdown, DeviceShutdownDone } from '@tapflowio/protocol' // L5b′. `device:boot` / `device:shutdown` correlate by `requestId`, and unlike the app commands the // correlator on every reply is **optional** — `device:ready`, `device:boot-error` and @@ -42,11 +42,11 @@ describe('lifecycle correlation (device:boot / device:shutdown)', () => { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) agent.send(JSON.stringify({ - type: 'agent:register', + type: 'agent:register', platform: 'ios', agentName: 'lifecycleCorrelation-1', devices: [{ id: 'devA', name: 'iPhone A', platform: 'ios', status }], })) - const reply = await waitForType(agent, 'agent:registered') - return { agent, sessionId: reply.registeredSessions![0]!.sessionId } + const reply = await waitForType(agent, 'agent:registered') + return { agent, sessionId: reply.registeredSessions[0]!.sessionId } } async function joinAs(sessionId: string) { @@ -75,7 +75,7 @@ describe('lifecycle correlation (device:boot / device:shutdown)', () => { type: 'device:boot', sessionId: 'no-such-session', requestId: 'rq-unknown', payload: { deviceId: 'devA' }, })) - const err = await waitForType(browser, 'device:boot-error') + const err = await waitForType(browser, 'device:boot-error') expect(err.message).toBe('Session not found') expect(err.requestId).toBe('rq-unknown') @@ -94,7 +94,7 @@ describe('lifecycle correlation (device:boot / device:shutdown)', () => { browser.send(JSON.stringify({ type: 'device:boot', sessionId, requestId: 'rq-offline', payload: { deviceId: 'devA' }, })) - const err = await waitForType(browser, 'device:boot-error') + const err = await waitForType(browser, 'device:boot-error') // The two diagnoses are deliberately different — reporting a stale session id as a dead Mac sends // the reader after the wrong problem — so both exits need the echo, not just one. @@ -108,7 +108,7 @@ describe('lifecycle correlation (device:boot / device:shutdown)', () => { const { agent, sessionId } = await registerAgent() const browser = await joinAs(sessionId) - const forwarded = waitForType(agent, 'device:boot') + const forwarded = waitForType(agent, 'device:boot') browser.send(JSON.stringify({ type: 'device:boot', sessionId, requestId: 'rq-fwd', payload: { deviceId: 'devA' }, })) @@ -124,7 +124,7 @@ describe('lifecycle correlation (device:boot / device:shutdown)', () => { const { agent, sessionId } = await registerAgent() const browser = await joinAs(sessionId) - const forwarded = waitForType(agent, 'device:shutdown') + const forwarded = waitForType(agent, 'device:shutdown') browser.send(JSON.stringify({ type: 'device:shutdown', sessionId, requestId: 'rq-down', payload: { deviceId: 'devA' }, })) @@ -161,8 +161,8 @@ describe('lifecycle correlation (device:boot / device:shutdown)', () => { await barrier(browser) await barrier(agent) - const forwarded = await waitForTypeOrNull(agent, 'device:boot', 0) - const answered = await waitForTypeOrNull(browser, 'device:boot-error', 0) + const forwarded = await waitForTypeOrNull(agent, 'device:boot', 0) + const answered = await waitForTypeOrNull(browser, 'device:boot-error', 0) agent.close(); browser.close() return { forwarded, answered } } @@ -198,7 +198,7 @@ describe('lifecycle correlation (device:boot / device:shutdown)', () => { const { agent, sessionId } = await registerAgent() const browser = await joinAs(sessionId) - const forwarded = waitForType(agent, 'device:shutdown') + const forwarded = waitForType(agent, 'device:shutdown') browser.send(JSON.stringify({ type: 'device:shutdown', sessionId, payload: { deviceId: 'devA' } })) const msg = await forwarded @@ -234,7 +234,7 @@ describe('lifecycle correlation (device:boot / device:shutdown)', () => { browser.send(JSON.stringify({ type: 'session:start', sessionId })) await waitForType(browser, 'session:joined') await barrier(browser) - const ready = await waitForTypeOrNull(browser, 'device:ready', 0) + const ready = await waitForTypeOrNull(browser, 'device:ready', 0) expect(ready).not.toBeNull() expect('sessionId' in ready!).toBe(false) @@ -251,13 +251,13 @@ describe('lifecycle correlation (device:boot / device:shutdown)', () => { const { agent, sessionId } = await registerAgent() const browser = await joinAs(sessionId) - const ready = waitForType(browser, 'device:ready') + const ready = waitForType(browser, 'device:ready') agent.send(JSON.stringify({ type: 'device:ready', sessionId, requestId: 'rq-echoed', payload: { deviceId: 'devA' }, })) expect((await ready).requestId).toBe('rq-echoed') - const done = waitForType(browser, 'device:shutdown-done') + const done = waitForType(browser, 'device:shutdown-done') agent.send(JSON.stringify({ type: 'device:shutdown-done', sessionId, payload: { deviceId: 'devA' }, })) diff --git a/packages/relay/src/__tests__/screenshot.test.ts b/packages/relay/src/__tests__/screenshot.test.ts index ba71ac94..2dd3aff2 100644 --- a/packages/relay/src/__tests__/screenshot.test.ts +++ b/packages/relay/src/__tests__/screenshot.test.ts @@ -6,10 +6,17 @@ import path from 'path' import { WebSocket } from 'ws' import { RelayServer } from '../RelayServer' import { initDb, closeDb } from '../db' -import type { RelayMessage } from '../types' import { signJwt } from '../middleware/auth' import { waitForOpen, waitForType } from '@tapflowio/test-utils' +import type { AgentRegistered, BrowserToRelay, RelayToAgent } from '@tapflowio/protocol' + +/** What an agent socket actually receives. `RelayToAgent` is only the half the relay + * originates or rebuilds; browser commands it forwards verbatim (`app:clear-state`, + * `clipboard:*`, `input:*`) arrive unchanged and are declared in `BrowserToRelay`. The + * protocol has no single union for this — #557. */ +type AgentSocketInbound = RelayToAgent | BrowserToRelay + const FAKE_PNG = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]) // PNG magic bytes const FAKE_JPEG = Buffer.from([0xff, 0xd8, 0xff, 0xe0]) // JPEG magic bytes @@ -67,11 +74,11 @@ describe('GET /api/v1/sessions/:sessionId/screenshot', () => { async function setupAgent(devices = [{ id: 'dev-1', name: 'iPhone', platform: 'ios', status: 'booted' }]) { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) - const reply = await new Promise((resolve) => + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'screenshot-1', devices })) + const reply = await new Promise((resolve) => agent.once('message', (d) => resolve(JSON.parse(d.toString()))), ) - const sessionId = reply.registeredSessions![0].sessionId + const sessionId = reply.registeredSessions[0]!.sessionId return { agent, sessionId } } @@ -80,7 +87,7 @@ describe('GET /api/v1/sessions/:sessionId/screenshot', () => { // Agent: screenshot:request를 수신하면 즉시 screenshot:done 응답 agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'screenshot:request') { agent.send(JSON.stringify({ type: 'screenshot:done', @@ -117,7 +124,7 @@ describe('GET /api/v1/sessions/:sessionId/screenshot', () => { try { const { agent, sessionId } = await setupAgent() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'screenshot:request') { agent.send(JSON.stringify({ type: 'screenshot:done', @@ -151,7 +158,7 @@ describe('GET /api/v1/sessions/:sessionId/screenshot', () => { try { const { agent, sessionId } = await setupAgent() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'screenshot:request') { agent.send(JSON.stringify({ type: 'screenshot:done', @@ -180,7 +187,7 @@ describe('GET /api/v1/sessions/:sessionId/screenshot', () => { const { agent, sessionId } = await setupAgent() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'screenshot:request') { expect(msg.format).toBe('jpeg') agent.send(JSON.stringify({ @@ -231,7 +238,7 @@ describe('GET /api/v1/sessions/:sessionId/screenshot', () => { // agent는 screenshot:request를 받으면 즉시 종료 (응답 없이) agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'screenshot:request') agent.close() }) @@ -263,7 +270,7 @@ describe('GET /api/v1/sessions/:sessionId/screenshot', () => { const { agent, sessionId } = await setupAgent() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'screenshot:request') { agent.send(JSON.stringify({ type: 'screenshot:error', @@ -307,7 +314,7 @@ describe('GET /api/v1/sessions/:sessionId/screenshot', () => { const { agent, sessionId } = await setupAgent() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'screenshot:request') { agent.send(JSON.stringify({ type: 'screenshot:done', @@ -336,7 +343,7 @@ describe('GET /api/v1/sessions/:sessionId/screenshot', () => { // 잘못된 requestId로 응답 먼저 보내고, 올바른 requestId로 나중에 응답 let correctRequestId: string | undefined agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'screenshot:request') { correctRequestId = msg.requestId // 잘못된 requestId 먼저 @@ -380,12 +387,17 @@ describe('GET /api/v1/sessions/:sessionId/screenshot', () => { let browserGotScreenshotRequest = false browser.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + // **Deliberately not `BrowserInbound`.** This assertion exists to catch the relay putting an + // agent-bound frame on a browser socket, and `screenshot:request` is not a member of that union + // — so typing it by the direction makes the comparison provably false and the compiler says so. + // Assuming the property under test is what the test is for. The relay serializes raw JSON, so a + // routing bug produces this frame here whatever the declarations say. + const msg = JSON.parse(data.toString()) as { type: string } if (msg.type === 'screenshot:request') browserGotScreenshotRequest = true }) agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'screenshot:request') { agent.send(JSON.stringify({ type: 'screenshot:done', @@ -410,18 +422,18 @@ describe('GET /api/v1/sessions/:sessionId/screenshot', () => { const devices = [{ id: 'dev-1', name: 'iPhone', platform: 'ios', status: 'booted' }] const agent1 = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent1) - agent1.send(JSON.stringify({ type: 'agent:register', agentId: 'uuid-1', platform: 'ios', devices })) - const reply = await waitForType(agent1, 'agent:registered') - const sessionId = reply.registeredSessions![0].sessionId + agent1.send(JSON.stringify({ type: 'agent:register', agentName: 'screenshot-1', agentId: 'uuid-1', platform: 'ios', devices })) + const reply = await waitForType(agent1, 'agent:registered') + const sessionId = reply.registeredSessions[0]!.sessionId // On the screenshot request, the same Mac reconnects on a fresh socket → evicts agent1. let agent2: WebSocket | undefined agent1.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'screenshot:request') { agent2 = new WebSocket(`ws://localhost:${port}`) agent2.on('open', () => - agent2!.send(JSON.stringify({ type: 'agent:register', agentId: 'uuid-1', platform: 'ios', devices })), + agent2!.send(JSON.stringify({ type: 'agent:register', agentName: 'screenshot-2', agentId: 'uuid-1', platform: 'ios', devices })), ) } }) diff --git a/packages/relay/src/__tests__/sessionRebind.test.ts b/packages/relay/src/__tests__/sessionRebind.test.ts index 0a5bf070..694cdae0 100644 --- a/packages/relay/src/__tests__/sessionRebind.test.ts +++ b/packages/relay/src/__tests__/sessionRebind.test.ts @@ -6,7 +6,7 @@ import { WebSocket } from 'ws' import { RelayServer } from '../RelayServer' import { initDb, closeDb } from '../db' import { barrier, waitForOpen, waitForType, waitForTypeOrNull } from '@tapflowio/test-utils' -import type { RelayMessage } from '../types' +import type { AgentRegistered, AgentsListed, GenericError, SessionJoined, SessionRebound, SessionTerminated } from '@tapflowio/protocol' // #426 stage 2. Restarting an agent used to end every session it held: the browser was told // `session:terminated` and sent back to the Mac list, losing its navigation for something that @@ -53,8 +53,8 @@ describe('a session survives its agent restarting (#426)', () => { agentId: 'mac-1', agentName: 'the-mac', platform: 'ios', devices, capabilities, })) - const reply = await waitForType(agent, 'agent:registered') - const byDevice = new Map(reply.registeredSessions!.map((r) => [r.deviceId, r.sessionId])) + const reply = await waitForType(agent, 'agent:registered') + const byDevice = new Map(reply.registeredSessions.map((r) => [r.deviceId, r.sessionId])) return { agent, byDevice, registered: reply.registeredSessions! } } @@ -64,14 +64,14 @@ describe('a session survives its agent restarting (#426)', () => { const browser = new WebSocket(`ws://localhost:${port}`) await waitForOpen(browser) browser.send(JSON.stringify({ type: 'session:start', sessionId })) - const joined = await waitForType(browser, 'session:joined') + const joined = await waitForType(browser, 'session:joined') return Object.assign(browser, { joined }) } /** The device list as the dashboard sees it, flattened across agents. */ async function devices(ws: WebSocket) { ws.send(JSON.stringify({ type: 'agents:list' })) - const listed = await waitForType(ws, 'agents:listed') + const listed = await waitForType(ws, 'agents:listed') return (listed.sessions ?? []).flatMap((s) => s.devices) } @@ -82,7 +82,7 @@ describe('a session survives its agent restarting (#426)', () => { const second = await register([DEV_A]) - const rebound = await waitForType(browser, 'session:rebound') + const rebound = await waitForType(browser, 'session:rebound') expect(rebound.sessionId).toBe(sessionId) // The same id came back to the agent too, or its own bookkeeping would point at a session the // relay has since replaced. @@ -174,7 +174,7 @@ describe('a session survives its agent restarting (#426)', () => { const second = await register([DEV_B]) - const ended = await waitForType(browser, 'session:terminated') + const ended = await waitForType(browser, 'session:terminated') expect(ended.reason).toBe('agent-disconnected') expect((await devices(second.agent)).map((d) => d.id)).toEqual(['devB']) @@ -189,7 +189,7 @@ describe('a session survives its agent restarting (#426)', () => { const second = await register([DEV_A]) - expect((await waitForType(browserA, 'session:rebound')).sessionId).toBe(keptA) + expect((await waitForType(browserA, 'session:rebound')).sessionId).toBe(keptA) await waitForType(browserB, 'session:terminated') expect((await devices(second.agent)).map((d) => d.id)).toEqual(['devA']) @@ -209,7 +209,7 @@ describe('a session survives its agent restarting (#426)', () => { ['clipboard', 'audio'], ) - const rebound = await waitForType(browser, 'session:rebound') + const rebound = await waitForType(browser, 'session:rebound') expect(rebound.capabilities).toEqual(['clipboard', 'audio']) // ...but that one only proves the register frame was echoed: the relay copies `msg.capabilities` // into it directly, so it holds even if the session was never updated. `session:joined` is what @@ -312,7 +312,7 @@ describe('a session survives its agent restarting (#426)', () => { const refused = new WebSocket(`ws://localhost:${port}`) await waitForOpen(refused) refused.send(JSON.stringify({ type: 'session:start', sessionId })) - expect((await waitForType(refused, 'error')).message).toBe('Agent resources exhausted') + expect((await waitForType(refused, 'error')).message).toBe('Agent resources exhausted') refused.close() const second = await register([DEV_A]) diff --git a/packages/relay/src/__tests__/socketHelpers.test.ts b/packages/relay/src/__tests__/socketHelpers.test.ts index 38bff82b..13ea4c11 100644 --- a/packages/relay/src/__tests__/socketHelpers.test.ts +++ b/packages/relay/src/__tests__/socketHelpers.test.ts @@ -133,7 +133,7 @@ describe('socket test helpers are order-proof (#452)', () => { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) agent.send(JSON.stringify({ - type: 'agent:register', + type: 'agent:register', platform: 'ios', agentName: 'socketHelpers-1', devices: [{ id: 'devA', name: 'iPhone A', platform: 'ios', status: 'shutdown' }], })) await waitForType(agent, 'agent:registered') diff --git a/packages/relay/src/__tests__/uiTree.test.ts b/packages/relay/src/__tests__/uiTree.test.ts index 1c9078d9..1f82bea4 100644 --- a/packages/relay/src/__tests__/uiTree.test.ts +++ b/packages/relay/src/__tests__/uiTree.test.ts @@ -6,9 +6,17 @@ import path from 'path' import { WebSocket } from 'ws' import { RelayServer } from '../RelayServer' import { initDb, closeDb } from '../db' -import type { RelayMessage, UIElement } from '../types' + +import type { UIElement } from '../types' import { signJwt } from '../middleware/auth' import { waitForOpen, waitForType } from '@tapflowio/test-utils' +import type { AgentRegistered, BrowserToRelay, RelayToAgent } from '@tapflowio/protocol' + +/** What an agent socket actually receives. `RelayToAgent` is only the half the relay + * originates or rebuilds; browser commands it forwards verbatim (`app:clear-state`, + * `clipboard:*`, `input:*`) arrive unchanged and are declared in `BrowserToRelay`. The + * protocol has no single union for this — #557. */ +type AgentSocketInbound = RelayToAgent | BrowserToRelay const ELEMENTS: UIElement[] = [ { @@ -75,11 +83,11 @@ describe('GET /api/v1/sessions/:sessionId/ui-tree', () => { async function setupAgent(devices = [{ id: 'dev-1', name: 'iPhone', platform: 'ios', status: 'booted' }]) { const agent = new WebSocket(`ws://localhost:${port}`) await waitForOpen(agent) - agent.send(JSON.stringify({ type: 'agent:register', devices })) + agent.send(JSON.stringify({ type: 'agent:register', platform: 'ios', agentName: 'uiTree-1', devices })) // Through the shared recorder, not a raw `once`: the recorder queues the frame either way, and // consuming it here keeps a later wait on this socket from finding it. - const reply = await waitForType(agent, 'agent:registered') - const sessionId = reply.registeredSessions![0]!.sessionId + const reply = await waitForType(agent, 'agent:registered') + const sessionId = reply.registeredSessions[0]!.sessionId return { agent, sessionId } } @@ -87,7 +95,7 @@ describe('GET /api/v1/sessions/:sessionId/ui-tree', () => { const { agent, sessionId } = await setupAgent() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'ui:tree:request') { agent.send(JSON.stringify({ type: 'ui:tree:response', @@ -148,7 +156,7 @@ describe('GET /api/v1/sessions/:sessionId/ui-tree', () => { const { agent, sessionId } = await setupAgent() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'ui:tree:request') { agent.send(JSON.stringify({ type: 'ui:tree:error', @@ -189,7 +197,7 @@ describe('GET /api/v1/sessions/:sessionId/ui-tree', () => { const { agent, sessionId } = await setupAgent() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'ui:tree:request') agent.close() }) @@ -207,7 +215,7 @@ describe('GET /api/v1/sessions/:sessionId/ui-tree', () => { const { agent, sessionId } = await setupAgent() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'ui:tree:request') { agent.send(JSON.stringify({ type: 'ui:tree:response', @@ -254,7 +262,7 @@ describe('GET /api/v1/sessions/:sessionId/ui-tree', () => { const { agent, sessionId } = await setupAgent() agent.on('message', (data) => { - const msg = JSON.parse(data.toString()) as RelayMessage + const msg = JSON.parse(data.toString()) as AgentSocketInbound if (msg.type === 'ui:tree:request') { agent.send(JSON.stringify({ type: 'ui:tree:response', diff --git a/packages/relay/src/index.ts b/packages/relay/src/index.ts index 08f8d14e..885fde21 100644 --- a/packages/relay/src/index.ts +++ b/packages/relay/src/index.ts @@ -11,4 +11,3 @@ export { startTlsBackgroundTasks } from './lib/tlsTasks.js' export type { DnsProviderEntry } from './lib/cert/index.js' export type { TlsConfig } from './lib/cert/index.js' export type { Session } from './SessionManager.js' -export type { RelayMessage, MessageType } from './types.js' diff --git a/packages/relay/src/types.ts b/packages/relay/src/types.ts index ef318f2b..75cd5e9c 100644 --- a/packages/relay/src/types.ts +++ b/packages/relay/src/types.ts @@ -1,137 +1,24 @@ -export type MessageType = - | 'agent:register' - | 'agent:registered' - | 'agent:resources' - | 'agents:list' - | 'agents:listed' - | 'session:start' - | 'session:joined' - | 'session:chrome' - | 'session:deviceInfo' - | 'session:end' - | 'session:leave' - | 'session:terminated' - | 'session:agent-away' - | 'session:rebound' - | 'stream:register' - | 'stream:registered' - // Was missing. The relay sends it from two places and its own union did not declare it — the - // very drift `protocol/AGENTS.md` cites as this package's reason to exist, still alive in the - // copy underneath it. The assertion below is what makes a third omission a compile error. - | 'stream:request-idr' - | 'device:boot' - | 'device:booting' - | 'device:ready' - | 'device:boot-error' - | 'device:shutdown' - | 'device:shutdown-done' - | 'app:install' - | 'app:install-done' - | 'app:install-error' - | 'app:launch' - | 'app:launch-done' - | 'app:launch-error' - | 'open-url' - | 'open-url:done' - | 'open-url:error' - | 'app:clear-state' - | 'app:clear-state-done' - | 'app:clear-state-error' - | 'input:touch:start' - | 'input:touch:move' - | 'input:touch:end' - | 'input:pinch:start' - | 'input:pinch:move' - | 'input:pinch:end' - | 'input:key' - | 'input:type' - | 'input:type-done' - | 'input:type-error' - | 'input:done' - | 'input:error' - | 'input:button' - | 'input:rotate' - | 'input:keyboard:toggle' - | 'keyboard:toggled' - | 'screenshot:request' - | 'screenshot:done' - | 'screenshot:error' - | 'ui:tree:request' - | 'ui:tree:response' - | 'ui:tree:error' - | 'clipboard:read' - | 'clipboard:write' - | 'clipboard:data' - | 'clipboard:write-done' - | 'clipboard:error' - | 'error' +// The wire contract lives in @tapflowio/protocol. This file holds what the relay adds on top of it — +// today only the re-exports and aliases below. +// +// **`RelayMessage` and `MessageType` were removed here (#550).** They were a flat interface where +// `type` was the only required member and a hand-copied union of 62 literals beside it, and they were +// the relay's *inbound* type: `route` took a `RelayMessage`, so every field it read was optional by +// construction and every one it needed came with a `!`. That is why the two type systems could +// disagree about the same wire field — `format?` here against a required `format` in the protocol — +// with nothing to report it. +// +// What replaced them is not another declaration but a parse: `@tapflowio/protocol/validate` turns an +// inbound frame into a discriminated union at the door, and the relay's own membership assertions +// went with the literal list they were holding, because there is no longer a second copy to hold. import type { AgentResources, UIElement } from '@tapflowio/agent-core' export type { AgentResources, UIElement } // The wire contract lives in @tapflowio/protocol so the relay, the dashboard and mcp-server cannot // drift apart. `DeviceInfo` is kept as an alias while call sites move over. -import type { AnyWireMessage, DeviceSummary, SessionInfo, SessionTerminatedReason } from '@tapflowio/protocol' +import type { DeviceSummary } from '@tapflowio/protocol' export type { DeviceDetails, DeviceReport, DeviceSummary, SessionInfo, SessionTerminatedReason } from '@tapflowio/protocol' export type DeviceInfo = DeviceSummary -// ── membership, enforced in both directions (#532) ─────────────────────────────────────────────── -// -// The wire-contract program made every message's *fields* checked and left its *set membership* -// unchecked in one direction. Narrowing was held by the compiler and held well — `sendTo` refuses a -// message outside its union. **Widening was free**: measured on `main`, adding `DeviceBooting` to -// `BrowserToRelay` left `pnpm typecheck` at zero errors and all 294 static tests green. -// -// These are type-level and cost nothing at runtime. `Assert` fails to instantiate when its argument -// is not `true`, so a violated invariant is a compile error at the declaration rather than a test -// somebody has to run. -export type Assert = T -/** `[T] extends [never]` rather than `T extends never`: a bare conditional distributes over a union - * and answers `never` for an empty one *and* for a non-empty one, which would pass either way. */ -export type IsEmpty = [T] extends [never] ? true : false - -/** - * The relay's own literal list is complete against the protocol. - * - * Both directions. Missing an entry is what happened to `stream:request-idr`; an extra one means a - * literal the protocol no longer declares, which reads as deliberate and gates nothing. - */ -type _MessageTypeCoversProtocol = Assert>> -type _MessageTypeInventsNothing = Assert>> - - -// `agents:listed` groups devices by agent machine. Protocol owns the shape; this file used to -// declare an identical copy. Its `devices` element is protocol's `DeviceSummary`, which the alias -// below already points at. - -export interface RelayMessage { - type: MessageType - sessionId?: string - payload?: unknown - message?: string - agentName?: string - // agent:register: stable per-machine id (macOS IOPlatformUUID). Unique per Mac, unlike agentName - // (os.hostname() can collide). Absent from older agents → relay falls back to agentName for dedup. - agentId?: string - // agent:register: raw device list (without sessionId/busy — added by relay) - devices?: Array<{ id: string; name: string; platform: string; status: string; osVersion?: string }> - platform?: string // agent:register: agent platform ('ios' | 'android') - // agent:register: what this agent implements (e.g. ['clipboard']). Absent from agents that - // predate a capability, which is exactly how a viewer tells them apart — see agent-core. - // Echoed back on session:joined so the dashboard knows before it sends anything. - capabilities?: string[] - // agents:listed: grouped by agent - sessions?: SessionInfo[] - // agent:registered: per-device sessionId assignments - registeredSessions?: Array<{ deviceId: string; sessionId: string }> - buildId?: number - resources?: AgentResources - requestId?: string - data?: string - format?: 'png' | 'jpeg' - // ui:tree:response: unified element schema (normalized 0-1 frames), mapped agent-side - elements?: UIElement[] - // session:terminated: why the relay dropped the session - reason?: SessionTerminatedReason -} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b887c8d8..c1b5b3fa 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -478,10 +478,17 @@ importers: version: 4.1.10(@types/node@22.20.1)(@vitest/coverage-v8@4.1.10)(jsdom@29.1.1)(vite@6.4.3(@types/node@22.20.1)(jiti@1.21.7)(tsx@4.23.11)(yaml@2.9.0)) packages/protocol: + dependencies: + zod: + specifier: ^4.4.3 + version: 4.4.3 devDependencies: typescript: specifier: ^5.0.0 version: 5.9.3 + vitest: + specifier: ^4.1.10 + version: 4.1.10(@types/node@22.20.1)(@vitest/coverage-v8@4.1.10)(jsdom@29.1.1)(vite@6.4.3(@types/node@22.20.1)(jiti@1.21.7)(tsx@4.23.11)(yaml@2.9.0)) packages/relay: dependencies: diff --git a/scripts/__tests__/browserInboundRouting.test.mjs b/scripts/__tests__/browserInboundRouting.test.mjs index 7aafdcac..cba31471 100644 --- a/scripts/__tests__/browserInboundRouting.test.mjs +++ b/scripts/__tests__/browserInboundRouting.test.mjs @@ -37,7 +37,7 @@ function forwardedToBrowser(src) { body += line + '\n' depth += (line.match(/\{/g) ?? []).length - (line.match(/\}/g) ?? []).length if (depth <= 0) { - if (/browserSocket\.send\(JSON\.stringify\(msg\)\)/.test(body)) { + if (FORWARD_TO_BROWSER.test(body)) { for (const l of blockLabels) found.add(l) } blockLabels = null @@ -62,6 +62,20 @@ function forwardedToBrowser(src) { return found } +/** + * A browser-bound forward, and it must serialise **`raw`** — the frame as it arrived. + * + * It was `JSON.stringify(msg)` until #444 made the inbound frame a parse product. `z.object` strips + * keys it does not declare, so forwarding the product here would silently delete a field a newer agent + * added, in the one direction where the sender is the more recently updated side. The browser→agent + * forwards are the mirror and deliberately send `msg`: there the stripping is the point, because the + * sender may be an attacker with devtools open. + * + * Anchored on `raw` rather than accepting either, so a forward that goes back to the product fails + * here instead of shipping a compatibility break nothing else would report. + */ +const FORWARD_TO_BROWSER = /browserSocket\.send\(JSON\.stringify\(raw\)\)/ + /** `export interface Name { … }` bodies, by name. L1 moved every message out of its union and into one * of these, so a parser that reads only union bodies now finds nothing — it did, and this file's * `stream:registered` assertion is what said so. */ @@ -157,10 +171,18 @@ describe('browser-inbound routing matches the protocol union', () => { // nested literal to 6 of 11 fields and the by-name assertion passed anyway. it('the parser reached every forward site', () => { expect(forwarded.size).toBe(22) - const sends = (relaySrc.match(/browserSocket\.send\(JSON\.stringify\(msg\)\)/g) ?? []).length + const sends = (relaySrc.match(/browserSocket\.send\(JSON\.stringify\(raw\)\)/g) ?? []).length expect(sends).toBe(8) // 6 single-label blocks + the 13-label block + the clipboard block }) + // The other half of the rule above, and the one a count cannot see: a forward that switched back to + // the parse product would keep the count at 8 while stripping every field the schemas do not declare + // — which for the Envelope tier is *every* payload. The symptom would be a viewer that renders + // nothing, from a change that looks like a rename. + it('no browser-bound forward serialises the parse product', () => { + expect(relaySrc).not.toMatch(/browserSocket\.send\(JSON\.stringify\(msg\)\)/) + }) + it('RelayOrAgentToBrowser is shared by both directions rather than copied', () => { const shared = unionMembers(protocolSrc, 'RelayOrAgentToBrowser') expect(shared.size).toBe(11) diff --git a/scripts/__tests__/correlatedRequestsGated.test.mjs b/scripts/__tests__/correlatedRequestsGated.test.mjs index dc71140a..7ddc7e81 100644 --- a/scripts/__tests__/correlatedRequestsGated.test.mjs +++ b/scripts/__tests__/correlatedRequestsGated.test.mjs @@ -14,11 +14,17 @@ import ts from 'typescript' // check can see it (see the note above `OpenUrlReplyBody`). // // The member set is **derived** from the protocol, so an eighth correlated request added later fails here -// rather than being noticed by whoever reads the comment. Two gate forms count, and both are real: +// rather than being noticed by whoever reads the comment. // -// - `isCorrelated(msg)` inside the `case` — the inline form. -// - dispatch to a handler whose parameter is narrowed to `{ requestId: string }` — then the **compiler** -// enforces the gate, since an ungated `msg` does not satisfy the signature. +// **Where the gate lives moved, and this file moved with it.** It used to be `isCorrelated(msg)` written +// into each `case`, or a dispatch to a handler whose parameter was narrowed to `{ requestId: string }`. +// Since #444 the door is a parse: `@tapflowio/protocol/validate` refuses a frame whose schema declares +// `requestId` before `route` ever runs. So the property to check is that each correlated request's +// **schema** demands the correlator — the same claim, one layer earlier, and now covering the empty +// string as well as the absent field without either being spelled out at a call site. +// +// Checking the schema rather than the case body is also why this survives the next refactor of the +// switch: the gate is no longer something a `case` can forget to write. const root = join(import.meta.dirname, '../..') const read = (p) => readFileSync(join(root, p), 'utf8') @@ -57,53 +63,32 @@ function correlatedRequestTypes(proto) { return out } -/** Handlers whose `msg` parameter is narrowed to carry the correlator — the compiler-enforced gate form. */ -function narrowedHandlers(sf) { - const names = new Set() +/** + * For each literal in the inbound schema map, the text of its schema expression. + * + * Parsed rather than grepped: a `z.object({ … })` spans lines and nests, so a line-based match would + * stop at the first `}` and read a request's gate off its payload's shape. + */ +function schemaBodies(sf) { + const out = new Map() const visit = (node) => { - if (ts.isMethodDeclaration(node) && node.name) { - const p = node.parameters.find((x) => x.name.getText(sf) === 'msg') - if (p?.type && /requestId:\s*string/.test(p.type.getText(sf))) names.add(node.name.getText(sf)) + if (ts.isPropertyAssignment(node) && ts.isStringLiteral(node.name)) { + out.set(node.name.text, node.initializer.getText(sf)) } ts.forEachChild(node, visit) } visit(sf) - return names -} - -/** For each `case '':` in the relay's message switch, the text of its clause. */ -function caseBodies(sf) { - const bodies = new Map() - const visit = (node) => { - if (ts.isCaseClause(node) && ts.isStringLiteral(node.expression)) { - // Fall-through clauses (`case 'a': case 'b': { … }`) have an empty statement list; the following - // clause carries the body, so accumulate until one is non-empty. - bodies.set(node.expression.text, node.statements.map((s) => s.getText(sf)).join('\n')) - } - ts.forEachChild(node, visit) - } - visit(sf) - - // Resolve fall-through: an empty clause shares the next non-empty one. - const entries = [...bodies.entries()] - for (let i = 0; i < entries.length; i++) { - if (entries[i][1] !== '') continue - for (let j = i + 1; j < entries.length; j++) { - if (entries[j][1] !== '') { bodies.set(entries[i][0], entries[j][1]); break } - } - } - return bodies + return out } describe('every correlated browser request is gated at the relay door', () => { const proto = read('packages/protocol/src/index.ts') - const relayPath = 'packages/relay/src/RelayServer.ts' - const relaySrc = read(relayPath) - const sf = sourceOf(relayPath, relaySrc) + const validatePath = 'packages/protocol/src/validate/index.ts' + const validateSrc = read(validatePath) + const sf = sourceOf(validatePath, validateSrc) const types = correlatedRequestTypes(proto) - const handlers = narrowedHandlers(sf) - const bodies = caseBodies(sf) + const bodies = schemaBodies(sf) it('finds the correlated request set, derived rather than listed', () => { // If this drops to zero the derivation broke and every assertion below would vacuously pass — the @@ -116,25 +101,28 @@ describe('every correlated browser request is gated at the relay door', () => { for (const type of types) { it(`${type} is gated`, () => { const body = bodies.get(type) - expect(body, `${type} has no case in the relay's switch`).toBeDefined() + expect(body, `${type} has no schema in the inbound map — the door would refuse it as unknown-type`) + .toBeDefined() - const inline = body.includes('isCorrelated(msg)') - const viaHandler = [...handlers].some((h) => body.includes(`this.${h}(`)) expect( - inline || viaHandler, - `${type} reaches the relay with no correlator gate: no isCorrelated(msg), and no dispatch to a ` + - `handler whose msg parameter requires requestId (candidates: ${[...handlers].join(', ') || 'none'})`, + /(^|[^.\w])requestId\b/.test(body), + `${type} declares a required requestId but its schema does not demand one, so the door forwards ` + + `an uncorrelatable request and the reply it produces cannot be attributed`, ).toBe(true) + expect( + body.includes('requestId: requestId.optional()'), + `${type} declares a required requestId and its schema makes it optional — the two disagree, and ` + + `the schema is the one the wire obeys`, + ).toBe(false) }) } it('the gate tests both halves — absent and empty', () => { - // A gate that only checked `!== undefined` would let `''` through, and one that only checked `typeof` - // would let `''` through too. Both halves have a relay test behind them; this pins the predicate the - // derivation above trusts. - const decl = relaySrc.match(/function isCorrelated\([^)]*\)[^{]*\{([\s\S]*?)\n\}/) - expect(decl, 'isCorrelated is gone').not.toBeNull() - expect(decl[1]).toMatch(/typeof msg\.requestId === 'string'/) - expect(decl[1]).toMatch(/msg\.requestId !== ''/) + // The predicate this replaced rejected `''` as well as absence, and a bare `z.string()` accepts it. + // Nothing type-level can hold that: `.min(1)` does not change what `z.output` infers, which is + // exactly why it costs the tier assertions nothing — so it is checked here and exercised in + // `protocol`'s own `rejects an empty requestId`. + expect(validateSrc).toMatch(/^const requestId = z\.string\(\)\.min\(1\)$/m) + expect(validateSrc).toMatch(/^const sessionId = z\.string\(\)\.min\(1\)$/m) }) }) From ec85451facecee3894be2a314178feb7d8656fd8 Mon Sep 17 00:00:00 2001 From: Duchan Date: Sat, 15 Aug 2026 18:01:14 +0900 Subject: [PATCH 2/5] =?UTF-8?q?fix:=20apply=20the=20pre-PR=20review=20?= =?UTF-8?q?=E2=80=94=20tolerate=20an=20older=20agent's=20register,=20and?= =?UTF-8?q?=20log=20every=20rejection?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reviews, two lenses. Eight findings, six fixed here and two corrections to prose that argued for a design this change removed. **`agent:register` rejected an agent that omits `platform` or `agentName`.** The schema's own comment claimed every `.default()` mirrored a `??` in the relay, and it was false: `agentName ?? agentId ?? 'unknown'` and `platform ?? 'unknown'` are in the connect log, and neither field was defaulted. That message is the most expensive rejection in the protocol — no `agent:registered` goes back, the agent's handshake promise never resolves, and the whole Mac disappears from the dashboard. Both are `.default('')` now, deliberately falsy because the relay's eviction runs `if (identity)` where identity is `agentId ?? agentName`; the log line takes `||`. **Two of three rejection reasons never reached the log.** `settleRole` returns false for them, and the log sat behind it — so a malformed handshake on a role-less socket was dropped in silence, which is the exact skew the log exists to make visible. Logging now happens first, once per failure. **`correlatedRequestsGated` accepted an inline `requestId: z.string()`.** Its header claims to cover the empty string; a text regex could not see a single call site dropping `.min(1)`, and no other gate can either — that is why `.min(1)` costs the tier assertions nothing. It walks the AST for the top-level property now, and the mutation the reviewer described is caught. Prose corrected: eleven `msg.sessionId!` in protocol/AGENTS.md (there are none), 62 literals (63), "the relay validates nothing on the way in", four sites still naming `isAddressed`/`isCorrelated`/`AGENT_MSG_TYPES` as live, and two claiming agent replies are forwarded as `JSON.stringify(msg)`. The changeset promised an error reply for every refused request; only `app:install`/`app:launch` keep one, and #563 is the decision for the rest. The zod dependency and the `./validate` subpath were missing from both release notes. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/relay-inbound-validated.md | 13 +++- CHANGELOG.md | 6 +- packages/ios-agent/src/IOSAgent.ts | 3 +- packages/mcp-server/AGENTS.md | 2 +- packages/protocol/AGENTS.md | 35 ++++++---- .../protocol/src/__tests__/validate.test.ts | 18 +++++ packages/protocol/src/index.ts | 9 +-- packages/protocol/src/validate/index.ts | 19 ++++-- packages/relay/src/RelayServer.ts | 33 ++++++---- packages/relay/src/types.ts | 2 +- .../correlatedRequestsGated.test.mjs | 66 ++++++++++++++----- 11 files changed, 146 insertions(+), 60 deletions(-) diff --git a/.changeset/relay-inbound-validated.md b/.changeset/relay-inbound-validated.md index c562918d..071f50b7 100644 --- a/.changeset/relay-inbound-validated.md +++ b/.changeset/relay-inbound-validated.md @@ -20,15 +20,22 @@ 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. Where the - request has an error reply the caller still gets one; where it has none it is dropped and logged with - the field that failed, instead of silently doing nothing. + — 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 diff --git a/CHANGELOG.md b/CHANGELOG.md index cfb809d9..390034ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 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 62 literals beside it — and they disagreed with `@tapflowio/protocol` about + 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 @@ -56,6 +56,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **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 diff --git a/packages/ios-agent/src/IOSAgent.ts b/packages/ios-agent/src/IOSAgent.ts index 9463d172..85dbdf6d 100644 --- a/packages/ios-agent/src/IOSAgent.ts +++ b/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 diff --git a/packages/mcp-server/AGENTS.md b/packages/mcp-server/AGENTS.md index 7f1d9115..7d1f6dd9 100644 --- a/packages/mcp-server/AGENTS.md +++ b/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` + 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` + 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). diff --git a/packages/protocol/AGENTS.md b/packages/protocol/AGENTS.md index cb02f999..239b20e1 100644 --- a/packages/protocol/AGENTS.md +++ b/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. @@ -308,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 @@ -352,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 @@ -385,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 @@ -402,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 @@ -422,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 diff --git a/packages/protocol/src/__tests__/validate.test.ts b/packages/protocol/src/__tests__/validate.test.ts index f7d01522..72c7b0ed 100644 --- a/packages/protocol/src/__tests__/validate.test.ts +++ b/packages/protocol/src/__tests__/validate.test.ts @@ -110,6 +110,24 @@ describe('an agent older than a field still registers', () => { expect(ok(raw).msg).toMatchObject({ capabilities: ['clipboard'], devices: [] }) }) + // **The most expensive rejection in the protocol, so the most tolerant schema.** A first draft + // required `platform` and `agentName` — both declared required, both sent by both agents here — and + // the relay reads each through a `??`. An agent omitting either would have had its frame refused, so + // no `agent:registered` goes back, so its handshake promise never resolves: the whole Mac and every + // device on it absent from the dashboard, with one relay-side warn as the only trace. + it('registers an agent that sends neither a platform nor a name', () => { + const r = ok({ type: 'agent:register', devices: [] }) + expect(r.msg).toMatchObject({ platform: '', agentName: '', capabilities: [], devices: [] }) + }) + + // `''` and not `undefined` is what lets `z.output` match the interface, and it has to stay falsy: + // the relay's eviction runs only `if (identity)`, where identity is `agentId ?? agentName`. A + // placeholder like `'unknown'` there would make every nameless agent evict every other one. + it('leaves a defaulted name falsy, because identity keys on it', () => { + const r = ok({ type: 'agent:register', devices: [] }) + expect((r.msg as { agentName: string }).agentName).toBeFalsy() + }) + // The tolerance is for *absence*, not for a wrong shape — otherwise `.default()` would be // indistinguishable from not checking the field at all. it('still refuses capabilities that are not strings', () => { diff --git a/packages/protocol/src/index.ts b/packages/protocol/src/index.ts index a780d196..80f639d6 100644 --- a/packages/protocol/src/index.ts +++ b/packages/protocol/src/index.ts @@ -344,7 +344,7 @@ export type InputErrorReason = // `stream:registered` goes to a stream socket rather than a viewer. It is grouped here because the // relay treats "everything that is not an agent" alike on the way out; splitting the outbound union // by socket role is a later refinement, and the roles are already distinguished at runtime -// (`wsRoles`, `AGENT_MSG_TYPES`). +// (`wsRoles`, and `directionOf` from this package's `validate` entry). /** Browser-inbound messages with **two** producers: an agent sends it and the relay also originates * its own copy — replaying session state to a re-joining viewer, or failing fast when it cannot @@ -616,7 +616,7 @@ export type SessionStartFailure = * the reason has a single producer inside `handleSessionStart`. Both were in HEAD at once for two months. * * L5c settled it by removing the general role rather than the specific one: a request that names no session - * is now **dropped at the relay's door** (`isAddressed`), because answering it would ship a frame whose own + * is now **dropped at the relay's door** — by the inbound schema since #444 — 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, every producer of this message answers one specific join. @@ -747,12 +747,13 @@ export interface ClipboardWriteDone { requestId: string } -/** Messages an agent produces. The relay forwards them byte-for-byte (`JSON.stringify(msg)`) rather +/** Messages an agent produces. The relay forwards them byte-for-byte (`JSON.stringify(raw)`, the frame + * exactly as it arrived, so a field a newer agent adds is not stripped by a relay that does not know it) rather * than re-creating them, so it never constructs one — which is exactly why they were missing from * this file until L3: nothing on the relay's own send path referenced them. * * The twelve declared below carry `sessionId` as required, on two independent grounds: both agents - * include it in every send literal, and the relay's forward gate resolves `sessions.get(msg.sessionId!)` + * include it in every send literal, and the relay's forward gate resolves `sessions.get(msg.sessionId)` * before forwarding, so a message with no sessionId never reaches a browser by this path. * * Nine of the ten inherited from `RelayOrAgentToBrowser` now carry it too. Three of them diff --git a/packages/protocol/src/validate/index.ts b/packages/protocol/src/validate/index.ts index 5f8e6c7e..a905de40 100644 --- a/packages/protocol/src/validate/index.ts +++ b/packages/protocol/src/validate/index.ts @@ -199,12 +199,23 @@ const BROWSER_INBOUND = { const AGENT_CONSUMED = { 'agent:register': z.object({ type: z.literal('agent:register'), - platform: z.string(), - // Required on the interface; absent from agents that predate the field, which is how a viewer - // tells them apart. `RelayServer` carried `msg.capabilities ?? []` for exactly this. + // **All four defaults come from a `??` in `RelayServer`, and the list is exhaustive by + // construction** — `agentId ?? agentName` for identity, `devices ?? []`, `capabilities ?? []`, and + // `agentName ?? agentId ?? 'unknown'` / `platform ?? 'unknown'` in the connect log. A first draft + // defaulted only `capabilities` and `devices` and required these two, which would have made an + // agent omitting either **never register at all**: the frame is refused, no `agent:registered` + // goes back, and the agent's handshake promise never resolves — the whole Mac and every device on + // it simply absent from the dashboard, with one relay-side warn as the only trace. That is the + // most expensive rejection in the protocol, so this message is the one to be most tolerant on. + // + // `''` rather than `undefined` because `z.output` must match the interface, and it is behaviourally + // the same everywhere it reaches: `identity` stays falsy so no eviction runs, and the log line uses + // `||` for exactly this. + platform: z.string().default(''), + // How a viewer tells an agent that predates a capability from one that has it. capabilities: z.array(z.string()).default([]), agentId: z.string().optional(), - agentName: z.string(), + agentName: z.string().default(''), // Deduplication by device id stays in the handler — it is a policy about the *set*, not a shape. devices: z.array(z.object({ id: z.string(), name: z.string(), platform: z.string(), status: z.string(), diff --git a/packages/relay/src/RelayServer.ts b/packages/relay/src/RelayServer.ts index de5488fd..790d6402 100644 --- a/packages/relay/src/RelayServer.ts +++ b/packages/relay/src/RelayServer.ts @@ -563,11 +563,14 @@ export class RelayServer { // browser socket had spoofed *badly* — the 1008 that closes such a socket never fired, so the // spoofer kept its connection. The direction is a fact about the `type` alone, and the type is // known on a shape failure too, so nothing about that check needs the payload to be valid. + // **Logged before the gate, not after it.** A first draft called `settleRole` first and logged in + // the `!ok` branch below it — which never ran for two of the three failure reasons, because + // `settleRole` returns `false` for them. The case that mattered was a malformed handshake on a + // role-less socket: dropped in silence, which is precisely the agent-registration skew this log + // exists to make visible. + if (!inbound.ok) logInboundRejection(inbound) if (!this.settleRole(ws, inbound)) return - if (!inbound.ok) { - logInboundRejection(inbound) - return - } + if (!inbound.ok) return try { this.route(ws, inbound.msg, inbound.raw) } catch (e) { @@ -638,7 +641,7 @@ export class RelayServer { if (!this.wsRoles.has(ws)) { // A handshake that did not parse confers nothing. Returning here rather than falling through to // `browser` is what keeps an agent whose register is malformed from being closed with - // `Forbidden` — its frame is dropped and logged, and the next one still gets to introduce it. + // `Forbidden` — the caller has already logged it, and the next frame still gets to introduce it. if (handshake && !inbound.ok) return false if (type === 'agent:register') this.wsRoles.set(ws, 'agent') else if (type === 'stream:register') this.wsRoles.set(ws, 'stream') @@ -685,9 +688,10 @@ export class RelayServer { // which is why `not-session-owner` needed real copy in the dashboard rather than the `null` a first // draft gave it. // - // `msg.sessionId &&` rather than `isAddressed`: a falsy check already rejects both an absent id and an - // empty one, and the miss below rejects a non-string, so the predicate would add only its log here — - // and a mutation confirmed there is nothing observable to hold it with. + // No address check here at all any more: the schema declares `sessionId` with `.min(1)`, so an absent + // id, an empty one and a non-string are all refused before this case is reached. It used to be a bare + // `msg.sessionId &&` rather than the `isAddressed` predicate, because a falsy check already covered + // both halves and the predicate would have added only its log. // // **Dropped rather than refused, and that is the contract**: neither has a reply, so there is no // waiter to tell. The same asymmetry as the input frames nothing acks. Inventing a @@ -831,11 +835,11 @@ export class RelayServer { // ── Browser → Agent ──────────────────────────────────────────────────── // // These two shared one fall-through clause. Separated because the sharing is the trap for - // whoever adds the door gate: an `isCorrelated(msg)` written into a shared body would gate + // whoever adds the door gate: a correlator check written into a shared body would gate // `device:shutdown` too, and the relay originates that message with no id — so the dashboard's // four senders and the relay's own idle timer would stop reaching the agent, silently, in the - // one direction no reply reports. `correlatedRequestsGated` resolves fall-through by sharing the - // next non-empty body, so it would have read the gate as covering both and passed. + // one direction no reply reports. That gate is a schema now and cannot be written into a case at + // all, which is what makes the trap unreachable rather than merely avoided. case 'device:boot': { // The door gate that stood here is now the schema: `device:boot` declares `sessionId` and // `requestId` required, and the parser rejects an absent **or empty** one before this case is @@ -1229,13 +1233,16 @@ export class RelayServer { // The startup banner prints "Waiting for agents..." once and then the relay says nothing either // way, so a terminal gives no signal about whether an agent is attached. One line per // transition, matching the disconnect line in evictAgentSocket. - logger.info(`agent connected: ${msg.agentName ?? msg.agentId ?? 'unknown'} (${msg.platform ?? 'unknown'}) — ${registeredSessions.length} device(s)`) + // `||`, not `??`: the schema defaults both of these to `''` for an agent that omits them, so `??` + // would print an empty name and an empty platform where this used to print `unknown`. + logger.info(`agent connected: ${msg.agentName || msg.agentId || 'unknown'} (${msg.platform || 'unknown'}) — ${registeredSessions.length} device(s)`) } /** The only producer of `error` — all four exits below, and nothing else in the repo sends that message. * * That is what makes the address possible rather than aspirational: `msg.sessionId` is narrowed to a - * non-empty `string` by the door (`isAddressed`), so every refusal can name the join it refuses. Before + * non-empty `string` by the door — the inbound schema, `isAddressed` before it — so every refusal can + * name the join it refuses. Before * L5d they carried none, and the clients' join waiters matched `sessionId === undefined || sessionId === * mine` — with no such key the left half was always true, so any refusal resolved any pending join. */ private handleSessionStart(ws: WebSocket, msg: Inbound<'session:start'>): void { diff --git a/packages/relay/src/types.ts b/packages/relay/src/types.ts index 75cd5e9c..4822614a 100644 --- a/packages/relay/src/types.ts +++ b/packages/relay/src/types.ts @@ -2,7 +2,7 @@ // today only the re-exports and aliases below. // // **`RelayMessage` and `MessageType` were removed here (#550).** They were a flat interface where -// `type` was the only required member and a hand-copied union of 62 literals beside it, and they were +// `type` was the only required member and a hand-copied union of 63 literals beside it, and they were // the relay's *inbound* type: `route` took a `RelayMessage`, so every field it read was optional by // construction and every one it needed came with a `!`. That is why the two type systems could // disagree about the same wire field — `format?` here against a required `format` in the protocol — diff --git a/scripts/__tests__/correlatedRequestsGated.test.mjs b/scripts/__tests__/correlatedRequestsGated.test.mjs index 7ddc7e81..5a89bdf8 100644 --- a/scripts/__tests__/correlatedRequestsGated.test.mjs +++ b/scripts/__tests__/correlatedRequestsGated.test.mjs @@ -64,17 +64,39 @@ function correlatedRequestTypes(proto) { } /** - * For each literal in the inbound schema map, the text of its schema expression. + * For each literal in the inbound schema map, the **top-level** `requestId` property of its schema. * - * Parsed rather than grepped: a `z.object({ … })` spans lines and nests, so a line-based match would - * stop at the first `}` and read a request's gate off its payload's shape. + * Parsed rather than grepped, and the difference is the whole check. A first draft matched + * `/(^|[^.\w])requestId\b/` against the schema's source text, which passes on a `requestId` nested + * inside a payload, on the word appearing in a comment, and — the one that matters — on an inline + * `requestId: z.string()` written in place of the shared `.min(1)` constant. That last one is + * invisible to every other gate in the repo: `SchemaExact` cannot see it, because `.min(1)` does not + * change what `z.output` infers, which is exactly why it costs the tier assertions nothing. The + * empty-string half would have gone back to being unguarded per message while the whole suite stayed + * green, reproducing the `clipboard:error` defect this file exists for. + * + * Returns `null` when there is no `requestId` property, and otherwise the initializer's text. */ -function schemaBodies(sf) { +function correlatorOf(sf) { const out = new Map() const visit = (node) => { - if (ts.isPropertyAssignment(node) && ts.isStringLiteral(node.name)) { - out.set(node.name.text, node.initializer.getText(sf)) + // `'app:install': z.object({ … })` — a string-literal key whose value is a call. + if (!ts.isPropertyAssignment(node) || !ts.isStringLiteral(node.name)) return ts.forEachChild(node, visit) + const shape = ts.isCallExpression(node.initializer) ? node.initializer.arguments[0] : undefined + if (!shape || !ts.isObjectLiteralExpression(shape)) { + out.set(node.name.text, { present: false, init: null }) + return ts.forEachChild(node, visit) } + const prop = shape.properties.find( + (p) => + (ts.isPropertyAssignment(p) || ts.isShorthandPropertyAssignment(p)) && + p.name && ts.isIdentifier(p.name) && p.name.text === 'requestId', + ) + out.set(node.name.text, { + present: prop !== undefined, + // A shorthand `requestId,` *is* the shared constant; a longhand carries its own expression. + init: prop === undefined ? null : ts.isShorthandPropertyAssignment(prop) ? 'requestId' : prop.initializer.getText(sf), + }) ts.forEachChild(node, visit) } visit(sf) @@ -88,7 +110,7 @@ describe('every correlated browser request is gated at the relay door', () => { const sf = sourceOf(validatePath, validateSrc) const types = correlatedRequestTypes(proto) - const bodies = schemaBodies(sf) + const bodies = correlatorOf(sf) it('finds the correlated request set, derived rather than listed', () => { // If this drops to zero the derivation broke and every assertion below would vacuously pass — the @@ -100,28 +122,36 @@ describe('every correlated browser request is gated at the relay door', () => { for (const type of types) { it(`${type} is gated`, () => { - const body = bodies.get(type) - expect(body, `${type} has no schema in the inbound map — the door would refuse it as unknown-type`) + const entry = bodies.get(type) + expect(entry, `${type} has no schema in the inbound map — the door would refuse it as unknown-type`) .toBeDefined() expect( - /(^|[^.\w])requestId\b/.test(body), - `${type} declares a required requestId but its schema does not demand one, so the door forwards ` + - `an uncorrelatable request and the reply it produces cannot be attributed`, + entry.present, + `${type} declares a required requestId but its schema has no top-level requestId, so the door ` + + `forwards an uncorrelatable request and the reply it produces cannot be attributed`, + ).toBe(true) + + // The shared constant, or something that carries its `.min(1)` — and never `.optional()`. + // Both halves matter: absence lets the request through uncorrelated, and `''` produces a reply + // whose required correlator is present-but-empty, which every correlating consumer discards. + expect( + entry.init === 'requestId' || /\.min\(1\)/.test(entry.init), + `${type}'s schema declares requestId as \`${entry.init}\`, which does not carry the non-empty ` + + `constraint. Use the shared \`requestId\` constant — an inline z.string() accepts '' and no ` + + `type-level assertion can see the difference.`, ).toBe(true) expect( - body.includes('requestId: requestId.optional()'), + /\.optional\(\)|\.nullish\(\)/.test(entry.init), `${type} declares a required requestId and its schema makes it optional — the two disagree, and ` + `the schema is the one the wire obeys`, ).toBe(false) }) } - it('the gate tests both halves — absent and empty', () => { - // The predicate this replaced rejected `''` as well as absence, and a bare `z.string()` accepts it. - // Nothing type-level can hold that: `.min(1)` does not change what `z.output` infers, which is - // exactly why it costs the tier assertions nothing — so it is checked here and exercised in - // `protocol`'s own `rejects an empty requestId`. + it('the shared constants carry the non-empty half', () => { + // The per-type assertions above accept the shared constant by name, so this is what gives that name + // its meaning. Weakening the constant fails here; weakening one call site fails above. expect(validateSrc).toMatch(/^const requestId = z\.string\(\)\.min\(1\)$/m) expect(validateSrc).toMatch(/^const sessionId = z\.string\(\)\.min\(1\)$/m) }) From e67800fe11924d1bb2209ab3b98aabc21a80f2e5 Mon Sep 17 00:00:00 2001 From: Duchan Date: Sat, 15 Aug 2026 18:23:57 +0900 Subject: [PATCH 3/5] feat(relay): answer a refused payload instead of dropping it (#563) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Filed as a follow-up and that was wrong twice. It is not a follow-up. Today a malformed `open-url` reaches the agent and the agent's own guard answers `open-url:error` — `IOSAgent.ts:1124` says so beside that guard and names this validation as "#444, which will take this over". Taking the responsibility without taking the answer converts an answered failure into silence, so it is a regression the door itself ships, not a pre-existing gap. Worst on the inputs: `awaitInputAck` reports silence from a session that has never acked as success (#457), so a dropped `input:key` reads to an MCP caller as an input that landed. Nor does it need a per-message design decision, which was the stated reason for splitting it. The envelope is judged separately from the payload, so a frame with a good `sessionId` and `requestId` carries everything a reply needs — one mechanism for all twelve. `reason: 'malformed'` is not a new member either; the input vocabulary already had it, and until now only agents produced it. `buildId`'s `.catch(NaN)` goes with it. It existed to keep one answer alive through a special case, and it was the wrong answer anyway: `Build not found` describes a lookup, and for a malformed id no lookup ran. Both handler guards it kept alive are now unreachable and deleted. Three lists have to agree and only one is derived — the correlated request set from the protocol, `ANSWERABLE`, and the replies in `refuseMalformed`. Nothing compared them, so `correlatedRequestsGated` now does, including that a non-input request must be answered by name rather than falling to the `input:error` default. 7 mutations, 7 caught. One of them is the bug this commit shipped and fixed in place: `settleRole` did not extract the type for `bad-payload`, so the twelve were classified correctly and dropped anyway, one line above the code that answers them. Closes #563. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/relay-inbound-validated.md | 18 +++-- CHANGELOG.md | 6 +- packages/protocol/AGENTS.md | 5 ++ .../protocol/src/__tests__/validate.test.ts | 81 +++++++++++++++---- packages/protocol/src/validate/index.ts | 76 ++++++++++++----- packages/relay/AGENTS.md | 10 +++ packages/relay/src/RelayServer.ts | 81 +++++++++++++++---- .../correlatedRequestsGated.test.mjs | 58 +++++++++++++ 8 files changed, 275 insertions(+), 60 deletions(-) diff --git a/.changeset/relay-inbound-validated.md b/.changeset/relay-inbound-validated.md index 071f50b7..ded7b8da 100644 --- a/.changeset/relay-inbound-validated.md +++ b/.changeset/relay-inbound-validated.md @@ -18,13 +18,17 @@ into an invisible `msg.payload`, with the compiler vouching for JSON that arrive 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 malformed command is refused before it reaches a device, and the caller is told which field was + wrong.** A `device:boot` with no payload, an `open-url` with no URL, an `app:install` whose `buildId` + is an object — these were forwarded to an agent before, and the agent's own guard answered if it had + one. The relay answers now, in the shape that request's waiter reads, so the diagnosis arrives sooner + and does not depend on which agent is on the other end. A request that has no reply at all is dropped + and logged with the field that failed. No client shipped here can produce any of these; a third-party + one can. +- **A command with no usable session id or request id is refused outright**, including the empty + string, which type-checks and which an LLM driving the MCP tools could produce. Answering one is not + possible — the reply's own required fields would be missing, and every client discards such a frame — + so it is dropped with a log rather than turned into a caller waiting out its deadline. - **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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 390034ce..d3e231f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,9 +50,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 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. + caller waiting out its deadline. A refused command is now answered where the request has a reply, so + the caller is told which field was wrong instead of waiting; where it has none, the frame is dropped + and the log names the field. 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. diff --git a/packages/protocol/AGENTS.md b/packages/protocol/AGENTS.md index 239b20e1..7cc029f9 100644 --- a/packages/protocol/AGENTS.md +++ b/packages/protocol/AGENTS.md @@ -44,6 +44,11 @@ The cost of a broad name is ambiguity about what belongs — answered by the two - **`SchemaExact` ties each schema to its interface**, and refuses `z.custom()` and a `const s: z.ZodType` annotation by kind, because both produce `T` with no `any` for `IsAny` to catch and would compare `T` with itself. + - **The envelope is judged before the payload**, so a payload failure on one of the twelve correlated + browser requests comes back as `bad-payload` carrying the address and the correlator — which is what + lets the relay answer it instead of dropping it. `ANSWERABLE` is that set; keeping it equal to the + correlated request set is `scripts/__tests__/correlatedRequestsGated.test.mjs`'s job, because + nothing else compares the two. ## Scope — what does not diff --git a/packages/protocol/src/__tests__/validate.test.ts b/packages/protocol/src/__tests__/validate.test.ts index 72c7b0ed..9094a294 100644 --- a/packages/protocol/src/__tests__/validate.test.ts +++ b/packages/protocol/src/__tests__/validate.test.ts @@ -59,12 +59,14 @@ describe('the door rejects what it cannot name', () => { }, ) + // `session:chrome` rather than a browser request: the twelve answerable ones report `bad-payload` + // instead, which carries its own type and is covered below. it('reports the type it refused on a shape failure, so a log can name it', () => { - const r = fail({ type: 'device:boot', sessionId: 's', requestId: 'r' }) + const r = fail({ type: 'session:chrome' }) expect(r.reason).toBe('bad-shape') if (r.reason === 'bad-shape') { - expect(r.type).toBe('device:boot') - expect(r.detail).toMatch(/payload/i) + expect(r.type).toBe('session:chrome') + expect(r.detail).toMatch(/sessionId/i) } }) }) @@ -195,20 +197,9 @@ describe('a browser frame is stripped, because its product is what gets forwarde expect(r.msg).toEqual({ type: 'input:touch:end', sessionId: 's', requestId: 'r' }) }) - // **`buildId` is carried through as `NaN` rather than refused, and that is deliberate.** It is the one - // browser-side shape failure the relay *answers*: the handler checks `Number.isInteger` and replies - // `Build not found`, so a caller learns why instead of waiting out its deadline. Refusing the frame - // here deleted that answer — the door has no socket and no correlator policy, so it cannot answer in - // the handler's place, and six relay tests asserting "answers … without going silent" went silent. - // - // What the schema still buys is that better-sqlite3 never sees the object or array that made it - // *throw* — an exception the message loop swallowed, which is the silence `Number.isInteger` was - // added to remove in the first place. - it('turns an unusable buildId into NaN rather than refusing the frame', () => { + it('refuses a buildId that is not an integer, answerably', () => { for (const buildId of [{}, [], '3', 1.5, null, undefined]) { - const r = ok({ type: 'app:install', sessionId: 's', requestId: 'r', buildId }) - expect(r.msg).toMatchObject({ type: 'app:install' }) - expect(Number.isInteger((r.msg as { buildId: number }).buildId)).toBe(false) + expect(fail({ type: 'app:install', sessionId: 's', requestId: 'r', buildId }).reason).toBe('bad-payload') } }) @@ -242,3 +233,61 @@ describe('directionOf replaces the hand-written agent list', () => { expect(directionOf(r.msg.type)).toBe('agent') }) }) + +describe('a payload failure the caller can be told about', () => { + // **The regression this exists to prevent, and it is one the door itself would have shipped.** Today + // a malformed `open-url` reaches the agent and the agent answers `open-url:error` from its own + // guard — `IOSAgent.ts` says so beside that guard and names this validation as what takes it over. + // Taking it over without taking the answer over turns an answered failure into silence. + // + // Worst on the inputs, and not obviously: `awaitInputAck` reports silence from a session that has + // never acked as **success** (#457), so a dropped `input:key` would be reported to an MCP caller as + // an input that landed. + it.each([ + ['open-url', { payload: {} }], + ['app:clear-state', { payload: { bundleId: 7 } }], + ['clipboard:write', { payload: {} }], + ['input:key', { payload: { modifiers: 1 } }], + ['input:button', { payload: {} }], + ['input:type', { payload: { text: null } }], + ['device:boot', {}], + ['app:install', { buildId: {} }], + ] as const)('reports %s as answerable, carrying the address and the correlator', (type, rest) => { + const r = fail({ type, sessionId: 'sess-1', requestId: 'rq-1', ...rest }) + expect(r.reason).toBe('bad-payload') + if (r.reason === 'bad-payload') { + expect(r.type).toBe(type) + expect(r.sessionId).toBe('sess-1') + expect(r.requestId).toBe('rq-1') + } + }) + + // The mirror, and why the envelope is judged first rather than the payload being retried leniently: + // with no usable correlator there is nothing to answer *with*. A reply carrying an empty `requestId` + // is discarded by every correlating consumer, which is the deadline-burning non-answer this whole + // door exists to stop shipping. + it.each([ + ['no requestId', { type: 'open-url', sessionId: 's', payload: {} }], + ['an empty requestId', { type: 'open-url', sessionId: 's', requestId: '', payload: {} }], + ['an empty sessionId', { type: 'open-url', sessionId: '', requestId: 'r', payload: {} }], + ])('refuses %s outright rather than answerably', (_name, raw) => { + expect(fail(raw).reason).toBe('bad-shape') + }) + + // A request with no error reply must not be reported as answerable — the relay would have nothing + // to send, and a `bad-payload` that cannot be answered is a lie about what the door achieved. + // + // Broken through `sessionId` rather than `payload`, and that is not incidental: `session:leave` + // declares no payload, so an undeclared key there is *stripped* and the frame parses. Which is the + // right answer — the tier only ever promises what it declares — and a first draft of this test + // asserted a rejection that never happens. + it.each(['session:leave', 'input:touch:move'])('leaves %s unanswerable', (type) => { + expect(fail({ type, sessionId: 7 }).reason).toBe('bad-shape') + }) + + // A well-formed frame is untouched by the second stage. + it('does not report a valid request as anything', () => { + const raw = { type: 'open-url', sessionId: 's', requestId: 'r', payload: { url: 'x://y' } } + expect(ok(raw).msg).toEqual(raw) + }) +}) diff --git a/packages/protocol/src/validate/index.ts b/packages/protocol/src/validate/index.ts index a905de40..769cf255 100644 --- a/packages/protocol/src/validate/index.ts +++ b/packages/protocol/src/validate/index.ts @@ -125,24 +125,13 @@ const BROWSER_INBOUND = { requestId: requestId.optional(), payload: z.object({ deviceId: z.string() }), }), - // **`.catch(NaN)` rather than a plain `z.number().int()`, and the reason is a measured regression.** - // - // A bad `buildId` is the one browser-side shape failure the relay *answers* today: the handler checks - // `Number.isInteger` and replies `Build not found`, so a caller learns why instead of waiting out its - // deadline. Rejecting the frame at the door deleted that answer — the parse fails, `route` never runs, - // and six tests that assert "answers … without going silent" went silent. The door has no socket and - // no correlator policy, so it cannot answer in the handler's place. - // - // So the schema carries the value through as `NaN` and the handler keeps answering. `z.output` is - // still `number`, which is what the tier assertion compares, and better-sqlite3 never sees the - // object or array that made it throw — that exception, swallowed by the message-loop catch, is the - // silence `Number.isInteger` was added to remove in the first place. - 'app:install': z.object({ - type: z.literal('app:install'), sessionId, requestId, buildId: z.number().int().catch(Number.NaN), - }), - 'app:launch': z.object({ - type: z.literal('app:launch'), sessionId, requestId, buildId: z.number().int().catch(Number.NaN), - }), + // Strict, because `ANSWERABLE` below answers a bad one rather than dropping it. A draft carried it + // through as `NaN` so the handler's `Number.isInteger` could keep replying `Build not found` — which + // worked, and left this message as the single special case in a class the door now handles uniformly. + // It was also the wrong diagnosis: nothing was looked up, so "not found" describes a query that never + // ran. + 'app:install': z.object({ type: z.literal('app:install'), sessionId, requestId, buildId: z.number().int() }), + 'app:launch': z.object({ type: z.literal('app:launch'), sessionId, requestId, buildId: z.number().int() }), 'app:clear-state': z.object({ type: z.literal('app:clear-state'), sessionId, requestId, payload: z.object({ bundleId: z.string() }), @@ -304,6 +293,39 @@ const STREAM_INBOUND = { const INBOUND = { ...BROWSER_INBOUND, ...AGENT_INBOUND, ...STREAM_INBOUND } as const +// ── the requests whose payload failure can be answered ─────────────────────── +// +// **The envelope is judged separately from the payload, and that is what makes an answer possible.** +// A frame whose `sessionId` and `requestId` are both good and whose payload is not carries everything +// a reply needs: an address and a correlator. Refusing it wholesale would be the regression this door +// otherwise ships — today a malformed `open-url` reaches the agent, which answers `open-url:error` +// from its own guard (`IOSAgent.ts` says so in writing: "validating third-party frames at the relay's +// door is #444, which will take this over"). Taking it over must not mean losing the answer. +// +// The cost of losing it is worst on the inputs, and not obviously: `awaitInputAck` reports silence +// from a session that has never acked as **success** (#457), so a dropped `input:key` would be +// reported to an MCP caller as an input that landed. +// +// Exactly the twelve browser requests that declare a required `requestId`. The relay maps each to the +// reply its own waiter reads; `scripts/__tests__/correlatedRequestsGated.test.mjs` derives that set +// from the protocol and holds all three lists to it. +const ANSWERABLE = { + 'device:boot': envC('device:boot'), + 'app:install': envC('app:install'), + 'app:launch': envC('app:launch'), + 'app:clear-state': envC('app:clear-state'), + 'open-url': envC('open-url'), + 'input:touch:end': envC('input:touch:end'), + 'input:pinch:end': envC('input:pinch:end'), + 'input:key': envC('input:key'), + 'input:button': envC('input:button'), + 'input:type': envC('input:type'), + 'clipboard:read': envC('clipboard:read'), + 'clipboard:write': envC('clipboard:write'), +} as const + +export type AnswerableType = keyof typeof ANSWERABLE + // ── what the door proved ───────────────────────────────────────────────────── /** @@ -325,6 +347,8 @@ export type ParseFailure = | { ok: false; reason: 'not-an-object' } | { ok: false; reason: 'unknown-type'; type: string } | { ok: false; reason: 'bad-shape'; type: InboundType; detail: string } + /** The payload is wrong but the envelope is not, so the caller can be told. */ + | { ok: false; reason: 'bad-payload'; type: AnswerableType; sessionId: string; requestId: string; detail: string } export type ParseResult = | { @@ -396,7 +420,18 @@ export function parseInbound(raw: unknown): ParseResult { // inert today. const result = INBOUND[known].safeParse(frame) if (!result.success) { - return { ok: false, reason: 'bad-shape', type: known, detail: z.prettifyError(result.error) } + const detail = z.prettifyError(result.error) + // Second stage, and only for the twelve. If the envelope stands on its own, the failure is in the + // payload and the relay has an address and a correlator to answer with — see `ANSWERABLE`. + if (Object.hasOwn(ANSWERABLE, known)) { + const answerable = known as AnswerableType + const envelope = ANSWERABLE[answerable].safeParse(frame) + if (envelope.success) { + const { sessionId: s, requestId: r } = envelope.data + return { ok: false, reason: 'bad-payload', type: answerable, sessionId: s, requestId: r, detail } + } + } + return { ok: false, reason: 'bad-shape', type: known, detail } } return { ok: true, msg: result.data as ParsedInbound, raw: frame } } @@ -426,6 +461,9 @@ type _AgentCovers = Assert< type _AgentInventsNothing = Assert< IsEmpty> > +/** Every answerable request is a browser request. An agent reply is not something the relay answers. */ +type _AnswerableIsBrowser = Assert>> + type _StreamCovers = Assert>> type _StreamInventsNothing = Assert>> diff --git a/packages/relay/AGENTS.md b/packages/relay/AGENTS.md index 6cfc0ea0..eace72ce 100644 --- a/packages/relay/AGENTS.md +++ b/packages/relay/AGENTS.md @@ -43,6 +43,16 @@ iOS build format: `.app.zip` **or** `.tar.gz`/`.tgz` (EAS `eas build` simulator - **agent → browser: the original frame (`raw`).** `z.object` strips undeclared keys, so forwarding the parse product would delete a field a newer agent added — the one direction where the sender is the more recently updated side. - **browser → agent: the parse product (`msg`).** Here the stripping is the point: a key a viewer appended from devtools is gone before any agent sees it. + **A refused browser request is answered, not just dropped.** The envelope is judged separately from + the payload, so a frame whose `sessionId` and `requestId` are good carries everything a reply needs — + and `refuseMalformed` sends the error type that request's own waiter reads, with `reason: 'malformed'` + on the input pair. Without it this door would have converted an answered failure into silence: a + malformed `open-url` used to reach the agent, whose guard answered, and `IOSAgent.ts` names this + validation as what takes that over. The cost is worst on the inputs, because `awaitInputAck` reports + silence from a never-acked session as **success** (#457). The twelve answerable requests, + `ANSWERABLE` in the protocol and the replies in `refuseMalformed` are held to one derived set by + `scripts/__tests__/correlatedRequestsGated.test.mjs`. + Agent payloads are **deliberately not validated**, and the reason is not a deferral. `AgentRegister.platform` is `string`, open so a third-party platform can register through `AgentRegistry.register()` (OCP), while `ChromePayload` is a closed two-member union — so a platform this repo promises to support has no valid `session:chrome` variant, and refusing one would cost it bezel and buttons for the life of the session. The six messages the relay *consumes* (`agent:register`, `agent:resources`, the screenshot and ui-tree replies) are validated, with a `.default()` for every field the relay used to read through a `??`. - **Clipboard bridge** (`clipboard:*`): browser→agent `clipboard:read` (`payload.press`: `'copy' | 'cut'` presses that chord on the device first) and `clipboard:write` (`payload.text`, `payload.pasteAfter`); agent→browser `clipboard:data` / `clipboard:write-done` / `clipboard:error`, correlated by `requestId`. Unlike the other agent→browser replies these are **bound to the session's own `agentSocket`** — their payload lands on the viewer's host OS clipboard, so a second agent must not be able to address someone else's session. An undeliverable request answers `clipboard:error` immediately rather than letting the caller's deadline expire. Agents advertise `capabilities: ['clipboard']` in `agent:register`; the relay echoes them on `session:joined` so a viewer can tell a capable agent from one that predates the feature instead of inferring it from silence. - **An input the relay cannot dispatch is answered here, with a reason.** The four terminal frames get an diff --git a/packages/relay/src/RelayServer.ts b/packages/relay/src/RelayServer.ts index 790d6402..0b1cfa93 100644 --- a/packages/relay/src/RelayServer.ts +++ b/packages/relay/src/RelayServer.ts @@ -141,7 +141,8 @@ function logInboundRejection(failure: ParseFailure): void { logger.debug(`[tapflow] inbound frame of unknown type ${failure.type} — dropped`) return } - logger.warn(`[tapflow] inbound ${failure.type} does not match the contract — dropped:\n${failure.detail}`) + const outcome = failure.reason === 'bad-payload' ? 'refused, and the sender told' : 'dropped' + logger.warn(`[tapflow] inbound ${failure.type} does not match the contract — ${outcome}:\n${failure.detail}`) } /** One member of the parse product, by literal. `route` narrows to these; a handler that takes one * gets exactly what the door proved for that type and nothing else. */ @@ -570,7 +571,12 @@ export class RelayServer { // exists to make visible. if (!inbound.ok) logInboundRejection(inbound) if (!this.settleRole(ws, inbound)) return - if (!inbound.ok) return + if (!inbound.ok) { + // **After the role gate, never before it.** A browser spoofing an agent-only type gets 1008 + // and no reply; only a request this socket is allowed to send earns an answer. + if (inbound.reason === 'bad-payload') this.refuseMalformed(ws, inbound) + return + } try { this.route(ws, inbound.msg, inbound.raw) } catch (e) { @@ -630,7 +636,13 @@ export class RelayServer { * handshake that must not confer a role. */ private settleRole(ws: WebSocket, inbound: ParseResult): boolean { - const type = inbound.ok ? inbound.msg.type : inbound.reason === 'bad-shape' ? inbound.type : undefined + // Every reason that carries a type, which is all of them but the two that have none to carry. + // Missing `bad-payload` here returned `false` before the caller could answer, so the twelve + // answerable requests were classified correctly and then dropped anyway — the regression this + // whole path exists to prevent, reintroduced one line above it. + const type = inbound.ok ? inbound.msg.type + : inbound.reason === 'bad-shape' || inbound.reason === 'bad-payload' ? inbound.type + : undefined if (type === undefined) return false // **The role comes from the two handshake literals, deliberately not from `directionOf`.** Reading @@ -1427,6 +1439,49 @@ export class RelayServer { this.refuseInput(ws, msg, session ? 'agent offline' : 'Session not found', 'channel-unavailable') } + /** + * Tells the sender its payload was refused, in the shape that request's own waiter reads. + * + * **This is what keeps the door from turning an answered failure into silence.** Before it, a + * malformed `open-url` reached the agent and the agent's own guard answered `open-url:error`; + * `IOSAgent.ts` says so beside that guard, and names this validation as what would take it over. + * Taking the responsibility without taking the answer would have been a regression — worst on the + * inputs, and not obviously: `awaitInputAck` reports silence from a session that has never acked as + * **success** (#457), so a dropped `input:key` reads to an MCP caller as an input that landed. + * + * The address and the correlator come from the envelope, which `parseInbound` judges separately from + * the payload for exactly this reason — a frame with a good envelope carries everything a reply + * needs. `reason: 'malformed'` is not a new member; the input vocabulary already had it, and until + * now only agents produced it. + * + * No ownership check, deliberately: the reply goes to the socket that sent the frame and says only + * that its own message was malformed, so it discloses nothing about the session. Every other refusal + * in this file answers a question about the session's state and is gated for that reason. + */ + private refuseMalformed(ws: WebSocket, f: Extract): void { + if (ws.readyState !== WebSocket.OPEN) return + const { sessionId, requestId } = f + const message = `malformed ${f.type} payload` + switch (f.type) { + case 'device:boot': this.sendTo(ws, { type: 'device:boot-error', sessionId, requestId, message }); break + case 'app:install': this.sendTo(ws, { type: 'app:install-error', sessionId, requestId, message }); break + case 'app:launch': this.sendTo(ws, { type: 'app:launch-error', sessionId, requestId, message }); break + case 'app:clear-state': this.sendTo(ws, { type: 'app:clear-state-error', sessionId, requestId, message }); break + case 'open-url': this.sendTo(ws, { type: 'open-url:error', sessionId, requestId, message }); break + // Its waiters key on the `input:type-*` pair and ignore an `input:error` entirely — the same + // reason `refuseInput` below splits these two. + case 'input:type': + this.sendTo(ws, { type: 'input:type-error', sessionId, requestId, message, reason: 'malformed' }); break + case 'clipboard:read': + case 'clipboard:write': this.sendTo(ws, { type: 'clipboard:error', sessionId, requestId, message }); break + // The four remaining acked inputs. A `default` rather than four labels because the union is + // closed and exhaustive: adding a thirteenth answerable request without a case here would land + // it on `input:error`, which `answerableRequestsAnswered` is what stops. + default: + this.sendTo(ws, { type: 'input:error', sessionId, requestId, message, reason: 'malformed' }) + } + } + /** Answers the sender in the shape its waiter is keyed on. * * `input:type` needs `input:type-error`, not `input:error` — its waiters in `mcp-server` and @@ -1474,16 +1529,14 @@ export class RelayServer { // The schema already refused a non-integer `buildId`, so this is now belt-and-braces rather than // the only guard. Kept because it is also the *answer*: the parser drops a bad frame silently and - // this tells the caller `Build not found` instead of leaving it on its deadline. - // better-sqlite3 binds a missing value as NULL but *throws* on an object or array — and that - // exception is swallowed by the message-loop catch, which is the silence this PR exists to - // remove. The schema refuses the object and array outright; this catches the `NaN` it carries - // through for the rest, which is what keeps the caller answered instead of silently dropped. - if (!Number.isInteger(msg.buildId)) return fail('Build not found') - + // No `Number.isInteger` guard here any more: `buildId` is `z.number().int()` at the door, and a bad + // one is answered there by `refuseMalformed` with a diagnosis this branch could not give — "not + // found" describes a lookup, and for a malformed id no lookup ran. The guard existed because + // better-sqlite3 binds a missing value as NULL but **throws** on an object or array, and that + // exception was swallowed by the message-loop catch; the door makes both unreachable. const build = getDb() .prepare('SELECT file_path, bundle_id FROM builds WHERE id = ?') - .get(msg.buildId!) as { file_path: string; bundle_id: string | null } | undefined + .get(msg.buildId) as { file_path: string; bundle_id: string | null } | undefined if (!build) return fail('Build not found') // Answer now rather than letting the caller time out — the same shape as `open-url` above. @@ -1521,12 +1574,10 @@ export class RelayServer { // someone else was testing, with the reply going to that session's browser rather than to it. if (!this.ownsSession(ws, session)) return fail(ownershipRefusal(session)) - // See `handleBrowserAppInstall` — the schema refuses it first; this is what answers the caller. - if (!Number.isInteger(msg.buildId)) return fail('Bundle ID not available for this build') - + // See `handleBrowserAppInstall` — the door refuses a malformed id and answers it. const build = getDb() .prepare('SELECT bundle_id FROM builds WHERE id = ?') - .get(msg.buildId!) as { bundle_id: string | null } | undefined + .get(msg.buildId) as { bundle_id: string | null } | undefined if (!build?.bundle_id) return fail('Bundle ID not available for this build') if (session.agentSocket.readyState !== WebSocket.OPEN) return fail('agent offline') diff --git a/scripts/__tests__/correlatedRequestsGated.test.mjs b/scripts/__tests__/correlatedRequestsGated.test.mjs index 5a89bdf8..8cb1af9e 100644 --- a/scripts/__tests__/correlatedRequestsGated.test.mjs +++ b/scripts/__tests__/correlatedRequestsGated.test.mjs @@ -80,6 +80,14 @@ function correlatedRequestTypes(proto) { function correlatorOf(sf) { const out = new Map() const visit = (node) => { + // **Only the inbound maps.** `ANSWERABLE` sits in the same file and is keyed by the same literals, + // but its values are `envC('…')` calls rather than `z.object({ … })` — so a walk that read it would + // find no `requestId` property and report every correlated request as ungated. Skipping it by name + // rather than by shape, because "no shape I recognise" is exactly the answer a broken parser gives. + if ( + ts.isVariableDeclaration(node) && ts.isIdentifier(node.name) && + !['BROWSER_INBOUND', 'AGENT_CONSUMED', 'AGENT_FORWARDED', 'STREAM_INBOUND'].includes(node.name.text) + ) return // `'app:install': z.object({ … })` — a string-literal key whose value is a call. if (!ts.isPropertyAssignment(node) || !ts.isStringLiteral(node.name)) return ts.forEachChild(node, visit) const shape = ts.isCallExpression(node.initializer) ? node.initializer.arguments[0] : undefined @@ -149,6 +157,56 @@ describe('every correlated browser request is gated at the relay door', () => { }) } + // ── the three lists that must not drift apart ───────────────────────────────────────────────── + // + // A correlated browser request is refused at the door when its payload is wrong, and refusing it + // without answering turns a diagnosis into a caller waiting out its deadline — which for the acked + // inputs is worse than it sounds, since `awaitInputAck` reports silence from a never-acked session + // as **success** (#457). So three lists have to agree, and only one of them is derived: + // + // 1. the correlated request set, from the protocol (above) + // 2. `ANSWERABLE` in `protocol/src/validate/index.ts`, which decides what gets a second parse + // 3. `refuseMalformed` in the relay, which decides which reply each one gets + // + // Nothing else compares them: a request missing from (2) is refused with `bad-shape` and dropped, and + // one missing from (3) falls to a `default` that answers `input:error` — a reply whose waiter does + // not exist for a non-input request. + + /** The literal keys of the `ANSWERABLE` map. */ + function answerableTypes(src) { + const body = src.match(/const ANSWERABLE = \{([\s\S]*?)\n\} as const/) + expect(body, 'ANSWERABLE is gone from protocol/src/validate').not.toBeNull() + return new Set([...body[1].matchAll(/^\s*'([^']+)':/gm)].map((m) => m[1])) + } + + /** The literals `refuseMalformed` names explicitly, i.e. everything not left to its `default`. */ + function explicitlyAnswered(src) { + const body = src.match(/private refuseMalformed\([\s\S]*?\n \}/) + expect(body, 'refuseMalformed is gone from the relay').not.toBeNull() + return new Set([...body[0].matchAll(/case '([^']+)':/g)].map((m) => m[1])) + } + + it('every correlated request can be answered when its payload is refused', () => { + const answerable = answerableTypes(validateSrc) + expect([...types].filter((t) => !answerable.has(t)).sort()).toEqual([]) + expect([...answerable].filter((t) => !types.includes(t)).sort()).toEqual([]) + }) + + it('a non-input request is answered by name, not by the input fallback', () => { + // `refuseMalformed`'s `default` sends `input:error`, which is right for the four remaining acked + // inputs and wrong for anything else — `mcp-server` and `flow-runner` key their waiters on the + // pair each request declares, so an `app:launch` answered with `input:error` is not an answer. + const named = explicitlyAnswered(read('packages/relay/src/RelayServer.ts')) + const unnamed = [...answerableTypes(validateSrc)].filter((t) => !named.has(t)) + expect( + unnamed.filter((t) => !t.startsWith('input:')).sort(), + 'these fall to refuseMalformed\'s input:error default and need a case of their own', + ).toEqual([]) + // Non-vacuous: if the parser found no cases at all, everything would look unnamed and the filter + // above would still pass for the inputs alone. + expect(named.size).toBeGreaterThanOrEqual(7) + }) + it('the shared constants carry the non-empty half', () => { // The per-type assertions above accept the shared constant by name, so this is what gives that name // its meaning. Weakening the constant fails here; weakening one call site fails above. From 0bec9d29327577f2c133b98e78972a067f8ab534 Mon Sep 17 00:00:00 2001 From: Duchan Date: Sat, 15 Aug 2026 18:39:47 +0900 Subject: [PATCH 4/5] fix(scripts): pair each refusal with the reply it sends, not just its case label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate added one commit ago collected `case` labels and stopped there, and a label proves nothing: `sendTo` takes `RelayOutbound`, so every `*-error` literal is a valid member and the compiler is indifferent to which one a branch sends. Substituting `input:error` for `open-url:error`, or leaving a case empty, passed it and every other check — while `flow-runner`'s waiter keys on the `open-url:*` pair, so the caller would burn its full deadline. That is the regression `refuseMalformed` exists to prevent, reintroduced inside it. The reply table is listed where the key set is derived, and the asymmetry is deliberate: which request is answerable is a fact about the protocol, while which reply answers it is a fact about consumers' waiters and the naming is not uniform enough to compute — `open-url` answers `open-url:error`, both clipboard requests share one reply, and four inputs share another. Also: two comments in `appCommandErrors.test.ts` still described the `Number.isInteger` guards this branch deleted, and the note beside the new call site claimed the role gate stops more than it does — an agent-role socket sending a malformed browser request passes it and is answered, which is harmless and worth saying rather than implying otherwise. 5 mutations, 5 caught, including the reviewer's exact case. Co-Authored-By: Claude Opus 5 (1M context) --- packages/relay/src/RelayServer.ts | 8 +- .../src/__tests__/appCommandErrors.test.ts | 12 ++- .../correlatedRequestsGated.test.mjs | 85 +++++++++++++++---- 3 files changed, 82 insertions(+), 23 deletions(-) diff --git a/packages/relay/src/RelayServer.ts b/packages/relay/src/RelayServer.ts index 0b1cfa93..54eac494 100644 --- a/packages/relay/src/RelayServer.ts +++ b/packages/relay/src/RelayServer.ts @@ -572,8 +572,12 @@ export class RelayServer { if (!inbound.ok) logInboundRejection(inbound) if (!this.settleRole(ws, inbound)) return if (!inbound.ok) { - // **After the role gate, never before it.** A browser spoofing an agent-only type gets 1008 - // and no reply; only a request this socket is allowed to send earns an answer. + // **After the role gate, never before it**, so a browser spoofing an agent-only type gets 1008 + // and no reply. That is the whole of the claim: an agent- or stream-role socket sending a + // malformed browser request passes the gate and *is* answered, because the gate only refuses a + // `browser` role sending a non-browser type. Harmless, and worth stating rather than implying + // otherwise — such a socket already gets an answer on the well-formed path (`refuseInput` tells + // it `not-session-owner`), and this reply says strictly less than that one. if (inbound.reason === 'bad-payload') this.refuseMalformed(ws, inbound) return } diff --git a/packages/relay/src/__tests__/appCommandErrors.test.ts b/packages/relay/src/__tests__/appCommandErrors.test.ts index d01c1a46..b95249df 100644 --- a/packages/relay/src/__tests__/appCommandErrors.test.ts +++ b/packages/relay/src/__tests__/appCommandErrors.test.ts @@ -200,8 +200,10 @@ describe('app command failures reach the caller (#445)', () => { browser.close() }) - // A missing or non-numeric buildId reaches the DB query unvalidated (#444 is open on inbound - // validation generally). better-sqlite3 treats both as "no row" rather than throwing, so the + // A missing or non-numeric buildId no longer reaches the DB query at all: `buildId` is + // `z.number().int()` at the door, so the frame is refused there and answered by `refuseMalformed`. + // These assert that an answer still arrives — the property they were written for — rather than + // which prose it carries. Historically better-sqlite3 treated both as "no row" rather than throwing, so the // caller still gets a correlated answer instead of an exception killing the handler — which is // the property this PR is about. The message is imprecise, not absent. it.each([ @@ -220,8 +222,10 @@ describe('app command failures reach the caller (#445)', () => { agent.close(); browser.close() }) - // The schema refuses a non-integer `buildId` at the door now, so this asserts the *answer* the - // handler gives rather than the parse — the caller gets `Build not found` instead of silence. + // Answered by the door rather than by the handler, and with a different diagnosis: `malformed + // app:install payload` rather than `Build not found`, because no lookup ran. These assert the + // correlation and the type, which is the property that matters — a caller that gets *some* addressed, + // correlated reply stops waiting. // An object or array makes better-sqlite3 throw, and that exception used to be caught by the // message loop alongside genuine parse failures — the caller got nothing at all. This is the // same silence the rest of the file is about, reached through the type system's blind spot. diff --git a/scripts/__tests__/correlatedRequestsGated.test.mjs b/scripts/__tests__/correlatedRequestsGated.test.mjs index 8cb1af9e..5f65987b 100644 --- a/scripts/__tests__/correlatedRequestsGated.test.mjs +++ b/scripts/__tests__/correlatedRequestsGated.test.mjs @@ -111,7 +111,6 @@ function correlatorOf(sf) { return out } -describe('every correlated browser request is gated at the relay door', () => { const proto = read('packages/protocol/src/index.ts') const validatePath = 'packages/protocol/src/validate/index.ts' const validateSrc = read(validatePath) @@ -179,11 +178,59 @@ describe('every correlated browser request is gated at the relay door', () => { return new Set([...body[1].matchAll(/^\s*'([^']+)':/gm)].map((m) => m[1])) } - /** The literals `refuseMalformed` names explicitly, i.e. everything not left to its `default`. */ - function explicitlyAnswered(src) { +/** + * Which reply each answerable request must be refused with. + * + * **Listed, where the keys above are derived, and the asymmetry is the point.** The set of answerable + * requests is a fact about the protocol and derives from it; *which reply answers which request* is a + * fact about consumers' waiters and derives from nothing — `mcp-server` and `flow-runner` key on the + * pair each request declares, and the naming is not uniform enough to compute (`open-url` answers + * `open-url:error`, not `open-url-error`; both clipboard requests answer one `clipboard:error`; the + * four remaining inputs share `input:error`). So the keys catch a new request and this catches a + * wrong reply, and the two assertions below need both. + */ +const EXPECTED_REPLY = { + 'device:boot': 'device:boot-error', + 'app:install': 'app:install-error', + 'app:launch': 'app:launch-error', + 'app:clear-state': 'app:clear-state-error', + 'open-url': 'open-url:error', + 'input:type': 'input:type-error', + 'clipboard:read': 'clipboard:error', + 'clipboard:write': 'clipboard:error', + 'input:touch:end': 'input:error', + 'input:pinch:end': 'input:error', + 'input:key': 'input:error', + 'input:button': 'input:error', +} + +describe('every correlated browser request is gated at the relay door', () => { + /** + * `case '':` → the reply literal the branch sends, or `null` for a bare `default`. + * + * **Pairs them, where a first draft collected labels alone** — and a label proves nothing, because + * `sendTo` takes `RelayOutbound` and every `*-error` literal is a valid member, so the compiler is + * indifferent to which one a case sends. Substituting `input:error` for `open-url:error`, or leaving + * a case empty, passed that draft and every other check: `flow-runner`'s waiter keys on the + * `open-url:*` pair, so the caller would burn its full deadline — the regression `refuseMalformed` + * exists to prevent, reintroduced inside it. + */ + function repliesByCase(src) { const body = src.match(/private refuseMalformed\([\s\S]*?\n \}/) expect(body, 'refuseMalformed is gone from the relay').not.toBeNull() - return new Set([...body[0].matchAll(/case '([^']+)':/g)].map((m) => m[1])) + const out = new Map() + let pending = [] + for (const line of body[0].split('\n')) { + const label = line.match(/case '([^']+)':/) + if (label) pending.push(label[1]) + const reply = line.match(/type: '([^']+)'/) + if (reply) { + // A `default:` sends without a pending label; record it under the sentinel. + for (const l of pending.length > 0 ? pending : ['*']) out.set(l, reply[1]) + pending = [] + } + } + return out } it('every correlated request can be answered when its payload is refused', () => { @@ -192,19 +239,23 @@ describe('every correlated browser request is gated at the relay door', () => { expect([...answerable].filter((t) => !types.includes(t)).sort()).toEqual([]) }) - it('a non-input request is answered by name, not by the input fallback', () => { - // `refuseMalformed`'s `default` sends `input:error`, which is right for the four remaining acked - // inputs and wrong for anything else — `mcp-server` and `flow-runner` key their waiters on the - // pair each request declares, so an `app:launch` answered with `input:error` is not an answer. - const named = explicitlyAnswered(read('packages/relay/src/RelayServer.ts')) - const unnamed = [...answerableTypes(validateSrc)].filter((t) => !named.has(t)) - expect( - unnamed.filter((t) => !t.startsWith('input:')).sort(), - 'these fall to refuseMalformed\'s input:error default and need a case of their own', - ).toEqual([]) - // Non-vacuous: if the parser found no cases at all, everything would look unnamed and the filter - // above would still pass for the inputs alone. - expect(named.size).toBeGreaterThanOrEqual(7) + it('each request is refused with the reply its own waiter reads', () => { + const replies = repliesByCase(read('packages/relay/src/RelayServer.ts')) + // The `default` arm covers whatever has no case of its own; it must be the input reply, since that + // is the only one shared by more than one request without being named. + expect(replies.get('*'), 'refuseMalformed has no default arm').toBe('input:error') + for (const type of types) { + expect(replies.get(type) ?? replies.get('*'), `${type} is refused with the wrong reply`) + .toBe(EXPECTED_REPLY[type]) + } + // Non-vacuous: an empty parse would make every lookup fall to the default and agree with the four + // inputs by accident. + expect(replies.size).toBeGreaterThanOrEqual(9) + }) + + it('the reply table covers exactly the correlated set', () => { + expect([...types].filter((t) => !(t in EXPECTED_REPLY)).sort()).toEqual([]) + expect(Object.keys(EXPECTED_REPLY).filter((t) => !types.includes(t)).sort()).toEqual([]) }) it('the shared constants carry the non-empty half', () => { From 1b05faf8ab7c103677397aceb7fd39a78ff392e4 Mon Sep 17 00:00:00 2001 From: Duchan Date: Sun, 16 Aug 2026 00:14:15 +0900 Subject: [PATCH 5/5] fix(relay): throttle the rejection log per socket, and scope two claims (CodeRabbit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings on #564. **The rejection warn was one line per rejected frame**, on the direction a viewer with devtools controls and at the rate a gesture produces — the unbounded, attacker-driven log volume this very file already refuses at `forwardUnacked`, with the reason written beside it. I wrote that reason and then broke it two hundred lines up. The suggested fix was a module-level timestamp, and that fixes the volume by breaking the diagnostic: one noisy socket would swallow the *first* bad frame from another, which is the skewed-client case (`mcp-server` upgraded without the relay) the log exists to surface. So the state is keyed by socket in a `WeakMap` that needs no cleanup, the first rejection from any socket is always written, and the next line says how many were swallowed. `unknown-type` shares the throttle so turning debug on cannot reintroduce the volume. `a second socket still gets its first line` is the assertion that carries the design — it is the only one a global throttle fails, and it does. Also: the release notes said the relay validates "every message" while the same notes explain that agent payloads deliberately are not, so both now say what is checked for every frame and what is checked in full. And a line in `protocol/AGENTS.md` began with `#490`, which markdown renders as a heading. 4 mutations, 4 caught, including the global-throttle sketch. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/relay-inbound-validated.md | 2 +- CHANGELOG.md | 3 +- packages/protocol/AGENTS.md | 4 +- packages/relay/src/RelayServer.ts | 79 ++++++++++----- .../src/__tests__/inboundRejectionLog.test.ts | 96 +++++++++++++++++++ 5 files changed, 158 insertions(+), 26 deletions(-) create mode 100644 packages/relay/src/__tests__/inboundRejectionLog.test.ts diff --git a/.changeset/relay-inbound-validated.md b/.changeset/relay-inbound-validated.md index ded7b8da..a03e4063 100644 --- a/.changeset/relay-inbound-validated.md +++ b/.changeset/relay-inbound-validated.md @@ -3,7 +3,7 @@ '@tapflowio/relay': minor --- -Validate every message the relay receives, and make the inbound frame a discriminated union +Check every message the relay receives against the contract, 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 diff --git a/CHANGELOG.md b/CHANGELOG.md index d3e231f0..0fd28f6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,7 +47,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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 + break it.** Every frame is checked for its type, its address and its correlator; a command sent by a + browser is checked in full, down to its payload. 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. A refused command is now answered where the request has a reply, so diff --git a/packages/protocol/AGENTS.md b/packages/protocol/AGENTS.md index 7cc029f9..04801621 100644 --- a/packages/protocol/AGENTS.md +++ b/packages/protocol/AGENTS.md @@ -358,8 +358,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(raw)` — the frame exactly as it arrived — 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 diff --git a/packages/relay/src/RelayServer.ts b/packages/relay/src/RelayServer.ts index 54eac494..57f628fc 100644 --- a/packages/relay/src/RelayServer.ts +++ b/packages/relay/src/RelayServer.ts @@ -119,30 +119,19 @@ function ownershipRefusal(session: Session): string { return session.browserSocket ? 'session held by another client' : 'session not joined' } -/** - * What the door refused, said once and in a form an operator can act on. - * - * This is where `isAddressed` and `isCorrelated` ended up. Both were predicates whose whole - * observable output was a `console.warn` — an id-less request resolved no session and was dropped by - * the miss anyway — and the schemas now reject the same frames earlier, including the empty-string - * case a bare `z.string()` would have let through. What is new is that the log names the *field*: - * "requestId: Too small" instead of "dropped, cannot correlate a reply". - * - * Worth logging at all for the reason `isCorrelated` gave: the three places a bad frame can be - * dropped are otherwise silent, and an operator who upgrades the relay but not an independently - * installed `mcp-server` would watch commands do nothing with no trace. - * - * `unknown-type` stays at debug: eleven relay-produced literals land here whenever a client echoes - * one back, and none of them is a defect. - */ -function logInboundRejection(failure: ParseFailure): void { - if (failure.reason === 'not-an-object') return +/** One line per rejecting socket per second, at most. See `RelayServer.logInboundRejection`. */ +const REJECT_LOG_INTERVAL_MS = 1_000 + +/** What the door refused, as one line. The throttling that decides whether to write it is the + * caller's, because it is per socket and this function has no state. */ +function describeRejection(failure: ParseFailure, suppressed: number): string | null { + if (failure.reason === 'not-an-object') return null + const also = suppressed > 0 ? ` (+${suppressed} more from this socket in the last second)` : '' if (failure.reason === 'unknown-type') { - logger.debug(`[tapflow] inbound frame of unknown type ${failure.type} — dropped`) - return + return `[tapflow] inbound frame of unknown type ${failure.type} — dropped${also}` } const outcome = failure.reason === 'bad-payload' ? 'refused, and the sender told' : 'dropped' - logger.warn(`[tapflow] inbound ${failure.type} does not match the contract — ${outcome}:\n${failure.detail}`) + return `[tapflow] inbound ${failure.type} does not match the contract — ${outcome}${also}:\n${failure.detail}` } /** One member of the parse product, by literal. `route` narrows to these; a handler that takes one * gets exactly what the door proved for that type and nothing else. */ @@ -183,6 +172,9 @@ export class RelayServer { // Per-session throttled "request an IDR from the agent" callbacks (drop recovery). private idrRequesters = new Map void>() private wsRoles = new Map() + /** Per-socket throttle state for `logInboundRejection`. A `WeakMap` so a closed socket's entry goes + * with the socket — there is no cleanup to forget, unlike the maps keyed by session id nearby. */ + private readonly rejectionLog = new WeakMap() // Agent sockets whose sessions are being held open, and the timer that gives up on each. // Keyed by the dead socket, never by session id: a rebind moves sessions off that socket, so an // expiry that fires late has nothing left to evict. That is the invariant — releasing the hold @@ -569,7 +561,7 @@ export class RelayServer { // `settleRole` returns `false` for them. The case that mattered was a malformed handshake on a // role-less socket: dropped in silence, which is precisely the agent-registration skew this log // exists to make visible. - if (!inbound.ok) logInboundRejection(inbound) + if (!inbound.ok) this.logInboundRejection(ws, inbound) if (!this.settleRole(ws, inbound)) return if (!inbound.ok) { // **After the role gate, never before it**, so a browser spoofing an agent-only type gets 1008 @@ -679,6 +671,49 @@ export class RelayServer { return true } + /** + * What the door refused, said once and in a form an operator can act on. + * + * This is where `isAddressed` and `isCorrelated` ended up. Both were predicates whose whole + * observable output was a `console.warn` — an id-less request resolved no session and was dropped by + * the miss anyway — and the schemas now reject the same frames earlier, including the empty-string + * case a bare `z.string()` would have let through. What is new is that the log names the *field*: + * "requestId: Too small" instead of "dropped, cannot correlate a reply". + * + * Worth logging at all for the reason `isCorrelated` gave: the three places a bad frame can be + * dropped are otherwise silent, and an operator who upgrades the relay but not an independently + * installed `mcp-server` would watch commands do nothing with no trace. + * + * **Throttled per socket, and the "per socket" is the whole design.** A first draft wrote one line + * per rejected frame, on the direction a viewer with devtools controls and at the rate a gesture + * produces — the unbounded, attacker-driven log volume this file already refuses at + * `forwardUnacked`, with the reason written beside it. A single module-level timestamp would fix the + * volume and break the diagnostic: one noisy socket would silence the *other* socket's first bad + * frame, which is the skewed-client case the log exists for. So the state is keyed by socket, in a + * `WeakMap` that needs no cleanup, and **the first rejection from any socket is always written** — + * the one that names the skew is never the hundredth. What is dropped is only repetition, and the + * next line says how much. + * + * `unknown-type` stays at debug: eleven relay-produced literals land here whenever a client echoes + * one back, and none of them is a defect. It shares the throttle so turning debug on cannot + * reintroduce the volume. + */ + private logInboundRejection(ws: WebSocket, failure: ParseFailure): void { + const now = Date.now() + const state = this.rejectionLog.get(ws) + if (state && now - state.at < REJECT_LOG_INTERVAL_MS) { + state.suppressed++ + return + } + const line = describeRejection(failure, state?.suppressed ?? 0) + // Recorded even when there is nothing to write, so a burst of `not-an-object` frames cannot reset + // the window for the reasons that do write. + this.rejectionLog.set(ws, { at: now, suppressed: 0 }) + if (line === null) return + if (failure.reason === 'unknown-type') logger.debug(line) + else logger.warn(line) + } + private route(ws: WebSocket, msg: ParsedInbound, raw: Readonly>): void { switch (msg.type) { diff --git a/packages/relay/src/__tests__/inboundRejectionLog.test.ts b/packages/relay/src/__tests__/inboundRejectionLog.test.ts new file mode 100644 index 00000000..196cb434 --- /dev/null +++ b/packages/relay/src/__tests__/inboundRejectionLog.test.ts @@ -0,0 +1,96 @@ +import { describe, it, expect, vi, beforeAll, afterAll, beforeEach, afterEach } from 'vitest' +import fs from 'fs' +import os from 'os' +import path from 'path' +import { WebSocket } from 'ws' +import { RelayServer } from '../RelayServer' +import { initDb, closeDb } from '../db' +import { waitForOpen, waitForType } from '@tapflowio/test-utils' + +let tmpDir: string + +beforeAll(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tapflow-rejectlog-test-')) + initDb(path.join(tmpDir, 'test.db')) +}) + +afterAll(() => { + closeDb() + fs.rmSync(tmpDir, { recursive: true }) +}) + +describe('the rejection log is throttled per socket', () => { + // **A first draft wrote one line per rejected frame.** That is the unbounded, attacker-driven log + // volume this relay already refuses at `forwardUnacked`, with the reason written beside it — a + // viewer with devtools open can send malformed frames at gesture rate. + // + // The obvious fix is a module-level timestamp, and it breaks the thing the log is for: one noisy + // socket would swallow the *first* bad frame from another, which is the skewed-client case + // (`mcp-server` upgraded without the relay) the diagnostic exists to surface. So the state is keyed + // by socket, and `a second socket still gets its first line` is the assertion that says so — it is + // the only one a global throttle fails. + let server: RelayServer + let port: number + let warn: ReturnType + + beforeEach(async () => { + server = new RelayServer({ port: 0 }) + await server.start() + port = (server.address() as { port: number }).port + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + }) + + afterEach(async () => { + warn.mockRestore() + await server.stop() + }) + + const mismatches = (): string[] => + warn.mock.calls.flat().filter((c: unknown): c is string => typeof c === 'string' && c.includes('does not match the contract')) + + /** A socket that has sent `n` malformed frames and waited for the relay to have handled them. */ + async function spam(n: number): Promise { + const ws = new WebSocket(`ws://localhost:${port}`) + await waitForOpen(ws) + for (let i = 0; i < n; i++) { + ws.send(JSON.stringify({ type: 'session:start', sessionId: 7 })) + } + // A round-trip proves the relay has finished with everything sent before it — an answer rather + // than a sleep, which is what `barrier` exists for in this repo's socket helpers. + ws.send(JSON.stringify({ type: 'agents:list' })) + await waitForType(ws, 'agents:listed') + return ws + } + + it('writes the first rejection and suppresses the burst behind it', async () => { + const ws = await spam(40) + expect(mismatches()).toHaveLength(1) + expect(mismatches()[0]).toContain('session:start') + ws.close() + }) + + it('a second socket still gets its first line', async () => { + const a = await spam(20) + const b = await spam(20) + // Two sockets, two first lines. A shared timestamp gives one. + expect(mismatches()).toHaveLength(2) + a.close() + b.close() + }) + + it('says how many it swallowed when the next one is written', async () => { + const ws = await spam(5) + vi.setSystemTime(Date.now() + 2_000) + try { + ws.send(JSON.stringify({ type: 'session:start', sessionId: 7 })) + ws.send(JSON.stringify({ type: 'agents:list' })) + await waitForType(ws, 'agents:listed') + const lines = mismatches() + expect(lines).toHaveLength(2) + expect(lines[1]).toMatch(/\+4 more from this socket/) + } finally { + vi.useRealTimers() + ws.close() + } + }) +})