Skip to content

Commit ed5cf9f

Browse files
authored
fix(premium): fan out ALL PRO-gated loaders on Free→Pro + close supply-chain race (koala73#3828)
* fix(premium): fan out ALL PRO-gated loaders on Free→Pro + close supply-chain race Two empty-on-mobile bug reports, one root cause: PRO-gated loaders silently strand when hasPremiumAccess() returns false at the boot tick. ## Symptom 1 — PREMIUM STOCK ANALYSIS empty body data-loader.ts:456 gates loadStockAnalysis on `hasPremiumAccess() && shouldLoad(...)`. If Clerk/Convex haven't hydrated by the loadAllData(true) boot call, the task is skipped. App.ts:1017-1031 firePremiumLoaders only re-fired loadTradePolicy on Free→Pro — every other PRO-gated loader was on its own. The scheduled refresh that WOULD have caught up is gated to SITE_VARIANT === 'finance' (App.ts:1495), so on the full variant the panel sits empty for the entire session. Secondary contributor: Panel.ts:954 showRetrying and Panel.ts:1045 setContent both `if (this._locked) return;` silently, so even if loadStockAnalysis DID run while the panel was rendering its locked CTA, the data render was swallowed; unlockPanel then restored the pre-lock _savedContent (an empty body). Fix: expand firePremiumLoaders to fan out stock-analysis, stock-backtest, daily-market-brief, market-implications, wsb-tickers, and resilience-ranking. Each loader is idempotent and self-checks hasPremiumAccess() internally, so the calls are safe to fire even if they were already covered by a scheduler. ## Symptom 2 — Route Explorer "No modeled lane" for HK→DE (Pro user) src/services/supply-chain/index.ts had 8 fetchers gated by the BARE `hasPremiumAccess()` (no auth state) which reads only the cached isProUser() Convex tier signal. RouteExplorer.fetchLane uses `hasPremiumAccess(getAuthState())` which ALSO accepts Clerk's user.role === 'pro'. The two diverge during the Clerk-resolved-but-Convex-stale window: RouteExplorer's outer gate passes, then fetchRouteExplorerLane's inner gate fails and returns `{ ...emptyRouteExplorerLane, ...args }` — which has noModeledLane: true. The UI then renders "No modeled lane for this pair", the SAME message shown when the dataset genuinely lacks the lane. The user can't tell the two cases apart. Fix: introduce a `gatedPremium()` helper that always calls `hasPremiumAccess(getAuthState())`, and route all 8 supply-chain fetcher gates through it. Eliminates the divergence window for every supply-chain consumer in one shot. ## Audit-locking regression test tests/premium-loaders-fan-out-coverage.test.mts statically extracts every `this.loadX()` call sitting inside an `if (hasPremiumAccess(...))` block in data-loader.ts and asserts each appears in App.ts:firePremiumLoaders (with a documented ALLOWLIST escape hatch). The next contributor who adds a PRO-gated loader without wiring fan-out gets a CI failure with a clear "add `void this.dataLoader.loadX();` to firePremiumLoaders" message. * fix(premium): address PR koala73#3828 review — revert no-op, fix real HK→DE cause, fix test regex Three follow-ups to PR koala73#3828 review findings: ## P1: Reverted supply-chain auth-aware change — it was a no-op Reviewer is correct: bare `hasPremiumAccess()` already includes the Clerk signal because `panel-gating.ts:31` calls `isProUser()`, and `widget-store.ts:175` `isProUser()` already does `getAuthState().user?.role === 'pro'`. So `hasPremiumAccess()` and `hasPremiumAccess(getAuthState())` return identically — passing auth state only saves a redundant inner check. My `gatedPremium()` helper closed no race window. Reverted the 8 supply-chain call sites + helper + comment. ## P1 followup: actual HK→DE root cause — port-cluster data is wrong Hong Kong's port cluster in `scripts/shared/country-port-clusters.json` only lists `["china-us-west", "intra-asia-container"]`. Germany lists the Asia-Europe routes (`china-europe-suez`, `asia-europe-cape`, `transatlantic`). Zero overlap → `sharedRoutes.length === 0` → `noModeledLane = true`. The Route Explorer message is technically correct: the lane wasn't modeled. The DATA was wrong — HK is one of the world's busiest container ports and ships to Europe via Suez routinely. Added `china-europe-suez` and `asia-europe-cape` to HK's cluster. HK→DE now has 2 shared routes (was 0). Verified other Asian ports (TW, KR, JP, VN, TH, PH) have the same gap — left for a follow-up PR since each needs review against actual shipping data. ## P2: Fan-out coverage test was extracting the wrong loaders Reviewer caught that the regex missed shape (c) single-line gates like `if (hasPremiumAccess() && shouldLoad('wsb-ticker-scanner')) tasks.push(...)`. On investigation it was worse: the broadened regex ONLY matched shape (b) `if (hasPremiumAccess()) { ... }`. The other loaders happened to appear inside an init() handler that ALSO matches shape (b), giving false confidence that the production gates at lines 456/459/462 were covered. Replaced the regex-only approach with a line-walking extractor: 1. Find every line containing `hasPremiumAccess(` 2. Filter to `if`/`else if` gates 3. Collect calls from the line itself (covers shape c) 4. If the line ends with `{`, walk forward with brace-depth tracking until the matching close (covers shapes a and b) Now extracts all 6 PRO-gated loaders (was 4, with 2 of those caught coincidentally via init()). Added a fixture test that proves all 3 shapes extract correctly — a future "regex simplification" that drops a shape fails immediately with a clear message.
1 parent 9a4e40f commit ed5cf9f

3 files changed

Lines changed: 169 additions & 4 deletions

File tree

scripts/shared/country-port-clusters.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@
9090
"LA": { "nearestRouteIds": ["intra-asia-container"], "coastSide": "landlocked" },
9191
"MN": { "nearestRouteIds": ["china-us-west"], "coastSide": "landlocked" },
9292
"NP": { "nearestRouteIds": ["india-se-asia"], "coastSide": "landlocked" },
93-
"HK": { "nearestRouteIds": ["china-us-west", "intra-asia-container"], "coastSide": "pacific" },
93+
"HK": { "nearestRouteIds": ["china-europe-suez", "asia-europe-cape", "china-us-west", "intra-asia-container"], "coastSide": "pacific" },
9494
"MO": { "nearestRouteIds": ["china-us-west"], "coastSide": "pacific" },
9595
"ZW": { "nearestRouteIds": ["brazil-china-bulk", "asia-europe-cape"], "coastSide": "landlocked" },
9696
"ZM": { "nearestRouteIds": ["brazil-china-bulk"], "coastSide": "landlocked" },

src/App.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,10 +1022,21 @@ export class App {
10221022
// Entitlement just resolved → fire PRO-gated initial loads that were
10231023
// skipped at boot. Each loader early-returns if the panel isn't
10241024
// mounted and re-checks hasPremiumAccess() internally, so these
1025-
// calls are safe and idempotent. Without this, trade-policy would
1026-
// sit empty for up to REFRESH_INTERVALS.tradePolicy (~10 min) after
1027-
// sign-in because the scheduler's viewport gate is the only retry.
1025+
// calls are safe and idempotent. Without this, panels would sit empty
1026+
// until the next scheduled refresh (10+ min for trade-policy; FOREVER
1027+
// on the full variant for stock-analysis / stock-backtest / daily-
1028+
// market-brief / market-implications because their schedulers are
1029+
// gated to SITE_VARIANT === 'finance'). The audit-locking regression
1030+
// test in tests/premium-loaders-fan-out-coverage.test.mts asserts
1031+
// every `hasPremiumAccess() && shouldLoad('X')` gate in data-loader.ts
1032+
// has a matching call here.
10281033
void this.dataLoader.loadTradePolicy();
1034+
void this.dataLoader.loadStockAnalysis();
1035+
void this.dataLoader.loadStockBacktest();
1036+
void this.dataLoader.loadDailyMarketBrief();
1037+
void this.dataLoader.loadMarketImplications();
1038+
void this.dataLoader.loadWsbTickers();
1039+
void this.dataLoader.loadResilienceRanking();
10291040
}
10301041
_prevHadPremium = nowPremium;
10311042
};
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
import { describe, it } from 'node:test';
2+
import assert from 'node:assert/strict';
3+
import { readFileSync } from 'node:fs';
4+
import { resolve } from 'node:path';
5+
6+
// REGRESSION GUARD for PR #3828 (Free→Pro hydration race).
7+
//
8+
// `App.ts:firePremiumLoaders` fans out PRO-gated loaders on the Free→Pro
9+
// entitlement transition. If a NEW loader is gated behind
10+
// `hasPremiumAccess() && shouldLoad('X')` in `data-loader.ts` but a
11+
// corresponding `this.dataLoader.loadX()` line is missing from
12+
// `firePremiumLoaders`, the panel sits empty for the WHOLE SESSION on any
13+
// variant where the scheduled refresh isn't registered (the stock-analysis /
14+
// stock-backtest / daily-market-brief / market-implications schedulers are
15+
// gated to SITE_VARIANT === 'finance').
16+
//
17+
// This test extracts the gated loader names from data-loader.ts and asserts
18+
// each appears in firePremiumLoaders. It's deliberately static-grep based so
19+
// it catches the omission at the moment of commit, without needing to wire
20+
// up the full App.ts runtime.
21+
22+
const REPO_ROOT = resolve(import.meta.dirname, '..');
23+
const APP_TS = readFileSync(resolve(REPO_ROOT, 'src/App.ts'), 'utf8');
24+
const DATA_LOADER_TS = readFileSync(resolve(REPO_ROOT, 'src/app/data-loader.ts'), 'utf8');
25+
26+
// Loaders we deliberately do NOT include in the fan-out, with rationale.
27+
// Add an entry here (not silently in source) if you skip a loader.
28+
const ALLOWLIST: Record<string, string> = {
29+
// None today. Format: `loaderName: 'why this is intentionally excluded'`.
30+
};
31+
32+
/** Pull every `this.loadX()` call that sits inside an `if (hasPremiumAccess(...))` gate. */
33+
function extractGatedLoaders(src: string): Set<string> {
34+
const loaders = new Set<string>();
35+
const callRe = /this\.load([A-Z][A-Za-z0-9]+)\(\)/g;
36+
const addCallsFromBlock = (block: string): void => {
37+
for (const c of block.matchAll(callRe)) loaders.add(`load${c[1]}`);
38+
};
39+
40+
// Three gate shapes appear in data-loader.ts and all three strand the
41+
// loader on boot if Pro hasn't hydrated yet:
42+
// (a) `if (hasPremiumAccess() && shouldLoad('X')) { tasks.push(...load...()) }`
43+
// (b) `if (hasPremiumAccess()) { await Promise.allSettled([this.load...()]) }`
44+
// (c) `if (hasPremiumAccess() && shouldLoad('X')) tasks.push(...);` ← single-line, no braces
45+
//
46+
// Regex with nested parens (a/c) and balanced braces (a/b) is fragile —
47+
// PR #3828 review caught the original regex only matching shape (b)
48+
// (the other loaders happened to appear via an init()-handler block that
49+
// ALSO matches shape (b), giving false confidence). Walk line-by-line
50+
// instead: find every line containing `hasPremiumAccess(`, then collect
51+
// calls from that line PLUS the subsequent block (braced or single-line).
52+
const lines = src.split('\n');
53+
for (let i = 0; i < lines.length; i++) {
54+
const line = lines[i]!;
55+
if (!/\bhasPremiumAccess\(/.test(line)) continue;
56+
if (!/^\s*(?:if|else if)\s*\(/.test(line)) continue; // only `if` gates, not call sites in other contexts
57+
58+
addCallsFromBlock(line); // single-line gates (shape c)
59+
60+
// If the gate ends with `{`, collect from following lines until matching `}`.
61+
if (/\{\s*$/.test(line)) {
62+
let depth = 1;
63+
for (let j = i + 1; j < lines.length && depth > 0; j++) {
64+
const inner = lines[j]!;
65+
addCallsFromBlock(inner);
66+
for (const ch of inner) {
67+
if (ch === '{') depth++;
68+
else if (ch === '}') depth--;
69+
if (depth === 0) break;
70+
}
71+
}
72+
}
73+
}
74+
return loaders;
75+
}
76+
77+
/** Pull every `void this.dataLoader.loadX()` call from inside the firePremiumLoaders block. */
78+
function extractFanOutLoaders(src: string): Set<string> {
79+
const start = src.indexOf('const firePremiumLoaders');
80+
assert.ok(start >= 0, 'could not locate firePremiumLoaders in App.ts — refactor would silently bypass this guard');
81+
// Match until the closing `};` of the function body. The function ends with `_prevHadPremium = nowPremium;\n };`.
82+
const end = src.indexOf('\n };', start);
83+
assert.ok(end > start, 'could not locate end of firePremiumLoaders block');
84+
const block = src.slice(start, end);
85+
86+
const loaders = new Set<string>();
87+
const re = /void\s+this\.dataLoader\.load([A-Z][A-Za-z0-9]+)\(\)/g;
88+
for (const match of block.matchAll(re)) {
89+
loaders.add(`load${match[1]}`);
90+
}
91+
return loaders;
92+
}
93+
94+
describe('firePremiumLoaders fan-out coverage', () => {
95+
const gatedLoaders = extractGatedLoaders(DATA_LOADER_TS);
96+
const fanOutLoaders = extractFanOutLoaders(APP_TS);
97+
98+
it('PR #3828 review fix: extracts loaders from single-line gates (no braces)', () => {
99+
// Synthetic fixture covering all three shapes the production regex must
100+
// handle. If a future "simplification" of the regex drops shape (c) again,
101+
// this test fails immediately with a clear message instead of giving a
102+
// false-positive pass on the production source.
103+
const fixture = `
104+
// (a) braced + viewport gate
105+
if (hasPremiumAccess() && shouldLoad('a-panel')) {
106+
tasks.push({ name: 'a', task: runGuarded('a', () => this.loadAaa()) });
107+
}
108+
// (b) braced + premium-only
109+
if (hasPremiumAccess()) {
110+
await Promise.allSettled([
111+
this.loadBbb(),
112+
this.loadCcc(),
113+
]);
114+
}
115+
// (c) single-line, NO braces — the shape #3828 review caught me missing
116+
if (hasPremiumAccess() && shouldLoad('d-panel')) tasks.push({ name: 'd', task: runGuarded('d', () => this.loadDdd()) });
117+
`;
118+
const extracted = extractGatedLoaders(fixture);
119+
assert.ok(extracted.has('loadAaa'), 'missed shape (a) braced + viewport gate');
120+
assert.ok(extracted.has('loadBbb'), 'missed shape (b) braced + premium-only [first call]');
121+
assert.ok(extracted.has('loadCcc'), 'missed shape (b) braced + premium-only [second call]');
122+
assert.ok(extracted.has('loadDdd'), 'missed shape (c) single-line gate — fan-out coverage would have a blind spot');
123+
});
124+
125+
it('extracts at least one PRO-gated loader from data-loader.ts (sanity)', () => {
126+
// If this fails, the regex stopped matching — likely because data-loader.ts
127+
// changed the gate shape (e.g. `hasPremiumAccess()` got replaced or moved).
128+
// Update extractGatedLoaders before bumping this test or you risk silently
129+
// turning off the coverage check.
130+
assert.ok(gatedLoaders.size > 0, `no PRO-gated loaders found via regex — gate-shape changed?`);
131+
});
132+
133+
it('extracts at least one fan-out loader from App.ts (sanity)', () => {
134+
assert.ok(fanOutLoaders.size > 0, 'firePremiumLoaders has no `void this.dataLoader.loadX()` calls — has the function been renamed?');
135+
});
136+
137+
it('every PRO-gated loader is fanned out on Free→Pro transition', () => {
138+
const missing: string[] = [];
139+
for (const loader of gatedLoaders) {
140+
if (fanOutLoaders.has(loader)) continue;
141+
if (loader in ALLOWLIST) continue;
142+
missing.push(loader);
143+
}
144+
assert.deepEqual(
145+
missing,
146+
[],
147+
`${missing.length} PRO-gated loader(s) in data-loader.ts are NOT re-fired by App.ts firePremiumLoaders on Free→Pro transition:\n` +
148+
missing.map((l) => ` - ${l}`).join('\n') +
149+
`\n\nFix: add \`void this.dataLoader.${missing[0] ?? 'loadX'}();\` to the firePremiumLoaders block in App.ts.\n` +
150+
`If you intentionally do NOT want this loader re-fired (e.g. it has its own entitlement subscription),\n` +
151+
`add it to the ALLOWLIST in this test with a one-line rationale.`,
152+
);
153+
});
154+
});

0 commit comments

Comments
 (0)