fix(web): stop calling revalidatePath during render on first sign-in

docs/key-files.md

@@ -10,7 +10,9 @@ else. Grouped loosely by subsystem in migration/PR order.
 | `packages/mcp-core/src/scope/scope.ts` | Canonical scope validation + wildcard expansion |
 | `packages/mcp-core/src/telemetry/telemetry.ts` | Shared tracer/meter getters, `lorekit.tool.duration` histogram |
 | `packages/web/src/lib/scope.ts` | Lightweight copy of `scopeType` for Next.js bundle (no OTel deps), plus `scopeRepoUrl` and `scopeRepoRef` — the ONE derivation of what a scope already says about a repository (`{repo, branch}`, null for `global`/`project`/malformed). `scopeRepoUrl` is built on `scopeRepoRef`, and `lib/origin.ts` consumes it so the stored provenance rows can never duplicate the scope-derived Repo row |
-| `packages/web/src/lib/tokens.ts` | Server actions: `generateToken`, `listTokens`, `revokeToken` |
+| `packages/web/src/lib/tokens.ts` | Server actions: `generateToken`, `listTokens`, `revokeToken`, `setTokenScoping`. `generateToken` is a thin wrapper over `token-mint.ts`'s `mintToken` that revalidates `/lore` and the `/settings` layout on success only — the action path, triggered by a form submission |
+| `packages/web/src/lib/token-mint.ts` | `server-only` (not a Server Action) mint core: `mintToken`, `applyScoping`, `TOKEN_COLUMNS`, the `ApiToken`/`TokenPermission` types. Does NOT call `revalidatePath` — that is what lets `/lore`'s Server Component render mint a token directly (via `first-token.ts`) without hitting Next 15's "revalidatePath during render" error, which previously minted an orphan `api_tokens` row on every first sign-in |
+| `packages/web/src/lib/first-token.ts` | `resolveFirstToken` — the first-sign-in auto-mint decision for `/lore`'s render, with the mint injected so it is node-env testable. Replaces a bare `catch {}` that used to swallow the `revalidatePath`-during-render error above along with every other failure |
 | `packages/web/src/lib/api/` | The dashboard's client for LoreKit's OWN REST API. `rest.ts` (runtime-agnostic `restFetch` — takes an access token rather than reaching for a session, so one module serves browser hooks and server actions; `RestApiError` carries the `{error, code}` envelope and a non-JSON body still yields a usable message), `memories.ts` (typed wrappers whose request/response types come from `@lorekit/schemas`, so the dashboard cannot invent a parameter the API does not accept), `session-browser.ts` / `session-server.ts` (the user's own Supabase session token, split by runtime because the server half imports `next/headers`). See `packages/web/CLAUDE.md` → "Data fetching goes through LoreKit's REST API" |
 | `packages/web/src/lib/lesson-entry.ts` | Pure `lessonFromMemoryEntry` — the ONE translation from a REST `MemoryEntry` to the dashboard's `LessonEntry`. Replaced three hand-written `row.key as string` mappings that had already drifted (one populated `id`, another did not) |
 | `packages/schemas/src/shared/tags.ts` | Pure `normalizeTagList` / `parseTagsParam` / `pgArrayLiteral` — label normalisation and Postgres array-literal quoting, shared by the `GET /memories` handler's `tags_mode=all` branch and the dashboard's label picker (which re-exports them from `lib/tag-filter.ts`). The `filter.ts` pattern: pure, part of the wire contract, needed by more than one runtime |

docs/otel.md

@@ -804,11 +804,18 @@ and the outbound Supabase fetch calls; the custom spans add business context
 | `lorekit.org.invite.accept` | `lorekit.invite.id` |
 | `lorekit.org.invite.decline` | `lorekit.invite.id` |
 
-### API token management (`lib/tokens.ts`)
+### API token management (`lib/token-mint.ts`, `lib/tokens.ts`)
+
+The `lorekit.api_token.generate` span is emitted from `lib/token-mint.ts`'s `mintToken` —
+the `server-only` mint core shared by both callers: `lib/tokens.ts`'s `generateToken`
+action (dashboard "Generate token" button, revalidates on success) and `lib/first-token.ts`'s
+`resolveFirstToken` (the `/lore` first-sign-in auto-mint, which runs during a Server
+Component render and must never revalidate). `lorekit.api_token.origin` tells the two
+apart in telemetry rather than guessing from auth tier.
 
 | Span name | Key attributes |
 |-----------|---------------|
-| `lorekit.api_token.generate` | `lorekit.api_token.permissions`, `lorekit.api_token.id` (on success), `lorekit.api_token.limit_reached` (when cap is hit) |
+| `lorekit.api_token.generate` | `lorekit.api_token.permissions`, `lorekit.api_token.origin` (`onboarding`\|`dashboard`), `lorekit.api_token.id` (on success), `lorekit.api_token.limit_reached` (when cap is hit) |
 | `lorekit.api_token.revoke` | `lorekit.api_token.id`, `lorekit.api_token.prefix` |
 
 On **any RPC/DB failure** the span also carries:

packages/web/src/lib/first-token.spec.ts

@@ -0,0 +1,94 @@
+/**
+ * `resolveFirstToken` owns the first-sign-in auto-mint decision for `/lore`'s
+ * render. It replaces a bare `catch {}` that used to hide the production bug
+ * (`token-mint.ts`'s header): these specs pin the decision table on its own,
+ * with `mint` injected so no Supabase/Next mocking is needed.
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { resolveFirstToken } from './first-token';
+import type { ApiToken } from './token-mint';
+
+const EXISTING: ApiToken = {
+  id: 'tok-existing',
+  name: 'Old token',
+  token_prefix: 'lk_rw_old...',
+  permissions: ['read', 'write'],
+  scopes: [],
+  org_access: 'all',
+  org_ids: [],
+  last_used_at: null,
+  created_at: '2025-01-01T00:00:00.000Z',
+};
+
+const MINTED: ApiToken = {
+  id: 'tok-new',
+  name: 'My first token',
+  token_prefix: 'lk_rw_new...',
+  permissions: ['read', 'write'],
+  scopes: [],
+  org_access: 'all',
+  org_ids: [],
+  last_used_at: null,
+  created_at: '2026-01-01T00:00:00.000Z',
+};
+
+beforeEach(() => {
+  vi.clearAllMocks();
+});
+
+describe('resolveFirstToken', () => {
+  it('mints exactly once and returns the record + token when there are no existing tokens', async () => {
+    const mint = vi.fn().mockResolvedValue({ token: 'lk_rw_abc', record: MINTED });
+
+    const result = await resolveFirstToken({ existingTokens: [], autoGenerate: true, mint });
+
+    expect(mint).toHaveBeenCalledTimes(1);
+    expect(result).toEqual({ tokens: [MINTED], autoGeneratedToken: 'lk_rw_abc' });
+  });
+
+  it('does not mint when existing tokens are non-empty', async () => {
+    const mint = vi.fn();
+
+    const result = await resolveFirstToken({ existingTokens: [EXISTING], autoGenerate: true, mint });
+
+    expect(mint).not.toHaveBeenCalled();
+    expect(result).toEqual({ tokens: [EXISTING], autoGeneratedToken: null });
+  });
+
+  it('does not mint when autoGenerate is false', async () => {
+    const mint = vi.fn();
+
+    const result = await resolveFirstToken({ existingTokens: [], autoGenerate: false, mint });
+
+    expect(mint).not.toHaveBeenCalled();
+    expect(result).toEqual({ tokens: [], autoGeneratedToken: null });
+  });
+
+  it('degrades to existing tokens when the mint returns an error', async () => {
+    const mint = vi.fn().mockResolvedValue({ error: 'cap reached' });
+
+    const result = await resolveFirstToken({ existingTokens: [], autoGenerate: true, mint });
+
+    expect(result).toEqual({ tokens: [], autoGeneratedToken: null });
+  });
+
+  it('degrades to existing tokens when the mint throws a non-Next error', async () => {
+    const mint = vi.fn().mockRejectedValue(new Error('unexpected'));
+
+    const result = await resolveFirstToken({ existingTokens: [EXISTING], autoGenerate: true, mint });
+
+    expect(result).toEqual({ tokens: [EXISTING], autoGeneratedToken: null });
+  });
+
+  it('re-throws a Next control-flow error (redirect) rather than degrading', async () => {
+    const redirectError = new Error('NEXT_REDIRECT');
+    redirectError.name = 'NEXT_REDIRECT';
+    // Mirrors Next's internal digest convention that `unstable_rethrow` keys off.
+    (redirectError as unknown as { digest: string }).digest = 'NEXT_REDIRECT;replace;/login;307;';
+    const mint = vi.fn().mockRejectedValue(redirectError);
+
+    await expect(resolveFirstToken({ existingTokens: [], autoGenerate: true, mint })).rejects.toThrow(
+      'NEXT_REDIRECT',
+    );
+  });
+});

packages/web/src/lib/first-token.ts

@@ -0,0 +1,65 @@
+/**
+ * The first-sign-in auto-mint decision for `/lore`'s onboarding render.
+ *
+ * Pulled out of `onboarding-steps.tsx` so it is a plain function a node-env
+ * `.spec.ts` can exercise with an injected `mint` — the render itself must
+ * never call a `'use server'` action whose success path revalidates (that is
+ * the bug `lib/token-mint.ts` exists to fix), so this module only ever calls
+ * the injected `mint`, never `generateToken`.
+ *
+ * Previously this decision lived inline behind a bare `catch {}`, which is
+ * exactly what hid the production bug: the mint's `revalidatePath` call threw
+ * during render, and the bare catch swallowed it along with every other kind
+ * of failure, leaving an orphan `api_tokens` row and no token on screen.
+ * `unstable_rethrow` restores the one distinction that catch erased — Next's
+ * own control-flow errors (redirect, not-found, the dynamic-rendering bailout)
+ * are not failures and must propagate, while a genuine mint failure degrades
+ * to "no auto token" so the page still renders.
+ */
+import { unstable_rethrow } from 'next/navigation';
+import type { ApiToken } from '@/lib/token-mint';
+
+export interface ResolveFirstTokenArgs {
+  /** Tokens the user already has, as read by `listTokens()`. */
+  existingTokens: ApiToken[];
+  /** Whether the caller wants a first-sign-in auto-mint at all. */
+  autoGenerate: boolean;
+  /** Injected effect — the render path passes `mintToken(..., 'onboarding')`. */
+  mint: () => Promise<{ token: string; record: ApiToken } | { error: string }>;
+}
+
+export interface ResolveFirstTokenResult {
+  /** `existingTokens` unchanged, or `[record]` when a mint just succeeded. */
+  tokens: ApiToken[];
+  /** The freshly minted token string, shown once, or `null`. */
+  autoGeneratedToken: string | null;
+}
+
+export async function resolveFirstToken({
+  existingTokens,
+  autoGenerate,
+  mint,
+}: ResolveFirstTokenArgs): Promise<ResolveFirstTokenResult> {
+  if (!autoGenerate || existingTokens.length > 0) {
+    return { tokens: existingTokens, autoGeneratedToken: null };
+  }
+
+  try {
+    const result = await mint();
+    if ('token' in result) {
+      return { tokens: [result.record], autoGeneratedToken: result.token };
+    }
+    // `{ error }` — a DB failure is already logged by `mintToken`; the
+    // expected refusals (not authenticated, token cap reached) are not
+    // logged anywhere. Either way, degrade to no auto token.
+    return { tokens: existingTokens, autoGeneratedToken: null };
+  } catch (error) {
+    // Next's own control-flow errors (redirect, not-found, the dynamic-
+    // rendering bailout) are not mint failures and must propagate.
+    unstable_rethrow(error);
+    // Any other thrown error is a genuine, unexpected mint failure. `withSpan`
+    // inside `mintToken` already logged it — degrade to no auto token so the
+    // page still renders rather than taking down the whole route.
+    return { tokens: existingTokens, autoGeneratedToken: null };
+  }
+}

packages/web/src/lib/onboarding-steps.spec.ts

@@ -0,0 +1,62 @@
+/**
+ * Pins the render-path binding the first-sign-in fix depends on: the `/lore`
+ * onboarding render mints through `mintToken(..., 'onboarding')`, never the
+ * `'use server'` `generateToken` action whose success path calls
+ * `revalidatePath` (which Next 15 forbids during render). Pointing the render
+ * back at `generateToken` — the original bug — fails this spec.
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import * as React from 'react';
+
+// The node-env vitest transform compiles JSX to `React.createElement`, so the
+// module under test needs React in scope at call time.
+vi.stubGlobal('React', React);
+
+vi.mock('server-only', () => ({}));
+
+const listTokens = vi.fn();
+const generateToken = vi.fn();
+vi.mock('@/lib/tokens', () => ({
+  listTokens: () => listTokens(),
+  generateToken: (...args: unknown[]) => generateToken(...args),
+}));
+
+const mintToken = vi.fn();
+vi.mock('@/lib/token-mint', () => ({ mintToken: (...args: unknown[]) => mintToken(...args) }));
+
+vi.mock('@/lib/key-scope-catalog', () => ({
+  getKeyScopeCatalog: () => Promise.resolve({ scopes: [], orgs: [] }),
+}));
+vi.mock('@/components/dashboard/OnboardingStepContent', () => ({
+  OnboardingStepContent: () => null,
+}));
+
+const { buildOnboardingSteps } = await import('./onboarding-steps');
+
+const RECORD = { id: 'tok-1', name: 'My first token' };
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  mintToken.mockResolvedValue({ token: 'lk_rw_x', record: RECORD });
+});
+
+describe('buildOnboardingSteps', () => {
+  it('auto-mints a first token through mintToken with the onboarding origin, never generateToken', async () => {
+    listTokens.mockResolvedValue([]);
+
+    await buildOnboardingSteps({ autoGenerateToken: true });
+
+    expect(mintToken).toHaveBeenCalledTimes(1);
+    expect(mintToken).toHaveBeenCalledWith('My first token', ['read', 'write'], undefined, 'onboarding');
+    expect(generateToken).not.toHaveBeenCalled();
+  });
+
+  it('mints nothing when the user already has a token', async () => {
+    listTokens.mockResolvedValue([RECORD]);
+
+    await buildOnboardingSteps({ autoGenerateToken: true });
+
+    expect(mintToken).not.toHaveBeenCalled();
+    expect(generateToken).not.toHaveBeenCalled();
+  });
+});

packages/web/src/lib/onboarding-steps.tsx

@@ -1,7 +1,9 @@
 import { Bot } from 'lucide-react';
 import type { OnboardingStep } from './onboarding';
 import { OnboardingStepContent } from '@/components/dashboard/OnboardingStepContent';
-import { listTokens, generateToken } from '@/lib/tokens';
+import { listTokens } from '@/lib/tokens';
+import { mintToken } from '@/lib/token-mint';
+import { resolveFirstToken } from '@/lib/first-token';
 import { resolveMcpUrls } from '@/lib/mcp-url';
 import { getKeyScopeCatalog } from '@/lib/key-scope-catalog';
 import { SETTINGS_LANDING_HREF } from '@/lib/settings-routes';
@@ -22,8 +24,14 @@ import { SETTINGS_LANDING_HREF } from '@/lib/settings-routes';
  *
  * First login has no tokens: with `autoGenerateToken`, we mint a read+write
  * token so the user can copy a ready-to-use config with zero extra clicks.
- * Wrapped in try/catch: if generation fails the page still renders, just
- * without the auto-filled token.
+ * The mint runs through `mintToken` (the `server-only` core in
+ * `lib/token-mint.ts`), NOT `lib/tokens.ts`'s `'use server'` generate action —
+ * a Server Component render must never call that action, because its success
+ * path calls `revalidatePath`, which Next 15 forbids during render. This
+ * render needs no revalidation anyway: it returns the freshly minted record
+ * as `initialTokens` itself. `resolveFirstToken` (`lib/first-token.ts`) owns
+ * the decision and degrades to "no auto token" on failure so the page still
+ * renders.
  */
 export async function buildOnboardingSteps(
   { autoGenerateToken = false }: { autoGenerateToken?: boolean } = {},
@@ -39,19 +47,11 @@ export async function buildOnboardingSteps(
   // same shape `settings/api-keys/page.tsx` already uses for the same pair.
   const [existingTokens, catalog] = await Promise.all([listTokens(), getKeyScopeCatalog()]);
 
-  let tokens = existingTokens;
-  let autoGeneratedToken: string | null = null;
-  if (autoGenerateToken && existingTokens.length === 0) {
-    try {
-      const result = await generateToken('My first token', ['read', 'write']);
-      if ('token' in result) {
-        autoGeneratedToken = result.token;
-        tokens = [result.record];
-      }
-    } catch {
-      // Non-fatal — page renders without the auto-generated token.
-    }
-  }
+  const { tokens, autoGeneratedToken } = await resolveFirstToken({
+    existingTokens,
+    autoGenerate: autoGenerateToken,
+    mint: () => mintToken('My first token', ['read', 'write'], undefined, 'onboarding'),
+  });
 
   const { mcpUrl } = resolveMcpUrls();