fix(git-sync): never trash a page that only MOVED (pageId-identity, not git rename heuristics) — data loss
CRITICAL data-loss bug: creating pages in Docmost (which start UNTITLED) and then typing a title could soft-delete OTHER pages. Untitled pages all serialize to the `_` fallback filename; the layout disambiguates them (`_.md`, `_ ~slug.md`). Retitling one frees the bare `_` and another untitled page's file relocates into it. git's rename detection (`-M`) can't see the move (the tiny meta-only files are too dissimilar), so `git diff` reports it as DELETE(old) + ADD/MODIFY(new). The push took the DELETE literally and trashed a live page. Root cause is that the push trusted git's path-level rename heuristic for page IDENTITY. Identity is the pageId. Fix: before emitting any delete, coalesce by pageId — a pageId that is BOTH deleted (pre-image) AND present on the surviving side (current meta of an ADD or a MODIFY, since a relocation into an occupied path shows as M) is one page that MOVED, classified as a rename/move and NEVER a delete. Reproduced + verified on a live stand: 4 untitled pages + retitle one trashed a different page before; after the fix, retitling one (and stress-retitling all) trashes nothing. Engine suite 598 green; 3 new computePushActions cases (ghost D+A move -> rename; real delete still deletes; unrelated D+A stay delete+update). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -223,3 +223,60 @@ describe('computePushActions — mixed batch', () => {
|
||||
expect(actions.skipped).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('computePushActions — ghost-move coalescing (data-loss guard)', () => {
|
||||
// git's `-M` rename detection misses a move when the files are too dissimilar
|
||||
// (tiny meta-only files after a layout reshuffle of `_`-fallback names). git
|
||||
// then reports the move as a DELETE of the old path + an ADD of the new one.
|
||||
// Taken literally this soft-deletes a page that merely MOVED. The classifier
|
||||
// must recognize the shared pageId and emit a rename/move, never a delete.
|
||||
it('D(old)+A(new) of the SAME pageId -> rename/move, NOT a delete', () => {
|
||||
const changes: DiffEntry[] = [
|
||||
{ status: 'D', path: '_ ~slug.md' },
|
||||
{ status: 'A', path: '_.md' },
|
||||
];
|
||||
const metaAt = metaTable({
|
||||
'_ ~slug.md|prev': meta({ pageId: 'p1', title: '', spaceId: 'sp1' }),
|
||||
'_.md|current': meta({ pageId: 'p1', title: '', spaceId: 'sp1' }),
|
||||
});
|
||||
const actions = computePushActions({ changes, metaAt });
|
||||
expect(actions.deletes).toEqual([]); // the page is NEVER trashed
|
||||
expect(actions.updates).toEqual([]); // not a spurious update either
|
||||
expect(actions.renamesMoves).toEqual([
|
||||
{ pageId: 'p1', oldPath: '_ ~slug.md', newPath: '_.md' },
|
||||
]);
|
||||
// The suppressed delete is recorded as a skip with a clear reason.
|
||||
expect(actions.skipped).toEqual([
|
||||
{
|
||||
path: '_ ~slug.md',
|
||||
status: 'D',
|
||||
reason: 'ghost-move (re-added at a new path) — not a deletion',
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('a real delete (no matching add) is STILL a delete', () => {
|
||||
const changes: DiffEntry[] = [{ status: 'D', path: 'Gone.md' }];
|
||||
const metaAt = metaTable({
|
||||
'Gone.md|prev': meta({ pageId: 'p9', title: 'Gone', spaceId: 'sp1' }),
|
||||
});
|
||||
const actions = computePushActions({ changes, metaAt });
|
||||
expect(actions.deletes).toEqual([{ pageId: 'p9' }]);
|
||||
expect(actions.renamesMoves).toEqual([]);
|
||||
});
|
||||
|
||||
it('an unrelated D + A (different pageIds) are a real delete + a real update', () => {
|
||||
const changes: DiffEntry[] = [
|
||||
{ status: 'D', path: 'A.md' },
|
||||
{ status: 'A', path: 'B.md' },
|
||||
];
|
||||
const metaAt = metaTable({
|
||||
'A.md|prev': meta({ pageId: 'pa', title: 'A', spaceId: 'sp1' }),
|
||||
'B.md|current': meta({ pageId: 'pb', title: 'B', spaceId: 'sp1' }),
|
||||
});
|
||||
const actions = computePushActions({ changes, metaAt });
|
||||
expect(actions.deletes).toEqual([{ pageId: 'pa' }]);
|
||||
expect(actions.updates).toEqual([{ pageId: 'pb', path: 'B.md' }]);
|
||||
expect(actions.renamesMoves).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user