diff --git a/src/i18n/locales/en.ts b/src/i18n/locales/en.ts index 8bf6f56..975d2c9 100644 --- a/src/i18n/locales/en.ts +++ b/src/i18n/locales/en.ts @@ -177,10 +177,10 @@ const en = { 'sourceControl.filter.needsSync': 'Needs Sync', 'sourceControl.filter.changes': 'Changes', 'sourceControl.filter.local': 'Local', - 'sourceControl.filter.remote': 'Remote', + 'sourceControl.filter.remote': 'Incoming', 'sourceControl.filter.conflict': 'Conflict', 'sourceControl.filter.readyToPush': 'Ready to Push', - 'sourceControl.filter.remoteChanges': 'Remote Changes', + 'sourceControl.filter.remoteChanges': 'Incoming', 'sourceControl.filter.conflicts': 'Conflicts', 'sourceControl.filter.synced': 'Synced', 'sourceControl.filter.showSynced': 'Show synced', diff --git a/src/i18n/locales/zh-cn.ts b/src/i18n/locales/zh-cn.ts index 86432f0..6b1ef3f 100644 --- a/src/i18n/locales/zh-cn.ts +++ b/src/i18n/locales/zh-cn.ts @@ -179,10 +179,10 @@ const zhCn: Partial> = { 'sourceControl.filter.needsSync': '待同步', 'sourceControl.filter.changes': '更改', 'sourceControl.filter.local': '本地', - 'sourceControl.filter.remote': '远程', + 'sourceControl.filter.remote': '传入', 'sourceControl.filter.conflict': '冲突', 'sourceControl.filter.readyToPush': '待推送', - 'sourceControl.filter.remoteChanges': '远程更改', + 'sourceControl.filter.remoteChanges': '传入', 'sourceControl.filter.conflicts': '冲突', 'sourceControl.filter.synced': '已同步', 'sourceControl.filter.showSynced': '显示已同步', diff --git a/src/i18n/locales/zh-tw.ts b/src/i18n/locales/zh-tw.ts index 044d1b3..800c56a 100644 --- a/src/i18n/locales/zh-tw.ts +++ b/src/i18n/locales/zh-tw.ts @@ -179,10 +179,10 @@ const zhTw: Partial> = { 'sourceControl.filter.needsSync': '待同步', 'sourceControl.filter.changes': '變更', 'sourceControl.filter.local': '本地', - 'sourceControl.filter.remote': '遠端', + 'sourceControl.filter.remote': '傳入', 'sourceControl.filter.conflict': '衝突', 'sourceControl.filter.readyToPush': '待推送', - 'sourceControl.filter.remoteChanges': '遠端變更', + 'sourceControl.filter.remoteChanges': '傳入', 'sourceControl.filter.conflicts': '衝突', 'sourceControl.filter.synced': '已同步', 'sourceControl.filter.showSynced': '顯示已同步', diff --git a/src/logic/source-control/ChangeActionPolicy.ts b/src/logic/source-control/ChangeActionPolicy.ts index 548bb58..c36bbb7 100644 --- a/src/logic/source-control/ChangeActionPolicy.ts +++ b/src/logic/source-control/ChangeActionPolicy.ts @@ -36,10 +36,11 @@ export function defaultSyncAction(kind: SyncChangeKind): DefaultSyncAction { /** * Whether a change kind has something on the remote it can pull/restore — - * `remote-only` (never existed locally) and `local-deleted` (tracked file - * removed locally, still present on remote) both do. Drives whether a row - * renders the inline Download button. + * `remote-only` (never existed locally), `remote-modified` (tracked file + * changed only on the remote), and `local-deleted` (tracked file removed + * locally, still present on remote) all do. Drives whether a row renders the + * inline Download button. */ export function canDownload(kind: SyncChangeKind): boolean { - return kind === 'remote-only' || kind === 'local-deleted'; + return kind === 'remote-only' || kind === 'remote-modified' || kind === 'local-deleted'; } diff --git a/src/logic/source-control/FileStatusAdapter.ts b/src/logic/source-control/FileStatusAdapter.ts index 2d9a9f9..4230017 100644 --- a/src/logic/source-control/FileStatusAdapter.ts +++ b/src/logic/source-control/FileStatusAdapter.ts @@ -4,6 +4,7 @@ import { toChangeId, type SyncChange, type SyncChangeKind } from './types'; const KIND_BY_STATUS: Record = { synced: 'synced', modified: 'local-modified', + 'remote-modified': 'remote-modified', unsynced: 'local-only', 'remote-only': 'remote-only', 'local-deleted': 'local-deleted', @@ -16,14 +17,9 @@ const KIND_BY_STATUS: Record = { * Source Control `ChangeRepository` / `SourceControlViewModel` layer added in * Phase 1. * - * Two known gaps versus the full `SyncChangeKind` model, both pre-existing - * limits of `FileStatus` rather than anything introduced here: + * One known gap versus the full `SyncChangeKind` model, a pre-existing limit + * of `FileStatus` rather than anything introduced here: * - * - `FileStatus.status` never distinguishes which side changed for a - * two-sided diff (`SyncStatusService.classify` collapses both directions - * into `'modified'`), so `'modified'` maps to `'local-modified'` as a - * best-effort approximation: the row therefore offers both push and pull - * regardless of which side actually changed. * - No `FileStatus` value ever produces `'conflict'`: conflicts are only * detected during `SyncManager.pushFiles` (via `SyncPlanner.classify` * against a stored base sha) and resolved interactively through diff --git a/src/logic/sync-status-service.ts b/src/logic/sync-status-service.ts index 92f0d38..0758a74 100644 --- a/src/logic/sync-status-service.ts +++ b/src/logic/sync-status-service.ts @@ -12,7 +12,7 @@ import type { TFile } from 'obsidian'; * "Deleted locally" and the other as "Remote available" instead of conflating * the two under a single `remote-only` state. */ -export type SyncStatus = 'synced' | 'modified' | 'unsynced' | 'remote-only' | 'local-deleted' | 'moved'; +export type SyncStatus = 'synced' | 'modified' | 'remote-modified' | 'unsynced' | 'remote-only' | 'local-deleted' | 'moved'; /** The complete status record presented by a sync-status view. */ export interface FileStatus { @@ -35,12 +35,18 @@ export interface FileStatus { * file was previously synced locally (sync metadata exists for the path) and * has since been removed, so it classifies as `local-deleted` rather than * `remote-only`. + * + * When both sides exist and differ, `localChanged`/`remoteChanged` (each + * relative to the last-synced baseline sha) let `classify` tell "only the + * remote side moved" apart from "the local side moved" or "both did" — + * without them (no baseline on record) the two-sided diff falls back to the + * direction-blind `modified`. */ export type SyncStatusFacts = | { movedFrom: string } | { localExists: true; remoteExists: false } | { localExists: false; remoteExists: true; wasTracked?: boolean } - | { localExists: true; remoteExists: true; contentsEqual: boolean }; + | { localExists: true; remoteExists: true; contentsEqual: boolean; localChanged?: boolean; remoteChanged?: boolean }; /** Resolves sync facts into the one status the UI may present for a file. */ export class SyncStatusService { @@ -51,7 +57,9 @@ export class SyncStatusService { if ('movedFrom' in facts) return 'moved'; if (facts.localExists && !facts.remoteExists) return 'unsynced'; if (!facts.localExists && facts.remoteExists) return facts.wasTracked ? 'local-deleted' : 'remote-only'; - return facts.contentsEqual ? 'synced' : 'modified'; + if (facts.contentsEqual) return 'synced'; + if (facts.localChanged === false && facts.remoteChanged === true) return 'remote-modified'; + return 'modified'; } get size(): number { return this.statuses.size; } diff --git a/src/logic/sync/SyncStatusRefreshService.ts b/src/logic/sync/SyncStatusRefreshService.ts index 9c95f2e..e6b5236 100644 --- a/src/logic/sync/SyncStatusRefreshService.ts +++ b/src/logic/sync/SyncStatusRefreshService.ts @@ -210,6 +210,24 @@ export class SyncStatusRefreshService { return isSyncMetadataAtPath(pathMetadata, vaultPath) && !pathMetadata.renamedFrom; } + /** The last-synced blob sha on record for `path`, or undefined if never tracked there. */ + private baseShaFor(path: string): string | undefined { + const metadata = this.dependencies.settings().syncMetadata; + const pathMetadata = metadata ? metadata[path] : undefined; + return isSyncMetadataAtPath(pathMetadata, path) ? pathMetadata.lastSyncedSha : undefined; + } + + /** + * Direction facts for a two-sided diff, relative to the last-synced + * baseline: undefined for both when there is no baseline on record (the + * two-sided diff then falls back to the direction-blind `modified`). + */ + private diffDirection(path: string, localSha: string, remoteSha: string): { localChanged?: boolean; remoteChanged?: boolean } { + const baseSha = this.baseShaFor(path); + if (baseSha === undefined) return {}; + return { localChanged: localSha !== baseSha, remoteChanged: remoteSha !== baseSha }; + } + async reconcileOutOfBandMoves(remoteMap: Map): Promise { const orphansBySha = this.orphanedMoveSourcesBySha(remoteMap); if (orphansBySha.size === 0) return; @@ -322,13 +340,18 @@ export class SyncStatusRefreshService { if (!current || current.file !== file) return true; let status: FileStatus['status'] = current.status; if (current.status !== 'moved') { - status = current.remoteSha === undefined - ? this.statuses.classify({ localExists: true, remoteExists: false }) - : this.statuses.classify({ + const remoteSha = current.remoteSha; + if (remoteSha === undefined) { + status = this.statuses.classify({ localExists: true, remoteExists: false }); + } else { + const localSha = await gitBlobSha(localContent); + status = this.statuses.classify({ localExists: true, remoteExists: true, - contentsEqual: await gitBlobSha(localContent) === current.remoteSha, + contentsEqual: localSha === remoteSha, + ...this.diffDirection(file.path, localSha, remoteSha), }); + } } this.statuses.set(file.path, { ...current, status, localContent }); return true; @@ -430,10 +453,13 @@ export class SyncStatusRefreshService { const binary = isBinaryPath(path); const symlinkMode = getEffectiveSymlinkHandling(this.dependencies.settings()); const localContent = await this.readLocalContentForSha(fileOrPath, isStringPath, binary, remoteEntry.symlink, symlinkMode); + const localSha = await gitBlobSha(localContent); + const remoteSha = remoteEntry.sha; const status = this.statuses.classify({ localExists: true, remoteExists: true, - contentsEqual: await gitBlobSha(localContent) === remoteEntry.sha, + contentsEqual: localSha === remoteSha, + ...(remoteSha !== undefined ? this.diffDirection(path, localSha, remoteSha) : {}), }); if (status === 'synced' && remoteEntry.sha) { await this.dependencies.syncManager().updateMetadata(path, remoteEntry.sha); @@ -457,13 +483,18 @@ export class SyncStatusRefreshService { this.dependencies.getNormalizedPath(path), this.dependencies.settings().branch, ); - const status = remote.sha - ? this.statuses.classify({ + let status: FileStatus['status']; + if (!remote.sha) { + status = this.statuses.classify({ localExists: true, remoteExists: false }); + } else { + const equal = contentsEqual(localContent, remote.content); + status = this.statuses.classify({ localExists: true, remoteExists: true, - contentsEqual: contentsEqual(localContent, remote.content), - }) - : this.statuses.classify({ localExists: true, remoteExists: false }); + contentsEqual: equal, + ...(equal ? {} : this.diffDirection(path, await gitBlobSha(localContent), remote.sha)), + }); + } if (status === 'synced' && remote.sha) { await this.dependencies.syncManager().updateMetadata(path, remote.sha); } diff --git a/src/ui/source-control/ChangeItem.ts b/src/ui/source-control/ChangeItem.ts index a2e6140..e7d5151 100644 --- a/src/ui/source-control/ChangeItem.ts +++ b/src/ui/source-control/ChangeItem.ts @@ -12,9 +12,10 @@ export interface ChangeItemCallbacks { onOpenDiff: (item: SourceControlItem) => void; /** * Pulls a single change into the local vault. Invoked for rows where - * {@link canDownload} is true — `remote-only` (add it locally) and - * `local-deleted` (restore it locally) — the Download button renders - * only for those kinds, so the callback never has to re-classify. + * {@link canDownload} is true — `remote-only` (add it locally), + * `remote-modified` (overwrite the local copy), and `local-deleted` + * (restore it locally) — the Download button renders only for those + * kinds, so the callback never has to re-classify. */ onDownload?: (item: SourceControlItem) => void; /** Looks up a cached diff stat for a row, if one has been computed. */ @@ -88,10 +89,11 @@ export function renderChangeItem( renderDiffStat(row, callbacks.getDiffStat?.(item.id)); // A change with something to pull from remote (remote-only: add it - // locally; local-deleted: restore it locally) carries a direct Download - // action so the user can pull it without first adding it to the Sync - // Queue. The button stops propagation so clicking it doesn't also - // trigger the row's open-diff/open-remote behavior. + // locally; remote-modified: overwrite the local copy; local-deleted: + // restore it locally) carries a direct Download action so the user can + // pull it without first adding it to the Sync Queue. The button stops + // propagation so clicking it doesn't also trigger the row's + // open-diff/open-remote behavior. if (canDownload(item.kind) && callbacks.onDownload) { renderDownloadAction(row, item, callbacks.onDownload); } diff --git a/src/ui/source-control/FilterMenu.ts b/src/ui/source-control/FilterMenu.ts index 545c9bc..6208dec 100644 --- a/src/ui/source-control/FilterMenu.ts +++ b/src/ui/source-control/FilterMenu.ts @@ -15,7 +15,7 @@ import type { SourceControlCounts } from '../../logic/source-control/SourceContr * bucket in the view (see `SourceControlView`) rather than via a domain * change. Surfaced as an opt-in overview; the default stays on Needs Sync * so a quiet workspace stays quiet. - * - **Remote / Conflict / Synced** — the matching domain filters. + * - **Incoming / Conflict / Synced** — the matching domain filters (chip id stays `remote`; the label reads "Incoming" — a file only on the remote, or changed only on the remote, is something coming *in*). * * "Local" (domain `changes`) is intentionally not a chip: Needs Sync already * covers local-side changes, and a standalone local-only view added a @@ -56,7 +56,7 @@ export interface FilterMenuOptions { /** * Renders the Source Control filter row: five chips — All / Needs Sync / - * Remote / Conflict / Synced. On mobile a single `` dropdown * replaces the chips (same chip ids, counts inline as "Label (N)"). * * Per-filter counts come straight from the ViewModel's single-source counts diff --git a/tests/logic/source-control/ChangeActionPolicy.test.ts b/tests/logic/source-control/ChangeActionPolicy.test.ts index d4ad57b..f958a84 100644 --- a/tests/logic/source-control/ChangeActionPolicy.test.ts +++ b/tests/logic/source-control/ChangeActionPolicy.test.ts @@ -52,10 +52,13 @@ describe('canDownload', () => { expect(canDownload('local-deleted')).toBe(true); }); + it('allows download for remote-modified (overwrite the local copy)', () => { + expect(canDownload('remote-modified')).toBe(true); + }); + it('disallows download for kinds with no separate remote-restore action', () => { expect(canDownload('local-only')).toBe(false); expect(canDownload('local-modified')).toBe(false); - expect(canDownload('remote-modified')).toBe(false); expect(canDownload('moved')).toBe(false); expect(canDownload('conflict')).toBe(false); expect(canDownload('synced')).toBe(false); diff --git a/tests/logic/source-control/FileStatusAdapter.test.ts b/tests/logic/source-control/FileStatusAdapter.test.ts index da396dd..5f38f32 100644 --- a/tests/logic/source-control/FileStatusAdapter.test.ts +++ b/tests/logic/source-control/FileStatusAdapter.test.ts @@ -8,6 +8,7 @@ describe('toSyncChanges', () => { const statuses: FileStatus[] = [ { path: 'synced.md', status: 'synced' }, { path: 'modified.md', status: 'modified' }, + { path: 'remote-modified.md', status: 'remote-modified' }, { path: 'unsynced.md', status: 'unsynced' }, { path: 'remote.md', status: 'remote-only' }, { path: 'gone.md', status: 'local-deleted' }, @@ -17,6 +18,7 @@ describe('toSyncChanges', () => { expect(toSyncChanges(statuses)).toEqual([ { id: toChangeId('synced.md'), path: 'synced.md', previousPath: undefined, kind: 'synced' }, { id: toChangeId('modified.md'), path: 'modified.md', previousPath: undefined, kind: 'local-modified' }, + { id: toChangeId('remote-modified.md'), path: 'remote-modified.md', previousPath: undefined, kind: 'remote-modified' }, { id: toChangeId('unsynced.md'), path: 'unsynced.md', previousPath: undefined, kind: 'local-only' }, { id: toChangeId('remote.md'), path: 'remote.md', previousPath: undefined, kind: 'remote-only' }, { id: toChangeId('gone.md'), path: 'gone.md', previousPath: undefined, kind: 'local-deleted' }, diff --git a/tests/logic/sync-status-service.test.ts b/tests/logic/sync-status-service.test.ts index cf5e027..c35c9e2 100644 --- a/tests/logic/sync-status-service.test.ts +++ b/tests/logic/sync-status-service.test.ts @@ -11,7 +11,10 @@ describe('SyncStatusService', () => { ['a previously-tracked file removed locally', { localExists: false, remoteExists: true, wasTracked: true }, 'local-deleted'], ['a never-tracked remote file with wasTracked false', { localExists: false, remoteExists: true, wasTracked: false }, 'remote-only'], ['matching local and remote content', { localExists: true, remoteExists: true, contentsEqual: true }, 'synced'], - ['different local and remote content', { localExists: true, remoteExists: true, contentsEqual: false }, 'modified'], + ['different content with no baseline on record', { localExists: true, remoteExists: true, contentsEqual: false }, 'modified'], + ['only the local side changed since baseline', { localExists: true, remoteExists: true, contentsEqual: false, localChanged: true, remoteChanged: false }, 'modified'], + ['only the remote side changed since baseline', { localExists: true, remoteExists: true, contentsEqual: false, localChanged: false, remoteChanged: true }, 'remote-modified'], + ['both sides changed since baseline', { localExists: true, remoteExists: true, contentsEqual: false, localChanged: true, remoteChanged: true }, 'modified'], ])('classifies %s as %s', (_description, facts, expected) => { expect(service.classify(facts)).toBe(expected); }); diff --git a/tests/logic/sync/SyncStatusRefreshService.test.ts b/tests/logic/sync/SyncStatusRefreshService.test.ts index 47d252d..a4dcd95 100644 --- a/tests/logic/sync/SyncStatusRefreshService.test.ts +++ b/tests/logic/sync/SyncStatusRefreshService.test.ts @@ -3,6 +3,7 @@ import { TFile } from 'obsidian'; import { SyncStatusRefreshService } from '../../../src/logic/sync/SyncStatusRefreshService'; import { SyncStatusService } from '../../../src/logic/sync-status-service'; import type { SyncStatusRefreshDependencies } from '../../../src/logic/sync/SyncStatusRefreshService'; +import { gitBlobSha } from '../../../src/utils/git-blob-sha'; vi.mock('obsidian'); @@ -347,6 +348,35 @@ describe('SyncStatusRefreshService local-change handlers', () => { expect(statuses.has('note.md')).toBe(false); }); + + it('classifies remote-modified, not modified, when the local content still matches the last-synced baseline but the remote sha has moved (regression: was silently routed to push)', async () => { + const statuses = new SyncStatusService(); + const file = makeFile('note.md'); + const baselineContent = 'baseline content'; + const baselineSha = await gitBlobSha(baselineContent); + const service = buildService(statuses, { + app: { + vault: { + read: vi.fn().mockResolvedValue(baselineContent), + readBinary: vi.fn(), + adapter: { stat: vi.fn().mockResolvedValue(null), read: vi.fn() }, + }, + } as never, + settings: () => ({ + syncMetadata: { 'note.md': { lastSyncedSha: baselineSha, lastSyncedAt: 1 } }, + vaultFolder: '', + rootPath: '', + }) as never, + }); + + // Row tracked at a remote sha that has since moved away from the + // baseline, while the local content read back is unchanged. + statuses.set({ file, path: 'note.md', status: 'synced', remoteSha: 'b'.repeat(40) }); + + await service.handleFileModified(file); + + expect(statuses.get('note.md')?.status).toBe('remote-modified'); + }); }); describe('identifyExtraFiles local-deleted classification', () => { diff --git a/tests/ui/source-control/ChangeTree.test.ts b/tests/ui/source-control/ChangeTree.test.ts index 0131f3b..d910f91 100644 --- a/tests/ui/source-control/ChangeTree.test.ts +++ b/tests/ui/source-control/ChangeTree.test.ts @@ -67,12 +67,15 @@ describe('renderChangeTree', () => { expect(callbacks.onDownload).toHaveBeenCalledWith(items[0]); }); - it('does not render a Download button on a remote-modified row', () => { + it('renders an inline Download button on a remote-modified row when onDownload is wired', () => { const items = [item({ id: toChangeId('c-1'), path: 'both.md', kind: 'remote-modified' })]; callbacks.onDownload = vi.fn(); renderChangeTree(container, items, new Set(), callbacks); - expect(container.querySelector('.scv-change-download')).toBeNull(); + const btn = container.querySelector('.scv-change-download') as HTMLButtonElement; + expect(btn).toBeTruthy(); + btn.click(); + expect(callbacks.onDownload).toHaveBeenCalledWith(items[0]); }); it('does not render an inline status subtitle (the kind label lives on the badge tooltip)', () => { diff --git a/tests/ui/source-control/FilterMenu.test.ts b/tests/ui/source-control/FilterMenu.test.ts index b2def9a..02339e4 100644 --- a/tests/ui/source-control/FilterMenu.test.ts +++ b/tests/ui/source-control/FilterMenu.test.ts @@ -19,7 +19,7 @@ describe('renderFilterMenu', () => { callbacks = { onFilterChange: vi.fn() }; }); - it('renders the five chips (All/Needs Sync/Remote/Conflict/Synced)', () => { + it('renders the five chips (All/Needs Sync/Incoming/Conflict/Synced)', () => { renderFilterMenu(container, { filter: 'all', showSynced: false }, zeroCounts, callbacks); const chips = Array.from(container.querySelectorAll('.scv-filter-option')).map(el => el.getAttribute('data-filter')); @@ -30,7 +30,7 @@ describe('renderFilterMenu', () => { renderFilterMenu(container, { filter: 'all', showSynced: false }, zeroCounts, callbacks); const labels = Array.from(container.querySelectorAll('.scv-filter-option .scv-filter-label')).map(el => el.textContent); - expect(labels).toEqual(['All', 'Needs Sync', 'Remote', 'Conflict', 'Synced']); + expect(labels).toEqual(['All', 'Needs Sync', 'Incoming', 'Conflict', 'Synced']); }); it('marks the current (filter, showSynced) chip as active', () => {