Error sweep: APNs token pruning, honest severities, unauth redirects, baseball RLS-denial parity

.github/scripts/changed-files.sh

@@ -12,5 +12,13 @@ fi
 if [ -z "$base_sha" ]; then
   git ls-files -- "$@"
 else
-  git diff --name-only --diff-filter=ACMRT "$base_sha" "$head_sha" -- "$@"
+  # BASE..HEAD is a two-dot TREE diff, but consumers scan the PR's *merge-ref
+  # checkout*. A branch that lags main's deletions therefore gets paths listed
+  # (present on the branch, deleted on main) that don't exist on disk — and
+  # scanners like semgrep hard-fail on nonexistent roots ("Invalid scanning
+  # root"). Keep only paths present in the checkout.
+  git diff --name-only --diff-filter=ACMRT "$base_sha" "$head_sha" -- "$@" \
+    | while IFS= read -r f; do
+        if [ -e "$f" ]; then printf '%s\n' "$f"; fi
+      done
 fi

src/app/actions/messages.ts

@@ -14,6 +14,7 @@ import { notifyNewMessage } from '@/lib/notifications';
 import { sendPushNotification } from '@/lib/notifications/push';
 import { createAdminClient } from '@/lib/supabase/admin';
 import { logServerError } from '@/lib/server-error-logger';
+import { maybeCaptureRlsDenial } from '@/lib/admin/rls-denial';
 import { resolveCoachTeamIdWithCookie } from '@/lib/golf/resolve-team-server';
 import { getCoachTeamSwitchContext } from '@/lib/golf/resolve-team';
 import { withAdminObserved } from '@/lib/admin/observed-action';
@@ -74,6 +75,7 @@ async function isBaseballMessageNotificationEnabled(conversationId: string): Pro
  * @param sport - The sport context (for revalidation paths)
  * @param createNotifications - Whether to create notifications for other participants (default: true)
  */
+// nosemgrep: helmv3-action-missing-revalidate -- realtime-subscribed messages UI; revalidatePath caused a full reload on every send (see note above the return)
 export async function sendMessage({
   conversationId,
   content,
@@ -148,6 +150,14 @@ export async function sendMessage({
           hint: messageError.hint,
         },
       });
+      maybeCaptureRlsDenial(messageError, {
+        table: messagesTable,
+        verb: 'insert',
+        action: 'messages.sendMessage',
+        feature: sport === 'golf' ? 'messaging' : 'baseball_messages',
+        sport,
+        userId: user.id,
+      });
       throw new Error(`Failed to send message: ${messageError.message}`);
     }
 
@@ -205,7 +215,7 @@ export async function sendMessage({
 
     // NOTE: Removed revalidatePath calls - messages page uses real-time subscriptions
     // Revalidation was causing unnecessary page reloads on every message send
-    // SEMGREP-ALLOW: realtime-subscribed messages UI; revalidate would cause reload loop
+    // (the nosemgrep suppression for this lives on the function declaration)
 
     return { success: true };
   } catch (err) {
@@ -305,6 +315,14 @@ export async function createConversation({
         insertData,
       },
     });
+    maybeCaptureRlsDenial(convError, {
+      table: conversationsTable,
+      verb: 'insert',
+      action: 'messages.createConversation',
+      feature: sport === 'golf' ? 'messaging' : 'baseball_messages',
+      sport,
+      userId: user.id,
+    });
     throw new Error(`Failed to create conversation: ${convError?.message || 'Unknown error'}`);
   }
 
@@ -332,6 +350,14 @@ export async function createConversation({
         userId: user.id,
       },
     });
+    maybeCaptureRlsDenial(participantsError, {
+      table: participantsTable,
+      verb: 'insert',
+      action: 'messages.createConversation',
+      feature: sport === 'golf' ? 'messaging' : 'baseball_messages',
+      sport,
+      userId: user.id,
+    });
     await supabase.from(conversationsTable as any).delete().eq('id', conversationId);
     throw new Error(`Failed to add participants: ${participantsError.message}`);
   }
