refactor(git-sync): internalize the engine — first-class ESM, no vendoring bridge (#119 review)

Closes the architecture item from the #119 review: drop the "vendored from
docmost-sync" framing and the CJS↔ESM `Function('import()')` bridge so the engine
is a normal first-class gitmost package.

Part 1 — vendoring markers removed (prose only, zero behavior change): reworded
"VENDORED into gitmost" / "vendored from docmost-sync" / "Engine LOGIC is
byte-identical" / "it's a port" comments across the engine. Behavior-bearing
strings are untouched: BOT_AUTHOR_NAME/EMAIL and the `Docmost-Sync-Source:`
provenance trailers (changing them would break git authorship + the loop-guard).

Part 2 — the package is now ESM (matching the sibling @docmost/mcp): `type: module`,
tsconfig Node16, `.js` extensions on relative imports, and a static
`import { marked }` replacing the `new Function('return import(...)')` /
`loadMarked` hack — the bridge is GONE from the package. The CommonJS NestJS
server loads the now-ESM engine via a new `git-sync.loader.ts` that mirrors the
existing `docmost-client.loader.ts` mcp loader exactly (Function-indirected
dynamic import + cached promise + retry-on-reject). The 4 server consumers
(orchestrator/datasource/vault-registry/git-http-backend) call `await loadGitSync()`
for value exports; types stay `import type` (erased). The converter-gate spec —
which needs the real converter — loads the package's TS source via a jest
moduleNameMapper + isolatedModules (documented in that spec); the other git-sync
specs mock the loader.

Verified: engine builds pure ESM (no Function/require leftover), vitest 614,
editor-ext build, server + client tsc, full server jest 1397/0. Live stand
smoke-test: server starts clean on the ESM engine (no ERR_REQUIRE_ESM), a real
sync cycle runs through the loader, and the basic e2e suite is 12/12 (clone via
git-http-backend, push, pull, delete, 3-way merge — all through the new loader).

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

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

@@ -1,7 +1,17 @@
 /**
+ * JEST CONFIG NOTE (#119 ESM refactor): this is the one spec that needs the REAL
+ * `@docmost/git-sync` converter (not a mock). The package is now ESM, which jest
+ * cannot `require()` nor `import()` without --experimental-vm-modules, so the
+ * server jest config `moduleNameMapper`s `@docmost/git-sync` to its TS SOURCE and
+ * strips the ESM `.js` import suffixes. ts-jest then type-checks that source under
+ * the server's (looser) tsconfig and trips a benign narrowing; the global
+ * `isolatedModules: true` on the ts-jest transform (apps/server/package.json)
+ * makes it transpile-only so this spec loads. Full type-checking of the package
+ * is still enforced by its own `tsc`/vitest gates and the server `tsc --noEmit`.
+ *
  * §13.1 IDEMPOTENCY GATE — the blocking gate for git-sync Phase B.
  *
- * Proves the vendored `@docmost/git-sync` pure converter is schema-compatible
+ * Proves the `@docmost/git-sync` pure converter is schema-compatible
  * with the server's REAL editor-ext document schema: a representative corpus of
  * editor-ext ProseMirror documents must survive a full round trip through the
  * actual server write path without losing any node / mark / attribute.
@@ -19,7 +29,7 @@
  *      validation that runs on a git-sync write (plan §3.3).
  *   4. assert docsCanonicallyEqual(canon(original), canon(normalized)) === true
  *
- * Any node / mark / attr that editor-ext drops (because the vendored
+ * Any node / mark / attr that editor-ext drops (because the git-sync
  * docmost-schema named it differently, or declares a different default) makes
  * the gate FAIL for that document — exactly the schema-divergence plan §3.3 /
  * §13.1 warn about. Genuine, irreducible divergences are isolated into the
@@ -31,9 +41,11 @@
  */
 import { TiptapTransformer } from '@hocuspocus/transformer';
 // Import the server's real schema FIRST so `@docmost/editor-ext` resolves to its
-// built CJS `dist` (its `main`). Importing the ESM `@docmost/git-sync` package
-// first flips jest's resolver to editor-ext's `module` (src) field, which then
-// drags in React node views (navigator-less) and breaks the node test env.
+// built CJS `dist` (its `main`). The ESM-only `@docmost/git-sync` package is
+// mapped to its TS SOURCE by the jest `moduleNameMapper` (the built ESM cannot
+// be `require()`d nor dynamically `import()`ed under jest's node VM), so ts-jest
+// transpiles the real converter to CJS here — exercising the actual converter
+// the server ships, not a stub.
 import { tiptapExtensions } from './collaboration.util';
 import {
   convertProseMirrorToMarkdown,

apps/server/src/integrations/git-sync/git-sync.loader.ts

@@ -0,0 +1,59 @@
+import { pathToFileURL } from 'node:url';
+import type {
+  VaultGit as VaultGitClass,
+  vaultGitEnv as vaultGitEnvFn,
+  runCycle as runCycleFn,
+  parseDocmostMarkdown as parseDocmostMarkdownFn,
+  markdownToProseMirror as markdownToProseMirrorFn,
+} from '@docmost/git-sync';
+
+/**
+ * Runtime value-export surface of the ESM-only `@docmost/git-sync` package that
+ * the server consumes. Types are imported with `import type` (erased at compile,
+ * no runtime require); only the VALUE exports below need the dynamic-load
+ * treatment so a CJS `require()` of the ESM package never happens.
+ */
+interface GitSyncModule {
+  VaultGit: typeof VaultGitClass;
+  vaultGitEnv: typeof vaultGitEnvFn;
+  runCycle: typeof runCycleFn;
+  parseDocmostMarkdown: typeof parseDocmostMarkdownFn;
+  markdownToProseMirror: typeof markdownToProseMirrorFn;
+}
+
+// TS with module:commonjs downlevels a literal `import()` to `require()`, which
+// cannot load the ESM-only `@docmost/git-sync` package. Indirect through
+// Function so the real dynamic `import()` survives compilation and can load ESM
+// from CommonJS at runtime (same trick as
+// apps/server/src/core/ai-chat/tools/docmost-client.loader.ts and
+// integrations/mcp/mcp.service.ts).
+const esmImport = new Function(
+  'specifier',
+  'return import(specifier)',
+) as (specifier: string) => Promise<unknown>;
+
+// Memoize the in-flight/loaded module so the dynamic import runs at most once.
+let modulePromise: Promise<GitSyncModule> | null = null;
+
+/**
+ * Lazily load the ESM-only `@docmost/git-sync` package (cached). Resolves the
+ * package entry to an absolute path, then imports it as a `file://` URL so the
+ * package "exports" map is honoured without bare-specifier resolution-base
+ * fragility.
+ */
+export async function loadGitSync(): Promise<GitSyncModule> {
+  if (!modulePromise) {
+    modulePromise = (async () => {
+      const entry = require.resolve('@docmost/git-sync');
+      const mod = (await esmImport(
+        pathToFileURL(entry).href,
+      )) as GitSyncModule;
+      return mod;
+    })().catch((err) => {
+      // Do not cache a rejected import — allow the next call to retry.
+      modulePromise = null;
+      throw err;
+    });
+  }
+  return modulePromise;
+}

apps/server/src/integrations/git-sync/http/git-http-backend.service.spec.ts

@@ -8,9 +8,13 @@ import { spawn } from 'node:child_process';
 // fake child lets us drive every stdout/stderr/error/close branch by hand.
 jest.mock('node:child_process', () => ({ spawn: jest.fn() }));
 // vaultGitEnv just builds the CGI env overlay; stub it to a passthrough so the
-// service constructs without the real engine.
-jest.mock('@docmost/git-sync', () => ({
-  vaultGitEnv: (overlay: Record<string, string>) => overlay,
+// service runs without the real engine. The service loads it at runtime via the
+// `loadGitSync()` bridge (the ESM `@docmost/git-sync` package cannot be
+// `require()`d under jest), so we mock that loader rather than the package.
+jest.mock('../git-sync.loader', () => ({
+  loadGitSync: jest.fn(async () => ({
+    vaultGitEnv: (overlay: Record<string, string>) => overlay,
+  })),
 }));
 
 import {
@@ -81,6 +85,12 @@ function buildService() {
   return new GitHttpBackendService(env as any);
 }
 
+// `run()` now awaits the async `loadGitSync()` bridge before it spawns the
+// child, so the spawn (and its stream-handler wiring) happens one microtask
+// after `run()` is called. These tests drive the fake child synchronously, so
+// flush the microtask queue first to let `run()` reach the spawn.
+const flush = () => new Promise((resolve) => setImmediate(resolve));
+
 describe('GitHttpBackendService.run', () => {
   beforeEach(() => {
     spawnMock.mockReset();
@@ -96,6 +106,7 @@ describe('GitHttpBackendService.run', () => {
     const res = fakeRes();
 
     const p = service.run(baseRequest, fakeReq(), res);
+    await flush();
     // Emit a child 'error' before any stdout -> 500, headers not already sent.
     child.emit('error', new Error('ENOENT spawn git'));
     await p;
@@ -112,6 +123,7 @@ describe('GitHttpBackendService.run', () => {
     const res = fakeRes();
 
     const p = service.run(baseRequest, fakeReq(), res);
+    await flush();
     // stderr diagnostics, then a close with no valid CGI output -> 500.
     child.stderr.emit('data', Buffer.from('fatal: boom'));
     child.emit('close', 128);
@@ -128,6 +140,7 @@ describe('GitHttpBackendService.run', () => {
     const res = fakeRes();
 
     const p = service.run(baseRequest, fakeReq(), res);
+    await flush();
     // A full CGI response: status line + header + blank line + body.
     child.stdout.emit(
       'data',
@@ -157,6 +170,7 @@ describe('GitHttpBackendService.run', () => {
     const warnSpy = jest.spyOn(Logger.prototype, 'warn');
 
     const p = service.run(baseRequest, fakeReq(), res);
+    await flush();
     // The stdout 'error' handler must absorb this — no unhandled throw, no 500.
     expect(() => child.stdout.emit('error', new Error('EPIPE'))).not.toThrow();
     expect(() => child.stderr.emit('error', new Error('EPIPE'))).not.toThrow();

apps/server/src/integrations/git-sync/http/git-http-backend.service.ts

@@ -1,7 +1,7 @@
 import { Injectable, Logger } from '@nestjs/common';
 import { spawn } from 'node:child_process';
 import type { IncomingMessage, ServerResponse } from 'node:http';
-import { vaultGitEnv } from '@docmost/git-sync';
+import { loadGitSync } from '../git-sync.loader';
 import { EnvironmentService } from '../../environment/environment.service';
 
 /** The parsed first part of a CGI response: the HTTP status + header pairs. */
@@ -152,6 +152,7 @@ export class GitHttpBackendService {
     rawReq: IncomingMessage,
     rawRes: ServerResponse,
   ): Promise<void> {
+    const { vaultGitEnv } = await loadGitSync();
     const projectRoot = this.environmentService.getGitSyncDataDir();
     // Build the CGI env from the engine's cwd-isolated base (strips GIT_DIR /
     // GIT_WORK_TREE), then layer the http-backend CGI variables. PATH is

apps/server/src/integrations/git-sync/services/git-sync.orchestrator.spec.ts

@@ -1,4 +1,4 @@
-// Unit tests for the git-sync control plane. The vendored engine's `runCycle`
+// Unit tests for the git-sync control plane. The engine's `runCycle`
 // (which owns the PULL->PUSH branch choreography) is mocked so we exercise ONLY
 // the orchestrator's wiring: gating, the Redis leader lock + in-process mutex
 // (via SpaceLockService), the delete-cap POLICY it injects as `resolveApplyClient`,
@@ -7,13 +7,18 @@
 // mechanics themselves are covered by the engine's own cycle round-trip spec.
 //
 // The engine mock must be declared before importing the orchestrator so the
-// module-graph import binds to the mocked function.
-jest.mock('@docmost/git-sync', () => ({
-  runCycle: jest.fn(),
+// runtime `loadGitSync()` bridge resolves to the mocked `runCycle` (the ESM
+// `@docmost/git-sync` package cannot be `require()`d under jest). The `mock`
+// prefix lets the hoisted factory reference it.
+const mockRunCycle = jest.fn();
+
+jest.mock('../git-sync.loader', () => ({
+  loadGitSync: jest.fn(async () => ({
+    runCycle: mockRunCycle,
+  })),
 }));
 
 import { Logger } from '@nestjs/common';
-import { runCycle } from '@docmost/git-sync';
 import {
   GitSyncOrchestrator,
   GitSyncLockHeldError,
@@ -22,7 +27,7 @@ import { SpaceLockService } from './space-lock.service';
 
 type AnyMock = jest.Mock;
 
-const runCycleMock = runCycle as unknown as AnyMock;
+const runCycleMock = mockRunCycle as unknown as AnyMock;
 
 /** The default happy-path cycle result the engine returns. */
 const OK_CYCLE = {

apps/server/src/integrations/git-sync/services/git-sync.orchestrator.ts

@@ -9,7 +9,8 @@ import { mkdir, readFile, rm, writeFile } from 'node:fs/promises';
 import { InjectKysely } from 'nestjs-kysely';
 import { KyselyDB } from '@docmost/db/types/kysely.types';
 import { sql } from 'kysely';
-import { type Settings, runCycle } from '@docmost/git-sync';
+import type { Settings } from '@docmost/git-sync';
+import { loadGitSync } from '../git-sync.loader';
 import { EnvironmentService } from '../../environment/environment.service';
 import { GitmostDataSourceService } from './gitmost-datasource.service';
 import { VaultRegistryService } from './vault-registry.service';
@@ -246,6 +247,7 @@ export class GitSyncOrchestrator implements OnModuleInit, OnModuleDestroy {
     workspaceId: string,
     serviceUserId: string,
   ): Promise<GitSyncRunStatus> {
+    const { runCycle } = await loadGitSync();
     const settings = this.buildSettings(spaceId);
     const vault = await this.vaultRegistry.getVault(spaceId);
     const client = this.dataSource.bind({ workspaceId, userId: serviceUserId });

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

@@ -30,6 +30,21 @@ jest.mock('@hocuspocus/transformer', () => {
 jest.mock('@docmost/editor-ext', () => ({
   markdownToHtml: jest.fn(),
 }));
+// The service loads `parseDocmostMarkdown` / `markdownToProseMirror` at runtime
+// via the `loadGitSync()` bridge (the ESM `@docmost/git-sync` package cannot be
+// `require()`d under jest). Stub the loader: the real conversion is exercised by
+// the @docmost/git-sync converter tests and the converter gate; here the mocked
+// TiptapTransformer.toYdoc ignores the converted doc anyway, so a passthrough
+// body + a minimal ProseMirror doc is sufficient.
+jest.mock('../git-sync.loader', () => ({
+  loadGitSync: jest.fn(async () => ({
+    parseDocmostMarkdown: (md: string) => ({ meta: {}, body: md }),
+    markdownToProseMirror: async () => ({
+      type: 'doc',
+      content: [{ type: 'paragraph' }],
+    }),
+  })),
+}));
 
 import * as Y from 'yjs';
 import { GitmostDataSourceService } from './gitmost-datasource.service';

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

@@ -1,12 +1,11 @@
 import { Injectable, Logger, NotFoundException } from '@nestjs/common';
 import { TiptapTransformer } from '@hocuspocus/transformer';
 import { generateJitteredKeyBetween } from 'fractional-indexing-jittered';
-import {
-  type GitSyncClient,
-  type GitSyncPageNodeLite,
-  parseDocmostMarkdown,
-  markdownToProseMirror,
+import type {
+  GitSyncClient,
+  GitSyncPageNodeLite,
 } from '@docmost/git-sync';
+import { loadGitSync } from '../git-sync.loader';
 import { PageRepo } from '@docmost/db/repos/page/page.repo';
 import { SpaceRepo } from '@docmost/db/repos/space/space.repo';
 import { InjectKysely } from 'nestjs-kysely';
@@ -177,6 +176,7 @@ export class GitmostDataSourceService {
     fullMarkdown: string,
     baseMarkdown?: string | null,
   ): Promise<{ updatedAt?: string }> {
+    const { parseDocmostMarkdown, markdownToProseMirror } = await loadGitSync();
     const { body } = parseDocmostMarkdown(fullMarkdown);
     const doc = await markdownToProseMirror(body);
 
@@ -213,6 +213,7 @@ export class GitmostDataSourceService {
     );
 
     // The shell is created without body; push the markdown body through collab.
+    const { parseDocmostMarkdown, markdownToProseMirror } = await loadGitSync();
     const { body } = parseDocmostMarkdown(content);
     const doc = await markdownToProseMirror(body);
     await this.writeBody(page.id, doc, ctx.userId);

apps/server/src/integrations/git-sync/services/vault-registry.service.spec.ts

@@ -1,17 +1,30 @@
 // Unit tests for the per-space vault path resolver + lazy VaultGit cache
-// `mkdir` and `VaultGit` are mocked so construction is cheap and
+// `mkdir` and the git-sync loader are mocked so construction is cheap and
 // no real filesystem / git work happens. We assert the path normalization
 // (trailing slash) and the one-VaultGit-per-space caching contract.
+//
+// The service loads `VaultGit` (and `vaultGitEnv`) at runtime via the
+// `loadGitSync()` bridge (the ESM `@docmost/git-sync` package cannot be
+// `require()`d under jest), so we mock that loader rather than the package.
 import { mkdir } from 'node:fs/promises';
-import { VaultGit } from '@docmost/git-sync';
+import { loadGitSync } from '../git-sync.loader';
 
 jest.mock('node:fs/promises', () => ({
   mkdir: jest.fn(async () => undefined),
 }));
 
 // Cheap VaultGit stub: records the path it was constructed with; no shell-out.
-jest.mock('@docmost/git-sync', () => ({
-  VaultGit: jest.fn().mockImplementation((path: string) => ({ path })),
+// Declared with a `mock`-prefixed name so jest allows referencing it inside the
+// hoisted `jest.mock` factory below.
+const mockVaultGit = jest
+  .fn()
+  .mockImplementation((path: string) => ({ path }));
+
+jest.mock('../git-sync.loader', () => ({
+  loadGitSync: jest.fn(async () => ({
+    VaultGit: mockVaultGit,
+    vaultGitEnv: jest.fn(() => ({})),
+  })),
 }));
 
 import { VaultRegistryService } from './vault-registry.service';
@@ -19,7 +32,8 @@ import { VaultRegistryService } from './vault-registry.service';
 type AnyMock = jest.Mock;
 
 const mkdirMock = mkdir as unknown as AnyMock;
-const VaultGitMock = VaultGit as unknown as AnyMock;
+const VaultGitMock = mockVaultGit;
+void loadGitSync;
 
 function build(dataDir: string): { service: VaultRegistryService } {
   const env = {

apps/server/src/integrations/git-sync/services/vault-registry.service.ts

@@ -2,7 +2,8 @@ import { Injectable, Logger } from '@nestjs/common';
 import { mkdir } from 'node:fs/promises';
 import { execFile } from 'node:child_process';
 import { promisify } from 'node:util';
-import { VaultGit, vaultGitEnv } from '@docmost/git-sync';
+import type { VaultGit } from '@docmost/git-sync';
+import { loadGitSync } from '../git-sync.loader';
 import { EnvironmentService } from '../../environment/environment.service';
 
 const execFileAsync = promisify(execFile);
@@ -41,6 +42,7 @@ export class VaultRegistryService {
 
     const path = this.vaultPath(spaceId);
     await mkdir(path, { recursive: true });
+    const { VaultGit } = await loadGitSync();
     const vault = new VaultGit(path);
     this.vaults.set(spaceId, vault);
     return vault;
@@ -66,6 +68,7 @@ export class VaultRegistryService {
    * every request.
    */
   async ensureServable(spaceId: string): Promise<string> {
+    const { vaultGitEnv } = await loadGitSync();
     const vault = await this.getVault(spaceId);
     const path = this.vaultPath(spaceId);