vvzvlad/gitmost
1
0
Fork 0
Code Issues 14 Pull Requests 12 Packages Projects Releases Activity

fix(git-sync): never trash a page that only MOVED (pageId-identity, not git rename heuristics) — data loss

Browse Source 
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:
claude code agent 227
2026-06-24 03:32:22 +03:00
parent 805d1a45a2
commit 756a6eaf06
2 changed files with 120 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

66
packages/git-sync/src/engine/push.ts
View File

@@ -237,12 +237,54 @@ export function computePushActions(input: PushActionsInput): PushActions {
skipped: [],
};
// GHOST-MOVE coalescing (⭐ data-loss guard). git's rename detection (`-M`)
// can miss a move when the two files are too dissimilar — which is exactly the
// case for the tiny `docmost:meta`-only files a layout RESHUFFLE produces (e.g.
// several untitled pages sharing the `_` fallback name; retitling one frees the
// bare `_` and another page's file relocates `_ ~slug.md` -> `_.md`). git then
// reports the move as a DELETE of the old path + an ADD of the new one. Taken
// literally that soft-deletes a page that merely MOVED — a live page vanishing
// into Trash. Identity is the pageId, not git's heuristic: a pageId that is
// BOTH deleted (pre-image) and added (current) is one page that relocated, so
// we classify it as a rename/move and NEVER as a delete.
// A pageId can land at its new path two ways: as an ADD (the path was free) or
// as a MODIFY (the path was occupied by ANOTHER page that left — the reshuffle
// case, where `_.md`'s occupant changes pageId). Both are "the page survives at
// a new path", so the surviving side is the CURRENT-meta pageId of A *and* M.
const deletedPath = new Map<string, string>();
const survivingPath = new Map<string, string>();
for (const change of changes) {
if (change.status === "D") {
const pid = metaAt(change.path, "prev")?.pageId;
if (pid) deletedPath.set(pid, change.path);
} else if (change.status === "A" || change.status === "M") {
const pid = metaAt(change.path, "current")?.pageId;
if (pid) survivingPath.set(pid, change.path);
}
}
const ghostMove = new Map<string, { oldPath: string; newPath: string }>();
for (const [pid, oldPath] of deletedPath) {
const newPath = survivingPath.get(pid);
if (newPath && newPath !== oldPath) {
ghostMove.set(pid, { oldPath, newPath });
}
}
for (const change of changes) {
switch (change.status) {
case "A": {
const meta = metaAt(change.path, "current");
const pageId = meta?.pageId;
if (pageId) {
if (pageId && ghostMove.has(pageId)) {
// Half of a git-undetected move (a matching DELETE exists): record it
// as a rename/move (like a real `R`), NOT an update — the `D` side is
// suppressed so the page is never soft-deleted.
actions.renamesMoves.push({
pageId,
oldPath: ghostMove.get(pageId)!.oldPath,
newPath: change.path,
});
} else if (pageId) {
// Added but already carries a pageId (restored/copied file): the page
// exists in Docmost, so push content as an UPDATE — never a duplicate.
actions.updates.push({ pageId, path: change.path });
@@ -266,7 +308,16 @@ export function computePushActions(input: PushActionsInput): PushActions {
case "M": {
const meta = metaAt(change.path, "current");
const pageId = meta?.pageId;
if (pageId) {
if (pageId && ghostMove.has(pageId)) {
// This path's occupant changed pageId: the previous page left and THIS
// page relocated here (a reshuffle). Its old file was DELETED elsewhere
// — coalesce into a rename/move so the page is never trashed.
actions.renamesMoves.push({
pageId,
oldPath: ghostMove.get(pageId)!.oldPath,
newPath: change.path,
});
} else if (pageId) {
actions.updates.push({ pageId, path: change.path });
} else {
// A modified file with no pageId has no Docmost target to update.
@@ -283,7 +334,16 @@ export function computePushActions(input: PushActionsInput): PushActions {
// (the version last pushed to Docmost) so we delete the RIGHT page.
const prevMeta = metaAt(change.path, "prev");
const pageId = prevMeta?.pageId;
if (pageId) {
if (pageId && ghostMove.has(pageId)) {
// The same pageId was re-ADDED at a new path: this is a git-undetected
// MOVE, handled by the `A` branch above. Suppress the delete so a moved
// page is never trashed (⭐ data-loss guard).
actions.skipped.push({
path: change.path,
status: "D",
reason: "ghost-move (re-added at a new path) — not a deletion",
});
} else if (pageId) {
actions.deletes.push({ pageId });
} else {
// Untracked-file guard (SPEC §8): a file with no recoverable pageId was

57
packages/git-sync/test/compute-push-actions.test.ts
View File

@@ -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([]);
});
});