feat(sync): FS->Docmost push #2 — loop-close (§6.3/§10) + fix flaky property timeout
- git.ts: fastForwardBranch(branch, toCommit) — advances ONLY on a true
fast-forward (merge-base --is-ancestor), refuses a non-ff without clobbering
divergent docmost history
- push.ts: after a CLEAN push (failures===0) advance both refs/docmost/last-pushed
AND fast-forward the docmost mirror, so the next pull sees no diff for pushed
pages (loop-guard, git-native); a partial push advances NEITHER (§12)
- push.ts: per-page error isolation (one bad page doesn't block the batch,
failures recorded); create requires a non-empty spaceId else skipped (§8 spirit)
- loop-guard.ts: bodyHash() (sha256) + per-page pushed:[{pageId,updatedAt?,bodyHash}]
record for the §10 self-write suppression (pull-side consumption deferred)
- test: markdown-roundtrip property tests get a 30s per-test timeout (deterministic
inputs via fixed seed; the only flakiness was wall-clock under parallel load,
which intermittently failed CI/docker)
- 709 -> 724 green (3x stable); build clean; corpus STABLE
Deferred (next/final increment): move/rename apply, pull-side loop-guard consumption,
FS-watcher/debounce (§7.1), git-remote push (§7.2), runnable live main(),
escalate-on-divergent-docmost.
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { applyPushActions, LAST_PUSHED_REF } from '../src/push.js';
|
||||
import { bodyHash } from '../src/loop-guard.js';
|
||||
import type { ApplyPushDeps, PushActions } from '../src/push.js';
|
||||
import {
|
||||
parseDocmostMarkdown,
|
||||
@@ -36,15 +37,24 @@ function makeClient(opts?: { createId?: string }) {
|
||||
return client;
|
||||
}
|
||||
|
||||
/** A recording git fake (only updateRef is used by the push applier). */
|
||||
function makeGit() {
|
||||
/**
|
||||
* A recording git fake: `updateRef` (advance last-pushed) and `fastForwardBranch`
|
||||
* (advance the `docmost` mirror, the loop-close). `ffResult` configures what the
|
||||
* ff returns (default a successful advance).
|
||||
*/
|
||||
function makeGit(opts?: { ffResult?: { ok: boolean; reason?: string } }) {
|
||||
const updateRefCalls: { ref: string; target: string }[] = [];
|
||||
const ffCalls: { branch: string; toCommit: string }[] = [];
|
||||
const git = {
|
||||
updateRef: vi.fn(async (ref: string, target: string) => {
|
||||
updateRefCalls.push({ ref, target });
|
||||
}),
|
||||
fastForwardBranch: vi.fn(async (branch: string, toCommit: string) => {
|
||||
ffCalls.push({ branch, toCommit });
|
||||
return opts?.ffResult ?? { ok: true };
|
||||
}),
|
||||
};
|
||||
return { git, updateRefCalls };
|
||||
return { git, updateRefCalls, ffCalls };
|
||||
}
|
||||
|
||||
/** A recording fs fake over a path->text store. */
|
||||
@@ -199,10 +209,10 @@ describe('applyPushActions — rename/move is DEFERRED (NEXT increment)', () =>
|
||||
});
|
||||
});
|
||||
|
||||
describe('applyPushActions — last-pushed ref advance (SPEC §6 step 3)', () => {
|
||||
it('advances refs/docmost/last-pushed to the pushed commit', async () => {
|
||||
describe('applyPushActions — loop-close: ref advance + docmost ff (SPEC §6 step 3 / §10)', () => {
|
||||
it('advances last-pushed AND fast-forwards the docmost mirror on a clean push', async () => {
|
||||
const client = makeClient();
|
||||
const { git, updateRefCalls } = makeGit();
|
||||
const { git, updateRefCalls, ffCalls } = makeGit();
|
||||
const fs = makeFs();
|
||||
|
||||
const res = await applyPushActions(
|
||||
@@ -215,9 +225,34 @@ describe('applyPushActions — last-pushed ref advance (SPEC §6 step 3)', () =>
|
||||
expect(updateRefCalls).toEqual([
|
||||
{ ref: LAST_PUSHED_REF, target: 'commit-sha-abc' },
|
||||
]);
|
||||
// The loop-close: the docmost mirror is fast-forwarded to the pushed commit.
|
||||
expect(ffCalls).toEqual([{ branch: 'docmost', toCommit: 'commit-sha-abc' }]);
|
||||
expect(res.docmostFastForward).toEqual({ ok: true });
|
||||
});
|
||||
|
||||
it('does NOT advance the ref when no pushed commit is given', async () => {
|
||||
it('surfaces a REFUSED non-fast-forward (mirror NOT clobbered)', async () => {
|
||||
const client = makeClient();
|
||||
// The ff is refused because docmost is not an ancestor of the pushed commit.
|
||||
const { git, updateRefCalls, ffCalls } = makeGit({
|
||||
ffResult: { ok: false, reason: 'not-fast-forward' },
|
||||
});
|
||||
const fs = makeFs();
|
||||
|
||||
const res = await applyPushActions(
|
||||
deps(client, git, fs),
|
||||
actions({ deletes: [{ pageId: 'p' }] }),
|
||||
'sha-div',
|
||||
);
|
||||
|
||||
// last-pushed still advances (it is our own marker), but the ff result is
|
||||
// surfaced so the caller can log the refusal.
|
||||
expect(res.lastPushedAdvanced).toBe(true);
|
||||
expect(updateRefCalls).toEqual([{ ref: LAST_PUSHED_REF, target: 'sha-div' }]);
|
||||
expect(ffCalls).toEqual([{ branch: 'docmost', toCommit: 'sha-div' }]);
|
||||
expect(res.docmostFastForward).toEqual({ ok: false, reason: 'not-fast-forward' });
|
||||
});
|
||||
|
||||
it('does NOT advance either ref when no pushed commit is given', async () => {
|
||||
const client = makeClient();
|
||||
const { git, updateRefCalls } = makeGit();
|
||||
const fs = makeFs();
|
||||
@@ -229,7 +264,117 @@ describe('applyPushActions — last-pushed ref advance (SPEC §6 step 3)', () =>
|
||||
|
||||
expect(res.lastPushedAdvanced).toBe(false);
|
||||
expect(updateRefCalls).toEqual([]);
|
||||
expect(res.docmostFastForward).toBeNull();
|
||||
expect(git.updateRef).not.toHaveBeenCalled();
|
||||
expect(git.fastForwardBranch).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('applyPushActions — per-page error isolation + refs gated on success (SPEC §12)', () => {
|
||||
it('continues the batch when an update throws; records the failure; refs NOT advanced', async () => {
|
||||
// A client whose 2nd importPageMarkdown call throws — the 1st and 3rd must
|
||||
// still be applied, the 2nd recorded as a failure, and NO ref advanced.
|
||||
let call = 0;
|
||||
const client = {
|
||||
importPageMarkdown: vi.fn(async (_pageId: string, _md: string) => {
|
||||
call++;
|
||||
if (call === 2) throw new Error('boom on page 2');
|
||||
return { success: true };
|
||||
}),
|
||||
createPage: vi.fn(),
|
||||
deletePage: vi.fn(),
|
||||
};
|
||||
const { git, updateRefCalls, ffCalls } = makeGit();
|
||||
const fs = makeFs({
|
||||
'A.md': 'a body',
|
||||
'B.md': 'b body',
|
||||
'C.md': 'c body',
|
||||
});
|
||||
|
||||
const res = await applyPushActions(
|
||||
deps(client, git, fs),
|
||||
actions({
|
||||
updates: [
|
||||
{ pageId: 'p-a', path: 'A.md' },
|
||||
{ pageId: 'p-b', path: 'B.md' },
|
||||
{ pageId: 'p-c', path: 'C.md' },
|
||||
],
|
||||
}),
|
||||
'sha-partial',
|
||||
);
|
||||
|
||||
// The 1st and 3rd were applied; the 2nd threw.
|
||||
expect(res.updated).toBe(2);
|
||||
expect(client.importPageMarkdown).toHaveBeenCalledTimes(3);
|
||||
expect(client.importPageMarkdown).toHaveBeenNthCalledWith(1, 'p-a', 'a body');
|
||||
expect(client.importPageMarkdown).toHaveBeenNthCalledWith(3, 'p-c', 'c body');
|
||||
|
||||
// The failure is recorded with kind/pageId/path/error.
|
||||
expect(res.failures).toEqual([
|
||||
{ kind: 'update', pageId: 'p-b', path: 'B.md', error: 'boom on page 2' },
|
||||
]);
|
||||
|
||||
// Only the successful pages carry a loop-guard push record.
|
||||
expect(res.pushed.map((p) => p.pageId)).toEqual(['p-a', 'p-c']);
|
||||
|
||||
// A PARTIAL push advances NEITHER ref, so a re-run retries cleanly (§12).
|
||||
expect(res.lastPushedAdvanced).toBe(false);
|
||||
expect(updateRefCalls).toEqual([]);
|
||||
expect(ffCalls).toEqual([]);
|
||||
expect(res.docmostFastForward).toBeNull();
|
||||
expect(git.updateRef).not.toHaveBeenCalled();
|
||||
expect(git.fastForwardBranch).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('applyPushActions — loop-guard push record (SPEC §10)', () => {
|
||||
it('records pageId + updatedAt + bodyHash per applied update', async () => {
|
||||
const fileBody =
|
||||
'<!-- docmost:meta\n{"version":1,"pageId":"p-1"}\n-->\n\nupdated body\n';
|
||||
const client = {
|
||||
importPageMarkdown: vi.fn(async (_pageId: string, _md: string) => ({
|
||||
// The write returns an updatedAt the loop-guard records.
|
||||
data: { updatedAt: '2026-06-20T10:00:00.000Z' },
|
||||
success: true,
|
||||
})),
|
||||
createPage: vi.fn(),
|
||||
deletePage: vi.fn(),
|
||||
};
|
||||
const { git } = makeGit();
|
||||
const fs = makeFs({ 'Doc.md': fileBody });
|
||||
|
||||
const res = await applyPushActions(
|
||||
deps(client, git, fs),
|
||||
actions({ updates: [{ pageId: 'p-1', path: 'Doc.md' }] }),
|
||||
);
|
||||
|
||||
expect(res.pushed).toHaveLength(1);
|
||||
expect(res.pushed[0].pageId).toBe('p-1');
|
||||
expect(res.pushed[0].updatedAt).toBe('2026-06-20T10:00:00.000Z');
|
||||
// The bodyHash is a stable sha256 hex of the pushed markdown.
|
||||
expect(res.pushed[0].bodyHash).toBe(bodyHash(fileBody));
|
||||
expect(res.pushed[0].bodyHash).toMatch(/^[0-9a-f]{64}$/);
|
||||
});
|
||||
|
||||
it('omits updatedAt when the client result does not expose one', async () => {
|
||||
const newFile = serializeDocmostMarkdownBody(
|
||||
{ version: 1, title: 'N', spaceId: 'sp' },
|
||||
'fresh body',
|
||||
);
|
||||
const client = makeClient({ createId: 'created-9' });
|
||||
const { git } = makeGit();
|
||||
const fs = makeFs({ 'N.md': newFile });
|
||||
|
||||
const res = await applyPushActions(
|
||||
deps(client, git, fs),
|
||||
actions({ creates: [{ path: 'N.md' }] }),
|
||||
);
|
||||
|
||||
expect(res.pushed).toHaveLength(1);
|
||||
expect(res.pushed[0].pageId).toBe('created-9');
|
||||
expect(res.pushed[0].updatedAt).toBeUndefined();
|
||||
// bodyHash of the ORIGINAL pushed file text (what createPage received).
|
||||
expect(res.pushed[0].bodyHash).toBe(bodyHash(newFile));
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -36,10 +36,40 @@ describe('computePushActions — A (added)', () => {
|
||||
expect(actions.skipped).toEqual([]);
|
||||
});
|
||||
|
||||
it('added file with NO meta at all -> create (treated as new)', () => {
|
||||
it('added file with NO meta at all -> skipped (a create needs a spaceId)', () => {
|
||||
// No meta -> no spaceId -> cannot create (Docmost create_page requires it).
|
||||
const changes: DiffEntry[] = [{ status: 'A', path: 'Plain.md' }];
|
||||
const actions = computePushActions({ changes, metaAt: metaTable({}) });
|
||||
expect(actions.creates).toEqual([{ path: 'Plain.md' }]);
|
||||
expect(actions.creates).toEqual([]);
|
||||
expect(actions.skipped).toEqual([
|
||||
{ path: 'Plain.md', status: 'A', reason: 'create-without-spaceId' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('added file with meta but NO spaceId -> skipped (create-without-spaceId)', () => {
|
||||
// Partial human meta (title only, no spaceId) -> refuse to create.
|
||||
const changes: DiffEntry[] = [{ status: 'A', path: 'Partial.md' }];
|
||||
const metaAt = metaTable({
|
||||
'Partial.md|current': meta({ title: 'Partial' }),
|
||||
});
|
||||
const actions = computePushActions({ changes, metaAt });
|
||||
expect(actions.creates).toEqual([]);
|
||||
expect(actions.skipped).toEqual([
|
||||
{ path: 'Partial.md', status: 'A', reason: 'create-without-spaceId' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('added file with an EMPTY-string spaceId -> skipped (create-without-spaceId)', () => {
|
||||
// An empty spaceId is not a usable target either.
|
||||
const changes: DiffEntry[] = [{ status: 'A', path: 'Empty.md' }];
|
||||
const metaAt = metaTable({
|
||||
'Empty.md|current': meta({ title: 'E', spaceId: '' }),
|
||||
});
|
||||
const actions = computePushActions({ changes, metaAt });
|
||||
expect(actions.creates).toEqual([]);
|
||||
expect(actions.skipped).toEqual([
|
||||
{ path: 'Empty.md', status: 'A', reason: 'create-without-spaceId' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('added file WITH a pageId (restored/copied) -> update (page exists)', () => {
|
||||
|
||||
@@ -622,4 +622,89 @@ describe('VaultGit (integration; temp repo)', () => {
|
||||
expect(preImage).toBe(meta);
|
||||
expect(preImage).toContain('page-123');
|
||||
});
|
||||
|
||||
it('fastForwardBranch advances a true fast-forward (the loop-close, SPEC §6 step 3)', async () => {
|
||||
if (!available) return;
|
||||
const vault = await freshDir();
|
||||
const git = new VaultGit(vault);
|
||||
await git.ensureRepo();
|
||||
|
||||
// docmost branches off main at the initial commit; main then moves ahead.
|
||||
await git.ensureBranch('docmost', 'main');
|
||||
const base = await git.revParse('refs/heads/docmost');
|
||||
|
||||
await writeFile(join(vault, 'page.md'), 'pushed content\n', 'utf8');
|
||||
await git.stageAll();
|
||||
await git.commit('push page', { authorName: BOT_AUTHOR_NAME, authorEmail: BOT_AUTHOR_EMAIL });
|
||||
const mainTip = await git.revParse('HEAD');
|
||||
|
||||
// docmost is BEHIND main and an ancestor -> a true fast-forward advances it.
|
||||
expect(await git.revParse('refs/heads/docmost')).toBe(base);
|
||||
const res = await git.fastForwardBranch('docmost', mainTip!);
|
||||
expect(res).toEqual({ ok: true });
|
||||
// The branch now points at the pushed main commit (mirror reflects Docmost).
|
||||
expect(await git.revParse('refs/heads/docmost')).toBe(mainTip);
|
||||
|
||||
// It does NOT touch the working tree / current branch (still on main).
|
||||
expect(await git.currentBranch()).toBe('main');
|
||||
});
|
||||
|
||||
it('fastForwardBranch is a no-op (ok) when the branch is already at the target', async () => {
|
||||
if (!available) return;
|
||||
const vault = await freshDir();
|
||||
const git = new VaultGit(vault);
|
||||
await git.ensureRepo();
|
||||
await git.ensureBranch('docmost', 'main');
|
||||
const mainTip = await git.revParse('HEAD');
|
||||
|
||||
// Already equal -> a degenerate fast-forward, still ok, branch unchanged.
|
||||
const res = await git.fastForwardBranch('docmost', mainTip!);
|
||||
expect(res).toEqual({ ok: true });
|
||||
expect(await git.revParse('refs/heads/docmost')).toBe(mainTip);
|
||||
});
|
||||
|
||||
it('fastForwardBranch REFUSES a non-fast-forward (never clobbers divergent history)', async () => {
|
||||
if (!available) return;
|
||||
const vault = await freshDir();
|
||||
const git = new VaultGit(vault);
|
||||
await git.ensureRepo();
|
||||
|
||||
// Make docmost diverge: it has a commit that main does NOT contain.
|
||||
await git.checkout('main'); // ensure on main first
|
||||
await git.ensureBranch('docmost', 'main');
|
||||
await git.checkout('docmost');
|
||||
await writeFile(join(vault, 'only-on-docmost.md'), 'mirror-only\n', 'utf8');
|
||||
await git.stageAll();
|
||||
await git.commit('docmost-only commit', { authorName: BOT_AUTHOR_NAME, authorEmail: BOT_AUTHOR_EMAIL });
|
||||
const docmostTip = await git.revParse('refs/heads/docmost');
|
||||
|
||||
// main moves ahead independently (divergent from docmost).
|
||||
await git.checkout('main');
|
||||
await writeFile(join(vault, 'only-on-main.md'), 'main-only\n', 'utf8');
|
||||
await git.stageAll();
|
||||
await git.commit('main-only commit', { authorName: BOT_AUTHOR_NAME, authorEmail: BOT_AUTHOR_EMAIL });
|
||||
const mainTip = await git.revParse('HEAD');
|
||||
|
||||
// docmost is NOT an ancestor of main -> the ff is REFUSED, branch untouched.
|
||||
const res = await git.fastForwardBranch('docmost', mainTip!);
|
||||
expect(res).toEqual({ ok: false, reason: 'not-fast-forward' });
|
||||
expect(await git.revParse('refs/heads/docmost')).toBe(docmostTip);
|
||||
});
|
||||
|
||||
it('fastForwardBranch refuses a missing branch / unresolved target with a reason', async () => {
|
||||
if (!available) return;
|
||||
const vault = await freshDir();
|
||||
const git = new VaultGit(vault);
|
||||
await git.ensureRepo();
|
||||
const mainTip = await git.revParse('HEAD');
|
||||
|
||||
const noBranch = await git.fastForwardBranch('nope', mainTip!);
|
||||
expect(noBranch.ok).toBe(false);
|
||||
expect(noBranch.reason).toContain('nope');
|
||||
|
||||
await git.ensureBranch('docmost', 'main');
|
||||
const noTarget = await git.fastForwardBranch('docmost', 'deadbeefdeadbeef');
|
||||
expect(noTarget.ok).toBe(false);
|
||||
expect(noTarget.reason).toContain('deadbeefdeadbeef');
|
||||
});
|
||||
});
|
||||
|
||||
41
test/loop-guard.test.ts
Normal file
41
test/loop-guard.test.ts
Normal file
@@ -0,0 +1,41 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { createHash } from 'node:crypto';
|
||||
import { bodyHash } from '../src/loop-guard.js';
|
||||
|
||||
// Loop-guard body hash (SPEC §10 "хэш тела"). The hash is the signal a future
|
||||
// pull-side poll-suppression uses to recognize our OWN write. It MUST be
|
||||
// deterministic (same input -> same hash) and discriminating (different input ->
|
||||
// different hash).
|
||||
|
||||
describe('bodyHash (pure, SPEC §10)', () => {
|
||||
it('is deterministic — same input yields the same hash', () => {
|
||||
const body = '# Title\n\nsome body with <span data-comment-id="x">mark</span>\n';
|
||||
expect(bodyHash(body)).toBe(bodyHash(body));
|
||||
});
|
||||
|
||||
it('differs for different input', () => {
|
||||
expect(bodyHash('alpha')).not.toBe(bodyHash('beta'));
|
||||
// Even a one-character difference produces a different digest.
|
||||
expect(bodyHash('alpha')).not.toBe(bodyHash('alphb'));
|
||||
});
|
||||
|
||||
it('returns lowercase sha256 hex (64 chars)', () => {
|
||||
const h = bodyHash('hello');
|
||||
expect(h).toMatch(/^[0-9a-f]{64}$/);
|
||||
// Matches an independent sha256 of the same UTF-8 bytes.
|
||||
expect(h).toBe(createHash('sha256').update('hello', 'utf8').digest('hex'));
|
||||
});
|
||||
|
||||
it('hashes the empty string to the well-known sha256 empty digest', () => {
|
||||
expect(bodyHash('')).toBe(
|
||||
'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855',
|
||||
);
|
||||
});
|
||||
|
||||
it('is sensitive to UTF-8 content (Cyrillic body)', () => {
|
||||
expect(bodyHash('Колонка')).not.toBe(bodyHash('Колонкa'));
|
||||
expect(bodyHash('Колонка')).toBe(
|
||||
createHash('sha256').update('Колонка', 'utf8').digest('hex'),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -1,5 +1,12 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import fc from 'fast-check';
|
||||
|
||||
// These property tests run real ProseMirror<->Markdown conversion × NUM_RUNS, so
|
||||
// each takes ~4–5s. Inputs are DETERMINISTIC (fixed SEED below) — the only source
|
||||
// of flakiness is wall-clock: under the full suite's parallel worker load they can
|
||||
// exceed vitest's default 5000ms per-test timeout. Give them ample headroom so CI
|
||||
// (which gates the docker build, AGENTS.md) is deterministic regardless of load.
|
||||
vi.setConfig({ testTimeout: 30000 });
|
||||
// Import the converter DIRECTLY from src (NOT the docmost-client barrel) so we
|
||||
// match the path used by the other converter unit tests.
|
||||
import { convertProseMirrorToMarkdown } from '../packages/docmost-client/src/lib/markdown-converter.js';
|
||||
|
||||
Reference in New Issue
Block a user