feat(factory): construct PluginManager after Rc::new_cyclic

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

5 files changed, 61 insertions, 49 deletions

diff --git a/crates/shirabe/src/factory.rs b/crates/shirabe/src/factory.rs
index 0084c73..4c10db3 100644
--- a/crates/shirabe/src/factory.rs
+++ b/crates/shirabe/src/factory.rs
@@ -789,32 +789,6 @@ impl Factory {
                         }
                     }
 
-                    if let Some(full_composer) = composer.as_full_mut() {
-                        let global_composer = if !full_composer.is_global() {
-                            self.create_global_composer(
-                                io.clone(),
-                                &*config.borrow(),
-                                disable_plugins,
-                                disable_scripts,
-                                false,
-                            )
-                        } else {
-                            None
-                        };
-                        let mut pm = self.create_plugin_manager(
-                            &*io.borrow(),
-                            ComposerWeakHandle::from_weak(composer_weak.clone()),
-                            global_composer,
-                            disable_plugins,
-                        );
-                        if full_composer.is_global() {
-                            pm.set_running_in_global_dir(true);
-                        }
-                        pm.load_installed_plugins();
-                        full_composer
-                            .set_plugin_manager(std::rc::Rc::new(std::cell::RefCell::new(pm)));
-                    }
-
                     Ok(composer)
                 };
                 match build() {
@@ -830,6 +804,47 @@ impl Factory {
             return Err(e);
         }
 
+        // initialize plugin manager
+        //
+        // PluginManager::new upgrades the Composer back-reference to read its config and locker, so
+        // it must be built after Rc::new_cyclic returns; inside the closure the Rc is not yet
+        // constructed and the weak handle cannot upgrade.
+        let (is_full, is_global) = {
+            let c = composer.borrow();
+            (c.is_full(), c.is_global())
+        };
+        if is_full {
+            let global_composer = if !is_global {
+                self.create_global_composer(
+                    io.clone(),
+                    &*config.borrow(),
+                    disable_plugins,
+                    disable_scripts,
+                    false,
+                )
+            } else {
+                None
+            };
+
+            let pm = self.create_plugin_manager(
+                io.clone(),
+                ComposerWeakHandle::from_weak(std::rc::Rc::downgrade(&composer)),
+                global_composer,
+                disable_plugins,
+            );
+            let pm = std::rc::Rc::new(std::cell::RefCell::new(pm));
+            composer
+                .borrow_mut()
+                .as_full_mut()
+                .unwrap()
+                .set_plugin_manager(pm.clone());
+
+            if is_global {
+                pm.borrow_mut().set_running_in_global_dir(true);
+            }
+            pm.borrow_mut().load_installed_plugins()?;
+        }
+
         if full_load {
             // The back-reference is now upgradeable, so dispatching the INIT event (which may read
             // the Composer through the dispatcher) is safe only after the Rc has been constructed.
@@ -1160,15 +1175,12 @@ impl Factory {
 
     fn create_plugin_manager(
         &self,
-        io: &dyn IOInterface,
+        io: std::rc::Rc<std::cell::RefCell<dyn IOInterface>>,
         composer: ComposerWeakHandle,
         global_composer: Option<PartialComposerHandle>,
         disable_plugins: DisablePlugins,
     ) -> PluginManager {
-        // TODO(phase-b): PluginManager::new takes ownership of Composer/PartialComposer; PHP
-        // class semantics requires Rc<RefCell<>> for shared access. Stubbed for now.
-        let _ = (io, composer, global_composer, disable_plugins);
-        todo!("PluginManager::new requires shared Composer/PartialComposer")
+        PluginManager::new(io, composer, global_composer, disable_plugins)
     }
 
     pub fn create_installation_manager(
diff --git a/crates/shirabe/src/plugin/plugin_manager.rs b/crates/shirabe/src/plugin/plugin_manager.rs
index a03fcaa..e2c39af 100644
--- a/crates/shirabe/src/plugin/plugin_manager.rs
+++ b/crates/shirabe/src/plugin/plugin_manager.rs
@@ -19,6 +19,7 @@ use shirabe_semver::constraint::SimpleConstraint;
 use crate::composer::PartialComposerHandle;
 use crate::composer::{ComposerHandle, ComposerWeakHandle};
 use crate::event_dispatcher::EventSubscriberInterface;
+use crate::factory::DisablePlugins;
 use crate::installer::InstallerInterface;
 use crate::io::IOInterface;
 use crate::io::IOInterfaceImmutable;
@@ -40,15 +41,6 @@ use crate::repository::RepositoryUtils;
 use crate::repository::RootPackageRepository;
 use crate::util::PackageSorter;
 
-/// Marker for the disablePlugins variant: false | "local" | "global" | true.
-#[derive(Debug, Clone, PartialEq)]
-pub enum DisablePlugins {
-    False,
-    Local,
-    Global,
-    True,
-}
-
 #[derive(Debug)]
 pub struct PluginManager {
     pub(crate) composer: ComposerWeakHandle,
@@ -816,7 +808,7 @@ impl PluginManager {
 
     pub fn are_plugins_disabled(&self, r#type: &str) -> bool {
         match (&self.disable_plugins, r#type) {
-            (DisablePlugins::True, _) => true,
+            (DisablePlugins::All, _) => true,
             (DisablePlugins::Local, "local") => true,
             (DisablePlugins::Global, "global") => true,
             _ => false,
@@ -824,7 +816,7 @@ impl PluginManager {
     }
 
     pub fn disable_plugins(&mut self) {
-        self.disable_plugins = DisablePlugins::True;
+        self.disable_plugins = DisablePlugins::All;
     }
 
     pub fn is_plugin_allowed(
diff --git a/crates/shirabe/src/util/git.rs b/crates/shirabe/src/util/git.rs
index 3be37f6..1f6fd0a 100644
--- a/crates/shirabe/src/util/git.rs
+++ b/crates/shirabe/src/util/git.rs
@@ -148,9 +148,11 @@ impl Git {
         let mut last_command: PhpMixed = PhpMixed::String(String::new());
 
         // Ensure we are allowed to use this URL by config
-        self.config
-            .borrow_mut()
-            .prohibit_url_by_config(url, Some(&*self.io.borrow()), &IndexMap::new())?;
+        self.config.borrow_mut().prohibit_url_by_config(
+            url,
+            Some(&*self.io.borrow()),
+            &IndexMap::new(),
+        )?;
 
         let orig_cwd: Option<String> = if initial_clone {
             cwd.map(|s| s.to_string())
diff --git a/crates/shirabe/src/util/http/curl_downloader.rs b/crates/shirabe/src/util/http/curl_downloader.rs
index 87ea8ef..71c0b5a 100644
--- a/crates/shirabe/src/util/http/curl_downloader.rs
+++ b/crates/shirabe/src/util/http/curl_downloader.rs
@@ -339,9 +339,11 @@ impl CurlDownloader {
         if !Preg::is_match(r"{^http://(repo\.)?packagist\.org/p/}", url).unwrap_or(false)
             || (strpos(url, "$").is_none() && strpos(url, "%24").is_none())
         {
-            self.config
-                .borrow_mut()
-                .prohibit_url_by_config(url, Some(&*self.io.borrow()), &options)?;
+            self.config.borrow_mut().prohibit_url_by_config(
+                url,
+                Some(&*self.io.borrow()),
+                &options,
+            )?;
         }
 
         let curl_handle = curl_init();
diff --git a/crates/shirabe/src/util/remote_filesystem.rs b/crates/shirabe/src/util/remote_filesystem.rs
index 8d50fa4..9aa40bd 100644
--- a/crates/shirabe/src/util/remote_filesystem.rs
+++ b/crates/shirabe/src/util/remote_filesystem.rs
@@ -337,7 +337,11 @@ impl RemoteFilesystem {
                             PhpMixed::Array(m) => m.into_iter().map(|(k, v)| (k, *v)).collect(),
                             _ => IndexMap::new(),
                         };
-                        let _ = HttpDownloader::output_warnings(&*self.io.borrow(), origin_url, &parsed_map);
+                        let _ = HttpDownloader::output_warnings(
+                            &*self.io.borrow(),
+                            origin_url,
+                            &parsed_map,
+                        );
                     }
 
                     if [401_i64, 403].contains(&code) && retry_auth_failure {