fix(ai-chat): ревью-раунд #500 — красный серверный сьют + регрессия Redis-health

B1: переименование строки ошибки (#394) не обновило 4 предсуществующих
share-ассерта → полный серверный сьют был красный. Обновлены ассерты в
public-share-chat.spec.ts и public-share-chat-tools.service.spec.ts под
новую классифицированную строку.

B2: /health первая проба после старта врала DOWN при живом Redis
(lazyConnect + enableOfflineQueue:false + maxRetriesPerRequest:1 → первый
ping до открытия сокета). Добавлен ensureConnected() с bounded-таймаутом
перед первым ping; покрывает и путь пересоздания после onModuleDestroy.
Тест UP-с-первой-пробы против реального ioredis (mutation-verified).

Устранён open-handle leak в redis.health.spec (drain ioredis
force-destroy-таймера в teardown; без forceExit) и окно ложного DOWN при
конкурентных пробах (мемоизированный connectingPromise).

B3: комментарий про orphan-чат при провале beginRun (insert до begin).
B4: описание listPages упоминает поле truncated в tree-режиме.

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

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

@@ -758,6 +758,13 @@ export class AiChatService implements OnModuleInit {
       // or violate the page_id FK on insert (this runs after res.hijack(), so a
       // DB error would break the stream).
       const originPageId: string | null = openPageContext?.id ?? null;
+      // ORPHAN-ON-BEGIN-FAILURE tradeoff (#486, B3): the chat row is inserted
+      // HERE, before runHooks.begin below. If begin fails (e.g. a 503 / run-slot
+      // rejection) the turn aborts before the client is told this new chatId, so
+      // an empty chat is left behind and a retry mints ANOTHER one. We accept this
+      // over reordering: begin needs a chatId to bind the run to, and inserting
+      // the chat first keeps the id stable + the FK/history-join invariants above
+      // intact. Orphan empty chats are cheap and swept by normal chat cleanup.
       const chat = await this.aiChatRepo.insert({
         creatorId: user.id,
         workspaceId: workspace.id,

apps/server/src/core/ai-chat/public-share-chat.spec.ts

@@ -808,7 +808,7 @@ describe('PublicShareChatToolsService share scoping', () => {
     };
 
     await expect(getSharePage.execute({ pageId: 'p-outside' })).rejects.toThrow(
-      /not part of this published share/i,
+      /not available in this share/i,
     );
     // The tool delegated the resolve to the canonical boundary with the
     // forShare-scoped shareId, and returned NO content for a non-resolving page.
@@ -841,7 +841,7 @@ describe('PublicShareChatToolsService share scoping', () => {
 
     await expect(
       getSharePage.execute({ pageId: 'p-restricted' }),
-    ).rejects.toThrow(/not part of this published share/i);
+    ).rejects.toThrow(/not available in this share/i);
     // No content was ever sanitized/returned for the blocked page.
     expect(shareService.updatePublicAttachments).not.toHaveBeenCalled();
   });
@@ -1003,7 +1003,7 @@ describe('public-share assistant boundary locks (red-team regression guards)', (
     };
     await expect(
       getSharePage.execute({ pageId: 'p-elsewhere' }),
-    ).rejects.toThrow(/not part of this published share/i);
+    ).rejects.toThrow(/not available in this share/i);
     // The forged share id is the scope the boundary re-derivation rejects against.
     expect(shareService.resolveReadableSharePage).toHaveBeenCalledWith(
       'FORGED-SHARE',

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

@@ -224,7 +224,7 @@ describe('PublicShareChatToolsService.forShare', () => {
         (tools.getSharePage as unknown as ToolExec).execute({
           pageId: 'page-1',
         }),
-      ).rejects.toThrow('That page is not part of this published share.');
+      ).rejects.toThrow('The requested page is not available in this share.');
 
       // No content is ever fetched/returned for a non-resolving page.
       expect(shareService.updatePublicAttachments).not.toHaveBeenCalled();

apps/server/src/integrations/health/redis.health.spec.ts

@@ -16,7 +16,72 @@ import type { EnvironmentService } from '../environment/environment.service';
  * implementation (requireActual) — only the constructor is wrapped to COUNT the
  * real clients it creates, which is precisely the leaking resource.
  */
-const mockLiveClients: Array<{ status: string; disconnect: () => void }> = [];
+import type { Redis } from 'ioredis';
+
+const mockLiveClients: Redis[] = [];
+
+/**
+ * Fully tear a REAL ioredis client down so NO timer survives jest's 1s exit
+ * window (this suite must exit cleanly WITHOUT forceExit; see #382).
+ *
+ * `connector.disconnect()` arms a ~12s "force-destroy the stream" `setTimeout`
+ * that is cleared ONLY by the stream's 'close' event — but only when the
+ * connector still holds a stream. Two problem cases:
+ *  - a LIVE/connecting socket: disconnect arms the timer and 'close' may lag
+ *    past jest's window, so we destroy the socket to make 'close' fire NOW;
+ *  - a client BETWEEN reconnect attempts to a dead port: the held socket is
+ *    ALREADY destroyed (its 'close' fired long ago), so disconnect would arm a
+ *    timer whose clearing 'close' can never come again. We drop that dead stream
+ *    reference BEFORE disconnect so the doomed timer is never armed.
+ * `disconnect()` itself also clears ioredis' own reconnect backoff timer.
+ */
+type DrainableStream = { destroyed?: boolean; destroy?: () => void } | null;
+type DrainableClient = {
+  removeAllListeners: (event: string) => void;
+  disconnect: () => void;
+  stream?: DrainableStream;
+  connector?: { stream?: DrainableStream };
+};
+
+async function drainClient(client: Redis): Promise<void> {
+  if (!client || client.status === 'end') return;
+  const c = client as unknown as DrainableClient;
+  c.removeAllListeners('error');
+
+  // Drop an already-dead held socket so disconnect() can't arm a timer whose
+  // clearing 'close' will never fire again.
+  if (c.connector?.stream && c.connector.stream.destroyed) {
+    c.connector.stream = null;
+  }
+  if (c.stream && c.stream.destroyed) {
+    c.stream = null;
+  }
+
+  await new Promise<void>((resolve) => {
+    let done = false;
+    const finish = () => {
+      if (done) return;
+      done = true;
+      resolve();
+    };
+    client.once('end', finish);
+    // reconnect=false (the default): stop the retry loop and close the socket.
+    client.disconnect();
+    // Force any still-live socket closed NOW so the connector's stream-destroy
+    // timer clears inside jest's window instead of lagging behind a real 'close'.
+    if (c.stream && !c.stream.destroyed) {
+      c.stream.destroy?.();
+    }
+    // Fallback for a client with no live stream to emit 'end' (unref'd so it
+    // can never itself hold the loop open).
+    const fallback = setTimeout(finish, 500);
+    (fallback as { unref?: () => void }).unref?.();
+  });
+}
+
+async function drainAll(): Promise<void> {
+  await Promise.all(mockLiveClients.map((c) => drainClient(c)));
+}
 
 jest.mock('ioredis', () => {
   const actual = jest.requireActual('ioredis');
@@ -53,17 +118,12 @@ describe('RedisHealthIndicator handle leak (#486)', () => {
     indicator = new RedisHealthIndicator(indicatorService, environmentService);
   });
 
-  afterEach(() => {
+  afterEach(async () => {
+    // Drain (destroy socket + AWAIT 'end') every client the test created FIRST,
+    // so each is fully 'end' before onModuleDestroy's disconnect runs — that way
+    // no ioredis reconnect / stream-destroy timer outlives jest's exit window.
+    await drainAll();
     indicator.onModuleDestroy();
-    // Belt-and-braces: tear down anything the test created so ioredis reconnect
-    // timers do not keep the jest worker alive.
-    for (const c of mockLiveClients) {
-      try {
-        c.disconnect();
-      } catch {
-        /* already gone */
-      }
-    }
   });
 
   it('creates exactly ONE Redis client across many probes while Redis is DOWN', async () => {
@@ -93,3 +153,59 @@ describe('RedisHealthIndicator handle leak (#486)', () => {
     expect(mockLiveClients).toHaveLength(2);
   });
 });
+
+/**
+ * Happy-path regression guard (#486, B2): the FIRST probe against a LIVE Redis
+ * must report UP.
+ *
+ * With `lazyConnect: true` + `enableOfflineQueue: false`, a freshly-built client
+ * is in the `wait` state and the socket opens lazily. If the very first `ping()`
+ * is issued before an explicit `connect()`, ioredis rejects it instantly with
+ * "Stream isn't writeable and enableOfflineQueue options is false" — a FALSE
+ * DOWN even though Redis is alive. The fix opens the socket before the first
+ * ping. This exercises a REAL ioredis client against a REAL TCP redis server
+ * (not a mock), so a regression genuinely reddens it.
+ */
+describe('RedisHealthIndicator live Redis first-probe (#486, B2)', () => {
+  const indicatorService = {
+    check: (key: string) => ({
+      up: () => ({ [key]: { status: 'up' } }),
+      down: (message: string) => ({ [key]: { status: 'down', message } }),
+    }),
+  } as unknown as HealthIndicatorService;
+
+  // A REAL running redis (see the neighboring harness / CI env).
+  const environmentService = {
+    getRedisUrl: () => 'redis://127.0.0.1:6379/0',
+  } as unknown as EnvironmentService;
+
+  let indicator: RedisHealthIndicator;
+
+  beforeEach(() => {
+    mockLiveClients.length = 0;
+    indicator = new RedisHealthIndicator(indicatorService, environmentService);
+  });
+
+  afterEach(async () => {
+    // Await full socket close of every live client (see drainClient) BEFORE
+    // onModuleDestroy: a real, connected ioredis client MUST be drained to 'end'
+    // or its stream-destroy timer keeps the jest worker alive past the 1s window.
+    await drainAll();
+    indicator.onModuleDestroy();
+  });
+
+  it('reports UP on the FIRST probe against a live Redis', async () => {
+    // The VERY FIRST probe — no warm-up ping — must be UP.
+    const result = await indicator.pingCheck('redis');
+    expect(result.redis.status).toBe('up');
+  });
+
+  it('stays UP on a probe AFTER onModuleDestroy re-creates the client', async () => {
+    await indicator.pingCheck('redis');
+    indicator.onModuleDestroy();
+    // The re-created client is again in `wait`; the first ping on it must still
+    // open the socket (the false-DOWN also recurs on the post-destroy path).
+    const result = await indicator.pingCheck('redis');
+    expect(result.redis.status).toBe('up');
+  });
+});

apps/server/src/integrations/health/redis.health.ts

@@ -20,6 +20,26 @@ export class RedisHealthIndicator implements OnModuleDestroy {
    */
   private probeClient: Redis | null = null;
 
+  /**
+   * How long the first-ping `connect()` may take before a probe gives up and
+   * reports DOWN. A `connect()` against a truly-down Redis never settles on its
+   * own (ioredis retries the socket indefinitely per its retryStrategy), so the
+   * probe MUST bound it or the /health handler would hang. Kept short so a real
+   * outage is reported fast; localhost/live Redis connects well within it.
+   */
+  private static readonly CONNECT_TIMEOUT_MS = 2000;
+
+  /**
+   * The single in-flight first-`connect()`, memoized so CONCURRENT probes share
+   * it. k8s liveness+readiness hit /health in parallel on startup: without this,
+   * probe A drives `connect()` (the client leaves the `wait` state) and probe B,
+   * seeing a not-`wait`/not-`ready` client, would skip connect and fire `ping()`
+   * at a still-opening socket → an instant FALSE DOWN. With the memo, B awaits
+   * the SAME connect. Cleared once it settles so a later disconnect / re-create
+   * starts a fresh connect.
+   */
+  private connectingPromise: Promise<void> | null = null;
+
   constructor(
     private readonly healthIndicatorService: HealthIndicatorService,
     private environmentService: EnvironmentService,
@@ -52,11 +72,82 @@ export class RedisHealthIndicator implements OnModuleDestroy {
     return this.probeClient;
   }
 
+  /**
+   * Open the probe socket BEFORE the first ping. `lazyConnect: true` leaves a
+   * freshly-built (or post-destroy re-built) client in the `wait` state: the
+   * socket is NOT open yet, so with `enableOfflineQueue: false` the very first
+   * `ping()` rejects instantly with "Stream isn't writeable and
+   * enableOfflineQueue options is false" even when Redis is perfectly alive — a
+   * false DOWN on the happy path. We drive `connect()` ONLY from `wait`; once
+   * the client is connected, ioredis owns its own (re)connect loop and a ping
+   * issued while it reconnects still fast-fails to a correct DOWN (offline queue
+   * stays off). A failed/timed-out connect rejects → reported DOWN, which is the
+   * right signal for a truly-down Redis.
+   */
+  private ensureConnected(client: Redis): Promise<void> {
+    // Already open — steady state, nothing to do.
+    if (client.status === 'ready') return Promise.resolve();
+    // A first-connect is already in flight (possibly started by a CONCURRENT
+    // probe): await the SAME one instead of racing a second connect() (ioredis
+    // throws "already connecting") or firing ping() at a not-yet-open socket.
+    if (this.connectingPromise) return this.connectingPromise;
+    // Only DRIVE connect() from the initial `wait` state (fresh / post-destroy
+    // re-created client). In any other non-ready state ioredis already owns its
+    // (re)connect loop; a ping there fast-fails to a correct DOWN, so we must not
+    // start a competing connect.
+    if (client.status !== 'wait') return Promise.resolve();
+
+    const promise = this.connectWithTimeout(client).finally(() => {
+      // Clear only if still ours, so a later disconnect / re-create can connect
+      // again. Whether it resolved or rejected, the memo has served its window.
+      if (this.connectingPromise === promise) {
+        this.connectingPromise = null;
+      }
+    });
+    this.connectingPromise = promise;
+    return promise;
+  }
+
+  private connectWithTimeout(client: Redis): Promise<void> {
+    return new Promise<void>((resolve, reject) => {
+      let settled = false;
+      const timer = setTimeout(() => {
+        if (settled) return;
+        settled = true;
+        reject(new Error('Redis probe connect timed out'));
+      }, RedisHealthIndicator.CONNECT_TIMEOUT_MS);
+      // Never let THIS timer alone keep the event loop (or a jest worker) alive;
+      // it is cleared on settle anyway, this is belt-and-braces.
+      timer.unref?.();
+      // `.catch` is always attached, so a connect() that rejects AFTER we have
+      // already timed out is handled here (guarded by `settled`) and never
+      // surfaces as an unhandled rejection.
+      client
+        .connect()
+        .then(() => {
+          if (settled) return;
+          settled = true;
+          clearTimeout(timer);
+          resolve();
+        })
+        .catch((err) => {
+          if (settled) return;
+          settled = true;
+          clearTimeout(timer);
+          reject(err);
+        });
+    });
+  }
+
   async pingCheck(key: string): Promise<HealthIndicatorResult> {
     const indicator = this.healthIndicatorService.check(key);
 
     try {
       const redis = this.getProbeClient();
+      // Open the socket before the first ping (see ensureConnected); without
+      // this the first probe after (re)creation falsely reports DOWN on a live
+      // Redis because lazyConnect defers the connect past the first ping.
+      await this.ensureConnected(redis);
       await redis.ping();
       return indicator.up();
     } catch (e) {
@@ -75,5 +166,9 @@ export class RedisHealthIndicator implements OnModuleDestroy {
       this.probeClient.disconnect();
       this.probeClient = null;
     }
+    // Drop any in-flight first-connect memo so the NEXT client (lazily rebuilt on
+    // the next probe) starts a fresh connect rather than awaiting a promise tied
+    // to the client we just tore down.
+    this.connectingPromise = null;
   }
 }

packages/mcp/src/tool-specs.ts

@@ -923,9 +923,10 @@ export const SHARED_TOOL_SPECS = {
       'List the most recent pages (ordered by updatedAt, descending), ' +
       'optionally scoped to a single space. Returns a bounded list (default ' +
       '50, max 100) — use search for lookups in large spaces. tree:true (with ' +
-      "spaceId) returns the space's full page hierarchy as a nested tree, but " +
-      'is DEPRECATED — use getTree instead (leaner nodes, plus rootPageId / ' +
-      'maxDepth).',
+      "spaceId) returns { tree, truncated } — the space's full page hierarchy " +
+      'as a nested tree, plus a `truncated` flag that is true when the tree was ' +
+      'capped and is INCOMPLETE — but is DEPRECATED, use getTree instead ' +
+      '(leaner nodes, plus rootPageId / maxDepth).',
     tier: 'core',
     catalogLine:
       "listPages — list recent pages (tree:true is deprecated; use getTree for the hierarchy).",