From 32cca56f801e52cbe8495b37f7024005ac59241c Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 23 Jun 2026 12:55:42 -0400 Subject: [PATCH 1/2] Fix generate config target fallback --- crates/cli/src/spacetime_config.rs | 14 ++ crates/cli/src/subcommands/generate.rs | 190 ++++++++++++++---- .../tests/smoketests/cli/generate.rs | 67 ++++++ 3 files changed, 229 insertions(+), 42 deletions(-) diff --git a/crates/cli/src/spacetime_config.rs b/crates/cli/src/spacetime_config.rs index b3316f2c318..b78b72b9cb0 100644 --- a/crates/cli/src/spacetime_config.rs +++ b/crates/cli/src/spacetime_config.rs @@ -391,6 +391,20 @@ impl Default for CommandSchemaBuilder { } impl CommandSchema { + /// Return only values accepted by this command schema. + /// + /// `SpacetimeConfig` stores entity-level fields like `database`, `server`, and + /// `module-path` together, while each command accepts only a subset of them. + /// This keeps command fallbacks from reimplementing schema-key filtering. + pub fn filter_config_values(&self, values: &HashMap) -> HashMap { + let valid_config_keys: HashSet = self.keys.iter().map(|k| k.config_name().to_string()).collect(); + values + .iter() + .filter(|(key, _)| valid_config_keys.contains(&key.replace('-', "_"))) + .map(|(key, value)| (key.clone(), value.clone())) + .collect() + } + /// Get a value from clap arguments only (not from config). /// Useful for filtering or checking if a value was provided via CLI. pub fn get_clap_arg( diff --git a/crates/cli/src/subcommands/generate.rs b/crates/cli/src/subcommands/generate.rs index 94686939cba..75749cf1bdc 100644 --- a/crates/cli/src/subcommands/generate.rs +++ b/crates/cli/src/subcommands/generate.rs @@ -17,7 +17,8 @@ use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use crate::spacetime_config::{ - find_and_load_with_env, CommandConfig, CommandSchema, CommandSchemaBuilder, Key, LoadedConfig, SpacetimeConfig, + find_and_load_with_env, CommandConfig, CommandSchema, CommandSchemaBuilder, FlatTarget, Key, LoadedConfig, + SpacetimeConfig, }; use crate::tasks::csharp::dotnet_format; use crate::tasks::rust::rustfmt; @@ -71,46 +72,7 @@ fn get_filtered_generate_configs<'a>( schema: &'a CommandSchema, args: &'a clap::ArgMatches, ) -> Result>, anyhow::Error> { - // Get all database targets from config with parent→child inheritance - let all_targets = spacetime_config.collect_all_targets_with_inheritance(); - - if all_targets.is_empty() { - return Ok(vec![]); - } - - // Filter by database name pattern (glob) if provided via CLI - let filtered_targets = if let Some(cli_database) = args.get_one::("database") { - let pattern = - glob::Pattern::new(cli_database).with_context(|| format!("Invalid glob pattern: {cli_database}"))?; - - let matched: Vec<_> = all_targets - .into_iter() - .filter(|target| { - target - .fields - .get("database") - .and_then(|v| v.as_str()) - .is_some_and(|db| pattern.matches(db)) - }) - .collect(); - - if matched.is_empty() { - anyhow::bail!( - "No database target matches '{}'. Available databases: {}", - cli_database, - spacetime_config - .collect_all_targets_with_inheritance() - .iter() - .filter_map(|t| t.fields.get("database").and_then(|v| v.as_str())) - .collect::>() - .join(", ") - ); - } - - matched - } else { - all_targets - }; + let filtered_targets = get_filtered_generate_targets(spacetime_config, args)?; // Collect generate entries from matched targets, inheriting entity fields // Deduplicate by (module_path, serialized_generate_entry) @@ -165,6 +127,80 @@ fn get_filtered_generate_configs<'a>( Ok(generate_configs) } +fn get_filtered_generate_targets( + spacetime_config: &SpacetimeConfig, + args: &clap::ArgMatches, +) -> Result, anyhow::Error> { + // Get all database targets from config with parent→child inheritance + let all_targets = spacetime_config.collect_all_targets_with_inheritance(); + + if all_targets.is_empty() { + return Ok(vec![]); + } + + // Filter by database name pattern (glob) if provided via CLI + if let Some(cli_database) = args.get_one::("database") { + let pattern = + glob::Pattern::new(cli_database).with_context(|| format!("Invalid glob pattern: {cli_database}"))?; + + let matched: Vec<_> = all_targets + .into_iter() + .filter(|target| { + target + .fields + .get("database") + .and_then(|v| v.as_str()) + .is_some_and(|db| pattern.matches(db)) + }) + .collect(); + + if matched.is_empty() { + anyhow::bail!( + "No database target matches '{}'. Available databases: {}", + cli_database, + spacetime_config + .collect_all_targets_with_inheritance() + .iter() + .filter_map(|t| t.fields.get("database").and_then(|v| v.as_str())) + .collect::>() + .join(", ") + ); + } + + Ok(matched) + } else { + Ok(all_targets) + } +} + +fn fallback_generate_configs_from_targets<'a>( + spacetime_config: &SpacetimeConfig, + command: &clap::Command, + schema: &'a CommandSchema, + args: &'a clap::ArgMatches, +) -> anyhow::Result>> { + let targets = get_filtered_generate_targets(spacetime_config, args)?; + let mut configs = Vec::with_capacity(targets.len()); + for target in targets { + configs.push(CommandConfig::new( + schema, + schema.filter_config_values(&target.fields), + args, + )?); + } + + schema.validate_no_generate_entry_specific_cli_args(command, args, configs.len())?; + schema.validate_no_module_specific_cli_args_for_multiple_targets( + command, + args, + configs.len(), + "generating for multiple targets", + "Please specify the database name to select a single target, or remove these arguments.", + )?; + + Ok(configs) +} + pub fn cli() -> clap::Command { clap::Command::new("generate") .about("Generate client files for a spacetime module.") @@ -629,7 +665,12 @@ pub async fn exec_ex( let (using_config, generate_configs) = if let Some(loaded) = loaded_config_ref { let filtered = get_filtered_generate_configs(&loaded.config, &cmd, &schema, args)?; if filtered.is_empty() { - (false, vec![CommandConfig::new(&schema, HashMap::new(), args)?]) + let fallback = fallback_generate_configs_from_targets(&loaded.config, &cmd, &schema, args)?; + if fallback.is_empty() { + (false, vec![CommandConfig::new(&schema, HashMap::new(), args)?]) + } else { + (true, fallback) + } } else { (true, filtered) } @@ -1357,6 +1398,71 @@ mod tests { ); } + #[test] + fn test_fallback_generate_config_uses_selected_child_target_fields() { + let cmd = cli(); + let schema = build_generate_config_schema(&cmd).unwrap(); + + let mut root_fields = HashMap::new(); + root_fields.insert("module-path".to_string(), serde_json::json!("./root-module")); + + let mut child_fields = HashMap::new(); + child_fields.insert("database".to_string(), serde_json::json!("child-db")); + child_fields.insert("module-path".to_string(), serde_json::json!("./child-module")); + + let spacetime_config = SpacetimeConfig { + additional_fields: root_fields, + children: Some(vec![SpacetimeConfig { + additional_fields: child_fields, + ..Default::default() + }]), + ..Default::default() + }; + + let matches = cmd.clone().get_matches_from(vec![ + "generate", + "child-db", + "--lang", + "rust", + "--bin-path", + "dummy.wasm", + ]); + let fallback = fallback_generate_configs_from_targets(&spacetime_config, &cmd, &schema, &matches).unwrap(); + + assert_eq!(fallback.len(), 1); + assert_eq!( + fallback[0].get_one::("module_path").unwrap(), + Some(PathBuf::from("./child-module")) + ); + } + + #[test] + fn test_fallback_generate_config_filters_unsupported_entity_fields() { + let cmd = cli(); + let schema = build_generate_config_schema(&cmd).unwrap(); + + let mut root_fields = HashMap::new(); + root_fields.insert("database".to_string(), serde_json::json!("my-db")); + root_fields.insert("server".to_string(), serde_json::json!("local")); + root_fields.insert("module-path".to_string(), serde_json::json!("./server")); + + let spacetime_config = SpacetimeConfig { + additional_fields: root_fields, + ..Default::default() + }; + + let matches = cmd + .clone() + .get_matches_from(vec!["generate", "--lang", "rust", "--bin-path", "dummy.wasm"]); + let fallback = fallback_generate_configs_from_targets(&spacetime_config, &cmd, &schema, &matches).unwrap(); + + assert_eq!(fallback.len(), 1); + assert_eq!( + fallback[0].get_one::("module_path").unwrap(), + Some(PathBuf::from("./server")) + ); + } + #[test] fn test_language_serde_deserialize_all_variants() { // Verify all Language variants deserialize correctly from config JSON strings. diff --git a/crates/smoketests/tests/smoketests/cli/generate.rs b/crates/smoketests/tests/smoketests/cli/generate.rs index 307d66c9e95..a2b50473b58 100644 --- a/crates/smoketests/tests/smoketests/cli/generate.rs +++ b/crates/smoketests/tests/smoketests/cli/generate.rs @@ -89,3 +89,70 @@ fn cli_generate_with_config_but_no_match_uses_cli_args() { "Generated files should exist in CLI-specified output directory" ); } + +#[test] +fn cli_generate_uses_root_module_path_without_generate_entries() { + let temp_dir = tempfile::tempdir().expect("failed to create temp dir"); + + let output = cli_cmd() + .args([ + "init", + "--non-interactive", + "--lang", + "rust", + "--project-path", + temp_dir.path().to_str().unwrap(), + "test-project", + ]) + .current_dir(temp_dir.path()) + .output() + .expect("failed to execute"); + assert!( + output.status.success(), + "init failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let project_dir = temp_dir.path().to_path_buf(); + let default_module_dir = project_dir.join("spacetimedb"); + let custom_module_dir = project_dir.join("server-rust"); + std::fs::rename(&default_module_dir, &custom_module_dir).expect("failed to move module dir"); + patch_module_cargo_to_local_bindings(&custom_module_dir).expect("failed to patch module Cargo.toml"); + + std::fs::write( + project_dir.join("spacetime.json"), + r#"{ + "module-path": "server-rust" +}"#, + ) + .expect("failed to write config"); + + let output = cli_cmd() + .args(["build", "--module-path", custom_module_dir.to_str().unwrap()]) + .output() + .expect("failed to execute"); + assert!( + output.status.success(), + "build failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let output_dir = project_dir.join("bindings"); + let output = cli_cmd() + .args(["generate", "--lang", "rust", "--out-dir", output_dir.to_str().unwrap()]) + .current_dir(&project_dir) + .output() + .expect("failed to execute"); + assert!( + output.status.success(), + "generate failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + + assert!( + predicate::path::exists().eval(&output_dir.join("lib.rs")) + || predicate::path::exists().eval(&output_dir.join("mod.rs")), + "generated Rust bindings should exist in configured output directory" + ); +} From 0735592392be71862af63b58a223408159e536da Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 23 Jun 2026 19:57:52 -0400 Subject: [PATCH 2/2] Add child target generate smoketests --- .../tests/smoketests/cli/generate.rs | 165 ++++++++++++++++++ 1 file changed, 165 insertions(+) diff --git a/crates/smoketests/tests/smoketests/cli/generate.rs b/crates/smoketests/tests/smoketests/cli/generate.rs index a2b50473b58..0d19bd7c90d 100644 --- a/crates/smoketests/tests/smoketests/cli/generate.rs +++ b/crates/smoketests/tests/smoketests/cli/generate.rs @@ -156,3 +156,168 @@ fn cli_generate_uses_root_module_path_without_generate_entries() { "generated Rust bindings should exist in configured output directory" ); } + +#[test] +fn cli_generate_uses_selected_child_module_path_without_generate_entries() { + let temp_dir = tempfile::tempdir().expect("failed to create temp dir"); + + let output = cli_cmd() + .args([ + "init", + "--non-interactive", + "--lang", + "rust", + "--project-path", + temp_dir.path().to_str().unwrap(), + "test-project", + ]) + .current_dir(temp_dir.path()) + .output() + .expect("failed to execute"); + assert!( + output.status.success(), + "init failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let project_dir = temp_dir.path().to_path_buf(); + let default_module_dir = project_dir.join("spacetimedb"); + let child_module_dir = project_dir.join("child-server"); + std::fs::rename(&default_module_dir, &child_module_dir).expect("failed to move module dir"); + patch_module_cargo_to_local_bindings(&child_module_dir).expect("failed to patch module Cargo.toml"); + + std::fs::write( + project_dir.join("spacetime.json"), + r#"{ + "module-path": "missing-root-server", + "children": [ + { + "database": "child-db", + "module-path": "child-server" + } + ] +}"#, + ) + .expect("failed to write config"); + + let output = cli_cmd() + .args(["build", "--module-path", child_module_dir.to_str().unwrap()]) + .output() + .expect("failed to execute"); + assert!( + output.status.success(), + "build failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let output_dir = project_dir.join("child-bindings"); + let output = cli_cmd() + .args([ + "generate", + "child-db", + "--lang", + "rust", + "--out-dir", + output_dir.to_str().unwrap(), + ]) + .current_dir(&project_dir) + .output() + .expect("failed to execute"); + assert!( + output.status.success(), + "generate failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + + assert!( + predicate::path::exists().eval(&output_dir.join("lib.rs")) + || predicate::path::exists().eval(&output_dir.join("mod.rs")), + "generated Rust bindings should exist in child target output directory" + ); +} + +#[test] +fn cli_generate_uses_inherited_grandchild_module_path_without_generate_entries() { + let temp_dir = tempfile::tempdir().expect("failed to create temp dir"); + + let output = cli_cmd() + .args([ + "init", + "--non-interactive", + "--lang", + "rust", + "--project-path", + temp_dir.path().to_str().unwrap(), + "test-project", + ]) + .current_dir(temp_dir.path()) + .output() + .expect("failed to execute"); + assert!( + output.status.success(), + "init failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let project_dir = temp_dir.path().to_path_buf(); + let default_module_dir = project_dir.join("spacetimedb"); + let regional_module_dir = project_dir.join("regional-server"); + std::fs::rename(&default_module_dir, ®ional_module_dir).expect("failed to move module dir"); + patch_module_cargo_to_local_bindings(®ional_module_dir).expect("failed to patch module Cargo.toml"); + + std::fs::write( + project_dir.join("spacetime.json"), + r#"{ + "module-path": "missing-root-server", + "children": [ + { + "database": "region", + "module-path": "regional-server", + "children": [ + { + "database": "region-leaf" + } + ] + } + ] +}"#, + ) + .expect("failed to write config"); + + let output = cli_cmd() + .args(["build", "--module-path", regional_module_dir.to_str().unwrap()]) + .output() + .expect("failed to execute"); + assert!( + output.status.success(), + "build failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let output_dir = project_dir.join("grandchild-bindings"); + let output = cli_cmd() + .args([ + "generate", + "region-leaf", + "--lang", + "rust", + "--out-dir", + output_dir.to_str().unwrap(), + ]) + .current_dir(&project_dir) + .output() + .expect("failed to execute"); + assert!( + output.status.success(), + "generate failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + + assert!( + predicate::path::exists().eval(&output_dir.join("lib.rs")) + || predicate::path::exists().eval(&output_dir.join("mod.rs")), + "generated Rust bindings should exist in grandchild target output directory" + ); +}