Zayin/Signage
3
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 4 Wiki Activity

fix(display): define the display-server messages once, in src/shared #4

Merged
Zayin_Admin merged 2 commits from fix/display-protocol-contract into main 2026-10-06 02:30:41 +00:00
Conversation 1 Commits 2 Files Changed 10
+782 -201
ekklesia-bot commented 2026-10-05 23:33:00 +00:00
Member
Copy Link
Copy Source

fix(display): define the display–server messages once, in src/shared

Cause of the production "Offline / Not offline ready" roster.

  • The display sends reportedContributionId: null for every native-image slide.
  • The heartbeat route's optionalString accepted only a missing field, so it refused null with a 400.
  • Every heartbeat from such a screen was rejected silently.
  • lastSeenAt moved only when a page opened its viewer session, so the roster said Offline 90 s later while the screen was fine.
  • offlineReady and cachedAssetCount were never recorded.

Reproduced in a real browser against platform v0.1.1 + Signage v0.1.1 (Ekklesia draft PR #56): 27 of 27 heartbeats in one display's 5.5-minute run were 400.

Why the tests missed it: each side was tested only against its own idea of the other. The display tests stubbed the heartbeat route with {}, and the viewer tests called the service directly with hand-written objects.

Commit 1, the failing test first

tests/browser/display-contract.test.ts answers the real display controller's requests with the real viewer route handlers, over the in-memory store.

  • The routes gain an optional store factory for this; production still uses the module database.
  • On main: POST /viewers/:viewerId/heartbeat → 400 SIGNAGE_INVALID_REQUEST, and the viewer's reported selection stays null.

Commit 2, the fix

src/shared/display-protocol.ts defines every display ↔ server message, and both halves import it:

Message Server Display
POST viewers/session parses the request, types the response parses the response (was a cast)
POST viewers/:id/heartbeat parses the request, types the response builds the request as the shared type
GET channels/:id/display-state its state type extends the wire type, so drift is a compile error reads the wire type (was its own copy)
realtime display-state the publication is typed reads the group revisions it needs

Rules for every message:

  • A nullable field accepts null explicitly.
  • An absent field means "not reported", so the receiver keeps what it had.
  • An unknown field is ignored, so a newer display is never refused by an older server.

A refusal is Signage's sanitized SIGNAGE_INVALID_REQUEST naming the field.

One behaviour change: an absent reported* field used to clear the stored value, and now keeps it. The display always sends every field, so no real display is affected.

Test stubs: they described server responses more loosely than the server sends them. They now match the real responses (viewerName, stale, full media records) and are typed as the protocol.

Verification

Signage has no CI. Run locally in an Ekklesia main (= v0.1.1) workspace:

  • pnpm --filter @ekklesia/signage test: 220 passed, including 20 new protocol tests and the 2 contract tests;
  • pnpm --filter @ekklesia/signage typecheck: clean;
  • pnpm --filter @ekklesia/signage lint: clean.

Not in this PR

Retry and recovery for the display's own requests (the next PR) and blob-URL release (the PR after). Showing rejected reports in the roster is a later PR.

🤖 Generated with Claude Code

## fix(display): define the display–server messages once, in `src/shared` **Cause of the production "Offline / Not offline ready" roster.** - The display sends `reportedContributionId: null` for every native-image slide. - The heartbeat route's `optionalString` accepted only a *missing* field, so it refused `null` with a 400. - Every heartbeat from such a screen was rejected silently. - `lastSeenAt` moved only when a page opened its viewer session, so the roster said Offline 90 s later while the screen was fine. - `offlineReady` and `cachedAssetCount` were never recorded. Reproduced in a real browser against platform v0.1.1 + Signage v0.1.1 (Ekklesia draft PR #56): 27 of 27 heartbeats in one display's 5.5-minute run were `400`. **Why the tests missed it:** each side was tested only against its own idea of the other. The display tests stubbed the heartbeat route with `{}`, and the viewer tests called the service directly with hand-written objects. ### Commit 1, the failing test first `tests/browser/display-contract.test.ts` answers the **real** display controller's requests with the **real** viewer route handlers, over the in-memory store. - The routes gain an optional store factory for this; production still uses the module database. - On `main`: `POST /viewers/:viewerId/heartbeat` → `400 SIGNAGE_INVALID_REQUEST`, and the viewer's reported selection stays `null`. ### Commit 2, the fix `src/shared/display-protocol.ts` defines every display ↔ server message, and both halves import it: | Message | Server | Display | |---|---|---| | `POST viewers/session` | parses the request, types the response | **parses** the response (was a cast) | | `POST viewers/:id/heartbeat` | parses the request, types the response | builds the request as the shared type | | `GET channels/:id/display-state` | its state type **extends** the wire type, so drift is a compile error | reads the wire type (was its own copy) | | realtime `display-state` | the publication is typed | reads the group revisions it needs | Rules for every message: - A nullable field accepts `null` explicitly. - An absent field means "not reported", so the receiver keeps what it had. - An unknown field is ignored, so a newer display is never refused by an older server. A refusal is Signage's sanitized `SIGNAGE_INVALID_REQUEST` naming the field. **One behaviour change:** an *absent* `reported*` field used to clear the stored value, and now keeps it. The display always sends every field, so no real display is affected. **Test stubs:** they described server responses more loosely than the server sends them. They now match the real responses (`viewerName`, `stale`, full media records) and are typed as the protocol. ### Verification Signage has no CI. Run locally in an Ekklesia `main` (= v0.1.1) workspace: - `pnpm --filter @ekklesia/signage test`: 220 passed, including 20 new protocol tests and the 2 contract tests; - `pnpm --filter @ekklesia/signage typecheck`: clean; - `pnpm --filter @ekklesia/signage lint`: clean. ### Not in this PR Retry and recovery for the display's own requests (the next PR) and blob-URL release (the PR after). Showing rejected reports in the roster is a later PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Zayin_Admin added 2 commits 2026-10-06 02:25:45 +00:00
test(display): run the real display controller against the real viewer routes 458043280a 
Every display test answered the controller with a hand-written stub, and
every viewer test called the service with a hand-written object, so
neither side was ever tested against the other. In production every
heartbeat was refused with a 400.

This suite answers the real controller's requests with the real viewer
route handlers over the in-memory store. The routes gain an optional
store factory for that; production still uses the module database.

Fails today: POST /viewers/:viewerId/heartbeat -> 400
SIGNAGE_INVALID_REQUEST, because the display sends
reportedContributionId: null and the route accepts only a missing field.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
fix(display): define the display-server messages once, in src/shared
CI / lint (pull_request) Successful in 13s
Details
CI / typecheck (pull_request) Successful in 15s
Details
CI / test (pull_request) Successful in 21s
Details
CI / build (pull_request) Successful in 18s
Details
CI / integration (pull_request) Successful in 3m32s
Details
f1b7d67aa5 
The display controller and the viewer routes each described the
heartbeat their own way and disagreed about one thing: the display sent
null for "nothing to report", and the route accepted only a missing
field. Every heartbeat from a screen showing a native image was refused
with a 400. The roster therefore marked working screens Offline 90 s
after they opened, and never recorded offlineReady or cachedAssetCount.

src/shared/display-protocol.ts now defines every message between a
display and the server:
- viewer session request and response;
- heartbeat request and response;
- channel state;
- the realtime display payload.

Both halves import it. Its rules: a nullable field accepts null
explicitly, an absent field means "not reported", and an unknown field
is ignored, so a newer display is never refused by an older server.

- The routes parse with the shared parsers, and a refusal names the
  field with Signage's sanitized code.
- The server's channel state extends the wire type, so a field the
  display relies on cannot drift without a compile error.
- The display builds its heartbeat as the shared type, and reads its
  registration with the shared parser instead of casting it.
- Test stubs that described server responses more loosely than the
  server sends them (no viewerName, no stale, partial media) now match
  the real responses and are typed as the protocol.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Zayin_Admin force-pushed fix/display-protocol-contract from 776540ff78 to f1b7d67aa5 2026-10-06 02:25:45 +00:00 Compare
Zayin_Admin merged commit 27ee279f28 into main 2026-10-06 02:30:41 +00:00
Zayin_Admin deleted branch fix/display-protocol-contract 2026-10-06 02:30:41 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Zayin_Admin
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
Joey Zayin_Admin ekklesia-bot
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Zayin/Signage#4