Skip to content

Commit 81db020

Browse files
tomymaritanotomymaritano
authored
feat(desktop): add first-class conflict screen (#497)
## Summary Sync conflicts used to live only in Settings → Account, and auto-sync never put them on the renderer store. This is the last Inkdrop-gap item (D.10): one screen, three actions. - Modal in the main window (still available inline in Settings) - **Keep this device** / **Keep other** / **Open both** (saves `{title} (remote)`, keeps local, opens both windows) - Auto-sync `sync-success` now carries `conflicts` so the screen actually appears - Footer and header say **Review**, not “resolve in Settings” Does not start a new sync engine. ## Type of Change - [x] New feature ## Related Issues Inkdrop gap plan D.10 ## Checklist - [x] Tests pass locally (`pnpm --filter desktop exec vitest run` on the new files + renderer utils) - [x] Typecheck and eslint on the touched files - [x] PR targets `develop` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a sync conflict review screen with side-by-side and unified comparison views. * Users can keep either version, dismiss conflicts, or open both versions as separate notes. * Sync indicators now show conflict counts and provide a **Review** action. * Added responsive layouts, keyboard controls, progress states, and error feedback. * **Documentation** * Updated the shipped features list to include the new sync conflict workflow. * **Tests** * Added coverage for conflict handling, merging, title formatting, and opening both versions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent a6b958d commit 81db020

16 files changed

Lines changed: 809 additions & 375 deletions

‎apps/desktop/src/main/services/sync/SyncService.ts‎

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import type {
3434
import { MAX_BACKOFF_MS, isNetworkError, isAuthError } from './helpers.js';
3535
import { parseSyncedNote, serializeSyncedNote } from './envelope.js';
3636
import type { SyncCursorStore } from './cursorStore.js';
37+
import { chosenConflictContent, needsLocalRestore } from './resolveNoteConflict.js';
3738

3839
// ============================================================================
3940
// SyncService Class
@@ -51,6 +52,7 @@ export class SyncService {
5152
private abortController: AbortController | null = null;
5253
private statusListener: SyncStatusListener | null = null;
5354
private cursorStore: SyncCursorStore | null = null;
55+
private pendingConflicts = new Map<string, SyncConflict>();
5456

5557
constructor(
5658
apiClient: ApiClient,
@@ -610,6 +612,7 @@ export class SyncService {
610612
type: 'sync-success',
611613
changesApplied: totalApplied,
612614
changesPushed: totalPushed,
615+
conflicts: pullResult.conflicts,
613616
});
614617

615618
return {
@@ -714,16 +717,53 @@ export class SyncService {
714717
throw new Error(`Note ${noteId} not found`);
715718
}
716719

720+
const pending = this.pendingConflicts.get(noteId);
721+
const id = createNoteId(noteId);
722+
const copy = pending?.localCopyId
723+
? await this.noteRepository.get(createNoteId(pending.localCopyId))
724+
: null;
725+
717726
if (resolution === 'local') {
718-
// Keep local version, mark for push to server
719-
this.noteRepository.resetSyncTracking(createNoteId(noteId));
720-
console.warn(`Conflict resolved: keeping local version for ${noteId}, marked for sync`);
727+
const localContent = copy?.content ?? pending?.localContent;
728+
if (localContent != null && needsLocalRestore(note.content, localContent)) {
729+
const title = this.extractTitle(localContent);
730+
await this.noteRepository.save({
731+
...(copy ?? note),
732+
id,
733+
content: localContent,
734+
title,
735+
metadata: {
736+
...(copy ?? note).metadata,
737+
title,
738+
updatedAt: createTimestamp(new Date()),
739+
},
740+
});
741+
}
742+
this.noteRepository.resetSyncTracking(id);
743+
} else if (pending) {
744+
const remoteContent = chosenConflictContent('remote', pending);
745+
if (note.content !== remoteContent) {
746+
const title = this.extractTitle(remoteContent);
747+
await this.noteRepository.save({
748+
...note,
749+
content: remoteContent,
750+
title,
751+
metadata: {
752+
...note.metadata,
753+
title,
754+
updatedAt: createTimestamp(new Date()),
755+
},
756+
});
757+
}
758+
this.noteRepository.markAsSynced(id);
721759
} else {
722-
// Keep remote version (already applied during pull)
723-
// Just mark as synced to clear the conflict state
724-
this.noteRepository.markAsSynced(createNoteId(noteId));
725-
console.warn(`Conflict resolved: keeping remote version for ${noteId}`);
760+
this.noteRepository.markAsSynced(id);
726761
}
762+
763+
if (copy) {
764+
await this.noteRepository.delete(copy.id);
765+
}
766+
this.pendingConflicts.delete(noteId);
727767
}
728768

729769
/**
@@ -881,21 +921,23 @@ export class SyncService {
881921
hasLocalEdits && change.deviceId !== this.apiClient['deviceInfo'].deviceId;
882922

883923
if (isConflict) {
884-
// Store conflict for user resolution
885-
conflicts.push({
924+
const localCopyId = `${change.noteId}-conflict-${Date.now()}`;
925+
const conflict: SyncConflict = {
886926
noteId: change.noteId,
887927
localContent: existingNote.content,
888928
remoteContent: payload.content,
889929
localVersion: change.version - 1, // Estimate
890930
remoteVersion: change.version,
891931
timestamp: new Date().toISOString(),
892-
});
932+
localCopyId,
933+
};
934+
conflicts.push(conflict);
935+
this.pendingConflicts.set(change.noteId, conflict);
893936

894-
// Create a conflict copy
895937
const conflictTitle = `${existingNote.title} (Conflict ${new Date().toLocaleString()})`;
896938
await this.noteRepository.save({
897939
...existingNote,
898-
id: createNoteId(`${change.noteId}-conflict-${Date.now()}`),
940+
id: createNoteId(localCopyId),
899941
title: conflictTitle,
900942
metadata: {
901943
...existingNote.metadata,

‎apps/desktop/src/main/services/sync/__tests__/resolveNoteConflict.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { chosenConflictContent, needsLocalRestore } from '../resolveNoteConflict';
3+
4+
const versions = {
5+
localContent: '# Meeting\n\nLocal',
6+
remoteContent: '# Meeting\n\nRemote',
7+
};
8+
9+
describe('chosenConflictContent', () => {
10+
it('returns the local body when keeping this device', () => {
11+
expect(chosenConflictContent('local', versions)).toBe(versions.localContent);
12+
});
13+
14+
it('returns the remote body when keeping the other device', () => {
15+
expect(chosenConflictContent('remote', versions)).toBe(versions.remoteContent);
16+
});
17+
});
18+
19+
describe('needsLocalRestore', () => {
20+
it('is true after pull overwrote the note with remote', () => {
21+
expect(needsLocalRestore(versions.remoteContent, versions.localContent)).toBe(true);
22+
});
23+
24+
it('is false when the note still has the local body', () => {
25+
expect(needsLocalRestore(versions.localContent, versions.localContent)).toBe(false);
26+
});
27+
});

‎apps/desktop/src/main/services/sync/resolveNoteConflict.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
export interface ConflictVersions {
2+
localContent: string;
3+
remoteContent: string;
4+
}
5+
6+
/** Content that should live on the original note after the user picks a side. */
7+
export function chosenConflictContent(
8+
resolution: 'local' | 'remote',
9+
versions: ConflictVersions
10+
): string {
11+
return resolution === 'local' ? versions.localContent : versions.remoteContent;
12+
}
13+
14+
/**
15+
* Pull already writes the remote body onto the note. Keep-local must restore
16+
* the captured local body before we mark the note dirty for push.
17+
*/
18+
export function needsLocalRestore(currentContent: string, localContent: string): boolean {
19+
return currentContent !== localContent;
20+
}

‎apps/desktop/src/main/services/sync/types.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ export interface SyncConflict {
99
localVersion: number;
1010
remoteVersion: number;
1111
timestamp: string;
12+
/** Disk backup of the local body created before pull overwrote the note. */
13+
localCopyId?: string;
1214
}
1315

1416
export interface SyncResult {
@@ -31,7 +33,12 @@ export interface SyncState {
3133

3234
export type SyncStatusEvent =
3335
| { type: 'sync-start' }
34-
| { type: 'sync-success'; changesApplied: number; changesPushed: number }
36+
| {
37+
type: 'sync-success';
38+
changesApplied: number;
39+
changesPushed: number;
40+
conflicts?: SyncConflict[];
41+
}
3542
| { type: 'sync-error'; error: string; isNetworkError: boolean; consecutiveFailures: number }
3643
| { type: 'needs-setup'; error: string }
3744
| { type: 'auth-expired' };

‎apps/desktop/src/renderer/App.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { NoteWindow } from './components/NoteWindow';
88
import { Sidebar } from './components/sidebar';
99
import { GraphView } from './components/GraphView';
1010
import { CommandPalette } from './components/CommandPalette';
11+
import { ConflictResolver } from './components/sync/ConflictResolver';
1112
import { AiPanel } from './components/ai/AiPanel';
1213
import { LicenseProvider } from './contexts/LicenseContext';
1314
import { ToastProvider } from './components/Toast';
@@ -493,6 +494,7 @@ function NotesApp() {
493494
setTagFilter(name);
494495
}}
495496
/>
497+
<ConflictResolver variant="modal" />
496498
<Toaster />
497499
</div>
498500
</LicenseProvider>

‎apps/desktop/src/renderer/components/sidebar/SidebarFooter.tsx‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { memo, useState, useEffect, useRef, useCallback } from 'react';
2-
import { Cloud, CloudOff, RefreshCw, AlertCircle, Check } from 'lucide-react';
2+
import { Cloud, CloudOff, RefreshCw, AlertCircle, AlertTriangle, Check } from 'lucide-react';
33
import { useAuthStore } from '../../stores/authStore';
44
import {
55
useSyncStore,
@@ -8,6 +8,7 @@ import {
88
selectConsecutiveFailures,
99
selectPendingCount,
1010
selectError,
11+
selectConflicts,
1112
} from '../../stores/syncStore';
1213
import { syncFooterAction, syncFooterErrorLabel } from '../../utils/syncFooterCopy';
1314
import { sc } from './sc';
@@ -53,6 +54,8 @@ const SyncProgressIndicator = memo(function SyncProgressIndicator({
5354
const syncError = useSyncStore(selectError);
5455
const syncNow = useSyncStore(state => state.syncNow);
5556
const refreshPendingCount = useSyncStore(state => state.refreshPendingCount);
57+
const conflicts = useSyncStore(selectConflicts);
58+
const openConflictScreen = useSyncStore(state => state.openConflictScreen);
5659

5760
// Force re-render every 60s so relative time text stays fresh
5861
const [, forceUpdate] = useState(0);
@@ -132,6 +135,25 @@ const SyncProgressIndicator = memo(function SyncProgressIndicator({
132135
);
133136
}
134137

138+
if (conflicts.length > 0) {
139+
const n = conflicts.length;
140+
return (
141+
<div className={sc('sidebar-footer-progress', 'sidebar-footer-progress--conflict')}>
142+
<AlertTriangle size={11} />
143+
<span>
144+
{n} conflict{n === 1 ? '' : 's'}
145+
</span>
146+
<button
147+
type="button"
148+
className={sc('sidebar-footer-progress-retry')}
149+
onClick={() => openConflictScreen()}
150+
>
151+
Review
152+
</button>
153+
</div>
154+
);
155+
}
156+
135157
// Just synced flash
136158
if (showSynced) {
137159
return (

‎apps/desktop/src/renderer/components/sidebar/sidebar.module.css‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,6 +1178,10 @@
11781178
color: var(--danger);
11791179
}
11801180

1181+
.sidebar-footer-progress--conflict {
1182+
color: #f59e0b;
1183+
}
1184+
11811185
.sidebar-footer-progress--offline {
11821186
color: var(--text-muted);
11831187
}

0 commit comments

Comments
 (0)