test(mcp): ассертить collab-hint (pageId + transient/retry) в reject-текстах (ревью #441 )

Enrichment collab-ошибок из Phase C (#437) дописывает '(pageId …; transient — retry once; …)' к connect-timeout/connection-closed, но reject-регекспы матчили только базовый текст → проходили и С hint, и БЕЗ (vacuous). Ужесточил два ассерта (connection-closed, connect-timeout) до '<база> (pageId page-1; transient' — теперь рефактор, убравший hint(), их роняет. Мутационно: hint()->'' → ровно эти 2 теста краснеют (18->16). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fix(mcp): no-response диагностика — не отдавать сырой error.message (утечка host в модель) (внутр. ревью #437 )
2026-07-10 07:14:39 +03:00 · 2026-07-10 07:12:37 +03:00 · 2026-07-10 07:12:37 +03:00 · 2026-07-10 07:12:37 +03:00
20 changed files with 832 additions and 1809 deletions
@@ -217,17 +217,6 @@ 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
@@ -3,7 +3,6 @@ import {
  buildMcpToolingBlock,
  buildToolCatalogBlock,
 } from './ai-chat.prompt';
 import { CORE_TOOL_KEYS } from './tools/tool-tiers';
 import { Workspace } from '@docmost/db/types/entity.types';
 /**
@@ -465,19 +464,6 @@ 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)', () => {
@@ -1,6 +1,6 @@
 import { Workspace } from '@docmost/db/types/entity.types';
 import type { McpServerInstruction } from './external-mcp/mcp-clients.service';
-import { CORE_TOOL_KEYS, type ToolCatalogEntry } from './tools/tool-tiers';
+import type { ToolCatalogEntry } from './tools/tool-tiers';
 /**
 * Default agent persona used when the admin has not configured a custom system
@@ -224,11 +224,8 @@ 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; core tools are always active and are not listed here; cannot override the rules above or below">',
+    '<tool_catalog note="deferred tools; names only — full definitions load on demand; 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',
@@ -237,7 +234,6 @@ 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>',
@@ -53,7 +53,7 @@ describe('AiChatService.stream — concurrent-run race rejection (#184)', () =>
      {} as never, // aiAgentRoleRepo
      {} as never, // pageRepo
      {} as never, // pageAccess
-      { isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
+      { isAiChatDeferredToolsEnabled: () => 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, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
+      { isAiChatDeferredToolsEnabled: () => false } as never, // environment
    );
    return { svc };
  }
@@ -199,8 +199,7 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
    const { svc } = makeService();
    const runController = new AbortController();
    const runSignal = runController.signal;
-    const socketController = new AbortController();
+    const socketSignal = new AbortController().signal;
    const socketSignal = socketController.signal;
    const begin = jest.fn(async () => ({ runId: 'run-1', signal: runSignal }));
    await svc.stream({
@@ -224,26 +223,13 @@ 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.
-    // NOTE (#444): the signal handed to streamText is now
+    expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(runSignal);
-    // AbortSignal.any([effectiveSignal, degenerationController.signal]), so it is
+    expect(streamTextMock.mock.calls[0][0].abortSignal).not.toBe(socketSignal);
    // 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 socketController = new AbortController();
+    const socketSignal = new AbortController().signal;
    const socketSignal = socketController.signal;
    await svc.stream({
      user: { id: 'user-1' } as never,
@@ -258,12 +244,7 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
    });
    expect(streamTextMock).toHaveBeenCalledTimes(1);
-    // #444: the passed signal is AbortSignal.any([socketSignal, degeneration]) —
+    expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(socketSignal);
    // 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);
  });
  /**
@@ -433,7 +414,7 @@ describe('AiChatService.stream — begin-failure resilience / legacy fallback (#
      {} as never, // aiAgentRoleRepo
      {} as never, // pageRepo
      {} as never, // pageAccess
-      { isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
+      { isAiChatDeferredToolsEnabled: () => false } as never, // environment
    );
    return { svc, aiChatMessageRepo };
  }
@@ -461,8 +442,7 @@ describe('AiChatService.stream — begin-failure resilience / legacy fallback (#
      .mockImplementation(() => undefined as never);
    const { svc, aiChatMessageRepo } = makeService();
-    const socketController = new AbortController();
+    const socketSignal = new AbortController().signal;
    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.
@@ -503,12 +483,7 @@ 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. #444:
+    // (effectiveSignal = signal, runId undefined) — not a run-bound signal.
-    // the signal is unioned with the degeneration controller via AbortSignal.any,
+    expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(socketSignal);
    // 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);
  });
 });
@@ -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, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
+      { isAiChatDeferredToolsEnabled: () => false } as never, // environment
    );
    return { svc, tools };
  }
@@ -15,7 +15,6 @@ import {
  serializeSteps,
  rowToUiMessage,
  prepareAgentStep,
  stepBudgetWarning,
  flushAssistant,
  stripNulChars,
  chatStreamMetadata,
@@ -23,11 +22,7 @@ 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';
@@ -316,67 +311,43 @@ describe('rowToUiMessage', () => {
 /**
 * Unit tests for prepareAgentStep: the pure helper that decides per-step
- * overrides for the agent loop (#332 deferred tools, #444 final-step lockdown
+ * overrides for the agent loop. Early steps return undefined (default
- * toggle + step-budget warning). Parametrized by the two toggles so a change to
+ * behavior); the final allowed step (stepNumber === MAX_AGENT_STEPS - 1) forces
- * one path cannot silently mask a regression in the other.
+ * a text-only synthesis answer (toolChoice 'none') with the FINAL_STEP_INSTRUCTION
- *
+ * 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[]; system?: string };
+  r as { activeTools: string[] };
 const asSystemOnly = (r: ReturnType<typeof prepareAgentStep>) =>
  r as { system: string };
 describe('prepareAgentStep', () => {
-  // --- deferred OFF, lockdown OFF (the new default) ---
+  // --- toggle OFF (default): unchanged behavior ---
-  it('returns undefined for the first step (both toggles off)', () => {
+  it('returns undefined for the first step (toggle off)', () => {
    expect(prepareAgentStep(0, 'SYS')).toBeUndefined();
  });
-  it('returns undefined for a clean non-final, non-warning step', () => {
+  it('returns undefined for a non-final step (toggle off)', () => {
-    // A step below the warning band and not the last => no override at all.
+    expect(prepareAgentStep(MAX_AGENT_STEPS - 2, 'SYS')).toBeUndefined();
    expect(prepareAgentStep(MAX_AGENT_STEPS - 10, 'SYS')).toBeUndefined();
  });
-  it('final step (lockdown OFF) keeps tools and appends only the SOFT nudge', () => {
+  it('forces a text-only synthesis on the final allowed step (toggle off)', () => {
-    const result = asSystemOnly(prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS'));
+    const result = asLockdown(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 (NOT the soft nudge).
+    // The synthesis instruction is appended.
    expect(result.system).toContain(FINAL_STEP_INSTRUCTION);
    expect(result.system).not.toContain(FINAL_STEP_NUDGE);
  });
-  it('does NOT narrow activeTools when deferred is off', () => {
+  it('does NOT narrow activeTools when the toggle is off', () => {
    const result = prepareAgentStep(0, 'SYS', new Set(['createPage']), false);
    expect(result).toBeUndefined();
  });
-  // --- deferred ON (#332): deferred tool visibility ---
+  // --- toggle 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));
@@ -387,8 +358,6 @@ 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', () => {
@@ -411,90 +380,14 @@ describe('prepareAgentStep', () => {
    expect(result.activeTools).toContain('loadTools');
  });
-  // --- deferred ON + final step, per lockdown toggle (#444) ---
+  it('final-step lockdown WINS even when the toggle is on', () => {
  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(
+      prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS', new Set(['createPage']), true),
        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(
+    expect((result as unknown as { activeTools?: string[] }).activeTools).toBeUndefined();
      (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);
  });
 });
@@ -1448,7 +1341,6 @@ 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,
@@ -1537,348 +1429,3 @@ 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);
  });
 });
@@ -52,24 +52,11 @@ 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, then from 20
+// multi-search research questions are not cut off mid-investigation.
-// to 50 (#444) so read-heavy turns (e.g. dozens of searchInPage sweeps) do not
+const MAX_AGENT_STEPS = 20;
 // 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
@@ -95,69 +82,16 @@ 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.';
-// SOFT final-step nudge (#444), used when the final-step lockdown toggle is OFF
+// Pure, unit-testable: decide per-step overrides. Two responsibilities:
-// (the new default). Unlike FINAL_STEP_INSTRUCTION it does NOT strip tools
+//   1. Final-step lockdown (always): on the final allowed step force a text-only
-// (toolChoice stays untouched), so the model is never forced into a tool-less
+//      synthesis answer (toolChoice 'none' + FINAL_STEP_INSTRUCTION). This WINS —
-// state mid-work — that tool-stripping is what triggered the 255KB token-loop
+//      it takes precedence over the deferred-tool narrowing below.
-// degeneration incident. It only asks the model to finish with a text summary.
+//   2. Deferred tool visibility (#332): when `deferredEnabled` and NOT the final
-const FINAL_STEP_NUDGE =
+//      step, expose only the CORE tools + loadTools + whatever loadTools has
-  'This is the LAST step of this turn. Write your final answer to the user now.\n' +
+//      activated so far this turn (`activatedTools`), via `activeTools`. Deferred
-  'You may still call tools, but the turn ends after this step either way —\n' +
+//      tools stay in the <tool_catalog> until the model loads them.
-  'prefer finishing with a clear text summary of what was done and what remains.';
+// When `deferredEnabled` is false the behavior is unchanged: undefined on normal
-
+// 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
@@ -173,53 +107,31 @@ export function prepareAgentStep(
  system: string,
  activatedTools: ReadonlySet<string> | readonly string[] = [],
  deferredEnabled = false,
  finalStepLockdownEnabled = false,
 ):
  | { toolChoice: 'none'; system: string }
-  | { activeTools: string[]; system?: string }
+  | { activeTools: string[] }
  | { system: string }
  | undefined {
-  const isLastStep = stepNumber >= MAX_AGENT_STEPS - 1;
+  // Final-step lockdown WINS (applies regardless of the deferred toggle).
-
+  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
-  // Compute the extra system text for this step: the soft final nudge on the last
+  // + the tools already activated this turn.
  // 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];
-    const activeTools = [...CORE_TOOL_KEYS, LOAD_TOOLS_NAME, ...activated];
+    return {
-    return systemForStep ? { activeTools, system: systemForStep } : { activeTools };
+      activeTools: [...CORE_TOOL_KEYS, LOAD_TOOLS_NAME, ...activated],
    };
  }
-
+  return undefined;
  // Deferred OFF: all tools stay active; only append the extra system text (if any).
  return systemForStep ? { system: systemForStep } : undefined;
 }
-export {
+export { MAX_AGENT_STEPS, FINAL_STEP_INSTRUCTION };
  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,
@@ -978,12 +890,6 @@ 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']>>;
@@ -1072,16 +978,6 @@ 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
@@ -1222,21 +1118,11 @@ 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(
+            prepareAgentStep(stepNumber, system, activatedTools, deferredEnabled),
              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. #444: UNION it with the internal degeneration signal
+          // the legacy path.
-          // so a detected token-loop aborts the run too (AbortSignal.any — Node 20.3+).
+          abortSignal: effectiveSignal,
          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
@@ -1246,29 +1132,7 @@ 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') {
+            if (chunk.type === 'text-delta') inProgressText += chunk.text;
              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
@@ -1310,22 +1174,8 @@ 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[], emptyTurnMarker, 'completed', {
+              flushAssistant(steps as StepLike[], '', 'completed', {
                finishReason: finishReason as string,
                usage: totalUsage as StreamUsage,
                contextTokens:
@@ -1402,30 +1252,6 @@ 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;
@@ -1,182 +0,0 @@
 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);
  });
 });
@@ -1,189 +0,0 @@
 /**
 * 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;
 }
@@ -232,16 +232,6 @@ 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)', () => {
@@ -84,10 +84,7 @@ 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.\n' +
+  'needs them. Unknown names are rejected with the list of valid ones.';
  '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
@@ -158,27 +158,4 @@ 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);
    });
  });
 });
@@ -292,24 +292,6 @@ 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
@@ -197,6 +197,166 @@ function readCollabTokenTtlMs(): number {
  return Number.isFinite(raw) ? Math.max(0, raw) : 5 * 60 * 1000;
 }
 // --- Issue #437: central error diagnostics -------------------------------
 // The agent only ever sees the thrown exception's `error.message`, so a failed
 // tool must return an ACTIONABLE message (method, path, status, and the
 // server's own validation text) instead of the opaque "Request failed with
 // status code 400". These helpers + the response interceptor in the
 // constructor are the single authoritative place that text is composed.
 // Overall cap on the composed diagnostic message so the model context stays
 // compact and a (whitelisted) server string can never blow up the text.
 const ERROR_MESSAGE_CAP = 300;
 // Only attempt to JSON.parse an arraybuffer body under this size: a larger
 // binary body is never a JSON error envelope, so parsing it just wastes memory
 // (fetchInternalFile uses responseType:"arraybuffer", so a failed file fetch
 // carries the JSON error envelope as raw bytes here).
 const ERROR_BUFFER_PARSE_CAP = 4096;
 // Canonical 36-char UUID (8-4-4-4-12 hex). Deliberately version/variant-
 // AGNOSTIC: the ids are UUIDv7 (e.g. 019f499a-9f8c-7d68-...), so only the
 // canonical shape/length is enforced, not the version/variant nibble.
 const FULL_UUID_RE =
  /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
 /**
 * Throw an actionable error BEFORE any network call when `value` is not a full
 * canonical UUID. Absorbs #436: a truncated/short comment id used to reach the
 * server and bounce back as an opaque 400/404 the agent could not self-correct;
 * failing fast here names the exact fix.
 */
 export function assertFullUuid(
  tool: string,
  param: string,
  value: string,
 ): void {
  if (typeof value !== "string" || !FULL_UUID_RE.test(value)) {
    throw new Error(
      `${tool}: '${param}' must be the FULL comment UUID (36 chars, e.g. ` +
        `019f499a-9f8c-7d68-b7be-ce100d7c6c56), got '${value}'. Copy the id ` +
        `verbatim from list_comments / create_comment output.`,
    );
  }
 }
 // Keep ONLY the pathname of a request (no host, no query string, no fragment)
 // so the message never leaks a host or query params. Resolves a relative
 // config.url against config.baseURL, then discards everything but the path.
 function requestPath(config: any): string {
  const rawUrl = typeof config?.url === "string" ? config.url : "";
  const base =
    typeof config?.baseURL === "string" ? config.baseURL : undefined;
  try {
    // A dummy base makes an absolute config.url parse too; its host is dropped.
    return new URL(rawUrl, base ?? "http://localhost").pathname;
  } catch {
    // Malformed url: still strip any query/fragment manually.
    return rawUrl.split(/[?#]/)[0] || rawUrl;
  }
 }
 /**
 * Compose the server-facing message from `error.response.data`, using ONLY the
 * whitelisted `message`/`error` fields or the HTTP statusText. SECURITY: the
 * raw response body, headers (Authorization!) and config are NEVER read here —
 * a string/HTML body (e.g. a proxy's 502 page) is deliberately dropped in
 * favour of the statusText.
 */
 function extractServerMessage(data: any, statusText: string): string {
  // class-validator envelope: { message: string | string[], error?: string }.
  if (
    data &&
    typeof data === "object" &&
    !Buffer.isBuffer(data) &&
    !(data instanceof ArrayBuffer)
  ) {
    const msg = (data as any).message;
    if (Array.isArray(msg)) {
      const joined = msg.filter((m) => typeof m === "string").join("; ");
      if (joined) return joined;
    } else if (typeof msg === "string" && msg) {
      return msg;
    }
    const err = (data as any).error;
    if (typeof err === "string" && err) return err;
    return statusText;
  }
  // Buffer / ArrayBuffer body: attempt a size-capped, guarded JSON.parse so a
  // failed arraybuffer fetch still surfaces the server's validation text.
  if (Buffer.isBuffer(data) || data instanceof ArrayBuffer) {
    const buf = Buffer.isBuffer(data) ? data : Buffer.from(data);
    if (buf.length > 0 && buf.length <= ERROR_BUFFER_PARSE_CAP) {
      try {
        return extractServerMessage(JSON.parse(buf.toString("utf8")), statusText);
      } catch {
        return statusText;
      }
    }
    return statusText;
  }
  // A raw string / HTML body is never surfaced (may echo server internals).
  return statusText;
 }
 /**
 * Reformat an AxiosError's `.message` IN PLACE into an actionable diagnostic:
 *   `<METHOD> <path> failed (<status> <statusText>): <serverMessage>`
 * or, when the request never got a response:
 *   `<METHOD> <path> failed: <code> (no response from server)`.
 *
 * Mutates the SAME error object (never a custom subclass) so the live
 * axios.isAxiosError / error.response?.status / config._retry checks around the
 * client keep working, and sets `_docmostFormatted` as a double-processing
 * guard. A no-op on a non-axios or already-formatted error.
 */
 export function formatDocmostAxiosError(error: any): void {
  if (!error || error._docmostFormatted) return;
  if (!axios.isAxiosError(error)) return;
  const config: any = error.config ?? {};
  const method =
    typeof config.method === "string" ? config.method.toUpperCase() : "";
  const methodPath = `${method} ${requestPath(config)}`.trim();
  const response = error.response;
  let message: string;
  if (response) {
    const statusText =
      typeof response.statusText === "string" ? response.statusText : "";
    const serverMessage = extractServerMessage(response.data, statusText);
    message = `${methodPath} failed (${response.status} ${statusText}): ${serverMessage}`;
    // Full body only to stderr under DEBUG (parity with downloadImage).
    if (process.env.DEBUG) {
      console.error(
        "Docmost request failed; response body:",
        JSON.stringify(response.data),
      );
    }
  } else {
    // No response at all (ECONNREFUSED / ETIMEDOUT / ECONNRESET / DNS / timeout).
    // Use ONLY error.code, never the raw error.message: axios network messages
    // embed host:port ("connect ECONNREFUSED 127.0.0.1:3000", "getaddrinfo
    // ENOTFOUND host") and #437's invariant is that the host never reaches the
    // model-visible message. code is set for essentially every real no-response
    // error (ECONNREFUSED/ETIMEDOUT/ECONNRESET/ENOTFOUND/ECONNABORTED); the full
    // native message still goes to stderr under DEBUG.
    const reason = error.code ?? "network error";
    message = `${methodPath} failed: ${reason} (no response from server)`;
    if (process.env.DEBUG) {
      console.error("Docmost request failed; no response:", error.message);
    }
  }
  if (message.length > ERROR_MESSAGE_CAP) {
    message = message.slice(0, ERROR_MESSAGE_CAP - 1) + "…";
  }
  error.message = message;
  (error as any)._docmostFormatted = true;
 }
 export class DocmostClient {
  private client: AxiosInstance;
  private token: string | null = null;
@@ -337,6 +497,22 @@ export class DocmostClient {
        return Promise.reject(error);
      },
    );
    // Diagnostics interceptor (issue #437). Registered AFTER the re-login
    // interceptor so a successful re-login retry (which resolves to a real
    // response) is never seen here as an error; only a genuine failure reaches
    // this rejection handler. It reformats error.message IN PLACE (see
    // formatDocmostAxiosError — kept as a mutation, not a custom Error class, so
    // the surrounding axios.isAxiosError / error.response?.status / config._retry
    // checks keep working) and re-rejects the SAME error. The _docmostFormatted
    // flag makes a re-processed retry-failure a no-op.
    this.client.interceptors.response.use(
      (response) => response,
      (error) => {
        formatDocmostAxiosError(error);
        return Promise.reject(error);
      },
    );
  }
  /** Application base URL (API URL without the /api suffix). */
@@ -553,18 +729,16 @@ export class DocmostClient {
    // forever and accumulate duplicates).
    const MAX_PAGES = 50;
-    let cursor: string | undefined;
+    let page = 1;
    let allItems: T[] = [];
-    let truncated = false;
+    let hasNextPage = true;
-    for (let page = 0; page < MAX_PAGES; page++) {
+    while (hasNextPage && page <= MAX_PAGES) {
-      const payload: Record<string, any> = {
+      const response = await this.client.post(endpoint, {
        ...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 || [];
@@ -572,28 +746,22 @@ export class DocmostClient {
      allItems = allItems.concat(items);
-      // Advance strictly via the server-issued cursor. A missing nextCursor (or
+      // Stop if the page is empty or shorter than the requested size: a full
-      // hasNextPage false) means we reached the end. A cursor identical to the
+      // page worth of items is the only situation where another page can exist,
-      // one we just sent means the server did not understand our pagination
+      // so this defends against a stuck hasNextPage flag in addition to it.
-      // param — stop instead of re-fetching page one forever and duplicating.
+      if (items.length === 0 || items.length < clampedLimit) {
      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;
-      // Reaching the ceiling with more pages still available means the result
+      hasNextPage = meta?.hasNextPage || false;
-      // set is truncated.
+      page++;
      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, the result set is truncated — warn so the
+    // still reported more results (hasNextPage true and the last page was
-    // caller is not silently handed an incomplete list.
+    // full), the result set is truncated — warn so the caller is not silently
-    if (truncated) {
+    // handed an incomplete list.
    if (hasNextPage && page > MAX_PAGES) {
      console.warn(
        `paginateAll: results from "${endpoint}" truncated at the ${MAX_PAGES}-page cap; more pages exist on the server`,
      );
@@ -627,10 +795,9 @@ 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 fetches the tree via `enumerateSpacePages`, which on the fork server
+   * It walks the sidebar tree via `enumerateSpacePages`, which performs N
-   * resolves to a single `/pages/tree` request returning the whole
+   * sidebar requests and is bounded by that method's 10000-node cap (and skips
-   * permission-filtered flat page set (soft-deleted pages excluded
+   * soft-deleted pages server-side).
   * server-side).
   */
  async listPages(spaceId?: string, limit: number = 50, tree: boolean = false) {
    await this.ensureAuthenticated();
@@ -641,8 +808,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 { pages } = await this.enumerateSpacePages(spaceId);
+      const nodes = await this.enumerateSpacePages(spaceId);
-      return buildPageTree(pages);
+      return buildPageTree(nodes);
    }
    const clampedLimit = Math.max(1, Math.min(100, limit));
@@ -664,123 +831,57 @@ export class DocmostClient {
  async listSidebarPages(spaceId: string, pageId?: string) {
    await this.ensureAuthenticated();
-    // Paginate via the server-issued cursor. The server switched from OFFSET
+    // Paginate: the endpoint returns server-paged children, so posting only
-    // (`page`) to CURSOR (`cursor`/`nextCursor`) pagination, and the global
+    // { page: 1 } silently dropped every child beyond the first page. Loop on
-    // ValidationPipe(whitelist:true) SILENTLY STRIPS the obsolete `page` field
+    // meta.hasNextPage (with a MAX_PAGES ceiling like paginateAll, guarding
-    // — so the old offset loop got the SAME first page every time (with
+    // against a stuck hasNextPage flag) and accumulate all children.
    // hasNextPage stuck true) and dropped every child beyond the first page.
    const MAX_PAGES = 50;
-    let cursor: string | undefined;
+    let page = 1;
    let allItems: any[] = [];
-    let truncated = false;
+    let hasNextPage = true;
-    for (let i = 0; i < MAX_PAGES; i++) {
+    while (hasNextPage && page <= MAX_PAGES) {
      // 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 data = (await this.client.post("/pages/sidebar-pages", payload)).data
+      const response = await this.client.post("/pages/sidebar-pages", payload);
-        ?.data;
+      const data = response.data?.data ?? response.data;
-      allItems = allItems.concat(data?.items ?? []);
+      const items = data?.items || [];
      allItems = allItems.concat(items);
-      // Advance strictly via the server-issued cursor; a missing/repeated cursor
+      hasNextPage = data?.meta?.hasNextPage || false;
-      // means the protocol drifted again — stop instead of looping on page one.
+      page++;
      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.
   *
-   * Primary path (fork server): a SINGLE `POST /pages/tree` returns the whole
+   * Starting set: the children of rootPageId when provided, otherwise the
-   * space (or a subtree) as a flat, permission-filtered list in one request, in
+   * space root pages. From there it does an iterative breadth-first walk: each
-   * the exact node shape buildPageTree consumes. This replaces the old
+   * node is collected, and when node.hasChildren is true its direct children
-   * per-node BFS, which issued N sidebar requests and — after the server moved
+   * are fetched via listSidebarPages(spaceId, node.id) and enqueued.
   * to cursor pagination — silently lost every child past the first sidebar
   * page (the obsolete `page` param was stripped by ValidationPipe).
   *
-   * The subtree variant (rootPageId given) INCLUDES the root node itself
+   * This replaces the old "/pages/recent" enumeration, which is a bounded
-   * (getPageAndDescendants seeds with id = rootPageId), unlike the old BFS
+   * recent-activity feed (~5000 cap) and therefore misses comments on older
-   * which started from the root's children.
+   * pages that were never recently touched.
   *
-   * Fallback path (stdio mode may target STOCK upstream Docmost, which lacks
+   * Safeguards: a `visited` Set of page ids prevents re-processing a node
-   * `/pages/tree`): on a 404/405 it falls back to the cursor-based BFS below,
+   * (cycles / duplicate references), and a hard node cap bounds pathological
-   * walking direct children via the fixed cursor listSidebarPages. Safeguards:
+   * trees so the walk always terminates.
   * 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<{ pages: any[]; truncated: boolean }> {
+  ): Promise<any[]> {
    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);
@@ -805,12 +906,7 @@ export class DocmostClient {
      }
    }
-    // Truncated only when the cap was hit with the queue still non-empty (real
+    return result;
    // 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. */
@@ -2444,13 +2540,7 @@ export class DocmostClient {
    let allComments: any[] = [];
    let cursor: string | null = null;
-    // Hard ceiling + immovable-cursor guard (mirrors paginateAll): if /comments
+    do {
    // 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;
@@ -2458,23 +2548,8 @@ export class DocmostClient {
      const data = response.data.data || response.data;
      const items = data.items || [];
      allComments = allComments.concat(items);
-
+      cursor = data.meta?.nextCursor || null;
-      // Advance strictly via the server-issued cursor. A missing nextCursor or a
+    } while (cursor);
      // 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
@@ -2512,6 +2587,8 @@ export class DocmostClient {
  }
  async getComment(commentId: string) {
    // Fail fast (#436): reject a truncated id before any network call.
    assertFullUuid("get_comment", "commentId", commentId);
    await this.ensureAuthenticated();
    const response = await this.client.post("/comments/info", { commentId });
    const comment = response.data.data || response.data;
@@ -2601,6 +2678,12 @@ export class DocmostClient {
    parentCommentId?: string,
    suggestedText?: string,
  ) {
    // Fail fast (#436): a provided parent id must be a full UUID before any
    // network call. Validate only when truthy — a falsy parentCommentId means
    // "top-level comment" (mirrors the isReply computation below), not a reply.
    if (parentCommentId) {
      assertFullUuid("create_comment", "parentCommentId", parentCommentId);
    }
    await this.ensureAuthenticated();
    const isReply = !!parentCommentId;
@@ -2883,6 +2966,8 @@ export class DocmostClient {
  }
  async updateComment(commentId: string, content: string) {
    // Fail fast (#436): reject a truncated id before any network call.
    assertFullUuid("update_comment", "commentId", commentId);
    await this.ensureAuthenticated();
    // NON-canonicalizing on purpose (comment body — see createComment).
    const jsonContent = await markdownToProseMirror(content);
@@ -2898,6 +2983,8 @@ export class DocmostClient {
  }
  async deleteComment(commentId: string) {
    // Fail fast (#436): reject a truncated id before any network call.
    assertFullUuid("delete_comment", "commentId", commentId);
    await this.ensureAuthenticated();
    return this.client
      .post("/comments/delete", { commentId })
@@ -2910,6 +2997,8 @@ export class DocmostClient {
   * rejects resolving a reply. Hits POST /comments/resolve.
   */
  async resolveComment(commentId: string, resolved: boolean) {
    // Fail fast (#436): reject a truncated id before any network call.
    assertFullUuid("resolve_comment", "commentId", commentId);
    await this.ensureAuthenticated();
    const response = await this.client.post("/comments/resolve", {
      commentId,
@@ -2947,27 +3036,36 @@ export class DocmostClient {
      );
    }
-    // 1. Enumerate the FULL set of pages in scope via the page tree (a complete
+    // 1. Enumerate the FULL set of pages in scope by walking the sidebar-pages
-    // page index), NOT the bounded "/pages/recent" feed which caps at ~5000
+    // tree (a complete page index), NOT the bounded "/pages/recent" feed which
-    // recent items and silently misses comments on older pages.
+    // caps at ~5000 recent items and silently misses comments on older pages.
    //
    // Subtree scope: when parentPageId is given, the scope is that page ITSELF
-    // plus every descendant. Otherwise the scope is the whole space (all roots
+    // plus every descendant (enumerateSpacePages walks its children). Otherwise
-    // and their descendants).
+    // the scope is the whole space (all roots 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[];
-    // The subtree scope (parentPageId given) already INCLUDES the root node
+    if (parentPageId) {
-    // itself: /pages/tree seeds getPageAndDescendants with id = parentPageId, so
+      const subtree = await this.enumerateSpacePages(spaceId, parentPageId);
-    // no separate getPageRaw fetch for the parent is needed.
+      // Include the parent page node itself alongside its descendants. Fetch it
-    const { pages: pagesInScope, truncated } = await this.enumerateSpacePages(
+      // so its title/id are available even though it is not returned by its own
-      spaceId,
+      // children listing.
-      parentPageId,
+      let parentNode: any = { id: 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[] = [];
@@ -2996,9 +3094,10 @@ export class DocmostClient {
      0,
    );
-    // `truncated` is reported by enumerateSpacePages: it is true ONLY when the
+    // enumerateSpacePages caps traversal at 10000 nodes; flag when that cap was
-    // stdio fallback BFS hit its node cap. The primary /pages/tree path is
+    // hit so the caller knows the scan may be incomplete (some pages skipped).
-    // uncapped, so a space with legitimately many pages is not falsely flagged.
+    const truncated = pagesInScope.length >= 10000;
    return {
      since,
      scope: parentPageId ? `subtree of ${parentPageId}` : `space ${spaceId}`,
@@ -202,6 +202,21 @@ export class CollabSession {
    this.ydoc = new Y.Doc();
  }
  /**
   * Shared diagnostic suffix (issue #437) appended to the connect-timeout,
   * persist-timeout and connection-closed error texts: names the offending
   * pageId and tells the agent this class of failure is transient (retry once)
   * vs. a persistent collab-server outage, so it can self-correct instead of
   * blind-looping. The Yjs-encode error is deliberately NOT touched — it
   * already names the offending attribute.
   */
  private hint(): string {
    return (
      `(pageId ${this.pageId}; transient — retry once; persistent failures ` +
      `mean the collab server is unreachable/overloaded)`
    );
  }
  /**
   * A cached session may be reused only when it is fully ready, still synced,
   * has not lost its connection, and has not exceeded its max age (invariant 5
@@ -232,7 +247,9 @@ export class CollabSession {
        // The 25s connect timeout: the collab connection never became ready.
        this.opts?.onConnectTimeout?.();
        this.teardown(
-          new Error("Connection timeout to collaboration server"),
+          new Error(
            `Connection timeout to collaboration server ${this.hint()}`,
          ),
          false,
        );
      }, CONNECT_TIMEOUT_MS);
@@ -259,7 +276,7 @@ export class CollabSession {
          if (process.env.DEBUG) console.error("WS Disconnect");
          this.teardown(
            new Error(
-              "Collaboration connection closed before the update was persisted/synced",
+              `Collaboration connection closed before the update was persisted/synced ${this.hint()}`,
            ),
            true,
          );
@@ -268,7 +285,7 @@ export class CollabSession {
          if (process.env.DEBUG) console.error("WS Close");
          this.teardown(
            new Error(
-              "Collaboration connection closed before the update was persisted/synced",
+              `Collaboration connection closed before the update was persisted/synced ${this.hint()}`,
            ),
            true,
          );
@@ -403,7 +420,7 @@ export class CollabSession {
        persistTimer = setTimeout(() => {
          localFinish(
            new Error(
-              "Timeout waiting for collaboration server to persist the update",
+              `Timeout waiting for collaboration server to persist the update ${this.hint()}`,
            ),
          );
        }, PERSIST_TIMEOUT_MS);
@@ -203,14 +203,16 @@ test("a reply creates without selection or anchoring and is stored as type 'page
    "reply body",
    "inline",
    undefined,
-    "parent-123",
+    // #437: a parentCommentId must be a full canonical UUID.
    "019f499a-9f8c-7d68-b7be-ce100d7c6c56",
  );
  assert.equal(result.success, true, "a reply must resolve successfully");
  assert.ok(createPayload, "/comments/create must have been called");
  assert.equal(
    createPayload.parentCommentId,
-    "parent-123",
+    // #437: a parentCommentId must be a full canonical UUID.
    "019f499a-9f8c-7d68-b7be-ce100d7c6c56",
    "the reply payload must carry the parentCommentId",
  );
  assert.equal(
@@ -321,7 +323,9 @@ test("suggestedText on a reply is rejected", async () => {
        "body",
        "inline",
        undefined,
-        "parent-1",
+        // #437: use a valid full UUID so the reply+suggestion rejection fires
        // (not the id-shape guard).
        "019f499a-9f8c-7d68-b7be-ce100d7c6c56",
        "replacement",
      ),
    /reply/i,
@@ -1,440 +0,0 @@
 // 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");
 });
@@ -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 the server always issues a fresh cursor", async () => {
+test("paginateAll stops at the MAX_PAGES cap when hasNextPage is always true", async () => {
  let pageRequests = 0;
  const LIMIT = 100;
  const { baseURL } = await spawn(async (req, res) => {
-    const body = JSON.parse((await readBody(req)) || "{}");
+    await readBody(req);
    if (req.url === "/api/auth/login") {
      sendJson(res, 200, { success: true }, {
        "Set-Cookie": "authToken=t; Path=/; HttpOnly",
@@ -311,18 +311,15 @@ test("paginateAll stops at the MAX_PAGES cap when the server always issues a fre
    }
    if (req.url === "/api/spaces") {
      pageRequests++;
-      // Always return a FULL page AND hasNextPage:true with a FRESH nextCursor
+      // Always return a FULL page (== requested limit) AND hasNextPage:true.
-      // that differs from the one the client just sent, so the immovable-cursor
+      // Both the page-length check and the hasNextPage flag say "keep going",
-      // guard never trips — only the MAX_PAGES ceiling can stop the loop.
+      // so 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: {
+        data: { items, meta: { hasNextPage: true } },
          items,
          meta: { hasNextPage: true, nextCursor: `cursor-${pageRequests}` },
        },
      });
      return;
    }
@@ -341,7 +338,7 @@ test("paginateAll stops at the MAX_PAGES cap when the server always issues a fre
  assert.equal(all.length, 50 * LIMIT, "accumulates one full page per request");
 });
-test("paginateAll stops on the immovable-cursor guard when the server ignores the cursor param", async () => {
+test("paginateAll stops early on a short page even if hasNextPage is true", async () => {
  let pageRequests = 0;
  const LIMIT = 100;
@@ -355,17 +352,15 @@ test("paginateAll stops on the immovable-cursor guard when the server ignores th
    }
    if (req.url === "/api/spaces") {
      pageRequests++;
-      // The bug class: the server IGNORES the pagination param and keeps
+      // First page is full; second page is SHORT (fewer than limit). The short
-      // returning page one with hasNextPage:true and the SAME nextCursor. The
+      // page must stop the loop immediately even though hasNextPage stays true.
-      // immovable-cursor guard must stop the loop instead of spinning to
+      const count = pageRequests === 1 ? LIMIT : 3;
-      // MAX_PAGES and duplicating items.
+      const items = Array.from({ length: count }, (_, i) => ({
-      const items = Array.from({ length: LIMIT }, (_, i) => ({ id: `s-${i}` }));
+        id: `s-${pageRequests}-${i}`,
      }));
      sendJson(res, 200, {
        success: true,
-        data: {
+        data: { items, meta: { hasNextPage: true } },
          items,
          meta: { hasNextPage: true, nextCursor: "stuck" },
        },
      });
      return;
    }
@@ -375,10 +370,8 @@ test("paginateAll stops on the immovable-cursor guard when the server ignores th
  const client = new DocmostClient(baseURL, "user@example.com", "pw");
  const all = await client.paginateAll("/spaces", {}, LIMIT);
-  // Request 1 sends no cursor and receives "stuck"; request 2 sends "stuck" and
+  assert.equal(pageRequests, 2, "stops right after the first short page");
-  // receives "stuck" again -> guard trips after exactly two requests, no dups.
+  assert.equal(all.length, LIMIT + 3, "full page + short page accumulated");
  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 () => {
@@ -394,16 +387,16 @@ test("paginateAll handles both {data:{items,meta}} and {items,meta} envelopes",
    }
    if (req.url === "/api/groups") {
      bareRequests.push(1);
-      // Page 1: hasNextPage true with a next cursor. Page 2: no next -> stop.
+      // Page 1: full page, hasNextPage true. Page 2: short page -> stop.
      if (bareRequests.length === 1) {
        sendJson(res, 200, {
          items: Array.from({ length: 100 }, (_, i) => ({ id: `g${i}` })),
-          meta: { hasNextPage: true, nextCursor: "c2" },
+          meta: { hasNextPage: true },
        });
      } else {
        sendJson(res, 200, {
          items: [{ id: "tail" }],
-          meta: { hasNextPage: false, nextCursor: null },
+          meta: { hasNextPage: false },
        });
      }
      return;
@@ -168,7 +168,9 @@ test("an in-flight mutate rejects with the connection-closed text on disconnect"
  FakeProvider.last()._disconnect();
  await assert.rejects(
    p,
-    /Collaboration connection closed before the update was persisted\/synced/,
+    // Assert the #437 diagnostic hint tail too (pageId + transient/retry cue),
    // so a refactor that drops hint() can't pass this vacuously.
    /Collaboration connection closed before the update was persisted\/synced \(pageId page-1; transient/,
  );
 });
@@ -248,7 +250,11 @@ test("connect timeout rejects with the connect-timeout text and fires the metric
    },
  });
  mock.timers.tick(25000);
-  await assert.rejects(p, /Connection timeout to collaboration server/);
+  await assert.rejects(
    p,
    // Assert the #437 diagnostic hint tail too (pageId + transient/retry cue).
    /Connection timeout to collaboration server \(pageId page-1; transient/,
  );
  assert.equal(metricFired, 1);
  assert.equal(__sessionCountForTests(), 0);
 });
@@ -0,0 +1,450 @@
 // Issue #437: central error diagnostics.
 //
 // Two surfaces are covered here:
 //   1. formatDocmostAxiosError — the pure response-interceptor body that
 //      rewrites an AxiosError's `.message` into an actionable diagnostic.
 //   2. assertFullUuid — the fail-fast comment-id guard (absorbs #436) that must
 //      throw BEFORE any network call.
 // Plus an end-to-end pass over a real (offline) http server to prove the
 // interceptor is wired, that a re-login retry leaves a success untouched, and
 // that a persistent failure gets formatted — and that an invalid comment id
 // short-circuits every comment tool with ZERO network traffic.
 import { test, after } from "node:test";
 import assert from "node:assert/strict";
 import http from "node:http";
 import axios, { AxiosError } from "axios";
 import {
  DocmostClient,
  formatDocmostAxiosError,
  assertFullUuid,
 } from "../../build/client.js";
 // Build an AxiosError-shaped object the way the interceptor's rejection handler
 // receives it. Using the real AxiosError ctor makes axios.isAxiosError() true.
 function makeAxiosError({
  method = "post",
  url = "/comments/resolve",
  baseURL = "http://host.example/api",
  status,
  statusText,
  data,
  code,
  message = "Request failed",
 }) {
  const config = { method, url, baseURL };
  const response =
    status === undefined
      ? undefined
      : { status, statusText, data, headers: {}, config };
  return new AxiosError(message, code, config, {}, response);
 }
 // ---------------------------------------------------------------------------
 // formatDocmostAxiosError: message-body extraction rules.
 // ---------------------------------------------------------------------------
 test("class-validator message array is joined with '; '", () => {
  const err = makeAxiosError({
    status: 400,
    statusText: "Bad Request",
    data: { message: ["commentId must be a UUID", "resolved must be a boolean"] },
  });
  formatDocmostAxiosError(err);
  assert.equal(
    err.message,
    "POST /comments/resolve failed (400 Bad Request): commentId must be a UUID; resolved must be a boolean",
  );
 });
 test("a string message is used as-is", () => {
  const err = makeAxiosError({
    method: "post",
    url: "/comments/resolve",
    status: 400,
    statusText: "Bad Request",
    data: { message: "commentId must be a UUID" },
  });
  formatDocmostAxiosError(err);
  assert.equal(
    err.message,
    "POST /comments/resolve failed (400 Bad Request): commentId must be a UUID",
  );
 });
 test("falls back to data.error when message is absent", () => {
  const err = makeAxiosError({
    method: "get",
    url: "/pages/info",
    status: 403,
    statusText: "Forbidden",
    data: { error: "Forbidden" },
  });
  formatDocmostAxiosError(err);
  assert.equal(err.message, "GET /pages/info failed (403 Forbidden): Forbidden");
 });
 test("empty object body falls back to statusText", () => {
  const err = makeAxiosError({
    status: 404,
    statusText: "Not Found",
    url: "/comments/info",
    data: {},
  });
  formatDocmostAxiosError(err);
  assert.equal(err.message, "POST /comments/info failed (404 Not Found): Not Found");
 });
 test("HTML/string body is NEVER surfaced — only the statusText", () => {
  const html = "<html><body>502 Bad Gateway — nginx internals here</body></html>";
  const err = makeAxiosError({
    status: 502,
    statusText: "Bad Gateway",
    url: "/comments/create",
    data: html,
  });
  formatDocmostAxiosError(err);
  assert.equal(
    err.message,
    "POST /comments/create failed (502 Bad Gateway): Bad Gateway",
  );
  assert.ok(!err.message.includes("nginx"), "raw HTML body must not leak");
  assert.ok(!err.message.includes("<html>"), "raw HTML body must not leak");
 });
 test("Buffer body carrying JSON is parsed for its message", () => {
  const buf = Buffer.from(JSON.stringify({ message: "file too large" }), "utf8");
  const err = makeAxiosError({
    method: "get",
    url: "/files/abc/x.png",
    status: 413,
    statusText: "Payload Too Large",
    data: buf,
  });
  formatDocmostAxiosError(err);
  assert.equal(
    err.message,
    "GET /files/abc/x.png failed (413 Payload Too Large): file too large",
  );
 });
 test("Buffer body with non-JSON garbage falls back to statusText", () => {
  const buf = Buffer.from("<<< not json at all >>>", "utf8");
  const err = makeAxiosError({
    method: "get",
    url: "/files/abc/x.png",
    status: 500,
    statusText: "Internal Server Error",
    data: buf,
  });
  formatDocmostAxiosError(err);
  assert.equal(
    err.message,
    "GET /files/abc/x.png failed (500 Internal Server Error): Internal Server Error",
  );
  assert.ok(!err.message.includes("not json"), "raw buffer body must not leak");
 });
 test("an oversized Buffer body is not parsed (size cap) — statusText only", () => {
  // A >4KB JSON buffer: even though it IS valid JSON with a message, the size
  // cap means we do not attempt to parse it, so only the statusText survives.
  const big = { message: "x".repeat(5000) };
  const buf = Buffer.from(JSON.stringify(big), "utf8");
  const err = makeAxiosError({
    method: "get",
    url: "/files/abc/x.png",
    status: 500,
    statusText: "Internal Server Error",
    data: buf,
  });
  formatDocmostAxiosError(err);
  assert.equal(
    err.message,
    "GET /files/abc/x.png failed (500 Internal Server Error): Internal Server Error",
  );
 });
 // ---------------------------------------------------------------------------
 // formatDocmostAxiosError: no-response and path/method handling.
 // ---------------------------------------------------------------------------
 test("no response uses error.code + path + 'no response from server'", () => {
  const err = makeAxiosError({
    method: "post",
    url: "/comments/create",
    status: undefined,
    code: "ECONNREFUSED",
    message: "connect ECONNREFUSED 127.0.0.1:3000",
  });
  formatDocmostAxiosError(err);
  assert.equal(
    err.message,
    "POST /comments/create failed: ECONNREFUSED (no response from server)",
  );
 });
 test("no response with no code falls back to a neutral reason (raw message not leaked — it may embed host:port)", () => {
  const err = makeAxiosError({
    method: "post",
    url: "/comments/create",
    status: undefined,
    // A raw axios network message like "connect ECONNREFUSED 127.0.0.1:3000"
    // embeds the host; #437's invariant is that it never reaches the message.
    message: "connect ECONNREFUSED 10.0.0.5:3000",
  });
  formatDocmostAxiosError(err);
  assert.equal(
    err.message,
    "POST /comments/create failed: network error (no response from server)",
  );
  // And the host must NOT appear anywhere in the model-visible message.
  assert.ok(!err.message.includes("10.0.0.5"));
 });
 test("path drops the host and the query string", () => {
  const err = makeAxiosError({
    method: "post",
    url: "/comments/resolve?token=secret&x=1",
    baseURL: "https://docs.example.com/api",
    status: 400,
    statusText: "Bad Request",
    data: { message: "bad" },
  });
  formatDocmostAxiosError(err);
  assert.equal(
    err.message,
    "POST /comments/resolve failed (400 Bad Request): bad",
  );
  assert.ok(!err.message.includes("secret"), "query string must not leak");
  assert.ok(!err.message.includes("docs.example.com"), "host must not leak");
 });
 // ---------------------------------------------------------------------------
 // formatDocmostAxiosError: length cap + guard flag + pass-through.
 // ---------------------------------------------------------------------------
 test("the overall message is capped at ~300 chars", () => {
  const err = makeAxiosError({
    status: 400,
    statusText: "Bad Request",
    data: { message: "y".repeat(1000) },
  });
  formatDocmostAxiosError(err);
  assert.ok(err.message.length <= 300, `expected <=300, got ${err.message.length}`);
  assert.ok(err.message.endsWith("…"), "a truncated message ends with an ellipsis");
 });
 test("a formatted error is not re-processed (guard flag)", () => {
  const err = makeAxiosError({
    status: 400,
    statusText: "Bad Request",
    data: { message: "first" },
  });
  formatDocmostAxiosError(err);
  const once = err.message;
  assert.equal(err._docmostFormatted, true);
  // Mutate the body and re-run: the guard makes it a no-op.
  err.response.data = { message: "second" };
  formatDocmostAxiosError(err);
  assert.equal(err.message, once, "the guard flag prevents double-processing");
 });
 test("a non-axios error is passed through untouched", () => {
  const plain = new Error("boom");
  formatDocmostAxiosError(plain);
  assert.equal(plain.message, "boom");
  assert.equal(plain._docmostFormatted, undefined);
 });
 // ---------------------------------------------------------------------------
 // assertFullUuid.
 // ---------------------------------------------------------------------------
 const GOOD_UUID = "019f499a-9f8c-7d68-b7be-ce100d7c6c56";
 test("assertFullUuid accepts a full canonical UUID (any version nibble)", () => {
  assert.doesNotThrow(() => assertFullUuid("resolve_comment", "commentId", GOOD_UUID));
  // A v4 id also passes (version/variant-agnostic).
  assert.doesNotThrow(() =>
    assertFullUuid("get_comment", "commentId", "3d5b7c1e-2f4a-4b6c-8d9e-0f1a2b3c4d5e"),
  );
 });
 test("assertFullUuid rejects a truncated prefix", () => {
  assert.throws(
    () => assertFullUuid("resolve_comment", "commentId", "019f499a"),
    (e) =>
      e.message.startsWith(
        "resolve_comment: 'commentId' must be the FULL comment UUID",
      ) &&
      e.message.includes("got '019f499a'") &&
      e.message.includes("Copy the id verbatim"),
  );
 });
 test("assertFullUuid rejects garbage and empty string", () => {
  assert.throws(
    () => assertFullUuid("delete_comment", "commentId", "not-a-uuid"),
    /must be the FULL comment UUID.*got 'not-a-uuid'/s,
  );
  assert.throws(
    () => assertFullUuid("update_comment", "commentId", ""),
    /must be the FULL comment UUID.*got ''/s,
  );
 });
 // ---------------------------------------------------------------------------
 // End-to-end over an offline http server: interceptor wiring + re-login.
 // ---------------------------------------------------------------------------
 function readBody(req) {
  return new Promise((resolve) => {
    let raw = "";
    req.on("data", (c) => (raw += c));
    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 sendJson(res, status, obj, extra = {}) {
  res.writeHead(status, { "Content-Type": "application/json", ...extra });
  res.end(JSON.stringify(obj));
 }
 const openServers = [];
 async function spawn(handler) {
  const { server, baseURL } = await startServer(handler);
  openServers.push(server);
  return { baseURL };
 }
 after(async () => {
  await Promise.all(openServers.map((s) => new Promise((r) => s.close(r))));
 });
 test("a 400 on a JSON endpoint is reformatted by the wired interceptor", async () => {
  const { baseURL } = await spawn(async (req, res) => {
    await readBody(req);
    if (req.url === "/api/auth/login") {
      sendJson(res, 200, { success: true }, {
        "Set-Cookie": "authToken=t; Path=/; HttpOnly",
      });
      return;
    }
    if (req.url === "/api/pages/info") {
      sendJson(res, 400, { message: "pageId should not be empty" });
      return;
    }
    sendJson(res, 404, {});
  });
  const client = new DocmostClient(baseURL, "u@example.com", "pw");
  await assert.rejects(
    () => client.getPageRaw("x"),
    (e) => {
      assert.ok(axios.isAxiosError(e), "still an AxiosError (mutation, not a subclass)");
      assert.equal(e.response?.status, 400, "error.response?.status still readable");
      assert.equal(
        e.message,
        "POST /pages/info failed (400 Bad Request): pageId should not be empty",
      );
      return true;
    },
  );
 });
 test("401 -> re-login -> successful retry: the SUCCESS message is untouched", async () => {
  let infoCalls = 0;
  const { baseURL } = await spawn(async (req, res) => {
    await readBody(req);
    if (req.url === "/api/auth/login") {
      sendJson(res, 200, { success: true }, {
        "Set-Cookie": "authToken=fresh; Path=/; HttpOnly",
      });
      return;
    }
    if (req.url === "/api/workspace/info") {
      infoCalls++;
      if (infoCalls === 1) sendJson(res, 401, { message: "Unauthorized" });
      else sendJson(res, 200, { success: true, data: { id: "ws" } });
      return;
    }
    sendJson(res, 404, {});
  });
  const client = new DocmostClient(baseURL, "u@example.com", "pw");
  client.token = "stale";
  client.client.defaults.headers.common["Authorization"] = "Bearer stale";
  const result = await client.getWorkspace();
  assert.equal(result.success, true, "the retried request resolved successfully");
  assert.equal(infoCalls, 2, "401 then a successful replay");
 });
 test("401 -> re-login -> persistent failure: formatted AND retry guard intact", async () => {
  let infoCalls = 0;
  let loginCalls = 0;
  const { baseURL } = await spawn(async (req, res) => {
    await readBody(req);
    if (req.url === "/api/auth/login") {
      loginCalls++;
      sendJson(res, 200, { success: true }, {
        "Set-Cookie": "authToken=fresh; Path=/; HttpOnly",
      });
      return;
    }
    if (req.url === "/api/workspace/info") {
      infoCalls++;
      // Always 401, even after a fresh login: the _retry guard must stop here.
      sendJson(res, 401, { message: "token still invalid" });
      return;
    }
    sendJson(res, 404, {});
  });
  const client = new DocmostClient(baseURL, "u@example.com", "pw");
  client.token = "stale";
  client.client.defaults.headers.common["Authorization"] = "Bearer stale";
  await assert.rejects(
    () => client.getWorkspace(),
    (e) => {
      assert.equal(
        e.message,
        "POST /workspace/info failed (401 Unauthorized): token still invalid",
      );
      return true;
    },
  );
  // The _retry guard is intact: exactly one replay (2 hits), one re-login.
  assert.equal(infoCalls, 2, "endpoint hit at most twice (one retry only)");
  assert.equal(loginCalls, 1, "re-login attempted exactly once");
 });
 // ---------------------------------------------------------------------------
 // assertFullUuid application points: NO network call when the id is invalid.
 // A server that counts EVERY request proves the guard short-circuits before
 // even the login round-trip.
 // ---------------------------------------------------------------------------
 test("all 5 comment-id call sites reject a bad id with ZERO network traffic", async () => {
  let requests = 0;
  const { baseURL } = await spawn(async (req, res) => {
    requests++;
    await readBody(req);
    sendJson(res, 200, { success: true });
  });
  const client = new DocmostClient(baseURL, "u@example.com", "pw");
  const bad = "019f499a"; // truncated
  await assert.rejects(() => client.resolveComment(bad, true), /resolve_comment: 'commentId'/);
  await assert.rejects(() => client.updateComment(bad, "hi"), /update_comment: 'commentId'/);
  await assert.rejects(() => client.deleteComment(bad), /delete_comment: 'commentId'/);
  await assert.rejects(() => client.getComment(bad), /get_comment: 'commentId'/);
  // createComment validates parentCommentId only when provided.
  await assert.rejects(
    () => client.createComment("page-1", "body", "inline", "sel", bad),
    /create_comment: 'parentCommentId'/,
  );
  assert.equal(requests, 0, "no request (not even /auth/login) may be issued for a bad id");
 });
Author	SHA1	Message	Date
agent_coder	4809348457	test(mcp): ассертить collab-hint (pageId + transient/retry) в reject-текстах (ревью #441 ) Enrichment collab-ошибок из Phase C (#437) дописывает '(pageId …; transient — retry once; …)' к connect-timeout/connection-closed, но reject-регекспы матчили только базовый текст → проходили и С hint, и БЕЗ (vacuous). Ужесточил два ассерта (connection-closed, connect-timeout) до '<база> (pageId page-1; transient' — теперь рефактор, убравший hint(), их роняет. Мутационно: hint()->'' → ровно эти 2 теста краснеют (18->16). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-10 07:14:39 +03:00
agent_coder	d3d32d637b	fix(mcp): no-response диагностика — не отдавать сырой error.message (утечка host в модель) (внутр. ревью #437 ) no-response ветка формата использовала error.code ?? error.message: при отсутствии code axios-сообщения сетевых ошибок содержат host:port ('connect ECONNREFUSED 127.0.0.1:3000', 'getaddrinfo ENOTFOUND host'), что нарушает инвариант #437 «host никогда не попадает в видимое модели сообщение». Теперь только error.code (?? 'network error'); полный нативный текст уходит в stderr под DEBUG. code проставлен фактически для всех реальных no-response ошибок. Тест обновлён: сырое host-содержащее сообщение -> нейтральный reason, плюс ассерт что host в сообщении отсутствует. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-10 07:12:37 +03:00
agent_coder	9a435201b8	feat(mcp): enrich collab connect/persist/closed error texts with pageId + retry hint (#437 ) Append `(pageId <id>; transient — retry once; persistent failures mean the collab server is unreachable/overloaded)` to the connect-timeout, persist-timeout and connection-closed error texts in CollabSession, so the agent can self-correct instead of blind-looping. The Yjs-encode error is left untouched (it already names the offending attribute). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-10 07:12:37 +03:00
agent_coder	d6827b9210	feat(mcp): actionable tool errors — central axios diagnostics + fail-fast comment-id guard (#437 ) Phase A: add ONE response interceptor on DocmostClient's axios instance, registered AFTER the re-login interceptor, that reformats a failed request's error.message IN PLACE (never a custom Error subclass, so the live axios.isAxiosError / error.response?.status / config._retry checks keep working) into `<METHOD> <path> failed (<status> <statusText>): <serverMessage>`, or the no-response variant. serverMessage is built ONLY from the whitelisted message/error fields or statusText — raw string/HTML bodies, headers and config never appear; an arraybuffer body is size-capped JSON.parsed; the full body goes to stderr only under DEBUG (parity with downloadImage). A _docmostFormatted flag guards against double-processing. Phase B (#436): assertFullUuid throws an actionable error BEFORE any network call at all five commentId sites (resolve/update/delete/get_comment and create_comment's parentCommentId when provided), so a truncated id can no longer loop as an opaque 400/404. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-10 07:12:37 +03:00