From 23763970e72bdfbe84ebff1b5fae7f61a566a19d Mon Sep 17 00:00:00 2001 From: flipvanhaaren Date: Tue, 7 Jul 2026 19:45:53 +0200 Subject: [PATCH] feat: graft recorded sync point so merges ignore squash-stale base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Squash syncs advance the recorded sync point (manifest + refs/cella/last-sync) without advancing git's commit graph. The 3-way merge, however, ran against git's own merge-base — stale after any squash — so every sync replayed upstream hunks the fork had already integrated or deliberately resolved away, surfacing as clean auto-merges (e.g. a duplicated block reappearing on each sync) or repeat conflicts. getEffectiveMergeBase already computed the correct base for analysis, but the merge itself did not use it. Add withTemporarySyncBaseGraft: it wraps the merge in a local-only 'git replace --graft' that adds the recorded sync point as a parent of the fork HEAD, so native git resolves the merge against the manifest-tracked base. The replace ref is deleted in a finally (never leaks into pushed history), no-ops when the base is already a native ancestor, and never clobbers an existing replacement. Wired into both the direct-mode merge and the analyze-preview worktree merge. Co-Authored-By: Claude Opus 4.8 --- src/services/merge-engine.ts | 16 ++- src/utils/git.ts | 44 +++++++++ tests/e2e/sync-base-graft.test.ts | 158 ++++++++++++++++++++++++++++++ 3 files changed, 215 insertions(+), 3 deletions(-) create mode 100644 tests/e2e/sync-base-graft.test.ts diff --git a/src/services/merge-engine.ts b/src/services/merge-engine.ts index 4511727..9f51c4e 100644 --- a/src/services/merge-engine.ts +++ b/src/services/merge-engine.ts @@ -50,6 +50,7 @@ import { restoreToHead, stagePath, storeLastSyncRef, + withTemporarySyncBaseGraft, } from '../utils/git'; import { isManagedFile } from '../utils/managed-files'; import { MANIFEST_FILE, type SyncManifest, writeSyncManifest } from '../utils/manifest'; @@ -154,8 +155,13 @@ async function applyDirectMerge( // treat it as a real in-progress merge (3-way conflict view, `git merge --abort` works). // The finishing rerun squashes the commit to a single parent (`commitSquash`) — upstream // ancestry is tracked via the manifest, never via a pushed two-parent merge commit. + // The temporary graft makes the merge 3-way against the recorded sync point instead of + // git's own (squash-stale) merge-base — without it, upstream hunks already integrated by + // previous syncs re-apply or re-conflict on every run. onProgress?.('starting merge in fork...'); - await merge(forkPath, upstreamRef, { noCommit: true, noEdit: true }); + await withTemporarySyncBaseGraft(forkPath, 'HEAD', mergeBaseRef, () => + merge(forkPath, upstreamRef, { noCommit: true, noEdit: true }), + ); // Phase 4: Immediately batch-restore pinned/ignored files. // Single git command restores all files at once, minimizing the window @@ -579,9 +585,13 @@ async function runAnalyzePreview( await createWorktree(forkPath, worktreePath, 'HEAD'); onStep?.('worktree created', worktreePath); - // Perform merge in worktree (always real merge, never --squash, for correct 3-way) + // Perform merge in worktree (always real merge, never --squash, for correct 3-way). + // Replace refs are shared across worktrees, so the temporary graft on the fork's HEAD + // commit steers this worktree merge to the recorded sync point too. onProgress?.('performing merge in worktree...'); - await merge(worktreePath, ctx.upstreamRef, { noCommit: true, noEdit: true }); + await withTemporarySyncBaseGraft(worktreePath, 'HEAD', ctx.mergeBase, () => + merge(worktreePath, ctx.upstreamRef, { noCommit: true, noEdit: true }), + ); onStep?.('merge complete', 'upstream merged into worktree'); // Analyze all files, then enrich with change dates and commit hashes diff --git a/src/utils/git.ts b/src/utils/git.ts index 325ec99..4f5e063 100644 --- a/src/utils/git.ts +++ b/src/utils/git.ts @@ -1097,3 +1097,47 @@ export async function getEffectiveMergeBase( return { base: gitBase, isStale: false, storedRef: recorded }; } + +/** + * Run `fn` with a temporary local graft that makes `baseSha` an ancestor of `headRef`, so + * native git operations — merge-base resolution and the 3-way merge itself — see the recorded + * sync point instead of a stale historical ancestor. + * + * Squash syncs advance the recorded sync point (manifest + `refs/cella/last-sync`) without + * advancing git's commit graph, so a plain `git merge` replays every upstream change since the + * last true merge ancestor: hunks the fork already integrated (or deliberately resolved away) + * re-apply as clean auto-merges or re-conflict on every sync. The graft adds `baseSha` as an + * extra parent of the `headRef` commit via `git replace --graft`; replace refs are local-only + * and the ref is deleted right after `fn`, so nothing ever leaks into pushed history. + * + * Runs `fn` without grafting when the graft is unnecessary or unsafe: + * - `baseSha` is already an ancestor of `headRef` (git's own merge-base is not stale), or + * - a replacement object already exists for the `headRef` commit (never clobber, e.g. the + * scaffold root graft from {@link ensureSyncBase} when the root commit is HEAD). + */ +export async function withTemporarySyncBaseGraft( + cwd: string, + headRef: string, + baseSha: string, + fn: () => Promise, +): Promise { + const headSha = await git(['rev-parse', headRef], cwd, { ignoreErrors: true }); + if (!headSha || !(await commitObjectExists(cwd, baseSha))) return fn(); + + // Native ancestry already covers the base — nothing to graft. + if (await isAncestor(cwd, baseSha, headSha)) return fn(); + + // Never clobber an existing replacement object for this commit. + const existingReplacement = await git(['replace', '-l', headSha], cwd, { ignoreErrors: true }); + if (existingReplacement) return fn(); + + const parentsRaw = await git(['log', '-1', '--format=%P', headSha], cwd, { ignoreErrors: true }); + const parents = parentsRaw ? parentsRaw.split(/\s+/) : []; + + await git(['replace', '-f', '--graft', headSha, ...parents, baseSha], cwd); + try { + return await fn(); + } finally { + await git(['replace', '-d', headSha], cwd, { ignoreErrors: true }); + } +} diff --git a/tests/e2e/sync-base-graft.test.ts b/tests/e2e/sync-base-graft.test.ts new file mode 100644 index 0000000..148bd79 --- /dev/null +++ b/tests/e2e/sync-base-graft.test.ts @@ -0,0 +1,158 @@ +/** + * E2E tests for withTemporarySyncBaseGraft. + * + * Squash syncs advance the recorded sync point (manifest + refs/cella/last-sync) without + * advancing git's commit graph. A plain `git merge` then resolves 3-way against a stale + * historical base and replays every upstream change since — hunks the fork already + * integrated or deliberately resolved away re-apply as clean auto-merges (the classic + * symptom: a duplicated block appearing in a fork file on every sync). The graft makes the + * merge 3-way against the recorded sync point, scoped to the merge and local-only. + */ +import { execSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { getEffectiveMergeBase, merge, withTemporarySyncBaseGraft } from '../../src/utils/git'; + +const UPSTREAM_REMOTE = 'cella-upstream'; +const GIT_USER = 'git config user.email "test@cellajs.com" && git config user.name "Cella Test"'; + +function exec(cmd: string, cwd: string): string { + return execSync(cmd, { cwd, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }).trim(); +} + +function write(repoPath: string, files: Record): void { + for (const [rel, content] of Object.entries(files)) { + const full = path.join(repoPath, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, content); + } +} + +function commitAll(repoPath: string, message: string): string { + exec('git add -A', repoPath); + exec(`git commit -m "${message}"`, repoPath); + return exec('git rev-parse HEAD', repoPath); +} + +const FILE_BASE = 'export function getItems() {\n const a = 1;\n const b = 2;\n return [a, b];\n}\n'; +/** Upstream inserts this block; the fork's squash sync deliberately resolves it away. */ +const UPSTREAM_BLOCK = ' // upstream-only scope block\n const upstreamScope = true;\n'; +const FILE_WITH_BLOCK = `export function getItems() {\n${UPSTREAM_BLOCK} const a = 1;\n const b = 2;\n return [a, b];\n}\n`; + +describe('withTemporarySyncBaseGraft', () => { + let testDir: string; + let upstreamPath: string; + let forkPath: string; + let upstreamBlockSha: string; + + beforeEach(() => { + testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cella-graft-')); + upstreamPath = path.join(testDir, 'upstream'); + forkPath = path.join(testDir, 'fork'); + + // Upstream with real history the fork shares (a true fork, not a scaffold). + fs.mkdirSync(upstreamPath); + exec('git init -b main', upstreamPath); + exec(GIT_USER, upstreamPath); + write(upstreamPath, { 'backend/get-items.ts': FILE_BASE, 'shared/util.ts': 'export const util = 1;\n' }); + commitAll(upstreamPath, 'Initial commit'); + + exec(`git clone "${upstreamPath}" "${forkPath}"`, testDir); + exec(GIT_USER, forkPath); + exec(`git remote add ${UPSTREAM_REMOTE} "${upstreamPath}"`, forkPath); + + // Upstream adds a block the fork does not want. + write(upstreamPath, { 'backend/get-items.ts': FILE_WITH_BLOCK }); + upstreamBlockSha = commitAll(upstreamPath, 'feat: add upstream scope block'); + + // The fork integrates that sync as a SQUASH commit (single parent — git's merge-base + // stays at the initial commit) and resolves the block away, recording the sync point + // in the committed manifest exactly like the sync engine does. + write(forkPath, { + 'cella.manifest.json': `${JSON.stringify({ upstream: { commit: upstreamBlockSha } }, null, 2)}\n`, + }); + commitAll(forkPath, 'chore: sync upstream (squash, block resolved away)'); + + // Upstream moves on with an unrelated change, so the next sync has real content. + write(upstreamPath, { 'shared/util.ts': 'export const util = 2;\n' }); + commitAll(upstreamPath, 'feat: bump util'); + exec(`git fetch ${UPSTREAM_REMOTE}`, forkPath); + }); + + afterEach(() => { + fs.rmSync(testDir, { recursive: true, force: true }); + }); + + it('plain merge replays the resolved-away upstream block (the bug being fixed)', async () => { + // Control: without the graft, git merges 3-way against the stale initial-commit base, + // so the old upstream hunk re-applies as a clean auto-merge. + await merge(forkPath, `${UPSTREAM_REMOTE}/main`, { noCommit: true, noEdit: true }); + + const merged = fs.readFileSync(path.join(forkPath, 'backend/get-items.ts'), 'utf-8'); + expect(merged).toContain('upstreamScope'); + }); + + it('grafted merge honors the recorded sync point: no ghost re-application, upstream news still lands', async () => { + const { base, isStale } = await getEffectiveMergeBase(forkPath, 'HEAD', `${UPSTREAM_REMOTE}/main`); + expect(isStale).toBe(true); + expect(base).toBe(upstreamBlockSha); + + const result = await withTemporarySyncBaseGraft(forkPath, 'HEAD', base, () => + merge(forkPath, `${UPSTREAM_REMOTE}/main`, { noCommit: true, noEdit: true }), + ); + + expect(result.conflicts).toEqual([]); + // The block the fork resolved away must NOT come back... + const merged = fs.readFileSync(path.join(forkPath, 'backend/get-items.ts'), 'utf-8'); + expect(merged).not.toContain('upstreamScope'); + // ...while genuinely new upstream work still syncs. + expect(fs.readFileSync(path.join(forkPath, 'shared/util.ts'), 'utf-8')).toContain('util = 2'); + // A real in-progress merge is left for the caller (MERGE_HEAD intact)... + expect(exec('git rev-parse -q --verify MERGE_HEAD', forkPath)).not.toBe(''); + // ...and the temporary replace ref is gone. + expect(exec('git replace -l', forkPath)).toBe(''); + }); + + it('removes the replace ref even when the merge conflicts', async () => { + // Fork edits the same line upstream changes after the sync point → genuine conflict. + write(forkPath, { 'shared/util.ts': 'export const util = "fork";\n' }); + commitAll(forkPath, 'fork: own util'); + + const { base } = await getEffectiveMergeBase(forkPath, 'HEAD', `${UPSTREAM_REMOTE}/main`); + const result = await withTemporarySyncBaseGraft(forkPath, 'HEAD', base, () => + merge(forkPath, `${UPSTREAM_REMOTE}/main`, { noCommit: true, noEdit: true }), + ); + + expect(result.conflicts).toContain('shared/util.ts'); + expect(exec('git replace -l', forkPath)).toBe(''); + // The conflict is 3-way against the sync point, so the resolved-away block stays gone. + const merged = fs.readFileSync(path.join(forkPath, 'backend/get-items.ts'), 'utf-8'); + expect(merged).not.toContain('upstreamScope'); + }); + + it('no-ops when the base is already a native ancestor (fresh merge-commit history)', async () => { + // A real two-parent merge of the sync point: git's own merge-base is current. + exec(`git merge -s ours --no-edit ${upstreamBlockSha}`, forkPath); + + let replaceListDuringMerge: string | null = null; + await withTemporarySyncBaseGraft(forkPath, 'HEAD', upstreamBlockSha, async () => { + replaceListDuringMerge = exec('git replace -l', forkPath); + return merge(forkPath, `${UPSTREAM_REMOTE}/main`, { noCommit: true, noEdit: true }); + }); + + expect(replaceListDuringMerge).toBe(''); + }); + + it('never clobbers an existing replacement object on the HEAD commit', async () => { + const head = exec('git rev-parse HEAD', forkPath); + const parent = exec('git rev-parse HEAD^', forkPath); + exec(`git replace -f --graft ${head} ${parent}`, forkPath); + + await withTemporarySyncBaseGraft(forkPath, 'HEAD', upstreamBlockSha, async () => 'ok'); + + // The pre-existing replacement must survive untouched. + expect(exec('git replace -l', forkPath)).toBe(head); + }); +});