refactor(ai): retry outside instrumentation + retry-exhaustion test (#179 review)

- Invert the transport layers so the pre-response retry is OUTERMOST and the
  provider-HTTP instrumentation is INNER. Before, the retry lived inside
  createStreamingFetch (under the instrumentation), so a reset the retry
  recovered from logged only a clean "OK status=200" — the
  "PRE-RESPONSE FAILED ... ECONNRESET ... idleSincePrevCall" signal went blind
  exactly when the fix works, and AI_STREAM_KEEPALIVE_MS couldn't be tuned from
  prod data. Now createStreamingFetch is the dispatcher-bound BASE (no retry) and
  a new withPreResponseRetry() wraps it; ai.service composes
  withPreResponseRetry(createInstrumentedFetch('AiService:provider-http',
  createStreamingFetch())), so every attempt — including recovered resets — flows
  through the instrumentation. (Also expresses the keepAlive-config vs retry-
  behavior boundary structurally, per review #3.)
- Add the retry-exhaustion test: a server that resets EVERY connection, asserting
  the call rejects with a retryable connection error AND exactly
  PRE_RESPONSE_CONNECT_RETRIES + 1 (= 3) requests reached the server — pinning the
  bound and that the final error propagates (guards an off-by-one / infinite loop
  / swallowed error). Existing happy-retry + abort tests moved onto
  withPreResponseRetry.

Verified on the stand: a normal turn still streams (reasoning + finish) and the
provider-HTTP telemetry still logs. server tsc + ai/mcp specs green (30).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

.env.example

@@ -136,6 +136,19 @@ MCP_DOCMOST_PASSWORD=
 # A slow/hung embeddings endpoint fails after this and the batch continues.
 # AI_EMBEDDING_TIMEOUT_MS=120000
 
+# Silence timeout (ms) for streaming chat/agent AI calls AND external-MCP traffic.
+# Bounds time-to-first-byte and the gap BETWEEN chunks (NOT the total turn length),
+# so an arbitrarily long turn that keeps streaming is never cut. Finite so a hung
+# provider is eventually broken instead of leaking forever. Default 900000 (15 min).
+# AI_STREAM_TIMEOUT_MS=900000
+
+# Keep-alive recycle window (ms) for streaming chat/agent AI + external-MCP calls.
+# A pooled connection idle longer than this is closed instead of reused, so a
+# NAT / egress firewall / reverse proxy that silently drops idle connections
+# cannot poison a reused socket into a PRE-RESPONSE `read ECONNRESET`. Lower it if
+# your egress drops idle connections faster than ~10s. Default 10000 (10 s).
+# AI_STREAM_KEEPALIVE_MS=10000
+
 # --- Anonymous public-share AI assistant ---
 # Opt-in per workspace (AI settings -> "public share assistant"; off by default).
 # When enabled, anonymous visitors of a published share can ask an AI about that

CHANGELOG.md

@@ -20,9 +20,35 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   `UPDATE users SET is_agent = true WHERE email = '<mcp-account>'`. Never flag a
   human or shared account, or its normal edits get mis-attributed as AI. See the
   AI-agent block in `.env.example`. (#143)
+- **Footnote import diagnostics.** The MCP page-write tools (`create_page`,
+  `update_page`, `import_page_markdown`) now return a `footnoteWarnings` array
+  flagging dangling references, empty or duplicate definitions, and `[^id]`
+  markers inside table rows, so an agent can fix its own markup. The page is
+  still created; the field is omitted when there are no problems. (#166)
+- **AI chat "Protocol" setting (`chatApiStyle`).** A new admin choice in AI
+  settings for the `openai` driver: `openai-compatible` (default) routes chat
+  through `@ai-sdk/openai-compatible`, which surfaces a provider's streamed
+  reasoning (`reasoning_content` → reasoning parts) for z.ai/GLM, DeepSeek,
+  OpenRouter, etc.; `openai` uses the official provider (real-OpenAI
+  reasoning-model request shaping). Chosen explicitly rather than inferred from
+  the base URL, since a custom URL can front real OpenAI too. (#175, #177)
 
 ### Changed
 
+- **AI chat default provider is now `openai-compatible` (reasoning surfaced).**
+  For the `openai` driver the chat provider defaults to the openai-compatible
+  implementation, so a workspace pointing at z.ai/GLM/DeepSeek now streams the
+  model's reasoning out of the box. An endpoint that is real OpenAI behind a
+  custom base URL should set the new `chatApiStyle` "Protocol" to `openai`. (#177)
+
+- **Footnotes now reuse (Pandoc semantics).** Multiple `[^a]` references to the
+  same id are ONE footnote — one number, one definition, several back-references
+  — instead of being renamed to `a__2`, `a__3`. Duplicate `[^a]:` definitions are
+  first-wins on import (the rest are dropped and reported via `footnoteWarnings`),
+  and a reference with no definition yields a single empty footnote rather than
+  one per occurrence. This supersedes the 0.93.0 "survive duplicate-id
+  definitions" behavior for the import path. (#166)
+
 - **Public share AI: default per-workspace hourly assistant cap lowered
   300 → 100.** The limiter falls back to this default whenever
   `SHARE_AI_WORKSPACE_MAX_PER_HOUR` is unset, so a `0.93.0` deployment that

apps/client/public/locales/en-US/translation.json

@@ -1307,5 +1307,9 @@
   "Page tree (child pages, recursive)": "Page tree (child pages, recursive)",
   "Render the full nested tree of all descendant pages": "Render the full nested tree of all descendant pages",
   "Showing {{count}} subpages_one": "Showing {{count}} subpage",
-  "Showing {{count}} subpages_other": "Showing {{count}} subpages"
+  "Showing {{count}} subpages_other": "Showing {{count}} subpages",
+  "Protocol": "Protocol",
+  "How chat requests are sent and how reasoning is surfaced": "How chat requests are sent and how reasoning is surfaced",
+  "OpenAI-compatible (surfaces reasoning)": "OpenAI-compatible (surfaces reasoning)",
+  "OpenAI (official)": "OpenAI (official)"
 }

apps/client/public/locales/ru-RU/translation.json

@@ -1160,5 +1160,9 @@
   "Render the full nested tree of all descendant pages": "Показать полное вложенное дерево всех дочерних страниц",
   "Showing {{count}} subpages_one": "Показано {{count}} подстраница",
   "Showing {{count}} subpages_few": "Показано {{count}} подстраницы",
-  "Showing {{count}} subpages_many": "Показано {{count}} подстраниц"
+  "Showing {{count}} subpages_many": "Показано {{count}} подстраниц",
+  "Protocol": "Протокол",
+  "How chat requests are sent and how reasoning is surfaced": "Как отправляются запросы чата и как показывается reasoning",
+  "OpenAI-compatible (surfaces reasoning)": "OpenAI-совместимый (показывает reasoning)",
+  "OpenAI (official)": "OpenAI (официальный)"
 }

apps/client/src/features/workspace/components/settings/components/ai-mcp-server-form.tsx

@@ -56,7 +56,13 @@ function buildInitialValues(server?: IAiMcpServer): FormValues {
     transport: server?.transport ?? "http",
     url: server?.url ?? "",
     authHeader: "",
-    toolAllowlist: server?.toolAllowlist ?? [],
+    // Defensive: TagsInput calls `.map`, so a non-array here (e.g. an API that
+    // returns the jsonb column as a JSON string) would crash the whole page. The
+    // server normalizes this now, but guard anyway so a bad shape can never take
+    // the settings UI down.
+    toolAllowlist: Array.isArray(server?.toolAllowlist)
+      ? server.toolAllowlist
+      : [],
     enabled: server?.enabled ?? true,
   };
 }

apps/client/src/features/workspace/components/settings/components/ai-provider-settings.tsx

@@ -38,6 +38,7 @@ import {
   AiTestCapability,
   IAiSettingsUpdate,
   SttApiStyle,
+  ChatApiStyle,
 } from "@/features/workspace/services/ai-settings-service.ts";
 import { useAiRolesQuery } from "@/features/ai-chat/queries/ai-chat-query.ts";
 import { IAiRole } from "@/features/ai-chat/types/ai-chat.types.ts";
@@ -82,6 +83,8 @@ const STT_LANGUAGE_OPTIONS: { value: string; label: string }[] = [
 // (empty means "leave unchanged" unless explicitly cleared).
 const formSchema = z.object({
   chatModel: z.string(),
+  // Chat provider implementation (reasoning surfacing). Default openai-compatible.
+  chatApiStyle: z.enum(["openai-compatible", "openai"]),
   // Cheap model id for the anonymous public-share assistant; empty = use chatModel.
   publicShareChatModel: z.string(),
   // Agent-role id whose persona the public-share assistant adopts; empty =
@@ -308,6 +311,7 @@ export default function AiProviderSettings() {
     validate: zod4Resolver(formSchema),
     initialValues: {
       chatModel: "",
+      chatApiStyle: "openai-compatible" as ChatApiStyle,
       publicShareChatModel: "",
       publicShareAssistantRoleId: "",
       embeddingModel: "",
@@ -330,6 +334,7 @@ export default function AiProviderSettings() {
     if (!settings) return;
     form.setValues({
       chatModel: settings.chatModel ?? "",
+      chatApiStyle: settings.chatApiStyle ?? "openai-compatible",
       publicShareChatModel: settings.publicShareChatModel ?? "",
       publicShareAssistantRoleId: settings.publicShareAssistantRoleId ?? "",
       embeddingModel: settings.embeddingModel ?? "",
@@ -359,6 +364,7 @@ export default function AiProviderSettings() {
       // Everything is OpenAI-compatible.
       driver: "openai",
       chatModel: values.chatModel,
+      chatApiStyle: values.chatApiStyle,
       // Cheap model id for the anonymous public-share assistant; empty falls
       // back to chatModel server-side.
       publicShareChatModel: values.publicShareChatModel,
@@ -761,6 +767,24 @@ export default function AiProviderSettings() {
           {t("Resolves to {{url}}", { url: chatResolved })}
         </Text>
 
+        <Select
+          mt="sm"
+          label={t("Protocol")}
+          description={t(
+            "How chat requests are sent and how reasoning is surfaced",
+          )}
+          data={[
+            {
+              value: "openai-compatible",
+              label: t("OpenAI-compatible (surfaces reasoning)"),
+            },
+            { value: "openai", label: t("OpenAI (official)") },
+          ]}
+          allowDeselect={false}
+          disabled={isLoading}
+          {...form.getInputProps("chatApiStyle")}
+        />
+
         {/* Anonymous public-share assistant: a single master toggle + an
             optional cheaper model id. Reuses this card's driver/URL/key. */}
         <Group justify="space-between" align="center" wrap="nowrap" mt="md">

apps/client/src/features/workspace/services/ai-settings-service.ts

@@ -9,6 +9,12 @@ export type AiDriver = "openai" | "gemini" | "ollama";
 //   - 'json'      -> JSON body with base64-encoded audio (OpenRouter)
 export type SttApiStyle = "multipart" | "json";
 
+// Chat provider implementation for the `openai` driver (chosen explicitly):
+//   - 'openai-compatible' -> maps streamed reasoning_content to reasoning parts
+//     (z.ai/GLM, DeepSeek, OpenRouter, ...). Default.
+//   - 'openai'            -> official provider; real-OpenAI reasoning-model shaping.
+export type ChatApiStyle = "openai-compatible" | "openai";
+
 // Masked AI provider settings returned by the server.
 // No API key is ever returned; only `hasApiKey` / `hasEmbeddingApiKey` indicate
 // whether one is stored. `embeddingBaseUrl` is the RAW stored value (empty means
@@ -16,6 +22,7 @@ export type SttApiStyle = "multipart" | "json";
 export interface IAiSettings {
   driver?: AiDriver;
   chatModel?: string;
+  chatApiStyle?: ChatApiStyle;
   // Cheap model id for the anonymous public-share assistant; empty = chatModel.
   publicShareChatModel?: string;
   // Agent-role id whose persona the public-share assistant adopts; empty =
@@ -49,6 +56,7 @@ export interface IAiSettings {
 export interface IAiSettingsUpdate {
   driver?: AiDriver;
   chatModel?: string;
+  chatApiStyle?: ChatApiStyle;
   publicShareChatModel?: string;
   // Agent-role id whose persona the public-share assistant adopts; empty =
   // built-in locked persona.

apps/server/src/core/ai-chat/ai-chat.controller.ts

@@ -159,6 +159,9 @@ export class AiChatController {
     // we also drop it on response `finish` so it never lingers after the stream
     // completes normally (the AI SDK pipes the response fire-and-forget, so we
     // cannot simply remove it once `stream()` returns).
+    // DIAGNOSTIC (Safari stream-drop investigation) — temporary: wall-clock at
+    // which a Safari disconnect is observed, measured from request receipt.
+    const reqStartedAt = Date.now();
     const controller = new AbortController();
     const onClose = (): void => {
       // A genuine disconnect leaves the response unfinished (unlike a normal
@@ -167,7 +170,8 @@ export class AiChatController {
       // so log it here before aborting the agent loop.
       if (!res.raw.writableEnded) {
         this.logger.warn(
-          'AI chat stream: client disconnected before completion; aborting turn',
+          `AI chat stream: client disconnected before completion; aborting turn ` +
+            `(elapsed=${Date.now() - reqStartedAt}ms since request received)`,
         );
         controller.abort();
       }

apps/server/src/core/ai-chat/ai-chat.service.ts

@@ -380,6 +380,15 @@ export class AiChatService {
     const capturedSteps: StepLike[] = [];
     let inProgressText = '';
 
+    // DIAGNOSTIC (Safari stream-drop investigation) — temporary. Measure
+    // first-chunk latency, the model-silent gap right before a disconnect, and
+    // how many SSE heartbeats were written, so a Safari drop can be classified
+    // (idle-gap vs hard wall-clock cap vs slow first chunk).
+    const streamStartedAt = Date.now();
+    let firstModelChunkAt: number | undefined;
+    let lastModelChunkAt = streamStartedAt;
+    let heartbeatsSent = 0;
+
     // NOTE: streamText is synchronous in v6 — do NOT await it. A synchronous
     // failure here (or in pipe below) would skip the terminal callbacks, so the
     // catch releases the leased external clients to avoid a connection leak.
@@ -404,6 +413,12 @@ export class AiChatService {
       prepareStep: ({ stepNumber }) => prepareAgentStep(stepNumber, system),
       abortSignal: signal,
       onChunk: ({ chunk }) => {
+        // DIAGNOSTIC (Safari stream-drop investigation) — temporary. Any model
+        // output chunk means the stream is actively emitting bytes; track first
+        // + most-recent activity timestamps.
+        const now = Date.now();
+        firstModelChunkAt ??= now;
+        lastModelChunkAt = now;
         // 'text-delta' is the assistant's prose; tool-call args are separate chunk
         // types — so this mirrors exactly what streams to the client.
         if (chunk.type === 'text-delta') inProgressText += chunk.text;
@@ -415,6 +430,14 @@ export class AiChatService {
         inProgressText = '';
       },
       onFinish: async ({ text, finishReason, totalUsage, usage, steps }) => {
+        // DIAGNOSTIC (Safari stream-drop investigation) — temporary: success
+        // baseline for Safari comparison.
+        const diagNow = Date.now();
+        this.logger.log(
+          `AI chat stream DIAGNOSTIC (finish): elapsed=${diagNow - streamStartedAt}ms ` +
+            `firstChunkLatency=${firstModelChunkAt ? firstModelChunkAt - streamStartedAt : 'none'}ms ` +
+            `heartbeatsSent=${heartbeatsSent} steps=${steps.length}`,
+        );
         await persistAssistant({
           text,
           toolCalls: serializeSteps(steps),
@@ -464,6 +487,14 @@ export class AiChatService {
         const e = error as { stack?: string };
         const errorText = describeProviderError(error, String(error));
         this.logger.error(`AI chat stream error: ${errorText}`, e?.stack);
+        // DIAGNOSTIC (Safari stream-drop investigation) — temporary: timing of
+        // an error-terminated stream.
+        const diagNow = Date.now();
+        this.logger.warn(
+          `AI chat stream DIAGNOSTIC (error): elapsed=${diagNow - streamStartedAt}ms ` +
+            `firstChunkLatency=${firstModelChunkAt ? firstModelChunkAt - streamStartedAt : 'none'}ms ` +
+            `silentGapBeforeDrop=${diagNow - lastModelChunkAt}ms heartbeatsSent=${heartbeatsSent}`,
+        );
         // Persist the PARTIAL answer streamed before the failure (text + any
         // finished tool steps) WITH the error in metadata, so the turn shows what
         // the user already saw plus the cause — not just a bare error.
@@ -488,6 +519,15 @@ export class AiChatService {
           `AI chat stream aborted (chat ${chatId}) after ${steps.length} ` +
             `step(s), ${partialChars} chars partial text; persisting partial turn.`,
         );
+        // DIAGNOSTIC (Safari stream-drop investigation) — temporary: THE key
+        // line — classifies the Safari drop.
+        const diagNow = Date.now();
+        this.logger.warn(
+          `AI chat stream DIAGNOSTIC (abort/disconnect): elapsed=${diagNow - streamStartedAt}ms ` +
+            `firstChunkLatency=${firstModelChunkAt ? firstModelChunkAt - streamStartedAt : 'none'}ms ` +
+            `silentGapBeforeDrop=${diagNow - lastModelChunkAt}ms heartbeatsSent=${heartbeatsSent} ` +
+            `steps=${steps.length}`,
+        );
         await persistAssistant(
           buildPartialAssistantRecord(capturedSteps, inProgressText, 'aborted'),
         );
@@ -566,7 +606,11 @@ export class AiChatService {
       // headers are sent, and is guarded for response-likes that lack it.
       res.raw.flushHeaders?.();
       // Heartbeat: keep the SSE stream progressing during silent tool/think gaps (Safari/proxy idle timeout).
-      startSseHeartbeat(res.raw);
+      // DIAGNOSTIC (Safari stream-drop investigation) — temporary: count beats so a disconnect log can show
+      // how many pings were written before Safari dropped.
+      startSseHeartbeat(res.raw, 15_000, () => {
+        heartbeatsSent += 1;
+      });
     } catch (err) {
       // Synchronous failure before/while wiring the stream: the terminal
       // callbacks will not run, so release the leased external clients here and

apps/server/src/core/ai-chat/external-mcp/mcp-clients.service.ts

@@ -6,6 +6,7 @@ import { createMCPClient } from '@ai-sdk/mcp';
 import { Agent, type Dispatcher } from 'undici';
 import { AiMcpServerRepo } from '@docmost/db/repos/ai-chat/ai-mcp-server.repo';
 import { AiMcpServer } from '@docmost/db/types/entity.types';
+import { streamingDispatcherOptions } from '../../../integrations/ai/ai-streaming-fetch';
 import { SecretBoxService } from '../../../integrations/crypto/secret-box';
 import { isUrlAllowed, isIpAllowed } from './ssrf-guard';
 
@@ -400,6 +401,16 @@ export function validateResolvedAddresses(
  */
 function buildPinnedDispatcher(): Agent {
   return new Agent({
+    // Raise undici's default 300s headers/body timeouts on external MCP traffic
+    // to the same generous-but-finite silence timeout the chat fetch uses (#175).
+    // A long agent turn keeps an SSE transport (e.g. crawl4ai's /mcp/sse) open
+    // across the whole turn; that connection can idle BETWEEN tool calls longer
+    // than 5 min, and undici's bodyTimeout would otherwise sever it mid-task — a
+    // tool-call failure that aborts the streamed turn and shows the user "Lost
+    // connection to the AI provider". A slow single tool call (a crawl) can
+    // likewise exceed headersTimeout. The timeout stays FINITE so a genuinely
+    // hung server is still broken eventually.
+    ...streamingDispatcherOptions(),
     connect: {
       lookup: (hostname, _options, callback) => {
         // Always resolve ALL addresses ourselves; do not trust the caller's

apps/server/src/core/ai-chat/sse-resilience.ts

@@ -28,15 +28,24 @@ import type { ServerResponse } from 'node:http';
  * the response finishes or the socket closes. The interval is unref()'d so it
  * never keeps the process alive, and writes are guarded so we never write to an
  * already-ended/destroyed socket.
+ *
+ * `onBeat` is an OPTIONAL diagnostic hook invoked once after each heartbeat that
+ * was actually written (only when the write did not throw). It is purely for
+ * telemetry/counters and never affects the heartbeat behavior.
  */
 export function startSseHeartbeat(
   res: ServerResponse,
   intervalMs = 15_000,
+  onBeat?: () => void,
 ): () => void {
   const timer = setInterval(() => {
     if (res.writableEnded || res.destroyed) return;
     try {
       res.write(': ping\n\n');
+      // DIAGNOSTIC (Safari stream-drop investigation) — temporary. Notify the
+      // optional hook only after a successful write, so beat counters reflect
+      // pings that actually reached the socket.
+      onBeat?.();
     } catch {
       // Socket vanished between the guard and the write; nothing to do.
     }

apps/server/src/database/repos/ai-chat/ai-mcp-server.repo.spec.ts

@@ -0,0 +1,48 @@
+import { parseToolAllowlist } from './ai-mcp-server.repo';
+
+/**
+ * The `tool_allowlist` jsonb column historically round-trips as a JSON STRING
+ * (rows written by the old double-encoding `jsonbArray`), so the driver hands
+ * back `'["a","b"]'` instead of an array. `parseToolAllowlist` normalizes both
+ * shapes to the `string[] | null` the entity type promises — fixing the settings
+ * UI crash (TagsInput `.map` on a string) and the tool-allowlist enforcement
+ * (which did `Array.isArray(allow)` and silently allowed ALL tools for a string).
+ */
+describe('parseToolAllowlist', () => {
+  it('passes a real string array through unchanged', () => {
+    expect(parseToolAllowlist(['search', 'crawl'])).toEqual(['search', 'crawl']);
+  });
+
+  it('parses a JSON-string array (the double-encoded read) into an array', () => {
+    // This is exactly what the DB returns for an old row: a jsonb string scalar.
+    expect(parseToolAllowlist('["alpha","beta"]')).toEqual(['alpha', 'beta']);
+  });
+
+  it('returns null for null / undefined (unrestricted)', () => {
+    expect(parseToolAllowlist(null)).toBeNull();
+    expect(parseToolAllowlist(undefined)).toBeNull();
+  });
+
+  it('returns [] for an empty array (no items, but a present allowlist)', () => {
+    expect(parseToolAllowlist([])).toEqual([]);
+  });
+
+  it('returns null for a JSON string that is not an array', () => {
+    expect(parseToolAllowlist('"justastring"')).toBeNull();
+    expect(parseToolAllowlist('{"a":1}')).toBeNull();
+  });
+
+  it('returns null for an unparseable string', () => {
+    expect(parseToolAllowlist('not json at all')).toBeNull();
+  });
+
+  it('returns null when elements are not all strings (defensive)', () => {
+    expect(parseToolAllowlist([1, 2, 3] as unknown)).toBeNull();
+    expect(parseToolAllowlist('[1,2,3]')).toBeNull();
+  });
+
+  it('returns null for a non-string, non-array primitive', () => {
+    expect(parseToolAllowlist(42 as unknown)).toBeNull();
+    expect(parseToolAllowlist(true as unknown)).toBeNull();
+  });
+});

apps/server/src/database/repos/ai-chat/ai-mcp-server.repo.ts

@@ -21,32 +21,35 @@ export class AiMcpServerRepo {
     id: string,
     workspaceId: string,
   ): Promise<AiMcpServer | undefined> {
-    return this.db
+    const row = await this.db
       .selectFrom('aiMcpServers')
       .selectAll('aiMcpServers')
       .where('id', '=', id)
       .where('workspaceId', '=', workspaceId)
       .executeTakeFirst();
+    return row ? normalizeRow(row) : row;
   }
 
   async listByWorkspace(workspaceId: string): Promise<AiMcpServer[]> {
-    return this.db
+    const rows = await this.db
       .selectFrom('aiMcpServers')
       .selectAll('aiMcpServers')
       .where('workspaceId', '=', workspaceId)
       .orderBy('createdAt', 'asc')
       .execute();
+    return rows.map(normalizeRow);
   }
 
   /** Enabled servers only — used by the agent loop to build the toolset. */
   async listEnabled(workspaceId: string): Promise<AiMcpServer[]> {
-    return this.db
+    const rows = await this.db
       .selectFrom('aiMcpServers')
       .selectAll('aiMcpServers')
       .where('workspaceId', '=', workspaceId)
       .where('enabled', '=', true)
       .orderBy('createdAt', 'asc')
       .execute();
+    return rows.map(normalizeRow);
   }
 
   async insert(
@@ -130,6 +133,14 @@ export class AiMcpServerRepo {
  * Encode a string[] as a jsonb bind for the `tool_allowlist` column. Passing a
  * plain JS array to the postgres driver would serialize it as a Postgres array
  * literal (incompatible with jsonb), so we bind the JSON text and cast it.
+ *
+ * The cast is `::text::jsonb`, NOT `::jsonb`: if the parameter is bound straight
+ * to a jsonb cast, node-postgres infers its type as jsonb and JSON-stringifies
+ * the (already-JSON) string a SECOND time, so the column ends up holding a jsonb
+ * STRING SCALAR (`"[\"a\"]"`) instead of a jsonb ARRAY. Forcing the param through
+ * `::text` first binds it as text (sent verbatim), and `::jsonb` then parses it
+ * into a real array. (`normalizeRow` below repairs rows written the old way.)
+ *
  * Returns null for null/empty arrays (an empty allowlist means "no restriction"
  * is not intended — callers pass null to clear; an empty array is normalized to
  * null here so it never round-trips as `[]`).
@@ -139,5 +150,37 @@ function jsonbArray(value: string[] | null | undefined) {
     return null;
   }
   // Typed as string[] so it is assignable to the toolAllowlist column.
-  return sql<string[]>`${JSON.stringify(value)}::jsonb`;
+  return sql<string[]>`${JSON.stringify(value)}::text::jsonb`;
+}
+
+/**
+ * Parse the `toolAllowlist` value read from the DB into the `string[] | null`
+ * the entity type promises. The jsonb column historically round-trips as a JSON
+ * STRING (rows written by the old double-encoding `jsonbArray`, see above), so
+ * the driver hands back a string like `'["a","b"]'` rather than an array. Be
+ * tolerant: an already-parsed array passes through; a JSON string is parsed; null
+ * / a non-array / unparseable value becomes null (unrestricted).
+ */
+export function parseToolAllowlist(value: unknown): string[] | null {
+  if (value == null) return null;
+  if (Array.isArray(value)) {
+    return value.every((v) => typeof v === 'string') ? (value as string[]) : null;
+  }
+  if (typeof value === 'string') {
+    try {
+      const parsed = JSON.parse(value);
+      return Array.isArray(parsed) &&
+        parsed.every((v) => typeof v === 'string')
+        ? (parsed as string[])
+        : null;
+    } catch {
+      return null;
+    }
+  }
+  return null;
+}
+
+/** Normalize a DB row so `toolAllowlist` is always `string[] | null`. */
+function normalizeRow(row: AiMcpServer): AiMcpServer {
+  return { ...row, toolAllowlist: parseToolAllowlist(row.toolAllowlist) };
 }

apps/server/src/database/repos/workspace/workspace.repo.ts

@@ -10,6 +10,29 @@ import {
 import { ExpressionBuilder, sql } from 'kysely';
 import { DB, Workspaces } from '@docmost/db/types/db';
 
+/**
+ * Writable `settings.ai.provider` keys, enforced at this generic SQL layer. This
+ * repo cannot import AI-feature types, so this list is its own copy; a parity
+ * test (ai-provider-settings-keys.spec.ts) asserts it equals
+ * PROVIDER_SETTINGS_KEYS in ai.types so a future drift fails in CI rather than
+ * silently dropping a field at this boundary.
+ */
+export const AI_PROVIDER_SETTINGS_ALLOWED: readonly string[] = [
+  'driver',
+  'chatModel',
+  'chatApiStyle',
+  'embeddingModel',
+  'baseUrl',
+  'embeddingBaseUrl',
+  'sttModel',
+  'sttBaseUrl',
+  'sttApiStyle',
+  'sttLanguage',
+  'systemPrompt',
+  'publicShareChatModel',
+  'publicShareAssistantRoleId',
+];
+
 @Injectable()
 export class WorkspaceRepo {
   public baseFields: Array<keyof Workspaces> = [
@@ -239,9 +262,8 @@ export class WorkspaceRepo {
     // is a real jsonb object, never a double-encoded string. The CASE self-heals
     // workspaces whose settings.ai.provider was previously corrupted into an
     // array/string.
-    const ALLOWED = ['driver', 'chatModel', 'embeddingModel', 'baseUrl', 'embeddingBaseUrl', 'sttModel', 'sttBaseUrl', 'sttApiStyle', 'sttLanguage', 'systemPrompt', 'publicShareChatModel', 'publicShareAssistantRoleId'];
     const entries = Object.entries(provider).filter(
-      ([k, v]) => v !== undefined && ALLOWED.includes(k),
+      ([k, v]) => v !== undefined && AI_PROVIDER_SETTINGS_ALLOWED.includes(k),
     );
     const patch = entries.length
       ? sql`jsonb_build_object(${sql.join(

apps/server/src/integrations/ai/ai-provider-http.spec.ts

@@ -0,0 +1,40 @@
+import { createInstrumentedFetch } from './ai-provider-http';
+
+/**
+ * createInstrumentedFetch must be behavior-neutral: it delegates to the supplied
+ * baseFetch with the SAME input/init, returns the Response object untouched (so
+ * the streamed SSE body is never read/cloned), and rethrows the same error. The
+ * baseFetch injection is the seam that carries the streaming fetch (#175) onto
+ * the chat provider, so it is tested directly.
+ */
+describe('createInstrumentedFetch', () => {
+  it('delegates to the injected baseFetch with the same input/init', async () => {
+    const fakeResponse = new Response('ok', { status: 200 });
+    const baseFetch = jest.fn().mockResolvedValue(fakeResponse);
+    const instrumented = createInstrumentedFetch('test', baseFetch as never);
+
+    const init = { method: 'POST', body: '{"q":1}' };
+    const res = await instrumented('https://example.com/v1/chat', init);
+
+    expect(baseFetch).toHaveBeenCalledTimes(1);
+    expect(baseFetch).toHaveBeenCalledWith('https://example.com/v1/chat', init);
+    // The Response is returned UNTOUCHED (same reference — never read/cloned).
+    expect(res).toBe(fakeResponse);
+  });
+
+  it('rethrows the base fetch error unchanged (pre-response failure)', async () => {
+    const err = Object.assign(new TypeError('fetch failed'), {
+      cause: { code: 'ECONNRESET' },
+    });
+    const baseFetch = jest.fn().mockRejectedValue(err);
+    const instrumented = createInstrumentedFetch('test', baseFetch as never);
+
+    await expect(instrumented('https://example.com/')).rejects.toBe(err);
+  });
+
+  it('defaults to the global fetch when no baseFetch is given', () => {
+    // Constructing without a baseFetch must not throw — it simply wraps global
+    // fetch (the non-chat default).
+    expect(() => createInstrumentedFetch('test')).not.toThrow();
+  });
+});

apps/server/src/integrations/ai/ai-provider-http.ts

@@ -0,0 +1,87 @@
+import { Logger } from '@nestjs/common';
+
+/**
+ * The provider HTTP fetch used by the chat path: a thin, behavior-neutral
+ * instrumentation wrapper around a supplied `fetch`.
+ *
+ * It defaults to the global `fetch`, but the chat provider passes the streaming
+ * fetch (which RAISES undici's 300s stream timeouts to a generous-but-finite
+ * silence timeout so a long agent turn is not severed mid-stream — #175). So this
+ * wrapper observes the EXACT transport a turn uses. It NEVER retries, times out,
+ * swaps the dispatcher, or reads/clones the response body — the Response is
+ * returned untouched (streaming unaffected) and any error is rethrown unchanged.
+ *
+ * Per provider HTTP call it logs: time-to-response-headers + status + request
+ * body size on success; and on a pre-response rejection the failure latency +
+ * error code/cause + request body size + the idle gap since the previous call.
+ * This telemetry is intentional and kept (it diagnoses provider connection
+ * resets / mid-stream cuts), and it is load-bearing: the streaming fetch reaches
+ * the chat provider THROUGH this wrapper, so the two are one construct.
+ *
+ * How to read the result (a long agentic turn makes one provider call per step):
+ *  - a failed turn whose last provider line is "PRE-RESPONSE FAILED ... ECONNRESET"
+ *    => the reset is in the CONNECTION phase of a step's request (the provider
+ *    never replied) — usually a poisoned keep-alive socket or the provider/middle
+ *    box resetting that request (large body / idle gap are the suspects, hence
+ *    reqBytes + idleSincePrevCall below).
+ *  - the last line is "OK status=200" and the turn still errors with NO
+ *    "PRE-RESPONSE FAILED" => the cut happened MID-STREAM (after headers), a
+ *    different failure mode.
+ *
+ * The seq/last-call timestamps are module-level, so under concurrent turns the
+ * idle-gap figure is approximate (fine for single-user diagnosis).
+ */
+export function createInstrumentedFetch(
+  context: string,
+  // The underlying fetch to instrument. Defaults to the global fetch; the chat
+  // provider passes the streaming fetch (raised, finite undici stream timeouts,
+  // #175) so the telemetry observes the SAME transport the long agent turn uses.
+  baseFetch: typeof fetch = fetch,
+): typeof fetch {
+  const logger = new Logger(context);
+  let callSeq = 0;
+  let lastCallStartedAt: number | undefined;
+
+  return async (input: Parameters<typeof fetch>[0], init?: Parameters<typeof fetch>[1]): Promise<Response> => {
+    const callId = ++callSeq;
+    const startedAt = Date.now();
+    const idleSincePrev =
+      lastCallStartedAt === undefined ? undefined : startedAt - lastCallStartedAt;
+    lastCallStartedAt = startedAt;
+    // Request body size: the chat payload is a JSON string. Used to test whether
+    // failures correlate with the large accumulated context on later agent steps.
+    const body = init?.body as unknown;
+    const bodyBytes =
+      typeof body === 'string'
+        ? body.length
+        : body instanceof Uint8Array
+          ? body.byteLength
+          : undefined;
+    try {
+      // Delegate to the base fetch; return the Response UNTOUCHED (never read/
+      // clone the body) so the streamed SSE response is unaffected.
+      const res = await baseFetch(input, init);
+      logger.log(
+        `provider HTTP: call#${callId} OK ` +
+          `headersAfter=${Date.now() - startedAt}ms status=${res.status} ` +
+          `reqBytes=${bodyBytes ?? 'n/a'} idleSincePrevCall=${idleSincePrev ?? 'n/a'}ms`,
+      );
+      return res;
+    } catch (err) {
+      // fetch() rejected => PRE-RESPONSE failure (no headers/body received yet):
+      // the connection/request phase. Log it and rethrow the SAME error.
+      const e = err as {
+        name?: string;
+        message?: string;
+        cause?: { code?: string; message?: string };
+      };
+      logger.warn(
+        `provider HTTP: call#${callId} PRE-RESPONSE FAILED ` +
+          `after=${Date.now() - startedAt}ms code=${e?.cause?.code ?? 'none'} ` +
+          `name=${e?.name ?? 'Error'} cause=${e?.cause?.message ?? e?.message ?? 'unknown'} ` +
+          `reqBytes=${bodyBytes ?? 'n/a'} idleSincePrevCall=${idleSincePrev ?? 'n/a'}ms`,
+      );
+      throw err;
+    }
+  };
+}

apps/server/src/integrations/ai/ai-provider-settings-keys.spec.ts

@@ -0,0 +1,43 @@
+import { validate } from 'class-validator';
+import { plainToInstance } from 'class-transformer';
+import { PROVIDER_SETTINGS_KEYS } from './ai.types';
+import { AI_PROVIDER_SETTINGS_ALLOWED } from '@docmost/db/repos/workspace/workspace.repo';
+import { UpdateAiSettingsDto } from './dto/update-ai-settings.dto';
+
+/**
+ * Drift guard: the writable provider-settings keys are maintained in two layers
+ * that TypeScript cannot cross-check — PROVIDER_SETTINGS_KEYS (ai.types, used by
+ * the settings service) and AI_PROVIDER_SETTINGS_ALLOWED (the generic workspace
+ * repo's SQL boundary). A key missing from the repo copy silently drops the field
+ * on persist (exactly what happened to chatApiStyle), so this asserts they match.
+ */
+describe('provider-settings key allowlist parity', () => {
+  it('the repo SQL allowlist equals PROVIDER_SETTINGS_KEYS', () => {
+    expect([...AI_PROVIDER_SETTINGS_ALLOWED].sort()).toEqual(
+      [...PROVIDER_SETTINGS_KEYS].sort(),
+    );
+  });
+});
+
+/** DTO validation for the new chatApiStyle field (@IsIn(CHAT_API_STYLES)). */
+describe('UpdateAiSettingsDto.chatApiStyle', () => {
+  const errorsFor = async (chatApiStyle: unknown) =>
+    validate(plainToInstance(UpdateAiSettingsDto, { chatApiStyle }));
+
+  it('accepts both valid values', async () => {
+    for (const v of ['openai-compatible', 'openai']) {
+      const errs = await errorsFor(v);
+      expect(errs.find((e) => e.property === 'chatApiStyle')).toBeUndefined();
+    }
+  });
+
+  it('rejects an unknown value', async () => {
+    const errs = await errorsFor('definitely-not-a-style');
+    expect(errs.find((e) => e.property === 'chatApiStyle')).toBeDefined();
+  });
+
+  it('accepts the field being omitted (optional)', async () => {
+    const errs = await validate(plainToInstance(UpdateAiSettingsDto, {}));
+    expect(errs.find((e) => e.property === 'chatApiStyle')).toBeUndefined();
+  });
+});

apps/server/src/integrations/ai/ai-settings.service.ts

@@ -14,6 +14,8 @@ import {
   MaskedAiSettings,
   ResolvedAiConfig,
   SttApiStyle,
+  ChatApiStyle,
+  PROVIDER_SETTINGS_KEYS,
 } from './ai.types';
 
 /**
@@ -24,6 +26,7 @@ import {
 export interface UpdateAiSettingsInput {
   driver?: AiDriver;
   chatModel?: string;
+  chatApiStyle?: ChatApiStyle;
   embeddingModel?: string;
   baseUrl?: string;
   embeddingBaseUrl?: string;
@@ -157,6 +160,8 @@ export class AiSettingsService {
     const config: ResolvedAiConfig = {
       driver: provider.driver,
       chatModel: provider.chatModel,
+      // Plain passthrough; getChatModel defaults unset to 'openai-compatible'.
+      chatApiStyle: provider.chatApiStyle,
       // Cheap model id for the anonymous public-share assistant; reuses the chat
       // driver/baseUrl/apiKey. Empty/unset → callers fall back to chatModel.
       publicShareChatModel: provider.publicShareChatModel,
@@ -238,6 +243,7 @@ export class AiSettingsService {
     return {
       driver: provider.driver,
       chatModel: provider.chatModel,
+      chatApiStyle: provider.chatApiStyle,
       embeddingModel: provider.embeddingModel,
       baseUrl: provider.baseUrl,
       embeddingBaseUrl: provider.embeddingBaseUrl,
@@ -275,20 +281,8 @@ export class AiSettingsService {
 
     // Persist non-secret provider fields (only those present in the partial).
     const providerPatch: Partial<AiProviderSettings> = {};
-    for (const key of [
-      'driver',
-      'chatModel',
-      'embeddingModel',
-      'baseUrl',
-      'embeddingBaseUrl',
-      'sttModel',
-      'sttBaseUrl',
-      'sttApiStyle',
-      'sttLanguage',
-      'systemPrompt',
-      'publicShareChatModel',
-      'publicShareAssistantRoleId',
-    ] as const) {
+    // Single source of truth for the writable provider keys (see ai.types).
+    for (const key of PROVIDER_SETTINGS_KEYS) {
       if (nonSecret[key] !== undefined) {
         (providerPatch as Record<string, unknown>)[key] = nonSecret[key];
       }

apps/server/src/integrations/ai/ai-streaming-fetch.spec.ts

@@ -0,0 +1,235 @@
+import * as http from 'node:http';
+import {
+  createStreamingFetch,
+  withPreResponseRetry,
+  streamTimeoutMs,
+  streamKeepAliveMs,
+  streamingDispatcherOptions,
+  isRetryableConnectError,
+} from './ai-streaming-fetch';
+
+/**
+ * #175: undici's default 300s headers/body timeouts severed long agent turns.
+ * The streaming fetch raises them to a generous-but-FINITE silence timeout (not
+ * 0 — a true hang must still break). We pin: the configured value + env override,
+ * that both dispatcher timeouts use it, and that a delayed response streams.
+ */
+describe('streamTimeoutMs', () => {
+  const ORIG = process.env.AI_STREAM_TIMEOUT_MS;
+  afterEach(() => {
+    if (ORIG === undefined) delete process.env.AI_STREAM_TIMEOUT_MS;
+    else process.env.AI_STREAM_TIMEOUT_MS = ORIG;
+  });
+
+  it('defaults to a generous-but-finite 15 minutes', () => {
+    delete process.env.AI_STREAM_TIMEOUT_MS;
+    expect(streamTimeoutMs()).toBe(900_000);
+    // Finite — NOT disabled (0 would let a hung provider leak forever).
+    expect(streamTimeoutMs()).toBeGreaterThan(0);
+    expect(Number.isFinite(streamTimeoutMs())).toBe(true);
+  });
+
+  it('honours a positive AI_STREAM_TIMEOUT_MS override', () => {
+    process.env.AI_STREAM_TIMEOUT_MS = '120000';
+    expect(streamTimeoutMs()).toBe(120000);
+  });
+
+  it('ignores an invalid / non-positive override (falls back to default)', () => {
+    for (const bad of ['0', '-5', 'abc', '']) {
+      process.env.AI_STREAM_TIMEOUT_MS = bad;
+      expect(streamTimeoutMs()).toBe(900_000);
+    }
+  });
+
+  it('applies the silence timeout + keep-alive recycle window to the dispatcher', () => {
+    delete process.env.AI_STREAM_TIMEOUT_MS;
+    delete process.env.AI_STREAM_KEEPALIVE_MS;
+    expect(streamingDispatcherOptions()).toEqual({
+      headersTimeout: 900_000,
+      bodyTimeout: 900_000,
+      keepAliveTimeout: 10_000,
+      keepAliveMaxTimeout: 10_000,
+    });
+  });
+});
+
+describe('streamKeepAliveMs', () => {
+  const ORIG = process.env.AI_STREAM_KEEPALIVE_MS;
+  afterEach(() => {
+    if (ORIG === undefined) delete process.env.AI_STREAM_KEEPALIVE_MS;
+    else process.env.AI_STREAM_KEEPALIVE_MS = ORIG;
+  });
+
+  it('defaults to 10s (recycle idle sockets so a NAT/proxy drop cannot poison reuse)', () => {
+    delete process.env.AI_STREAM_KEEPALIVE_MS;
+    expect(streamKeepAliveMs()).toBe(10_000);
+  });
+
+  it('honours a positive override and ignores invalid/non-positive', () => {
+    process.env.AI_STREAM_KEEPALIVE_MS = '4000';
+    expect(streamKeepAliveMs()).toBe(4000);
+    for (const bad of ['0', '-1', 'x', '']) {
+      process.env.AI_STREAM_KEEPALIVE_MS = bad;
+      expect(streamKeepAliveMs()).toBe(10_000);
+    }
+  });
+});
+
+describe('isRetryableConnectError', () => {
+  it('matches connection-level codes on the error or its cause', () => {
+    expect(isRetryableConnectError({ cause: { code: 'ECONNRESET' } })).toBe(true);
+    expect(isRetryableConnectError({ cause: { code: 'UND_ERR_SOCKET' } })).toBe(true);
+    expect(isRetryableConnectError({ code: 'ECONNREFUSED' })).toBe(true);
+  });
+  it('does NOT match aborts / unrelated errors', () => {
+    expect(isRetryableConnectError({ name: 'AbortError', cause: { code: 'ABORT_ERR' } })).toBe(false);
+    expect(isRetryableConnectError({ cause: { code: 'UND_ERR_HEADERS_TIMEOUT' } })).toBe(false);
+    expect(isRetryableConnectError(new Error('plain'))).toBe(false);
+    expect(isRetryableConnectError(undefined)).toBe(false);
+  });
+});
+
+describe('createStreamingFetch — against a delayed server', () => {
+  const ORIG = process.env.AI_STREAM_TIMEOUT_MS;
+  let server: http.Server;
+  let url: string;
+  // The server waits before sending ANY byte (a long time-to-first-token). It is
+  // > undici's ~1s timeout-timer granularity so a sub-second configured timeout
+  // fires deterministically in the load-bearing test below.
+  const DELAY = 1500;
+
+  beforeAll(async () => {
+    server = http.createServer((_req, res) => {
+      setTimeout(() => {
+        res.writeHead(200, { 'Content-Type': 'text/plain' });
+        res.end('ok');
+      }, DELAY);
+    });
+    await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
+    const addr = server.address() as import('node:net').AddressInfo;
+    url = `http://127.0.0.1:${addr.port}/`;
+  });
+
+  afterAll(async () => {
+    await new Promise<void>((resolve) => server.close(() => resolve()));
+  });
+
+  afterEach(() => {
+    if (ORIG === undefined) delete process.env.AI_STREAM_TIMEOUT_MS;
+    else process.env.AI_STREAM_TIMEOUT_MS = ORIG;
+  });
+
+  it('streams the delayed response at the default (generous) timeout', async () => {
+    delete process.env.AI_STREAM_TIMEOUT_MS; // default 15 min >> DELAY
+    const streamingFetch = createStreamingFetch();
+    const res = await streamingFetch(url);
+    expect(res.status).toBe(200);
+    expect(await res.text()).toBe('ok');
+  });
+
+  it('LOAD-BEARING: a sub-DELAY AI_STREAM_TIMEOUT_MS actually severs the response', async () => {
+    // Proves the configured dispatcher is wired into the fetch: with the timeout
+    // set below DELAY the call must reject with undici's headers-timeout. If the
+    // dispatcher were lost (fallback to global fetch's 300s default), the 1.5s
+    // response would slip through and this would NOT throw.
+    process.env.AI_STREAM_TIMEOUT_MS = '500';
+    const streamingFetch = createStreamingFetch();
+    let caught: unknown;
+    const startedAt = Date.now();
+    try {
+      await streamingFetch(url).then((r) => r.text());
+    } catch (e) {
+      caught = e;
+    }
+    // It rejected (a lost dispatcher -> global 300s default would NOT reject on a
+    // 1.5s response) and it did so BEFORE the response would have arrived (DELAY).
+    // Use `.name` (realm-safe) — undici's TypeError fails cross-realm instanceof.
+    expect(caught).toBeDefined();
+    expect((caught as Error)?.name).toBe('TypeError');
+    expect(Date.now() - startedAt).toBeLessThan(DELAY);
+    // When present, the undici cause is the headers timeout.
+    const code = (caught as { cause?: { code?: string } })?.cause?.code;
+    if (code) expect(code).toBe('UND_ERR_HEADERS_TIMEOUT');
+  });
+});
+
+describe('withPreResponseRetry', () => {
+  // The retry is the OUTERMOST layer (over the dispatcher-bound streaming fetch),
+  // matching ai.service's withPreResponseRetry(instrument(createStreamingFetch())).
+  // PRE_RESPONSE_CONNECT_RETRIES is 2 -> at most 3 total attempts.
+  const MAX_ATTEMPTS = 3;
+  let server: http.Server;
+  let url: string;
+  let requests = 0;
+  // 'first' resets only the first connection; 'all' resets every connection.
+  let resetMode: 'first' | 'all' = 'first';
+
+  const retryingFetch = () => withPreResponseRetry(createStreamingFetch());
+
+  beforeAll(async () => {
+    server = http.createServer((req, res) => {
+      requests += 1;
+      const shouldReset = resetMode === 'all' || requests === 1;
+      if (shouldReset) {
+        // Reset before any response byte (a poisoned/stale keep-alive socket).
+        const sock = req.socket as import('node:net').Socket & {
+          resetAndDestroy?: () => void;
+        };
+        if (typeof sock.resetAndDestroy === 'function') sock.resetAndDestroy();
+        else sock.destroy();
+        return;
+      }
+      res.writeHead(200, { 'Content-Type': 'text/plain' });
+      res.end('ok');
+    });
+    await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
+    const addr = server.address() as import('node:net').AddressInfo;
+    url = `http://127.0.0.1:${addr.port}/`;
+  });
+
+  afterAll(async () => {
+    await new Promise<void>((resolve) => server.close(() => resolve()));
+  });
+
+  beforeEach(() => {
+    requests = 0;
+    resetMode = 'first';
+  });
+
+  it('retries a pre-response reset on a fresh connection and succeeds', async () => {
+    resetMode = 'first';
+    const res = await retryingFetch()(url);
+    expect(res.status).toBe(200);
+    expect(await res.text()).toBe('ok');
+    // first request reset -> retry -> second request served.
+    expect(requests).toBe(2);
+  });
+
+  it('gives up after the retry bound and rethrows the original reset', async () => {
+    resetMode = 'all'; // every attempt resets -> retries exhaust
+    let caught: unknown;
+    try {
+      await retryingFetch()(url);
+    } catch (e) {
+      caught = e;
+    }
+    expect(caught).toBeDefined();
+    // A retryable connection error reached the caller (not swallowed).
+    expect(isRetryableConnectError(caught)).toBe(true);
+    // Bounded: exactly PRE_RESPONSE_CONNECT_RETRIES + 1 attempts hit the server
+    // (pins both the limit and that the final error propagates — guards an
+    // off-by-one or an infinite loop).
+    expect(requests).toBe(MAX_ATTEMPTS);
+  });
+
+  it('does NOT retry an aborted request (no retry storm)', async () => {
+    resetMode = 'all';
+    const ctrl = new AbortController();
+    ctrl.abort();
+    await expect(
+      retryingFetch()(url, { signal: ctrl.signal }),
+    ).rejects.toBeDefined();
+    // Pre-aborted: the request never reached the server, so nothing was retried.
+    expect(requests).toBe(0);
+  });
+});

apps/server/src/integrations/ai/ai-streaming-fetch.ts

@@ -0,0 +1,156 @@
+import { Agent } from 'undici';
+
+/**
+ * Default SILENCE timeout for streaming AI calls (15 min). Generous, but FINITE.
+ *
+ * Node's global fetch (undici) defaults headersTimeout and bodyTimeout to
+ * 300_000ms, which severed legitimate long agent turns mid-stream — surfacing as
+ * "Lost connection to the AI provider" (#175): a late step with a huge context
+ * pushes the model's time-to-first-token past 5 min, or a reasoning model pauses
+ * >5 min between chunks. We do NOT disable the timeout (0) — that would let a
+ * genuinely hung provider, with the client still connected, hang forever
+ * (abortSignal only fires on client disconnect). Instead we raise it well above
+ * any realistic gap while keeping it finite so a true hang is eventually broken.
+ *
+ * This bounds SILENCE (time-to-first-byte and the gap BETWEEN chunks), NOT total
+ * turn duration — so an arbitrarily long turn that keeps streaming bytes is never
+ * cut; only a stream that goes quiet for longer than this is treated as a hang.
+ */
+const DEFAULT_STREAM_TIMEOUT_MS = 900_000;
+
+/**
+ * Default keep-alive recycle window (10s). A pooled connection idle longer than
+ * this is CLOSED rather than reused.
+ *
+ * Long agent turns leave gaps of tens of seconds between provider calls (one
+ * call per step; a crawl/search tool runs in between). A NAT / reverse proxy /
+ * conntrack in front of the deployment silently drops an idle connection after
+ * its own timeout; undici, not knowing, then reuses that dead socket and the
+ * next request fails PRE-RESPONSE with `read ECONNRESET` (#175 prod telemetry:
+ * the resets correlate with idleSincePrevCall ~42s, while a direct path to the
+ * provider does NOT reset). Recycling idle sockets well below such a drop window
+ * means a long-gap call opens a fresh connection instead of reusing a stale one.
+ * `keepAliveMaxTimeout` also caps a server-advertised keep-alive so the provider
+ * cannot push the reuse window back up.
+ */
+const DEFAULT_STREAM_KEEPALIVE_MS = 10_000;
+
+/**
+ * How many times to retry a PRE-RESPONSE connection failure (a reset/timeout
+ * before ANY response byte) on a fresh connection. Safe because `fetch()` only
+ * rejects before the Response resolves — a started stream is never replayed.
+ */
+const PRE_RESPONSE_CONNECT_RETRIES = 2;
+
+/** undici cause codes for a connection-level failure that occurred PRE-RESPONSE. */
+const RETRYABLE_CONNECT_CODES = new Set([
+  'ECONNRESET',
+  'ECONNREFUSED',
+  'EPIPE',
+  'ETIMEDOUT',
+  'UND_ERR_SOCKET',
+  'UND_ERR_CONNECT_TIMEOUT',
+]);
+
+function positiveEnv(name: string, fallback: number): number {
+  const raw = Number(process.env[name]);
+  return Number.isFinite(raw) && raw > 0 ? raw : fallback;
+}
+
+/**
+ * The configured silence timeout (ms). Override with `AI_STREAM_TIMEOUT_MS`; a
+ * missing/invalid/non-positive value falls back to {@link DEFAULT_STREAM_TIMEOUT_MS}.
+ */
+export function streamTimeoutMs(): number {
+  return positiveEnv('AI_STREAM_TIMEOUT_MS', DEFAULT_STREAM_TIMEOUT_MS);
+}
+
+/** Keep-alive recycle window (ms). Override with `AI_STREAM_KEEPALIVE_MS`. */
+export function streamKeepAliveMs(): number {
+  return positiveEnv('AI_STREAM_KEEPALIVE_MS', DEFAULT_STREAM_KEEPALIVE_MS);
+}
+
+/**
+ * undici `Agent` options for streaming AI traffic — the (generous, finite)
+ * silence timeouts plus the keep-alive recycle window. Shared by the chat
+ * provider fetch and the external-MCP dispatcher so they behave identically.
+ */
+export function streamingDispatcherOptions(): {
+  headersTimeout: number;
+  bodyTimeout: number;
+  keepAliveTimeout: number;
+  keepAliveMaxTimeout: number;
+} {
+  const t = streamTimeoutMs();
+  const ka = streamKeepAliveMs();
+  return {
+    headersTimeout: t,
+    bodyTimeout: t,
+    keepAliveTimeout: ka,
+    keepAliveMaxTimeout: ka,
+  };
+}
+
+/** True for a connection-level error worth retrying on a fresh connection. */
+export function isRetryableConnectError(err: unknown): boolean {
+  const e = err as { code?: string; cause?: { code?: string } } | undefined;
+  const code = e?.cause?.code ?? e?.code;
+  return typeof code === 'string' && RETRYABLE_CONNECT_CODES.has(code);
+}
+
+/**
+ * Build a `fetch` for long-lived streaming AI calls (the agent chat turn) backed
+ * by a dedicated undici dispatcher (finite silence timeouts + keep-alive
+ * recycling, #175). A single shared dispatcher is returned (callers hold it for
+ * the service lifetime) so its connection pool is reused.
+ *
+ * This is the BASE transport — no retry. The chat path wraps it as
+ * `withPreResponseRetry(createInstrumentedFetch(ctx, createStreamingFetch()))`
+ * so the retry is the OUTERMOST layer and the instrumentation observes EVERY
+ * attempt (a recovered reset is still logged — see withPreResponseRetry).
+ */
+export function createStreamingFetch(): typeof fetch {
+  const dispatcher = new Agent(streamingDispatcherOptions());
+  return ((input: Parameters<typeof fetch>[0], init?: RequestInit) =>
+    fetch(input, {
+      ...(init ?? {}),
+      // `dispatcher` is an undici-specific init field (not in the DOM
+      // RequestInit type); Node's global fetch reads it. Cast to satisfy it.
+      dispatcher,
+    } as RequestInit & { dispatcher: Agent })) as typeof fetch;
+}
+
+/**
+ * Wrap a fetch so a PRE-RESPONSE connection reset (`baseFetch` rejects before the
+ * Response resolves — so nothing has streamed) is retried a few times on a fresh
+ * connection (#175). A poisoned keep-alive socket is destroyed by undici on the
+ * reset, so the retry lands on a new connection. An abort (client disconnect) is
+ * never retried.
+ *
+ * This is the OUTERMOST transport layer by design: composing it as
+ * `withPreResponseRetry(instrumentedFetch)` means every attempt — including the
+ * resets that the retry recovers from — flows through the instrumentation, so the
+ * "PRE-RESPONSE FAILED ... ECONNRESET ... idleSincePrevCall" telemetry stays
+ * visible precisely when the fix is working (and AI_STREAM_KEEPALIVE_MS can be
+ * tuned from real data). A retry INSIDE the transport would hide it.
+ */
+export function withPreResponseRetry(baseFetch: typeof fetch): typeof fetch {
+  return (async (input: Parameters<typeof fetch>[0], init?: RequestInit) => {
+    for (let attempt = 0; ; attempt++) {
+      try {
+        return await baseFetch(input, init);
+      } catch (err) {
+        const aborted = init?.signal?.aborted === true;
+        if (
+          aborted ||
+          attempt >= PRE_RESPONSE_CONNECT_RETRIES ||
+          !isRetryableConnectError(err)
+        ) {
+          throw err;
+        }
+        // Brief backoff before the fresh-connection retry.
+        await new Promise((resolve) => setTimeout(resolve, 150 * (attempt + 1)));
+      }
+    }
+  }) as typeof fetch;
+}

apps/server/src/integrations/ai/ai.service.include-usage.spec.ts

@@ -0,0 +1,58 @@
+// `.provider` alone cannot prove the openai-compatible factory was called with
+// `includeUsage: true` — a regression dropping it (which zeroes streamed token
+// usage / reasoning-token metadata) would still pass. So mock the factory and
+// assert the exact args. jest.mock is module-scoped, hence a dedicated file.
+
+const mockCompatibleModel = { provider: 'openai-compatible.chat', modelId: 'm' };
+// jest allows `mock`-prefixed vars inside a jest.mock factory.
+const mockCreateOpenAICompatible = jest.fn(
+  (_settings: unknown) => () => mockCompatibleModel,
+);
+
+jest.mock('@ai-sdk/openai-compatible', () => ({
+  createOpenAICompatible: (settings: unknown) =>
+    mockCreateOpenAICompatible(settings),
+}));
+
+import { AiService } from './ai.service';
+
+describe('AiService.getChatModel openai-compatible factory args', () => {
+  function serviceWith(chatApiStyle?: 'openai-compatible' | 'openai') {
+    const aiSettings = {
+      resolve: jest.fn().mockResolvedValue({
+        driver: 'openai',
+        chatModel: 'glm-5.2',
+        apiKey: 'the-key',
+        baseUrl: 'https://api.z.ai/v4',
+        chatApiStyle,
+      }),
+    };
+    return new AiService(
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      aiSettings as any,
+      { find: jest.fn() } as never,
+      { decryptSecret: jest.fn() } as never,
+    );
+  }
+
+  beforeEach(() => mockCreateOpenAICompatible.mockClear());
+
+  it('passes includeUsage:true plus baseURL/apiKey/fetch (default style)', async () => {
+    await serviceWith().getChatModel('ws-1'); // unset -> openai-compatible
+    expect(mockCreateOpenAICompatible).toHaveBeenCalledTimes(1);
+    expect(mockCreateOpenAICompatible).toHaveBeenCalledWith(
+      expect.objectContaining({
+        name: 'openai-compatible',
+        baseURL: 'https://api.z.ai/v4',
+        apiKey: 'the-key',
+        includeUsage: true,
+        fetch: expect.any(Function),
+      }),
+    );
+  });
+
+  it("does NOT use the openai-compatible factory for chatApiStyle 'openai'", async () => {
+    await serviceWith('openai').getChatModel('ws-1');
+    expect(mockCreateOpenAICompatible).not.toHaveBeenCalled();
+  });
+});

apps/server/src/integrations/ai/ai.service.spec.ts

@@ -285,3 +285,64 @@ describe('AiService.getChatModel role model override', () => {
     );
   });
 });
+
+/**
+ * Chat provider selection by the EXPLICIT `chatApiStyle` (NOT inferred from
+ * baseUrl): 'openai-compatible' (default) uses @ai-sdk/openai-compatible, which
+ * maps streamed reasoning_content to reasoning parts; 'openai' uses the official
+ * provider; and openai-compatible without a baseURL safely falls back to the
+ * official provider (it has no default endpoint). Asserted via `.provider`.
+ */
+describe('AiService.getChatModel chatApiStyle provider selection', () => {
+  function serviceWith(opts: {
+    baseUrl?: string;
+    chatApiStyle?: 'openai-compatible' | 'openai';
+  }) {
+    const aiSettings = {
+      resolve: jest.fn().mockResolvedValue({
+        driver: 'openai',
+        chatModel: 'glm-5.2',
+        apiKey: 'key',
+        baseUrl: opts.baseUrl,
+        chatApiStyle: opts.chatApiStyle,
+      }),
+    };
+    return new AiService(
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      aiSettings as any,
+      { find: jest.fn() } as never,
+      { decryptSecret: jest.fn() } as never,
+    );
+  }
+
+  const providerOf = async (svc: AiService) =>
+    (
+      (await svc.getChatModel('ws-1')) as { provider: string }
+    ).provider;
+
+  it("'openai-compatible' + baseURL -> openai-compatible provider", async () => {
+    expect(
+      await providerOf(
+        serviceWith({ baseUrl: 'https://api.z.ai/v4', chatApiStyle: 'openai-compatible' }),
+      ),
+    ).toContain('openai-compatible');
+  });
+
+  it("'openai' + baseURL -> official openai provider", async () => {
+    expect(
+      await providerOf(serviceWith({ baseUrl: 'https://api.z.ai/v4', chatApiStyle: 'openai' })),
+    ).toBe('openai.chat');
+  });
+
+  it('unset + baseURL -> defaults to openai-compatible', async () => {
+    expect(
+      await providerOf(serviceWith({ baseUrl: 'https://api.z.ai/v4' })),
+    ).toContain('openai-compatible');
+  });
+
+  it("'openai-compatible' WITHOUT baseURL -> safe fallback to official openai", async () => {
+    expect(
+      await providerOf(serviceWith({ chatApiStyle: 'openai-compatible' })),
+    ).toBe('openai.chat');
+  });
+});

apps/server/src/integrations/ai/ai.service.ts

@@ -7,6 +7,7 @@ import {
   type LanguageModel,
 } from 'ai';
 import { createOpenAI } from '@ai-sdk/openai';
+import { createOpenAICompatible } from '@ai-sdk/openai-compatible';
 import { createGoogleGenerativeAI } from '@ai-sdk/google';
 import { createOllama } from 'ai-sdk-ollama';
 import { AiSettingsService } from './ai-settings.service';
@@ -14,6 +15,11 @@ import { AiNotConfiguredException } from './ai-not-configured.exception';
 import { AiEmbeddingNotConfiguredException } from './ai-embedding-not-configured.exception';
 import { AiSttNotConfiguredException } from './ai-stt-not-configured.exception';
 import { describeProviderError } from './ai-error.util';
+import { createInstrumentedFetch } from './ai-provider-http';
+import {
+  createStreamingFetch,
+  withPreResponseRetry,
+} from './ai-streaming-fetch';
 import { AiProviderCredentialsRepo } from '@docmost/db/repos/ai-chat/ai-provider-credentials.repo';
 import { SecretBoxService } from '../crypto/secret-box';
 import { AiDriver } from './ai.types';
@@ -43,6 +49,17 @@ export interface ChatModelOverride {
 export class AiService {
   private readonly logger = new Logger(AiService.name);
 
+  // Provider HTTP fetch for the chat path, layered so each transport concern is
+  // observed (#175). Inside-out: the streaming fetch (finite silence timeouts +
+  // keep-alive recycling) → provider-HTTP instrumentation (logs every attempt) →
+  // pre-response connection-reset retry as the OUTERMOST layer. Retry-outer means
+  // a reset the retry recovers from is still logged with its idle-gap, instead of
+  // collapsing into a clean "OK". Held for the service lifetime to reuse the
+  // streaming dispatcher's connection pool.
+  private readonly aiProviderFetch = withPreResponseRetry(
+    createInstrumentedFetch('AiService:provider-http', createStreamingFetch()),
+  );
+
   constructor(
     private readonly aiSettings: AiSettingsService,
     private readonly aiProviderCredentialsRepo: AiProviderCredentialsRepo,
@@ -83,6 +100,10 @@ export class AiService {
 
     let apiKey = cfg.apiKey;
     let baseUrl = cfg.baseUrl;
+    // Chat provider implementation, chosen EXPLICITLY by the admin (not inferred
+    // from baseUrl). Unset → 'openai-compatible' so reasoning is surfaced by
+    // default for this fork's openai+baseUrl setups.
+    const chatApiStyle = cfg.chatApiStyle ?? 'openai-compatible';
 
     // A driver override that differs from the workspace driver needs that
     // driver's own creds (the workspace driver's key would be wrong/absent).
@@ -133,14 +154,41 @@ export class AiService {
     }
 
     switch (driver) {
-      case 'openai':
-        // baseURL (when set) covers openai-compatible endpoints. Use Chat
-        // Completions (/chat/completions) — the portable OpenAI-compatible
-        // endpoint. The default callable createOpenAI(...)(model) targets the
-        // Responses API (/responses), which OpenAI-compatible gateways
-        // (OpenRouter, etc.) reject on multi-turn requests (history with
-        // assistant messages) → 400.
-        return createOpenAI({ apiKey, baseURL: baseUrl }).chat(chatModel);
+      case 'openai': {
+        // The provider implementation is chosen by the admin's `chatApiStyle`
+        // (NOT inferred from baseUrl — a custom URL can front real OpenAI too).
+        // Both branches hit Chat Completions (/chat/completions); the provider
+        // fetch is the instrumented streaming fetch (finite-but-generous stream
+        // timeouts, #175).
+        //
+        // 'openai-compatible' (default) maps the third-party provider's streamed
+        // `reasoning_content` to reasoning parts (z.ai/GLM, DeepSeek, ...) — the
+        // point of #175. It has no default endpoint, so it requires a baseURL;
+        // when there is none (real OpenAI, or a role's cross-driver override that
+        // cleared baseUrl) we fall back to the official provider.
+        if (chatApiStyle === 'openai-compatible' && baseUrl) {
+          return createOpenAICompatible({
+            name: 'openai-compatible',
+            apiKey,
+            baseURL: baseUrl,
+            // Keep streamed token usage (stream_options.include_usage): without
+            // it @ai-sdk/openai-compatible omits usage, zeroing the live token
+            // counter and reasoning-token metadata. The official provider always
+            // sent it, so this preserves parity.
+            includeUsage: true,
+            fetch: this.aiProviderFetch,
+          })(chatModel);
+        }
+        // Official @ai-sdk/openai: real-OpenAI reasoning-model request shaping;
+        // `.chat()` targets Chat Completions (the default callable targets the
+        // Responses API, which openai-compatible gateways 400 on multi-turn
+        // history). In this fork baseUrl is normally set; undefined = real OpenAI.
+        return createOpenAI({
+          apiKey,
+          baseURL: baseUrl,
+          fetch: this.aiProviderFetch,
+        }).chat(chatModel);
+      }
       case 'gemini':
         return createGoogleGenerativeAI({ apiKey })(chatModel);
       case 'ollama':

apps/server/src/integrations/ai/ai.types.ts

@@ -16,6 +16,15 @@ export const AI_DRIVERS: AiDriver[] = ['openai', 'gemini', 'ollama'];
 export type SttApiStyle = 'multipart' | 'json';
 export const STT_API_STYLES: SttApiStyle[] = ['multipart', 'json'];
 
+// Chat provider implementation for the `openai` driver. Chosen explicitly by the
+// admin (NOT inferred from baseUrl — a custom URL can front real OpenAI too).
+// 'openai-compatible' = @ai-sdk/openai-compatible: maps streamed
+//   `reasoning_content` to reasoning parts (z.ai/GLM, DeepSeek, OpenRouter, ...).
+// 'openai' = official @ai-sdk/openai: real-OpenAI reasoning-model request shaping
+//   (max_completion_tokens, the 'developer' role), no third-party reasoning map.
+export type ChatApiStyle = 'openai-compatible' | 'openai';
+export const CHAT_API_STYLES: ChatApiStyle[] = ['openai-compatible', 'openai'];
+
 /**
  * Non-secret provider settings persisted under `settings.ai.provider`.
  * The API key is intentionally absent here.
@@ -23,6 +32,9 @@ export const STT_API_STYLES: SttApiStyle[] = ['multipart', 'json'];
 export interface AiProviderSettings {
   driver: AiDriver;
   chatModel: string;
+  // Chat provider implementation for the `openai` driver. Unset → defaults to
+  // 'openai-compatible' (so reasoning is surfaced by default). See ChatApiStyle.
+  chatApiStyle?: ChatApiStyle;
   embeddingModel?: string;
   baseUrl?: string;
   // Embedding-specific base URL. Falls back to `baseUrl` when empty/unset.
@@ -45,6 +57,34 @@ export interface AiProviderSettings {
   publicShareAssistantRoleId?: string;
 }
 
+/**
+ * The persisted, non-secret provider setting keys — the SINGLE source of truth
+ * for which fields a settings update may write through to `settings.ai.provider`.
+ * `satisfies readonly (keyof AiProviderSettings)[]` makes the compiler reject a
+ * typo or a key that is not a real provider setting.
+ *
+ * The settings service consumes this directly. The generic workspace repo cannot
+ * import AI types, so it keeps its own copy of the same keys, guarded by a parity
+ * test against this constant (so any future drift fails in CI, not silently in
+ * prod — a missing key there validates fine, passes the service, and is then
+ * dropped at the SQL boundary with no error).
+ */
+export const PROVIDER_SETTINGS_KEYS = [
+  'driver',
+  'chatModel',
+  'chatApiStyle',
+  'embeddingModel',
+  'baseUrl',
+  'embeddingBaseUrl',
+  'sttModel',
+  'sttBaseUrl',
+  'sttApiStyle',
+  'sttLanguage',
+  'systemPrompt',
+  'publicShareChatModel',
+  'publicShareAssistantRoleId',
+] as const satisfies readonly (keyof AiProviderSettings)[];
+
 /**
  * Fully resolved provider config, including the decrypted API key for the
  * stored driver. Returned by `AiSettingsService.resolve`. The keys are held in
@@ -76,6 +116,7 @@ export interface ResolvedAiConfig extends Partial<AiProviderSettings> {
 export interface MaskedAiSettings {
   driver?: AiDriver;
   chatModel?: string;
+  chatApiStyle?: ChatApiStyle;
   embeddingModel?: string;
   baseUrl?: string;
   embeddingBaseUrl?: string;

apps/server/src/integrations/ai/dto/update-ai-settings.dto.ts

@@ -1,5 +1,12 @@
 import { IsIn, IsOptional, IsString } from 'class-validator';
-import { AI_DRIVERS, AiDriver, STT_API_STYLES, SttApiStyle } from '../ai.types';
+import {
+  AI_DRIVERS,
+  AiDriver,
+  CHAT_API_STYLES,
+  ChatApiStyle,
+  STT_API_STYLES,
+  SttApiStyle,
+} from '../ai.types';
 
 /**
  * Admin update payload for the workspace AI provider settings.
@@ -18,6 +25,10 @@ export class UpdateAiSettingsDto {
   @IsString()
   chatModel?: string;
 
+  @IsOptional()
+  @IsIn(CHAT_API_STYLES)
+  chatApiStyle?: ChatApiStyle;
+
   @IsOptional()
   @IsString()
   embeddingModel?: string;

packages/editor-ext/src/lib/footnote/footnote-markdown.test.ts

@@ -55,10 +55,11 @@ describe("footnote markdown round-trip", () => {
     expect(html).not.toContain("data-footnote-def");
   });
 
-  it("extractFootnoteDefinitions de-duplicates colliding ids and rewrites markers", () => {
-    // Two definitions share id `d`, and the body has two `[^d]` markers. The
-    // output must keep BOTH definitions with DISTINCT ids and rewrite the second
-    // marker so the (reference, definition) pairing stays 1:1.
+  it("extractFootnoteDefinitions keeps the FIRST duplicate definition and reuses markers", () => {
+    // Two definitions share id `d`, and the body has two `[^d]` markers. Under
+    // the import model (#166) duplicate definition ids are FIRST-WINS: only the
+    // first definition is kept; markers are NEVER rewritten, so the two `[^d]`
+    // references reuse the single footnote.
     const md = [
       "See here[^d] and there[^d].",
       "",
@@ -68,30 +69,23 @@ describe("footnote markdown round-trip", () => {
 
     const { body, section } = extractFootnoteDefinitions(md);
 
-    // Pull out the def ids from the section in order.
     const defIds = Array.from(
       section.matchAll(/data-footnote-def data-id="([^"]+)"/g),
     ).map((m) => m[1]);
-    expect(defIds.length).toBe(2);
-    expect(new Set(defIds).size).toBe(2); // distinct
-    expect(defIds[0]).toBe("d"); // first definition keeps the id
-
-    // Both definition texts survive.
+    expect(defIds).toEqual(["d"]); // first-wins: one definition
     expect(section).toContain("first");
-    expect(section).toContain("second");
+    expect(section).not.toContain("second"); // duplicate dropped
 
-    // The body still has two markers, now pointing at the two distinct ids.
+    // Both markers stay `[^d]` (reuse) — no `d__2` minting.
     const refIds = Array.from(body.matchAll(/\[\^([^\]\s]+)\]/g)).map(
       (m) => m[1],
     );
-    expect(refIds.length).toBe(2);
-    expect(refIds.sort()).toEqual(defIds.sort());
+    expect(refIds).toEqual(["d", "d"]);
   });
 
-  it("extractFootnoteDefinitions dedups DETERMINISTICALLY (same input -> same ids)", () => {
-    // The derived id must be a pure function of the input markdown so importing
-    // the same source twice (or via the editor and the MCP mirror) yields
-    // identical ids — never random/time-based.
+  it("extractFootnoteDefinitions is DETERMINISTIC and stable (same input -> same output)", () => {
+    // The output must be a pure function of the input markdown so importing the
+    // same source twice (or via the editor and the MCP mirror) is identical.
     const md = [
       "See[^d] one[^d] two[^d].",
       "",
@@ -113,15 +107,13 @@ describe("footnote markdown round-trip", () => {
 
     const a = run();
     const b = run();
-    // Identical across runs (this is what would FAIL on the random-id version).
-    expect(a.defIds).toEqual(b.defIds);
-    expect(a.refIds).toEqual(b.refIds);
-    // Deterministic derived scheme: keeper "d", duplicates "d__2", "d__3".
-    expect(a.defIds).toEqual(["d", "d__2", "d__3"]);
-    expect(a.refIds.sort()).toEqual(a.defIds.sort());
+    expect(a).toEqual(b);
+    // First-wins: one kept definition `d`; all three reuse markers stay `d`.
+    expect(a.defIds).toEqual(["d"]);
+    expect(a.refIds).toEqual(["d", "d", "d"]);
   });
 
-  it("markdownToHtml with duplicate ids renders two distinct footnote defs", async () => {
+  it("markdownToHtml with a reused id renders ONE shared footnote def", async () => {
     const md = [
       "See here[^d] and there[^d].",
       "",
@@ -132,9 +124,8 @@ describe("footnote markdown round-trip", () => {
     const defIds = Array.from(
       html.matchAll(/data-footnote-def data-id="([^"]+)"/g),
     ).map((m) => m[1]);
-    expect(defIds.length).toBe(2);
-    expect(new Set(defIds).size).toBe(2);
+    expect(defIds).toEqual(["d"]); // one shared definition
     expect(html).toContain("first");
-    expect(html).toContain("second");
+    expect(html).not.toContain("second");
   });
 });

packages/editor-ext/src/lib/footnote/footnote-paste.test.ts

@@ -0,0 +1,226 @@
+import { describe, it, expect } from "vitest";
+import { Editor } from "@tiptap/core";
+import { Document } from "@tiptap/extension-document";
+import { Paragraph } from "@tiptap/extension-paragraph";
+import { Text } from "@tiptap/extension-text";
+import { Node as PMNode, Fragment, Slice } from "@tiptap/pm/model";
+import { FootnoteReference } from "./footnote-reference";
+import { FootnotesList } from "./footnotes-list";
+import { FootnoteDefinition } from "./footnote-definition";
+import { footnotePastePlugin } from "./footnote-sync";
+import {
+  FOOTNOTE_REFERENCE_NAME,
+  FOOTNOTE_DEFINITION_NAME,
+  FOOTNOTES_LIST_NAME,
+} from "./footnote-util";
+
+// transformPasted reuse semantics (#166): a pasted reference to an id that
+// already exists must KEEP the id (reuse → resolves to the existing footnote);
+// only a pasted DEFINITION that collides is re-id'd (it would otherwise clobber
+// the existing definition's text), and its paired references follow it.
+
+const extensions = [
+  Document,
+  Paragraph,
+  Text,
+  FootnoteReference,
+  FootnotesList,
+  FootnoteDefinition,
+];
+
+/** An editor whose doc already contains footnote "a" (ref + definition). */
+function makeEditorWithFootnoteA() {
+  return new Editor({
+    extensions,
+    content: {
+      type: "doc",
+      content: [
+        {
+          type: "paragraph",
+          content: [
+            { type: "text", text: "x" },
+            { type: FOOTNOTE_REFERENCE_NAME, attrs: { id: "a" } },
+          ],
+        },
+        {
+          type: FOOTNOTES_LIST_NAME,
+          content: [
+            {
+              type: FOOTNOTE_DEFINITION_NAME,
+              attrs: { id: "a" },
+              content: [
+                { type: "paragraph", content: [{ type: "text", text: "note A" }] },
+              ],
+            },
+          ],
+        },
+      ],
+    },
+  });
+}
+
+/** Run footnotePastePlugin's transformPasted against the editor's current doc. */
+function paste(editor: Editor, slice: Slice): Slice {
+  const plugin = footnotePastePlugin();
+  return plugin.props!.transformPasted!(slice, editor.view);
+}
+
+/** Collect the ids of footnote refs/defs in a slice, in order (single DFS). */
+function sliceFootnoteIds(slice: Slice): Array<{ kind: string; id: string }> {
+  const out: Array<{ kind: string; id: string }> = [];
+  const walk = (frag: Fragment) => {
+    frag.forEach((node: PMNode) => {
+      if (node.type.name === FOOTNOTE_REFERENCE_NAME)
+        out.push({ kind: "ref", id: node.attrs.id });
+      if (node.type.name === FOOTNOTE_DEFINITION_NAME)
+        out.push({ kind: "def", id: node.attrs.id });
+      walk(node.content);
+    });
+  };
+  walk(slice.content);
+  return out;
+}
+
+describe("footnotePastePlugin — reuse-aware id remap", () => {
+  it("keeps a pasted lone reference to an existing id (reuse, no remap)", () => {
+    const editor = makeEditorWithFootnoteA();
+    const { schema } = editor;
+    // Paste: a paragraph containing only a reference to the existing id "a".
+    const slice = new Slice(
+      Fragment.from(
+        schema.nodes.paragraph.create(null, [
+          schema.text("see "),
+          schema.nodes[FOOTNOTE_REFERENCE_NAME].create({ id: "a" }),
+        ]),
+      ),
+      0,
+      0,
+    );
+    const out = paste(editor, slice);
+    // The reference keeps id "a" so it reuses the existing footnote.
+    expect(sliceFootnoteIds(out)).toEqual([{ kind: "ref", id: "a" }]);
+    editor.destroy();
+  });
+
+  it("re-ids a pasted DEFINITION (and its paired reference) that collides", () => {
+    const editor = makeEditorWithFootnoteA();
+    const { schema } = editor;
+    // Paste: a reference AND a definition both carrying the existing id "a". The
+    // definition would clobber the existing one, so both are remapped together.
+    const slice = new Slice(
+      Fragment.fromArray([
+        schema.nodes.paragraph.create(null, [
+          schema.text("dup "),
+          schema.nodes[FOOTNOTE_REFERENCE_NAME].create({ id: "a" }),
+        ]),
+        schema.nodes[FOOTNOTES_LIST_NAME].create(null, [
+          schema.nodes[FOOTNOTE_DEFINITION_NAME].create({ id: "a" }, [
+            schema.nodes.paragraph.create(null, [schema.text("pasted note")]),
+          ]),
+        ]),
+      ]),
+      0,
+      0,
+    );
+    const out = paste(editor, slice);
+    const ids = sliceFootnoteIds(out);
+    // Both the pasted ref and def were remapped to the SAME fresh id (paired),
+    // and it is the deterministic derived id (not "a").
+    const remappedIds = new Set(ids.map((x) => x.id));
+    expect(remappedIds.size).toBe(1);
+    expect(remappedIds.has("a")).toBe(false);
+    expect([...remappedIds][0]).toBe("a__2");
+    editor.destroy();
+  });
+
+  it("re-ids TWO colliding pasted definitions to DISTINCT ids (reservation works)", () => {
+    // Existing doc has footnotes "a" and "b". Paste a slice that defines BOTH —
+    // each must get its own fresh id; the reservation (existing.add(newId)) keeps
+    // the second from deriving onto the first's new id.
+    const editor = new Editor({
+      extensions,
+      content: {
+        type: "doc",
+        content: [
+          {
+            type: "paragraph",
+            content: [
+              { type: FOOTNOTE_REFERENCE_NAME, attrs: { id: "a" } },
+              { type: FOOTNOTE_REFERENCE_NAME, attrs: { id: "b" } },
+            ],
+          },
+          {
+            type: FOOTNOTES_LIST_NAME,
+            content: [
+              {
+                type: FOOTNOTE_DEFINITION_NAME,
+                attrs: { id: "a" },
+                content: [{ type: "paragraph", content: [{ type: "text", text: "A" }] }],
+              },
+              {
+                type: FOOTNOTE_DEFINITION_NAME,
+                attrs: { id: "b" },
+                content: [{ type: "paragraph", content: [{ type: "text", text: "B" }] }],
+              },
+            ],
+          },
+        ],
+      },
+    });
+    const { schema } = editor;
+    const slice = new Slice(
+      Fragment.fromArray([
+        schema.nodes.paragraph.create(null, [
+          schema.nodes[FOOTNOTE_REFERENCE_NAME].create({ id: "a" }),
+          schema.nodes[FOOTNOTE_REFERENCE_NAME].create({ id: "b" }),
+        ]),
+        schema.nodes[FOOTNOTES_LIST_NAME].create(null, [
+          schema.nodes[FOOTNOTE_DEFINITION_NAME].create({ id: "a" }, [
+            schema.nodes.paragraph.create(null, [schema.text("pasted A")]),
+          ]),
+          schema.nodes[FOOTNOTE_DEFINITION_NAME].create({ id: "b" }, [
+            schema.nodes.paragraph.create(null, [schema.text("pasted B")]),
+          ]),
+        ]),
+      ]),
+      0,
+      0,
+    );
+    const out = paste(editor, slice);
+    const ids = sliceFootnoteIds(out);
+    const distinct = new Set(ids.map((x) => x.id));
+    // Two ids, both remapped off the originals, and distinct from each other.
+    expect(distinct.size).toBe(2);
+    expect(distinct.has("a")).toBe(false);
+    expect(distinct.has("b")).toBe(false);
+    expect([...distinct].sort()).toEqual(["a__2", "b__2"]);
+    editor.destroy();
+  });
+
+  it("leaves the slice untouched when no pasted definition collides", () => {
+    const editor = makeEditorWithFootnoteA();
+    const { schema } = editor;
+    // A pasted reference+definition for a BRAND-NEW id "b" — no collision.
+    const slice = new Slice(
+      Fragment.fromArray([
+        schema.nodes.paragraph.create(null, [
+          schema.text("new "),
+          schema.nodes[FOOTNOTE_REFERENCE_NAME].create({ id: "b" }),
+        ]),
+        schema.nodes[FOOTNOTES_LIST_NAME].create(null, [
+          schema.nodes[FOOTNOTE_DEFINITION_NAME].create({ id: "b" }, [
+            schema.nodes.paragraph.create(null, [schema.text("note B")]),
+          ]),
+        ]),
+      ]),
+      0,
+      0,
+    );
+    const out = paste(editor, slice);
+    expect(sliceFootnoteIds(out)).toEqual([
+      { kind: "ref", id: "b" },
+      { kind: "def", id: "b" },
+    ]);
+    editor.destroy();
+  });
+});

packages/editor-ext/src/lib/footnote/footnote-sync.ts

@@ -29,9 +29,9 @@ interface DefOccurrence {
 
 interface FootnoteScan {
   /**
-   * Every reference occurrence in document order (NOT de-duplicated). Needed so
-   * that duplicate ids — which would otherwise be silently collapsed — can be
-   * detected and (together with their definitions) re-id'd instead of dropped.
+   * Every reference occurrence in document order (NOT de-duplicated). Repeated
+   * ids are kept so the FIRST appearance fixes definition order; later repeats
+   * are reuse (same footnote) and are never re-id'd.
    */
   refOccurrences: RefOccurrence[];
   /**
@@ -67,77 +67,66 @@ function scan(doc: ProseMirrorNode): FootnoteScan {
 }
 
 /**
- * Result of resolving id collisions: a 1:1, de-duplicated pairing plan plus the
- * concrete reference re-id edits that must be applied to the body so the doc no
- * longer contains two footnotes sharing a single id.
+ * Result of resolving the footnote id topology: the distinct reference order and
+ * one definition node per id.
  *
- * The overriding invariant is that NO definition is ever dropped here: every
- * definition occurrence ends up with a unique id and therefore survives the
- * canonical rebuild. Duplicate references are likewise re-id'd (and paired with
- * a duplicate definition when one exists) so importing/pasting `[^d]` twice with
- * two `[^d]:` definitions yields TWO distinct footnotes rather than one.
+ * References are NEVER re-id'd here — repeated ids are REUSE (one footnote). Only
+ * duplicate DEFINITIONS are re-id'd; lacking a matching reference, a re-id'd
+ * duplicate is then dropped by the orphan policy. No definition is ever dropped
+ * for COLLIDING — only for being an orphan.
  */
 interface CollisionPlan {
   /**
-   * Reference ids in document order, de-duplicated AFTER re-id. This is the
-   * source of truth for definition order/numbering, exactly as before — only
-   * now collisions have been resolved so it no longer hides duplicates.
+   * Distinct reference ids in document order (first appearance). Repeated ids
+   * are reuse and collapse to a single entry. Source of truth for definition
+   * order/numbering.
    */
   referenceIds: string[];
-  /** id -> definition node, after duplicates were re-id'd. One entry per id. */
+  /** id -> definition node, after duplicate definitions were re-id'd. One per id. */
   definitions: Map<string, ProseMirrorNode>;
-  /**
-   * Body reference re-id edits to apply (position of a reference node -> the
-   * fresh id it must carry). Empty when there are no colliding references.
-   */
-  refReids: Array<{ pos: number; node: ProseMirrorNode; newId: string }>;
-  /** True when any collision required a re-id (refs and/or defs). */
+  /** True when a duplicate definition required a re-id. */
   changed: boolean;
 }
 
 /**
- * Resolve duplicate-id collisions among references and definitions WITHOUT ever
- * dropping a definition.
+ * Resolve the footnote id topology WITHOUT ever dropping a definition.
  *
- * Strategy:
- *  - Walk references in document order. The FIRST reference for an id keeps it.
- *    Any later reference sharing that id is a duplicate and gets a fresh unique
- *    id; if a still-unclaimed duplicate definition with the original id exists,
- *    it is re-id'd to the SAME fresh id so the (ref, def) pair stays matched.
- *  - Walk definitions in document order. The FIRST definition for an id keeps
- *    it; later duplicates that were not already claimed by a duplicate reference
- *    get their own fresh unique id (surviving as a distinct footnote/orphan).
+ * Reference REUSE (Pandoc semantics, #166): repeated `[^a]` references that share
+ * an id are the SAME footnote — they get one number and one definition and are
+ * NEVER re-id'd. So the reference walk only records the FIRST occurrence of each
+ * id (de-duplicating in document order); later occurrences are reuse and produce
+ * no mutation at all.
  *
- * Re-id determinism: every fresh id is DERIVED from document state via
- * deriveFootnoteId (e.g. `X__2`, `X__3`, collision-bumped against the set of ids
- * already present) — NEVER random/time-based. Because the sync plugin runs
- * identically on every collaborating client, a deterministic re-id is the only
- * way they can converge on the SAME ids; a random id (the previous
- * implementation) made two clients editing the same duplicate-id document mint
- * DIFFERENT ids for the same duplicate, causing permanent Yjs divergence.
+ * Duplicate DEFINITIONS (two `[^d]:` nodes sharing an id reaching the LIVE editor
+ * via paste/collab merge) keep the never-lose policy: the first keeps the id, and
+ * each later duplicate is re-id'd to a DETERMINISTIC fresh id (deriveFootnoteId:
+ * `X__2`, `X__3`, collision-bumped) so it survives as a distinct footnote — which,
+ * having no matching reference, then falls under the normal orphan policy. It is
+ * only ever dropped for lacking a reference, never for colliding. The IMPORT
+ * paths (footnote.marked.ts / MCP extractFootnotes) instead apply first-wins +
+ * drop + warn for duplicate definitions; that divergence is intentional — import
+ * is an agent-authored artifact we sanitize, the editor is live user data we must
+ * not lose.
+ *
+ * Re-id determinism: every fresh id is DERIVED from document state, NEVER
+ * random/time-based, because the sync plugin runs identically on every
+ * collaborating client and a random id would make two clients mint DIFFERENT ids
+ * for the same duplicate, causing permanent Yjs divergence.
  */
 function resolveCollisions(scan: FootnoteScan): CollisionPlan {
   const definitions = new Map<string, ProseMirrorNode>();
-  const refReids: Array<{
-    pos: number;
-    node: ProseMirrorNode;
-    newId: string;
-  }> = [];
   const referenceIds: string[] = [];
   const seenRefIds = new Set<string>();
   let changed = false;
 
-  // `taken` is the set of every id that must be avoided when minting a derived
-  // id: all original reference + definition ids in the document PLUS every id we
-  // mint during this pass. It is pure document state, so the derivation stays
-  // deterministic across clients. Per-original occurrence counters make the k-th
-  // duplicate of `X` deterministically become `X__2`, `X__3`, ...
+  // `taken` is the set of every id to avoid when minting a derived id for a
+  // duplicate definition: all original reference + definition ids PLUS every id
+  // minted in this pass. Pure document state, so the derivation is deterministic
+  // across clients.
   const taken = new Set<string>();
   for (const occ of scan.refOccurrences) taken.add(occ.id);
   for (const occ of scan.defOccurrences) taken.add(occ.id);
   const occurrenceOf = new Map<string, number>();
-  // Mint a deterministic unique id for a duplicate of `originalId`. The first
-  // duplicate is occurrence 2 (the keeper is occurrence 1), then 3, 4, ...
   const mintId = (originalId: string): string => {
     const next = (occurrenceOf.get(originalId) ?? 1) + 1;
     occurrenceOf.set(originalId, next);
@@ -146,70 +135,30 @@ function resolveCollisions(scan: FootnoteScan): CollisionPlan {
     return id;
   };
 
-  // Bucket definition occurrences by their original id so a duplicate reference
-  // can claim a matching (as-yet-unclaimed) duplicate definition and re-id the
-  // pair together. defByOriginalId[id] is consumed front-to-back.
-  const defByOriginalId = new Map<string, DefOccurrence[]>();
-  for (const occ of scan.defOccurrences) {
-    const arr = defByOriginalId.get(occ.id);
-    if (arr) arr.push(occ);
-    else defByOriginalId.set(occ.id, [occ]);
-  }
-  // The FIRST definition for each id is the canonical keeper of that id.
-  const claimed = new Set<DefOccurrence>();
-
+  // References: record each DISTINCT id once, in first-appearance order. Repeated
+  // ids are reuse — nothing to mint, nothing to re-id.
   for (const ref of scan.refOccurrences) {
     if (!seenRefIds.has(ref.id)) {
-      // First reference with this id keeps it.
       seenRefIds.add(ref.id);
       referenceIds.push(ref.id);
-      continue;
-    }
-    // Duplicate reference: assign a deterministic derived id. Pair it with the
-    // next unclaimed duplicate definition (NOT the first keeper) carrying the
-    // same original id, if one exists, so the (ref, def) pairing is preserved
-    // 1:1.
-    const newId = mintId(ref.id);
-    refReids.push({ pos: ref.pos, node: ref.node, newId });
-    seenRefIds.add(newId);
-    referenceIds.push(newId);
-    changed = true;
-
-    const candidates = defByOriginalId.get(ref.id) ?? [];
-    // Skip the first occurrence (it keeps the original id); pick the first
-    // duplicate not already claimed.
-    for (let i = 1; i < candidates.length; i++) {
-      const cand = candidates[i];
-      if (!claimed.has(cand)) {
-        claimed.add(cand);
-        definitions.set(newId, cand.node);
-        break;
-      }
     }
   }
 
-  // Now place every definition under a unique id. The first occurrence of each
-  // original id keeps it; remaining duplicates either were paired with a
-  // duplicate reference above (already placed) or get a fresh standalone id.
+  // Definitions: the first occurrence of each id keeps it; a later duplicate is
+  // re-id'd deterministically so it is never silently dropped (never-lose).
   const seenDefIds = new Set<string>();
   for (const occ of scan.defOccurrences) {
-    if (claimed.has(occ)) continue; // already placed against a duplicate ref id
     if (!seenDefIds.has(occ.id)) {
       seenDefIds.add(occ.id);
       definitions.set(occ.id, occ.node);
     } else {
-      // Duplicate definition with no duplicate reference to pair with: keep it
-      // with a deterministic derived id so it is NEVER silently dropped. (It
-      // becomes an orphan and is then subject to the normal orphan policy — but
-      // only ever because it has no matching reference, never because it
-      // collided.)
       const newId = mintId(occ.id);
       definitions.set(newId, occ.node);
       changed = true;
     }
   }
 
-  return { referenceIds, definitions, refReids, changed };
+  return { referenceIds, definitions, changed };
 }
 
 /**
@@ -245,14 +194,13 @@ function resolveCollisions(scan: FootnoteScan): CollisionPlan {
  * ping-pong forever (list moved to end -> trailing paragraph appended -> list
  * no longer last -> moved again ...).
  *
- * Duplicate-id collisions (two references and/or two definitions sharing one
- * id — produced by importing `[^d]: a` / `[^d]: b`, or by pasting/duplicating a
- * reference+definition pair) are resolved up front by resolveCollisions(): the
- * duplicates are re-id'd to fresh unique ids so BOTH survive as distinct
- * footnotes. This guarantees the overriding invariant — no footnoteDefinition is
- * ever silently deleted by this automatic (addToHistory:false) transaction. A
+ * The id topology is resolved up front by resolveCollisions() (#166): repeated
+ * references sharing an id are REUSE — one footnote, never re-id'd — while a
+ * duplicate DEFINITION (from pasting/duplicating a definition, or a collab merge)
+ * is re-id'd to a fresh unique id. No footnoteDefinition is ever silently deleted
+ * by this automatic (addToHistory:false) transaction because of a COLLISION; a
  * definition is only ever removed when it has NO matching reference (orphan
- * policy), never because its id collided with another.
+ * policy) — which is also what then drops a re-id'd duplicate definition.
  */
 export function footnoteSyncPlugin(
   isRemoteTransaction?: (tr: Transaction) => boolean,
@@ -283,18 +231,16 @@ export function footnoteSyncPlugin(
 
       const info = scan(doc);
 
-      // 0) Resolve duplicate-id collisions (two references and/or two
-      //    definitions sharing one id) by re-id'ing duplicates to fresh unique
-      //    ids. This is the critical defense: the old last-wins Map silently
-      //    dropped all but the last definition for a shared id; here EVERY
-      //    definition survives with a unique id, and duplicate references are
-      //    paired with duplicate definitions so two same-id imports/pastes yield
-      //    two distinct footnotes instead of one.
+      // 0) Resolve the id topology (#166): repeated references that share an id
+      //    are REUSE — collapsed to one entry in `referenceIds`, never re-id'd —
+      //    while a duplicate DEFINITION is re-id'd to a fresh deterministic id
+      //    (and, lacking a matching reference, removed by the orphan policy
+      //    below). No definition is dropped for COLLIDING, only for being orphan.
       const plan = resolveCollisions(info);
       const referenceIds = plan.referenceIds;
 
-      // The set of ids that must have a definition, in reference order (after
-      // collision re-id). De-duplicated already by resolveCollisions.
+      // The set of ids that must have a definition, in reference order.
+      // De-duplicated already by resolveCollisions.
       const referenceIdSet = new Set(referenceIds);
 
       // 1) For each definition occurrence, compute the id it should END UP with
@@ -397,21 +343,15 @@ export function footnoteSyncPlugin(
 
       // 6) Apply the targeted, minimal mutations in ONE transaction. We never
       //    delete-and-recreate an unchanged definition subtree; we only:
-      //      (a) re-id specific colliding references and definitions (attr-only),
+      //      (a) re-id colliding definitions (attr-only),
       //      (b) delete genuine orphan definitions and extra/empty lists,
       //      (c) insert genuinely-missing empty definitions and migrate defs out
       //          of extra lists into the primary list,
       //      (d) create the primary list if references exist but none does yet.
+      //    References are never re-id'd (reuse), so there is no reference edit.
       const tr = newState.tr;
 
-      // 6a) Re-id colliding references (inline atoms: attr-only, size-stable).
-      for (const reid of plan.refReids) {
-        tr.setNodeMarkup(tr.mapping.map(reid.pos), undefined, {
-          ...reid.node.attrs,
-          id: reid.newId,
-        });
-      }
-      // 6b) Re-id colliding definitions IN PLACE (attr-only). This preserves the
+      // 6a) Re-id colliding definitions IN PLACE (attr-only). This preserves the
       //     definition's content subtree — never delete+recreate it.
       for (const reid of defReidsToApply) {
         tr.setNodeMarkup(tr.mapping.map(reid.pos), undefined, {
@@ -546,13 +486,17 @@ export const footnotePastePluginKey = new PluginKey("footnotePaste");
  * Without this, pasting a reference+definition pair copied from elsewhere — or
  * duplicating one in place — would merge with (or clobber) the existing footnote
  * of the same id. The schema-sync plugin already guarantees no definition is
- * ever silently deleted after the fact (it re-id's collisions), but regenerating
- * at paste time keeps the pasted footnote cleanly separate from the start and
- * avoids any transient merge.
+ * ever silently deleted after the fact (it re-id's duplicate definitions), but
+ * regenerating at paste time keeps the pasted footnote cleanly separate from the
+ * start and avoids any transient merge.
  *
- * Only COLLIDING ids are remapped: a self-paste of a lone reference whose id is
- * not present elsewhere is left untouched (so it still resolves to its existing
- * definition).
+ * REUSE-aware (#166): only a colliding DEFINITION forces a remap. Pasting a lone
+ * reference whose id already exists is REUSE — it must keep the id so it resolves
+ * to the existing footnote (one number, shared definition). So we remap an id
+ * only when the pasted slice itself carries a `footnoteDefinition` for it (which
+ * would otherwise clobber the existing definition's text); the matching pasted
+ * references are remapped along with it to stay paired. A self-paste of just a
+ * reference is left untouched.
  */
 export function footnotePastePlugin(): Plugin {
   return new Plugin({
@@ -572,31 +516,35 @@ export function footnotePastePlugin(): Plugin {
         });
         if (existing.size === 0) return slice;
 
-        // Build a remap (old id -> fresh id) for every COLLIDING id found in the
-        // pasted slice, shared by references and definitions so a pasted pair
-        // stays matched. A paste is a distinct local user action (not a
-        // shared-state convergence point), so determinism is not strictly
-        // required here — but we derive the new id deterministically anyway
-        // (deriveFootnoteId against the current doc's id set) for consistency
-        // with the sync/import paths and to keep Math.random off this code path.
-        const remap = new Map<string, string>();
-        const collectColliding = (node: ProseMirrorNode) => {
-          if (
-            node.type.name === FOOTNOTE_REFERENCE_NAME ||
-            node.type.name === FOOTNOTE_DEFINITION_NAME
-          ) {
+        // Ids the pasted slice DEFINES (carries a footnoteDefinition for). Only
+        // these can clobber an existing footnote's text, so only these force a
+        // remap; a pasted reference to an already-existing id is reuse and keeps
+        // its id.
+        const sliceDefIds = new Set<string>();
+        const collectDefIds = (node: ProseMirrorNode) => {
+          if (node.type.name === FOOTNOTE_DEFINITION_NAME) {
             const id = node.attrs.id;
-            if (id && existing.has(id) && !remap.has(id)) {
-              const newId = deriveFootnoteId(id, 2, existing);
-              remap.set(id, newId);
-              // Reserve it so a second colliding id deriving to the same base
-              // bumps instead of clashing.
-              existing.add(newId);
-            }
+            if (id) sliceDefIds.add(id);
           }
-          node.descendants(collectColliding);
+          node.descendants(collectDefIds);
         };
-        slice.content.descendants(collectColliding);
+        slice.content.descendants(collectDefIds);
+
+        // Build a remap (old id -> fresh id) for every colliding id the slice
+        // DEFINES, shared by references and definitions so a pasted pair stays
+        // matched. The new id is derived deterministically (deriveFootnoteId
+        // against the current doc's id set) for consistency with the sync/import
+        // paths and to keep Math.random off this code path.
+        const remap = new Map<string, string>();
+        for (const id of sliceDefIds) {
+          if (existing.has(id) && !remap.has(id)) {
+            const newId = deriveFootnoteId(id, 2, existing);
+            remap.set(id, newId);
+            // Reserve it so a second colliding id deriving to the same base
+            // bumps instead of clashing.
+            existing.add(newId);
+          }
+        }
         if (remap.size === 0) return slice;
 
         // Rewrite the colliding ids throughout the slice.

packages/editor-ext/src/lib/footnote/footnote-util.derive-id.test.ts

@@ -4,16 +4,12 @@ import { deriveFootnoteId } from "./footnote-util";
 /**
  * GOLDEN TABLE for `deriveFootnoteId` (and its private alphabetic `suffix`).
  *
- * deriveFootnoteId is DELIBERATELY duplicated in
- *   packages/mcp/src/lib/collaboration.ts
- * and the two copies MUST stay byte-for-byte equivalent in behavior so the same
- * markdown imported through the editor and through the MCP path yields identical
- * footnote ids. This table is the SHARED contract: the parity test
- *   packages/mcp/test/unit/derive-id-parity.test.mjs
- * pins the exact SAME (input -> expected) pairs against the COMPILED mcp build.
- * If either copy drifts, one of the two tests goes red.
- *
- * Keep this constant in sync with GOLDEN in the mcp parity test.
+ * `deriveFootnoteId` lives ONLY in editor-ext now — it is used by
+ * `resolveCollisions` (re-id of a duplicate definition) and `footnotePastePlugin`
+ * (re-id of a pasted colliding definition). The MCP/marked import paths no longer
+ * derive ids (duplicate definitions there are first-wins-dropped, #166), so there
+ * is no cross-package copy and no parity test to keep in sync. This table pins the
+ * deterministic scheme so a future change to it is a conscious one.
  */
 export const DERIVE_GOLDEN: Array<{
   originalId: string;
@@ -56,7 +52,7 @@ function singleLetterSuffixes(): string[] {
   return Array.from({ length: 25 }, (_, i) => String.fromCharCode(98 + i));
 }
 
-describe("deriveFootnoteId golden table (cross-package drift guard)", () => {
+describe("deriveFootnoteId golden table (deterministic-scheme pin)", () => {
   for (const row of DERIVE_GOLDEN) {
     it(`derive("${row.originalId}", ${row.occurrence}, {${row.taken.join(",")}}) === "${row.expected}" — ${row.why}`, () => {
       const got = deriveFootnoteId(

packages/editor-ext/src/lib/footnote/footnote-util.ts

@@ -62,10 +62,11 @@ export function generateFootnoteId(): string {
  * `taken` is consulted but NOT mutated here; the caller adds the returned id to
  * its own seen-set before requesting the next derived id.
  *
- * NOTE: this implementation is intentionally duplicated in
- *   packages/mcp/src/lib/collaboration.ts (deriveFootnoteId)
- * and MUST stay in sync with it so markdown imported through either path yields
- * identical ids.
+ * Used only inside editor-ext now (resolveCollisions for a re-id'd duplicate
+ * DEFINITION, and footnotePastePlugin). The MCP/marked import paths no longer
+ * derive ids — duplicate definitions there are first-wins-dropped (#166) — so
+ * there is no cross-package copy to keep in sync. The golden table in
+ * footnote-util.derive-id.test.ts pins the scheme.
  */
 export function deriveFootnoteId(
   originalId: string,

packages/editor-ext/src/lib/footnote/footnote.test.ts

@@ -307,13 +307,12 @@ describe("footnote sync plugin (orphans)", () => {
     editor.destroy();
   });
 
-  it("two definitions sharing an id (with two matching references) BOTH survive the first edit (no data loss)", () => {
-    // Reproduces the verified data-loss bug: two footnoteDefinition nodes share
-    // id "d", and there are two references with id "d". The OLD code built the
-    // definitions Map last-wins and emitted exactly one definition for the
-    // de-duplicated reference, so the very first keystroke's sync transaction
-    // deleted the whole list and rebuilt it from one definition — silently
-    // destroying "first" and keeping only "second".
+  it("repeated references REUSE one footnote; a duplicate definition is dropped (first-wins)", () => {
+    // Reuse semantics (#166): two references with id "d" are the SAME footnote
+    // (one number, shared definition) — they are NEVER re-id'd. Two definitions
+    // sharing id "d" are first-wins: the first keeps "d", the second is re-id'd
+    // to a deterministic orphan id and then dropped by the orphan policy (it has
+    // no matching reference). So the result is ONE reused footnote on "first".
     const editor = makeEditor({
       type: "doc",
       content: [
@@ -351,8 +350,8 @@ describe("footnote sync plugin (orphans)", () => {
     editor.commands.insertContentAt(1, " ");
 
     const doc = editor.state.doc;
-    // BOTH definitions survive.
-    expect(countType(doc, FOOTNOTE_DEFINITION_NAME)).toBe(2);
+    // One shared definition survives (first-wins); the duplicate is dropped.
+    expect(countType(doc, FOOTNOTE_DEFINITION_NAME)).toBe(1);
     const defTexts: string[] = [];
     const defIds: string[] = [];
     doc.descendants((node) => {
@@ -361,27 +360,23 @@ describe("footnote sync plugin (orphans)", () => {
         defTexts.push(node.textContent);
       }
     });
-    // No content was lost: both "first" and "second" are still present.
-    expect(defTexts.sort()).toEqual(["first", "second"]);
-    // The colliding ids were made distinct.
-    expect(new Set(defIds).size).toBe(2);
-    // Each definition's id matches exactly one reference (1:1 pairing).
+    expect(defTexts).toEqual(["first"]);
+    expect(defIds).toEqual(["d"]);
+    // Both references keep id "d" (reuse — not re-id'd).
     const refIds: string[] = [];
     doc.descendants((node) => {
       if (node.type.name === FOOTNOTE_REFERENCE_NAME) refIds.push(node.attrs.id);
     });
-    expect(refIds.sort()).toEqual(defIds.sort());
+    expect(refIds).toEqual(["d", "d"]);
     editor.destroy();
   });
 
-  it("re-ids colliding duplicates DETERMINISTICALLY (two clients converge to identical ids)", () => {
+  it("reuse outcome is DETERMINISTIC across clients (Yjs convergence)", () => {
     // Cross-client determinism guard. Two collaborating clients each see the
-    // SAME duplicate-id document and each make a local edit. The sync plugin
-    // runs identically on every client, so it MUST mint the SAME new ids on both
-    // — otherwise the two clients diverge permanently over Yjs (duplicated
-    // footnotes). This is exactly the blocker the previous random-id
-    // (generateFootnoteId / Math.random) implementation caused: it would mint
-    // DIFFERENT ids on each client and this assertion would fail.
+    // SAME document and make a local edit; the sync plugin runs identically, so
+    // the resolved state MUST be identical (else they diverge over Yjs). Under
+    // reuse the three "d" references collapse to one footnote and the duplicate
+    // definitions are dropped (first-wins) — deterministically on every client.
     const duplicateDoc = {
       type: "doc",
       content: [
@@ -435,30 +430,28 @@ describe("footnote sync plugin (orphans)", () => {
       editor.commands.insertContentAt(1, " "); // local keystroke -> sync runs
       const refIds: string[] = [];
       const defIds: string[] = [];
+      const defTexts: string[] = [];
       editor.state.doc.descendants((node) => {
         if (node.type.name === FOOTNOTE_REFERENCE_NAME)
           refIds.push(node.attrs.id);
-        if (node.type.name === FOOTNOTE_DEFINITION_NAME)
+        if (node.type.name === FOOTNOTE_DEFINITION_NAME) {
           defIds.push(node.attrs.id);
+          defTexts.push(node.textContent);
+        }
       });
       editor.destroy();
-      return { refIds, defIds };
+      return { refIds, defIds, defTexts };
     };
 
     const clientA = idsAfterLocalEdit();
     const clientB = idsAfterLocalEdit();
 
-    // Both clients computed IDENTICAL ids (the property that makes Yjs converge).
-    expect(clientA.refIds).toEqual(clientB.refIds);
-    expect(clientA.defIds).toEqual(clientB.defIds);
-
-    // And the ids are deterministic-derived (not random uuid-style): the keeper
-    // keeps "d", the duplicates become "d__2", "d__3".
-    expect(new Set(clientA.refIds)).toEqual(new Set(["d", "d__2", "d__3"]));
-    // Every definition survived with a unique id, 1:1 with the references.
-    expect(clientA.defIds.length).toBe(3);
-    expect(new Set(clientA.defIds).size).toBe(3);
-    expect([...clientA.refIds].sort()).toEqual([...clientA.defIds].sort());
+    // Both clients resolved to IDENTICAL state (the Yjs-convergence property).
+    expect(clientA).toEqual(clientB);
+    // Reuse: the three references stay "d"; one definition survives (first-wins).
+    expect(clientA.refIds).toEqual(["d", "d", "d"]);
+    expect(clientA.defIds).toEqual(["d"]);
+    expect(clientA.defTexts).toEqual(["one"]);
   });
 
   it("removes an orphan definition with no matching reference", () => {

packages/editor-ext/src/lib/markdown/utils/footnote.marked.orphan.test.ts

@@ -13,36 +13,33 @@ function bodyMarkers(body: string): string[] {
   return [...body.matchAll(/\[\^([^\]\s]+)\]/g)].map((m) => m[1]);
 }
 
-describe("extractFootnoteDefinitions: more definitions than markers (orphans)", () => {
-  // Body has ONE `[^d]` reference marker but THREE `[^d]:` definitions. The
-  // surplus definitions have no marker to pair with — they must NOT be silently
-  // merged into one footnote (the editor's last-wins sync would otherwise drop
-  // two of them). The dedup gives each colliding definition a deterministic
-  // derived id so all three survive as distinct footnoteDefinition nodes.
+describe("extractFootnoteDefinitions: duplicate definition ids (first-wins)", () => {
+  // Body has ONE `[^d]` reference but THREE `[^d]:` definitions. Under the
+  // import model (#166) a duplicate definition id is FIRST-WINS: only the first
+  // definition is kept; the rest are DROPPED (and surfaced by analyzeFootnotes,
+  // not silently re-id'd into orphan footnotes as before). Reference markers are
+  // never rewritten, so repeated references would reuse the single footnote.
   const md = ["See[^d].", "", "[^d]: a", "[^d]: b", "[^d]: c"].join("\n");
 
-  it("emits 3 DISTINCT definition ids: d, d__2, d__3 (derived scheme, in order)", () => {
+  it("keeps only the FIRST definition for the id (first-wins)", () => {
     const { section } = extractFootnoteDefinitions(md);
     const ids = defIds(section);
-    expect(ids).toEqual(["d", "d__2", "d__3"]);
-    // All distinct: nothing was merged away.
-    expect(new Set(ids).size).toBe(3);
+    expect(ids).toEqual(["d"]);
   });
 
-  it("preserves each definition's text against its (possibly derived) id", () => {
+  it("keeps the first definition's text and drops the duplicates", () => {
     const { section } = extractFootnoteDefinitions(md);
-    // First definition keeps the original id and its text.
     expect(section).toContain('data-footnote-def data-id="d"><p>a</p>');
-    // The two surplus definitions survive as orphans with derived ids.
-    expect(section).toContain('data-footnote-def data-id="d__2"><p>b</p>');
-    expect(section).toContain('data-footnote-def data-id="d__3"><p>c</p>');
+    // No derived `d__2` / `d__3` ids are emitted anymore.
+    expect(section).not.toContain("d__2");
+    expect(section).not.toContain("d__3");
+    // The dropped duplicate texts are not in the section.
+    expect(section).not.toContain("<p>b</p>");
+    expect(section).not.toContain("<p>c</p>");
   });
 
-  it("leaves the SINGLE body marker as [^d] (no surplus marker to rewrite)", () => {
+  it("leaves the SINGLE body marker as [^d] (markers are never rewritten)", () => {
     const { body } = extractFootnoteDefinitions(md);
-    // There is exactly one reference marker and it is untouched: the keeper
-    // definition pairs with it. The orphan defs have no marker, so the body is
-    // unchanged except for the stripped definition lines.
     expect(bodyMarkers(body)).toEqual(["d"]);
     expect(body).toContain("See[^d].");
     // The definition lines themselves were pulled OUT of the body.
@@ -55,9 +52,21 @@ describe("extractFootnoteDefinitions: more definitions than markers (orphans)",
     const { section } = extractFootnoteDefinitions(md);
     expect(section.startsWith("<section data-footnotes>")).toBe(true);
     expect(section.endsWith("</section>")).toBe(true);
-    // Exactly three definition divs.
-    expect(
-      [...section.matchAll(/<div data-footnote-def/g)],
-    ).toHaveLength(3);
+    // Exactly one definition div (first-wins).
+    expect([...section.matchAll(/<div data-footnote-def/g)]).toHaveLength(1);
+  });
+});
+
+describe("extractFootnoteDefinitions: reuse (repeated references, one definition)", () => {
+  // Pandoc semantics: many `[^a]` references + one `[^a]:` definition = one
+  // footnote, shared. Markers are left intact so the editor numbers them as one.
+  const md = ["A[^a] B[^a] C[^a].", "", "[^a]: shared note"].join("\n");
+
+  it("emits exactly one definition and leaves every reference marker as [^a]", () => {
+    const { section, body } = extractFootnoteDefinitions(md);
+    expect(defIds(section)).toEqual(["a"]);
+    expect(section).toContain('data-footnote-def data-id="a"><p>shared note</p>');
+    // All three reference markers stay `a` (no `a__2`/`a__3` minting).
+    expect(bodyMarkers(body)).toEqual(["a", "a", "a"]);
   });
 });

packages/editor-ext/src/lib/markdown/utils/footnote.marked.ts

@@ -1,5 +1,4 @@
 import { marked } from "marked";
-import { deriveFootnoteId } from "../../footnote/footnote-util";
 
 /**
  * Pandoc/GFM footnote support for the marked (Markdown -> HTML) pipeline.
@@ -13,8 +12,12 @@ import { deriveFootnoteId } from "../../footnote/footnote-util";
  *    single <section data-footnotes> with one <div data-footnote-def> per
  *    definition, so the round-trip rebuilds footnotesList + footnoteDefinition.
  *
- * Only definitions that have a matching reference are emitted (and vice-versa
- * the sync plugin fills any gaps on the editor side), keeping the output valid.
+ * Every FIRST definition line is emitted — duplicate ids are first-wins (the
+ * rest are dropped, and surfaced via analyzeFootnotes), and reference markers are
+ * left untouched so repeated `[^a]` references reuse the one footnote (#166).
+ * Orphan definitions (no matching reference) are still emitted here; the editor's
+ * sync plugin reconciles the final reference/definition set (drops orphans,
+ * synthesizes a single empty definition for a reference that lacks one).
  */
 
 const DEFINITION_RE = /^\[\^([^\]\s]+)\]:[ \t]*(.*)$/;
@@ -53,10 +56,6 @@ function escapeAttr(value: string): string {
   return String(value).replace(/&/g, "&amp;").replace(/"/g, "&quot;");
 }
 
-function escapeRegExp(value: string): string {
-  return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
-}
-
 /**
  * Extract `[^id]: text` definition lines from the markdown body, returning the
  * cleaned body plus a rendered <section data-footnotes> (empty string when no
@@ -101,70 +100,32 @@ export function extractFootnoteDefinitions(markdown: string): {
     return { body: markdown, section: "" };
   }
 
-  // De-duplicate colliding definition ids. Two definitions sharing an id (e.g.
-  // `[^d]: first` / `[^d]: second`) would otherwise collapse into one footnote
-  // downstream (the editor's last-wins sync). Rename each colliding id to a
-  // DETERMINISTIC derived one AND rewrite the corresponding `[^id]` reference
-  // marker so the (reference, definition) pairing stays 1:1. The FIRST
-  // definition keeps the id and pairs with the FIRST `[^id]` marker; the Nth
-  // duplicate gets the derived id `${id}__${N}` and rewrites the Nth `[^id]`
-  // marker. If there are fewer markers than definitions, the surplus definition
-  // keeps a derived (orphan) id so it is never silently merged away.
-  //
-  // The id is derived (deriveFootnoteId), NOT random: importing the same
-  // markdown through two paths (here and the MCP mirror) must yield identical
-  // ids, and re-importing the same markdown twice must be stable.
-  let dedupedBody = bodyLines.join("\n");
-  // Every original definition id is reserved up front so a derived id can never
-  // collide with an unrelated original id present in the document.
-  const taken = new Set<string>(definitions.map((d) => d.id));
-  const seenDefIds = new Map<string, number>(); // original id -> how many seen
+  // Duplicate definition ids (e.g. `[^d]: first` / `[^d]: second`): FIRST WINS,
+  // the rest are DROPPED. Reference markers are left UNTOUCHED so repeated `[^a]`
+  // references reuse the single footnote (Pandoc semantics, #166). This differs
+  // from the live editor's never-lose policy (resolveCollisions re-ids a
+  // duplicate definition into an orphan) on purpose: an import is an
+  // agent-authored artifact we sanitize, and the dropped duplicate is surfaced
+  // to the caller via analyzeFootnotes' `duplicateDefinitions` warning instead.
+  const firstById = new Map<string, string>(); // id -> first definition text
   for (const def of definitions) {
-    const originalId = def.id;
-    const count = seenDefIds.get(originalId) ?? 0;
-    seenDefIds.set(originalId, count + 1);
-    if (count === 0) continue; // first definition keeps its id
-
-    // count is the 0-based number of PRIOR occurrences; this is occurrence
-    // (count + 1), i.e. 2 for the first duplicate, 3 for the next, ...
-    const newId = deriveFootnoteId(originalId, count + 1, taken);
-    taken.add(newId);
-    def.id = newId;
-
-    // Rewrite the NEXT still-unrewritten `[^originalId]` marker that does not
-    // belong to the keeper definition. After a prior duplicate rewrote its
-    // marker (to `[^someNewId]`), it no longer matches `[^originalId]`, so the
-    // remaining matches are: index 0 = the keeper's marker (left alone), index 1
-    // = this duplicate's marker. Rewrite index 1.
-    let occurrence = 0;
-    let rewritten = false;
-    const re = new RegExp(`\\[\\^${escapeRegExp(originalId)}\\]`, "g");
-    dedupedBody = dedupedBody.replace(re, (match) => {
-      const idx = occurrence++;
-      if (!rewritten && idx === 1) {
-        rewritten = true;
-        return `[^${newId}]`;
-      }
-      return match;
-    });
-    // If there was no second marker (more definitions than references), the
-    // duplicate simply survives as an orphan with its fresh id — no body change.
+    if (!firstById.has(def.id)) firstById.set(def.id, def.text);
   }
 
-  const defsHtml = definitions
-    .map((d) => {
+  const defsHtml = [...firstById.entries()]
+    .map(([id, text]) => {
       // Render the definition text as inline markdown so emphasis/links inside
       // a footnote survive the round-trip; wrap in a paragraph (the node's
       // content is paragraph+).
-      const inner = marked.parseInline(d.text || "");
+      const inner = marked.parseInline(text || "");
       return `<div data-footnote-def data-id="${escapeAttr(
-        d.id,
+        id,
       )}"><p>${inner}</p></div>`;
     })
     .join("");
 
   return {
-    body: dedupedBody,
+    body: bodyLines.join("\n"),
     section: `<section data-footnotes>${defsHtml}</section>`,
   };
 }

packages/mcp/build/client.js

@@ -9,6 +9,7 @@ import WebSocket from "ws";
 import { convertProseMirrorToMarkdown } from "./lib/markdown-converter.js";
 import { updatePageContentRealtime, replacePageContent, markdownToProseMirror, mutatePageContent, buildCollabWsUrl, assertYjsEncodable, } from "./lib/collaboration.js";
 import { docmostExtensions } from "./lib/docmost-schema.js";
+import { footnoteWarningsField } from "./lib/footnote-analyze.js";
 import { buildPageTree } from "./lib/tree.js";
 import { serializeDocmostMarkdown, parseDocmostMarkdown, } from "./lib/markdown-document.js";
 import { replaceNodeById, deleteNodeById, insertNodeRelative, buildOutline, getNodeByRef, readTable, insertTableRow, deleteTableRow, updateTableCell, } from "./lib/node-ops.js";
@@ -566,7 +567,9 @@ export class DocmostClient {
         // Always fetch subpages to provide context to the agent
         let subpages = [];
         try {
-            subpages = await this.listSidebarPages(resultData.spaceId, pageId);
+            // `pageId` may be a slugId, but the sidebar-pages endpoint requires the
+            // UUID; `resultData.id` holds the resolved UUID returned by getPageRaw.
+            subpages = await this.listSidebarPages(resultData.spaceId, resultData.id);
         }
         catch (e) {
             console.warn("Failed to fetch subpages:", e);
@@ -814,7 +817,10 @@ export class DocmostClient {
         if (title) {
             await this.client.post("/pages/update", { pageId: newPageId, title });
         }
-        return this.getPage(newPageId);
+        const page = await this.getPage(newPageId);
+        // Surface non-fatal footnote problems (dangling refs, empty/duplicate
+        // definitions, markers in tables) so the agent can fix its markup (#166).
+        return { ...page, ...footnoteWarningsField(content) };
     }
     /**
      * Update a page's content from markdown and optionally its title.
@@ -850,6 +856,8 @@ export class DocmostClient {
             message: "Page updated successfully.",
             pageId: pageId,
             verify: mutation.verify,
+            // Non-fatal footnote diagnostics (#166); omitted when there are none.
+            ...footnoteWarningsField(content),
         };
     }
     /**
@@ -1119,6 +1127,11 @@ export class DocmostClient {
         if (meta?.pageId && meta.pageId !== pageId) {
             result.warning = `File was exported from page ${meta.pageId} but is being imported into ${pageId}.`;
         }
+        // Non-fatal footnote diagnostics (#166), analyzed on the BODY (the part after
+        // the docmost:meta / docmost:comments blocks) — so a `[^x]`-like token inside
+        // those JSON blocks never produces a false warning, while real markers in the
+        // body do. `body` comes from parseDocmostMarkdown(fullMarkdown) above.
+        Object.assign(result, footnoteWarningsField(body));
         return result;
     }
     /**
@@ -2422,9 +2435,9 @@ export class DocmostClient {
             const raw = await this.getPageRaw(pageId);
             const current = raw.content || { type: "doc", content: [] };
             runTransform(current);
-            // Exercise the same Yjs encoder the apply path uses, so the preview
-            // fails with the SAME descriptive error when the doc is not encodable
-            // instead of returning a misleadingly-green diff.
+            // Run an independent Yjs-encodability check (same sanitize + schema as the
+            // apply path), so the preview fails with the same descriptive error when
+            // the doc is not encodable instead of returning a misleadingly-green diff.
             assertYjsEncodable(newDoc);
             return {
                 pushed: false,

packages/mcp/build/lib/collaboration.js

@@ -10,6 +10,7 @@ import { JSDOM } from "jsdom";
 import { docmostExtensions, docmostSchema } from "./docmost-schema.js";
 import { withPageLock } from "./page-lock.js";
 import { sanitizeForYjs, findUnstorableAttr } from "./node-ops.js";
+import { lexFootnoteLines } from "./footnote-lex.js";
 import { summarizeChange } from "./diff.js";
 /**
  * Build the descriptive error for an opaque Yjs encode failure ("Unexpected
@@ -280,49 +281,12 @@ function bridgeTaskLists(html) {
 // Mirror of packages/editor-ext footnote markdown handling. A `[^id]` inline
 // marker becomes <sup data-footnote-ref data-id="id">, and `[^id]: text`
 // definition lines are collected into a single <section data-footnotes>.
-const FOOTNOTE_DEF_RE = /^\[\^([^\]\s]+)\]:[ \t]*(.*)$/;
+// Definition detection + fence handling are shared with analyzeFootnotes via
+// lexFootnoteLines (footnote-lex.js). FOOTNOTE_REF_RE is the inline tokenizer's.
 const FOOTNOTE_REF_RE = /\[\^([^\]\s]+)\]/;
 function escapeFootnoteAttr(value) {
     return String(value).replace(/&/g, "&amp;").replace(/"/g, "&quot;");
 }
-function escapeFootnoteRegExp(value) {
-    return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
-}
-/**
- * Derive a DETERMINISTIC unique footnote id for the k-th (k >= 2) occurrence of
- * an original id `X` during definition dedup.
- *
- * EXACT MIRROR of editor-ext `deriveFootnoteId`
- * (packages/editor-ext/src/lib/footnote/footnote-util.ts). These two copies MUST
- * STAY IN SYNC: the same markdown imported through the editor and through this
- * MCP path has to produce identical ids, and the sync plugin (which re-ids on
- * every collaborating client) relies on the same scheme to converge. NEVER use
- * Math.random()/Date.now()/uuid here — a random id would diverge across clients.
- *
- * Scheme: base candidate `${originalId}__${occurrence}` (e.g. `X__2`), bumped
- * with a stable alphabetic suffix (`X__2b`, `X__2c`, ...) until it is not in
- * `taken` (the set of ids already present / already minted — pure doc state).
- */
-function deriveFootnoteId(originalId, occurrence, taken) {
-    let candidate = `${originalId}__${occurrence}`;
-    let n = 0;
-    while (taken.has(candidate)) {
-        n += 1;
-        candidate = `${originalId}__${occurrence}${footnoteSuffix(n)}`;
-    }
-    return candidate;
-}
-/** Map 1 -> "b", 2 -> "c", ... (mirror of editor-ext `suffix`). */
-function footnoteSuffix(n) {
-    let out = "";
-    let x = n;
-    while (x > 0) {
-        const rem = (x - 1) % 25;
-        out = String.fromCharCode(98 + rem) + out; // 98 = 'b'
-        x = Math.floor((x - 1) / 25);
-    }
-    return out;
-}
 const footnoteRefMarkedExtension = {
     name: "footnoteRef",
     level: "inline",
@@ -346,68 +310,36 @@ marked.use({ extensions: [footnoteRefMarkedExtension] });
  * <section data-footnotes> for them (or "" when there are none).
  */
 function extractFootnotes(markdown) {
-    const lines = markdown.split("\n");
     const bodyLines = [];
     const defs = [];
-    // Track fenced-code state so a `[^id]: ...` line shown inside a ``` / ~~~ code
-    // block is preserved verbatim and not treated as a footnote definition.
-    let fence = null;
-    for (const line of lines) {
-        const fenceMatch = /^(\s*)(`{3,}|~{3,})/.exec(line);
-        if (fenceMatch) {
-            const marker = fenceMatch[2][0];
-            if (fence === null)
-                fence = marker;
-            else if (marker === fence)
-                fence = null;
-            bodyLines.push(line);
-            continue;
-        }
-        const m = fence === null ? FOOTNOTE_DEF_RE.exec(line) : null;
-        if (m)
-            defs.push({ id: m[1], text: m[2] });
+    // Shared lexer (footnote-lex): a `[^id]: ...` line inside a ``` / ~~~ code
+    // block is inert and stays in the body verbatim; only real definition lines
+    // are pulled out. analyzeFootnotes() consumes the SAME lexer so its diagnostics
+    // match exactly what import keeps/strips (#166).
+    for (const tok of lexFootnoteLines(markdown)) {
+        if (!tok.inFence && tok.definition)
+            defs.push(tok.definition);
         else
-            bodyLines.push(line);
+            bodyLines.push(tok.line);
     }
     if (defs.length === 0)
         return { body: markdown, section: "" };
-    // De-duplicate colliding definition ids (mirror of editor-ext
-    // extractFootnoteDefinitions). Two definitions sharing an id would otherwise
-    // collapse into one footnote downstream; rename each colliding id to a
-    // DETERMINISTIC derived one (NOT random) and rewrite the corresponding `[^id]`
-    // marker so the (reference, definition) pairing stays 1:1. Determinism lets
-    // the same markdown imported here and via the editor produce identical ids.
-    let dedupedBody = bodyLines.join("\n");
-    const taken = new Set(defs.map((d) => d.id));
-    const seenDefIds = new Map();
+    // Duplicate definition ids: FIRST WINS, the rest are DROPPED (mirror of
+    // editor-ext extractFootnoteDefinitions). Reference markers are left untouched
+    // so repeated `[^a]` references reuse the single footnote (Pandoc semantics,
+    // #166). The dropped duplicate is surfaced to the caller via analyzeFootnotes
+    // (`duplicateDefinitions`), not silently lost. MUST stay in sync with the
+    // editor-ext mirror.
+    const firstById = new Map(); // id -> first definition text
     for (const def of defs) {
-        const originalId = def.id;
-        const count = seenDefIds.get(originalId) ?? 0;
-        seenDefIds.set(originalId, count + 1);
-        if (count === 0)
-            continue; // first definition keeps its id
-        const newId = deriveFootnoteId(originalId, count + 1, taken);
-        taken.add(newId);
-        def.id = newId;
-        // Remaining `[^originalId]` matches: index 0 = keeper's marker (left alone),
-        // index 1 = this duplicate's marker. Rewrite index 1.
-        let occurrence = 0;
-        let rewritten = false;
-        const re = new RegExp(`\\[\\^${escapeFootnoteRegExp(originalId)}\\]`, "g");
-        dedupedBody = dedupedBody.replace(re, (match) => {
-            const idx = occurrence++;
-            if (!rewritten && idx === 1) {
-                rewritten = true;
-                return `[^${newId}]`;
-            }
-            return match;
-        });
+        if (!firstById.has(def.id))
+            firstById.set(def.id, def.text);
     }
-    const inner = defs
-        .map((d) => `<div data-footnote-def data-id="${escapeFootnoteAttr(d.id)}"><p>${marked.parseInline(d.text || "")}</p></div>`)
+    const inner = [...firstById.entries()]
+        .map(([id, text]) => `<div data-footnote-def data-id="${escapeFootnoteAttr(id)}"><p>${marked.parseInline(text || "")}</p></div>`)
         .join("");
     return {
-        body: dedupedBody,
+        body: bodyLines.join("\n"),
         section: `<section data-footnotes>${inner}</section>`,
     };
 }

packages/mcp/build/lib/footnote-analyze.js

@@ -0,0 +1,101 @@
+/**
+ * Footnote diagnostics for imported Markdown (issue #166).
+ *
+ * A PURE, fence-aware text scan (independent of the Markdown->ProseMirror
+ * conversion path, so it reports the same problems for `create_page`,
+ * `update_page` and `import_page_markdown`). It never changes the document — the
+ * importer still creates the page; this only surfaces footnote problems to the
+ * caller so an agent can fix its own markup instead of shipping broken footnotes.
+ *
+ * Detected problems:
+ *  - danglingReferences: a `[^id]` reference with no `[^id]:` definition.
+ *  - emptyDefinitions:   a `[^id]:` whose (kept) text is empty/whitespace.
+ *  - duplicateDefinitions: an id defined by two or more `[^id]:` lines (only the
+ *    first is kept on import — first-wins; see extractFootnotes).
+ *  - referencesInTables: a `[^id]` marker found in a GFM table row (heuristic:
+ *    the line, trimmed, starts with `|`) — footnotes in table cells often do not
+ *    render as expected.
+ */
+import { lexFootnoteLines, forEachFootnoteReference, } from "./footnote-lex.js";
+/**
+ * Analyze the footnotes in a Markdown string. Pure; safe to call on any body.
+ */
+export function analyzeFootnotes(markdown) {
+    // Distinct reference ids in first-appearance order, plus the set of ids seen
+    // inside a table row.
+    const refIds = [];
+    const refIdSet = new Set();
+    const referencesInTables = new Set();
+    const addRef = (id, inTable) => {
+        if (!refIdSet.has(id)) {
+            refIdSet.add(id);
+            refIds.push(id);
+        }
+        if (inTable)
+            referencesInTables.add(id);
+    };
+    // Definition texts per id, in first-appearance order of the id.
+    const defTextsById = new Map();
+    // Same lexer the importer uses, so the analysis matches exactly what import
+    // keeps/strips (#166): fenced lines are inert, definition lines are pulled.
+    for (const tok of lexFootnoteLines(markdown)) {
+        if (tok.inFence)
+            continue;
+        if (tok.definition) {
+            const { id, text } = tok.definition;
+            const arr = defTextsById.get(id);
+            if (arr)
+                arr.push(text);
+            else
+                defTextsById.set(id, [text]);
+            // A definition's TEXT can itself reference another footnote (`[^a]: see
+            // [^b]`); count those so such a `[^b]` is not falsely reported dangling.
+            forEachFootnoteReference(text, (rid) => addRef(rid, false));
+            continue;
+        }
+        const inTable = tok.line.trimStart().startsWith("|");
+        forEachFootnoteReference(tok.line, (id) => addRef(id, inTable));
+    }
+    const danglingReferences = refIds.filter((id) => !defTextsById.has(id));
+    const duplicateDefinitions = [];
+    const emptyDefinitions = [];
+    for (const [id, texts] of defTextsById) {
+        if (texts.length >= 2)
+            duplicateDefinitions.push(id);
+        // First-wins: the kept definition is the first one; flag it if it is blank.
+        if ((texts[0] ?? "").trim().length === 0)
+            emptyDefinitions.push(id);
+    }
+    const tableRefs = [...referencesInTables];
+    const warnings = [];
+    const list = (ids) => ids.map((id) => `[^${id}]`).join(", ");
+    if (danglingReferences.length > 0) {
+        warnings.push(`Footnote reference(s) with no matching definition: ${list(danglingReferences)} (each will render as an empty footnote in the editor).`);
+    }
+    if (emptyDefinitions.length > 0) {
+        warnings.push(`Footnote definition(s) with empty text: ${list(emptyDefinitions)}.`);
+    }
+    if (duplicateDefinitions.length > 0) {
+        warnings.push(`Footnote id(s) defined more than once (only the first definition was kept): ${list(duplicateDefinitions)}.`);
+    }
+    if (tableRefs.length > 0) {
+        warnings.push(`Footnote marker(s) inside a table row (footnotes in table cells may not render as expected): ${list(tableRefs)}.`);
+    }
+    return {
+        danglingReferences,
+        emptyDefinitions,
+        duplicateDefinitions,
+        referencesInTables: tableRefs,
+        warnings,
+    };
+}
+/**
+ * The optional `footnoteWarnings` field for a page-write tool result: present
+ * (with the warning lines) only when `markdown` has footnote problems, omitted
+ * otherwise. One helper so all three call sites (create/update/import) attach the
+ * field identically. Spread into the result: `{ ...result, ...footnoteWarningsField(text) }`.
+ */
+export function footnoteWarningsField(markdown) {
+    const { warnings } = analyzeFootnotes(markdown);
+    return warnings.length > 0 ? { footnoteWarnings: warnings } : {};
+}

packages/mcp/build/lib/footnote-lex.js

@@ -0,0 +1,55 @@
+/**
+ * Shared, fence-aware line lexer for footnote markdown (MCP-internal).
+ *
+ * Both the importer (`extractFootnotes` in collaboration.ts, which strips
+ * definition lines and rebuilds a footnotes section) and the diagnostics
+ * (`analyzeFootnotes` in footnote-analyze.ts) must agree EXACTLY on which lines
+ * are definitions and which lines are inert (inside a code fence). Sharing one
+ * lexer makes "the analyzer sees what the importer leaves" a structural property
+ * instead of two hand-kept copies that can drift (#166 review).
+ *
+ * NOTE: this is deliberately NOT shared with editor-ext's
+ * `extractFootnoteDefinitions` — that lives in a different package and the
+ * decoupling between the editor and the MCP mirror is intentional.
+ */
+/** A footnote DEFINITION line: `[^id]: text` (id + text captured). */
+export const FOOTNOTE_DEF_RE = /^\[\^([^\]\s]+)\]:[ \t]*(.*)$/;
+/** Every footnote REFERENCE `[^id]` in a line (global; id captured). */
+export const FOOTNOTE_REF_RE_G = /\[\^([^\]\s]+)\]/g;
+/** Opening/closing code fence marker (``` or ~~~). */
+const FENCE_RE = /^(\s*)(`{3,}|~{3,})/;
+/** Classify every line of `markdown`, tracking fenced-code state. Pure. */
+export function lexFootnoteLines(markdown) {
+    const out = [];
+    let fence = null;
+    for (const line of markdown.split("\n")) {
+        const fenceMatch = FENCE_RE.exec(line);
+        if (fenceMatch) {
+            const marker = fenceMatch[2][0];
+            if (fence === null)
+                fence = marker; // opening fence
+            else if (marker === fence)
+                fence = null; // matching closing fence
+            out.push({ line, inFence: true, definition: null });
+            continue;
+        }
+        if (fence !== null) {
+            out.push({ line, inFence: true, definition: null });
+            continue;
+        }
+        const m = FOOTNOTE_DEF_RE.exec(line);
+        out.push({
+            line,
+            inFence: false,
+            definition: m ? { id: m[1], text: m[2] } : null,
+        });
+    }
+    return out;
+}
+/** Scan a line for every `[^id]` reference, invoking `onRef(id)` for each. */
+export function forEachFootnoteReference(line, onRef) {
+    FOOTNOTE_REF_RE_G.lastIndex = 0;
+    let m;
+    while ((m = FOOTNOTE_REF_RE_G.exec(line)) !== null)
+        onRef(m[1]);
+}

packages/mcp/src/client.ts

@@ -23,6 +23,7 @@ import {
   MutationResult,
 } from "./lib/collaboration.js";
 import { docmostExtensions } from "./lib/docmost-schema.js";
+import { footnoteWarningsField } from "./lib/footnote-analyze.js";
 import { buildPageTree } from "./lib/tree.js";
 import {
   serializeDocmostMarkdown,
@@ -1054,7 +1055,10 @@ export class DocmostClient {
       await this.client.post("/pages/update", { pageId: newPageId, title });
     }
 
-    return this.getPage(newPageId);
+    const page = await this.getPage(newPageId);
+    // Surface non-fatal footnote problems (dangling refs, empty/duplicate
+    // definitions, markers in tables) so the agent can fix its markup (#166).
+    return { ...page, ...footnoteWarningsField(content) };
   }
 
   /**
@@ -1101,6 +1105,8 @@ export class DocmostClient {
       message: "Page updated successfully.",
       pageId: pageId,
       verify: mutation.verify,
+      // Non-fatal footnote diagnostics (#166); omitted when there are none.
+      ...footnoteWarningsField(content),
     };
   }
 
@@ -1416,6 +1422,11 @@ export class DocmostClient {
     if (meta?.pageId && meta.pageId !== pageId) {
       result.warning = `File was exported from page ${meta.pageId} but is being imported into ${pageId}.`;
     }
+    // Non-fatal footnote diagnostics (#166), analyzed on the BODY (the part after
+    // the docmost:meta / docmost:comments blocks) — so a `[^x]`-like token inside
+    // those JSON blocks never produces a false warning, while real markers in the
+    // body do. `body` comes from parseDocmostMarkdown(fullMarkdown) above.
+    Object.assign(result, footnoteWarningsField(body));
     return result;
   }
 

packages/mcp/src/lib/collaboration.ts

@@ -10,6 +10,7 @@ import { JSDOM } from "jsdom";
 import { docmostExtensions, docmostSchema } from "./docmost-schema.js";
 import { withPageLock } from "./page-lock.js";
 import { sanitizeForYjs, findUnstorableAttr } from "./node-ops.js";
+import { lexFootnoteLines } from "./footnote-lex.js";
 import { summarizeChange, VerifyReport } from "./diff.js";
 
 /**
@@ -316,58 +317,14 @@ function bridgeTaskLists(html: string): string {
 // Mirror of packages/editor-ext footnote markdown handling. A `[^id]` inline
 // marker becomes <sup data-footnote-ref data-id="id">, and `[^id]: text`
 // definition lines are collected into a single <section data-footnotes>.
-const FOOTNOTE_DEF_RE = /^\[\^([^\]\s]+)\]:[ \t]*(.*)$/;
+// Definition detection + fence handling are shared with analyzeFootnotes via
+// lexFootnoteLines (footnote-lex.js). FOOTNOTE_REF_RE is the inline tokenizer's.
 const FOOTNOTE_REF_RE = /\[\^([^\]\s]+)\]/;
 
 function escapeFootnoteAttr(value: string): string {
   return String(value).replace(/&/g, "&amp;").replace(/"/g, "&quot;");
 }
 
-function escapeFootnoteRegExp(value: string): string {
-  return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
-}
-
-/**
- * Derive a DETERMINISTIC unique footnote id for the k-th (k >= 2) occurrence of
- * an original id `X` during definition dedup.
- *
- * EXACT MIRROR of editor-ext `deriveFootnoteId`
- * (packages/editor-ext/src/lib/footnote/footnote-util.ts). These two copies MUST
- * STAY IN SYNC: the same markdown imported through the editor and through this
- * MCP path has to produce identical ids, and the sync plugin (which re-ids on
- * every collaborating client) relies on the same scheme to converge. NEVER use
- * Math.random()/Date.now()/uuid here — a random id would diverge across clients.
- *
- * Scheme: base candidate `${originalId}__${occurrence}` (e.g. `X__2`), bumped
- * with a stable alphabetic suffix (`X__2b`, `X__2c`, ...) until it is not in
- * `taken` (the set of ids already present / already minted — pure doc state).
- */
-function deriveFootnoteId(
-  originalId: string,
-  occurrence: number,
-  taken: Set<string>,
-): string {
-  let candidate = `${originalId}__${occurrence}`;
-  let n = 0;
-  while (taken.has(candidate)) {
-    n += 1;
-    candidate = `${originalId}__${occurrence}${footnoteSuffix(n)}`;
-  }
-  return candidate;
-}
-
-/** Map 1 -> "b", 2 -> "c", ... (mirror of editor-ext `suffix`). */
-function footnoteSuffix(n: number): string {
-  let out = "";
-  let x = n;
-  while (x > 0) {
-    const rem = (x - 1) % 25;
-    out = String.fromCharCode(98 + rem) + out; // 98 = 'b'
-    x = Math.floor((x - 1) / 25);
-  }
-  return out;
-}
-
 const footnoteRefMarkedExtension = {
   name: "footnoteRef",
   level: "inline" as const,
@@ -398,69 +355,39 @@ function extractFootnotes(markdown: string): {
   body: string;
   section: string;
 } {
-  const lines = markdown.split("\n");
   const bodyLines: string[] = [];
   const defs: Array<{ id: string; text: string }> = [];
-  // Track fenced-code state so a `[^id]: ...` line shown inside a ``` / ~~~ code
-  // block is preserved verbatim and not treated as a footnote definition.
-  let fence: string | null = null;
-  for (const line of lines) {
-    const fenceMatch = /^(\s*)(`{3,}|~{3,})/.exec(line);
-    if (fenceMatch) {
-      const marker = fenceMatch[2][0];
-      if (fence === null) fence = marker;
-      else if (marker === fence) fence = null;
-      bodyLines.push(line);
-      continue;
-    }
-    const m = fence === null ? FOOTNOTE_DEF_RE.exec(line) : null;
-    if (m) defs.push({ id: m[1], text: m[2] });
-    else bodyLines.push(line);
+  // Shared lexer (footnote-lex): a `[^id]: ...` line inside a ``` / ~~~ code
+  // block is inert and stays in the body verbatim; only real definition lines
+  // are pulled out. analyzeFootnotes() consumes the SAME lexer so its diagnostics
+  // match exactly what import keeps/strips (#166).
+  for (const tok of lexFootnoteLines(markdown)) {
+    if (!tok.inFence && tok.definition) defs.push(tok.definition);
+    else bodyLines.push(tok.line);
   }
   if (defs.length === 0) return { body: markdown, section: "" };
 
-  // De-duplicate colliding definition ids (mirror of editor-ext
-  // extractFootnoteDefinitions). Two definitions sharing an id would otherwise
-  // collapse into one footnote downstream; rename each colliding id to a
-  // DETERMINISTIC derived one (NOT random) and rewrite the corresponding `[^id]`
-  // marker so the (reference, definition) pairing stays 1:1. Determinism lets
-  // the same markdown imported here and via the editor produce identical ids.
-  let dedupedBody = bodyLines.join("\n");
-  const taken = new Set<string>(defs.map((d) => d.id));
-  const seenDefIds = new Map<string, number>();
+  // Duplicate definition ids: FIRST WINS, the rest are DROPPED (mirror of
+  // editor-ext extractFootnoteDefinitions). Reference markers are left untouched
+  // so repeated `[^a]` references reuse the single footnote (Pandoc semantics,
+  // #166). The dropped duplicate is surfaced to the caller via analyzeFootnotes
+  // (`duplicateDefinitions`), not silently lost. MUST stay in sync with the
+  // editor-ext mirror.
+  const firstById = new Map<string, string>(); // id -> first definition text
   for (const def of defs) {
-    const originalId = def.id;
-    const count = seenDefIds.get(originalId) ?? 0;
-    seenDefIds.set(originalId, count + 1);
-    if (count === 0) continue; // first definition keeps its id
-    const newId = deriveFootnoteId(originalId, count + 1, taken);
-    taken.add(newId);
-    def.id = newId;
-    // Remaining `[^originalId]` matches: index 0 = keeper's marker (left alone),
-    // index 1 = this duplicate's marker. Rewrite index 1.
-    let occurrence = 0;
-    let rewritten = false;
-    const re = new RegExp(`\\[\\^${escapeFootnoteRegExp(originalId)}\\]`, "g");
-    dedupedBody = dedupedBody.replace(re, (match) => {
-      const idx = occurrence++;
-      if (!rewritten && idx === 1) {
-        rewritten = true;
-        return `[^${newId}]`;
-      }
-      return match;
-    });
+    if (!firstById.has(def.id)) firstById.set(def.id, def.text);
   }
 
-  const inner = defs
+  const inner = [...firstById.entries()]
     .map(
-      (d) =>
+      ([id, text]) =>
         `<div data-footnote-def data-id="${escapeFootnoteAttr(
-          d.id,
-        )}"><p>${marked.parseInline(d.text || "")}</p></div>`,
+          id,
+        )}"><p>${marked.parseInline(text || "")}</p></div>`,
     )
     .join("");
   return {
-    body: dedupedBody,
+    body: bodyLines.join("\n"),
     section: `<section data-footnotes>${inner}</section>`,
   };
 }

packages/mcp/src/lib/footnote-analyze.ts

@@ -0,0 +1,129 @@
+/**
+ * Footnote diagnostics for imported Markdown (issue #166).
+ *
+ * A PURE, fence-aware text scan (independent of the Markdown->ProseMirror
+ * conversion path, so it reports the same problems for `create_page`,
+ * `update_page` and `import_page_markdown`). It never changes the document — the
+ * importer still creates the page; this only surfaces footnote problems to the
+ * caller so an agent can fix its own markup instead of shipping broken footnotes.
+ *
+ * Detected problems:
+ *  - danglingReferences: a `[^id]` reference with no `[^id]:` definition.
+ *  - emptyDefinitions:   a `[^id]:` whose (kept) text is empty/whitespace.
+ *  - duplicateDefinitions: an id defined by two or more `[^id]:` lines (only the
+ *    first is kept on import — first-wins; see extractFootnotes).
+ *  - referencesInTables: a `[^id]` marker found in a GFM table row (heuristic:
+ *    the line, trimmed, starts with `|`) — footnotes in table cells often do not
+ *    render as expected.
+ */
+
+import {
+  lexFootnoteLines,
+  forEachFootnoteReference,
+} from "./footnote-lex.js";
+
+export interface FootnoteDiagnostics {
+  /** Reference ids (distinct, document order) with no matching definition. */
+  danglingReferences: string[];
+  /** Definition ids whose first (kept) text is empty/whitespace. */
+  emptyDefinitions: string[];
+  /** Ids defined by two or more `[^id]:` lines (only the first is kept). */
+  duplicateDefinitions: string[];
+  /** Reference ids found inside a GFM table row (heuristic). */
+  referencesInTables: string[];
+  /** Human-readable warning lines for the tool result (one per problem class). */
+  warnings: string[];
+}
+
+/**
+ * Analyze the footnotes in a Markdown string. Pure; safe to call on any body.
+ */
+export function analyzeFootnotes(markdown: string): FootnoteDiagnostics {
+  // Distinct reference ids in first-appearance order, plus the set of ids seen
+  // inside a table row.
+  const refIds: string[] = [];
+  const refIdSet = new Set<string>();
+  const referencesInTables = new Set<string>();
+  const addRef = (id: string, inTable: boolean) => {
+    if (!refIdSet.has(id)) {
+      refIdSet.add(id);
+      refIds.push(id);
+    }
+    if (inTable) referencesInTables.add(id);
+  };
+
+  // Definition texts per id, in first-appearance order of the id.
+  const defTextsById = new Map<string, string[]>();
+
+  // Same lexer the importer uses, so the analysis matches exactly what import
+  // keeps/strips (#166): fenced lines are inert, definition lines are pulled.
+  for (const tok of lexFootnoteLines(markdown)) {
+    if (tok.inFence) continue;
+    if (tok.definition) {
+      const { id, text } = tok.definition;
+      const arr = defTextsById.get(id);
+      if (arr) arr.push(text);
+      else defTextsById.set(id, [text]);
+      // A definition's TEXT can itself reference another footnote (`[^a]: see
+      // [^b]`); count those so such a `[^b]` is not falsely reported dangling.
+      forEachFootnoteReference(text, (rid) => addRef(rid, false));
+      continue;
+    }
+    const inTable = tok.line.trimStart().startsWith("|");
+    forEachFootnoteReference(tok.line, (id) => addRef(id, inTable));
+  }
+
+  const danglingReferences = refIds.filter((id) => !defTextsById.has(id));
+  const duplicateDefinitions: string[] = [];
+  const emptyDefinitions: string[] = [];
+  for (const [id, texts] of defTextsById) {
+    if (texts.length >= 2) duplicateDefinitions.push(id);
+    // First-wins: the kept definition is the first one; flag it if it is blank.
+    if ((texts[0] ?? "").trim().length === 0) emptyDefinitions.push(id);
+  }
+  const tableRefs = [...referencesInTables];
+
+  const warnings: string[] = [];
+  const list = (ids: string[]) => ids.map((id) => `[^${id}]`).join(", ");
+  if (danglingReferences.length > 0) {
+    warnings.push(
+      `Footnote reference(s) with no matching definition: ${list(danglingReferences)} (each will render as an empty footnote in the editor).`,
+    );
+  }
+  if (emptyDefinitions.length > 0) {
+    warnings.push(
+      `Footnote definition(s) with empty text: ${list(emptyDefinitions)}.`,
+    );
+  }
+  if (duplicateDefinitions.length > 0) {
+    warnings.push(
+      `Footnote id(s) defined more than once (only the first definition was kept): ${list(duplicateDefinitions)}.`,
+    );
+  }
+  if (tableRefs.length > 0) {
+    warnings.push(
+      `Footnote marker(s) inside a table row (footnotes in table cells may not render as expected): ${list(tableRefs)}.`,
+    );
+  }
+
+  return {
+    danglingReferences,
+    emptyDefinitions,
+    duplicateDefinitions,
+    referencesInTables: tableRefs,
+    warnings,
+  };
+}
+
+/**
+ * The optional `footnoteWarnings` field for a page-write tool result: present
+ * (with the warning lines) only when `markdown` has footnote problems, omitted
+ * otherwise. One helper so all three call sites (create/update/import) attach the
+ * field identically. Spread into the result: `{ ...result, ...footnoteWarningsField(text) }`.
+ */
+export function footnoteWarningsField(markdown: string): {
+  footnoteWarnings?: string[];
+} {
+  const { warnings } = analyzeFootnotes(markdown);
+  return warnings.length > 0 ? { footnoteWarnings: warnings } : {};
+}

packages/mcp/src/lib/footnote-lex.ts

@@ -0,0 +1,71 @@
+/**
+ * Shared, fence-aware line lexer for footnote markdown (MCP-internal).
+ *
+ * Both the importer (`extractFootnotes` in collaboration.ts, which strips
+ * definition lines and rebuilds a footnotes section) and the diagnostics
+ * (`analyzeFootnotes` in footnote-analyze.ts) must agree EXACTLY on which lines
+ * are definitions and which lines are inert (inside a code fence). Sharing one
+ * lexer makes "the analyzer sees what the importer leaves" a structural property
+ * instead of two hand-kept copies that can drift (#166 review).
+ *
+ * NOTE: this is deliberately NOT shared with editor-ext's
+ * `extractFootnoteDefinitions` — that lives in a different package and the
+ * decoupling between the editor and the MCP mirror is intentional.
+ */
+
+/** A footnote DEFINITION line: `[^id]: text` (id + text captured). */
+export const FOOTNOTE_DEF_RE = /^\[\^([^\]\s]+)\]:[ \t]*(.*)$/;
+/** Every footnote REFERENCE `[^id]` in a line (global; id captured). */
+export const FOOTNOTE_REF_RE_G = /\[\^([^\]\s]+)\]/g;
+/** Opening/closing code fence marker (``` or ~~~). */
+const FENCE_RE = /^(\s*)(`{3,}|~{3,})/;
+
+export interface FootnoteLine {
+  /** The raw line, verbatim. */
+  line: string;
+  /**
+   * True for a code-fence marker line AND every line inside a fence — footnote
+   * syntax on such lines is inert (example text, not real markup). The importer
+   * keeps these in the body; the analyzer skips them.
+   */
+  inFence: boolean;
+  /** The parsed definition, when this is a `[^id]: text` line OUTSIDE any fence. */
+  definition: { id: string; text: string } | null;
+}
+
+/** Classify every line of `markdown`, tracking fenced-code state. Pure. */
+export function lexFootnoteLines(markdown: string): FootnoteLine[] {
+  const out: FootnoteLine[] = [];
+  let fence: string | null = null;
+  for (const line of markdown.split("\n")) {
+    const fenceMatch = FENCE_RE.exec(line);
+    if (fenceMatch) {
+      const marker = fenceMatch[2][0];
+      if (fence === null) fence = marker; // opening fence
+      else if (marker === fence) fence = null; // matching closing fence
+      out.push({ line, inFence: true, definition: null });
+      continue;
+    }
+    if (fence !== null) {
+      out.push({ line, inFence: true, definition: null });
+      continue;
+    }
+    const m = FOOTNOTE_DEF_RE.exec(line);
+    out.push({
+      line,
+      inFence: false,
+      definition: m ? { id: m[1], text: m[2] } : null,
+    });
+  }
+  return out;
+}
+
+/** Scan a line for every `[^id]` reference, invoking `onRef(id)` for each. */
+export function forEachFootnoteReference(
+  line: string,
+  onRef: (id: string) => void,
+): void {
+  FOOTNOTE_REF_RE_G.lastIndex = 0;
+  let m: RegExpExecArray | null;
+  while ((m = FOOTNOTE_REF_RE_G.exec(line)) !== null) onRef(m[1]);
+}

packages/mcp/test/mock/footnote-warnings.test.mjs

@@ -0,0 +1,110 @@
+// Mock-HTTP test for the footnoteWarnings plumbing (#166). createPage is the
+// representative path that is fully plain-HTTP (import + getPage) and so is
+// mockable here; updatePage / importPageMarkdown attach footnoteWarnings with the
+// IDENTICAL wiring (`analyzeFootnotes(...)` + spread-when-non-empty) but run their
+// mutation over the Hocuspocus collab WebSocket, which this plain-HTTP harness
+// does not stand up. The analyzer itself is unit-tested in footnote-analyze.test.
+import { test, after } from "node:test";
+import assert from "node:assert/strict";
+import http from "node:http";
+import { DocmostClient } from "../../build/client.js";
+
+function readBody(req) {
+  return new Promise((resolve) => {
+    let raw = "";
+    req.on("data", (c) => (raw += c));
+    req.on("end", () => resolve(raw));
+  });
+}
+
+function sendJson(res, status, obj, extraHeaders = {}) {
+  res.writeHead(status, { "Content-Type": "application/json", ...extraHeaders });
+  res.end(JSON.stringify(obj));
+}
+
+const openServers = [];
+function spawn(handler) {
+  return new Promise((resolve) => {
+    const server = http.createServer(handler);
+    openServers.push(server);
+    server.listen(0, "127.0.0.1", () => {
+      const { port } = server.address();
+      resolve(`http://127.0.0.1:${port}/api`);
+    });
+  });
+}
+
+after(async () => {
+  await Promise.all(
+    openServers.map((s) => new Promise((r) => s.close(r))),
+  );
+});
+
+// A handler that imports a page, lets getPage read it back, and 404s everything
+// else (listSidebarPages fails gracefully inside getPage).
+function pageHandler() {
+  return async (req, res) => {
+    await readBody(req);
+    if (req.url === "/api/auth/login") {
+      sendJson(res, 200, { success: true }, {
+        "Set-Cookie": "authToken=t; Path=/; HttpOnly",
+      });
+      return;
+    }
+    if (req.url === "/api/pages/import") {
+      sendJson(res, 200, { data: { id: "new-1" } });
+      return;
+    }
+    if (req.url === "/api/pages/update") {
+      // The title-restore step after import.
+      sendJson(res, 200, { data: { id: "new-1" } });
+      return;
+    }
+    if (req.url === "/api/pages/info") {
+      sendJson(res, 200, {
+        data: {
+          id: "new-1",
+          slugId: "slug-1",
+          title: "T",
+          spaceId: "sp-1",
+          content: { type: "doc", content: [] },
+        },
+      });
+      return;
+    }
+    sendJson(res, 404, { message: "not found" });
+  };
+}
+
+test("createPage attaches footnoteWarnings when the content has footnote problems", async () => {
+  const baseURL = await spawn(pageHandler());
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+  // A dangling reference + a duplicate definition + a table marker.
+  const content = [
+    "Intro[^missing] and| cell[^t] |.",
+    "",
+    "[^d]: one",
+    "[^d]: two",
+    "[^t]: in table",
+  ].join("\n");
+  const result = await client.createPage("T", content, "sp-1");
+  assert.ok(Array.isArray(result.footnoteWarnings), "footnoteWarnings present");
+  const joined = result.footnoteWarnings.join("\n");
+  assert.match(joined, /no matching definition/); // dangling [^missing]
+  assert.match(joined, /defined more than once/); // duplicate [^d]
+  // The page itself is still returned.
+  assert.equal(result.success, true);
+});
+
+test("createPage omits footnoteWarnings when the content is clean", async () => {
+  const baseURL = await spawn(pageHandler());
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+  const content = ["A[^a] and reuse[^a].", "", "[^a]: fine"].join("\n");
+  const result = await client.createPage("T", content, "sp-1");
+  assert.equal(
+    "footnoteWarnings" in result,
+    false,
+    "no footnoteWarnings field on clean input",
+  );
+  assert.equal(result.success, true);
+});

packages/mcp/test/unit/derive-id-parity.test.mjs

@@ -1,134 +0,0 @@
-import { test } from "node:test";
-import assert from "node:assert/strict";
-
-import { markdownToProseMirror } from "../../build/lib/collaboration.js";
-
-/**
- * CROSS-PACKAGE DRIFT GUARD for the footnote id derivation scheme.
- *
- * `deriveFootnoteId` is duplicated in two places that MUST behave identically:
- *   - packages/editor-ext/src/lib/footnote/footnote-util.ts (exported)
- *   - packages/mcp/src/lib/collaboration.ts                  (internal helper)
- * so the same markdown imported through the editor and through the MCP path
- * derives identical footnote ids.
- *
- * The mcp copy is NOT exported from the compiled build (it is an internal helper
- * of collaboration.js), and production source must not be modified to export it.
- * So this test exercises the REAL compiled `deriveFootnoteId` *indirectly*, the
- * same way production does: through `markdownToProseMirror`, which runs
- * extractFootnotes -> deriveFootnoteId during duplicate-id dedup. We craft the
- * `taken` set via literal pre-existing definition ids and read back the derived
- * footnoteDefinition ids.
- *
- * GOLDEN below mirrors DERIVE_GOLDEN in
- *   packages/editor-ext/src/lib/footnote/footnote-util.derive-id.test.ts
- * (asserted there by a DIRECT call). Same (originalId, occurrence, taken) ->
- * same expected id. If the two copies drift, one of the two suites goes red.
- */
-
-/** The 25 single-letter suffixes the scheme uses (n=1..25): b, c, ..., z. */
-function singleLetterSuffixes() {
-  return Array.from({ length: 25 }, (_, i) => String.fromCharCode(98 + i));
-}
-
-// Identical matrix + expected values to the editor-ext golden table.
-const GOLDEN = [
-  { originalId: "d", occurrence: 2, taken: [], expected: "d__2" },
-  { originalId: "d", occurrence: 3, taken: [], expected: "d__3" },
-  { originalId: "d", occurrence: 2, taken: ["d__2"], expected: "d__2b" },
-  { originalId: "d", occurrence: 2, taken: ["d__2", "d__2b"], expected: "d__2c" },
-  {
-    originalId: "d",
-    occurrence: 2,
-    taken: ["d__2", "d__2b", "d__2c", "d__2d"],
-    expected: "d__2e",
-  },
-  {
-    originalId: "d",
-    occurrence: 2,
-    taken: ["d__2", ...singleLetterSuffixes().map((s) => `d__2${s}`)],
-    expected: "d__2bb",
-  },
-];
-
-/** Recursively collect every node of `type`. */
-function findAll(node, type, acc = []) {
-  if (!node || typeof node !== "object") return acc;
-  if (node.type === type) acc.push(node);
-  if (Array.isArray(node.content)) for (const c of node.content) findAll(c, type, acc);
-  return acc;
-}
-
-/**
- * Build markdown that drives the real `deriveFootnoteId(originalId, occurrence,
- * taken)`:
- *  - `occurrence` duplicate definitions of `[^originalId]` so the dedup walk
- *    reaches the requested occurrence (occurrence=2 -> 1 keeper + 1 duplicate;
- *    occurrence=3 -> keeper + 2 duplicates, of which the LAST is the one whose
- *    id we read);
- *  - one literal pre-existing definition for every id in `taken`, each with its
- *    own reference marker so it is a real (non-orphan) definition. Those ids are
- *    reserved up-front in the dedup `taken` set, exactly forcing the bump.
- *
- * Returns the derived id of the FINAL duplicate of `originalId`.
- */
-async function deriveViaMarkdown(originalId, occurrence, takenIds) {
-  // References: one [^originalId] per definition (keeper + duplicates) so each
-  // duplicate has a marker to pair with, plus one marker per taken id.
-  const dupCount = occurrence; // keeper + (occurrence-1) duplicates = `occurrence` defs
-  const refMarkers = [];
-  for (let i = 0; i < dupCount; i++) refMarkers.push(`[^${originalId}]`);
-  for (const id of takenIds) refMarkers.push(`[^${id}]`);
-  const refLine = `Body ${refMarkers.join(" ")}.`;
-
-  // Definitions: `occurrence` copies of [^originalId]: ... then the taken ids.
-  const defLines = [];
-  for (let i = 0; i < dupCount; i++) {
-    defLines.push(`[^${originalId}]: copy ${i}`);
-  }
-  for (const id of takenIds) {
-    defLines.push(`[^${id}]: reserved ${id}`);
-  }
-
-  const md = [refLine, "", ...defLines].join("\n");
-  const json = await markdownToProseMirror(md);
-  const defIds = findAll(json, "footnoteDefinition").map((d) => d.attrs.id);
-
-  // The derived id we want is the one that is neither the keeper (originalId),
-  // nor any reserved taken id, nor a lower-occurrence derived id. For
-  // occurrence=2 that is the single bumped id; for occurrence=3 it is the
-  // highest `${originalId}__3...` id. Compute it generically: among the def ids
-  // that start with `${originalId}__${occurrence}`, the expected one is present.
-  return { defIds, json };
-}
-
-for (const row of GOLDEN) {
-  test(`parity: derive("${row.originalId}", ${row.occurrence}, {${row.taken.join(",")}}) -> "${row.expected}"`, async () => {
-    const { defIds } = await deriveViaMarkdown(
-      row.originalId,
-      row.occurrence,
-      row.taken,
-    );
-    // The real compiled deriveFootnoteId must have minted exactly the golden id.
-    assert.ok(
-      defIds.includes(row.expected),
-      `expected derived id "${row.expected}" among def ids ${JSON.stringify(defIds)}`,
-    );
-    // And every id is distinct: nothing collapsed.
-    assert.equal(new Set(defIds).size, defIds.length, "all def ids distinct");
-  });
-}
-
-test("parity: the simple keeper+two-duplicate case mints d, d__2, d__3", async () => {
-  // The canonical no-collision path, asserted as a whole set for clarity.
-  const md = [
-    "See[^d] one[^d] two[^d].",
-    "",
-    "[^d]: first",
-    "[^d]: second",
-    "[^d]: third",
-  ].join("\n");
-  const json = await markdownToProseMirror(md);
-  const defIds = findAll(json, "footnoteDefinition").map((d) => d.attrs.id);
-  assert.deepEqual([...defIds].sort(), ["d", "d__2", "d__3"]);
-});

packages/mcp/test/unit/footnote-analyze.test.mjs

@@ -0,0 +1,106 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+import { analyzeFootnotes } from "../../build/lib/footnote-analyze.js";
+
+test("clean footnotes produce no diagnostics", () => {
+  const md = ["A[^a] and B[^b].", "", "[^a]: first", "[^b]: second"].join("\n");
+  const d = analyzeFootnotes(md);
+  assert.deepEqual(d.danglingReferences, []);
+  assert.deepEqual(d.emptyDefinitions, []);
+  assert.deepEqual(d.duplicateDefinitions, []);
+  assert.deepEqual(d.referencesInTables, []);
+  assert.deepEqual(d.warnings, []);
+});
+
+test("reuse (repeated references to one definition) is NOT a warning", () => {
+  const md = ["A[^a] B[^a] C[^a].", "", "[^a]: shared"].join("\n");
+  const d = analyzeFootnotes(md);
+  assert.deepEqual(d.danglingReferences, []);
+  assert.deepEqual(d.warnings, []);
+});
+
+test("dangling reference (no definition) is reported", () => {
+  const md = ["See[^missing] and[^a].", "", "[^a]: defined"].join("\n");
+  const d = analyzeFootnotes(md);
+  assert.deepEqual(d.danglingReferences, ["missing"]);
+  assert.equal(d.warnings.length, 1);
+  assert.match(d.warnings[0], /no matching definition/);
+  assert.match(d.warnings[0], /\[\^missing\]/);
+});
+
+test("empty definition text is reported", () => {
+  const md = ["See[^a].", "", "[^a]:   "].join("\n");
+  const d = analyzeFootnotes(md);
+  assert.deepEqual(d.emptyDefinitions, ["a"]);
+  assert.match(d.warnings.join("\n"), /empty text/);
+});
+
+test("duplicate definition id is reported (first-wins)", () => {
+  const md = ["See[^d].", "", "[^d]: first", "[^d]: second"].join("\n");
+  const d = analyzeFootnotes(md);
+  assert.deepEqual(d.duplicateDefinitions, ["d"]);
+  assert.match(d.warnings.join("\n"), /defined more than once/);
+});
+
+test("reference inside a GFM table row is reported (heuristic)", () => {
+  const md = [
+    "| Col |",
+    "| --- |",
+    "| cell[^t] |",
+    "",
+    "[^t]: table note",
+  ].join("\n");
+  const d = analyzeFootnotes(md);
+  assert.deepEqual(d.referencesInTables, ["t"]);
+  assert.match(d.warnings.join("\n"), /table/);
+  // It is defined, so it is NOT also dangling.
+  assert.deepEqual(d.danglingReferences, []);
+});
+
+test("footnote syntax inside a code fence is ignored", () => {
+  const md = [
+    "Intro.",
+    "",
+    "```",
+    "Example[^demo]",
+    "[^demo]: not a real definition",
+    "```",
+    "",
+    "Outro[^a].",
+    "",
+    "[^a]: real",
+  ].join("\n");
+  const d = analyzeFootnotes(md);
+  // `[^demo]` lives only in the fenced block, so it is neither a reference nor a
+  // dangling one, and `[^demo]:` is not counted as a definition.
+  assert.deepEqual(d.danglingReferences, []);
+  assert.deepEqual(d.duplicateDefinitions, []);
+  assert.deepEqual(d.warnings, []);
+});
+
+test("a reference that only appears inside a definition's text is not dangling", () => {
+  // `[^b]` is referenced from within [^a]'s text and has its own definition.
+  const md = ["See[^a].", "", "[^a]: see also [^b]", "[^b]: the other"].join(
+    "\n",
+  );
+  const d = analyzeFootnotes(md);
+  assert.deepEqual(d.danglingReferences, []);
+});
+
+test("multiple problem classes accumulate distinct warnings", () => {
+  const md = [
+    "Ref[^x] and[^dup].",
+    "",
+    "[^dup]: one",
+    "[^dup]: two",
+    "[^empty]:",
+  ].join("\n");
+  const d = analyzeFootnotes(md);
+  // x has no definition; dup is defined twice; empty is empty AND has no ref.
+  assert.ok(d.danglingReferences.includes("x"));
+  assert.deepEqual(d.duplicateDefinitions, ["dup"]);
+  assert.deepEqual(d.emptyDefinitions, ["empty"]);
+  // One warning line per problem class present.
+  assert.ok(d.warnings.length >= 3);
+});

packages/mcp/test/unit/footnote-warnings-import.test.mjs

@@ -0,0 +1,63 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+import {
+  analyzeFootnotes,
+  footnoteWarningsField,
+} from "../../build/lib/footnote-analyze.js";
+import {
+  serializeDocmostMarkdown,
+  parseDocmostMarkdown,
+} from "../../build/lib/markdown-document.js";
+
+// Pins the footnoteWarnings PLUMBING contract (#169 review): the field is
+// present only on problems and omitted on clean input, AND `import_page_markdown`
+// analyzes the BODY (after the docmost:meta / docmost:comments blocks) — so a
+// footnote-like token inside those JSON blocks never warns, while a real marker
+// in the body does. importPageMarkdown does exactly
+// `footnoteWarningsField(parseDocmostMarkdown(full).body)` over a collab socket
+// this harness does not stand up, so we test the same pure composition directly.
+
+test("footnoteWarningsField is present on problems and omitted on clean input", () => {
+  const problem = footnoteWarningsField("See[^missing].\n\n[^a]: defined");
+  assert.ok(Array.isArray(problem.footnoteWarnings));
+  assert.match(problem.footnoteWarnings.join("\n"), /no matching definition/);
+
+  const clean = footnoteWarningsField("A[^a] and reuse[^a].\n\n[^a]: fine");
+  assert.deepEqual(clean, {}); // no key at all on clean input
+});
+
+test("import analyzes the BODY only — tokens inside meta/comments never warn", () => {
+  // meta + comments JSON carry `[^metaonly]` / `[^commentonly]`-looking text; the
+  // BODY has a genuinely dangling `[^bodyref]`.
+  const full = serializeDocmostMarkdown(
+    { pageId: "p1", note: "front-matter mentions [^metaonly] in text" },
+    "Body with a dangling[^bodyref] marker.",
+    [{ id: "c1", content: "a comment that says [^commentonly]" }],
+  );
+
+  const { body } = parseDocmostMarkdown(full);
+  // Sanity: the meta/comments markers are NOT in the parsed body.
+  assert.ok(!body.includes("[^metaonly]"));
+  assert.ok(!body.includes("[^commentonly]"));
+
+  const field = footnoteWarningsField(body);
+  const joined = (field.footnoteWarnings ?? []).join("\n");
+  // ONLY the body's dangling reference is flagged.
+  assert.match(joined, /\[\^bodyref\]/);
+  assert.ok(!joined.includes("metaonly"));
+  assert.ok(!joined.includes("commentonly"));
+
+  // Cross-check against analyzeFootnotes directly (same composition the importer uses).
+  assert.deepEqual(analyzeFootnotes(body).danglingReferences, ["bodyref"]);
+});
+
+test("import on a clean body yields no footnoteWarnings field", () => {
+  const full = serializeDocmostMarkdown(
+    { pageId: "p1" },
+    "Clean body[^a] reusing[^a].\n\n[^a]: ok",
+    [],
+  );
+  const { body } = parseDocmostMarkdown(full);
+  assert.deepEqual(footnoteWarningsField(body), {});
+});

packages/mcp/test/unit/footnotes.test.mjs

@@ -90,11 +90,10 @@ test("JSON -> MD -> JSON preserves footnote ids and text", async () => {
   assert.match(md2, /\[\^fn2\]: Second note\./);
 });
 
-test("duplicate-id markdown dedups DETERMINISTICALLY (same input -> same ids)", async () => {
-  // The MCP import must derive duplicate ids deterministically (NOT random) so
-  // the same markdown imported here and via the editor produces identical ids,
-  // and re-importing is stable. This is the test that would FAIL on the old
-  // Math.random()/Date.now() implementation.
+test("repeated references REUSE one footnote; duplicate definitions are first-wins (#166)", async () => {
+  // Reuse semantics: many `[^d]` references + several `[^d]:` definitions import
+  // as ONE footnote — the references all keep id "d" (reuse), and only the FIRST
+  // definition is kept (first-wins). Deterministic and stable across re-imports.
   const md = [
     "See[^d] one[^d] two[^d].",
     "",
@@ -106,21 +105,26 @@ test("duplicate-id markdown dedups DETERMINISTICALLY (same input -> same ids)",
   const idsOf = async () => {
     const json = await markdownToProseMirror(md);
     const refs = findAll(json, "footnoteReference").map((r) => r.attrs.id);
-    const defs = findAll(json, "footnoteDefinition").map((d) => d.attrs.id);
-    return { refs, defs };
+    const defs = findAll(json, "footnoteDefinition");
+    return {
+      refs,
+      defIds: defs.map((d) => d.attrs.id),
+      defText: defs
+        .map((d) => JSON.stringify(d).match(/"text":"([^"]*)"/)?.[1])
+        .join("|"),
+    };
   };
 
   const a = await idsOf();
   const b = await idsOf();
 
-  // Identical across runs.
-  assert.deepEqual(a.refs, b.refs);
-  assert.deepEqual(a.defs, b.defs);
-  // Deterministic derived scheme: keeper "d", duplicates "d__2", "d__3".
-  assert.deepEqual([...a.defs].sort(), ["d", "d__2", "d__3"]);
-  // 1:1 reference <-> definition pairing, all distinct.
-  assert.equal(new Set(a.defs).size, 3);
-  assert.deepEqual([...a.refs].sort(), [...a.defs].sort());
+  // Stable across runs.
+  assert.deepEqual(a, b);
+  // Reuse: all three reference markers stay "d".
+  assert.deepEqual(a.refs, ["d", "d", "d"]);
+  // First-wins: a single definition "d" with the FIRST text.
+  assert.deepEqual(a.defIds, ["d"]);
+  assert.equal(a.defText, "first");
 });
 
 test("a [^id]: line inside a fenced code block is NOT treated as a definition", async () => {