Files
gitmost/packages/git-sync/test/run-push-realgit.test.ts
claude_code 68515ef947 fix(git-sync): address PR #119 review (#1571)
Resolve the code-review findings from comment #1571 on PR #119.

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

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

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

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-26 00:06:44 +03:00

146 lines
5.6 KiB
TypeScript

import { execFile } from 'node:child_process';
import { mkdtemp, rm, writeFile } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { promisify } from 'node:util';
import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest';
import { runPush, LAST_PUSHED_REF } from '../src/engine/push';
import type { PushDeps } from '../src/engine/push';
import { VaultGit } from '../src/engine/git';
import type { Settings } from '../src/engine/settings';
import { serializeDocmostMarkdownBody } from '../src/lib/index';
const execFileAsync = promisify(execFile);
// runPush `--apply` against a REAL VaultGit in a temp repo (NO Docmost — the
// client is faked). This guards the real-git BINDING contract that the plain-
// object git fakes in run-push.test.ts cannot catch: the applier's git deps
// (`updateRef`/`fastForwardBranch`/`showFileAtRef`) call `this.run`/`this.runRaw`
// internally, so they only work when their `this` receiver is preserved. Passing
// bare method references (`git.updateRef`, …) would throw `this.runRaw is not a
// function` here. Only the LOCAL temp git is mutated; nothing is sent to Docmost.
/** True if a usable `git` binary is on PATH (skip the suite otherwise). */
async function gitAvailable(): Promise<boolean> {
try {
await execFileAsync('git', ['--version']);
return true;
} catch {
return false;
}
}
/** A minimal valid Settings fixture (only fields runPush reads matter). */
function makeSettings(vaultPath: string): Settings {
return {
docmostApiUrl: 'https://docmost.example.com',
docmostEmail: 'you@example.com',
docmostPassword: 'secret',
docmostSpaceId: 'space-1',
vaultPath,
pollIntervalMs: 15000,
debounceMs: 2000,
logLevel: 'info',
};
}
/** 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,
})),
createPage: vi.fn(async (title: string) => ({
data: { id: 'new-id', title, updatedAt: '2026-06-20T00:00:00.000Z' },
success: true,
})),
deletePage: vi.fn(async () => ({ success: true })),
movePage: vi.fn(async () => ({ success: true })),
renamePage: vi.fn(async () => ({ success: true })),
};
}
describe('runPush --apply against a REAL VaultGit (binding contract)', () => {
let available = false;
let dir: string;
beforeAll(async () => {
available = await gitAvailable();
});
afterEach(async () => {
if (dir) {
await rm(dir, { recursive: true, force: true });
}
});
it('writes through real git: createPage runs, last-pushed advances, no throw', async () => {
if (!available) return; // skip gracefully when git is unavailable
// Temp vault repo under the OS tmpdir (mirrors test/git.test.ts setup).
dir = await mkdtemp(join(tmpdir(), 'docmost-push-realgit-'));
const vault = dir;
const git = new VaultGit(vault);
await git.ensureRepo();
// The `docmost` mirror branches off `main` at the initial commit; this is
// also the diff base (last-pushed is unset, so runPush falls back to it).
await git.ensureBranch('docmost', 'main');
// A brand-new local file with meta carrying title + spaceId but NO pageId,
// committed on `main` AHEAD of the base -> computePushActions yields a CREATE.
const newFile = serializeDocmostMarkdownBody(
{ version: 1, title: 'New', spaceId: 'sp-1' },
'fresh body',
);
await writeFile(join(vault, 'New.md'), newFile, 'utf8');
await git.stageAll();
await git.commit('add New.md', {
authorName: 'Human',
authorEmail: 'human@local',
});
// last-pushed must be UNSET so the run actually advances it for the first time.
expect(await git.revParse(LAST_PUSHED_REF)).toBeNull();
const client = makeClientFake();
const logs: string[] = [];
const deps: PushDeps = {
settings: makeSettings(vault),
// The WHOLE real VaultGit — its methods must keep their `this` binding.
git,
makeClient: () => client as any,
readFile: (path) =>
import('node:fs/promises').then((fs) =>
fs.readFile(join(vault, ...path.split('/')), 'utf8'),
),
writeFile: async (path, text) => {
const fs = await import('node:fs/promises');
await fs.writeFile(join(vault, ...path.split('/')), text, 'utf8');
},
log: (line) => logs.push(line),
};
// The run must NOT throw — this is what FAILS before Fix 1 (the bare-method
// git deps would throw `this.runRaw is not a function` on the real VaultGit).
const res = await runPush(deps, { dryRun: false });
expect(res.mode).toBe('apply');
expect(res.failures).toEqual([]);
// The FAKE client was actually called (the write path ran).
expect(client.createPage).toHaveBeenCalledTimes(1);
expect(res.applied?.created).toBe(1);
// The assigned pageId was written back to disk + committed.
expect(res.applied?.writtenBack).toEqual([{ path: 'New.md', pageId: 'new-id' }]);
// CRITICALLY: refs/docmost/last-pushed ACTUALLY advanced in the real repo —
// it now resolves to a real commit (proving updateRef ran with binding).
const lastPushed = await git.revParse(LAST_PUSHED_REF);
expect(lastPushed).toMatch(/^[0-9a-f]{40}$/);
expect(res.divergentDocmost).toBe(false);
});
});