test(git-sync): add reviewer-requested coverage across engine, server, client
Implements the test cases called out in the PR #119 review threads (code-review, test-strategy report, red-team) — TESTS ONLY, no production code changes. packages/git-sync (vitest): - lib converter/markdown gaps: pageBreak data-loss (it.fails repro), subpages lossy round-trip, nested/fenced callouts, ol->taskList bridge, column.width number<->string drift, empty details. - engine units: parentFolderFile, planReconciliation swap/chained move, buildVaultLayout last-resort-by-id, firstDivergence, applyPushActions / applyPullActions failure isolation. - real temp-git integration: diffNameStatus -z rename+add/modify alignment, copy-line behavior, per-invocation committer identity (no leak into repo/global config). - ENFORCED type-level GitSyncClient contract via vitest typecheck over a *.test-d.ts file (tsconfig.vitest.json; build tsconfig untouched). apps/server (jest): - orchestrator: delete-cap neutralization + fail-safe, Redis lock / mutex skip ladder + release-on-throw, merge guard, pull/push order, remote template substitution, poll lifecycle. - page-change listener: loop-guard, debounce coalescing, id resolution, error swallowing. - vault registry, controller authz (trigger + status), env validation/getters, page.service git-sync provenance stamping, persistence precedence (agent > git-sync > user) + no boundary snapshot, space.service audit-delta, space.repo jsonb-merge, converter-gate corpus extension (mention/math/details/marks). apps/client (vitest + testing-library): - history-item git-sync badge: render gating + non-clickable. - edit-space-form toggle: initial state, optimistic payload, rollback on error, disabled states. 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
876a401268
commit
f1a894ab79
435
packages/git-sync/test/engine-gaps.test.ts
Normal file
435
packages/git-sync/test/engine-gaps.test.ts
Normal file
@@ -0,0 +1,435 @@
|
||||
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { parentFolderFile, applyPushActions } from '../src/engine/push';
|
||||
import type { ApplyPushDeps, PushActions } from '../src/engine/push';
|
||||
import { planReconciliation } from '../src/engine/reconcile';
|
||||
import { buildVaultLayout, type PageNode } from '../src/engine/layout';
|
||||
import { sanitizeTitle } from '../src/engine/sanitize';
|
||||
import { firstDivergence } from '../src/engine/roundtrip-helpers';
|
||||
import { applyPullActions } from '../src/engine/pull';
|
||||
import type { PullActions, ApplyPullActionsDeps } from '../src/engine/pull';
|
||||
import type { DeletionDecision } from '../src/engine/reconcile';
|
||||
import { serializeDocmostMarkdownBody } from '../src/lib/index';
|
||||
|
||||
// Engine-layer coverage gaps flagged by the PR #119 reviewers (test-strategy
|
||||
// report, Module 2 `src/engine`). Each block targets a specific under-covered
|
||||
// branch directly. PURE units (no IO) are driven by plain inputs; the push/pull
|
||||
// appliers are driven by FAKES that record calls — no real git/fs/network.
|
||||
|
||||
// --- 1. push.ts:parentFolderFile — move<->rename classification lynchpin -----
|
||||
//
|
||||
// `parentFolderFile(path)` returns the parent FOLDER's `.md` file for a vault-
|
||||
// relative path (the enclosing folder one level up, SPEC §5 path-as-truth), or
|
||||
// `null` for a root-level path with no enclosing folder. It is the lynchpin of
|
||||
// the move-vs-rename classifier, so it is tested directly here (it was only
|
||||
// covered indirectly before): root-level, deep nesting, and — critically —
|
||||
// names CONTAINING DOTS (the lastIndexOf('/') split must not be confused by a
|
||||
// dot in a segment; only the LAST slash matters).
|
||||
describe('parentFolderFile (push.ts)', () => {
|
||||
it('returns null for a root-level path (no enclosing folder)', () => {
|
||||
expect(parentFolderFile('Child.md')).toBeNull();
|
||||
// A bare name with no slash at all is also root-level.
|
||||
expect(parentFolderFile('README.md')).toBeNull();
|
||||
});
|
||||
|
||||
it('returns the immediate enclosing folder file for a one-level path', () => {
|
||||
expect(parentFolderFile('Space/Child.md')).toBe('Space.md');
|
||||
});
|
||||
|
||||
it('returns the DEEPEST enclosing folder file for a deeply nested path', () => {
|
||||
// Only the last slash matters: the parent is the immediate folder, turned
|
||||
// into its `<folder>.md` page file (NOT the space root).
|
||||
expect(parentFolderFile('Space/Parent/Sub/Child.md')).toBe(
|
||||
'Space/Parent/Sub.md',
|
||||
);
|
||||
});
|
||||
|
||||
it('handles names CONTAINING DOTS without splitting on the dot', () => {
|
||||
// A dot in a folder/file segment must not be mistaken for the path split.
|
||||
// The split is purely on the LAST '/', so the `.md` is appended to the whole
|
||||
// parent dir verbatim (dots and all).
|
||||
expect(parentFolderFile('Space/v1.2.3/Child.md')).toBe('Space/v1.2.3.md');
|
||||
expect(parentFolderFile('a.b/c.d.md')).toBe('a.b.md');
|
||||
// A dotted root-level name still has no enclosing folder.
|
||||
expect(parentFolderFile('v1.2.3.md')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// --- 2. reconcile.ts:planReconciliation — chained/swap move (no data loss) ----
|
||||
//
|
||||
// A collision where one move's TARGET equals another move's OLD path is the
|
||||
// classic data-loss trap: naively removing the second move's old path would
|
||||
// clobber the first move's freshly-written file. The planner must flag the
|
||||
// reused old path `removeOldPath:false` so the caller never removes it. Both the
|
||||
// chained-move and the full swap are asserted (no clobber, no loss).
|
||||
describe('planReconciliation (reconcile.ts) — chained / swap move', () => {
|
||||
it('chained move: A target == B old path -> B keeps its old path (no clobber)', () => {
|
||||
// B is at b.md and moves to c.md; A is at a.md and moves to b.md. A's TARGET
|
||||
// path (b.md) is exactly B's OLD path. Removing b.md for B's move would
|
||||
// destroy A's just-written file, so B's move must record removeOldPath:false.
|
||||
const live = [
|
||||
{ pageId: 'A', relPath: 'b.md' },
|
||||
{ pageId: 'B', relPath: 'c.md' },
|
||||
];
|
||||
const existing = [
|
||||
{ pageId: 'A', relPath: 'a.md' },
|
||||
{ pageId: 'B', relPath: 'b.md' },
|
||||
];
|
||||
const plan = planReconciliation(live, existing);
|
||||
|
||||
// Both pages are (re)written at their new paths; nothing is absence-deleted.
|
||||
expect(plan.toWrite).toEqual([
|
||||
{ pageId: 'A', relPath: 'b.md' },
|
||||
{ pageId: 'B', relPath: 'c.md' },
|
||||
]);
|
||||
expect(plan.toDelete).toEqual([]);
|
||||
|
||||
const moveOf = (id: string) => plan.moved.find((m) => m.pageId === id)!;
|
||||
// A's old path (a.md) is free -> safe to remove.
|
||||
expect(moveOf('A')).toEqual({
|
||||
pageId: 'A',
|
||||
fromRelPath: 'a.md',
|
||||
toRelPath: 'b.md',
|
||||
removeOldPath: true,
|
||||
});
|
||||
// B's old path (b.md) is reused by A's write -> MUST NOT be removed.
|
||||
expect(moveOf('B')).toEqual({
|
||||
pageId: 'B',
|
||||
fromRelPath: 'b.md',
|
||||
toRelPath: 'c.md',
|
||||
removeOldPath: false,
|
||||
});
|
||||
});
|
||||
|
||||
it('swap move: A<->B exchange paths -> BOTH old paths are kept (no loss)', () => {
|
||||
// A and B swap: A a.md -> b.md, B b.md -> a.md. Each old path is the OTHER
|
||||
// page's live target, so NEITHER may be removed (the writes own them).
|
||||
const live = [
|
||||
{ pageId: 'A', relPath: 'b.md' },
|
||||
{ pageId: 'B', relPath: 'a.md' },
|
||||
];
|
||||
const existing = [
|
||||
{ pageId: 'A', relPath: 'a.md' },
|
||||
{ pageId: 'B', relPath: 'b.md' },
|
||||
];
|
||||
const plan = planReconciliation(live, existing);
|
||||
|
||||
expect(plan.toDelete).toEqual([]);
|
||||
// Both pages written at their swapped destinations.
|
||||
expect(plan.toWrite).toEqual([
|
||||
{ pageId: 'A', relPath: 'b.md' },
|
||||
{ pageId: 'B', relPath: 'a.md' },
|
||||
]);
|
||||
// Both moves recorded, both with removeOldPath:false (the swap is loss-free).
|
||||
expect(plan.moved).toEqual([
|
||||
{
|
||||
pageId: 'A',
|
||||
fromRelPath: 'a.md',
|
||||
toRelPath: 'b.md',
|
||||
removeOldPath: false,
|
||||
},
|
||||
{
|
||||
pageId: 'B',
|
||||
fromRelPath: 'b.md',
|
||||
toRelPath: 'a.md',
|
||||
removeOldPath: false,
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
// --- 3. layout.ts:buildVaultLayout — last-resort-by-id branch (~L135-139) ------
|
||||
//
|
||||
// The final full-path uniqueness pass has two fallbacks for a colliding leaf:
|
||||
// first re-stem with the sanitized slugId, and — if STILL colliding — append the
|
||||
// globally-unique sanitized pageId as a last resort. That id branch is reached
|
||||
// when FOUR pages share the SAME title AND slugId in the SAME (orphan) bucket:
|
||||
// the name pass only calls `disambiguate` ONCE, so the 3rd and 4th pages collide
|
||||
// in the FINAL pass, where the 4th's slugId-disambiguated stem ALSO collides
|
||||
// (with the 3rd's), forcing the id suffix.
|
||||
describe('buildVaultLayout (layout.ts) — last-resort-by-id disambiguation', () => {
|
||||
it('falls through to the globally-unique pageId when title+slugId both collide', () => {
|
||||
// Four orphans (parent outside the input set -> they all bucket at the root)
|
||||
// with identical title "A" and identical slugId "s".
|
||||
const pages: PageNode[] = [
|
||||
{ id: 'id1', title: 'A', slugId: 's', parentPageId: 'missing' },
|
||||
{ id: 'id2', title: 'A', slugId: 's', parentPageId: 'missing' },
|
||||
{ id: 'id3', title: 'A', slugId: 's', parentPageId: 'missing' },
|
||||
{ id: 'id4', title: 'A', slugId: 's', parentPageId: 'missing' },
|
||||
];
|
||||
const layout = buildVaultLayout(pages);
|
||||
|
||||
// The disambiguation ladder:
|
||||
// id1 -> "A" (name pass, free)
|
||||
// id2 -> "A ~s" (name pass, slugId suffix)
|
||||
// id3 -> "A ~s ~s" (FINAL pass, first attempt: slugId suffix)
|
||||
// id4 -> "A ~s ~s ~id4" (FINAL pass, LAST RESORT: sanitized pageId suffix)
|
||||
expect(layout.get('id1')!.stem).toBe('A');
|
||||
expect(layout.get('id2')!.stem).toBe('A ~s');
|
||||
expect(layout.get('id3')!.stem).toBe('A ~s ~s');
|
||||
// The last-resort branch appends the sanitized id (globally unique).
|
||||
expect(layout.get('id4')!.stem).toBe(`A ~s ~s ~${sanitizeTitle('id4')}`);
|
||||
|
||||
// All four full paths are unique (the invariant the branch protects).
|
||||
const pathOf = (e: { segments: string[]; stem: string }) =>
|
||||
[...e.segments, e.stem].join('/');
|
||||
const paths = ['id1', 'id2', 'id3', 'id4'].map((id) =>
|
||||
pathOf(layout.get(id)!),
|
||||
);
|
||||
expect(new Set(paths).size).toBe(4);
|
||||
// All orphans bucket at the vault root (segments: []).
|
||||
for (const id of ['id1', 'id2', 'id3', 'id4']) {
|
||||
expect(layout.get(id)!.segments).toEqual([]);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// --- 4. roundtrip-helpers.ts:firstDivergence — exported but 0% covered --------
|
||||
//
|
||||
// `firstDivergence(a, b)` deep-compares two values and returns either `null`
|
||||
// (equal) or `{ path, a, b }` locating the FIRST point of difference. Contract
|
||||
// learned by reading the function: arrays compare length first (`$.length`),
|
||||
// nested paths build a JSON-pointer-ish `$.x.y[i].z`, and a type/null mismatch
|
||||
// is reported at the current path with the raw differing values.
|
||||
describe('firstDivergence (roundtrip-helpers.ts)', () => {
|
||||
it('returns null for deeply equal values (no divergence)', () => {
|
||||
expect(firstDivergence({ a: 1, b: [1, 2, { c: 'x' }] }, { a: 1, b: [1, 2, { c: 'x' }] })).toBeNull();
|
||||
expect(firstDivergence(42, 42)).toBeNull();
|
||||
expect(firstDivergence(null, null)).toBeNull();
|
||||
expect(firstDivergence([], [])).toBeNull();
|
||||
});
|
||||
|
||||
it('locates a divergence at a leaf by path', () => {
|
||||
expect(firstDivergence({ a: 1 }, { a: 2 })).toEqual({ path: '$.a', a: 1, b: 2 });
|
||||
});
|
||||
|
||||
it('locates a divergence deep inside a nested array/object by path', () => {
|
||||
const d = firstDivergence(
|
||||
{ x: { y: [1, { z: 'a' }] } },
|
||||
{ x: { y: [1, { z: 'b' }] } },
|
||||
);
|
||||
expect(d).toEqual({ path: '$.x.y[1].z', a: 'a', b: 'b' });
|
||||
});
|
||||
|
||||
it('reports an array length mismatch at `<path>.length`', () => {
|
||||
expect(firstDivergence([1, 2], [1, 2, 3])).toEqual({
|
||||
path: '$.length',
|
||||
a: 2,
|
||||
b: 3,
|
||||
});
|
||||
});
|
||||
|
||||
it('reports a type mismatch (and null vs object) at the current path', () => {
|
||||
expect(firstDivergence(1, '1')).toEqual({ path: '$', a: 1, b: '1' });
|
||||
expect(firstDivergence(null, {})).toEqual({ path: '$', a: null, b: {} });
|
||||
// array vs object at the same path
|
||||
expect(firstDivergence([], {})).toEqual({ path: '$', a: [], b: {} });
|
||||
});
|
||||
});
|
||||
|
||||
// --- 5. push.ts:applyPushActions — prefetch-move failure isolation ------------
|
||||
//
|
||||
// The reviewer asked to exercise the per-entry try/catch around the rename/move
|
||||
// PREFETCH (push.ts ~L644-672): one move's prefetch should fail in isolation
|
||||
// while OTHER actions still apply. IMPORTANT FINDING (documented, not a skip of
|
||||
// the invariant): the prefetch helpers (`resolveParentPageIdViaTree`,
|
||||
// `metaAtViaTree`) SWALLOW their own IO errors internally (each wraps readFile /
|
||||
// showFileAtRef / parseDocmostMarkdown in try/catch and returns null), so an
|
||||
// injected `readFile`/`showFileAtRef` throw NEVER propagates into the L644-672
|
||||
// catch — that catch is defensive dead code reachable only by a future change to
|
||||
// the helpers (the source comment says exactly this). It therefore cannot be hit
|
||||
// through the public deps WITHOUT modifying production code (forbidden here).
|
||||
//
|
||||
// What IS testable — and is the invariant the reviewer cares about — is the
|
||||
// OBSERVABLE isolation: a move whose tree files are unreadable is isolated (it
|
||||
// resolves to a no-op Docmost call, never aborting the batch) while updates,
|
||||
// creates and deletes in the SAME batch still apply, and the refs still advance.
|
||||
describe('applyPushActions (push.ts) — move prefetch isolation', () => {
|
||||
beforeEach(() => {
|
||||
vi.spyOn(console, 'log').mockImplementation(() => {});
|
||||
vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
});
|
||||
afterEach(() => vi.restoreAllMocks());
|
||||
|
||||
function makeClient() {
|
||||
return {
|
||||
importPageMarkdown: vi.fn(async () => ({ updatedAt: 'u' })),
|
||||
createPage: vi.fn(async () => ({ data: { id: 'new-id' } })),
|
||||
deletePage: vi.fn(async () => ({})),
|
||||
movePage: vi.fn(async () => ({})),
|
||||
renamePage: vi.fn(async () => ({})),
|
||||
};
|
||||
}
|
||||
|
||||
it('isolates a move whose tree reads are unreadable; other actions still apply', async () => {
|
||||
const client = makeClient();
|
||||
const git = {
|
||||
updateRef: vi.fn(async () => {}),
|
||||
fastForwardBranch: vi.fn(async () => ({ ok: true })),
|
||||
// The OLD-side parent/meta reads resolve to null (absent at last-pushed).
|
||||
showFileAtRef: vi.fn(async () => null),
|
||||
};
|
||||
// The update file exists and is readable; the move's NEW-path tree reads
|
||||
// throw (simulating an unreadable/missing parent folder file at `current`).
|
||||
const store: Record<string, string> = {
|
||||
'Up.md': serializeDocmostMarkdownBody(
|
||||
{ version: 1, pageId: 'u1', title: 'U', spaceId: 'sp' } as any,
|
||||
'body',
|
||||
),
|
||||
};
|
||||
const deps: ApplyPushDeps = {
|
||||
client,
|
||||
git,
|
||||
readFile: vi.fn(async (p: string) => {
|
||||
if (p in store) return store[p];
|
||||
throw new Error(`unreadable ${p}`);
|
||||
}),
|
||||
writeFile: vi.fn(async () => {}),
|
||||
};
|
||||
const actions: PushActions = {
|
||||
creates: [],
|
||||
updates: [{ pageId: 'u1', path: 'Up.md' }],
|
||||
deletes: [{ pageId: 'd1' }],
|
||||
renamesMoves: [
|
||||
{ pageId: 'pg', oldPath: 'Old/C.md', newPath: 'New/C.md' },
|
||||
],
|
||||
skipped: [],
|
||||
};
|
||||
|
||||
const res = await applyPushActions(deps, actions, 'COMMIT-SHA');
|
||||
|
||||
// The update and the delete in the SAME batch still applied.
|
||||
expect(res.updated).toBe(1);
|
||||
expect(res.deleted).toBe(1);
|
||||
expect(client.importPageMarkdown).toHaveBeenCalledWith('u1', store['Up.md']);
|
||||
expect(client.deletePage).toHaveBeenCalledWith('d1');
|
||||
|
||||
// The broken move was ISOLATED: no movePage/renamePage call, recorded as a
|
||||
// graceful no-op (both parents resolve to ROOT/null, no title -> nothing to
|
||||
// do), NOT a fatal error.
|
||||
expect(client.movePage).not.toHaveBeenCalled();
|
||||
expect(client.renamePage).not.toHaveBeenCalled();
|
||||
expect(res.moved).toBe(0);
|
||||
expect(res.renamed).toBe(0);
|
||||
expect(res.noops).toHaveLength(1);
|
||||
expect(res.noops[0]).toMatchObject({ pageId: 'pg', reason: 'path-only-rename' });
|
||||
|
||||
// No failures -> the refs advance (a clean batch is not blocked by the
|
||||
// isolated, gracefully-handled move).
|
||||
expect(res.failures).toEqual([]);
|
||||
expect(res.lastPushedAdvanced).toBe(true);
|
||||
expect(git.updateRef).toHaveBeenCalledWith(expect.any(String), 'COMMIT-SHA');
|
||||
});
|
||||
});
|
||||
|
||||
// --- 6. pull.ts:applyPullActions — failedPageIds keyed per-pageId -------------
|
||||
//
|
||||
// `failedPageIds` is keyed by pageId: when MULTIPLE moves each want their old
|
||||
// path removed, but ONE page's new-path write fails, ONLY that page's old path
|
||||
// must be KEPT (the ⭐ data-loss guard) — every OTHER page's old path is still
|
||||
// removed. This proves the set is keyed by pageId (the failing one only), not a
|
||||
// coarse all-or-nothing gate.
|
||||
describe('applyPullActions (pull.ts) — failedPageIds keyed per-pageId', () => {
|
||||
const VAULT = '/vault';
|
||||
const APPLY: DeletionDecision = { apply: true };
|
||||
|
||||
beforeEach(() => {
|
||||
vi.spyOn(console, 'log').mockImplementation(() => {});
|
||||
vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
});
|
||||
afterEach(() => vi.restoreAllMocks());
|
||||
|
||||
function makeClient() {
|
||||
return {
|
||||
getPageJson: vi.fn(async (pageId: string) => ({
|
||||
id: pageId,
|
||||
slugId: `slug-${pageId}`,
|
||||
title: `Title ${pageId}`,
|
||||
spaceId: 'space',
|
||||
parentPageId: null,
|
||||
updatedAt: '2026-01-01T00:00:00.000Z',
|
||||
content: {
|
||||
type: 'doc',
|
||||
content: [
|
||||
{ type: 'paragraph', content: [{ type: 'text', text: pageId }] },
|
||||
],
|
||||
},
|
||||
})),
|
||||
};
|
||||
}
|
||||
function makeGit() {
|
||||
return {
|
||||
stageAll: vi.fn(async () => {}),
|
||||
commit: vi.fn(async () => true),
|
||||
checkout: vi.fn(async () => {}),
|
||||
merge: vi.fn(async () => ({ ok: true, conflict: false, output: '' })),
|
||||
};
|
||||
}
|
||||
function makeFs(failWriteFor: Set<string>) {
|
||||
const rms: string[] = [];
|
||||
const fs = {
|
||||
writeFile: vi.fn(async (abs: string) => {
|
||||
if (failWriteFor.has(abs)) throw new Error(`write failed for ${abs}`);
|
||||
}),
|
||||
mkdir: vi.fn(async () => {}),
|
||||
rm: vi.fn(async (abs: string) => {
|
||||
rms.push(abs);
|
||||
}),
|
||||
};
|
||||
return { fs, rms };
|
||||
}
|
||||
|
||||
it('keeps ONLY the failing page old path; the other moves still remove theirs', async () => {
|
||||
// Two moves, both removeOldPath:true. Page "ok" writes fine; page "bad"
|
||||
// fails its new-path write. Only "bad"'s old path must be kept.
|
||||
const client = makeClient();
|
||||
const git = makeGit();
|
||||
const fs = makeFs(new Set(['/vault/NewBad/Bad.md']));
|
||||
|
||||
const deps: ApplyPullActionsDeps = {
|
||||
client,
|
||||
git,
|
||||
writeFile: fs.fs.writeFile,
|
||||
mkdir: fs.fs.mkdir,
|
||||
rm: fs.fs.rm,
|
||||
};
|
||||
const actions: PullActions = {
|
||||
toWrite: [
|
||||
{ pageId: 'ok', relPath: 'NewOk/Ok.md' },
|
||||
{ pageId: 'bad', relPath: 'NewBad/Bad.md' },
|
||||
],
|
||||
moved: [
|
||||
{
|
||||
pageId: 'ok',
|
||||
fromRelPath: 'OldOk/Ok.md',
|
||||
toRelPath: 'NewOk/Ok.md',
|
||||
removeOldPath: true,
|
||||
},
|
||||
{
|
||||
pageId: 'bad',
|
||||
fromRelPath: 'OldBad/Bad.md',
|
||||
toRelPath: 'NewBad/Bad.md',
|
||||
removeOldPath: true,
|
||||
},
|
||||
],
|
||||
toDelete: [],
|
||||
deletionDecision: APPLY,
|
||||
existingCount: 2,
|
||||
plannedDeleteCount: 0,
|
||||
};
|
||||
|
||||
const res = await applyPullActions(deps, actions, VAULT);
|
||||
|
||||
// One write succeeded ("ok"), one failed ("bad").
|
||||
expect(res.written).toBe(1);
|
||||
expect(res.failed).toBe(1);
|
||||
|
||||
// The healthy page's old path WAS removed; the failing page's old path was
|
||||
// KEPT (failedPageIds is keyed by pageId -> only "bad" is suppressed).
|
||||
expect(fs.rms).toContain('/vault/OldOk/Ok.md');
|
||||
expect(fs.rms).not.toContain('/vault/OldBad/Bad.md');
|
||||
// Exactly one move old-path removal applied (the healthy one).
|
||||
expect(res.movedApplied).toBe(1);
|
||||
expect(fs.rms).toEqual(['/vault/OldOk/Ok.md']);
|
||||
});
|
||||
});
|
||||
236
packages/git-sync/test/git-integration-gaps.test.ts
Normal file
236
packages/git-sync/test/git-integration-gaps.test.ts
Normal file
@@ -0,0 +1,236 @@
|
||||
import { execFile } from 'node:child_process';
|
||||
import { copyFile, 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 } from 'vitest';
|
||||
import {
|
||||
VaultGit,
|
||||
BOT_AUTHOR_NAME,
|
||||
BOT_AUTHOR_EMAIL,
|
||||
} from '../src/engine/git';
|
||||
|
||||
// Integration coverage gaps for `git.ts` flagged by the PR #119 reviewers
|
||||
// (test-strategy report, Module 2). These create REAL temp git repos (mirroring
|
||||
// test/git.test.ts's setup/teardown) to exercise the actual `git` binary, since
|
||||
// the behaviors under test (the `-z` NUL-token alignment, copy detection, and
|
||||
// per-invocation committer identity) only manifest against real git.
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
|
||||
/** True if a usable `git` binary is on PATH (skip gracefully otherwise). */
|
||||
async function gitAvailable(): Promise<boolean> {
|
||||
try {
|
||||
await execFileAsync('git', ['--version']);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/** Read the author "Name <email>" of HEAD in a repo dir. */
|
||||
async function headAuthor(dir: string): Promise<string> {
|
||||
const { stdout } = await execFileAsync(
|
||||
'git',
|
||||
['--no-pager', 'log', '-1', '--pretty=%an <%ae>'],
|
||||
{ cwd: dir },
|
||||
);
|
||||
return stdout.trim();
|
||||
}
|
||||
|
||||
/** Read the committer "Name <email>" of HEAD in a repo dir. */
|
||||
async function headCommitter(dir: string): Promise<string> {
|
||||
const { stdout } = await execFileAsync(
|
||||
'git',
|
||||
['--no-pager', 'log', '-1', '--pretty=%cn <%ce>'],
|
||||
{ cwd: dir },
|
||||
);
|
||||
return stdout.trim();
|
||||
}
|
||||
|
||||
/** Read a LOCAL git config value (or '' if unset) in a repo dir. */
|
||||
async function localConfig(dir: string, key: string): Promise<string> {
|
||||
const r = await execFileAsync('git', ['config', '--local', '--get', key], {
|
||||
cwd: dir,
|
||||
}).catch(() => ({ stdout: '' }) as { stdout: string });
|
||||
return r.stdout.trim();
|
||||
}
|
||||
|
||||
describe('VaultGit integration gaps (temp repo)', () => {
|
||||
let available = false;
|
||||
let dir: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
available = await gitAvailable();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
if (dir) {
|
||||
await rm(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
async function freshDir(): Promise<string> {
|
||||
dir = await mkdtemp(join(tmpdir(), 'docmost-vault-gap-'));
|
||||
return dir;
|
||||
}
|
||||
|
||||
// --- 7. diffNameStatus: rename mixed with add + modify in ONE diff ----------
|
||||
//
|
||||
// The `-z` parser walks NUL-delimited tokens pulling 1 or 2 path tokens per
|
||||
// status (R/C take TWO: old + new; A/M/D take ONE). A misalignment — pulling
|
||||
// the wrong number of tokens for any row — would SHIFT every subsequent path
|
||||
// and misclassify a move as a delete (or vice versa). This test mixes an R
|
||||
// (rename) with an A (add) and an M (modify) in a SINGLE diff so the walk MUST
|
||||
// stay aligned across the 2-token R row and the 1-token A/M rows.
|
||||
it('diffNameStatus keeps -z token alignment with R + A + M in one diff', async (ctx) => {
|
||||
// Truly SKIP (not silently pass) when git is unavailable — a green result on
|
||||
// a git-less machine would falsely claim this integration ran.
|
||||
if (!available) ctx.skip();
|
||||
const vault = await freshDir();
|
||||
const git = new VaultGit(vault);
|
||||
await git.ensureRepo();
|
||||
|
||||
// Base commit: `keep.md` (to be modified) and `old-name.md` (to be renamed).
|
||||
const renameBody = 'line a\nline b\nline c\nline d\n';
|
||||
await writeFile(join(vault, 'keep.md'), 'v1\n', 'utf8');
|
||||
await writeFile(join(vault, 'old-name.md'), renameBody, 'utf8');
|
||||
await git.stageAll();
|
||||
await git.commit('base', {
|
||||
authorName: BOT_AUTHOR_NAME,
|
||||
authorEmail: BOT_AUTHOR_EMAIL,
|
||||
});
|
||||
const base = await git.revParse('HEAD');
|
||||
expect(base).toBeTruthy();
|
||||
|
||||
// Second commit: MODIFY keep.md, ADD fresh.md, RENAME old-name.md ->
|
||||
// new-name.md (identical content so -M detects a rename, not delete+add).
|
||||
await writeFile(join(vault, 'keep.md'), 'v2\n', 'utf8');
|
||||
await writeFile(join(vault, 'fresh.md'), 'brand new\n', 'utf8');
|
||||
await rm(join(vault, 'old-name.md'));
|
||||
await writeFile(join(vault, 'new-name.md'), renameBody, 'utf8');
|
||||
await git.stageAll();
|
||||
await git.commit('mixed change', {
|
||||
authorName: BOT_AUTHOR_NAME,
|
||||
authorEmail: BOT_AUTHOR_EMAIL,
|
||||
});
|
||||
|
||||
const entries = await git.diffNameStatus(base!, 'HEAD');
|
||||
const byPath = new Map(entries.map((e) => [e.path, e]));
|
||||
|
||||
// The modify and the add are each classified correctly (1 path token each).
|
||||
expect(byPath.get('keep.md')).toEqual({ status: 'M', path: 'keep.md' });
|
||||
expect(byPath.get('fresh.md')).toEqual({ status: 'A', path: 'fresh.md' });
|
||||
|
||||
// The rename is a SINGLE R row carrying BOTH old + new paths (2 path tokens)
|
||||
// — proof the walk consumed exactly two tokens here and stayed aligned. If
|
||||
// alignment were off, the rename would surface as a D (delete) of
|
||||
// old-name.md and/or an A of new-name.md instead.
|
||||
const r = byPath.get('new-name.md');
|
||||
expect(r?.status).toBe('R');
|
||||
expect(r?.oldPath).toBe('old-name.md');
|
||||
expect(r?.score).toBe(100);
|
||||
|
||||
// Exactly three rows, and crucially NO stray D/A for the renamed file (which
|
||||
// is the tell-tale of a -z misalignment).
|
||||
expect(entries.length).toBe(3);
|
||||
expect(entries.some((e) => e.status === 'D')).toBe(false);
|
||||
expect(byPath.has('old-name.md')).toBe(false);
|
||||
});
|
||||
|
||||
// --- 8. diffNameStatus: copy (C) status lines -------------------------------
|
||||
//
|
||||
// DOCUMENTED OUTCOME (reported as such): `C` (copy) rows are NOT reachable
|
||||
// through the engine's actual git invocation. `diffNameStatus` invokes
|
||||
// `git diff --name-status -M -z` — `-M` enables rename detection ONLY; copy
|
||||
// detection requires `-C`/`--find-copies`, which the engine does NOT pass. So a
|
||||
// file that is a verbatim COPY of another (the original is KEPT) is reported as
|
||||
// a plain ADD (`A`), never `C`. This test pins that real behavior so a future
|
||||
// change that turns on `-C` (and would start emitting `C` rows) is caught.
|
||||
it('diffNameStatus reports a pure copy as A, not C (engine uses -M only)', async (ctx) => {
|
||||
if (!available) ctx.skip();
|
||||
const vault = await freshDir();
|
||||
const git = new VaultGit(vault);
|
||||
await git.ensureRepo();
|
||||
|
||||
// Base: a single source file with enough content to be copy-detectable.
|
||||
const body = 'aaa\nbbb\nccc\nddd\neee\nfff\n';
|
||||
await writeFile(join(vault, 'src.md'), body, 'utf8');
|
||||
await git.stageAll();
|
||||
await git.commit('add src', {
|
||||
authorName: BOT_AUTHOR_NAME,
|
||||
authorEmail: BOT_AUTHOR_EMAIL,
|
||||
});
|
||||
const base = await git.revParse('HEAD');
|
||||
|
||||
// KEEP src.md and add an identical copy dup.md (a pure copy, not a rename).
|
||||
await copyFile(join(vault, 'src.md'), join(vault, 'dup.md'));
|
||||
await git.stageAll();
|
||||
await git.commit('add copy of src', {
|
||||
authorName: BOT_AUTHOR_NAME,
|
||||
authorEmail: BOT_AUTHOR_EMAIL,
|
||||
});
|
||||
|
||||
const entries = await git.diffNameStatus(base!, 'HEAD');
|
||||
|
||||
// With -M only (no -C), git does NOT emit a C row: the copy is a plain add.
|
||||
expect(entries).toEqual([{ status: 'A', path: 'dup.md' }]);
|
||||
expect(entries.some((e) => e.status === 'C')).toBe(false);
|
||||
});
|
||||
|
||||
// --- 9. commit: per-invocation committer/author does NOT leak into config ----
|
||||
//
|
||||
// The engine sets author + committer identity via GIT_AUTHOR_*/GIT_COMMITTER_*
|
||||
// env vars per `git commit` invocation (commitRaw). This underpins the §10
|
||||
// provenance/loop-guard: the identity must travel WITH the commit, not be
|
||||
// written into the repo config (which would make it global to every later
|
||||
// hand-run commit). We commit with the distinct "Local" identity (different
|
||||
// from the repo's default `user.name`/`user.email`, which ensureRepo seeds as
|
||||
// the bot identity) and assert the commit carries the passed identity while the
|
||||
// repo config is UNCHANGED (still the bot default).
|
||||
it('commit passes committer/author per-invocation without mutating repo config', async (ctx) => {
|
||||
if (!available) ctx.skip();
|
||||
const vault = await freshDir();
|
||||
const git = new VaultGit(vault);
|
||||
await git.ensureRepo();
|
||||
|
||||
// ensureRepo seeds the repo's LOCAL user.* with the bot identity. Capture it
|
||||
// so we can prove the per-commit identity does NOT overwrite it.
|
||||
expect(await localConfig(vault, 'user.name')).toBe(BOT_AUTHOR_NAME);
|
||||
expect(await localConfig(vault, 'user.email')).toBe(BOT_AUTHOR_EMAIL);
|
||||
|
||||
// Commit with a DIFFERENT identity, passed per-invocation only.
|
||||
const LOCAL_NAME = 'Local';
|
||||
const LOCAL_EMAIL = 'local@local';
|
||||
await writeFile(join(vault, 'page.md'), 'hello\n', 'utf8');
|
||||
await git.stageAll();
|
||||
const made = await git.commit('docmost: sync 1 page(s)', {
|
||||
authorName: LOCAL_NAME,
|
||||
authorEmail: LOCAL_EMAIL,
|
||||
});
|
||||
expect(made).toBe(true);
|
||||
|
||||
// The commit's author AND committer are the passed per-invocation identity
|
||||
// (committer matches author via GIT_COMMITTER_* — not the repo default).
|
||||
expect(await headAuthor(vault)).toBe(`${LOCAL_NAME} <${LOCAL_EMAIL}>`);
|
||||
expect(await headCommitter(vault)).toBe(`${LOCAL_NAME} <${LOCAL_EMAIL}>`);
|
||||
|
||||
// CRITICAL: the per-commit identity did NOT leak into the repo config — the
|
||||
// LOCAL user.* is still the bot default ensureRepo seeded.
|
||||
expect(await localConfig(vault, 'user.name')).toBe(BOT_AUTHOR_NAME);
|
||||
expect(await localConfig(vault, 'user.email')).toBe(BOT_AUTHOR_EMAIL);
|
||||
|
||||
// And the identity never reached the GLOBAL config either (the env-var path
|
||||
// writes no config at all). `--global --get` exits non-zero / empty when the
|
||||
// value differs or is unset; assert it is NOT the per-commit identity.
|
||||
const globalName = await execFileAsync('git', [
|
||||
'config',
|
||||
'--global',
|
||||
'--get',
|
||||
'user.name',
|
||||
])
|
||||
.then((r) => r.stdout.trim())
|
||||
.catch(() => '');
|
||||
expect(globalName).not.toBe(LOCAL_NAME);
|
||||
});
|
||||
});
|
||||
157
packages/git-sync/test/git-sync-client.contract.test-d.ts
Normal file
157
packages/git-sync/test/git-sync-client.contract.test-d.ts
Normal file
@@ -0,0 +1,157 @@
|
||||
import { describe, it, expect, expectTypeOf } from 'vitest';
|
||||
import type {
|
||||
GitSyncClient,
|
||||
GitSyncPageNodeLite,
|
||||
} from '../src/engine/client.types';
|
||||
|
||||
// Contract / type-level guard of the `GitSyncClient` seam (src/engine/client.types.ts).
|
||||
//
|
||||
// The engine reads specific fields off each client result; if the server-side
|
||||
// native adapter drifts from this shape, `assignedPageId` (from createPage's
|
||||
// `data.id`) would become `undefined` and the create path would loop forever
|
||||
// re-creating the same page. These are COMPILE-TIME assertions (a typed dummy
|
||||
// object that must `satisfies GitSyncClient`, plus `expectTypeOf` checks on the
|
||||
// exact result fields the engine consumes) — the assertions live in the TYPE
|
||||
// system, not the runtime body.
|
||||
//
|
||||
// ENFORCEMENT (Finding #1): this file is a vitest TYPE test (`.test-d.ts`).
|
||||
// `vitest.config.ts` enables `test.typecheck` scoped to `test/**/*.test-d.ts`,
|
||||
// so `npx vitest run` runs `tsc` over THIS file and turns every `expectTypeOf` /
|
||||
// `@ts-expect-error` / `satisfies GitSyncClient` below into a real build-time
|
||||
// assertion. If the GitSyncClient result shapes drift (e.g. createPage stops
|
||||
// returning `{ data: { id: string } }`), the typecheck pass FAILS and the whole
|
||||
// `vitest run` goes red. (The 35 runtime `*.test.ts` suites are NOT typechecked
|
||||
// — the `-d` include scopes this to the contract file only.) The trivial
|
||||
// `expect(true)` calls just keep the test reporter honest; they are NOT the
|
||||
// guard.
|
||||
|
||||
describe('GitSyncClient contract (type-level)', () => {
|
||||
it('createPage returns { data: { id } } (+ optional updatedAt)', () => {
|
||||
// The exact field the engine reads back to assign the new pageId: the result
|
||||
// must EXTEND `{ data: { id: string } }` (carry at least that shape).
|
||||
expectTypeOf<
|
||||
Awaited<ReturnType<GitSyncClient['createPage']>>
|
||||
>().toExtend<{ data: { id: string } }>();
|
||||
// `data.id` is a string (NOT possibly-undefined): the anti-loop invariant.
|
||||
expectTypeOf<
|
||||
Awaited<ReturnType<GitSyncClient['createPage']>>['data']['id']
|
||||
>().toEqualTypeOf<string>();
|
||||
expect(true).toBe(true);
|
||||
});
|
||||
|
||||
it('importPageMarkdown returns an optional updatedAt', () => {
|
||||
expectTypeOf<
|
||||
Awaited<ReturnType<GitSyncClient['importPageMarkdown']>>['updatedAt']
|
||||
>().toEqualTypeOf<string | undefined>();
|
||||
expect(true).toBe(true);
|
||||
});
|
||||
|
||||
it('getPageJson surfaces the fields the pull side writes into meta', () => {
|
||||
type Page = Awaited<ReturnType<GitSyncClient['getPageJson']>>;
|
||||
expectTypeOf<Page['id']>().toEqualTypeOf<string>();
|
||||
expectTypeOf<Page['slugId']>().toEqualTypeOf<string>();
|
||||
expectTypeOf<Page['title']>().toEqualTypeOf<string>();
|
||||
expectTypeOf<Page['parentPageId']>().toEqualTypeOf<string | null>();
|
||||
expectTypeOf<Page['spaceId']>().toEqualTypeOf<string>();
|
||||
expectTypeOf<Page['updatedAt']>().toEqualTypeOf<string>();
|
||||
expectTypeOf<Page['content']>().toEqualTypeOf<unknown>();
|
||||
expect(true).toBe(true);
|
||||
});
|
||||
|
||||
it('listSpaceTree returns { pages, complete } (complete gates §8 suppression)', () => {
|
||||
type Tree = Awaited<ReturnType<GitSyncClient['listSpaceTree']>>;
|
||||
expectTypeOf<Tree['complete']>().toEqualTypeOf<boolean>();
|
||||
expectTypeOf<Tree['pages']>().toEqualTypeOf<GitSyncPageNodeLite[]>();
|
||||
expect(true).toBe(true);
|
||||
});
|
||||
|
||||
it('a structurally-correct adapter satisfies GitSyncClient (drift => compile error)', () => {
|
||||
// A minimal dummy adapter mirroring the EXACT result shapes the engine reads.
|
||||
// The `satisfies GitSyncClient` clause is the contract guard: any drift in a
|
||||
// method arg/result shape makes this FAIL TO COMPILE (and the run errors).
|
||||
const adapter = {
|
||||
listSpaceTree: async (_spaceId: string, _rootPageId?: string) => ({
|
||||
pages: [] as GitSyncPageNodeLite[],
|
||||
complete: true,
|
||||
}),
|
||||
getPageJson: async (pageId: string) => ({
|
||||
id: pageId,
|
||||
slugId: 'slug',
|
||||
title: 'Title',
|
||||
parentPageId: null,
|
||||
spaceId: 'space',
|
||||
updatedAt: '2026-01-01T00:00:00.000Z',
|
||||
content: { type: 'doc' } as unknown,
|
||||
}),
|
||||
importPageMarkdown: async (_pageId: string, _md: string) => ({
|
||||
updatedAt: '2026-01-01T00:00:00.000Z',
|
||||
}),
|
||||
// The anti-loop shape: createPage MUST return data.id so the engine can
|
||||
// write the assigned pageId back into the file meta.
|
||||
createPage: async (
|
||||
_title: string,
|
||||
_content: string,
|
||||
_spaceId: string,
|
||||
_parentPageId?: string,
|
||||
) => ({
|
||||
data: { id: 'assigned-id' },
|
||||
updatedAt: '2026-01-01T00:00:00.000Z',
|
||||
}),
|
||||
deletePage: async (_pageId: string) => ({ success: true }),
|
||||
movePage: async (
|
||||
_pageId: string,
|
||||
_parentPageId: string | null,
|
||||
_position?: string,
|
||||
) => ({ success: true }),
|
||||
renamePage: async (_pageId: string, _title: string) => ({ success: true }),
|
||||
listRecentSince: async (
|
||||
_spaceId: string | undefined,
|
||||
_sinceIso: string | null,
|
||||
_hardPageCap?: number,
|
||||
) => [] as unknown[],
|
||||
listTrash: async (_spaceId: string) => [] as unknown[],
|
||||
restorePage: async (_pageId: string) => ({ success: true }),
|
||||
} satisfies GitSyncClient;
|
||||
|
||||
// Runtime sanity: the dummy createPage really does carry data.id (so the
|
||||
// engine's `result.data.id` read yields a string, never undefined).
|
||||
expect(typeof adapter).toBe('object');
|
||||
return adapter
|
||||
.createPage('t', 'c', 's')
|
||||
.then((r) => expect(r.data.id).toBe('assigned-id'));
|
||||
});
|
||||
|
||||
it('an adapter MISSING data.id is NOT assignable (negative compile guard)', () => {
|
||||
// This object intentionally omits `data.id` from createPage. The `@ts-expect-error`
|
||||
// asserts the assignment FAILS to type-check — i.e. the contract would catch a
|
||||
// server adapter that drifts to a shape making `assignedPageId` undefined. If
|
||||
// the contract ever loosened to accept this, the directive would become an
|
||||
// UNUSED @ts-expect-error and the file would fail to compile (the guard holds
|
||||
// in BOTH directions).
|
||||
const bad = {
|
||||
listSpaceTree: async () => ({ pages: [] as GitSyncPageNodeLite[], complete: true }),
|
||||
getPageJson: async (pageId: string) => ({
|
||||
id: pageId,
|
||||
slugId: 's',
|
||||
title: 't',
|
||||
parentPageId: null,
|
||||
spaceId: 'sp',
|
||||
updatedAt: 'now',
|
||||
content: {} as unknown,
|
||||
}),
|
||||
importPageMarkdown: async () => ({}),
|
||||
// Drifted: returns a bare object with NO data.id.
|
||||
createPage: async () => ({ success: true }),
|
||||
deletePage: async () => ({}),
|
||||
movePage: async () => ({}),
|
||||
renamePage: async () => ({}),
|
||||
listRecentSince: async () => [] as unknown[],
|
||||
listTrash: async () => [] as unknown[],
|
||||
restorePage: async () => ({}),
|
||||
};
|
||||
// @ts-expect-error createPage is missing the required `data: { id }` shape.
|
||||
const _assert: GitSyncClient = bad;
|
||||
void _assert;
|
||||
expect(true).toBe(true);
|
||||
});
|
||||
});
|
||||
196
packages/git-sync/test/markdown-converter-gaps.test.ts
Normal file
196
packages/git-sync/test/markdown-converter-gaps.test.ts
Normal file
@@ -0,0 +1,196 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
// Import the converter DIRECTLY from src (NOT the docmost-client barrel, which
|
||||
// pulls in collaboration.ts and mutates the global DOM at import time), matching
|
||||
// the other converter unit tests. markdownToProseMirror is imported for the
|
||||
// round-trip cases; loading it mutates the global DOM via jsdom (required for
|
||||
// @tiptap/html's generateJSON under Node) — this is expected.
|
||||
import { convertProseMirrorToMarkdown } from '../src/lib/markdown-converter.js';
|
||||
import { markdownToProseMirror } from '../src/lib/markdown-to-prosemirror.js';
|
||||
|
||||
// Wrap one or more nodes in a minimal ProseMirror doc. The top-level converter
|
||||
// joins doc children with "\n\n" then .trim()s, so a single-node doc yields
|
||||
// exactly that node's rendered (trimmed) string.
|
||||
const doc = (...nodes: any[]) => ({ type: 'doc', content: nodes });
|
||||
const text = (t: string) => ({ type: 'text', text: t });
|
||||
const para = (...inline: any[]) => ({ type: 'paragraph', content: inline });
|
||||
|
||||
// Run a full export -> import -> export cycle and return both markdown strings
|
||||
// plus the intermediate ProseMirror doc (mirrors the property test's helper).
|
||||
async function roundTrip(node: any): Promise<{ md1: string; doc2: any; md2: string }> {
|
||||
const md1 = convertProseMirrorToMarkdown(doc(node));
|
||||
const doc2 = await markdownToProseMirror(md1);
|
||||
const md2 = convertProseMirrorToMarkdown(doc2);
|
||||
return { md1, doc2, md2 };
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 1. pageBreak DATA LOSS (markdown-converter.ts has NO `case "pageBreak"`).
|
||||
//
|
||||
// The schema declares a `pageBreak` block atom (docmost-schema.ts ~L1009), so a
|
||||
// real document CAN legally contain one. The converter's switch has no branch
|
||||
// for it, so it falls through to `default`, which renders only the node's
|
||||
// children — and a pageBreak atom has NONE. It therefore exports to "" and the
|
||||
// node silently disappears: an exported markdown file can never carry a page
|
||||
// break, and a round-trip cannot reconstruct it. We pin this as a known
|
||||
// divergence with an `it.fails` round-trip repro (mirroring the package's two
|
||||
// existing documented `it.fails` bugs in markdown-roundtrip.property.test.ts).
|
||||
// ---------------------------------------------------------------------------
|
||||
describe('pageBreak data loss (no converter case — SPEC §11 divergence)', () => {
|
||||
it('exports a pageBreak node to the empty string (the node disappears)', () => {
|
||||
// Direct, NON-failing assertion of the lossy emission so the data loss is
|
||||
// unambiguous: a standalone pageBreak yields "" (the .trim() of nothing).
|
||||
expect(convertProseMirrorToMarkdown(doc({ type: 'pageBreak' }))).toBe('');
|
||||
});
|
||||
|
||||
it('drops a pageBreak sitting BETWEEN two paragraphs on export', () => {
|
||||
// With surrounding content the lost node leaves no trace at all: the output
|
||||
// is just the two paragraphs joined as if the page break were never there.
|
||||
const out = convertProseMirrorToMarkdown(
|
||||
doc(para(text('before')), { type: 'pageBreak' }, para(text('after'))),
|
||||
);
|
||||
// The pageBreak renders to "", so the only trace it leaves is a doubled
|
||||
// blank gap from the doc "\n\n" join ("before" + "" + "after"): no marker,
|
||||
// no placeholder — the divider itself is gone (data loss). The leftover
|
||||
// blank line is itself a phantom-diff hazard, but the node is unrecoverable.
|
||||
expect(out).toBe('before\n\n\n\nafter');
|
||||
expect(out).not.toContain('pageBreak');
|
||||
});
|
||||
|
||||
// KNOWN, DOCUMENTED non-roundtrip data loss (kept honest as it.fails): a
|
||||
// pageBreak node cannot survive an export -> import -> export cycle because it
|
||||
// is erased on the FIRST export. The assertion below is what we WISH held (the
|
||||
// node round-trips); it fails today, which `it.fails` turns green while keeping
|
||||
// the divergence visible. Source must NOT change — this only documents it.
|
||||
it.fails(
|
||||
'BUG: a pageBreak node is lost on export and cannot round-trip',
|
||||
async () => {
|
||||
const { md1, doc2 } = await roundTrip({ type: 'pageBreak' });
|
||||
// What we want: the placeholder is non-empty and the node comes back.
|
||||
expect(md1).not.toBe('');
|
||||
const types = (doc2.content || []).map((n: any) => n.type);
|
||||
expect(types).toContain('pageBreak');
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 2. subpages LOSSY round-trip (`case "subpages"` emits `{{SUBPAGES}}`).
|
||||
//
|
||||
// The golden test only pins the EMISSION string. The token has no markdown or
|
||||
// HTML meaning, so on re-import marked treats `{{SUBPAGES}}` as ordinary text:
|
||||
// the subpages BLOCK comes back as a plain PARAGRAPH carrying that literal
|
||||
// string, NOT a `subpages` node. The export is "lossy but legible" by design;
|
||||
// this test pins the actual lossy round-trip behavior.
|
||||
// ---------------------------------------------------------------------------
|
||||
describe('subpages lossy round-trip ({{SUBPAGES}} placeholder)', () => {
|
||||
it('emits {{SUBPAGES}} which re-imports as a paragraph, not a subpages node', async () => {
|
||||
const { md1, doc2 } = await roundTrip({ type: 'subpages' });
|
||||
expect(md1).toBe('{{SUBPAGES}}');
|
||||
|
||||
// The re-imported doc has a single paragraph holding the literal token.
|
||||
const top = doc2.content || [];
|
||||
expect(top).toHaveLength(1);
|
||||
expect(top[0].type).toBe('paragraph');
|
||||
expect(top[0].content?.[0]).toMatchObject({ type: 'text', text: '{{SUBPAGES}}' });
|
||||
|
||||
// The subpages node itself is gone: nothing in the doc is a subpages node.
|
||||
const allTypes = top.map((n: any) => n.type);
|
||||
expect(allTypes).not.toContain('subpages');
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 3. column.width number<->string drift (`case "column"` + width parseHTML).
|
||||
//
|
||||
// The converter emits the width verbatim into `data-width="..."` (a STRING in
|
||||
// the HTML, as all HTML attributes are). On import the schema's `column.width`
|
||||
// parseHTML does `parseFloat(value)`, so the attribute always comes back as a
|
||||
// NUMBER. A document authored/stored with a STRING fractional width therefore
|
||||
// DRIFTS to a number across a round-trip at the ProseMirror-doc level — even
|
||||
// though the emitted MARKDOWN stays byte-stable (the number prints the same).
|
||||
// Pinned here as a documented attribute-type divergence (SPEC §11).
|
||||
// ---------------------------------------------------------------------------
|
||||
describe('column.width number<->string drift (schema parseFloat — SPEC §11)', () => {
|
||||
const columnsWith = (width: any) => ({
|
||||
type: 'columns',
|
||||
attrs: { layout: 'two' },
|
||||
content: [
|
||||
{ type: 'column', attrs: { width }, content: [para(text('L'))] },
|
||||
{ type: 'column', content: [para(text('R'))] },
|
||||
],
|
||||
});
|
||||
|
||||
it('a STRING fractional width drifts to a NUMBER across the round-trip', async () => {
|
||||
const { md1, doc2, md2 } = await roundTrip(columnsWith('33.3'));
|
||||
|
||||
// The emitted markdown carries the value as an HTML attribute string and is
|
||||
// byte-stable across the cycle (the divergence is at the doc level only).
|
||||
expect(md1).toContain('data-width="33.3"');
|
||||
expect(md2).toBe(md1);
|
||||
|
||||
// But the doc attribute type changed: authored as string "33.3", it comes
|
||||
// back as the number 33.3 (schema's parseFloat). This is the drift.
|
||||
const rtWidth = doc2.content?.[0]?.content?.[0]?.attrs?.width;
|
||||
expect(typeof rtWidth).toBe('number');
|
||||
expect(rtWidth).toBe(33.3);
|
||||
});
|
||||
|
||||
it('a NUMBER fractional width keeps its value (no precision loss) and is byte-stable', async () => {
|
||||
const { md1, doc2, md2 } = await roundTrip(columnsWith(33.333333));
|
||||
expect(md1).toContain('data-width="33.333333"');
|
||||
expect(md2).toBe(md1);
|
||||
const rtWidth = doc2.content?.[0]?.content?.[0]?.attrs?.width;
|
||||
expect(typeof rtWidth).toBe('number');
|
||||
expect(rtWidth).toBe(33.333333);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 5b. EMPTY detailsContent (`case "details"` with an empty body).
|
||||
//
|
||||
// detailsContent's schema content is `block*` (docmost-schema.ts ~L474), so an
|
||||
// empty details body is legal. The converter must handle a `detailsContent`
|
||||
// with no children without crashing and without emitting invalid output that
|
||||
// breaks the round-trip. This pins that an empty details body exports cleanly
|
||||
// and re-imports as a valid `details` whose body is an empty `detailsContent`.
|
||||
// ---------------------------------------------------------------------------
|
||||
describe('empty detailsContent (schema allows block*)', () => {
|
||||
const emptyDetails = doc({
|
||||
type: 'details',
|
||||
content: [
|
||||
{ type: 'detailsSummary', content: [text('Summary')] },
|
||||
{ type: 'detailsContent', content: [] },
|
||||
],
|
||||
});
|
||||
|
||||
it('exports an empty details body without crashing or producing junk', () => {
|
||||
const md = convertProseMirrorToMarkdown(emptyDetails);
|
||||
// The summary survives and the <details> wrapper closes; the empty body adds
|
||||
// no content of its own.
|
||||
expect(md).toContain('<summary>Summary</summary>');
|
||||
expect(md).toContain('</details>');
|
||||
expect(md).not.toContain('undefined');
|
||||
expect(md).not.toContain('null');
|
||||
});
|
||||
|
||||
it('round-trips to a valid details with an empty detailsContent body', async () => {
|
||||
const md1 = convertProseMirrorToMarkdown(emptyDetails);
|
||||
const doc2 = await markdownToProseMirror(md1);
|
||||
const md2 = convertProseMirrorToMarkdown(doc2);
|
||||
// Export is byte-stable (no growth / no junk on the second pass).
|
||||
expect(md2).toBe(md1);
|
||||
|
||||
// The re-imported tree is a details with summary + an empty content body.
|
||||
const details = doc2.content?.[0];
|
||||
expect(details?.type).toBe('details');
|
||||
const childTypes = (details?.content || []).map((c: any) => c.type);
|
||||
expect(childTypes).toEqual(['detailsSummary', 'detailsContent']);
|
||||
const detailsContent = details.content.find(
|
||||
(c: any) => c.type === 'detailsContent',
|
||||
);
|
||||
// block* — an empty body has no (or empty) content, which is valid.
|
||||
expect(detailsContent.content == null || detailsContent.content.length === 0).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
});
|
||||
153
packages/git-sync/test/markdown-to-prosemirror-gaps.test.ts
Normal file
153
packages/git-sync/test/markdown-to-prosemirror-gaps.test.ts
Normal file
@@ -0,0 +1,153 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
// markdownToProseMirror lives next to the markdown->HTML preprocessors
|
||||
// (preprocessCallouts, bridgeTaskLists). Those helpers are NOT exported, so we
|
||||
// exercise them through the public entry point, which runs the full
|
||||
// markdown -> preprocessCallouts -> marked -> bridgeTaskLists -> generateJSON
|
||||
// pipeline. Importing this module mutates the global DOM via jsdom (required for
|
||||
// @tiptap/html under Node) — expected, same as the property test.
|
||||
import { markdownToProseMirror } from '../src/lib/markdown-to-prosemirror.js';
|
||||
|
||||
// Find every node of a given type anywhere in a ProseMirror doc tree.
|
||||
const findAll = (node: any, type: string, acc: any[] = []): any[] => {
|
||||
if (node && node.type === type) acc.push(node);
|
||||
for (const child of node?.content || []) findAll(child, type, acc);
|
||||
return acc;
|
||||
};
|
||||
// Concatenate all text within a subtree (order-preserving).
|
||||
const allText = (node: any): string => {
|
||||
if (node?.type === 'text') return node.text || '';
|
||||
return (node?.content || []).map(allText).join('');
|
||||
};
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 3. preprocessCallouts — two uncovered branches.
|
||||
//
|
||||
// (a) NESTED callouts: an inner `:::type ... :::` inside an outer callout body
|
||||
// must be matched at its own nesting level (the depth counter) and emerge as
|
||||
// a callout NESTED inside the outer callout — not flattened or mis-closed.
|
||||
// (b) A `:::` line INSIDE a fenced code block must NOT be treated as a callout
|
||||
// delimiter: the scanner tracks code fences and copies their lines verbatim,
|
||||
// so the outer callout's matching `:::` is the one AFTER the fence closes.
|
||||
// ---------------------------------------------------------------------------
|
||||
describe('preprocessCallouts: nested callouts + code-fenced ":::"', () => {
|
||||
it('(a) parses a callout nested inside another callout', async () => {
|
||||
const md = [
|
||||
':::info',
|
||||
'outer text',
|
||||
':::warning',
|
||||
'inner text',
|
||||
':::',
|
||||
':::',
|
||||
].join('\n');
|
||||
|
||||
const docNode = await markdownToProseMirror(md);
|
||||
|
||||
// Exactly two callouts, and one is nested inside the other.
|
||||
const callouts = findAll(docNode, 'callout');
|
||||
expect(callouts).toHaveLength(2);
|
||||
|
||||
const outer = docNode.content?.[0];
|
||||
expect(outer?.type).toBe('callout');
|
||||
expect(outer?.attrs?.type).toBe('info');
|
||||
|
||||
// The inner callout is a CHILD of the outer one (not a sibling at doc level).
|
||||
const innerCallouts = (outer?.content || []).filter(
|
||||
(n: any) => n.type === 'callout',
|
||||
);
|
||||
expect(innerCallouts).toHaveLength(1);
|
||||
expect(innerCallouts[0].attrs?.type).toBe('warning');
|
||||
|
||||
// Both bodies kept their text.
|
||||
expect(allText(outer)).toContain('outer text');
|
||||
expect(allText(innerCallouts[0])).toContain('inner text');
|
||||
});
|
||||
|
||||
it('(b) a ":::" line inside a fenced code block is NOT a callout delimiter', async () => {
|
||||
// The inner ``` ... ``` fence contains a `:::` line. If preprocessCallouts
|
||||
// treated it as the closing fence, the callout would terminate early and the
|
||||
// code text would leak out. The correct behavior: the fence content survives
|
||||
// verbatim in a codeBlock, and the callout closes at the LAST ":::".
|
||||
const md = [
|
||||
':::info',
|
||||
'before code',
|
||||
'```',
|
||||
':::',
|
||||
'still inside the code fence',
|
||||
'```',
|
||||
'after code',
|
||||
':::',
|
||||
].join('\n');
|
||||
|
||||
const docNode = await markdownToProseMirror(md);
|
||||
|
||||
// One callout wrapping everything (it did not close early on the fenced ":::")
|
||||
const callouts = findAll(docNode, 'callout');
|
||||
expect(callouts).toHaveLength(1);
|
||||
const callout = callouts[0];
|
||||
|
||||
// The code block is a CHILD of the callout and still contains the ":::" line.
|
||||
const codeBlocks = findAll(callout, 'codeBlock');
|
||||
expect(codeBlocks).toHaveLength(1);
|
||||
expect(allText(codeBlocks[0])).toContain(':::');
|
||||
expect(allText(codeBlocks[0])).toContain('still inside the code fence');
|
||||
|
||||
// The text before and after the fence is part of the callout, not a stray
|
||||
// top-level paragraph created by an early close.
|
||||
expect(allText(callout)).toContain('before code');
|
||||
expect(allText(callout)).toContain('after code');
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 4. bridgeTaskLists — numbered checklist + mixed-list negative.
|
||||
//
|
||||
// (a) A NUMBERED checklist (`1. [x] ...`) is rendered by marked as an <ol> of
|
||||
// checkbox <li>s. The bridge must convert it to a taskList AND rename the
|
||||
// <ol> to a <ul> so generateJSON does NOT also match the orderedList rule
|
||||
// and emit a phantom empty orderedList beside the real taskList.
|
||||
// (b) NEGATIVE: a MIXED list (some items have checkboxes, some don't) must NOT
|
||||
// be converted — it stays an ordinary bullet/numbered list.
|
||||
// ---------------------------------------------------------------------------
|
||||
describe('bridgeTaskLists: numbered checklist + mixed-list negative', () => {
|
||||
it('(a) a numbered <ol> checklist becomes a taskList with NO phantom orderedList', async () => {
|
||||
const md = ['1. [x] done', '2. [ ] todo'].join('\n');
|
||||
|
||||
const docNode = await markdownToProseMirror(md);
|
||||
|
||||
// It became a taskList...
|
||||
const taskLists = findAll(docNode, 'taskList');
|
||||
expect(taskLists).toHaveLength(1);
|
||||
|
||||
const items = (taskLists[0].content || []).filter(
|
||||
(n: any) => n.type === 'taskItem',
|
||||
);
|
||||
expect(items).toHaveLength(2);
|
||||
expect(items[0].attrs?.checked).toBe(true);
|
||||
expect(items[1].attrs?.checked).toBe(false);
|
||||
expect(allText(items[0])).toContain('done');
|
||||
expect(allText(items[1])).toContain('todo');
|
||||
|
||||
// ...and NO phantom (empty) orderedList survived the <ol> -> <ul> rename.
|
||||
const orderedLists = findAll(docNode, 'orderedList');
|
||||
expect(orderedLists).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('(b) a MIXED list (some items checkboxed, some not) is NOT converted to a taskList', async () => {
|
||||
const md = ['- [x] checked item', '- plain item'].join('\n');
|
||||
|
||||
const docNode = await markdownToProseMirror(md);
|
||||
|
||||
// The bridge requires EVERY direct <li> to carry its own checkbox; one plain
|
||||
// item disqualifies the whole list, so it stays a bulletList.
|
||||
expect(findAll(docNode, 'taskList')).toHaveLength(0);
|
||||
expect(findAll(docNode, 'taskItem')).toHaveLength(0);
|
||||
|
||||
const bulletLists = findAll(docNode, 'bulletList');
|
||||
expect(bulletLists).toHaveLength(1);
|
||||
const listItems = findAll(bulletLists[0], 'listItem');
|
||||
expect(listItems).toHaveLength(2);
|
||||
// Both items survive as ordinary list items (text preserved).
|
||||
expect(allText(bulletLists[0])).toContain('checked item');
|
||||
expect(allText(bulletLists[0])).toContain('plain item');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user