@@ -375,6 +401,14 @@ export async function markMessagesAsRead({
 
   if (participantError) {
     await logServerError(`[Messages] Failed to update last_read_at: ${participantError instanceof Error ? participantError.message : String(participantError)}`, { action: 'messages.markMessagesAsRead' });
+    maybeCaptureRlsDenial(participantError, {
+      table: participantsTable,
+      verb: 'update',
+      action: 'messages.markMessagesAsRead',
+      feature: sport === 'golf' ? 'messaging' : 'baseball_messages',
+      sport,
+      userId: user.id,
+    });
     throw new Error('Failed to mark messages as read');
   }
 
@@ -393,6 +427,14 @@ export async function markMessagesAsRead({
 
   if (messagesError) {
     await logServerError(`[Messages] Failed to mark messages as read: ${messagesError instanceof Error ? messagesError.message : String(messagesError)}`, { action: 'messages.markMessagesAsRead' });
+    maybeCaptureRlsDenial(messagesError, {
+      table: messagesTable,
+      verb: 'update',
+      action: 'messages.markMessagesAsRead',
+      feature: sport === 'golf' ? 'messaging' : 'baseball_messages',
+      sport,
+      userId: user.id,
+    });
   }
 
   revalidatePath(`/${sport}/dashboard/messages/${conversationId}`);

src/app/baseball/(dashboard)/dashboard/decision-room/__tests__/page.test.tsx

@@ -0,0 +1,91 @@
+// =============================================================================
+// DecisionRoomPage — BaseballUnauthorizedError must redirect, not raw-throw.
+//
+// getDecisionRoomData (withBaseballAction) independently re-resolves auth, so
+// a session that expires between the page's own supabase.auth.getUser() check
+// and this call throws BaseballUnauthorizedError. Before this fix, that error
+// propagated straight out of the Server Component render to error.tsx and the
+// error tracker (Sentry/Vercel) — the same class of bug fixed on the
+// scout-packet/preview and scout-packets pages. This test pins the fix: the
+// unauthorized case now redirects to /baseball/login (preserving returnTo),
+// while any OTHER thrown error (a real failure for a signed-in, authorized
+// coach) still propagates so error.tsx keeps handling genuine failures.
+// =============================================================================
+
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+const mocks = vi.hoisted(() => ({
+  getUser: vi.fn(async () => ({ data: { user: { id: 'coach-1' } } })),
+  getDecisionRoomData: vi.fn(),
+  redirect: vi.fn((path: string) => {
+    throw new Error(`REDIRECT:${path}`);
+  }),
+}));
+
+vi.mock('next/navigation', () => ({ redirect: mocks.redirect }));
+
+vi.mock('@/lib/supabase/server', () => ({
+  createClient: vi.fn(async () => ({
+    auth: { getUser: mocks.getUser },
+  })),
+}));
+
+vi.mock('@/app/baseball/actions/decision-room', () => ({
+  getDecisionRoomData: mocks.getDecisionRoomData,
+}));
+
+// Mocked locally (not the real with-baseball-action module) so the page's
+// `instanceof BaseballUnauthorizedError` check runs against the SAME class
+// reference this test constructs errors with — no need to pull in the real
+// module's Sentry/Supabase/capability dependency graph just to reach one
+// error class. Defined inside vi.hoisted since vi.mock factories are hoisted
+// above normal top-level declarations.
+const { FakeBaseballUnauthorizedError } = vi.hoisted(() => ({
+  FakeBaseballUnauthorizedError: class FakeBaseballUnauthorizedError extends Error {
+    readonly status = 401;
+    constructor(message = 'You must be signed in.') {
+      super(message);
+      this.name = 'BaseballUnauthorizedError';
+    }
+  },
+}));
+vi.mock('@/lib/baseball/with-baseball-action', () => ({
+  BaseballUnauthorizedError: FakeBaseballUnauthorizedError,
+}));
+
+vi.mock('@/components/baseball/staff-decision-room/StaffDecisionRoomClient', () => ({
+  StaffDecisionRoomClient: () => null,
+}));
+
+import DecisionRoomPage from '../page';
+
+describe('DecisionRoomPage — BaseballUnauthorizedError redirects instead of raw-throwing', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mocks.getUser.mockResolvedValue({ data: { user: { id: 'coach-1' } } });
+  });
+
+  it('redirects to /baseball/login (with returnTo) when the session expired between the auth check and the data fetch', async () => {
+    mocks.getDecisionRoomData.mockRejectedValue(new FakeBaseballUnauthorizedError());
+
+    await expect(DecisionRoomPage()).rejects.toThrow(
+      'REDIRECT:/baseball/login?returnTo=' +
+        encodeURIComponent('/baseball/dashboard/decision-room'),
+    );
+  });
+
+  it('re-throws any OTHER error (a real failure for a signed-in, authorized coach) instead of redirecting', async () => {
+    mocks.getDecisionRoomData.mockRejectedValue(new Error('decision room query failed'));
+
+    await expect(DecisionRoomPage()).rejects.toThrow('decision room query failed');
+    expect(mocks.redirect).not.toHaveBeenCalled();
+  });
+
+  it('renders normally when the data fetch succeeds', async () => {
+    mocks.getDecisionRoomData.mockResolvedValue({ items: [] });
+
+    const element = await DecisionRoomPage();
+    expect(element).toBeTruthy();
+    expect(mocks.redirect).not.toHaveBeenCalled();
+  });
+});

src/app/baseball/(dashboard)/dashboard/decision-room/page.tsx

@@ -29,6 +29,8 @@ import { redirect } from 'next/navigation';
 
 import { createClient } from '@/lib/supabase/server';
 import { getDecisionRoomData } from '@/app/baseball/actions/decision-room';
+import { BaseballUnauthorizedError } from '@/lib/baseball/with-baseball-action';
+import { redirectOnUnauthorized } from '@/lib/baseball/redirect-on-unauthorized';
 import { StaffDecisionRoomClient } from '@/components/baseball/staff-decision-room/StaffDecisionRoomClient';
 import { fairwayScope } from '@/lib/redesign/flag';
 
@@ -50,9 +52,17 @@ export default async function DecisionRoomPage() {
   }
 
   // getDecisionRoomData resolves the active team + viewer capability and
-  // enforces auth/context server-side. Any failure (no active team, no coach
-  // role, missing capability) surfaces through error.tsx.
-  const data = await getDecisionRoomData();
+  // enforces auth/context server-side, independently re-resolving auth
+  // (withBaseballAction). A session that expires in the narrow window between
+  // the check above and this call throws BaseballUnauthorizedError, which
+  // must redirect to login rather than raw-throw to error.tsx/Sentry. Any
+  // OTHER failure (no active team, no coach role, missing capability) is a
+  // genuine failure and keeps propagating to error.tsx.
+  const data = await redirectOnUnauthorized(
+    () => getDecisionRoomData(),
+    (error) => error instanceof BaseballUnauthorizedError,
+    '/baseball/dashboard/decision-room',
+  );
 
   return (
     <div className={fairwayScope('min-h-full')}>

src/app/baseball/(dashboard)/dashboard/operations/error.tsx

@@ -0,0 +1,23 @@
+'use client';
+
+import { RouteErrorBoundary } from '@/components/errors';
+
+export default function Error({
+  error,
+  reset,
+}: {
+  error: Error & { digest?: string };
+  reset: () => void;
+}) {
+  return (
+    <RouteErrorBoundary
+      error={error}
+      reset={reset}
+      route="/baseball/dashboard/operations"
+      component="OperationsPage"
+      title="Couldn't load Operations"
+      message="We couldn't load the team logistics hub. Please try again."
+      homePath="/baseball/dashboard/roster"
+    />
+  );
+}

src/app/baseball/(dashboard)/dashboard/performance/builder/error.tsx

@@ -0,0 +1,23 @@
+'use client';
+
+import { RouteErrorBoundary } from '@/components/errors';
+
+export default function Error({
+  error,
+  reset,
+}: {
+  error: Error & { digest?: string };
+  reset: () => void;
+}) {
+  return (
+    <RouteErrorBoundary
+      error={error}
+      reset={reset}
+      route="/baseball/dashboard/performance/builder"
+      component="LiftBuilderPage"
+      title="Couldn't load the Lift Builder"
+      message="We couldn't load the lift planning tools. Please try again."
+      homePath="/baseball/dashboard/performance"
+    />
+  );
+}

src/app/baseball/(dashboard)/dashboard/players/[id]/scout-packet/preview/__tests__/page.test.tsx

@@ -0,0 +1,108 @@
+// =============================================================================
+// CoachScoutPacketPreviewPage — BaseballUnauthorizedError must redirect, not
+// raw-throw.
+//
+// getScoutPacketPreview (withBaseballAction) independently re-resolves auth,
+// so a session that expires between the page's own getActiveBaseballContext()
+// check and this call throws BaseballUnauthorizedError. Before this fix, that
+// error propagated straight out of the Server Component render to error.tsx
+// and the error tracker (Sentry/Vercel) — the same class of bug fixed on the
+// baseball announcements page (ROOT CAUSE 3). This test pins the fix: the
+// unauthenticated case now redirects to /baseball/login (preserving
+// returnTo), while any OTHER thrown error (a real failure for a signed-in,
+// authorized coach) still propagates so error.tsx keeps handling genuine
+// failures.
+// =============================================================================
+
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+const mocks = vi.hoisted(() => ({
+  getActiveBaseballContext: vi.fn(),
+  getScoutPacketPreview: vi.fn(),
+  redirect: vi.fn((path: string) => {
+    throw new Error(`REDIRECT:${path}`);
+  }),
+  notFound: vi.fn(() => {
+    throw new Error('NOT_FOUND');
+  }),
+}));
+
+vi.mock('next/navigation', () => ({
+  redirect: mocks.redirect,
+  notFound: mocks.notFound,
+}));
+
+vi.mock('@/lib/baseball/active-context', () => ({
+  getActiveBaseballContext: mocks.getActiveBaseballContext,
+}));
+
+vi.mock('@/app/baseball/actions/scout-packet', () => ({
+  getScoutPacketPreview: mocks.getScoutPacketPreview,
+}));
+
+// Mocked locally (not the real with-baseball-action module) so the page's
+// `instanceof BaseballUnauthorizedError` check runs against the SAME class
+// reference this test constructs errors with — no need to pull in the real
+// module's Sentry/Supabase/capability dependency graph just to reach one
+// error class. Defined inside vi.hoisted since vi.mock factories are hoisted
+// above normal top-level declarations.
+const { FakeBaseballUnauthorizedError } = vi.hoisted(() => ({
+  FakeBaseballUnauthorizedError: class FakeBaseballUnauthorizedError extends Error {
+    readonly status = 401;
+    constructor(message = 'You must be signed in.') {
+      super(message);
+      this.name = 'BaseballUnauthorizedError';
+    }
+  },
+}));
+vi.mock('@/lib/baseball/with-baseball-action', () => ({
+  BaseballUnauthorizedError: FakeBaseballUnauthorizedError,
+}));
+
+vi.mock('@/components/baseball/passport/ScoutPacketView', () => ({
+  ScoutPacketView: () => null,
+}));
+
+import CoachScoutPacketPreviewPage from '../page';
+
+const COACH_CONTEXT = {
+  activeTeamId: 'team-1',
+  activeRole: 'coach' as const,
+  activeCoachId: 'coach-1',
+  activePlayerId: null,
+};
+
+describe('CoachScoutPacketPreviewPage — BaseballUnauthorizedError redirects instead of raw-throwing', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mocks.getActiveBaseballContext.mockResolvedValue(COACH_CONTEXT);
+  });
+
+  it('redirects to /baseball/login (with returnTo) when the session expired between the context check and the preview fetch', async () => {
+    mocks.getScoutPacketPreview.mockRejectedValue(new FakeBaseballUnauthorizedError());
+
+    await expect(CoachScoutPacketPreviewPage({ params: Promise.resolve({ id: 'player-1' }) })).rejects.toThrow(
+      'REDIRECT:/baseball/login?returnTo=' +
+        encodeURIComponent('/baseball/dashboard/players/player-1/scout-packet/preview'),
+    );
+  });
+
+  it('re-throws any OTHER error (a real failure for a signed-in, authorized coach) instead of redirecting', async () => {
+    mocks.getScoutPacketPreview.mockRejectedValue(new Error('assembly failed'));
+
+    await expect(
+      CoachScoutPacketPreviewPage({ params: Promise.resolve({ id: 'player-1' }) }),
+    ).rejects.toThrow('assembly failed');
+    expect(mocks.redirect).not.toHaveBeenCalled();
+  });
+
+  it('renders normally when the preview fetch succeeds', async () => {
+    mocks.getScoutPacketPreview.mockResolvedValue({ ok: true });
+
+    const element = await CoachScoutPacketPreviewPage({
+      params: Promise.resolve({ id: 'player-1' }),
+    });
+    expect(element).toBeTruthy();
+    expect(mocks.redirect).not.toHaveBeenCalled();
+  });
+});