fix(git-sync): deliver body on rename+edit via honest 3-way merge; cover CREATE strip; fix env doc (F4-F6)

F4: a rename/move + body edit in one diff used to lose the edit (renamed pages
    went only into renamesMoves, never updates). Now computePushActions also
    emits an updates entry for renames, AND threads the OLD path via a new
    UpdateAction.basePath so applyPushActions resolves the 3-way merge base from
    the pre-rename file. Without it the base lookup at the new path returns null
    and degrades to a 2-way merge that rolls back concurrent Docmost edits; with
    it the edited block wins while a concurrent edit to another block survives.
    A plain (status M) update carries no basePath and is byte-identical to before.
F5: test the CREATE path stripping conflict markers (autoMergeConflicts on).
F6: .env.example documents GIT_SYNC_REMOTE_TEMPLATE as deferred/inert scaffolding.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
2026-06-29 14:39:35 +03:00
parent fbaaa84419
commit c6d3566475
6 changed files with 245 additions and 10 deletions
@@ -187,6 +187,76 @@ describe('applyPushActions — update (collab path, SPEC §2/§15.6)', () => {
);
expect(git.showFileAtRef).toHaveBeenCalledWith(LAST_PUSHED_REF, 'Doc.md');
});
it('RENAME-derived update resolves the 3-way base from the OLD path (basePath), not the new path', async () => {
// The residual flaw after F4: a rename+edit emits an UPDATE whose `path` is the
// NEW path, but at refs/docmost/last-pushed the file lived at the OLD path. If
// the base were looked up at the NEW path it would return null and the merge
// would degrade to a 2-way (clobbering a concurrent Docmost-side edit). The fix
// threads `basePath = oldPath` so the base is the pre-rename file (honest 3-way).
const client = makeClient();
// The pre-image tree only has the OLD path; the NEW path does NOT exist there.
const { git } = makeGit({
prevTree: { 'Old/Path.md': fileFor('p-mv', 'base body') },
});
// The working tree has the file at its NEW path with the EDITED body.
const fs = makeFs({ 'New/Path.md': fileFor('p-mv', 'edited body') });
await applyPushActions(
deps(client, git, fs),
actions({
updates: [
{ pageId: 'p-mv', path: 'New/Path.md', basePath: 'Old/Path.md' },
],
}),
);
// The BODY is read from the NEW path (working tree) -> the EDITED body is pushed
// (not lost). The BASE is resolved from the OLD path -> a NON-NULL common
// ancestor ('base body'), so importPageMarkdown does an honest 3-way merge and
// a concurrent Docmost-side edit to a different block is preserved (not rolled
// back to git's body, which a null 2-way base would have done).
expect(client.importPageMarkdown).toHaveBeenCalledWith(
'p-mv',
'edited body',
'base body',
);
// The base lookup hit the OLD path, NOT the new path (the core of the fix).
expect(git.showFileAtRef).toHaveBeenCalledWith(LAST_PUSHED_REF, 'Old/Path.md');
expect(git.showFileAtRef).not.toHaveBeenCalledWith(
LAST_PUSHED_REF,
'New/Path.md',
);
});
it('PURE rename (no body edit) still 3-way merges against the old-path base (no 2-way clobber)', async () => {
// Even a rename with NO body change pushes a body update (F4). Before this fix
// its base would be null (new path absent at last-pushed) -> a 2-way merge that
// could roll a concurrent Docmost edit back to git's body. With basePath=oldPath
// the base equals the (unchanged) body, so the 3-way merge of
// (base=oldBody, incoming=oldBody) is a no-op and any live Docmost edit survives.
const client = makeClient();
const { git } = makeGit({
prevTree: { 'Old.md': fileFor('p-pure', 'same body') },
});
const fs = makeFs({ 'New.md': fileFor('p-pure', 'same body') });
await applyPushActions(
deps(client, git, fs),
actions({
updates: [{ pageId: 'p-pure', path: 'New.md', basePath: 'Old.md' }],
}),
);
// Incoming body == base body == 'same body' -> the 3-way merge is a no-op AND
// preserves any concurrent live edit (the base is the real ancestor, not null).
expect(client.importPageMarkdown).toHaveBeenCalledWith(
'p-pure',
'same body',
'same body',
);
expect(git.showFileAtRef).toHaveBeenCalledWith(LAST_PUSHED_REF, 'Old.md');
});
});
describe('applyPushActions — create (assigned pageId written back to meta)', () => {
@@ -162,12 +162,44 @@ describe('computePushActions — R/C (renamed/moved)', () => {
expect(actions.renamesMoves).toEqual([
{ pageId: 'p-moved', oldPath: 'Old/Path.md', newPath: 'New/Path.md' },
]);
// It is NOT also recorded as a create/update/delete.
// It is ALSO recorded as an UPDATE for the new path (F4) so a body edit
// riding along the rename in the same diff is pushed, not lost. The update
// carries `basePath = oldPath` so the 3-way merge base is resolved from where
// the file lived at last-pushed (the OLD path), not the new path (which would
// return null and degrade to a 2-way clobber). Never a create/delete though.
expect(actions.creates).toEqual([]);
expect(actions.updates).toEqual([]);
expect(actions.updates).toEqual([
{ pageId: 'p-moved', path: 'New/Path.md', basePath: 'Old/Path.md' },
]);
expect(actions.deletes).toEqual([]);
});
it('rename + body edit in one diff -> emits BOTH a rename/move action AND a body update (F4)', () => {
// git `-M` reports a rename WITH a body edit as a single `R` row. The
// move/rename ops never carry page content, so the body edit must ALSO be
// emitted as an UPDATE targeting the NEW path + same pageId — otherwise it is
// silently and permanently lost (F4, the critical data-loss bug).
const changes: DiffEntry[] = [
{ status: 'R', path: 'New/Path.md', oldPath: 'Old/Path.md', score: 75 },
];
const metaAt = metaTable({
'New/Path.md|current': meta({ pageId: 'p-edited' }),
});
const actions = computePushActions({ changes, metaAt });
expect(actions.renamesMoves).toEqual([
{ pageId: 'p-edited', oldPath: 'Old/Path.md', newPath: 'New/Path.md' },
]);
// The body update carries the NEW path so importPageMarkdown reads the moved
// file and targets the moved page by its (stable) pageId; `basePath` is the OLD
// path so the merge base is the pre-rename file (honest 3-way merge).
expect(actions.updates).toEqual([
{ pageId: 'p-edited', path: 'New/Path.md', basePath: 'Old/Path.md' },
]);
expect(actions.creates).toEqual([]);
expect(actions.deletes).toEqual([]);
expect(actions.skipped).toEqual([]);
});
it('copy (C) is recorded like a rename for the deferred apply', () => {
const changes: DiffEntry[] = [
{ status: 'C', path: 'Copy.md', oldPath: 'Src.md', score: 90 },
@@ -179,6 +211,13 @@ describe('computePushActions — R/C (renamed/moved)', () => {
expect(actions.renamesMoves).toEqual([
{ pageId: 'p-copy', oldPath: 'Src.md', newPath: 'Copy.md' },
]);
// The body also rides along as an UPDATE for the new path (F4). For a COPY the
// `basePath` is the SOURCE path (`Src.md`): the source still exists in the
// last-pushed tree, so the copy's body 3-way-merges against its source's
// last-synced text — a real common ancestor, not a null 2-way base.
expect(actions.updates).toEqual([
{ pageId: 'p-copy', path: 'Copy.md', basePath: 'Src.md' },
]);
});
it('renamed file with NO pageId -> skipped', () => {
@@ -212,9 +251,14 @@ describe('computePushActions — mixed batch', () => {
const actions = computePushActions({ changes, metaAt });
expect(actions.creates).toEqual([{ path: 'Fresh.md' }]);
// The R row contributes BOTH a rename/move AND a body update for the new path
// (F4), so `p-mv` appears in updates too — in diff-row order, last.
expect(actions.updates).toEqual([
{ pageId: 'p-rest', path: 'Restored.md' },
{ pageId: 'p-edit', path: 'Edited.md' },
// The rename-derived body update carries basePath = the OLD path (`Srcc.md`)
// for an honest 3-way merge; the plain A/M updates carry no basePath.
{ pageId: 'p-mv', path: 'Dst.md', basePath: 'Srcc.md' },
]);
expect(actions.deletes).toEqual([{ pageId: 'p-rm' }]);
expect(actions.renamesMoves).toEqual([
@@ -241,7 +285,12 @@ describe('computePushActions — ghost-move coalescing (data-loss guard)', () =>
});
const actions = computePushActions({ changes, metaAt });
expect(actions.deletes).toEqual([]); // the page is NEVER trashed
expect(actions.updates).toEqual([]); // not a spurious update either
// The coalesced move ALSO carries a body update for the new path (F4): a body
// edit accompanying the relocation must be pushed, not lost. `basePath` is the
// OLD (deleted) path so the 3-way merge base is the pre-move file, not null.
expect(actions.updates).toEqual([
{ pageId: 'p1', path: '_.md', basePath: '_ ~slug.md' },
]);
expect(actions.renamesMoves).toEqual([
{ pageId: 'p1', oldPath: '_ ~slug.md', newPath: '_.md' },
]);
@@ -270,6 +270,61 @@ describe('#13 conflict markers reach Docmost', () => {
expect(res.applied?.lastPushedAdvanced).toBe(false);
expect(calls.updateRef).toHaveLength(0);
});
it('CREATE branch (autoMergeConflicts on): strips the markers and createPage gets a CLEAN body; the vault file is rewritten clean', async () => {
// The CREATE path strips conflict markers (stripConflictMarkers(rawBody)) and
// passes the CLEANED body to createPage — but only the OFF-case (no createPage)
// and the UPDATE-ON case were covered. A regression passing the RAW body to
// createPage would publish `<<<<<<<`/`=======`/`>>>>>>>` into a brand-new page
// with NO test catching it. Pin: createPage IS called with a marker-free body
// that preserves BOTH sides, and the file on disk is rewritten with that body.
const { git } = makePushGit({
changes: [{ status: 'A', path: 'New.md' }],
});
const createPage = vi.fn(async () => ({ data: { id: 'new-1' } }));
const client = {
listSpaceTree: vi.fn(async () => ({ pages: [], complete: true })),
importPageMarkdown: vi.fn(),
createPage,
deletePage: vi.fn(),
movePage: vi.fn(),
renamePage: vi.fn(),
};
const deps: PushDeps = {
settings: { ...makeSettings(), autoMergeConflicts: true },
git,
makeClient: () => client as any,
// Raw conflict body with NO gitmost_id frontmatter -> classified as CREATE.
readFile: vi.fn(async (path: string) => {
if (path === 'New.md') return conflictBody;
throw new Error(`no such file: ${path}`);
}),
writeFile: vi.fn(async () => {}),
log: () => {},
};
const res = await runPush(deps, { dryRun: false });
expect(res.mode).toBe('apply');
// The page WAS created. The body (2nd positional arg) is marker-free but keeps
// both sides' content.
expect(createPage).toHaveBeenCalledTimes(1);
const createdBody: string = createPage.mock.calls[0][1] as any;
expect(createdBody).not.toContain('<<<<<<<');
expect(createdBody).not.toContain('=======');
expect(createdBody).not.toContain('>>>>>>>');
expect(createdBody).toContain('my line');
expect(createdBody).toContain('their line');
// The file on disk is rewritten with the CLEAN body (no raw markers) so the
// published vault never carries the conflict syntax.
const writeCalls = (deps.writeFile as any).mock.calls as [string, string][];
const newWrite = writeCalls.find(([p]) => p === 'New.md');
expect(newWrite).toBeDefined();
expect(newWrite![1]).not.toMatch(/[<>=]{7}/);
expect(newWrite![1]).toContain('my line');
expect(newWrite![1]).toContain('their line');
});
});
// ---------------------------------------------------------------------------