fix(git-sync): never trash a page whose pageId still exists in the tree (cross-cycle move) + browser e2e
Follow-up to 4376c5a6, found by a real BROWSER e2e (the flow the in-diff fix
missed). When the layout reshuffle's two halves land in SEPARATE sync cycles, the
later cycle's diff has only the DELETE of the old path — the matching add was
already pushed — so in-diff D+A coalescing can't see it, and the live page was
still trashed.
Robust fix on the identity invariant the reviewer (and the user) called out: a
page EXISTS iff its pageId is in the vault, regardless of filename. runPush now
collects the pageIds present at ANY path in the current `main` tree and passes
them to computePushActions; a deleted file whose pageId is still tracked
elsewhere is a MOVE, never a deletion. (Built only when the diff has deletes.)
Adds apps/server/test/git-sync-browser-e2e.cjs — a Playwright test that drives the
REAL Docmost web UI: log in, create several untitled pages, type a title, sync,
assert NOTHING is trashed. Reproduced the data loss before this fix; 5/5 green and
stable after. Engine suite 600 green (+2 computePushActions cases:
pageId-still-present -> skip; pageId-gone -> real delete).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -280,3 +280,41 @@ describe('computePushActions — ghost-move coalescing (data-loss guard)', () =>
|
||||
expect(actions.renamesMoves).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('computePushActions — currentPageIds guard (cross-cycle move)', () => {
|
||||
it('a D whose pageId still exists in the tree (no matching A in THIS diff) is NOT deleted', () => {
|
||||
// The move happened across cycles: the new file landed earlier, so this diff
|
||||
// only has the old path D. The pageId still lives in the tree -> not a delete.
|
||||
const changes: DiffEntry[] = [{ status: 'D', path: '_ ~old.md' }];
|
||||
const metaAt = metaTable({
|
||||
'_ ~old.md|prev': meta({ pageId: 'pX', title: '', spaceId: 'sp1' }),
|
||||
});
|
||||
const actions = computePushActions({
|
||||
changes,
|
||||
metaAt,
|
||||
currentPageIds: new Set(['pX']), // pX is still tracked somewhere on main
|
||||
});
|
||||
expect(actions.deletes).toEqual([]);
|
||||
expect(actions.skipped).toEqual([
|
||||
{
|
||||
path: '_ ~old.md',
|
||||
status: 'D',
|
||||
reason: 'pageId still present in the tree (moved) — not a deletion',
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('a D whose pageId is GONE from the tree is a real delete', () => {
|
||||
const changes: DiffEntry[] = [{ status: 'D', path: 'Removed.md' }];
|
||||
const metaAt = metaTable({
|
||||
'Removed.md|prev': meta({ pageId: 'pY', title: 'Removed', spaceId: 'sp1' }),
|
||||
});
|
||||
const actions = computePushActions({
|
||||
changes,
|
||||
metaAt,
|
||||
currentPageIds: new Set(['pOther']), // pY is NOT present -> genuinely deleted
|
||||
});
|
||||
expect(actions.deletes).toEqual([{ pageId: 'pY' }]);
|
||||
expect(actions.skipped).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user