sync(feat[update_repo]) Return SyncResult from update_repo()

why: GitSync.update_repo() catches CommandError at 10+ locations and
silently returns None, making it impossible for callers to detect
sync failures. This causes vcspull to report "✓ Synced" even when
git operations (fetch, rebase, checkout) fail.

what:
- Add SyncResult and SyncError dataclasses to sync/base.py
- Change BaseSync.update_repo() return type from None to SyncResult
- Transform GitSync.update_repo() except handlers to record errors
  in SyncResult while preserving existing control flow
- Update SvnSync and HgSync for API consistency
- Add tests for success and fetch-failure scenarios

Author: tony

src/libvcs/sync/base.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import dataclasses
 import logging
 import pathlib
 import typing as t
@@ -14,6 +15,66 @@
 logger = logging.getLogger(__name__)
 
 
+@dataclasses.dataclass
+class SyncError:
+    """An error encountered during a sync step.
+
+    Examples
+    --------
+    >>> error = SyncError(step="fetch", message="remote not found")
+    >>> error.step
+    'fetch'
+    >>> error.message
+    'remote not found'
+    >>> error.exception is None
+    True
+    """
+
+    step: str
+    message: str
+    exception: Exception | None = None
+
+
+@dataclasses.dataclass
+class SyncResult:
+    """Result of a repository synchronization.
+
+    Examples
+    --------
+    >>> result = SyncResult()
+    >>> result.ok
+    True
+    >>> bool(result)
+    True
+
+    >>> result = SyncResult()
+    >>> result.add_error(step="fetch", message="remote not found")
+    >>> result.ok
+    False
+    >>> bool(result)
+    False
+    >>> result.errors[0].step
+    'fetch'
+    """
+
+    ok: bool = True
+    errors: list[SyncError] = dataclasses.field(default_factory=list)
+
+    def __bool__(self) -> bool:
+        """Return True if the sync succeeded without errors."""
+        return self.ok
+
+    def add_error(
+        self,
+        step: str,
+        message: str,
+        exception: Exception | None = None,
+    ) -> None:
+        """Record an error and mark the result as failed."""
+        self.ok = False
+        self.errors.append(SyncError(step=step, message=message, exception=exception))
+
+
 class VCSLocation(t.NamedTuple):
     """Generic VCS Location (URL and optional revision)."""
 
@@ -200,7 +261,7 @@ def ensure_dir(self, *args: t.Any, **kwargs: t.Any) -> bool:
 
         return True
 
-    def update_repo(self, *args: t.Any, **kwargs: t.Any) -> None:
+    def update_repo(self, *args: t.Any, **kwargs: t.Any) -> SyncResult:
         """Pull latest changes to here from remote repository."""
         raise NotImplementedError
 

src/libvcs/sync/git.py

@@ -29,6 +29,7 @@
 from libvcs.cmd.git import Git
 from libvcs.sync.base import (
     BaseSync,
+    SyncResult,
     VCSLocation,
     convert_pip_url as base_convert_pip_url,
 )
@@ -380,17 +381,22 @@ def update_repo(
         set_remotes: bool = False,
         *args: t.Any,
         **kwargs: t.Any,
-    ) -> None:
+    ) -> SyncResult:
         """Pull latest changes from git remote."""
+        result = SyncResult()
         self.ensure_dir()
 
         if not pathlib.Path(self.path / ".git").is_dir():
             self.obtain()
-            self.update_repo(set_remotes=set_remotes)
-            return
+            return self.update_repo(set_remotes=set_remotes)
 
         if set_remotes:
-            self.set_remotes(overwrite=True)
+            try:
+                self.set_remotes(overwrite=True)
+            except exc.CommandError as e:
+                self.log.exception("Failed to set remotes")
+                result.add_error("set-remotes", str(e), exception=e)
+                return result
 
         # Get requested revision or tag
         url, git_tag = self.url, getattr(self, "rev", None)
@@ -412,7 +418,7 @@ def update_repo(
             )
         except exc.CommandError:
             self.log.exception("Failed to get the hash for HEAD")
-            return
+            return result
 
         self.log.debug("head_sha: %s", head_sha)
 
