Bridge / REPL / SDK cleanups (#3034)

* remove completions spinner because of glitches

* cargo colors

* type naming cleanups

* preserve boolean case position

* do not use global counters for enum cases.

* union ordering fixes

* fix optional boolean

* lint / fmt

* update test target

* extract owners

* format

* format

* update tests

* increase defaults

* test updates

* test updates

* also increase for download

* fix accidentally removed command env

* test updates

* update CLI goldenfiles

Author: noise64

Makefile.toml

@@ -11,8 +11,6 @@
 # - `cargo make worker-executor-tests`: runs worker executor tests only
 # - `cargo make integration-tests`: runs integration tests only
 # - `cargo make sharding-tests-debug`: runs sharding integration tests with file logging enabled, also accepts test name filter arguments
-# - `cargo make api-tests-http`: runs api integration tests using HTTP API only
-# - `cargo make api-tests-grpc`: runs api integration tests using GRPC API only
 # - `cargo make test`: runs all unit tests, worker executor tests and integration tests
 # - `cargo make check-openapi`: generates openapi spec from the code and checks if it is the same as the one in the openapi directory (for CI)
 # - `cargo make generate-openapi`: generates openapi spec from the code and saves it to the openapi directory
@@ -349,7 +347,6 @@ description = "Runs all unit tests, worker executor tests and integration tests"
 dependencies = [
     "unit-tests",
     "worker-executor-tests",
-    "api-tests",
     "integration-tests",
 ]
 

cli/golem-cli/src/app/build/command.rs

@@ -372,12 +372,16 @@ pub async fn execute_external_command(
                 )
                 .await?;
 
-                let mut cmd = Command::new(command_tokens[0].clone());
-                cmd.args(command_tokens.iter().skip(1))
+                let mut process = Command::new(command_tokens[0].clone());
+
+                process
+                    .args(command_tokens.iter().skip(1))
                     .current_dir(&build_dir)
-                    .envs(&command.env)
-                    .stream_and_run(&command_tokens[0])
-                    .await
+                    .envs(&command.env);
+
+                configure_external_command_env(&mut process, command_tokens[0].as_str(), ctx);
+
+                process.stream_and_run(&command_tokens[0]).await
             },
             || {
                 log_skipping_up_to_date(format!(
@@ -420,3 +424,22 @@ pub async fn ensure_common_deps_for_tool(
         _ => Ok(()),
     }
 }
+
+fn configure_external_command_env(command: &mut Command, executable: &str, ctx: &BuildContext<'_>) {
+    let executable_name = Path::new(executable)
+        .file_name()
+        .and_then(|name| name.to_str());
+
+    if let Some("cargo") = executable_name {
+        apply_cargo_term_color_env(command, ctx.should_colorize())
+    }
+}
+
+fn apply_cargo_term_color_env(command: &mut Command, should_colorize: bool) {
+    if std::env::var_os("CARGO_TERM_COLOR").is_some() {
+        return;
+    }
+
+    let color = if should_colorize { "always" } else { "never" };
+    command.env("CARGO_TERM_COLOR", color);
+}

cli/golem-cli/src/app/context.rs

@@ -87,6 +87,10 @@ impl<'a> BuildContext<'a> {
         self.build_config.skip_up_to_date_checks
     }
 
+    pub fn should_colorize(&self) -> bool {
+        self.application_config().should_colorize
+    }
+
     pub fn tools_with_ensured_common_deps(&self) -> &ToolsWithEnsuredCommonDeps {
         &self.application_context.tools_with_ensured_common_deps
     }

cli/golem-cli/src/bridge_gen/type_naming/mod.rs

@@ -76,10 +76,21 @@ pub struct TypeNaming<TN: TypeName> {
 
 impl<TN: TypeName> TypeNaming<TN> {
     pub fn new(agent_type: &AgentType, same_language: bool) -> anyhow::Result<Self> {
+        Self::new_with_reserved_names(agent_type, same_language, std::iter::empty::<TN>())
+    }
+
+    pub fn new_with_reserved_names(
+        agent_type: &AgentType,
+        same_language: bool,
+        reserved_names: impl IntoIterator<Item = TN>,
+    ) -> anyhow::Result<Self> {
+        let mut type_names = HashSet::new();
+        type_names.extend(reserved_names);
+
         let mut type_naming = Self {
             named_type_locations: Default::default(),
             anonymous_type_locations: Default::default(),
-            type_names: HashSet::new(),
+            type_names,
             types: HashMap::new(),
             same_language,
         };

cli/golem-cli/src/bridge_gen/type_naming/type_location.rs

@@ -24,7 +24,7 @@ impl TypeLocation {
     pub fn to_type_naming_segments(&self) -> Vec<Vec<&str>> {
         match &self.root {
             TypeLocationRoot::AgentConstructorInput { input_name } => {
-                let mut segments = vec![vec!["ConstructorInput", input_name.as_str()]];
+                let mut segments = vec![vec!["AgentParam", input_name.as_str()]];
                 if let Some(path) = &self.path {
                     segments.extend(path.to_type_naming_segments());
                 }
@@ -34,7 +34,7 @@ impl TypeLocation {
                 method_name,
                 input_name,
             } => {
-                let mut segments = vec![vec![method_name.as_str(), "Input", input_name.as_str()]];
+                let mut segments = vec![vec![method_name.as_str(), "In", input_name.as_str()]];
                 if let Some(path) = &self.path {
                     segments.extend(path.to_type_naming_segments());
                 }
@@ -44,7 +44,7 @@ impl TypeLocation {
                 method_name,
                 output_name,
             } => {
-                let mut segments = vec![vec![method_name.as_str(), "Output", output_name.as_str()]];
+                let mut segments = vec![vec![method_name.as_str(), "Out", output_name.as_str()]];
                 if let Some(path) = &self.path {
                     segments.extend(path.to_type_naming_segments());
                 }
@@ -183,6 +183,7 @@ impl TypeLocationPath {
                     if let Some(name) = name {
                         subsegments.push(name.as_str());
                     }
+                    subsegments.push("Ok");
                     segments.push(subsegments);
                     if let Some(inner) = inner {
                         collect(segments, inner.as_ref());
@@ -196,6 +197,7 @@ impl TypeLocationPath {
                     if let Some(name) = name {
                         subsegments.push(name.as_str());
                     }
+                    subsegments.push("Err");
                     segments.push(subsegments);
                     if let Some(inner) = inner {
                         collect(segments, inner.as_ref());
@@ -370,3 +372,90 @@ impl Display for TypeLocationPath {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{TypeLocation, TypeLocationPath, TypeLocationRoot};
+    use test_r::test;
+
+    #[test]
+    fn root_segments_use_short_in_out_suffixes() {
+        let constructor_in = TypeLocation {
+            root: TypeLocationRoot::AgentConstructorInput {
+                input_name: "payload".to_string(),
+            },
+            path: None,
+        };
+        assert_eq!(
+            constructor_in.to_type_naming_segments(),
+            vec![vec!["AgentParam", "payload"]]
+        );
+
+        let method_in = TypeLocation {
+            root: TypeLocationRoot::AgentMethodInput {
+                method_name: "run".to_string(),
+                input_name: "request".to_string(),
+            },
+            path: None,
+        };
+        assert_eq!(
+            method_in.to_type_naming_segments(),
+            vec![vec!["run", "In", "request"]]
+        );
+
+        let method_out = TypeLocation {
+            root: TypeLocationRoot::AgentMethodOutput {
+                method_name: "run".to_string(),
+                output_name: "response".to_string(),
+            },
+            path: None,
+        };
+        assert_eq!(
+            method_out.to_type_naming_segments(),
+            vec![vec!["run", "Out", "response"]]
+        );
+    }
+
+    #[test]
+    fn result_path_segments_include_ok_and_err_markers() {
+        let ok_location = TypeLocation {
+            root: TypeLocationRoot::AgentMethodOutput {
+                method_name: "run".to_string(),
+                output_name: "response".to_string(),
+            },
+            path: Some(TypeLocationPath::ResultOk {
+                name: Some("result".to_string()),
+                owner: Some("my-owner".to_string()),
+                inner: None,
+            }),
+        };
+
+        assert_eq!(
+            ok_location.to_type_naming_segments(),
+            vec![
+                vec!["run", "Out", "response"],
+                vec!["my-owner", "result", "Ok"]
+            ]
+        );
+
+        let err_location = TypeLocation {
+            root: TypeLocationRoot::AgentMethodOutput {
+                method_name: "run".to_string(),
+                output_name: "response".to_string(),
+            },
+            path: Some(TypeLocationPath::ResultErr {
+                name: Some("result".to_string()),
+                owner: Some("my-owner".to_string()),
+                inner: None,
+            }),
+        };
+
+        assert_eq!(
+            err_location.to_type_naming_segments(),
+            vec![
+                vec!["run", "Out", "response"],
+                vec!["my-owner", "result", "Err"]
+            ]
+        );
+    }
+}

cli/golem-cli/src/config.rs

@@ -386,9 +386,9 @@ impl HttpClientConfig {
     pub fn new_for_service_calls(allow_insecure: bool) -> Self {
         Self {
             allow_insecure,
-            timeout: Some(Duration::from_secs(10)),
+            timeout: Some(Duration::from_secs(120)),
             connect_timeout: Some(Duration::from_secs(10)),
-            read_timeout: Some(Duration::from_secs(10)),
+            read_timeout: Some(Duration::from_secs(60)),
         }
         .with_env_overrides("GOLEM_HTTP")
     }
@@ -416,7 +416,7 @@ impl HttpClientConfig {
     pub fn new_for_file_download(allow_insecure: bool) -> Self {
         Self {
             allow_insecure,
-            timeout: Some(Duration::from_secs(60)),
+            timeout: Some(Duration::from_secs(120)),
             connect_timeout: Some(Duration::from_secs(10)),
             read_timeout: Some(Duration::from_secs(60)),
         }

cli/golem-cli/src/context.rs

@@ -325,6 +325,7 @@ impl Context {
             selected_context_logging: std::sync::OnceLock::new(),
             app_context_state: tokio::sync::RwLock::new(ApplicationContextState::new(
                 yes,
+                SHOULD_COLORIZE.should_colorize(),
                 app_source_mode,
             )),
             caches: Caches::new(),
@@ -698,16 +699,18 @@ impl ApplicationContextConfig {
 #[derive()]
 pub struct ApplicationContextState {
     yes: bool,
+    should_colorize: bool,
     app_source_mode: Option<ApplicationSourceMode>,
     pub silent_init: bool,
 
     app_context: Option<Result<Option<ApplicationContext>, Arc<anyhow::Error>>>,
 }
 
 impl ApplicationContextState {
-    pub fn new(yes: bool, source_mode: ApplicationSourceMode) -> Self {
+    pub fn new(yes: bool, should_colorize: bool, source_mode: ApplicationSourceMode) -> Self {
         Self {
             yes,
+            should_colorize,
             app_source_mode: Some(source_mode),
             silent_init: false,
             app_context: None,
@@ -735,6 +738,7 @@ impl ApplicationContextState {
         let app_config = ApplicationConfig {
             offline: config.wasm_rpc_client_build_offline,
             dev_mode: config.dev_mode,
+            should_colorize: self.should_colorize,
             enable_wasmtime_fs_cache: config.enable_wasmtime_fs_cache,
         };
 

cli/golem-cli/src/evcxr_repl/repl.rs

@@ -235,14 +235,6 @@ impl Repl {
     }
 
     fn run_interactive(&self, outputs: EvalContextOutputs) -> anyhow::Result<()> {
-        let mut editor = Self::build_editor(self.command_context.clone(), self.cli_repl.clone())?;
-
-        let history_path = PathBuf::from(&self.config.base_config.history_file);
-        editor.load_history(&history_path)?;
-
-        let buffered_output = BufferedEvalOutput::start(outputs.stdout, outputs.stderr);
-        let mut printer = editor.create_external_printer()?;
-
         let prompt = {
             if self.config.script_mode() {
                 "".to_string()
@@ -258,6 +250,14 @@ impl Repl {
             }
         };
 
+        let mut editor = Self::build_editor(self.command_context.clone(), self.cli_repl.clone())?;
+
+        let history_path = PathBuf::from(&self.config.base_config.history_file);
+        editor.load_history(&history_path)?;
+
+        let buffered_output = BufferedEvalOutput::start(outputs.stdout, outputs.stderr);
+        let mut printer = editor.create_external_printer()?;
+
         let mut interrupted = false;
         loop {
             let line = editor.readline(&prompt);
@@ -644,12 +644,9 @@ impl Completer for ReplRustyLineEditorHelper {
             return Ok((result.start, pairs));
         }
 
-        let completions = {
-            let _spinner = SpinnerGuard::start_stdout("Completing...", true);
-            match self.context.borrow_mut().completions(line, pos) {
-                Ok(completions) => completions,
-                Err(_) => return Ok((pos, Vec::new())),
-            }
+        let completions = match self.context.borrow_mut().completions(line, pos) {
+            Ok(completions) => completions,
+            Err(_) => return Ok((pos, Vec::new())),
         };
 
         let mut candidates = Vec::with_capacity(completions.completions.len());

cli/golem-cli/src/model/app.rs

@@ -106,6 +106,7 @@ impl BuildConfig {
 pub struct ApplicationConfig {
     pub offline: bool,
     pub dev_mode: bool,
+    pub should_colorize: bool,
     pub enable_wasmtime_fs_cache: bool,
 }