Skip to content

Commit 6e4c9e2

Browse files
committed
fix(sync): use pre-pull commit as merge base after git pull
After git pull, the sync engine was using HEAD (post-pull) as the 3-way merge base, making it appear that only the target changed. This caused local files to always overwrite remote changes. Now captures HEAD before pulling and uses it as the correct base for post-pull sync. v0.2.2
1 parent 759ad04 commit 6e4c9e2

File tree

11 files changed

+304
-8
lines changed

11 files changed

+304
-8
lines changed

docs/changelog.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Changelog
22

3+
## v0.2.2
4+
5+
- Fix post-pull sync using wrong base: after git pull, the sync engine now uses the pre-pull commit as the merge base, correctly detecting remote vs local changes instead of always keeping local
6+
37
## v0.2.1
48

59
- Handle store config conflicts during git pull

docs/documentation.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,11 @@ Untrack patterns are persisted in the local database and in `sync-settings.json`
245245

246246
If your data directory is connected to a remote git repository, use the **Push** button in the footer to push your store to the remote. This backs up your AI configs and makes them available on other machines.
247247

248-
Use the **Pull** button to pull the latest changes from the remote. This will merge the remote changes into your local store.
248+
Use the **Pull** button to pull the latest changes from the remote. After pulling, the sync engine automatically runs a full sync pass using the **pre-pull state** as the merge base. This correctly detects which files were changed by the remote vs which were changed locally:
249+
250+
- **Only remote changed** — updates are applied to local targets automatically
251+
- **Only local changed** — local changes are preserved and pushed back to the store
252+
- **Both changed** — non-overlapping edits are auto-merged; overlapping edits create a conflict for manual resolution
249253

250254
## Setting Up on a New Machine
251255

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "ai-sync",
33
"author": "Anh-Thi Dinh",
4-
"version": "0.2.1",
4+
"version": "0.2.2",
55
"repository": "https://github.com/anhthiding/ai-sync",
66
"private": true,
77
"license": "MIT",
@@ -39,5 +39,5 @@
3939
"esbuild"
4040
]
4141
},
42-
"packageManager": "pnpm@10.29.3+sha512.498e1fb4cca5aa06c1dcf2611e6fafc50972ffe7189998c409e90de74566444298ffe43e6cd2acdc775ba1aa7cc5e092a8b7054c811ba8c5770f84693d33d2dc"
42+
"packageManager": "pnpm@10.30.2+sha512.36cdc707e7b7940a988c9c1ecf88d084f8514b5c3f085f53a2e244c2921d3b2545bc20dd4ebe1fc245feec463bb298aecea7a63ed1f7680b877dc6379d8d0cb4"
4343
}

packages/server/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "@ai-sync/server",
33
"author": "Anh-Thi Dinh",
4-
"version": "0.2.1",
4+
"version": "0.2.2",
55
"repository": "https://github.com/anhthiding/ai-sync",
66
"private": true,
77
"license": "MIT",

packages/server/src/routes/sync.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,15 @@ export function registerSyncRoutes(app: FastifyInstance, state: AppState): void
2929

