Files
gitmost/packages/git-sync/test/apply-pull-actions.test.ts
claude code agent 227 8b14a2e7b1 test(git-sync): exhaustive converter coverage + fix 3 round-trip data-loss bugs
Coder↔reviewer design loop (9 rounds, reviewer verdict: exhaustive) produced
92 specs; implemented +123 tests (465 -> 588 passing). The new round-trip
coverage exposed three genuine data-loss bugs in the Markdown<->ProseMirror
converter, all now FIXED (round-trip is lossless for these):

1. pageBreak was lost on export (no converter case -> rendered to "" and the
   node vanished). Now emits <div data-type="pageBreak"></div>, which the schema
   parses back -> round-trips.
2. A block image between blocks left an empty <p> artifact after import-hoisting,
   producing a phantom blank-gap diff on every sync. markdownToProseMirror now
   strips content-less paragraphs after generateJSON — with a schema-validity
   guard that keeps the obligatory single empty paragraph in `content: "block+"`
   containers (tableCell/tableHeader/blockquote/column/callout/doc), so empty
   cells/quotes never become an invalid `content: []`.
3. The `code` mark combined with another mark was not byte-stable (emitted nested
   HTML that the schema's `code` `excludes:"_"` collapsed on import). The
   converter now emits code-only when `code` co-occurs, matching the editor.

New coverage spans media/diagram/details/columns/math/mention attribute
round-trips, converter emission branches, git error paths, and engine decision
branches. A dedicated test pins the empty-container schema validity (the review
catch on the bug-2 fix).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-24 13:43:40 +03:00

665 lines
22 KiB
TypeScript

import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
import { applyPullActions } from '../src/engine/pull';
import type {
PullActions,
ApplyPullActionsDeps,
} from '../src/engine/pull';
import type { DeletionDecision } from '../src/engine/reconcile';
// R-Pull-2 (test-strategy report §5): `applyPullActions` is the THIN IO half of
// the pull cycle. These tests drive it with FAKES that record every call — no
// real git, fs, or network — so the ordering and the ⭐ move-on-success
// data-loss guard are verifiable. SPEC §8 (delete suppression) + SPEC §5 (commit
// subject reflects ACTUAL counts) are asserted here.
const VAULT = '/vault';
/** A getPageJson fake: returns a minimal page whose content stabilizes cheaply. */
function makeClient(opts?: { failFor?: Set<string> }) {
const calls: string[] = [];
const client = {
getPageJson: vi.fn(async (pageId: string) => {
calls.push(pageId);
if (opts?.failFor?.has(pageId)) {
throw new Error(`fetch failed for ${pageId}`);
}
return {
id: pageId,
slugId: `slug-${pageId}`,
title: `Title ${pageId}`,
spaceId: 'space',
parentPageId: null,
updatedAt: '2026-01-01T00:00:00.000Z',
// A trivial doc so stabilizePageFile (the real one) runs fast.
content: {
type: 'doc',
content: [
{ type: 'paragraph', content: [{ type: 'text', text: pageId }] },
],
},
};
}),
};
return { client, calls };
}
/** A git fake recording the order of ops; merge result is configurable. */
function makeGit(merge: { ok: boolean; conflict: boolean; output?: string } = {
ok: true,
conflict: false,
}) {
const order: string[] = [];
let committedSubject: string | undefined;
const git = {
stageAll: vi.fn(async () => {
order.push('stageAll');
}),
commit: vi.fn(async (subject: string) => {
order.push(`commit:${subject}`);
committedSubject = subject;
return true;
}),
checkout: vi.fn(async (branch: string) => {
order.push(`checkout:${branch}`);
}),
merge: vi.fn(async () => {
order.push('merge');
return { ok: merge.ok, conflict: merge.conflict, output: merge.output ?? '' };
}),
};
return {
git,
order,
get committedSubject() {
return committedSubject;
},
};
}
/** A recording fs fake: writes/mkdirs/rms tracked in arrays. */
function makeFs(opts?: { failWriteFor?: Set<string> }) {
const writes: { abs: string; text: string }[] = [];
const mkdirs: string[] = [];
const rms: string[] = [];
const fs = {
writeFile: vi.fn(async (abs: string, text: string) => {
// Fail a specific destination path if asked (to simulate a write failure).
if (opts?.failWriteFor?.has(abs)) {
throw new Error(`write failed for ${abs}`);
}
writes.push({ abs, text });
}),
mkdir: vi.fn(async (abs: string) => {
mkdirs.push(abs);
}),
rm: vi.fn(async (abs: string) => {
rms.push(abs);
}),
};
return { fs, writes, mkdirs, rms };
}
function deps(
client: any,
git: any,
fs: ReturnType<typeof makeFs>,
): ApplyPullActionsDeps {
return {
client,
git,
writeFile: fs.fs.writeFile,
mkdir: fs.fs.mkdir,
rm: fs.fs.rm,
};
}
const APPLY: DeletionDecision = { apply: true };
function actions(partial: Partial<PullActions>): PullActions {
return {
toWrite: [],
moved: [],
toDelete: [],
deletionDecision: APPLY,
existingCount: 0,
plannedDeleteCount: 0,
...partial,
};
}
beforeEach(() => {
vi.spyOn(console, 'log').mockImplementation(() => {});
vi.spyOn(console, 'warn').mockImplementation(() => {});
vi.spyOn(console, 'error').mockImplementation(() => {});
});
afterEach(() => {
vi.restoreAllMocks();
});
describe('applyPullActions — happy path (write + commit + merge)', () => {
it('fetches, writes each page, stages, commits, checks out main, merges', async () => {
const { client } = makeClient();
const g = makeGit();
const fs = makeFs();
const res = await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [
{ pageId: 'p1', relPath: 'A.md' },
{ pageId: 'p2', relPath: 'Sub/B.md' },
],
}),
VAULT,
);
expect(res.written).toBe(2);
expect(res.failed).toBe(0);
expect(res.committed).toBe(true);
expect(res.merge).toEqual({ ok: true, conflict: false, output: '' });
// Both pages were fetched and written at their absolute paths.
expect(client.getPageJson).toHaveBeenCalledTimes(2);
const writtenPaths = fs.writes.map((w) => w.abs).sort();
expect(writtenPaths).toEqual(['/vault/A.md', '/vault/Sub/B.md']);
// The git op order is: stageAll -> commit -> checkout main -> merge.
expect(g.order).toEqual([
'stageAll',
`commit:docmost: sync 2 page(s)`,
'checkout:main',
'merge',
]);
});
});
describe('applyPullActions — ordering (write before move/delete before commit)', () => {
it('does writes, then move-old-path removals, then deletes, then commit/merge', async () => {
const { client } = makeClient();
const g = makeGit();
const fs = makeFs();
await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [{ pageId: 'm', relPath: 'New/M.md' }],
moved: [
{
pageId: 'm',
fromRelPath: 'Old/M.md',
toRelPath: 'New/M.md',
removeOldPath: true,
},
],
toDelete: ['Dead.md'],
plannedDeleteCount: 1,
existingCount: 3,
}),
VAULT,
);
// The write to the new path happened (the page was fetched first).
expect(fs.writes.map((w) => w.abs)).toEqual(['/vault/New/M.md']);
// The move old-path removal AND the absence delete both ran, old path first.
expect(fs.rms).toEqual(['/vault/Old/M.md', '/vault/Dead.md']);
// git ops happen AFTER all fs work.
expect(g.order).toEqual([
'stageAll',
'commit:docmost: sync 1 page(s), 1 deleted',
'checkout:main',
'merge',
]);
});
});
describe('applyPullActions — ⭐ data-loss guard (move-on-success)', () => {
it('does NOT remove the OLD path when the new-path write FAILS', async () => {
// The page "m" is being moved Old/M.md -> New/M.md, but its new-path write
// FAILS. Removing the old path now would erase the only copy of the page.
// The guard must KEEP the old path.
const { client } = makeClient();
const g = makeGit();
const fs = makeFs({ failWriteFor: new Set(['/vault/New/M.md']) });
const res = await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [{ pageId: 'm', relPath: 'New/M.md' }],
moved: [
{
pageId: 'm',
fromRelPath: 'Old/M.md',
toRelPath: 'New/M.md',
removeOldPath: true,
},
],
}),
VAULT,
);
// The write failed -> recorded as a failure, nothing written.
expect(res.failed).toBe(1);
expect(res.written).toBe(0);
expect(fs.writes).toEqual([]);
// ⭐ The OLD path was NOT removed: the data-loss guard kept it.
expect(fs.rms).not.toContain('/vault/Old/M.md');
expect(fs.rms).toEqual([]);
expect(res.movedApplied).toBe(0);
// The commit subject reflects ACTUAL counts: 0 written, 0 deleted.
expect(g.committedSubject).toBe('docmost: sync 0 page(s)');
});
it('DOES remove the old path when the new-path write SUCCEEDS', async () => {
// Same move, but the write succeeds -> the old path is safely removed. This
// is the positive control proving the guard is keyed on write success.
const { client } = makeClient();
const g = makeGit();
const fs = makeFs(); // no write failures
const res = await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [{ pageId: 'm', relPath: 'New/M.md' }],
moved: [
{
pageId: 'm',
fromRelPath: 'Old/M.md',
toRelPath: 'New/M.md',
removeOldPath: true,
},
],
}),
VAULT,
);
expect(res.written).toBe(1);
expect(res.movedApplied).toBe(1);
expect(fs.rms).toContain('/vault/Old/M.md');
expect(g.committedSubject).toBe('docmost: sync 1 page(s)');
});
it('honours removeOldPath:false (path reused by another live page is kept)', async () => {
const { client } = makeClient();
const g = makeGit();
const fs = makeFs();
await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [{ pageId: 'm', relPath: 'New/M.md' }],
moved: [
{
pageId: 'm',
fromRelPath: 'X.md',
toRelPath: 'New/M.md',
removeOldPath: false, // X.md is a live target of another page
},
],
}),
VAULT,
);
// The reused old path is never removed.
expect(fs.rms).not.toContain('/vault/X.md');
expect(fs.rms).toEqual([]);
});
});
describe('applyPullActions — deletion suppression (SPEC §8)', () => {
it('skips deletions when the decision SUPPRESSES them (toDelete already empty)', async () => {
// computePullActions empties toDelete when suppressed, but assert the applier
// ALSO does no removals and the subject omits the deleted count.
const { client } = makeClient();
const g = makeGit();
const fs = makeFs();
const res = await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [{ pageId: 'p1', relPath: 'A.md' }],
// Suppressed: toDelete is empty even though 5 were planned.
toDelete: [],
deletionDecision: { apply: false, reason: 'incomplete-fetch' },
plannedDeleteCount: 5,
existingCount: 6,
}),
VAULT,
);
expect(res.deleted).toBe(0);
expect(fs.rms).toEqual([]);
// 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.stringMatching(/tree fetch incomplete/),
);
});
it('applies deletions present in toDelete when the decision allows them', async () => {
const { client } = makeClient();
const g = makeGit();
const fs = makeFs();
const res = await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [{ pageId: 'p1', relPath: 'A.md' }],
toDelete: ['Dead1.md', 'Dead2.md'],
deletionDecision: APPLY,
plannedDeleteCount: 2,
existingCount: 5,
}),
VAULT,
);
expect(res.deleted).toBe(2);
expect(fs.rms).toEqual(['/vault/Dead1.md', '/vault/Dead2.md']);
// Subject reflects ACTUAL written + deleted counts.
expect(g.committedSubject).toBe('docmost: sync 1 page(s), 2 deleted');
});
});
describe('applyPullActions — commit subject reflects ACTUAL counts', () => {
it('counts only SUCCESSFUL writes when some page fetches fail', async () => {
// p2 fetch fails; the subject must say 1 page (only p1 was written), not 2.
const { client } = makeClient({ failFor: new Set(['p2']) });
const g = makeGit();
const fs = makeFs();
const res = await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [
{ pageId: 'p1', relPath: 'A.md' },
{ pageId: 'p2', relPath: 'B.md' },
],
}),
VAULT,
);
expect(res.written).toBe(1);
expect(res.failed).toBe(1);
expect(g.committedSubject).toBe('docmost: sync 1 page(s)');
});
});
describe('applyPullActions — merge result is surfaced, not swallowed', () => {
it('returns conflict:true on a conflicting merge (no auto-resolve)', async () => {
const { client } = makeClient();
const g = makeGit({ ok: false, conflict: true, output: 'CONFLICT' });
const fs = makeFs();
const res = await applyPullActions(
deps(client, g.git, fs),
actions({ toWrite: [{ pageId: 'p1', relPath: 'A.md' }] }),
VAULT,
);
expect(res.merge.conflict).toBe(true);
expect(res.merge.ok).toBe(false);
});
it('returns ok:false conflict:false on a non-conflict merge failure', async () => {
const { client } = makeClient();
const g = makeGit({ ok: false, conflict: false, output: 'some error' });
const fs = makeFs();
const res = await applyPullActions(
deps(client, g.git, fs),
actions({ toWrite: [{ pageId: 'p1', relPath: 'A.md' }] }),
VAULT,
);
expect(res.merge.ok).toBe(false);
expect(res.merge.conflict).toBe(false);
});
});
// ===========================================================================
// R-Pull-2 coverage gaps (review-driven): the suppression warning FORKS for
// `empty-live` and `mass-delete` reasons (pull.ts 278-290), and the
// fault-tolerant `removePath` catch branch (pull.ts 354-364) where `deps.rm`
// REJECTS. The existing block above only exercises the `incomplete-fetch`
// reason and an rm that always succeeds.
//
// Helper: build a deps object whose `rm` rejects for a chosen set of absolute
// paths and resolves otherwise. We override the recording fs's `rm` (a vi.fn)
// in place so `fs.rms` still records the SUCCESSFUL calls only (a rejecting rm
// throws before pushing), matching the real `node:fs/promises` semantics where
// a thrown rm performed no removal.
function makeFsWithRejectingRm(rejectFor: Set<string>) {
const base = makeFs();
base.fs.rm = vi.fn(async (abs: string) => {
if (rejectFor.has(abs)) {
throw new Error(`rm failed for ${abs}`);
}
base.rms.push(abs);
});
return base;
}
describe('applyPullActions — suppression warning forks (empty-live / mass-delete)', () => {
it('emits the empty-live warning (with existingCount) and performs no removals', async () => {
// SPEC §8 empty-live fork: live fetch returned 0 pages but files are
// tracked. Mirrors the incomplete-fetch suppression test, but the message
// text + its `existingCount` interpolation are a DISTINCT branch.
const { client } = makeClient();
const g = makeGit();
const fs = makeFs();
const res = await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [{ pageId: 'p1', relPath: 'A.md' }],
toDelete: [], // suppressed -> already empty
deletionDecision: { apply: false, reason: 'empty-live' },
plannedDeleteCount: 3,
existingCount: 4,
}),
VAULT,
);
expect(res.deleted).toBe(0);
expect(fs.rms).toEqual([]);
// The empty-live message names the tracked-file count and "deletions
// suppressed".
expect(console.warn).toHaveBeenCalledWith(
expect.stringMatching(/live fetch returned 0 pages but 4 file\(s\) are tracked/),
);
expect(console.warn).toHaveBeenCalledWith(
expect.stringMatching(/deletions suppressed/),
);
});
it('emits the mass-delete guard warning (with planned AND existing counts) and performs no removals', async () => {
// SPEC §8 mass-delete fork (the final else branch): the message
// interpolates BOTH plannedDeleteCount and existingCount ("would delete N
// of M"), distinct from the other two suppression messages.
const { client } = makeClient();
const g = makeGit();
const fs = makeFs();
const res = await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [{ pageId: 'p1', relPath: 'A.md' }],
toDelete: [],
deletionDecision: { apply: false, reason: 'mass-delete' },
plannedDeleteCount: 5,
existingCount: 6,
}),
VAULT,
);
expect(res.deleted).toBe(0);
expect(fs.rms).toEqual([]);
expect(console.warn).toHaveBeenCalledWith(
expect.stringMatching(/plan would delete 5 of 6 tracked file\(s\) \(mass-delete guard\)/),
);
expect(console.warn).toHaveBeenCalledWith(
expect.stringMatching(/deletions suppressed/),
);
});
});
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
// that always succeeds, leaving this catch branch uncovered.
const { client } = makeClient();
const g = makeGit();
const fs = makeFsWithRejectingRm(new Set(['/vault/Dead.md']));
const res = await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [],
toDelete: ['Dead.md'],
deletionDecision: APPLY,
plannedDeleteCount: 1,
existingCount: 1,
}),
VAULT,
);
// 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.stringMatching(/failed to .* Dead\.md/),
expect.anything(),
);
// The (would-be) removal never succeeded, so nothing was recorded.
expect(fs.rms).toEqual([]);
});
it('counts ONLY successful removals on a partial-failure delete batch (1 reject of 3)', async () => {
// pull.ts 388-391 increments `deleted` only when removePath returns true.
// Here Dead1/Dead3 succeed and Dead2 rejects -> deleted === 2, and the
// deleted>0 subject branch (399-400) fires with written=0.
const { client } = makeClient();
const g = makeGit();
const fs = makeFsWithRejectingRm(new Set(['/vault/Dead2.md']));
const res = await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [],
moved: [],
toDelete: ['Dead1.md', 'Dead2.md', 'Dead3.md'],
deletionDecision: APPLY,
plannedDeleteCount: 3,
existingCount: 5,
}),
VAULT,
);
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(),
);
// The run still reached commit + checkout + merge.
expect(g.order).toEqual([
'stageAll',
'commit:docmost: sync 0 page(s), 2 deleted',
'checkout:main',
'merge',
]);
});
});
describe('applyPullActions — move old-path removal rejects vs move-write fails', () => {
it('a move old-path rm REJECTION does not increment movedApplied but an independent delete still succeeds', async () => {
// pull.ts 383 increments movedApplied only when removePath of the old path
// succeeds. Here the new-path write SUCCEEDS (so the page is not in
// failedPageIds and the move loop proceeds to rm) but the old-path rm
// REJECTS — distinct from the move-write-failure guard at 376. An absence
// delete in the same run must still succeed independently.
const { client } = makeClient();
const g = makeGit();
const fs = makeFsWithRejectingRm(new Set(['/vault/Old/M.md']));
const res = await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [{ pageId: 'm', relPath: 'New/M.md' }],
moved: [
{
pageId: 'm',
fromRelPath: 'Old/M.md',
toRelPath: 'New/M.md',
removeOldPath: true,
},
],
toDelete: ['Dead.md'],
deletionDecision: APPLY,
plannedDeleteCount: 1,
existingCount: 3,
}),
VAULT,
);
expect(res.written).toBe(1);
expect(res.movedApplied).toBe(0); // old-path rm failed -> not counted
expect(res.deleted).toBe(1); // independent absence delete still succeeded
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.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).
const { client } = makeClient();
const g = makeGit();
const fs = makeFs({ failWriteFor: new Set(['/vault/New/M.md']) });
const res = await applyPullActions(
deps(client, g.git, fs),
actions({
toWrite: [{ pageId: 'm', relPath: 'New/M.md' }],
moved: [
{
pageId: 'm',
fromRelPath: 'Old/M.md',
toRelPath: 'New/M.md',
removeOldPath: true,
},
],
toDelete: [],
deletionDecision: APPLY,
plannedDeleteCount: 0,
existingCount: 1,
}),
VAULT,
);
expect(res.written).toBe(0);
expect(res.movedApplied).toBe(0);
// The old path was NEVER removed (rm not even attempted for it).
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
.map((c: unknown[]) => String(c[0]))
.filter((m: string) => m.includes('move write for m failed'));
expect(warnCalls.length).toBe(1);
expect(warnCalls[0]).toContain('keeping old path Old/M.md');
// deleted === 0 -> no ", N deleted" suffix.
expect(g.committedSubject).toBe('docmost: sync 0 page(s)');
});
});