perf(timeline): the reader-id fix shipped in a function the page never calls

__tests__/unit/services/current-user-id-cache.test.ts

@@ -12,12 +12,23 @@
  * onto one request rather than racing to make several.
  */
 
-import { getCurrentUserId, __resetCurrentUserIdCache } from '@/services/timeline/processors/social-shared';
+import {
+  getCurrentUserId,
+  __resetCurrentUserIdCache,
+} from '@/services/supabase/auth/session';
+// The timeline re-exports it rather than defining its own; both names must be
+// the same function, or the cache would only cover half the callers.
+import { getCurrentUserId as timelineGetCurrentUserId } from '@/services/timeline/processors/social-shared';
 
 const getUser = jest.fn();
 jest.mock('@/lib/supabase/browser', () => ({
   __esModule: true,
-  default: { auth: { getUser: (...a: unknown[]) => getUser(...a) } },
+  default: {
+    auth: {
+      getUser: (...a: unknown[]) => getUser(...a),
+      onAuthStateChange: () => ({ data: { subscription: { unsubscribe: () => {} } } }),
+    },
+  },
 }));
 
 describe('getCurrentUserId', () => {
@@ -46,18 +57,43 @@ describe('getCurrentUserId', () => {
     expect(getUser).toHaveBeenCalledTimes(1);
   });
 
-  it('does not cache a failure as "signed out"', async () => {
+  it('does not cache a thrown failure as "signed out"', async () => {
     __resetCurrentUserIdCache();
     getUser.mockRejectedValueOnce(new Error('network blip'));
 
     expect(await getCurrentUserId()).toBeNull();
 
-    // A blip must not outlive itself and render the whole timeline as
-    // signed-out for the rest of the page.
+    // A blip must not outlive itself and render the whole app as signed-out
+    // for the rest of the page.
     getUser.mockResolvedValue({ data: { user: { id: 'u1' } } });
     expect(await getCurrentUserId()).toBe('u1');
   });
 
+  it('does not cache a REPORTED failure as "signed out" either', async () => {
+    // The auth layer catches its own errors and reports them in `error` rather
+    // than throwing, so "could not ask" arrives looking almost exactly like
+    // "nobody is signed in". Only the error field tells them apart.
+    __resetCurrentUserIdCache();
+    getUser.mockResolvedValueOnce({
+      data: { user: null },
+      error: { message: 'fetch failed' },
+    });
+
+    expect(await getCurrentUserId()).toBeNull();
+
+    getUser.mockResolvedValue({ data: { user: { id: 'u1' } } });
+    expect(await getCurrentUserId()).toBe('u1');
+  });
+
+  it('is the SAME function the timeline uses, so one cache covers both', async () => {
+    expect(timelineGetCurrentUserId).toBe(getCurrentUserId);
+
+    await getCurrentUserId();
+    await timelineGetCurrentUserId();
+
+    expect(getUser).toHaveBeenCalledTimes(1);
+  });
+
   it('caches a genuine signed-out answer', async () => {
     __resetCurrentUserIdCache();
     getUser.mockResolvedValue({ data: { user: null } });

__tests__/unit/services/timeline-warms-reader.test.ts

@@ -0,0 +1,92 @@
+/**
+ * Every timeline read learns who is reading BEFORE it asks for the posts.
+ *
+ * Enrichment needs the reader's id to mark which posts they already reacted
+ * to, and that id is a round-trip to /auth/v1/user. It used to be requested
+ * only once the feed came back, with the three reaction queries then queued
+ * behind it — three serial waves where two could overlap. Measured on a cold
+ * timeline load: feed 3147-3520ms, then /auth/v1/user 3552-3784, then
+ * reactions 3811-4076.
+ *
+ * The ordering is the whole point, so that is what these assert. A test that
+ * only checked "the warm happens" would pass with the call left at the end,
+ * which is the bug.
+ *
+ * The first version of this fix warmed inside getUserFeed — a function the
+ * timeline page does not call. It went to production doing nothing. Hence the
+ * table below: every read the facade exposes, not the one I happened to open.
+ */
+
+const order: string[] = [];
+
+const warmCurrentUserId = jest.fn(() => {
+  order.push('warm');
+});
+
+jest.mock('@/services/supabase/auth/session', () => ({
+  warmCurrentUserId: () => warmCurrentUserId(),
+  getCurrentUserId: jest.fn(async () => 'u1'),
+}));
+
+const record = (name: string) =>
+  jest.fn(async () => {
+    order.push(name);
+    return { success: true, events: [], posts: [], replies: [], pagination: {}, total: 0 };
+  });
+
+const queries = {
+  getUserFeed: record('getUserFeed'),
+  getProjectFeed: record('getProjectFeed'),
+  getProfileFeed: record('getProfileFeed'),
+  getFollowedUsersFeed: record('getFollowedUsersFeed'),
+  getCommunityFeed: record('getCommunityFeed'),
+  getEnrichedUserFeed: record('getEnrichedUserFeed'),
+  getEnrichedFollowingFeed: record('getEnrichedFollowingFeed'),
+  getEventById: record('getEventById'),
+  getReplies: record('getReplies'),
+  searchPosts: record('searchPosts'),
+  getThreadPosts: record('getThreadPosts'),
+};
+
+jest.mock('@/services/timeline/queries', () => queries);
+
+import { timelineService } from '@/services/timeline';
+
+/** Each read, and how the facade exposes it. */
+const READS: Array<[keyof typeof queries, () => Promise<unknown>]> = [
+  ['getUserFeed', () => timelineService.getUserFeed('u1')],
+  ['getProjectFeed', () => timelineService.getProjectFeed('p1')],
+  ['getProfileFeed', () => timelineService.getProfileFeed('pr1')],
+  ['getFollowedUsersFeed', () => timelineService.getFollowedUsersFeed('u1')],
+  ['getCommunityFeed', () => timelineService.getCommunityFeed()],
+  ['getEnrichedUserFeed', () => timelineService.getEnrichedUserFeed('u1')],
+  ['getEnrichedFollowingFeed', () => timelineService.getEnrichedFollowingFeed('u1')],
+  ['getEventById', () => timelineService.getEventById('e1')],
+  ['getReplies', () => timelineService.getReplies('e1')],
+  ['searchPosts', () => timelineService.searchPosts('cat')],
+  ['getThreadPosts', () => timelineService.getThreadPosts('t1')],
+];
+
+describe('timeline reads warm the reader id first', () => {
+  beforeEach(() => {
+    order.length = 0;
+    warmCurrentUserId.mockClear();
+  });
+
+  it.each(READS)('%s warms before it queries', async (name, call) => {
+    await call();
+
+    expect(order).toEqual(['warm', name]);
+  });
+
+  it('covers every read the facade exposes', () => {
+    // If a new read is added to the service without warming, this catches it
+    // even when nobody thinks to add a case above.
+    const exposed = Object.getOwnPropertyNames(
+      Object.getPrototypeOf(timelineService)
+    ).filter(m => /^(get|search)/.test(m) && m !== 'getEventCounts' && m !== 'getEventComments' && m !== 'getCommentReplies');
+    const covered = new Set(READS.map(([name]) => name as string));
+
+    expect([...exposed].filter(m => !covered.has(m))).toEqual([]);
+  });
+});

package.json

@@ -37,10 +37,11 @@
     "check:schema-columns": "node scripts/check-schema-columns.mjs",
     "check:currency-units": "node scripts/check-currency-units.mjs",
     "check:rpc-exists": "node scripts/check-rpc-exists.mjs",
+    "check:one-current-user": "node scripts/check-one-current-user.mjs",
     "check:client-ip": "node scripts/check-client-ip.mjs",
     "check:ai-models": "node scripts/check-ai-models.mjs",
     "check:mdx": "node scripts/check-mdx.mjs",
-    "verify": "npm run ci:docs && npm run check:accent-ink && npm run type-check && npm run type-check:scripts && npm run check:sizes && npm run audit:routes && npm run lint && npm run check:duplication && npm run check:dead-fields && npm run check:migration-versions && npm run check:schema-columns && npm run check:currency-units && npm run check:rpc-exists && npm run check:client-ip && npm run check:mdx && npm run test:unit -- --watchAll=false",
+    "verify": "npm run ci:docs && npm run check:accent-ink && npm run type-check && npm run type-check:scripts && npm run check:sizes && npm run audit:routes && npm run lint && npm run check:duplication && npm run check:dead-fields && npm run check:migration-versions && npm run check:schema-columns && npm run check:currency-units && npm run check:rpc-exists && npm run check:one-current-user && npm run check:client-ip && npm run check:mdx && npm run test:unit -- --watchAll=false",
     "audit:schema": "node scripts/db/audit-schema-drift.mjs",
     "audit:routes": "node scripts/audit-routes.mjs",
     "gen:types": "bash scripts/db/gen-types.sh",

scripts/check-one-current-user.mjs

@@ -0,0 +1,72 @@
+#!/usr/bin/env node
+/**
+ * "Who is reading?" gets exactly one answer, from one place.
+ *
+ * Six copies of `getCurrentUserId` had accumulated across services — timeline
+ * queries, timeline processors, groups, loans, projects, and the auth layer —
+ * each one an uncached network call to /auth/v1/user. A single timeline load
+ * asked the server who the reader was repeatedly, and on the critical path:
+ * the lookup ran after the feed returned and gated the reaction queries.
+ *
+ * They were collapsed onto src/services/supabase/auth/session.ts, which caches
+ * the in-flight promise. This gate exists so the seventh copy is a red build
+ * rather than another silent round-trip: a duplicate is easy to reintroduce,
+ * costs nothing visible, and nothing else would ever catch it.
+ *
+ * Definitions are what this counts. Re-exports (`export { getCurrentUserId }
+ * from ...`) are the intended way to expose it from a service's own module.
+ */
+
+import { readFileSync } from 'node:fs';
+import { execSync } from 'node:child_process';
+
+const OWNER = 'src/services/supabase/auth/session.ts';
+
+/**
+ * A caller may legitimately pass its own Supabase client — a per-request server
+ * client, where a module-level cache would hand one request's user to the next.
+ * Those wrappers take a client parameter and delegate when there isn't one.
+ */
+const DELEGATING_WRAPPERS = new Set(['src/services/groups/utils/helpers.ts']);
+
+const files = execSync('git ls-files "src/**/*.ts" "src/**/*.tsx"', { encoding: 'utf8' })
+  .split('\n')
+  .filter(Boolean);
+
+const defines = [];
+for (const file of files) {
+  const body = readFileSync(file, 'utf8');
+  // A definition, not a re-export or an import.
+  if (/export\s+(async\s+)?function\s+getCurrentUserId\b/.test(body)) {
+    defines.push(file);
+  }
+}
+
+const unexpected = defines.filter(f => f !== OWNER && !DELEGATING_WRAPPERS.has(f));
+
+if (!defines.includes(OWNER)) {
+  console.error(`✗ ${OWNER} no longer defines getCurrentUserId.`);
+  console.error('  That file is the single definition. Move it back, or update this gate');
+  console.error('  deliberately — do not let the definition drift somewhere unnoticed.');
+  process.exit(1);
+}
+
+if (unexpected.length > 0) {
+  console.error('✗ getCurrentUserId is defined in more than one place:');
+  for (const f of unexpected) {
+    console.error(`  - ${f}`);
+  }
+  console.error('');
+  console.error(`Import or re-export it from ${OWNER} instead:`);
+  console.error("  export { getCurrentUserId } from '@/services/supabase/auth/session';");
+  console.error('');
+  console.error('Every copy is an uncached round-trip to /auth/v1/user. Six of them once');
+  console.error('shared one page load. If your caller must pass its own Supabase client,');
+  console.error('add it to DELEGATING_WRAPPERS in this script and delegate when it has none.');
+  process.exit(1);
+}
+
+console.log(
+  `check:one-current-user passed — getCurrentUserId defined once (${OWNER})` +
+    (DELEGATING_WRAPPERS.size ? `, ${DELEGATING_WRAPPERS.size} delegating wrapper(s)` : '')
+);

src/services/groups/utils/helpers.ts

@@ -13,17 +13,25 @@ import { logger } from '@/utils/logger';
 import { DATABASE_TABLES } from '@/config/database-tables';
 import { slugify } from '@/utils/string';
 import type { AnySupabaseClient } from '@/lib/supabase/types';
+import { getCurrentUserId as getSharedCurrentUserId } from '@/services/supabase/auth/session';
 import { fromTable } from '../db-helpers';
 
 /**
- * Get current authenticated user ID
+ * Get current authenticated user ID.
+ *
+ * With no client this is the shared, cached answer from the auth layer. With an
+ * explicit client it asks that client directly and caches nothing: a caller
+ * passing its own client is usually passing a per-request server client, and a
+ * module-level cache there would hand one request's user to the next.
  */
 export async function getCurrentUserId(client?: AnySupabaseClient): Promise<string | null> {
+  if (!client) {
+    return getSharedCurrentUserId();
+  }
   try {
-    const supabaseClient = client || supabase;
     const {
       data: { user },
-    } = await supabaseClient.auth.getUser();
+    } = await client.auth.getUser();
     return user?.id || null;
   } catch (error) {
     logger.error('Error getting current user ID', error, 'Groups');

src/services/loans/utils/auth.ts

@@ -6,18 +6,6 @@
  * Last Modified Summary: Extracted from loans/index.ts for modularity
  */
 
-import supabase from '@/lib/supabase/browser';
 
-/**
- * Get current authenticated user ID
- */
-export async function getCurrentUserId(): Promise<string | null> {
-  try {
-    const {
-      data: { user },
-    } = await supabase.auth.getUser();
-    return user?.id || null;
-  } catch {
-    return null;
-  }
-}
+// Defined once, in the auth layer, where it is cached. See session.ts.
+export { getCurrentUserId } from '@/services/supabase/auth/session';

src/services/projects/support/helpers.ts

@@ -8,29 +8,9 @@
  * Last Modified Summary: Created project support helper functions
  */
 
-import supabase from '@/lib/supabase/browser';
-import { logger } from '@/utils/logger';
 
-/**
- * Get current user ID
- */
-export async function getCurrentUserId(): Promise<string | null> {
-  try {
-    const {
-      data: { user },
-      error,
-    } = await supabase.auth.getUser();
-
-    if (error || !user) {
-      return null;
-    }
-
-    return user.id;
-  } catch (error) {
-    logger.error('Error getting current user ID', error, 'ProjectSupport');
-    return null;
-  }
-}
+// Defined once, in the auth layer, where it is cached. See session.ts.
+export { getCurrentUserId } from '@/services/supabase/auth/session';
 
 // Re-export formatSats from SSOT to avoid duplication
 // Use useDisplayCurrency hook in components for user-preferred currency