diff --git a/src/app/panel-layout.ts b/src/app/panel-layout.ts index 59c698fac..80fb129ff 100644 --- a/src/app/panel-layout.ts +++ b/src/app/panel-layout.ts @@ -780,7 +780,16 @@ export class PanelLayoutManager implements AppModule { this.createPanel('trade-policy', () => new TradePolicyPanel()); this.createPanel('sanctions-pressure', () => new SanctionsPressurePanel()); - this.createPanel('supply-chain', () => new SupplyChainPanel()); + const supplyChainPanel = this.createPanel('supply-chain', () => new SupplyChainPanel()); + if (supplyChainPanel) { + supplyChainPanel.setOnScenarioActivate((id, result) => { + this.ctx.map?.activateScenario(id, result); + }); + supplyChainPanel.setOnDismissScenario(() => { + this.ctx.map?.deactivateScenario(); + }); + this.ctx.map?.setSupplyChainPanel(supplyChainPanel); + } this.createNewsPanel('africa', 'panels.africa'); this.createNewsPanel('latam', 'panels.latam'); diff --git a/src/components/DeckGLMap.ts b/src/components/DeckGLMap.ts index 10eb3b2f4..a86ec4f3f 100644 --- a/src/components/DeckGLMap.ts +++ b/src/components/DeckGLMap.ts @@ -401,6 +401,7 @@ export class DeckGLMap { private tradeRouteSegments: TradeRouteSegment[] = resolveTradeRouteSegments(); private storedChokepointData: GetChokepointStatusResponse | null = null; private scenarioState: ScenarioVisualState | null = null; + private affectedIso2Set: Set = new Set(); private positiveEvents: PositiveGeoEvent[] = []; private kindnessPoints: KindnessPoint[] = []; private imageryScenes: ImageryScene[] = []; @@ -1744,6 +1745,9 @@ export class DeckGLMap { const sanctionsLayer = this.createSanctionsChoroplethLayer(); if (sanctionsLayer) layers.push(sanctionsLayer); } + // Scenario heat layer (affected countries tint) + const scenarioHeat = this.scenarioState ? this.createScenarioHeatLayer() : null; + if (scenarioHeat) layers.push(scenarioHeat); // Phase 8: Species recovery zones if (mapLayers.speciesRecovery && this.speciesRecoveryZones.length > 0) { layers.push(this.createSpeciesRecoveryLayer()); @@ -3526,6 +3530,23 @@ export class DeckGLMap { }); } + private createScenarioHeatLayer(): GeoJsonLayer | null { + if (!this.affectedIso2Set.size || !this.countriesGeoJsonData) return null; + return new GeoJsonLayer({ + id: 'scenario-heat-layer', + data: this.countriesGeoJsonData, + stroked: false, + filled: true, + extruded: false, + pickable: false, + getFillColor: (feature: { properties?: Record }) => { + const code = feature.properties?.['ISO3166-1-Alpha-2'] as string | undefined; + return (code && this.affectedIso2Set.has(code) ? [220, 60, 40, 80] : [0, 0, 0, 0]) as [number, number, number, number]; + }, + updateTriggers: { getFillColor: [this.scenarioState?.scenarioId ?? null] }, + }); + } + private createSpeciesRecoveryLayer(): ScatterplotLayer { return new ScatterplotLayer({ id: 'species-recovery-layer', @@ -5434,6 +5455,7 @@ export class DeckGLMap { */ public setScenarioState(state: ScenarioVisualState | null): void { this.scenarioState = state; + this.affectedIso2Set = new Set(state?.affectedIso2s ?? []); this.render(); } diff --git a/src/components/GlobeMap.ts b/src/components/GlobeMap.ts index da5dede32..a99437a3a 100644 --- a/src/components/GlobeMap.ts +++ b/src/components/GlobeMap.ts @@ -51,6 +51,7 @@ import { pinWebcam, isPinned } from '@/services/webcams/pinned-store'; import type { WebcamEntry, WebcamCluster } from '@/generated/client/worldmonitor/webcam/v1/service_client'; import type { TrafficAnomaly as ProtoTrafficAnomaly, DdosLocationHit } from '@/generated/client/worldmonitor/infrastructure/v1/service_client'; import type { RadiationObservation } from '@/services/radiation'; +import type { ScenarioVisualState } from '@/config/scenario-templates'; const SAT_COUNTRY_COLORS: Record = { CN: '#ff2020', RU: '#ff8800', US: '#4488ff', EU: '#44cc44', KR: '#aa66ff', IN: '#ff66aa', TR: '#ff4466', OTHER: '#ccccff' }; const SAT_TYPE_EMOJI: Record = { sar: '\u{1F4E1}', optical: '\u{1F4F7}', military: '\u{1F396}', sigint: '\u{1F4FB}' }; @@ -396,7 +397,7 @@ interface GlobePath { interface GlobePolygon { coords: number[][][]; name: string; - _kind: 'cii' | 'conflict' | 'imageryFootprint' | 'forecastCone'; + _kind: 'cii' | 'conflict' | 'imageryFootprint' | 'forecastCone' | 'scenario'; level?: string; score?: number; @@ -526,6 +527,7 @@ export class GlobeMap { private cableDegradedIds = new Set(); private ciiScoresMap: Map = new Map(); private countriesGeoData: FeatureCollection | null = null; + private scenarioPolygons: GlobePolygon[] = []; // Current layers state private layers: MapLayers; @@ -828,6 +830,7 @@ export class GlobeMap { if (d._kind === 'conflict') return GlobeMap.CONFLICT_CAP[d.intensity!] ?? GlobeMap.CONFLICT_CAP.low; if (d._kind === 'imageryFootprint') return 'rgba(0,0,0,0)'; if (d._kind === 'forecastCone') return 'rgba(255,140,60,0.2)'; + if (d._kind === 'scenario') return 'rgba(220,60,40,0.3)'; return 'rgba(255,60,60,0.15)'; }) .polygonSideColor((d: GlobePolygon) => { @@ -835,6 +838,7 @@ export class GlobeMap { if (d._kind === 'conflict') return GlobeMap.CONFLICT_SIDE[d.intensity!] ?? GlobeMap.CONFLICT_SIDE.low; if (d._kind === 'imageryFootprint') return 'rgba(0,0,0,0)'; if (d._kind === 'forecastCone') return 'rgba(255,140,60,0.1)'; + if (d._kind === 'scenario') return 'rgba(0,0,0,0)'; return 'rgba(255,60,60,0.08)'; }) .polygonStrokeColor((d: GlobePolygon) => { @@ -842,6 +846,7 @@ export class GlobeMap { if (d._kind === 'conflict') return GlobeMap.CONFLICT_STROKE[d.intensity!] ?? GlobeMap.CONFLICT_STROKE.low; if (d._kind === 'imageryFootprint') return '#00b4ff'; if (d._kind === 'forecastCone') return 'rgba(255,140,60,0.5)'; + if (d._kind === 'scenario') return 'transparent'; return '#ff4444'; }) .polygonAltitude((d: GlobePolygon) => { @@ -2088,11 +2093,33 @@ export class GlobeMap { polys.push(...this.stormConePolygons); } + if (this.scenarioPolygons.length) { + polys.push(...this.scenarioPolygons); + } + (this.globe as any).polygonsData(polys); } // ─── Public data setters ────────────────────────────────────────────────── + public setScenarioState(state: ScenarioVisualState | null): void { + this.scenarioPolygons = []; + if (state?.affectedIso2s?.length && this.countriesGeoData) { + const affected = new Set(state.affectedIso2s); + for (const feat of this.countriesGeoData.features) { + const code = feat.properties?.['ISO3166-1-Alpha-2'] as string | undefined; + if (!code || !affected.has(code)) continue; + const geom = feat.geometry; + if (!geom) continue; + const rings = geom.type === 'Polygon' ? [geom.coordinates] : geom.type === 'MultiPolygon' ? geom.coordinates : []; + for (const ring of rings) { + this.scenarioPolygons.push({ coords: ring as number[][][], name: code, _kind: 'scenario' }); + } + } + } + this.flushPolygons(); + } + public setCIIScores(scores: Array<{ code: string; score: number; level: string }>): void { this.ciiScoresMap = new Map(scores.map(s => [s.code, { score: s.score, level: s.level }])); this.flushPolygons(); diff --git a/src/components/Map.ts b/src/components/Map.ts index e0677a437..f8d71a9c0 100644 --- a/src/components/Map.ts +++ b/src/components/Map.ts @@ -61,6 +61,7 @@ import { getAlertsNearLocation } from '@/services/geo-convergence'; import { getCountryAtCoordinates, getCountryBbox } from '@/services/country-geometry'; import type { CountryClickPayload } from './DeckGLMap'; import { t } from '@/services/i18n'; +import type { ScenarioVisualState } from '@/config/scenario-templates'; export type TimeRange = '1h' | '6h' | '24h' | '48h' | '7d' | 'all'; export type MapView = 'global' | 'america' | 'mena' | 'eu' | 'asia' | 'latam' | 'africa' | 'oceania'; @@ -3407,6 +3408,10 @@ export class MapComponent { this.popup.setChokepointData(data); } + public setScenarioState(_state: ScenarioVisualState | null): void { + // SVG renderer: scenario fill deferred (no iso2 data binding on country elements) + } + public setLayerLoading(layer: keyof MapLayers, loading: boolean): void { const btn = this.container.querySelector(`.layer-toggle[data-layer="${layer}"]`); if (btn) { diff --git a/src/components/MapContainer.ts b/src/components/MapContainer.ts index 8596604af..62e7760b2 100644 --- a/src/components/MapContainer.ts +++ b/src/components/MapContainer.ts @@ -49,6 +49,9 @@ import type { TrafficAnomaly as ProtoTrafficAnomaly, DdosLocationHit } from '@/g import type { DiseaseOutbreakItem } from '@/services/disease-outbreaks'; import type { GetChokepointStatusResponse } from '@/services/supply-chain'; import type { ScenarioVisualState, ScenarioResult } from '@/config/scenario-templates'; +import { getAuthState } from '@/services/auth-state'; +import { hasPremiumAccess } from '@/services/panel-gating'; +import { trackGateHit } from '@/services/analytics'; export type { ScenarioVisualState, ScenarioResult }; @@ -90,6 +93,7 @@ export class MapContainer { private deckGLMap: DeckGLMap | null = null; private svgMap: MapComponent | null = null; private globeMap: GlobeMap | null = null; + private supplyChainPanel: import('@/components/SupplyChainPanel').SupplyChainPanel | null = null; private initialState: MapContainerState; private useDeckGL: boolean; private useGlobe: boolean; @@ -970,22 +974,31 @@ export class MapContainer { // ─── Scenario Engine ───────────────────────────────────────────────────────── + public setSupplyChainPanel(panel: import('@/components/SupplyChainPanel').SupplyChainPanel): void { + this.supplyChainPanel = panel; + } + /** * Activate a scenario across all active renderers. - * PRO-gated — free users trigger `trackGateHit('scenario')` only. + * PRO-gated — free users trigger `trackGateHit('scenario-engine')` only. * * @param scenarioId Template ID from scenario-templates.ts * @param result Computed result from the scenario worker */ public activateScenario(scenarioId: string, result: ScenarioResult): void { + if (!hasPremiumAccess(getAuthState())) { + trackGateHit('scenario-engine'); + return; + } const state: ScenarioVisualState = { scenarioId, disruptedChokepointIds: result.affectedChokepointIds, affectedIso2s: result.topImpactCountries.map((c: { iso2: string }) => c.iso2), }; - // DeckGL is the primary renderer for scenario visuals. - // Globe and SVG support is deferred to Sprint D. this.deckGLMap?.setScenarioState(state); + this.svgMap?.setScenarioState(state); + this.globeMap?.setScenarioState(state); + this.supplyChainPanel?.showScenarioSummary(scenarioId, result); } /** @@ -993,6 +1006,9 @@ export class MapContainer { */ public deactivateScenario(): void { this.deckGLMap?.setScenarioState(null); + this.svgMap?.setScenarioState(null); + this.globeMap?.setScenarioState(null); + this.supplyChainPanel?.hideScenarioSummary(); } // Utility methods diff --git a/src/components/SupplyChainPanel.ts b/src/components/SupplyChainPanel.ts index 0dcf0f7a7..361c435e6 100644 --- a/src/components/SupplyChainPanel.ts +++ b/src/components/SupplyChainPanel.ts @@ -6,6 +6,8 @@ import type { GetShippingStressResponse, } from '@/services/supply-chain'; import { fetchBypassOptions } from '@/services/supply-chain'; +import type { ScenarioResult } from '@/config/scenario-templates'; +import { SCENARIO_TEMPLATES } from '@/config/scenario-templates'; import { TransitChart } from '@/utils/transit-chart'; import { t } from '@/services/i18n'; import { escapeHtml } from '@/utils/sanitize'; @@ -14,6 +16,7 @@ import { isDesktopRuntime } from '@/services/runtime'; import { getAuthState, subscribeAuthState } from '@/services/auth-state'; import { hasPremiumAccess } from '@/services/panel-gating'; import { trackGateHit } from '@/services/analytics'; +import { premiumFetch } from '@/services/premium-fetch'; type TabId = 'chokepoints' | 'shipping' | 'indicators' | 'minerals' | 'stress'; @@ -31,6 +34,10 @@ export class SupplyChainPanel extends Panel { private chartMountTimer: ReturnType | null = null; private bypassUnsubscribe: (() => void) | null = null; private bypassGateTracked = false; + private onDismissScenario: (() => void) | null = null; + private onScenarioActivate: ((scenarioId: string, result: ScenarioResult) => void) | null = null; + private activeScenarioState: { scenarioId: string; result: ScenarioResult } | null = null; + private scenarioPollController: AbortController | null = null; constructor() { super({ id: 'supply-chain', title: t('panels.supplyChain'), defaultRowSpan: 2, infoTooltip: t('components.supplyChain.infoTooltip') }); @@ -181,6 +188,16 @@ export class SupplyChainPanel extends Panel { this.chartMountTimer = null; }, 220); } + + // Re-insert scenario banner after setContent replaces inner content. + if (this.activeScenarioState) { + this.showScenarioSummary(this.activeScenarioState.scenarioId, this.activeScenarioState.result); + } + + // Attach scenario trigger buttons for expanded chokepoint cards. + if (this.activeTab === 'chokepoints' && this.expandedChokepoint) { + this.attachScenarioTriggers(); + } } private renderBypassSection(container: HTMLElement, chokepointId: string): void { @@ -305,6 +322,20 @@ export class SupplyChainPanel extends Panel { ? `
Loading bypass options\u2026
` : ''; + const scenarioSection = expanded ? (() => { + const template = SCENARIO_TEMPLATES.find(tmpl => + tmpl.affectedChokepointIds.includes(cp.id) && tmpl.type !== 'tariff_shock' + ); + if (!template) return ''; + const isPro = hasPremiumAccess(getAuthState()); + const btnClass = isPro ? 'sc-scenario-btn' : 'sc-scenario-btn sc-scenario-btn--gated'; + return `
+ +
`; + })() : ''; + return `
${escapeHtml(cp.name)} @@ -345,6 +376,7 @@ export class SupplyChainPanel extends Panel { ${actionRow} ${chartPlaceholder} ${bypassSection} + ${scenarioSection}
`; }).join('')} @@ -591,4 +623,91 @@ export class SupplyChainPanel extends Panel { `; } + + // ─── Scenario banner ───────────────────────────────────────────────────────── + + public showScenarioSummary(scenarioId: string, result: ScenarioResult): void { + this.activeScenarioState = { scenarioId, result }; + this.content.querySelector('.sc-scenario-banner')?.remove(); + const top5 = result.topImpactCountries.slice(0, 5); + const countriesHtml = top5.map(c => + `${escapeHtml(c.iso2)} ${(c.impactPct * 100).toFixed(0)}%` + ).join(' \u00B7 '); + const banner = document.createElement('div'); + banner.className = 'sc-scenario-banner'; + const scenarioName = SCENARIO_TEMPLATES.find(tmpl => tmpl.id === scenarioId)?.name ?? scenarioId.replace(/-/g, ' '); + banner.innerHTML = `\u26A0${escapeHtml(scenarioName)}${countriesHtml}`; + banner.querySelector('.sc-scenario-dismiss')!.addEventListener('click', () => this.onDismissScenario?.()); + this.content.prepend(banner); + } + + public hideScenarioSummary(): void { + this.activeScenarioState = null; + this.content.querySelector('.sc-scenario-banner')?.remove(); + this.content.querySelectorAll('.sc-scenario-btn').forEach(btn => { + btn.disabled = false; + btn.textContent = 'Simulate Closure'; + }); + } + + public setOnDismissScenario(cb: () => void): void { + this.onDismissScenario = cb; + } + + public setOnScenarioActivate(cb: (scenarioId: string, result: ScenarioResult) => void): void { + this.onScenarioActivate = cb; + } + + private attachScenarioTriggers(): void { + this.content.querySelectorAll('.sc-scenario-trigger').forEach(el => { + el.querySelector('.sc-scenario-btn')?.addEventListener('click', async (e) => { + e.stopPropagation(); + const btn = el.querySelector('.sc-scenario-btn')!; + if (btn.dataset.gated === '1') { + trackGateHit('scenario-engine'); + return; + } + this.scenarioPollController?.abort(); + this.scenarioPollController = new AbortController(); + const { signal } = this.scenarioPollController; + + const scenarioId = el.dataset.scenarioId!; + btn.disabled = true; + btn.textContent = 'Computing\u2026'; + try { + const runResp = await premiumFetch('/api/scenario/v1/run', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ scenarioId }), + signal, + }); + if (!runResp.ok) throw new Error('Run failed'); + const { jobId } = await runResp.json() as { jobId: string }; + let result: ScenarioResult | null = null; + for (let i = 0; i < 30; i++) { + if (signal.aborted || !this.content.isConnected) return; + if (i > 0) await new Promise(r => setTimeout(r, 2000)); + const statusResp = await premiumFetch(`/api/scenario/v1/status?jobId=${encodeURIComponent(jobId)}`, { signal }); + if (!statusResp.ok) throw new Error(`Status poll failed: ${statusResp.status}`); + const status = await statusResp.json() as { status: string; result?: ScenarioResult }; + if (status.status === 'done') { + const r = status.result; + if (!r || !Array.isArray(r.topImpactCountries)) throw new Error('done without valid result'); + result = r; + break; + } + if (status.status === 'failed') throw new Error('Scenario failed'); + } + if (!result) throw new Error('Timeout'); + if (signal.aborted || !this.content.isConnected) return; + this.onScenarioActivate?.(scenarioId, result); + btn.textContent = 'Active'; + } catch (err) { + if (err instanceof Error && err.name === 'AbortError') return; + btn.textContent = 'Error \u2014 retry'; + btn.disabled = false; + } + }); + }); + } } diff --git a/src/config/scenario-templates.ts b/src/config/scenario-templates.ts index 48fc74ad4..6c54855a8 100644 --- a/src/config/scenario-templates.ts +++ b/src/config/scenario-templates.ts @@ -10,3 +10,5 @@ export type { ScenarioVisualState, ScenarioResult, } from '../../server/worldmonitor/supply-chain/v1/scenario-templates'; + +export { SCENARIO_TEMPLATES } from '../../server/worldmonitor/supply-chain/v1/scenario-templates'; diff --git a/src/services/supply-chain/index.ts b/src/services/supply-chain/index.ts index d2a5eedbd..36a97a90e 100644 --- a/src/services/supply-chain/index.ts +++ b/src/services/supply-chain/index.ts @@ -9,6 +9,7 @@ import { type GetCountryChokepointIndexResponse, type GetBypassOptionsResponse, type GetCountryCostShockResponse, + type GetSectorDependencyResponse, type ShippingIndex, type ChokepointInfo, type CriticalMineral, @@ -28,6 +29,7 @@ export type { GetCountryChokepointIndexResponse, GetBypassOptionsResponse, GetCountryCostShockResponse, + GetSectorDependencyResponse, ShippingIndex, ChokepointInfo, CriticalMineral, @@ -149,3 +151,21 @@ export async function fetchCountryCostShock( return empty; } } + +const emptySectorDependency: GetSectorDependencyResponse = { + iso2: '', hs2: '27', hs2Label: '', flags: [], + primaryExporterIso2: '', primaryExporterShare: 0, + primaryChokepointId: '', primaryChokepointExposure: 0, + hasViableBypass: false, fetchedAt: '', +}; + +export async function fetchSectorDependency( + iso2: string, + hs2 = '27', +): Promise { + try { + return await client.getSectorDependency({ iso2, hs2 }); + } catch { + return { ...emptySectorDependency, iso2, hs2 }; + } +} diff --git a/src/shared/premium-paths.ts b/src/shared/premium-paths.ts index 3133f2805..7744489da 100644 --- a/src/shared/premium-paths.ts +++ b/src/shared/premium-paths.ts @@ -20,4 +20,6 @@ export const PREMIUM_RPC_PATHS = new Set([ '/api/sanctions/v1/list-sanctions-pressure', '/api/trade/v1/list-comtrade-flows', '/api/trade/v1/get-tariff-trends', + '/api/scenario/v1/run', + '/api/scenario/v1/status', ]); diff --git a/tests/edge-functions.test.mjs b/tests/edge-functions.test.mjs index 426076cce..4782960cf 100644 --- a/tests/edge-functions.test.mjs +++ b/tests/edge-functions.test.mjs @@ -340,3 +340,103 @@ describe('Edge Function module isolation', () => { }); } }); + +describe('Scenario run endpoint (api/scenario/v1/run.ts)', () => { + const runPath = join(root, 'api', 'scenario', 'v1', 'run.ts'); + + it('exports edge config with runtime: edge', () => { + const src = readFileSync(runPath, 'utf-8'); + assert.ok( + src.includes("runtime: 'edge'") || src.includes('runtime: "edge"'), + 'run.ts: must export config with runtime: edge', + ); + }); + + it('has a default export handler function', () => { + const src = readFileSync(runPath, 'utf-8'); + assert.ok( + src.includes('export default') && src.includes('function handler'), + 'run.ts: must have a default export handler function', + ); + }); + + it('returns 405 for non-POST requests', () => { + const src = readFileSync(runPath, 'utf-8'); + assert.ok( + src.includes('405'), + 'run.ts: must return 405 for non-POST requests', + ); + assert.ok( + src.includes("!== 'POST'") || src.includes('!== "POST"'), + 'run.ts: must check for POST method and reject other methods with 405', + ); + }); + + it('validates scenarioId is required', () => { + const src = readFileSync(runPath, 'utf-8'); + assert.ok( + src.includes('scenarioId'), + 'run.ts: must validate scenarioId field', + ); + assert.ok( + src.includes('400'), + 'run.ts: must return 400 for invalid/missing scenarioId', + ); + }); + + it('uses per-user rate limiting', () => { + const src = readFileSync(runPath, 'utf-8'); + assert.ok( + src.includes('rate') && src.includes('429'), + 'run.ts: must implement rate limiting with 429 response', + ); + }); + + it('uses AbortSignal.timeout on Redis fetch', () => { + const src = readFileSync(runPath, 'utf-8'); + assert.ok( + src.includes('AbortSignal.timeout'), + 'run.ts: all Redis fetches must have AbortSignal.timeout to prevent hanging edge isolates', + ); + }); +}); + +describe('Scenario status endpoint (api/scenario/v1/status.ts)', () => { + const statusPath = join(root, 'api', 'scenario', 'v1', 'status.ts'); + + it('exports edge config with runtime: edge', () => { + const src = readFileSync(statusPath, 'utf-8'); + assert.ok( + src.includes("runtime: 'edge'") || src.includes('runtime: "edge"'), + 'status.ts: must export config with runtime: edge', + ); + }); + + it('returns 400 for missing or invalid jobId', () => { + const src = readFileSync(statusPath, 'utf-8'); + assert.ok( + src.includes('400'), + 'status.ts: must return 400 for missing or invalid jobId', + ); + assert.ok( + src.includes('jobId'), + 'status.ts: must validate jobId query parameter', + ); + }); + + it('validates jobId format to guard against path traversal', () => { + const src = readFileSync(statusPath, 'utf-8'); + assert.ok( + src.includes('JOB_ID_RE') || src.includes('/^scenario:'), + 'status.ts: must validate jobId against a regex to prevent path traversal attacks (e.g. ../../etc/passwd)', + ); + }); + + it('uses AbortSignal.timeout on Redis fetch', () => { + const src = readFileSync(statusPath, 'utf-8'); + assert.ok( + src.includes('AbortSignal.timeout'), + 'status.ts: Redis fetch must have AbortSignal.timeout to prevent hanging edge isolates', + ); + }); +}); diff --git a/todos/152-complete-p1-t-shadows-i18n-import.md b/todos/152-complete-p1-t-shadows-i18n-import.md new file mode 100644 index 000000000..90ac1f048 --- /dev/null +++ b/todos/152-complete-p1-t-shadows-i18n-import.md @@ -0,0 +1,48 @@ +--- +status: complete +priority: p1 +issue_id: "152" +tags: [code-review, quality, supply-chain] +dependencies: [] +--- + +# `t` Parameter Shadows i18n `t` Import in `renderChokepoints` + +## Problem Statement +In `SupplyChainPanel.ts:324`, the arrow function parameter `t` inside `SCENARIO_TEMPLATES.find(t => ...)` shadows the module-level `import { t } from '@/services/i18n'` at line 12. Inside the callback, `t` refers to the template object, not the i18n function. Any translated string added inside this callback (or nearby refactor) will silently use the template object as a function, throwing at runtime. + +## Findings +- **File:** `src/components/SupplyChainPanel.ts:324` +- **Code:** `const template = SCENARIO_TEMPLATES.find(t => t.affectedChokepointIds.includes(cp.id) && t.type !== 'tariff_shock');` +- Not a crash today — `t(...)` is not called inside the callback — but it is a maintenance trap +- Biome may not catch this as an error (depends on shadow rules configured) + +## Proposed Solutions + +### Option A: Rename the arrow function parameter (Recommended) +```ts +const template = SCENARIO_TEMPLATES.find(tmpl => + tmpl.affectedChokepointIds.includes(cp.id) && tmpl.type !== 'tariff_shock' +); +``` +**Pros:** One-line fix, obvious, eliminates shadow entirely +**Cons:** None +**Effort:** Small | **Risk:** None + +## Recommended Action +_Apply Option A immediately — 1-line fix._ + +## Technical Details +- **Affected files:** `src/components/SupplyChainPanel.ts` +- **Line:** 324 + +## Acceptance Criteria +- [ ] Arrow function parameter renamed from `t` to `tmpl` (or similar) +- [ ] `npm run lint` passes with no shadow warnings +- [ ] `npm run typecheck` passes + +## Work Log +- 2026-04-10: Identified by kieran-typescript-reviewer during PR #2910 review + +## Resources +- PR: #2910 diff --git a/todos/153-complete-p1-polling-no-statusresp-ok-check.md b/todos/153-complete-p1-polling-no-statusresp-ok-check.md new file mode 100644 index 000000000..368bdebe4 --- /dev/null +++ b/todos/153-complete-p1-polling-no-statusresp-ok-check.md @@ -0,0 +1,56 @@ +--- +status: complete +priority: p1 +issue_id: "153" +tags: [code-review, quality, supply-chain, reliability] +dependencies: [] +--- + +# Missing `statusResp.ok` Guard in Scenario Polling Loop + +## Problem Statement +In `SupplyChainPanel.ts:677`, the polling loop calls `statusResp.json()` without first checking `statusResp.ok`. A 429, 500, or network error causes `.json()` to throw or produce garbage, silently burning through all 30 iterations. The button ends up stuck on "Computing…" for 60 seconds before showing "Error — retry" with no useful signal. + +## Findings +- **File:** `src/components/SupplyChainPanel.ts`, line 677 +- **Code:** + ```ts + const statusResp = await fetch(`/api/scenario/v1/status?jobId=${encodeURIComponent(jobId)}`); + const status = await statusResp.json() as { status: string; result?: ScenarioResult }; + ``` +- No check on `statusResp.ok` before consuming body +- 30 × 2s = 60s of silent failure with no feedback to the user + +## Proposed Solutions + +### Option A: Guard before `.json()` (Recommended) +```ts +const statusResp = await fetch(`/api/scenario/v1/status?jobId=${encodeURIComponent(jobId)}`); +if (!statusResp.ok) throw new Error(`Status poll failed: ${statusResp.status}`); +const status = await statusResp.json() as { status: string; result?: ScenarioResult }; +``` +**Pros:** Fails fast on server error, exits loop immediately +**Cons:** None +**Effort:** Small | **Risk:** None + +### Option B: Retry on non-fatal errors, throw on fatal +Skip 429/503, throw on 4xx. More complex, adds ~10 lines, probably not worth it for this use case. +**Effort:** Medium | **Risk:** Low + +## Recommended Action +_Apply Option A — guard before `.json()`. One line._ + +## Technical Details +- **Affected files:** `src/components/SupplyChainPanel.ts` +- **Line:** 677 + +## Acceptance Criteria +- [ ] `statusResp.ok` checked before calling `.json()` +- [ ] Loop exits immediately (throws) on non-OK response +- [ ] Button shows "Error — retry" within one polling cycle on server error + +## Work Log +- 2026-04-10: Identified by kieran-typescript-reviewer during PR #2910 review + +## Resources +- PR: #2910 diff --git a/todos/154-complete-p1-result-nonnull-assertion-without-guard.md b/todos/154-complete-p1-result-nonnull-assertion-without-guard.md new file mode 100644 index 000000000..4de2fffe2 --- /dev/null +++ b/todos/154-complete-p1-result-nonnull-assertion-without-guard.md @@ -0,0 +1,64 @@ +--- +status: complete +priority: p1 +issue_id: "154" +tags: [code-review, quality, supply-chain, type-safety] +dependencies: [153] +--- + +# Non-Null Assertion on Optional `result` Without Guard in Polling Loop + +## Problem Statement +In `SupplyChainPanel.ts:679`, `result = status.result!` uses a non-null assertion on a field typed as optional. If the server sends `{ status: 'done' }` without `result` (version mismatch or worker bug), `undefined` is silently passed to `onScenarioActivate`, which crashes in `activateScenario` when it calls `result.topImpactCountries.map(...)`. Additionally, `impactPct * 100` in `showScenarioSummary` could produce `NaN` if the upstream type changes (prior art: `feedback_pizzint_spike_magnitude_type.md`). + +## Findings +- **File:** `src/components/SupplyChainPanel.ts`, line 679 +- **Code:** `if (status.status === 'done') { result = status.result!; break; }` +- `status.result` is typed as optional (`result?: ScenarioResult`) +- If `status.result` is absent, `undefined` propagates to `activateScenario` → crash in `topImpactCountries.map(...)` +- `impactPct` is numeric but upstream could change to string (see MEMORY: `feedback_pizzint_spike_magnitude_type.md`) + +## Proposed Solutions + +### Option A: Explicit guard + type narrowing (Recommended) +```ts +if (status.status === 'done') { + const r = status.result; + if (!r || !Array.isArray(r.topImpactCountries)) throw new Error('done without valid result'); + result = r; + break; +} +``` +**Pros:** Explicit, catches server bugs, type-safe without `!` +**Cons:** Slight verbosity +**Effort:** Small | **Risk:** None + +### Option B: Minimal guard only +```ts +if (status.status === 'done') { + if (!status.result) throw new Error('done without result'); + result = status.result; + break; +} +``` +Protects against `undefined` but not malformed `topImpactCountries`. +**Effort:** Small | **Risk:** Low + +## Recommended Action +_Apply Option A — adds one guard line, eliminates the `!` assertion._ + +## Technical Details +- **Affected files:** `src/components/SupplyChainPanel.ts` +- **Lines:** 679 + +## Acceptance Criteria +- [ ] Non-null assertion `!` removed from `status.result` +- [ ] `topImpactCountries` presence verified before use +- [ ] `npm run typecheck` passes without `!` assertion + +## Work Log +- 2026-04-10: Identified by kieran-typescript-reviewer during PR #2910 review + +## Resources +- PR: #2910 +- MEMORY: `feedback_pizzint_spike_magnitude_type.md` diff --git a/todos/155-complete-p1-scenario-callbacks-not-wired-in-panel-layout.md b/todos/155-complete-p1-scenario-callbacks-not-wired-in-panel-layout.md new file mode 100644 index 000000000..9e046c889 --- /dev/null +++ b/todos/155-complete-p1-scenario-callbacks-not-wired-in-panel-layout.md @@ -0,0 +1,61 @@ +--- +status: complete +priority: p1 +issue_id: "155" +tags: [code-review, architecture, supply-chain, correctness] +dependencies: [] +--- + +# Scenario Callbacks Never Wired in `panel-layout.ts` — Feature Dead on Arrival + +## Problem Statement +`SupplyChainPanel.setOnScenarioActivate()` and `setOnDismissScenario()` are declared and implemented but never called anywhere. When the user clicks "Simulate Closure" and the poll completes, `this.onScenarioActivate?.(...)` fires — but the callback is null, so no map overlays are applied. The dismiss button also does nothing. The entire scenario visual engine is non-functional in production. + +## Findings +- **File:** `src/app/panel-layout.ts` — missing wiring +- `setOnScenarioActivate` and `setOnDismissScenario` public methods exist on `SupplyChainPanel` but are never called +- `MapContainer.setSupplyChainPanel()` also exists but is never called +- The feature compiles and type-checks but is a silent no-op at runtime +- Confirmed by architecture reviewer: "wiring is declared but not connected" + +## Proposed Solutions + +### Option A: Wire callbacks in `panel-layout.ts` (Recommended) +In `src/app/panel-layout.ts`, after `SupplyChainPanel` is created: +```ts +supplyChainPanel.setOnScenarioActivate((id, result) => { + this.ctx.map?.activateScenario(id, result); +}); +supplyChainPanel.setOnDismissScenario(() => { + this.ctx.map?.deactivateScenario(); +}); +``` +This follows the existing pattern for all other panel callbacks (e.g. `MonitorPanel.onChanged`). +**Pros:** Correct architecture, no circular reference, follows established pattern +**Cons:** Requires checking how `this.ctx.map` is typed/available in panel-layout.ts +**Effort:** Small | **Risk:** Low + +### Option B: Keep `setSupplyChainPanel` on MapContainer, call it from panel-layout.ts +Also call `mapContainer.setSupplyChainPanel(supplyChainPanel)` in panel-layout.ts. Keeps the current back-reference architecture but at least makes it functional. +**Pros:** Minimal change to the current design +**Cons:** Preserves the bidirectional coupling (see todo #161) +**Effort:** Small | **Risk:** Low + +## Recommended Action +_Apply Option A — wire callbacks in `panel-layout.ts`. This is the minimum fix to make the feature functional and also the architecturally correct approach (see #161 for the full coupling cleanup)._ + +## Technical Details +- **Affected files:** `src/app/panel-layout.ts` (wiring), `src/components/SupplyChainPanel.ts` (callbacks declared) +- **Lines in panel-layout.ts:** find where `SupplyChainPanel` is constructed (grep: `new SupplyChainPanel`) + +## Acceptance Criteria +- [ ] `setOnScenarioActivate` wired in `panel-layout.ts` → calls `mapContainer.activateScenario` +- [ ] `setOnDismissScenario` wired in `panel-layout.ts` → calls `mapContainer.deactivateScenario` +- [ ] Clicking "Simulate Closure" (PRO, done scenario) applies arc recolor + heat layer on DeckGL +- [ ] Dismiss × on banner restores normal visual state + +## Work Log +- 2026-04-10: Identified by architecture-strategist during PR #2910 review + +## Resources +- PR: #2910 diff --git a/todos/156-complete-p2-globe-scenario-missing-stroke-color-branch.md b/todos/156-complete-p2-globe-scenario-missing-stroke-color-branch.md new file mode 100644 index 000000000..34b0babe9 --- /dev/null +++ b/todos/156-complete-p2-globe-scenario-missing-stroke-color-branch.md @@ -0,0 +1,56 @@ +--- +status: complete +priority: p2 +issue_id: "156" +tags: [code-review, quality, supply-chain, visual] +dependencies: [] +--- + +# GlobeMap `polygonStrokeColor` Missing `scenario` Branch — Falls Through to Red + +## Problem Statement +In `GlobeMap.ts`, `polygonStrokeColor` has no `'scenario'` case. When scenario polygons are rendered, the callback falls through to the catch-all `return '#ff4444'` (conflict red), giving affected countries a visible red border that clashes with the intended orange/amber heat tint and visually conflates scenario state with conflict data. + +## Findings +- **File:** `src/components/GlobeMap.ts` +- `polygonCapColor` and `polygonSideColor` both have `'scenario'` branches added in Sprint E +- `polygonStrokeColor` was missed — falls through to `'#ff4444'` (conflict) or `'transparent'` +- At runtime: scenario-affected countries appear with a red border matching conflict polygons +- Identified during architecture review of PR #2910 + +## Proposed Solutions + +### Option A: Add `scenario` branch returning transparent (Recommended) +```ts +// In polygonStrokeColor callback: +if (d._kind === 'scenario') return 'transparent'; +``` +Scenario heat overlay has no border — consistent with how the DeckGL `GeoJsonLayer` has `stroked: false`. +**Pros:** Zero visual noise, matches DeckGL behavior, 1-line fix +**Cons:** None +**Effort:** Small | **Risk:** None + +### Option B: Use a subtle amber stroke +```ts +if (d._kind === 'scenario') return 'rgba(220,120,40,0.4)'; +``` +Adds a faint amber outline to distinguish from other polygon types. +**Effort:** Small | **Risk:** Low (visual choice) + +## Recommended Action +_Apply Option A — transparent stroke for scenario polygons, matching DeckGL's `stroked: false`._ + +## Technical Details +- **Affected files:** `src/components/GlobeMap.ts` +- **Grep:** `polygonStrokeColor` — find the callback and add the missing branch before the `conflict` case + +## Acceptance Criteria +- [ ] `polygonStrokeColor` returns `'transparent'` (or no-stroke equivalent) when `d._kind === 'scenario'` +- [ ] No red border visible on Globe scenario overlay +- [ ] `npm run typecheck` passes + +## Work Log +- 2026-04-10: Identified during /ce-review of PR #2910 (GlobeMap polygon rendering) + +## Resources +- PR: #2910 diff --git a/todos/157-complete-p2-scenario-templates-server-import-boundary.md b/todos/157-complete-p2-scenario-templates-server-import-boundary.md new file mode 100644 index 000000000..b7d6bae13 --- /dev/null +++ b/todos/157-complete-p2-scenario-templates-server-import-boundary.md @@ -0,0 +1,60 @@ +--- +status: complete +priority: p2 +issue_id: "157" +tags: [code-review, architecture, supply-chain, import-boundaries] +dependencies: [] +--- + +# `SCENARIO_TEMPLATES` Imported Directly from `../../server/` — Module Boundary Violation + +## Problem Statement +`SupplyChainPanel.ts` imports `SCENARIO_TEMPLATES` via a relative path crossing into `server/`: +```ts +import { SCENARIO_TEMPLATES } from '../../server/worldmonitor/supply-chain/v1/scenario-templates'; +``` +Client components must not import from `server/` directly — the `src/` → `server/` boundary keeps server-only deps (gRPC stubs, proto codegen) out of the browser bundle. This import will silently work today but will break if any server-only module is added upstream of `scenario-templates.ts`. + +## Findings +- **File:** `src/components/SupplyChainPanel.ts` +- Direct relative import crosses `src/` → `server/` module boundary +- `src/config/scenario-templates.ts` already re-exports the `ScenarioResult` and `ScenarioVisualState` types from this file +- The `SCENARIO_TEMPLATES` const is not yet re-exported from `src/config/scenario-templates.ts` +- Pattern in codebase: all other panel files import from `@/config/`, `@/services/`, or `@/components/` +- Identified by architecture-strategist during PR #2910 review + +## Proposed Solutions + +### Option A: Re-export `SCENARIO_TEMPLATES` via `src/config/scenario-templates.ts` (Recommended) +In `src/config/scenario-templates.ts`, add: +```ts +export { SCENARIO_TEMPLATES } from '../../server/worldmonitor/supply-chain/v1/scenario-templates'; +``` +Then update `SupplyChainPanel.ts`: +```ts +import { SCENARIO_TEMPLATES } from '@/config/scenario-templates'; +``` +**Pros:** One canonical boundary crossing point, consistent with existing type re-exports, no runtime impact +**Cons:** Needs one extra line in config file +**Effort:** Small | **Risk:** None + +### Option B: Add a `src/` copy or adapter +Duplicate the const in `src/config/`. More files, divergence risk. +**Effort:** Medium | **Risk:** Medium (divergence) + +## Recommended Action +_Apply Option A — add one re-export to `src/config/scenario-templates.ts` and update the import in `SupplyChainPanel.ts`._ + +## Technical Details +- **Affected files:** `src/config/scenario-templates.ts` (add re-export), `src/components/SupplyChainPanel.ts` (update import) + +## Acceptance Criteria +- [ ] `src/components/SupplyChainPanel.ts` no longer imports from `../../server/` +- [ ] `SCENARIO_TEMPLATES` accessible via `@/config/scenario-templates` +- [ ] `npm run typecheck` passes + +## Work Log +- 2026-04-10: Identified by architecture-strategist during PR #2910 review + +## Resources +- PR: #2910 diff --git a/todos/158-complete-p2-scenario-heat-layer-set-alloc-every-build.md b/todos/158-complete-p2-scenario-heat-layer-set-alloc-every-build.md new file mode 100644 index 000000000..ec0b23fa3 --- /dev/null +++ b/todos/158-complete-p2-scenario-heat-layer-set-alloc-every-build.md @@ -0,0 +1,73 @@ +--- +status: complete +priority: p2 +issue_id: "158" +tags: [code-review, performance, supply-chain, deckgl] +dependencies: [] +--- + +# `createScenarioHeatLayer` Allocates New `Set` on Every `buildLayers()` Call + +## Problem Statement +`DeckGLMap.createScenarioHeatLayer()` constructs `new Set(this.scenarioState.affectedIso2s)` inside the method body. `buildLayers()` is called on every frame/render cycle by DeckGL when layers need to be rebuilt. The `Set` allocation is O(n) on the number of affected ISO-2 codes and runs inside the hot DeckGL render path. + +## Findings +- **File:** `src/components/DeckGLMap.ts` +- **Code:** + ```ts + private createScenarioHeatLayer(): GeoJsonLayer | null { + if (!this.scenarioState?.affectedIso2s?.length || !this.countriesGeoJsonData) return null; + const affected = new Set(this.scenarioState.affectedIso2s); // ← allocated every call + return new GeoJsonLayer({ ..., getFillColor: (feature) => { const code = ...; return affected.has(code) ? ... } }); + } + ``` +- `buildLayers()` is called whenever deck viewport or layers change — potentially dozens of times per second during pan/zoom +- The `Set` contents only change when `setScenarioState()` is called (rare) +- Identified by performance-oracle during PR #2910 review + +## Proposed Solutions + +### Option A: Cache the Set in `setScenarioState()` (Recommended) +```ts +private affectedIso2Set: Set = new Set(); + +public setScenarioState(state: ScenarioVisualState | null): void { + this.scenarioState = state; + this.affectedIso2Set = new Set(state?.affectedIso2s ?? []); + this.rebuildLayers(); +} + +private createScenarioHeatLayer(): GeoJsonLayer | null { + if (!this.affectedIso2Set.size || !this.countriesGeoJsonData) return null; + return new GeoJsonLayer({ ..., getFillColor: (feature) => { + const code = feature.properties?.['ISO3166-1-Alpha-2'] as string | undefined; + return (code && this.affectedIso2Set.has(code) ? [220, 60, 40, 80] : [0, 0, 0, 0]) as [number,number,number,number]; + }}); +} +``` +**Pros:** Set allocated once per state change (not per render), correct `updateTriggers` still invalidates DeckGL cache +**Cons:** Small memory overhead for the cached Set field +**Effort:** Small | **Risk:** Low + +### Option B: Keep as-is with a comment +Acceptable if `buildLayers()` is only called on state change. But DeckGL calls it more often. +**Effort:** None | **Risk:** High (performance regression on active globe interactions) + +## Recommended Action +_Apply Option A — cache the Set in `setScenarioState()`._ + +## Technical Details +- **Affected files:** `src/components/DeckGLMap.ts` +- Add private `affectedIso2Set: Set = new Set()` field +- Move Set construction to `setScenarioState()` + +## Acceptance Criteria +- [ ] `createScenarioHeatLayer` does not allocate a new `Set` on each call +- [ ] `setScenarioState()` rebuilds the cached Set +- [ ] `npm run typecheck` passes + +## Work Log +- 2026-04-10: Identified by performance-oracle during PR #2910 review + +## Resources +- PR: #2910 diff --git a/todos/159-pending-p2-globe-flush-polygons-scenario-precompute.md b/todos/159-pending-p2-globe-flush-polygons-scenario-precompute.md new file mode 100644 index 000000000..e86ac27ab --- /dev/null +++ b/todos/159-pending-p2-globe-flush-polygons-scenario-precompute.md @@ -0,0 +1,73 @@ +--- +status: pending +priority: p2 +issue_id: "159" +tags: [code-review, performance, supply-chain, globe] +dependencies: [] +--- + +# `flushPolygons` Iterates All 250 GeoJSON Features on Every Flush — Pre-compute Scenario Polygons + +## Problem Statement +`GlobeMap.flushPolygons()` iterates over `countriesGeoJsonData.features` to build scenario `GlobePolygon` objects on every call. `flushPolygons()` is called whenever ANY polygon layer changes (CII update, conflict update, imagery toggle, storm cone render). With ~250 GeoJSON features, this is ~250 object allocations + ISO-2 Set lookups on every flush, even when the scenario state hasn't changed. + +## Findings +- **File:** `src/components/GlobeMap.ts` +- `flushPolygons()` constructs scenario polygons inline from `this.scenarioState.affectedIso2s` + `countriesGeoJsonData` on every call +- Every other polygon layer (CII, conflict, imagery) pre-builds its polygon array before `flushPolygons()` and simply concatenates +- `setScenarioState()` calls `flushPolygons()` but does not cache the pre-built polygon array +- Identified by performance-oracle during PR #2910 review + +## Proposed Solutions + +### Option A: Pre-compute scenario polygons in `setScenarioState()` (Recommended) +```ts +private scenarioPolygons: GlobePolygon[] = []; + +public setScenarioState(state: ScenarioVisualState | null): void { + this.scenarioState = state; + if (!state?.affectedIso2s?.length || !this.countriesGeoJsonData) { + this.scenarioPolygons = []; + } else { + const affected = new Set(state.affectedIso2s); + this.scenarioPolygons = this.countriesGeoJsonData.features + .filter(f => affected.has(f.properties?.['ISO3166-1-Alpha-2'] as string)) + .map(f => ({ ...buildGlobePolygon(f), _kind: 'scenario' as const })); + } + this.flushPolygons(); +} +``` +Then in `flushPolygons()`: +```ts +const polys = [...this.ciiPolygons, ...this.conflictPolygons, ...this.scenarioPolygons, ...]; +(this.globe as any).polygonsData(polys); +``` +**Pros:** O(1) in `flushPolygons()` for scenario layer, consistent with existing pattern for other polygon types +**Cons:** Slightly larger memory footprint (cached array) +**Effort:** Small | **Risk:** Low + +### Option B: Keep inline, add early-exit guard +```ts +if (!this.scenarioState?.affectedIso2s?.length) { /* skip scenario loop */ } +``` +Reduces cost to near-zero when no scenario active, but still O(n) when active. +**Effort:** Trivial | **Risk:** None (acceptable trade-off) + +## Recommended Action +_Apply Option A if the inline loop is confirmed to run frequently (profile first). Option B is an acceptable minimal fix if profiling shows low impact._ + +## Technical Details +- **Affected files:** `src/components/GlobeMap.ts` +- Add `private scenarioPolygons: GlobePolygon[] = []` +- Move construction into `setScenarioState()` + +## Acceptance Criteria +- [ ] `flushPolygons()` does not iterate GeoJSON features when scenario state is unchanged +- [ ] `setScenarioState()` pre-builds `scenarioPolygons` +- [ ] `npm run typecheck` passes + +## Work Log +- 2026-04-10: Identified by performance-oracle during PR #2910 review + +## Resources +- PR: #2910 diff --git a/todos/160-complete-p2-scenario-fetch-missing-auth-header.md b/todos/160-complete-p2-scenario-fetch-missing-auth-header.md new file mode 100644 index 000000000..c902d82a1 --- /dev/null +++ b/todos/160-complete-p2-scenario-fetch-missing-auth-header.md @@ -0,0 +1,64 @@ +--- +status: complete +priority: p2 +issue_id: "160" +tags: [code-review, security, supply-chain, auth] +dependencies: [] +--- + +# Scenario Fetch Calls in `attachScenarioTriggers` Missing Auth Header + +## Problem Statement +The `fetch` calls in `SupplyChainPanel.attachScenarioTriggers()` — both the initial `POST /api/scenario/v1/run` and the polling `GET /api/scenario/v1/status` — send no authentication header. The API endpoints use `validateApiKey()` which, for browser origins (trusted), silently returns `required: false` and skips key validation. This means PRO-gated endpoints are reachable unauthenticated from any browser tab that happens to have the right origin header. + +## Findings +- **File:** `src/components/SupplyChainPanel.ts` — `attachScenarioTriggers()` polling loop +- No `X-WorldMonitor-Key` or `Authorization` header on either fetch call +- `validateApiKey(req)` with no `{ forceKey: true }` returns `{ required: false }` for browser origins (see MEMORY: `feedback_validateapikey_forcekey_vendor.md`) +- The PRO gate is only checked client-side in `MapContainer.activateScenario()` — bypassing it server-side is trivial via curl with `Origin: https://worldmonitor.app` +- Identified by security-sentinel during PR #2910 review + +## Proposed Solutions + +### Option A: Add auth header using `getApiKey()` (Recommended) +```ts +import { getApiKey } from '@/services/auth-state'; + +// In attachScenarioTriggers(): +const headers: Record = { 'Content-Type': 'application/json' }; +const apiKey = getApiKey(); +if (apiKey) headers['X-WorldMonitor-Key'] = apiKey; + +const runResp = await fetch('/api/scenario/v1/run', { method: 'POST', headers, body: JSON.stringify(payload) }); +// ... +const statusResp = await fetch(`/api/scenario/v1/status?jobId=...`, { headers }); +``` +**Pros:** Consistent with other authenticated panel fetches, enables server-side PRO gating +**Cons:** None +**Effort:** Small | **Risk:** None + +### Option B: Add `forceKey: true` on the API endpoints + client auth header +Also update the API endpoints to use `validateApiKey(req, { forceKey: true })` to reject unauthenticated browser requests. +**Pros:** Defense in depth — server-side gate enforced regardless of client behavior +**Cons:** Requires API changes too +**Effort:** Small | **Risk:** Low + +## Recommended Action +_Apply Option A immediately (client auth header). Option B should follow as a separate API hardening change._ + +## Technical Details +- **Affected files:** `src/components/SupplyChainPanel.ts` +- `api/scenario/v1/run.ts` and `api/scenario/v1/status.ts` — consider `forceKey: true` follow-up + +## Acceptance Criteria +- [ ] Both fetch calls in `attachScenarioTriggers` include `X-WorldMonitor-Key` header when available +- [ ] No regression for users without an API key (header omitted gracefully) +- [ ] `npm run typecheck` passes + +## Work Log +- 2026-04-10: Identified by security-sentinel during PR #2910 review + +## Resources +- PR: #2910 +- MEMORY: `feedback_validateapikey_forcekey_vendor.md` +- MEMORY: `feedback_is_caller_premium_trusted_origin.md` diff --git a/todos/161-pending-p2-mapcontainer-supply-chain-panel-bidirectional-coupling.md b/todos/161-pending-p2-mapcontainer-supply-chain-panel-bidirectional-coupling.md new file mode 100644 index 000000000..a10ea2622 --- /dev/null +++ b/todos/161-pending-p2-mapcontainer-supply-chain-panel-bidirectional-coupling.md @@ -0,0 +1,59 @@ +--- +status: pending +priority: p2 +issue_id: "161" +tags: [code-review, architecture, supply-chain, coupling] +dependencies: [155] +--- + +# MapContainer ↔ SupplyChainPanel Bidirectional Coupling via `setSupplyChainPanel` + +## Problem Statement +`MapContainer` has a `setSupplyChainPanel(panel)` setter that stores a direct reference to `SupplyChainPanel`. This creates a bidirectional dependency: `SupplyChainPanel` calls into `MapContainer` via callbacks (correct direction), and `MapContainer` calls back into `SupplyChainPanel` via `showScenarioSummary` / `hideScenarioSummary` (reverse direction). This circular architecture makes both components harder to test and violates the unidirectional data flow established by every other panel in the system. + +## Findings +- **File:** `src/components/MapContainer.ts` — `setSupplyChainPanel()` and `deactivateScenario()` calling `this.supplyChainPanel?.hideScenarioSummary()` +- **File:** `src/components/SupplyChainPanel.ts` — `setOnDismissScenario` / `setOnScenarioActivate` callbacks point to `MapContainer` +- All other panels (MonitorPanel, CountryDeepDivePanel, etc.) use one-way callbacks: panel → map, never map → panel +- The bidirectional link was noted in the Sprint E plan as deferred to this issue +- Identified by architecture-strategist during PR #2910 review (see todo #155 for the more urgent wiring issue) + +## Proposed Solutions + +### Option A: Remove `showScenarioSummary`/`hideScenarioSummary` from `MapContainer` dispatch (Recommended) +Instead of `MapContainer` calling `panel.showScenarioSummary()` and `panel.hideScenarioSummary()`: +- Have `SupplyChainPanel` observe scenario state changes through its existing `onScenarioActivate` and `onDismissScenario` callbacks +- The panel already receives `(id, result)` in `onScenarioActivate` — it can call `showScenarioSummary` on itself +- `MapContainer.activateScenario` fires `onScenarioActivate` → panel handles its own UI update +- `MapContainer.deactivateScenario` fires `onDismissScenario` → panel handles its own dismiss +- Remove `setSupplyChainPanel` from `MapContainer` entirely + +**Pros:** Breaks the circular reference, consistent with all other panels +**Cons:** Requires refactoring how the panel receives activation events (it already does via callbacks) +**Effort:** Medium | **Risk:** Low + +### Option B: Keep `setSupplyChainPanel` but document the coupling explicitly +Add a JSDoc comment acknowledging the bidirectional dependency and track via this todo. +**Pros:** No refactor needed now +**Cons:** Coupling persists indefinitely, harder to unit-test +**Effort:** Trivial | **Risk:** None (deferred tech debt) + +## Recommended Action +_Apply Option A in a follow-up PR after P1 issues (#155) are fixed. The coupling is architectural debt but not a blocking bug._ + +## Technical Details +- **Affected files:** `src/components/MapContainer.ts`, `src/components/SupplyChainPanel.ts` +- Dependency: Fix todo #155 first (wiring), then restructure the callback flow here + +## Acceptance Criteria +- [ ] `MapContainer` has no direct reference to `SupplyChainPanel` +- [ ] `setSupplyChainPanel()` removed from `MapContainer` +- [ ] Panel banner updates triggered via existing `onScenarioActivate` / `onDismissScenario` callbacks +- [ ] `npm run typecheck` passes + +## Work Log +- 2026-04-10: Identified by architecture-strategist during PR #2910 review + +## Resources +- PR: #2910 +- Related: todo #155 (callbacks not wired — fix first) diff --git a/todos/162-complete-p2-attach-scenario-triggers-no-in-flight-guard.md b/todos/162-complete-p2-attach-scenario-triggers-no-in-flight-guard.md new file mode 100644 index 000000000..858cae002 --- /dev/null +++ b/todos/162-complete-p2-attach-scenario-triggers-no-in-flight-guard.md @@ -0,0 +1,71 @@ +--- +status: complete +priority: p2 +issue_id: "162" +tags: [code-review, quality, supply-chain, reliability] +dependencies: [] +--- + +# `attachScenarioTriggers` Has No In-Flight Guard — Concurrent Polling Possible + +## Problem Statement +`SupplyChainPanel.attachScenarioTriggers()` launches a polling loop but has no guard against being called while a previous poll is still running. If the user clicks "Simulate Closure" a second time before the first job completes (e.g., after navigating away and back to the chokepoint), two polling loops run concurrently. Both can call `onScenarioActivate`, resulting in a race between two job results applying visual state in an undefined order. + +## Findings +- **File:** `src/components/SupplyChainPanel.ts` +- `attachScenarioTriggers()` is called from the "Simulate Closure" button click handler +- No `isPolling` flag or AbortController checked before starting a new poll +- Existing guard: `if (!button.isConnected) break` only exits on DOM removal, not on second click +- Two concurrent polls can overlap and call `onScenarioActivate` with different results +- Identified by kieran-typescript-reviewer during PR #2910 review + +## Proposed Solutions + +### Option A: AbortController + in-flight flag (Recommended) +```ts +private scenarioPollController: AbortController | null = null; + +private async attachScenarioTriggers(button: HTMLButtonElement, cp: Chokepoint): Promise { + // Cancel any in-flight poll + this.scenarioPollController?.abort(); + this.scenarioPollController = new AbortController(); + const { signal } = this.scenarioPollController; + + // ... in the polling loop: + if (signal.aborted || !button.isConnected) break; + const statusResp = await fetch(`...`, { signal }); +} +``` +**Pros:** Clean cancellation, prevents concurrent polls, mirrors standard fetch abort patterns +**Cons:** Need to handle `AbortError` gracefully (not show error banner) +**Effort:** Small | **Risk:** Low + +### Option B: Simple boolean in-flight flag +```ts +private isScenarioPolling = false; + +if (this.isScenarioPolling) return; +this.isScenarioPolling = true; +try { /* poll */ } finally { this.isScenarioPolling = false; } +``` +Blocks second trigger rather than cancelling first. Simpler but less responsive (user can't restart a stalled poll). +**Effort:** Small | **Risk:** None + +## Recommended Action +_Apply Option A — AbortController for clean cancellation, matching the codebase's existing fetch patterns._ + +## Technical Details +- **Affected files:** `src/components/SupplyChainPanel.ts` +- Add `private scenarioPollController: AbortController | null = null` +- Handle `AbortError` in the catch block (do not show error banner on abort) + +## Acceptance Criteria +- [ ] Second click aborts previous polling loop +- [ ] `AbortError` not surfaced to user as "Error — retry" +- [ ] `npm run typecheck` passes + +## Work Log +- 2026-04-10: Identified by kieran-typescript-reviewer during PR #2910 review + +## Resources +- PR: #2910 diff --git a/todos/163-complete-p3-scenario-banner-shows-raw-id.md b/todos/163-complete-p3-scenario-banner-shows-raw-id.md new file mode 100644 index 000000000..cbdcc6653 --- /dev/null +++ b/todos/163-complete-p3-scenario-banner-shows-raw-id.md @@ -0,0 +1,45 @@ +--- +status: complete +priority: p3 +issue_id: "163" +tags: [code-review, quality, supply-chain, ux] +dependencies: [] +--- + +# Scenario Banner Shows Raw Template ID Instead of Human-Readable Name + +## Problem Statement +The scenario activation banner in `SupplyChainPanel.showScenarioSummary()` displays the raw `scenarioId` string (e.g., `"suez-full-closure-2026"`) rather than the template's `.name` property (e.g., `"Suez Canal Full Closure"`). Users see a machine identifier instead of a localized, readable label. + +## Findings +- **File:** `src/components/SupplyChainPanel.ts` +- Banner HTML likely contains `${scenarioId}` where it should use `SCENARIO_TEMPLATES.find(tmpl => tmpl.id === scenarioId)?.name ?? scenarioId` +- `SCENARIO_TEMPLATES` is already imported in the same file +- Minor UX issue — doesn't block functionality +- Identified during code review of PR #2910 + +## Proposed Solutions + +### Option A: Look up template name with fallback +```ts +const templateName = SCENARIO_TEMPLATES.find(tmpl => tmpl.id === scenarioId)?.name ?? scenarioId; +// Use templateName in banner heading +``` +**Pros:** Shows human-readable label, graceful fallback to ID if template not found +**Effort:** Small | **Risk:** None + +## Recommended Action +_Apply Option A — 1-line lookup._ + +## Technical Details +- **Affected files:** `src/components/SupplyChainPanel.ts` — `showScenarioSummary()` + +## Acceptance Criteria +- [ ] Banner heading shows template `.name`, not raw `scenarioId` +- [ ] Falls back to raw ID if template not found + +## Work Log +- 2026-04-10: Identified during PR #2910 review + +## Resources +- PR: #2910 diff --git a/todos/164-pending-p3-fetch-sector-dependency-no-circuit-breaker.md b/todos/164-pending-p3-fetch-sector-dependency-no-circuit-breaker.md new file mode 100644 index 000000000..c667ca751 --- /dev/null +++ b/todos/164-pending-p3-fetch-sector-dependency-no-circuit-breaker.md @@ -0,0 +1,59 @@ +--- +status: pending +priority: p3 +issue_id: "164" +tags: [code-review, quality, supply-chain, reliability] +dependencies: [] +--- + +# `fetchSectorDependency` Has No Circuit Breaker — Retries Indefinitely on Persistent Failure + +## Problem Statement +`src/services/supply-chain/index.ts:fetchSectorDependency()` catches all errors and returns `emptySectorDependency`. While this prevents crashes, it means every call during a persistent outage (e.g., server restart, network partition) makes a live gRPC attempt before falling back. If the supply-chain panel calls this per-chokepoint on every render, a 30-chokepoint list during an outage = 30 sequential timeouts per render cycle. + +## Findings +- **File:** `src/services/supply-chain/index.ts` +- **Code:** + ```ts + export async function fetchSectorDependency(iso2, hs2 = '27') { + try { + return await client.getSectorDependency({ iso2, hs2 }); + } catch { + return { ...emptySectorDependency, iso2, hs2 }; + } + } + ``` +- No timeout, no cached failure state, no back-off +- Minor issue today (called rarely), but will matter at scale +- Identified by kieran-typescript-reviewer during PR #2910 review + +## Proposed Solutions + +### Option A: Add a short timeout to the gRPC call +```ts +const ac = new AbortController(); +setTimeout(() => ac.abort(), 3000); +return await client.getSectorDependency({ iso2, hs2 }, { signal: ac.signal }); +``` +Fails fast, reduces hung call duration. +**Effort:** Small | **Risk:** Low + +### Option B: Deduplicate in-flight requests with a request Map +Cache the promise keyed by `${iso2}:${hs2}` — deduplicates concurrent calls for the same country. +**Effort:** Small | **Risk:** Low + +## Recommended Action +_Combine A + B: timeout + in-flight dedup. Low-effort, prevents worst-case pile-up._ + +## Technical Details +- **Affected files:** `src/services/supply-chain/index.ts` + +## Acceptance Criteria +- [ ] `fetchSectorDependency` times out within ~3s rather than hanging indefinitely +- [ ] Concurrent calls for the same `(iso2, hs2)` share one in-flight promise + +## Work Log +- 2026-04-10: Identified by kieran-typescript-reviewer during PR #2910 review + +## Resources +- PR: #2910 diff --git a/todos/165-pending-p3-scenario-rate-limiter-keys-off-ip-not-identity.md b/todos/165-pending-p3-scenario-rate-limiter-keys-off-ip-not-identity.md new file mode 100644 index 000000000..8012a0960 --- /dev/null +++ b/todos/165-pending-p3-scenario-rate-limiter-keys-off-ip-not-identity.md @@ -0,0 +1,49 @@ +--- +status: pending +priority: p3 +issue_id: "165" +tags: [code-review, security, supply-chain, rate-limiting] +dependencies: [] +--- + +# Scenario Rate Limiter Keys Off IP — Shared Egress Customers Share Rate Bucket + +## Problem Statement +`api/scenario/v1/run.ts` rate-limits requests by client IP. In enterprise or office environments where many users share a single egress IP (NAT, VPN), all users share one rate bucket. A single heavy user can exhaust the quota for all colleagues on the same IP. The correct key for a PRO-gated endpoint is the authenticated API key identity. + +## Findings +- **File:** `api/scenario/v1/run.ts` +- Rate limit key: likely `getClientIp(req)` (standard pattern in codebase) +- PRO endpoints in the codebase that handle multiple users per IP should key by API key identity +- See MEMORY: `feedback_is_caller_premium_trusted_origin.md` — the API key is extractable from `X-WorldMonitor-Key` +- Minor issue: scenario endpoint is PRO-only, low traffic volume — not urgent +- Identified by security-sentinel during PR #2910 review + +## Proposed Solutions + +### Option A: Key rate limit by API key when present, fall back to IP +```ts +const apiKey = req.headers.get('x-worldmonitor-key'); +const rateLimitKey = apiKey ? `scenario:key:${apiKey}` : `scenario:ip:${getClientIp(req)}`; +``` +**Pros:** Per-identity limiting for authenticated users, IP fallback for unauth +**Cons:** Unauthenticated requests still IP-keyed (acceptable) +**Effort:** Small | **Risk:** None + +## Recommended Action +_Apply Option A in a follow-up. Not blocking — scenario endpoint is PRO-only and low-traffic._ + +## Technical Details +- **Affected files:** `api/scenario/v1/run.ts` + +## Acceptance Criteria +- [ ] Rate limit key uses `X-WorldMonitor-Key` when present +- [ ] Falls back to IP for requests without a key +- [ ] Existing rate limit tests pass + +## Work Log +- 2026-04-10: Identified by security-sentinel during PR #2910 review + +## Resources +- PR: #2910 +- MEMORY: `feedback_is_caller_premium_trusted_origin.md`