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 {