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>
This commit is contained in:
claude_code
committed by
claude code agent 227
claude code agent 227
parent
c8660ea765
commit
f1a8f40b9f
@@ -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`.
|
||||
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string>('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.
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 <key> <value>` 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 <key> <value>` 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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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<T>(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);
|
||||
}
|
||||
}
|
||||
@@ -114,6 +114,7 @@ export async function runCycle(deps: RunCycleDeps): Promise<RunCycleResult> {
|
||||
writeFile: (absPath, text) => fs.writeFile(absPath, text),
|
||||
mkdir: (absDir) => fs.mkdir(absDir),
|
||||
rm: (absPath) => fs.rm(absPath),
|
||||
log,
|
||||
},
|
||||
pullActions,
|
||||
vaultRoot,
|
||||
|
||||
@@ -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<string | null> {
|
||||
return this.revParse(ref);
|
||||
|
||||
@@ -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).
|
||||
*
|
||||
|
||||
@@ -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<void>;
|
||||
/** Remove a file by ABSOLUTE path (force: a missing file is a no-op). */
|
||||
rm: (absPath: string) => Promise<void>;
|
||||
/**
|
||||
* 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<ApplyResult> {
|
||||
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 };
|
||||
}
|
||||
|
||||
@@ -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<string, string> | 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") };
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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 : [];
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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 () => ({})),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 },
|
||||
|
||||
116
packages/git-sync/test/schema-surface-snapshot.test.ts
Normal file
116
packages/git-sync/test/schema-surface-snapshot.test.ts
Normal file
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
3
pnpm-lock.yaml
generated
3
pnpm-lock.yaml
generated
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user