@@ -460,21 +466,23 @@ def update_repo(
         somethings_up = (error_code, is_remote_ref, tag_sha != head_sha)
         if all(not x for x in somethings_up):
             self.log.info("Already up-to-date.")
-            return
+            return result
 
         try:
             process = self.cmd.fetch(log_in_real_time=True, check_returncode=True)
-        except exc.CommandError:
+        except exc.CommandError as e:
             self.log.exception("Failed to fetch repository '%s'", url)
-            return
+            result.add_error("fetch", str(e), exception=e)
+            return result
 
         if is_remote_ref:
             # Check if stash is needed
             try:
                 process = self.cmd.status(porcelain=True, untracked_files="no")
-            except exc.CommandError:
+            except exc.CommandError as e:
                 self.log.exception("Failed to get the status")
-                return
+                result.add_error("status", str(e), exception=e)
+                return result
             need_stash = len(process) > 0
 
             # If not in clean state, stash changes in order to be able
@@ -484,22 +492,25 @@ def update_repo(
                 git_stash_save_options = "--quiet"
                 try:
                     process = self.cmd.stash.save(message=git_stash_save_options)
-                except exc.CommandError:
+                except exc.CommandError as e:
                     self.log.exception("Failed to stash changes")
+                    result.add_error("stash-save", str(e), exception=e)
 
             # Checkout the remote branch
             try:
                 process = self.cmd.checkout(branch=git_tag)
-            except exc.CommandError:
+            except exc.CommandError as e:
                 self.log.exception("Failed to checkout tag: '%s'", git_tag)
-                return
+                result.add_error("checkout", str(e), exception=e)
+                return result
 
             # Rebase changes from the remote branch
             try:
                 process = self.cmd.rebase(upstream=git_remote_name + "/" + git_tag)
             except exc.CommandError as e:
                 if any(msg in str(e) for msg in ["invalid_upstream", "Aborting"]):
                     self.log.exception("Invalid upstream remote. Rebase aborted.")
+                    result.add_error("rebase", str(e), exception=e)
                 else:
                     # Rebase failed: Restore previous state.
                     self.cmd.rebase(abort=True)
@@ -510,7 +521,8 @@ def update_repo(
                         f"\nFailed to rebase in: '{self.path}'.\n"
                         "You will have to resolve the conflicts manually",
                     )
-                    return
+                    result.add_error("rebase", str(e), exception=e)
+                    return result
 
             if need_stash:
                 try:
@@ -520,7 +532,7 @@ def update_repo(
                     self.cmd.reset(hard=True, quiet=True)
                     try:
                         process = self.cmd.stash.pop(quiet=True)
-                    except exc.CommandError:
+                    except exc.CommandError as e:
                         # Stash pop failed: Restore previous state.
                         self.cmd.reset(pathspec=head_sha, hard=True, quiet=True)
                         self.cmd.stash.pop(index=True, quiet=True)
@@ -529,16 +541,19 @@ def update_repo(
                             "You will have to resolve the "
                             "conflicts manually",
                         )
-                        return
+                        result.add_error("stash-pop", str(e), exception=e)
+                        return result
 
         else:
             try:
                 process = self.cmd.checkout(branch=git_tag)
-            except exc.CommandError:
+            except exc.CommandError as e:
                 self.log.exception("Failed to checkout tag: '%s'", git_tag)
-                return
+                result.add_error("checkout", str(e), exception=e)
+                return result
 
         self.cmd.submodule.update(recursive=True, init=True, log_in_real_time=True)
+        return result
 
     def remotes(self) -> GitSyncRemoteDict:
         """Return remotes like git remote -v.

src/libvcs/sync/hg.py

@@ -15,10 +15,11 @@
 import pathlib
 import typing as t
 
+from libvcs import exc
 from libvcs._internal.types import StrPath
 from libvcs.cmd.hg import Hg
 
-from .base import BaseSync
+from .base import BaseSync, SyncResult
 
 logger = logging.getLogger(__name__)
 
@@ -64,11 +65,16 @@ def get_revision(self) -> str:
         """Get latest revision of this mercurial repository."""
         return self.run(["parents", "--template={rev}"])
 
-    def update_repo(self, *args: t.Any, **kwargs: t.Any) -> None:
+    def update_repo(self, *args: t.Any, **kwargs: t.Any) -> SyncResult:
         """Pull changes from remote Mercurial repository into this one."""
+        result = SyncResult()
         if not pathlib.Path(self.path / ".hg").exists():
             self.obtain()
-            self.update_repo()
+            return self.update_repo()
         else:
-            self.cmd.update()
-            self.cmd.pull(update=True)
+            try:
+                self.cmd.update()
+                self.cmd.pull(update=True)
+            except exc.CommandError as e:
+                result.add_error("pull", str(e), exception=e)
+        return result

src/libvcs/sync/svn.py

@@ -21,10 +21,11 @@
 import re
 import typing as t
 
+from libvcs import exc
 from libvcs._internal.types import StrPath
 from libvcs.cmd.svn import Svn
 
-from .base import BaseSync
+from .base import BaseSync, SyncResult
 
 logger = logging.getLogger(__name__)
 
@@ -148,22 +149,27 @@ def update_repo(
         dest: str | None = None,
         *args: t.Any,
         **kwargs: t.Any,
-    ) -> None:
+    ) -> SyncResult:
         """Fetch changes from SVN repository to local working copy."""
+        result = SyncResult()
         self.ensure_dir()
         if pathlib.Path(self.path / ".svn").exists():
-            self.cmd.checkout(
-                url=self.url,
-                username=self.username,
-                password=self.password,
-                non_interactive=True,
-                quiet=True,
-                check_returncode=True,
-                **kwargs,
-            )
+            try:
+                self.cmd.checkout(
+                    url=self.url,
+                    username=self.username,
+                    password=self.password,
+                    non_interactive=True,
+                    quiet=True,
+                    check_returncode=True,
+                    **kwargs,
+                )
+            except exc.CommandError as e:
+                result.add_error("checkout", str(e), exception=e)
         else:
             self.obtain()
-            self.update_repo()
+            return self.update_repo()
+        return result
 
     @classmethod
     def _get_svn_url_rev(cls, location: str) -> tuple[str | None, int]:

tests/sync/test_git.py

@@ -15,6 +15,7 @@
 from libvcs import exc
 from libvcs._internal.run import run
 from libvcs._internal.shortcuts import create_project
+from libvcs.sync.base import SyncResult
 from libvcs.sync.git import (
     GitRemote,
     GitStatus,
@@ -923,3 +924,71 @@ def test_repo_git_remote_checkout(
 
     assert git_repo_checkout_dir.exists()
     assert pathlib.Path(git_repo_checkout_dir / ".git").exists()
+
+
+def test_update_repo_success_returns_sync_result(
+    create_git_remote_bare_repo: CreateRepoPytestFixtureFn,
+    tmp_path: pathlib.Path,
+) -> None:
+    """Test that a successful update_repo() returns SyncResult with ok=True."""
+    git_server = create_git_remote_bare_repo()
+    git_repo = GitSync(
+        path=tmp_path / "myrepo",
+        url=git_server.as_uri(),
+    )
+    git_repo.obtain()
+
+    # Make an initial commit so rev-list HEAD succeeds
+    initial_file = git_repo.path / "initial_file"
+    initial_file.write_text("content", encoding="utf-8")
+    git_repo.run(["add", str(initial_file)])
+    git_repo.run(["commit", "-m", "initial commit"])
+    git_repo.run(["push"])
+
+    result = git_repo.update_repo()
+
+    assert isinstance(result, SyncResult)
+    assert result.ok is True
+    assert result.errors == []
+    assert bool(result) is True
+
+
+def test_update_repo_fetch_failure_returns_sync_result(
+    create_git_remote_bare_repo: CreateRepoPytestFixtureFn,
+    tmp_path: pathlib.Path,
+) -> None:
+    """Test that a fetch failure in update_repo() returns SyncResult with error."""
+    git_server = create_git_remote_bare_repo()
+    git_repo = GitSync(
+        path=tmp_path / "myrepo",
+        url=git_server.as_uri(),
+    )
+    git_repo.obtain()
+
+    # Make a commit and push so the repo has a valid HEAD
+    initial_file = git_repo.path / "initial_file"
+    initial_file.write_text("content", encoding="utf-8")
+    git_repo.run(["add", str(initial_file)])
+    git_repo.run(["commit", "-m", "a commit"])
+    git_repo.run(["push"])
+
+    # Make another commit, push, then reset to create a "behind" state
+    another_file = git_repo.path / "another_file"
+    another_file.write_text("more content", encoding="utf-8")
+    git_repo.run(["add", str(another_file)])
+    git_repo.run(["commit", "-m", "second commit"])
+    git_repo.run(["push"])
+    git_repo.run(["reset", "--hard", "HEAD^"])
+
+    # Delete the remote directory to cause a fetch failure
+    shutil.rmtree(git_server)
+
+    result = git_repo.update_repo()
+
+    assert isinstance(result, SyncResult)
+    assert result.ok is False
+    assert bool(result) is False
+    assert len(result.errors) > 0
+    assert result.errors[0].step == "fetch"
+    assert result.errors[0].exception is not None
+    assert isinstance(result.errors[0].exception, exc.CommandError)