From aa4e9ff27b974343beeccdd14e07e13f0e261599 Mon Sep 17 00:00:00 2001
From: nsfisis <nsfisis@gmail.com>
Date: Mon, 22 Jun 2026 23:42:25 +0900
Subject: feat(solver): surface SolverProblemsException via two-layer Result

solve() previously dropped the SolverProblemsException and returned a
placeholder anyhow error, because its Rc<RefCell<Rule>> payload is not
Send+Sync and cannot ride anyhow::Error. Return
anyhow::Result<Result<LockTransaction, SolverProblemsException>> instead,
keeping fatal errors on the outer Result and the recoverable exception on
the inner one.

This lets the three Installer callsites port their PHP catch handlers
faithfully (pretty-string output, GithubActionError emit, getCode-based
exit codes) and lets solver_test.rs assert getProblems/getCode/
getPrettyString. The four ported assertion tests stay ignored: their
pretty-string path still reaches unimplemented todo!()s downstream.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 crates/shirabe/src/dependency_resolver/solver.rs | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

(limited to 'crates/shirabe/src/dependency_resolver')

diff --git a/crates/shirabe/src/dependency_resolver/solver.rs b/crates/shirabe/src/dependency_resolver/solver.rs
index 52c4580..cd00757 100644
--- a/crates/shirabe/src/dependency_resolver/solver.rs
+++ b/crates/shirabe/src/dependency_resolver/solver.rs
@@ -230,7 +230,7 @@ impl Solver {
         &mut self,
         request: &Request,
         platform_requirement_filter: Option<Rc<dyn PlatformRequirementFilterInterface>>,
-    ) -> anyhow::Result<LockTransaction> {
+    ) -> anyhow::Result<Result<LockTransaction, SolverProblemsException>> {
         let platform_requirement_filter = platform_requirement_filter
             .unwrap_or_else(|| PlatformRequirementFilterFactory::ignore_nothing());
 
@@ -272,27 +272,25 @@ impl Solver {
         );
 
         if !self.problems.is_empty() {
-            // TODO(phase-c): SolverProblemsException stores `Rc<RefCell<Rule>>` which is not
-            // `Send + Sync`, so it cannot satisfy `anyhow::Error`'s bounds. Returning a
-            // placeholder error preserves control flow until the solver error path is reworked to
-            // a dedicated (non-anyhow) error type or the exception drops its dyn Rule payload.
-            let _ = SolverProblemsException::new(
+            // SolverProblemsException stores `Rc<RefCell<Rule>>` which is not `Send + Sync`, so it
+            // cannot satisfy `anyhow::Error`'s bounds. It is the recoverable exception PHP callers
+            // `catch`, so it rides the inner `Result` while fatal errors stay on the outer one.
+            return Ok(Err(SolverProblemsException::new(
                 std::mem::take(&mut self.problems),
                 std::mem::take(&mut self.learned_pool),
-            );
-            return Err(anyhow::anyhow!("solver problems"));
+            )));
         }
 
         // LockTransaction stores PackageInterfaceHandle maps; widen the request's BasePackageHandle
         // maps into them.
         let present_map = request.get_present_map(false)?.into_iter().collect();
         let unlockable_map = request.get_fixed_packages_map().into_iter().collect();
-        Ok(LockTransaction::new(
+        Ok(Ok(LockTransaction::new(
             &self.pool.borrow(),
             present_map,
             unlockable_map,
             &self.decisions,
-        ))
+        )))
     }
 
     /// Makes a decision and propagates it to all rules.
-- 
cgit v1.3.1