Fix: Build clients race condition

crates/stellar-scaffold-cli/src/commands/build/clients.rs

@@ -107,7 +107,7 @@ impl Args {
 
         let Some(current_env) = env_toml::Environment::get(
             workspace_root,
-            &self.clone().stellar_scaffold_env(ScaffoldEnv::Production),
+            &self.stellar_scaffold_env(ScaffoldEnv::Production),
         )?
         else {
             return Ok(());
@@ -131,7 +131,7 @@ impl Args {
         Ok(())
     }
 
-    fn stellar_scaffold_env(self, default: ScaffoldEnv) -> String {
+    fn stellar_scaffold_env(&self, default: ScaffoldEnv) -> String {
         self.env.unwrap_or(default).to_string().to_lowercase()
     }
 
@@ -253,103 +253,119 @@ impl Args {
         config_dir.save_contract_id(&passphrase, contract_id, name)
     }
 
-    fn write_contract_template(self, name: &str, contract_id: &str) -> Result<(), Error> {
-        let allow_http =
-            if self.clone().stellar_scaffold_env(ScaffoldEnv::Production) == "development" {
-                "\n  allowHttp: true,"
-            } else {
-                ""
-            };
+    fn create_contract_template(&self, name: &str, contract_id: &str) -> String {
+        let allow_http = if self.stellar_scaffold_env(ScaffoldEnv::Production) == "production" {
+            "\n  allowHttp: true,"
+        } else {
+            ""
+        };
         let network = std::env::var("STELLAR_NETWORK_PASSPHRASE")
             .expect("No STELLAR_NETWORK_PASSPHRASE environment variable set");
-        let template = format!(
+        format!(
             r"import * as Client from '{name}';
 import {{ rpcUrl }} from './util';
-    
+
 export default new Client.Client({{
   networkPassphrase: '{network}',
   contractId: '{contract_id}',
   rpcUrl,{allow_http}
   publicKey: undefined,
 }});
 "
-        );
-        let workspace_root = self
-            .workspace_root
-            .as_ref()
-            .expect("workspace_root not set");
-        let path = workspace_root.join(format!("src/contracts/{name}.ts"));
-        std::fs::write(path, template)?;
-        Ok(())
+        )
     }
 
-    async fn generate_contract_bindings(self, name: &str, contract_id: &str) -> Result<(), Error> {
+    async fn generate_contract_bindings(
+        &self,
+        name: &str,
+        contract_id: &str,
+    ) -> Result<String, Error> {
         eprintln!("🎭 binding {name:?} contract");
         let workspace_root = self
             .workspace_root
             .as_ref()
             .expect("workspace_root not set");
-        let output_dir = workspace_root.join(format!("packages/{name}"));
+        let final_output_dir = workspace_root.join(format!("packages/{name}"));
+
+        // Create a temporary directory for building the new client
+        let temp_dir = workspace_root.join(format!("target/packages/{name}"));
+        let temp_dir_display = temp_dir.display();
+
         cli::contract::bindings::typescript::Cmd::parse_arg_vec(&[
             "--contract-id",
             contract_id,
             "--output-dir",
-            output_dir
-                .to_str()
-                .expect("we do not support non-utf8 paths"),
-            "--overwrite",
+            temp_dir.to_str().expect("we do not support non-utf8 paths"),
             "--config-dir",
             workspace_root
                 .to_str()
                 .expect("we do not support non-utf8 paths"),
+            "--overwrite",
         ])?
         .run()
         .await?;
 
-        eprintln!("🍽️ importing {name:?} contract");
-        self.write_contract_template(name, contract_id)?;
-
-        // Run `npm i` in the output directory
-        eprintln!("🔧 running 'npm install' in {output_dir:?}");
+        // Run `npm i` in the temp directory
+        eprintln!("🔧 running 'npm install' in {temp_dir_display}");
         let output = std::process::Command::new("npm")
-            .current_dir(&output_dir)
+            .current_dir(&temp_dir)
             .arg("install")
             .arg("--loglevel=error") // Reduce noise from warnings
             .arg("--no-workspaces") // fix issue where stellar sometimes isnt installed locally causing tsc to fail
             .output()?;
 
         if !output.status.success() {
+            // Clean up temp directory on failure
+            let _ = std::fs::remove_dir_all(&temp_dir);
             return Err(Error::NpmCommandFailure(
-                output_dir.clone(),
+                temp_dir.clone(),
                 format!(
                     "npm install failed with status: {:?}\nError: {}",
                     output.status.code(),
                     String::from_utf8_lossy(&output.stderr)
                 ),
             ));
         }
-        eprintln!("✅ 'npm install' succeeded in {output_dir:?}");
+        eprintln!("✅ 'npm install' succeeded in {temp_dir_display}");
 
-        eprintln!("🔨 running 'npm run build' in {output_dir:?}");
+        eprintln!("🔨 running 'npm run build' in {temp_dir_display}");
         let output = std::process::Command::new("npm")
-            .current_dir(&output_dir)
+            .current_dir(&temp_dir)
             .arg("run")
             .arg("build")
             .arg("--loglevel=error") // Reduce noise from warnings
             .output()?;
 
         if !output.status.success() {
+            // Clean up temp directory on failure
+            let _ = std::fs::remove_dir_all(&temp_dir);
             return Err(Error::NpmCommandFailure(
-                output_dir.clone(),
+                temp_dir.clone(),
                 format!(
                     "npm run build failed with status: {:?}\nError: {}",
                     output.status.code(),
                     String::from_utf8_lossy(&output.stderr)
                 ),
             ));
         }
-        eprintln!("✅ 'npm run build' succeeded in {output_dir:?}");
-        Ok(())
+        eprintln!("✅ 'npm run build' succeeded in {temp_dir_display}",);
+
+        // Now atomically replace the old directory with the new one
+        if final_output_dir.exists() {
+            if let Err(e) = std::fs::rename(&temp_dir, &final_output_dir) {
+                // Failed to move new directory, clean up temp directory
+                std::fs::remove_dir_all(&temp_dir)?;
+                return Err(Error::Io(e));
+            }
+            eprintln!("✅ Client {name:?} updated successfully");
+        } else {
+            // No existing directory, just move temp to final location
+            std::fs::rename(&temp_dir, &final_output_dir)?;
+            eprintln!("✅ Client {name:?} created successfully");
+        }
+
+        // Return the contract template content instead of writing it immediately
+        Ok(self.create_contract_template(name, contract_id))
     }
 
     async fn handle_accounts(
@@ -487,7 +503,7 @@ export default new Client.Client({{
             return Ok(());
         }
 
-        let env = self.clone().stellar_scaffold_env(ScaffoldEnv::Production);
+        let env = self.stellar_scaffold_env(ScaffoldEnv::Production);
         if env == "production" || env == "staging" {
             if let Some(contracts) = contracts {
                 self.handle_production_contracts(contracts).await?;
@@ -498,6 +514,7 @@ export default new Client.Client({{
         self.validate_contract_names(contracts)?;
 
         let names = Self::maintain_user_ordering(&package_names, contracts);
+
         let mut results: Vec<(String, Result<(), String>)> = Vec::new();
 
         for name in names {
@@ -515,7 +532,7 @@ export default new Client.Client({{
                 .process_single_contract(&name, settings, network, &env)
                 .await
             {
-                Ok(()) => {
+                Ok(_) => {
                     eprintln!("✅ Successfully generated client for: {name}");
                     results.push((name, Ok(())));
                 }
@@ -582,7 +599,7 @@ export default new Client.Client({{
         settings: env_toml::Contract,
         network: &Network,
         env: &str,
-    ) -> Result<(), Error> {
+    ) -> Result<String, Error> {
         // First check if we have an ID in settings
         let contract_id = if let Some(id) = &settings.id {
             Contract::from_string(id).map_err(|_| Error::InvalidContractID(id.clone()))?
@@ -601,7 +618,10 @@ export default new Client.Client({{
                     .await?
                 {
                     eprintln!("✅ Contract {name:?} is up to date");
-                    return Ok(());
+                    let template = self
+                        .generate_contract_bindings(name, &existing_contract_id.to_string())
+                        .await?;
+                    return Ok(template);
                 }
                 eprintln!("🔄 Updating contract {name:?}");
             }
@@ -622,11 +642,11 @@ export default new Client.Client({{
             contract_id
         };
 
-        self.clone()
+        let template = self
             .generate_contract_bindings(name, &contract_id.to_string())
             .await?;
 
-        Ok(())
+        Ok(template)
     }
 
     async fn upload_contract_wasm(