diff --git a/docs/merge-strategy.md b/docs/merge-strategy.md index 55154386f..7997158f2 100644 --- a/docs/merge-strategy.md +++ b/docs/merge-strategy.md @@ -56,7 +56,7 @@ This document outlines the strategy for resolving merge conflicts in Codex proje ### 6. Codex Cell Files (`files/*.codex`) -- **Strategy**: +- **Strategy**: 1. Parse both versions as JSON 2. Take the newest `metadata` object 3. Merge cells array from both versions @@ -67,6 +67,19 @@ This document outlines the strategy for resolving merge conflicts in Codex proje 5. Complete the merge and sync the entire project. 6. This approach will trigger the merge conflict view of the codex cell editor. The user will be forced to resolve the cell-level conflicts manually when they open the file. +### 7. Unrecognized Files (Default: Simple 3-Way) + +- **Files**: Any file not matching the patterns above (e.g., `.yaml`, `.py`, `.txt`, etc.) +- **Strategy**: `SIMPLE_3WAY` — 3-way base comparison + 1. Compare `ours` (local) against `base` (common ancestor) + 2. Compare `theirs` (remote) against `base` + 3. Resolution: + - If only remote changed from base → accept remote version + - If only local changed from base → keep local version + - If both changed from base → keep local version (local wins) + - If neither changed → keep local version +- **Rationale**: Prevents silent data loss when remote-only changes to unrecognized file types are discarded during diverged merges. Previously, the `OVERRIDE` strategy was used as the default, which unconditionally kept the local version regardless of whether it had actually changed. + Note: we also need to just ignore some files, like `complete_drafts.txt`, as they are auto-generated and not meant to be merged. ## Implementation Steps diff --git a/src/projectManager/utils/merge/resolvers.ts b/src/projectManager/utils/merge/resolvers.ts index 2d99d432b..9dcfcc77d 100644 --- a/src/projectManager/utils/merge/resolvers.ts +++ b/src/projectManager/utils/merge/resolvers.ts @@ -124,7 +124,7 @@ function mergeValidatedByLists( * - Rule 1: Preserve local isOldProject in each entry (never overwrite from remote) * - Rule 2: Merge swapEntries by swapUUID (unique identifier for each swap) * - Rule 3: For matching entries, merge swappedUsers arrays - * + * * All swap info is now contained in each entry (swapUUID, isOldProject, oldProjectUrl, etc.) */ function mergeProjectSwap( @@ -237,10 +237,10 @@ function mergeProjectSwap( * Union by hash: combine all entries. For same hash, merge referencedBy and originalNames. */ export function mergeOriginalFilesHashes( - base: { version?: number; files?: Record; fileNameToHash?: Record } | undefined, - ours: { version?: number; files?: Record; fileNameToHash?: Record } | undefined, - theirs: { version?: number; files?: Record; fileNameToHash?: Record } | undefined -): { version: number; files: Record; fileNameToHash: Record } | undefined { + base: { version?: number; files?: Record; fileNameToHash?: Record; } | undefined, + ours: { version?: number; files?: Record; fileNameToHash?: Record; } | undefined, + theirs: { version?: number; files?: Record; fileNameToHash?: Record; } | undefined +): { version: number; files: Record; fileNameToHash: Record; } | undefined { const ourFiles = ours?.files || {}; const theirFiles = theirs?.files || {}; const baseFiles = base?.files || {}; @@ -604,6 +604,32 @@ export async function resolveConflictFile( break; } + // SIMPLE_3WAY = "simple-3way", // 3-way base comparison for unknown file types + case ConflictResolutionStrategy.SIMPLE_3WAY: { + debugLog("Resolving with simple 3-way merge for:", conflict.filepath); + const oursMatchesBase = conflict.ours === conflict.base; + const theirsMatchesBase = conflict.theirs === conflict.base; + + if (oursMatchesBase && !theirsMatchesBase) { + // Only remote changed from base — accept remote version + debugLog("Only remote changed, taking theirs for:", conflict.filepath); + resolvedContent = conflict.theirs; + } else if (!oursMatchesBase && theirsMatchesBase) { + // Only local changed from base — keep our version + debugLog("Only local changed, keeping ours for:", conflict.filepath); + resolvedContent = conflict.ours; + } else if (oursMatchesBase && theirsMatchesBase) { + // Neither changed from base (shouldn't be a conflict, but handle gracefully) + debugLog("Neither side changed from base, keeping ours for:", conflict.filepath); + resolvedContent = conflict.ours; + } else { + // Both changed from base — local wins (safe default) + debugLog("Both sides changed from base, keeping ours for:", conflict.filepath); + resolvedContent = conflict.ours; + } + break; + } + default: resolvedContent = conflict.ours; // Default to our version } @@ -1434,7 +1460,7 @@ export async function resolveCommentThreadsConflict( ); } - // Convert to relative paths + // Convert to relative paths migratedTheirThread = convertThreadToRelativePaths(migratedTheirThread); if (!existingThread) { @@ -1618,7 +1644,7 @@ export async function resolveCommentThreadsConflict( /** * Merges audio attachments from two cell versions, preserving all recordings * @param ourAttachments Our version of attachments - * @param theirAttachments Their version of attachments + * @param theirAttachments Their version of attachments * @returns Merged attachments object */ export function mergeAttachments( diff --git a/src/projectManager/utils/merge/strategies.ts b/src/projectManager/utils/merge/strategies.ts index 8cf1aa8bd..e16674596 100644 --- a/src/projectManager/utils/merge/strategies.ts +++ b/src/projectManager/utils/merge/strategies.ts @@ -37,6 +37,10 @@ export const filePatternsToResolve: Record // JSON settings files - 3-way merge with intelligent conflict resolution [ConflictResolutionStrategy.JSON_MERGE_3WAY]: [".vscode/settings.json"], + + // Simple 3-way merge for unrecognized files (used as the default fallback) + // If only one side changed from base, take that side; if both changed, local wins + [ConflictResolutionStrategy.SIMPLE_3WAY]: [], }; export function determineStrategy(filePath: string): ConflictResolutionStrategy { @@ -75,7 +79,7 @@ export function determineStrategy(filePath: string): ConflictResolutionStrategy console.warn( "No merge strategy found for file:", filePath, - "defaulting to OVERRIDE (take the newest version)" + "defaulting to SIMPLE_3WAY (3-way base comparison)" ); - return ConflictResolutionStrategy.OVERRIDE; + return ConflictResolutionStrategy.SIMPLE_3WAY; } diff --git a/src/projectManager/utils/merge/types.ts b/src/projectManager/utils/merge/types.ts index f3c0b7399..5e8c27c3e 100644 --- a/src/projectManager/utils/merge/types.ts +++ b/src/projectManager/utils/merge/types.ts @@ -9,6 +9,7 @@ export enum ConflictResolutionStrategy { CODEX_CUSTOM_MERGE = "codex", // Special merge process for cell arrays JSONL = "jsonl", // Combine and deduplicate JSONL files JSON_MERGE_3WAY = "json-merge-3way", // 3-way merge for JSON settings with chatSystemMessage tie-breaker + SIMPLE_3WAY = "simple-3way", // 3-way base comparison: accept remote-only changes, keep local-only changes, local wins on true conflicts } export interface SmartEdit { diff --git a/src/test/suite/mergeStrategies.test.ts b/src/test/suite/mergeStrategies.test.ts index 09a8e8d34..01b6a4ab8 100644 --- a/src/test/suite/mergeStrategies.test.ts +++ b/src/test/suite/mergeStrategies.test.ts @@ -49,10 +49,10 @@ suite("Merge Strategies Test Suite", () => { assert.strictEqual(strategy5, ConflictResolutionStrategy.IGNORE); }); - test("should default to OVERRIDE for unknown file types", () => { + test("should default to SIMPLE_3WAY for unknown file types", () => { const unknownFile = "unknown_file.txt"; const strategy = determineStrategy(unknownFile); - assert.strictEqual(strategy, ConflictResolutionStrategy.OVERRIDE); + assert.strictEqual(strategy, ConflictResolutionStrategy.SIMPLE_3WAY); }); test("should handle path separators correctly", () => {