Skip to content

feat(push): report push capability so clients stop offering a feature that cannot work - #521

Open
RonenMars wants to merge 2 commits into
mainfrom
feat/push-capability-surface
Open

feat(push): report push capability so clients stop offering a feature that cannot work#521
RonenMars wants to merge 2 commits into
mainfrom
feat/push-capability-surface

Conversation

@RonenMars

Copy link
Copy Markdown
Owner

Closes #519.

The server decides at boot whether Live Activity push can work and logs it (live_activity.enabled / live_activity.disabled), but that decision never reached a client. So mobile offered a feature that silently no-ops for every user who is not the maintainer — the APNs .p8 is a server-side maintainer-owned signing secret bound to the app's bundle id.

A feature that is absent is fine. A feature that is present, enabled in the UI, and silently does nothing is not.

What changed

GET /api/info and GET /api/push/health both carry an additive push object:

push: {
  liveActivity: boolean          // can this server send an ActivityKit push right now
  notifications: boolean         // ordinary push. always false today
  liveActivityReason?: string    // present only when liveActivity is false
}

liveActivity comes from the server's own wiring state (liveActivityNotifier !== null) rather than a re-read of the environment, because credentials alone do not enable it — the sender is only built when the push token store opened too.

notifications is a hardcoded false, reported rather than omitted precisely so a client cannot infer ordinary push works because Live Activity push is configured: expo-server-sdk is not a dependency and PushRepository.listDeliverable() has no caller.

liveActivityReason names the missing environment variable, never its value — one test locks that down. The half-configured case (APNS_KEY set, team/bundle missing) is the hardest to diagnose, so it names them explicitly.

available was deliberately NOT changed

available on /api/push/health reports whether the SQLite token store opened, not whether credentials exist — which reads like the bug. It was left alone because the released app renders it verbatim:

tb-mobile/app/notification-health.tsx maps it to "Push store is available / unavailable (registration cannot persist)".

Retargeting it at credentials would make every non-maintainer server tell its user that registrations cannot persist — false, since the repository is open and register() works — and would point debugging at the database instead of the missing .p8. parsePushHealthResponse only type-checks it as a boolean, so the wrong sentence would render with no parse error. That trades one lie for another.

The new push object sits alongside it. On /api/push/health the pairing of "available": true with "liveActivity": false is the honest statement of the situation.

Backward compatibility

Purely additive. parsePushHealthResponse builds its result explicitly and ignores unknown keys, so released clients are byte-for-byte unaffected. A client that does not know push must read its absence as unknown, not unavailable.

Needs a paired tb-mobile issue: mobile should hide or disable the Live Activity affordance when push.liveActivity is false rather than registering tokens nothing will ever send to.

Verification

npm run lint exits 0. 67 push tests and 171 server/capabilities/http-log tests pass.

Positive control: forcing the wiring to a constant in each direction turned exactly the opposite two HTTP tests red (liveActivityPushEnabled: () => true → both "without credentials" cases; () => false → both "with credentials" cases). The tests read the server's real state, not a constant.

Note

docs/compatibility/tb-mobile.md did not list the push endpoints at all, despite mobile having shipped consumers for both. Closed in this PR.

RonenMars added a commit that referenced this pull request Aug 11, 2026
Closes #528.

The streamer could send no ordinary notification at all.
Mobile registered an Expo token, `POST /api/push/register` stored it,
and nothing consumed it except `LiveActivitySender` — so the one moment
the away-from-desk workflow depends on, the agent finishing its turn,
went unserved.

## Why Expo rather than the existing APNs path

`ApnsClient` signs with an APNs `.p8` for
`${bundleId}.push-type.liveactivity`, and Apple issues those keys per
developer team.
A self-hosted streamer's key can never sign for the published app's
bundle id, so anything built on that path works for the maintainer and
nobody else.

Expo already holds the app's APNs and FCM credentials and mobile already
registers an Expo token, so a plain batched POST to
`https://exp.host/--/api/v2/push/send` needs no Apple credential and
covers iOS and Android in one path.
`ApnsClient` and the Live Activity path are untouched; this is additive.

