fix(git-sync): subpages round-trips (was {{SUBPAGES}} literal) + exhaustive all-node round-trip test
subpages exported to the literal `{{SUBPAGES}}`, which has no markdown/HTML
inverse, so on re-import it came back as a plain paragraph holding the visible
text "{{SUBPAGES}}" — the embed rendered as that literal string on the page
after a sync (round-trip data loss, seen live). It now emits the schema-matching
`<div data-type="subpages">` like every other embed node, so the schema's
parseHTML rebuilds the subpages node. Also dropped the leaf-atom content-hole
in the subpages renderHTML.
New committed regression coverage:
- packages/git-sync/test/roundtrip-all-nodes.test.ts — exhaustive serialize ->
deserialize round trip for ALL 40 node/mark types; each asserts the node/mark
survives and no `{{...}}` literal leaks. This is the test that caught subpages.
- §13.1 gate (git-sync-converter-gate.spec.ts): subpages added to the green
corpus (round-trips through the REAL server schema).
- Corrected two PR-authored tests that asserted the old {{SUBPAGES}} loss as
"by design" — they now assert the fixed round trip.
Also folds in review #1679 coverage-gap tests (no prod change): orchestrator
pollTick/enabledSpaces, datasource 3-way merge dispatch, page.repo
last_updated_source provenance SQL.
git-sync vitest 659 (+1 expected-fail), server tsc clean, server specs green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -19,6 +19,14 @@ jest.mock('../git-sync.loader', () => ({
|
||||
}));
|
||||
|
||||
import { Logger } from '@nestjs/common';
|
||||
import {
|
||||
Kysely,
|
||||
DummyDriver,
|
||||
PostgresAdapter,
|
||||
PostgresIntrospector,
|
||||
PostgresQueryCompiler,
|
||||
CompiledQuery,
|
||||
} from 'kysely';
|
||||
import {
|
||||
GitSyncOrchestrator,
|
||||
GitSyncLockHeldError,
|
||||
@@ -466,4 +474,91 @@ describe('GitSyncOrchestrator', () => {
|
||||
expect(built.scheduler.addInterval).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
// The poll-safety backstop: each tick enumerates the STRICT opt-in spaces and
|
||||
// reconciles each one under its own lock. We drive the private `pollTick()`
|
||||
// directly and (separately) compile `enabledSpaces()` to assert its opt-in SQL.
|
||||
describe('pollTick + enabledSpaces (strict opt-in backstop)', () => {
|
||||
it('runs runOnce exactly once per enabled space, with the right (spaceId, workspaceId)', async () => {
|
||||
const built = build();
|
||||
// Isolate the tick wiring from the cycle machinery: stub the enumeration
|
||||
// and count runOnce (it never throws; here we don't exercise its body).
|
||||
const runOnce = jest
|
||||
.spyOn(built.orchestrator, 'runOnce')
|
||||
.mockResolvedValue({ spaceId: 'x', ran: true });
|
||||
jest
|
||||
.spyOn(built.orchestrator as any, 'enabledSpaces')
|
||||
.mockResolvedValue([
|
||||
{ spaceId: 'space-1', workspaceId: 'ws-1' },
|
||||
{ spaceId: 'space-2', workspaceId: 'ws-2' },
|
||||
]);
|
||||
|
||||
await (built.orchestrator as any).pollTick();
|
||||
|
||||
expect(runOnce).toHaveBeenCalledTimes(2);
|
||||
// Per-space isolation: each space is reconciled with its OWN workspace id.
|
||||
expect(runOnce).toHaveBeenNthCalledWith(1, 'space-1', 'ws-1');
|
||||
expect(runOnce).toHaveBeenNthCalledWith(2, 'space-2', 'ws-2');
|
||||
});
|
||||
|
||||
it('does NOT throw and runs nothing when the enabled-spaces query throws (try/catch backstop)', async () => {
|
||||
jest.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined);
|
||||
const built = build();
|
||||
const runOnce = jest.spyOn(built.orchestrator, 'runOnce');
|
||||
jest
|
||||
.spyOn(built.orchestrator as any, 'enabledSpaces')
|
||||
.mockRejectedValue(new Error('db down'));
|
||||
|
||||
// A failed enumeration must never break the interval — pollTick swallows it.
|
||||
await expect(
|
||||
(built.orchestrator as any).pollTick(),
|
||||
).resolves.toBeUndefined();
|
||||
expect(runOnce).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('early-returns (no enumeration, no runOnce) when git-sync is disabled', async () => {
|
||||
const built = build({ enabled: false });
|
||||
const enabled = jest.spyOn(built.orchestrator as any, 'enabledSpaces');
|
||||
const runOnce = jest.spyOn(built.orchestrator, 'runOnce');
|
||||
|
||||
await (built.orchestrator as any).pollTick();
|
||||
|
||||
// Gated on the master switch before any DB work.
|
||||
expect(enabled).not.toHaveBeenCalled();
|
||||
expect(runOnce).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('compiles the STRICT opt-in enumeration SQL (spaces, deletedAt is null, enabled flag)', async () => {
|
||||
// Inject a compile-only Kysely (DummyDriver) whose `log` hook captures the
|
||||
// exact SQL `enabledSpaces()` runs — no fake builder, the real query is
|
||||
// compiled. DummyDriver yields no rows; we only assert the SQL shape.
|
||||
const built = build();
|
||||
let captured: CompiledQuery | undefined;
|
||||
const compileDb = new Kysely<any>({
|
||||
dialect: {
|
||||
createAdapter: () => new PostgresAdapter(),
|
||||
createDriver: () => new DummyDriver(),
|
||||
createIntrospector: (d) => new PostgresIntrospector(d),
|
||||
createQueryCompiler: () => new PostgresQueryCompiler(),
|
||||
},
|
||||
log: (event) => {
|
||||
if (event.level === 'query') captured = event.query as CompiledQuery;
|
||||
},
|
||||
});
|
||||
// Swap the orchestrator's injected db for the compile-only instance.
|
||||
(built.orchestrator as any).db = compileDb;
|
||||
|
||||
const rows = await (built.orchestrator as any).enabledSpaces();
|
||||
// DummyDriver returns no rows -> empty opt-in list (the no-space default).
|
||||
expect(rows).toEqual([]);
|
||||
|
||||
expect(captured).toBeDefined();
|
||||
const sql = captured!.sql.replace(/\s+/g, ' ');
|
||||
expect(sql).toContain('from "spaces"');
|
||||
// deletedAt-is-null guard (live spaces only).
|
||||
expect(sql).toContain('"deletedAt" is null');
|
||||
// STRICT per-space opt-in: the raw jsonb flag predicate, verbatim.
|
||||
expect(sql).toContain(`settings->'gitSync'->>'enabled' = 'true'`);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -48,6 +48,11 @@ jest.mock('../git-sync.loader', () => ({
|
||||
|
||||
import * as Y from 'yjs';
|
||||
import { GitmostDataSourceService } from './gitmost-datasource.service';
|
||||
// The body-write seam picks 2-way vs 3-way merge based on whether a base doc was
|
||||
// built. We spy on the real module exports (ts-jest CJS output references them
|
||||
// through the namespace object, so the spies intercept the SUT's calls) and let
|
||||
// them call through, so we assert WHICH merge ran without mocking the behaviour.
|
||||
import * as bodyMerge from './yjs-body-merge';
|
||||
|
||||
// Focused unit/contract test for the native GitSyncClient adapter.
|
||||
// No DB, no real collab server: the repos/services/gateway are mocked and we
|
||||
@@ -271,6 +276,46 @@ describe('GitmostDataSourceService', () => {
|
||||
// The body fragment is non-empty: the incoming block was merged in.
|
||||
expect(realDoc.getXmlFragment('default').length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
// The 2-way path (no base) is covered above; this exercises the THREE-WAY
|
||||
// branch that only fires when a `baseMarkdown` is supplied (review #5).
|
||||
describe('with a baseMarkdown (three-way merge)', () => {
|
||||
afterEach(() => jest.restoreAllMocks());
|
||||
|
||||
it('builds a base doc and dispatches to mergeXmlFragments3Way (not the 2-way merge)', async () => {
|
||||
const { service, mocks } = build();
|
||||
mocks.pageRepo.findById.mockResolvedValue({
|
||||
id: 'p1',
|
||||
updatedAt: new Date('2026-06-20T11:00:00.000Z'),
|
||||
});
|
||||
// Spy through to the real implementations so we observe the dispatch.
|
||||
const merge3 = jest.spyOn(bodyMerge, 'mergeXmlFragments3Way');
|
||||
const merge2 = jest.spyOn(bodyMerge, 'mergeXmlFragments');
|
||||
|
||||
await service
|
||||
.bind(CTX)
|
||||
.importPageMarkdown('p1', '# Full\n\ngit', '# Base\n\nbase');
|
||||
|
||||
// The body write was staged through collab as before.
|
||||
expect(mocks.conn.transact).toHaveBeenCalledTimes(1);
|
||||
expect(typeof mocks.conn.capturedFn).toBe('function');
|
||||
|
||||
// Running the captured merge against a real live doc takes the 3-way path:
|
||||
// the base was parsed/built and the 3-way helper is invoked with three
|
||||
// fragments; the 2-way fallback is NOT used.
|
||||
const liveDoc = new Y.Doc();
|
||||
expect(() => mocks.conn.capturedFn?.(liveDoc)).not.toThrow();
|
||||
|
||||
expect(merge3).toHaveBeenCalledTimes(1);
|
||||
expect(merge2).not.toHaveBeenCalled();
|
||||
const [liveFrag, gitFrag, baseFrag] = merge3.mock.calls[0];
|
||||
expect(liveFrag).toBeInstanceOf(Y.XmlFragment);
|
||||
expect(gitFrag).toBeInstanceOf(Y.XmlFragment);
|
||||
// The third arg is the BASE fragment — proof the base markdown was parsed
|
||||
// and converted into its own doc for the common-ancestor comparison.
|
||||
expect(baseFrag).toBeInstanceOf(Y.XmlFragment);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('createPage', () => {
|
||||
|
||||
Reference in New Issue
Block a user