fix(git-sync): address PR #119 review (#1571)

Resolve the code-review findings from comment #1571 on PR #119.

Engine (packages/git-sync):
- Idempotent CREATE on retry: before createPage, look the page up in the
  live Docmost tree by (parentPageId, title) and ADOPT it instead of
  duplicating when a prior cycle created it but failed to persist the
  pageId back to disk. Only trust a COMPLETE tree for the lookup; fall
  back to createPage otherwise. Covered by new tests incl. a complete=false
  regression-lock.
- Route applyPullActions diagnostics through an injected logger instead of
  bare console (thread log from the cycle).
- Add a timeout to the git execFile chokepoint (runRaw) so a hung git
  subprocess cannot wedge a sync cycle.
- Translate remaining Russian code comments to English.
- Remove dead standalone-CLI code (parseArgs/PushParsedArgs,
  parseSettings/envSchema, loadSettingsOrExit + config-errors.ts) and the
  matching index exports/specs; keep the Settings type.
- Fix the dangling docs link in package.json.
- Add a schema-surface snapshot guard so any drift in the vendored
  document schema is a loud, must-review CI failure (+ provenance header).

Server (apps/server):
- Add a configurable watchdog timeout to the spawned git http-backend so a
  stalled push cannot hold the per-space lock forever
  (GIT_SYNC_BACKEND_TIMEOUT_MS).
- Close the in-process TOCTOU window in SpaceLockService.withSpaceLock by
  reserving the slot synchronously before acquire.
- Add tests: removePage git-sync provenance (both branches), ensureServable
  force-push-protection git configs, and the phase-B+ datasource methods.

Docs / build:
- AGENTS.md: list git-sync as the fifth workspace package and note the
  three schema mirrors; fix the dangling git-sync-plan.md backlog link.
- pnpm-lock.yaml: add the missing @docmost/git-sync workspace link so
  pnpm install --frozen-lockfile (CI default) succeeds.

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

packages/git-sync/test/apply-pull-actions.test.ts

@@ -99,17 +99,25 @@ function makeFs(opts?: { failWriteFor?: Set<string> }) {
   return { fs, writes, mkdirs, rms };
 }
 
+// A single injected `log` spy mirrors the push side: applyPullActions now routes
+// EVERY cycle diagnostic through `deps.log` (one channel), so tests inspect this
+// spy instead of console.warn/console.error. `deps()` creates a fresh spy per
+// call and stashes it on `lastLog` for the current test to assert against.
+let lastLog: ReturnType<typeof vi.fn>;
+
 function deps(
   client: any,
   git: any,
   fs: ReturnType<typeof makeFs>,
 ): ApplyPullActionsDeps {
+  lastLog = vi.fn();
   return {
     client,
     git,
     writeFile: fs.fs.writeFile,
     mkdir: fs.fs.mkdir,
     rm: fs.fs.rm,
+    log: lastLog,
   };
 }
 