## What it does

`ExpoPushSender` fans out over `PushRepository.listDeliverable()`, which
is already Expo-only, so an ActivityKit token can never reach the relay.
Expo returns one ticket per token in a single response, so sends are
independent by construction — a dead device is recorded against its own
row and never silences the others.
A `DeviceNotRegistered` ticket revokes the token, mirroring how
`LiveActivitySender` expires a dead APNs one; every other ticket error
only counts toward the existing failure streak.

`WaitingInputNotifier` hooks the same `onStatusChange` funnel
`LiveActivityNotifier` uses rather than adding a detector, so all three
`markReady` call sites and the Codex runner are covered by one hook.
A turn opens on `waiting_input → running` and the notification fires on
the matching `running → waiting_input`, so boot and resume ready notify
nothing — you are never told about the session you just started.

## Three things worth a reviewer's attention

**The Expo enhanced-security footgun.**
Sends are unauthenticated by default, with
`THREADBASE_EXPO_ACCESS_TOKEN` as an opt-in `Authorization: Bearer`.
Requiring a token would lock out self-hosters, who do not own the Expo
project and cannot obtain one — and for the same reason, **enabling
enhanced security in the Expo project dashboard would break every
self-hosted streamer at once**.
That is a switch in a web UI with no representation in this repo, which
is why it is written down here and in the guide.

**The suppression trade-off.**
No push goes out while any live WebSocket client is subscribed to that
session, which is the available signal for "the user is already looking"
— mobile subscribes when the session screen opens and the socket dies on
backgrounding.
It errs toward silence in one case: a desktop client left subscribed to
a session suppresses the phone's notification too.
The alternative needs per-device foreground state that mobile does not
report today.

**The payload is a privacy decision.**
`title: <projectName>`, `body: "Waiting for your input"`, `data: {
sessionId, serverId }` — the two ids are exactly what mobile's
`sessionRouteFromNotificationData` needs to route a tap, so tapping
opens the session with no tb-mobile change.
Deliberately absent are `lastOutput` (raw PTY output, so any session
printing a token would put it in a notification) and `sessionName`
(derived from the user's first message).
Carrying both is the divergence RonenMars/threadbase-mobile#636 is open
about on the Live Activity path, and this must not repeat it.
The test seeds the fixture with a fake secret in `lastOutput` and a
prompt-derived `sessionName`, then asserts the serialised request
contains neither — a leak test rather than a shape test, so widening the
payload later fails CI rather than a policy review.

## Overlap with #521

That PR reports `notifications: false` as a hardcoded capability,
reasoned from "`listDeliverable()` has no caller".
This PR gives it one, so whichever of the two lands second has to flip
that field — it is no longer true that ordinary push cannot work.
Cross-linked rather than merged into one change, since the capability
surface and the sender are independently reviewable.

## Verification

`npm run lint`, `npm run build`, and the full vitest suite were run on
this branch.
The 15 new tests cover send-on-ready, no double-send within a turn, no
send at boot ready, suppression while watched, dead-token eviction, one
token failing without blocking the rest, and the payload leak
assertions.
Each negative assertion was mutation-checked: disabling suppression,
removing the per-turn guard, adding `lastOutput` to the payload, and
removing the eviction each turn the relevant tests red.

The nine files failing locally (`server.close()` hook timeouts in
`cors-middleware`, `discovery-cache`, `pair-endpoints`,
`security-hardening`, `watch-for-jsonl`, `webhook-update`, `codex-*`)
fail identically with this change stashed, at the same durations, and
are the dirty-box artifact documented in
`docs/testing/cross-platform-ci.md`.
The HTTP block asserted Live Activity capability from APNs credentials alone, which was true when the test was written and stopped being true once the feature moved behind the liveActivityPush flag.
It also never cleared THREADBASE_FEATURE_LIVE_ACTIVITY_PUSH, which outranks the config rung, so an exported var in a developer's shell decided the result instead of the test.
State the flag per case and clear the env var at boot; each block's afterEach already restores the whole environment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

P0: the server never tells clients whether Live Activity push can work, so mobile offers a feature that silently no-ops

1 participant