Fix: Fetch remote shards before push to enable proper merging

CRITICAL BUG FIX:
- Push was overwriting remote data because getRemoteItemData only
  handled type='items' but we switched to type='sharded'
- This caused machines to completely overwrite each other's data
  instead of merging

Solution:
- Added fetchResolvedShards() to load shard data before push
- Pass resolved shards through the push pipeline
- Now push properly merges local items with remote items

Refactored sync-engine.ts into operations.ts to stay under 200 lines.

This implements CRDT-style additive merging where both machines'
items are preserved instead of one overwriting the other.

Author: ErcinDedeoglu

eslint.config.js

@@ -25,6 +25,7 @@ export default tseslint.config(
     rules: {
       // ═══════════════════════════════════════════════════════════════════════
       // COMPLEXITY RULES - Keep code readable for humans and LLMs
+      // ⚠️ DO NOT MODIFY THESE LIMITS - Refactor code instead of changing rules
       // ═══════════════════════════════════════════════════════════════════════
 
       // Max 200 lines per file - keeps files small and LLM-friendly

src/sync/engine/helpers.ts

@@ -11,11 +11,15 @@ import type {
   SyncConfig,
   Tombstone,
 } from '../../types/index.js';
-import type { CategoryData, PassphraseOption } from '../operations/types.js';
+import { isShardedRef } from '../../types/index.js';
+import type { CategoryData, PassphraseOption, ResolvedShard } from '../operations/types.js';
 import { isItemCategoryData } from '../operations/types.js';
 import type { PullOptions } from '../operations/pull.js';
 import type { PreparePushResult } from '../operations/push.js';
 import { preparePushData } from '../operations/push.js';
+import type { StorageBackend } from '../../storage/index.js';
+import { fetchCategoryShard } from './manifest.js';
+import { syncLog } from './logger.js';
 
 /**
  * Build checksums map from local item category data for merge-based pull.
@@ -63,8 +67,19 @@ export interface PushOptsBase {
 }
 
 /** Execute push and return files for storage */
-export function executePush(opts: PushOptsBase, remoteManifest?: Manifest): PreparePushResult {
-  return preparePushData(remoteManifest ? { ...opts, remoteManifest } : opts);
+export function executePush(
+  opts: PushOptsBase,
+  remoteManifest?: Manifest,
+  resolvedShards?: Record<SyncCategory, ResolvedShard>
+): PreparePushResult {
+  const pushOpts = { ...opts } as Parameters<typeof preparePushData>[0];
+  if (remoteManifest) {
+    pushOpts.remoteManifest = remoteManifest;
+  }
+  if (resolvedShards) {
+    pushOpts.resolvedShards = resolvedShards;
+  }
+  return preparePushData(pushOpts);
 }
 
 /** Convert push files to storage format */
@@ -105,3 +120,38 @@ export function extractTombstoneIds(
   }
   return result;
 }
+
+/**
+ * Fetch resolved shard data for sharded categories.
+ * This ensures we merge with existing remote data instead of overwriting.
+ */
+export async function fetchResolvedShards(
+  backend: StorageBackend,
+  remote?: Manifest
+): Promise<Record<SyncCategory, ResolvedShard> | undefined> {
+  if (!remote) return undefined;
+
+  const shardedCategories: SyncCategory[] = ['sessions', 'messages'];
+  const resolved: Record<SyncCategory, ResolvedShard> = {} as Record<SyncCategory, ResolvedShard>;
+  let hasAny = false;
+
+  for (const category of shardedCategories) {
+    const info = remote.categories[category];
+    if (info && isShardedRef(info)) {
+      syncLog(`[PUSH] Fetching shard for ${category}: ${info.shardFile}`);
+      const shard = await fetchCategoryShard(backend, info.shardFile);
+      if (shard) {
+        resolved[category] = {
+          items: shard.items,
+          tombstones: shard.tombstones,
+        };
+        hasAny = true;
+        syncLog(
+          `[PUSH] Loaded ${String(Object.keys(shard.items).length)} remote ${category} for merge`
+        );
+      }
+    }
+  }
+
+  return hasAny ? resolved : undefined;
+}

src/sync/engine/operations.ts

@@ -0,0 +1,121 @@
+/**
+ * Sync Engine Operations
+ *
+ * Push, pull, and conflict resolution operations for the sync engine.
+ */
+
+import type { StorageBackend } from '../../storage/index.js';
+import type { Manifest, SyncResult, LocalSyncState, SyncConfig } from '../../types/index.js';
+import type { CategoryData } from '../operations/types.js';
+import { pullCategories } from '../operations/pull.js';
+import { mergeAllCategories } from '../operations/merge-operation.js';
+import { syncLog } from './logger.js';
+import { MANIFEST_FILENAME } from './types.js';
+import { buildLocalState, getStorageFilesMap, mergeDataForState } from './state.js';
+import {
+  buildPushResult,
+  buildPullResult,
+  buildConflictResult,
+  buildErrorResult,
+} from './result.js';
+import {
+  buildPullOptions,
+  executePush,
+  toStorageFiles,
+  buildCryptoOptions,
+  extractTombstoneIds,
+  fetchResolvedShards,
+} from './helpers.js';
+
+export interface OperationContext {
+  backend: StorageBackend;
+  config: SyncConfig;
+  passphrase: string | undefined;
+  oldPassphrase: string | undefined;
+  getStorageId: () => string;
+}
+
+export interface PushContext extends OperationContext {
+  localState: LocalSyncState | null;
+}
+
+/** Execute a push operation and return updated local state */
+export async function executePushOperation(
+  ctx: PushContext,
+  data: CategoryData[],
+  remote?: Manifest
+): Promise<{ result: SyncResult; newState: LocalSyncState }> {
+  const existing = (await ctx.backend.listFiles()).map((f) => f.filename);
+
+  // Pre-fetch shards for sharded categories to enable proper merging
+  const resolvedShards = await fetchResolvedShards(ctx.backend, remote);
+
+  const opts = {
+    localData: data,
+    config: ctx.config,
+    localState: ctx.localState,
+    passphrase: buildCryptoOptions(ctx.passphrase, ctx.oldPassphrase),
+    existingFiles: existing,
+  };
+  const { files, manifest, changedCategories } = executePush(opts, remote, resolvedShards);
+  const fileCount = Object.keys(files).length;
+  syncLog(`[SYNC] Push: ${String(fileCount)} files, ${String(existing.length)} existing`);
+  await ctx.backend.updateFiles(
+    toStorageFiles(files, MANIFEST_FILENAME, JSON.stringify(manifest, null, 2))
+  );
+  const newState = buildLocalState(manifest, data, ctx.getStorageId(), ctx.config.machineId);
+  return { result: buildPushResult(changedCategories), newState };
+}
+
+/** Execute a pull operation and return updated local state */
+export async function executePullOperation(
+  ctx: OperationContext,
+  remote?: Manifest,
+  data?: CategoryData[]
+): Promise<{ result: SyncResult; newState: LocalSyncState | null }> {
+  if (!remote) return { result: buildErrorResult('No remote data found'), newState: null };
+  const sf = await getStorageFilesMap(ctx.backend);
+  const opts = buildPullOptions(
+    {
+      manifest: remote,
+      storageFiles: sf,
+      enabledCategories: ctx.config.sync,
+      passphrase: buildCryptoOptions(ctx.passphrase, ctx.oldPassphrase),
+      backend: ctx.backend,
+    },
+    data
+  );
+  const { pulledData, changedCategories, tombstonedItems } = await pullCategories(opts);
+  syncLog(`[SYNC] Pull: ${String(changedCategories.length)} categories`);
+  const mergedData = mergeDataForState(data, pulledData);
+  const newState = buildLocalState(remote, mergedData, ctx.getStorageId(), ctx.config.machineId);
+  const tombstoneIds = extractTombstoneIds(tombstonedItems);
+  return {
+    result: buildPullResult({ changedCategories, pulledData, tombstonedItems: tombstoneIds }),
+    newState,
+  };
+}
+
+export interface ConflictContext extends PushContext {
+  pushFn: (data: CategoryData[], retry: number, manifest: Manifest) => Promise<SyncResult>;
+}
+
+/** Handle conflict by merging and pushing */
+export async function executeConflictOperation(
+  ctx: ConflictContext,
+  data: CategoryData[],
+  manifest: Manifest,
+  retry: number
+): Promise<SyncResult> {
+  const sf = await getStorageFilesMap(ctx.backend);
+  const mergeCtx = {
+    remoteManifest: manifest,
+    storageFiles: sf,
+    localState: ctx.localState,
+    passphrase: buildCryptoOptions(ctx.passphrase, ctx.oldPassphrase),
+    machineId: ctx.config.machineId,
+    backend: ctx.backend,
+  };
+  const { mergedData, conflicts } = await mergeAllCategories(data, mergeCtx);
+  return buildConflictResult(await ctx.pushFn(mergedData, retry, manifest), conflicts);
+}

src/sync/engine/sync-engine.ts

@@ -1,8 +1,6 @@
 import type { StorageBackend } from '../../storage/index.js';
 import { RepoConflictError } from '../../storage/index.js';
 import { syncLog } from './logger.js';
-import { pullCategories } from '../operations/pull.js';
-import { mergeAllCategories } from '../operations/merge-operation.js';
 import {
   createEmptyManifest,
   type Manifest,
@@ -11,32 +9,18 @@ import {
 } from '../../types/index.js';
 import type { CategoryData } from '../operations/types.js';
 import type { SyncEngineOptions } from './types.js';
-import { MANIFEST_FILENAME } from './types.js';
 import { fetchManifest } from './manifest.js';
-import {
-  buildLocalState,
-  isLockedByOther,
-  getStorageFilesMap,
-  mergeDataForState,
-} from './state.js';
-import {
-  buildPushResult,
-  buildPullResult,
-  buildConflictResult,
-  buildErrorResult,
-  buildSkippedResult,
-  handleSyncError,
-} from './result.js';
+import { isLockedByOther } from './state.js';
+import { buildErrorResult, buildSkippedResult, handleSyncError } from './result.js';
 import { checkMaxRetries, calculateBackoff, sleep } from './retry.js';
 import { acquireLock, releaseLock, getLockHolder } from '../local-lock.js';
-import {
-  buildPullOptions,
-  executePush,
-  toStorageFiles,
-  buildCryptoOptions,
-  extractTombstoneIds,
-} from './helpers.js';
 import { determineAction, executeRoute } from './routing.js';
+import {
+  executePushOperation,
+  executePullOperation,
+  executeConflictOperation,
+  type PushContext,
+} from './operations.js';
 
 export { type CategoryData };
 
@@ -109,10 +93,22 @@ export class SyncEngine {
   private hasStorageConfigured(): boolean {
     return Boolean(this.config.repoOwner && this.config.repoName);
   }
+
   private getStorageId(): string {
     return `${this.config.repoOwner ?? ''}/${this.config.repoName ?? ''}`;
   }
 
+  private getOperationContext(): PushContext {
+    return {
+      backend: this.backend,
+      config: this.config,
+      passphrase: this.passphrase,
+      oldPassphrase: this.oldPassphrase,
+      getStorageId: () => this.getStorageId(),
+      localState: this.localState,
+    };
+  }
+
   private async syncWithRetry(data: CategoryData[], retry: number): Promise<SyncResult> {
     if (!this.hasStorageConfigured()) return buildErrorResult('No storage configured');
     try {
@@ -149,61 +145,29 @@ export class SyncEngine {
   }
 
   private async performPush(data: CategoryData[], remote?: Manifest): Promise<SyncResult> {
-    const existing = (await this.backend.listFiles()).map((f) => f.filename);
-    const opts = {
-      localData: data,
-      config: this.config,
-      localState: this.localState,
-      passphrase: buildCryptoOptions(this.passphrase, this.oldPassphrase),
-      existingFiles: existing,
-    };
-    const { files, manifest, changedCategories } = executePush(opts, remote);
-    const fileCount = Object.keys(files).length;
-    syncLog(`[SYNC] Push: ${String(fileCount)} files, ${String(existing.length)} existing`);
-    await this.backend.updateFiles(
-      toStorageFiles(files, MANIFEST_FILENAME, JSON.stringify(manifest, null, 2))
-    );
-    this.localState = buildLocalState(manifest, data, this.getStorageId(), this.config.machineId);
-    return buildPushResult(changedCategories);
+    const ctx = this.getOperationContext();
+    const { result, newState } = await executePushOperation(ctx, data, remote);
+    this.localState = newState;
+    return result;
   }
 
   private async performPull(remote?: Manifest, data?: CategoryData[]): Promise<SyncResult> {
     const m = remote ?? (await fetchManifest(this.backend));
-    if (!m) return buildErrorResult('No remote data found');
-    const sf = await getStorageFilesMap(this.backend);
-    const opts = buildPullOptions(
-      {
-        manifest: m,
-        storageFiles: sf,
-        enabledCategories: this.config.sync,
-        passphrase: buildCryptoOptions(this.passphrase, this.oldPassphrase),
-        backend: this.backend,
-      },
-      data
-    );
-    const { pulledData, changedCategories, tombstonedItems } = await pullCategories(opts);
-    syncLog(`[SYNC] Pull: ${String(changedCategories.length)} categories`);
-    const mergedData = mergeDataForState(data, pulledData);
-    this.localState = buildLocalState(m, mergedData, this.getStorageId(), this.config.machineId);
-    const tombstoneIds = extractTombstoneIds(tombstonedItems);
-    return buildPullResult({ changedCategories, pulledData, tombstonedItems: tombstoneIds });
+    const ctx = this.getOperationContext();
+    const { result, newState } = await executePullOperation(ctx, m ?? undefined, data);
+    if (newState) this.localState = newState;
+    return result;
   }
 
   private async handleConflict(
     data: CategoryData[],
     m: Manifest,
     retry: number
   ): Promise<SyncResult> {
-    const sf = await getStorageFilesMap(this.backend);
     const ctx = {
-      remoteManifest: m,
-      storageFiles: sf,
-      localState: this.localState,
-      passphrase: buildCryptoOptions(this.passphrase, this.oldPassphrase),
-      machineId: this.config.machineId,
-      backend: this.backend,
+      ...this.getOperationContext(),
+      pushFn: (d: CategoryData[], r: number, man: Manifest) => this.push(d, r, man),
     };
-    const { mergedData, conflicts } = await mergeAllCategories(data, ctx);
-    return buildConflictResult(await this.push(mergedData, retry, m), conflicts);
+    return executeConflictOperation(ctx, data, m, retry);
   }
 }