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>
2026-06-24 14:23:40 +03:00
parent 3a91e0eca9
commit 5da12e89f9
29 changed files with 270 additions and 205 deletions
@@ -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 = {
@@ -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 });
@@ -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';
@@ -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);
@@ -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 = {
@@ -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);