refactor(registry): generate bootstrap and health dataset registries#4
Closed
lspassos1 wants to merge 1 commit into
Closed
refactor(registry): generate bootstrap and health dataset registries#4lspassos1 wants to merge 1 commit into
lspassos1 wants to merge 1 commit into
Conversation
Bootstrap and health registrations had drifted across manual maps in api/bootstrap.js, api/health.js, tests, and cache key exports. Move the dataset contracts into registry/datasets.ts, generate the edge/server artifacts, and wire the health and bootstrap endpoints to consume those outputs. Also add registry freshness checks to pre-push and the typecheck workflow. Validation: - npm run registry:check - node --test tests/bootstrap.test.mjs - node --test tests/edge-functions.test.mjs - npm run test:data - npm run typecheck - npm run typecheck:api
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
8 tasks
lspassos1
pushed a commit
that referenced
this pull request
Apr 27, 2026
…a73#3207) (koala73#3242) * chore(api): enforce sebuf contract via exceptions manifest (koala73#3207) Adds api/api-route-exceptions.json as the single source of truth for non-proto /api/ endpoints, with scripts/enforce-sebuf-api-contract.mjs gating every PR via npm run lint:api-contract. Fixes the root-only blind spot in the prior allowlist (tests/edge-functions.test.mjs), which only scanned top-level *.js files and missed nested paths and .ts endpoints — the gap that let api/supply-chain/v1/country-products.ts and friends drift under proto domain URL prefixes unchallenged. Checks both directions: every api/<domain>/v<N>/[rpc].ts must pair with a generated service_server.ts (so a deleted proto fails CI), and every generated service must have an HTTP gateway (no orphaned generated code). Manifest entries require category + reason + owner, with removal_issue mandatory for temporary categories (deferred, migration-pending) and forbidden for permanent ones. .github/CODEOWNERS pins the manifest to @SebastienMelki so new exceptions don't slip through review. The manifest only shrinks: migration-pending entries (19 today) will be removed as subsequent commits in this PR land each migration. * refactor(maritime): migrate /api/ais-snapshot → maritime/v1.GetVesselSnapshot (koala73#3207) The proto VesselSnapshot was carrying density + disruptions but the frontend also needed sequence, relay status, and candidate_reports to drive the position-callback system. Those only lived on the raw relay passthrough, so the client had to keep hitting /api/ais-snapshot whenever callbacks were registered and fall back to the proto RPC only when the relay URL was gone. This commit pushes all three missing fields through the proto contract and collapses the dual-fetch-path into one proto client call. Proto changes (proto/worldmonitor/maritime/v1/): - VesselSnapshot gains sequence, status, candidate_reports. - GetVesselSnapshotRequest gains include_candidates (query: include_candidates). Handler (server/worldmonitor/maritime/v1/get-vessel-snapshot.ts): - Forwards include_candidates to ?candidates=... on the relay. - Separate 5-min in-memory caches for the candidates=on and candidates=off variants; they have very different payload sizes and should not share a slot. - Per-request in-flight dedup preserved per-variant. Frontend (src/services/maritime/index.ts): - fetchSnapshotPayload now calls MaritimeServiceClient.getVesselSnapshot directly with includeCandidates threaded through. The raw-relay path, SNAPSHOT_PROXY_URL, DIRECT_RAILWAY_SNAPSHOT_URL and LOCAL_SNAPSHOT_FALLBACK are gone — production already routed via Vercel, the "direct" branch only ever fired on localhost, and the proto gateway covers both. - New toLegacyCandidateReport helper mirrors toDensityZone/toDisruptionEvent. api/ais-snapshot.js deleted; manifest entry removed. Only reduced the codegen scope to worldmonitor.maritime.v1 (buf generate --path) — regenerating the full tree drops // @ts-nocheck from every client/server file and surfaces pre-existing type errors across 30+ unrelated services, which is not in scope for this PR. Shape-diff vs legacy payload: - disruptions / density: proto carries the same fields, just with the GeoCoordinates wrapper and enum strings (remapped client-side via existing toDisruptionEvent / toDensityZone helpers). - sequence, status.{connected,vessels,messages}: now populated from the proto response — was hardcoded to 0/false in the prior proto fallback. - candidateReports: same shape; optional numeric fields come through as 0 instead of undefined, which the legacy consumer already handled. * refactor(sanctions): migrate /api/sanctions-entity-search → LookupSanctionEntity (koala73#3207) The proto docstring already claimed "OFAC + OpenSanctions" coverage but the handler only fuzzy-matched a local OFAC Redis index — narrower than the legacy /api/sanctions-entity-search, which proxied OpenSanctions live (the source advertised in docs/api-proxies.mdx). Deleting the legacy without expanding the handler would have been a silent coverage regression for external consumers. Handler changes (server/worldmonitor/sanctions/v1/lookup-entity.ts): - Primary path: live search against api.opensanctions.org/search/default with an 8s timeout and the same User-Agent the legacy edge fn used. - Fallback path: the existing OFAC local fuzzy match, kept intact for when OpenSanctions is unreachable / rate-limiting. - Response source field flips between 'opensanctions' (happy path) and 'ofac' (fallback) so clients can tell which index answered. - Query validation tightened: rejects q > 200 chars (matches legacy cap). Rate limiting: - Added /api/sanctions/v1/lookup-entity to ENDPOINT_RATE_POLICIES at 30/min per IP — matches the legacy createIpRateLimiter budget. The gateway already enforces per-endpoint policies via checkEndpointRateLimit. Docs: - docs/api-proxies.mdx — dropped the /api/sanctions-entity-search row (plus the orphaned /api/ais-snapshot row left over from the previous commit in this PR). - docs/panels/sanctions-pressure.mdx — points at the new RPC URL and describes the OpenSanctions-primary / OFAC-fallback semantics. api/sanctions-entity-search.js deleted; manifest entry removed. * refactor(military): migrate /api/military-flights → ListMilitaryFlights (koala73#3207) Legacy /api/military-flights read a pre-baked Redis blob written by the seed-military-flights cron and returned flights in a flat app-friendly shape (lat/lon, lowercase enums, lastSeenMs). The proto RPC takes a bbox, fetches OpenSky live, classifies server-side, and returns nested GeoCoordinates + MILITARY_*_TYPE_* enum strings + lastSeenAt — same data, different contract. fetchFromRedis in src/services/military-flights.ts was doing nothing sebuf-aware. Renamed it to fetchViaProto and rewrote to: - Instantiate MilitaryServiceClient against getRpcBaseUrl(). - Iterate MILITARY_QUERY_REGIONS (PACIFIC + WESTERN) in parallel — same regions the desktop OpenSky path and the seed cron already use, so dashboard coverage tracks the analytic pipeline. - Dedup by hexCode across regions. - Map proto → app shape via new mapProtoFlight helper plus three reverse enum maps (AIRCRAFT_TYPE_REVERSE, OPERATOR_REVERSE, CONFIDENCE_REVERSE). The seed cron (scripts/seed-military-flights.mjs) stays put: it feeds regional-snapshot mobility, cross-source signals, correlation, and the health freshness check (api/health.js: 'military:flights:v1'). None of those read the legacy HTTP endpoint; they read the Redis key directly. The proto handler uses its own per-bbox cache keys under the same prefix, so dashboard traffic no longer races the seed cron's blob — the two paths diverge by a small refresh lag, which is acceptable. Docs: dropped the /api/military-flights row from docs/api-proxies.mdx. api/military-flights.js deleted; manifest entry removed. Shape-diff vs legacy: - f.location.{latitude,longitude} → f.lat, f.lon - f.aircraftType: MILITARY_AIRCRAFT_TYPE_TANKER → 'tanker' via reverse map - f.operator: MILITARY_OPERATOR_USAF → 'usaf' via reverse map - f.confidence: MILITARY_CONFIDENCE_LOW → 'low' via reverse map - f.lastSeenAt (number) → f.lastSeen (Date) - f.enrichment → f.enriched (with field renames) - Extra fields registration / aircraftModel / origin / destination / firstSeenAt now flow through where proto populates them. * fix(supply-chain): thread includeCandidates through chokepoint status (koala73#3207) Caught by tsconfig.api.json typecheck in the pre-push hook (not covered by the plain tsc --noEmit run that ran before I pushed the ais-snapshot commit). The chokepoint status handler calls getVesselSnapshot internally with a static no-auth request — now required to include the new includeCandidates bool from the proto extension. Passing false: server-internal callers don't need per-vessel reports. * test(maritime): update getVesselSnapshot cache assertions (koala73#3207) The ais-snapshot migration replaced the single cachedSnapshot/cacheTimestamp pair with a per-variant cache so candidates-on and candidates-off payloads don't evict each other. Pre-push hook surfaced that tests/server-handlers still asserted the old variable names. Rewriting the assertions to match the new shape while preserving the invariants they actually guard: - Freshness check against slot TTL. - Cache read before relay call. - Per-slot in-flight dedup. - Stale-serve on relay failure (result ?? slot.snapshot). * chore(proto): restore // @ts-nocheck on regenerated maritime files (koala73#3207) I ran 'buf generate --path worldmonitor/maritime/v1' to scope the proto regen to the one service I was changing (to avoid the toolchain drift that drops @ts-nocheck from 60+ unrelated files — separate issue). But the repo convention is the 'make generate' target, which runs buf and then sed-prepends '// @ts-nocheck' to every generated .ts file. My scoped command skipped the sed step. The proto-check CI enforces the sed output, so the two maritime files need the directive restored. * refactor(enrichment): decomm /api/enrichment/{company,signals} legacy edge fns (koala73#3207) Both endpoints were already ported to IntelligenceService: - getCompanyEnrichment (/api/intelligence/v1/get-company-enrichment) - listCompanySignals (/api/intelligence/v1/list-company-signals) No frontend callers of the legacy /api/enrichment/* paths exist. Removes: - api/enrichment/company.js, signals.js, _domain.js - api-route-exceptions.json migration-pending entries (58 remain) - docs/api-proxies.mdx rows for /api/enrichment/{company,signals} - docs/architecture.mdx reference updated to the IntelligenceService RPCs Verified: typecheck, typecheck:api, lint:api-contract (89 files / 58 entries), lint:boundaries, tests/edge-functions.test.mjs (136 pass), tests/enrichment-caching.test.mjs (14 pass — still guards the intelligence/v1 handlers), make generate is zero-diff. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * refactor(leads): migrate /api/{contact,register-interest} → LeadsService (koala73#3207) New leads/v1 sebuf service with two POST RPCs: - SubmitContact → /api/leads/v1/submit-contact - RegisterInterest → /api/leads/v1/register-interest Handler logic ported 1:1 from api/contact.js + api/register-interest.js: - Turnstile verification (desktop sources bypass, preserved) - Honeypot (website field) silently accepts without upstream calls - Free-email-domain gate on SubmitContact (422 ApiError) - validateEmail (disposable/offensive/typo-TLD/MX) on RegisterInterest - Convex writes via ConvexHttpClient (contactMessages:submit, registerInterest:register) - Resend notification + confirmation emails (HTML templates unchanged) Shared helpers moved to server/_shared/: - turnstile.ts (getClientIp + verifyTurnstile) - email-validation.ts (disposable/offensive/MX checks) Rate limits preserved via ENDPOINT_RATE_POLICIES: - submit-contact: 3/hour per IP (was in-memory 3/hr) - register-interest: 5/hour per IP (was in-memory 5/hr; desktop sources previously capped at 2/hr via shared in-memory map — now 5/hr like everyone else, accepting the small regression in exchange for Upstash-backed global limiting) Callers updated: - pro-test/src/App.tsx contact form → new submit-contact path - src-tauri/sidecar/local-api-server.mjs cloud-fallback rewrites /api/register-interest → /api/leads/v1/register-interest when proxying; keeps local path for older desktop builds - src/services/runtime.ts isKeyFreeApiTarget allows both old and new paths through the WORLDMONITOR_API_KEY-optional gate Tests: - tests/contact-handler.test.mjs rewritten to call submitContact handler directly; asserts on ValidationError / ApiError - tests/email-validation.test.mjs + tests/turnstile.test.mjs point at the new server/_shared/ modules Deleted: api/contact.js, api/register-interest.js, api/_ip-rate-limit.js, api/_turnstile.js, api/_email-validation.js, api/_turnstile.test.mjs. Manifest entries removed (58 → 56). Docs updated (api-platform, api-commerce, usage-rate-limits). Verified: npm run typecheck + typecheck:api + lint:api-contract (88 files / 56 entries) + lint:boundaries pass; full test:data (5852 tests) passes; make generate is zero-diff. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * chore(pro-test): rebuild bundle for leads/v1 contact form (koala73#3207) Updates the enterprise contact form to POST to /api/leads/v1/submit-contact (old path /api/contact removed in the previous commit). Bundle is rebuilt from pro-test/src/App.tsx source change in 9ccd309. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(review): address HIGH review findings 1-3 (koala73#3207) Three review findings from @koala73 on the sebuf-migration PR, all silent bugs that would have shipped to prod: ### 1. Sanctions rate-limit policy was dead code ENDPOINT_RATE_POLICIES keyed the 30/min budget under /api/sanctions/v1/lookup-entity, but the generated route (from the proto RPC LookupSanctionEntity) is /api/sanctions/v1/lookup-sanction-entity. hasEndpointRatePolicy / getEndpointRatelimit are exact-string pathname lookups, so the mismatch meant the endpoint fell through to the generic 600/min global limiter instead of the advertised 30/min. Net effect: the live OpenSanctions proxy endpoint (unauthenticated, external upstream) had 20x the intended rate budget. Fixed by renaming the policy key to match the generated route. ### 2. Lost stale-seed fallback on military-flights Legacy api/military-flights.js cascaded military:flights:v1 → military:flights:stale:v1 before returning empty. The new proto handler went straight to live OpenSky/relay and returned null on miss. Relay or OpenSky hiccup used to serve stale seeded data (24h TTL); under the new handler it showed an empty map. Both keys are still written by scripts/seed-military-flights.mjs on every run — fix just reads the stale key when the live fetch returns null, converts the seed's app-shape flights (flat lat/lon, lowercase enums, lastSeenMs) to the proto shape (nested GeoCoordinates, enum strings, lastSeenAt), and filters to the request bbox. Read via getRawJson (unprefixed) to match the seed cron's writes, which bypass the env-prefix system. ### 3. Hex-code casing mismatch broke getFlightByHex The seed cron writes hexCode: icao24.toUpperCase() (uppercase); src/services/military-flights.ts:getFlightByHex uppercases the lookup input: f.hexCode === hexCode.toUpperCase(). The new proto handler preserved OpenSky's lowercase icao24, and mapProtoFlight is a pass-through. getFlightByHex was silently returning undefined for every call after the migration. Fix: uppercase in the proto handler (live + stale paths), and document the invariant in a comment on MilitaryFlight.hex_code in military_flight.proto so future handlers don't re-break it. ### Verified - typecheck + typecheck:api clean - lint:api-contract (56 entries) / lint:boundaries clean - tests/edge-functions.test.mjs 130 pass - make generate zero-diff (openapi spec regenerated for proto comment) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(review): restore desktop 2/hr rate cap on register-interest (koala73#3207) Addresses HIGH review finding #4 from @koala73. The legacy api/register-interest.js applied a nested 2/hr per-IP cap when `source === 'desktop-settings'`, on top of the generic 5/hr endpoint budget. The sebuf migration lost this — desktop-source requests now enjoy the full 5/hr cap. Since `source` is an unsigned client-supplied field, anyone sending `source: 'desktop-settings'` skips Turnstile AND gets 5/hr. Without the tighter cap the Turnstile bypass is cheaper to abuse. Added `checkScopedRateLimit` to `server/_shared/rate-limit.ts` — a reusable second-stage Upstash limiter keyed on an opaque scope string + caller identifier. Fail-open on Redis errors to match existing checkRateLimit / checkEndpointRateLimit semantics. Handlers that need per-subscope caps on top of the gateway-level endpoint budget use this helper. In register-interest: when `isDesktopSource`, call checkScopedRateLimit with scope `/api/leads/v1/register-interest#desktop`, limit=2, window=1h, IP as identifier. On exceeded → throw ApiError(429). ### What this does not fix This caps the blast radius of the Turnstile bypass but does not close it — an attacker sending `source: 'desktop-settings'` still skips Turnstile (just at 2/hr instead of 5/hr). The proper fix is a signed desktop-secret header that authenticates the bypass; filed as follow-up koala73#3252. That requires coordinated Tauri build + Vercel env changes out of scope for koala73#3207. ### Verified - typecheck + typecheck:api clean - lint:api-contract (56 entries) - tests/edge-functions.test.mjs + contact-handler.test.mjs (147 pass) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(review): MEDIUM + LOW + rate-limit-policy CI check (koala73#3207) Closes out the remaining @koala73 review findings from koala73#3242 that didn't already land in the HIGH-fix commits, plus the requested CI check that would have caught HIGH #1 (dead-code policy key) at review time. ### MEDIUM #5 — Turnstile missing-secret policy default Flip `verifyTurnstile`'s default `missingSecretPolicy` from `'allow'` to `'allow-in-development'`. Dev with no secret = pass (expected local); prod with no secret = reject + log. submit-contact was already explicitly overriding to `'allow-in-development'`; register-interest was silently getting `'allow'`. Safe default now means a future missing-secret misconfiguration in prod gets caught instead of silently letting bots through. Removed the now-redundant override in submit-contact. ### MEDIUM #6 — Silent enum fallbacks in maritime client `toDisruptionEvent` mapped `AIS_DISRUPTION_TYPE_UNSPECIFIED` / unknown enum values → `gap_spike` / `low` silently. Refactored to return null when either enum is unknown; caller filters nulls out of the array. Handler doesn't produce UNSPECIFIED today, but the `gap_spike` default would have mislabeled the first new enum value the proto ever adds — dropping unknowns is safer than shipping wrong labels. ### LOW — Copy drift in register-interest email Email template hardcoded `435+ Sources`; PR koala73#3241 bumped marketing to `500+`. Bumped in the rewritten file to stay consistent. The `as any` on Convex mutation names carried over from legacy and filed as follow-up koala73#3253. ### Rate-limit-policy coverage lint `scripts/enforce-rate-limit-policies.mjs` validates every key in `ENDPOINT_RATE_POLICIES` resolves to a proto-generated gateway route by cross-referencing `docs/api/*.openapi.yaml`. Fails with the sanctions-entity-search incident referenced in the error message so future drift has a paper trail. Wired into package.json (`lint:rate-limit-policies`) and the pre-push hook alongside `lint:boundaries`. Smoke-tested both directions — clean repo passes (5 policies / 175 routes), seeded drift (the exact HIGH #1 typo) fails with the advertised remedy text. ### Verified - `lint:rate-limit-policies` ✓ - `typecheck` + `typecheck:api` ✓ - `lint:api-contract` ✓ (56 entries) - `lint:boundaries` ✓ - edge-functions + contact-handler tests (147 pass) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * refactor(commit 5): decomm /api/eia/* + migrate /api/satellites → IntelligenceService (koala73#3207) Both targets turned out to be decomm-not-migration cases. The original plan called for two new services (economic/v1.GetEiaSeries + natural/v1.ListSatellitePositions) but research found neither was needed: ### /api/eia/[[...path]].js — pure decomm, zero consumers The "catch-all" is a misnomer — only two paths actually worked, /api/eia/health and /api/eia/petroleum, both Redis-only readers. Zero frontend callers in src/. Zero server-side readers. Nothing consumes the `energy:eia-petroleum:v1` key that seed-eia-petroleum.mjs writes daily. The EIA data the frontend actually uses goes through existing typed RPCs in economic/v1: GetEnergyPrices, GetCrudeInventories, GetNatGasStorage, GetEnergyCapacity. None of those touch /api/eia/*. Building GetEiaSeries would have been dead code. Deleted the legacy file + its test (tests/api-eia-petroleum.test.mjs — it only covered the legacy endpoint, no behavior to preserve). Empty api/eia/ dir removed. **Note for review:** the Redis seed cron keeps running daily and nothing consumes it. If that stays unused, seed-eia-petroleum.mjs should be retired too (separate PR). Out of scope for sebuf-migration. ### /api/satellites.js — Learning #2 strikes again IntelligenceService.ListSatellites already exists at /api/intelligence/v1/list-satellites, reads the same Redis key (intelligence:satellites:tle:v1), and supports an optional country filter the legacy didn't have. One frontend caller in src/services/satellites.ts needed to switch from `fetch(toApiUrl('/api/satellites'))` to the typed IntelligenceServiceClient.listSatellites. Shape diff was tiny — legacy `noradId` became proto `id` (handler line 36 already picks either), everything else identical. alt/velocity/inclination in the proto are ignored by the caller since it propagates positions client-side via satellite.js. Kept the client-side cache + failure cooldown + 20s timeout (still valid concerns at the caller level). ### Manifest + docs - api-route-exceptions.json: 56 → 54 entries (both removed) - docs/api-proxies.mdx: dropped the two rows from the Raw-data passthroughs table ### Verified - typecheck + typecheck:api ✓ - lint:api-contract (54 entries) / lint:boundaries / lint:rate-limit-policies ✓ - tests/edge-functions.test.mjs 127 pass (down from 130 — 3 tests were for the deleted eia endpoint) - make generate zero-diff (no proto changes) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * refactor(commit 6): migrate /api/supply-chain/v1/{country-products,multi-sector-cost-shock} → SupplyChainService (koala73#3207) Both endpoints were hand-rolled TS handlers sitting under a proto URL prefix — the exact drift the manifest guardrail flagged. Promoted both to typed RPCs: - GetCountryProducts → /api/supply-chain/v1/get-country-products - GetMultiSectorCostShock → /api/supply-chain/v1/get-multi-sector-cost-shock Handlers preserve the existing semantics: PRO-gate via isCallerPremium(ctx.request), iso2 / chokepointId validation, raw bilateral-hs4 Redis read (skip env-prefix to match seeder writes), CHOKEPOINT_STATUS_KEY for war-risk tier, and the math from _multi-sector-shock.ts unchanged. Empty-data and non-PRO paths return the typed empty payload (no 403 — the sebuf gateway pattern is empty-payload-on-deny). Client wrapper switches from premiumFetch to client.getCountryProducts/ client.getMultiSectorCostShock. Legacy MultiSectorShock / MultiSectorShockResponse / CountryProductsResponse names remain as type aliases of the generated proto types so CountryBriefPanel + CountryDeepDivePanel callsites compile with zero churn. Manifest 54 → 52. Rate-limit gateway routes 175 → 177. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(gateway): add cache-tier entries for new supply-chain RPCs (koala73#3207) Pre-push tests/route-cache-tier.test.mjs caught the missing entries. Both PRO-gated, request-varying — match the existing supply-chain PRO cohort (get-country-cost-shock, get-bypass-options, etc.) at slow-browser tier. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * refactor(commit 7): migrate /api/scenario/v1/{run,status,templates} → ScenarioService (koala73#3207) Promote the three literal-filename scenario endpoints to a typed sebuf service with three RPCs: POST /api/scenario/v1/run-scenario (RunScenario) GET /api/scenario/v1/get-scenario-status (GetScenarioStatus) GET /api/scenario/v1/list-scenario-templates (ListScenarioTemplates) Preserves all security invariants from the legacy handlers: - 405 for wrong method (sebuf service-config method gate) - scenarioId validation against SCENARIO_TEMPLATES registry - iso2 regex ^[A-Z]{2}$ - JOB_ID_RE path-traversal guard on status - Per-IP 10/min rate limit (moved to gateway ENDPOINT_RATE_POLICIES) - Queue-depth backpressure (>100 → 429) - PRO gating via isCallerPremium - AbortSignal.timeout on every Redis pipeline (runRedisPipeline helper) Wire-level diffs vs legacy: - Per-user RL now enforced at the gateway (same 10/min/IP budget). - Rate-limit response omits Retry-After header; retryAfter is in the body per error-mapper.ts convention. - ListScenarioTemplates emits affectedHs2: [] when the registry entry is null (all-sectors sentinel); proto repeated cannot carry null. - RunScenario returns { jobId, status } (no statusUrl field — unused by SupplyChainPanel, drop from wire). Gateway wiring: - server/gateway.ts RPC_CACHE_TIER: list-scenario-templates → 'daily' (matches legacy max-age=3600); get-scenario-status → 'slow-browser' (premium short-circuit target, explicit entry required by tests/route-cache-tier.test.mjs). - src/shared/premium-paths.ts: swap old run/status for the new run-scenario/get-scenario-status paths. - api/scenario/v1/{run,status,templates}.ts deleted; 3 manifest exceptions removed (63 → 52 → 49 migration-pending). Client: - src/services/scenario/index.ts — typed client wrapper using premiumFetch (injects Clerk bearer / API key). - src/components/SupplyChainPanel.ts — polling loop swapped from premiumFetch strings to runScenario/getScenarioStatus. Hard 20s timeout on run preserved via AbortSignal.any. Tests: - tests/scenario-handler.test.mjs — 18 new handler-level tests covering every security invariant + the worker envelope coercion. - tests/edge-functions.test.mjs — scenario sections removed, replaced with a breadcrumb pointer to the new test file. Docs: api-scenarios.mdx, scenario-engine.mdx, usage-rate-limits.mdx, usage-errors.mdx, supply-chain.mdx refreshed with new paths. Verified: typecheck, typecheck:api, lint:api-contract (49 entries), lint:rate-limit-policies (6/180), lint:boundaries, route-cache-tier (parity), full edge-functions (117) + scenario-handler (18). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * refactor(commit 8): migrate /api/v2/shipping/{route-intelligence,webhooks} → ShippingV2Service (koala73#3207) Partner-facing endpoints promoted to a typed sebuf service. Wire shape preserved byte-for-byte (camelCase field names, ISO-8601 fetchedAt, the same subscriberId/secret formats, the same SET + SADD + EXPIRE 30-day Redis pipeline). Partner URLs /api/v2/shipping/* are unchanged. RPCs landed: - GET /route-intelligence → RouteIntelligence (PRO, slow-browser) - POST /webhooks → RegisterWebhook (PRO) - GET /webhooks → ListWebhooks (PRO, slow-browser) The existing path-parameter URLs remain on the legacy edge-function layout because sebuf's HTTP annotations don't currently model path params (grep proto/**/*.proto for `path: "{…}"` returns zero). Those endpoints are split into two Vercel dynamic-route files under api/v2/shipping/webhooks/, behaviorally identical to the previous hybrid file but cleanly separated: - GET /webhooks/{subscriberId} → [subscriberId].ts - POST /webhooks/{subscriberId}/rotate-secret → [subscriberId]/[action].ts - POST /webhooks/{subscriberId}/reactivate → [subscriberId]/[action].ts Both get manifest entries under `migration-pending` pointing at koala73#3207. Other changes - scripts/enforce-sebuf-api-contract.mjs: extended GATEWAY_RE to accept api/v{N}/{domain}/[rpc].ts (version-first) alongside the canonical api/{domain}/v{N}/[rpc].ts; first-use of the reversed ordering is shipping/v2 because that's the partner contract. - vite.config.ts: dev-server sebuf interceptor regex extended to match both layouts; shipping/v2 import + allRoutes entry added. - server/gateway.ts: RPC_CACHE_TIER entries for /api/v2/shipping/ route-intelligence + /webhooks (slow-browser; premium-gated endpoints short-circuit to slow-browser but the entries are required by tests/route-cache-tier.test.mjs). - src/shared/premium-paths.ts: route-intelligence + webhooks added. - tests/shipping-v2-handler.test.mjs: 18 handler-level tests covering PRO gate, iso2/cargoType/hs2 coercion, SSRF guards (http://, RFC1918, cloud metadata, IMDS), chokepoint whitelist, alertThreshold range, secret/subscriberId format, pipeline shape + 30-day TTL, cross-tenant owner isolation, `secret` omission from list response. Manifest delta - Removed: api/v2/shipping/route-intelligence.ts, api/v2/shipping/webhooks.ts - Added: api/v2/shipping/webhooks/[subscriberId].ts (migration-pending) - Added: api/v2/shipping/webhooks/[subscriberId]/[action].ts (migration-pending) - Added: api/internal/brief-why-matters.ts (internal-helper) — regression surface from the koala73#3248 main merge, which introduced the file without a manifest entry. Filed here to keep the lint green; not strictly in scope for commit 8 but unblocking. Net result: 49 → 47 `migration-pending` entries (one net-removal even though webhook path-params stay pending, because two files collapsed into two dynamic routes). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(review HIGH 1): SupplyChainServiceClient must use premiumFetch (koala73#3207) Signed-in browser pro users were silently hitting 401 on 8 supply-chain premium endpoints (country-products, multi-sector-cost-shock, country-chokepoint-index, bypass-options, country-cost-shock, sector-dependency, route-explorer-lane, route-impact). The shared client was constructed with globalThis.fetch, so no Clerk bearer or X-WorldMonitor-Key was injected. The gateway's validateApiKey runs with forceKey=true for PREMIUM_RPC_PATHS and 401s before isCallerPremium is consulted. The generated client's try/catch collapses the 401 into an empty-fallback return, leaving panels blank with no visible error. Fix is one line at the client constructor: swap globalThis.fetch for premiumFetch. The same pattern is already in use for insider-transactions, stock-analysis, stock-backtest, scenario, trade (premiumClient) — this was an omission on this client, not a new pattern. premiumFetch no-ops safely when no credentials are available, so the 5 non-premium methods on this client (shippingRates, chokepointStatus, chokepointHistory, criticalMinerals, shippingStress) continue to work unchanged. This also fixes two panels that were pre-existing latently broken on main (chokepoint-index, bypass-options, etc. — predating koala73#3207, not regressions from it). Commit 6 expanded the surface by routing two more methods through the same buggy client; this commit fixes the class. From koala73 review (koala73#3242 second-pass, HIGH new #1): > Exact class PR koala73#3233 fixed for RegionalIntelligenceBoard / > DeductionPanel / trade / country-intel. Supply-chain was not in > koala73#3233's scope. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(review HIGH 2): restore 400 on input-shape errors for 2 supply-chain handlers (koala73#3207) Commit 6 collapsed all non-happy paths into empty-200 on `get-country-products` and `get-multi-sector-cost-shock`, including caller-bug cases that legacy returned 400 for: - get-country-products: malformed iso2 → empty 200 (was 400) - get-multi-sector-cost-shock: malformed iso2 / missing chokepointId / unknown chokepointId → empty 200 (was 400) The commit message for 6 called out the 403-for-non-pro → empty-200 shift ("sebuf gateway pattern is empty-payload-on-deny") but not the 400 shift. They're different classes: - Empty-payload-200 for PRO-deny: intentional contract change, already documented and applied across the service. Generated clients treat "you lack PRO" as "no data" — fine. - Empty-payload-200 for malformed input: caller bug silently masked. External API consumers can't distinguish "bad wiring" from "genuinely no data", test harnesses lose the signal, bad calling code doesn't surface in Sentry. Fix: `throw new ValidationError(violations)` on the 3 input-shape branches. The generated sebuf server maps ValidationError → HTTP 400 (see src/generated/server/.../service_server.ts and leads/v1 which already uses this pattern). PRO-gate deny stays as empty-200 — that contract shift was intentional and is preserved. Regression tests added at tests/supply-chain-validation.test.mjs (8 cases) pinning the three-way contract: - bad input → 400 (ValidationError) - PRO-gate deny on valid input → 200 empty - valid PRO input, no data in Redis → 200 empty (unchanged) From koala73 review (koala73#3242 second-pass, HIGH new #2). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(review HIGH 3): restore statusUrl on RunScenarioResponse + document 202→200 wire break (koala73#3207) Commit 7 silently shifted /api/scenario/v1/run-scenario's response contract in two ways that the commit message covered only partially: 1. HTTP 202 Accepted → HTTP 200 OK 2. Dropped `statusUrl` string from the response body The `statusUrl` drop was mentioned as "unused by SupplyChainPanel" but not framed as a contract change. The 202 → 200 shift was not mentioned at all. This is a same-version (v1 → v1) migration, so external callers that key off either signal — `response.status === 202` or `response.body.statusUrl` — silently branch incorrectly. Evaluated options: (a) sebuf per-RPC status-code config — not available. sebuf's HttpConfig only models `path` and `method`; no status annotation. (b) Bump to scenario/v2 — judged heavier than the break itself for a single status-code shift. No in-repo caller uses 202 or statusUrl; the docs-level impact is containable. (c) Accept the break, document explicitly, partially restore. Took option (c): - Restored `statusUrl` in the proto (new field `string status_url = 3` on RunScenarioResponse). Server computes `/api/scenario/v1/get-scenario-status?jobId=<encoded job_id>` and populates it on every successful enqueue. External callers that followed this URL keep working unchanged. - 202 → 200 is not recoverable inside the sebuf generator, so it is called out explicitly in two places: - docs/api-scenarios.mdx now includes a prominent `<Warning>` block documenting the v1→v1 contract shift + the suggested migration (branch on response body shape, not HTTP status). - RunScenarioResponse proto comment explains why 200 is the new success status on enqueue. OpenAPI bundle regenerated to reflect the restored statusUrl field. - Regression test added in tests/scenario-handler.test.mjs pinning `statusUrl` to the exact URL-encoded shape — locks the invariant so a future proto rename or handler refactor can't silently drop it again. From koala73 review (koala73#3242 second-pass, HIGH new #3). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(review HIGH 1/2): close webhook tenant-isolation gap on shipping/v2 (koala73#3207) Koala flagged this as a merge blocker in PR koala73#3242 review. server/worldmonitor/shipping/v2/{register-webhook,list-webhooks}.ts migrated without reinstating validateApiKey(req, { forceKey: true }), diverging from both the sibling api/v2/shipping/webhooks/[subscriberId] routes and the documented "X-WorldMonitor-Key required" contract in docs/api-shipping-v2.mdx. Attack surface: the gateway accepts Clerk bearer auth as a pro signal. A Clerk-authenticated pro user with no X-WorldMonitor-Key reaches the handler, callerFingerprint() falls back to 'anon', and every such caller collapses into a shared webhook:owner:anon:v1 bucket. The defense-in-depth ownerTag !== ownerHash check in list-webhooks.ts doesn't catch it because both sides equal 'anon' — every Clerk-session holder could enumerate / overwrite every other Clerk-session pro tenant's registered webhook URLs. Fix: reinstate validateApiKey(ctx.request, { forceKey: true }) at the top of each handler, throwing ApiError(401) when absent. Matches the sibling routes exactly and the published partner contract. Tests: - tests/shipping-v2-handler.test.mjs: two existing "non-PRO → 403" tests for register/list were using makeCtx() with no key, which now fails at the 401 layer first. Renamed to "no API key → 401 (tenant-isolation gate)" with a comment explaining the failure mode being tested. 18/18 pass. Verified: typecheck:api, lint:api-contract (no change), lint:boundaries, lint:rate-limit-policies, test:data (6005/6005). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(review HIGH 2/2): restore v1 path aliases on scenario + supply-chain (koala73#3207) Koala flagged this as a merge blocker in PR koala73#3242 review. Commits 6 + 7 of koala73#3207 renamed five documented v1 URLs to the sebuf method-derived paths and deleted the legacy edge-function files: POST /api/scenario/v1/run → run-scenario GET /api/scenario/v1/status → get-scenario-status GET /api/scenario/v1/templates → list-scenario-templates GET /api/supply-chain/v1/country-products → get-country-products GET /api/supply-chain/v1/multi-sector-cost-shock → get-multi-sector-cost-shock server/router.ts is an exact static-match table (Map keyed on `METHOD PATH`), so any external caller — docs, partner scripts, grep-the- internet — hitting the old documented URL would 404 on first request after merge. Commit 8 (shipping/v2) preserved partner URLs byte-for- byte; the scenario + supply-chain renames missed that discipline. Fix: add five thin alias edge functions that rewrite the pathname to the canonical sebuf path and delegate to the domain [rpc].ts gateway via a new server/alias-rewrite.ts helper. Premium gating, rate limits, entitlement checks, and cache-tier lookups all fire on the canonical path — aliases are pure URL rewrites, not a duplicate handler pipeline. api/scenario/v1/{run,status,templates}.ts api/supply-chain/v1/{country-products,multi-sector-cost-shock}.ts Vite dev parity: file-based routing at api/ is a Vercel concern, so the dev middleware (vite.config.ts) gets a matching V1_ALIASES rewrite map before the router dispatch. Manifest: 5 new entries under `deferred` with removal_issue=koala73#3282 (tracking their retirement at the next v1→v2 break). lint:api-contract stays green (89 files checked, 55 manifest entries validated). Docs: - docs/api-scenarios.mdx: migration callout at the top with the full old→new URL table and a link to the retirement issue. - CHANGELOG.md + docs/changelog.mdx: Changed entry documenting the rename + alias compat + the 202→200 shift (from commit 23c821a). Verified: typecheck:api, lint:api-contract, lint:rate-limit-policies, lint:boundaries, test:data (6005/6005). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
lspassos1
pushed a commit
that referenced
this pull request
Apr 27, 2026
…koala73#3358) * fix(insights): trust cluster rank, stop LLM from re-picking top story WORLD BRIEF panel published "Iran's new supreme leader was seriously wounded, leading him to delegate power to the Revolutionary Guards. This development comes amid an ongoing war with Israel." to every visitor for 3h. Payload: openrouter / gemini-2.5-flash. Root cause: callLLM sent all 10 clustered headlines with "pick the ONE most significant and summarize ONLY that story". Clustering ranked Lebanon journalist killing #1 (2 corroborating sources); News24 Iran rumor ranked #3 (1 source). Gemini overrode the rank, picked #3, and embellished with war framing from story #4. Objective rank (sourceCount, velocity, isAlert) lost to model vibe. Shrink the LLM's job to phrasing. Clustering already ranks — pass only topStories[0].primaryTitle and instruct the model to rewrite it using ONLY facts from the headline. No name/place/context invention. Also: - temperature 0.3 -> 0.1 (factual summary, not creative) - CACHE_TTL 3h -> 30m so a bad brief ages out in one cron cycle - Drop dead MAX_HEADLINES const Payload shape unchanged; frontend untouched. * fix(insights): corroboration gate + revert TTL + drop unconditional WHERE Follow-up to review feedback on the ranking contract, TTL, and prompt: 1. Corroboration gate (P1a). scoreImportance() in scripts/_clustering.mjs is keyword-heavy (violence +125 on a single word, flashpoint +75, ^1.5 multiplier when both hit), so a single-source sensational rumor can outrank a 2-source lead purely on lexical signals. Blindly trusting topStories[0] would let the ranker's keyword bias still pick bad stories. Walk topStories for sourceCount >= 2 instead — corroboration becomes a hard requirement, not a tiebreaker. If no cluster qualifies, publish status=degraded with no brief (frontend already handles this). 2. CACHE_TTL back to 10800 (P1b). 30m TTL == one cron cadence means the key expires on any missed or delayed run and /api/bootstrap loses insights entirely (api/bootstrap.js reads news:insights:v1 directly, no LKG across TTL-gap). The short TTL was defense-in-depth for bad content; the real safety is now upstream (corroboration gate + grounded prompt), so the LKG window doesn't need to be sacrificed for it. 3. Prompt: location conditional (P2). "Use ONLY facts present" + "Lead with WHAT happened and WHERE" conflicted for headlines without an explicit location and pushed the model toward inferred-place hallucination. Replaced with "Include a location, person, or organization ONLY if it appears in the headline." * test(insights): lock corroboration gate + grounded-prompt invariants Review P2: the corroboration gate and the prompt's no-invention rules had no tests, so future edits to selectTopStories() ordering or prompt text could silently reintroduce the original hallucination. Extract the brief-selection helper and prompt builders into a pure module (scripts/_insights-brief.mjs) so tests can import them without triggering seed-insights.mjs's top-level runSeed() call: - pickBriefCluster(topStories) returns first sourceCount>=2 cluster - briefSystemPrompt(dateISO) returns the system prompt - briefUserPrompt(headline) returns the user prompt Regression tests (tests/seed-insights-brief.test.mjs, 12 cases) lock: - pickBriefCluster skips single-source rumors even when ranked above a multi-sourced lead (explicit regression: News24 Iran supreme leader 2026-04-23 scenario with realistic scores) - pickBriefCluster tolerates missing/null entries - briefSystemPrompt forbids invented facts and proper nouns - briefSystemPrompt's "location" rule is conditional (no unconditional "Lead with WHAT and WHERE" directive that would push the model toward place-inference when the headline has no location) - briefSystemPrompt does not contain "pick the most important" style language (ranking is done by pickBriefCluster upstream) - briefUserPrompt passes the headline verbatim and instructs "only facts from this headline" Also fix a misleading comment on CACHE_TTL: corroboration is gated at brief-selection time, not on the topStories payload itself (which still includes single-source clusters rendered as the headline list). test:data: 6657/6657 pass (was 6645; +12).
lspassos1
pushed a commit
that referenced
this pull request
Apr 27, 2026
…istic GCC identity) (koala73#3374) * docs(resilience): PR 5.3 — foodWater scorer audit (construct-deterministic GCC identity) PR 5.3 of cohort-audit plan 2026-04-24-002. Stacked on PR 5.2 (koala73#3373) so the known-limitations.md section append is additive. Read-only static audit of scoreFoodWater. Findings 1. The observed GCC-all-score-53 is CONSTRUCT-DETERMINISTIC, not a regional-default leak. Pinned mathematically: - IPC/HDX doesn't publish active food-crisis data for food-secure states → scorer's fao-null branch imputes IMPUTE.ipcFood=88 (class='stable-absence', cov=0.7) at combined weight 0.6 - WB indicator ER.H2O.FWST.ZS (labelled 'water stress') for GCC is EXTREME (KW ~3200%, BH ~3400%, UAE ~2080%, QA ~770%) — all clamp to sub-score 0 under the scorer's lower-better 0..100 normaliser at weight 0.4 - Blended with peopleInCrisis=0 (fao block present with zero): (100 * 0.45 + 0 * 0.4) / (0.45 + 0.4) = 45 / 0.85 ≈ 53 Every GCC country has the same inputs → same outputs. That's construct math, not a regional lookup. 2. Indicator-keyword routing is code-correct. `'water stress'`, `'withdrawal'`, `'dependency'` route to lower-better; `'availability'`, `'renewable'`, `'access'` route to higher-better; unrecognized indicators fall through to a value-range heuristic with a WARN log. 3. No bug or methodology decision required. The 53-all-GCC output is a correct summary statement: "non-crisis food security + severe water-withdrawal stress." A future construct decision might split foodWater into separate food and water dims so one saturated sub-signal doesn't dominate the combined dim for desert economies — but that's a construct redesign, not a bug. Shipped - `docs/methodology/known-limitations.md` — extended with a new section documenting the foodWater audit findings, the exact blend math that yields ~53 for GCC, cohort-determinism vs regional-default, and a follow-up data-side spot-check list gated on API-key access. - `tests/resilience-foodwater-field-mapping.test.mts` — 8 new regression-guard tests: 1. indicator='water stress' routes to lower-better 2. GCC extreme-withdrawal anchor (value=2000 → blended score 53) 3. indicator='renewable water availability' routes to higher-better 4. fao=null with static record → imputes 88; imputationClass=null because observed AQUASTAT wins (weightedBlend T1.7 rule) 5. fully-imputed (fao=null + aquastat=null) surfaces imputationClass='stable-absence' 6. static-record absent entirely → coverage=0, NOT impute 7. Cohort determinism — identical inputs → identical scores 8. Different water-profile inputs → different scores (rules out regional-default hypothesis) Verified - `npx tsx --test tests/resilience-foodwater-field-mapping.test.mts` — 8 pass / 0 fail - `npm run test:data` — 6711 pass / 0 fail (PR 5.2's 9 + PR 5.3's 8 = 17 new stacked) - `npm run typecheck` / `typecheck:api` — green - `npm run lint` / `lint:md` — clean * fix(resilience): PR 5.3 review — pin IMPUTE branch for GCC anchor; fix comment math Addresses 3 P2 Greptile findings on koala73#3374 — all variations of the same root cause: the test fixture + doc described two different code paths that coincidentally both produce ~53 for GCC inputs. Changes 1. GCC anchor test now drives the IMPUTE branch (`fao: null`), matching what the static seeder emits for GCC in production. The else branch (`fao: { peopleInCrisis: 0 }`) happens to converge on ~52.94 by coincidence but is NOT the live code path for GCC. 2. Doc finding #4 updated to show the IMPUTE-branch math `(88×0.6 + 0×0.4) / 1.0 = 52.8 → 53` and explicitly notes the else-branch convergence as a coincidence — not the construct's intent. 3. Comment math off-by-one fix at line 107: (88×0.6 + 80×0.4) / (0.6+0.4) = 52.8 + 32.0 = 84.8 → 85 (was incorrectly stated as 85.6 → 86) Test assertion `>= 80 && <= 90` still accepts 85 so behaviour is unchanged; this was a comment-only error that would have misled anyone reproducing the math by hand. Verified - `npx tsx --test tests/resilience-foodwater-field-mapping.test.mts` — 8 pass / 0 fail (IMPUTE-branch anchor test produces 53 as expected) - `npm run lint:md` — clean Also rebased onto updated koala73#3373 (which landed a backtick-escape fix).
lspassos1
pushed a commit
that referenced
this pull request
Apr 27, 2026
koala73#3378) * feat(energy-atlas): EnergyDisruptionsPanel standalone timeline (§L #4) Closes gap #4 from docs/internal/energy-atlas-registry-expansion.md §L. Before this PR, the 52 disruption events in `energy:disruptions:v1` were only reachable by drilling into a specific pipeline or storage facility — PipelineStatusPanel and StorageFacilityMapPanel each render an asset-scoped slice of the log inside their drawers, but no surface listed the global event log. This panel makes the full log first-class. Shape: - Reverse-chronological table (newest first) of every event. - Filter chips: event type (sabotage, sanction, maintenance, mechanical, weather, war, commercial, other) + "ongoing only" toggle. - Row click dispatches the existing `energy:open-pipeline-detail` or `energy:open-storage-facility-detail` CustomEvent with `{assetId, highlightEventId}` — no new open-panel protocol introduced. Mirrors the CountryDeepDivePanel disruption row contract from PR koala73#3377. - Uses `src/shared/disruption-timeline.ts` formatters (formatEventWindow, formatCapacityOffline, statusForEvent) that PipelineStatus/StorageFacilityMap already use — consistent UI across all three disruption surfaces. Wiring: - `src/components/EnergyDisruptionsPanel.ts` — new (~230 lines). - `src/components/index.ts` — export. - `src/app/panel-layout.ts` — `this.createPanel('energy-disruptions', () => new EnergyDisruptionsPanel())` alongside the other three atlas panels at :892. - `src/config/panels.ts` — add to `FULL_PANELS` (priority 2, next to fuel-shortages) + `ENERGY_PANELS` (priority 1, top tier) + `PANEL_CATEGORY_MAP.marketsFinance` list alongside the other atlas panels. - `src/config/commands.ts` — CMD+K entry `panel:energy-disruptions` with keywords matching the user vocabulary (sabotage, sanctions events, force majeure, drone strike, nord stream sabotage). Not done in this PR: - No new map pin layer — per plan §Q (Codex approved), disruptions stay a tabular/timeline surface; map assets (pipelines + storage) already show disruption markers on click. - No direct globe-mode or SVG-fallback rendering needs — panel is pure DOM, not a map layer. Test plan: - [x] npm run typecheck (clean) - [x] npm run test:data (6694/6694 pass) - [ ] Manual: CMD+K "disruption log" → panel opens with 52 events, newest first. Click "Sabotage" chip → narrows to sabotage events only. Click a Nord Stream row → PipelineStatusPanel opens with that event highlighted. * fix(energy-atlas): drop highlightEventId emission + respect empty-state (review P2) Two Codex P2 findings on this PR: 1. Row click dispatched `highlightEventId` but neither PipelineStatusPanel nor StorageFacilityMapPanel consumes it. The UI's implicit promise (event-specific highlighting) wasn't delivered — clickthrough was asset-generic, and the extra field on the wire was a misleading API surface. Fix: drop `highlightEventId` from the dispatched detail. Row click now opens the asset drawer with just {pipelineId, facilityId}, the fields the receivers actually consume. User sees the full disruption timeline for that asset and locates the event visually. A future PR can add real highlight support by: - drawers accept `highlightEventId` in their openDetailHandler - loadDetail stores it and renderDisruptionTimeline scrolls + emphasises the matching event - re-add `highlightEventId` to the dispatch here, symmetrically in CountryDeepDivePanel (which has the same wire emission) The internal `_eventId` parameter is kept as a plumb-through so that future work is a drawer-side change, not a re-plumb. 2. `events.length === 0` was conflated with `upstreamUnavailable` and triggered the error UI. The server contract (list-energy-disruptions handler) returns `upstreamUnavailable: false` with an empty events array when Redis is up but has no entries matching the filter — a legitimate empty state, not a fetch failure. Fix: gate `showError` on `upstreamUnavailable` alone. Empty results fall through to the normal render, where the table's `No events match the current filter` row already handles the case. Typecheck clean, test:data 6694/6694 pass. * fix(energy-atlas): event delegation on persistent content (review P1) Codex P1: Panel.setContent() debounces the DOM write by 150ms (see Panel.ts:1025), so attaching listeners in render() via `this.element.querySelector(...)` targets the STALE DOM — chips, rows, and the ongoing-toggle button are silently non-interactive. Visually the panel renders correctly after the debounce fires, but every click is permanently dead. Fix: register a single delegated click handler on `this.content` (persistent element) in the constructor. The handler uses `closest('[data-filter-type]')`, `closest('[data-toggle-ongoing]')`, and `closest('tr.ed-row')` to route by data-attribute. Works regardless of when setContent flushes or how many times render() re-rewrites the inner HTML. Also fixes Codex P2 on the same PR: filterEvents() was called twice per render (once for row HTML, again for filteredCount). Now computed once, reused. Trivial for 52 events but eliminates the redundant sort. Typecheck clean. * fix(energy-atlas): remap orphan disruption assetIds to real pipelines Two events referenced pipeline ids that do not exist in scripts/data/pipelines-oil.json: - cpc-force-majeure-2022: assetId "cpc-pipeline" → "cpc" - pdvsa-designation-2019: assetId "ve-petrol-2026-q1" → "venezuela-anzoategui-puerto-la-cruz" Without this, clicking those rows in EnergyDisruptionsPanel dead-ends at "Pipeline detail unavailable", so the panel shipped with broken navigation on real data. Mirrors the same fix on PR koala73#3377 (gap #5a registry); applying it on this branch as well so PR koala73#3378 is independently correct regardless of merge order. The two changes will dedupe cleanly on rebase since the edits are byte-identical.
lspassos1
pushed a commit
that referenced
this pull request
Apr 27, 2026
…read (koala73#3385) * feat(seed): BUNDLE_RUN_STARTED_AT_MS env + runSeed SIGTERM cleanup Prereq for the re-export-share Comtrade seeder (plan 2026-04-24-003), usable by any cohort seeder whose consumer needs bundle-level freshness. Two coupled changes: 1. `_bundle-runner.mjs` injects `BUNDLE_RUN_STARTED_AT_MS` into every spawned child. All siblings in a single bundle run share one value (captured at `runBundle` start, not spawn time). Consumers use this to detect stale peer keys — if a peer's seed-meta predates the current bundle run, fall back to a hard default rather than read a cohort-peer's last-week output. 2. `_seed-utils.mjs::runSeed` registers a `process.once('SIGTERM')` handler that releases the acquired lock and extends existing-data TTL before exiting 143. `_bundle-runner.mjs` sends SIGTERM on section timeout, then SIGKILL after KILL_GRACE_MS (5s). Without this handler the `finally` path never runs on SIGKILL, leaving the 30-min acquireLock reservation in place until its own TTL expires — the next cron tick silently skips the resource. Regression guard memory: `bundle-runner-sigkill-leaks-child-lock` (PR koala73#3128 root cause). Tests added: - bundle-runner env injection (value within run bounds) - sibling sections share the same timestamp (critical for the consumer freshness guard) - runSeed SIGTERM path: exit 143 + cleanup log - process.once contract: second SIGTERM does not re-enter handler * fix(seed): address P1/P2 review findings on SIGTERM + bundle contracts Addresses PR koala73#3384 review findings (todos 256, 257, 259, 260): koala73#256 (P1) — SIGTERM handler narrowed to fetch phase only. Was installed at runSeed entry and armed through every `process.exit` path; could race `emptyDataIsFailure: true` strict-floor exits (IMF-External, WB-bulk) and extend seed-meta TTL when the contract forbids it — silently re-masking 30-day outages. Now the handler is attached immediately before `withRetry(fetchFn)` and removed in a try/finally that covers all fetch-phase exit branches. koala73#257 (P1) — `BUNDLE_RUN_STARTED_AT_MS` now has a first-class helper. Exported `getBundleRunStartedAtMs()` from `_seed-utils.mjs` with JSDoc describing the bundle-freshness contract. Fleet-wide helper so the next consumer seeder imports instead of rediscovering the idiom. koala73#259 (P2) — SIGTERM cleanup runs `Promise.allSettled` on disjoint-key ops (`releaseLock` + `extendExistingTtl`). Serialising compounded Upstash latency during the exact failure mode (Redis degraded) this handler exists to handle, risking breach of the 5s SIGKILL grace. koala73#260 (P2) — `_bundle-runner.mjs` asserts topological order on optional `dependsOn` section field. Throws on unknown-label refs and on deps appearing at a later index. Fleet-wide contract replacing the previous prose-comment ordering guarantee. Tests added/updated: - New: SIGTERM handler removed after fetchFn completes (narrowed-scope contract — post-fetch SIGTERM must NOT trigger TTL extension) - New: dependsOn unknown-label + out-of-order + happy-path (3 tests) Full test suite: 6,866 tests pass (+4 net). * fix(seed): getBundleRunStartedAtMs returns null outside a bundle run Review follow-up: the earlier `Math.floor(Date.now()/1000)*1000` fallback regressed standalone (non-bundle) runs. A consumer seeder invoked manually just after its peer wrote `fetchedAt = (now - 5s)` would see `bundleStartMs = Date.now()`, reject the perfectly-fresh peer envelope as "stale", and fall back to defaults — defeating the point of the peer-read path outside the bundle. Returning null when `BUNDLE_RUN_STARTED_AT_MS` is unset/invalid keeps the freshness gate scoped to its real purpose (across-bundle-tick staleness) and lets standalone runs skip the gate entirely. Consumers check `bundleStartMs != null` before applying the comparison; see the companion `seed-sovereign-wealth.mjs` change on the stacked PR. * test(seed): SIGTERM cleanup test now verifies Redis DEL + EXPIRE calls Greptile review P2 on PR koala73#3384: the existing test only asserted exit code + log line, not that the Redis ops were actually issued. The log claim was ahead of the test. Fixture now logs every Upstash fetch call's shape (EVAL / pipeline- EXPIRE / other) to stderr. Test asserts: - >=1 EVAL op was issued during SIGTERM cleanup (releaseLock Lua script on the lock key) - >=1 pipeline-EXPIRE op was issued (extendExistingTtl on canonical + seed-meta keys) - The EVAL body carries the runSeed-generated runId (proves it's THIS run's release, not a phantom op) - The EXPIRE pipeline touches both the canonicalKey AND the seed-meta key (proves the keys[] array was built correctly including the extraKeys merge path) Full test suite: 6,866 tests pass, typecheck clean. * feat(resilience): Comtrade-backed re-export-share seeder + SWF Redis read Plan ref: docs/plans/2026-04-24-003-feat-reexport-share-comtrade-seeder-plan.md Motivating case. Before this PR, the SWF `rawMonths` denominator for the `sovereignFiscalBuffer` dimension used GROSS annual imports for every country. For re-export hubs (goods transiting without domestic settlement), this structurally under-reports resilience: UAE's 2023 $941B of imports include $334B of transit flow that never represents domestic consumption. Net imports = gross × (1 − reexport_share). The previous (PR 3A) design flattened a hand-curated YAML into Redis; the YAML shipped empty and never populated, so the correction never applied and the cohort audit showed no movement. Gap #2 (this PR). Two coupled changes to make the correction actually apply: 1. Comtrade-backed seeder (`scripts/seed-recovery-reexport-share.mjs`). Rewritten to fetch UN Comtrade `flowCode=RX` (re-exports) and `flowCode=M` (imports) per cohort member, compute share = RX/M at the latest co-populated year, clamp to [0.05, 0.95], publish the envelope. Header auth (`Ocp-Apim-Subscription-Key`) — subscription key never reaches URL/logs/Redis. `maxRecords=250000` cap with truncation detection. Sequential + retry-on-429 with backoff. Hub cohort resolved by Phase 0 empirical probe (plan §Phase 0): ['AE', 'PA']. Six candidates (SG/HK/NL/BE/MY/LT) return HTTP 200 with zero RX rows — Comtrade doesn't expose RX for those reporters. 2. SWF seeder reads from Redis (`scripts/seed-sovereign-wealth.mjs`). Swaps `loadReexportShareByCountry()` (YAML) for `loadReexportShareFromRedis()` (Redis key written by #1). Guarded by bundle-run freshness: if the sibling Reexport-Share seeder's `seed-meta` predates `BUNDLE_RUN_STARTED_AT_MS` (set by the prereq PR's `_bundle-runner.mjs` env-injection), HARD fallback to gross imports rather than apply last-month's stale share. Health registries. Both new keys registered in BOTH `api/health.js` SEED_META (60-day alert threshold) and `api/seed-health.js` SEED_DOMAINS (43200min interval). feedback_two_health_endpoints_must_match. Bundle wiring. `seed-bundle-resilience-recovery` Reexport-Share timeout bumped 60s → 300s (Comtrade + retry can take 2-3 min worst-case). Ordering preserved: Reexport-Share before Sovereign- Wealth so the SWF seeder reads a freshly-written key in the same cron tick. Deletions. YAML + loader + 7 obsolete loader tests removed; single source of truth is now Comtrade → Redis. Prereq. Stacks on PR koala73#3384 (feat/bundle-runner-env-sigterm) which adds BUNDLE_RUN_STARTED_AT_MS env injection + runSeed SIGTERM cleanup. This PR's bundle-freshness guard depends on that env variable. Tests (19 new, 7 deleted, +12 net): - Pure math: parseComtradeFlowResponse, computeShareFromFlows, clampShare, declareRecords + credential-leak source scan (15) - Integration (Gap #2 regression guards): SWF seeder loadReexport ShareFromRedis — fresh/absent/malformed/stale-meta/missing-meta (5) - Health registry dual-registry drift guard — scoped to this PR's keys, respecting pre-existing asymmetry (4) - Bundle-ordering + timeout assertions (2) Phase 0 cohort validation committed to plan. Full test suite passes: 6,881 tests. * fix(resilience): address P1/P2 review findings — adopt shared helpers, pin freshness boundary Addresses PR koala73#3385 review findings: koala73#257 (P1) consumer — `seed-sovereign-wealth.mjs` imports the shared `getBundleRunStartedAtMs` helper from `_seed-utils.mjs` (added in the prereq commit) instead of its own `getBundleStartMs`. Single source of truth for the bundle-freshness contract. koala73#258 (P2) — `seed-recovery-reexport-share.mjs` isMain guard uses the canonical `pathToFileURL(process.argv[1]).href === import.meta.url` form instead of basename-suffix matching. Handles symlinks, case- different paths on macOS HFS+, and Windows path separators without string munging. koala73#260 (P2) consumer — Sovereign-Wealth declares `dependsOn: ['Reexport-Share']` in the bundle spec. `_bundle-runner.mjs` (prereq commit) now enforces topological order on load and throws on violation — replaces the previous prose-comment ordering contract. koala73#261 (P2) — added a test to `tests/seed-sovereign-wealth-reads-redis- reexport-share.test.mts` pinning the inclusive-boundary semantic: `fetchedAtMs === bundleStartMs` must be treated as FRESH. Guards against a future refactor to `<=` that would silently reject peers writing at the very first millisecond of the bundle run. Rebased onto updated prereq. Full test suite: 6,886 tests pass (+5 net). * fix(resilience): freshness gate skipped in standalone mode; meta still required Review catch: the previous `bundleStartMs = Date.now()` fallback made standalone/manual `seed-sovereign-wealth.mjs` runs ALWAYS reject any previously-seeded re-export-share meta as "stale" — even when the operator ran the Reexport seeder milliseconds beforehand. Defeated the point of the peer-read path outside the bundle. With `getBundleRunStartedAtMs()` now returning null outside a bundle (companion commit on the prereq branch), the consumer only applies the freshness gate when `bundleStartMs != null`. Standalone runs accept any `fetchedAt` — the operator is responsible for ordering. Two guards survive the change: - Meta MUST exist (absence = peer-outage fail-safe, both modes) - In-bundle: meta MUST be at or after `BUNDLE_RUN_STARTED_AT_MS` Two new tests pin both modes: - standalone: accepts meta written 10 min before this process started - standalone: still rejects missing meta (peer-outage fail-safe survives gate bypass) Rebased onto updated prereq. Full test suite: 6,888 tests (+2 net). * fix(resilience): filter world-aggregate Comtrade rows + skip final-retry sleep Greptile review of PR koala73#3385 flagged two P2s in the Comtrade seeder. Finding #3 (parseComtradeFlowResponse double-count risk): `cmdCode=TOTAL` without a partner filter currently returns only world-aggregate rows in practice — but `parseComtradeFlowResponse` summed every row unconditionally. A future refactor adding per- partner querying would silently double-count (world-aggregate row + partner-level rows for the same year), cutting the derived share in half with no test signal. Fix: explicit `partnerCode ∈ {'0', 0, null/undefined}` filter. Matches current empirical behavior (aggregate-only responses) and makes the construct robust to a future partner-level query. Finding #4 (wasted backoff on final retry): 429 and 5xx branches slept `backoffMs` before `continue`, but on `attempt === RETRY_MAX_ATTEMPTS` the loop condition fails immediately after — the sleep was pure waste. Added early-return (parallel to the existing pattern in the network-error catch branch) so the final attempt exits the retry loop at the first non-success response without extra latency. Tests: - 3 new `parseComtradeFlowResponse` variants: world-only filter, numeric-0 partnerCode shape, rows without partnerCode field - Existing tests updated: the double-count assertion replaced with a "per-partner rows must NOT sum into the world-aggregate total" assertion that pins the new contract Rebased onto updated prereq. Full test suite: 6,890 tests (+2 net).
lspassos1
pushed a commit
that referenced
this pull request
Apr 27, 2026
…lead divergence (koala73#3396) * feat(brief-llm): canonical synthesis prompt + v3 cache key Extends generateDigestProse to be the single source of truth for brief executive-summary synthesis (canonicalises what was previously split between brief-llm's generateDigestProse and seed-digest- notifications.mjs's generateAISummary). Ports Brain B's prompt features into buildDigestPrompt: - ctx={profile, greeting, isPublic} parameter (back-compat: 4-arg callers behave like today) - per-story severity uppercased + short-hash prefix [h:XXXX] so the model can emit rankedStoryHashes for stable re-ranking - profile lines + greeting opener appear only when ctx.isPublic !== true validateDigestProseShape gains optional rankedStoryHashes (≥4-char strings, capped to MAX_STORIES_PER_USER × 2). v2-shaped rows still pass — field defaults to []. hashDigestInput v3: - material includes profile-SHA, greeting bucket, isPublic flag, per-story hash - isPublic=true substitutes literal 'public' for userId in the cache key so all share-URL readers of the same (date, sensitivity, pool) hit ONE cache row (no PII in public cache key) Adds generateDigestProsePublic(stories, sensitivity, deps) wrapper — no userId param by design — for the share-URL surface. Cache prefix bumped brief:llm:digest:v2 → v3. v2 rows expire on TTL. Per the v1→v2 precedent (see hashDigestInput comment), one-tick cost on rollout is acceptable for cache-key correctness. Tests: 72/72 passing in tests/brief-llm.test.mjs (8 new for the v3 behaviors), full data suite 6952/6952. Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md Step 1, Codex-approved (5 rounds). * feat(brief): envelope v3 — adds digest.publicLead for share-URL surface Bumps BRIEF_ENVELOPE_VERSION 2 → 3. Adds optional BriefDigest.publicLead — non-personalised executive lead generated by generateDigestProsePublic (already in this branch from the previous commit) for the public share-URL surface. Personalised `lead` is the canonical synthesis for authenticated channels; publicLead is its profile-stripped sibling so api/brief/public/* never serves user-specific content (watched assets/regions). SUPPORTED_ENVELOPE_VERSIONS = [1, 2, 3] keeps v1 + v2 envelopes in the 7-day TTL window readable through the rollout — the composer only ever writes the current version, but readers must tolerate older shapes that haven't expired yet. Same rollout pattern used at the v1 → v2 bump. Renderer changes (server/_shared/brief-render.js): - ALLOWED_DIGEST_KEYS gains 'publicLead' (closed-key-set still enforced; v2 envelopes pass because publicLead === undefined is the v2 shape). - assertBriefEnvelope: new isNonEmptyString check on publicLead when present. Type contract enforced; absence is OK. Tests (tests/brief-magazine-render.test.mjs): - New describe block "v3 publicLead field": v3 envelope renders; malformed publicLead rejected; v2 envelope still passes; ad-hoc digest keys (e.g. synthesisLevel) still rejected — confirming the closed-key-set defense holds for the cron-local-only fields the orchestrator must NOT persist. - BRIEF_ENVELOPE_VERSION pin updated 2 → 3 with rollout-rationale comment. Test results: 182 brief-related tests pass; full data suite 6956/6956. Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md Step 2, Codex Round-3 Medium #2. * feat(brief): synthesis splice + rankedStoryHashes pre-cap re-order Plumbs the canonical synthesis output (lead, threads, signals, publicLead, rankedStoryHashes from generateDigestProse) through the pure composer so the orchestration layer can hand pre-resolved data into envelope.digest. Composer stays sync / no I/O — Codex Round-2 High #2 honored. Changes: scripts/lib/brief-compose.mjs: - digestStoryToUpstreamTopStory now emits `hash` (the digest story's stable identifier, falls back to titleHash when absent). Without this, rankedStoryHashes from the LLM has nothing to match against. - composeBriefFromDigestStories accepts opts.synthesis = {lead, threads, signals, rankedStoryHashes?, publicLead?}. When passed, splices into envelope.digest after the stub is built. Partial synthesis (e.g. only `lead` populated) keeps stub defaults for the other fields — graceful degradation when L2 fallback fires. shared/brief-filter.js: - filterTopStories accepts optional rankedStoryHashes. New helper applyRankedOrder re-orders stories by short-hash prefix match BEFORE the cap is applied, so the model's editorial judgment of importance survives MAX_STORIES_PER_USER. Stable for ties; stories not in the ranking come after in original order. Empty/missing ranking is a no-op (legacy callers unchanged). shared/brief-filter.d.ts: - filterTopStories signature gains rankedStoryHashes?: string[]. - UpstreamTopStory gains hash?: unknown (carried through from digestStoryToUpstreamTopStory). Tests added (tests/brief-from-digest-stories.test.mjs): - synthesis substitutes lead/threads/signals/publicLead. - legacy 4-arg callers (no synthesis) keep stub lead. - partial synthesis (only lead) keeps stub threads/signals. - rankedStoryHashes re-orders pool before cap. - short-hash prefix match (model emits 8 chars; story carries full). - unranked stories go after in original order. Test results: 33/33 in brief-from-digest-stories; 182/182 across all brief tests; full data suite 6956/6956. Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md Step 3, Codex Round-2 Low + Round-2 High #2. * feat(brief): single canonical synthesis per user; rewire all channels Restructures the digest cron's per-user compose + send loops to produce ONE canonical synthesis per user per issueSlot — the lead text every channel (email HTML, plain-text, Telegram, Slack, Discord, webhook) and the magazine show is byte-identical. This eliminates the "two-brain" divergence that was producing different exec summaries on different surfaces (observed 2026-04-25 0802). Architecture: composeBriefsForRun (orchestration): - Pre-annotates every eligible rule with lastSentAt + isDue once, before the per-user pass. Same getLastSentAt helper the send loop uses so compose + send agree on lastSentAt for every rule. composeAndStoreBriefForUser (per-user): - Two-pass winner walk: try DUE rules first (sortedDue), fall back to ALL eligible rules (sortedAll) for compose-only ticks. Preserves today's dashboard refresh contract for weekly / twice_daily users on non-due ticks (Codex Round-4 High #1). - Within each pass, walk by compareRules priority and pick the FIRST candidate with a non-empty pool — mirrors today's behavior at scripts/seed-digest-notifications.mjs:1044 and prevents the "highest-priority but empty pool" edge case (Codex Round-4 Medium #2). - Three-level synthesis fallback chain: L1: generateDigestProse(fullPool, ctx={profile,greeting,!public}) L2: generateDigestProse(envelope-sized slice, ctx={}) L3: stub from assembleStubbedBriefEnvelope Distinct log lines per fallback level so ops can quantify failure-mode distribution. - Generates publicLead in parallel via generateDigestProsePublic (no userId param; cache-shared across all share-URL readers). - Splices synthesis into envelope via composer's optional `synthesis` arg (Step 3); rankedStoryHashes re-orders the pool BEFORE the cap so editorial importance survives MAX_STORIES. - synthesisLevel stored in the cron-local briefByUser entry — NOT persisted in the envelope (renderer's assertNoExtraKeys would reject; Codex Round-2 Medium #5). Send loop: - Reads lastSentAt via shared getLastSentAt helper (single source of truth with compose flow). - briefLead = brief?.envelope?.data?.digest?.lead — the canonical lead. Passed to buildChannelBodies (text/Telegram/Slack/Discord), injectEmailSummary (HTML email), and sendWebhook (webhook payload's `summary` field). All-channel parity (Codex Round-1 Medium #6). - Subject ternary reads cron-local synthesisLevel: 1 or 2 → "Intelligence Brief", 3 → "Digest" (preserves today's UX for fallback paths; Codex Round-1 Missing #5). Removed: - generateAISummary() — the second LLM call that produced the divergent email lead. ~85 lines. - AI_SUMMARY_CACHE_TTL constant — no longer referenced. The digest:ai-summary:v1:* cache rows expire on their existing 1h TTL (no cleanup pass). Helpers added: - getLastSentAt(rule) — extracted Upstash GET for digest:last-sent so compose + send both call one source of truth. - buildSynthesisCtx(rule, nowMs) — formats profile + greeting for the canonical synthesis call. Preserves all today's prefs-fetch failure-mode behavior. Composer: - compareRules now exported from scripts/lib/brief-compose.mjs so the cron can sort each pass identically to groupEligibleRulesByUser. Test results: full data suite 6962/6962 (was 6956 pre-Step 4; +6 new compose-synthesis tests from Step 3). Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md Steps 4 + 4b. Codex-approved (5 rounds). * fix(brief-render): public-share lead fail-safe — never leak personalised lead Public-share render path (api/brief/public/[hash].ts → renderer publicMode=true) MUST NEVER serve the personalised digest.lead because that string can carry profile context — watched assets, saved-region names, etc. — written by generateDigestProse with ctx.profile populated. Previously: redactForPublic redacted user.name and stories.whyMatters but passed digest.lead through unchanged. Codex Round-2 High (security finding). Now (v3 envelope contract): - redactForPublic substitutes digest.lead = digest.publicLead when the v3 envelope carries one (generated by generateDigestProsePublic with profile=null, cache-shared across all public readers). - When publicLead is absent (v2 envelope still in TTL window OR v3 envelope where publicLead generation failed), redactForPublic sets digest.lead to empty string. - renderDigestGreeting: when lead is empty, OMIT the <blockquote> pull-quote entirely. Page still renders complete (greeting + horizontal rule), just without the italic lead block. - NEVER falls back to the original personalised lead. assertBriefEnvelope still validates publicLead's contract (when present, must be a non-empty string) BEFORE redactForPublic runs, so a malformed publicLead throws before any leak risk. Tests added (tests/brief-magazine-render.test.mjs): - v3 envelope renders publicLead in pull-quote, personalised lead text never appears. - v2 envelope (no publicLead) omits pull-quote; rest of page intact. - empty-string publicLead rejected by validator (defensive). - private render still uses personalised lead. Test results: 68 brief-magazine-render tests pass; full data suite remains green from prior commit. Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md Step 5, Codex Round-2 High (security). * feat(digest): brief lead parity log + extra acceptance tests Adds the parity-contract observability line and supplementary acceptance tests for the canonical synthesis path. Parity log (per send, after successful delivery): [digest] brief lead parity user=<id> rule=<v>:<s>:<lang> synthesis_level=<1|2|3> exec_len=<n> brief_lead_len=<n> channels_equal=<bool> public_lead_len=<n> When channels_equal=false an extra WARN line fires — "PARITY REGRESSION user=… — email lead != envelope lead." Sentry's existing console-breadcrumb hook lifts this without an explicit captureMessage call. Plan acceptance criterion A5. Tests added (tests/brief-llm.test.mjs, +9): - generateDigestProsePublic: two distinct callers with identical (sensitivity, story-pool) hit the SAME cache row (per Codex Round-2 Medium #4 — "no PII in public cache key"). - public + private writes never collide on cache key (defensive). - greeting bucket change re-keys the personalised cache (Brain B parity). - profile change re-keys the personalised cache. - v3 cache prefix used (no v2 writes). Test results: 77/77 in brief-llm; full data suite 6971/6971 (was 6962 pre-Step-7; +9 new public-cache tests). Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md Steps 6 (partial) + 7. Acceptance A5, A6.g, A6.f. * test(digest): backfill A6.h/i/l/m acceptance tests via helper extraction * fix(brief): close two correctness regressions on multi-rule + public surface Two findings from human review of the canonical-synthesis PR: 1. Public-share redaction leaked personalised signals + threads. The new prompt explicitly personalises both `lead` and `signals` ("personalise lead and signals"), but redactForPublic only substituted `lead` — leaving `signals` and `threads` intact. Public renderer's hasSignals gate would emit the signals page whenever `digest.signals.length > 0`, exposing watched-asset / region phrasing to anonymous readers. Same privacy bug class the original PR was meant to close, just on different fields. 2. Multi-rule users got cross-pool lead/storyList mismatch. composeAndStoreBriefForUser picks ONE winning rule for the canonical envelope. The send loop then injected that ONE `briefLead` into every due rule's channel body — even though each rule's storyList came from its own (per-rule) digest pool. Multi-rule users (e.g. `full` + `finance`) ended up with email bodies leading on geopolitics while listing finance stories. Cross-rule editorial mismatch reintroduced after the cross- surface fix. Fix 1 — public signals + threads: - Envelope shape: BriefDigest gains `publicSignals?: string[]` + `publicThreads?: BriefThread[]` (sibling fields to publicLead). Renderer's ALLOWED_DIGEST_KEYS extended; assertBriefEnvelope validates them when present. - generateDigestProsePublic already returned a full prose object (lead + signals + threads) — orchestration now captures all three instead of just `.lead`. Composer splices each into its envelope slot. - redactForPublic substitutes: digest.lead ← publicLead (or empty → omits pull-quote) digest.signals ← publicSignals (or empty → omits signals page) digest.threads ← publicThreads (or category-derived stub via new derivePublicThreadsStub helper — never falls back to the personalised threads) - New tests cover all three substitutions + their fail-safes. Fix 2 — per-rule synthesis in send loop: - Each due rule independently calls runSynthesisWithFallback over ITS OWN pool + ctx. Channel body lead is internally consistent with the storyList (both from the same pool). - Cache absorbs the cost: when this is the winner rule, the synthesis hits the cache row written during the compose pass (same userId/sensitivity/pool/ctx) — no extra LLM call. Only multi-rule users with non-overlapping pools incur additional LLM calls. - magazineUrl still points at the winner's envelope (single brief per user per slot — `(userId, issueSlot)` URL contract). Channel lead vs magazine lead may differ for non-winner rule sends; documented as acceptable trade-off (URL/key shape change to support per-rule magazines is out of scope for this PR). - Parity log refined: adds `winner_match=<bool>` field. The PARITY REGRESSION warning now fires only when winner_match=true AND the channel lead differs from the envelope lead (the actual contract regression). Non-winner sends with legitimately different leads no longer spam the alert. Test results: - tests/brief-magazine-render.test.mjs: 75/75 (+7 new for public signals/threads + validator + private-mode-ignores-public-fields) - Full data suite: 6995/6995 (was 6988; +7 net) - typecheck + typecheck:api: clean Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md Addresses 2 review findings on PR koala73#3396 not anticipated in the 5-round Codex review. * fix(brief): unify compose+send window, fall through filter-rejection Address two residual risks in PR koala73#3396 (single-canonical-brain refactor): Risk 1 — canonical lead synthesized from a fixed 24h pool while the send loop ships stories from `lastSentAt ?? 24h`. For weekly users that meant a 24h-pool lead bolted onto a 7d email body — the same cross-surface divergence the refactor was meant to eliminate, just in a different shape. Twice-daily users hit a 12h-vs-24h variant. Fix: extract the window formula to `digestWindowStartMs(lastSentAt, nowMs, defaultLookbackMs)` in digest-orchestration-helpers.mjs and call it from BOTH the compose path's digestFor closure AND the send loop. The compose path now derives windowStart per-candidate from `cand.lastSentAt`, identical to what the send loop will use for that rule. Removed the now-unused BRIEF_STORY_WINDOW_MS constant. Side-effect: digestFor now receives the full annotated candidate (`cand`) instead of just the rule, so it can reach `cand.lastSentAt`. Backwards-compatible at the helper level — pickWinningCandidateWithPool forwards `cand` instead of `cand.rule`. Cache memo hit rate drops since lastSentAt varies per-rule, but correctness > a few extra Upstash GETs. Risk 2 — pickWinningCandidateWithPool returned the first candidate with a non-empty raw pool as winner. If composeBriefFromDigestStories then dropped every story (URL/headline/shape filters), the caller bailed without trying lower-priority candidates. Pre-PR behaviour was to keep walking. This regressed multi-rule users whose top-priority rule's pool happens to be entirely filter-rejected. Fix: optional `tryCompose(cand, stories)` callback on pickWinningCandidateWithPool. When provided, the helper calls it after the non-empty pool check; falsy return → log filter-rejected and walk to the next candidate; truthy → returns `{winner, stories, composeResult}` so the caller can reuse the result. Without the callback, legacy semantics preserved (existing tests + callers unaffected). Caller composeAndStoreBriefForUser passes a no-synthesis compose call as tryCompose — cheap pure-JS, no I/O. Synthesis only runs once after the winner is locked in, so the perf cost is one extra compose per filter-rejected candidate, no extra LLM round-trips. Tests: - 10 new cases in tests/digest-orchestration-helpers.test.mjs covering: digestFor receiving full candidate; tryCompose fall-through to lower-priority; all-rejected returns null; composeResult forwarded; legacy semantics without tryCompose; digestWindowStartMs lastSentAt-vs-default branches; weekly + twice-daily window parity assertions; epoch-zero ?? guard. - Updated tests/digest-cache-key-sensitivity.test.mjs static-shape regex to match the new `cand.rule.sensitivity` cache-key shape (intent unchanged: cache key MUST include sensitivity). Stacked on PR koala73#3396 — targets feat/brief-two-brain-divergence.
lspassos1
pushed a commit
that referenced
this pull request
Apr 27, 2026
…populates (koala73#3410) * fix(ais-relay): subscribe to ShipStaticData so tanker layer actually populates User-reported on energy.worldmonitor.app: Live Tanker Positions layer renders zero vessels despite PR koala73#3402 wiring being correct end-to-end. Root cause: AISStream's PositionReport.MetaData does NOT carry `ShipType` per their schema — that field only arrives in ShipStaticData (Type 5) frames. PR koala73#3402 shipped tanker capture predicated on `meta.ShipType`: const shipType = Number(meta.ShipType); // always NaN if (Number.isFinite(shipType) && shipType >= 80 && shipType <= 89) { tankerReports.set(mmsi, ...); // never executes } The relay subscribed to AISStream with `FilterMessageTypes: ['PositionReport']` only — ShipStaticData never reached the relay, so the predicate always failed (NaN), `tankerReports` stayed permanently empty, and getVesselSnapshot returned `tankerReports: []` to every caller. The frontend layer correctly rendered zero vessels. Military detection (isLikelyMilitaryCandidate) survived because it has fallback paths (NAVAL_PREFIX_RE on ShipName + MMSI-suffix 00/99). Tanker detection had zero fallback because tanker MMSIs and names don't follow a single regex pattern. Fix: - Add 'ShipStaticData' to AISStream's FilterMessageTypes — Type 5 frames are broadcast every ~6 min per vessel (vs every 2-10s for PositionReport while underway), so the volume add is small. - Dispatch ShipStaticData → new processShipStaticDataForMeta handler that caches { shipType, shipName, lastSeen } by MMSI in a vesselMeta Map. - Tanker capture in processPositionReportForSnapshot now falls back to vesselMeta.get(mmsi).shipType when meta.ShipType is missing. - vesselMeta gets TTL eviction (24h) in cleanupAggregates so a long- running relay doesn't accumulate metadata for vessels that have left all tracked regions. 24h covers vessels with intermittent visibility while bounding memory growth. Tests: - tests/relay-tanker-shipstatic.test.mjs (new, 5/5 pass) — pins the fix shape so a regression can't flip FilterMessageTypes back to PositionReport-only. - `node -c` clean. Deploy: Requires a Railway redeploy of the AIS-relay service to pick up the new subscription filter. After redeploy, vesselMeta needs ~5-10 min to populate from ShipStaticData broadcasts before tanker classification reaches steady state for vessels in the 6 chokepoint bboxes. * fix(ais-relay): tighten ShipStaticData parsing — UserID fallback + field-name pinning (PR3410 review) Three review findings on PR koala73#3410: P2 — Static-analysis tests don't pin field names (correctness/testing): Pre-fix tests asserted `vesselMeta.set(... { shipType` which only checks the property name, not the source field. A typo regression like Number(sd.Typ) or Number(sd.shipType) (lowercase) would still pass — silently re-emptying the tanker layer. Added two new tests: - "reads ShipType from sd.Type (NOT meta.ShipType)" — pins the exact Number(sd.Type) substring; also pins sd.Name for shipName fallback. - "accepts MMSI from meta.MMSI OR sd.UserID" — pins the new fallback shape (see #2 below). P2 — MMSI-extraction lacks UserID fallback (correctness): AISStream's ShipStaticData payload sample mirrors MMSI as `UserID` on the message body, while PositionReport puts it under MetaData.MMSI. If a wrapper-schema variant ever ships a Type 5 frame without MetaData.MMSI, processShipStaticDataForMeta early-returned and vesselMeta stayed empty — silent re-empty of the tanker layer. Defensive: `String(meta.MMSI || sd.UserID || '')`. P3 — Test #4 order check used file-wide indexOf (testing): RELAY.indexOf('vesselMeta.get(mmsi)') finds the FIRST occurrence in the file. A future earlier vesselMeta.get(...) elsewhere would let the in-tanker-path lookup be removed without the test failing. Scoped the substring search to the body of processPositionReportForSnapshot (slice from fn declaration to the next top-level `function ` line). Tests: 7/7 pass (was 5). `node -c` clean. * fix(ais-relay): broaden shipType fix — chokepoint transit + military candidate paths also use vesselMeta User-reported review finding on PR koala73#3410: the previous commit fixed only the tanker snapshot path. Two more sites in processPositionReportForSnapshot read the same broken meta.ShipType field: Site 1 (line ~6651, vessels record): vessels.set(mmsi, { ..., shipType: meta.ShipType, ... }); Consumer: classifyVesselType(vessel?.shipType) at the chokepoint transit logging site (line ~6545). With shipType permanently undefined, classifyVesselType always returned 'other' — silently breaking per-type transit counts in /seedChokepointTransits and any downstream consumer using transit-by-type breakdowns. Site 2 (line ~6692, military candidate record): candidateReports.set(mmsi, { ..., shipType: meta.ShipType, ... }); isLikelyMilitaryCandidate(meta) — read meta.ShipType for the 35/55/50-59 type-based arms. Consequence: the military classifier survived in production only via the NAVAL_PREFIX_RE + MMSI-suffix fallbacks. The type-based arm never fired. candidateReports also exposed shipType: undefined to callers. Fix: - Compute `effectiveShipType` ONCE at the top of processPositionReportForSnapshot, preferring meta.ShipType when present (defense for any future schema enrichment) and falling back to vesselMeta cache from ShipStaticData. - Pass effectiveShipType to vessels.set, isLikelyMilitaryCandidate, and the tanker-capture predicate. All three sites now classify correctly. - Refactor isLikelyMilitaryCandidate(meta) → isLikelyMilitaryCandidate(meta, resolvedShipType) with backward-compat fallback to meta.ShipType when no override is passed. Same root cause, same vesselMeta cache solves all three sites. Tests: 10/10 (was 7) — added 3 new regression tests pinning: - vessels.set shipType field comes from `effectiveShipType` - isLikelyMilitaryCandidate signature accepts resolvedShipType param - candidateReports.set shipType field comes from `effectiveShipType` node -c clean. * fix(ais-relay): hard size cap + Type=0 guard on vesselMeta (PR3410 Greptile review) Two findings on PR koala73#3410: P1 — vesselMeta has TTL but no hard size cap: Every peer Map in cleanupAggregates (tankerReports, candidateReports, densityGrid, vesselHistory) follows its TTL loop with evictMapByTimestamp. vesselMeta did not — leaving memory growth unbounded against a hostile or buggy upstream flooding unique MMSIs faster than TTL drains them. Added MAX_VESSEL_META=50000 (covers ~50-70k active global AIS fleet with headroom) + evictMapByTimestamp call after the TTL loop. P2 — Number(null) === 0 could overwrite valid cache entries: processShipStaticDataForMeta's `if (!Number.isFinite(shipType)) return` guard accepts shipType=0 (AIS code 0 = "Not available" per ITU-R M.1371). A vessel that broadcasts {Type: 85} then later {Type: null} would have its cached tanker classification overwritten with shipType=0, downgrading it to non-tanker on the next PositionReport. Tightened to `|| shipType <= 0`. Tests: 11/11 (was 10) — added 2 new pins: - vesselMeta has TTL AND hard size cap (asserts MAX_VESSEL_META + evictMapByTimestamp(vesselMeta, ...)) - processShipStaticDataForMeta rejects shipType <= 0 (asserts the `<= 0` guard so a refactor can't silently drop it) node -c clean.
lspassos1
pushed a commit
that referenced
this pull request
Apr 27, 2026
…rs (koala73#3432) * fix(resilience): apply mrv=5 + null-skip recipe to remaining 4 WB seeders PR koala73#3427 fixed the mrv=1 + Number(null)=0 compound trap in 2 recovery seeders (external-debt + reserve-adequacy). A subsequent sweep (`grep -lE "mrv=1[^0-9]" scripts/*.mjs`) found 4 OTHER seeders with the same trap shape: - `seed-fossil-electricity-share.mjs` (EG.ELC.FOSL.ZS) - `seed-low-carbon-generation.mjs` (EG.ELC.{NUCL,RNEW,HYRO}.ZS) - `seed-power-reliability.mjs` (EG.ELC.LOSS.ZS) - `seed-sovereign-wealth.mjs` (NE.IMP.GNFS.CD via pickLatestPerCountry) All four migrated to the established recipe: 1. mrv=1 → mrv=5 + per-country pickLatest 2. per_page=500 → per_page=2000 (5x records per country = need bigger page) 3. Explicit `if (record?.value == null) continue` BEFORE Number() coercion 4. Year sanity check + per-country latest comparison **Why explicit null-skip matters more for the energy seeders:** The 3 energy seeders use "% of" indicators (EG.ELC.{FOSL,NUCL,RNEW, HYRO,LOSS}.ZS) where 0 IS a legitimate value (country has 0% nuclear, 0% fossil, perfect grid reliability, etc.). The accidental null-protection trick `if (value <= 0) continue` from the SWF picker WOULD WRONGLY DROP legitimate zeros for these. Must skip null explicitly so the `value === 0` case can flow through to scoring. **`seed-sovereign-wealth.mjs` was already mostly correct** (mrv=5 + pickLatest helper + accidental null protection via `value <= 0` since imports must be > 0). Added explicit null-skip as defense-in-depth so a future copy-paste of `pickLatestPerCountry` for a different indicator family doesn't silently lose null protection. **Lineage:** Plan 2026-04-26-001 cohort dry-run → user audit Priority Check #4 ("Replace mrv=1 with mrv=5 + latest non-null in ALL seeders") → this PR. Companion to PR koala73#3427. **Memory `wb-bulk-mrv1-null-coverage-trap`** was updated post-PR-koala73#3427 with the explicit `Number(null) === 0` warning + production case study — that memory is the recipe these 4 seeders now follow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix(resilience): null-skip in seed-wb-external-debt + bis-lbs GDP fetcher (review fixup on PR koala73#3432) Reviewer found one more WB seeder I missed in the original PR koala73#3432 sweep: **P1: `scripts/seed-wb-external-debt.mjs:69` had the exact same compound trap.** `Number(record?.value)` before any null check, then year-based latest-picker overwrites. Unlike the SWF imports picker (where `value <= 0` accidentally catches Number(null)=0 because imports must be positive), this script's downstream `combineExternalDebt` filter at `:98` only rejects negative debt — `debt.value < 0`, not `<= 0`. So a `value: null` record from a late-reporting LMIC (KW/QA/AE publish IDS data 1-2y behind G7) would coerce to `0`, win the year comparison, and propagate as a false `debtToReservesRatio: 0` → `0% short-term-debt-to-GNI` for the country. This feeds `economic:wb-external-debt:v1` consumed by the new `financialSystemExposure` dim (PR koala73#3412 / koala73#3407), so the false 0% would silently inflate the dim's score for affected LMICs. Fix: same recipe as PRs koala73#3427 and koala73#3432 — explicit `if (record?.value == null) continue;` BEFORE `Number()` coercion. **Defense-in-depth: `scripts/seed-bis-lbs.mjs:204` (the WB GDP fetcher inside the BIS LBS seeder) also gets the explicit null-skip.** This one was technically safe because GDP > 0 is a real-world invariant and the `value <= 0` filter catches Number(null)=0 by side effect. But per the lesson now codified in skill `wb-bulk-mrv1-null-coverage-trap`, the protection is fragile — future indicator-family changes that relax `value <= 0` would silently lose null protection. Adding the explicit null-skip makes the picker null-safe regardless of filter relaxation. **End state after this commit + PR koala73#3427 + PR koala73#3432 + this fixup:** ALL WB-bulk seeders in `scripts/seed-*.mjs` either (a) have an explicit `value == null` skip BEFORE coercion, or (b) use a non-WB endpoint shape (BIS SDMX, IMF SDMX, etc.) where the trap doesn't apply. Final sweep recipe documented in the commit; next reviewer can run it verbatim to audit any new seeder. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * docs: fix stale 'mrv=1' top-comment in seed-low-carbon-generation.mjs Reviewer follow-up on PR koala73#3432: the top-of-file rationale comment in `seed-low-carbon-generation.mjs:22` still claimed "We fetch the most-recent value (mrv=1)" even though the actual fetch was migrated to mrv=5 + null-skip in commit dd0be80. Non-blocking but worth cleaning up — this is the same doc-drift pattern memory `feedback_doc_drift_after_behavior_fix_needs_grep_sweep` warns about. Updated the comment to describe the current mrv=5 + per-country pickLatest behavior and reference the relevant skill + this PR. The other mrv=1 references in PR koala73#3432's touched files are intentional — they appear inside rationale comments that explain the trap before documenting why the seeder uses mrv=5 instead. Those should NOT change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]>
lspassos1
pushed a commit
that referenced
this pull request
May 20, 2026
* feat(convex): add mcpProTokens table + issue/validate/revoke + http routes (U1)
Non-key Pro MCP identity layer for the Pro-tier MCP access plan. Mirrors
apiKeys.ts shape but stores no key material — the row's _id IS the
bearer identifier (referenced from OAuth code/token records as
mcpTokenId).
- mcpProTokens table: {userId, clientId?, name?, createdAt, lastUsedAt?,
revokedAt?} indexed by_userId; userApiKeys untouched.
- Internal mutations: issueProMcpToken (tier ≥ 1 gate, 5-row cap with
silent oldest-revoke rotation, audit-preserving), validateProMcpToken,
internalRevokeProMcpToken (server-side rollback path for U5 — no Clerk
context needed), touchProMcpTokenLastUsed (debounced 5min).
- Public mutations: revokeProMcpToken + listProMcpTokens (Clerk-auth).
- HTTP internal routes: /api/internal-{issue,validate,revoke}-pro-mcp-token
with x-convex-shared-secret guard. Validate route schedules touch via
ctx.scheduler.runAfter, matching apiKeys pattern (http.ts:839).
28 new tests; full convex suite 245/245.
Plan unit U1: docs/plans/2026-05-10-001-feat-pro-mcp-clerk-auth-quota-plan.md
* feat(server): pro-mcp-token edge helper — issue/validate/revoke (U2)
Edge-runtime-safe wrappers around U1's Convex internal HTTP routes. No
positive cache on validate (revoke takes effect on next request);
negative-cache only at pro-mcp-token-neg:<tokenId> (60s TTL) for
known-bad bearers.
- issueProMcpTokenForUser: typed ProMcpIssueFailed (pro-required /
invalid-user-id / config / network), 3s timeout.
- validateProMcpToken: neg-cache short-circuit BEFORE Convex hit; fail-soft
on 5xx/timeout (returns null but does NOT write neg-cache sentinel —
prevents transient-blip poisoning of legitimate tokens). Convex's
validate route schedules touchProMcpTokenLastUsed internally; no
edge-side waitUntil needed.
- revokeProMcpToken: returns result object (no throw) so rollback callers
don't have their original error masked. Sets neg-cache sentinel after
successful revoke.
- invalidateProMcpTokenCache: public sentinel writer for U9.
25 tests including a load-bearing integration test for revoke → next
validate short-circuits without Convex round-trip.
Plan unit U2.
* feat(oauth): apex Clerk grant page + signed-grant mint API (U3)
Bridge between the apex Clerk session and the api-subdomain Pro MCP
consent flow. Apex /mcp-grant SPA reads the OAuth nonce + registered
client metadata so users see the real client_name + redirect host
(anti-phishing). On Authorize the page POSTs to mcp-grant-mint, gets
back a fixed redirect URL with a HMAC-signed grant token, and navigates.
- mcp-grant.html + src/mcp-grant-main.ts: Vite multi-page entry (matches
existing settings.html / live-channels.html convention).
- api/internal/mcp-grant-mint.ts: Clerk-auth POST. Re-checks tier ≥ 1,
re-validates redirect_uri allowlist (defense-in-depth), HMAC-signs
{userId, nonce, exp+5min}, SETEX mcp-grant:<nonce>, returns FIXED
redirect URL (https://api.worldmonitor.app/oauth/authorize-pro).
- api/internal/mcp-grant-context.ts: GET companion for the SPA to fetch
the real client_name + redirect_host. Same Pro-tier gate as mint.
- api/_mcp-grant-hmac.ts: shared sign/verify helper. Signature is over
exact post-base64url-decode bytes (key-order/whitespace independent).
U5 imports verifyGrant from this module.
- vercel.json: /mcp-grant → /mcp-grant.html rewrite + no-store header
rule. Catch-all SPA fallback adjusted to exclude the new page.
- api/oauth/register.js: export isAllowedRedirectUri so DCR allowlist is
reused (no parallel impl).
- 35 tests; full deploy-config + edge-functions + mcp + pro-mcp-token
suites green.
Plan unit U3.
* feat(oauth): consent page Pro-CTA-default + 'Use API key instead' (U4)
R1 of the plan: a logged-in Pro user authorising MCP from claude.ai
never sees the API-key input field by default. The consent page now
leads with a brand-green "Sign in with WorldMonitor Pro" CTA pointing
at the apex /mcp-grant page; the existing API-key form is hidden
behind a "Use API key instead" disclosure for Starter+ holders.
- Default state: form display:none; Pro CTA dominant.
- Error state: existing errorMsg path still works — when ke.textContent
is non-empty (or XHR-retry returns invalid_key), inline script auto-
reveals the form so the user doesn't lose context after a bad key.
- Deep-link: /oauth/authorize?...#api-key shows the form on initial
load, so Starter+ users can bookmark.
- Pro CTA href: https://worldmonitor.app/mcp-grant?nonce=<URL-encoded n>.
Apex page reads the OAuth nonce server-side (no client_name forwarded
via URL — already covered by U3's mcp-grant-context).
- nonce shared with U5: the same oauth:nonce:<n> row that U5's
/oauth/authorize-pro will GETDEL. Handler still mints exactly one
nonce per GET — no double-issue.
- XSS-escape preserved on client_name + redirect_host + nonce + errorMsg.
- 18 new tests; existing handler logic untouched.
Plan unit U4.
* feat(oauth): /oauth/authorize-pro bounce-back endpoint (U5)
Receives the apex grant bounce-back, validates the HMAC-signed grant,
atomically consumes both Redis nonces (mcp-grant + oauth:nonce), issues
a non-key Pro identity row in Convex via U2, and writes the OAuth code
with {kind:'pro', userId, mcpTokenId, ...} for U6 to read at exchange
time.
Security ordering (HMAC-first to avoid burning nonces on forged tokens):
1. verifyGrant (U3's _mcp-grant-hmac, no Redis)
2. URL-nonce vs payload-nonce match
3. GETDEL mcp-grant:<n>
4. strict {userId, exp} tuple match vs grant payload (forge defense)
5. GETDEL oauth:nonce:<n>
6. GET oauth:client:<client_id> + redirect_uri allowlist re-check
7. getEntitlements(userId) tier ≥ 1 re-check (lapse defense)
8. issueProMcpTokenForUser (U2)
9. SETEX oauth:code:<code> (kind:'pro', 600s)
10. 302 redirect_uri?code=<code>[&state=...] with Cache-Control:no-store
oauth:code shape (load-bearing for U6):
{kind:'pro', userId, mcpTokenId, client_id, redirect_uri,
code_challenge, scope:'mcp_pro'}
Best-effort revoke when SETEX fails after issueProMcpTokenForUser
succeeds — calls U2's no-throw revokeProMcpToken; logs orphan-row id
on revoke failure but never masks the original 500.
35 tests; vercel rewrite + api-route-exceptions registered as
external-protocol.
Plan unit U5.
* feat(oauth): McpAuthContext discriminated union + Pro token shape (U6)
Token endpoint and bearer resolver learn about Pro-shape tokens without
breaking Starter+ legacy paths.
api/_oauth-token.js:
- New resolveBearerToContext(token) returns a discriminated union:
{kind:'env_key', apiKey} -- legacy WORLDMONITOR_VALID_KEYS resolution
{kind:'pro', userId, mcpTokenId} -- new Pro identity
- resolveApiKeyFromBearer kept as backward-compat wrapper. Returns
apiKey for env_key kind, null for pro kind (so legacy callers can't
mis-handle a Pro bearer).
api/oauth/token.js → api/oauth/token.ts:
- Converted to TypeScript so it can import validateProMcpToken from
server/_shared/pro-mcp-token cleanly. Vercel routes by basename;
/oauth/token rewrite unaffected.
- Default handler wires injected deps via tokenHandler(req, deps) for
testability (mirrors U3/U5 pattern).
- authorization_code branch: dispatches on codeData.kind. Pro path
validates client_id + redirect_uri bind, PKCE-verifies, mints tokens
via storeProTokens (object shape). Legacy path unchanged.
- refresh_token branch: Pro refreshes call validateProMcpToken
(per-request Convex hit, no positive cache); revoked row → invalid_grant.
Cross-user defensive guard: Convex-returned userId must match bearer's.
family_id, mcpTokenId, scope ('mcp_pro') preserved across rotation.
- client_credentials grant untouched.
oauth:token:<uuid> shapes:
- Legacy (preserved): JSON.stringify("<sha256-hex-64>") or "<fingerprint-16>"
- Pro (new): JSON.stringify({kind:'pro', userId, mcpTokenId})
- oauth:refresh:<uuid> Pro shape includes client_id, scope, family_id
for rotation discipline (matches legacy semantics).
26 new tests; sibling suites (oauth-authorize, oauth-authorize-pro, mcp,
pro-mcp-token — 101 tests) regress green. tsc -p tsconfig.api.json clean.
Plan unit U6.
* feat(mcp): Pro identity, atomic INCR-first quota, internal HMAC fetch (U7)
Behavioral heart of the Pro MCP plan. api/mcp.ts now resolves bearers
as McpAuthContext (env_key | pro), runs Pro-specific pre-checks, and
enforces a hard 50/UTC-day cap via INCR-first reservation with
DECR-rollback on cap-exceed and on tool-dispatch failure.
Pre-dispatch chain for Pro context (synchronous, in order):
1. validateProMcpToken(mcpTokenId) — null = 401 revoked
2. defensive userId match — 401 cross-user
3. getEntitlements(userId) tier ≥ 1, mcpAccess: true, validUntil — fail-closed
4. per-minute slidingWindow(60, 60s) keyed pro-user:<userId> — fail-open
5. for tools/call ONLY: pipeline INCR + EXPIRE 172800
newCount > 50 → DECR rollback + -32029 + 429 + Retry-After
INCR Redis transient → -32603 + 503 + Retry-After:5 (hard-cap correctness)
6. dispatch tool; on throw → DECR rollback + -32603
Internal-HMAC tool fetches (replaces X-WorldMonitor-Key for Pro):
payload = ${ts}:${METHOD}:${pathname}:${queryHash}:${bodyHash}:${userId}
queryHash = SHA-256(canonicalQueryString) sorted keys, URL-encoded
bodyHash = SHA-256(bodyBytes) SHA-256("") for empty
sig = HMAC-SHA-256(MCP_INTERNAL_HMAC_SECRET, payload)
headers = X-WM-MCP-Internal: ${ts}.${b64url(sig)}, X-WM-MCP-User-Id
Replay defense: payload binds method+path+queryHash+bodyHash, so a
captured signature for /api/news/v1/list-feed-digest cannot be reused
on /api/intelligence/v1/deduct-situation. ±30s window.
server/_shared/mcp-internal-hmac.ts: single source of truth for sign
helpers. U8 verify imports the same canonicalisation primitives.
server/_shared/pro-mcp-token.ts: dailyCounterKey(userId), 50/172800s
constants exported for U9 to read the SAME Redis key the enforcement
writes.
_execute signatures changed (params, base, context) — option A from
the plan. Cache-only tools unchanged.
waitUntil unused: Convex internal-validate-pro-mcp-token schedules
touchProMcpTokenLastUsed itself (verified at convex/http.ts:1035-1040
from U1's commit 4530bdf).
22 new tests + 23 Starter+ regression tests in tests/mcp.test.mjs;
sibling chain (oauth-token, pro-mcp-token, oauth-authorize-pro,
mcp-grant-mint, mcp-proxy) all green. tsc + biome clean.
Plan unit U7.
* feat(gateway): HMAC-verify internal-MCP requests + sanitised propagation (U8)
Verifier counterpart to U7's signer. Gateway accepts X-WM-MCP-Internal
HMAC headers in lieu of an API key for Pro internal-MCP traffic, then
hands a freshly-constructed Request with sanitised trusted markers to
the downstream handler. isCallerPremium learns to honour those markers
so Pro framework/systemAppend semantics survive in summarize-article,
get-country-intel-brief, deduct-situation.
Gateway flow at the top of createDomainGateway's handler:
1. STRIP inbound x-wm-mcp-internal-verified + x-user-id (anti-injection)
2. If X-WM-MCP-Internal present:
verifyInternalMcpRequest re-canonicalises and HMAC-verifies the
request shape (method+pathname+queryHash+bodyHash+userId), ±30s
window, timing-safe compare. Failure → 401 invalid_internal_mcp_
signature (no fall-through to validateApiKey — present-but-bad is
a deliberate forge attempt).
getEntitlements(userId): tier ≥ 1 + mcpAccess === true (fail-closed).
Construct NEW Request via new Request(url, {method, body, headers})
with x-wm-mcp-internal-verified=<per-process nonce> +
x-user-id=<verified userId>. Skip validateApiKey + IP rate limit.
isCallerPremium adds a NEW first branch: timing-safe compare of
x-wm-mcp-internal-verified against the per-process nonce + non-empty
x-user-id + defensive getEntitlements re-fetch. Catches direct-edge-
function consumers (api/widget-agent.ts, chat-analyst.ts, me/entitlement.ts,
v2/shipping/webhooks/...) that don't run the gateway strip step.
DEFENSE-IN-DEPTH BEYOND PLAN: trusted marker is a per-process random
16-byte nonce, NOT the literal '1'. Sweep found direct edge functions
calling isCallerPremium without gateway-strip protection — a constant
marker would be spoofable from outside. The nonce is born once at
process startup, only the gateway knows it, comparison is timing-safe.
verifyInternalMcpRequest lives in server/_shared/mcp-internal-hmac.ts
next to U7's sign helpers — single source of truth for canonical form
+ payload + ±30s window. Drift between sign and verify is structurally
impossible.
26 new tests + 100 sibling regression tests (gateway-cdn-origin-policy,
premium-stock-gateway, premium-fetch, pro-mcp-token, mcp). tsc clean.
Plan unit U8.
* feat(catalog): mcpAccess feature flag + env vars + Pro-MCP docs (U10)
Catalog, schema, type, env, and docs scaffolding to make the Pro MCP
flow deployable end-to-end. U9 unblocks on this — settings UI gates on
hasFeature('mcpAccess') (distinct from existing apiAccess paywall).
- convex/config/productCatalog.ts: PlanFeatures.mcpAccess on every tier.
Free=false. Pro_monthly/Pro_annual=true. API_starter / API_starter_annual
/ API_business=true. Enterprise=true. Pro plan marketing copy mentions
"MCP access for Claude Desktop & other AI clients (50 calls/day)".
- convex/schema.ts + convex/payments/cacheActions.ts: mcpAccess optional
field on the entitlements validator; legacy rows pass.
- server/_shared/entitlement-check.ts: CachedEntitlements.features
mcpAccess?: boolean (consumed by U7 + U8).
- src/services/entitlements.ts: EntitlementState.features.mcpAccess?:
boolean — unblocks hasFeature('mcpAccess') for U9's settings tab gate.
- .env.example: new "Pro MCP" section with MCP_PRO_GRANT_HMAC_SECRET
(apex grant bridge, U3/U5) and MCP_INTERNAL_HMAC_SECRET (gateway
service-auth, U7/U8). Both 32-byte base64; both required.
- docs/mcp-server.mdx: "Pro sign-in flow" section + 50/day quota
callout + "Connected MCP clients" management pointer.
Bundled fixups for U7/U8 strict-mode TS errors:
- mcp-internal-hmac.ts: bufferToBase64Url uses for-of (avoids
noUncheckedIndexedAccess error on bytes[i]).
- gateway.ts: drop unused INTERNAL_MCP_USER_ID_HEADER import.
tsc -p tsconfig.api.json + tsc -p tsconfig.json clean.
9 entitlements tests + 71 gateway-internal-mcp + mcp tests pass.
Plan unit U10.
* feat(settings): Connected MCP clients tab + quota endpoint + revoke (U9)
User-facing surface that makes the Pro MCP plan visible. Settings UI
gets a new "Connected MCP clients" tab gated on hasFeature('mcpAccess')
(distinct from the existing apiAccess-gated 'API Keys' tab — Pro users
without apiAccess see only the new tab).
Endpoints:
- GET /api/user/mcp-quota → {used, limit:50, resetsAt}. Reads the
SAME Redis key (dailyCounterKey from U2) that U7 enforcement writes.
Single source of truth.
- POST /api/user/mcp-revoke {tokenId} → forwards Clerk-derived userId
(NOT client-supplied) to Convex internal-revoke-pro-mcp-token, then
calls invalidateProMcpTokenCache so any in-flight bearer with this
tokenId 401s within 60s. 404 on NOT_FOUND, 409 on ALREADY_REVOKED.
Tenancy enforced inside Convex (row.userId === userId).
UI (in src/components/UnifiedSettings.ts, no child component split —
matches existing API Keys tab pattern):
- Tab gated on hasFeature('mcpAccess') so Pro users see it without
needing apiAccess.
- Quota header auto-refreshes every 30s; interval cleared on tab-
switch / close / destroy.
- Each row: name, createdAt, lastUsedAt (relative), revokedAt
(struck-through + badge); active rows show Revoke button with
confirm() dialog (matches existing pattern).
- Empty state: click-to-copy https://api.worldmonitor.app/mcp.
Frontend service in src/services/mcp-clients.ts: listMcpClients (Convex
query), revokeMcpClient (POST /api/user/mcp-revoke), fetchMcpQuota.
11 + 14 = 25 new tests; api-route-exceptions registered as
internal-helper.
Plan unit U9.
---
PLAN COMPLETE — 10/10 units shipped. Follow-up polish noted in U9
report: CSS rules for .mcp-clients-* classes (suggest copy from
.api-keys-* rules); empty-state URL is hardcoded to
api.worldmonitor.app/mcp.
* style(settings): mcp-clients CSS rules mirroring api-keys (U9 polish)
* fix(pro-mcp): apply Tier-2 code-review residuals — security + correctness
Review pass after U10 (3 reviewers: code-reviewer, ce-security-reviewer,
ce-adversarial-reviewer). Findings BLOCKING + HIGH + MEDIUM + LOW
applied per user direction.
BLOCKING (gateway):
- Add validUntil check to gateway's HMAC-verify entitlement re-check.
Defense-in-depth against Convex-fallback paths returning a stale row
past its expiry.
HIGH — adv-001: cross-user OAuth nonce hijack:
- mcp-grant-mint now claims oauth nonces atomically via Upstash SET NX
semantics. If the nonce is already claimed by a DIFFERENT userId,
return 403 NONCE_CLAIMED_BY_OTHER_USER. Same userId multi-tab retry
is idempotent: re-sign the grant using the existing claim's exp so
authorize-pro's strict tuple equality still matches.
- mcp-grant-context performs the same cross-user check so the apex SPA
refuses to render consent context for a hijacked nonce.
MEDIUM — adv-008: refresh-token loss during Convex transient:
- validateProMcpToken returns a discriminated union {ok:'valid',userId}
| {ok:'revoked'} | {ok:'transient'}. Negative-cache writes only on
'revoked'. validateProMcpTokenOrNull wrapper preserves backward
compat for callers that don't need the distinction (per-request MCP
edge stays fail-closed).
- token.ts refresh path: on 'transient', best-effort restore the
consumed refresh token to Redis with the original TTL and return
503 server_error + Retry-After:5. Client retries; if Convex recovers,
refresh succeeds. No more permanent session loss on a Convex blip.
MEDIUM — adv-002: counter overshoot lockout:
- mcp.ts: on cap-exceeded after DECR rollback, if newCount is still
far above PRO_DAILY_QUOTA_LIMIT (overshoot from prior transient),
pipelined INCR+DECR probe + bounded DECR-sweep to converge the
counter back near the limit. Prevents one Redis hiccup from locking
out a paying Pro user for the rest of the UTC day.
MEDIUM — code-reviewer #4: 5-row cap race in Convex:
- mcpProTokens.issueProMcpToken now revokes ALL active rows beyond
MAX-1 (sorted by createdAt) on each issue, so concurrent inserts
that briefly produce 6 active rows converge back to 5 on the next
call.
LOW:
- adv-003: executeTool throws cache_all_null when every cache read
returns null AND there were keys configured — triggers DECR rollback
in dispatchToolsCall instead of silently burning quota on a degenerate
empty result.
- code-reviewer #10: gateway strips X-WM-MCP-Internal + X-WM-MCP-User-Id
from the trusted Request before forwarding to handler (no info leak).
- adv-004: 256 KB body cap (Content-Length + post-buffer count) on
both gateway strip and HMAC-verify paths — bounds memory amplification.
- adv-006: dailyCounterKey now env-prefixes (preview deploys can't
collide with production traffic on the same Upstash instance).
Reader (mcp-quota.ts) and writer (mcp.ts) share byte-identical keys
via the helper.
- adv-007 / code-reviewer #5: signInternalMcpRequest throws on
Blob/FormData/ReadableStream/plain-object bodies instead of silently
JSON.stringify'ing them (which would produce a hash that can't match
the wire bytes).
- code-reviewer #9: timestamp regex tightened to ^[0-9]{1,15}$ so
future ms-precision timestamps don't silently truncate through Number().
- code-reviewer #8: MCP_INTERNAL_HMAC_SECRET preflight in runProPreChecks
surfaces config errors at auth-resolution rather than mid-tool-fetch.
NITPICK:
- code-reviewer #6: rewritten readNegCache comment.
- code-reviewer #7: malformed-body error now reports reason
'malformed_request' instead of misleading 'auth_401'. New
RequestReason value added to usage.ts enum.
30 new tests + ~14 adapted; full convex suite 247/247, edge/server
suites 254/254. tsc clean across both tsconfig.json and tsconfig.api.json.
Plan: docs/plans/2026-05-10-001-feat-pro-mcp-clerk-auth-quota-plan.md
* chore(pro-marketing): regenerate /pro bundle for U10 catalog copy
U10's productCatalog.ts added an mcpAccess feature flag and updated the
Pro plan description copy to "MCP access for Claude Desktop & other AI
clients (50 calls/day)". scripts/generate-product-config.mjs cascades
that copy into pro-test/src/generated/tiers.json, which the /pro
marketing app builds into public/pro/assets/.
Vercel does NOT rebuild pro-test on deploy — public/pro/ ships whatever
is committed. Pre-push hook auto-ran the build and produced a new bundle
hash; this commit lands the regenerated artifacts so deploy picks them up.
Plan unit U10 (follow-on artifact).
* fix(pro-mcp): apply external review round-2 findings (P1+P2+P3)
Round-2 review on PR koala73#3646 surfaced 5 must-fix issues:
P1 — vercel.json `(?:...)` rejected by Vercel CI:
- Replace `mcp-grant(?:\\.html)?` with explicit `mcp-grant\\.html|mcp-grant`
inside the SPA catch-all negative-lookahead (lines 18 + 134).
- Split the headers `/mcp-grant(?:\\.html)?` source rule into two explicit
entries — Vercel's path-to-regexp `source` field doesn't accept `(?:...)`
with `?` quantifier as a standalone source.
- Update tests/deploy-config.test.mjs expectations to match.
P1 — api/api-route-exceptions.json points at deleted file:
- Update the entry path from `api/oauth/token.js` (deleted in U6) to
`api/oauth/token.ts`. `node scripts/enforce-sebuf-api-contract.mjs`
was failing pre-push CI.
P1 — mcpAccess migration gap on existing entitlement rows:
- convex/entitlements.ts now read-time merges `getFeaturesForPlan(planKey)`
defaults under stored features. Pre-U10 rows lacking `mcpAccess` see the
catalog default surfaced immediately, with NO wait for the next Dodo
webhook to rewrite the row. Stored features still win on conflict
(preserves per-user overrides). Mirrored explicitly in
convex/mcpProTokens.ts:55 (issueProMcpToken's direct ctx.db.query path
computes the same merge inline since it bypasses the query handler).
P2 — grant/issue path missing mcpAccess gate (let tier-1-without-mcpAccess
users complete OAuth then fail every tools/call at the gateway):
- mcp-grant-context.ts:95, mcp-grant-mint.ts:236, authorize-pro.ts:356,
convex/mcpProTokens.ts:55 now ALL gate on `tier ≥ 1 && mcpAccess === true`,
matching the downstream MCP-edge runProPreChecks check. Widens the
`getEntitlements` deps return type at all three handlers.
P3 — inline theme script blocked by global CSP:
- mcp-grant.html:34 inline `<script>` is removed (global CSP at vercel.json:92
is hash-allowlist-based and we don't allowlist this page's hashes).
- Theme application moved into the bundled module at src/mcp-grant-main.ts
(top of file, runs on module evaluation). Brief default-theme flash on
light-preference users is acceptable for a transient consent UI.
10 new tests covering: mcpAccess: false at each of the 4 gates,
mcpAccess: undefined (legacy row) at each of the 4 gates, and the
read-time catalog merge in both directions (legacy gets default,
override preserved).
Full gate post-fix:
- Convex 249/249 (+2 new merge tests).
- Edge/server 280/280 (+10 new gate tests).
- tsc both configs clean.
- sebuf contract clean (was failing before P1.2).
- Sentry coverage clean.
- deploy-config tests assert new explicit-source rules.
Plan: docs/plans/2026-05-10-001-feat-pro-mcp-clerk-auth-quota-plan.md
* fix(entitlement-cache): treat pre-U10 cache entries lacking mcpAccess as stale
Reviewer round-2 P2 (cache layer): _getEntitlementsImpl returned hot
Redis cache entries as-is, bypassing the read-time catalog merge that
convex/entitlements.ts:50 applies on the Convex read path. Paying users
with cache entries written before plan 2026-05-10-001 U10 deployed see
mcpAccess !== true at the grant/MCP gates and are blocked for up to
the 15-min cache TTL.
Cache predicate now also requires `typeof features.mcpAccess === 'boolean'`.
Legacy entries (mcpAccess: undefined) fall through to Convex, which
returns the merged shape and the cache is rewritten with the post-U10
layout. Self-healing, bounded to one extra Convex round-trip per
affected user during the migration window.
Targeted choice over alternatives:
- Importing convex/config/productCatalog into server/_shared to apply
the merge at the cache layer would pull non-edge-safe deps. Rejected.
- Treating ALL cached entries as stale would defeat the cache layer.
Rejected.
- typeof check on the new field is the minimum delta that fixes the
migration window without restructuring.
Test fixture in server/__tests__/entitlement-check.test.ts updated:
makeEntitlements now includes mcpAccess (tier ≥ 1 → true). Two new
focused tests:
1. Legacy cache entry without mcpAccess → cache rejected → Convex
round-trip → result returned (verifies fetch called once).
2. Post-U10 cache entry WITH mcpAccess → cache honored → no Convex
round-trip (verifies fetch called zero times).
Convex 251/251 (+2 new cache tests).
Plan: docs/plans/2026-05-10-001-feat-pro-mcp-clerk-auth-quota-plan.md
lspassos1
pushed a commit
that referenced
this pull request
May 20, 2026
…eads (koala73#3667) * fix(brief): grounding gate on digest synthesis — block hallucinated leads The 2026-05-12 0801 send shipped a "President Biden announced a new executive order targeting cryptocurrency mixers and privacy coins" lead to users whose actual story pool was Trump-era geopolitics (Iran ceasefire, Israeli strikes in Lebanon, Sudan drones, Cuba rhetoric, Russia/Ukraine, EU sanctions). The magazine envelope rendered the correct grounded lead; the email Executive Summary block rendered a fully fabricated narrative with four threads all about the imaginary Biden EO. Shape was valid — content was a hallucination from training- data priors. Root cause: validateDigestProseShape only checks shape (lengths, types, array presence). Any well-formed JSON whose lead names entities absent from the input pool was happily cached for 4h and re-served on every cache hit until the row TTL'd out. The canonical-brain promise from PR koala73#3396 ("compose-phase synthesis is reused on send-phase cache read") only guarantees same-output-per-cache-hit; it does not guard against the LLM committing to a fabricated row in the first place. This adds checkLeadGrounding(synthesis, stories): extract proper-noun tokens (capitalised, length ≥4) from story headlines; require ≥2 distinct hits in the lead + thread teasers (relaxed to 1 when the corpus has <4 anchor tokens). Length cap of 4 deliberately filters short-form acronyms (US/EU/UN/RSF) which are too generic to discriminate. Plumbed through validateDigestProseShape (new optional stories arg, back-compat preserved) → parseDigestProse → generateDigestProse cache-hit path. Cache key bumped v4 → v5 so existing v4 rows (which may carry shape-valid hallucinations) are evicted on rollout; 4h paid-for-once cost matching the established pattern at this function. When a cached row fails the grounding gate, the cron's existing 3-level fallback chain falls through to L2 (capped pool, no profile/greeting) and ultimately L3 (stub with "Digest" subject). A user gets either a re-rolled grounded lead or a degraded subject line — never a hallucinated headline. Test coverage: the verbatim May 12 hallucinated lead + the verbatim 2026-05-12 story headlines as fixtures, asserting the validator rejects. Plus the actual magazine lead from the same day as positive control, plus skip-on-empty-stories back-compat, plus single-story threshold relaxation, plus short-acronym filtering, plus a generateDigestProse cache-hit re-LLM regression test. 86/86 brief-llm tests pass. 179/179 brief-related tests pass. typecheck clean. biome lint clean. * fix(brief): address PR koala73#3667 review — lead must ground independently + token-set matching Two real bugs caught on review of the grounding validator. #1 — Combined haystack let a hallucinated lead pass when teasers happened to mention real entities. Before: lead + threads[].teaser were joined into one string and any ≥2 anchor hits across the combination passed. So a synthesis with a fabricated "President Biden announced..." lead and grounded teasers like "Trump rejected Tehran's response" would accept — even though the visible top-of-email lead stayed fabricated. After: lead must independently hit ≥1 anchor. Combined check (≥2 hits, or ≥1 for sparse corpora) still applies on top of that. A hallucinated lead with grounded teasers now correctly rejects. #2 — haystack.includes(tok) accepted unrelated entities via substring match. Before: anchor "iran" hit on "tirana", "oman" on "romania", "india" on "indiana". Any anchor that happens to appear as a substring of an unrelated word in the synthesis prose counted. After: both sides tokenise on the same delimiter regex into Sets and match by membership. Story-side keeps the proper-noun capitalisation filter; synthesis-side does not (so possessive forms / sentence-medial mentions still count as anchor hits). Both regressions covered by new golden tests: - hallucinated-lead-grounded-threads: rejects with the new lead independence requirement, accepts when the lead is also grounded - substring-trap-corpus (Iran/Oman/India + Tirana/Romania/Indiana): rejects under token-set matching, accepts under real word matches Tests: 88/88 brief-llm, 8604/8604 full suite, typecheck clean, biome clean. * fix(brief): address PR koala73#3667 review round 2 — stopword filter + Unicode delimiters Two more bugs in the grounding validator caught on second review. #1 (HIGH) — Generic capitalised words leaked into the anchor set. Before: any capitalised word ≥4 chars from a headline became an anchor. So "President Trump signed Iran sanctions" added "president" to storyTokens. A hallucinated lead like "President Biden announced..." passed the lead-anchor check via the shared "president" token, and a teaser mentioning Iran satisfied the combined threshold — recreating the exact failure mode this PR is trying to block. After: GROUNDING_ANCHOR_STOPWORDS filters honorifics ("President", "Senator", "Minister"), generic role labels ("Officials", "Members", "Forces"), quasi-adjectives ("Senior", "Federal", "Former"), sentence-start filler ("After", "Following", "Despite"), and calendar words. Specific entity names (Iran, Trump, Israel, EU) are deliberately NOT on the list. "May" is also omitted (Theresa May, May Day, May the month all collide). #2 (MEDIUM) — Unicode apostrophes weren't delimiters. Before: GROUNDING_TOKEN_DELIMS only included ASCII apostrophe. Reuters/AP/Guardian headlines use U+2019 ("China's", "Iran's", "DPRK's"). The regex didn't split on U+2019, so "China's" became a single token "china's" that no normal lead saying "China" could ever match — a false negative that would reject genuinely grounded leads. After: U+2018, U+2019, U+201C, U+201D, U+00B4 added alongside ASCII counterparts. Both regressions covered by new golden tests: - "President Trump signed..." headlines + "President Biden announced..." hallucinated lead must REJECT - U+2019 headlines + ASCII-quote grounded lead must ACCEPT Tests: 90/90 brief-llm, 8606/8606 full suite, typecheck clean, biome clean. * fix(brief): address PR koala73#3667 review round 3 — bigram-leading title stopwords Round 2 added "President" to the anchor stopword list. Round 3 review caught that other common bigram titles still leak through. Before: "Prime Minister Netanyahu says Iran threats continue" added "prime" to anchors. A hallucinated "Prime Minister Trudeau announced cryptocurrency restrictions..." passed the lead-anchor check via the shared "prime" token. A teaser mentioning Iran satisfied the combined threshold. Same shape worked for Chief Justice / Cardinal X / Chancellor X / Speaker X / Ambassador X. After: GROUNDING_ANCHOR_STOPWORDS extended with bigram-leading titles: prime, chief, premier, chancellor, speaker, ambassador, envoy, commissioner, attorney, cardinal, archbishop, monsignor, reverend, pastor, bishop, lord, lady, dame, congressman/woman/person, representative, delegate, baron(ess). Regression test covers Prime Minister / Chief Justice / Cardinal ride-along leads, plus a counter-control naming the actual entity (Netanyahu) which still passes. Tests: 91/91 brief-llm, 8607/8607 full suite, typecheck clean, biome clean. * fix(brief): observability for digest synthesis failures (PR koala73#3667 review #3) Greptile review caught: when generateDigestProse returns null, ops can't tell whether the LLM threw, returned no content, returned malformed JSON, or returned a shape-valid hallucination that the grounding gate rejected. All four classes look identical in logs — just "L1 returned null" — so a sustained model regression looks identical to an infra blip during on-call triage. Add two distinct console.warn lines: - "LLM call threw" — provider/network failure (catches the actual error message for triage) - "ungrounded or malformed output" — provider returned but parsing or grounding rejected (logs text length so 0 vs ~900-char distinguishes "no content" from "model drift/hallucination") Cost note documented inline: we deliberately do NOT cache the failure with a sentinel under the v5 key. At temperature 0.4 the next cron tick may roll a grounded output for the same prompt; caching the failure would block legitimate retries. The L1→L2→L3 fallback in runSynthesisWithFallback handles user-visible degradation; these logs handle ops visibility. Tests: 91/91 brief-llm pass, typecheck clean, biome clean. * fix(brief): PR koala73#3667 review round 4 cleanup — extract grounding helpers + fixture comment Two reviewer-flagged cleanups: - P2: extract `extractAnchors` / `tokensetOf` from inside checkLeadGrounding to file-level `extractAnchorTokens` / `groundingTokenSet`. Avoids closure re-instantiation per call, separates the two helpers cleanly, and makes them individually inspectable / unit-testable. Behaviour unchanged — same callers, same inputs, same outputs. - P3: add a load-bearing comment on the `story()` test factory documenting that the default headline ("Iran threatens to close Strait of Hormuz...") is what grounds every `validJson` lead used by `generateDigestProse` / `generateDigestProsePublic` cache-shape tests. If a future contributor changes the default headline so it no longer mentions Iran/Hormuz, those tests would silently reject via the v5 grounding gate with cascading "expected truthy, got null" failures whose root cause is invisible. Comment names the invariant + the escape hatch (override headline + update validJson). Tests: 91/91 brief-llm pass, typecheck clean, biome clean. * fix(brief): PR koala73#3667 review round 5 — JSDoc attachment, blind-spot warn, stopword heuristic Three reviewer findings: #1 — JSDoc block was orphaned by the round-4 helper extraction. The big "Cheap content-grounding check..." doc sat at line 519, BEFORE extractAnchorTokens and groundingTokenSet — JSDoc parsers attach a doc block to the NEXT declaration, so the rich docs were describing the wrong function. Moved directly above checkLeadGrounding where they belong. #2 — Lowercase-headline blind spot. If a feed ever produces all- lowercase or all-≤3-char headlines, extractAnchorTokens returns empty for every story, storyTokens.size === 0, and the gate silently returns true (skip). Added a console.warn gated on stories.length >= 3 so synthetic single-headline test corpora don't spam logs but real production feed regressions surface in cron output. #3 — Stopword maintenance heuristic. Added a comment to GROUNDING_ANCHOR_STOPWORDS describing the detection rule: dump a week of real headlines, tokenise with stopwords disabled, count frequencies, inspect any token appearing in >~10% of headlines that isn't a known proper noun. The Prime/Chief/Cardinal gaps caught on rounds 2-3 would have surfaced from such a frequency audit. Captures the maintenance burden as an actionable signal rather than guesswork. Tests: 91/91 brief-llm pass, typecheck clean, biome clean. The new console.warn calls are intentionally surface in test output when triggered (reviewer round 5 #4 — informational, no action).
lspassos1
pushed a commit
that referenced
this pull request
May 20, 2026
…oala73#3748) * feat(brief): add feelgood-classifier shared module (U1) Sibling to classifyOpinion (PR koala73#3690). Stamps/filters editorially-unfit feel-good and lifestyle content out of the brief pool. Design (round-2 ce-doc-review): - HARD-NEWS VETO (R3a) runs FIRST; overrides every classification path (STRONG URL, STRONG headline prefix, CORROBORATING). Expanded with morphology variants so natural news prose vetoes correctly ("strike kills six" — not requiring exact "airstrike" / "killed"). - CORROBORATING threshold raised from 2 to 3 distinct group names (adv-R2-003 — prevents reachable hard-news FPs on 2-token stacks). - Distinct-token identity is the alternation-group LABEL, not raw match text (adv-R2-002 — collapses reunite/reunited/reuniting into reunite_group so RSS-typical inflection echoes count once). - /local/, /photos/, /travel/, /style/ are CORROBORATING only — major outlets file legitimate hard news under those segments (adv-R2-001). - URL match uses new URL().pathname (try/catch), not raw .includes() — closes the tracking-param injection vector (?utm=/local/promo). - "restored" and "meet the" excluded from token list (FPs on art-restitution news and diplomacy headlines). See docs/plans/2026-05-17-001-fix-feelgood-lifestyle-filter-plan.md for full design rationale + Veterans-warplanes anchor case (May 17 0802 brief, card #4). 36 tests; biome clean. * feat(brief): stamp isFeelGood on story:track:v1 at ingest (U2) Sibling to the isOpinion stamp PR koala73#3690 added. parseRssXml computes classifyFeelGood({title, link, description}) and tags every ParsedItem; buildStoryTrackHsetFields persists 'isFeelGood' as '1'|'0' on the Redis HSET row so buildDigest's read-path filter can exclude feel-good content without re-classifying. Mirrors the four isOpinion sites exactly: - Import at :17 (sibling to opinion-classifier import) - ParsedItem field at :163 (sibling to isOpinion) - parseRssXml stamp at :478 (sibling to classifyOpinion call) - buildStoryTrackHsetFields persist at :911 (sibling to isOpinion field) Tests: 2 new (parseRssXml carries the flag + Veterans anchor; HSET persistence + legacy-residue → '0'). Existing baseItem fixture gains isFeelGood: false default (matches PR koala73#3690's isOpinion pattern). * feat(brief): drop isFeelGood rows in buildDigest + telemetry (U3) Sibling to the buildDigest opinion filter PR koala73#3690 added. Trusts the ingest stamp (isFeelGood === "1") AND re-classifies stamp-missing residue rows from persisted title/link/description so the filter is effective immediately on rollout, not only after the 48h TTL window. Mirrors the four opinion filter sites exactly: - Import classifyFeelGood at :34 (sibling to opinion-classifier import) - droppedFeelGood counter at :490 (sibling to droppedOpinion) - Filter block immediately after opinion block (:522-549), BEFORE derivePhase / matchesSensitivity — earlier filtering keeps downstream telemetry clean - Conditional log at :567-573 mirrors the opinion log byte-for-byte: "[digest] buildDigest feel-good filter dropped N feel-good/lifestyle item(s) from the pool (variant=… lang=… sensitivity=…)". Per FEAS-001, no invented stamped/residue parenthetical (opinion mirror has none). M6 / adv-005 — counter asymmetry documented in inline comment: a row matched by BOTH classifiers (columnist-nostalgia essay; op-ed with tribute framing) increments only droppedOpinion (opinion `continue`s first). droppedFeelGood is therefore "rows the feel-good filter dropped *after* opinion passed on them this run," not "all feel-good content seen." Applies stamped/residue/mixed paths equally. See plan Operational Notes for the operator-runbook version. Per-attempt [digest] brief filter drops log is intentionally unchanged — it does not carry dropped_opinion= today and must not gain dropped_feelgood= either (C1). 14 source-textual tests in greenfield tests/digest-buildDigest-feelgood-filter.test.mjs + no regressions across 218 tests in brief-from-digest-stories / brief-llm / seed-envelope-parity sweep. * chore(deploy): COPY feelgood-classifier into digest-notifications image (U4) Without this COPY the Railway cron crashes at startup with ERR_MODULE_NOT_FOUND on the new '../server/_shared/feelgood-classifier.js' import that U3 added to scripts/seed-digest-notifications.mjs. Same failure mode the transitive-import-closure test caught for the opinion-classifier in PR koala73#3690 — that test still passes here by construction. * test(carousel): bump PNG-render per-test timeout (orthogonal to feel-good filter) PNG cold-render (font + resvg-wasm init) measures ~10s on a warm machine and longer under concurrent test load. The node:test default per-test timeout (5s in tsx --test) false-fails these three tests in the pre-push full-suite sweep while they pass in isolation. Bumping to 30s removes the false failure without masking real regressions. Pre-existing flake, unrelated to this PR's feel-good filter — surfaced because adding new tests/*.test.mjs files flipped the pre-push hook into RUN_ALL mode. * fix(brief): bump rss:feed cache prefix v3→v4 to close pre-PR ParseResult leakage PR-review finding: pre-PR cached ParseResults in rss:feed:v3 contain ParsedItems without isFeelGood. If a cache hit returned one during the 1h healthy-cache rollout window, buildStoryTrackHsetFields would write `'isFeelGood', undefined ? '1' : '0'` → '0' onto the fresh story:track:v1 row. buildDigest's stampMissing check (`typeof !== 'string' || length === 0`) treats '0' as a genuine "not feel-good" verdict and skips the residue catch — feel-good content could silently slip through for up to one hour post-deploy. Fix: bump the cache prefix v3→v4 so every pre-PR ParseResult is invalidated on deploy. The first post-deploy cron tick is a cold read for every feed; fresh parseRssXml runs stamp isFeelGood correctly before buildStoryTrackHsetFields ever sees the item. Mirrors the v2→v3 precedent on this same cache for the same class of parsed-cache-shape change. Same latent bug exists in PR koala73#3690's isOpinion path (1h leakage window after that PR shipped, masked by the TTL). A separate backport can address it; out of scope for this PR. New test: tests/news-story-track-description-persistence.test.mts asserts the v4 cache prefix is present in fetchAndParseRss source — locks the cutover so a refactor cannot silently revert to v3. Updated comment on the existing missing-field test to clarify that the '0' fallback is buildStoryTrackHsetFields' own behavior; the cache-leakage failure mode is closed by the v4 prefix bump above.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This moves bootstrap and health dataset registration behind a generated contract registry in the fork. The runtime behavior stays aligned with the existing bootstrap and health semantics, while the source of truth moves to
registry/datasets.tsand drift is enforced in CI and pre-push.Related issue: #5.
Root cause
Bootstrap keys, health keys, seed metadata, and tier definitions had drifted across manual maps in
api/bootstrap.js,api/health.js,server/_shared/cache-keys.ts, and multiple tests. That made parity fixes and new dataset additions depend on synchronized edits across unrelated files.Changes
registry/datasets.tsplusscripts/generate-dataset-registry.tsandscripts/check-dataset-registry.mjsapi/_generated/dataset-registry.js,api/_generated/health-registry.js, andserver/_shared/_generated/bootstrap-registry.tsconsumerPricesOverview,consumerPricesCategories,consumerPricesMovers, andconsumerPricesSpreadbootstrap aliasesapi/health.jsto generated health registries while preserving bootstrap/standalone grouping, seed status handling, cascade logic, and compact mode behaviorserver/_shared/cache-keys.tsnpm run registry:checkin.husky/pre-pushand.github/workflows/typecheck.ymldocs/architecture/dataset-registry.mdValidation
npm run registry:checknode --test tests/bootstrap.test.mjsnode --test tests/edge-functions.test.mjsnpm run test:datanpm run typechecknpm run typecheck:apiRisk
Low to moderate. The runtime code path is intentionally conservative, but the generated registry now feeds both bootstrap and health, so future contract edits can affect both endpoints at once.