diff --git a/AGENTS.md b/AGENTS.md index 50f86b17..0c9d1d2c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -182,7 +182,7 @@ tea issues create --repo vvzvlad/gitmost --labels feature \ ## Monorepo layout -pnpm workspace (`pnpm@10.4.0`) orchestrated by **Nx**. Four workspace packages: +pnpm workspace (`pnpm@10.4.0`) orchestrated by **Nx**. Five workspace packages: | Path | Name | Stack | Role | | --- | --- | --- | --- | @@ -190,6 +190,7 @@ pnpm workspace (`pnpm@10.4.0`) orchestrated by **Nx**. Four workspace packages: | `apps/client` | `client` | React 18 + Vite + Mantine 8 + TanStack Query + Jotai | SPA frontend | | `packages/editor-ext` | `@docmost/editor-ext` | Tiptap/ProseMirror | Shared Tiptap node/mark extensions, imported by both the client and the server | | `packages/mcp` | `@docmost/mcp` | MCP SDK, Tiptap, Yjs | Standalone MCP server, also bundled into the server at `/mcp`. Does **not** import `editor-ext` — it keeps its own vendored mirror of the schema in `packages/mcp/src/lib/` | +| `packages/git-sync` | `@docmost/git-sync` | Tiptap/ProseMirror, Yjs, git | Pure ProseMirror↔Markdown converter plus the two-way Docmost↔git Markdown sync engine. Bundled into the server (loaded over the ESM bridge), built in CI and the Dockerfile. Does **not** import `editor-ext` — it keeps its own vendored mirror of the document schema (kept in sync with `editor-ext`). | `build` targets are Nx-cached and dependency-ordered (`dependsOn: ["^build"]`), so `editor-ext` builds before the apps. `nx.json` sets `affected.defaultBase: main`. @@ -263,7 +264,7 @@ The API server is a Fastify app with a global `/api` prefix (`main.ts` excludes ### Client structure Vite SPA. Code is organized by feature under `apps/client/src/features/*` (mirrors the server domains: `page`, `space`, `comment`, `ai-chat`, `editor`, …). Conventions: - **TanStack Query** for server state (one `queries/` file per feature), **Jotai** atoms for local/shared UI state, **Mantine 8** + CSS modules (`*.module.css`) + `postcss-preset-mantine` for UI. -- The editor is Tiptap; shared node/mark extensions live in `packages/editor-ext` and are imported by **both the client and the server** (collaboration, import/export) — editor schema changes often need to be made in `editor-ext`, not just the client. Note `packages/mcp` does *not* depend on `editor-ext`; it carries its own mirrored copy of the schema, so keep the two in sync manually when the document schema changes. +- The editor is Tiptap; shared node/mark extensions live in `packages/editor-ext` and are imported by **both the client and the server** (collaboration, import/export) — editor schema changes often need to be made in `editor-ext`, not just the client. Note neither `packages/mcp` nor `packages/git-sync` depends on `editor-ext`; each carries its own mirrored copy of the schema. There are now **three** independent copies (`editor-ext` is canonical, plus `packages/mcp` and `packages/git-sync`), so keep all three in sync manually when the document schema changes. - API access goes through `apps/client/src/lib/api-client.ts` (axios). The `@` alias maps to `apps/client/src`. - Runtime config is injected at build time by `vite.config.ts` via `define` (`APP_URL`, `COLLAB_URL`, `APP_VERSION`, …) — these come from the root `.env`, not from `import.meta.env`. diff --git a/apps/server/src/core/page/services/page.service.spec.ts b/apps/server/src/core/page/services/page.service.spec.ts index 707ac51f..2375bf3c 100644 --- a/apps/server/src/core/page/services/page.service.spec.ts +++ b/apps/server/src/core/page/services/page.service.spec.ts @@ -713,5 +713,65 @@ describe('PageService', () => { expect(payload.lastUpdatedSource).toBeUndefined(); }); }); + + describe('removePage()', () => { + // removePage forwards a `source` 4th arg to pageRepo.removePage: 'git-sync' + // for a git-sync-driven soft-delete (so the change-listener loop-guard skips + // its own write), undefined otherwise. + const makeService = () => { + const pageRepo = { + removePage: jest.fn().mockResolvedValue(undefined), + }; + + const svc = new PageService( + pageRepo as any, // pageRepo + {} as any, // pagePermissionRepo + {} as any, // attachmentRepo + {} as any, // db + {} as any, // storageService + {} as any, // attachmentQueue + {} as any, // aiQueue + {} as any, // generalQueue + {} as any, // eventEmitter + {} as any, // collaborationGateway + {} as any, // watcherService + {} as any, // transclusionService + ); + + return { svc, pageRepo }; + }; + + it("forwards 'git-sync' as the source for a git-sync soft-delete", async () => { + const { svc, pageRepo } = makeService(); + + await svc.removePage('page-1', 'user-1', 'ws-1', GIT_SYNC); + + expect(pageRepo.removePage).toHaveBeenCalledTimes(1); + const [pageId, userId, workspaceId, source] = + pageRepo.removePage.mock.calls[0]; + expect(pageId).toBe('page-1'); + expect(userId).toBe('user-1'); + expect(workspaceId).toBe('ws-1'); + expect(source).toBe('git-sync'); + }); + + it('forwards undefined as the source for a plain user delete', async () => { + const { svc, pageRepo } = makeService(); + + await svc.removePage('page-1', 'user-1', 'ws-1', USER_PROVENANCE); + + const [, , , source] = pageRepo.removePage.mock.calls[0]; + expect(source).toBeUndefined(); + }); + + it('forwards undefined as the source when no provenance is given', async () => { + const { svc, pageRepo } = makeService(); + + await svc.removePage('page-1', 'user-1', 'ws-1'); + + const [, , , source] = pageRepo.removePage.mock.calls[0]; + expect(source).toBeUndefined(); + }); + }); }); }); diff --git a/apps/server/src/integrations/environment/environment.service.ts b/apps/server/src/integrations/environment/environment.service.ts index 6353f6b5..d602ba08 100644 --- a/apps/server/src/integrations/environment/environment.service.ts +++ b/apps/server/src/integrations/environment/environment.service.ts @@ -384,6 +384,21 @@ export class EnvironmentService { return Number.isFinite(parsed) && parsed > 0 ? parsed : 15000; } + /** + * Spawned `git http-backend` watchdog timeout in ms (default 120000). Bounds a + * single smart-HTTP request so a stalled `git-receive-pack` cannot hold the + * per-space lock forever (the child is killed and a 500 sent on expiry). A NaN / + * non-positive value falls back to the default so a bad override can never + * disable the watchdog. + */ + getGitSyncBackendTimeoutMs(): number { + const v = parseInt( + this.configService.get('GIT_SYNC_BACKEND_TIMEOUT_MS', '120000'), + 10, + ); + return Number.isFinite(v) && v > 0 ? v : 120000; + } + /** * Event debounce window in ms (default 2000). A NaN / non-positive value falls * back to the default so a bad override can never disable the debounce. diff --git a/apps/server/src/integrations/git-sync/http/git-http-backend.service.spec.ts b/apps/server/src/integrations/git-sync/http/git-http-backend.service.spec.ts index 8fd3b9de..e7834a20 100644 --- a/apps/server/src/integrations/git-sync/http/git-http-backend.service.spec.ts +++ b/apps/server/src/integrations/git-sync/http/git-http-backend.service.spec.ts @@ -38,6 +38,8 @@ function fakeChild() { end: jest.fn(), write: jest.fn(), }); + // The watchdog kills the child on timeout; capture the signal. + child.kill = jest.fn(); return child; } @@ -80,8 +82,13 @@ const baseRequest: GitHttpBackendRequest = { remoteUser: 'alice@example.com', }; -function buildService() { - const env = { getGitSyncDataDir: jest.fn(() => '/vaults') }; +function buildService(backendTimeoutMs = 120000) { + const env = { + getGitSyncDataDir: jest.fn(() => '/vaults'), + // The watchdog timeout for the spawned git http-backend. Tests inject a tiny + // value (or use fake timers) to drive the timeout branch. + getGitSyncBackendTimeoutMs: jest.fn(() => backendTimeoutMs), + }; return new GitHttpBackendService(env as any); } @@ -182,6 +189,56 @@ describe('GitHttpBackendService.run', () => { await p; }); + it('(d) timeout: a child that never closes is killed and a 500 is sent', async () => { + // The child never emits stdout/close (a stalled git-receive-pack). With a + // tiny injected watchdog timeout the run() promise must still resolve: the + // child is killed and a clean 500 is sent (no headers were sent yet). + const child = fakeChild(); + spawnMock.mockReturnValue(child); + const service = buildService(5); // 5ms watchdog + const res = fakeRes(); + const warnSpy = jest.spyOn(Logger.prototype, 'warn'); + + // run() resolves only via the watchdog firing (no close/error emitted). + await service.run(baseRequest, fakeReq(), res); + + expect(child.kill).toHaveBeenCalledWith('SIGTERM'); + expect(warnSpy).toHaveBeenCalled(); + expect(res.statusCode).toBe(500); + expect(res.end).toHaveBeenCalledWith('Internal server error'); + }); + + it('(d) timeout watchdog is cleared on a normal close (no kill, no 500)', async () => { + // A normal request that completes well within the watchdog window must NOT be + // killed and must NOT trip the timeout 500 — the timer is cleared on close. + jest.useFakeTimers(); + try { + const child = fakeChild(); + spawnMock.mockReturnValue(child); + const service = buildService(120000); + const res = fakeRes(); + + const p = service.run(baseRequest, fakeReq(), res); + // loadGitSync resolves on a real microtask; advance it under fake timers. + await Promise.resolve(); + await Promise.resolve(); + + child.stdout.emit( + 'data', + Buffer.from('Status: 200 OK\r\nContent-Type: text/plain\r\n\r\nOK', 'utf8'), + ); + child.emit('close', 0); + await p; + + // The watchdog never fired even if we advance past its window. + jest.advanceTimersByTime(200000); + expect(child.kill).not.toHaveBeenCalled(); + expect(res.statusCode).toBe(200); + } finally { + jest.useRealTimers(); + } + }); + it('spawn throwing synchronously -> 500 (spawn-failed)', async () => { spawnMock.mockImplementation(() => { throw new Error('spawn EACCES'); diff --git a/apps/server/src/integrations/git-sync/http/git-http-backend.service.ts b/apps/server/src/integrations/git-sync/http/git-http-backend.service.ts index fb114dcf..fd138504 100644 --- a/apps/server/src/integrations/git-sync/http/git-http-backend.service.ts +++ b/apps/server/src/integrations/git-sync/http/git-http-backend.service.ts @@ -176,6 +176,45 @@ export class GitHttpBackendService { return done(); } + // Watchdog: a client that opens git-receive-pack and stalls keeps the + // child alive forever, so run() never resolves and (because this runs + // inside withSpaceLock) the per-space lock is held + heartbeat-refreshed + // indefinitely. Bound the request: on expiry kill the child, send a clean + // 500 if nothing was sent yet, and settle the promise. The log carries no + // client echo / credentials / body. `.unref()` so the timer never keeps the + // event loop alive; ALWAYS cleared in the close/error handlers below. + const timer = setTimeout(() => { + this.logger.warn( + `git http-backend timed out after ` + + `${this.environmentService.getGitSyncBackendTimeoutMs()}ms; killing child`, + ); + try { + child.kill('SIGTERM'); + // Escalate to SIGKILL shortly after in case SIGTERM is ignored. + const sigkill = setTimeout(() => { + try { + child.kill('SIGKILL'); + } catch { + /* ignore */ + } + }, 2000); + sigkill.unref?.(); + } catch { + /* ignore */ + } + if (!headerParsed && !rawRes.headersSent) { + this.send500(rawRes, 'timeout'); + } else { + try { + rawRes.end(); + } catch { + /* ignore */ + } + } + done(); + }, this.environmentService.getGitSyncBackendTimeoutMs()); + timer.unref?.(); + // Accumulate stdout until we have the full CGI header block, then write the // parsed status/headers and start streaming the remaining body bytes. let headerParsed = false; @@ -221,6 +260,7 @@ export class GitHttpBackendService { }); child.on('error', (err) => { + clearTimeout(timer); if (!headerParsed && !rawRes.headersSent) { this.send500(rawRes, 'child-error', err); } else { @@ -235,6 +275,7 @@ export class GitHttpBackendService { }); child.on('close', (code) => { + clearTimeout(timer); if (!headerParsed && !rawRes.headersSent) { // The child exited before emitting a complete CGI header block. this.logger.error( diff --git a/apps/server/src/integrations/git-sync/services/gitmost-datasource.service.spec.ts b/apps/server/src/integrations/git-sync/services/gitmost-datasource.service.spec.ts index feb93f34..e55cbc3c 100644 --- a/apps/server/src/integrations/git-sync/services/gitmost-datasource.service.spec.ts +++ b/apps/server/src/integrations/git-sync/services/gitmost-datasource.service.spec.ts @@ -375,13 +375,109 @@ describe('GitmostDataSourceService', () => { describe('restorePage', () => { it('restores via the repo restore path scoped to the workspace', async () => { const { service, mocks } = build(); - await service.bind(CTX).restorePage('p1'); + const res = await service.bind(CTX).restorePage('p1'); // Stamps lastUpdatedSource='git-sync' on restore (loop-guard, PR #119). expect(mocks.pageRepo.restorePage).toHaveBeenCalledWith( 'p1', 'ws-1', 'git-sync', ); + expect(res).toEqual({ id: 'p1' }); + }); + }); + + // Phase-B+ continuous-sync methods: not yet called by the engine but wired into + // the GitSyncClient seam (PR #119 review #5). Exercised via the bound client. + describe('listRecentSince', () => { + it('queries non-deleted pages newest-first and ISO-stringifies updatedAt', async () => { + const rows = [ + { + id: 'p1', + slugId: 's1', + title: 'A', + parentPageId: null, + spaceId: 'space-1', + updatedAt: new Date('2026-06-20T10:00:00.000Z'), + }, + ]; + const { service, mocks } = build(rows); + const qb = mocks.db.selectFrom.mock.results; // populated after the call + + const out = (await service + .bind(CTX) + .listRecentSince('space-1', '2026-06-19T00:00:00.000Z', 100)) as any[]; + + // Query builder shaped against the `pages` table with the expected chain. + expect(mocks.db.selectFrom).toHaveBeenCalledWith('pages'); + const builder = qb[0].value; + expect(builder.select).toHaveBeenCalled(); + expect(builder.orderBy).toHaveBeenCalledWith('updatedAt', 'desc'); + // deletedAt is null + the conditional spaceId / since / cap clauses. + const whereArgs = builder.where.mock.calls.map((c: any[]) => c[0]); + expect(whereArgs).toContain('deletedAt'); + expect(whereArgs).toContain('spaceId'); + expect(whereArgs).toContain('updatedAt'); + expect(builder.limit).toHaveBeenCalledWith(100); + + expect(out).toEqual([ + { + id: 'p1', + slugId: 's1', + title: 'A', + parentPageId: null, + spaceId: 'space-1', + updatedAt: '2026-06-20T10:00:00.000Z', + }, + ]); + }); + + it('omits the spaceId / since / cap clauses when not supplied', async () => { + const { service, mocks } = build([]); + + await service.bind(CTX).listRecentSince(undefined, null); + + const builder = mocks.db.selectFrom.mock.results[0].value; + const whereArgs = builder.where.mock.calls.map((c: any[]) => c[0]); + // Only the deletedAt-is-null guard; no spaceId / updatedAt> clauses. + expect(whereArgs).toEqual(['deletedAt']); + expect(builder.limit).not.toHaveBeenCalled(); + }); + }); + + describe('listTrash', () => { + it('queries soft-deleted pages and ISO-stringifies deletedAt (null stays null)', async () => { + const rows = [ + { + id: 'p1', + slugId: 's1', + title: 'Trashed', + parentPageId: null, + spaceId: 'space-1', + deletedAt: new Date('2026-06-21T09:00:00.000Z'), + }, + { + id: 'p2', + slugId: 's2', + title: 'NoDate', + parentPageId: null, + spaceId: 'space-1', + deletedAt: null, + }, + ]; + const { service, mocks } = build(rows); + + const out = (await service.bind(CTX).listTrash('space-1')) as any[]; + + expect(mocks.db.selectFrom).toHaveBeenCalledWith('pages'); + const builder = mocks.db.selectFrom.mock.results[0].value; + const whereCalls = builder.where.mock.calls; + // deletedAt is-not null (the trash predicate) + spaceId filter. + expect(whereCalls).toContainEqual(['deletedAt', 'is not', null]); + expect(whereCalls).toContainEqual(['spaceId', '=', 'space-1']); + expect(builder.orderBy).toHaveBeenCalledWith('deletedAt', 'desc'); + + expect(out[0].deletedAt).toBe('2026-06-21T09:00:00.000Z'); + expect(out[1].deletedAt).toBeNull(); }); }); }); diff --git a/apps/server/src/integrations/git-sync/services/space-lock.service.ts b/apps/server/src/integrations/git-sync/services/space-lock.service.ts index b0cd999d..33f774bd 100644 --- a/apps/server/src/integrations/git-sync/services/space-lock.service.ts +++ b/apps/server/src/integrations/git-sync/services/space-lock.service.ts @@ -111,25 +111,33 @@ export class SpaceLockService { if (this.running.has(spaceId)) { return { skipped: 'in-progress' }; } - if (!(await this.acquire(spaceId))) { - return { skipped: 'lock-held' }; - } + // Reserve the in-process slot synchronously (before any await) so two + // concurrent same-space calls on THIS instance cannot both pass the guard and + // race acquire(). Redis NX is already authoritative across replicas; this just + // closes the in-process TOCTOU window. Released in the outer finally on every + // path (acquire-failure, fn-throw, normal completion). this.running.add(spaceId); - // Heartbeat: periodically (≈ TTL/3) extend the lock's TTL while `fn` runs so - // a long push (client-controlled receive-pack + the Docmost cycle) cannot - // outlive the fixed TTL and let a concurrent cycle race the working tree. The - // refresh is CAS-guarded (only extends while WE own it). `.unref()` keeps the - // timer from holding the event loop open; it is ALWAYS cleared in `finally`. - const heartbeat = setInterval(() => { - void this.refreshLock(spaceId); - }, Math.max(1, Math.floor(GIT_SYNC_LOCK_TTL_MS / 3))); - heartbeat.unref?.(); try { - return await fn(); + if (!(await this.acquire(spaceId))) { + return { skipped: 'lock-held' }; + } + // Heartbeat: periodically (≈ TTL/3) extend the lock's TTL while `fn` runs so + // a long push (client-controlled receive-pack + the Docmost cycle) cannot + // outlive the fixed TTL and let a concurrent cycle race the working tree. The + // refresh is CAS-guarded (only extends while WE own it). `.unref()` keeps the + // timer from holding the event loop open; it is ALWAYS cleared in `finally`. + const heartbeat = setInterval(() => { + void this.refreshLock(spaceId); + }, Math.max(1, Math.floor(GIT_SYNC_LOCK_TTL_MS / 3))); + heartbeat.unref?.(); + try { + return await fn(); + } finally { + clearInterval(heartbeat); + await this.release(spaceId); + } } finally { - clearInterval(heartbeat); this.running.delete(spaceId); - await this.release(spaceId); } } } diff --git a/apps/server/src/integrations/git-sync/services/vault-registry.service.spec.ts b/apps/server/src/integrations/git-sync/services/vault-registry.service.spec.ts index abd32270..08658575 100644 --- a/apps/server/src/integrations/git-sync/services/vault-registry.service.spec.ts +++ b/apps/server/src/integrations/git-sync/services/vault-registry.service.spec.ts @@ -7,18 +7,33 @@ // `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 { execFile } from 'node:child_process'; import { loadGitSync } from '../git-sync.loader'; jest.mock('node:fs/promises', () => ({ mkdir: jest.fn(async () => undefined), })); +// ensureServable shells out via `promisify(execFile)`; mock execFile with a +// callback-style fn so promisify resolves. Each `git config ` call +// is recorded so the four config writes (incl. the security-critical +// receive.denyNonFastForwards=true) can be asserted. +jest.mock('node:child_process', () => ({ + execFile: jest.fn((_cmd: string, _args: string[], _opts: any, cb: any) => + cb(null, { stdout: '', stderr: '' }), + ), +})); + // Cheap VaultGit stub: records the path it was constructed with; no shell-out. -// Declared with a `mock`-prefixed name so jest allows referencing it inside the -// hoisted `jest.mock` factory below. +// `ensureRepo` is a resolved jest.fn so ensureServable can call it. 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 })); + .mockImplementation((path: string) => ({ + path, + ensureRepo: jest.fn().mockResolvedValue(undefined), + })); jest.mock('../git-sync.loader', () => ({ loadGitSync: jest.fn(async () => ({ @@ -32,6 +47,7 @@ import { VaultRegistryService } from './vault-registry.service'; type AnyMock = jest.Mock; const mkdirMock = mkdir as unknown as AnyMock; +const execFileMock = execFile as unknown as AnyMock; const VaultGitMock = mockVaultGit; void loadGitSync; @@ -78,4 +94,52 @@ describe('VaultRegistryService', () => { }); }); }); + + describe('ensureServable', () => { + it('ensures the repo then writes the four force-push-protection git configs', async () => { + const { service } = build('/vaults'); + + const path = await service.ensureServable('space-1'); + expect(path).toBe('/vaults/space-1'); + + // ensureRepo ran first on the cached vault. + const vault = await service.getVault('space-1'); + expect((vault as any).ensureRepo).toHaveBeenCalledTimes(1); + + // Collect every `git config ` write. + const configWrites = execFileMock.mock.calls + .filter(([cmd, args]) => cmd === 'git' && args[0] === 'config') + .map(([, args]) => [args[1], args[2]]); + + expect(configWrites).toEqual([ + ['receive.denyCurrentBranch', 'updateInstead'], + // Security-critical: blocks force-push / history rewrites on main. + ['receive.denyNonFastForwards', 'true'], + ['http.receivepack', 'true'], + ['http.uploadpack', 'true'], + ]); + + // Every config write targets THIS vault's cwd. + for (const [cmd, args, opts] of execFileMock.mock.calls) { + if (cmd === 'git' && args[0] === 'config') { + expect(opts.cwd).toBe('/vaults/space-1'); + } + } + }); + + it('rejects (and writes no git config) when ensureRepo rejects', async () => { + const { service } = build('/vaults'); + const vault = await service.getVault('space-1'); + (vault as any).ensureRepo.mockRejectedValueOnce(new Error('init failed')); + + await expect(service.ensureServable('space-1')).rejects.toThrow( + 'init failed', + ); + + const configWrites = execFileMock.mock.calls.filter( + ([cmd, args]) => cmd === 'git' && args[0] === 'config', + ); + expect(configWrites).toHaveLength(0); + }); + }); }); diff --git a/packages/git-sync/package.json b/packages/git-sync/package.json index bc6d879e..65340bf2 100644 --- a/packages/git-sync/package.json +++ b/packages/git-sync/package.json @@ -1,7 +1,7 @@ { "name": "@docmost/git-sync", "version": "0.1.0", - "description": "Pure converter + pure sync engine for the Docmost <-> git Markdown sync. See docs/git-sync-plan.md.", + "description": "Pure converter + pure sync engine for the Docmost <-> git Markdown sync. See docs/backlog/git-sync-thin-meta.md.", "private": true, "type": "module", "main": "./build/index.js", diff --git a/packages/git-sync/src/engine/config-errors.ts b/packages/git-sync/src/engine/config-errors.ts deleted file mode 100644 index a4c7a21b..00000000 --- a/packages/git-sync/src/engine/config-errors.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { ZodError } from 'zod'; - -// Turn a ZodError from settings validation into a clear, actionable startup -// message that names the offending env var(s), then exit(1) — no raw stack -// trace. Mirrors the Python new-project skeleton's load_settings_or_exit. -// A non-ZodError is left to propagate unchanged. -export function loadSettingsOrExit(factory: () => T): T { - try { - return factory(); - } catch (err) { - if (!(err instanceof ZodError)) throw err; - const missing: string[] = []; - const invalid: string[] = []; - for (const issue of err.issues) { - const name = issue.path.length ? String(issue.path[0]) : '?'; - // A missing required variable surfaces as an `invalid_type` issue whose - // received value was `undefined`. zod 3 exposed `issue.received` directly; - // zod 4 dropped that field and instead folds it into the message - // ("expected string, received undefined"). Detect both shapes so the - // missing-vs-invalid split holds across zod majors. NOTE: an invalid (but - // present) value uses a different code (invalid_format / invalid_value) or - // an `invalid_type` message that reports a non-undefined received (e.g. - // "received NaN" from a coerced number), so neither is misread as missing. - const i = issue as { received?: unknown; message?: string }; - const isMissing = - issue.code === 'invalid_type' && - (i.received === 'undefined' || - /received undefined/i.test(i.message ?? '')); - if (isMissing) missing.push(name); - else invalid.push(`${name}: ${issue.message}`); - } - const lines = ['Configuration error in environment / .env:']; - if (missing.length) { - lines.push(' Missing required variable(s):'); - for (const n of [...new Set(missing)]) lines.push(` - ${n}`); - } - if (invalid.length) { - lines.push(' Invalid value(s):'); - for (const item of invalid) lines.push(` - ${item}`); - } - lines.push(''); - lines.push('Set them in .env (see .env.example) and try again.'); - process.stderr.write(lines.join('\n') + '\n'); - process.exit(1); - } -} diff --git a/packages/git-sync/src/engine/cycle.ts b/packages/git-sync/src/engine/cycle.ts index 952575a0..f019115f 100644 --- a/packages/git-sync/src/engine/cycle.ts +++ b/packages/git-sync/src/engine/cycle.ts @@ -114,6 +114,7 @@ export async function runCycle(deps: RunCycleDeps): Promise { writeFile: (absPath, text) => fs.writeFile(absPath, text), mkdir: (absDir) => fs.mkdir(absDir), rm: (absPath) => fs.rm(absPath), + log, }, pullActions, vaultRoot, diff --git a/packages/git-sync/src/engine/git.ts b/packages/git-sync/src/engine/git.ts index f63acf35..894e0ee4 100644 --- a/packages/git-sync/src/engine/git.ts +++ b/packages/git-sync/src/engine/git.ts @@ -24,6 +24,12 @@ import { promisify } from "node:util"; const execFileAsync = promisify(execFile); +// Safety net: kill a hung git subprocess. This engine performs only LOCAL git +// operations (no network pushes), so a legitimate call never approaches this +// bound; it only prevents an indefinitely-stuck subprocess from wedging a sync +// cycle (the same risk the http-backend watchdog guards on the server side). +const GIT_EXEC_TIMEOUT_MS = 120_000; + /** Bot identity used for engine-authored vault commits (SPEC §7.3). */ export const BOT_AUTHOR_NAME = "Docmost Sync"; export const BOT_AUTHOR_EMAIL = "docmost-sync@local"; @@ -32,7 +38,7 @@ export const BOT_AUTHOR_EMAIL = "docmost-sync@local"; export const DEFAULT_BRANCH = "main"; /** - * One row of `git diff --name-status` (SPEC §6 "ФС → Docmost"). `status` is the + * One row of `git diff --name-status` (SPEC §6 "FS -> Docmost"). `status` is the * single-letter change code (`-M` rename detection on), `path` is the (new) file * path; for a rename/copy (`R`/`C`) `oldPath` is the source and `path` is the * destination, with `score` carrying git's similarity index (0–100). @@ -146,6 +152,7 @@ export class VaultGit { // can be sizable. ...(cwd !== undefined ? { cwd } : {}), maxBuffer: 64 * 1024 * 1024, + timeout: GIT_EXEC_TIMEOUT_MS, env: vaultGitEnv(opts?.env), }, ); @@ -413,7 +420,7 @@ export class VaultGit { * the listing, e.g. `"*.md"`. * * The target wiki is RUSSIAN, so vault file names routinely contain Cyrillic - * (e.g. `Колонка.md`). With git's DEFAULT `core.quotepath=true`, `ls-files` + * (e.g. `Column.md` in Cyrillic). With git's DEFAULT `core.quotepath=true`, `ls-files` * returns non-ASCII paths octal-escaped and double-quoted (`"\320\232..."`), * which `src/pull.ts` `readExisting` would then parse as garbage paths, * breaking move/duplicate detection. We defeat that two ways at once: @@ -519,7 +526,7 @@ export class VaultGit { /** * Read a ref to its SHA, or `null` if unset. Thin alias over `revParse`, * named for the push direction's marker `refs/docmost/last-pushed` (SPEC §5: - * "что из `main` уже отражено в Docmost"). + * "what of `main` is already reflected in Docmost"). */ async readRef(ref: string): Promise { return this.revParse(ref); diff --git a/packages/git-sync/src/engine/loop-guard.ts b/packages/git-sync/src/engine/loop-guard.ts index fb66534c..bef51e01 100644 --- a/packages/git-sync/src/engine/loop-guard.ts +++ b/packages/git-sync/src/engine/loop-guard.ts @@ -13,7 +13,7 @@ import { createHash } from "node:crypto"; /** - * Stable hash of a page's markdown BODY (SPEC §10 "хэш тела"). Deterministic: + * Stable hash of a page's markdown BODY (SPEC §10 "body hash"). Deterministic: * the same input string always yields the same digest, a different input a * different one. Used to recognize our own write later (loop suppression). * diff --git a/packages/git-sync/src/engine/pull.ts b/packages/git-sync/src/engine/pull.ts index 9a246199..6e79f552 100644 --- a/packages/git-sync/src/engine/pull.ts +++ b/packages/git-sync/src/engine/pull.ts @@ -1,5 +1,5 @@ /** - * Pull cycle — Docmost -> vault (SPEC §6 "Docmost -> ФС"). + * Pull cycle — Docmost -> vault (SPEC §6 "Docmost -> FS"). * * This increment turns the read-only mirror into the git-backed pull cycle: * @@ -225,6 +225,11 @@ export interface ApplyPullActionsDeps { mkdir: (absDir: string) => Promise; /** Remove a file by ABSOLUTE path (force: a missing file is a no-op). */ rm: (absPath: string) => Promise; + /** + * Injected logger for cycle diagnostics (mirrors the push side). Optional — + * falls back to `console.log` so existing callers stay green. + */ + log?: (line: string) => void; } /** Outcome counters from `applyPullActions` (for the summary + tests). */ @@ -259,22 +264,25 @@ export async function applyPullActions( vaultRoot: string, ): Promise { const { client, git } = deps; + // One channel, mirroring the push side: route every cycle diagnostic through + // the injected logger; fall back to `console.log` when none is supplied. + const log = deps.log ?? ((line: string) => console.log(line)); // Emit the SPEC §8 suppression warnings (preserved from the original `main`). const decision = actions.deletionDecision; if (!decision.apply) { if (decision.reason === "incomplete-fetch") { - console.warn( + log( "pull: tree fetch incomplete — deletions suppressed this cycle (SPEC §8)", ); } else if (decision.reason === "empty-live") { - console.warn( + log( `pull: live fetch returned 0 pages but ${actions.existingCount} file(s) are ` + `tracked — deletions suppressed this cycle (SPEC §8). Re-run when ` + `Docmost is reachable.`, ); } else { - console.warn( + log( `pull: plan would delete ${actions.plannedDeleteCount} of ${actions.existingCount} ` + `tracked file(s) (mass-delete guard) — deletions suppressed this ` + `cycle (SPEC §8). Verify the live Docmost tree, then re-run.`, @@ -311,14 +319,14 @@ export async function applyPullActions( } catch (err) { failed++; failedPageIds.add(w.pageId); - console.error( - `pull: failed page ${w.pageId}:`, - err instanceof Error ? err.message : String(err), + log( + `pull: failed page ${w.pageId}: ` + + (err instanceof Error ? err.message : String(err)), ); } finally { completed++; if (completed % PROGRESS_EVERY === 0) { - console.log(`pulled ${completed}/${actions.toWrite.length}`); + log(`pulled ${completed}/${actions.toWrite.length}`); } } }; @@ -346,9 +354,9 @@ export async function applyPullActions( await deps.rm(relToAbs(vaultRoot, rel)); return true; } catch (err) { - console.error( - `pull: failed to ${what} ${rel}:`, - err instanceof Error ? err.message : String(err), + log( + `pull: failed to ${what} ${rel}: ` + + (err instanceof Error ? err.message : String(err)), ); return false; } @@ -364,7 +372,7 @@ export async function applyPullActions( for (const m of actions.moved) { if (!m.removeOldPath) continue; if (failedPageIds.has(m.pageId)) { - console.warn( + log( `pull: move write for ${m.pageId} failed — keeping old path ` + `${m.fromRelPath} (SPEC §8)`, ); @@ -401,15 +409,15 @@ export async function applyPullActions( await git.checkout(DEFAULT_BRANCH); const merge = await git.merge(DOCMOST_BRANCH); if (merge.conflict) { - console.error( + log( "pull: merge of docmost -> main CONFLICTED. Conflict markers were left " + "in the vault for manual resolution (SPEC §9). Nothing is pushed to " + "Docmost (read-only). Resolve locally, then re-run.", ); } else if (!merge.ok) { - console.error(`pull: merge of docmost -> main failed: ${merge.output}`); + log(`pull: merge of docmost -> main failed: ${merge.output}`); } - console.log("pull: git push to remote is DEFERRED in this increment (SPEC §7)."); + log("pull: git push to remote is DEFERRED in this increment (SPEC §7)."); return { written, movedApplied, deleted, failed, committed, merge }; } diff --git a/packages/git-sync/src/engine/push.ts b/packages/git-sync/src/engine/push.ts index 2f08f3b6..166c9643 100644 --- a/packages/git-sync/src/engine/push.ts +++ b/packages/git-sync/src/engine/push.ts @@ -1,5 +1,5 @@ /** - * Push cycle — vault -> Docmost (SPEC §6 "ФС → Docmost"), FIRST increment. + * Push cycle — vault -> Docmost (SPEC §6 "FS -> Docmost"), FIRST increment. * * This module mirrors the structure of `./pull.ts`: a set of VaultGit diff/ref * primitives (in `./git.ts`), a PURE planner (`computePushActions`) that turns @@ -65,7 +65,7 @@ export interface RenameMoveAction { /** * A CLASSIFIED rename/move (push #3): a `RenameMoveAction` resolved into the * Docmost op(s) it actually needs. The file PATH is the source of truth for tree - * position (SPEC §5: "истина связи — pageId, не путь" — the path is COSMETIC and + * position (SPEC §5: "the identity is the pageId, not the path" — the path is COSMETIC and * LOCAL, the page identity is its pageId), so we compare the RESOLVED parent of * the new path against the resolved parent of the old path, and the title in the * current meta against the title in the previous meta. Each sub-op is emitted @@ -235,7 +235,7 @@ export interface PushActionsInput { */ export function computePushActions(input: PushActionsInput): PushActions { const { metaAt, currentPageIds } = input; - // PAGE-FILE FILTER (design §"Адопция"): only `.md` files OUTSIDE any dot-folder + // PAGE-FILE FILTER (design §"Adoption"): only `.md` files OUTSIDE any dot-folder // are Docmost pages. `.obsidian/*`, attachments, and other non-page files are // committed to the vault (no `.gitignore`) and so appear in the diff, but they // are NEVER pages — Obsidian owns them. Without this filter every ADDED such @@ -445,6 +445,7 @@ export const DOCMOST_BRANCH = "docmost"; export interface ApplyPushDeps { client: Pick< GitSyncClient, + | "listSpaceTree" | "importPageMarkdown" | "createPage" | "deletePage" @@ -489,7 +490,7 @@ export interface PushedPageRecord { * exposed one. Absent when the (fake) client did not return it. */ updatedAt?: string; - /** Stable hash of the markdown BODY that was pushed (SPEC §10 "хэш тела"). */ + /** Stable hash of the markdown BODY that was pushed (SPEC §10 "body hash"). */ bodyHash: string; } @@ -675,8 +676,34 @@ export async function applyPushActions( } // 2. CREATES — create the page, then write the assigned pageId back to meta so - // the file becomes tracked (SPEC §4 "записать присвоенный pageId обратно"). + // the file becomes tracked (SPEC §4 "write the assigned pageId back"). // Isolated per page like updates. + // + // RETRY-ADOPT (#1 idempotency): create is NOT atomic with the pageId write-back + // (createPage runs, then writeFile, then the write-back commit at runPush 7a). If + // the write-back dies in between, the file on disk still has no pageId and the + // next cycle re-classifies it as a CREATE -> a DUPLICATE page would be created. + // To guard against this, build a (parentPageId|root, title) -> existing pageId map + // ONCE from the LIVE Docmost tree (only when there is at least one create). The + // native-Obsidian layout makes filenames — and therefore titles — unique within a + // folder, so (parentPageId, title) identifies the page; a match means a prior + // cycle already created it, so we ADOPT instead of duplicating. + let liveByParentTitle: Map | null = null; + if (actions.creates.length > 0) { + const live = await client.listSpaceTree(deps.spaceId); + // Only trust a COMPLETE tree for retry-adopt: a truncated tree could miss an + // already-created page and let us create a DUPLICATE (the very thing adopt + // prevents). The native client always returns complete:true (reads the DB); + // on an incomplete tree we leave the map null -> fall back to plain createPage. + if (live.complete) { + liveByParentTitle = new Map(); + for (const n of live.pages) { + const key = `${n.parentPageId ?? " root"} ${n.title ?? ""}`; + // Keep the FIRST node for a key (the layout makes this unique in practice). + if (!liveByParentTitle.has(key)) liveByParentTitle.set(key, n.id); + } + } + } for (const c of actions.creates) { try { const text = await deps.readFile(c.path); @@ -687,6 +714,26 @@ export async function applyPushActions( const title = titleFromPath(c.path); const parentPageId = (await resolveParentPageIdViaTree(deps, c.path, "current")) ?? undefined; + // Retry-adopt (#1 idempotency): a prior cycle already created this page in + // Docmost but failed to persist the pageId back to the file, so it was + // re-seen as a create. Adopt the existing page instead of duplicating it: + // write the id back (file becomes tracked) and push the body as an UPDATE + // (idempotent — targets by pageId). Do NOT call createPage again. + const adoptKey = `${parentPageId ?? " root"} ${title}`; + const existingId = liveByParentTitle?.get(adoptKey); + if (existingId) { + const rewritten = serializePageFile(existingId, body); + await deps.writeFile(c.path, rewritten); + writtenBack.push({ path: c.path, pageId: existingId }); + const adopted = await client.importPageMarkdown(existingId, body, null); + pushed.push({ + pageId: existingId, + ...extractUpdatedAt(adopted), + bodyHash: bodyHash(body), + }); + created++; + continue; + } const result = await client.createPage( title, body, @@ -896,7 +943,7 @@ export function parentFolderFile(path: string): string | null { } /** - * Whether a vault path is a Docmost PAGE file (design §"Адопция"): a `.md` file + * Whether a vault path is a Docmost PAGE file (design §"Adoption"): a `.md` file * with NO dot-segment anywhere in its path. This excludes `.obsidian/` config, * `.trash/`, dotfiles (`.foo.md`), and every non-`.md` file (attachments, JSON, * …) — Obsidian owns those; they live in the vault but are never pages. Used to @@ -954,7 +1001,7 @@ function nativeMeta( * then read its `gitmost_id` frontmatter and return that page's pageId. A root-level path * (no enclosing folder), a missing/unreadable parent file, or a parent file with * no parseable pageId all resolve to `null` (parent is ROOT / unknown -> - * `parentPageId: null`, SPEC §16 "parentPageId: null -> в корень"). + * `parentPageId: null`, SPEC §16 "parentPageId: null -> to root"). * * The IO is async, so this returns an ASYNC resolver; the call sites prefetch the * parent pageIds (the classifier itself stays pure/sync over a plain table). @@ -1112,7 +1159,7 @@ export interface PushRunResult { } /** - * Run one FS->Docmost push cycle (SPEC §6 "ФС → Docmost"), DRY-RUN BY DEFAULT. + * Run one FS->Docmost push cycle (SPEC §6 "FS -> Docmost"), DRY-RUN BY DEFAULT. * * Steps (mirrors `pull.ts`): * 1. Preflight git: `assertGitAvailable` + `ensureRepo`; ABORT (clear message + @@ -1426,17 +1473,3 @@ function logPlan( for (const s of actions.skipped) log(` skipped [${s.status}] ${s.path}: ${s.reason}`); } - -/** Parsed `push` CLI flags. DRY-RUN is the default; `--apply` opts into writes. */ -export interface PushParsedArgs { - /** True when `--apply` was passed (the ONLY path that writes to Docmost). */ - apply: boolean; -} - -/** - * Parse the `push` CLI flags. SAFE BY DEFAULT: without `--apply` the run is a - * DRY-RUN (plan only). Exported so the flag handling is unit-testable. - */ -export function parseArgs(argv: string[]): PushParsedArgs { - return { apply: argv.includes("--apply") }; -} diff --git a/packages/git-sync/src/engine/settings.ts b/packages/git-sync/src/engine/settings.ts index a56ed325..05073ea1 100644 --- a/packages/git-sync/src/engine/settings.ts +++ b/packages/git-sync/src/engine/settings.ts @@ -2,41 +2,10 @@ * Engine settings. * * The engine is driven IN-PROCESS by the NestJS server, which builds the - * `Settings` object from `EnvironmentService` — so this module must NOT reach - * into `process.env`. It exposes only: - * - the `Settings` type the engine consumes, and - * - `parseSettings(env)` as a PURE function (validate a raw env object -> typed - * `Settings`), kept for unit tests and for the server to reuse if it wants - * to validate an env-shaped object. - * There is no `.env`-loading side-effecting entry point. + * `Settings` object from `EnvironmentService`. This module therefore exposes + * ONLY the `Settings` type the engine consumes — there is no `.env`-loading + * side-effecting entry point and no env-validation here (the server owns that). */ -import { z } from 'zod'; - -// Schema keyed by the real ENV variable names so validation errors name the -// exact variable. Credentials and the address of our OWN Docmost instance have -// NO default — a missing value must fail at startup, never silently fall back. -export const envSchema = z.object({ - // Docmost connection — address of our own instance, no default. - DOCMOST_API_URL: z.string().url(), - // Credentials for /auth/login — no default, never hardcoded. - DOCMOST_EMAIL: z.string().min(1), - DOCMOST_PASSWORD: z.string().min(1), - // Which Docmost space to mirror. - DOCMOST_SPACE_ID: z.string().min(1), - - // Local git vault (state store) — kept under data/ so the volume persists it. - VAULT_PATH: z.string().min(1).default('data/vault'), - // Optional git remote the vault pushes to. Empty string is treated as unset. - GIT_REMOTE: z.preprocess( - (v) => (v === '' ? undefined : v), - z.string().min(1).optional(), - ), - - // Non-secret tunables — sensible defaults are fine. - POLL_INTERVAL_MS: z.coerce.number().int().positive().default(15000), - DEBOUNCE_MS: z.coerce.number().int().positive().default(2000), - LOG_LEVEL: z.enum(['debug', 'info', 'warn', 'error']).default('info'), -}); export type Settings = { docmostApiUrl: string; @@ -49,20 +18,3 @@ export type Settings = { debounceMs: number; logLevel: 'debug' | 'info' | 'warn' | 'error'; }; - -// Pure: validate a raw environment object and map it to a typed Settings. -// Throws ZodError on bad config. No side effects — safe to import in tests. -export function parseSettings(env: NodeJS.ProcessEnv): Settings { - const e = envSchema.parse(env); - return { - docmostApiUrl: e.DOCMOST_API_URL, - docmostEmail: e.DOCMOST_EMAIL, - docmostPassword: e.DOCMOST_PASSWORD, - docmostSpaceId: e.DOCMOST_SPACE_ID, - vaultPath: e.VAULT_PATH, - gitRemote: e.GIT_REMOTE, - pollIntervalMs: e.POLL_INTERVAL_MS, - debounceMs: e.DEBOUNCE_MS, - logLevel: e.LOG_LEVEL, - }; -} diff --git a/packages/git-sync/src/engine/stabilize.ts b/packages/git-sync/src/engine/stabilize.ts index becd946f..a075b634 100644 --- a/packages/git-sync/src/engine/stabilize.ts +++ b/packages/git-sync/src/engine/stabilize.ts @@ -1,5 +1,5 @@ /** - * Normalize-on-write helper (SPEC §11 "Резолюция"). + * Normalize-on-write helper (SPEC §11 "Resolution"). * * git diffs byte-for-byte, so writing a page in a NON-fixpoint markdown form * would make the next pull re-export it to a slightly different (but stable) diff --git a/packages/git-sync/src/index.ts b/packages/git-sync/src/index.ts index 0ecc92a3..fc4e5c84 100644 --- a/packages/git-sync/src/index.ts +++ b/packages/git-sync/src/index.ts @@ -81,7 +81,6 @@ export { applyPushActions, runPush, parentFolderFile, - parseArgs, LAST_PUSHED_REF, DOCMOST_BRANCH, LOCAL_AUTHOR_NAME, @@ -106,14 +105,10 @@ export type { ApplyPushResult, PushDeps, PushRunResult, - PushParsedArgs, } from "./engine/push.js"; -export { parseSettings, envSchema } from "./engine/settings.js"; export type { Settings } from "./engine/settings.js"; -export { loadSettingsOrExit } from "./engine/config-errors.js"; - export { runCycle } from "./engine/cycle.js"; export type { RunCycleDeps, diff --git a/packages/git-sync/src/lib/canonicalize.ts b/packages/git-sync/src/lib/canonicalize.ts index 081490af..99ff5bc6 100644 --- a/packages/git-sync/src/lib/canonicalize.ts +++ b/packages/git-sync/src/lib/canonicalize.ts @@ -1,6 +1,6 @@ /** * Semantic canonicalization of ProseMirror/TipTap documents for the round-trip - * idempotency check (SPEC §11, "Задача №0", option (б): compare a CANONICALIZED + * idempotency check (SPEC §11, "Task #0", option (b): compare a CANONICALIZED * form rather than raw bytes). * * `markdownToProseMirror` reconstructs schema DEFAULT attributes (e.g. diff --git a/packages/git-sync/src/lib/diff.ts b/packages/git-sync/src/lib/diff.ts index 25b19148..94f7d447 100644 --- a/packages/git-sync/src/lib/diff.ts +++ b/packages/git-sync/src/lib/diff.ts @@ -103,8 +103,8 @@ function countUniqueLinks(doc: any): number { /** * Parse the ordered list of integers from `[N]` footnote markers found in the - * BODY only (every top-level block before the first "Примечания..." notes - * heading; if no such heading, the whole doc). Returned in reading order. + * BODY only (every top-level block before the first notes heading; if no such + * heading, the whole doc). Returned in reading order. */ function footnoteMarkers(doc: any, notesHeading: string): number[] { const top: any[] = Array.isArray(doc?.content) ? doc.content : []; diff --git a/packages/git-sync/src/lib/docmost-schema.ts b/packages/git-sync/src/lib/docmost-schema.ts index c45c275a..9259af39 100644 --- a/packages/git-sync/src/lib/docmost-schema.ts +++ b/packages/git-sync/src/lib/docmost-schema.ts @@ -6,6 +6,14 @@ * (node ids, image sizing, link targets). Every code path that converts * to or from ProseMirror JSON must use THIS set, otherwise a round-trip * loses content. + * + * PROVENANCE / KEEP IN SYNC: this file is a VENDORED MIRROR of the canonical + * Docmost document schema in `@docmost/editor-ext`. The node/mark/attribute + * surface MUST be kept in sync with editor-ext — anything present there but + * missing here is silently dropped on a round-trip (data loss). The exported + * `docmostExtensions` surface is guarded by `test/schema-surface-snapshot.test.ts`, + * which fails loudly on any drift; when it does, re-verify parity against + * `@docmost/editor-ext` before updating the snapshot. */ import StarterKit from "@tiptap/starter-kit"; import Image from "@tiptap/extension-image"; diff --git a/packages/git-sync/test/apply-pull-actions.test.ts b/packages/git-sync/test/apply-pull-actions.test.ts index fc57e83d..762046e6 100644 --- a/packages/git-sync/test/apply-pull-actions.test.ts +++ b/packages/git-sync/test/apply-pull-actions.test.ts @@ -99,17 +99,25 @@ function makeFs(opts?: { failWriteFor?: Set }) { 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; + function deps( client: any, git: any, fs: ReturnType, ): 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).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); diff --git a/packages/git-sync/test/apply-push-actions.test.ts b/packages/git-sync/test/apply-push-actions.test.ts index 0a277fec..e508c727 100644 --- a/packages/git-sync/test/apply-push-actions.test.ts +++ b/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(); diff --git a/packages/git-sync/test/config-errors-invalid.test.ts b/packages/git-sync/test/config-errors-invalid.test.ts deleted file mode 100644 index 14fabe12..00000000 --- a/packages/git-sync/test/config-errors-invalid.test.ts +++ /dev/null @@ -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 `: ` 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 `: ` 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'); - }); -}); diff --git a/packages/git-sync/test/config-errors.test.ts b/packages/git-sync/test/config-errors.test.ts deleted file mode 100644 index 6ecf7093..00000000 --- a/packages/git-sync/test/config-errors.test.ts +++ /dev/null @@ -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(); - }); -}); diff --git a/packages/git-sync/test/engine-gaps.test.ts b/packages/git-sync/test/engine-gaps.test.ts index f6530aec..ca99dfc0 100644 --- a/packages/git-sync/test/engine-gaps.test.ts +++ b/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 () => ({})), diff --git a/packages/git-sync/test/run-push-realgit.test.ts b/packages/git-sync/test/run-push-realgit.test.ts index 72d6ae8a..a550c9b2 100644 --- a/packages/git-sync/test/run-push-realgit.test.ts +++ b/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, diff --git a/packages/git-sync/test/run-push.test.ts b/packages/git-sync/test/run-push.test.ts index 289e9899..43cfb622 100644 --- a/packages/git-sync/test/run-push.test.ts +++ b/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 }, diff --git a/packages/git-sync/test/schema-surface-snapshot.test.ts b/packages/git-sync/test/schema-surface-snapshot.test.ts new file mode 100644 index 00000000..319899e3 --- /dev/null +++ b/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); + }); +}); diff --git a/packages/git-sync/test/settings.test.ts b/packages/git-sync/test/settings.test.ts deleted file mode 100644 index ed7efa01..00000000 --- a/packages/git-sync/test/settings.test.ts +++ /dev/null @@ -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(); - }); -}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c4750144..441ec519 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -528,6 +528,9 @@ importers: '@clickhouse/client': specifier: ^1.18.2 version: 1.18.2 + '@docmost/git-sync': + specifier: workspace:* + version: link:../../packages/git-sync '@docmost/mcp': specifier: workspace:* version: link:../../packages/mcp