fix: error-overview sweep — TDZ import cycle, expired-session UX, Sentry double-reports, miner deadlock retry

memory/features/auth-onboarding-join.md

@@ -72,6 +72,7 @@ Join code
 - Server actions must call `supabase.auth.getUser()` before database access.
 - Join codes are team-scoped and should be treated case-insensitively where the route expects that behavior.
 - Incomplete player onboarding should redirect to player onboarding with join context preserved.
+- A session that expires mid-request on `/golf/dashboard` (passes the top-of-page check, fails the data-fetch re-validation with `Not authenticated`) redirects to `/golf/login?returnTo=/golf/dashboard` instead of hitting the error boundary. Retryable auth failures (network / GoTrue 5xx) still surface to the error boundary — only a genuinely missing/expired session redirects.
 - College/coach/player role rules must be respected before granting dashboard access.
 - Service-role logic must stay server-only and admin-bounded.
 

src/app/api/cron/coachhelm-roster-sweep/route.ts

@@ -14,6 +14,11 @@
  */
 import { NextResponse, type NextRequest } from 'next/server';
 import { createAdminClient } from '@/lib/supabase/admin';
+// Side-effect-only: guarantees insights.ts's module-scope registration
+// (__registerTriggerPlayerInsightsAfterRound) has run at module-init time —
+// the bridge no longer lazily imports insights.ts itself (that dynamic
+// back-edge was the cold-start TDZ cycle; see trigger-insights-bridge.ts).
+import '@/app/golf/actions/insights';
 import { triggerPlayerInsightsAfterRound } from '@/lib/coachhelm/v2/trigger-insights-bridge';
 import { logServerError } from '@/lib/server-error-logger';
 import { requireCronAuth } from '@/lib/cron/auth';

src/app/golf/(dashboard)/dashboard/page.tsx

@@ -25,6 +25,20 @@ export const dynamic = 'force-dynamic';
 
 const VALID_RANGES = new Set(['7d', '30d', '90d', 'season', 'all']);
 
