fix(mcp): fail-fast гард на одновременный mutate одной CollabSession (ревью #431)

mutate трекает in-flight одним полем inflightReject; наложенный второй вызов
перезаписал бы рехджектор первого -> при disconnect отклонился бы только второй,
первый бы висел до PERSIST_TIMEOUT_MS (20с). В проде безопасно (оба call-site
сериализуют через per-page withPageLock), но это футган на разделяемом примитиве.
Гард в начале mutate (после ready-проверки, до касания inflightReject): наличие
in-flight -> reject нового вызова без порчи состояния первого. Docstring CONCURRENCY.
Последовательные mutate не задеты (localFinish синхронно чистит inflightReject до
резолва). +2 теста: конкурентный второй реджектится, первый цел; последовательные
оба успешны.

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

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

@@ -309,6 +309,14 @@ export class CollabSession {
    * unsyncedChanges is already 0, else wait for the unsyncedChanges->0 event
    * (PERSIST_TIMEOUT_MS), guarded by connectionLost so a reconnect handshake
    * cannot report a false success.
+   *
+   * CONCURRENCY: not safe to invoke concurrently on ONE session — the caller
+   * MUST serialize (hold the per-page lock), mirroring acquireCollabSession.
+   * The in-flight op is tracked in a single `inflightReject` field, so an
+   * overlapping second call would clobber the first's rejector and leave it
+   * hanging on disconnect. A fail-fast guard below rejects the overlap instead.
+   * Sequential (awaited) mutates are fine: localFinish clears inflightReject
+   * before the promise settles, so the guard is clear by the time the next runs.
    */
   mutate(
     transform: (liveDoc: any) => any | null,
@@ -327,6 +335,18 @@ export class CollabSession {
       );
     }
 
+    // Fail-fast on concurrent use: a second overlapping mutate would overwrite
+    // the first's inflightReject, so a disconnect would only reject the second
+    // and hang the first until PERSIST_TIMEOUT_MS. Reject the overlap WITHOUT
+    // touching the in-flight op's state (no localFinish/teardown here).
+    if (this.inflightReject) {
+      return Promise.reject(
+        new Error(
+          "mutate already in-flight; caller must serialize (hold the page lock)",
+        ),
+      );
+    }
+
     return new Promise<MutationResult>((resolve, reject) => {
       let settled = false;
       let persistTimer: ReturnType<typeof setTimeout> | undefined;

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

@@ -210,6 +210,34 @@ test("a pending write resolves when the server acks (unsyncedChanges -> 0)", asy
   assert.ok(r.doc);
 });
 
+test("concurrent mutate on one session: the second rejects, the first is unaffected", async () => {
+  __setCollabProviderFactory(factory({ unsynced: 1 })); // first stays pending after write
+  const session = await acquireCollabSession("page-1", "tok", "http://h/api");
+  // First mutate: write happens synchronously, persistence ack still pending.
+  const first = session.mutate(() => docWith("first"));
+  // Second overlapping mutate on the SAME session must fail fast.
+  await assert.rejects(
+    session.mutate(() => docWith("second")),
+    /mutate already in-flight; caller must serialize \(hold the page lock\)/,
+  );
+  // The first op is untouched: its rejector was not clobbered. Ack it now.
+  FakeProvider.last()._ack();
+  const r = await first;
+  assert.ok(r.doc, "the first in-flight mutate still resolves on its own ack");
+  assert.equal(FakeProvider.connectCount, 1, "no reconnect from the rejected overlap");
+});
+
+test("sequential mutates on one session both succeed (the guard doesn't break serialized use)", async () => {
+  const session = await acquireCollabSession("page-1", "tok", "http://h/api");
+  // Await the first fully, then run the second: inflightReject was cleared by
+  // localFinish before the first settled, so the guard is clear for the second.
+  const r1 = await session.mutate(() => docWith("one"));
+  assert.ok(r1.doc, "first sequential mutate resolves");
+  const r2 = await session.mutate(() => docWith("two"));
+  assert.ok(r2.doc, "second sequential mutate resolves — guard not tripped");
+  assert.equal(FakeProvider.connectCount, 1, "sequential mutates reuse one provider");
+});
+
 test("connect timeout rejects with the connect-timeout text and fires the metric hook", async () => {
   mock.timers.enable({ apis: ["setTimeout"] });
   __setCollabProviderFactory(factory({ autoSync: false }));