From 478ddb13e9787ab0b4a44e3ca11ac8e54b555d94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Mon, 24 Aug 2026 15:13:39 +0000 Subject: [PATCH] a commander can say something in its own words MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit asked whether the voice-memos commander could send a notification, the answer was no, three times over. the platform skill gives it the prefix /notify and not the route. the route wants a catalogue code, not a title and a message. and of the six codes that exist, none means "a commander has something to tell you" — the nearest renders "Test notification". so: officer/core:commander, one entry rather than one per commander, with the commander travelling as {id}. arming a new one needs no catalogue change and no app resync. and `title`/`body` on the notification itself, which override the catalogue. this is a deliberate exception to the rule the whole design rests on — content does not travel — and it is the owner's decision, made 2026-08-24. a commander's output is one sentence only it could write; there is no entry to render it from, and "1 item" is not worth waking a phone for. sendoff.landed already carries the same relaxation for the same kind of reason. the control sits at the PRODUCER, not the transport. the commander's CLAUDE.md now says what may go in those two fields and what may never — and that a notification is an interruption, so send few. this file cannot judge what is sensitive and does not try. `title` is the opt-in, not `body`. the existing smuggle test passes a stray `body` and asserts it does not reach the wire; a body that travelled on its own would have silently defeated it. pinned on both transports. FCM gets a `notification` block only for producer text — data-only is what lets the app compose from its catalogue, and text with no entry to compose from would arrive and show nothing. worth recording: there is NO Notification Service Extension, in this repo or the mobile one. mutable-content: 1 is set for something never built, so aps.alert is what the phone actually shows and this works with no app release. i said the opposite an hour ago, from the comments rather than from looking. Co-Authored-By: Claude Opus 5 Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043 --- src/servers/api/command-hq/directory.ts | 25 +++++++++++++++ src/servers/notifications/catalogue.ts | 20 ++++++++++++ src/servers/sidecar/notify/apns-payload.ts | 22 +++++++++---- src/servers/sidecar/notify/apns.test.ts | 31 ++++++++++++++++++ src/servers/sidecar/notify/fcm.test.ts | 37 ++++++++++++++++++++++ src/servers/sidecar/notify/fcm.ts | 27 ++++++++++++---- src/servers/sidecar/notify/render.ts | 6 ++++ src/servers/sidecar/notify/types.ts | 29 +++++++++++++++-- 8 files changed, 183 insertions(+), 14 deletions(-) diff --git a/src/servers/api/command-hq/directory.ts b/src/servers/api/command-hq/directory.ts index 03f75e5c..f24ef3e2 100644 --- a/src/servers/api/command-hq/directory.ts +++ b/src/servers/api/command-hq/directory.ts @@ -74,6 +74,31 @@ export function credentialSection(credentialPath: string | null): string[] { 'A 403 is an answer, not a problem to route around. It means this commander was not given that, and', 'the owner is the one who decides otherwise. Say so rather than finding another way in.', '', + '### 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:', + '', + '```', + 'curl -X POST "$OFFICER_API_URL/api/notify/_officer/notify" \\', + ' -H "Authorization: Bearer $OFFICER_API_KEY" -H "Host: $OFFICER_API_HOST" \\', + ' -H "content-type: application/json" \\', + ' -d \'{"code":"officer/core:commander","id":"1","title":"Voice Memos","body":"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.', + '', + '**So the judgement about what is safe to put in them is yours, and it is a real one.** A count, a', + 'name you chose, what you did — fine. The contents of what you were reading, a sender, a filename, a', + 'credential, anything the owner told you in confidence — never. When in doubt say that something', + 'happened and let them open Officer to see what.', + '', + 'Send few. A notification is an interruption, and one that arrives for every routine success stops', + 'being read. Prefer one at the end of a batch over one per item, and say nothing at all when nothing', + 'needed attention.', + '', ]; } diff --git a/src/servers/notifications/catalogue.ts b/src/servers/notifications/catalogue.ts index 3aed15bc..c93328fc 100644 --- a/src/servers/notifications/catalogue.ts +++ b/src/servers/notifications/catalogue.ts @@ -123,6 +123,26 @@ const CORE_ENTRIES: Record = { route: 'officer://jobs/{id}', thread: 'officer/core:job', }, + /* + A commander with something to say. + + ONE entry rather than one per commander, and the commander travels as `{id}` — so arming a new + commander needs no catalogue change and no app resync. `{id}` is the commander's row id, which is what + `officer://command-hq/{id}` opens. + + This entry is what a commander sees when it sends nothing but a code and an id: "Voice Memos" would be + a lie without knowing which one, so the generic is honest and the deeplink does the rest. In practice a + commander supplies its own `title` and `body`, which override this — that is the exception described in + `sidecar/notify/types.ts`, and the whole reason a commander can notify usefully at all. The entry still + matters: it is the fallback when it doesn't, and it is what makes the thread group per commander rather + than piling every commander into one stack. + */ + 'officer/core:commander': { + title: 'Commander', + target: 'mobile', + route: 'officer://command-hq/{id}', + thread: 'commander:{id}', + }, }; /** FNV-1a, 32-bit. Small, stable across processes, and not a security boundary. */ diff --git a/src/servers/sidecar/notify/apns-payload.ts b/src/servers/sidecar/notify/apns-payload.ts index 17e627f3..bfd5b06c 100644 --- a/src/servers/sidecar/notify/apns-payload.ts +++ b/src/servers/sidecar/notify/apns-payload.ts @@ -8,7 +8,8 @@ import { FALLBACK_TITLE, threadFor } from './render'; // arrives in the app untouched. The split is the whole design: // // - `aps.alert` is what iOS draws if nothing else happens. It is a fixed generic, because it is the one -// part Apple can read. +// part Apple can read — UNLESS the producer supplied its own words, which is the deliberate +// exception described in `types.ts`. // - `officer` carries the code, which the Notification Service Extension resolves against its synced // catalogue and uses to REWRITE the banner before it is shown. // @@ -19,7 +20,7 @@ import { FALLBACK_TITLE, threadFor } from './render'; export type ApnsPayload = { aps: { - alert: { title: string }; + alert: { title: string; body?: string }; sound: string; /** Lets the extension replace the banner. Without it, `alert` is final. */ 'mutable-content': 1; @@ -28,11 +29,13 @@ export type ApnsPayload = { /** Lets the app group/replace banners rather than stacking one per event. */ 'thread-id'?: string; }; - /** Ours. Ids and integers, never content. */ + /** Ours. Ids and integers, plus a route when the destination varies per notification. */ officer: { c: string; n?: number; id?: string | number; + /** Where tapping goes, when the catalogue's own route cannot express it. */ + r?: string; /** Catalogue version this was rendered against; the device resyncs when it differs from its own. */ mv: number; }; @@ -41,9 +44,15 @@ export type ApnsPayload = { export function buildApnsPayload(n: Notification, catalogue: Catalogue): ApnsPayload { return { aps: { - // Deliberately NOT the catalogue's title: sending the real text would hand Apple the category - // vocabulary in plaintext, which is exactly what the code exists to withhold. - alert: { title: FALLBACK_TITLE }, + /* + Deliberately NOT the catalogue's title: sending the real text would hand Apple the category + vocabulary in plaintext, which is exactly what the code exists to withhold. + + A producer's OWN title is different and does go here. It is not the catalogue's vocabulary, it is + 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 }, sound: 'default', 'mutable-content': 1, ...(typeof n.n === 'number' ? { badge: n.n } : {}), @@ -53,6 +62,7 @@ export function buildApnsPayload(n: Notification, catalogue: Catalogue): ApnsPay c: n.code, ...(n.n !== undefined ? { n: n.n } : {}), ...(n.id !== undefined ? { id: n.id } : {}), + ...(n.route ? { r: n.route } : {}), mv: catalogue.version, }, }; diff --git a/src/servers/sidecar/notify/apns.test.ts b/src/servers/sidecar/notify/apns.test.ts index 45705cc6..76e6e69b 100644 --- a/src/servers/sidecar/notify/apns.test.ts +++ b/src/servers/sidecar/notify/apns.test.ts @@ -98,6 +98,37 @@ describe('APNs payload', () => { expect(json).not.toContain('please pay'); }); + /* + Producer-supplied text — the deliberate exception, added 2026-08-24 for commanders. + + A commander's output is one sentence only it could write, so there is no catalogue entry to render + 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", () => { + const p = buildApnsPayload( + { code: 'officer/core:commander', userId: 1, id: 12, title: 'Voice Memos', body: '3 memos filed' }, + catalogue, + ); + expect(p.aps.alert).toEqual({ title: 'Voice Memos', body: '3 memos filed' }); + }); + + // `title` is the opt-in, and the smuggle guard above depends on that being true: it passes a stray + // `body` and asserts it does not travel. A body that could travel on its own would silently defeat it. + test('a body with no title does not travel', () => { + const p = buildApnsPayload({ code: 'officer/core:mail', userId: 1, body: 'please pay' } as Notification, catalogue); + expect(p.aps.alert.title).toBe(FALLBACK_TITLE); + expect(JSON.stringify(p)).not.toContain('please pay'); + }); + + test('a per-notification route rides as `r`, for a destination the catalogue cannot express', () => { + const p = buildApnsPayload( + { code: 'officer/core:commander', userId: 1, id: 12, route: 'officer://command-hq/12?files=1' }, + catalogue, + ); + expect(p.officer.r).toBe('officer://command-hq/12?files=1'); + }); + test('an unknown code still sends, and threads by itself', () => { // The mapping has not caught up. The event is real, so it goes — the device renders its own // unknown-code fallback rather than the user losing the notification. diff --git a/src/servers/sidecar/notify/fcm.test.ts b/src/servers/sidecar/notify/fcm.test.ts index 4f5d325e..acef9479 100644 --- a/src/servers/sidecar/notify/fcm.test.ts +++ b/src/servers/sidecar/notify/fcm.test.ts @@ -105,3 +105,40 @@ describe('FCM message', () => { expect(json).not.toContain('example.com'); }); }); + +/* + Data-only is the design, and the one exception to it. + + Android composes the banner from its synced catalogue, which is why there is normally no `notification` + block. Producer text has no entry to compose from — without a block the message arrives and nothing is + shown, which is worse than not sending it. +*/ +describe('FCM message', () => { + const catalogue: Catalogue = { + version: 7, + entries: { 'officer/core:mail': { title: '{n} new emails', thread: 'officer/core:mail' } }, + }; + const build = (n: Notification) => __testing.buildMessage('tok', n, { catalogue }); + + test('a coded notification is data-only', () => { + const m = build({ code: 'officer/core:mail', userId: 1, n: 3 }); + expect(m.message.notification).toBeUndefined(); + expect(m.message.data).toEqual({ c: 'officer/core:mail', mv: '7', n: '3' }); + }); + + 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' }); + }); + + test('a body with no title stays off the wire', () => { + const m = build({ code: 'officer/core:mail', userId: 1, body: 'please pay' } as Notification); + expect(m.message.notification).toBeUndefined(); + expect(JSON.stringify(m)).not.toContain('please pay'); + }); + + test('a route rides in the data map', () => { + const m = build({ code: 'officer/core:commander', userId: 1, route: 'officer://command-hq/12' }); + expect(m.message.data.r).toBe('officer://command-hq/12'); + }); +}); diff --git a/src/servers/sidecar/notify/fcm.ts b/src/servers/sidecar/notify/fcm.ts index 2e95de63..0f03437f 100644 --- a/src/servers/sidecar/notify/fcm.ts +++ b/src/servers/sidecar/notify/fcm.ts @@ -53,17 +53,22 @@ type FcmMessage = { message: { token: string; /** - * DATA-ONLY — deliberately no `notification` block. + * DATA-ONLY, except for a producer that wrote its own words. * - * A `notification` block is rendered by the system, which would put the code on the lock screen and - * give the app no chance to resolve it. Data-only hands the message to the app, which composes the - * banner from its synced catalogue — the same thing the Notification Service Extension does on iOS, - * except Android needs no extension for it. + * A `notification` block is rendered by the system, which for a CODE would put the code itself on the + * lock screen and give the app no chance to resolve it. Data-only hands the message to the app, which + * composes the banner from its synced catalogue — the same thing the Notification Service Extension + * does on iOS, except Android needs no extension for it. + * + * Producer text has no catalogue entry to compose from, so it needs the block: without one the message + * arrives and nothing is shown. See `types.ts` for why that text is allowed to travel at all. * * The cost is that `priority: high` is load-bearing: a normal-priority data message can be deferred * indefinitely in Doze, and a notification that arrives tomorrow is not a notification. */ data: Record; + /** Only for producer-supplied text. Absent for every coded notification — see above. */ + notification?: { title: string; body?: string }; android: { priority: 'high'; }; @@ -75,8 +80,18 @@ function buildMessage(token: string, n: Notification, ctx: SendContext): FcmMess const data: Record = { c: n.code, mv: String(ctx.catalogue.version) }; if (n.id !== undefined) data.id = String(n.id); if (n.n !== undefined) data.n = String(n.n); + if (n.route) data.r = n.route; - return { message: { token, data, android: { priority: 'high' } } }; + /* + Data-only, except when the producer wrote its own words. + + Data-only is what lets the app build the banner from its synced catalogue, which is the whole design. + 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; + + return { message: { token, data, ...(notification ? { notification } : {}), android: { priority: 'high' } } }; } type SendOutcome = { ok: boolean; disposition?: Disposition; reason?: string }; diff --git a/src/servers/sidecar/notify/render.ts b/src/servers/sidecar/notify/render.ts index fd8fc516..42876150 100644 --- a/src/servers/sidecar/notify/render.ts +++ b/src/servers/sidecar/notify/render.ts @@ -32,6 +32,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 }; const body = entry.body ? interpolate(entry.body, n) : undefined; diff --git a/src/servers/sidecar/notify/types.ts b/src/servers/sidecar/notify/types.ts index 55f8134a..c15d88b9 100644 --- a/src/servers/sidecar/notify/types.ts +++ b/src/servers/sidecar/notify/types.ts @@ -6,8 +6,19 @@ import type { Catalogue } from '../../notifications/catalogue'; // (`src/servers/notifications/catalogue.ts`), which is synced to the device — so a new kind of // notification is a server change and never an app build. See docs/push-notifications.md. // -// Content does not travel. Apple and Google see the code and the numbers, never a subject line, a -// sender, a filename, an error string or a path. Anything readable is fetched over the tailnet by id. +// Content does not travel BY DEFAULT. Apple and Google see the code and the numbers, never a subject +// line, a sender, a filename, an error string or a path. Anything readable is fetched over the tailnet +// by id. +// +// A producer may override that per notification with `title`/`body`, and one kind of producer needs to: +// a commander, whose whole output is a sentence only it could write. "3 items" and a deeplink is not +// worth waking a phone for. The owner's decision, on 2026-08-24, and the same relaxation `sendoff.landed` +// already carries. +// +// What that buys and what it costs: the text is on the lock screen, and it went through Apple or Google +// to get there. The control is at the PRODUCER, not here — a commander is told what not to put in a +// message, which is where the judgement about what is sensitive actually lives. This file cannot make +// that call and does not try. export type Notification = { /** @@ -23,6 +34,20 @@ export type Notification = { n?: number; /** The thing this is about. The device fetches by it; never a human-readable string. */ id?: string | number; + /** + * Text the producer wrote, shown as-is instead of the catalogue's template. + * + * Absent for almost everything — the catalogue is the right answer whenever the words are known in + * advance, because then they never leave the tailnet. Present when only the producer knows what + * happened. See the note above for the trade. + */ + title?: string; + body?: string; + /** + * Where tapping it goes — `officer://command-hq/12`. Overrides the catalogue's `route`, which cannot + * express a destination that varies per notification beyond `{id}`. + */ + route?: string; }; /** What every channel is handed alongside the notification. */