fix(git-sync): don't clobber pages with a live editing session; crash-safe body write

Review finding #5: the git -> page body write (writeBody) did a full-body replace
(delete-all + re-insert) on the shared Yjs doc. Applied while a human is editing
the page, it discarded their in-flight changes; and TiptapTransformer.toYdoc ran
AFTER the fragment was cleared, so a conversion failure could leave the page with
an empty body.

Fixes:
- Active-session guard: CollaborationGateway.getActiveEditorCount(documentName)
  reports live human (websocket) editor sessions for a doc, excluding server-side
  direct connections. writeBody now throws ActiveEditSessionError when an editor
  is connected. The engine's push loop already isolates each importPageMarkdown in
  try/catch and does not advance the loop-guard on failure, so the write is simply
  retried on the next poll once the editor disconnects — never a clobber.
- Crash-safe conversion: build the replacement Yjs update BEFORE opening the
  connection / clearing the fragment, so a transform failure can never leave the
  body empty.

Also updates the server-side converter gate spec to the corrected round-trip
shape: the block-image hoist no longer leaves a leading empty paragraph (the
git-sync converter fix in 7d39c16b, now reaching the built package).

A true merge of git content into a live Yjs session is out of scope (it needs a
real 3-way text merge with no shared update lineage); deferring the write while a
page is being edited is the safe, owner-approved minimum.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

apps/server/src/collaboration/collaboration.gateway.ts

@@ -137,6 +137,21 @@ export class CollaborationGateway {
     return this.hocuspocus.getDocumentsCount();
   }
 
+  /**
+   * Number of LIVE human editor sessions (websocket connections) currently open
+   * on a document, or 0 if the document is not loaded. Unlike
+   * `Document.getConnectionsCount()` this deliberately excludes server-side
+   * direct connections (`directConnectionsCount`, e.g. the git-sync writer
+   * itself), so callers can tell whether a real person is editing right now.
+   *
+   * NOTE: this reflects only THIS instance. In a Redis-clustered deployment an
+   * editor attached to another node is not counted; for the single-instance
+   * deployments this guards (git-sync) that is exactly the live set.
+   */
+  getActiveEditorCount(documentName: string): number {
+    return this.hocuspocus.documents.get(documentName)?.connections.size ?? 0;
+  }
+
   handleYjsEvent<TName extends keyof CollabEventHandlers>(
     eventName: TName,
     documentName: string,

apps/server/src/collaboration/git-sync-converter-gate.spec.ts

@@ -401,18 +401,19 @@ describe('git-sync converter §13.1 KNOWN DIVERGENCE (markdown image lossiness)'
     },
   });
 
