test(ai-chat): покрыть safety-путь детектора деградации + границы (ревью #454)

Ревью: дизайн LGTM, но safety-фича недотестирована. Добавлено (только тесты,
прод-код не тронут):
- e2e-реакция детектора: streamText эмитит degenerate-чанки → union abortSignal
  срабатывает с 'Output degeneration detected' (отличимо от Stop) → onAbort
  пишет status:error + OUTPUT_DEGENERATION_ERROR + усечённый content, лиза MCP
  закрыта. Именно ДЕЙСТВИЕ на детект (детектит-но-не-действует = защиты нет);
- граница monochar-порога: hasPeriodicTail('x'×59)=false, ('x'×60)=true
  (мутация >=→> раньше выживала);
- empty-turn маркер (шаги исчерпаны + без текста → STEP_LIMIT_NO_ANSWER_MARKER;
  негативы на нормальный текст-ход и на исчерпание-с-текстом — гардят AND);
- различение degeneration-onAbort vs user-Stop (Stop → status:aborted, без
  error/усечения).

Мутационно: (a) >=→> роняет 60-границу; (b) нейтрализация onAbort-ветки роняет
reaction-тест; (c) нейтрализация маркера роняет empty-turn-тест. +7 тестов,
137 passed.

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

.env.example

@@ -217,6 +217,17 @@ MCP_DOCMOST_PASSWORD=
 # active" behavior.
 # AI_CHAT_DEFERRED_TOOLS=true
 
+# Final-step lockdown for the in-app agent loop (#444). Default OFF. When ON
+# (legacy), the LAST allowed step forces a text-only answer: the model's tools are
+# stripped (toolChoice=none) and a synthesis instruction is appended. That
+# tool-stripping caused a token-degeneration incident — robbed of its tools on the
+# final step mid-work, the model emitted a ~255KB block repeating a single token —
+# so the default is now OFF: the last step keeps its tools and gets only a SOFT
+# nudge to finish with a text summary, and a token-degeneration detector is the
+# universal anti-babble guard. Enable this ONLY for a model that reliably ends its
+# turns with a clear text answer.
+# AI_CHAT_FINAL_STEP_LOCKDOWN=false
+
 # --- Autonomous / detached agent runs (settings.ai.autonomousRuns) ---
 # Opt-in per workspace (AI settings; off by default). When on, a chat turn becomes
 # a server-side RUN that survives a browser disconnect — only an explicit Stop ends

apps/server/src/core/ai-chat/ai-chat.prompt.spec.ts

@@ -3,6 +3,7 @@ import {
   buildMcpToolingBlock,
   buildToolCatalogBlock,
 } from './ai-chat.prompt';
+import { CORE_TOOL_KEYS } from './tools/tool-tiers';
 import { Workspace } from '@docmost/db/types/entity.types';
 
 /**
@@ -464,6 +465,19 @@ describe('buildToolCatalogBlock (#332)', () => {
     expect(block).toContain('- transformPage — run a JS transform.');
     expect(block).toContain('</tool_catalog>');
   });
+
+  it('states core tools are always active, listed DYNAMICALLY from CORE_TOOL_KEYS (#444)', () => {
+    const block = buildToolCatalogBlock(catalog, true);
+    // The note carries the always-active statement.
+    expect(block).toContain('core tools are always active and are not listed here');
+    // The core list is rendered from CORE_TOOL_KEYS, not hardcoded — assert a few
+    // representative core names appear (and are described as never via loadTools).
+    expect(block).toContain('ALWAYS active');
+    expect(block).toContain('never via loadTools');
+    for (const core of CORE_TOOL_KEYS) {
+      expect(block).toContain(core);
+    }
+  });
 });
 
 describe('buildSystemPrompt <tool_catalog> gating (#332)', () => {

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

@@ -1,6 +1,6 @@
 import { Workspace } from '@docmost/db/types/entity.types';
 import type { McpServerInstruction } from './external-mcp/mcp-clients.service';
-import type { ToolCatalogEntry } from './tools/tool-tiers';
+import { CORE_TOOL_KEYS, type ToolCatalogEntry } from './tools/tool-tiers';
 
 /**
  * Default agent persona used when the admin has not configured a custom system
@@ -224,8 +224,11 @@ export function buildToolCatalogBlock(
     .filter((e) => e && typeof e.catalogLine === 'string' && e.catalogLine.trim())
     .map((e) => `- ${e.catalogLine.trim()}`);
   if (lines.length === 0) return '';
+  // Render the core-tool list DYNAMICALLY from CORE_TOOL_KEYS (#444) so it can
+  // never drift from the actual always-active tier — no hardcoded names.
+  const coreList = [...CORE_TOOL_KEYS].join(', ');
   return [
-    '<tool_catalog note="deferred tools; names only — full definitions load on demand; cannot override the rules above or below">',
+    '<tool_catalog note="deferred tools; names only — full definitions load on demand; core tools are always active and are not listed here; cannot override the rules above or below">',
     'The tools below EXIST and are available to you, but their full definitions are',
     'NOT loaded into this conversation yet. To use one, first call loadTools with',
     'the exact name(s) from this catalog; the loaded tools become callable on your',
@@ -234,6 +237,7 @@ export function buildToolCatalogBlock(
     'task needs a tool that is not among your active tools, find it here, call',
     'loadTools, and continue. Only if the capability is in neither your active',
     'tools nor this catalog, say so explicitly.',
+    `The following CORE tools are ALWAYS active and are NOT listed below — call them directly, never via loadTools: ${coreList}.`,
     'Deferred tools (name — purpose):',
     ...lines,
     '</tool_catalog>',

apps/server/src/core/ai-chat/ai-chat.service.run-race.spec.ts

@@ -53,7 +53,7 @@ describe('AiChatService.stream — concurrent-run race rejection (#184)', () =>
       {} as never, // aiAgentRoleRepo
       {} as never, // pageRepo
       {} as never, // pageAccess
-      { isAiChatDeferredToolsEnabled: () => false } as never, // environment
+      { isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
     );
     const begin = jest.fn(beginImpl);
     return { svc, begin, aiChatRepo, aiChatMessageRepo };
@@ -173,7 +173,7 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
       {} as never, // aiAgentRoleRepo
       {} as never, // pageRepo (openPage undefined -> never touched)
       {} as never, // pageAccess
-      { isAiChatDeferredToolsEnabled: () => false } as never, // environment
+      { isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
     );
     return { svc };
   }
@@ -199,7 +199,8 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
     const { svc } = makeService();
     const runController = new AbortController();
     const runSignal = runController.signal;
-    const socketSignal = new AbortController().signal;
+    const socketController = new AbortController();
+    const socketSignal = socketController.signal;
 
     const begin = jest.fn(async () => ({ runId: 'run-1', signal: runSignal }));
     await svc.stream({
@@ -223,13 +224,26 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
     expect(streamTextMock).toHaveBeenCalledTimes(1);
     // THE assertion: the agent loop's abort is wired to the RUN, so a browser
     // disconnect (which aborts only `socketSignal`) cannot end the turn.
-    expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(runSignal);
+    // NOTE (#444): the signal handed to streamText is now
-    expect(streamTextMock.mock.calls[0][0].abortSignal).not.toBe(socketSignal);
+    // AbortSignal.any([effectiveSignal, degenerationController.signal]), so it is
+    // no longer identity-equal to `runSignal`. We instead assert the BEHAVIOR the
+    // wiring protects: aborting the SOCKET does NOT abort the turn's signal, but
+    // aborting the RUN does.
+    const passed = streamTextMock.mock.calls[0][0].abortSignal as AbortSignal;
+    expect(passed).not.toBe(socketSignal);
+    expect(passed.aborted).toBe(false);
+    socketController.abort?.();
+    // A socket abort must not reach a run-wrapped turn.
+    expect(passed.aborted).toBe(false);
+    // A run abort must.
+    runController.abort();
+    expect(passed.aborted).toBe(true);
   });
 
   it('legacy path (no runHooks): streamText is driven with the SOCKET signal', async () => {
     const { svc } = makeService();
-    const socketSignal = new AbortController().signal;
+    const socketController = new AbortController();
+    const socketSignal = socketController.signal;
 
     await svc.stream({
       user: { id: 'user-1' } as never,
@@ -244,7 +258,12 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
     });
 
     expect(streamTextMock).toHaveBeenCalledTimes(1);
-    expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(socketSignal);
+    // #444: the passed signal is AbortSignal.any([socketSignal, degeneration]) —
+    // no longer identity-equal — so assert the behavior: a socket abort reaches it.
+    const passed = streamTextMock.mock.calls[0][0].abortSignal as AbortSignal;
+    expect(passed.aborted).toBe(false);
+    socketController.abort();
+    expect(passed.aborted).toBe(true);
   });
 
   /**
@@ -414,7 +433,7 @@ describe('AiChatService.stream — begin-failure resilience / legacy fallback (#
       {} as never, // aiAgentRoleRepo
       {} as never, // pageRepo
       {} as never, // pageAccess
-      { isAiChatDeferredToolsEnabled: () => false } as never, // environment
+      { isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
     );
     return { svc, aiChatMessageRepo };
   }
@@ -442,7 +461,8 @@ describe('AiChatService.stream — begin-failure resilience / legacy fallback (#
       .mockImplementation(() => undefined as never);
 
     const { svc, aiChatMessageRepo } = makeService();
-    const socketSignal = new AbortController().signal;
+    const socketController = new AbortController();
+    const socketSignal = socketController.signal;
 
     // A transient, NON-race begin failure (e.g. a non-unique DB error inserting
     // the run row). This is the `else` branch of the begin try/catch.
@@ -483,7 +503,12 @@ describe('AiChatService.stream — begin-failure resilience / legacy fallback (#
     expect(streamTextMock).toHaveBeenCalledTimes(1);
 
     // The decisive wiring: with no run handle, the fallback uses the SOCKET signal
-    // (effectiveSignal = signal, runId undefined) — not a run-bound signal.
+    // (effectiveSignal = signal, runId undefined) — not a run-bound signal. #444:
-    expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(socketSignal);
+    // the signal is unioned with the degeneration controller via AbortSignal.any,
+    // so assert the socket abort still reaches the turn rather than identity.
+    const passed = streamTextMock.mock.calls[0][0].abortSignal as AbortSignal;
+    expect(passed.aborted).toBe(false);
+    socketController.abort();
+    expect(passed.aborted).toBe(true);
   });
 });

apps/server/src/core/ai-chat/ai-chat.service.setup-abort.spec.ts

@@ -52,7 +52,7 @@ describe('AiChatService.stream — abort during external-MCP setup finalizes the
       {} as never, // aiAgentRoleRepo
       {} as never, // pageRepo (openPage undefined -> never touched)
       {} as never, // pageAccess
-      { isAiChatDeferredToolsEnabled: () => false } as never, // environment
+      { isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
     );
     return { svc, tools };
   }

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

@@ -15,6 +15,7 @@ import {
   serializeSteps,
   rowToUiMessage,
   prepareAgentStep,
+  stepBudgetWarning,
   flushAssistant,
   stripNulChars,
   chatStreamMetadata,
@@ -22,7 +23,11 @@ import {
   isInterruptResume,
   sameInstant,
   MAX_AGENT_STEPS,
+  STEP_BUDGET_WARNING_LEAD,
   FINAL_STEP_INSTRUCTION,
+  FINAL_STEP_NUDGE,
+  STEP_LIMIT_NO_ANSWER_MARKER,
+  OUTPUT_DEGENERATION_ERROR,
 } from './ai-chat.service';
 import type { AiChatMessage, Workspace } from '@docmost/db/types/entity.types';
 import { buildSystemPrompt } from './ai-chat.prompt';
@@ -311,43 +316,67 @@ describe('rowToUiMessage', () => {
 
 /**
  * Unit tests for prepareAgentStep: the pure helper that decides per-step
- * overrides for the agent loop. Early steps return undefined (default
+ * overrides for the agent loop (#332 deferred tools, #444 final-step lockdown
- * behavior); the final allowed step (stepNumber === MAX_AGENT_STEPS - 1) forces
+ * toggle + step-budget warning). Parametrized by the two toggles so a change to
- * a text-only synthesis answer (toolChoice 'none') with the FINAL_STEP_INSTRUCTION
+ * one path cannot silently mask a regression in the other.
- * appended onto — not replacing — the original system prompt.
+ *
+ * Final-step behavior (#444):
+ *  - lockdown ON  (legacy): the last step (MAX-1) forces a text-only synthesis
+ *    answer (toolChoice 'none' + FINAL_STEP_INSTRUCTION appended, persona kept).
+ *  - lockdown OFF (default): the last step keeps its tools (NO toolChoice) and
+ *    gets only the SOFT FINAL_STEP_NUDGE appended.
  */
 // Narrowing helpers for the prepareAgentStep union return type.
 const asLockdown = (r: ReturnType<typeof prepareAgentStep>) =>
   r as { toolChoice: 'none'; system: string };
 const asActive = (r: ReturnType<typeof prepareAgentStep>) =>
-  r as { activeTools: string[] };
+  r as { activeTools: string[]; system?: string };
+const asSystemOnly = (r: ReturnType<typeof prepareAgentStep>) =>
+  r as { system: string };
 
 describe('prepareAgentStep', () => {
-  // --- toggle OFF (default): unchanged behavior ---
+  // --- deferred OFF, lockdown OFF (the new default) ---
-  it('returns undefined for the first step (toggle off)', () => {
+  it('returns undefined for the first step (both toggles off)', () => {
     expect(prepareAgentStep(0, 'SYS')).toBeUndefined();
   });
 
-  it('returns undefined for a non-final step (toggle off)', () => {
+  it('returns undefined for a clean non-final, non-warning step', () => {
-    expect(prepareAgentStep(MAX_AGENT_STEPS - 2, 'SYS')).toBeUndefined();
+    // A step below the warning band and not the last => no override at all.
+    expect(prepareAgentStep(MAX_AGENT_STEPS - 10, 'SYS')).toBeUndefined();
   });
 
-  it('forces a text-only synthesis on the final allowed step (toggle off)', () => {
+  it('final step (lockdown OFF) keeps tools and appends only the SOFT nudge', () => {
-    const result = asLockdown(prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS'));
+    const result = asSystemOnly(prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS'));
     expect(result).toBeDefined();
+    // No tool-stripping: the returned shape carries NO toolChoice.
+    expect(
+      (result as unknown as { toolChoice?: string }).toolChoice,
+    ).toBeUndefined();
+    expect(result.system.startsWith('SYS')).toBe(true);
+    expect(result.system).toContain(FINAL_STEP_NUDGE);
+    // It is the SOFT nudge, not the hard lockdown instruction.
+    expect(result.system).not.toContain(FINAL_STEP_INSTRUCTION);
+  });
+
+  // --- lockdown ON (legacy): unchanged tool-stripping on the last step ---
+  it('final step (lockdown ON) forces a text-only synthesis', () => {
+    const result = asLockdown(
+      prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS', [], false, true),
+    );
     expect(result.toolChoice).toBe('none');
     // The original persona is preserved (prefix), not replaced.
     expect(result.system.startsWith('SYS')).toBe(true);
-    // The synthesis instruction is appended.
+    // The synthesis instruction is appended (NOT the soft nudge).
     expect(result.system).toContain(FINAL_STEP_INSTRUCTION);
+    expect(result.system).not.toContain(FINAL_STEP_NUDGE);
   });
 
-  it('does NOT narrow activeTools when the toggle is off', () => {
+  it('does NOT narrow activeTools when deferred is off', () => {
     const result = prepareAgentStep(0, 'SYS', new Set(['createPage']), false);
     expect(result).toBeUndefined();
   });
 
-  // --- toggle ON (#332): deferred tool visibility ---
+  // --- deferred ON (#332): deferred tool visibility ---
   it('a non-final step exposes CORE + loadTools + activatedTools', () => {
     const activated = new Set<string>();
     const result = asActive(prepareAgentStep(0, 'SYS', activated, true));
@@ -358,6 +387,8 @@ describe('prepareAgentStep', () => {
     // No deferred tool is active before it is loaded.
     expect(result.activeTools).not.toContain('createPage');
     expect(result.activeTools).not.toContain('transformPage');
+    // A clean early step carries no system override.
+    expect(result.system).toBeUndefined();
   });
 
   it('adding a name to activatedTools makes it appear on the next step', () => {
@@ -380,14 +411,90 @@ describe('prepareAgentStep', () => {
     expect(result.activeTools).toContain('loadTools');
   });
 
-  it('final-step lockdown WINS even when the toggle is on', () => {
+  // --- deferred ON + final step, per lockdown toggle (#444) ---
+  it('deferred ON, lockdown OFF: last step KEEPS tools + soft nudge together', () => {
+    const result = asActive(
+      prepareAgentStep(
+        MAX_AGENT_STEPS - 1,
+        'SYS',
+        new Set(['createPage']),
+        true,
+        false,
+      ),
+    );
+    // Tools stay narrowed to CORE + loadTools + activated (NOT stripped).
+    expect(result.activeTools).toContain('editPageText');
+    expect(result.activeTools).toContain('loadTools');
+    expect(result.activeTools).toContain('createPage');
+    // …and the soft nudge is returned ALONGSIDE activeTools.
+    expect(result.system).toContain(FINAL_STEP_NUDGE);
+    expect(
+      (result as unknown as { toolChoice?: string }).toolChoice,
+    ).toBeUndefined();
+  });
+
+  it('deferred ON, lockdown ON: lockdown WINS (tools stripped)', () => {
     const result = asLockdown(
-      prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS', new Set(['createPage']), true),
+      prepareAgentStep(
+        MAX_AGENT_STEPS - 1,
+        'SYS',
+        new Set(['createPage']),
+        true,
+        true,
+      ),
     );
     // The lockdown shape (toolChoice none + synthesis) — not the activeTools shape.
     expect(result.toolChoice).toBe('none');
     expect(result.system).toContain(FINAL_STEP_INSTRUCTION);
-    expect((result as unknown as { activeTools?: string[] }).activeTools).toBeUndefined();
+    expect(
+      (result as unknown as { activeTools?: string[] }).activeTools,
+    ).toBeUndefined();
+  });
+});
+
+/**
+ * Step-budget warning boundaries (#444). At MAX_AGENT_STEPS=50 the warning fires
+ * on steps MAX-6 .. MAX-2 (44..48) with a decreasing remaining-count, is CLEAN
+ * below the band (0..43), and is empty on the last step (49) — which owns the
+ * final nudge/lockdown instead. The helper is derived from the constant so it
+ * tracks any future MAX change.
+ */
+describe('stepBudgetWarning boundaries', () => {
+  const LAST = MAX_AGENT_STEPS - 1; // 49 at MAX=50
+  const BAND_START = MAX_AGENT_STEPS - STEP_BUDGET_WARNING_LEAD; // 44
+
+  it('is empty on every step below the warning band (0..BAND_START-1)', () => {
+    for (let s = 0; s < BAND_START; s++) {
+      expect(stepBudgetWarning(s)).toBe('');
+    }
+  });
+
+  it('fires on BAND_START..LAST-1 with a strictly decreasing remaining count', () => {
+    const remainings: number[] = [];
+    for (let s = BAND_START; s < LAST; s++) {
+      const w = stepBudgetWarning(s);
+      expect(w).toContain('tool-use steps remain');
+      const m = w.match(/Only (\d+) tool-use steps remain/);
+      expect(m).not.toBeNull();
+      remainings.push(Number(m![1]));
+    }
+    // Exactly STEP_BUDGET_WARNING_LEAD-1 warning steps (44..48).
+    expect(remainings).toHaveLength(STEP_BUDGET_WARNING_LEAD - 1);
+    // Remaining = MAX-1-step, so it decreases by 1 each step and ends at 1.
+    for (let i = 1; i < remainings.length; i++) {
+      expect(remainings[i]).toBe(remainings[i - 1] - 1);
+    }
+    expect(remainings[remainings.length - 1]).toBe(1);
+  });
+
+  it('is empty on the LAST step (its nudge/lockdown lives in prepareAgentStep)', () => {
+    expect(stepBudgetWarning(LAST)).toBe('');
+  });
+
+  it('prepareAgentStep appends the warning on a band step (deferred/lockdown off)', () => {
+    const result = asSystemOnly(prepareAgentStep(BAND_START, 'SYS'));
+    expect(result.system).toContain('Stop exploring and start acting now');
+    expect(result.system).not.toContain(FINAL_STEP_NUDGE);
   });
 });
 
@@ -1341,6 +1448,7 @@ describe('AiChatService.stream — resumable pipe options (#184 phase 1.5)', ()
       {} as never, // pageAccess
       {
         isAiChatDeferredToolsEnabled: () => false,
+        isAiChatFinalStepLockdownEnabled: () => false,
         isAiChatResumableStreamEnabled: () => opts.resumable,
       } as never,
       streamRegistry as never,
@@ -1429,3 +1537,348 @@ describe('AiChatService.stream — resumable pipe options (#184 phase 1.5)', ()
     expect(streamRegistry.abortEntry).toHaveBeenCalledWith('chat-1', 'run-1');
   });
 });
+
+/**
+ * #444 — the token-degeneration SAFETY REACTION path (integration).
+ *
+ * output-degeneration.spec.ts proves the detector DETECTS; this proves the wired
+ * REACTION: a degenerate stream must (1) trip the detector in onChunk, (2) abort
+ * the turn via the INTERNAL degeneration controller (distinct from a user Stop),
+ * (3) truncate the runaway tail before persist in onAbort, (4) persist status
+ * 'error' with the OUTPUT_DEGENERATION_ERROR message (not a bare 'aborted' and not
+ * a swept 'streaming'), and (5) still release the leased external MCP clients.
+ *
+ * Harness: streamText is the SAME jest.fn mocked at the top of this file. Unlike
+ * the pipe-options suite above (which only inspects the pipe call), this mock
+ * CAPTURES the streamText options (onChunk/onAbort/onFinish + abortSignal) so the
+ * test can drive the callbacks exactly as the AI SDK would — feeding degenerate
+ * text-delta chunks through onChunk until the service's own AbortController fires,
+ * then invoking onAbort (which the SDK does on an aborted signal). No new mocking
+ * style is invented; it reuses the makeRes / service-construction shape above.
+ */
+describe('AiChatService.stream — token-degeneration reaction (#444)', () => {
+  const streamTextMock = streamText as unknown as jest.Mock;
+
+  beforeEach(() => {
+    streamTextMock.mockReset();
+    jest
+      .spyOn(Logger.prototype, 'log')
+      .mockImplementation(() => undefined as never);
+    jest
+      .spyOn(Logger.prototype, 'error')
+      .mockImplementation(() => undefined as never);
+    jest
+      .spyOn(Logger.prototype, 'warn')
+      .mockImplementation(() => undefined as never);
+  });
+
+  afterEach(() => jest.restoreAllMocks());
+
+  function makeRes() {
+    return {
+      raw: {
+        writeHead: jest.fn(),
+        write: jest.fn(),
+        once: jest.fn(),
+        on: jest.fn(),
+        flushHeaders: jest.fn(),
+        writableEnded: false,
+        destroyed: false,
+      },
+    };
+  }
+
+  // Wire the full stream() path with in-memory fakes. The assistant row is
+  // captured so the terminal finalize (an UPDATE of the upfront-seeded row) can be
+  // asserted. One external MCP client with a close() spy lets us assert leases are
+  // released on the terminal path. lockdown OFF (default) so the detector is the
+  // active guard.
+  function makeService() {
+    // The upfront insert seeds the assistant row; findById/insert stamp a stable
+    // id so planFinalizeAssistant picks the UPDATE path.
+    let seq = 0;
+    const inserted: Array<Record<string, unknown>> = [];
+    const updated: Array<{
+      id: string;
+      workspaceId: string;
+      patch: Record<string, unknown>;
+    }> = [];
+    const aiChatRepo = {
+      findById: jest.fn(async () => ({ id: 'chat-1', workspaceId: 'ws-1' })),
+      insert: jest.fn(),
+    };
+    const aiChatMessageRepo = {
+      insert: jest.fn(async (row: Record<string, unknown>) => {
+        inserted.push(row);
+        return { id: row.role === 'assistant' ? 'assistant-1' : `user-${++seq}` };
+      }),
+      findAllByChat: jest.fn(async () => []),
+      update: jest.fn(
+        async (
+          id: string,
+          workspaceId: string,
+          patch: Record<string, unknown>,
+        ) => {
+          updated.push({ id, workspaceId, patch });
+          return { id };
+        },
+      ),
+    };
+    const aiSettings = { resolve: jest.fn(async () => ({})) };
+    const tools = { forUser: jest.fn(async () => ({})) };
+    const mcpClose = jest.fn(async () => undefined);
+    const mcpClients = {
+      toolsFor: jest.fn(async () => ({
+        tools: {},
+        clients: [{ close: mcpClose }],
+        outcomes: [],
+        instructions: [],
+      })),
+    };
+    const streamRegistry = { open: jest.fn(), bind: jest.fn(), abortEntry: jest.fn() };
+    const svc = new AiChatService(
+      {} as never,
+      aiChatRepo as never,
+      aiChatMessageRepo as never,
+      {} as never, // aiChatPageSnapshotRepo (no open page -> never touched)
+      aiSettings as never,
+      tools as never,
+      mcpClients as never,
+      {} as never, // aiAgentRoleRepo
+      {} as never, // pageRepo (no open page)
+      {} as never, // pageAccess
+      {
+        isAiChatDeferredToolsEnabled: () => false,
+        // lockdown OFF => the degeneration detector is the anti-babble guard.
+        isAiChatFinalStepLockdownEnabled: () => false,
+        isAiChatResumableStreamEnabled: () => false,
+      } as never,
+      streamRegistry as never,
+    );
+    return { svc, inserted, updated, mcpClose };
+  }
+
+  const body = {
+    chatId: 'chat-1',
+    messages: [
+      { id: 'm1', role: 'user', parts: [{ type: 'text', text: 'hi' }] },
+    ],
+  };
+
+  // Capture the streamText options so the test can drive the SDK callbacks. The
+  // returned result stub is enough for the post-streamText wiring (consumeStream +
+  // pipeUIMessageStreamToResponse are no-ops here).
+  function captureStreamText(): { opts: () => Record<string, any> } {
+    let captured: Record<string, any> | undefined;
+    streamTextMock.mockImplementation((options: Record<string, any>) => {
+      captured = options;
+      return {
+        consumeStream: jest.fn(),
+        pipeUIMessageStreamToResponse: jest.fn(),
+      };
+    });
+    return {
+      opts: () => {
+        if (!captured) throw new Error('streamText was not called');
+        return captured;
+      },
+    };
+  }
+
+  async function drive(svc: AiChatService): Promise<void> {
+    await svc.stream({
+      user: { id: 'u1' } as never,
+      workspace: { id: 'ws-1' } as never,
+      sessionId: 's1',
+      body: body as never,
+      res: makeRes() as never,
+      signal: new AbortController().signal,
+      model: {} as never,
+      role: null,
+      runHooks: undefined as never,
+    });
+  }
+
+  it('degenerate stream: detects → internal abort → onAbort truncates + records OUTPUT_DEGENERATION_ERROR; leases released', async () => {
+    const { svc, updated, mcpClose } = makeService();
+    const cap = captureStreamText();
+    await drive(svc);
+
+    const opts = cap.opts();
+    // The turn's abort signal is the UNION of the socket/run signal and the
+    // internal degeneration controller — untripped before any output.
+    expect(opts.abortSignal.aborted).toBe(false);
+
+    // Feed a runaway "loadTools.\n" loop the way the SDK streams it: many small
+    // text-delta chunks. The onChunk throttle only re-checks every ~2000 chars, so
+    // deliver well past that so the detector's identical-line rule (>=25 lines)
+    // and the ~2000-char throttle both fire.
+    const line = 'loadTools.\n';
+    let delivered = 0;
+    for (let i = 0; i < 400 && !opts.abortSignal.aborted; i++) {
+      opts.onChunk({ chunk: { type: 'text-delta', text: line } });
+      delivered += line.length;
+    }
+
+    // The detector must have tripped and aborted via the INTERNAL controller — the
+    // reason carries the degeneration message, distinguishing it from a user Stop
+    // (which aborts with no such reason) or a socket disconnect.
+    expect(opts.abortSignal.aborted).toBe(true);
+    expect(delivered).toBeGreaterThan(2000);
+    expect(String(opts.abortSignal.reason)).toContain(
+      'Output degeneration detected',
+    );
+
+    // The SDK reacts to the aborted signal by invoking onAbort. `steps` is empty
+    // (the runaway never finished a step); the in-progress runaway text is what
+    // gets truncated + persisted.
+    await opts.onAbort({ steps: [] });
+
+    // Terminal finalize = an UPDATE of the upfront-seeded assistant row (assistant
+    // row was inserted upfront, so planFinalizeAssistant -> UPDATE).
+    expect(updated).toHaveLength(1);
+    const patch = updated[0].patch as {
+      status: string;
+      content: string;
+      metadata: Record<string, unknown>;
+    };
+    // (4) status 'error' with the degeneration message — NOT 'aborted' and NOT a
+    // swept 'streaming'. This distinguishes it from a user Stop / server restart.
+    expect(patch.status).toBe('error');
+    expect(patch.metadata.error).toBe(OUTPUT_DEGENERATION_ERROR);
+    expect(patch.metadata.finishReason).toBe('error');
+    // (3) the runaway tail is TRUNCATED, not the full multi-KB babble: the marker
+    // is present and the persisted content is far shorter than what was streamed.
+    expect(patch.content).toContain('output truncated');
+    expect(patch.content.length).toBeLessThan(delivered);
+    // Only a few loop reps survive (truncateDegeneratedTail keeps a handful).
+    expect((patch.content.match(/loadTools\./g) ?? []).length).toBeLessThan(10);
+
+    // (5) the leased external MCP client is still released on this terminal path.
+    expect(mcpClose).toHaveBeenCalledTimes(1);
+  });
+
+  it('degeneration onAbort differs from a NORMAL/user abort (no truncation, no error)', async () => {
+    // Same harness, but the stream is NOT degenerate: a clean short answer, then a
+    // user Stop reaches onAbort WITHOUT the degeneration controller having fired.
+    const { svc, updated, mcpClose } = makeService();
+    const cap = captureStreamText();
+    await drive(svc);
+    const opts = cap.opts();
+
+    opts.onChunk({ chunk: { type: 'text-delta', text: 'A normal partial answer.' } });
+    // The detector never tripped -> the union signal is NOT aborted by us.
+    expect(opts.abortSignal.aborted).toBe(false);
+
+    // A user Stop / disconnect drives onAbort with the partial (clean) text.
+    await opts.onAbort({ steps: [] });
+
+    expect(updated).toHaveLength(1);
+    const patch = updated[0].patch as {
+      status: string;
+      content: string;
+      metadata: Record<string, unknown>;
+    };
+    // A normal abort persists status 'aborted' with NO error and NO truncation
+    // marker — the branch is genuinely distinguished from the degeneration path.
+    expect(patch.status).toBe('aborted');
+    expect('error' in patch.metadata).toBe(false);
+    expect(patch.content).toBe('A normal partial answer.');
+    expect(patch.content).not.toContain('output truncated');
+    // Cleanup still runs on the normal abort path too.
+    expect(mcpClose).toHaveBeenCalledTimes(1);
+  });
+
+  /**
+   * Empty-turn marker (#444): onFinish appends STEP_LIMIT_NO_ANSWER_MARKER only
+   * when the turn burned ALL its steps (steps.length >= MAX_AGENT_STEPS) AND never
+   * produced any text. The negative: a normal turn ending WITH text is left alone.
+   */
+  it('empty turn (no text + steps exhausted) persists the STEP_LIMIT_NO_ANSWER_MARKER', async () => {
+    const { svc, updated } = makeService();
+    const cap = captureStreamText();
+    await drive(svc);
+    const opts = cap.opts();
+
+    // MAX_AGENT_STEPS text-less steps (only tool calls) => step-exhausted, no text.
+    const steps = Array.from({ length: MAX_AGENT_STEPS }, () => ({
+      text: '',
+      toolCalls: [{ toolCallId: 'c1', toolName: 'searchPages', input: {} }],
+      toolResults: [
+        { toolCallId: 'c1', toolName: 'searchPages', output: { hits: [] } },
+      ],
+    }));
+    await opts.onFinish({
+      text: '',
+      finishReason: 'tool-calls',
+      totalUsage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 },
+      usage: { inputTokens: 1, outputTokens: 1 },
+      steps,
+    });
+
+    expect(updated).toHaveLength(1);
+    const patch = updated[0].patch as { status: string; content: string };
+    expect(patch.status).toBe('completed');
+    // The synthetic marker is the trailing text of the persisted content.
+    expect(patch.content).toContain(STEP_LIMIT_NO_ANSWER_MARKER);
+  });
+
+  it('normal turn ending WITH text does NOT get the empty-turn marker', async () => {
+    const { svc, updated } = makeService();
+    const cap = captureStreamText();
+    await drive(svc);
+    const opts = cap.opts();
+
+    // A single step that produced a real answer, well under the step cap.
+    const steps = [
+      { text: 'Here is the finished answer.', toolCalls: [], toolResults: [] },
+    ];
+    await opts.onFinish({
+      text: 'Here is the finished answer.',
+      finishReason: 'stop',
+      totalUsage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 },
+      usage: { inputTokens: 1, outputTokens: 1 },
+      steps,
+    });
+
+    expect(updated).toHaveLength(1);
+    const patch = updated[0].patch as { status: string; content: string };
+    expect(patch.status).toBe('completed');
+    expect(patch.content).toBe('Here is the finished answer.');
+    expect(patch.content).not.toContain(STEP_LIMIT_NO_ANSWER_MARKER);
+  });
+
+  it('step-exhausted turn that DID produce text keeps the text, no marker (guards the AND)', async () => {
+    // Exhausting the step budget alone must NOT append the marker when SOME step
+    // produced text — the marker keys off "no text" too. Drive the real onFinish
+    // with MAX_AGENT_STEPS steps where the last one carries the answer.
+    const { svc, updated } = makeService();
+    const cap = captureStreamText();
+    await drive(svc);
+    const opts = cap.opts();
+
+    const steps = Array.from({ length: MAX_AGENT_STEPS }, (_, i) => ({
+      text: i === MAX_AGENT_STEPS - 1 ? 'Final synthesized answer.' : '',
+      toolCalls:
+        i === MAX_AGENT_STEPS - 1
+          ? []
+          : [{ toolCallId: `c${i}`, toolName: 'searchPages', input: {} }],
+      toolResults:
+        i === MAX_AGENT_STEPS - 1
+          ? []
+          : [{ toolCallId: `c${i}`, toolName: 'searchPages', output: {} }],
+    }));
+    await opts.onFinish({
+      text: 'Final synthesized answer.',
+      finishReason: 'stop',
+      totalUsage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 },
+      usage: { inputTokens: 1, outputTokens: 1 },
+      steps,
+    });
+
+    expect(updated).toHaveLength(1);
+    const patch = updated[0].patch as { content: string };
+    expect(patch.content).toContain('Final synthesized answer.');
+    expect(patch.content).not.toContain(STEP_LIMIT_NO_ANSWER_MARKER);
+  });
+});

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

@@ -52,11 +52,24 @@ import {
   startSseHeartbeat,
   stripStreamingHopByHopHeaders,
 } from './sse-resilience';
+import {
+  isDegenerateOutput,
+  truncateDegeneratedTail,
+} from './output-degeneration';
 
 // Max agent steps per turn. One step = one model generation; a step that calls
 // tools is followed by another step carrying the tool results. Raised from 8 so
-// multi-search research questions are not cut off mid-investigation.
+// multi-search research questions are not cut off mid-investigation, then from 20
-const MAX_AGENT_STEPS = 20;
+// to 50 (#444) so read-heavy turns (e.g. dozens of searchInPage sweeps) do not
+// exhaust the budget before acting.
+const MAX_AGENT_STEPS = 50;
+
+// How many steps before the LAST one the step-budget warning starts firing
+// (#444). At MAX-STEP_BUDGET_WARNING_LEAD .. MAX-2 the model is told to stop
+// exploring and start acting, with the remaining count decreasing each step; the
+// last step (MAX-1) has its own final nudge / lockdown instead (see
+// prepareAgentStep).
+const STEP_BUDGET_WARNING_LEAD = 6;
 
 // Wall-clock ceiling for building the external MCP toolset during the per-turn
 // setup phase (before streamText owns the lifecycle). Defense-in-depth ABOVE the
@@ -82,16 +95,69 @@ const FINAL_STEP_INSTRUCTION =
   'language. If the information is incomplete, say so explicitly: summarize ' +
   'what you found, what is still missing, and give your best partial conclusion.';
 
-// Pure, unit-testable: decide per-step overrides. Two responsibilities:
+// SOFT final-step nudge (#444), used when the final-step lockdown toggle is OFF
-//   1. Final-step lockdown (always): on the final allowed step force a text-only
+// (the new default). Unlike FINAL_STEP_INSTRUCTION it does NOT strip tools
-//      synthesis answer (toolChoice 'none' + FINAL_STEP_INSTRUCTION). This WINS —
+// (toolChoice stays untouched), so the model is never forced into a tool-less
-//      it takes precedence over the deferred-tool narrowing below.
+// state mid-work — that tool-stripping is what triggered the 255KB token-loop
-//   2. Deferred tool visibility (#332): when `deferredEnabled` and NOT the final
+// degeneration incident. It only asks the model to finish with a text summary.
-//      step, expose only the CORE tools + loadTools + whatever loadTools has
+const FINAL_STEP_NUDGE =
-//      activated so far this turn (`activatedTools`), via `activeTools`. Deferred
+  'This is the LAST step of this turn. Write your final answer to the user now.\n' +
-//      tools stay in the <tool_catalog> until the model loads them.
+  'You may still call tools, but the turn ends after this step either way —\n' +
-// When `deferredEnabled` is false the behavior is unchanged: undefined on normal
+  'prefer finishing with a clear text summary of what was done and what remains.';
-// steps (all tools active), lockdown on the final step.
+
+// Synthetic marker text appended in onFinish when a step-exhausted turn produced
+// NO text at all (#444, mitigates the "empty turn" the lockdown used to prevent
+// when the toggle is OFF). Makes the exhausted-without-answer state explicit to
+// the user and, on replay, to the model on the next turn.
+const STEP_LIMIT_NO_ANSWER_MARKER =
+  '(Достигнут лимит шагов — итоговый ответ не сформулирован; работа могла ' +
+  'остаться незавершённой. Напишите «продолжай», чтобы агент продолжил.)';
+
+// Reason recorded in ai_chat_runs.error / the assistant row when the token-
+// degeneration detector (#444) aborts a run. Distinct from a user Stop (no error)
+// and from a server restart ('streaming' -> swept to 'aborted' with no message).
+const OUTPUT_DEGENERATION_ERROR =
+  'Output degeneration detected (repeated token loop)';
+
+/**
+ * Compute the step-budget warning text (#444), or '' when this step is outside
+ * the warning band. The warning fires on steps
+ * MAX_AGENT_STEPS-STEP_BUDGET_WARNING_LEAD .. MAX_AGENT_STEPS-2 (NOT the last
+ * step, which has its own final nudge/lockdown), telling the model to stop
+ * exploring and start acting. `N` is the number of tool-use steps still
+ * remaining (`MAX_AGENT_STEPS - 1 - stepNumber`), so it decreases toward the
+ * end. Pure.
+ */
+export function stepBudgetWarning(stepNumber: number): string {
+  const isLastStep = stepNumber >= MAX_AGENT_STEPS - 1;
+  const inBand = stepNumber >= MAX_AGENT_STEPS - STEP_BUDGET_WARNING_LEAD;
+  if (isLastStep || !inBand) return '';
+  const remaining = MAX_AGENT_STEPS - 1 - stepNumber;
+  return (
+    `Only ${remaining} tool-use steps remain in this turn. Stop exploring and start acting now\n` +
+    '(make the edits / create the comments / produce results). Leave room to finish\n' +
+    'with a final text answer.'
+  );
+}
+
+// Pure, unit-testable: decide per-step overrides. Responsibilities:
+//   1. Final-step handling. Two modes, chosen by `finalStepLockdownEnabled`:
+//      - toggle ON (legacy): on the final allowed step force a text-only
+//        synthesis answer (toolChoice 'none' + FINAL_STEP_INSTRUCTION). This WINS
+//        — it takes precedence over the deferred-tool narrowing below.
+//      - toggle OFF (new default, #444): do NOT touch toolChoice — tools stay
+//        available on every step incl. the last, so the model is never stripped
+//        of its tools mid-work (the cause of the token-loop degeneration
+//        incident). A SOFT nudge (FINAL_STEP_NUDGE) is appended to `system`, and
+//        the deferred-tool `activeTools` narrowing still applies to the last step
+//        (both `activeTools` and `system` are returned together).
+//   2. Step-budget warning (#444): on steps in the warning band (but not the
+//      last, which has its own nudge/lockdown) append stepBudgetWarning(...) to
+//      `system` so the model starts acting before it runs out of steps.
+//   3. Deferred tool visibility (#332): when `deferredEnabled`, expose only the
+//      CORE tools + loadTools + whatever loadTools has activated so far this turn
+//      (`activatedTools`), via `activeTools`. Deferred tools stay in the
+//      <tool_catalog> until the model loads them.
 //
 // `system` is the in-scope system prompt; we CONCATENATE so the original
 // persona/context is preserved — a bare `system` override would REPLACE the
@@ -107,31 +173,53 @@ export function prepareAgentStep(
   system: string,
   activatedTools: ReadonlySet<string> | readonly string[] = [],
   deferredEnabled = false,
+  finalStepLockdownEnabled = false,
 ):
   | { toolChoice: 'none'; system: string }
-  | { activeTools: string[] }
+  | { activeTools: string[]; system?: string }
+  | { system: string }
   | undefined {
-  // Final-step lockdown WINS (applies regardless of the deferred toggle).
+  const isLastStep = stepNumber >= MAX_AGENT_STEPS - 1;
-  if (stepNumber >= MAX_AGENT_STEPS - 1) {
+
+  // Legacy final-step lockdown (toggle ON): text-only synthesis. WINS over the
+  // deferred narrowing AND drops tools for this step.
+  if (isLastStep && finalStepLockdownEnabled) {
     return {
       toolChoice: 'none',
       system: `${system}\n\n${FINAL_STEP_INSTRUCTION}`,
     };
   }
-  // Deferred tool loading: narrow this step's visible tools to CORE + loadTools
+
-  // + the tools already activated this turn.
+  // Compute the extra system text for this step: the soft final nudge on the last
+  // step (toggle OFF), or the step-budget warning in the warning band. At most one
+  // of these applies (stepBudgetWarning returns '' on the last step).
+  const extra = isLastStep ? FINAL_STEP_NUDGE : stepBudgetWarning(stepNumber);
+  const systemForStep = extra ? `${system}\n\n${extra}` : undefined;
+
+  // Deferred tool loading: narrow this step's visible tools to CORE + loadTools +
+  // the tools already activated this turn. Applies on EVERY step incl. the last
+  // (toggle OFF), so the model keeps its core tools available while being nudged
+  // to finish. Return `system` alongside `activeTools` when we have extra text.
   if (deferredEnabled) {
     const activated = Array.isArray(activatedTools)
       ? activatedTools
       : [...activatedTools];
-    return {
+    const activeTools = [...CORE_TOOL_KEYS, LOAD_TOOLS_NAME, ...activated];
-      activeTools: [...CORE_TOOL_KEYS, LOAD_TOOLS_NAME, ...activated],
+    return systemForStep ? { activeTools, system: systemForStep } : { activeTools };
-    };
   }
-  return undefined;
+
+  // Deferred OFF: all tools stay active; only append the extra system text (if any).
+  return systemForStep ? { system: systemForStep } : undefined;
 }
 
-export { MAX_AGENT_STEPS, FINAL_STEP_INSTRUCTION };
+export {
+  MAX_AGENT_STEPS,
+  STEP_BUDGET_WARNING_LEAD,
+  FINAL_STEP_INSTRUCTION,
+  FINAL_STEP_NUDGE,
+  STEP_LIMIT_NO_ANSWER_MARKER,
+  OUTPUT_DEGENERATION_ERROR,
+};
 
 // Pure, unit-testable post-processing for a model-generated title (#199): trim
 // whitespace, strip a single pair of surrounding quotes the model often adds,
@@ -890,6 +978,12 @@ export class AiChatService implements OnModuleInit {
       // tools (fat/rare in-app tools + ALL external MCP tools) load on demand. When
       // OFF, every tool is active and nothing below changes.
       const deferredEnabled = this.environment.isAiChatDeferredToolsEnabled();
+      // Final-step lockdown toggle (#444). Default OFF: the last step keeps its
+      // tools and gets only a soft nudge (prepareAgentStep), and the token-
+      // degeneration detector (onChunk below) is the anti-babble guard. ON =
+      // legacy tool-stripping lockdown on the last step.
+      const finalStepLockdownEnabled =
+        this.environment.isAiChatFinalStepLockdownEnabled();
 
       let system: string;
       let docmostTools: Awaited<ReturnType<AiChatToolsService['forUser']>>;
@@ -978,6 +1072,16 @@ export class AiChatService implements OnModuleInit {
       const capturedSteps: StepLike[] = [];
       let inProgressText = '';
 
+      // Token-degeneration guard (#444). When the final-step lockdown is OFF, a
+      // runaway repetition loop (the 255KB "loadTools." incident) is aborted via
+      // this internal controller, unioned with the run/socket signal below. The
+      // detector runs on `inProgressText` in onChunk, throttled by growth so the
+      // pure rules only fire every ~DEGENERATION_CHECK_STEP bytes.
+      const degenerationController = new AbortController();
+      let degenerationDetected = false;
+      let lastDegenerationCheckLen = 0;
+      const DEGENERATION_CHECK_STEP = 2000;
+
       // Step-granular durability (#183): create the assistant row UPFRONT in the
       // 'streaming' state (before any token), then UPDATE it as each step finishes
       // and finalize it once on the terminal callback. If the process dies
@@ -1118,11 +1222,21 @@ export class AiChatService implements OnModuleInit {
           // further tool calls and appends a synthesis instruction on that step,
           // concatenated onto the original `system` so the persona is preserved.
           prepareStep: ({ stepNumber }) =>
-            prepareAgentStep(stepNumber, system, activatedTools, deferredEnabled),
+            prepareAgentStep(
+              stepNumber,
+              system,
+              activatedTools,
+              deferredEnabled,
+              finalStepLockdownEnabled,
+            ),
           // #184: the RUN's signal (explicit-stop) when a run wraps this turn, else
           // the socket-bound signal (legacy). A browser disconnect aborts only in
-          // the legacy path.
+          // the legacy path. #444: UNION it with the internal degeneration signal
-          abortSignal: effectiveSignal,
+          // so a detected token-loop aborts the run too (AbortSignal.any — Node 20.3+).
+          abortSignal: AbortSignal.any([
+            effectiveSignal,
+            degenerationController.signal,
+          ]),
           onChunk: ({ chunk }) => {
             // DIAGNOSTIC (Safari stream-drop investigation) — temporary. Any model
             // output chunk means the stream is actively emitting bytes; track first
@@ -1132,7 +1246,29 @@ export class AiChatService implements OnModuleInit {
             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;
+            if (chunk.type === 'text-delta') {
+              inProgressText += chunk.text;
+              // Token-degeneration guard (#444). Throttled: only re-run the pure
+              // rules once the text has grown ~DEGENERATION_CHECK_STEP bytes since
+              // the last check, so the tail heuristics cost is amortized. On a
+              // trigger, abort the run ONCE with a distinguishable reason.
+              if (
+                !degenerationDetected &&
+                inProgressText.length - lastDegenerationCheckLen >=
+                  DEGENERATION_CHECK_STEP
+              ) {
+                lastDegenerationCheckLen = inProgressText.length;
+                if (isDegenerateOutput(inProgressText)) {
+                  degenerationDetected = true;
+                  this.logger.warn(
+                    `AI chat stream aborted (chat ${chatId}): ${OUTPUT_DEGENERATION_ERROR}`,
+                  );
+                  degenerationController.abort(
+                    new Error(OUTPUT_DEGENERATION_ERROR),
+                  );
+                }
+              }
+            }
           },
           onStepFinish: (step) => {
             // The finished step's full text is now in `step.text`; fold it in and reset
@@ -1174,8 +1310,22 @@ export class AiChatService implements OnModuleInit {
             // plain-text projection (full-text search / fallback). A multi-step
             // turn's `content` therefore now holds all steps' prose, not just the
             // last block.
+            // Empty-turn mitigation (#444, toggle OFF). If the turn burned all its
+            // steps WITHOUT ever producing text (every step's text is empty) and
+            // the model stopped because it hit the step cap, there is no answer to
+            // show — the lockdown used to force one. Append a synthetic marker as
+            // the trailing text so the exhausted-without-answer state is explicit
+            // to the user and, on replay, to the model next turn. `flushAssistant`
+            // takes this as the `inProgressText` trailing text arg (empty here
+            // otherwise). `stepCountIs(MAX_AGENT_STEPS)` surfaces as
+            // finishReason === 'tool-calls' (or a length/other cap), so we key off
+            // "no text produced" rather than a single finishReason string.
+            const producedText = (steps as StepLike[]).some((s) => s.text?.trim());
+            const stepExhausted = steps.length >= MAX_AGENT_STEPS;
+            const emptyTurnMarker =
+              !producedText && stepExhausted ? STEP_LIMIT_NO_ANSWER_MARKER : '';
             await finalizeAssistant(
-              flushAssistant(steps as StepLike[], '', 'completed', {
+              flushAssistant(steps as StepLike[], emptyTurnMarker, 'completed', {
                 finishReason: finishReason as string,
                 usage: totalUsage as StreamUsage,
                 contextTokens:
@@ -1252,6 +1402,30 @@ export class AiChatService implements OnModuleInit {
             await snapshotTurnEnd();
           },
           onAbort: async ({ steps }) => {
+            // #444: distinguish a degeneration abort (our internal controller) from
+            // a user Stop / disconnect. On degeneration we truncate the runaway tail
+            // before persist (so hundreds of KB of garbage never reach the DB /
+            // replay) and record it as an ERROR with a clear, distinguishable reason
+            // — NOT a bare 'aborted' (a user Stop) and NOT a swept 'streaming' (a
+            // server restart).
+            if (degenerationDetected) {
+              const truncated = truncateDegeneratedTail(inProgressText);
+              await finalizeAssistant(
+                flushAssistant(capturedSteps, truncated, 'error', {
+                  error: OUTPUT_DEGENERATION_ERROR,
+                  pageChanged,
+                }),
+              );
+              if (runId)
+                await runHooks?.onSettled?.(
+                  runId,
+                  'error',
+                  OUTPUT_DEGENERATION_ERROR,
+                );
+              await closeExternalClients();
+              await snapshotTurnEnd();
+              return;
+            }
             const partialChars =
               capturedSteps.reduce((n, s) => n + (s.text?.length ?? 0), 0) +
               inProgressText.length;

apps/server/src/core/ai-chat/output-degeneration.spec.ts

@@ -0,0 +1,182 @@
+import {
+  hasRepeatedLineRun,
+  hasPeriodicTail,
+  isDegenerateOutput,
+  truncateDegeneratedTail,
+  REPEATED_LINES_THRESHOLD,
+  MIN_PERIOD_REPEATS,
+} from './output-degeneration';
+
+/**
+ * Unit tests for the token-degeneration detector (#444) — the sole anti-babble
+ * guard once the final-step lockdown is OFF. The two rules must fire on real
+ * degeneration (the "loadTools." incident, a no-newline repeat) and MUST NOT fire
+ * on legitimate long output (edit lists, tables, code).
+ */
+describe('hasRepeatedLineRun (rule 1: identical-line run)', () => {
+  it('POSITIVE: fires on "loadTools.\\n" repeated many times (the incident)', () => {
+    const text = 'Here is my plan.\n' + 'loadTools.\n'.repeat(300);
+    expect(hasRepeatedLineRun(text)).toBe(true);
+    expect(isDegenerateOutput(text)).toBe(true);
+  });
+
+  it('POSITIVE: fires at exactly the threshold', () => {
+    const text = 'x\n'.repeat(REPEATED_LINES_THRESHOLD);
+    expect(hasRepeatedLineRun(text)).toBe(true);
+  });
+
+  it('NEGATIVE: does NOT fire just below the threshold', () => {
+    // threshold-1 identical lines followed by a distinct line.
+    const text = 'x\n'.repeat(REPEATED_LINES_THRESHOLD - 1) + 'done\n';
+    expect(hasRepeatedLineRun(text)).toBe(false);
+  });
+
+  it('NEGATIVE: a long edit list of DISTINCT lines never trips', () => {
+    const lines: string[] = [];
+    for (let i = 0; i < 200; i++) lines.push(`- edited section ${i}: fixed typo`);
+    const text = lines.join('\n');
+    expect(hasRepeatedLineRun(text)).toBe(false);
+    expect(isDegenerateOutput(text)).toBe(false);
+  });
+
+  it('NEGATIVE: a markdown table with blank separators does not trip', () => {
+    // Repeated identical rows are unusual, but blank lines break any run.
+    const block = ['| a | b |', '| - | - |', '', '| a | b |', ''];
+    const text = Array.from({ length: 60 }, () => block.join('\n')).join('\n');
+    expect(hasRepeatedLineRun(text)).toBe(false);
+  });
+
+  it('NEGATIVE: blank lines do NOT count toward a run', () => {
+    const text = '\n'.repeat(100);
+    expect(hasRepeatedLineRun(text)).toBe(false);
+  });
+});
+
+describe('hasPeriodicTail (rule 2: no-newline suffix periodicity)', () => {
+  it('POSITIVE: fires on a single char repeated with no newlines', () => {
+    const text = 'answer: ' + 'a'.repeat(500);
+    expect(hasPeriodicTail(text)).toBe(true);
+    expect(isDegenerateOutput(text)).toBe(true);
+  });
+
+  it('POSITIVE: fires on a multi-char block repeat with no newlines', () => {
+    const text = 'prefix ' + 'abcdef'.repeat(100);
+    expect(hasPeriodicTail(text)).toBe(true);
+  });
+
+  it('POSITIVE: at least MIN_PERIOD_REPEATS repeats of a small block', () => {
+    const text = 'go'.repeat(MIN_PERIOD_REPEATS);
+    expect(hasPeriodicTail(text)).toBe(true);
+  });
+
+  it('NEGATIVE: prose does not look periodic', () => {
+    const text =
+      'The quick brown fox jumps over the lazy dog while the sun sets slowly ' +
+      'behind the distant mountains and the river winds through the valley below.';
+    expect(hasPeriodicTail(text)).toBe(false);
+    expect(isDegenerateOutput(text)).toBe(false);
+  });
+
+  it('NEGATIVE: a long code block is not flagged', () => {
+    const code = `
+function compute(values) {
+  let total = 0;
+  for (const v of values) {
+    total += v * 2;
+  }
+  return total / values.length;
+}
+export const helper = (x) => x + 1;
+const config = { retries: 3, timeout: 5000, backoff: 'exp' };
+`.repeat(3);
+    expect(isDegenerateOutput(code)).toBe(false);
+  });
+
+  it('NEGATIVE: a short string well under the repeat count is safe', () => {
+    expect(hasPeriodicTail('ababab')).toBe(false);
+  });
+
+  // Regression (#444): a trivial single-char period (p===1) must NOT flag
+  // legitimate divider/underline/whitespace runs. These are common in real
+  // model output and previously false-positived at ~20 identical chars, aborting
+  // the run and truncating output. They must all be treated as clean.
+  it('NEGATIVE: a markdown horizontal rule is not flagged', () => {
+    const text = 'text\n' + '-'.repeat(40);
+    expect(hasPeriodicTail(text)).toBe(false);
+    expect(isDegenerateOutput(text)).toBe(false);
+  });
+
+  it('NEGATIVE: a setext heading underline is not flagged', () => {
+    const text = 'Title\n' + '='.repeat(30);
+    expect(hasPeriodicTail(text)).toBe(false);
+    expect(isDegenerateOutput(text)).toBe(false);
+  });
+
+  it('NEGATIVE: a box-drawing divider with no trailing newline is not flagged', () => {
+    const text = 'done ' + '─'.repeat(50);
+    expect(hasPeriodicTail(text)).toBe(false);
+    expect(isDegenerateOutput(text)).toBe(false);
+  });
+
+  it('NEGATIVE: trailing spaces are not flagged', () => {
+    const text = 'answer' + ' '.repeat(40);
+    expect(hasPeriodicTail(text)).toBe(false);
+    expect(isDegenerateOutput(text)).toBe(false);
+  });
+
+  // TRIVIAL_MIN_REPEATS boundary (#444 review). The monochar-tail branch fires at
+  // EXACTLY 60 identical trailing chars (`run >= TRIVIAL_MIN_REPEATS`), so 59 is
+  // clean and 60 trips. These pin the `>=` and MUST fail if the comparison is
+  // flipped to `>` (the surviving mutation). The value 60 is HARD-CODED here on
+  // purpose: TRIVIAL_MIN_REPEATS is a private constant and the assert must lock
+  // the literal boundary the reviewer named, not track a constant edit.
+  it('NEGATIVE: 59 identical trailing chars is one below the monochar threshold', () => {
+    expect(hasPeriodicTail('x'.repeat(59))).toBe(false);
+    expect(isDegenerateOutput('x'.repeat(59))).toBe(false);
+  });
+
+  it('POSITIVE: 60 identical trailing chars hits the monochar threshold exactly', () => {
+    // Fails if `run >= TRIVIAL_MIN_REPEATS` is mutated to `run > …`.
+    expect(hasPeriodicTail('x'.repeat(60))).toBe(true);
+    expect(isDegenerateOutput('x'.repeat(60))).toBe(true);
+  });
+
+  // Positive counterparts: a GENUINE single-char runaway (hundreds+ of repeats)
+  // and the real incident (period>=2, "loadTools." ×N) must still fire.
+  it('POSITIVE: a genuine single-char runaway is still flagged', () => {
+    const text = 'x'.repeat(5000);
+    expect(hasPeriodicTail(text)).toBe(true);
+    expect(isDegenerateOutput(text)).toBe(true);
+  });
+
+  it('POSITIVE: the "loadTools." incident (period>=2) is still flagged', () => {
+    const text = 'loadTools.'.repeat(500);
+    expect(hasPeriodicTail(text)).toBe(true);
+    expect(isDegenerateOutput(text)).toBe(true);
+  });
+});
+
+describe('truncateDegeneratedTail', () => {
+  it('collapses a repeated-line loop to a few reps + marker', () => {
+    const text = 'plan\n' + 'loadTools.\n'.repeat(20000);
+    const out = truncateDegeneratedTail(text);
+    expect(out.length).toBeLessThan(text.length);
+    expect(out).toContain('output truncated');
+    // Keeps the leading context and a few loop reps.
+    expect(out).toContain('plan');
+    expect((out.match(/loadTools\./g) ?? []).length).toBeLessThan(10);
+  });
+
+  it('collapses a no-newline periodic loop to a few blocks + marker', () => {
+    const text = 'answer: ' + 'xy'.repeat(50000);
+    const out = truncateDegeneratedTail(text);
+    expect(out.length).toBeLessThan(text.length);
+    expect(out).toContain('output truncated');
+    expect(out).toContain('answer:');
+  });
+
+  it('returns non-degenerate text unchanged (by identity)', () => {
+    const text = 'A perfectly normal, finished assistant answer.';
+    expect(truncateDegeneratedTail(text)).toBe(text);
+  });
+});

apps/server/src/core/ai-chat/output-degeneration.ts

@@ -0,0 +1,189 @@
+/**
+ * Token-degeneration detector for the in-app agent stream (#444).
+ *
+ * When the final-step lockdown is OFF (the new default) there is no toolChoice
+ * override to strip the model's tools mid-work, so the anti-babble safety net is
+ * this detector. It watches the accumulating assistant text and, on a runaway
+ * repetition loop (the 255KB "loadTools." incident), aborts the run.
+ *
+ * Both rules are PURE functions of the text tail so they are cheap to run every
+ * few KB and are unit-testable in isolation. They operate on the TAIL only
+ * (`TAIL_WINDOW` chars) so the cost is bounded regardless of how long the turn is.
+ */
+
+/** How many trailing chars of the accumulated text the rules inspect. */
+export const TAIL_WINDOW = 3000;
+
+/** Rule 1 threshold: minimum consecutive identical non-empty lines to trigger. */
+export const REPEATED_LINES_THRESHOLD = 25;
+
+/** Rule 2: maximum length of a repeating block considered for periodicity. */
+export const MAX_PERIOD_LEN = 150;
+
+/** Rule 2: minimum number of consecutive block repeats to trigger. */
+export const MIN_PERIOD_REPEATS = 20;
+
+/**
+ * Rule 1 — ≥`REPEATED_LINES_THRESHOLD` consecutive IDENTICAL non-empty lines at
+ * the tail. Catches the classic newline-delimited loop ("loadTools.\n" ×N).
+ * Blank lines break a run (a table / list with blank separators never trips it);
+ * a run of ordinary distinct lines (an edit list, code) never reaches the count.
+ *
+ * NB: `REPEATED_LINES_THRESHOLD` (25) is only THIS rule's own trigger, not the
+ * effective floor for detecting a repeated-line loop. In practice a newline-
+ * delimited repeat also has a fixed period (line + '\n'), so rule 2 catches it
+ * via periodicity at `MIN_PERIOD_REPEATS` (20) repeats — the two rules combine
+ * (see `isDegenerateOutput`), so the effective lower bound for a short identical
+ * line loop is ~20, not 25. Pure.
+ */
+export function hasRepeatedLineRun(
+  text: string,
+  threshold = REPEATED_LINES_THRESHOLD,
+): boolean {
+  const tail = text.length > TAIL_WINDOW ? text.slice(-TAIL_WINDOW) : text;
+  const lines = tail.split('\n');
+  let run = 1;
+  let prev: string | null = null;
+  for (const line of lines) {
+    if (line.length > 0 && line === prev) {
+      run += 1;
+      if (run >= threshold) return true;
+    } else {
+      run = 1;
+    }
+    prev = line;
+  }
+  return false;
+}
+
+/**
+ * Rule 2 — cheap suffix-periodicity check: the tail ends in
+ * ≥`MIN_PERIOD_REPEATS` back-to-back repeats of a single block of length
+ * ≤`MAX_PERIOD_LEN`. Catches a no-newline repeat ("abcabcabc…") the line rule
+ * misses. For each candidate period length p we verify the last `repeats*p`
+ * chars are p-periodic; we stop at the smallest p that satisfies the repeat
+ * count. Bounded by MAX_PERIOD_LEN × TAIL_WINDOW comparisons — negligible. Pure.
+ */
+export function hasPeriodicTail(
+  text: string,
+  maxPeriod = MAX_PERIOD_LEN,
+  minRepeats = MIN_PERIOD_REPEATS,
+): boolean {
+  const tail = text.length > TAIL_WINDOW ? text.slice(-TAIL_WINDOW) : text;
+  const n = tail.length;
+  // Not even the shortest possible loop fits in the tail.
+  if (n < minRepeats) return false;
+  // A tail of ONE repeated char (a "trivial period") is common in LEGIT output —
+  // markdown rules (----/====), setext underlines, box-drawing dividers,
+  // trailing spaces routinely produce 20–50 identical chars. Such a run is
+  // p-periodic for EVERY p, so it would otherwise trip the block rule at p>=2
+  // too, not just p===1. We therefore split the check: a monochar tail needs far
+  // more repeats (a real single-char babble loop produces hundreds-to-thousands;
+  // 60 is well above any realistic divider yet a fifth of TAIL_WINDOW), while a
+  // genuine multi-char block repeat (>=2 distinct chars, e.g. the "loadTools."
+  // incident, period ~10) keeps the normal MIN_PERIOD_REPEATS threshold.
+  const TRIVIAL_MIN_REPEATS = 60;
+
+  // Monochar-tail check (the trivial-period case): count the trailing run of one
+  // identical char and require TRIVIAL_MIN_REPEATS of them.
+  {
+    const last = tail[n - 1];
+    let run = 1;
+    for (let i = n - 2; i >= 0 && tail[i] === last; i--) run++;
+    if (run >= TRIVIAL_MIN_REPEATS) return true;
+  }
+
+  const maxP = maxPeriod;
+  for (let p = 2; p <= maxP; p++) {
+    // Verify the last (minRepeats*p) chars are p-periodic AND not monochar (a
+    // monochar span is the trivial case handled above, so skip it here to avoid
+    // re-flagging a legit divider at a composite period).
+    const span = minRepeats * p;
+    // Not enough tail to hold this many repeats of this period.
+    if (span > n) continue;
+    const start = n - span;
+    let periodic = true;
+    let multiChar = false;
+    for (let i = n - 1; i >= start + p; i--) {
+      if (tail[i] !== tail[i - p]) {
+        periodic = false;
+        break;
+      }
+    }
+    if (!periodic) continue;
+    // Confirm the block itself has >=2 distinct chars (else it's monochar).
+    for (let i = start + 1; i < n; i++) {
+      if (tail[i] !== tail[start]) {
+        multiChar = true;
+        break;
+      }
+    }
+    if (multiChar) return true;
+  }
+  return false;
+}
+
+/**
+ * Combined guard used by the stream's onChunk: true when EITHER rule fires.
+ * Pure — the caller owns the abort side effect.
+ */
+export function isDegenerateOutput(text: string): boolean {
+  return hasRepeatedLineRun(text) || hasPeriodicTail(text);
+}
+
+/**
+ * Truncate a degenerated tail before persist so hundreds of KB of garbage never
+ * reach the DB / replay (#444). Keeps everything up to and including the FIRST
+ * `keepRepeats` repeats of the detected loop, then appends a short marker. If no
+ * loop is detected the text is returned unchanged (by identity).
+ *
+ * Implementation: find the shortest tail period (same check as hasPeriodicTail),
+ * keep the prefix before the loop plus `keepRepeats` copies of the block, drop
+ * the rest. This is best-effort cosmetic trimming; correctness does not depend on
+ * finding the exact minimal loop. Pure.
+ */
+export function truncateDegeneratedTail(
+  text: string,
+  keepRepeats = 3,
+): string {
+  const marker = '\n…[output truncated: repeated token loop detected]';
+  // Try the line rule first: collapse a long run of identical lines.
+  const lines = text.split('\n');
+  let runStart = -1;
+  let run = 1;
+  for (let i = 1; i < lines.length; i++) {
+    if (lines[i].length > 0 && lines[i] === lines[i - 1]) {
+      if (run === 1) runStart = i - 1;
+      run += 1;
+      if (run >= REPEATED_LINES_THRESHOLD) {
+        const kept = lines.slice(0, runStart + keepRepeats).join('\n');
+        return kept + marker;
+      }
+    } else {
+      run = 1;
+      runStart = -1;
+    }
+  }
+
+  // Fall back to periodicity over the whole string (bounded by the same block
+  // length). Find the smallest period that makes the SUFFIX highly repetitive.
+  const n = text.length;
+  const maxP = Math.min(MAX_PERIOD_LEN, Math.floor(n / MIN_PERIOD_REPEATS));
+  for (let p = 1; p <= maxP; p++) {
+    // Count how many trailing p-blocks are periodic.
+    let reps = 1;
+    let i = n - 1;
+    for (; i >= p; i--) {
+      if (text[i] !== text[i - p]) break;
+    }
+    // The loop above walks over the periodic suffix; its length is (n-1 - i).
+    const periodicLen = n - 1 - i;
+    reps = Math.floor(periodicLen / p) + 1;
+    if (reps >= MIN_PERIOD_REPEATS) {
+      const loopStart = n - reps * p; // start of the fully-periodic suffix
+      const kept = text.slice(0, loopStart + keepRepeats * p);
+      return kept + marker;
+    }
+  }
+  return text;
+}

apps/server/src/core/ai-chat/tools/tool-tiers.spec.ts

@@ -232,6 +232,16 @@ describe('applyLoadTools (#332)', () => {
     expect(LOAD_TOOLS_DESCRIPTION).toContain('only ACTIVATES them');
     expect(LOAD_TOOLS_DESCRIPTION).toContain('callable on your NEXT step');
   });
+
+  it('loadTools description tells the model CORE tools are always active (#444)', () => {
+    expect(LOAD_TOOLS_DESCRIPTION).toContain(
+      'Tools NOT listed in the catalog are CORE and ALWAYS active',
+    );
+    expect(LOAD_TOOLS_DESCRIPTION).toContain('NEVER via loadTools');
+    // Names it out explicitly so the model doesn't loadTools a core tool.
+    expect(LOAD_TOOLS_DESCRIPTION).toContain('createComment');
+    expect(LOAD_TOOLS_DESCRIPTION).toContain('searchInPage');
+  });
 });
 
 describe('editorial "Corrector" scenario is fully served by CORE (#332)', () => {

apps/server/src/core/ai-chat/tools/tool-tiers.ts

@@ -84,7 +84,10 @@ export const LOAD_TOOLS_DESCRIPTION =
   'block in your instructions. Pass the EXACT tool names from the catalog; this\n' +
   'call only ACTIVATES them and returns { loaded: [...] } — the tools become\n' +
   'callable on your NEXT step. Load several names in one call when the task clearly\n' +
-  'needs them. Unknown names are rejected with the list of valid ones.';
+  'needs them. Unknown names are rejected with the list of valid ones.\n' +
+  'Tools NOT listed in the catalog are CORE and ALWAYS active — call them directly,\n' +
+  'NEVER via loadTools (e.g. createComment, listComments, resolveComment,\n' +
+  'editPageText, searchInPage).';
 
 /**
  * Tier + catalogLine for the INLINE ai-chat tools — those defined per-layer in

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

@@ -158,4 +158,27 @@ describe('EnvironmentService', () => {
       ).toBe('https://app.example.com');
     });
   });
+
+  describe('isAiChatFinalStepLockdownEnabled (#444)', () => {
+    const build = (val?: string) =>
+      new EnvironmentService({
+        get: (key: string, def?: string) =>
+          key === 'AI_CHAT_FINAL_STEP_LOCKDOWN' ? (val ?? def) : def,
+      } as any);
+
+    it('defaults to OFF (false) when unset — the new anti-degeneration default', () => {
+      expect(build(undefined).isAiChatFinalStepLockdownEnabled()).toBe(false);
+    });
+
+    it('is true only for the exact opt-in "true" (case-insensitive)', () => {
+      expect(build('true').isAiChatFinalStepLockdownEnabled()).toBe(true);
+      expect(build('TRUE').isAiChatFinalStepLockdownEnabled()).toBe(true);
+    });
+
+    it('stays OFF for any other value', () => {
+      expect(build('false').isAiChatFinalStepLockdownEnabled()).toBe(false);
+      expect(build('1').isAiChatFinalStepLockdownEnabled()).toBe(false);
+      expect(build('yes').isAiChatFinalStepLockdownEnabled()).toBe(false);
+    });
+  });
 });

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

@@ -292,6 +292,24 @@ export class EnvironmentService {
     return enabled === 'true';
   }
 
+  /**
+   * Final-step lockdown for the in-app agent loop (#444). When ON (legacy), the
+   * LAST allowed step forces a text-only answer: tools are stripped
+   * (toolChoice:'none') and a synthesis instruction is appended. Defaults to OFF:
+   * stripping the tools mid-work triggered a token-loop degeneration incident
+   * (the model, robbed of its tools on the final step, emitted a 255KB block
+   * repeating a single token). With the toggle OFF the last step keeps its tools
+   * and gets only a SOFT nudge to finish with a text summary; the universal
+   * anti-babble guard is the token-degeneration detector instead. Enable this
+   * only for a model that does NOT reliably end its turns with a text answer.
+   */
+  isAiChatFinalStepLockdownEnabled(): boolean {
+    const enabled = this.configService
+      .get<string>('AI_CHAT_FINAL_STEP_LOCKDOWN', 'false')
+      .toLowerCase();
+    return enabled === 'true';
+  }
+
   /**
    * Resumable SSE transport for durable agent runs (#184 phase 1.5). When
    * enabled, a run tees its SSE frames into the in-memory run-stream registry so

packages/mcp/src/client.ts

@@ -553,16 +553,18 @@ export class DocmostClient {
     // forever and accumulate duplicates).
     const MAX_PAGES = 50;
 
-    let page = 1;
+    let cursor: string | undefined;
     let allItems: T[] = [];
-    let hasNextPage = true;
+    let truncated = false;
 
-    while (hasNextPage && page <= MAX_PAGES) {
+    for (let page = 0; page < MAX_PAGES; page++) {
-      const response = await this.client.post(endpoint, {
+      const payload: Record<string, any> = {
         ...basePayload,
         limit: clampedLimit,
-        page,
+      };
-      });
+      if (cursor) payload.cursor = cursor;
+
+      const response = await this.client.post(endpoint, payload);
 
       const data = response.data;
       const items = data.data?.items || data.items || [];
@@ -570,22 +572,28 @@ export class DocmostClient {
 
       allItems = allItems.concat(items);
 
-      // Stop if the page is empty or shorter than the requested size: a full
+      // Advance strictly via the server-issued cursor. A missing nextCursor (or
-      // page worth of items is the only situation where another page can exist,
+      // hasNextPage false) means we reached the end. A cursor identical to the
-      // so this defends against a stuck hasNextPage flag in addition to it.
+      // one we just sent means the server did not understand our pagination
-      if (items.length === 0 || items.length < clampedLimit) {
+      // param — stop instead of re-fetching page one forever and duplicating.
+      const next = meta?.hasNextPage ? meta?.nextCursor : null;
+      if (!next || next === cursor) {
+        // If the server still reports more pages but stopped issuing a usable
+        // cursor at the ceiling, flag the result as truncated below.
+        if (page === MAX_PAGES - 1 && meta?.hasNextPage) truncated = true;
         break;
       }
+      cursor = next;
 
-      hasNextPage = meta?.hasNextPage || false;
+      // Reaching the ceiling with more pages still available means the result
-      page++;
+      // set is truncated.
+      if (page === MAX_PAGES - 1) truncated = true;
     }
 
     // If the loop stopped because it hit the MAX_PAGES ceiling while the server
-    // still reported more results (hasNextPage true and the last page was
+    // still reported more results, the result set is truncated — warn so the
-    // full), the result set is truncated — warn so the caller is not silently
+    // caller is not silently handed an incomplete list.
-    // handed an incomplete list.
+    if (truncated) {
-    if (hasNextPage && page > MAX_PAGES) {
       console.warn(
         `paginateAll: results from "${endpoint}" truncated at the ${MAX_PAGES}-page cap; more pages exist on the server`,
       );
@@ -619,9 +627,10 @@ export class DocmostClient {
    * Tree (`tree` true): the space's FULL page hierarchy as a nested tree (each
    * node has a `children` array). This mode REQUIRES `spaceId` (a page tree is
    * scoped to one space) and IGNORES `limit` — the whole hierarchy is returned.
-   * It walks the sidebar tree via `enumerateSpacePages`, which performs N
+   * It fetches the tree via `enumerateSpacePages`, which on the fork server
-   * sidebar requests and is bounded by that method's 10000-node cap (and skips
+   * resolves to a single `/pages/tree` request returning the whole
-   * soft-deleted pages server-side).
+   * permission-filtered flat page set (soft-deleted pages excluded
+   * server-side).
    */
   async listPages(spaceId?: string, limit: number = 50, tree: boolean = false) {
     await this.ensureAuthenticated();
@@ -632,8 +641,8 @@ export class DocmostClient {
           "list_pages: tree mode requires a spaceId (a page tree is scoped to one space). Pass spaceId, or omit tree to get the recent-pages list.",
         );
       }
-      const nodes = await this.enumerateSpacePages(spaceId);
+      const { pages } = await this.enumerateSpacePages(spaceId);
-      return buildPageTree(nodes);
+      return buildPageTree(pages);
     }
 
     const clampedLimit = Math.max(1, Math.min(100, limit));
@@ -655,57 +664,123 @@ export class DocmostClient {
   async listSidebarPages(spaceId: string, pageId?: string) {
     await this.ensureAuthenticated();
 
-    // Paginate: the endpoint returns server-paged children, so posting only
+    // Paginate via the server-issued cursor. The server switched from OFFSET
-    // { page: 1 } silently dropped every child beyond the first page. Loop on
+    // (`page`) to CURSOR (`cursor`/`nextCursor`) pagination, and the global
-    // meta.hasNextPage (with a MAX_PAGES ceiling like paginateAll, guarding
+    // ValidationPipe(whitelist:true) SILENTLY STRIPS the obsolete `page` field
-    // against a stuck hasNextPage flag) and accumulate all children.
+    // — so the old offset loop got the SAME first page every time (with
+    // hasNextPage stuck true) and dropped every child beyond the first page.
     const MAX_PAGES = 50;
-    let page = 1;
+    let cursor: string | undefined;
     let allItems: any[] = [];
-    let hasNextPage = true;
+    let truncated = false;
 
-    while (hasNextPage && page <= MAX_PAGES) {
+    for (let i = 0; i < MAX_PAGES; i++) {
+      // limit: 100 is the server-side Max; cuts request count 5x vs the default 20.
+      const payload: Record<string, any> = { spaceId, limit: 100 };
       // Only send pageId when scoping to a page's children; omit it for roots.
-      const payload: Record<string, any> = { spaceId, page };
       if (pageId) payload.pageId = pageId;
+      if (cursor) payload.cursor = cursor;
 
-      const response = await this.client.post("/pages/sidebar-pages", payload);
+      const data = (await this.client.post("/pages/sidebar-pages", payload)).data
-      const data = response.data?.data ?? response.data;
+        ?.data;
-      const items = data?.items || [];
+      allItems = allItems.concat(data?.items ?? []);
-      allItems = allItems.concat(items);
 
-      hasNextPage = data?.meta?.hasNextPage || false;
+      // Advance strictly via the server-issued cursor; a missing/repeated cursor
-      page++;
+      // means the protocol drifted again — stop instead of looping on page one.
+      const next = data?.meta?.hasNextPage ? data?.meta?.nextCursor : null;
+      if (!next || next === cursor) break;
+      cursor = next;
+
+      // Reaching the ceiling with more pages still available means the child
+      // list is truncated (mirrors paginateAll).
+      if (i === MAX_PAGES - 1) truncated = true;
+    }
+
+    // Warn on real truncation (ceiling hit while the server still had pages) so
+    // the caller is not silently handed an incomplete child list.
+    if (truncated) {
+      console.warn(
+        `listSidebarPages: children of "${pageId ?? spaceId}" truncated at the ${MAX_PAGES}-page cap; more pages exist on the server`,
+      );
     }
 
     return allItems;
   }
 
   /**
-   * Enumerate EVERY page in a space (or in a subtree, when rootPageId is given)
+   * Enumerate EVERY page in a space (or in a subtree, when rootPageId is given).
-   * by walking the sidebar-pages tree.
    *
-   * Starting set: the children of rootPageId when provided, otherwise the
+   * Primary path (fork server): a SINGLE `POST /pages/tree` returns the whole
-   * space root pages. From there it does an iterative breadth-first walk: each
+   * space (or a subtree) as a flat, permission-filtered list in one request, in
-   * node is collected, and when node.hasChildren is true its direct children
+   * the exact node shape buildPageTree consumes. This replaces the old
-   * are fetched via listSidebarPages(spaceId, node.id) and enqueued.
+   * per-node BFS, which issued N sidebar requests and — after the server moved
+   * to cursor pagination — silently lost every child past the first sidebar
+   * page (the obsolete `page` param was stripped by ValidationPipe).
    *
-   * This replaces the old "/pages/recent" enumeration, which is a bounded
+   * The subtree variant (rootPageId given) INCLUDES the root node itself
-   * recent-activity feed (~5000 cap) and therefore misses comments on older
+   * (getPageAndDescendants seeds with id = rootPageId), unlike the old BFS
-   * pages that were never recently touched.
+   * which started from the root's children.
    *
-   * Safeguards: a `visited` Set of page ids prevents re-processing a node
+   * Fallback path (stdio mode may target STOCK upstream Docmost, which lacks
-   * (cycles / duplicate references), and a hard node cap bounds pathological
+   * `/pages/tree`): on a 404/405 it falls back to the cursor-based BFS below,
-   * trees so the walk always terminates.
+   * walking direct children via the fixed cursor listSidebarPages. Safeguards:
+   * a `visited` Set of page ids prevents re-processing a node (cycles /
+   * duplicate references), and a hard node cap bounds pathological trees so the
+   * walk always terminates.
+   *
+   * Returns `{ pages, truncated }`. `truncated` is true ONLY when the fallback
+   * BFS stopped at its MAX_NODES cap — the primary /pages/tree path is uncapped
+   * and always returns the complete set, so it never reports truncation.
    */
   private async enumerateSpacePages(
     spaceId: string,
     rootPageId?: string,
-  ): Promise<any[]> {
+  ): Promise<{ pages: any[]; truncated: boolean }> {
+    await this.ensureAuthenticated();
+
+    // Single request replaces the whole BFS: /pages/tree returns the full
+    // permission-filtered flat page set of a space (or a subtree) at once. This
+    // path is uncapped, so it is never truncated.
+    const payload = rootPageId ? { pageId: rootPageId } : { spaceId };
+    try {
+      const response = await this.client.post("/pages/tree", payload);
+      const pages = (response.data?.data ?? response.data)?.items ?? [];
+      return { pages, truncated: false };
+    } catch (e: any) {
+      // Only fall back when the endpoint is absent (stock upstream Docmost);
+      // any other error is a genuine failure and must propagate.
+      if (
+        !axios.isAxiosError(e) ||
+        (e.response?.status !== 404 && e.response?.status !== 405)
+      ) {
+        throw e;
+      }
+    }
+
+    // Fallback: cursor-based breadth-first walk via listSidebarPages.
     const MAX_NODES = 10000;
     const result: any[] = [];
     const visited = new Set<string>();
 
+    // Seed with the root node itself when scoping to a subtree, so its own
+    // comments aren't dropped: the primary /pages/tree seeds
+    // getPageAndDescendants with id = rootPageId (root included), but
+    // listSidebarPages(spaceId, rootPageId) returns only the root's CHILDREN.
+    // The `visited` set below prevents a double-add if the root also appears
+    // among the children. getPageRaw returns a page whose id/title/spaceId are
+    // exactly what buildPageTree and check_new_comments consume.
+    if (rootPageId) {
+      try {
+        const root = await this.getPageRaw(rootPageId);
+        if (root?.id) {
+          result.push(root);
+          visited.add(root.id);
+        }
+      } catch {
+        // Non-fatal: if the root can't be read, fall through to children-only.
+      }
+    }
+
     // Seed the queue with the starting level (subtree children or roots).
     const queue: any[] = await this.listSidebarPages(spaceId, rootPageId);
 
@@ -730,7 +805,12 @@ export class DocmostClient {
       }
     }
 
-    return result;
+    // Truncated only when the cap was hit with the queue still non-empty (real
+    // truncation, not a natural end at exactly MAX_NODES).
+    return {
+      pages: result,
+      truncated: result.length >= MAX_NODES && queue.length > 0,
+    };
   }
 
   /** Raw page info including the ProseMirror JSON content and slugId. */
@@ -2364,7 +2444,13 @@ export class DocmostClient {
     let allComments: any[] = [];
     let cursor: string | null = null;
 
-    do {
+    // Hard ceiling + immovable-cursor guard (mirrors paginateAll): if /comments
+    // ever stops advancing the cursor (the exact #442 drift scenario) this loop
+    // would otherwise spin forever accumulating duplicates.
+    const MAX_PAGES = 50;
+    let truncated = false;
+
+    for (let page = 0; page < MAX_PAGES; page++) {
       const payload: Record<string, any> = { pageId, limit: 100 };
       if (cursor) payload.cursor = cursor;
 
@@ -2372,8 +2458,23 @@ export class DocmostClient {
       const data = response.data.data || response.data;
       const items = data.items || [];
       allComments = allComments.concat(items);
-      cursor = data.meta?.nextCursor || null;
+
-    } while (cursor);
+      // Advance strictly via the server-issued cursor. A missing nextCursor or a
+      // cursor identical to the one we just sent means the end (or a server that
+      // ignores our pagination param) — stop instead of re-fetching page one.
+      const next: string | null = data.meta?.nextCursor || null;
+      if (!next || next === cursor) break;
+      cursor = next;
+
+      // Reaching the ceiling with a still-advancing cursor means truncation.
+      if (page === MAX_PAGES - 1) truncated = true;
+    }
+
+    if (truncated) {
+      console.warn(
+        `listComments: comments for "${pageId}" truncated at the ${MAX_PAGES}-page cap; more pages exist on the server`,
+      );
+    }
 
     const mapped = allComments.map((comment: any) => {
       const markdown = comment.content
@@ -2846,36 +2947,27 @@ export class DocmostClient {
       );
     }
 
-    // 1. Enumerate the FULL set of pages in scope by walking the sidebar-pages
+    // 1. Enumerate the FULL set of pages in scope via the page tree (a complete
-    // tree (a complete page index), NOT the bounded "/pages/recent" feed which
+    // page index), NOT the bounded "/pages/recent" feed which caps at ~5000
-    // caps at ~5000 recent items and silently misses comments on older pages.
+    // recent items and silently misses comments on older pages.
     //
     // Subtree scope: when parentPageId is given, the scope is that page ITSELF
-    // plus every descendant (enumerateSpacePages walks its children). Otherwise
+    // plus every descendant. Otherwise the scope is the whole space (all roots
-    // the scope is the whole space (all roots and their descendants).
+    // and their descendants).
     //
     // NOTE: do NOT pre-filter by page.updatedAt — creating a comment does not
     // bump it (verified on a live server), so such a filter silently misses
     // comments on pages that were not otherwise edited. The complete tree walk
     // already restricts the scope correctly, so no recent-feed allow-list is
     // needed any more.
-    let pagesInScope: any[];
+    //
-    if (parentPageId) {
+    // The subtree scope (parentPageId given) already INCLUDES the root node
-      const subtree = await this.enumerateSpacePages(spaceId, parentPageId);
+    // itself: /pages/tree seeds getPageAndDescendants with id = parentPageId, so
-      // Include the parent page node itself alongside its descendants. Fetch it
+    // no separate getPageRaw fetch for the parent is needed.
-      // so its title/id are available even though it is not returned by its own
+    const { pages: pagesInScope, truncated } = await this.enumerateSpacePages(
-      // children listing.
+      spaceId,
-      let parentNode: any = { id: parentPageId };
+      parentPageId,
-      try {
+    );
-        parentNode = await this.getPageRaw(parentPageId);
-      } catch (e: any) {
-        // Fall back to a minimal node if the parent can't be fetched; its
-        // comments are still attempted below (the fetch there is non-fatal).
-      }
-      pagesInScope = [parentNode, ...subtree];
-    } else {
-      pagesInScope = await this.enumerateSpacePages(spaceId);
-    }
 
     // 2. Fetch comments for each page, keep ones created after since
     const results: any[] = [];
@@ -2904,10 +2996,9 @@ export class DocmostClient {
       0,
     );
 
-    // enumerateSpacePages caps traversal at 10000 nodes; flag when that cap was
+    // `truncated` is reported by enumerateSpacePages: it is true ONLY when the
-    // hit so the caller knows the scan may be incomplete (some pages skipped).
+    // stdio fallback BFS hit its node cap. The primary /pages/tree path is
-    const truncated = pagesInScope.length >= 10000;
+    // uncapped, so a space with legitimately many pages is not falsely flagged.
-
     return {
       since,
       scope: parentPageId ? `subtree of ${parentPageId}` : `space ${spaceId}`,

packages/mcp/test/mock/pagination-cursor.test.mjs

@@ -0,0 +1,440 @@
+// Mock-HTTP tests for the cursor-pagination migration in DocmostClient (#442).
+//
+// The server switched its list endpoints from OFFSET (`page`) to CURSOR
+// (`cursor`/`nextCursor`) pagination, and the global ValidationPipe silently
+// strips the obsolete `page` field — so the old offset loops re-fetched page
+// one forever (hasNextPage stuck true), dropping every item past the first
+// page. These tests pin the new cursor behaviour and the immovable-cursor
+// guard that prevents a silent spin/duplication if the protocol drifts again.
+//
+// A local http.createServer stands in for Docmost so everything stays
+// deterministic and offline (same harness style as reauth.test.mjs).
+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", (chunk) => {
+      raw += chunk;
+    });
+    req.on("end", () => resolve(raw));
+  });
+}
+
+function startServer(handler) {
+  return new Promise((resolve) => {
+    const server = http.createServer(handler);
+    server.listen(0, "127.0.0.1", () => {
+      const { port } = server.address();
+      resolve({ server, baseURL: `http://127.0.0.1:${port}/api` });
+    });
+  });
+}
+
+function closeServer(server) {
+  return new Promise((resolve) => server.close(resolve));
+}
+
+function sendJson(res, status, obj, extraHeaders = {}) {
+  res.writeHead(status, { "Content-Type": "application/json", ...extraHeaders });
+  res.end(JSON.stringify(obj));
+}
+
+const openServers = [];
+async function spawn(handler) {
+  const { server, baseURL } = await startServer(handler);
+  openServers.push(server);
+  return { server, baseURL };
+}
+
+after(async () => {
+  await Promise.all(openServers.map((s) => closeServer(s)));
+});
+
+// A login handler shared by every server below.
+function handleLogin(req, res) {
+  if (req.url === "/api/auth/login") {
+    sendJson(res, 200, { success: true }, {
+      "Set-Cookie": "authToken=t; Path=/; HttpOnly",
+    });
+    return true;
+  }
+  return false;
+}
+
+// -----------------------------------------------------------------------------
+// 1) listSidebarPages: collects every cursor page; #requests == #pages.
+// -----------------------------------------------------------------------------
+test("listSidebarPages walks all cursor pages and collects every item", async () => {
+  // Three pages keyed by the cursor the client sends back.
+  const PAGES = {
+    "": { items: [{ id: "a" }, { id: "b" }], nextCursor: "c1" },
+    c1: { items: [{ id: "c" }, { id: "d" }], nextCursor: "c2" },
+    c2: { items: [{ id: "e" }], nextCursor: null },
+  };
+  let requests = 0;
+  const sentLimits = [];
+
+  const { baseURL } = await spawn(async (req, res) => {
+    const raw = await readBody(req);
+    if (handleLogin(req, res)) return;
+    if (req.url === "/api/pages/sidebar-pages") {
+      requests++;
+      const body = JSON.parse(raw || "{}");
+      sentLimits.push(body.limit);
+      const page = PAGES[body.cursor ?? ""] ?? { items: [], nextCursor: null };
+      sendJson(res, 200, {
+        success: true,
+        data: {
+          items: page.items,
+          meta: {
+            hasNextPage: page.nextCursor != null,
+            nextCursor: page.nextCursor,
+          },
+        },
+      });
+      return;
+    }
+    sendJson(res, 404, {});
+  });
+
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+  const all = await client.listSidebarPages("space-1");
+
+  assert.equal(requests, 3, "one request per cursor page");
+  assert.deepEqual(
+    all.map((p) => p.id),
+    ["a", "b", "c", "d", "e"],
+    "all items across all pages collected in order",
+  );
+  assert.ok(
+    sentLimits.every((l) => l === 100),
+    "requests limit:100 (server-side max)",
+  );
+});
+
+// -----------------------------------------------------------------------------
+// 2) REGRESSION on the bug class: server IGNORES the cursor param and always
+//    returns page one with hasNextPage:true -> the immovable-cursor guard must
+//    terminate the loop with no duplicates, NOT spin to MAX_PAGES.
+// -----------------------------------------------------------------------------
+test("listSidebarPages terminates (no dups) when the server ignores the cursor", async () => {
+  let requests = 0;
+
+  const { baseURL } = await spawn(async (req, res) => {
+    await readBody(req);
+    if (handleLogin(req, res)) return;
+    if (req.url === "/api/pages/sidebar-pages") {
+      requests++;
+      // Always the SAME first page with hasNextPage:true and the SAME cursor,
+      // exactly as a server that no longer understands our pagination param.
+      sendJson(res, 200, {
+        success: true,
+        data: {
+          items: [{ id: "x1" }, { id: "x2" }],
+          meta: { hasNextPage: true, nextCursor: "stuck" },
+        },
+      });
+      return;
+    }
+    sendJson(res, 404, {});
+  });
+
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+  const all = await client.listSidebarPages("space-1");
+
+  // Request 1 (no cursor) gets "stuck"; request 2 (cursor "stuck") gets "stuck"
+  // again -> guard trips. Far below the MAX_PAGES=50 ceiling; no runaway dups.
+  assert.equal(requests, 2, "stops as soon as the cursor stops moving");
+  assert.equal(all.length, 4, "no runaway accumulation / duplication");
+});
+
+// -----------------------------------------------------------------------------
+// 3a) enumerateSpacePages happy path: a SINGLE /pages/tree request.
+// -----------------------------------------------------------------------------
+test("enumerateSpacePages (via list_pages tree) uses one /pages/tree request", async () => {
+  let treeRequests = 0;
+  let sidebarRequests = 0;
+  let treeBody = null;
+
+  const NODES = [
+    { id: "r1", slugId: "r1s", title: "Root 1", parentPageId: null, hasChildren: true, spaceId: "space-1", position: "a", icon: null, canEdit: true },
+    { id: "c1", slugId: "c1s", title: "Child 1", parentPageId: "r1", hasChildren: false, spaceId: "space-1", position: "a", icon: null, canEdit: true },
+    { id: "r2", slugId: "r2s", title: "Root 2", parentPageId: null, hasChildren: false, spaceId: "space-1", position: "b", icon: null, canEdit: true },
+  ];
+
+  const { baseURL } = await spawn(async (req, res) => {
+    const raw = await readBody(req);
+    if (handleLogin(req, res)) return;
+    if (req.url === "/api/pages/tree") {
+      treeRequests++;
+      treeBody = JSON.parse(raw || "{}");
+      sendJson(res, 200, { success: true, data: { items: NODES } });
+      return;
+    }
+    if (req.url === "/api/pages/sidebar-pages") {
+      sidebarRequests++;
+      sendJson(res, 200, { success: true, data: { items: [], meta: {} } });
+      return;
+    }
+    sendJson(res, 404, {});
+  });
+
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+  // list_pages tree:true -> enumerateSpacePages(spaceId) -> buildPageTree.
+  const tree = await client.listPages("space-1", 50, true);
+
+  assert.equal(treeRequests, 1, "exactly one /pages/tree request for the space");
+  assert.equal(sidebarRequests, 0, "no per-node sidebar BFS requests");
+  assert.deepEqual(treeBody, { spaceId: "space-1" }, "space scope posts spaceId only");
+  // buildPageTree nests c1 under r1; two roots at the top level.
+  assert.equal(tree.length, 2, "two root nodes");
+  const r1 = tree.find((n) => n.id === "r1");
+  assert.equal(r1.children.length, 1, "child nested under its root");
+  assert.equal(r1.children[0].id, "c1");
+});
+
+// -----------------------------------------------------------------------------
+// 3b) enumerateSpacePages fallback: /pages/tree 404 -> cursor BFS via sidebar.
+// -----------------------------------------------------------------------------
+test("enumerateSpacePages falls back to the cursor BFS on /pages/tree 404", async () => {
+  let treeRequests = 0;
+  const sidebarCalls = [];
+
+  // Root level: one root with children. Child level (pageId=r1): one leaf.
+  const { baseURL } = await spawn(async (req, res) => {
+    const raw = await readBody(req);
+    if (handleLogin(req, res)) return;
+    if (req.url === "/api/pages/tree") {
+      treeRequests++;
+      // Stock upstream Docmost has no /pages/tree.
+      sendJson(res, 404, { message: "Not Found" });
+      return;
+    }
+    if (req.url === "/api/pages/sidebar-pages") {
+      const body = JSON.parse(raw || "{}");
+      sidebarCalls.push(body.pageId ?? "<root>");
+      if (!body.pageId) {
+        sendJson(res, 200, {
+          success: true,
+          data: {
+            items: [
+              { id: "r1", title: "Root", parentPageId: null, hasChildren: true },
+            ],
+            meta: { hasNextPage: false, nextCursor: null },
+          },
+        });
+      } else if (body.pageId === "r1") {
+        sendJson(res, 200, {
+          success: true,
+          data: {
+            items: [
+              { id: "c1", title: "Leaf", parentPageId: "r1", hasChildren: false },
+            ],
+            meta: { hasNextPage: false, nextCursor: null },
+          },
+        });
+      } else {
+        sendJson(res, 200, {
+          success: true,
+          data: { items: [], meta: { hasNextPage: false, nextCursor: null } },
+        });
+      }
+      return;
+    }
+    sendJson(res, 404, {});
+  });
+
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+  const tree = await client.listPages("space-1", 50, true);
+
+  assert.ok(treeRequests >= 1, "the tree endpoint was attempted first");
+  assert.deepEqual(
+    sidebarCalls,
+    ["<root>", "r1"],
+    "fell back to the sidebar BFS: roots then the root's children",
+  );
+  assert.equal(tree.length, 1, "one root in the built tree");
+  assert.equal(tree[0].children[0].id, "c1", "leaf nested via the BFS");
+});
+
+// -----------------------------------------------------------------------------
+// 3c) enumerateSpacePages fallback SUBTREE: /pages/tree 404 + a rootPageId ->
+//     the ROOT page itself must be seeded (via getPageRaw) so its own comments
+//     aren't dropped. listSidebarPages(spaceId, root) returns only the root's
+//     CHILDREN, so without the seed the root would be absent. (Finding 1.)
+// -----------------------------------------------------------------------------
+test("enumerateSpacePages fallback subtree seeds the ROOT page itself", async () => {
+  const sidebarCalls = [];
+  let infoRequests = 0;
+  const commentedPages = [];
+
+  const { baseURL } = await spawn(async (req, res) => {
+    const raw = await readBody(req);
+    if (handleLogin(req, res)) return;
+    if (req.url === "/api/pages/tree") {
+      // Stock upstream Docmost -> fall back to the BFS.
+      sendJson(res, 404, { message: "Not Found" });
+      return;
+    }
+    if (req.url === "/api/pages/info") {
+      // getPageRaw for the root seed. Shape mirrors a real page-info response.
+      infoRequests++;
+      sendJson(res, 200, {
+        success: true,
+        data: { id: "root", title: "Root", spaceId: "space-1", hasChildren: true },
+      });
+      return;
+    }
+    if (req.url === "/api/pages/sidebar-pages") {
+      const body = JSON.parse(raw || "{}");
+      sidebarCalls.push(body.pageId ?? "<root>");
+      // Children of the root: one leaf. (Root itself is NOT in this list.)
+      const items =
+        body.pageId === "root"
+          ? [{ id: "leaf", title: "Leaf", parentPageId: "root", hasChildren: false }]
+          : [];
+      sendJson(res, 200, {
+        success: true,
+        data: { items, meta: { hasNextPage: false, nextCursor: null } },
+      });
+      return;
+    }
+    if (req.url === "/api/comments") {
+      const body = JSON.parse(raw || "{}");
+      commentedPages.push(body.pageId);
+      const items =
+        body.pageId === "root"
+          ? [{ id: "cm1", createdAt: "2030-01-01T00:00:00.000Z", content: null }]
+          : [];
+      sendJson(res, 200, {
+        success: true,
+        data: { items, meta: { nextCursor: null } },
+      });
+      return;
+    }
+    sendJson(res, 404, {});
+  });
+
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+  // checkNewComments(space, since, parentPageId) exercises the subtree fallback.
+  const result = await client.checkNewComments(
+    "space-1",
+    "2020-01-01T00:00:00.000Z",
+    "root",
+  );
+
+  assert.equal(infoRequests, 1, "root was seeded via one getPageRaw");
+  assert.equal(sidebarCalls[0], "root", "BFS walked the root's children");
+  assert.ok(
+    commentedPages.includes("root"),
+    "the ROOT page is in scope (its comments were fetched) — not dropped",
+  );
+  assert.ok(commentedPages.includes("leaf"), "the descendant is in scope too");
+  assert.equal(result.checkedPages, 2, "root + one descendant scanned");
+  assert.equal(result.totalNewComments, 1, "the root's fresh comment found");
+});
+
+// -----------------------------------------------------------------------------
+// 5) listComments immovable-cursor guard: the server IGNORES the cursor and
+//    keeps returning the same nextCursor -> the loop must terminate (no
+//    infinite loop, no duplicates), not spin forever. (Finding 4.)
+// -----------------------------------------------------------------------------
+test("listComments terminates (no dups) when the server ignores the cursor", async () => {
+  let requests = 0;
+
+  const { baseURL } = await spawn(async (req, res) => {
+    await readBody(req);
+    if (handleLogin(req, res)) return;
+    if (req.url === "/api/comments") {
+      requests++;
+      // Always the SAME page with the SAME nextCursor, as a server that no
+      // longer advances the cursor would.
+      sendJson(res, 200, {
+        success: true,
+        data: {
+          items: [{ id: "cm1", createdAt: "2030-01-01T00:00:00.000Z", content: null }],
+          meta: { nextCursor: "stuck" },
+        },
+      });
+      return;
+    }
+    sendJson(res, 404, {});
+  });
+
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+  const { items } = await client.listComments("page-1", true);
+
+  // Request 1 (no cursor) gets "stuck"; request 2 (cursor "stuck") gets "stuck"
+  // again -> guard trips. Bounded far below MAX_PAGES=50, no runaway dups.
+  assert.equal(requests, 2, "stops as soon as the cursor stops moving");
+  assert.equal(items.length, 2, "no runaway accumulation / duplication");
+});
+
+// -----------------------------------------------------------------------------
+// 4) check_new_comments subtree: the root is included in scope WITHOUT a
+//    separate getPageRaw (/pages/info) request for the parent.
+// -----------------------------------------------------------------------------
+test("checkNewComments subtree includes the root without a separate getPageRaw", async () => {
+  let pageInfoRequests = 0;
+  let treeBody = null;
+  const commentedPages = [];
+
+  // /pages/tree (subtree) returns the parent itself plus a descendant, exactly
+  // as getPageAndDescendants seeds with id = parentPageId.
+  const NODES = [
+    { id: "parent", title: "Parent", parentPageId: null, hasChildren: true },
+    { id: "kid", title: "Kid", parentPageId: "parent", hasChildren: false },
+  ];
+
+  const { baseURL } = await spawn(async (req, res) => {
+    const raw = await readBody(req);
+    if (handleLogin(req, res)) return;
+    if (req.url === "/api/pages/tree") {
+      treeBody = JSON.parse(raw || "{}");
+      sendJson(res, 200, { success: true, data: { items: NODES } });
+      return;
+    }
+    if (req.url === "/api/pages/info") {
+      // If checkNewComments still fetched the parent separately this would fire.
+      pageInfoRequests++;
+      sendJson(res, 200, { success: true, data: { id: "parent" } });
+      return;
+    }
+    if (req.url === "/api/comments") {
+      const body = JSON.parse(raw || "{}");
+      commentedPages.push(body.pageId);
+      // One fresh comment on the parent, none elsewhere.
+      const items =
+        body.pageId === "parent"
+          ? [{ id: "cm1", createdAt: "2030-01-01T00:00:00.000Z", content: null }]
+          : [];
+      sendJson(res, 200, {
+        success: true,
+        data: { items, meta: { nextCursor: null } },
+      });
+      return;
+    }
+    sendJson(res, 404, {});
+  });
+
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+  const result = await client.checkNewComments(
+    "space-1",
+    "2020-01-01T00:00:00.000Z",
+    "parent",
+  );
+
+  assert.equal(pageInfoRequests, 0, "no separate getPageRaw for the root");
+  assert.deepEqual(treeBody, { pageId: "parent" }, "subtree scope posts pageId");
+  assert.ok(
+    commentedPages.includes("parent"),
+    "the root itself is in scope (comments fetched for it)",
+  );
+  assert.ok(commentedPages.includes("kid"), "descendants are in scope too");
+  assert.equal(result.checkedPages, 2, "root + one descendant scanned");
+  assert.equal(result.totalNewComments, 1, "the root's fresh comment found");
+});

packages/mcp/test/mock/reauth.test.mjs

@@ -297,12 +297,12 @@ test("a response with ONLY authTokenRefresh (no authToken) rejects login", async
 // -----------------------------------------------------------------------------
 // 5) paginateAll loop guards.
 // -----------------------------------------------------------------------------
-test("paginateAll stops at the MAX_PAGES cap when hasNextPage is always true", async () => {
+test("paginateAll stops at the MAX_PAGES cap when the server always issues a fresh cursor", async () => {
   let pageRequests = 0;
   const LIMIT = 100;
 
   const { baseURL } = await spawn(async (req, res) => {
-    await readBody(req);
+    const body = JSON.parse((await readBody(req)) || "{}");
     if (req.url === "/api/auth/login") {
       sendJson(res, 200, { success: true }, {
         "Set-Cookie": "authToken=t; Path=/; HttpOnly",
@@ -311,15 +311,18 @@ test("paginateAll stops at the MAX_PAGES cap when hasNextPage is always true", a
     }
     if (req.url === "/api/spaces") {
       pageRequests++;
-      // Always return a FULL page (== requested limit) AND hasNextPage:true.
+      // Always return a FULL page AND hasNextPage:true with a FRESH nextCursor
-      // Both the page-length check and the hasNextPage flag say "keep going",
+      // that differs from the one the client just sent, so the immovable-cursor
-      // so only the MAX_PAGES ceiling can stop the loop.
+      // guard never trips — only the MAX_PAGES ceiling can stop the loop.
       const items = Array.from({ length: LIMIT }, (_, i) => ({
         id: `s-${pageRequests}-${i}`,
       }));
       sendJson(res, 200, {
         success: true,
-        data: { items, meta: { hasNextPage: true } },
+        data: {
+          items,
+          meta: { hasNextPage: true, nextCursor: `cursor-${pageRequests}` },
+        },
       });
       return;
     }
@@ -338,7 +341,7 @@ test("paginateAll stops at the MAX_PAGES cap when hasNextPage is always true", a
   assert.equal(all.length, 50 * LIMIT, "accumulates one full page per request");
 });
 
-test("paginateAll stops early on a short page even if hasNextPage is true", async () => {
+test("paginateAll stops on the immovable-cursor guard when the server ignores the cursor param", async () => {
   let pageRequests = 0;
   const LIMIT = 100;
 
@@ -352,15 +355,17 @@ test("paginateAll stops early on a short page even if hasNextPage is true", asyn
     }
     if (req.url === "/api/spaces") {
       pageRequests++;
-      // First page is full; second page is SHORT (fewer than limit). The short
+      // The bug class: the server IGNORES the pagination param and keeps
-      // page must stop the loop immediately even though hasNextPage stays true.
+      // returning page one with hasNextPage:true and the SAME nextCursor. The
-      const count = pageRequests === 1 ? LIMIT : 3;
+      // immovable-cursor guard must stop the loop instead of spinning to
-      const items = Array.from({ length: count }, (_, i) => ({
+      // MAX_PAGES and duplicating items.
-        id: `s-${pageRequests}-${i}`,
+      const items = Array.from({ length: LIMIT }, (_, i) => ({ id: `s-${i}` }));
-      }));
       sendJson(res, 200, {
         success: true,
-        data: { items, meta: { hasNextPage: true } },
+        data: {
+          items,
+          meta: { hasNextPage: true, nextCursor: "stuck" },
+        },
       });
       return;
     }
@@ -370,8 +375,10 @@ test("paginateAll stops early on a short page even if hasNextPage is true", asyn
   const client = new DocmostClient(baseURL, "user@example.com", "pw");
   const all = await client.paginateAll("/spaces", {}, LIMIT);
 
-  assert.equal(pageRequests, 2, "stops right after the first short page");
+  // Request 1 sends no cursor and receives "stuck"; request 2 sends "stuck" and
-  assert.equal(all.length, LIMIT + 3, "full page + short page accumulated");
+  // receives "stuck" again -> guard trips after exactly two requests, no dups.
+  assert.equal(pageRequests, 2, "stops once the cursor stops moving");
+  assert.equal(all.length, 2 * LIMIT, "no runaway accumulation past the guard");
 });
 
 test("paginateAll handles both {data:{items,meta}} and {items,meta} envelopes", async () => {
@@ -387,16 +394,16 @@ test("paginateAll handles both {data:{items,meta}} and {items,meta} envelopes",
     }
     if (req.url === "/api/groups") {
       bareRequests.push(1);
-      // Page 1: full page, hasNextPage true. Page 2: short page -> stop.
+      // Page 1: hasNextPage true with a next cursor. Page 2: no next -> stop.
       if (bareRequests.length === 1) {
         sendJson(res, 200, {
           items: Array.from({ length: 100 }, (_, i) => ({ id: `g${i}` })),
-          meta: { hasNextPage: true },
+          meta: { hasNextPage: true, nextCursor: "c2" },
         });
       } else {
         sendJson(res, 200, {
           items: [{ id: "tail" }],
-          meta: { hasNextPage: false },
+          meta: { hasNextPage: false, nextCursor: null },
         });
       }
       return;