3030
try {
3131
const result = await pullStoreChanges();
32+
33+
// After a successful pull, trigger sync using the pre-pull base
34+
// so the engine correctly detects remote changes vs local changes
35+
if (result.pulled && result.prePullCommitHash && state.syncEngine) {
36+
state.syncEngine.syncAfterPull(result.prePullCommitHash).catch((err) => {
37+
console.error('Post-pull sync failed:', err);
38+
});
39+
}
40+
3241
return result;
3342
} catch (err) {
3443
const message = err instanceof Error ? err.message : 'Pull failed';

packages/server/src/services/__tests__/sync-engine.test.ts

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@ import { contentChecksum } from '../checksum.js';
1414
// Mock store-git module: control what "base" version git returns
1515
// and prevent actual git commit/init operations
1616
let mockBaseContent: string | null = null;
17+
let mockBaseContentAt: string | null = null;
1718

1819
vi.mock('../store-git.js', () => ({
1920
queueStoreCommit: vi.fn(),
2021
ensureStoreCommitted: vi.fn().mockResolvedValue(undefined),
2122
getCommittedContent: vi.fn(async () => mockBaseContent),
23+
getCommittedContentAt: vi.fn(async () => mockBaseContentAt),
2224
gitMergeFile: vi.fn(async (base: string, store: string, target: string) => {
2325
// Use real git merge-file via child_process
2426
const { execFile } = await import('node:child_process');
@@ -176,6 +178,10 @@ function setBase(content: string | null) {
176178
mockBaseContent = content;
177179
}
178180

181+
function setBaseAt(content: string | null) {
182+
mockBaseContentAt = content;
183+
}
184+
179185
// ── Setup / Teardown ─────────────────────────────────────────────────────────
180186

181187
beforeEach(async () => {
@@ -209,6 +215,7 @@ beforeEach(async () => {
209215

210216
// Reset mock base
211217
mockBaseContent = null;
218+
mockBaseContentAt = null;
212219

213220
// Create engine and capture broadcasts
214221
engine = new SyncEngine(db);
@@ -1025,3 +1032,211 @@ describe('SyncEngine.syncFile — Git-based 3-way merge', () => {
10251032
expect(getConflicts()).toHaveLength(0);
10261033
});
10271034
});
1035+
1036+
// ══════════════════════════════════════════════════════════════════════════════
1037+
// Post-pull sync tests — verifies correct base resolution after git pull
1038+
// ══════════════════════════════════════════════════════════════════════════════
1039+
1040+
describe('SyncEngine.syncAfterPull — post-pull base resolution', () => {
1041+
// ╔═══════════════════════════════════════════════════════════════════════╗
1042+
// ║ Scenario: Both sides changed after pull → conflict ║
1043+
// ╚═══════════════════════════════════════════════════════════════════════╝
1044+
it('creates conflict when both sides changed from pre-pull base', async () => {
1045+
// Machine A pushed "A2", Machine B has "B1"
1046+
// Pre-pull base was "A1" (original content both machines had)
1047+
// After pull: store=A2 (from remote), target=B1 (local edit)
1048+
setBaseAt('A1'); // getCommittedContentAt returns pre-pull base
1049+
1050+
await writeStoreFile('A2'); // pulled from remote
1051+
await writeTargetFile('B1'); // local edit
1052+
1053+
const checksum = contentChecksum('A1');
1054+
const tf = makeTrackedFile({
1055+
storeChecksum: checksum,
1056+
targetChecksum: checksum,
1057+
lastSyncedAt: new Date().toISOString(),
1058+
});
1059+
db.prepare(
1060+
"INSERT INTO tracked_files (id, repo_id, relative_path, store_checksum, target_checksum, sync_status, last_synced_at) VALUES (?, ?, ?, ?, ?, 'synced', ?)",
1061+
).run(tf.id, tf.repoId, tf.relativePath, tf.storeChecksum, tf.targetChecksum, tf.lastSyncedAt);
1062+
1063+
await engine.syncAfterPull('fake-pre-pull-hash');
1064+
1065+
// Both changed from base "A1" → should create conflict
1066+
const conflicts = getConflicts();
1067+
expect(conflicts).toHaveLength(1);
1068+
expect(conflicts[0].store_content).toBe('A2');
1069+
expect(conflicts[0].target_content).toBe('B1');
1070+
1071+
const updated = getTrackedFile()!;
1072+
expect(updated.syncStatus).toBe('conflict');
1073+
});
1074+
1075+
// ╔═══════════════════════════════════════════════════════════════════════╗
1076+
// ║ Scenario: Only store changed after pull → sync to target ║
1077+
// ╚═══════════════════════════════════════════════════════════════════════╝
1078+
it('syncs store→target when only store changed from pre-pull base', async () => {
1079+
// Store pulled "A2", target still has "A1" (unchanged)
1080+
setBaseAt('A1');
1081+
1082+
await writeStoreFile('A2');
1083+
await writeTargetFile('A1');
1084+
1085+
const checksum = contentChecksum('A1');
1086+
const tf = makeTrackedFile({
1087+
storeChecksum: checksum,
1088+
targetChecksum: checksum,
1089+
lastSyncedAt: new Date().toISOString(),
1090+
});
1091+
db.prepare(
1092+
"INSERT INTO tracked_files (id, repo_id, relative_path, store_checksum, target_checksum, sync_status, last_synced_at) VALUES (?, ?, ?, ?, ?, 'synced', ?)",
1093+
).run(tf.id, tf.repoId, tf.relativePath, tf.storeChecksum, tf.targetChecksum, tf.lastSyncedAt);
1094+
1095+
await engine.syncAfterPull('fake-pre-pull-hash');
1096+
1097+
// Only store changed → target should get the update
1098+
const targetContent = await readTargetFile();
1099+
expect(targetContent).toBe('A2');
1100+
expect(getConflicts()).toHaveLength(0);
1101+
1102+
const updated = getTrackedFile()!;
1103+
expect(updated.syncStatus).toBe('synced');
1104+
});
1105+
1106+
// ╔═══════════════════════════════════════════════════════════════════════╗
1107+
// ║ Scenario: Pulled content same as target → fast path ║
1108+
// ╚═══════════════════════════════════════════════════════════════════════╝
1109+
it('takes fast path when pulled content matches target content', async () => {
1110+
// Both sides end up with same content after pull
1111+
await writeStoreFile('Same');
1112+
await writeTargetFile('Same');
1113+
1114+
const oldChecksum = contentChecksum('Old');
1115+
const tf = makeTrackedFile({
1116+
storeChecksum: oldChecksum,
1117+
targetChecksum: oldChecksum,
1118+
lastSyncedAt: new Date().toISOString(),
1119+
});
1120+
db.prepare(
1121+
"INSERT INTO tracked_files (id, repo_id, relative_path, store_checksum, target_checksum, sync_status, last_synced_at) VALUES (?, ?, ?, ?, ?, 'synced', ?)",
1122+
).run(tf.id, tf.repoId, tf.relativePath, tf.storeChecksum, tf.targetChecksum, tf.lastSyncedAt);
1123+
1124+
await engine.syncAfterPull('fake-pre-pull-hash');
1125+
1126+
const updated = getTrackedFile()!;
1127+
expect(updated.syncStatus).toBe('synced');
1128+
expect(getConflicts()).toHaveLength(0);
1129+
});
1130+
1131+
// ╔═══════════════════════════════════════════════════════════════════════╗
1132+
// ║ Scenario: Non-overlapping changes → auto-merge ║
1133+
// ╚═══════════════════════════════════════════════════════════════════════╝
1134+
it('auto-merges non-overlapping changes after pull', async () => {
1135+
const originalContent = 'Line 1\nLine 2\nLine 3\n';
1136+
setBaseAt(originalContent);
1137+
1138+
// Store (pulled) adds line at top, target adds line at bottom
1139+
await writeStoreFile('Line 0\nLine 1\nLine 2\nLine 3\n');
1140+
await writeTargetFile('Line 1\nLine 2\nLine 3\nLine 4\n');
1141+
1142+
const checksum = contentChecksum(originalContent);
1143+
const tf = makeTrackedFile({
1144+
storeChecksum: checksum,
1145+
targetChecksum: checksum,
1146+
lastSyncedAt: new Date().toISOString(),
1147+
});
1148+
db.prepare(
1149+
"INSERT INTO tracked_files (id, repo_id, relative_path, store_checksum, target_checksum, sync_status, last_synced_at) VALUES (?, ?, ?, ?, ?, 'synced', ?)",
1150+
).run(tf.id, tf.repoId, tf.relativePath, tf.storeChecksum, tf.targetChecksum, tf.lastSyncedAt);
1151+
1152+
await engine.syncAfterPull('fake-pre-pull-hash');
1153+
1154+
const storeContent = await readStoreFile();
1155+
const targetContent = await readTargetFile();
1156+
expect(storeContent).toBe('Line 0\nLine 1\nLine 2\nLine 3\nLine 4\n');
1157+
expect(targetContent).toBe(storeContent);
1158+
expect(getConflicts()).toHaveLength(0);
1159+
1160+
const updated = getTrackedFile()!;
1161+
expect(updated.syncStatus).toBe('synced');
1162+
});
1163+
1164+
// ╔═══════════════════════════════════════════════════════════════════════╗
1165+
// ║ Scenario: Override cleared after sync completes ║
1166+
// ╚═══════════════════════════════════════════════════════════════════════╝
1167+
it('clears baseCommitOverride after syncAfterPull, normal sync uses HEAD', async () => {
1168+
// First: run syncAfterPull with a simple fast-path case
1169+
await writeStoreFile('Same');
1170+
await writeTargetFile('Same');
1171+
1172+
const checksum = contentChecksum('Old');
1173+
const tf = makeTrackedFile({
1174+
storeChecksum: checksum,
1175+
targetChecksum: checksum,
1176+
lastSyncedAt: new Date().toISOString(),
1177+
});
1178+
db.prepare(
1179+
"INSERT INTO tracked_files (id, repo_id, relative_path, store_checksum, target_checksum, sync_status, last_synced_at) VALUES (?, ?, ?, ?, ?, 'synced', ?)",
1180+
).run(tf.id, tf.repoId, tf.relativePath, tf.storeChecksum, tf.targetChecksum, tf.lastSyncedAt);
1181+
1182+
await engine.syncAfterPull('fake-hash');
1183+
1184+
// Clear mocks to track new calls
1185+
const storeGitMod = await import('../store-git.js');
1186+
vi.mocked(storeGitMod.getCommittedContent).mockClear();
1187+
vi.mocked(storeGitMod.getCommittedContentAt).mockClear();
1188+
1189+
// Now run normal syncFile — should use getCommittedContent (HEAD), not getCommittedContentAt
1190+
setBase('Normal base');
1191+
await writeStoreFile('Store v2');
1192+
await writeTargetFile('Normal base');
1193+
1194+
const updatedTf = getTrackedFile()!;
1195+
await engine.syncFile(updatedTf, makeRepo());
1196+
1197+
expect(storeGitMod.getCommittedContent).toHaveBeenCalled();
1198+
expect(storeGitMod.getCommittedContentAt).not.toHaveBeenCalled();
1199+
});
1200+
1201+
// ╔═══════════════════════════════════════════════════════════════════════╗
1202+
// ║ Scenario: Override cleared even on error ║
1203+
// ╚═══════════════════════════════════════════════════════════════════════╝
1204+
it('clears baseCommitOverride even when sync throws', async () => {
1205+
// Make syncAllRepos fail by closing the DB temporarily is hard,
1206+
// so we verify override is null after syncAfterPull by checking
1207+
// a subsequent normal sync doesn't use getCommittedContentAt
1208+
const storeGitMod = await import('../store-git.js');
1209+
1210+
// Run syncAfterPull (will succeed on empty repos list which is fine)
1211+
// Delete the repo from DB so syncAllRepos finds nothing
1212+
db.prepare('DELETE FROM repos').run();
1213+
await engine.syncAfterPull('some-hash');
1214+
1215+
// Re-insert repo
1216+
db.prepare(
1217+
"INSERT INTO repos (id, name, local_path, store_path, status) VALUES (?, ?, ?, ?, 'active')",
1218+
).run(REPO_ID, REPO_NAME, targetRepoPath, STORE_PATH);
1219+
1220+
vi.mocked(storeGitMod.getCommittedContent).mockClear();
1221+
vi.mocked(storeGitMod.getCommittedContentAt).mockClear();
1222+
1223+
// Normal sync should use HEAD
1224+
setBase('Base');
1225+
await writeStoreFile('Changed');
1226+
await writeTargetFile('Base');
1227+
1228+
const tf = makeTrackedFile({
1229+
storeChecksum: contentChecksum('Base'),
1230+
targetChecksum: contentChecksum('Base'),
1231+
lastSyncedAt: new Date().toISOString(),
1232+
});
1233+
db.prepare(
1234+
"INSERT INTO tracked_files (id, repo_id, relative_path, store_checksum, target_checksum, sync_status, last_synced_at) VALUES (?, ?, ?, ?, ?, 'synced', ?)",
1235+
).run(tf.id, tf.repoId, tf.relativePath, tf.storeChecksum, tf.targetChecksum, tf.lastSyncedAt);
1236+
1237+
await engine.syncFile(tf, makeRepo());
1238+
1239+
expect(storeGitMod.getCommittedContent).toHaveBeenCalled();
1240+
expect(storeGitMod.getCommittedContentAt).not.toHaveBeenCalled();
1241+
});
1242+
});

packages/server/src/services/store-git.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ export interface PullResult {
207207
pulled: boolean;
208208
message: string;
209209
storeConflicts?: StoreConfigConflict[];
210+
prePullCommitHash?: string;
210211
}
211212

212213
/** Parse conflict markers from a conflicted file, returning ours and theirs content. */
@@ -252,6 +253,10 @@ export async function pullStoreChanges(): Promise<PullResult> {
252253
}
253254

254255
const branch = await git.branchLocal();
256+
257+
// Capture HEAD before pulling — this is the correct base for 3-way merge
258+
const prePullHash = await getHeadCommitHash();
259+
255260
try {
256261
await git.pull(remote, branch.current);
257262
} catch (err) {
@@ -283,14 +288,19 @@ export async function pullStoreChanges(): Promise<PullResult> {
283288
pulled: true,
284289
message: `Pulled from ${remote}/${branch.current} with config conflicts`,
285290
storeConflicts,
291+
prePullCommitHash: prePullHash ?? undefined,
286292
};
287293
}
288294

289295
// Re-throw if no config conflicts (other errors like network issues)
290296
throw err;
291297
}
292298

293-
return { pulled: true, message: `Pulled from ${remote}/${branch.current}` };
299+
return {
300+
pulled: true,
301+
message: `Pulled from ${remote}/${branch.current}`,
302+
prePullCommitHash: prePullHash ?? undefined,
303+
};
294304
}
295305

296306
/**
@@ -372,6 +382,40 @@ export async function getCommittedContent(relativePath: string): Promise<string
372382
}
373383
}
374384

385+
/**
386+
* Get the committed content of a file at a specific commit ref.
387+
* Used to retrieve the pre-pull base for 3-way merge after git pull.
388+
* Returns null if the file doesn't exist at that commit.
389+
*/
390+
export async function getCommittedContentAt(
391+
relativePath: string,
392+
commitRef: string,
393+
): Promise<string | null> {
394+
if (!git) {
395+
git = createGit(config.storePath);
396+
}
397+
try {
398+
return await git.raw(['show', `${commitRef}:${relativePath}`]);
399+
} catch {
400+
return null;
401+
}
402+
}
403+
404+
/**
405+
* Get the current HEAD commit hash of the store repo.
406+
*/
407+
export async function getHeadCommitHash(): Promise<string | null> {
408+
if (!git) {
409+
git = createGit(config.storePath);
410+
}
411+
try {
412+
const hash = await git.raw(['rev-parse', 'HEAD']);
413+
return hash.trim();
414+
} catch {
415+
return null;
416+
}
417+
}
418+
375419
/**
376420
* Ensure all store changes are committed before using git for comparison.
377421
* This prevents stale HEAD when files were written but not yet committed.

0 commit comments

Comments
 (0)