-  it('drops width/height/align (markdown ![](src) cannot carry them) and hoists the block image past a leading empty paragraph', async () => {
+  it('drops width/height/align (markdown ![](src) cannot carry them); the block-image hoist no longer leaves an empty paragraph', async () => {
     const { md, canonNormalized } = await runGate(imageDoc);
 
     // Export is plain markdown image syntax — no dimensions/align survive.
     expect(md.trim()).toBe('![](https://example.com/pic.png)');
 
-    // The round-tripped doc is the documented lossy shape: a leading empty
+    // The round-tripped doc carries ONLY src (+ alt=""). The leading empty
-    // paragraph (block-image hoist) + an image carrying ONLY src (+ alt="").
+    // paragraph that the block-image hoist used to leave behind (a phantom
+    // blank-gap on every sync) is now stripped on import (git-sync fix), so the
+    // doc is just the image — no empty-paragraph artifact.
     expect(canonNormalized).toEqual({
       type: 'doc',
       content: [
-        { type: 'paragraph' },
         {
           type: 'image',
           attrs: { alt: '', src: 'https://example.com/pic.png' },
@@ -420,7 +421,8 @@ describe('git-sync converter §13.1 KNOWN DIVERGENCE (markdown image lossiness)'
       ],
     });
 
-    // And it is therefore NOT canonically equal to the original (lock the loss).
+    // Still NOT canonically equal to the original: width/height/align are an
+    // intrinsic markdown-transport loss (unrelated to the empty-paragraph fix).
     expect(docsCanonicallyEqual(imageDoc, canonNormalized)).toBe(false);
   });
 });

apps/server/src/integrations/git-sync/services/gitmost-datasource.service.spec.ts

@@ -7,13 +7,35 @@ jest.mock('../../../collaboration/collaboration.util', () => ({
   tiptapExtensions: [],
   getPageId: (name: string) => name.replace(/^page\./, ''),
 }));
+// writeBody now builds the replacement Yjs state eagerly (before clearing the
+// live doc), so TiptapTransformer.toYdoc runs in these unit tests. Real Tiptap
+// extensions are stubbed to [] above (they drag in the React graph), which can't
+// build a schema — so stub the transformer to return a small non-empty Y.Doc.
+// The real conversion is exercised by the @docmost/git-sync converter tests and
+// the integration tests.
+jest.mock('@hocuspocus/transformer', () => {
+  const Yjs = require('yjs');
+  return {
+    TiptapTransformer: {
+      toYdoc: jest.fn(() => {
+        const d = new Yjs.Doc();
+        d.getXmlFragment('default').insert(0, [new Yjs.XmlElement('paragraph')]);
+        return d;
+      }),
+    },
+  };
+});
 // PageService is only ever a mocked dependency here; stub the editor-ext entry
 // it imports so loading its module does not pull in the React graph either.
 jest.mock('@docmost/editor-ext', () => ({
   markdownToHtml: jest.fn(),
 }));
 
-import { GitmostDataSourceService } from './gitmost-datasource.service';
+import * as Y from 'yjs';
+import {
+  GitmostDataSourceService,
+  ActiveEditSessionError,
+} from './gitmost-datasource.service';
 
 // Focused unit/contract test for the native GitSyncClient adapter (plan §3).
 // No DB, no real collab server: the repos/services/gateway are mocked and we
@@ -34,7 +56,10 @@ interface Mocks {
     movePage: AnyMock;
     removePage: AnyMock;
   };
-  collabGateway: { openDirectConnection: AnyMock };
+  collabGateway: {
+    openDirectConnection: AnyMock;
+    getActiveEditorCount: AnyMock;
+  };
   // Minimal Kysely-ish chainable mock for the direct-query paths.
   db: any;
   // Captured collab connection (the fake conn the gateway returns).
@@ -85,6 +110,8 @@ function build(rows: any[] = []): {
         conn.context = ctx;
         return conn;
       }),
+      // Default: no live editor sessions, so body writes proceed.
+      getActiveEditorCount: jest.fn(() => 0),
     },
     db: {
       selectFrom: jest.fn(() => makeQueryBuilder(rows)),
@@ -219,6 +246,44 @@ describe('GitmostDataSourceService', () => {
 
       expect(res.updatedAt).toBe('2026-06-20T11:00:00.000Z');
     });
+
+    it('defers (throws ActiveEditSessionError) when a human is editing the page — never clobbers', async () => {
+      const { service, mocks } = build();
+      // A live editor session on this page.
+      mocks.collabGateway.getActiveEditorCount.mockReturnValue(1);
+
+      await expect(
+        service.bind(CTX).importPageMarkdown('p1', '# Hello\n\nworld'),
+      ).rejects.toBeInstanceOf(ActiveEditSessionError);
+
+      // The destructive full-body write must NOT have happened: no connection
+      // opened, no transact run. The engine's push loop catches this and retries
+      // on the next poll once the editor disconnects.
+      expect(mocks.collabGateway.getActiveEditorCount).toHaveBeenCalledWith(
+        'page.p1',
+      );
+      expect(mocks.collabGateway.openDirectConnection).not.toHaveBeenCalled();
+      expect(mocks.conn.transact).not.toHaveBeenCalled();
+    });
+
+    it('crash-safe: the captured write applies real content (update built before clearing)', async () => {
+      // The replacement Yjs update is computed BEFORE the connection opens / the
+      // fragment is cleared, so a transform failure can never leave the body
+      // emptied. Here we run the captured transact callback against a REAL Y.Doc
+      // and confirm it ends up with content (the precomputed update is valid and
+      // applied), i.e. the write produces a non-empty body rather than wiping it.
+      const { service, mocks } = build();
+      mocks.pageRepo.findById.mockResolvedValue({
+        id: 'p1',
+        updatedAt: new Date('2026-06-20T11:00:00.000Z'),
+      });
+      await service.bind(CTX).importPageMarkdown('p1', '# Hi\n\nthere');
+
+      const realDoc = new Y.Doc();
+      expect(() => mocks.conn.capturedFn?.(realDoc)).not.toThrow();
+      // The body fragment is non-empty: the markdown was converted and applied.
+      expect(realDoc.getXmlFragment('default').length).toBeGreaterThan(0);
+    });
   });
 
   describe('createPage', () => {

apps/server/src/integrations/git-sync/services/gitmost-datasource.service.ts

@@ -42,6 +42,23 @@ const GIT_SYNC_PROVENANCE: AuthProvenanceData = {
   aiChatId: null,
 };
 
+/**
+ * Thrown when a git -> page body write is skipped because a human is editing the
+ * page RIGHT NOW (a live collab session). The engine's push loop catches this
+ * per page, records it as a (non-fatal) failure, and does NOT advance the
+ * loop-guard for that page — so the write is retried on the next poll once the
+ * editor disconnects, instead of clobbering their in-flight edits with a
+ * full-body replace (plan §15.6 / review #5).
+ */
+export class ActiveEditSessionError extends Error {
+  constructor(pageId: string) {
+    super(
+      `git-sync: page ${pageId} has an active edit session; deferring body write`,
+    );
+    this.name = 'ActiveEditSessionError';
+  }
+}
+
 /**
  * Native, in-process implementation of the engine's `GitSyncClient` seam
  * (plan §3). Reads go through repositories (PageRepo/SpaceRepo); body writes go
@@ -379,7 +396,32 @@ export class GitmostDataSourceService {
     prosemirrorJson: unknown,
     userId: string,
   ): Promise<void> {
-    const conn = await this.collabGateway.openDirectConnection(`page.${pageId}`, {
+    const documentName = `page.${pageId}`;
+
+    // Do NOT clobber a page someone is editing right now. The write below is a
+    // full-body replace (delete-all + re-insert); applied over a live editing
+    // session it would discard the user's in-flight changes. If a human editor
+    // is connected, defer: throw so the engine retries on the next poll once
+    // they disconnect (review #5 — "не писать в страницу с активной сессией").
+    if (this.collabGateway.getActiveEditorCount(documentName) > 0) {
+      this.logger.debug(
+        `Skipping git-sync body write for ${documentName}: active edit session`,
+      );
+      throw new ActiveEditSessionError(pageId);
+    }
+
+    // Build the replacement Yjs state BEFORE touching the live doc. If the
+    // transform throws (a malformed/unsupported doc), we must NOT have already
+    // cleared the fragment — otherwise a conversion failure would leave the page
+    // with an empty body (review #5 — crash-safe conversion).
+    const next = TiptapTransformer.toYdoc(
+      prosemirrorJson,
+      'default',
+      tiptapExtensions,
+    );
+    const update = Y.encodeStateAsUpdate(next);
+
+    const conn = await this.collabGateway.openDirectConnection(documentName, {
       actor: 'git-sync',
       // PersistenceExtension reads `context.user.id` for lastUpdatedById, so the
       // service user is required on the context (unlike the bare `{ actor }`
@@ -390,12 +432,7 @@ export class GitmostDataSourceService {
       await conn.transact((doc) => {
         const fragment = doc.getXmlFragment('default');
         if (fragment.length > 0) fragment.delete(0, fragment.length);
-        const next = TiptapTransformer.toYdoc(
+        Y.applyUpdate(doc, update);
-          prosemirrorJson,
-          'default',
-          tiptapExtensions,
-        );
-        Y.applyUpdate(doc, Y.encodeStateAsUpdate(next));
       });
     } finally {
       await conn.disconnect();