From a4feabec81870e005dc46fd0b11b9923fee5f856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 12 Jul 2026 11:08:59 +0200 Subject: [PATCH] =?UTF-8?q?chore(release):=20v1.28.30=20=E2=80=94=20the=20?= =?UTF-8?q?daily=20briefing=20stops=20silently=20skipping=20days?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recurring no-briefing day had a chain of causes, closed end to end: Every failure path in the nightly pregenerate + the comprehensive generator now annotates a queryable cause (several classes — features-error, payload-too-large, aborted, generator-failed — previously bumped a counter with zero trace). A failed nightly warm enqueues one bounded retry ~45 minutes later (singleton per user, existing budgets apply), so a transient provider hiccup heals before morning. A cached payload that carries NO briefing no longer satisfies the 24h POST short-circuit nor the unchanged-content hash gate — a once-stripped briefing could previously block its own regeneration indefinitely. The card surfaces the omission reason persistently (grounding strip, generation failure, not attempted) with a forcing retry. Documentation corrected against code truth: Withings credentials are per-user (the documented env path never existed; the dead example entries and manifest rows are gone, and WITHINGS_WEBHOOK_SECRET is now actually whitelisted in compose — stock deploys could never deliver it); the classic Fitbit portal no longer accepts new registrations; the backup-restore example no longer mints a fresh encryption key and uses pnpm dlx tsx; the TLS guide describes the real client model (two CA-level SPKI pins, any-match, leaf renewals need no build); scaling/admin references fixed; the operator-shared subscription provider is documented. --- .env.production.example | 7 +- CHANGELOG.md | 23 +++ docker-compose.yml | 1 + docs/api/openapi.yaml | 2 +- docs/integrations/ai-providers.md | 39 +++- docs/integrations/fitbit.md | 37 ++-- docs/integrations/whoop.md | 6 +- docs/integrations/withings.md | 37 ++-- docs/ops/backup-restore.md | 22 ++- docs/ops/encryption-key-rotation.md | 4 +- docs/ops/tls-cert-pin.md | 113 ++++++----- docs/self-hosting/scaling.md | 9 +- package.json | 2 +- public/sw.js | 2 +- scripts/env-manifest.json | 25 +-- .../insights/generate/__tests__/route.test.ts | 167 ++++++++++++++++- src/app/api/insights/generate/route.ts | 103 +++++++++- .../use-insights-advisor-force.test.ts | 76 ++++++++ .../insights/use-insights-advisor.ts | 24 ++- .../comprehensive-generate-force.test.ts | 43 +++++ src/lib/insights/briefing-payload.ts | 26 +++ src/lib/insights/comprehensive-generate.ts | 120 +++++++++++- .../insight-pregenerate-retry-enqueue.test.ts | 67 +++++++ .../__tests__/insight-pregenerate.test.ts | 177 ++++++++++++++++++ src/lib/jobs/insight-pregenerate-shared.ts | 71 +++++++ src/lib/jobs/insight-pregenerate.ts | 135 ++++++++++++- 26 files changed, 1190 insertions(+), 148 deletions(-) create mode 100644 src/components/insights/__tests__/use-insights-advisor-force.test.ts create mode 100644 src/lib/insights/briefing-payload.ts create mode 100644 src/lib/jobs/__tests__/insight-pregenerate-retry-enqueue.test.ts diff --git a/.env.production.example b/.env.production.example index 94dfbfed9..1bf6cdfd5 100644 --- a/.env.production.example +++ b/.env.production.example @@ -29,10 +29,11 @@ API_TOKEN_HMAC_KEY="" # ----------------------------------------------------------------------------- -# Withings OAuth -- optional integration +# Withings -- optional integration # ----------------------------------------------------------------------------- -# WITHINGS_CLIENT_ID="" -# WITHINGS_CLIENT_SECRET="" +# The Withings OAuth client id/secret are per-user: each user registers their +# own Withings developer app and pastes the pair into Settings (stored +# encrypted in the DB). There is therefore no WITHINGS_CLIENT_ID/SECRET. # WITHINGS_WEBHOOK_SECRET="" diff --git a/CHANGELOG.md b/CHANGELOG.md index cbb1b6c12..47886ea99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,29 @@ ## [Unreleased] +## [1.28.30] — 2026-07-12 — The daily briefing stops silently skipping days + +The recurring "no briefing today" had a chain of causes, now closed end to end. +Every failure path in the nightly generation names its cause in the logs +(several classes previously failed without a trace). A failed night retries +itself about forty-five minutes later instead of waiting for the next night. +A cached insight payload that carries no briefing no longer counts as fresh — +previously it could satisfy the daily window and even the unchanged-data check, +so a once-stripped briefing could block its own regeneration indefinitely. And +the briefing card now reports persistently why a briefing is absent (withheld +by the number check, generation failed, or not yet attempted) with a retry +button that always generates. + +Documentation corrections, verified against the code: Withings credentials are +per-user in the app (the documented environment-variable path never existed — +and the webhook secret is now actually passed through in the compose file); +the classic Fitbit portal no longer accepts new registrations (new users: +Google Health); the backup-restore example no longer generates a fresh +encryption key (which could never decrypt an existing backup) and uses the +invocation that works in the production image; the certificate-pinning guide +now describes the real model (two CA-level pins, leaf renewals need no app +build); scaling and admin-endpoint references corrected. + ## [1.28.29] — 2026-07-11 — Dashboard edits show up on the way back; one scrollbar again Changing the dashboard tile selection now shows up immediately when you diff --git a/docker-compose.yml b/docker-compose.yml index 80dbb49fe..4fa828fbb 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -81,6 +81,7 @@ services: # docker-compose reads .env for `${VAR}` substitution at compose-up time). SESSION_COOKIE_SECURE: "${SESSION_COOKIE_SECURE:-}" NATIVE_CANVAS: "${NATIVE_CANVAS:-}" + WITHINGS_WEBHOOK_SECRET: "${WITHINGS_WEBHOOK_SECRET:-}" # v1.18.11 (W3) — client-IP resolution + geo lookup. Behind Cloudflare # the real visitor IP is in `cf-connecting-ip`; set # TRUST_CF_CONNECTING_IP=1 ONLY when Cloudflare is in front (off by diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 66b284554..0bd7fed38 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -1,7 +1,7 @@ openapi: 3.1.0 info: title: HealthLog API - version: 1.28.29 + version: 1.28.30 description: >- Self-hosted personal-health-tracking PWA — public API surface for the iOS native client and external ingest. diff --git a/docs/integrations/ai-providers.md b/docs/integrations/ai-providers.md index 841572745..e72bcae64 100644 --- a/docs/integrations/ai-providers.md +++ b/docs/integrations/ai-providers.md @@ -33,7 +33,9 @@ HealthLog supports two key-provisioning modes that coexist: `AppSettings.adminAiKeyEncrypted`. Users who have not configured their own key fall back to the admin key. This is the last entry in the default provider chain so it acts as a safety net rather - than the default route. + than the default route. Since v1.28.14 the operator can also share + a subscription-based account that users opt into — see + [Admin-shared subscription access](#admin-shared-subscription-access). Single-user instances usually pick one mode and stop there. Multi-user instances tend to run BYOK with the admin-shared key as a @@ -209,6 +211,41 @@ documented in `docs/codex-protocol-spec.md` for anyone who wants to audit the request shape; users who just want the integration to work do not need to read it. +## Admin-shared subscription access + +Besides the shared API key, the operator can connect a single +subscription-based AI account for the whole instance — an +operator-shared subscription provider that users without any +personal AI setup can opt into. + +- **Admin side.** Under **Admin → AI** the operator runs the same + device-code OAuth flow as the per-user connection + (`POST /api/admin/central-codex/device-start` + + `device-poll`). The tokens land AES-256-GCM-encrypted in the + `AppSettings` singleton and are never returned by any endpoint — + status and disconnect go through `GET` / `DELETE +/api/admin/central-codex`, which sit behind the cookie-only + `requireAdmin()` boundary. +- **User side.** Each user opts in individually in `/settings/ai` + (`PATCH /api/auth/me/use-central-codex`, body + `{ "useCentralCodex": boolean }`). The opt-in is off by default. +- **Chain semantics.** The shared connection is the internal + `admin-codex` chain entry. It is never part of the persisted chain + and never offered in the chain editor — it is appended **last** to + the resolved chain, and only when the user opted in AND the + operator has connected the account + (`src/lib/ai/provider-chain.ts`). A hand-crafted persisted entry + resolves to nothing. +- **Cost.** Generations through the shared connection run under the + operator's account, so usage and rate limits land on the operator, + not the user. + +The settings UI states this plainly: the shared access is a single +signed-in AI account the operator connected for everyone on the +server — not the user's own key. Users who want their data to reach +only a provider under their own contract should configure a personal +provider instead; their own entries always resolve first. + ## Privacy stance - **OpenAI** and **Anthropic** see the prompt + structured snapshot diff --git a/docs/integrations/fitbit.md b/docs/integrations/fitbit.md index 3fbb57481..66f0e0c51 100644 --- a/docs/integrations/fitbit.md +++ b/docs/integrations/fitbit.md @@ -5,10 +5,10 @@ HealthLog reads Fitbit and Pixel Watch data through the **classic Fitbit Web API** (`api.fitbit.com`). The OAuth app is a **Fitbit developer app registered -at [dev.fitbit.com](https://dev.fitbit.com/apps/new)** — **not** a Google Cloud +at [dev.fitbit.com](https://dev.fitbit.com/)** — **not** a Google Cloud OAuth client. Like WHOOP, credentials are brought per user: each user (or the -operator on their behalf) registers their own Fitbit app and pastes the client -id/secret into Settings. The internal integration key is `fitbit`. +operator on their behalf) supplies their own Fitbit app's client id/secret in +Settings. The internal integration key is `fitbit`. > **Common mistake:** the field wants a **Fitbit** Client ID (a short numeric > id from dev.fitbit.com), **not** a Google Cloud client id ending in @@ -16,30 +16,33 @@ id/secret into Settings. The internal integration key is `fitbit`. > consent screen with `unauthorized_client — Invalid client_id`, because Fitbit > and Google Cloud are separate client registries. -> **Heads-up — September 2026 sunset.** Google is retiring the classic Fitbit -> Web API in **September 2026** and moving to the Google Health API behind Google -> sign-in. That replacement currently requires Google Restricted-scope brand -> verification plus an annual third-party CASA security assessment, which does -> not fit a self-hosted bring-your-own-credentials model, so the path beyond the -> sunset is still being worked out. Until then the setup below is the supported -> way to connect Fitbit. +> **Heads-up — new registrations are closed.** Fitbit has **discontinued +> self-serve app registration**: the form at `dev.fitbit.com/apps/new` is +> disabled for new applications, and the classic Fitbit Web API sunsets in +> **September 2026**. If you already hold a registered Fitbit app, it keeps +> working until the sunset and the setup below still applies. If you are +> connecting Fitbit data for the first time, use the +> [Google Health integration](google-health.md) instead — Fitbit devices sync +> into Google's health stack, which HealthLog reads directly. -## 1. Register a Fitbit developer app +## 1. Use an existing Fitbit developer app -1. Sign in at and register a new application. -2. **OAuth 2.0 Application Type:** +Registration of new apps is no longer possible (see the note above). For an +app you registered before the shutdown, these are the settings that matter: + +1. **OAuth 2.0 Application Type:** - **Personal** — the app only ever sees the **owner's own** Fitbit account, and gets intraday (per-minute) heart-rate/steps automatically. This is the right choice for a single-person self-host. - **Server** (or **Client**) — needed when the instance connects **other people's** accounts. Intraday/heart-rate access is then granted case by case via Fitbit's Intraday Data Access request form. -3. **Callback URL:** `https://your-instance.example.com/api/fitbit/callback` +2. **Callback URL:** `https://your-instance.example.com/api/fitbit/callback` — absolute, **HTTPS**, and matching the `redirect_uri` HealthLog sends (see the redirect-URI note below). One entry per environment you connect from. -4. **Scopes:** tick the ones HealthLog requests (next section). -5. Save. Fitbit issues a **Client ID** (short, numeric) and a **Client Secret** - — keep the secret out of any chat log. +3. **Scopes:** the ones HealthLog requests (next section). +4. The app's settings page shows the **Client ID** (short, numeric) and the + **Client Secret** — keep the secret out of any chat log. ## 2. Scopes diff --git a/docs/integrations/whoop.md b/docs/integrations/whoop.md index 8359de2d0..731eb6fab 100644 --- a/docs/integrations/whoop.md +++ b/docs/integrations/whoop.md @@ -50,8 +50,10 @@ Click **Connect WHOOP**. The flow: 2. The user signs in on WHOOP and grants the requested scopes. 3. WHOOP redirects back to `/api/whoop/callback` with an auth code. 4. HealthLog exchanges the code for an access + refresh token, stores - them encrypted, registers the webhook, and triggers an initial - sync of recent history. + them encrypted, and triggers an initial sync of recent history. + (The webhook is **not** registered per user — it is configured once + at app level in the WHOOP developer console; see the env-var + section below.) 5. Subsequent syncs run on every webhook delivery plus a safety-net cron pull. diff --git a/docs/integrations/withings.md b/docs/integrations/withings.md index 465ad4465..3d08b370a 100644 --- a/docs/integrations/withings.md +++ b/docs/integrations/withings.md @@ -38,20 +38,25 @@ arrive via the sleep series endpoint. needed — `user.metrics` covers weight / BP / heart-list / sleep / SpO₂ / body comp / temperature / VO₂ max; `user.activity` is the workout-equivalent stream for steps / energy / distance / floors. - See `src/lib/withings/client.ts:42` (`WITHINGS_OAUTH_SCOPE`). + See `WITHINGS_OAUTH_SCOPE` in `src/lib/withings/client.ts`. 4. Save the application. Withings will issue a **Client ID** and a **Client Secret** — keep the secret out of any chat log; it grants read access to every linked account's full health history. -## 2. Wire the credentials into HealthLog +## 2. Wire the credentials in Settings → Integrations -Paste the client ID and secret into `.env`: +Withings credentials are stored per user, not in `.env`. Sign in as +the target user, open **Settings → Integrations → Withings**, and +paste the client ID and secret. They are encrypted AES-256-GCM at +rest (`src/lib/withings/credentials.ts`). On a shared instance every +user brings their own Withings app credentials; there is no +server-level client-id/secret fallback. + +Two server-level env vars remain: ```env -WITHINGS_CLIENT_ID="your-client-id" -WITHINGS_CLIENT_SECRET="your-client-secret" -WITHINGS_REDIRECT_URI="https://your-instance.example.com/api/withings/callback" WITHINGS_WEBHOOK_SECRET="$(openssl rand -hex 32)" +WITHINGS_REDIRECT_URI="https://your-instance.example.com/api/withings/callback" ``` `WITHINGS_WEBHOOK_SECRET` is the path-segment token the webhook @@ -60,10 +65,9 @@ that fits in a URL path segment works, but a 64-character hex string keeps it indistinguishable from random. Restart the `app` container after saving. -Per-user override is also possible: the Settings UI accepts a -client-ID/secret pair scoped to one user, useful if you want a -shared instance where every user brings their own Withings app -credentials. Server-level env vars cover the default case. +`WITHINGS_REDIRECT_URI` is optional — when unset the callback URL +defaults to `${NEXT_PUBLIC_APP_URL}/api/withings/callback`. Set it +only when the OAuth callback must differ from that default. ## 3. Set the webhook URL @@ -90,9 +94,8 @@ callback. The portal only owns the OAuth callback URL. ## 4. Link from the Settings page -With env vars in place and the app restarted, sign in as the target -user and open `/settings/integrations/withings`. Click **Connect -Withings**. The flow: +With the credentials saved in Settings, open +`/settings/integrations/withings` and click **Connect Withings**. The flow: 1. The Connect button redirects to Withings' `oauth2/authorize`. 2. The user signs in on Withings and grants the requested scopes. @@ -133,10 +136,10 @@ Override per-user via the Sources section of `/settings/thresholds`. ### The Connect button errors out -- **`Withings credentials not configured`** — `WITHINGS_CLIENT_ID` - or `WITHINGS_CLIENT_SECRET` is missing from `.env`. Confirm both - are set in the container's runtime environment (`docker compose -exec app env | grep WITHINGS`). +- **`Withings credentials not configured`** — the signed-in user has + not saved a client ID + secret under **Settings → Integrations → + Withings**. Credentials live per user in the database, not in + `.env` — paste the pair from the developer portal and retry. - **`Callback URI mismatch`** — the URL HealthLog sends to Withings must match an entry in the developer-portal app's callback list. Add the public URL exactly, including the `https://` scheme and diff --git a/docs/ops/backup-restore.md b/docs/ops/backup-restore.md index af110f832..e96ce18a2 100644 --- a/docs/ops/backup-restore.md +++ b/docs/ops/backup-restore.md @@ -68,20 +68,26 @@ The credentials are never returned. ## Restore Pick a key (e.g. `2026-05-08/user-clx123.json.enc`) from the bucket -and run: +and run the restore script with the same backup credentials and +encryption key the backup was written under — a freshly generated +`BACKUP_ENCRYPTION_KEY` cannot decrypt any existing object: ```bash -BACKUP_S3_ENDPOINT=https://...r2.cloudflarestorage.com \ -BACKUP_S3_BUCKET=healthlog-backups \ -BACKUP_S3_ACCESS_KEY=... \ -BACKUP_S3_SECRET_KEY=... \ -BACKUP_S3_REGION=auto \ -BACKUP_ENCRYPTION_KEY=$(openssl rand -hex 32) \ -pnpm tsx scripts/restore-backup.ts \ +BACKUP_S3_ENDPOINT=https://...r2.cloudflarestorage.com \ +BACKUP_S3_BUCKET=healthlog-backups \ +BACKUP_S3_ACCESS_KEY=... \ +BACKUP_S3_SECRET_KEY=... \ +BACKUP_S3_REGION=auto \ +BACKUP_ENCRYPTION_KEY= \ +pnpm dlx tsx scripts/restore-backup.ts \ 2026-05-08/user-clx123.json.enc \ /tmp/restored.json ``` +The production standalone image strips `tsx`, so a bare +`pnpm tsx scripts/...` fails inside the container — always invoke +one-shot scripts via `pnpm dlx tsx`. + The script downloads the object, decrypts it, and writes the JSON dump to disk. Importing the JSON back into a HealthLog instance is left to the operator (use `prisma db seed` or a custom script). diff --git a/docs/ops/encryption-key-rotation.md b/docs/ops/encryption-key-rotation.md index 9c3f7d4e0..bb236e8d3 100644 --- a/docs/ops/encryption-key-rotation.md +++ b/docs/ops/encryption-key-rotation.md @@ -44,7 +44,7 @@ under `v1` (the synthetic id assigned to the existing `ENCRYPTION_KEY`). 3. **Run the rotation script** to re-encrypt every existing encrypted column under the new active key: ``` - pnpm tsx scripts/rotate-encryption-key.ts v2 + pnpm dlx tsx scripts/rotate-encryption-key.ts v2 ``` The script is idempotent — running it again is a no-op for rows already prefixed with `v2.`. It rotates User, WithingsConnection, AppSettings, @@ -69,7 +69,7 @@ script reports zero `v2.`-prefixed rows remaining. ``` ENCRYPTION_KEYS='{"v2":"","v3":""}' ENCRYPTION_ACTIVE_KEY_ID="v3" -pnpm tsx scripts/rotate-encryption-key.ts v3 +pnpm dlx tsx scripts/rotate-encryption-key.ts v3 ENCRYPTION_KEYS='{"v3":""}' ``` diff --git a/docs/ops/tls-cert-pin.md b/docs/ops/tls-cert-pin.md index bc2588e58..fdb9cf293 100644 --- a/docs/ops/tls-cert-pin.md +++ b/docs/ops/tls-cert-pin.md @@ -1,26 +1,34 @@ -# TLS leaf SPKI pin — coupling, alarm, and re-pin runbook +# TLS SPKI pinning — coupling, alarm, and re-pin runbook -The native client SPKI-pins the server's TLS **leaf** certificate. This is a -deliberate hardening choice on the client side, but it couples the client to -the exact leaf keypair the server serves: when that leaf rotates, a client -that was only ever shipped the old pin refuses to connect. This page explains -the coupling, the server-side alarm that watches for a rotation, and the -operator / release-owner steps when it fires. +The native client SPKI-pins the server's TLS chain at the **CA level**. It +ships a small pin set — currently the issuing intermediate plus the root — +and accepts a connection only when the served chain passes baseline X.509 +evaluation AND at least one certificate anywhere in the chain matches a pin +(any-match). The leaf is deliberately **not** pinned, so routine leaf +renewals are a client no-op. The outage risk is a **chain change**: the +served chain stops terminating in the pinned CA keys — the server moves to a +different CA, or the CA rotates its intermediate / root keys. This page +explains the coupling, the server-side alarm that watches the served leaf, +and the operator / release-owner steps when it fires. ## The coupling -A SPKI pin is `base64(sha256(DER subjectPublicKeyInfo))` — a hash of the +A SPKI pin is `base64(sha256(DER subjectPublicKeyInfo))` — a hash of a certificate's public key, not of the whole certificate. The native client -carries a set of these pins and rejects any TLS connection whose leaf public -key is not in the set. +carries a set of these pins (build-time config, ≥ 2 well-formed pins +enforced for release builds) and rejects any TLS connection whose validated +chain contains no pinned key. Hosts outside the client's pinned-host list +fall back to plain system-trust validation, which is the documented model +for self-hosted instances that ship their own chain. -The app host's certificate is issued by Google Trust Services (GTS) and -auto-renews roughly every 90 days. Each renewal mints a **new leaf keypair**, -so the SPKI pin changes on every renewal. Intermediate and root rolls have -the same effect on the leaf chain. Because the client pins the leaf, every -such change is a hard outage for any client that hasn't been shipped the new -pin — and there is otherwise no server-side signal that a client is pinning -the served leaf at all. +The app host's certificate auto-renews roughly every 90 days and each +renewal mints a new leaf keypair. Because the client pins the CA keys rather +than the leaf, those renewals pass without a client build. What DOES break +every pinned client is a chain that no longer terminates in a pinned key — +and there is no server-side signal that clients are pinning the served chain +at all. The alarm below exists to catch exactly that early: it watches the +served **leaf**, the most frequently rotating element, as the canary for any +chain movement. ## The alarm @@ -46,13 +54,15 @@ successful probe returning a pin genuinely outside the known set. ### Baseline source: an env var, not auto-learning -`TLS_LEAF_SPKI_PINS` (comma-separated) is the known-good set. The baseline is -an env var on purpose, **not** a persisted "last-seen" row: +`TLS_LEAF_SPKI_PINS` (comma-separated) is the known-good set of **served +leaf** pins. It is the operator's explicit record of what the server is +expected to serve — distinct from the client's CA-level pin set, which lives +in the client build config. The baseline is an env var on purpose, **not** a +persisted "last-seen" row: -- The pinned client only trusts the pins it was shipped, so the operator must - derive the client's pin set from a single explicit source of truth. The env - var **is** that source — the same value goes into the client's pin set and - into `TLS_LEAF_SPKI_PINS`. +- An explicit baseline forces the operator to acknowledge every leaf + rotation and check the new chain against the client's pinned CA keys + before the alarm goes quiet. - A persisted last-seen baseline would silently adopt the first rotated pin and suppress the very alarm the pinned client needs. - When the env var is unset, the monitor fails **loud, not open**: it logs @@ -84,36 +94,43 @@ The variable is on the compose `environment:` whitelist and the whether it is set. It is optional: without it the monitor still runs and logs the served pin, but cannot alarm on a change. -## When the alarm fires — re-pin procedure +## When the alarm fires When you see a `tls.pin.leaf_changed` alert / `system.tls.pin_changed` audit -row, the served leaf has rotated. The pinned client will stop connecting once -its shipped pin no longer matches a served leaf. Act inside the certificate's -remaining validity, with margin for app review and client roll-out — target -**≥ 11 days before the old pin's certificate expires** (the alert body and -audit row both carry the new leaf's `validTo`). +row, the served leaf has rotated. First determine which of two cases you are +in: -1. **Re-extract the new leaf pin** with the `openssl` pipeline above. It - should equal the `servedPin` in the alert. -2. **Dual-pin.** Add the new pin to the client's pin set **alongside** the old - one (do not replace it yet), and update `TLS_LEAF_SPKI_PINS` to the - comma-separated pair, e.g. - `TLS_LEAF_SPKI_PINS=","`. Holding both pins means clients - on the old build keep working through the cutover and the server-side - alarm goes quiet because the served pin is now in the known set. Re-deploy - the server so the new env value takes effect. +1. **Routine renewal, same chain.** Inspect the new chain (the `openssl` + pipeline above, or the alert payload). If the issuing intermediate and + root are unchanged, the client's CA-level pins still match and no client + is at risk. Update `TLS_LEAF_SPKI_PINS` to the new served-leaf pin and + re-deploy so the alarm goes quiet. No client build. +2. **Chain change.** The new leaf chains through a different intermediate / + root (CA switch, or the CA rotated its keys). Every pinned client will + stop connecting once the old chain is no longer served — run the re-pin + procedure below inside the old chain's remaining validity, with margin + for app review and client roll-out (the alert body and audit row carry + the new leaf's `validTo`). + +### Re-pin procedure (chain change) + +1. **Extract the new chain's CA pins** (intermediate + root) with the + `openssl` pipeline / the client repo's SPKI-extraction script. +2. **Dual-pin.** Add the new CA pins to the client's pin set **alongside** + the old ones (do not replace them yet). Clients on the old build keep + working as long as the old chain is still served; the dual-pinned build + works against both chains. Update `TLS_LEAF_SPKI_PINS` to the currently + served leaf pin(s) so the server-side alarm reflects reality. 3. **Ship the client build** carrying the dual-pin set to TestFlight (and - onward to release). Give it long enough to roll out to the installed base - before the next renewal. -4. **Retire the old pin** once the old leaf is no longer served and the - dual-pinned client build has reached the installed base: drop the old pin - from both the client pin set and `TLS_LEAF_SPKI_PINS`, leaving only the - current leaf pin. + onward to release). Give it long enough to roll out to the installed + base before the old chain disappears. +4. **Retire the old pins** once the old chain is no longer served and the + dual-pinned client build has reached the installed base. -Treat the dual-pin window as standing practice: always ship the **next** pin -before the current leaf rotates, so a renewal never catches a single-pinned -client. If GTS renews on a fixed cadence, pre-extracting and dual-pinning the -upcoming leaf ahead of the renewal turns every rotation into a no-op. +CA-level pins move rarely — intermediates and roots carry multi-year +lifetimes and rotations are announced well ahead. Track the pinned +certificates' expiry dates and pre-stage the successor pins before the +cutover, so a chain change never catches a single-chain client. ## Environment assumption diff --git a/docs/self-hosting/scaling.md b/docs/self-hosting/scaling.md index 9e314b2a2..2ab4ea976 100644 --- a/docs/self-hosting/scaling.md +++ b/docs/self-hosting/scaling.md @@ -24,15 +24,18 @@ deployed in three modes: 3. `docker compose up -d --build`. Both containers connect to the same Postgres. pg-boss claims jobs -atomically via row-level locking, so it is safe to run multiple worker -replicas — they will share the load. +atomically via row-level locking (`FOR UPDATE SKIP LOCKED`), so +running multiple worker replicas is **safe** — no job runs twice — +but it doubles the DB load and muddies telemetry without adding +throughput a personal instance needs (`src/lib/process-type.ts`). +Run **one** worker container and scale the `web` containers instead. ## Healthchecks - `app` (web) healthcheck: `wget /api/version` every 30s. - `app-worker` does not expose a port; its liveness is reported into the `worker_status` table on every reminder pass and surfaced through - `/api/admin/worker-status`. + `/api/admin/status`. The web container does NOT depend on the worker, and vice versa, so neither container's startup can deadlock the other. The shared diff --git a/package.json b/package.json index 3bf1b91f2..da490eb20 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "healthlog", - "version": "1.28.29", + "version": "1.28.30", "description": "Self-hosted personal-health-tracking PWA with Withings integration, AI insights, and doctor-report PDF export.", "license": "PolyForm-Noncommercial-1.0.0", "homepage": "https://healthlog.dev", diff --git a/public/sw.js b/public/sw.js index 6021ae46b..903ab1775 100644 --- a/public/sw.js +++ b/public/sw.js @@ -36,7 +36,7 @@ try { // v1.4.38.4 → v1.4.42. Do not hand-edit; bump `package.json` and rebuild. const CACHE_VERSION = (typeof self !== "undefined" && self.__APP_VERSION__) || - /* @sw-version-fallback */ "v1.28.29"; + /* @sw-version-fallback */ "v1.28.30"; const STATIC_CACHE = `healthlog-static-${CACHE_VERSION}`; const PAGE_CACHE = `healthlog-pages-${CACHE_VERSION}`; // v1.18.6 — read-only data cache for a curated allowlist of safe GET `/api/*` diff --git a/scripts/env-manifest.json b/scripts/env-manifest.json index 74fa57795..bda14de83 100644 --- a/scripts/env-manifest.json +++ b/scripts/env-manifest.json @@ -37,17 +37,9 @@ }, { "name": "Withings OAuth", - "description": "Required to enable the Withings integration. App boots without these but Withings sync stays disabled.", + "description": "Withings API credentials are entered per user in Settings → Integrations (no env-var path). Only the webhook shared secret is operator-level.", "required": false, "variables": [ - { - "name": "WITHINGS_CLIENT_ID", - "purpose": "Withings OAuth client_id from https://developer.withings.com." - }, - { - "name": "WITHINGS_CLIENT_SECRET", - "purpose": "Withings OAuth client_secret." - }, { "name": "WITHINGS_WEBHOOK_SECRET", "purpose": "Random secret carried as a path segment in the Withings webhook callback URL. Generate with `openssl rand -hex 32`." @@ -167,9 +159,18 @@ "name": "BACKUP_S3_ENDPOINT", "purpose": "S3-compatible endpoint URL." }, - { "name": "BACKUP_S3_BUCKET", "purpose": "Bucket name." }, - { "name": "BACKUP_S3_ACCESS_KEY", "purpose": "S3 access key." }, - { "name": "BACKUP_S3_SECRET_KEY", "purpose": "S3 secret key." }, + { + "name": "BACKUP_S3_BUCKET", + "purpose": "Bucket name." + }, + { + "name": "BACKUP_S3_ACCESS_KEY", + "purpose": "S3 access key." + }, + { + "name": "BACKUP_S3_SECRET_KEY", + "purpose": "S3 secret key." + }, { "name": "BACKUP_ENCRYPTION_KEY", "purpose": "Separate AES key for the backup body — MUST NOT match ENCRYPTION_KEY." diff --git a/src/app/api/insights/generate/__tests__/route.test.ts b/src/app/api/insights/generate/__tests__/route.test.ts index 507a29e51..fb955dcee 100644 --- a/src/app/api/insights/generate/__tests__/route.test.ts +++ b/src/app/api/insights/generate/__tests__/route.test.ts @@ -405,11 +405,16 @@ describe("POST /api/insights/generate — cache write (v1.16.8)", () => { it("does NOT touch the cache when serving from the 24h DB cache", async () => { // Cached path: route returns early without touching the LLM or the - // cache write. + // cache write. v1.28.30 — the short-circuit requires the cached + // payload to CARRY a briefing (a briefingless cache is stale for a + // briefing-expecting caller), so the fixture carries one. vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ insightsPrivacyMode: "aggregated", insightsCachedAt: new Date(), - insightsCachedText: JSON.stringify({ changed: "still fresh" }), + insightsCachedText: JSON.stringify({ + changed: "still fresh", + dailyBriefing: { paragraph: "ok", keyFindings: [] }, + }), locale: "en", } as never); @@ -445,7 +450,10 @@ describe("POST /api/insights/generate — cache write (v1.16.8)", () => { vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ insightsPrivacyMode: "aggregated", insightsCachedAt: new Date(), - insightsCachedText: JSON.stringify({ changed: "still fresh" }), + insightsCachedText: JSON.stringify({ + changed: "still fresh", + dailyBriefing: { paragraph: "ok", keyFindings: [] }, + }), locale: "en", } as never); @@ -467,6 +475,99 @@ describe("POST /api/insights/generate — cache write (v1.16.8)", () => { }); }); +// v1.28.30 — the "no briefing today" chain: a failed nightly warm left a +// fresh-but-briefingless cache inside the 24 h window, and the POST's +// unconditional short-circuit served it to every regenerate attempt +// (`cached: true` three times in prod, zero generations, a full day +// without a briefing). The short-circuit now only holds when the cached +// payload carries a briefing OR the account has no usable provider. +describe("POST /api/insights/generate — briefingless fresh cache (v1.28.30)", () => { + function mockBriefinglessFreshCache() { + vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ + insightsPrivacyMode: "aggregated", + insightsCachedAt: new Date(Date.now() - 6 * 60 * 60 * 1000), + insightsCachedText: JSON.stringify({ + changed: "fresh but no briefing", + dailyBriefing: null, + }), + insightsExcludeMetrics: [], + locale: "en", + } as never); + } + + it("POST without force + fresh cache WITH briefing → served cached", async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ + insightsPrivacyMode: "aggregated", + insightsCachedAt: new Date(Date.now() - 6 * 60 * 60 * 1000), + insightsCachedText: JSON.stringify({ + dailyBriefing: { paragraph: "today", keyFindings: [] }, + }), + locale: "en", + } as never); + + const res = await POST(jsonRequest({}) as never); + expect(res.status).toBe(200); + const body = (await res.json()) as { data: { cached: boolean } }; + expect(body.data.cached).toBe(true); + expect(prisma.user.update).not.toHaveBeenCalled(); + }); + + it("POST without force + fresh cache WITHOUT briefing + provider → regenerates", async () => { + makeWorkingProvider(); + mockBriefinglessFreshCache(); + + const res = await POST(jsonRequest({}) as never); + expect(res.status).toBe(200); + const body = (await res.json()) as { data: { cached: boolean } }; + // The stale-for-briefing cache was bypassed: a real generation ran and + // wrote a fresh cache row. + expect(body.data.cached).toBe(false); + expect(prisma.user.update).toHaveBeenCalledTimes(1); + }); + + it("POST without force + fresh cache WITHOUT briefing + NO provider → serves cached (regeneration is futile)", async () => { + vi.mocked(hasUsableStatusProvider).mockResolvedValueOnce(false); + mockBriefinglessFreshCache(); + + const res = await POST(jsonRequest({}) as never); + expect(res.status).toBe(200); + const body = (await res.json()) as { data: { cached: boolean } }; + expect(body.data.cached).toBe(true); + expect(prisma.user.update).not.toHaveBeenCalled(); + }); + + it("degrades to the cached payload instead of 429 when the briefingless fall-through is rate-limited", async () => { + makeWorkingProvider(); + mockBriefinglessFreshCache(); + vi.mocked(checkRateLimit).mockResolvedValue({ + allowed: false, + remaining: 0, + resetAt: Date.now() + 3600_000, + }); + + const res = await POST(jsonRequest({}) as never); + // A POST-as-read caller never used to see a 429 on a fresh cache; + // exhausting the quota falls back to the old cached-serve behaviour. + expect(res.status).toBe(200); + const body = (await res.json()) as { data: { cached: boolean } }; + expect(body.data.cached).toBe(true); + expect(prisma.user.update).not.toHaveBeenCalled(); + }); + + it("explicit force keeps the honest 429 when rate-limited", async () => { + makeWorkingProvider(); + mockBriefinglessFreshCache(); + vi.mocked(checkRateLimit).mockResolvedValue({ + allowed: false, + remaining: 0, + resetAt: Date.now() + 3600_000, + }); + + const res = await POST(jsonRequest({ force: true }) as never); + expect(res.status).toBe(429); + }); +}); + // v1.4.36 QA H1 — when both the raw-mode extraction AND the aggregated // retry cross the 5 MB ceiling, the route used to throw uncaught → 500. // It now wraps the retry, downgrades further via the exclude filter, @@ -619,6 +720,66 @@ describe("GET /api/insights/generate — read-only advisor read", () => { expect(body.data.revalidating).toBe(false); }); + // v1.28.30 — a briefing the grounding gate withheld used to be visible + // only on the transient POST response; every later read showed a generic + // "no briefing yet". The marker written after the cache write now + // surfaces the omission on the read path, WITHOUT reading as a failure. + it("surfaces briefingOmittedReason from a briefing-ungrounded marker (not as a failure)", async () => { + const cachedAt = new Date(Date.now() - 60 * 60 * 1000); + vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ + insightsCachedAt: cachedAt, + insightsCachedText: JSON.stringify({ dailyBriefing: null }), + locale: "en", + } as never); + // Marker newer than the last successful generation (written right + // after the cache write on the strip path). + vi.mocked(prisma.auditLog.findFirst).mockResolvedValueOnce({ + createdAt: new Date(cachedAt.getTime() + 1000), + details: JSON.stringify({ reason: "briefing-ungrounded" }), + } as never); + + const res = await (GET as unknown as (req: Request) => Promise)( + new Request("http://localhost/api/insights/generate"), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { + briefingOmittedReason: string | null; + generationFailed: boolean; + generationFailureClass: string | null; + }; + }; + expect(body.data.briefingOmittedReason).toBe("ungrounded"); + // The generation SUCCEEDED (the briefing was withheld) — the card must + // render "withheld", not "couldn't generate". + expect(body.data.generationFailed).toBe(false); + expect(body.data.generationFailureClass).toBeNull(); + }); + + it("still reports generationFailed for a genuine failure marker", async () => { + const cachedAt = new Date(Date.now() - 60 * 60 * 1000); + vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ + insightsCachedAt: cachedAt, + insightsCachedText: JSON.stringify({ + dailyBriefing: { paragraph: "held", keyFindings: [] }, + }), + locale: "en", + } as never); + vi.mocked(prisma.auditLog.findFirst).mockResolvedValueOnce({ + createdAt: new Date(cachedAt.getTime() + 1000), + details: JSON.stringify({ reason: "provider-error", httpStatus: 503 }), + } as never); + + const res = await (GET as unknown as (req: Request) => Promise)( + new Request("http://localhost/api/insights/generate"), + ); + const body = (await res.json()) as { + data: { briefingOmittedReason: string | null; generationFailed: boolean }; + }; + expect(body.data.generationFailed).toBe(true); + expect(body.data.briefingOmittedReason).toBeNull(); + }); + it("reports hasProvider: true on a normal cached read", async () => { vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ insightsCachedAt: new Date(), diff --git a/src/app/api/insights/generate/route.ts b/src/app/api/insights/generate/route.ts index 84d16bda4..4ff5763aa 100644 --- a/src/app/api/insights/generate/route.ts +++ b/src/app/api/insights/generate/route.ts @@ -81,6 +81,7 @@ import { buildComparisonSnapshotForUser, enqueueStatusRefillForUser, } from "@/lib/insights/comprehensive-generate"; +import { cachedPayloadCarriesBriefing } from "@/lib/insights/briefing-payload"; export const dynamic = "force-dynamic"; @@ -168,10 +169,21 @@ export const GET = apiHandler(async (request: NextRequest) => { userId, since: cachedAt, }); - const generationFailed = briefingFailure !== null; + // v1.28.30 — a `briefing-ungrounded` marker is NOT a failure: the + // generation succeeded but the grounding gate withheld the briefing + // (marker written after the cache write, so it survives the `since` + // filter). Surface it as the persistent omission signal — the card's + // "briefing withheld" state used to exist only on the transient POST + // response, so a nightly strip read as a silent generic empty card. + const briefingOmittedReason: "ungrounded" | null = + briefingFailure?.reason === "briefing-ungrounded" ? "ungrounded" : null; + const generationFailed = + briefingFailure !== null && briefingOmittedReason === null; // v1.25.3 — the failure class lets the empty state point its hint at the // right lever (raise the response timeout vs re-check the provider). - const generationFailureClass = briefingFailure?.failureClass ?? null; + const generationFailureClass = generationFailed + ? (briefingFailure?.failureClass ?? null) + : null; // Read-only: never block on the provider. Warm out of band only when the // cached briefing is stale / missing AND a provider is configured (a @@ -222,6 +234,9 @@ export const GET = apiHandler(async (request: NextRequest) => { // v1.25.3 — coarse failure class for the empty-state hint (null when // the last attempt succeeded). generationFailureClass, + // v1.28.30 — persistent grounding-omission signal (see above), so a + // nightly-stripped briefing renders "withheld", not "no briefing yet". + briefingOmittedReason, }); } catch { // Invalid cache row — fall through to the empty payload below. The @@ -246,6 +261,8 @@ export const GET = apiHandler(async (request: NextRequest) => { generationFailed, // v1.25.3 — coarse failure class for the empty-state hint. generationFailureClass, + // v1.28.30 — see the cached branch above. + briefingOmittedReason, }); }); @@ -301,6 +318,24 @@ export const POST = apiHandler((request: NextRequest) => if (jsonError) return jsonError; const forceRefresh = body.force === true; + // v1.28.30 — a fresh cache WITHOUT a briefing must not satisfy a + // briefing-expecting caller: after a failed nightly warm the cached + // payload can sit inside the 24 h window all day, and the old + // unconditional short-circuit served it to every POST (three + // regenerate attempts, three `cached: true` responses, zero + // generations). The short-circuit now holds only when the cached + // payload actually carries a briefing OR the account has no usable + // provider (regenerating would be futile — serve what exists). The + // read-only GET is untouched: it never generates and its out-of-band + // revalidation behaviour stays as-is. When the briefingless fall- + // through is rate-limited below, the cached payload is served after + // all so a POST-as-read client degrades to the old behaviour instead + // of a 429. + let briefinglessCacheFallback: { + cached: unknown; + legacyPayload: boolean; + cachedAt: Date; + } | null = null; if ( !forceRefresh && dbUser?.insightsCachedAt && @@ -314,15 +349,33 @@ export const POST = apiHandler((request: NextRequest) => // We don't auto-regenerate — that would burn rate-limit tokens // on a cache-hit silently. User-initiated only. const legacyPayload = isLegacyInsightPayload(cached); - annotate({ - action: { name: "insights.generate" }, - meta: { cached: true, legacyPayload }, - }); - return apiSuccess({ - insights: cached, - cached: true, - cachedAt: dbUser.insightsCachedAt, + const carriesBriefing = cachedPayloadCarriesBriefing( + dbUser.insightsCachedText, + ); + if (carriesBriefing || !(await hasUsableStatusProvider(userId))) { + annotate({ + action: { name: "insights.generate" }, + meta: { + cached: true, + legacyPayload, + briefingless: !carriesBriefing, + }, + }); + return apiSuccess({ + insights: cached, + cached: true, + cachedAt: dbUser.insightsCachedAt, + legacyPayload, + }); + } + briefinglessCacheFallback = { + cached, legacyPayload, + cachedAt: dbUser.insightsCachedAt, + }; + annotate({ + action: { name: "insights.generate.briefingless_cache_bypassed" }, + meta: { locale }, }); } catch { // Invalid cache, regenerate @@ -346,6 +399,22 @@ export const POST = apiHandler((request: NextRequest) => 60 * 60 * 1000, ); if (!rl.allowed) { + // v1.28.30 — a POST-as-read caller whose fresh-but-briefingless cache + // fell through to regeneration degrades to the cached payload when + // the hourly quota is exhausted, instead of surfacing a 429 it never + // used to see. Explicit `force` keeps the honest 429. + if (briefinglessCacheFallback !== null) { + annotate({ + action: { name: "insights.generate" }, + meta: { cached: true, rate_limited_fallback: true }, + }); + return apiSuccess({ + insights: briefinglessCacheFallback.cached, + cached: true, + cachedAt: briefinglessCacheFallback.cachedAt, + legacyPayload: briefinglessCacheFallback.legacyPayload, + }); + } return apiError(`Maximum ${limit} insight generations per hour.`, 429); } @@ -855,6 +924,20 @@ export const POST = apiHandler((request: NextRequest) => }, }); + // v1.28.30 — persist the grounding omission (marker AFTER the cache + // write so it is newer than `insightsCachedAt`). The transient + // `briefingOmittedReason` on this response only reached the caller that + // forced the regenerate; every later read showed a generic "no briefing + // yet". The dated marker lets the GET surface the same honest + // "briefing withheld" state until the next successful generation. + if (briefingOmittedReason === "ungrounded") { + void recordBriefingFailure({ + userId, + reason: "briefing-ungrounded", + locale, + }); + } + // v1.16.8 — no blanket per-status eviction here any more (nuking ~45 // cache rows per manual regenerate deleted every hash baseline and was // the source of the post-regenerate card-regeneration storm). Instead, diff --git a/src/components/insights/__tests__/use-insights-advisor-force.test.ts b/src/components/insights/__tests__/use-insights-advisor-force.test.ts new file mode 100644 index 000000000..5348f35b5 --- /dev/null +++ b/src/components/insights/__tests__/use-insights-advisor-force.test.ts @@ -0,0 +1,76 @@ +/** + * v1.28.30 — wire-contract for the explicit regenerate. + * + * House rule: generation = nightly cron + explicit button. The button's + * mutation therefore MUST issue `POST { force: true }` — without `force` + * the route's 24 h short-circuit re-serves the cached payload and the + * button reads as doing nothing (one half of the recurring + * "no briefing today" chain). The read path stays a plain GET. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const apiFetchRaw = vi.fn(); + +vi.mock("@/lib/api/api-fetch", () => ({ + apiFetchRaw: (...a: unknown[]) => apiFetchRaw(...a), +})); + +import { fetchAdvisor } from "../use-insights-advisor"; + +function jsonResponse(data: unknown): Response { + return { + ok: true, + status: 200, + json: async () => ({ data }), + } as unknown as Response; +} + +beforeEach(() => { + vi.clearAllMocks(); + apiFetchRaw.mockResolvedValue( + jsonResponse({ insights: { dailyBriefing: null }, cached: false }), + ); +}); + +describe("fetchAdvisor — explicit regenerate forces a generation", () => { + it("POSTs force: true on the regenerate path", async () => { + const result = await fetchAdvisor({ force: true }); + + expect(apiFetchRaw).toHaveBeenCalledTimes(1); + const [url, init] = apiFetchRaw.mock.calls[0] as [ + string, + { method: string; body?: string }, + ]; + expect(url).toBe("/api/insights/generate"); + expect(init.method).toBe("POST"); + expect(JSON.parse(init.body ?? "{}")).toEqual({ force: true }); + expect(result.outcome).toBe("fresh"); + }); + + it("uses a read-only GET (no body, no force) on the read path", async () => { + await fetchAdvisor(); + + expect(apiFetchRaw).toHaveBeenCalledTimes(1); + const [url, init] = apiFetchRaw.mock.calls[0] as [ + string, + { method: string; body?: string }, + ]; + expect(url).toBe("/api/insights/generate"); + expect(init.method).toBe("GET"); + expect(init.body).toBeUndefined(); + }); + + it("lifts briefingOmittedReason through the payload for the card", async () => { + apiFetchRaw.mockResolvedValue( + jsonResponse({ + insights: { dailyBriefing: null }, + cached: true, + briefingOmittedReason: "ungrounded", + }), + ); + + const result = await fetchAdvisor(); + expect(result.payload?.briefingOmittedReason).toBe("ungrounded"); + expect(result.payload?.dailyBriefing).toBeNull(); + }); +}); diff --git a/src/components/insights/use-insights-advisor.ts b/src/components/insights/use-insights-advisor.ts index c6a504a0b..2f11cc4c7 100644 --- a/src/components/insights/use-insights-advisor.ts +++ b/src/components/insights/use-insights-advisor.ts @@ -92,8 +92,12 @@ export interface InsightAdvisorPayload { * v1.28.28 (#470) — set by the POST when the number-grounding gate stripped * the freshly generated briefing (after its one corrective retry). Without * it the 200 carried a silently-null briefing and the card's "no briefing - * yet" made the regenerate button read as doing nothing. Only ever present - * on a force-regenerate response; absent on the read path. + * yet" made the regenerate button read as doing nothing. + * + * v1.28.30 — also carried by the read-only GET, backed by a dated + * server-side marker: a briefing the nightly warm stripped now renders + * the same honest "withheld" state on every read until the next + * successful generation, not only on the regenerate response. */ briefingOmittedReason?: "ungrounded" | null; } @@ -169,7 +173,14 @@ interface AdvisorFetchResult { outcome: AdvisorFetchOutcome; } -async function fetchAdvisor( +/** + * Exported as a test seam (like `nextAdvisorPollInterval`): the explicit + * regenerate MUST post `force: true` — generation happens on the nightly + * cron and on explicit intent only, and a force-less button silently + * re-reads the 24 h cache (the "button does nothing" half of the + * no-briefing-today chain). The wire-contract test pins it. + */ +export async function fetchAdvisor( options: { force?: boolean } = {}, ): Promise { const controller = new AbortController(); @@ -409,9 +420,10 @@ export function useInsightsAdvisorQuery( generationFailed: query.data?.payload?.generationFailed ?? false, // Absent → no specific lever hint; the generic failed-description holds. generationFailureClass: query.data?.payload?.generationFailureClass ?? null, - // v1.28.28 (#470) — only a force-regenerate response carries the field - // (setQueryData writes that response into this cache); the read path's - // GET never does, so the hint clears on the next successful read. + // v1.28.28 (#470) — a force-regenerate response carries the field via + // setQueryData; since v1.28.30 the GET carries it too (marker-backed), + // so the "withheld" state persists across reads and clears on the next + // successful generation. briefingOmittedReason: query.data?.payload?.briefingOmittedReason ?? null, }; } diff --git a/src/lib/insights/__tests__/comprehensive-generate-force.test.ts b/src/lib/insights/__tests__/comprehensive-generate-force.test.ts index 318b5b665..92654eb4c 100644 --- a/src/lib/insights/__tests__/comprehensive-generate-force.test.ts +++ b/src/lib/insights/__tests__/comprehensive-generate-force.test.ts @@ -246,6 +246,49 @@ describe("generateComprehensiveInsight — content-hash gate (v1.16.8)", () => { expect(runRawCompletionWithFallback).toHaveBeenCalledTimes(1); }); + it("regenerates when the hash matches but the cached payload carries NO briefing (v1.28.30)", async () => { + // A grounding-stripped (or model-omitted) briefing left a briefingless + // payload WITH a stored hash: on unchanged data every warm re-stamped + // the timestamp and the briefing never came back. A briefingless cache + // must not satisfy the unchanged gate. + makeByokChain(); + findUnique.mockResolvedValue({ + insightsPrivacyMode: "aggregated", + insightsCachedAt: new Date(Date.now() - 26 * 60 * 60 * 1000), + insightsCachedText: JSON.stringify({ + dailyBriefing: null, + recommendations: [], + }), + insightsExcludeMetrics: [], + insightsSnapshotHash: FEATURES_HASH, + }); + runRawCompletionWithFallback.mockResolvedValue({ + result: { + content: JSON.stringify({ dailyBriefing: { p: "recovered" } }), + tokensUsed: 10, + providerType: "openai", + model: "m", + }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }); + + const outcome = await generateComprehensiveInsight("u1", { + locale: "de", + force: true, + }); + + expect(outcome).toEqual({ status: "generated", providerType: "openai" }); + expect(runRawCompletionWithFallback).toHaveBeenCalledTimes(1); + // A real cache write happened (not a timestamp-only refresh). + const write = userUpdate.mock.calls.find( + (c) => + (c[0] as { data: Record }).data.insightsCachedText !== + undefined, + ); + expect(write).toBeTruthy(); + }); + it("does not treat a matching hash as unchanged when there is no cached text to serve", async () => { makeByokChain(); findUnique.mockResolvedValue({ diff --git a/src/lib/insights/briefing-payload.ts b/src/lib/insights/briefing-payload.ts new file mode 100644 index 000000000..d507c0948 --- /dev/null +++ b/src/lib/insights/briefing-payload.ts @@ -0,0 +1,26 @@ +/** + * v1.28.30 — does a cached comprehensive payload actually carry a daily + * briefing? + * + * Shared by the generator's content-hash gate and the POST route's 24 h + * short-circuit: a fresh-but-briefingless cache must not satisfy a + * briefing-expecting caller for the rest of the day (the "one silent + * nightly failure + 24 h cache = a full day without a briefing" chain). + * Unparseable text counts as briefingless — regeneration repairs it. + * + * Lives in its own leaf module (no provider-tree imports) so the route + * can consume it without pulling the whole generation pipeline into its + * bundle, and tests that mock `comprehensive-generate` keep the real + * implementation. + */ +export function cachedPayloadCarriesBriefing( + cachedText: string | null | undefined, +): boolean { + if (!cachedText) return false; + try { + const parsed = JSON.parse(cachedText) as Record; + return parsed.dailyBriefing != null; + } catch { + return false; + } +} diff --git a/src/lib/insights/comprehensive-generate.ts b/src/lib/insights/comprehensive-generate.ts index 547e655f0..238fe1ed0 100644 --- a/src/lib/insights/comprehensive-generate.ts +++ b/src/lib/insights/comprehensive-generate.ts @@ -86,6 +86,7 @@ import { annotate } from "@/lib/logging/context"; import { AI_BUDGETS, resolveInsightsMaxTokens } from "@/lib/ai/ai-budgets"; import { resolveEffectiveTimeoutMs } from "@/lib/ai/effective-timeout"; import { recordBriefingFailure } from "@/lib/insights/briefing-failure-marker"; +import { cachedPayloadCarriesBriefing } from "@/lib/insights/briefing-payload"; // Scope invalidation + refill moved to a sibling module; re-exported so // every existing call site keeps importing from here. @@ -406,6 +407,35 @@ export async function buildComparisonSnapshotForUser( return { baseline, metrics }; } +/** + * v1.28.30 — typed `failed` return for the pre-provider stages (feature + * extraction / oversize payload). These paths used to return + * `{ status: "failed" }` with NO annotation and NO failure marker: the + * nightly cron counted them into its `failed` tally silently, and the + * read path could not tell the user the last attempt failed. One helper + * so every failure return is greppable under the same action name and + * leaves a dated marker for the briefing card's honest empty state. + */ +function failBeforeProvider( + userId: string, + locale: "de" | "en", + reason: "payload-too-large" | "features-error" | "aborted", + err?: unknown, +): GenerateOutcome { + const message = + err instanceof Error + ? err.message.slice(0, 240) + : err != null + ? String(err).slice(0, 240) + : null; + annotate({ + action: { name: "insights.generate.comprehensive_failed" }, + meta: { locale, reason, provider_status: null, provider_message: message }, + }); + void recordBriefingFailure({ userId, reason, locale }); + return { status: "failed", reason }; +} + interface GenerateOptions { /** Resolved UI locale for the prompt + cache row. */ locale: "de" | "en"; @@ -544,15 +574,20 @@ export async function generateComprehensiveInsight( aggregated, MAX_DOWNGRADE_TOKENS, ); - } catch { - return { status: "failed", reason: "payload-too-large" }; + } catch (finalErr) { + return failBeforeProvider( + userId, + locale, + "payload-too-large", + finalErr, + ); } } else { - return { status: "failed", reason: "features-error" }; + return failBeforeProvider(userId, locale, "features-error", retryErr); } } } else { - return { status: "failed", reason: "features-error" }; + return failBeforeProvider(userId, locale, "features-error", err); } } @@ -651,9 +686,30 @@ export async function generateComprehensiveInsight( referenceMetrics, ); + // v1.28.30 — the unchanged short-circuit only holds when the cached + // payload actually carries a briefing. A grounding-stripped (or model- + // omitted) briefing left a briefingless payload WITH a stored hash; + // on unchanged data every subsequent warm re-stamped the timestamp and + // the briefing never came back — a permanent silent hole. Treating the + // briefingless cache as "not unchanged" re-runs the full generation + // (bounded by the nightly budget bucket + forced-warm caps). + const cachedCarriesBriefing = cachedPayloadCarriesBriefing( + dbUser?.insightsCachedText, + ); if ( dbUser?.insightsCachedText && - dbUser.insightsSnapshotHash === snapshotHash + dbUser.insightsSnapshotHash === snapshotHash && + !cachedCarriesBriefing + ) { + annotate({ + action: { name: "insights.generate.briefingless_cache_regenerate" }, + meta: { locale }, + }); + } + if ( + dbUser?.insightsCachedText && + dbUser.insightsSnapshotHash === snapshotHash && + cachedCarriesBriefing ) { // v1.18.7 (MEDIUM-3) — findings are byte-stable, but re-roll the daily- // briefing PARAGRAPH once per calendar day so the prose reads fresh @@ -811,12 +867,41 @@ export async function generateComprehensiveInsight( }); result = retry.result; workingProviderType = retry.workingProvider.providerType; - } catch { + } catch (retryErr) { + // v1.28.30 — annotate alongside the marker so an invalid-JSON night + // is greppable under the same action as every other failure class. + annotate({ + action: { name: "insights.generate.comprehensive_failed" }, + meta: { + locale, + reason: "invalid-json", + provider_status: null, + provider_message: + retryErr instanceof Error + ? retryErr.message.slice(0, 240) + : "retry-transport-failed", + }, + }); void recordBriefingFailure({ userId, reason: "invalid-json", locale }); return { status: "failed", reason: "invalid-json" }; } insights = parseComprehensiveResult(result.content); if (insights === null) { + annotate({ + action: { name: "insights.generate.comprehensive_failed" }, + meta: { + locale, + reason: "invalid-json", + provider_status: null, + // v1.28.28 truncation-aware: a `length` finish means the token + // ceiling cut the JSON mid-string — a budget problem, not a + // model-quality one. + provider_message: + result.finishReason === "length" + ? "response-truncated-at-token-ceiling" + : "unparseable-after-retry", + }, + }); void recordBriefingFailure({ userId, reason: "invalid-json", locale }); return { status: "failed", reason: "invalid-json" }; } @@ -827,6 +912,11 @@ export async function generateComprehensiveInsight( // enforces the same cross-check: every number the briefing restates must // trace to a `features.signalsOfDay` figure. One corrective retry, then strip // the briefing rather than persist a fabricated number. + // v1.28.30 — true when the grounding gate stripped the briefing HARD (no + // previous-cached fallback available). Persisted as a dated marker after + // the cache write so the read path can render the honest "briefing + // withheld" state instead of a silent generic empty card. + let briefingStrippedHard = false; { const signals = features.signalsOfDay ?? null; let retryTransportFailed = false; @@ -895,6 +985,7 @@ export async function generateComprehensiveInsight( } } (insights as Record).dailyBriefing = fallbackBriefing; + briefingStrippedHard = fallbackBriefing == null; annotate({ action: { name: "insights.generate.briefing_grounding_stripped" }, meta: { @@ -913,7 +1004,10 @@ export async function generateComprehensiveInsight( // stamp a timestamp/text the caller no longer expects. The provider call // already cost what it cost; what we must NOT do now is touch the cache. if (signal?.aborted) { - return { status: "failed", reason: "aborted" }; + // v1.28.30 — annotated + marked like every other failure: an abandoned + // generation is a real "no fresh briefing tonight" outcome, and the read + // path needs the marker to render an honest state instead of silence. + return failBeforeProvider(userId, locale, "aborted"); } await prisma.user.update({ @@ -931,5 +1025,17 @@ export async function generateComprehensiveInsight( // every warm had to regenerate ~45 cards across both locales. invalidateUserInsights(userId); + // v1.28.30 — the generation SUCCEEDED but the grounding gate withheld the + // briefing. Record the marker AFTER the cache write so it is newer than + // `insightsCachedAt` and the read path surfaces "briefing withheld" + // instead of a silent generic empty card until the next generation. + if (briefingStrippedHard) { + void recordBriefingFailure({ + userId, + reason: "briefing-ungrounded", + locale, + }); + } + return { status: "generated", providerType: workingProviderType }; } diff --git a/src/lib/jobs/__tests__/insight-pregenerate-retry-enqueue.test.ts b/src/lib/jobs/__tests__/insight-pregenerate-retry-enqueue.test.ts new file mode 100644 index 000000000..01422fd6f --- /dev/null +++ b/src/lib/jobs/__tests__/insight-pregenerate-retry-enqueue.test.ts @@ -0,0 +1,67 @@ +/** + * v1.28.30 — bounded intra-day retry for a failed nightly comprehensive + * warm. Pins the enqueue contract: delayed start (~45 min), per-user + * singleton key distinct from the on-demand `force:` key, small retry + * policy, and a clean no-op (no throw) when no boss instance exists. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const send = vi.fn(); +let boss: { send: typeof send } | null = null; + +vi.mock("@/lib/jobs/boss-instance", () => ({ + getGlobalBoss: () => boss, +})); + +import { + enqueuePregenerateFailureRetry, + PREGENERATE_RETRY_DELAY_SECONDS, + INSIGHT_PREGENERATE_QUEUE, +} from "../insight-pregenerate-shared"; + +beforeEach(() => { + vi.clearAllMocks(); + boss = { send }; +}); + +describe("enqueuePregenerateFailureRetry", () => { + it("enqueues one delayed forced single-user warm with a per-user retry singleton", async () => { + await enqueuePregenerateFailureRetry({ userId: "u1", locale: "de" }); + + expect(send).toHaveBeenCalledTimes(1); + const [queue, payload, options] = send.mock.calls[0] as [ + string, + Record, + Record, + ]; + expect(queue).toBe(INSIGHT_PREGENERATE_QUEUE); + // Same forced single-user payload the on-demand warm uses, so the + // worker routes it through `forceWarmUser` (freshness re-check, + // failure backoff, daily cap all apply at execution time). + expect(payload).toEqual({ userId: "u1", force: true, locale: "de" }); + // Delayed so a transient nightly hiccup has time to clear; distinct + // singleton key so a page-open force warm cannot swallow the retry. + expect(options.startAfter).toBe(PREGENERATE_RETRY_DELAY_SECONDS); + expect(options.singletonKey).toBe("retry:u1"); + }); + + it("keeps the retry window inside the same morning (30-60 min)", () => { + expect(PREGENERATE_RETRY_DELAY_SECONDS).toBeGreaterThanOrEqual(30 * 60); + expect(PREGENERATE_RETRY_DELAY_SECONDS).toBeLessThanOrEqual(60 * 60); + }); + + it("no-ops without throwing when no boss instance is available", async () => { + boss = null; + await expect( + enqueuePregenerateFailureRetry({ userId: "u1", locale: "en" }), + ).resolves.toBeUndefined(); + expect(send).not.toHaveBeenCalled(); + }); + + it("swallows a send failure (best-effort; the nightly tick stays the catch-net)", async () => { + send.mockRejectedValueOnce(new Error("boss unavailable")); + await expect( + enqueuePregenerateFailureRetry({ userId: "u1", locale: "de" }), + ).resolves.toBeUndefined(); + }); +}); diff --git a/src/lib/jobs/__tests__/insight-pregenerate.test.ts b/src/lib/jobs/__tests__/insight-pregenerate.test.ts index 219204e0b..d3b359212 100644 --- a/src/lib/jobs/__tests__/insight-pregenerate.test.ts +++ b/src/lib/jobs/__tests__/insight-pregenerate.test.ts @@ -19,10 +19,23 @@ import path from "node:path"; const checkRateLimit = vi.fn(); const getAssistantFlags = vi.fn(); +const annotateSpy = vi.fn(); vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: (...a: unknown[]) => checkRateLimit(...a), })); +// v1.28.30 — spy on annotate so the no-silent-failure contract is pinned: +// every failure increment must emit a queryable action. Spread the actual +// module so `getEvent` (used by the feature-cache scope) stays real. +vi.mock("@/lib/logging/context", async () => { + const actual = await vi.importActual( + "@/lib/logging/context", + ); + return { + ...actual, + annotate: (...a: unknown[]) => annotateSpy(...a), + }; +}); vi.mock("@/lib/feature-flags", () => ({ getAssistantFlags: (...a: unknown[]) => getAssistantFlags(...a), })); @@ -342,6 +355,170 @@ describe("runInsightPregenerate — outcome tally", () => { }); }); +// v1.28.30 — the recurring "no briefing today" chain started with a nightly +// failure that was invisible: a generator resolving `{ status: "failed" }` +// bumped the tally through a switch case with NO annotation (only the +// bounded timeout/error path was annotated), and the failed user was not +// re-attempted until the next night. These tests pin both halves of the +// fix: every failure path annotates `insights.pregenerate.comprehensive_failed` +// with a stage + cause, and every failure enqueues exactly one bounded +// intra-day retry for that user. +describe("runInsightPregenerate — failure visibility + intra-day retry (v1.28.30)", () => { + function failureAnnotations() { + return annotateSpy.mock.calls + .map( + (call) => + call[0] as { + action?: { name?: string }; + meta?: Record; + }, + ) + .filter( + (a) => a.action?.name === "insights.pregenerate.comprehensive_failed", + ); + } + + it("annotates a generator-returned `failed` outcome (the previously silent switch case) and enqueues one retry", async () => { + const { prisma } = makePrisma([{ id: "u1", locale: "de" }]); + const generate = vi + .fn() + .mockResolvedValue({ status: "failed", reason: "invalid-json" }); + const statusGenerators = Array.from({ length: 7 }, () => + vi.fn().mockResolvedValue({ hasProvider: true, cached: true }), + ); + const warmGenericMetrics = vi.fn().mockResolvedValue(0); + const enqueueRetry = vi.fn().mockResolvedValue(undefined); + + const result = await runInsightPregenerate(prisma as never, { + generate, + statusGenerators, + warmGenericMetrics, + enqueueRetry, + }); + + expect(result.failed).toBe(1); + const failures = failureAnnotations(); + expect(failures).toHaveLength(1); + expect(failures[0].meta).toMatchObject({ + locale: "de", + stage: "nightly.generator", + cause: "generator:invalid-json", + }); + expect(enqueueRetry).toHaveBeenCalledTimes(1); + expect(enqueueRetry).toHaveBeenCalledWith({ userId: "u1", locale: "de" }); + }); + + it("annotates a thrown generator error with its message and enqueues one retry", async () => { + const { prisma } = makePrisma([{ id: "u1", locale: "en" }]); + const generate = vi + .fn() + .mockRejectedValue(new Error("ECONNRESET upstream")); + const statusGenerators = Array.from({ length: 7 }, () => + vi.fn().mockResolvedValue({ hasProvider: true, cached: true }), + ); + const warmGenericMetrics = vi.fn().mockResolvedValue(0); + const enqueueRetry = vi.fn().mockResolvedValue(undefined); + + const result = await runInsightPregenerate(prisma as never, { + generate, + statusGenerators, + warmGenericMetrics, + enqueueRetry, + }); + + expect(result.failed).toBe(1); + const failures = failureAnnotations(); + expect(failures).toHaveLength(1); + expect(failures[0].meta).toMatchObject({ + locale: "en", + stage: "nightly.bound", + cause: "error", + message: "ECONNRESET upstream", + }); + expect(enqueueRetry).toHaveBeenCalledWith({ userId: "u1", locale: "en" }); + }); + + it("annotates a bounded-budget timeout and enqueues one retry", async () => { + const { prisma } = makePrisma([{ id: "u1", locale: "de" }]); + const generate = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + setTimeout( + () => resolve({ status: "generated", providerType: "x" }), + 260_000, + ); + }), + ); + const statusGenerators = Array.from({ length: 7 }, () => + vi.fn().mockResolvedValue({ hasProvider: true, cached: true }), + ); + const warmGenericMetrics = vi.fn().mockResolvedValue(0); + const enqueueRetry = vi.fn().mockResolvedValue(undefined); + + vi.useFakeTimers(); + try { + const promise = runInsightPregenerate(prisma as never, { + generate, + statusGenerators, + warmGenericMetrics, + enqueueRetry, + }); + await vi.advanceTimersByTimeAsync(230_000); + const result = await promise; + + expect(result.failed).toBe(1); + const failures = failureAnnotations(); + expect(failures).toHaveLength(1); + expect(failures[0].meta).toMatchObject({ + stage: "nightly.bound", + cause: "timeout", + }); + expect(enqueueRetry).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + + it("does NOT enqueue a retry for generated / cached / unchanged / skipped / budget-blocked users", async () => { + const { prisma } = makePrisma([ + { id: "a", locale: "de" }, + { id: "b", locale: "de" }, + { id: "c", locale: "de" }, + { id: "d", locale: "de" }, + { id: "e", locale: "de" }, + ]); + // e is budget-blocked; a-d run the generator. + checkRateLimit + .mockResolvedValueOnce({ allowed: true }) + .mockResolvedValueOnce({ allowed: true }) + .mockResolvedValueOnce({ allowed: true }) + .mockResolvedValueOnce({ allowed: true }) + .mockResolvedValueOnce({ allowed: false }); + const generate = vi + .fn() + .mockResolvedValueOnce({ status: "generated", providerType: "x" }) + .mockResolvedValueOnce({ status: "cached" }) + .mockResolvedValueOnce({ status: "unchanged" }) + .mockResolvedValueOnce({ status: "skipped", reason: "no-provider" }); + const statusGenerators = Array.from({ length: 7 }, () => + vi.fn().mockResolvedValue({ hasProvider: true, cached: true }), + ); + const warmGenericMetrics = vi.fn().mockResolvedValue(0); + const enqueueRetry = vi.fn().mockResolvedValue(undefined); + + const result = await runInsightPregenerate(prisma as never, { + generate, + statusGenerators, + warmGenericMetrics, + enqueueRetry, + }); + + expect(result.failed).toBe(0); + expect(enqueueRetry).not.toHaveBeenCalled(); + expect(failureAnnotations()).toHaveLength(0); + }); +}); + describe("runInsightPregenerate — per-metric warm pass", () => { function warmGen(result: { hasProvider: boolean; cached: boolean }) { return vi.fn().mockResolvedValue(result); diff --git a/src/lib/jobs/insight-pregenerate-shared.ts b/src/lib/jobs/insight-pregenerate-shared.ts index 4ca7cfaa2..d4afd79db 100644 --- a/src/lib/jobs/insight-pregenerate-shared.ts +++ b/src/lib/jobs/insight-pregenerate-shared.ts @@ -92,3 +92,74 @@ export async function enqueueForceWarm(payload: { // cold until the next poll / nightly cron warms them. } } + +/** + * Delay before the single bounded retry of a failed nightly comprehensive + * warm. 45 minutes: far enough out that a transient 02:30–04:30 provider + * brown-out has usually cleared, early enough that the retry lands before + * the user's morning visit. + */ +export const PREGENERATE_RETRY_DELAY_SECONDS = 45 * 60; + +/** + * Bounded intra-day retry for a user whose NIGHTLY comprehensive warm + * failed. Without this, a single 04:30 provider hiccup left the user's + * briefing stale until the next night: the nightly tick is the only + * scheduled machinery, the GET path's warm enqueue only fires once the + * cache crosses the 24 h freshness line (often mid-evening), and the POST + * cache short-circuit served the stale payload all day. + * + * Enqueues ONE delayed forced single-user warm (the same `forceWarmUser` + * pipeline the on-demand route uses), so every existing bound still + * applies at execution time: the job-start freshness re-check, the + * failure backoff, and the per-user daily forced-warm cap. `singletonKey` + * collapses repeated failures within the window into one queued retry. + * Best-effort like `enqueueForceWarm`: no boss instance → annotated no-op, + * the nightly tick remains the catch-net. + */ +export async function enqueuePregenerateFailureRetry(payload: { + userId: string; + locale: "de" | "en"; +}): Promise { + const boss = getGlobalBoss(); + if (!boss) { + annotate({ + action: { name: "insights.pregenerate.retry.no_boss" }, + meta: { locale: payload.locale }, + }); + return; + } + try { + await boss.send( + INSIGHT_PREGENERATE_QUEUE, + { + userId: payload.userId, + force: true, + locale: payload.locale, + } satisfies InsightPregeneratePayload, + { + startAfter: PREGENERATE_RETRY_DELAY_SECONDS, + // Distinct from the on-demand `force:` key so a user's page-open + // warm can never swallow the scheduled retry (and vice versa); + // one retry per user per window. + singletonKey: `retry:${payload.userId}`, + singletonSeconds: 3600, + // A worker crash mid-warm re-runs the job; a warm that completes + // with a failed comprehensive does NOT throw, so this cannot loop. + retryLimit: 2, + retryDelay: 300, + retryBackoff: true, + }, + ); + annotate({ + action: { name: "insights.pregenerate.retry.enqueued" }, + meta: { + locale: payload.locale, + delay_seconds: PREGENERATE_RETRY_DELAY_SECONDS, + }, + }); + } catch { + // Best-effort: a failed enqueue leaves the nightly tick as the + // catch-net, exactly the pre-retry behaviour. + } +} diff --git a/src/lib/jobs/insight-pregenerate.ts b/src/lib/jobs/insight-pregenerate.ts index 9648014d2..d2d1f7da2 100644 --- a/src/lib/jobs/insight-pregenerate.ts +++ b/src/lib/jobs/insight-pregenerate.ts @@ -72,6 +72,7 @@ import { resolveEffectiveTimeoutMs } from "@/lib/ai/effective-timeout"; import { INSIGHT_PREGENERATE_QUEUE, enqueueForceWarm, + enqueuePregenerateFailureRetry, type InsightPregeneratePayload, } from "@/lib/jobs/insight-pregenerate-shared"; @@ -226,6 +227,17 @@ export function comprehensiveWarmBudgetMs( ); } +/** + * Short, redaction-safe error description for the failure annotations + * below. The central egress redaction (`redactSecrets` in the + * WideEventBuilder) is the safety net; keeping the excerpt short bounds + * the event size and avoids echoing whole provider bodies. + */ +function describeError(err: unknown): string { + const message = err instanceof Error ? err.message : String(err ?? "unknown"); + return message.slice(0, 240); +} + /** Per-user result of one forced full warm. */ export interface ForceWarmResult { /** @@ -321,9 +333,19 @@ async function warmPerStatusCaches( try { const outcome = await generate(userId, { locale, force: false }); return outcome.hasProvider && !outcome.cached ? 1 : 0; - } catch { + } catch (err) { // A single card's generation failing must not abort the rest of - // the warm pass for this user, nor the cron's user loop. + // the warm pass for this user, nor the cron's user loop — but it + // must not be invisible either: annotate so a failing card is + // diagnosable from one grep instead of a silent zero. + annotate({ + action: { name: "insights.pregenerate.warm_failed" }, + meta: { + stage: "status-card", + locale, + message: describeError(err), + }, + }); return 0; } }), @@ -360,8 +382,17 @@ async function warmStatusBatch( force: false, }); warmed += result.batched + result.fellBack; - } catch { - // The batch must never throw the cron's user loop off course. + } catch (err) { + // The batch must never throw the cron's user loop off course — but + // a swallowed batch failure left ~7 cards cold with zero signal. + annotate({ + action: { name: "insights.pregenerate.warm_failed" }, + meta: { + stage: "status-batch", + locale, + message: describeError(err), + }, + }); } } return warmed; @@ -400,7 +431,13 @@ async function warmGenericMetricCaches( orderBy: { type: "asc" }, }); typesWithData = new Set(rows.map((r) => r.type)); - } catch { + } catch (err) { + // Best-effort discovery — but a silently-failing grouped read used to + // zero out the whole generic warm with no trace. + annotate({ + action: { name: "insights.pregenerate.warm_failed" }, + meta: { stage: "metric-discovery", message: describeError(err) }, + }); return 0; } @@ -434,7 +471,16 @@ async function warmGenericMetricCaches( outcome.insufficient !== true ? 1 : 0; - } catch { + } catch (err) { + annotate({ + action: { name: "insights.pregenerate.warm_failed" }, + meta: { + stage: "metric-card", + metric, + locale, + message: describeError(err), + }, + }); return 0; } }), @@ -522,11 +568,22 @@ export async function runInsightPregenerate( userId: string, locales: ReadonlyArray<"de" | "en">, ) => Promise; + /** + * v1.28.30 — injected for the test. Defaults to the real delayed + * single-user retry enqueue; fired once per candidate whose + * comprehensive warm failed so a transient nightly provider hiccup + * heals within the day instead of waiting for the next tick. + */ + enqueueRetry?: (payload: { + userId: string; + locale: "de" | "en"; + }) => Promise; } = {}, ): Promise { const now = options.now ?? new Date(); const cap = options.cap ?? PREGENERATE_BATCH_CAP; const generate = options.generate ?? generateComprehensiveInsight; + const enqueueRetry = options.enqueueRetry ?? enqueuePregenerateFailureRetry; // v1.18.7 (HIGH-1) — when the caller injects its own `statusGenerators` // (the unit tests do), keep the per-card warm so those assertions hold; // production injects nothing, so the seven warm calls collapse into ONE @@ -608,12 +665,19 @@ export async function runInsightPregenerate( const warmBudgetMs = comprehensiveWarmBudgetMs( candidate.aiResponseTimeoutSeconds, ); + // Capture the rejection so the failure annotation can carry the + // actual error text — `withTimeout` folds a rejection into a bare + // `errored: true` envelope, which made the cause undiagnosable. + let thrown: unknown = null; const bounded = await withTimeout( () => generate(candidate.id, { locale, force: true, signal: controller.signal, + }).catch((err: unknown) => { + thrown = err; + throw err; }), warmBudgetMs, null, @@ -625,17 +689,26 @@ export async function runInsightPregenerate( // bump `failed` with no annotation, so a candidate whose comprehensive // generation timed out (e.g. the briefing clipped at the old 45 s // bound) left the cached block empty AND emitted nothing greppable. + // v1.28.30 — carry the stage + error text so ONE grep on + // `comprehensive_failed` names the failing catch and its cause. annotate({ action: { name: "insights.pregenerate.comprehensive_failed" }, meta: { locale, + stage: "nightly.bound", cause: bounded.timedOut ? "timeout" : bounded.errored ? "error" : "null", + budget_ms: warmBudgetMs, + message: thrown === null ? null : describeError(thrown), }, }); + // v1.28.30 — bounded intra-day retry: one delayed forced warm for + // just this user, so a transient 04:30 provider hiccup heals + // before the morning visit instead of waiting for the next night. + await enqueueRetry({ userId: candidate.id, locale }); } else { outcome = bounded.value; switch (outcome.status) { @@ -653,6 +726,24 @@ export async function runInsightPregenerate( break; case "failed": result.failed++; + // v1.28.30 — THE previously-silent failure path. A generator + // that resolves with `{ status: "failed" }` (features error, + // oversize payload, invalid JSON after retry, abort) bumped + // the tally with no annotation at all — the nightly summary + // showed `failed: 1` and nothing else was greppable. Reuse + // the `comprehensive_failed` shape with the generator's own + // reason as the cause. + annotate({ + action: { name: "insights.pregenerate.comprehensive_failed" }, + meta: { + locale, + stage: "nightly.generator", + cause: `generator:${outcome.reason}`, + budget_ms: warmBudgetMs, + message: null, + }, + }); + await enqueueRetry({ userId: candidate.id, locale }); break; } } @@ -896,12 +987,17 @@ export async function forceWarmUser( const warmBudgetMs = comprehensiveWarmBudgetMs( freshness?.aiResponseTimeoutSeconds, ); + // v1.28.30 — capture the rejection for the failure annotation. + let thrown: unknown = null; const comprehensive = await withTimeout( () => generate(userId, { locale, force: true, signal: controller.signal, + }).catch((err: unknown) => { + thrown = err; + throw err; }), warmBudgetMs, null, @@ -914,6 +1010,33 @@ export async function forceWarmUser( } else { result.comprehensive = comprehensive.value.status; } + // v1.28.30 — same queryable failure shape as the nightly loop, so + // one grep on `comprehensive_failed` covers both entry points. + if ( + result.comprehensive === "failed" || + result.comprehensive === "timeout" + ) { + annotate({ + action: { name: "insights.pregenerate.comprehensive_failed" }, + meta: { + locale, + stage: "force", + cause: comprehensive.timedOut + ? "timeout" + : comprehensive.errored + ? "error" + : comprehensive.value === null + ? "null" + : `generator:${ + comprehensive.value.status === "failed" + ? comprehensive.value.reason + : comprehensive.value.status + }`, + budget_ms: warmBudgetMs, + message: thrown === null ? null : describeError(thrown), + }, + }); + } // Stamp / clear the failure marker for the backoff gate above. A // `skipped` outcome (no provider / no consent) is not a failure —