Skip to content

Commit b2a1330

Browse files
authored
fix (sync): fix sync reconciliation for remote queries paths (#882)
<!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes sync reconciliation so that the correct remote queries path (read from the cluster's snapshot `helix.toml`) is used instead of always falling back to the local project path. It refactors several sync functions to accept explicit `current_queries_dir` / `target_queries_dir` arguments, adds enterprise-cluster parity to all sync flows, extracts a reusable `build_standard_deploy_payload` helper (now including `runtime_config`), and adds meaningful unit-test coverage for the new merge/extraction helpers. **Key issues:** - `pull_remote_snapshot_into_local` handles directory migration when the remote queries path differs from the local one, but the migration code path deletes files from `current_queries_dir` **before** writing to `target_queries_dir`, risking data loss if a write fails mid-way (the same-dir code path retains the safe write-then-delete order from the original implementation). - `insert_unique_cloud_instance_name` and `insert_unique_enterprise_instance_name` introduce redundant double-clones that can be simplified. <details><summary><h3>Sequence Diagram</h3></summary> ```mermaid sequenceDiagram participant CLI participant Cloud as Helix Cloud API participant FS as Local Filesystem Note over CLI: sync (project flow) CLI->>Cloud: fetch_project_clusters() CLI->>Cloud: fetch_standard/enterprise_cluster_snapshot_config(selected_cluster) Cloud-->>CLI: HelixConfig (remote helix.toml) CLI->>CLI: resolve_selected_project_queries_path(snapshot) alt Standard cluster CLI->>Cloud: fetch_sync_response(cluster_id) Cloud-->>CLI: SyncResponse (.hx files + helix_toml) CLI->>CLI: compare_manifests(local, remote) alt Pull CLI->>FS: pull_remote_snapshot_into_local(current_dir, target_dir, ...) CLI->>FS: update_project_queries_path_in_helix_toml() else Push CLI->>Cloud: push_local_snapshot_to_cluster() end else Enterprise cluster CLI->>Cloud: fetch_enterprise_sync_response(cluster_id) Cloud-->>CLI: EnterpriseSyncResponse (.rs files + helix_toml) CLI->>FS: write .rs files to target_queries_dir CLI->>FS: update_project_queries_path_in_helix_toml() end CLI->>Cloud: fetch_project_clusters_for_[standard|enterprise]_cluster() Cloud-->>CLI: CliProjectClusters CLI->>Cloud: fetch_[standard|enterprise]_cluster_snapshot_configs(all clusters) Cloud-->>CLI: snapshot HelixConfigs CLI->>FS: reconcile_project_config_from_cloud() → write helix.toml ``` </details> <sub>Last reviewed commit: c6b0d52</sub> > Greptile also left **1 inline comment** on this PR. <!-- /greptile_comment -->
2 parents 75fb8f9 + a243265 commit b2a1330

File tree

6 files changed

+1112
-288
lines changed

6 files changed

+1112
-288
lines changed

helix-cli/src/commands/integrations/helix.rs

Lines changed: 88 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,34 @@ pub struct HelixManager<'a> {
4242
project: &'a ProjectContext,
4343
}
4444

45+
fn build_standard_deploy_payload(
46+
schema_content: String,
47+
queries_map: HashMap<String, String>,
48+
cluster_name: &str,
49+
cluster_info: &CloudInstanceConfig,
50+
helix_toml_content: Option<String>,
51+
build_mode_override: Option<String>,
52+
) -> Result<serde_json::Value> {
53+
let build_mode = match cluster_info.build_mode {
54+
BuildMode::Dev => "dev",
55+
BuildMode::Release => "release",
56+
BuildMode::Debug => {
57+
return Err(eyre!("debug build mode is not supported for cloud deploys"));
58+
}
59+
};
60+
61+
Ok(json!({
62+
"schema": schema_content,
63+
"queries": queries_map,
64+
"env_vars": cluster_info.env_vars.clone(),
65+
"runtime_config": cluster_info.runtime_config(),
66+
"build_mode": build_mode,
67+
"instance_name": cluster_name,
68+
"helix_toml": helix_toml_content,
69+
"build_mode_override": build_mode_override,
70+
}))
71+
}
72+
4573
impl<'a> HelixManager<'a> {
4674
pub fn new(project: &'a ProjectContext) -> Self {
4775
Self { project }
@@ -58,7 +86,7 @@ impl<'a> HelixManager<'a> {
5886
let cluster_id = "YOUR_CLUSTER_ID".to_string();
5987

6088
// Use provided region or default to us-east-1
61-
let region = region.or_else(|| Some("us-east-1".to_string()));
89+
let region = region.or(Some("us-east-1".to_string()));
6290

6391
Ok(CloudInstanceConfig {
6492
cluster_id,
@@ -140,7 +168,7 @@ impl<'a> HelixManager<'a> {
140168
build_mode_override: Option<BuildMode>,
141169
) -> Result<()> {
142170
let credentials = require_auth().await?;
143-
let path = match get_path_or_cwd(path.as_ref()) {
171+
let path = match get_path_or_cwd(path.as_deref()) {
144172
Ok(path) => path,
145173
Err(e) => {
146174
return Err(eyre!("Error: failed to get path: {e}"));
@@ -241,22 +269,22 @@ impl<'a> HelixManager<'a> {
241269
// Prepare deployment payload
242270
let build_mode_override = build_mode_override
243271
.map(|mode| match mode {
244-
BuildMode::Dev => Ok("dev"),
245-
BuildMode::Release => Ok("release"),
272+
BuildMode::Dev => Ok("dev".to_string()),
273+
BuildMode::Release => Ok("release".to_string()),
246274
BuildMode::Debug => {
247275
Err(eyre!("debug build mode is not supported for cloud deploys"))
248276
}
249277
})
250278
.transpose()?;
251279

252-
let payload = json!({
253-
"schema": schema_content,
254-
"queries": queries_map,
255-
"env_vars": cluster_info.env_vars,
256-
"instance_name": cluster_name,
257-
"helix_toml": helix_toml_content,
258-
"build_mode_override": build_mode_override
259-
});
280+
let payload = build_standard_deploy_payload(
281+
schema_content,
282+
queries_map,
283+
&cluster_name,
284+
cluster_info,
285+
helix_toml_content,
286+
build_mode_override,
287+
)?;
260288

261289
// Initiate deployment with SSE streaming
262290
let client = reqwest::Client::new();
@@ -525,7 +553,7 @@ impl<'a> HelixManager<'a> {
525553
config: &crate::config::EnterpriseInstanceConfig,
526554
) -> Result<()> {
527555
let credentials = require_auth().await?;
528-
let path = match get_path_or_cwd(path.as_ref()) {
556+
let path = match get_path_or_cwd(path.as_deref()) {
529557
Ok(path) => path,
530558
Err(e) => {
531559
return Err(eyre!("Error: failed to get path: {e}"));
@@ -540,8 +568,7 @@ impl<'a> HelixManager<'a> {
540568
for entry in std::fs::read_dir(&queries_dir)? {
541569
let entry = entry?;
542570
let file_path = entry.path();
543-
if file_path.is_file() && file_path.extension().map(|e| e == "rs").unwrap_or(false)
544-
{
571+
if file_path.is_file() && file_path.extension().is_some_and(|e| e == "rs") {
545572
let filename = file_path.file_name().unwrap().to_string_lossy().to_string();
546573
let content = std::fs::read_to_string(&file_path)
547574
.map_err(|e| eyre!("Failed to read {}: {}", filename, e))?;
@@ -682,9 +709,54 @@ impl<'a> HelixManager<'a> {
682709
}
683710

684711
/// Returns the path or the current working directory if no path is provided
685-
pub fn get_path_or_cwd(path: Option<&String>) -> Result<PathBuf> {
712+
pub fn get_path_or_cwd(path: Option<&str>) -> Result<PathBuf> {
686713
match path {
687714
Some(p) => Ok(PathBuf::from(p)),
688715
None => Ok(env::current_dir()?),
689716
}
690717
}
718+
719+
#[cfg(test)]
720+
mod tests {
721+
use super::*;
722+
723+
#[test]
724+
fn build_standard_deploy_payload_includes_runtime_config_and_build_mode() {
725+
let mut queries = HashMap::new();
726+
queries.insert("search.hx".to_string(), "GetUsers {}".to_string());
727+
728+
let mut env_vars = HashMap::new();
729+
env_vars.insert("OPENAI_API_KEY".to_string(), "key".to_string());
730+
731+
let config = CloudInstanceConfig {
732+
cluster_id: "cluster-123".to_string(),
733+
region: Some("us-east-1".to_string()),
734+
build_mode: BuildMode::Release,
735+
env_vars,
736+
db_config: DbConfig {
737+
vector_config: crate::config::VectorConfig {
738+
db_max_size_gb: 42,
739+
..Default::default()
740+
},
741+
..Default::default()
742+
},
743+
};
744+
745+
let payload = build_standard_deploy_payload(
746+
"schema.hx".to_string(),
747+
queries,
748+
"prod",
749+
&config,
750+
Some("[project]\nname = \"demo\"\n".to_string()),
751+
Some("dev".to_string()),
752+
)
753+
.expect("payload should serialize");
754+
755+
assert_eq!(payload["build_mode"], "release");
756+
assert_eq!(payload["build_mode_override"], "dev");
757+
assert_eq!(payload["instance_name"], "prod");
758+
assert_eq!(payload["env_vars"]["OPENAI_API_KEY"], "key");
759+
assert_eq!(payload["runtime_config"]["db_max_size_gb"], 42);
760+
assert!(payload["helix_toml"].is_string());
761+
}
762+
}

0 commit comments

Comments
 (0)