+/**
+ * A session can pass the top-of-page check yet expire before the data fetch
+ * re-validates it (auth token refresh race — observed live as repeated
+ * 'Not authenticated' digests on /golf/dashboard). Send that narrow case to
+ * the login screen; anything else still surfaces to the route error boundary
+ * (P002/P426 — real outages must never be swallowed).
+ */
+function redirectToLoginOnExpiredSession(error: unknown): never {
+    if (error instanceof Error && error.message === 'Not authenticated') {
+        redirect('/golf/login?returnTo=/golf/dashboard');
+    }
+    throw error;
+}
+
 /**
  * Renders the coach dashboard inside the `.fairway-ds` scope on `bg-canvas`.
  * Fairway is the only tree (Wave W1) — the legacy CoachDashboard fork has
@@ -126,7 +140,7 @@ export default async function GolfDashboardPage({
             const [payload, joinRequestsResult] = await Promise.all([
                 getCachedCoachDashboardData(coach.id, userId, teamId, dateRange),
                 getTeamJoinRequests(),
-            ]);
+            ]).catch(redirectToLoginOnExpiredSession);
             const joinRequests: JoinRequestData[] =
                 joinRequestsResult.success && joinRequestsResult.data ? joinRequestsResult.data : [];
 
@@ -201,7 +215,7 @@ export default async function GolfDashboardPage({
         const [payload, hubData] = await Promise.all([
             getCachedPlayerDashboardData(player.id, userId, teamId),
             teamId ? getPlayerHubSummaryData(teamId, player.id) : Promise.resolve(null),
-        ]);
+        ]).catch(redirectToLoginOnExpiredSession);
         const nameParts = `${player.first_name} ${player.last_name}`.split(' ');
 
         const data: PlayerDashboardData = {

src/app/golf/actions/dashboard-data.ts

@@ -247,7 +247,15 @@ async function getCoachDashboardDataImpl(
             .maybeSingle(),
     ]);
 
-    const { data: { user } } = authResult;
+    const { data: { user }, error: authError } = authResult;
+    // A genuine auth outage (network failure / GoTrue 5xx — supabase-js marks
+    // these retryable) must surface to the route error boundary, not be
+    // misread as a signed-out user. AuthSessionMissingError and other
+    // no-session results fall through to 'Not authenticated', which the
+    // dashboard page maps to the login redirect.
+    if (authError && (authError.name === 'AuthRetryableFetchError' || (authError.status ?? 0) >= 500)) {
+        throw authError;
+    }
     if (!user) throw new Error('Not authenticated');
     if (user.id !== userId) throw new Error('Unauthorized');
 
@@ -755,7 +763,12 @@ async function getPlayerDashboardDataImpl(
             : Promise.resolve({ data: null }),
     ]);
 
-    const { data: { user } } = authResult;
+    const { data: { user }, error: authError } = authResult;
+    // Same outage-vs-expiry split as the coach payload above: retryable auth
+    // failures surface; a missing session becomes the login redirect.
+    if (authError && (authError.name === 'AuthRetryableFetchError' || (authError.status ?? 0) >= 500)) {
+        throw authError;
+    }
     if (!user) throw new Error('Not authenticated');
 
     const playerTeamTimezone = (playerTimezoneResult.data as { timezone?: string } | null)?.timezone || 'America/New_York';

src/instrumentation.ts

@@ -18,6 +18,21 @@ const sharedIgnoreErrors = [
   // suppressed; every other AuthApiError now reaches Sentry.
   /AuthApiError: Invalid Refresh Token/,
   /Refresh Token Not Found/,
+  // Baseball expected control-flow throws. withBaseballAction already
+  // classifies these as handled/expected (admin_events + Sentry warning with
+  // skipSentry) and then RE-RAISES so callers can branch — but the re-raise
+  // escapes the server-action boundary, where Next's own console.error +
+  // onRequestError capture it a second time as an unhandled Sentry Error
+  // (observed live: a logged-out tab's 60 s NotificationBell poll produced
+  // "BaseballUnauthorizedError: You must be signed in"). These stay fully
+  // visible in admin_events / Helm Bridge; only the duplicate Sentry Error
+  // is suppressed.
+  'BaseballUnauthorizedError',
+  'BaseballNoActiveTeamError',
+  'BaseballCapabilityError',
+  'BaseballDisabledSourceError',
+  'BaseballDemoReadOnlyError',
+  'PlayerAccessError',
 ];
 
 const scrubPii: Sentry.NodeOptions['beforeSend'] = (event) => {

src/lib/coachhelm/v2/mining/pattern-miner.ts

@@ -992,14 +992,31 @@ export class PatternMiner {
     const playerIds = [...new Set(patterns.map((p) => p.playerId).filter(Boolean))];
     if (ids.length > 0 && playerIds.length > 0) {
       const idList = `(${ids.map((id) => `"${id}"`).join(',')})`;
-      const { error: supersedeError } = await fromUntyped(supabase, 'golf_patterns_v2')
-        .update({ is_active: false, updated_at: new Date().toISOString() })
-        .in('player_id', playerIds)
-        .eq('is_active', true)
-        .not('id', 'in', idList)
-        // Keep coach-curated patterns visible; only retire auto-detected ones
-        // (NULL or non-preserved lifecycle_state).
-        .or('lifecycle_state.is.null,lifecycle_state.not.in.(confirmed,addressed,resolved,dismissed)');
+      // Multi-row UPDATE racing a concurrent mine's upserts/supersede over the
+      // same players can deadlock (observed live: 40P01 during the roster-sweep
+      // cron, round-submit trigger and cron batch mining the same team
+      // concurrently). Postgres aborts one victim wholesale and the supersede
+      // is idempotent (deterministic ids, converges on re-run), so a short
+      // retry absorbs it instead of surfacing a transient as an error.
+      let supersedeError: { code?: string } | null = null;
+      for (let attempt = 0; attempt < 3; attempt++) {
+        if (attempt > 0) {
+          // Jitter decorrelates retries from the concurrent miner we
+          // deadlocked with — fixed delays would re-collide in lock-step.
+          const backoff = 200 * attempt + Math.floor(Math.random() * 150);
+          await new Promise((resolve) => setTimeout(resolve, backoff));
+        }
+        const { error } = await fromUntyped(supabase, 'golf_patterns_v2')
+          .update({ is_active: false, updated_at: new Date().toISOString() })
+          .in('player_id', playerIds)
+          .eq('is_active', true)
+          .not('id', 'in', idList)
+          // Keep coach-curated patterns visible; only retire auto-detected ones
+          // (NULL or non-preserved lifecycle_state).
+          .or('lifecycle_state.is.null,lifecycle_state.not.in.(confirmed,addressed,resolved,dismissed)');
+        supersedeError = error;
+        if (!error || error.code !== '40P01') break;
+      }
       if (supersedeError) {
         await logServerError('pattern-miner.savePatterns supersede stale', {
           action: 'pattern-miner.savePatterns',

src/lib/coachhelm/v2/post-round-trigger.ts

@@ -17,6 +17,12 @@
  * NOT a replacement for triggerPlayerInsightsAfterRound — wraps it.
  */
 import type { SupabaseClient } from '@supabase/supabase-js';
