From ce61588e7db9085dc6c8fdbda6b964734e33fa5a Mon Sep 17 00:00:00 2001 From: tianyao Date: Fri, 7 Aug 2026 08:38:31 +0000 Subject: [PATCH] fix(provider): preserve sync correctness --- src/logic/sync-manager.ts | 53 +++++++++++++++++++------ src/services/git-service-base.ts | 15 +------ src/services/git-service-interface.ts | 4 ++ src/services/gitea-service.ts | 5 +-- src/services/github-service.ts | 9 +++-- src/services/gitlab-service.ts | 24 +++++++---- tests/logic/sync-manager-batch.test.ts | 40 +++++++++++++++++++ tests/services/git-service-base.test.ts | 4 +- tests/services/gitea-service.test.ts | 7 +--- tests/services/github-service.test.ts | 21 ++++++---- tests/services/gitlab-service.test.ts | 30 ++++++++++++-- 11 files changed, 154 insertions(+), 58 deletions(-) diff --git a/src/logic/sync-manager.ts b/src/logic/sync-manager.ts index 5708064..7799a03 100644 --- a/src/logic/sync-manager.ts +++ b/src/logic/sync-manager.ts @@ -22,7 +22,7 @@ type PlanClassification = { kind: 'addition' | 'modification' | 'move' | 'unchan type ToPushEntry = { path: string; name: string; repoPath: string; content: string | ArrayBuffer; existingSha?: string; existingRevision?: string }; /** A renamed file classified as a safe move, queued for the grouped batch-commit call. */ -type ToMoveEntry = { path: string; name: string; repoPath: string; oldPath: string; oldRepoPath: string; content: string | ArrayBuffer }; +type ToMoveEntry = { path: string; name: string; repoPath: string; oldPath: string; oldRepoPath: string; content: string | ArrayBuffer; oldRevision?: string }; /** * Result of a batch push. `syncedPaths` lists every path that's now confirmed @@ -682,6 +682,7 @@ export class SyncManager { } const treeEntry = treeByFullPath.get(this.getFullPathForTree(repoPath)); + await this.migrateGitLabLegacyBaseline(path, repoPath, treeEntry); const outcome = await this.classifyAgainstTreeEntry(path, content, treeEntry, true); if (outcome === 'queued') return { kind: treeEntry ? 'modification' : 'addition' }; // classifyAgainstTreeEntry's dry-run path only ever resolves to @@ -714,7 +715,7 @@ export class SyncManager { if (!renamedFrom) return undefined; const scratch: ToMoveEntry[] = []; - const outcome = this.queueMove(path, name, renamedFrom, content, treeByFullPath, scratch); + const outcome = await this.queueMove(path, name, renamedFrom, content, treeByFullPath, scratch); return outcome === 'queued' ? { kind: 'move', movedFrom: renamedFrom } : { kind: 'conflict' }; } @@ -742,6 +743,7 @@ export class SyncManager { const localSha = await gitBlobSha(await this.getFileContent(fileOrPath)); if (localSha === entry.sha) return 'unchanged'; + await this.migrateGitLabLegacyBaseline(path, repoPath, entry); const lastSynced = this.settings.syncMetadata[path]; if (lastSynced && entry.sha !== lastSynced.lastSyncedSha) return 'conflict'; return 'modification'; @@ -891,15 +893,18 @@ export class SyncManager { const trackedOldPath = this.settings.syncMetadata[path]?.renamedFrom; const renamedFrom = trackedOldPath ?? (hasOrphans ? await this.detectRename(fileOrPath, content, treeByFullPath) : null); if (renamedFrom) { - return this.queueMove(path, name, renamedFrom, content, treeByFullPath, toMove); + return await this.queueMove(path, name, renamedFrom, content, treeByFullPath, toMove); } } - const treeEntry = treeByFullPath.get(this.getFullPathForTree(repoPath)); + let treeEntry = treeByFullPath.get(this.getFullPathForTree(repoPath)); + await this.migrateGitLabLegacyBaseline(path, repoPath, treeEntry); + const revision = await this.refreshGitLabBatchRevision(repoPath, treeEntry); + if (revision) treeEntry = { ...treeEntry!, sha: revision.sha }; const outcome = await this.classifyAgainstTreeEntry(path, content, treeEntry); if (outcome !== 'queued') return outcome; - toPush.push({ path, name, repoPath, content, existingSha: treeEntry?.sha }); + toPush.push({ path, name, repoPath, content, existingSha: treeEntry?.sha, existingRevision: revision?.revision }); return 'queued'; } @@ -912,28 +917,50 @@ export class SyncManager { * silently deleted. Both surface as 'conflict' so the batch can't quietly * clobber either side the way a plain content push already refuses to. */ - private queueMove( + private async queueMove( path: string, name: string, oldPath: string, content: string | ArrayBuffer, treeByFullPath: Map, toMove: ToMoveEntry[] - ): BatchOutcome | 'queued' { + ): Promise { const repoPath = this.getNormalizedPath(path); const oldRepoPath = this.getNormalizedPath(oldPath); if (treeByFullPath.get(this.getFullPathForTree(repoPath))) return 'conflict'; - const oldEntry = treeByFullPath.get(this.getFullPathForTree(oldRepoPath)); + let oldEntry = treeByFullPath.get(this.getFullPathForTree(oldRepoPath)); + await this.migrateGitLabLegacyBaseline(oldPath, oldRepoPath, oldEntry); + const oldRevision = await this.refreshGitLabBatchRevision(oldRepoPath, oldEntry); + if (oldRevision) oldEntry = { ...oldEntry!, sha: oldRevision.sha }; const metadata = this.settings.syncMetadata[path] ?? this.settings.syncMetadata[oldPath]; const safeToDeleteOld = !oldEntry?.sha || !metadata?.lastSyncedSha || oldEntry.sha === metadata.lastSyncedSha; if (oldEntry?.sha && !safeToDeleteOld) return 'conflict'; - toMove.push({ path, name, repoPath, oldPath, oldRepoPath, content }); + toMove.push({ path, name, repoPath, oldPath, oldRepoPath, content, oldRevision: oldRevision?.revision }); return 'queued'; } + /** GitLab tree rows expose blob identity but not the commit revision needed + * for optimistic locking. Read it during planning and compare the fresh blob + * again before accepting the action; the stored revision then protects the + * interval between planning and the atomic commit. */ + private async refreshGitLabBatchRevision(repoPath: string, entry: GitTreeEntry | undefined): Promise<{ sha: string; revision?: string } | undefined> { + if (this.settings.serviceType !== 'gitlab' || !entry?.sha) return undefined; + const remote = await this.gitService.getFile(repoPath, this.settings.branch); + return remote.sha ? { sha: remote.sha, revision: remote.revision } : undefined; + } + + /** Migrates a legacy GitLab last_commit_id baseline only when the current + * file endpoint proves it still describes this tree blob. */ + private async migrateGitLabLegacyBaseline(path: string, repoPath: string, entry: GitTreeEntry | undefined): Promise { + const metadata = this.settings.syncMetadata[path]; + if (this.settings.serviceType !== 'gitlab' || !metadata?.lastSyncedSha || !entry?.sha || entry.sha === metadata.lastSyncedSha) return; + const remote = await this.gitService.getFile(repoPath, this.settings.branch); + if (remote.sha === entry.sha && remote.revision === metadata.lastSyncedSha) await this.updateMetadata(path, remote.sha); + } + /** * Decides a non-symlink, non-renamed file's outcome purely from a * pre-fetched tree entry and a locally-computed git blob sha β€” no network @@ -1060,7 +1087,7 @@ export class SyncManager { try { const commitMessage = `Push ${chunk.length} file(s) from Obsidian`; const batchResults = await this.gitService.pushBatch!( - chunk.map(f => ({ path: f.repoPath, content: f.content, existedRemotely: !!f.existingSha })), + chunk.map(f => ({ path: f.repoPath, content: f.content, existedRemotely: !!f.existingSha, revision: f.existingRevision })), this.settings.branch, commitMessage ); @@ -1106,8 +1133,8 @@ export class SyncManager { const commitMessage = this.combinedChunkCommitMessage(pushEntries.length, moveEntries.length); const batchResults = await this.gitService.commitBatch!( - pushEntries.map(f => ({ path: f.repoPath, content: f.content, existedRemotely: !!f.existingSha })), - moveEntries.map(f => ({ oldPath: f.oldRepoPath, newPath: f.repoPath, content: f.content })), + pushEntries.map(f => ({ path: f.repoPath, content: f.content, existedRemotely: !!f.existingSha, revision: f.existingRevision })), + moveEntries.map(f => ({ oldPath: f.oldRepoPath, newPath: f.repoPath, content: f.content, oldRevision: f.oldRevision })), this.settings.branch, commitMessage ); @@ -1249,6 +1276,8 @@ export class SyncManager { return 'unchanged'; } + await this.migrateGitLabLegacyBaseline(path, this.getNormalizedPath(path), entry); + // Same conflict check as the content path below: local differs and the // remote has moved since we last synced, so pulling would discard one of // the two changes. diff --git a/src/services/git-service-base.ts b/src/services/git-service-base.ts index f922529..1442351 100644 --- a/src/services/git-service-base.ts +++ b/src/services/git-service-base.ts @@ -1,6 +1,7 @@ import { requestUrl, RequestUrlResponse } from 'obsidian'; import { logger } from '../utils/logger'; import { GitTreeEntry } from './git-service-interface'; +import { isBinaryPath } from '../utils/path'; export interface GitFile { content: string | ArrayBuffer; @@ -195,18 +196,6 @@ export abstract class BaseGitService { return cleanRoot + path; } - protected isBinary(path: string): boolean { - const ext = path.split('.').pop()?.toLowerCase(); - if (!ext) return false; - const BINARY_EXTENSIONS = new Set([ - 'png', 'jpg', 'jpeg', 'gif', 'bmp', 'ico', 'pdf', 'zip', 'gz', '7z', 'rar', - 'mp3', 'mp4', 'wav', 'ogg', 'webm', 'mov', 'avi', 'wmv', 'webp', - 'doc', 'docx', 'xls', 'xlsx', 'ppt', 'pptx', 'epub', 'exe', 'dll', 'so', - 'ttf', 'woff', 'woff2', 'eot', 'wasm', 'dmg', 'iso' - ]); - return BINARY_EXTENSIONS.has(ext); - } - protected encodeContent(content: string | ArrayBuffer): string { if (typeof content === 'string') { const bytes = new TextEncoder().encode(content); @@ -235,7 +224,7 @@ export abstract class BaseGitService { bytes[i] = cp !== undefined ? cp : 0; } - return this.isBinary(path) ? bytes.buffer : new TextDecoder().decode(bytes); + return isBinaryPath(path) ? bytes.buffer : new TextDecoder().decode(bytes); } /** diff --git a/src/services/git-service-interface.ts b/src/services/git-service-interface.ts index 64c5192..bd36125 100644 --- a/src/services/git-service-interface.ts +++ b/src/services/git-service-interface.ts @@ -36,6 +36,8 @@ export interface BatchPushItem { * action 'create' vs 'update'); GitHub/Gitea's tree-based commit ignores it. */ existedRemotely?: boolean; + /** Revision read during batch planning, used by GitLab's optimistic lock. */ + revision?: string; } /** Result for one file after a batch push completes. */ @@ -52,6 +54,8 @@ export interface BatchMoveItem { /** Path relative to rootPath, where the file now lives. */ newPath: string; content: string | ArrayBuffer; + /** Revision of oldPath read during batch planning, used by GitLab's optimistic lock. */ + oldRevision?: string; } export interface GitServiceInterface { diff --git a/src/services/gitea-service.ts b/src/services/gitea-service.ts index b25f1ea..aaa5b55 100644 --- a/src/services/gitea-service.ts +++ b/src/services/gitea-service.ts @@ -1,6 +1,5 @@ import { GitServiceInterface, GitTreeEntry, BatchPushItem, BatchPushResult, BatchMoveItem } from './git-service-interface'; import { BaseGitService, ConnectionTestResult, GitFile, GitHubContentResponse, GitHubTreeResponse, GIT_SYMLINK_MODE, BLOB_CREATE_CONCURRENCY } from './git-service-base'; -import { logger } from '../utils/logger'; export class GiteaService extends BaseGitService implements GitServiceInterface { private baseUrl: string = ''; @@ -156,9 +155,7 @@ export class GiteaService extends BaseGitService implements GitServiceInterface const treeResponse = await this.safeRequest(treeUrl, 'GET'); const treeData = this.parseJson(treeResponse); - if (treeData.truncated) { - logger.warn('Gitea tree result is truncated. Some files might not be shown.'); - } + if (treeData.truncated) throw new Error(`Gitea tree for branch "${branch}" is truncated; sync stopped to avoid treating an incomplete remote tree as a snapshot.`); const entries = treeData.tree .filter(item => item.type === 'blob') diff --git a/src/services/github-service.ts b/src/services/github-service.ts index 3e5cbde..8bec584 100644 --- a/src/services/github-service.ts +++ b/src/services/github-service.ts @@ -1,6 +1,5 @@ import { GitServiceInterface, GitTreeEntry, BatchPushItem, BatchPushResult, BatchMoveItem } from './git-service-interface'; import { BaseGitService, ConnectionTestResult, GitFile, GitHubContentResponse, GitHubTreeResponse, GIT_SYMLINK_MODE, BLOB_CREATE_CONCURRENCY } from './git-service-base'; -import { logger } from '../utils/logger'; import { PushTimingCollector, PushTimingHandler, PushTimingRecord } from './push-timing'; /** @@ -104,6 +103,10 @@ export class GitHubService extends BaseGitService implements GitServiceInterface } async pushFile(path: string, content: string | ArrayBuffer, branch: string, message: string, _existingSha?: string, _revision?: string): Promise<{ path: string, sha?: string }> { + const entry = (await this.listFilesDetailed(branch, false)).find(item => item.path === this.getFullPath(path)); + if (entry?.symlink) { + throw new Error(`Cannot overwrite symlink "${path}" with a regular file.`); + } const [result] = await this.pushBatch([{ path, content }], branch, message); return result ?? { path }; } @@ -309,9 +312,7 @@ export class GitHubService extends BaseGitService implements GitServiceInterface throw this.branchNotFoundError(e, branch); } - if (data.truncated) { - logger.warn('GitHub tree result is truncated. Some files might not be shown.'); - } + if (data.truncated) throw new Error(`GitHub tree for branch "${branch}" is truncated; sync stopped to avoid treating an incomplete remote tree as a snapshot.`); const entries = data.tree .filter(item => item.type === 'blob') diff --git a/src/services/gitlab-service.ts b/src/services/gitlab-service.ts index 430d013..514370d 100644 --- a/src/services/gitlab-service.ts +++ b/src/services/gitlab-service.ts @@ -1,5 +1,6 @@ import { GitServiceInterface, GitTreeEntry, BatchPushItem, BatchPushResult, BatchMoveItem } from './git-service-interface'; import { BaseGitService, ConnectionTestResult, GitFile, GitLabFileResponse, GitLabTreeItem, GIT_SYMLINK_MODE } from './git-service-base'; +import { isBinaryPath } from '../utils/path'; export class GitLabService extends BaseGitService implements GitServiceInterface { private baseUrl: string = 'https://gitlab.com'; @@ -61,12 +62,13 @@ export class GitLabService extends BaseGitService implements GitServiceInterface const encodedProjectId = encodeURIComponent(this.projectId); const url = `${this.baseUrl}/api/v4/projects/${encodedProjectId}/repository/commits`; - const actions = items.map(item => ({ + const actions = await Promise.all(items.map(async item => ({ action: item.existedRemotely ? 'update' : 'create', file_path: this.getFullPath(item.path), content: this.encodeContent(item.content), encoding: 'base64', - })); + ...(item.existedRemotely && item.revision ? { last_commit_id: item.revision } : {}), + }))); await this.safeRequest(url, 'POST', { branch, commit_message: message, actions }); @@ -89,19 +91,21 @@ export class GitLabService extends BaseGitService implements GitServiceInterface const url = `${this.baseUrl}/api/v4/projects/${encodedProjectId}/repository/commits`; const actions = [ - ...additions.map(item => ({ + ...await Promise.all(additions.map(async item => ({ action: item.existedRemotely ? 'update' : 'create', file_path: this.getFullPath(item.path), content: this.encodeContent(item.content), encoding: 'base64', - })), - ...moves.map(item => ({ + ...(item.existedRemotely && item.revision ? { last_commit_id: item.revision } : {}), + }))), + ...await Promise.all(moves.map(async item => ({ action: 'move', file_path: this.getFullPath(item.newPath), previous_path: this.getFullPath(item.oldPath), content: this.encodeContent(item.content), encoding: 'base64', - })), + ...(item.oldRevision ? { last_commit_id: item.oldRevision } : {}), + }))), ]; await this.safeRequest(url, 'POST', { branch, commit_message: message, actions }); @@ -160,7 +164,7 @@ export class GitLabService extends BaseGitService implements GitServiceInterface const encodedProjectId = encodeURIComponent(this.projectId); const url = `${this.baseUrl}/api/v4/projects/${encodedProjectId}/repository/blobs/${sha}/raw`; const response = await this.safeRequest(url, 'GET'); - const content = this.isBinary(path) ? response.arrayBuffer : response.text; + const content = isBinaryPath(path) ? response.arrayBuffer : response.text; return { content, sha }; } @@ -179,7 +183,11 @@ export class GitLabService extends BaseGitService implements GitServiceInterface const encodedProjectId = encodeURIComponent(this.projectId); const url = `${this.baseUrl}/api/v4/projects/${encodedProjectId}/repository/commits`; - const actions = paths.map(path => ({ action: 'delete', file_path: this.getFullPath(path) })); + const actions = await Promise.all(paths.map(async path => ({ + action: 'delete', + file_path: this.getFullPath(path), + last_commit_id: (await this.getFile(path, branch)).revision, + }))); await this.safeRequest(url, 'POST', { branch, commit_message: message, actions }); } diff --git a/tests/logic/sync-manager-batch.test.ts b/tests/logic/sync-manager-batch.test.ts index 5eafa5d..b34edc5 100644 --- a/tests/logic/sync-manager-batch.test.ts +++ b/tests/logic/sync-manager-batch.test.ts @@ -148,6 +148,27 @@ describe('SyncManager Batch Operations', () => { ]); }); + it('reads and forwards GitLab revision for an existing batch update', async () => { + const path = 'locked.md'; + const adapter = mockApp.vault.adapter as Mocked; + mockSettings.serviceType = 'gitlab'; + mockSettings.syncMetadata = { + [path]: { lastSyncedSha: 'remote-blob', lastSyncedAt: 0, lastKnownPath: path } + }; + vi.mocked(adapter.exists).mockResolvedValue(true); + vi.mocked(adapter.read).mockResolvedValue('local edit'); + vi.mocked(mockGitService.listFilesDetailed).mockResolvedValue([{ path, symlink: false, sha: 'remote-blob' }]); + vi.mocked(mockGitService.getFile).mockResolvedValue({ content: 'remote original', sha: 'remote-blob', revision: 'remote-commit' }); + mockGitService.pushBatch = vi.fn().mockResolvedValue([{ path, sha: 'new-blob' }]); + + const results = await manager.pushAllFiles([path]); + + expect(results.success).toBe(1); + expect(mockGitService.pushBatch).toHaveBeenCalledWith([ + { path, content: 'local edit', existedRemotely: true, revision: 'remote-commit' }, + ], 'main', expect.any(String)); + }); + it('reports syncedPaths via the sequential fallback when the provider has no pushBatch', async () => { const files = ['a.md', 'b.md']; const adapter = mockApp.vault.adapter as Mocked; @@ -319,6 +340,25 @@ describe('SyncManager Batch Operations', () => { expect(adapter.write).not.toHaveBeenCalled(); }); + it('migrates a legacy GitLab last_commit_id baseline instead of creating a false pull conflict', async () => { + const path = 'legacy.md'; + const adapter = mockApp.vault.adapter as Mocked; + mockSettings.serviceType = 'gitlab'; + mockSettings.syncMetadata = { + [path]: { lastSyncedSha: 'legacy-last-commit', lastSyncedAt: 0, lastKnownPath: path } + }; + vi.mocked(adapter.exists).mockResolvedValue(true); + vi.mocked(adapter.read).mockResolvedValue('local old copy'); + vi.mocked(mockGitService.listFilesDetailed).mockResolvedValue([{ path, symlink: false, sha: 'remote-blob' }]); + vi.mocked(mockGitService.getFile).mockResolvedValue({ content: 'remote current copy', sha: 'remote-blob', revision: 'legacy-last-commit' }); + + const results = await manager.pullAllFiles([path]); + + expect(results.conflicts).toBe(0); + expect(results.success).toBe(1); + expect(mockSettings.syncMetadata[path]?.lastSyncedSha).toBe('remote-blob'); + }); + it('still downloads when the local file differs and the remote has not moved', async () => { const path = 'stale.md'; mockSettings.syncMetadata = { diff --git a/tests/services/git-service-base.test.ts b/tests/services/git-service-base.test.ts index 58c0b96..597a55c 100644 --- a/tests/services/git-service-base.test.ts +++ b/tests/services/git-service-base.test.ts @@ -177,6 +177,8 @@ describe('BaseGitService', () => { it('should correctly encode and decode UTF-8 content', async () => { const original = 'Hello, δΈ–η•Œ! 🌍'; vi.mocked(requestUrl) + // Tree mode check prevents a regular-file push from replacing a symlink. + .mockResolvedValueOnce({ status: 200, json: { tree: [{ path: 'test.md', type: 'blob', mode: '100644' }] } } as unknown as RequestUrlResponse) // Branch head for expectedHeadOid, read over GraphQL. .mockResolvedValueOnce({ status: 200, json: { data: { repository: { ref: { target: { oid: 'commit1' } } } } } } as unknown as RequestUrlResponse) .mockResolvedValueOnce({ status: 200, json: { data: { createCommitOnBranch: { commit: { oid: 'commit2' } } } } } as unknown as RequestUrlResponse); @@ -185,7 +187,7 @@ describe('BaseGitService', () => { await service.pushFile('test.md', original, 'main', 'test'); const calls = vi.mocked(requestUrl).mock.calls; - const body = JSON.parse((calls[1]?.[0] as { body: string }).body) as { variables: { input: { fileChanges: { additions: Array<{ contents: string }> } } } }; + const body = JSON.parse((calls[2]?.[0] as { body: string }).body) as { variables: { input: { fileChanges: { additions: Array<{ contents: string }> } } } }; const decoded = atob(body.variables.input.fileChanges.additions[0]?.contents.replace(/\s/g, '') ?? ''); const bytes = new Uint8Array(decoded.length); for (let i = 0; i < decoded.length; i++) { diff --git a/tests/services/gitea-service.test.ts b/tests/services/gitea-service.test.ts index 1008670..c2d7536 100644 --- a/tests/services/gitea-service.test.ts +++ b/tests/services/gitea-service.test.ts @@ -206,12 +206,9 @@ describe('GiteaService', () => { expect(await service.listFiles('main', false)).toEqual(['vault/file1.md', 'other/file2.md']); }); - it('should log warning and return files when result is truncated', async () => { + it('fails closed when the tree result is truncated', async () => { mockListFiles([{ path: 'file1.md', type: 'blob' }], true); - const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); - const result = await service.listFiles('main'); - expect(result).toEqual(['file1.md']); - warnSpy.mockRestore(); + await expect(service.listFiles('main')).rejects.toThrow(/truncated; sync stopped/); }); it('should throw a message naming the branch when the branch is not found', async () => { diff --git a/tests/services/github-service.test.ts b/tests/services/github-service.test.ts index f03615d..63020d6 100644 --- a/tests/services/github-service.test.ts +++ b/tests/services/github-service.test.ts @@ -343,17 +343,25 @@ describe('GitHubService', () => { describe('pushFile', () => { it('commits one regular file through GraphQL instead of the Contents API', async () => { vi.mocked(requestUrl) + .mockResolvedValueOnce({ status: 200, json: { tree: [{ path: 'note.md', type: 'blob', mode: '100644' }] } } as unknown as RequestUrlResponse) .mockResolvedValueOnce(headOidResponse('commit1')) .mockResolvedValueOnce({ status: 200, json: { data: { createCommitOnBranch: { commit: { oid: 'commit2' } } } } } as unknown as RequestUrlResponse); await expect(service.pushFile('note.md', 'new content', 'main', 'update', 'old-sha')).resolves.toEqual({ path: 'note.md' }); const calls = vi.mocked(requestUrl).mock.calls.map(call => call[0] as RequestUrlParam); - expect(calls).toHaveLength(2); - expect(calls[1]?.url).toBe('https://api.github.com/graphql'); - const body = JSON.parse(calls[1]?.body as string) as { variables: { input: { fileChanges: { additions: Array<{ path: string; contents: string }> } } } }; + expect(calls).toHaveLength(3); + expect(calls[2]?.url).toBe('https://api.github.com/graphql'); + const body = JSON.parse(calls[2]?.body as string) as { variables: { input: { fileChanges: { additions: Array<{ path: string; contents: string }> } } } }; expect(body.variables.input.fileChanges.additions).toEqual([{ path: 'note.md', contents: btoa('new content') }]); }); + + it('refuses to turn an in-repository symlink into a regular file', async () => { + mockRequest({ status: 200, json: { tree: [{ path: 'link.md', type: 'blob', mode: '120000' }] } }); + + await expect(service.pushFile('link.md', 'replacement', 'main', 'update')).rejects.toThrow(/Cannot overwrite symlink/); + expect(requestUrl).toHaveBeenCalledTimes(1); + }); }); describe('listFiles', () => { @@ -397,15 +405,12 @@ describe('GitHubService', () => { expect(await service.listFiles('main')).toEqual(['src/content/index.md']); }); - it('should return files and log warning when result is truncated', async () => { + it('fails closed when the tree result is truncated', async () => { mockRequest({ status: 200, json: { truncated: true, tree: [ { path: 'file1.md', type: 'blob' }, { path: 'file2.md', type: 'blob' }, ] } }); - const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); - const result = await service.listFiles('main'); - expect(result).toEqual(['file1.md', 'file2.md']); - warnSpy.mockRestore(); + await expect(service.listFiles('main')).rejects.toThrow(/truncated; sync stopped/); }); it('should throw a message naming the branch when the branch is not found', async () => { diff --git a/tests/services/gitlab-service.test.ts b/tests/services/gitlab-service.test.ts index 045d41e..0923d9e 100644 --- a/tests/services/gitlab-service.test.ts +++ b/tests/services/gitlab-service.test.ts @@ -251,7 +251,10 @@ describe('GitLabService', () => { }); it('posts a Commits API actions array with action: delete, no content/encoding', async () => { - mockRequest({ status: 201, json: { id: 'commit-sha' } }); + vi.mocked(requestUrl) + .mockResolvedValueOnce({ status: 200, json: { content: btoa('a'), blob_id: 'a-blob', last_commit_id: 'revision-a' } } as unknown as RequestUrlResponse) + .mockResolvedValueOnce({ status: 200, json: { content: btoa('b'), blob_id: 'b-blob', last_commit_id: 'revision-b' } } as unknown as RequestUrlResponse) + .mockResolvedValueOnce({ status: 201, json: { id: 'commit-sha' } } as unknown as RequestUrlResponse); await service.deleteBatch(['a.md', 'b.md'], 'main', 'Delete 2 file(s) from Obsidian'); @@ -262,8 +265,8 @@ describe('GitLabService', () => { expect(body.branch).toBe('main'); expect(body.commit_message).toBe('Delete 2 file(s) from Obsidian'); expect(body.actions).toEqual([ - { action: 'delete', file_path: 'a.md' }, - { action: 'delete', file_path: 'b.md' }, + { action: 'delete', file_path: 'a.md', last_commit_id: 'revision-a' }, + { action: 'delete', file_path: 'b.md', last_commit_id: 'revision-b' }, ]); }); }); @@ -275,6 +278,27 @@ describe('GitLabService', () => { expect(requestUrl).not.toHaveBeenCalled(); }); + it('sends last_commit_id for existing updates and moves', async () => { + vi.mocked(requestUrl) + .mockResolvedValueOnce({ status: 201, json: { id: 'commit-sha' } } as unknown as RequestUrlResponse) + .mockResolvedValueOnce({ status: 200, json: [ + { path: 'existing.md', type: 'blob', id: 'updated-blob' }, + { path: 'new.md', type: 'blob', id: 'moved-blob' }, + ] } as unknown as RequestUrlResponse); + + await service.commitBatch( + [{ path: 'existing.md', content: 'new', existedRemotely: true, revision: 'update-revision' }], + [{ oldPath: 'old.md', newPath: 'new.md', content: 'moved', oldRevision: 'move-revision' }], + 'main', 'locked batch' + ); + + const body = JSON.parse((vi.mocked(requestUrl).mock.calls[0]?.[0] as { body: string }).body) as { actions: Array<{ action: string; last_commit_id?: string }> }; + expect(body.actions).toEqual([ + expect.objectContaining({ action: 'update', last_commit_id: 'update-revision' }), + expect.objectContaining({ action: 'move', last_commit_id: 'move-revision' }), + ]); + }); + it('uses the Commits API native action: move for renames, alongside create/update for plain additions', async () => { vi.mocked(requestUrl) .mockResolvedValueOnce({ status: 201, json: { id: 'commit-sha' } } as unknown as RequestUrlResponse) // POST commits