fix(vcs-downloader): abort on local changes in parent cleanChanges path

The VcsDownloaderBase::clean_changes stub returned Ok(None), silently
swallowing the abort that PHP's parent::cleanChanges() raises when the
working copy has uncommitted changes. Drop the stub and give Git/Svn a
private fail_on_local_changes helper that performs the getLocalChanges
check and throws, matching the default VcsDownloader::clean_changes().

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

3 files changed, 38 insertions, 21 deletions

diff --git a/crates/shirabe/src/downloader/git_downloader.rs b/crates/shirabe/src/downloader/git_downloader.rs
index bc8db6d..078ac3a 100644
--- a/crates/shirabe/src/downloader/git_downloader.rs
+++ b/crates/shirabe/src/downloader/git_downloader.rs
@@ -680,6 +680,20 @@ impl GitDownloader {
 
         reference.to_string()
     }
+
+    /// The default `VcsDownloader::clean_changes()` behavior: fail if the working copy has
+    /// local changes.
+    fn fail_on_local_changes(&mut self, package: PackageInterfaceHandle, path: &str) -> Result<()> {
+        if self.get_local_changes(package, path)?.is_some() {
+            return Err(RuntimeException {
+                message: format!("Source directory {} has uncommitted changes.", path),
+                code: 0,
+            }
+            .into());
+        }
+
+        Ok(())
+    }
 }
 
 impl DvcsDownloaderInterface for GitDownloader {
@@ -1206,16 +1220,15 @@ impl VcsDownloader for GitDownloader {
             }
             if discard_changes.as_string() == Some("stash") {
                 if !update {
-                    return self
-                        .inner
-                        .clean_changes(package.clone(), &path, update)
-                        .await;
+                    self.fail_on_local_changes(package.clone(), &path)?;
+                    return Ok(None);
                 }
 
                 return self.stash_changes(&path).await;
             }
 
-            return self.inner.clean_changes(package, &path, update).await;
+            self.fail_on_local_changes(package, &path)?;
+            return Ok(None);
         }
 
         let changes: Vec<String> = array_map(
diff --git a/crates/shirabe/src/downloader/svn_downloader.rs b/crates/shirabe/src/downloader/svn_downloader.rs
index ef0fd16..95f25f8 100644
--- a/crates/shirabe/src/downloader/svn_downloader.rs
+++ b/crates/shirabe/src/downloader/svn_downloader.rs
@@ -84,6 +84,24 @@ impl SvnDownloader {
 
         Ok(None)
     }
+
+    /// The default `VcsDownloader::clean_changes()` behavior: fail if the working copy has
+    /// local changes.
+    fn fail_on_local_changes(
+        &mut self,
+        package: PackageInterfaceHandle,
+        path: &str,
+    ) -> anyhow::Result<()> {
+        if self.get_local_changes(package, path)?.is_some() {
+            return Err(RuntimeException {
+                message: format!("Source directory {} has uncommitted changes.", path),
+                code: 0,
+            }
+            .into());
+        }
+
+        Ok(())
+    }
 }
 
 impl VcsDownloader for SvnDownloader {
@@ -257,7 +275,8 @@ impl VcsDownloader for SvnDownloader {
                 return self.discard_changes(path).await;
             }
 
-            return self.inner.clean_changes(package, path, update).await;
+            self.fail_on_local_changes(package, path)?;
+            return Ok(None);
         }
 
         let changes_str = changes.unwrap();
diff --git a/crates/shirabe/src/downloader/vcs_downloader.rs b/crates/shirabe/src/downloader/vcs_downloader.rs
index 0637f08..a967c3b 100644
--- a/crates/shirabe/src/downloader/vcs_downloader.rs
+++ b/crates/shirabe/src/downloader/vcs_downloader.rs
@@ -55,21 +55,6 @@ impl VcsDownloaderBase {
         }
     }
 
-    /// Equivalent of PHP `parent::cleanChanges()`. Subclasses that override the trait method
-    /// call this when they need to invoke the base behavior. Since this lives on the data struct,
-    /// it cannot consult subclass-specific `get_local_changes`; it assumes any callers have
-    /// already verified that no local changes exist.
-    pub async fn clean_changes(
-        &self,
-        _package: PackageInterfaceHandle,
-        _path: &str,
-        _update: bool,
-    ) -> Result<Option<PhpMixed>> {
-        // TODO(phase-b): parent::cleanChanges() rechecks getLocalChanges via dynamic dispatch.
-        // Callers in subclasses must do that check themselves (they already have).
-        Ok(None)
-    }
-
     pub fn get_vcs_reference(&self, package: PackageInterfaceHandle, path: &str) -> Option<String> {
         let parser = VersionParser::new();
         let mut guesser = VersionGuesser::new(