Skip to content

feat(brief): route whyMatters through internal analyst-context endpoint#3248

Merged
koala73 merged 4 commits into
mainfrom
feat/brief-why-matters-analyst-endpoint
Apr 21, 2026
Merged

feat(brief): route whyMatters through internal analyst-context endpoint#3248
koala73 merged 4 commits into
mainfrom
feat/brief-why-matters-analyst-endpoint

Conversation

@koala73

@koala73 koala73 commented Apr 21, 2026

Copy link
Copy Markdown
Owner

Summary

The brief's "why this is important" callout is currently generated by Gemini on only {headline, source, threatLevel, category, country} with no live state. The LLM can't know whether a ceasefire is on day 2 or day 50, that IMF flagged >90% gas dependency in UAE/Qatar/Bahrain, or what today's forecasts look like — so it writes generic prose instead of the situational analysis WMAnalyst produces when given live context.

This PR adds an internal Vercel edge endpoint that reuses a trimmed variant of assembleAnalystContext (country-brief, risk scores, top-3 forecasts, macro signals, market data — no GDELT, no digest-search) and ships it through a one-sentence LLM call with the existing WHY_MATTERS_SYSTEM prompt.

Three-layer graceful degradation — endpoint → legacy Gemini-direct → stub. The brief always ships.

What's new

File Role
api/internal/brief-why-matters.ts New edge endpoint. Auth + cache + paths + shadow.
shared/brief-llm-core.js (+ mirror at scripts/shared/) Edge-safe core: WHY_MATTERS_SYSTEM, parseWhyMatters, async hashBriefStory (Web Crypto, zero node:crypto).
server/_shared/internal-auth.ts Bearer + subtle-crypto HMAC timing-safe compare.
server/_shared/country-normalize.ts ISO2 normalizer backed by shared/country-names.json.
server/worldmonitor/intelligence/v1/brief-story-context.ts Trimmed context — 6 parallel Redis GETs.
server/worldmonitor/intelligence/v1/brief-why-matters-prompt.ts Compact context block + story fields.
scripts/seed-digest-notifications.mjs New callAnalystWhyMatters(story) wrapper threaded into briefLlmDeps.
scripts/lib/brief-llm.mjs generateWhyMatters now tries the analyst endpoint first; falls through on any failure.

Env knobs

Var Default What
BRIEF_WHY_MATTERS_PRIMARY analyst analyst or gemini. Typo → gemini (safe fallback) + warn.
BRIEF_WHY_MATTERS_SHADOW 1 Only '0' disables shadow mode.
BRIEF_WHY_MATTERS_SHADOW_SAMPLE_PCT 100 Int 0–100. Deterministic on hash. Drop to 10–20 after initial validation to cut LLM spend.
BRIEF_WHY_MATTERS_ENDPOINT_URL (Railway only) ${WORLDMONITOR_PUBLIC_BASE_URL}/api/internal/brief-why-matters Override for local dev.

Secrets already in place on both Vercel + Railway: RELAY_SHARED_SECRET, UPSTASH_REDIS_REST_URL, UPSTASH_REDIS_REST_TOKEN, OPENROUTER_API_KEY, reasoning-LLM keys. No new secret to set.

Cache keys

  • brief:llm:whymatters:v3:{hash16} — envelope {whyMatters, producedBy, at}, 6h TTL. Endpoint-owned.
  • brief:llm:whymatters:shadow:v1:{hash16}{analyst, gemini, chosen, at}, 7d TTL. Fire-and-forget.
  • brief:llm:whymatters:v2:{hash16} — legacy. Cron's fallback path still reads/writes this during rollout; expires in ≤24h.

Hash material is the 5-field tuple from the existing hashBriefStory — unchanged, so cache identity is stable across the cron + endpoint.

