chat notifications reach the phone, on the same rule as the bell
the browser bell and phone push are now two subscribers to one signal. both
ask shouldNotify(); neither owns the rule.
deliberately a second subscriber rather than a branch inside notify-delivery.
the two channels share only the question "does this account want to hear about
this" — the browser needs an open tab and sends the chat's real title over a
loopback socket on the owner's own tailnet, the phone needs no tab and must
send NO title, because that hop goes through apple. one function doing both
means an `if (isPush)` at every step, and the first thing to go wrong is a
title leaking because the branch was missed in one place.
two codes, officer/core:chat.turn-ended and chat.input-needed. the route is
written out with its scheme rather than derived from the target: the hub has
no chat screen — chat is Butler, a sibling app — so these deep-link OUT over
officer-chat://, and two of the suite's twenty-one schemes already break the
officer-<app> pattern (officer-videos, officer-readaloud). deriving that works
until it silently does not.
`thread` interpolates now, so `chat:{id}` groups per conversation. a static
thread forces a choice between "three pings from one chat collapse" and "two
busy chats stay apart", and the wrong answer is that every chat is the same
chat.
dropped officer/core:agent: chat.turn-ended supersedes it and its route
pointed at officer://chat/{id}, a hub screen that does not exist.
the banner cannot name which conversation. a chat's title is content and
content does not travel; it says what happened, and tapping opens the chat,
which is where the name lives.
This commit is contained in:
+17
-1
@@ -101,6 +101,21 @@ index form to escape to — a primary key must be a constraint. `music_now_playi
|
||||
re-adds `pk_music_now_playing` on every push. Two statements, silent, no prompt even with rows in the
|
||||
table, data unaffected. Naming it explicitly does not help. Leave it.
|
||||
|
||||
**Four foreign keys are over 63 characters and churn the same way**, found 2026-08-19 while pushing
|
||||
`chat_notification_prefs`. These are the case the paragraph above says to name explicitly, and none of
|
||||
them is:
|
||||
|
||||
command_hq_session_tags_session_row_id_command_hq_sessions_id_fk 64
|
||||
command_hq_watcher_fires_session_row_id_command_hq_sessions_id_fk 65
|
||||
bitcoin_wallet_frozen_utxos_wallet_id_bitcoin_wallet_wallets_id_fk 66
|
||||
bitcoin_wallet_chain_cache_wallet_id_bitcoin_wallet_wallets_id_fk 65
|
||||
|
||||
The tell is in the plan itself: the `DROP` names them at 63 characters and the `ADD` at their full
|
||||
length, which is Postgres's truncation and drizzle's copy disagreeing. Eight statements per push, no
|
||||
prompt, data unaffected — deferred rather than fixed (see `TODO.md`), because it is noise rather than
|
||||
a fault. Two live in `officer_db/src/command-hq/schema.ts`; the other two are the bitcoin-wallet
|
||||
plugin's, in its own repository.
|
||||
|
||||
**Rules that still apply:**
|
||||
|
||||
1. **Never answer "Yes, truncate the table"** if a prompt ever does appear. It destroys rows and fixes
|
||||
@@ -111,7 +126,8 @@ table, data unaffected. Naming it explicitly does not help. Leave it.
|
||||
branch it takes at a truncate prompt.
|
||||
4. **Read the plan before applying it:**
|
||||
`bunx drizzle-kit push --config=drizzle.config.ts --verbose` prints every statement first. Expect the
|
||||
two `pk_music_now_playing` lines. Anything else is your change.
|
||||
two `pk_music_now_playing` lines and the eight from the four over-long foreign keys above — ten in
|
||||
all. Anything else is your change.
|
||||
|
||||
To test a schema change without touching `officer_dev`: `createdb officer_scratch`, then
|
||||
`POSTGRES_URL=postgresql://postgres:postgres@127.0.0.1:5432/officer_scratch bunx drizzle-kit push
|
||||
|
||||
@@ -18,6 +18,7 @@ import { isTokenBlacklisted, getUserById } from 'officerdb';
|
||||
import { terminalWebsocket } from './servers/api/terminal/websocket';
|
||||
import { chatWebsocket } from './servers/api/chat/websocket';
|
||||
import { chatNotifyWebsocket } from './servers/api/chat/notify-delivery';
|
||||
import { registerChatPushNotifications } from './servers/api/chat/notify-push';
|
||||
import { taskRunnerWebsocket } from './servers/api/tasks/task-executor';
|
||||
import { pipelineWebsocket } from './servers/api/tasks/pipeline-executor';
|
||||
// import { desktopWebsocket } from './servers/api/desktop/websocket';
|
||||
@@ -160,6 +161,11 @@ assertPermissionTotality({
|
||||
wsProviders: Object.keys(handlers),
|
||||
});
|
||||
|
||||
// Chat's phone channel. Subscribes to the same signal the browser bell uses and asks the same question;
|
||||
// it just answers it into APNs/FCM instead of a websocket. Registered here, and explicitly, because a
|
||||
// module whose only job is a side effect is one an import sweep quietly deletes.
|
||||
registerChatPushNotifications();
|
||||
|
||||
// That the working directory is the repo, because every path derives from its parent. First of the three,
|
||||
// since a wrong answer here makes the other two check the wrong files.
|
||||
assertInstallLayout();
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
import * as sidecar from '@@/sidecar-registry';
|
||||
import { getNotifyServerUrl } from '../notify/router';
|
||||
import { shouldNotify, type NotifyKind } from './notify-decision';
|
||||
|
||||
// The phone half of the bell: the same signal the browser socket listens to, delivered to APNs/FCM.
|
||||
//
|
||||
// ── Why this is a second subscriber and not a branch inside notify-delivery ──
|
||||
//
|
||||
// The two channels have nothing in common except the question "does this account want to hear about
|
||||
// this". That question lives in `notify-decision.ts` and both ask it; everything else differs. The
|
||||
// browser needs an open tab and sends the chat's real title over a loopback socket on the owner's own
|
||||
// tailnet. The phone needs no tab at all and must send NO title, because that hop goes through Apple.
|
||||
//
|
||||
// Folding both into one function would mean a `if (isPush)` inside every step of it, and the first thing
|
||||
// to go wrong would be a title leaking into a payload because the branch was missed in one place.
|
||||
|
||||
/** Which code a kind maps to. Both live in `notifications/catalogue.ts` as `officer/core:chat.*`. */
|
||||
const CODE_FOR: Record<NotifyKind, string> = {
|
||||
'turn-ended': 'officer/core:chat.turn-ended',
|
||||
'input-needed': 'officer/core:chat.input-needed',
|
||||
};
|
||||
|
||||
/**
|
||||
* Subscribe the push channel to chat's notify signal.
|
||||
*
|
||||
* Called explicitly from `server.tsx` rather than run on import: a module whose only purpose is a side
|
||||
* effect is one an unused-import sweep deletes, and the failure would be silent.
|
||||
*/
|
||||
export function registerChatPushNotifications(): void {
|
||||
sidecar.onClaudeNotify(async (sessionKey, userId, kind) => {
|
||||
try {
|
||||
if (!(await shouldNotify(userId, sessionKey, kind))) return;
|
||||
|
||||
const base = getNotifyServerUrl();
|
||||
// The sidecar has not reported in — either it is starting, or push is not running on this install.
|
||||
// Not an error: the browser bell still works, and this is the channel that is allowed to be absent.
|
||||
if (!base) return;
|
||||
|
||||
// `id` is the session key so the device can deep-link straight to the conversation. It is an
|
||||
// opaque uuid, which is the only reason it may travel: NO title, and no message content, because
|
||||
// this payload is read by Apple and Google. See docs/push-notifications.md.
|
||||
await fetch(`${base}/_officer/notify`, {
|
||||
method: 'POST',
|
||||
headers: { 'content-type': 'application/json', 'X-Officer-User': String(userId) },
|
||||
body: JSON.stringify({ code: CODE_FOR[kind], userId, id: sessionKey }),
|
||||
signal: AbortSignal.timeout(10_000),
|
||||
});
|
||||
} catch (err) {
|
||||
// A notification that fails must never disturb the turn that produced it.
|
||||
console.error('[chat:push] failed to dispatch:', err instanceof Error ? err.message : err);
|
||||
}
|
||||
});
|
||||
}
|
||||
@@ -34,7 +34,13 @@ export type NotificationEntry = {
|
||||
target?: string;
|
||||
/** The deep link, `{id}` interpolated. */
|
||||
route?: string;
|
||||
/** Grouping key, so like-notifications collapse rather than stack. Defaults to the code's namespace. */
|
||||
/**
|
||||
* Grouping key, so like-notifications collapse rather than stack. Defaults to the code's namespace.
|
||||
*
|
||||
* A TEMPLATE, like the others — `chat:{id}` groups per conversation, so two busy chats do not collapse
|
||||
* into one banner while three pings from the SAME chat still do. A static thread would force a choice
|
||||
* between those, and the wrong one is "every chat is the same chat".
|
||||
*/
|
||||
thread?: string;
|
||||
};
|
||||
|
||||
@@ -58,6 +64,29 @@ export type Catalogue = {
|
||||
* on a machine that has never installed anything, because it is what first light is proved with.
|
||||
*/
|
||||
const CORE_ENTRIES: Record<string, NotificationEntry> = {
|
||||
// ── Chat ──
|
||||
//
|
||||
// The hub has no chat screen; chat is Butler, a sibling app. So these route OUT of the hub over
|
||||
// `officer-chat://`, and the scheme is written here rather than derived from the target: two of the
|
||||
// suite's twenty-one schemes already break the `officer-<app>` pattern (`officer-videos`,
|
||||
// `officer-readaloud`), so deriving it works until it silently does not.
|
||||
//
|
||||
// The banner cannot name WHICH conversation — a chat's title is content, and content does not travel.
|
||||
// It says what happened; tapping opens the chat, which is where the name lives.
|
||||
'officer/core:chat.turn-ended': {
|
||||
title: 'Chat',
|
||||
body: 'Finished its turn',
|
||||
target: 'chat',
|
||||
route: 'officer-chat://session/{id}',
|
||||
thread: 'chat:{id}',
|
||||
},
|
||||
'officer/core:chat.input-needed': {
|
||||
title: 'Chat',
|
||||
body: 'Needs your input',
|
||||
target: 'chat',
|
||||
route: 'officer-chat://session/{id}',
|
||||
thread: 'chat:{id}',
|
||||
},
|
||||
'officer/core:test': {
|
||||
title: 'Officer',
|
||||
body: 'Test notification',
|
||||
@@ -76,12 +105,6 @@ const CORE_ENTRIES: Record<string, NotificationEntry> = {
|
||||
route: 'officer://jobs/{id}',
|
||||
thread: 'officer/core:job',
|
||||
},
|
||||
'officer/core:agent': {
|
||||
title: 'Agent finished',
|
||||
target: 'mobile',
|
||||
route: 'officer://chat/{id}',
|
||||
thread: 'officer/core:agent',
|
||||
},
|
||||
};
|
||||
|
||||
/** FNV-1a, 32-bit. Small, stable across processes, and not a security boundary. */
|
||||
|
||||
@@ -38,11 +38,12 @@ export function renderNotification(n: Notification, catalogue: Catalogue): Rende
|
||||
return {
|
||||
title: interpolate(entry.title, n),
|
||||
...(body ? { body } : {}),
|
||||
thread: entry.thread ?? n.code,
|
||||
thread: interpolate(entry.thread ?? n.code, n),
|
||||
};
|
||||
}
|
||||
|
||||
/** The grouping key for a code, whether or not the catalogue knows it. */
|
||||
export function threadFor(n: Notification, catalogue: Catalogue): string {
|
||||
return catalogue.entries[n.code]?.thread ?? n.code;
|
||||
// Interpolated, so `chat:{id}` groups per conversation rather than lumping every chat together.
|
||||
return interpolate(catalogue.entries[n.code]?.thread ?? n.code, n);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user