+// Side-effect-only: guarantees insights.ts's module-scope registration
+// (__registerTriggerPlayerInsightsAfterRound) has run before this module's
+// own top-level finishes — at synchronous module-init time, so there is no
+// async gap for a concurrent request on the same warm instance to race
+// against (the cold-start TDZ crash the bridge header documents).
+import '@/app/golf/actions/insights';
 import { triggerPlayerInsightsAfterRound } from '@/lib/coachhelm/v2/trigger-insights-bridge';
 import { logServerError, logServerEvent } from '@/lib/server-error-logger';
 import {

src/lib/coachhelm/v2/trigger-insights-bridge.ts

@@ -33,12 +33,21 @@ import 'server-only';
  *
  * Wiring: insights.ts calls `__registerTriggerPlayerInsightsAfterRound(...)`
  * at its own module scope (a side effect, not an export) to hand this
- * bridge a reference to the withAdminObserved-wrapped impl.
- * `triggerPlayerInsightsAfterRound` below lazily triggers that registration
- * (via a dynamic import of insights.ts) the first time it's called in a
- * given process, then delegates to the registered function on every call —
- * same object, same behavior, just never exported from a 'use server'
- * module. The one legitimate CLIENT-reachable path (a coach manually
+ * bridge a reference to the withAdminObserved-wrapped impl. Each legitimate
+ * caller (post-round-trigger.ts, the roster-sweep cron route) carries a
+ * side-effect `import '@/app/golf/actions/insights'` so that registration
+ * has ALREADY run, synchronously, by the time the caller's own module
+ * finishes initializing. This used to be a lazy `await import()` inside
+ * `triggerPlayerInsightsAfterRound` below — but that made this file the
+ * dynamic back-edge of a value-level import cycle with insights.ts
+ * (insights.ts statically imports the register function above), and on a
+ * cold Fluid Compute instance two concurrent first-touches of the pair
+ * (a background after() trigger racing a dashboard server action) could
+ * observe insights.ts mid-evaluation — the intermittent cold-start
+ * "Cannot access 'a' before initialization" TDZ crash in generateAlerts.
+ * `triggerPlayerInsightsAfterRound` now only delegates to the registered
+ * function — same object, same behavior, just never exported from a
+ * 'use server' module. The one legitimate CLIENT-reachable path (a coach manually
  * refreshing a player's analysis) is unaffected — it goes through the
  * already-authed sibling action `refreshPlayerAnalysisAsCoach`, which is
  * still exported from insights.ts and calls the private impl directly
@@ -90,15 +99,11 @@ export async function triggerPlayerInsightsAfterRound(
   partial?: boolean;
   code?: 'engine_no_recent_rounds' | 'engine_session_expired';
 }> {
-  if (!impl) {
-    // Load insights.ts so its registration side effect runs. Cheap after the
-    // first call — subsequent imports hit the module cache.
-    await import('@/app/golf/actions/insights');
-  }
   if (!impl) {
     throw new Error(
       '[trigger-insights-bridge] triggerPlayerInsightsAfterRound not registered — ' +
-        'insights.ts failed to load or its registration call was removed',
+        "the caller is missing its side-effect import of '@/app/golf/actions/insights' " +
+        '(see the wiring note in this file header), or the registration call was removed',
     );
   }
   return impl(playerId);

src/test/coachhelm/v2/post-round-trigger.test.ts

@@ -9,6 +9,12 @@ vi.mock('@/lib/coachhelm/v2/trigger-insights-bridge', () => ({
   triggerPlayerInsightsAfterRound: (...args: unknown[]) => mockTrigger(...args),
 }));
 
+// post-round-trigger.ts carries a side-effect `import '@/app/golf/actions/insights'`
+// (registers the bridge impl at module-init time — the fix for the cold-start
+// TDZ cycle). The bridge is fully mocked above, so the registration side effect
+// is irrelevant here — stub the whole 4,400-line 'use server' module out.
+vi.mock('@/app/golf/actions/insights', () => ({}));
+
 // Spy on the actual severity/skipSentry passed to the two logging entry
 // points. postRoundTrigger is a SECOND independent consumer of the
 // triggerPlayerInsightsAfterRound result (alongside the withAdminObserved