From e11e0b6475cabd053b439ad712fbbb1eebd27ca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sat, 15 Aug 2026 14:49:19 +0000 Subject: [PATCH] one port announcement instead of sixteen, so a plugin can name itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit protocol.ts declared a separate SidecarEvent member per sidecar — music, vault, slskd, headscale, transmission, invoiceshelf, jellyfin, photos, memos, gitea, caldav, wallet, pty, email, notify, opencode — each an identical one-liner. Two of them were for plugins that had already left the repository, which is the tell: the platform was declaring event types on behalf of code it no longer contains. The cost was that a plugin could not announce its own port. `connection.send` takes a SidecarEvent, so `{ type: 'notes:server', port }` did not typecheck until someone added a line to the platform. A third-party plugin needed a pull request against Officer before its sidecar would compile — for the one message every HTTP sidecar has to send. Now: export type SidecarPortAnnouncement = { type: `${string}:server`; port: number }; Nothing was lost at the point of use. The only consumer is `sidecar.on(`${name}:server`, …)` in create-proxy.ts, which already cast to read `.port`, because a handler typed `(event: SidecarEvent) => void` cannot narrow from a computed template string. The proof is a test that already existed: create-proxy.test.ts announces `testcar:server` — a name never in the union — and now compiles without its `as never`. Removing the two casts that escaped the old type found something else. They were not hiding union membership, they were hiding `port: number | undefined`: Bun types Server.port as optional because a unix-socket server has none. So `as SidecarEvent` on the whole object was suppressing a real nullability warning, and would have suppressed a genuinely wrong event shape too. Replaced with a narrow assertion on the port alone. Two further `as SidecarEvent` casts in the same file turned out to be unnecessary altogether and are gone. [open] The typo check is genuinely gone: a sidecar sending `muzik:server` while its proxy listens for `music:server` now compiles, and the symptom is every request answering 503 forever. The fix is not to restore the list — it is for createSidecarConnector to announce the port itself from the `name` it already holds, so the string is written once and the typo becomes unrepresentable. Recorded in protocol.ts. Verified live: officer and officer-music restarted, `[music] sidecar registered on port 38803`, /api/music/manifest and /api/offscale/_health both 200. tsgo clean, 797 tests, 787 pass, same 7. --- src/servers/sidecar/create-proxy.test.ts | 2 +- src/servers/sidecar/notify/index.ts | 10 ++-- src/servers/sidecar/protocol.ts | 68 +++++++++++++----------- 3 files changed, 45 insertions(+), 35 deletions(-) diff --git a/src/servers/sidecar/create-proxy.test.ts b/src/servers/sidecar/create-proxy.test.ts index 67a51f6c..5f0a2b81 100644 --- a/src/servers/sidecar/create-proxy.test.ts +++ b/src/servers/sidecar/create-proxy.test.ts @@ -106,7 +106,7 @@ beforeAll(async () => { const proxy = createSidecarProxy({ name: 'testcar', prefix: '/api/testcar' }); // What the registration socket would deliver when the sidecar reports its port. - handleSidecarMessage('test', { type: 'testcar:server', port: sidecar.port } as never); + handleSidecarMessage('test', { type: 'testcar:server', port: sidecar.port! }); const protectedRouter = createRouter(); protectedRouter.use(bodyParser()); diff --git a/src/servers/sidecar/notify/index.ts b/src/servers/sidecar/notify/index.ts index f50bdf83..07626c9f 100644 --- a/src/servers/sidecar/notify/index.ts +++ b/src/servers/sidecar/notify/index.ts @@ -98,13 +98,17 @@ const connection = createSidecarConnector({ capabilities: ['notify'], onCommand(cmd, reply) { const c = cmd as SidecarCommand; - if (c.type === 'ping') return reply({ type: 'pong', id: c.id } as SidecarEvent); - reply({ type: 'error', id: (c as { id?: string }).id, error: `Unknown command type: ${c.type}` } as SidecarEvent); + if (c.type === 'ping') return reply({ type: 'pong', id: c.id }); + reply({ type: 'error', id: (c as { id?: string }).id, error: `Unknown command type: ${c.type}` }); }, onConnected() { // Re-announced on every reconnect: the platform forgets the port when the socket drops, and this // listener outlives an officer restart. - connection.send({ type: 'notify:server', port: server.port } as SidecarEvent); + // + // Bun types `Server.port` as optional because a unix-socket server has none; this one is TCP, so it + // is a number. Asserted narrowly here — it used to be `as SidecarEvent` on the whole object, which + // suppressed this and would have suppressed a genuinely wrong event shape too. + connection.send({ type: 'notify:server', port: server.port! }); }, }); diff --git a/src/servers/sidecar/protocol.ts b/src/servers/sidecar/protocol.ts index 21f0a949..14b9f666 100644 --- a/src/servers/sidecar/protocol.ts +++ b/src/servers/sidecar/protocol.ts @@ -83,6 +83,14 @@ export type SidecarCommand = // ── Responses/Events (sidecar → API server) ── +/** + * A sidecar telling the platform where its HTTP server is listening. + * + * `:server`, where the name is the sidecar's own — `music:server`, `pty:server`, and whatever a + * plugin nobody has written yet decides to call itself. Deliberately open: see the note in SidecarEvent. + */ +export type SidecarPortAnnouncement = { type: `${string}:server`; port: number }; + export type SidecarEvent = | { type: 'pong'; id: string } | { type: 'state:sync'; id: string; state: ClaudeState } @@ -118,37 +126,35 @@ export type SidecarEvent = | { type: 'opencode:message'; sessionKey: string; msg: TurnMessage; seq?: number } | { type: 'opencode:session'; sessionKey: string; sessionId: string } | { type: 'opencode:error'; id: string; error: string } - // Music — the sidecar reports where its audio-streaming HTTP server is listening (random port) on connect - | { type: 'music:server'; port: number } - // Vault — the sidecar reports where its Vaultwarden reverse-proxy HTTP/WS server is listening on connect - | { type: 'vault:server'; port: number } - // slskd — the sidecar reports where its slskd reverse-proxy HTTP server is listening (random port) on connect - | { type: 'slskd:server'; port: number } - // Headscale — the sidecar reports where its HTTP server is listening (random port) on connect - | { type: 'headscale:server'; port: number } - // Transmission — the sidecar reports where its HTTP server is listening (random port) on connect - | { type: 'transmission:server'; port: number } - // InvoiceShelf — the sidecar reports where its HTTP server is listening (random port) on connect - | { type: 'invoiceshelf:server'; port: number } - // Jellyfin — the sidecar reports where its HTTP server is listening (random port) on connect - | { type: 'jellyfin:server'; port: number } - // Photos (Immich) — the sidecar reports where its HTTP server is listening (random port) on connect - | { type: 'photos:server'; port: number } - // Memos — the sidecar reports where its HTTP server is listening (random port) on connect - | { type: 'memos:server'; port: number } - // Gitea — the sidecar reports where its HTTP server is listening (random port) on connect - | { type: 'gitea:server'; port: number } - // CalDAV/CardDAV — the sidecar reports where its HTTP server is listening (random port) on connect. - // One port serves both doors: /dav (forwarded to Radicale) and /_officer (JSON for Officer's UI). - | { type: 'caldav:server'; port: number } - // Wallet — the sidecar reports where its HTTP server is listening (random port) on connect - | { type: 'wallet:server'; port: number } - // PTY — the sidecar reports where its terminal HTTP/WS server is listening (random port) on connect - | { type: 'pty:server'; port: number } - // Email — the sidecar reports where its mail HTTP server is listening (random port) on connect - | { type: 'email:server'; port: number } - // Notify — the sidecar reports where its notification HTTP server is listening (random port) on connect - | { type: 'notify:server'; port: number } + // Every HTTP sidecar's port announcement, as ONE member rather than sixteen. + // + // A sidecar binds an ephemeral loopback port and tells the platform over the registration socket; the + // proxy for its prefix subscribes to `:server` and remembers the number. See create-proxy.ts. + // + // ── Why this is not a list of names ── + // + // It was: music, vault, slskd, headscale, transmission, invoiceshelf, jellyfin, photos, memos, gitea, + // caldav, wallet, pty, email and notify, each with an identical one-line entry. Two of those were for + // plugins that had already left the repository, which is the tell — the platform was declaring event + // types on behalf of code it no longer contains. + // + // The real cost was that a plugin could not announce its own port. `connection.send` takes a + // SidecarEvent, so `{ type: 'notes:server', port }` did not typecheck until somebody added a line + // HERE. A third-party plugin needed a pull request against the platform before its sidecar would + // compile — for the one message every HTTP sidecar has to send. + // + // Nothing was lost by generalising. The only consumer is `sidecar.on(\`${name}:server\`, …)` in + // create-proxy.ts, which already casts to read `.port` because a handler typed + // `(event: SidecarEvent) => void` cannot narrow from a computed template string. The sixteen entries + // bought a typo check on the SENDING side and nothing at the point of use — and two call sites had + // already cast past it (`as SidecarEvent` in notify, `as never` in create-proxy.test.ts). + // + // `[open]` That typo check is genuinely gone: a sidecar sending `muzik:server` while its proxy listens + // for `music:server` now compiles, and the symptom is every request answering 503 forever because the + // port is never learned. The fix is not to restore the list — it is for `createSidecarConnector` to + // announce the port itself from the `name` it already holds, so the string is written once instead of + // twice and the typo becomes unrepresentable. See create-proxy.ts. + | SidecarPortAnnouncement // Generic | { type: 'error'; id?: string; error: string };