fix(display): define the display-server messages once, in src/shared
CI / lint (pull_request) Successful in 13s
CI / typecheck (pull_request) Successful in 15s
CI / test (pull_request) Successful in 21s
CI / build (pull_request) Successful in 18s
CI / integration (pull_request) Successful in 3m32s

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]>
This commit is contained in:
ekklesia
2026-10-05 21:25:21 -05:00
co-authored by Claude Opus 5.5
parent 458043280a
commit f1b7d67aa5
9 changed files with 555 additions and 200 deletions
+125
View File
@@ -0,0 +1,125 @@
import { describe, expect, it } from "vitest";
import {
DISPLAY_ERROR_DETAIL_MAX,
parseDisplayHeartbeat,
parseViewerSessionRequest,
parseViewerSessionResponse,
} from "../src/shared/display-protocol.js";
/**
* The rules every display message follows, pinned one by one, because the production failure was
* exactly a disagreement about one of them.
*/
const FULL_HEARTBEAT = {
cachedAssetCount: 3,
error: null,
offlineReady: true,
reportedContributionId: null,
reportedPlaybackRevision: 4,
reportedProviderId: "signage.native-images",
reportedSelectionId: "sel-1",
reportedTimelineId: "timeline-1",
};
describe("display heartbeat", () => {
it("accepts null for every nullable field: null is a value, not an omission", () => {
const result = parseDisplayHeartbeat({
...FULL_HEARTBEAT,
reportedContributionId: null,
reportedPlaybackRevision: null,
reportedProviderId: null,
reportedSelectionId: null,
reportedTimelineId: null,
});
expect(result).toEqual({
ok: true,
value: {
...FULL_HEARTBEAT,
reportedContributionId: null,
reportedPlaybackRevision: null,
reportedProviderId: null,
reportedSelectionId: null,
reportedTimelineId: null,
},
});
});
it("treats an absent field as not reported, so the receiver keeps what it had", () => {
const result = parseDisplayHeartbeat({ offlineReady: false });
expect(result).toEqual({ ok: true, value: { offlineReady: false } });
});
it("ignores a field it does not know, so a newer display is never refused by an older server", () => {
const result = parseDisplayHeartbeat({ ...FULL_HEARTBEAT, reportedSomethingNew: { a: 1 } });
expect(result).toEqual({ ok: true, value: FULL_HEARTBEAT });
});
it.each([
["cachedAssetCount", null],
["cachedAssetCount", -1],
["cachedAssetCount", 1.5],
["offlineReady", null],
["offlineReady", "yes"],
["reportedSelectionId", ""],
["reportedSelectionId", "x".repeat(65)],
["reportedProviderId", 7],
["reportedPlaybackRevision", -1],
["error", { code: "" }],
["error", { code: "BAD CODE" }],
["error", { code: "OK", detail: "y".repeat(DISPLAY_ERROR_DETAIL_MAX + 1) }],
["error", "SLIDE_UNRENDERABLE"],
])("refuses %s = %j, naming that field and nothing else", (field, value) => {
expect(parseDisplayHeartbeat({ ...FULL_HEARTBEAT, [field]: value })).toEqual({
field,
ok: false,
});
});
it("accepts a reported error with or without detail", () => {
expect(parseDisplayHeartbeat({ error: { code: "SLIDE_UNRENDERABLE", detail: null } })).toEqual({
ok: true,
value: { error: { code: "SLIDE_UNRENDERABLE", detail: null } },
});
expect(parseDisplayHeartbeat({ error: { code: "SLIDE_UNRENDERABLE" } })).toEqual({
ok: true,
value: { error: { code: "SLIDE_UNRENDERABLE", detail: null } },
});
});
it("refuses something that is not an object at all", () => {
for (const body of [null, [], "heartbeat", 3]) {
expect(parseDisplayHeartbeat(body)).toEqual({ field: "body", ok: false });
}
});
});
describe("viewer session", () => {
it("requires a well-formed viewer key", () => {
expect(parseViewerSessionRequest({ viewerKey: "kiosk-lobby-0001" })).toEqual({
ok: true,
value: { viewerKey: "kiosk-lobby-0001" },
});
for (const viewerKey of [undefined, null, "short", "has spaces in it", "x".repeat(129)]) {
expect(parseViewerSessionRequest({ viewerKey })).toEqual({ field: "viewerKey", ok: false });
}
});
it("a display accepts only a complete registration", () => {
const complete = {
channelId: "chan-1",
channelKey: "default",
groupId: "group-1",
heartbeatIntervalMs: 12_000,
viewerId: "11111111-1111-4111-8111-111111111111",
viewerName: "New display",
};
expect(parseViewerSessionResponse(complete)).toEqual({ ok: true, value: complete });
const { viewerId: _omitted, ...missing } = complete;
expect(parseViewerSessionResponse(missing)).toEqual({ field: "viewerId", ok: false });
expect(parseViewerSessionResponse({ ...complete, heartbeatIntervalMs: 0 })).toEqual({
field: "heartbeatIntervalMs",
ok: false,
});
});
});