@@ -341,7 +349,7 @@ describe('applyPullActions — deletion suppression (SPEC §8)', () => {
     // Subject reflects 0 deleted (no ", N deleted" suffix).
     expect(g.committedSubject).toBe('docmost: sync 1 page(s)');
     // The suppression warning was emitted.
-    expect(console.warn).toHaveBeenCalledWith(
+    expect(lastLog).toHaveBeenCalledWith(
       expect.stringMatching(/tree fetch incomplete/),
     );
   });
@@ -472,10 +480,10 @@ describe('applyPullActions — suppression warning forks (empty-live / mass-dele
     expect(fs.rms).toEqual([]);
     // The empty-live message names the tracked-file count and "deletions
     // suppressed".
-    expect(console.warn).toHaveBeenCalledWith(
+    expect(lastLog).toHaveBeenCalledWith(
       expect.stringMatching(/live fetch returned 0 pages but 4 file\(s\) are tracked/),
     );
-    expect(console.warn).toHaveBeenCalledWith(
+    expect(lastLog).toHaveBeenCalledWith(
       expect.stringMatching(/deletions suppressed/),
     );
   });
@@ -502,10 +510,10 @@ describe('applyPullActions — suppression warning forks (empty-live / mass-dele
 
     expect(res.deleted).toBe(0);
     expect(fs.rms).toEqual([]);
-    expect(console.warn).toHaveBeenCalledWith(
+    expect(lastLog).toHaveBeenCalledWith(
       expect.stringMatching(/plan would delete 5 of 6 tracked file\(s\) \(mass-delete guard\)/),
     );
-    expect(console.warn).toHaveBeenCalledWith(
+    expect(lastLog).toHaveBeenCalledWith(
       expect.stringMatching(/deletions suppressed/),
     );
   });
@@ -513,8 +521,8 @@ describe('applyPullActions — suppression warning forks (empty-live / mass-dele
 
 describe('applyPullActions — removePath fault tolerance (rm REJECTS)', () => {
   it('does NOT reject, logs the failure, and does not count the failed removal', async () => {
-    // pull.ts 354-364: when `deps.rm` throws, removePath logs via console.error
-    // and returns false; the run continues. Existing delete tests use an rm
+    // pull.ts removePath catch: when `deps.rm` throws, it logs via the injected
+    // `log` and returns false; the run continues. Existing delete tests use an rm
     // that always succeeds, leaving this catch branch uncovered.
     const { client } = makeClient();
     const g = makeGit();
@@ -535,9 +543,8 @@ describe('applyPullActions — removePath fault tolerance (rm REJECTS)', () => {
     // Resolved (not rejected) — the pull is fault-tolerant.
     expect(res.deleted).toBe(0);
     // removePath's catch logs "pull: failed to delete Dead.md: ...".
-    expect(console.error).toHaveBeenCalledWith(
+    expect(lastLog).toHaveBeenCalledWith(
       expect.stringMatching(/failed to .* Dead\.md/),
-      expect.anything(),
     );
     // The (would-be) removal never succeeded, so nothing was recorded.
     expect(fs.rms).toEqual([]);
@@ -567,12 +574,13 @@ describe('applyPullActions — removePath fault tolerance (rm REJECTS)', () => {
     expect(res.deleted).toBe(2);
     expect(fs.rms).toEqual(['/vault/Dead1.md', '/vault/Dead3.md']);
     expect(g.committedSubject).toBe('docmost: sync 0 page(s), 2 deleted');
-    // Exactly one rejection was logged (Dead2.md).
-    expect(console.error).toHaveBeenCalledTimes(1);
-    expect(console.error).toHaveBeenCalledWith(
-      expect.stringMatching(/failed to .* Dead2\.md/),
-      expect.anything(),
-    );
+    // Exactly one rejection was logged (Dead2.md). Other diagnostics share the
+    // `log` channel, so count ONLY the "failed to ..." failure lines.
+    const failLines = lastLog.mock.calls
+      .map((c: unknown[]) => String(c[0]))
+      .filter((m: string) => /failed to /.test(m));
+    expect(failLines.length).toBe(1);
+    expect(failLines[0]).toMatch(/failed to .* Dead2\.md/);
     // The run still reached commit + checkout + merge.
     expect(g.order).toEqual([
       'stageAll',
@@ -620,17 +628,16 @@ describe('applyPullActions — move old-path removal rejects vs move-write fails
     expect(fs.rms).toEqual(['/vault/Dead.md']); // Old/M.md rm threw, not recorded
     expect(g.committedSubject).toBe('docmost: sync 1 page(s), 1 deleted');
     // The failure log named the moved old path.
-    expect(console.error).toHaveBeenCalledWith(
+    expect(lastLog).toHaveBeenCalledWith(
       expect.stringMatching(/failed to .* Old\/M\.md/),
-      expect.anything(),
     );
   });
 
   it('a move-write FAILURE keeps the old path: rm is never attempted for it (data-loss guard, 374-383)', async () => {
     // Distinct branch from the move-old-path rm rejection above: here the
     // new-path WRITE itself fails, so `m` enters failedPageIds and the move
-    // loop short-circuits at line 376 BEFORE calling rm — emitting a
-    // console.warn and PRESERVING the old path (the only copy).
+    // loop short-circuits BEFORE calling rm — emitting a warning via the
+    // injected `log` and PRESERVING the old path (the only copy).
     const { client } = makeClient();
     const g = makeGit();
     const fs = makeFs({ failWriteFor: new Set(['/vault/New/M.md']) });
@@ -661,7 +668,7 @@ describe('applyPullActions — move old-path removal rejects vs move-write fails
     expect(fs.fs.rm).not.toHaveBeenCalledWith('/vault/Old/M.md');
     expect(fs.rms).toEqual([]);
     // The "keeping old path" warning fired exactly once for `m`.
-    const warnCalls = (console.warn as unknown as ReturnType<typeof vi.fn>).mock.calls
+    const warnCalls = lastLog.mock.calls
       .map((c: unknown[]) => String(c[0]))
       .filter((m: string) => m.includes('move write for m failed'));
     expect(warnCalls.length).toBe(1);

packages/git-sync/test/apply-push-actions.test.ts

@@ -18,6 +18,12 @@ const SPACE_ID = 'sp-test';
 /** A recording client fake; createPage returns a configurable assigned id. */
 function makeClient(opts?: { createId?: string }) {
   const client = {
+    // Empty live tree by default -> creates take the normal createPage path; the
+    // retry-adopt lookup only fires when a (parentPageId, title) node matches.
+    listSpaceTree: vi.fn(async () => ({
+      pages: [] as { id: string; parentPageId?: string | null; title?: string }[],
+      complete: true,
+    })),
     importPageMarkdown: vi.fn(async (_pageId: string, _md: string) => ({
       success: true,
     })),
@@ -227,6 +233,143 @@ describe('applyPushActions — create (assigned pageId written back to meta)', (
   });
 });
 
+describe('applyPushActions — create RETRY-ADOPT idempotency (#1)', () => {
+  // Create is NOT atomic with the pageId write-back: if a prior cycle created the
+  // page in Docmost but died before persisting the id back, the file is re-seen as
+  // a CREATE. The applier must ADOPT the existing page (write the id back + push the
+  // body as an idempotent UPDATE) instead of calling createPage again (which would
+  // duplicate the page). The live page is matched by (parentPageId, title).
+  it('ADOPTS an existing page (no createPage) when the live tree already has a match', async () => {
+    const client = makeClient({ createId: 'should-not-be-used' });
+    // The live Docmost tree already has the page this create targets:
+    //   title "My New Page" under the parent folder's page `parent-9`.
+    client.listSpaceTree.mockResolvedValue({
+      pages: [
+        { id: 'parent-9', parentPageId: null, title: 'Parent' },
+        { id: 'already-created-7', parentPageId: 'parent-9', title: 'My New Page' },
+      ],
+      complete: true,
+    });
+    const { git } = makeGit();
+    const fs = makeFs({
+      'Parent/My New Page.md': '# My New Page\n\nbody text\n',
+      'Parent/Parent.md': fileFor('parent-9'),
+    });
+
+    const res = await applyPushActions(
+      deps(client, git, fs),
+      actions({ creates: [{ path: 'Parent/My New Page.md' }] }),
+    );
+
+    expect(res.created).toBe(1);
+    // CRITICAL: createPage was NOT called — no duplicate page in Docmost.
+    expect(client.createPage).not.toHaveBeenCalled();
+    // The body was pushed as an UPDATE targeting the EXISTING id (idempotent).
+    expect(client.importPageMarkdown).toHaveBeenCalledTimes(1);
+    expect(client.importPageMarkdown).toHaveBeenCalledWith(
+      'already-created-7',
+      expect.stringContaining('body text'),
+      null,
+    );
+
+    // The file was rewritten with the EXISTING id as gitmost_id (now tracked).
+    expect(fs.writes.map((w) => w.path)).toEqual(['Parent/My New Page.md']);
+    const rewritten = fs.store['Parent/My New Page.md'];
+    expect(parsePageFile(rewritten).id).toBe('already-created-7');
+    expect(res.writtenBack).toEqual([
+      { path: 'Parent/My New Page.md', pageId: 'already-created-7' },
+    ]);
+  });
+
+  it('does NOT adopt from an INCOMPLETE tree even when a node matches (falls back to createPage)', async () => {
+    // Defensive guard: retry-adopt is only safe from a COMPLETE live tree. A
+    // TRUNCATED tree (complete:false) could miss an already-created page and let
+    // us duplicate it — the very thing adopt prevents. So on an incomplete tree
+    // the map is NOT built and we MUST fall back to the normal createPage path,
+    // even though this particular tree happens to carry a matching node.
+    const client = makeClient({ createId: 'page-new-55' });
+    // A node that WOULD match the create's (parentPageId 'parent-9', title
+    // 'My New Page') — but the tree is flagged incomplete, so it must be ignored.
+    client.listSpaceTree.mockResolvedValue({
+      pages: [
+        { id: 'parent-9', parentPageId: null, title: 'Parent' },
+        { id: 'already-created-7', parentPageId: 'parent-9', title: 'My New Page' },
+      ],
+      complete: false,
+    });
+    const { git } = makeGit();
+    const fs = makeFs({
+      'Parent/My New Page.md': '# My New Page\n\nbody text\n',
+      'Parent/Parent.md': fileFor('parent-9'),
+    });
+
+    const res = await applyPushActions(
+      deps(client, git, fs),
+      actions({ creates: [{ path: 'Parent/My New Page.md' }] }),
+    );
+
+    expect(res.created).toBe(1);
+    // CRITICAL: createPage ran (normal path) — adopt was suppressed by complete:false.
+    expect(client.createPage).toHaveBeenCalledTimes(1);
+    // No adopt-UPDATE happened: the matching node was NOT trusted.
+    expect(client.importPageMarkdown).not.toHaveBeenCalled();
+    // The file carries the NEWLY assigned id, not the would-be adopted one.
+    expect(parsePageFile(fs.store['Parent/My New Page.md']).id).toBe('page-new-55');
+    expect(res.writtenBack).toEqual([
+      { path: 'Parent/My New Page.md', pageId: 'page-new-55' },
+    ]);
+  });
+
+  it('a NORMAL create (empty live tree) STILL calls createPage', async () => {
+    // No matching live node -> the happy path: createPage runs, no adopt.
+    const client = makeClient({ createId: 'page-new-99' });
+    // makeClient's listSpaceTree returns an empty tree by default.
+    const { git } = makeGit();
+    const fs = makeFs({
+      'Parent/My New Page.md': '# My New Page\n\nbody text\n',
+      'Parent/Parent.md': fileFor('parent-9'),
+    });
+
+    const res = await applyPushActions(
+      deps(client, git, fs),
+      actions({ creates: [{ path: 'Parent/My New Page.md' }] }),
+    );
+
+    expect(res.created).toBe(1);
+    expect(client.createPage).toHaveBeenCalledTimes(1);
+    // No adopt-UPDATE happened (importPageMarkdown is the update path).
+    expect(client.importPageMarkdown).not.toHaveBeenCalled();
+    expect(parsePageFile(fs.store['Parent/My New Page.md']).id).toBe('page-new-99');
+  });
+
+  it('a thrown adopt is isolated as a `create` failure (per-page isolation, SPEC §12)', async () => {
+    const client = makeClient({ createId: 'unused' });
+    client.listSpaceTree.mockResolvedValue({
+      pages: [{ id: 'existing-1', parentPageId: null, title: 'Doc' }],
+      complete: true,
+    });
+    // The adopt pushes the body as an UPDATE; make that throw.
+    client.importPageMarkdown.mockRejectedValue(new Error('adopt boom'));
+    const { git, updateRefCalls } = makeGit();
+    const fs = makeFs({ 'Doc.md': '# Doc\n\nbody\n' });
+
+    const res = await applyPushActions(
+      deps(client, git, fs),
+      actions({ creates: [{ path: 'Doc.md' }] }),
+      'sha-adopt-fail',
+    );
+
+    expect(res.created).toBe(0);
+    expect(client.createPage).not.toHaveBeenCalled();
+    expect(res.failures).toEqual([
+      { kind: 'create', path: 'Doc.md', error: 'adopt boom' },
+    ]);
+    // A failure means the refs are NOT advanced (re-run retries cleanly, §12).
+    expect(res.lastPushedAdvanced).toBe(false);
+    expect(updateRefCalls).toEqual([]);
+  });
+});
+
 describe('applyPushActions — delete (soft-delete to Trash, SPEC §8)', () => {
   it('calls deletePage(pageId)', async () => {
     const client = makeClient();

packages/git-sync/test/config-errors-invalid.test.ts

@@ -1,139 +0,0 @@
-import { afterEach, describe, expect, it, vi } from 'vitest';
-import { z, ZodError } from 'zod';
-import { loadSettingsOrExit } from '../src/engine/config-errors';
-import { envSchema } from '../src/engine/settings';
-
-// Companion to test/config-errors.test.ts. That file covers the success path,
-// the MISSING-required (undefined -> invalid_type) -> exit branch, and the
-// non-ZodError passthrough. This file fills the remaining GAP: the
-// INVALID-VALUE branch (config-errors.ts lines ~20, 27-30). A ZodError whose
-// issue is a CONSTRAINT violation (bad URL, bad enum, too-short string) is NOT
-// a missing key, so it must be routed into the `invalid` bucket and reported
-// under the "Invalid value(s)" heading with a `<name>: <message>` line — a
-// distinct, operator-facing message from the missing-variable case.
-describe('loadSettingsOrExit — invalid-value branch', () => {
-  afterEach(() => {
-    vi.restoreAllMocks();
-  });
-
-  // Stub process.exit so it throws (control stops at the exit point without
-  // killing the runner) and capture everything written to stderr. Mirrors the
-  // approach in the existing config-errors.test.ts.
-  function stubExitAndStderr() {
-    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((
-      code?: number,
-    ) => {
-      throw new Error(`exit:${code}`);
-    }) as never);
-    const writeSpy = vi
-      .spyOn(process.stderr, 'write')
-      .mockImplementation(() => true);
-    const written = () => writeSpy.mock.calls.map((c) => String(c[0])).join('');
-    return { exitSpy, writeSpy, written };
-  }
-
-  it('exits(1) and reports an invalid value (bad URL) under "Invalid value(s)"', () => {
-    const { exitSpy, written } = stubExitAndStderr();
-
-    // A present-but-invalid DOCMOST_API_URL: the value exists (so it is NOT a
-    // missing-key issue), but fails the .url() constraint -> goes to `invalid`.
-    expect(() =>
-      loadSettingsOrExit(() =>
-        envSchema.parse({
-          DOCMOST_API_URL: 'not-a-url',
-          DOCMOST_EMAIL: 'ops@example.com',
-          DOCMOST_PASSWORD: 'hunter2',
-          DOCMOST_SPACE_ID: 'space-1',
-        }),
-      ),
-    ).toThrow('exit:1');
-
-    expect(exitSpy).toHaveBeenCalledWith(1);
-    const out = written();
-    // The invalid-value heading must appear...
-    expect(out).toContain('Invalid value(s)');
-    // ...and it must name the offending variable on a `<name>: <message>` line.
-    expect(out).toContain('DOCMOST_API_URL:');
-    // The header line is always present.
-    expect(out).toContain('Configuration error in environment / .env:');
-    // It must NOT misreport an invalid value as a missing one.
-    expect(out).not.toContain('Missing required variable(s)');
-  });
-
-  it('exits(1) and reports an invalid enum value (LOG_LEVEL)', () => {
-    const { exitSpy, written } = stubExitAndStderr();
-
-    // All required vars present and valid; only LOG_LEVEL violates the enum.
-    expect(() =>
-      loadSettingsOrExit(() =>
-        envSchema.parse({
-          DOCMOST_API_URL: 'https://docs.example.com/api',
-          DOCMOST_EMAIL: 'ops@example.com',
-          DOCMOST_PASSWORD: 'hunter2',
-          DOCMOST_SPACE_ID: 'space-1',
-          LOG_LEVEL: 'verbose', // not in ['debug','info','warn','error']
-        }),
-      ),
-    ).toThrow('exit:1');
-
-    expect(exitSpy).toHaveBeenCalledWith(1);
-    const out = written();
-    expect(out).toContain('Invalid value(s)');
-    expect(out).toContain('LOG_LEVEL:');
-    expect(out).not.toContain('Missing required variable(s)');
-  });
-
-  it('routes a hand-built constraint-violation ZodError into the invalid bucket', () => {
-    const { exitSpy, written } = stubExitAndStderr();
-
-    // Construct the ZodError directly from a min-length violation so the test
-    // does not depend on the project schema's exact field set. The issue has a
-    // non-empty path (so a variable name is printed) and code "too_small"
-    // (NOT invalid_type/undefined), so config-errors.ts classifies it as
-    // invalid rather than missing.
-    const zerr = new ZodError([
-      {
-        code: 'too_small',
-        minimum: 1,
-        type: 'string',
-        inclusive: true,
-        path: ['DOCMOST_PASSWORD'],
-        message: 'String must contain at least 1 character(s)',
-      } as z.ZodIssue,
-    ]);
-
-    expect(() =>
-      loadSettingsOrExit(() => {
-        throw zerr;
-      }),
-    ).toThrow('exit:1');
-
-    expect(exitSpy).toHaveBeenCalledWith(1);
-    const out = written();
-    expect(out).toContain('Invalid value(s)');
-    expect(out).toContain('DOCMOST_PASSWORD: String must contain at least 1');
-    expect(out).not.toContain('Missing required variable(s)');
-  });
-
-  it('reports missing AND invalid in their own sections when both occur', () => {
-    const { exitSpy, written } = stubExitAndStderr();
-
-    // DOCMOST_API_URL present but invalid (-> invalid section); the three other
-    // required vars absent (-> missing section). Confirms the two branches are
-    // populated and emitted independently.
-    expect(() =>
-      loadSettingsOrExit(() =>
-        envSchema.parse({
-          DOCMOST_API_URL: 'not-a-url',
-        }),
-      ),
-    ).toThrow('exit:1');
-
-    expect(exitSpy).toHaveBeenCalledWith(1);
-    const out = written();
-    expect(out).toContain('Missing required variable(s)');
-    expect(out).toContain('Invalid value(s)');
-    expect(out).toContain('DOCMOST_API_URL:');
-    expect(out).toContain('DOCMOST_EMAIL');
-  });
-});

packages/git-sync/test/config-errors.test.ts

@@ -1,56 +0,0 @@
-import { afterEach, describe, expect, it, vi } from 'vitest';
-import { z } from 'zod';
-import { loadSettingsOrExit } from '../src/engine/config-errors';
-
-describe('loadSettingsOrExit', () => {
-  afterEach(() => {
-    vi.restoreAllMocks();
-  });
-
-  it('returns the factory value and does not exit on success', () => {
-    const exitSpy = vi
-      .spyOn(process, 'exit')
-      .mockImplementation((() => undefined) as never);
-
-    const result = loadSettingsOrExit(() => ({ ok: true }));
-
-    expect(result).toEqual({ ok: true });
-    expect(exitSpy).not.toHaveBeenCalled();
-  });
-
-  it('prints a named-variable message and exits(1) on a ZodError', () => {
-    // Mock process.exit to throw so control stops at the exit point, mirroring
-    // the real exit-the-process behaviour without killing the test runner.
-    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((
-      code?: number,
-    ) => {
-      throw new Error(`exit:${code}`);
-    }) as never);
-    const writeSpy = vi
-      .spyOn(process.stderr, 'write')
-      .mockImplementation(() => true);
-
-    expect(() =>
-      loadSettingsOrExit(() => z.object({ FOO: z.string() }).parse({})),
-    ).toThrow('exit:1');
-
-    expect(exitSpy).toHaveBeenCalledWith(1);
-    const written = writeSpy.mock.calls.map((c) => String(c[0])).join('');
-    expect(written).toContain('Missing required variable(s)');
-    expect(written).toContain('FOO');
-  });
-
-  it('propagates a non-ZodError without exiting', () => {
-    const exitSpy = vi
-      .spyOn(process, 'exit')
-      .mockImplementation((() => undefined) as never);
-    const boom = new Error('x');
-
-    expect(() =>
-      loadSettingsOrExit(() => {
-        throw boom;
-      }),
-    ).toThrow(boom);
-    expect(exitSpy).not.toHaveBeenCalled();
-  });
-});

packages/git-sync/test/engine-gaps.test.ts

@@ -263,6 +263,7 @@ describe('applyPushActions (push.ts) — move prefetch isolation', () => {
 
   function makeClient() {
     return {
+      listSpaceTree: vi.fn(async () => ({ pages: [], complete: true })),
       importPageMarkdown: vi.fn(async () => ({ updatedAt: 'u' })),
       createPage: vi.fn(async () => ({ data: { id: 'new-id' } })),
       deletePage: vi.fn(async () => ({})),

packages/git-sync/test/run-push-realgit.test.ts

@@ -47,6 +47,9 @@ function makeSettings(vaultPath: string): Settings {
 /** A recording client fake; createPage returns an assigned id + updatedAt. */
 function makeClientFake() {
   return {
+    // Empty live tree -> the create takes the normal createPage path (the
+    // retry-adopt lookup matches only on a live (parentPageId, title) node).
+    listSpaceTree: vi.fn(async () => ({ pages: [], complete: true })),
     importPageMarkdown: vi.fn(async () => ({
       data: { updatedAt: '2026-06-20T00:00:00.000Z' },
       success: true,

packages/git-sync/test/run-push.test.ts

@@ -114,6 +114,9 @@ function makeGit(opts?: {
 /** A recording client fake; createPage returns a configurable assigned id. */
 function makeClientFake(opts?: { createId?: string }) {
   return {
+    // Empty live tree by default -> no retry-adopt match, so creates take the
+    // normal createPage path (the adopt lookup only fires on a (parent,title) hit).
+    listSpaceTree: vi.fn(async () => ({ pages: [], complete: true })),
     importPageMarkdown: vi.fn(async () => ({ success: true })),
     createPage: vi.fn(async (title: string) => ({
       data: { id: opts?.createId ?? 'assigned-id', title },

packages/git-sync/test/schema-surface-snapshot.test.ts

@@ -0,0 +1,116 @@
+import { describe, it, expect } from "vitest";
+import { getSchema } from "@tiptap/core";
+
+import { docmostExtensions } from "../src/lib/docmost-schema.js";
+
+// SCHEMA-DRIFT GUARD (must-review gate).
+//
+// `src/lib/docmost-schema.ts` is a VENDORED MIRROR of the canonical Docmost
+// document schema defined in `@docmost/editor-ext`. git-sync uses it to convert
+// pages to/from ProseMirror JSON; any node, mark, or attribute that exists in
+// the canonical schema but is missing here is silently dropped on a round-trip
+// (data loss). The reverse — a node/mark/attr here that no longer exists in the
+// canonical schema — is dead surface that can mask drift.
+//
+// This test derives a stable, sorted "schema surface" (every node/mark name and
+// its sorted attribute keys) and pins it against an INLINE expected constant.
+// It is intentionally a LOUD must-review gate rather than an automatic
+// editor-ext diff: editor-ext's Tiptap representation differs from this
+// vendored copy, so a cross-representation compare would be fragile. We do NOT
+// use toMatchSnapshot so the reference lives in this file and is reviewed in the
+// diff of every change.
+//
+// WHEN THIS TEST FAILS: do NOT blindly update `expectedSurface`. First confirm
+// the change matches `@docmost/editor-ext` (the canonical schema) so the
+// markdown <-> ProseMirror round-trip stays lossless, THEN copy the new surface
+// into the expected constant below.
+
+interface SurfaceEntry {
+  name: string;
+  kind: "node" | "mark";
+  attrs: string[];
+}
+
+/** Derive the deterministic schema surface from the vendored extension set. */
+function deriveSurface(): SurfaceEntry[] {
+  const schema = getSchema(docmostExtensions as never);
+  const surface: SurfaceEntry[] = [];
+  for (const [name, type] of Object.entries(schema.nodes)) {
+    surface.push({
+      name,
+      kind: "node",
+      attrs: Object.keys((type as { spec?: { attrs?: object } }).spec?.attrs ?? {}).sort(),
+    });
+  }
+  for (const [name, type] of Object.entries(schema.marks)) {
+    surface.push({
+      name,
+      kind: "mark",
+      attrs: Object.keys((type as { spec?: { attrs?: object } }).spec?.attrs ?? {}).sort(),
+    });
+  }
+  // Sort by name, then by kind, for a representation-independent ordering.
+  surface.sort((a, b) =>
+    a.name === b.name ? a.kind.localeCompare(b.kind) : a.name.localeCompare(b.name),
+  );
+  return surface;
+}
+
+// The committed reference surface. Built from the ACTUAL current schema; review
+// every change to this constant against `@docmost/editor-ext`.
+const expectedSurface: SurfaceEntry[] = [
+  { name: "attachment", kind: "node", attrs: ["attachmentId", "mime", "name", "placeholder", "size", "url"] },
+  { name: "audio", kind: "node", attrs: ["attachmentId", "placeholder", "size", "src"] },
+  { name: "blockquote", kind: "node", attrs: [] },
+  { name: "bold", kind: "mark", attrs: [] },
+  { name: "bulletList", kind: "node", attrs: [] },
+  { name: "callout", kind: "node", attrs: ["icon", "type"] },
+  { name: "code", kind: "mark", attrs: [] },
+  { name: "codeBlock", kind: "node", attrs: ["language"] },
+  { name: "column", kind: "node", attrs: ["width"] },
+  { name: "columns", kind: "node", attrs: ["layout", "widthMode"] },
+  { name: "comment", kind: "mark", attrs: ["commentId", "resolved"] },
+  { name: "details", kind: "node", attrs: ["open"] },
+  { name: "detailsContent", kind: "node", attrs: [] },
+  { name: "detailsSummary", kind: "node", attrs: [] },
+  { name: "doc", kind: "node", attrs: [] },
+  { name: "drawio", kind: "node", attrs: ["align", "alt", "aspectRatio", "attachmentId", "height", "size", "src", "title", "width"] },
+  { name: "embed", kind: "node", attrs: ["align", "height", "provider", "src", "width"] },
+  { name: "excalidraw", kind: "node", attrs: ["align", "alt", "aspectRatio", "attachmentId", "height", "size", "src", "title", "width"] },
+  { name: "hardBreak", kind: "node", attrs: [] },
+  { name: "heading", kind: "node", attrs: ["id", "indent", "level", "textAlign"] },
+  { name: "highlight", kind: "mark", attrs: ["color"] },
+  { name: "horizontalRule", kind: "node", attrs: [] },
+  { name: "image", kind: "node", attrs: ["align", "alt", "aspectRatio", "attachmentId", "height", "placeholder", "size", "src", "title", "width"] },
+  { name: "italic", kind: "mark", attrs: [] },
+  { name: "link", kind: "mark", attrs: ["class", "href", "internal", "rel", "target", "title"] },
+  { name: "listItem", kind: "node", attrs: [] },
+  { name: "mathBlock", kind: "node", attrs: ["text"] },
+  { name: "mathInline", kind: "node", attrs: ["text"] },
+  { name: "mention", kind: "node", attrs: ["anchorId", "creatorId", "entityId", "entityType", "id", "label", "slugId"] },
+  { name: "orderedList", kind: "node", attrs: ["start", "type"] },
+  { name: "pageBreak", kind: "node", attrs: [] },
+  { name: "paragraph", kind: "node", attrs: ["id", "indent", "textAlign"] },
+  { name: "pdf", kind: "node", attrs: ["attachmentId", "height", "name", "placeholder", "size", "src", "width"] },
+  { name: "strike", kind: "mark", attrs: [] },
+  { name: "subpages", kind: "node", attrs: [] },
+  { name: "subscript", kind: "mark", attrs: [] },
+  { name: "superscript", kind: "mark", attrs: [] },
+  { name: "table", kind: "node", attrs: [] },
+  { name: "tableCell", kind: "node", attrs: ["align", "backgroundColor", "backgroundColorName", "colspan", "colwidth", "rowspan"] },
+  { name: "tableHeader", kind: "node", attrs: ["align", "backgroundColor", "backgroundColorName", "colspan", "colwidth", "rowspan"] },
+  { name: "tableRow", kind: "node", attrs: [] },
+  { name: "taskItem", kind: "node", attrs: ["checked"] },
+  { name: "taskList", kind: "node", attrs: [] },
+  { name: "text", kind: "node", attrs: [] },
+  { name: "textStyle", kind: "mark", attrs: ["color"] },
+  { name: "underline", kind: "mark", attrs: [] },
+  { name: "video", kind: "node", attrs: ["align", "alt", "aspectRatio", "attachmentId", "height", "placeholder", "size", "src", "width"] },
+  { name: "youtube", kind: "node", attrs: ["align", "height", "src", "width"] },
+];
+
+describe("docmost schema surface", () => {
+  it("matches the committed reference surface (re-verify against @docmost/editor-ext on change)", () => {
+    expect(deriveSurface()).toEqual(expectedSurface);
+  });
+});

packages/git-sync/test/settings.test.ts

@@ -1,76 +0,0 @@
-import { describe, expect, it } from 'vitest';
-import { parseSettings } from '../src/engine/settings';
-
-// A minimal valid environment with every required variable set. Tests clone and
-// mutate this object so process.env is never touched (hermetic).
-const baseEnv = {
-  DOCMOST_API_URL: 'https://docmost.example.com',
-  DOCMOST_EMAIL: 'you@example.com',
-  DOCMOST_PASSWORD: 'secret',
-  DOCMOST_SPACE_ID: 'space-123',
-} as NodeJS.ProcessEnv;
-
-describe('parseSettings', () => {
-  it('maps a full valid env to the camelCase Settings object', () => {
-    const settings = parseSettings({
-      ...baseEnv,
-      VAULT_PATH: 'data/custom-vault',
-      GIT_REMOTE: 'git@github.com:you/vault.git',
-      POLL_INTERVAL_MS: '5000',
-      DEBOUNCE_MS: '1000',
-      LOG_LEVEL: 'debug',
-    });
-
-    expect(settings).toEqual({
-      docmostApiUrl: 'https://docmost.example.com',
-      docmostEmail: 'you@example.com',
-      docmostPassword: 'secret',
-      docmostSpaceId: 'space-123',
-      vaultPath: 'data/custom-vault',
-      gitRemote: 'git@github.com:you/vault.git',
-      pollIntervalMs: 5000,
-      debounceMs: 1000,
-      logLevel: 'debug',
-    });
-  });
-
-  it('applies defaults when optional vars are omitted', () => {
-    const settings = parseSettings({ ...baseEnv });
-
-    expect(settings.vaultPath).toBe('data/vault');
-    expect(settings.pollIntervalMs).toBe(15000);
-    expect(settings.debounceMs).toBe(2000);
-    expect(settings.logLevel).toBe('info');
-    expect(settings.gitRemote).toBeUndefined();
-  });
-
-  it('coerces numeric strings to numbers', () => {
-    const settings = parseSettings({ ...baseEnv, POLL_INTERVAL_MS: '3000' });
-
-    expect(settings.pollIntervalMs).toBe(3000);
-    expect(typeof settings.pollIntervalMs).toBe('number');
-  });
-
-  it('throws when a required var is missing', () => {
-    const { DOCMOST_API_URL: _omit, ...rest } = baseEnv;
-    void _omit;
-    expect(() => parseSettings(rest as NodeJS.ProcessEnv)).toThrow();
-  });
-
-  it('throws on an invalid LOG_LEVEL', () => {
-    expect(() =>
-      parseSettings({ ...baseEnv, LOG_LEVEL: 'verbose' }),
-    ).toThrow();
-  });
-
-  it('throws on a non-numeric POLL_INTERVAL_MS', () => {
-    expect(() =>
-      parseSettings({ ...baseEnv, POLL_INTERVAL_MS: 'soon' }),
-    ).toThrow();
-  });
-
-  it('treats an empty GIT_REMOTE as undefined', () => {
-    const settings = parseSettings({ ...baseEnv, GIT_REMOTE: '' });
-    expect(settings.gitRemote).toBeUndefined();
-  });
-});