Testing

  • 6022 tests pass (was 5915 + 12 edge-core + 36 endpoint + misc).
  • typecheck + typecheck:api clean.
  • biome check on changed files clean (pre-existing main() complexity warning unchanged).
  • tests/edge-functions.test.mjs extended to recurse api/** — the new api/internal/brief-why-matters.ts is covered by the node:-import guard.
  • Gemini path prompt parity pinned via snapshot test.
  • hashBriefStory parity test asserts the Web Crypto impl emits the exact 16-hex the pre-extract node:crypto impl did — deploy-time cache invalidation guard.

Plan

4-round Codex-approved plan at docs/plans/2026-04-21-001-feat-brief-why-matters-analyst-endpoint-plan.md.

Post-Deploy Monitoring & Validation

  • What to monitor/search
    • Cache hit rate: SCAN 0 MATCH brief:llm:whymatters:v3:* COUNT 1000 every 30 min.
    • Shadow key count: SCAN 0 MATCH brief:llm:whymatters:shadow:v1:* COUNT 1000.
    • Fallback-chain fires: Sentry/Railway log filter on [brief-llm] callAnalystWhyMatters → fallback:.
    • Endpoint latency: Vercel function metrics, P50/P95/P99.
    • Validation rejections: grep for [brief-why-matters] validation_reject.
  • Validation checks (curl)
    • curl -X POST -H "Authorization: Bearer \$RELAY_SHARED_SECRET" -H "Content-Type: application/json" -d '{"story":{"headline":"test","source":"test","threatLevel":"low","category":"General","country":"US"}}' https://worldmonitor.app/api/internal/brief-why-matters200 with JSON response.
    • curl -X POST -H "Authorization: Bearer wrong" …401.
    • curl -X POST … -d '{"story":{"threatLevel":"info",…}}'400 (bad threatLevel; only critical|high|medium|low accepted).
  • Expected healthy behavior
    • Cache hit rate ≥ 30% after 6h, ≥ 50% after 12h (6h TTL).
    • Brief story detail pages show richer whyMatters text than pre-PR.
    • Zero [digest] brief enrichment failed in Railway logs.
  • Failure signals / rollback trigger
    • Endpoint P99 > 20s sustained → flip BRIEF_WHY_MATTERS_PRIMARY=gemini on Vercel (instant, no deploy).
    • parseWhyMatters reject rate > 30% on analyst → tighten prompt or drop maxTokens.
    • Envelope validation failures in cron → PRIMARY=gemini.
    • Shadow records show analyst consistently worse → PRIMARY=gemini + keep shadow on.
  • Validation window & owner
    • Window: 48h shadow + 7 days post-shadow-flip.
    • Owner: koala73.
  • Rollback tiers (instant, no deploy)
    • Tier 1: BRIEF_WHY_MATTERS_PRIMARY=gemini.
    • Tier 2: BRIEF_WHY_MATTERS_SHADOW=0 (stops double-spend; keeps primary).
    • Tier 3: BRIEF_WHY_MATTERS_SHADOW_SAMPLE_PCT=0 (retain infra, spend nothing).
    • Tier 4 (deploy): revert cron's callAnalystWhyMatters wiring.

Shadow-diff validation procedure (post-deploy)

After 48h with SHADOW=1 + SAMPLE_PCT=100:

  1. Fetch: SCAN 0 MATCH brief:llm:whymatters:shadow:v1:*.
  2. Sample 20–30 pairs via a one-shot script.
  3. Score each on: situational accuracy, specificity (named actor/metric), editorial voice.
  4. If analyst wins ≥ 70%: drop SAMPLE_PCT=20 for 2 weeks, then SHADOW=0; delete legacy path in follow-up PR.

Compound Engineering v2.49.0
🤖 Generated with Claude Opus 4.7 (1M context, extended thinking) via Claude Code

The brief's "why this is important" callout currently calls Gemini on
only {headline, source, threatLevel, category, country} with no live
state. The LLM can't know whether a ceasefire is on day 2 or day 50,
that IMF flagged >90% gas dependency in UAE/Qatar/Bahrain, or what
today's forecasts look like. Output is generic prose instead of the
situational analysis WMAnalyst produces when given live context.

This PR adds an internal Vercel edge endpoint that reuses a trimmed
variant of the analyst context (country-brief, risk scores, top-3
forecasts, macro signals, market data — no GDELT, no digest-search)
and ships it through a one-sentence LLM call with the existing
WHY_MATTERS_SYSTEM prompt. The endpoint owns its own Upstash cache
(v3 prefix, 6h TTL), supports a shadow mode that runs both paths in
parallel for offline diffing, and is auth'd via RELAY_SHARED_SECRET.

Three-layer graceful degradation (endpoint → legacy Gemini-direct →
stub) keeps the brief shipping on any failure.

Env knobs:
- BRIEF_WHY_MATTERS_PRIMARY=analyst|gemini (default: analyst; typo → gemini)
- BRIEF_WHY_MATTERS_SHADOW=0|1 (default: 1; only '0' disables)
- BRIEF_WHY_MATTERS_SHADOW_SAMPLE_PCT=0..100 (default: 100)
- BRIEF_WHY_MATTERS_ENDPOINT_URL (Railway, optional override)

Cache keys:
- brief:llm:whymatters:v3:{hash16} — envelope {whyMatters, producedBy,
  at}, 6h TTL. Endpoint-owned.
- brief:llm:whymatters:shadow:v1:{hash16} — {analyst, gemini, chosen,
  at}, 7d TTL. Fire-and-forget.
- brief:llm:whymatters:v2:{hash16} — legacy. Cron's fallback path
  still reads/writes during the rollout window; expires in ≤24h.

Tests: 6022 pass (existing 5915 + 12 core + 36 endpoint + misc).
typecheck + typecheck:api + biome on changed files clean.

Plan (Codex-approved after 4 rounds):
docs/plans/2026-04-21-001-feat-brief-why-matters-analyst-endpoint-plan.md
@vercel

vercel Bot commented Apr 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
worldmonitor Ignored Ignored Preview Apr 21, 2026 10:00am

Request Review

@greptile-apps

greptile-apps Bot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR routes the brief's whyMatters enrichment through a new Vercel Edge endpoint (/api/internal/brief-why-matters) that assembles live analyst context (country-brief, risk scores, forecasts, macro signals, market data) before the LLM call, replacing the context-blind legacy Gemini path. A three-layer fallback (analyst endpoint → legacy Gemini cache+LLM → stub), shadow mode for A/B comparison, and env-knob rollback are all well-designed.

  • P1 — shadow writes may be silently lost: The fire-and-forget redisPipeline call for shadow records is not wrapped in ctx.waitUntil. On Vercel Edge, unawaited Promises can be dropped when the Response is returned, making the planned 48h A/B shadow-validation procedure (SCAN brief:llm:whymatters:shadow:v1:*) produce zero results. The handler needs a second ctx: { waitUntil } parameter, matching the existing repo pattern checked in tests/edge-functions.test.mjs.

Confidence Score: 4/5

Safe to merge for the primary analyst path; shadow-mode observability is broken until the ctx.waitUntil fix is applied.

One P1 finding: the fire-and-forget shadow Redis write will likely be dropped on Vercel Edge, silently breaking the post-deploy A/B validation procedure. The primary enrichment path, fallback chain, auth, caching, and validation are all correct. The P2 findings (context budget break-vs-continue, auth length short-circuit) are non-blocking.

api/internal/brief-why-matters.ts — shadow write needs ctx.waitUntil; server/worldmonitor/intelligence/v1/brief-why-matters-prompt.ts — budget break drops fitting sections.

Important Files Changed

Filename Overview
api/internal/brief-why-matters.ts New internal edge endpoint for analyst-backed whyMatters; well-structured with validation, caching, and shadow mode — but the fire-and-forget shadow Redis write lacks ctx.waitUntil, meaning shadow records may be silently dropped on Vercel Edge.
server/_shared/internal-auth.ts New bearer-token auth helper using Web Crypto HMAC for timing-safe comparison; logic is correct but the length-mismatch early-return slightly weakens the constant-time guarantee.
server/worldmonitor/intelligence/v1/brief-why-matters-prompt.ts Compact context-block builder with per-section caps and a total budget; the break on budget overflow drops all remaining sections even when smaller ones would fit.
server/worldmonitor/intelligence/v1/brief-story-context.ts Trimmed 6-parallel-fetch context assembler; clean use of Promise.allSettled with degraded-flag logic; looks correct.
shared/brief-llm-core.js Edge-safe pure helpers (WHY_MATTERS_SYSTEM, parseWhyMatters, hashBriefStory via Web Crypto); hash parity with the legacy node:crypto implementation is pinned by a fixture test.
scripts/lib/brief-llm.mjs generateWhyMatters gains analyst-endpoint priority path with clean three-layer fallback (analyst → legacy Gemini → stub); backcompat preserved when callAnalystWhyMatters is absent.
scripts/seed-digest-notifications.mjs callAnalystWhyMatters wrapper correctly wired into briefLlmDeps with AbortSignal.timeout and graceful null fallback; URL defaults through WORLDMONITOR_PUBLIC_BASE_URL.
tests/brief-why-matters-analyst.test.mjs Comprehensive tests covering country normalizer, cache-key stability, shadow sampling determinism, generateWhyMatters priority chain, body validation, prompt shape/budget, env flag parsing, and Gemini prompt parity snapshot.

Sequence Diagram

sequenceDiagram
    participant Cron as Railway Cron
    participant Edge as Vercel Edge Endpoint
    participant Redis as Upstash Redis
    participant LLM as LLM Provider

    Cron->>Edge: POST /api/internal/brief-why-matters
    Edge->>Edge: Auth + validation + country normalize
    Edge->>Redis: GET whymatters-v3 cache
    alt Cache hit
        Redis-->>Edge: cached envelope
        Edge-->>Cron: 200 with cached whyMatters
    else Cache miss, shadow mode on
        par Analyst path
            Edge->>Redis: 6 parallel GETs for context bundles
            Redis-->>Edge: worldBrief, riskScores, forecasts
            Edge->>LLM: Analyst prompt with live context
            LLM-->>Edge: analyst result
        and Gemini fallback path
            Edge->>LLM: Gemini prompt, story fields only
            LLM-->>Edge: gemini result
        end
        Edge->>Redis: SET whymatters-v3 cache
        Edge-)Redis: fire-and-forget shadow record write
        Edge-->>Cron: 200 with whyMatters and shadow comparison
    else Cache miss, shadow mode off
        Edge->>Redis: 6 parallel GETs for context bundles
        Edge->>LLM: Primary path call
        LLM-->>Edge: result
        Edge->>Redis: SET whymatters-v3 cache
        Edge-->>Cron: 200 with whyMatters
    end
    alt Endpoint fails or returns null
        Cron->>Redis: GET whymatters-v2 legacy cache
        alt Legacy cache hit
            Redis-->>Cron: cached string
        else Legacy cache miss
            Cron->>LLM: Direct LLM call
            LLM-->>Cron: result
            Cron->>Redis: SET whymatters-v2 cache
        end
    end
Loading

Reviews (1): Last reviewed commit: "feat(brief): route whyMatters through in..." | Re-trigger Greptile

Comment thread api/internal/brief-why-matters.ts Outdated
Comment on lines +397 to +402
// Intentionally not awaited — must not block response.
redisPipeline([['SET', shadowKey, JSON.stringify(record), 'EX', String(SHADOW_TTL_SEC)]])
.catch(() => {
// Silent — shadow is observability, not critical.
});
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Shadow write may be silently dropped on Vercel Edge

The redisPipeline call is fire-and-forget with .catch(() => {}), but Vercel Edge isolates are terminated as soon as the Response is returned. Any unawaited Promise without ctx.waitUntil may never complete. This will silently drop all shadow records, making the 48h A/B validation procedure described in the PR plan impossible — SCAN brief:llm:whymatters:shadow:v1:* would return zero keys.

The existing repo convention (enforced by the test in tests/edge-functions.test.mjs) is to use ctx.waitUntil. This handler would need to accept a second ctx parameter:

export default async function handler(
  req: Request,
  ctx: { waitUntil: (p: Promise<unknown>) => void },
): Promise<Response> {
  // ...
  if (runShadow) {
    ctx.waitUntil(
      redisPipeline([['SET', shadowKey, JSON.stringify(record), 'EX', String(SHADOW_TTL_SEC)]])
        .catch(() => { /* silent */ }),
    );
  }
  return json(response, 200);
}

