From d5bf76551c576970fb39c2cae0d36eb58d37f835 Mon Sep 17 00:00:00 2001 From: lukelzlz Date: Mon, 6 Jul 2026 19:22:06 +0800 Subject: [PATCH 1/5] fix(sync): preserve branch for remote content reads --- src/services/syncService.ts | 8 ++++-- src/views/SyncView.ts | 3 +- tests/services/syncService.test.ts | 41 +++++++++++++++++++++++++- tests/views/SyncView.test.ts | 46 ++++++++++++++++++++++++++++++ 4 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 tests/views/SyncView.test.ts diff --git a/src/services/syncService.ts b/src/services/syncService.ts index 846ec43..cc8965c 100644 --- a/src/services/syncService.ts +++ b/src/services/syncService.ts @@ -534,11 +534,12 @@ export class SyncService { owner: string, repo: string, remotePath: string, - localPath: string + localPath: string, + branch?: string ): Promise { try { console.debug(`[Sync] Pulling file: ${remotePath} -> ${localPath}`); - const content = await this.githubService.getFileContent(owner, repo, remotePath); + const content = await this.githubService.getFileContent(owner, repo, remotePath, branch); const isBin = isBinaryFile(localPath); if (isBin) { @@ -606,7 +607,8 @@ export class SyncService { owner, repo, remote.path, - absolutePath + absolutePath, + _branch ); // Store the relative path in the result for consistency result.path = change.path; diff --git a/src/views/SyncView.ts b/src/views/SyncView.ts index 5b56c6a..69bb515 100644 --- a/src/views/SyncView.ts +++ b/src/views/SyncView.ts @@ -371,7 +371,8 @@ export class SyncView extends ItemView { const remote = await this.plugin.githubService.getFileContent( this.plugin.settings.repo.owner, this.plugin.settings.repo.name, - path + path, + this.plugin.settings.repo.branch ); remoteContent = atob(remote.content); } catch { diff --git a/tests/services/syncService.test.ts b/tests/services/syncService.test.ts index 73d7551..f8a23da 100644 --- a/tests/services/syncService.test.ts +++ b/tests/services/syncService.test.ts @@ -1,4 +1,5 @@ -import { FileSyncState, LocalFileEntry, RemoteFileEntry, PersistedSyncState } from '../../src/services/syncService'; +import { App } from 'obsidian'; +import { SyncService, FileSyncState, LocalFileEntry, RemoteFileEntry, PersistedSyncState } from '../../src/services/syncService'; import { matchesIgnorePattern } from '../../src/utils/fileUtils'; /** @@ -294,6 +295,44 @@ describe('SyncService - Deletion Sync Logic', () => { }); }); +describe('SyncService - Branch-aware remote reads', () => { + it('passes the configured branch when pulling remote file contents', async () => { + const githubService = { + getFileContent: jest.fn().mockResolvedValue({ + path: 'notes/branch-only.md', + sha: 'remote-sha', + content: Buffer.from('branch content', 'utf8').toString('base64'), + encoding: 'base64', + size: 14, + }), + }; + + const app = new App(); + const service = new SyncService(app as never, githubService as never); + const changes: FileSyncState[] = [{ + path: 'notes/branch-only.md', + localHash: null, + remoteHash: 'remote-sha', + remoteSha: 'remote-sha', + status: 'added', + localModified: null, + remoteModified: null, + }]; + const remoteIndex = new Map([ + ['notes/branch-only.md', { path: 'notes/branch-only.md', sha: 'remote-sha' }], + ]); + + await service.pullChanges('octo', 'branch-repo', 'feature/sync-target', changes, remoteIndex); + + expect(githubService.getFileContent).toHaveBeenCalledWith( + 'octo', + 'branch-repo', + 'notes/branch-only.md', + 'feature/sync-target' + ); + }); +}); + describe('SyncService - Effective Ignore Patterns', () => { /** * Simulates getEffectiveIgnorePatterns logic for testing diff --git a/tests/views/SyncView.test.ts b/tests/views/SyncView.test.ts new file mode 100644 index 0000000..0507f71 --- /dev/null +++ b/tests/views/SyncView.test.ts @@ -0,0 +1,46 @@ +import { App } from 'obsidian'; +import { SyncView } from '../../src/views/SyncView'; + +describe('SyncView - Branch-aware remote reads', () => { + it('passes the configured branch when loading remote content for diff', async () => { + const app = new App(); + const githubService = { + getFileContent: jest.fn().mockResolvedValue({ + path: 'notes/branch-only.md', + sha: 'remote-sha', + content: Buffer.from('branch content', 'utf8').toString('base64'), + encoding: 'base64', + size: 14, + }), + }; + + const plugin = { + app, + settings: { + repo: { + owner: 'octo', + name: 'branch-repo', + branch: 'feature/sync-target', + }, + }, + githubService, + openDiffView: jest.fn().mockResolvedValue(null), + }; + + const view = Object.create(SyncView.prototype) as SyncView & { + app: App; + plugin: typeof plugin; + }; + view.app = app; + view.plugin = plugin; + + await view['openFileDiff']('notes/branch-only.md'); + + expect(githubService.getFileContent).toHaveBeenCalledWith( + 'octo', + 'branch-repo', + 'notes/branch-only.md', + 'feature/sync-target' + ); + }); +}); From 9e837258a16dbc52e85fa6689f53427f5bbc7f69 Mon Sep 17 00:00:00 2001 From: lukelzlz Date: Mon, 6 Jul 2026 20:02:11 +0800 Subject: [PATCH 2/5] fix(sync): mark sync as failed when file pulls error --- src/services/syncService.ts | 3 +++ tests/services/syncService.test.ts | 30 ++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/services/syncService.ts b/src/services/syncService.ts index cc8965c..710ffd5 100644 --- a/src/services/syncService.ts +++ b/src/services/syncService.ts @@ -863,6 +863,9 @@ export class SyncService { } result.filesProcessed = result.filesPulled + result.filesPushed + result.filesDeleted; + if (result.errors.length > 0) { + result.success = false; + } // Build new sync state const newLocalIndex = await this.buildLocalIndex(); diff --git a/tests/services/syncService.test.ts b/tests/services/syncService.test.ts index f8a23da..e6ee68d 100644 --- a/tests/services/syncService.test.ts +++ b/tests/services/syncService.test.ts @@ -333,6 +333,36 @@ describe('SyncService - Branch-aware remote reads', () => { }); }); +describe('SyncService - Error propagation', () => { + it('marks sync as failed when pull operations return file errors', async () => { + const githubService = { + getFileContent: jest.fn().mockRejectedValue(new Error('Not Found - https://docs.github.com/rest/repos/contents#get-repository-content')), + }; + + const app = new App(); + const service = new SyncService(app as never, githubService as never); + const remoteIndex = new Map([ + ['notes/missing.md', { path: 'notes/missing.md', sha: 'remote-sha' }], + ]); + + jest.spyOn(service, 'buildLocalIndex').mockResolvedValue(new Map()); + jest.spyOn(service, 'buildRemoteIndex').mockResolvedValue(remoteIndex); + + const { result } = await service.sync( + 'octo', + 'branch-repo', + 'feature/sync-target', + 'Sync test' + ); + + expect(result.success).toBe(false); + expect(result.filesProcessed).toBe(0); + expect(result.errors).toEqual([ + 'Not Found - https://docs.github.com/rest/repos/contents#get-repository-content', + ]); + }); +}); + describe('SyncService - Effective Ignore Patterns', () => { /** * Simulates getEffectiveIgnorePatterns logic for testing From 1b948a13a007e88618fd2146f60b641afd82b94c Mon Sep 17 00:00:00 2001 From: lukelzlz Date: Mon, 6 Jul 2026 20:09:49 +0800 Subject: [PATCH 3/5] fix(diff): resolve remote paths within repo subfolders --- src/views/SyncView.ts | 15 +++++++++++-- tests/views/SyncView.test.ts | 43 ++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/views/SyncView.ts b/src/views/SyncView.ts index 69bb515..8410b09 100644 --- a/src/views/SyncView.ts +++ b/src/views/SyncView.ts @@ -2,6 +2,7 @@ import { ItemView, WorkspaceLeaf, Notice, TFile } from 'obsidian'; import type GitHubOctokitPlugin from '../../main'; import { FileSyncState } from '../services/syncService'; import { LogEntry } from '../services/loggerService'; +import { normalizePath } from '../utils/fileUtils'; export const SYNC_VIEW_TYPE = 'github-octokit-sync-view'; @@ -31,6 +32,14 @@ export class SyncView extends ItemView { this.loadViewState(); } + private toRepoPath(path: string): string { + const subfolderPath = this.plugin.settings.subfolderPath; + if (!subfolderPath) { + return path; + } + return normalizePath(`${subfolderPath}/${path}`); + } + /** Restore persisted UI state from vault-specific localStorage */ private loadViewState(): void { const state = this.plugin.app.loadLocalStorage('github-octokit-sync-view') as string | null; @@ -348,7 +357,8 @@ export class SyncView extends ItemView { if (this.plugin.settings.repo) { const ghBtn = actions.createEl('button', { text: 'GitHub', cls: 'file-action' }); ghBtn.addEventListener('click', () => { - const url = `https://github.com/${this.plugin.settings.repo!.owner}/${this.plugin.settings.repo!.name}/blob/${this.plugin.settings.repo!.branch}/${file.path}`; + const repoPath = this.toRepoPath(file.path); + const url = `https://github.com/${this.plugin.settings.repo!.owner}/${this.plugin.settings.repo!.name}/blob/${this.plugin.settings.repo!.branch}/${repoPath}`; window.open(url, '_blank'); }); } @@ -368,10 +378,11 @@ export class SyncView extends ItemView { let remoteContent = ''; if (this.plugin.settings.repo) { try { + const repoPath = this.toRepoPath(path); const remote = await this.plugin.githubService.getFileContent( this.plugin.settings.repo.owner, this.plugin.settings.repo.name, - path, + repoPath, this.plugin.settings.repo.branch ); remoteContent = atob(remote.content); diff --git a/tests/views/SyncView.test.ts b/tests/views/SyncView.test.ts index 0507f71..d615a76 100644 --- a/tests/views/SyncView.test.ts +++ b/tests/views/SyncView.test.ts @@ -43,4 +43,47 @@ describe('SyncView - Branch-aware remote reads', () => { 'feature/sync-target' ); }); + + it('prefixes the configured subfolder when loading remote content for diff', async () => { + const app = new App(); + const githubService = { + getFileContent: jest.fn().mockResolvedValue({ + path: 'study-workspace/notes/branch-only.md', + sha: 'remote-sha', + content: Buffer.from('branch content', 'utf8').toString('base64'), + encoding: 'base64', + size: 14, + }), + }; + + const plugin = { + app, + settings: { + repo: { + owner: 'octo', + name: 'branch-repo', + branch: 'feature/sync-target', + }, + subfolderPath: 'study-workspace', + }, + githubService, + openDiffView: jest.fn().mockResolvedValue(null), + }; + + const view = Object.create(SyncView.prototype) as SyncView & { + app: App; + plugin: typeof plugin; + }; + view.app = app; + view.plugin = plugin; + + await view['openFileDiff']('notes/branch-only.md'); + + expect(githubService.getFileContent).toHaveBeenCalledWith( + 'octo', + 'branch-repo', + 'study-workspace/notes/branch-only.md', + 'feature/sync-target' + ); + }); }); From 662a13aa7a669174a024058d47b5f1a61be95104 Mon Sep 17 00:00:00 2001 From: Mark Rhoades-Brown Date: Sat, 29 Aug 2026 12:08:56 +0100 Subject: [PATCH 4/5] fix(diff): decode remote file content as UTF-8 Use decodeBase64 instead of atob so diffs handle multi-byte UTF-8 correctly. Rename the unused _branch parameter now that pullFile forwards it. --- src/services/syncService.ts | 4 ++-- src/views/SyncView.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/services/syncService.ts b/src/services/syncService.ts index 710ffd5..c89378e 100644 --- a/src/services/syncService.ts +++ b/src/services/syncService.ts @@ -585,7 +585,7 @@ export class SyncService { async pullChanges( owner: string, repo: string, - _branch: string, + branch: string, changes: FileSyncState[], remoteIndex: Map ): Promise { @@ -608,7 +608,7 @@ export class SyncService { repo, remote.path, absolutePath, - _branch + branch ); // Store the relative path in the result for consistency result.path = change.path; diff --git a/src/views/SyncView.ts b/src/views/SyncView.ts index 8410b09..429ac8b 100644 --- a/src/views/SyncView.ts +++ b/src/views/SyncView.ts @@ -2,7 +2,7 @@ import { ItemView, WorkspaceLeaf, Notice, TFile } from 'obsidian'; import type GitHubOctokitPlugin from '../../main'; import { FileSyncState } from '../services/syncService'; import { LogEntry } from '../services/loggerService'; -import { normalizePath } from '../utils/fileUtils'; +import { decodeBase64, normalizePath } from '../utils/fileUtils'; export const SYNC_VIEW_TYPE = 'github-octokit-sync-view'; @@ -385,7 +385,7 @@ export class SyncView extends ItemView { repoPath, this.plugin.settings.repo.branch ); - remoteContent = atob(remote.content); + remoteContent = decodeBase64(remote.content); } catch { // File doesn't exist on remote } From 36df16b9faaaf1d70094442821b3c8141993c59b Mon Sep 17 00:00:00 2001 From: Mark Rhoades-Brown Date: Sat, 29 Aug 2026 12:30:07 +0100 Subject: [PATCH 5/5] fix(diff): treat '/' as repo root in toRepoPath Match SyncService: an empty or '/' subfolderPath means the repository root, so Diff and GitHub actions must not prefix paths with '/'. --- src/views/SyncView.ts | 2 +- tests/views/SyncView.test.ts | 43 ++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/views/SyncView.ts b/src/views/SyncView.ts index 429ac8b..41e2c8c 100644 --- a/src/views/SyncView.ts +++ b/src/views/SyncView.ts @@ -34,7 +34,7 @@ export class SyncView extends ItemView { private toRepoPath(path: string): string { const subfolderPath = this.plugin.settings.subfolderPath; - if (!subfolderPath) { + if (!subfolderPath || subfolderPath === '/') { return path; } return normalizePath(`${subfolderPath}/${path}`); diff --git a/tests/views/SyncView.test.ts b/tests/views/SyncView.test.ts index d615a76..db4eef8 100644 --- a/tests/views/SyncView.test.ts +++ b/tests/views/SyncView.test.ts @@ -86,4 +86,47 @@ describe('SyncView - Branch-aware remote reads', () => { 'feature/sync-target' ); }); + + it('does not prefix a root subfolder path when loading remote content for diff', async () => { + const app = new App(); + const githubService = { + getFileContent: jest.fn().mockResolvedValue({ + path: 'notes/branch-only.md', + sha: 'remote-sha', + content: Buffer.from('branch content', 'utf8').toString('base64'), + encoding: 'base64', + size: 14, + }), + }; + + const plugin = { + app, + settings: { + repo: { + owner: 'octo', + name: 'branch-repo', + branch: 'feature/sync-target', + }, + subfolderPath: '/', + }, + githubService, + openDiffView: jest.fn().mockResolvedValue(null), + }; + + const view = Object.create(SyncView.prototype) as SyncView & { + app: App; + plugin: typeof plugin; + }; + view.app = app; + view.plugin = plugin; + + await view['openFileDiff']('notes/branch-only.md'); + + expect(githubService.getFileContent).toHaveBeenCalledWith( + 'octo', + 'branch-repo', + 'notes/branch-only.md', + 'feature/sync-target' + ); + }); });