diff --git a/CLAUDE.md b/CLAUDE.md index 9e68df69..fb38776f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -114,7 +114,7 @@ waiting_input / idle ──(idle reaper, 6h of agent silence)───► idle | `THREADBASE_SKIP_PERMISSION_MODE_PROMPT` | Set to `true` to disable the `serve` first-run interactive permission-mode prompt (see below); falls straight through to `acceptEdits`. | | `THREADBASE_SKIP_AUTO_RESUME_PROMPT` | Set to `true` to disable the `serve` first-run interactive auto-resume prompt (see [Auto-resume on boot](#auto-resume-on-boot)); resolves to `false` (no auto-resume). | | `THREADBASE_ALLOW_BROWSER_CORS` | Enables browser CORS (off by default; no web page can make authenticated requests without it). Set to `1`/`true`/`yes`/`on` to allow the localhost dev origins, or to a comma-separated origin list (e.g. `https://app.example.com`) to allow those on top of the dev defaults. Overrides `browser_cors:` in server.yaml when set. Mobile is unaffected (no `Origin` header). | -| `APNS_KEY` | **Contents** of the APNs p8 signing key (PEM), not a path. Enables iOS Live Activity push when set; the feature stays off (one info log, server boots normally) when unset. Never logged. See [docs/guides/live-activity-push.md](docs/guides/live-activity-push.md). | +| `APNS_KEY` | **Contents** of the APNs p8 signing key (PEM), not a path. Required for iOS Live Activity push but no longer sufficient on its own — the `liveActivityPush` feature flag must also be on. Either missing leaves the feature off with one info log and a normal boot. Never logged. See [docs/guides/live-activity-push.md](docs/guides/live-activity-push.md). | | `APNS_KEY_ID` | Key id of the p8 in `APNS_KEY`. Required when `APNS_KEY` is set; under launchd it is derived from the `AuthKey_.p8` filename. | | `APNS_TEAM_ID` | Apple Developer team id. Required when `APNS_KEY` is set; no default, so one deployment's Apple account is never baked into the source. | | `APNS_BUNDLE_ID` | App bundle id; the APNs topic is this plus `.push-type.liveactivity`. Required when `APNS_KEY` is set. | @@ -134,6 +134,8 @@ That transport choice is structural, not a preference. An APNs `.p8` signs only ## iOS Live Activity push +Gated by the `liveActivityPush` [feature flag](#feature-flags), **off by default** — `APNS_KEY` alone no longer brings this up, and a box with credentials configured logs `live_activity.disabled` at boot until the flag is set. + `APNS_KEY` enables direct-to-APNs Live Activity pushes (Lock Screen / Dynamic Island surfaces for running sessions). ActivityKit **cannot** go through Expo's relay — different token type, `.push-type.liveactivity` topic, p8 credential — so this path uses `node:http2` directly and never `expo-server-sdk`. Three token kinds now arrive from one device and are not interchangeable: `expo` (relay, ordinary notifications), `liveactivity_start` (push-to-start, app-wide), `liveactivity_update` (per-activity, short-lived). `PushRepository.listDeliverable()` is Expo-only — that query is what keeps the ordinary notification fan-out from handing an ActivityKit token to Expo. @@ -239,6 +241,7 @@ Current flags: |----|---------|-------| | `codexSystemPrompt` | off | Sending the built system prompt to fresh Codex sessions. Off because Codex has no `--system-prompt` flag — the prompt lands in the positional `[PROMPT]` argument, which Codex treats as the user's opening turn rather than a system instruction. | | `sessionRehydration` | **on** | Seeding the session list at boot from the durable registry, so sessions a previous run was interrupted mid-flight come back in `GET /api/sessions` as `ownership: "historical"` / `lifecycle: "resumable"` stubs instead of vanishing. On because it is the fix for the restart case, not an experiment — but it changes what `GET /api/sessions` contains for every client, so it ships with a kill switch rather than unconditionally. `GET /api/sessions/count` is unaffected: recovered stubs are filtered out of it. Turning this flag off does not disable `auto_resume_on_boot`; only `auto_resume_on_boot: false` prevents unattended starts. | +| `liveActivityPush` | off | Live Activity surfaces for running sessions, **both halves**. The streamer half gates `initLiveActivityPush()`, so an `APNS_KEY` that is present but unwanted is ignored rather than honoured — the boot log says so at `live_activity.disabled` instead of going quiet. tb-mobile reads this flag over `GET /api/config/feature-flags` and skips its own local ActivityKit path (and the Android ongoing-notification equivalent) when it is off, which is the point: the client half alone draws a Lock Screen card that only updates while the app is foregrounded, then freezes on backgrounding and expires silently after ~8h. A server too old to serve the endpoint reads as off — it is also too old to have been asked. Turn it on only where a push-to-start token is actually registered. | | `ptyHost` | off | Keeping live PTYs in a separate host process so a streamer restart can reconnect without restarting the agents. Startup replaces a host with an incompatible protocol, heartbeats keep the streamer lease current, and an empty host exits after its known-empty registry state and all leases expire. `tb-streamer prod doctor` reports host liveness, protocol version, and session count when the flag is enabled. Host-surviving sessions remain attached and replay from the preserved terminal screen; after a machine reboot the host is gone too, so registry rehydration remains the fallback. Windows smoke has observed a detached host preserving real ConPTY output after its launcher exits and reconnecting over a named pipe. The flag remains off by default because a real Claude or Codex `tb-streamer prod restart` through Task Scheduler is not exercised yet. | ## Dependencies diff --git a/__tests__/live-activity-flag.test.ts b/__tests__/live-activity-flag.test.ts new file mode 100644 index 00000000..49b9484c --- /dev/null +++ b/__tests__/live-activity-flag.test.ts @@ -0,0 +1,111 @@ +import { mkdtempSync, rmSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { findFeatureFlag } from "../src/feature-flags"; +import { StreamerServer } from "../src/server"; + +const h = vi.hoisted(() => ({ + calls: [] as Array<{ level: string; msg: string; fields: any }>, +})); + +vi.mock("../src/logger", () => { + const push = (level: string) => (msg: string, fields: any) => + h.calls.push({ level, msg, fields }); + const fake = { + debug: push("debug"), + info: push("info"), + warn: push("warn"), + error: push("error"), + log: () => {}, + pino: { isLevelEnabled: () => false }, + }; + return { getLogger: () => fake, logger: fake }; +}); + +const eventsOf = (event: string) => h.calls.filter((c) => c.fields?.event === event); + +// Complete, syntactically plausible, and entirely fake. ApnsClient's constructor +// is inert — it neither parses the PEM nor opens a session until the first send +// — so a bogus key is enough to make readApnsCredentialsFromEnv() succeed and +// drive initLiveActivityPush past the credential check to the flag check. +const FAKE_APNS = { + APNS_KEY: "-----BEGIN PRIVATE KEY-----\nnotarealkey\n-----END PRIVATE KEY-----\n", + APNS_KEY_ID: "FAKEKEY123", + APNS_TEAM_ID: "FAKETEAM99", + APNS_BUNDLE_ID: "com.example.threadbase", +}; + +let server: StreamerServer | null = null; +let cacheDir: string; +const savedEnv: Record = {}; + +async function bootWithFlag(liveActivityPush: boolean): Promise { + server = new StreamerServer({ + codexRoots: [], + scannerPersistent: false, + port: 0, + apiKey: "tb_test_key_for_live_activity_flag", + localNoAuth: false, + verbose: false, + disableDb: true, + cacheDir, + // The CLI rung. beforeEach clears the env var so it cannot outrank this. + featureFlags: { liveActivityPush }, + }); + await server.listen(0, { awaitReady: true }); +} + +beforeEach(() => { + h.calls.length = 0; + cacheDir = mkdtempSync(join(tmpdir(), "threadbase-la-flag-")); + for (const [k, v] of Object.entries(FAKE_APNS)) { + savedEnv[k] = process.env[k]; + process.env[k] = v; + } + // Env outranks the CLI rung, so a real THREADBASE_FEATURE_LIVE_ACTIVITY_PUSH + // in the developer's shell would otherwise silently decide both cases. + savedEnv.THREADBASE_FEATURE_LIVE_ACTIVITY_PUSH = + process.env.THREADBASE_FEATURE_LIVE_ACTIVITY_PUSH; + delete process.env.THREADBASE_FEATURE_LIVE_ACTIVITY_PUSH; +}); + +afterEach(async () => { + await server?.close(); + server = null; + for (const [k, v] of Object.entries(savedEnv)) { + if (v === undefined) delete process.env[k]; + else process.env[k] = v; + } + rmSync(cacheDir, { recursive: true, force: true }); +}); + +describe("liveActivityPush feature flag", () => { + it("keeps its env var name stable, and its default OFF", () => { + const flag = findFeatureFlag("liveActivityPush"); + expect(flag?.env).toBe("THREADBASE_FEATURE_LIVE_ACTIVITY_PUSH"); + expect(flag?.default).toBe(false); + }); + + // The positive control. Without it, the assertion below passes just as + // happily against a boot that failed for some unrelated reason — a bad + // cacheDir, a changed credential shape — and the flag would look load-bearing + // when nothing is reading it at all. + it("brings Live Activity push up when the flag is on and credentials are present", async () => { + await bootWithFlag(true); + expect(eventsOf("live_activity.enabled")).toHaveLength(1); + expect(eventsOf("live_activity.disabled")).toHaveLength(0); + }); + + it("leaves it down when the flag is off, despite the same credentials", async () => { + await bootWithFlag(false); + expect(eventsOf("live_activity.enabled")).toHaveLength(0); + + const disabled = eventsOf("live_activity.disabled"); + expect(disabled).toHaveLength(1); + // Distinguishes "the flag turned it off" from "the credentials went + // missing" — both log the same event, and confusing them costs an operator + // an hour checking a p8 that was fine. + expect(disabled[0].msg).toContain("liveActivityPush"); + }); +}); diff --git a/docs/guides/live-activity-push.md b/docs/guides/live-activity-push.md index 9c0899e4..942e33cf 100644 --- a/docs/guides/live-activity-push.md +++ b/docs/guides/live-activity-push.md @@ -25,6 +25,19 @@ Sending one to the wrong transport fails only at send time, with nothing at regi ## Configuration +**The `liveActivityPush` feature flag gates the whole surface, and it is off by default.** +Credentials alone no longer bring this up: `initLiveActivityPush()` checks the flag before it reads the environment, so a box with a valid p8 configured logs `live_activity.disabled` at boot and constructs no sender. +Turn it on with `THREADBASE_FEATURE_LIVE_ACTIVITY_PUSH=1`, `--feature liveActivityPush=true`, or `feature_flags: {"liveActivityPush":true}` in `server.yaml`, then restart — feature flags resolve once, at boot. + +The flag governs the client half too. +tb-mobile reads it over `GET /api/config/feature-flags` and, when it is off, skips its own local ActivityKit path — and the Android ongoing-notification equivalent — so one switch turns the feature off end to end. +That coupling is deliberate: the client half on its own starts an activity locally from WebSocket frames, which means it only updates while the app is foregrounded, freezes the moment the app backgrounds, and expires silently at iOS's ~8h cap. +Half the feature is worse than none of it. +A server too old to serve `/api/config/feature-flags` reads as off, on the grounds that it is also too old to have been asked — and it is the one case where surfaces stop appearing without anyone choosing that, so tb-mobile logs `liveActivity.legacyServer` when it happens. +From the phone the absence looks identical to a server that answered `false`; that log line is what separates "upgrade the streamer" from "check the flag". + +Turn the flag on where a `liveactivity_start` token is actually registered; without one, the push half has nothing to send to and only the degraded local path remains. + All credentials come from the environment. No key, key id, or team id is committed, and neither the key nor any device token is ever logged. diff --git a/src/feature-flags.ts b/src/feature-flags.ts index 9d95413c..6166886a 100644 --- a/src/feature-flags.ts +++ b/src/feature-flags.ts @@ -47,6 +47,17 @@ export const FEATURE_FLAGS: readonly FeatureFlagDefinition[] = [ default: true, env: "THREADBASE_FEATURE_SESSION_REHYDRATION", }, + { + id: "liveActivityPush", + description: + "Drive iOS Live Activity surfaces for running sessions. Off by default: the streamer half " + + "needs an APNs p8 and a registered push-to-start token, and without both, mobile falls back " + + "to starting the activity locally — which freezes the moment the app backgrounds and " + + "expires silently after ~8h. Mobile reads this flag and skips its local path too, so one " + + "switch turns the whole surface off.", + default: false, + env: "THREADBASE_FEATURE_LIVE_ACTIVITY_PUSH", + }, { id: "ptyHost", description: diff --git a/src/server.ts b/src/server.ts index 461064e0..613e6cb9 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1420,6 +1420,19 @@ export class StreamerServer { * on disk; neither it nor any device token is ever logged. */ private initLiveActivityPush(pushRepo: PushRepository): void { + // Logged rather than returned silently: a box with APNS_KEY configured used + // to print "Live Activity push enabled" here, so an operator who flips the + // flag off needs the credential to look ignored on purpose, not missing. + if (!this.featureFlags.liveActivityPush) { + this.log.info( + "Live Activity push is disabled by the liveActivityPush feature flag. " + + "Enable it with THREADBASE_FEATURE_LIVE_ACTIVITY_PUSH=1, --feature liveActivityPush=true, " + + "or feature_flags: in server.yaml.", + { event: "live_activity.disabled" }, + ); + return; + } + const creds = readApnsCredentialsFromEnv(); if (!creds) { const why = describeMissingApnsCredentials();