fix(gitlab): fix sha/revision semantics for optimistic locking

- GitFile.sha now represents blob identity (blob_id), not commit revision
- Add GitFile.revision for provider-specific write control (last_commit_id)
- Separate existingSha (blob detection) from existingRevision (lock token)
- GitLab pushFile now uses revision for optimistic locking, not blob SHA
- Update SyncManager conflict detection to support lazy migration
  - Metadata from older single-pull (commit revision) or batch (blob SHA)
    can coexist without false conflicts
  - isSameBaseline() checks if lastSyncedSha matches remote.sha or remote.revision
- Update all tests to match new pushFile signature and getFile semantics

Fixes issue #101 where GitLab single-file and batch flows used inconsistent
SHA semantics, causing conflicts and incorrect behavior on concurrent edits.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

Author: ClaudiaFang

src/logic/sync-manager.ts

@@ -18,7 +18,7 @@ type BatchOutcome = 'done' | 'unchanged' | 'conflict';
 type PlanClassification = { kind: 'addition' | 'modification' | 'move' | 'unchanged' | 'conflict' | 'skip'; movedFrom?: string };
 
 /** A file classified as needing a push, queued for the grouped batch-commit call. */
-type ToPushEntry = { path: string; name: string; repoPath: string; content: string | ArrayBuffer; existingSha?: string };
+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 };
@@ -195,15 +195,15 @@ export class SyncManager {
 
             const lastSynced = this.settings.syncMetadata[path];
 
-            if (remote.sha && lastSynced && remote.sha !== lastSynced.lastSyncedSha) {
+            if (remote.sha && lastSynced && !this.isSameBaseline(lastSynced.lastSyncedSha, remote)) {
                 this.openPushConflictModal(fileOrPath, { path, name }, content, remote);
                 return undefined;
             }
 
             const confirmed = await this.confirmPlan(this.singleEntryPlan(remote.sha ? 'modification' : 'addition', path, name), 'push');
             if (!confirmed) return undefined;
 
-            const sha = await this.performPush({ path, name }, content, remote.sha);
+            const sha = await this.performPush({ path, name }, content, remote.sha, remote.revision);
             return { sha };
         } catch (e) {
             this.handleError(`Failed to push ${name} to ${this.serviceName}`, e);
@@ -259,7 +259,7 @@ export class SyncManager {
                 try {
                     const fileRep = typeof fileOrPath === 'string' ? file : fileOrPath;
                     if (choice === 'local') {
-                        await this.performPush(file, content, remote.sha);
+                        await this.performPush(file, content, remote.sha, remote.revision);
                     } else {
                         await this.performPull(fileRep, remote.content, remote.sha, false, this.symlinkPullTarget(remote));
                     }
@@ -367,7 +367,7 @@ export class SyncManager {
             }
 
             const remoteAtOldPath = await this.gitService.getFile(oldRepoPath, this.settings.branch);
-            const safeToDeleteOld = !remoteAtOldPath.sha || !metadata?.lastSyncedSha || remoteAtOldPath.sha === metadata.lastSyncedSha;
+            const safeToDeleteOld = !remoteAtOldPath.sha || !metadata?.lastSyncedSha || this.isSameBaseline(metadata.lastSyncedSha, remoteAtOldPath);
 
             const newSha = await this.commitMove(repoPath, oldRepoPath, content, safeToDeleteOld && !!remoteAtOldPath.sha);
 
@@ -405,14 +405,15 @@ export class SyncManager {
         return newSha;
     }
 
-    private async performPush(file: {path: string, name: string}, content: string | ArrayBuffer, existingSha?: string, silent = false): Promise<string | undefined> {
+    private async performPush(file: {path: string, name: string}, content: string | ArrayBuffer, existingSha?: string, existingRevision?: string, silent = false): Promise<string | undefined> {
         const repoPath = this.getNormalizedPath(file.path);
         const result = await this.gitService.pushFile(
             repoPath,
             content,
             this.settings.branch,
             `Update ${file.name} from Obsidian`,
-            existingSha
+            existingSha,
+            existingRevision
         );
 
         // Update metadata
@@ -474,13 +475,13 @@ export class SyncManager {
             }
 
             // Conflict detection for pull (only if local exists)
-            if (exists && remote.sha && lastSynced && remote.sha !== lastSynced.lastSyncedSha) {
+            if (exists && remote.sha && lastSynced && !this.isSameBaseline(lastSynced.lastSyncedSha, remote)) {
                 new SyncConflictModal(this.app, name, (localContent as string) || '', remote.content as string, (choice) => {
                     void (async () => {
                         try {
                             const fileRep = typeof fileOrPath === 'string' ? { path, name } : fileOrPath;
                             if (choice === 'local') {
-                                await this.performPush(fileRep, localContent || '', remote.sha);
+                                await this.performPush(fileRep, localContent || '', remote.sha, remote.revision);
                             } else {
                                 await this.performPull(fileRep, remote.content, remote.sha, false, this.symlinkPullTarget(remote));
                             }
@@ -506,6 +507,11 @@ export class SyncManager {
         return contentsEqual(a, b);
     }
 
+    /** Check if remote baseline matches metadata, supporting lazy migration of old metadata. */
+    private isSameBaseline(lastSyncedSha: string, remoteFile: GitFile): boolean {
+        return lastSyncedSha === remoteFile.sha || lastSyncedSha === remoteFile.revision;
+    }
+
     private isBinary(path: string): boolean {
         return isBinaryPath(path);
     }
@@ -1034,7 +1040,7 @@ export class SyncManager {
     private async pushSequentialFallback(toPush: ToPushEntry[], results: PushResults): Promise<void> {
         for (const f of toPush) {
             try {
-                const sha = await this.performPush({ path: f.path, name: f.name }, f.content, f.existingSha, true);
+                const sha = await this.performPush({ path: f.path, name: f.name }, f.content, f.existingSha, f.existingRevision, true);
                 results.success++;
                 results.syncedPaths.push({ path: f.path, sha });
             } catch (e) {
@@ -1220,7 +1226,7 @@ export class SyncManager {
 
             // Same conflict check as the single-file flow (see processSingleBatchPush).
             const lastSynced = this.settings.syncMetadata[path];
-            if (lastSynced && remote.sha !== lastSynced.lastSyncedSha) {
+            if (lastSynced && !this.isSameBaseline(lastSynced.lastSyncedSha, remote)) {
                 return 'conflict';
             }
         }

src/services/git-service-base.ts

@@ -5,6 +5,7 @@ import { GitTreeEntry } from './git-service-interface';
 export interface GitFile {
     content: string | ArrayBuffer;
     sha: string;
+    revision?: string;
     isSymlink?: boolean;
     symlinkTarget?: string;
 }
@@ -376,7 +377,7 @@ export abstract class BaseGitService {
     }
 
     abstract getFile(path: string, branch: string): Promise<GitFile>;
-    abstract pushFile(path: string, content: string | ArrayBuffer, branch: string, message: string, sha?: string): Promise<{ path: string, sha?: string }>;
+    abstract pushFile(path: string, content: string | ArrayBuffer, branch: string, message: string, sha?: string, revision?: string): Promise<{ path: string, sha?: string }>;
     abstract listFilesDetailed(branch: string, useFilter?: boolean): Promise<GitTreeEntry[]>;
     abstract deleteFile(path: string, branch: string, message: string): Promise<void>;
     abstract testConnection(branch: string): Promise<ConnectionTestResult>;

src/services/git-service-interface.ts

@@ -2,7 +2,10 @@ import { ConnectionTestResult } from './git-service-base';
 
 export interface GitFile {
     content: string | ArrayBuffer;
+    /** The blob's git SHA — permanent blob identity, used to compare content across syncs. */
     sha: string;
+    /** Provider-specific revision for write/concurrency control (e.g., GitLab's last_commit_id). */
+    revision?: string;
     /** True when the remote blob is a symbolic link (mode 120000). */
     isSymlink?: boolean;
     /** The link target path, when isSymlink is true and it could be determined. */
@@ -54,7 +57,7 @@ export interface BatchMoveItem {
 export interface GitServiceInterface {
     updateConfig(...args: unknown[]): void;
     getFile(path: string, branch: string): Promise<GitFile>;
-    pushFile(path: string, content: string | ArrayBuffer, branch: string, commitMessage: string, existingSha?: string): Promise<{ path: string, sha?: string }>;
+    pushFile(path: string, content: string | ArrayBuffer, branch: string, commitMessage: string, existingSha?: string, existingRevision?: string): Promise<{ path: string, sha?: string }>;
     /** Returns the branch's current commit SHA when the provider can expose it cheaply. */
     getBranchHead?(branch: string): Promise<string>;
     /** Checks the repository is reachable and the given branch exists. */

src/services/gitea-service.ts

@@ -59,7 +59,7 @@ export class GiteaService extends BaseGitService implements GitServiceInterface
         }
     }
 
-    async pushFile(path: string, content: string | ArrayBuffer, branch: string, message: string, sha?: string): Promise<{ path: string, sha?: string }> {
+    async pushFile(path: string, content: string | ArrayBuffer, branch: string, message: string, sha?: string, _revision?: string): Promise<{ path: string, sha?: string }> {
         const url = this.getApiUrl(path);
         const body: { message: string; content: string; branch: string; sha?: string } = {
             message,

src/services/github-service.ts

@@ -103,7 +103,7 @@ export class GitHubService extends BaseGitService implements GitServiceInterface
         }
     }
 
-    async pushFile(path: string, content: string | ArrayBuffer, branch: string, message: string, _existingSha?: string): Promise<{ path: string, sha?: string }> {
+    async pushFile(path: string, content: string | ArrayBuffer, branch: string, message: string, _existingSha?: string, _revision?: string): Promise<{ path: string, sha?: string }> {
         const [result] = await this.pushBatch([{ path, content }], branch, message);
         return result ?? { path };
     }

src/services/gitlab-service.ts

@@ -28,26 +28,27 @@ export class GitLabService extends BaseGitService implements GitServiceInterface
             const url = `${this.getApiUrl(path)}?ref=${branch}`;
             const response = await this.safeRequest(url, 'GET');
             const data = this.parseJson<GitLabFileResponse>(response);
-            
+
             return {
                 content: this.decodeContent(data.content, path),
-                sha: data.last_commit_id
+                sha: data.blob_id,
+                revision: data.last_commit_id
             };
         } catch (e) {
             return this.handleFileNotFound(e);
         }
     }
 
-    async pushFile(path: string, content: string | ArrayBuffer, branch: string, message: string, sha?: string): Promise<{ path: string, sha?: string }> {
+    async pushFile(path: string, content: string | ArrayBuffer, branch: string, message: string, sha?: string, revision?: string): Promise<{ path: string, sha?: string }> {
         const url = this.getApiUrl(path);
         const body: { branch: string; content: string; encoding: string; commit_message: string; last_commit_id?: string } = {
             branch,
             content: this.encodeContent(content),
             encoding: 'base64',
             commit_message: message,
         };
-        // A blank sha means the file is new: create it (POST) without last_commit_id.
-        if (sha) body.last_commit_id = sha;
+        // A blank sha means the file is new: create it (POST). Use revision for GitLab's optimistic locking.
+        if (revision) body.last_commit_id = revision;
 
         const method = sha ? 'PUT' : 'POST';
         const response = await this.safeRequest(url, method, body);

tests/logic/sync-manager-batch.test.ts

@@ -452,7 +452,7 @@ describe('SyncManager Batch Operations', () => {
 
             expect(results.success).toBe(1);
             expect(results.conflicts).toBe(0);
-            expect(mockGitService.pushFile).toHaveBeenCalledWith(path, 'local content', 'main', expect.any(String), 'some-sha');
+            expect(mockGitService.pushFile).toHaveBeenCalledWith(path, 'local content', 'main', expect.any(String), 'some-sha', undefined);
         });
     });
 
@@ -473,7 +473,7 @@ describe('SyncManager Batch Operations', () => {
             await manager.pushAllFiles([mockFile]);
 
             expect(mockGitService.getFile).not.toHaveBeenCalled();
-            expect(mockGitService.pushFile).toHaveBeenCalledWith(newPath, 'new content', 'main', expect.any(String), undefined);
+            expect(mockGitService.pushFile).toHaveBeenCalledWith(newPath, 'new content', 'main', expect.any(String), undefined, undefined);
         });
     });
 
@@ -599,7 +599,7 @@ describe('SyncManager Batch Operations', () => {
 
             expect(results.success).toBe(1);
             expect(mockGitService.pushFile).toHaveBeenCalledWith(
-                pushedPath, 'unrelated content', 'main', `Update ${mockFile.name} from Obsidian`, 'remote-sha'
+                pushedPath, 'unrelated content', 'main', `Update ${mockFile.name} from Obsidian`, 'remote-sha', undefined
             );
             expect(mockSettings.syncMetadata[orphanedPath]).toBeDefined();
             // The tree doesn't list the orphaned path, so it can't be a rename

tests/logic/sync-manager-binary.test.ts

@@ -36,7 +36,7 @@ describe('SyncManager – binary file handling', () => {
             expect(mockAdapter.readBinary).toHaveBeenCalledWith('photo.png');
             expect(mockAdapter.read).not.toHaveBeenCalled();
             expect(mockGitService.pushFile).toHaveBeenCalledWith(
-                'photo.png', buf, 'main', expect.any(String), ''
+                'photo.png', buf, 'main', expect.any(String), '', undefined
             );
         });
 

tests/logic/sync-manager-hidden.test.ts

@@ -80,7 +80,7 @@ describe('SyncManager – hidden file support', () => {
             await manager.pushFile('.claude/CLAUDE.md');
 
             expect(mockGitService.pushFile).toHaveBeenCalledWith(
-                '.claude/CLAUDE.md', '# Memory\n\nsome content', 'main', expect.any(String), ''
+                '.claude/CLAUDE.md', '# Memory\n\nsome content', 'main', expect.any(String), '', undefined
             );
         });
 

tests/logic/sync-manager-mapping.test.ts

@@ -99,7 +99,8 @@ describe('SyncManager Mapping', () => {
             'content',
             'main',
             'Update test.md from Obsidian',
-            ''
+            '',
+            undefined
         );
     });