fix(mcp): ревью #513 — гонка LRU-эвикции + доводки контракта/доков

WARNING [stability]: эвикция могла убить ЧУЖУЮ ещё-connecting сессию как
idle-жертву — isBusy()=false во время open(), сессия вставлена в мапу до
await open(); параллельный acquire на ДРУГУЮ страницу при насыщении выселял
её → спурьёзный reject не-начатой записи / старвейшн новых записей. Фикс:
idle-victim скан теперь  — connecting
падает в last-resort oldestBusy, честный idle по-прежнему предпочитается,
кап держится, коалесинг того же ключа (per-page lock) не задет. Тест на
интерливинг (connecting не выселяется под насыщением), mutation-verified.

Доводки (проза, логику не меняют): drawioCreate description (nodeId:null на
nested-вставке); forward-коммент у writeWithCollabAuthRetry (ретрай только
auth; при расширении — проверять isCollabIndeterminateError, #435); хедер
comment-anchor (все 4 точки делегируют в resolveAnchorSelection); CHANGELOG.

Ребейзнут на develop (волна 1 смержена): только 4 коммита #494.

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

packages/mcp/src/client/context.ts

@@ -553,6 +553,13 @@ export abstract class DocmostClientContext {
     try {
       return await write(collabToken);
     } catch (e) {
+      // INVARIANT (#494/#435): this auto-retry MUST stay auth-only. A collab
+      // write can fail INDETERMINATE — its update may already have reached and
+      // persisted on the server (e.g. an LRU eviction of a busy session, tagged
+      // via isCollabIndeterminateError). Blindly retrying such a write duplicates
+      // it (the #435 double-apply class). If this gate is ever widened to retry a
+      // broader error class, FIRST check `isCollabIndeterminateError(e)` and do
+      // NOT retry an indeterminate write — re-read and verify before any retry.
       if (!isCollabAuthFailedError(e)) throw e;
       // The WS handshake rejected our token: drop it from the cache so it can't
       // be reused for the rest of the TTL, mint a fresh one (forceRefresh bypasses

packages/mcp/src/lib/collab-session.ts

@@ -732,20 +732,32 @@ export async function acquireCollabSession(
   // until there is room. PREFER an IDLE victim (#494): a session with an in-flight
   // mutate may have already sent (and persisted) its update, so evicting it would
   // reject that write as a FALSE failure → a retry-prone agent re-issues it →
-  // DUPLICATE write (the #435 class). So walk LRU order and skip busy sessions,
-  // evicting the oldest IDLE one. Only when EVERY cached session is busy (eviction
-  // unavoidable to admit this write) do we evict the LRU busy one — via
-  // evictForCap(), which rejects its in-flight op with a tagged INDETERMINATE
+  // DUPLICATE write (the #435 class). So walk LRU order and skip non-idle sessions,
+  // evicting the oldest IDLE one. Only when EVERY cached session is non-idle (eviction
+  // unavoidable to admit this write) do we evict the LRU non-idle one — via
+  // evictForCap(), which rejects an in-flight op with a tagged INDETERMINATE
   // "verify before retry" error rather than a plain failure.
+  //
+  // "Non-idle" = busy OR still opening. `sessions.set(key)` below runs BEFORE the
+  // `await session.open()` that resolves it, so a `connecting` session is in the
+  // map while its handshake is still in flight. Such a session is NOT busy yet
+  // (isBusy() needs an in-flight mutate), but it is ALSO not a legitimate idle
+  // victim: in multi-user HTTP two acquires for DIFFERENT pages interleave across
+  // that await, and under saturation the second acquire would otherwise pick the
+  // first's freshly-inserted `connecting` session as an "idle" victim and destroy
+  // it, rejecting the first's pending open() as "evicted (LRU cap)" — a spurious
+  // failure of a write that never even started. So exclude `state !== "ready"`
+  // from the idle scan; a connecting session falls into the last-resort bucket and
+  // is evicted ONLY when every other entry is busy-or-connecting too.
   while (sessions.size >= cfg.maxEntries) {
     let idleKey: string | undefined;
     let oldestBusyKey: string | undefined;
     for (const [k, s] of sessions) {
-      if (s.isBusy()) {
+      if (s.isBusy() || s.state !== "ready") {
         if (oldestBusyKey === undefined) oldestBusyKey = k;
         continue;
       }
-      idleKey = k; // first (LRU) idle session
+      idleKey = k; // first (LRU) genuinely-idle session
       break;
     }
     const victimKey = idleKey ?? oldestBusyKey;

packages/mcp/src/lib/comment-anchor.ts

@@ -22,14 +22,14 @@
  * inline markdown (`**bold**`, `` `code` ``, `[t](u)`), the raw locator will not
  * match the document's plain text. Exactly like editPageText's json-edit
  * fallback, we first try the verbatim selection and, ONLY if it anchors nowhere
- * in the whole document, retry with `stripInlineMarkdown` applied. `canAnchorInDoc`,
- * `getAnchoredText` and `applyAnchorInDoc` share this decision via
- * `resolveAnchorSelection`. `countAnchorMatches` keeps its OWN parallel exact-wins
- * implementation (it needs a raw match COUNT, not a single resolved locator), kept
- * deliberately in sync with `resolveAnchorSelection`: raw match ⇒ use raw, else fall
- * back to the stripped count. All four therefore agree on which locator matched —
- * the suggestion-uniqueness gate depends on count and can/get never disagreeing, so
- * these two exact-wins implementations MUST stay in sync if either is changed.
+ * in the whole document, retry with `stripInlineMarkdown` applied. All four entry
+ * points — `canAnchorInDoc`, `getAnchoredText`, `applyAnchorInDoc` and
+ * `countAnchorMatches` — share this exact-wins / strip-fallback decision through the
+ * SINGLE resolver `resolveAnchorSelection`; there is no second copy of the control
+ * flow. `countAnchorMatches` just asks the resolver which selection form wins and
+ * returns the raw occurrence count of that winning form. Because count and anchor
+ * derive from the same resolver, the suggestion-uniqueness gate (which depends on
+ * count) can never disagree with what actually anchors.
  */
 
 import { stripInlineMarkdown } from "./text-normalize.js";

packages/mcp/src/tool-specs.ts

@@ -1963,13 +1963,17 @@ export const SHARED_TOOL_SPECS = {
       'value escaping) — a violation returns a structured error naming the rule ' +
       'and cellId so you can fix and retry. `where` positions the block like ' +
       'insertNode: position before/after (with exactly one of anchorNodeId or ' +
-      'anchorText) or append. Returns { nodeId, attachmentId, warnings }. The ' +
-      'returned `nodeId` is an index-based "#<index>" handle (drawio nodes carry ' +
-      'no attrs.id): it addresses the new top-level block and can be fed straight ' +
-      'back into drawioGet / drawioUpdate for THIS document. It is positional, ' +
-      'so if you add or remove blocks before it, re-resolve via getOutline. The ' +
-      'diagram is editable in the draw.io editor and can be re-read with ' +
-      'drawioGet.' +
+      'anchorText) or append. Returns { nodeId, attachmentId, warnings }. On a ' +
+      'top-level insert the returned `nodeId` is an index-based "#<index>" handle ' +
+      '(drawio nodes carry no attrs.id): it addresses the new block and can be fed ' +
+      'straight back into drawioGet / drawioUpdate for THIS document. It is ' +
+      'positional, so if you add or remove blocks before it, re-resolve via ' +
+      'getOutline. When the block lands NESTED (e.g. anchored inside a callout or ' +
+      'table cell), "#<index>" cannot address it, so `nodeId` is `null` — the ' +
+      'write STILL SUCCEEDED (success:true, a warning explains this); do NOT ' +
+      're-create it. Re-read or edit that nested diagram by locating it via ' +
+      'getOutline / getPageJson using the returned attachmentId. The diagram is ' +
+      'editable in the draw.io editor and can be re-read with drawioGet.' +
       DRAWIO_HARD_RULES,
     tier: 'deferred',
     catalogLine:

packages/mcp/test/unit/collab-session.test.mjs

@@ -368,6 +368,74 @@ test("#494: evicting a busy session when all are busy rejects the write as INDET
   });
 });
 
+// #494 — a session whose open() has NOT resolved yet (state "connecting") sits in
+// the registry between `sessions.set(key)` and `await session.open()`, but it is
+// NOT an idle eviction victim: its write has not even started, and a parallel
+// acquire for a DIFFERENT page that interleaves across that await must not destroy
+// it (which would reject its pending open() as "evicted (LRU cap)" — a spurious
+// failure of a write that never began). Under saturation the connecting session is
+// spared; a genuinely-busy LRU session is evicted (INDETERMINATE) instead, and the
+// connecting session's open() still resolves.
+test("#494: a still-CONNECTING session is NOT evicted as an idle victim by a parallel acquire under saturation", async () => {
+  process.env.MCP_COLLAB_SESSION_MAX_ENTRIES = "2";
+
+  // A = the OLDEST (LRU) session, made BUSY by an in-flight (un-acked) mutate.
+  __setCollabProviderFactory(factory({ unsynced: 1 }));
+  const sA = await acquireCollabSession("page-1", "tok", "http://h/api");
+  const provA = FakeProvider.last();
+  const inflightA = sA.mutate(() => docWith("in-flight write")); // pending
+  // Attach a handler NOW so a later reject is never "unhandled"; capture it.
+  let inflightErr;
+  inflightA.catch((e) => {
+    inflightErr = e;
+  });
+
+  // B = a session still mid-handshake: open() never resolves under autoSync:false,
+  // so it stays "connecting". acquireCollabSession runs synchronously through
+  // `sessions.set(key)` and into open()'s executor (provider created) before it
+  // suspends at `await session.open()`, so B is already registered here.
+  __setCollabProviderFactory(factory({ autoSync: false }));
+  const pB = acquireCollabSession("page-2", "tok", "http://h/api"); // NOT awaited
+  const provB = FakeProvider.last();
+  assert.equal(__sessionCountForTests(), 2, "A (busy) + B (connecting) fill the cap");
+  assert.notEqual(provA, provB);
+
+  // C forces an eviction (cap 2). The idle-victim scan must treat B (connecting) as
+  // NON-idle and fall through to the busy LRU (A), evicting A as INDETERMINATE and
+  // SPARING the connecting B.
+  __setCollabProviderFactory(factory());
+  const sC = await acquireCollabSession("page-3", "tok", "http://h/api");
+  assert.equal(__sessionCountForTests(), 2);
+  assert.ok(sC);
+
+  assert.equal(
+    provB.destroyed,
+    false,
+    "the CONNECTING session must be spared — a parallel acquire must not evict a mid-handshake write",
+  );
+  assert.equal(
+    provA.destroyed,
+    true,
+    "the busy LRU session was the eviction victim instead",
+  );
+
+  // B's handshake completes -> its open() resolves and the acquire returns a live
+  // session (its write can now start), proving the eviction never touched it.
+  provB.config.onConnect?.();
+  provB.synced = true;
+  provB.config.onSynced?.();
+  const sB = await pB;
+  assert.ok(sB, "the spared connecting session's open() resolves normally");
+  assert.equal(provB.destroyed, false);
+
+  // The evicted busy write rejects as INDETERMINATE (verify-before-retry), not a
+  // plain failure — the existing all-busy guarantee still holds for A.
+  assert.ok(
+    isCollabIndeterminateError(inflightErr),
+    "the evicted busy write carries the INDETERMINATE marker",
+  );
+});
+
 test("MCP_COLLAB_SESSION_IDLE_MS=0 disables the cache (legacy provider-per-op)", async () => {
   process.env.MCP_COLLAB_SESSION_IDLE_MS = "0";
   const s1 = await acquireCollabSession("page-1", "tok", "http://h/api");