From 2d5e9835f50d39b7f96c771e158feaa8b8eb28d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Mon, 24 Aug 2026 15:24:17 +0000 Subject: [PATCH] the catalogue shapes a commander's words, not the commander MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "it should always say Commander: " was left to whether the commander remembered to write it. that is not a structure, it is a hope — and the producer's title reached the lock screen untouched, because both transports built the alert from n.title rather than from the renderer. so the code owns the shape and the producer only supplies values. officer/core:commander is `Commander: {title}` / `{body}`; the commander sends "Voice Memos", never the prefix. interpolation gains {title} and {body} alongside {n} and {id}, for the same reason the others exist. apns-payload and fcm now send the RENDERED text. that was the actual delivery bug: the catalogue could say whatever it liked and neither transport read it. a placeholder that resolves to nothing takes its punctuation with it, or `Commander: {title}` with no title ships `Commander: ` and every template has to assume its values are always present. Co-Authored-By: Claude Opus 5 Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043 --- src/servers/api/command-hq/directory.ts | 9 ++++-- src/servers/notifications/catalogue.ts | 7 ++++- src/servers/sidecar/notify/apns-payload.ts | 16 ++++++++-- src/servers/sidecar/notify/apns.test.ts | 24 +++++++++++++-- src/servers/sidecar/notify/fcm.test.ts | 13 ++++++-- src/servers/sidecar/notify/fcm.ts | 8 ++++- src/servers/sidecar/notify/render.ts | 35 +++++++++++++++------- 7 files changed, 91 insertions(+), 21 deletions(-) diff --git a/src/servers/api/command-hq/directory.ts b/src/servers/api/command-hq/directory.ts index f24ef3e2..91507a35 100644 --- a/src/servers/api/command-hq/directory.ts +++ b/src/servers/api/command-hq/directory.ts @@ -76,8 +76,11 @@ export function credentialSection(credentialPath: string | null): string[] { '', '### Telling the owner something', '', - "A push to their devices, in your own words. `id` is this directory's name, which groups your", - 'notifications together and is what the deeplink opens:', + 'A push to their devices, in your own words.', + '', + '`title` is YOUR NAME and nothing else — the platform renders it as `Commander: `, so never', + "write the prefix yourself. `body` is the sentence. `id` is this directory's name, which groups your", + 'notifications together and is what the deeplink opens.', '', '```', 'curl -X POST "$OFFICER_API_URL/api/notify/_officer/notify" \\', @@ -86,6 +89,8 @@ export function credentialSection(credentialPath: string | null): string[] { ' -d \'{"code":"officer/core:commander","id":"1","title":"Voice Memos","body":"3 memos filed"}\'', '```', '', + 'That arrives as **Commander: Voice Memos** / *3 memos filed*.', + '', 'Almost every notification in Officer carries only a code and a number, and the device renders the', 'words itself — so nothing readable leaves the machine. This one is the exception: `title` and `body`', 'are yours to write, and they go through Apple and Google to reach the lock screen.', diff --git a/src/servers/notifications/catalogue.ts b/src/servers/notifications/catalogue.ts index c93328fc..4dded5eb 100644 --- a/src/servers/notifications/catalogue.ts +++ b/src/servers/notifications/catalogue.ts @@ -138,7 +138,12 @@ const CORE_ENTRIES: Record = { than piling every commander into one stack. */ 'officer/core:commander': { - title: 'Commander', + // `{title}` is the commander's NAME and `{body}` its sentence — the commander supplies both and this + // decides what they look like. It sends "Voice Memos", never "Commander: Voice Memos": the prefix is + // the platform's to add, so every commander reads the same on the lock screen whether or not it + // remembered to. A commander that sends no title of its own falls through to the bare word. + title: 'Commander: {title}', + body: '{body}', target: 'mobile', route: 'officer://command-hq/{id}', thread: 'commander:{id}', diff --git a/src/servers/sidecar/notify/apns-payload.ts b/src/servers/sidecar/notify/apns-payload.ts index bfd5b06c..3ed019b4 100644 --- a/src/servers/sidecar/notify/apns-payload.ts +++ b/src/servers/sidecar/notify/apns-payload.ts @@ -1,6 +1,6 @@ import type { Catalogue } from '../../notifications/catalogue'; import type { Notification } from './types'; -import { FALLBACK_TITLE, threadFor } from './render'; +import { FALLBACK_TITLE, renderNotification, threadFor } from './render'; // The APNs payload. // @@ -41,6 +41,18 @@ export type ApnsPayload = { }; }; +/** + * The producer's words, in the shape its code declares. + * + * RENDERED, not raw. The catalogue owns presentation — `officer/core:commander` is `Commander: {title}` — + * so sending `n.title` straight through would put whatever the producer happened to write on the lock + * screen and leave "it should always say Commander" to whether it remembered. + */ +function alertFor(n: Notification, catalogue: Catalogue): { title: string; body?: string } { + const { title, body } = renderNotification(n, catalogue); + return { title, ...(body ? { body } : {}) }; +} + export function buildApnsPayload(n: Notification, catalogue: Catalogue): ApnsPayload { return { aps: { @@ -52,7 +64,7 @@ export function buildApnsPayload(n: Notification, catalogue: Catalogue): ApnsPay one sentence about one event that only the producer could write, and the alternative is waking a phone to say "1 item". The producer decides what belongs in it — see `types.ts`. */ - alert: n.title ? { title: n.title, ...(n.body ? { body: n.body } : {}) } : { title: FALLBACK_TITLE }, + alert: n.title ? alertFor(n, catalogue) : { title: FALLBACK_TITLE }, sound: 'default', 'mutable-content': 1, ...(typeof n.n === 'number' ? { badge: n.n } : {}), diff --git a/src/servers/sidecar/notify/apns.test.ts b/src/servers/sidecar/notify/apns.test.ts index 76e6e69b..ab2ede51 100644 --- a/src/servers/sidecar/notify/apns.test.ts +++ b/src/servers/sidecar/notify/apns.test.ts @@ -105,12 +105,30 @@ describe('APNs payload', () => { from and "1 item" is not worth waking a phone for. The owner's decision that this text may go through Apple; the judgement about WHICH text lives at the producer, not here. */ - test("a producer's own words replace the generic", () => { + test("a producer's own words replace the generic, in the shape its code declares", () => { + // The catalogue owns presentation. The commander sends its NAME, not the prefix — so every commander + // reads the same on the lock screen whether or not it remembered to write one. + const withShape: Catalogue = { + ...catalogue, + entries: { ...catalogue.entries, 'officer/core:commander': { title: 'Commander: {title}', body: '{body}' } }, + }; const p = buildApnsPayload( { code: 'officer/core:commander', userId: 1, id: 12, title: 'Voice Memos', body: '3 memos filed' }, - catalogue, + withShape, ); - expect(p.aps.alert).toEqual({ title: 'Voice Memos', body: '3 memos filed' }); + expect(p.aps.alert).toEqual({ title: 'Commander: Voice Memos', body: '3 memos filed' }); + }); + + // A template written with a separator must be safe when its value is missing, or every template has to + // assume its values are always present. + test('a placeholder that resolves to nothing takes its punctuation with it', () => { + const withShape: Catalogue = { + ...catalogue, + entries: { ...catalogue.entries, 'officer/core:commander': { title: 'Commander: {title}' } }, + }; + const p = buildApnsPayload({ code: 'officer/core:commander', userId: 1, title: 'x' }, withShape); + expect(p.aps.alert.title).toBe('Commander: x'); + expect(renderNotification({ code: 'officer/core:commander', userId: 1 }, withShape).title).toBe('Commander'); }); // `title` is the opt-in, and the smuggle guard above depends on that being true: it passes a stray diff --git a/src/servers/sidecar/notify/fcm.test.ts b/src/servers/sidecar/notify/fcm.test.ts index acef9479..3f2de3de 100644 --- a/src/servers/sidecar/notify/fcm.test.ts +++ b/src/servers/sidecar/notify/fcm.test.ts @@ -127,8 +127,17 @@ describe('FCM message', () => { }); test("a producer's own words get a notification block, or Android shows nothing", () => { - const m = build({ code: 'officer/core:commander', userId: 1, id: 12, title: 'Voice Memos', body: '3 filed' }); - expect(m.message.notification).toEqual({ title: 'Voice Memos', body: '3 filed' }); + const shaped: Catalogue = { + version: 7, + entries: { 'officer/core:commander': { title: 'Commander: {title}', body: '{body}' } }, + }; + const m = __testing.buildMessage( + 'tok', + { code: 'officer/core:commander', userId: 1, id: 12, title: 'Voice Memos', body: '3 filed' }, + { catalogue: shaped }, + ); + // Rendered, not raw — same rule as APNs: the code owns the shape. + expect(m.message.notification).toEqual({ title: 'Commander: Voice Memos', body: '3 filed' }); }); test('a body with no title stays off the wire', () => { diff --git a/src/servers/sidecar/notify/fcm.ts b/src/servers/sidecar/notify/fcm.ts index 0f03437f..caf046d8 100644 --- a/src/servers/sidecar/notify/fcm.ts +++ b/src/servers/sidecar/notify/fcm.ts @@ -2,6 +2,7 @@ import { getSecret } from 'officerdb/secret-store'; import { getPushDevices, deletePushDevice, recordPushFailure, markPushDeviceSeen } from 'officerdb'; import type { Channel, DeliveryResult, Notification, SendContext } from './types'; import { FcmAccessToken, parseServiceAccount } from './fcm-auth'; +import { renderNotification } from './render'; // The Google channel. FCM HTTP v1, called directly — no Expo, no firebase-admin. // @@ -89,7 +90,12 @@ function buildMessage(token: string, n: Notification, ctx: SendContext): FcmMess But a producer's own text has no catalogue entry to build from, so without a `notification` block Android would receive it and show nothing. See `types.ts` for why that text is allowed to travel. */ - const notification = n.title ? { title: n.title, ...(n.body ? { body: n.body } : {}) } : undefined; + // Rendered against the catalogue, for the reason `apns-payload.ts` gives: the code owns the shape, the + // producer only supplies the values that go in it. + const rendered = n.title ? renderNotification(n, ctx.catalogue) : null; + const notification = rendered + ? { title: rendered.title, ...(rendered.body ? { body: rendered.body } : {}) } + : undefined; return { message: { token, data, ...(notification ? { notification } : {}), android: { priority: 'high' } } }; } diff --git a/src/servers/sidecar/notify/render.ts b/src/servers/sidecar/notify/render.ts index 42876150..ba654dbc 100644 --- a/src/servers/sidecar/notify/render.ts +++ b/src/servers/sidecar/notify/render.ts @@ -16,11 +16,26 @@ import type { Notification } from './types'; /** The generic shown when a device cannot render for itself. Never a decoy sentence — see the doc. */ export const FALLBACK_TITLE = 'Officer'; +/* + `{n}` and `{id}` are the numbers a producer sends. `{title}` and `{body}` are its own words, and they + interpolate for the same reason the others do: so the catalogue decides the SHAPE and the producer only + supplies values. + + That distinction is the whole answer to "where is the structure defined". A commander sends its name and + a sentence; the catalogue is what turns those into `Commander Voice Memos`. Without this the producer's + title reached the lock screen untouched, and "it should always say Commander" was left to whether the + commander remembered — which is not a structure, it is a hope. +*/ const interpolate = (template: string, n: Notification): string => - template.replace(/\{(n|id)\}/g, (_, key: 'n' | 'id') => { - const value = key === 'n' ? n.n : n.id; - return value === undefined ? '' : String(value); - }); + template + .replace(/\{(n|id|title|body)\}/g, (_, key: 'n' | 'id' | 'title' | 'body') => { + const value = n[key]; + return value === undefined ? '' : String(value); + }) + // A placeholder that resolved to nothing leaves the punctuation that introduced it — `Commander: {title}` + // with no title renders `Commander: `. Trimming a dangling separator is what makes a template safe to + // write with one, rather than every template having to assume its values are always present. + .replace(/[\s:\u2014-]+$/, ''); export type Rendered = { title: string; body?: string; thread: string }; @@ -33,12 +48,12 @@ export type Rendered = { title: string; body?: string; thread: string }; export function renderNotification(n: Notification, catalogue: Catalogue): Rendered { const entry = catalogue.entries[n.code]; - // The producer's own words win. Only it knows what happened — a commander's whole output is a sentence - // the catalogue could not have held. Falls back to the code's template for everything else, which is - // almost everything. - if (n.title) return { title: n.title, ...(n.body ? { body: n.body } : {}), thread: threadFor(n, catalogue) }; - - if (!entry) return { title: FALLBACK_TITLE, thread: n.code }; + // A code the catalogue does not know, but with words of its own: send them as they came. There is no + // shape to impose and dropping a real event over a missing mapping would be worse. + if (!entry) { + if (n.title) return { title: n.title, ...(n.body ? { body: n.body } : {}), thread: n.code }; + return { title: FALLBACK_TITLE, thread: n.code }; + } const body = entry.body ? interpolate(entry.body, n) : undefined; return {