Comment on lines +63 to +68
// Keep adding sections until the total budget would overflow.
// +2 accounts for the blank line between sections.
if (used + section.length + 2 > CONTEXT_BUDGET_CHARS) break;
parts.push(section);
used += section.length + 2;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Context budget break skips sections that would still fit

When any section's length would exceed the remaining budget, the loop breaks entirely — all subsequent (lower-priority) sections are dropped even if they are small enough to fit. For example, if worldBrief clips to ~502 chars and countryBrief clips to ~402 chars, they collectively leave ~796 chars of the 1700-char budget — enough for riskScores (250), macroSignals (200), and marketData (200) individually — but the current break stops at whichever section first overshoots.

Consider continue instead of break so smaller remaining sections can still be appended:

Suggested change
// Keep adding sections until the total budget would overflow.
// +2 accounts for the blank line between sections.
if (used + section.length + 2 > CONTEXT_BUDGET_CHARS) break;
parts.push(section);
used += section.length + 2;
}
if (used + section.length + 2 > CONTEXT_BUDGET_CHARS) continue;

Comment on lines +25 to +50
async function timingSafeEqual(a: string, b: string): Promise<boolean> {
const encoder = new TextEncoder();
const aBuf = encoder.encode(a);
const bBuf = encoder.encode(b);
if (aBuf.byteLength !== bBuf.byteLength) return false;
const key = await crypto.subtle.importKey(
'raw',
aBuf,
{ name: 'HMAC', hash: 'SHA-256' },
false,
['sign'],
);
const sig = await crypto.subtle.sign('HMAC', key, bBuf);
const expected = await crypto.subtle.sign('HMAC', key, aBuf);
const sigArr = new Uint8Array(sig);
const expArr = new Uint8Array(expected);
const n = sigArr.length;
if (n !== expArr.length) return false;
let diff = 0;
for (let i = 0; i < n; i++) {
// non-null asserted: bounds checked via the for condition; TS just
// doesn't narrow Uint8Array index access to number under strict mode.
diff |= (sigArr[i] as number) ^ (expArr[i] as number);
}
return diff === 0;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Length short-circuit leaks token length to timing oracle

if (aBuf.byteLength !== bBuf.byteLength) return false returns immediately when auth and "Bearer ${secret}" differ in byte length, revealing the byte-length of the shared secret + 7 ("Bearer " prefix) to any caller who can observe response timing. For an internal-only endpoint this is very low risk, but the timingSafeEqual name implies fully constant-time semantics that the early-exit breaks. A common mitigation is to continue through the HMAC even on length mismatch and mask the final result.

Fixes 5 findings from multi-agent review, 2 of them P1:

- #241 P1: `.gitignore !api/internal/**` was too broad — it re-included
  `.env`, `.env.local`, and any future secret file dropped into that
  directory. Narrowed to explicit source extensions (`*.ts`, `*.js`,
  `*.mjs`) so parent `.env` / secrets rules stay in effect inside
  api/internal/.

- #242 P1: `Dockerfile.digest-notifications` did not COPY
  `shared/brief-llm-core.js` + `.d.ts`. Cron would have crashed at
  container start with ERR_MODULE_NOT_FOUND. Added alongside
  brief-envelope + brief-filter COPY lines.

- #243 P2: Cron dropped the endpoint's source/producedBy ground-truth
  signal, violating PR #3247's own round-3 memory
  (feedback_gate_on_ground_truth_not_configured_state.md). Added
  structured log at the call site: `[brief-llm] whyMatters source=<src>
  producedBy=<pb> hash=<h>`. Endpoint response now includes `hash` so
  log + shadow-record pairs can be cross-referenced.

- #244 P2: Defense-in-depth prompt-injection hardening. Story fields
  flowed verbatim into both LLM prompts, bypassing the repo's
  sanitizeForPrompt convention. Added sanitizeStoryFields helper and
  applied in both analyst and gemini paths.

- #245 P2: Removed redundant `validate` option from callLlmReasoning.
  With only openrouter configured in prod, a parse-reject walked the
  provider chain, then fell through to the other path (same provider),
  then the cron's own fallback (same model) — 3x billing on one reject.
  Post-call parseWhyMatters check already handles rejection cleanly.

Deferred to P3 follow-ups (todos 246-248): singleflight, v2 sunset,
misc polish (country-normalize LOC, JSDoc pruning, shadow waitUntil,
auto-sync mirror, context-assembly caching).

Tests: 6022 pass. typecheck + typecheck:api clean.
…acy fallback

Two P2 findings on PR #3248:

1. Shadow record was fire-and-forget without ctx.waitUntil on an Edge
   function. Vercel can terminate the isolate after response return,
   so the background redisPipeline write completes unreliably — i.e.
   the rollout-validation signal the shadow keys were supposed to
   provide was flaky in production.

   Fix: accept an optional EdgeContext 2nd arg. Build the shadow
   promise up front (so it starts executing immediately) then register
   it with ctx.waitUntil when present. Falls back to plain unawaited
   execution when ctx is absent (local harness / tests).

2. scripts/lib/brief-llm.mjs legacy fallback path called
   buildWhyMattersPrompt(story) on raw fields with no sanitization.
   The analyst endpoint sanitizes before its own prompt build, but
   the fallback is exactly what runs when the endpoint misses /
   errors — so hostile headlines / sources reached the LLM verbatim
   on that path.

   Fix: local sanitizeStoryForPrompt wrapper imports sanitizeForPrompt
   from server/_shared/llm-sanitize.js (existing pattern — see
   scripts/seed-digest-notifications.mjs:41). Wraps story fields
   before buildWhyMattersPrompt. Cache key unchanged (hash is over raw
   story), so cache parity with the analyst endpoint's v3 entries is
   preserved.

Regression guard: new test asserts the fallback prompt strips
"ignore previous instructions", "### Assistant:" line prefixes, and
`<|im_start|>` tokens when injection-crafted fields arrive.

Typecheck + typecheck:api clean. 6023 / 6023 data tests pass.
@koala73

koala73 commented Apr 21, 2026

Copy link
Copy Markdown
Owner Author

Both P2s addressed in 4eee220.

1. Shadow write → ctx.waitUntil

Handler signature now accepts an optional EdgeContext 2nd arg. The shadow redisPipeline promise is built up front (so it starts executing immediately) then registered with ctx.waitUntil(...) when present. Falls back to plain unawaited execution when ctx is absent (local harness / tests). Pattern mirrors api/referral/me.ts:80 and api/health.js:585.

interface EdgeContext {
  waitUntil?: (promise: Promise<unknown>) => void;
}

export default async function handler(req: Request, ctx?: EdgeContext): Promise<Response> {
  
  const shadowWrite = redisPipeline([['SET', shadowKey, ]])
    .then(() => undefined)
    .catch(() => { /* silent */ });
  if (typeof ctx?.waitUntil === 'function') {
    ctx.waitUntil(shadowWrite);
  }
}

2. Legacy fallback sanitization

Added sanitizeStoryForPrompt wrapper in scripts/lib/brief-llm.mjs that imports sanitizeForPrompt from server/_shared/llm-sanitize.js (same cross-dir import precedent as scripts/seed-digest-notifications.mjs:41). Wraps the five story fields before buildWhyMattersPrompt on the fallback path — mirrors what sanitizeStoryFields does in server/worldmonitor/intelligence/v1/brief-why-matters-prompt.ts for the analyst endpoint.

Cache key unchanged (hashBriefStory still hashes raw story material), so the fallback's brief:llm:whymatters:v2:{hash} entries remain interchangeable with the endpoint's v3 entries on hash identity.

Regression guard

New test in tests/brief-llm.test.mjs asserts the fallback prompt strips "ignore previous instructions", "### Assistant:" line prefixes, and <|im_start|> tokens when injection-crafted fields arrive. Inline role words (e.g. "SYSTEM:" mid-sentence) are intentionally preserved — false-positive stripping would mangle legitimate headlines, per the sanitizer's docstring.

Typecheck + typecheck:api clean. 6023 / 6023 data tests pass (was 6022, +1 new guard).

…cations image

Reviewer P1 on PR #3248: my previous commit (4eee220) added
`import sanitizeForPrompt from server/_shared/llm-sanitize.js` to
scripts/lib/brief-llm.mjs, but Dockerfile.digest-notifications cherry-
picks server/_shared/* files and doesn't copy llm-sanitize. Import is
top-level/static — the container would crash at module load with
ERR_MODULE_NOT_FOUND the moment seed-digest-notifications.mjs pulls in
scripts/lib/brief-llm.mjs. Not just on fallback — every startup.

Fix: add `COPY server/_shared/llm-sanitize.js server/_shared/llm-sanitize.d.ts`
next to the existing brief-render COPY line. Module is pure string
manipulation with zero transitive imports — nothing else needs to land.

Cites feedback_validation_docker_ship_full_scripts_dir.md in the comment
next to the COPY; the cherry-pick convention keeps biting when new
cross-dir imports land in scripts/lib/ or scripts/shared/.

Can't regression-test at build time from this branch without a
docker-build CI job, but the symptom is deterministic — local runs
remain green (they resolve against the live filesystem); only the
container crashes. Post-merge, Railway redeploy of seed-digest-
notifications should show a clean `Starting Container` log line
instead of the MODULE_NOT_FOUND crash my prior commit would have caused.
@koala73

koala73 commented Apr 21, 2026

Copy link
Copy Markdown
Owner Author

Reviewer was correct — fixed in 54e4dba.

My previous commit (4eee220) added import sanitizeForPrompt from '../../server/_shared/llm-sanitize.js' to scripts/lib/brief-llm.mjs. That's a top-level static import, so the digest-notifications container would crash at module-load with ERR_MODULE_NOT_FOUND the moment seed-digest-notifications.mjs pulled in scripts/lib/brief-llm.mjsevery startup, not just fallback paths.

Fix: added COPY server/_shared/llm-sanitize.js server/_shared/llm-sanitize.d.ts ./server/_shared/ next to the existing brief-render.js COPY at line 60. The module is pure string manipulation with zero transitive imports (verified via grep — no import / require statements in the file), so nothing else needs to land.

Cited feedback_validation_docker_ship_full_scripts_dir.md in the Dockerfile comment for future maintainers — the cherry-pick pattern keeps biting when cross-dir imports land in scripts/lib/ or scripts/shared/. Might be worth switching to COPY scripts/ ./scripts/ + COPY server/_shared/ ./server/_shared/ wholesale in a follow-up, but that's out of scope here.

Typecheck + typecheck:api clean. No test-suite impact (tests run against the live filesystem, not the container image — the symptom is deterministic container-startup failure only).

@koala73
koala73 merged commit 2f19d96 into main Apr 21, 2026
10 checks passed
@koala73
koala73 deleted the feat/brief-why-matters-analyst-endpoint branch April 21, 2026 10:03
koala73 added a commit that referenced this pull request Apr 21, 2026
…GEST_ONLY_USER filter (#3255)

* fix(brief): unblock whyMatters analyst endpoint + add DIGEST_ONLY_USER filter

Three changes, all operational for PR #3248's brief-why-matters feature.

1. middleware.ts PUBLIC_API_PATHS allowlist
Railway logs post-#3248 merge showed every cron call to
/api/internal/brief-why-matters returning 403 — middleware's "short
UA" guard (~L183) rejects Node undici's default UA before the
endpoint's own Bearer-auth runs. The feature never executed in prod;
three-layer fallback silently shipped legacy Gemini output. Same
class as /api/seed-contract-probe (2026-04-15). Endpoint still
carries its own subtle-crypto HMAC auth, so bypassing the UA gate
is safe.

2. Explicit UA on callAnalystWhyMatters fetch
Defense-in-depth. Explicit 'worldmonitor-digest-notifications/1.0'
keeps the endpoint reachable if PUBLIC_API_PATHS is ever refactored,
and makes cron traffic distinguishable from ops curl in logs.

3. DIGEST_ONLY_USER=user_xxx filter
Operator single-user test flag. Set on Railway to run compose + send
for one user on the next tick (then unset) — validates new features
end-to-end without fanning out. Empty/unset = normal fan-out. Applied
right after rule fetch so both compose and dispatch paths respect it.

Regression tests: 15 new cases in tests/middleware-bot-gate.test.mts
pin every PUBLIC_API_PATHS entry against 3 triggers (empty/short/curl
UA) plus a negative sibling-path suite so a future prefix-match
refactor can't silently unblock /api/internal/.

Tests: 6043 pass. typecheck + typecheck:api clean. biome: pre-existing
main() complexity warning bumped 74→78 by the filter block (unchanged
in character from pre-PR).

* test(middleware): expand sibling-path negatives to cover all 3 trigger UAs

Greptile flagged: `SIBLING_PATHS` was only tested with `EMPTY_UA`. Under
the current middleware chain this is sufficient (sibling paths hit the
short-UA OR BOT_UA 403 regardless), but it doesn't pin *which* guard
fires. A future refactor that moves `PUBLIC_API_PATHS.has(path)` later
in the chain could let a curl or undici UA pass on a sibling path
without this suite failing.

Fix: iterate the 3 sibling paths against all 3 trigger UAs (empty,
short/undici, curl). Every combination must still 403 regardless of
which guard catches it. 6 new test cases.

Tests: 35 pass in the middleware-bot-gate suite (was 29).
SebastienMelki added a commit that referenced this pull request Apr 21, 2026
…ooks} → ShippingV2Service (#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 #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 #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]>
SebastienMelki added a commit that referenced this pull request Apr 22, 2026
… (#3242)

* chore(api): enforce sebuf contract via exceptions manifest (#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 (#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 (#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 (#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 (#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 (#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 (#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 (#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 (#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 (#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 (#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 (#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 #3252. That requires coordinated Tauri build + Vercel env
changes out of scope for #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 (#3207)

Closes out the remaining @koala73 review findings from #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 #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 #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 (#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 (#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 (#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 (#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 (#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 #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 #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 (#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 #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 (#3242 second-pass, HIGH new #1):
> Exact class PR #3233 fixed for RegionalIntelligenceBoard /
> DeductionPanel / trade / country-intel. Supply-chain was not in
> #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 (#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 (#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 (#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 (#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 (#3207)

Koala flagged this as a merge blocker in PR #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 (#3207)

Koala flagged this as a merge blocker in PR #3242 review.

Commits 6 + 7 of #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=#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]>
SebastienMelki added a commit that referenced this pull request Apr 23, 2026
…nal-helper justification

Carried in from #3248 merge as a band-aid (called out in #3242 review followup
checklist item 7). The endpoint genuinely belongs in internal-helper —
RELAY_SHARED_SECRET-bearer auth, cron-only caller, never reached by dashboards
or partners. Same shape constraint as api/notify.ts.

Replaces the apologetic "filed here to keep the lint green" framing with a
proper structural justification: modeling it as a generated service would
publish internal cron plumbing as user-facing API surface.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
SebastienMelki added a commit that referenced this pull request Apr 23, 2026
…nal-helper justification

Carried in from #3248 merge as a band-aid (called out in #3242 review followup
checklist item 7). The endpoint genuinely belongs in internal-helper —
RELAY_SHARED_SECRET-bearer auth, cron-only caller, never reached by dashboards
or partners. Same shape constraint as api/notify.ts.

Replaces the apologetic "filed here to keep the lint green" framing with a
proper structural justification: modeling it as a generated service would
publish internal cron plumbing as user-facing API surface.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
SebastienMelki added a commit that referenced this pull request Apr 24, 2026
* chore(api-manifest): rewrite brief-why-matters reason as proper internal-helper justification

Carried in from #3248 merge as a band-aid (called out in #3242 review followup
checklist item 7). The endpoint genuinely belongs in internal-helper —
RELAY_SHARED_SECRET-bearer auth, cron-only caller, never reached by dashboards
or partners. Same shape constraint as api/notify.ts.

Replaces the apologetic "filed here to keep the lint green" framing with a
proper structural justification: modeling it as a generated service would
publish internal cron plumbing as user-facing API surface.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* feat(lint): premium-fetch parity check for ServiceClients (closes #3279)

Adds scripts/enforce-premium-fetch.mjs — AST-walks src/, finds every
`new <ServiceClient>(...)` (variable decl OR `this.foo =` assignment),
tracks which methods each instance actually calls, and fails if any
called method targets a path in src/shared/premium-paths.ts
PREMIUM_RPC_PATHS without `{ fetch: premiumFetch }` on the constructor.

Per-call-site analysis (not class-level) keeps the trade/index.ts pattern
clean — publicClient with globalThis.fetch + premiumClient with
premiumFetch on the same TradeServiceClient class — since publicClient
never calls a premium method.

Wired into:
- npm run lint:premium-fetch
- .husky/pre-push (right after lint:rate-limit-policies)
- .github/workflows/lint-code.yml (right after lint:api-contract)

Found and fixed three latent instances of the HIGH(new) #1 class from
#3242 review (silent 401 → empty fallback for signed-in browser pros):

- src/services/correlation-engine/engine.ts — IntelligenceServiceClient
  built with no fetch option called deductSituation. LLM-assessment overlay
  on convergence cards never landed for browser pros without a WM key.
- src/services/economic/index.ts — EconomicServiceClient with
  globalThis.fetch called getNationalDebt. National-debt panel rendered
  empty for browser pros.
- src/services/sanctions-pressure.ts — SanctionsServiceClient with
  globalThis.fetch called listSanctionsPressure. Sanctions-pressure panel
  rendered empty for browser pros.

All three swap to premiumFetch (single shared client, mirrors the
supply-chain/index.ts justification — premiumFetch no-ops safely on
public methods, so the public methods on those clients keep working).

Verification:
- lint:premium-fetch clean (34 ServiceClient classes, 28 premium paths,
  466 src/ files analyzed)
- Negative test: revert any of the three to globalThis.fetch → exit 1
  with file:line and called-premium-method names
- typecheck + typecheck:api clean
- lint:api-contract / lint:rate-limit-policies / lint:boundaries clean
- tests/sanctions-pressure.test.mjs + premium-fetch.test.mts: 16/16 pass

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix(military): fetchStaleFallback NEG_TTL=30s parity (closes #3277)

The legacy /api/military-flights handler had NEG_TTL = 30_000ms — a short
suppression window after a failed live + stale read so we don't Redis-hammer
the stale key during sustained relay+seed outages.

Carried into the sebuf list-military-flights handler:
- Module-scoped `staleNegUntil` timestamp (per-isolate on Vercel Edge,
  which is fine — each warm isolate gets its own 30s suppression window).
- Set whenever fetchStaleFallback returns null (key missing, parse fail,
  empty array after staleToProto filter, or thrown error).
- Checked at the entry of fetchStaleFallback before doing the Redis read.
- Test seam `_resetStaleNegativeCacheForTests()` exposed for unit tests.

Test pinned in tests/redis-caching.test.mjs: drives a stale-empty cycle
three times — first read hits Redis, second within window doesn't, after
test-only reset it does again.

Verified: 18/18 redis-caching tests pass, typecheck:api clean,
lint:premium-fetch clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* refactor(lint): rate-limit-policies regex → import() (closes #3278)

The previous lint regex-parsed ENDPOINT_RATE_POLICIES from the source
file. That worked because the literal happens to fit a single line per
key today, but a future reformat (multi-line key wrap, formatter swap,
etc.) would silently break the lint without breaking the build —
exactly the failure mode that's worse than no lint at all.

Fix:
- Export ENDPOINT_RATE_POLICIES from server/_shared/rate-limit.ts.
- Convert scripts/enforce-rate-limit-policies.mjs to async + dynamic
  import() of the policy object directly. Same TS module that the
  gateway uses at runtime → no source-of-truth drift possible.
- Run via tsx (already a dev dep, used by test:data) so the .mjs
  shebang can resolve a .ts import.
- npm script swapped to `tsx scripts/...`. .husky/pre-push uses
  `npm run lint:rate-limit-policies` so no hook change needed.

Verified:
- Clean: 6 policies / 182 gateway routes.
- Negative test (rename a key to the original sanctions typo
  /api/sanctions/v1/lookup-entity): exit 1 with the same incident-
  attributed remedy message as before.
- Reformat test (split a single-line entry across multiple lines):
  still passes — the property is what's read, not the source layout.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix(shipping/v2): alertThreshold: 0 preserved; drop dead validation branch (#3242 followup)

Before: alert_threshold was a plain int32. proto3 scalar default is 0, so
the handler couldn't distinguish "partner explicitly sent 0 (deliver every
disruption)" from "partner omitted the field (apply legacy default 50)" —
both arrived as 0 and got coerced to 50 by `> 0 ? : 50`. Silent intent-drop
for any partner who wanted every alert. The subsequent `alertThreshold < 0`
branch was also unreachable after that coercion.

After:
- Proto field is `optional int32 alert_threshold` — TS type becomes
  `alertThreshold?: number`, so omitted = undefined and explicit 0 stays 0.
- Handler uses `req.alertThreshold ?? 50` — undefined → 50, any number
  passes through unchanged.
- Dead `< 0 || > 100` runtime check removed; buf.validate `int32.gte = 0,
  int32.lte = 100` already enforces the range at the wire layer.

Partner wire contract: identical for the omit-field and 1..100 cases.
Only behavioural change is explicit 0 — previously impossible to request,
now honored per proto3 optional semantics.

Scoped `buf generate --path worldmonitor/shipping/v2` to avoid the full-
regen `@ts-nocheck` drift Seb documented in the #3242 PR comments.
Re-applied `@ts-nocheck` on the two regenerated files manually.

Tests:
- `alertThreshold 0 coerces to 50` flipped to `alertThreshold 0 preserved`.
- New test: `alertThreshold omitted (undefined) applies legacy default 50`.
- `rejects > 100` test removed — proto/wire validation handles it; direct
  handler calls intentionally bypass wire and the handler no longer carries
  a redundant runtime range check.

Verified: 18/18 shipping-v2-handler tests pass, typecheck + typecheck:api
clean, all 4 custom lints clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* docs(shipping/v2): document missing webhook delivery worker + DNS-rebinding contract (#3242 followup)

#3242 followup checklist item 6 from @koala73 — sanity-check that the
delivery worker honors the re-resolve-and-re-check contract that
isBlockedCallbackUrl explicitly delegates to it.

Audit finding: no delivery worker for shipping/v2 webhooks exists in this
repo. Grep across the entire tree (excluding generated/dist) shows the
only readers of webhook:sub:* records are the registration / inspection /
rotate-secret handlers themselves. No code reads them and POSTs to the
stored callbackUrl. The delivery worker is presumed to live in Railway
(separate repo) or hasn't been built yet — neither is auditable from
this repo.

Refreshes the comment block at the top of webhook-shared.ts to:
- explicitly state DNS rebinding is NOT mitigated at registration
- spell out the four-step contract the delivery worker MUST follow
  (re-validate URL, dns.lookup, re-check resolved IP against patterns,
   fetch with resolved IP + Host header preserved)
- flag the in-repo gap so anyone landing delivery code can't miss it

Tracking the gap as #3288 — acceptance there is "delivery worker imports
the patterns + helpers from webhook-shared.ts and applies the four steps
before each send." Action moves to wherever the delivery worker actually
lives (Railway likely).

No code change. Tests + lints unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* ci(lint): add rate-limit-policies step (greptile P1 #3287)

Pre-push hook ran lint:rate-limit-policies but the CI workflow did not,
so fork PRs and --no-verify pushes bypassed the exact drift check the
lint was added to enforce (closes #3278). Adding it right after
lint:api-contract so it runs in the same context the lint was designed
for.

* refactor(lint): premium-fetch regex → import() + loop classRe (greptile P2 #3287)

Two fragilities greptile flagged on enforce-premium-fetch.mjs:

1. loadPremiumPaths regex-parsed src/shared/premium-paths.ts with
   /'(\/api\/[^']+)'/g — same class of silent drift we just removed
   from enforce-rate-limit-policies in #3278. Reformatting the source
   Set (double quotes, spread, helper-computed entries) would drop
   paths from the lint while leaving the runtime untouched. Fix: flip
   the shebang to `#!/usr/bin/env -S npx tsx` and dynamic-import
   PREMIUM_RPC_PATHS directly, mirroring the rate-limit pattern.
   package.json lint:premium-fetch now invokes via tsx too so the
   npm-script path matches direct execution.

2. loadClientClassMap ran classRe.exec once, silently dropping every
   ServiceClient after the first if a file ever contained more than
   one. Current codegen emits one class per file so this was latent,
   but a template change would ship un-linted classes. Fix: collect
   every class-open match with matchAll, slice each class body with
   the next class's start as the boundary, and scan methods per-body
   so method-to-class binding stays correct even with multiple
   classes per file.

Verification:
- lint:premium-fetch clean (34 classes / 28 premium paths / 466 files
  — identical counts to pre-refactor, so no coverage regression).
- Negative test: revert src/services/economic/index.ts to
  globalThis.fetch → exit 1 with file:line, bound var name, and
  premium method list (getNationalDebt). Restore → clean.
- lint:rate-limit-policies still clean.

* fix(shipping/v2): re-add alertThreshold handler range guard (greptile nit 1 #3287)

Wire-layer buf.validate enforces 0..100, but direct handler invocation
(internal jobs, test harnesses, future transports) bypasses it. Cheap
invariant-at-the-boundary — rejects < 0 or > 100 with ValidationError
before the record is stored.

Tests: restored the rejects-out-of-range cases that were dropped when the
branch was (correctly) deleted as dead code on the previous commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* refactor(lint): premium-fetch method-regex → TS AST (greptile nits 2+5 #3287)

loadClientClassMap:
  The method regex `async (\w+)\s*\([^)]*\)\s*:\s*Promise<[^>]+>\s*\{\s*let
  path = "..."` assumed (a) no nested `)` in arg types, (b) no nested `>`
  in the return type, (c) `let path = "..."` as the literal first statement.
  Any codegen template shift would silently drop methods with the lint still
  passing clean — the same silent-drift class #3287 just closed on the
  premium-paths side.

  Now walks the service_client.ts AST, matches `export class *ServiceClient`,
  iterates `MethodDeclaration` members, and reads the first
  `let path: string = '...'` variable statement as a StringLiteral. Tolerant
  to any reformatting of arg/return types or method shape.

findCalls scope-blindness:
  Added limitation comment — the walker matches `<varName>.<method>()`
  anywhere in the file without respecting scope. Two constructions in
  different function scopes sharing a var name merge their called-method
  sets. No current src/ file hits this; the lint errs cautiously (flags
  both instances). Keeping the walker simple until scope-aware binding
  is needed.

webhook-shared.ts:
  Inlined issue reference (#3288) so the breadcrumb resolves without
  bouncing through an MDX that isn't in the diff.

Verification:
- lint:premium-fetch clean — 34 classes / 28 premium paths / 489 files.
  Pre-refactor: 34 / 28 / 466. Class + path counts identical; file bump
  is from the main-branch rebase, not the refactor.
- Negative test: revert src/services/economic/index.ts premiumFetch →
  globalThis.fetch. Lint exits 1 at `src/services/economic/index.ts:64:7`
  with `premium method(s) called: getNationalDebt`. Restore → clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* refactor(lint): rate-limit OpenAPI regex → yaml parser (greptile nit 3 #3287)

Input side (ENDPOINT_RATE_POLICIES) was flipped to live `import()` in
4e79d02. Output side (OpenAPI routes) still regex-scraped top-level
`paths:` keys with `/^\s{4}(\/api\/[^\s:]+):/gm` — hard-coded 4-space
indent. Any YAML formatter change (2-space indent, flow style, line
folding) would silently drop routes and let policy-drift slip through
— same silent-drift class the input-side fix closed.

Now uses the `yaml` package (already a dep) to parse each
.openapi.yaml and reads `doc.paths` directly.

Verification:
- Clean: 6 policies / 189 routes (was 182 — yaml parser picks up a
  handful the regex missed, closing a silent coverage gap).
- Negative test: rename policy key back to /api/sanctions/v1/lookup-entity
  → exits 1 with the same incident-attributed remedy. Restore → clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* chore(codegen): regenerate unified OpenAPI bundle for alert_threshold proto change

The shipping/v2 webhook alert_threshold field was flipped from `int32` to
`optional int32` with an expanded doc comment in f333946. That comment
now surfaces in the unified docs/api/worldmonitor.openapi.yaml bundle
(introduced by #3341). Regenerated with sebuf v0.11.1 to pick it up.

No behaviour change — bundle-only documentation drift.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant