Skip to content

Commit 3a753be

Browse files
authored
Cli dx improvements (#933)
<!-- greptile_comment --> <h3>Greptile Summary</h3> This PR makes two DX improvements to the Helix CLI: it allows `helix add` to run inside a project that currently has zero instances (so users can re-add an instance after deleting the last one), and it adds a new `helix cluster indexes` subcommand that lists vector, equality, and range indexes for an Enterprise cluster. - **Zero-instance tolerance for `helix add`**: `HelixConfig::from_file` and `ProjectContext::find_and_load` are each split into a strict and a lenient variant; `helix add` now uses the lenient path, with new unit tests covering both paths. - **`helix cluster indexes` command**: A new `Indexes` subcommand (aliased `indices`) is added under `ClusterConfigAction`; it auto-resolves the cluster ID from the current project's Enterprise instance when `--cluster-id` is omitted, and supports `--format json`. - **Improved error hint**: The `MissingInstances` error now surfaces a concrete `helix add` command as a hint. <details><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | helix-cli/src/commands/config.rs | Adds `list_indexes_for_cluster` handler and `resolve_cluster_id_for_indexes` fallback; error from `find_and_load` is silently swallowed, which can hide config-parse failures behind a generic hint message. | | helix-cli/src/enterprise_cloud.rs | Adds `CliClusterIndex`/`CliClusterIndexes` structs and `fetch_indexes_for_cluster`; alias-only rename means `--format json` serializes to `index_name`/`index_type` instead of the API's `name`/`type`. | | helix-cli/src/config.rs | Cleanly refactors `from_file` into a `from_file_inner(require_instances)` gate; new tests cover both strict and lenient validation paths. | | helix-cli/src/project.rs | Adds `find_and_load_allow_no_instances` by delegating to a shared `load_with(require_instances)` helper; logic is straightforward. | | helix-cli/src/commands/add.rs | Switches to `find_and_load_allow_no_instances` so `helix add` can re-add an instance when the last one was deleted. | | helix-cli/src/lib.rs | Adds `Indexes` subcommand under `ClusterConfigAction` with `--cluster-id`/`--format` args and `indices` alias; clean. | | helix-cli/src/errors.rs | Adds actionable `with_hint` to `MissingInstances` error message; no logic changes. | | helix-cli/src/main.rs | Adds a parser test for `cluster indexes --cluster-id ent_123`; straightforward test coverage. | </details> </details> <details><summary><h3>Flowchart</h3></summary> ```mermaid %%{init: {'theme': 'neutral'}}%% flowchart TD A[helix cluster indexes] --> B{--cluster-id provided?} B -- yes --> C[trim & validate non-empty] C --> D[Use provided cluster_id] B -- no --> E[ProjectContext::find_and_load\nrequire_instances=true] E -- error --> F[Return: Provide --cluster-id hint] E -- ok --> G{enterprise instances count} G -- 0 --> H[Error: No Enterprise instances\nin helix.toml] G -- 1 --> I[Use sole instance cluster_id] G -- many & interactive --> J[prompts::select_instance] G -- many & non-interactive --> K[Error: list available instances] J --> L[Lookup cluster_id from config] I --> L D --> M[require_auth + fetch_indexes_for_cluster] L --> M M --> N{--format json?} N -- yes --> O[print_json] N -- no --> P[print_cluster_indexes\nvector / equality / range] ``` </details> <sub>Reviews (1): Last reviewed commit: ["Implement lenient configuration loading ..."](68e79ed) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=35920797)</sub> > Greptile also left **2 inline comments** on this PR. <!-- /greptile_comment -->
2 parents dd7e250 + 933af88 commit 3a753be

10 files changed

Lines changed: 266 additions & 14 deletions

File tree

Cargo.lock

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

helix-cli/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "helix-cli"
3-
version = "3.0.4"
3+
version = "3.0.5"
44
edition = "2024"
55

66
[dependencies]

helix-cli/src/commands/add.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::prompts;
99
use eyre::{Result, eyre};
1010

1111
pub async fn run(target: Option<AddTarget>) -> Result<()> {
12-
let mut project = ProjectContext::find_and_load(None)?;
12+
let mut project = ProjectContext::find_and_load_allow_no_instances(None)?;
1313
let config_path = project.root.join("helix.toml");
1414
let target = match target {
1515
Some(target) => target,

helix-cli/src/commands/config.rs

Lines changed: 109 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use crate::commands::auth::require_auth;
22
use crate::config::WorkspaceConfig;
33
use crate::enterprise_cloud::{
4-
CliEnterpriseCluster, cloud_base_url, fetch_projects, fetch_workspaces, find_project_by_id,
4+
CliClusterIndex, CliClusterIndexes, CliEnterpriseCluster, cloud_base_url,
5+
fetch_indexes_for_cluster, fetch_projects, fetch_workspaces, find_project_by_id,
56
find_project_by_name, find_workspace_by_id, find_workspace_by_slug, list_clusters_for_context,
67
resolve_enterprise_cluster,
78
};
@@ -12,7 +13,7 @@ use crate::{
1213
WorkspaceConfigAction,
1314
};
1415
use color_eyre::owo_colors::OwoColorize;
15-
use eyre::{Result, eyre};
16+
use eyre::{Result, WrapErr, eyre};
1617
use serde::Serialize;
1718

1819
pub async fn run(action: Option<ConfigAction>) -> Result<()> {
@@ -63,8 +64,13 @@ pub async fn run_cluster(action: Option<ClusterConfigAction>) -> Result<()> {
6364
project_id,
6465
format,
6566
}) => cluster_list(workspace_id, project_id, format).await,
67+
Some(ClusterConfigAction::Indexes { cluster_id, format }) => {
68+
list_indexes_for_cluster(cluster_id, format).await
69+
}
6670
None if prompts::is_interactive() => cluster_select().await,
67-
None => Err(eyre!("Specify a cluster command: 'helix cluster list'")),
71+
None => Err(eyre!(
72+
"Specify a cluster command: 'helix cluster list' or 'helix cluster indexes'"
73+
)),
6874
}
6975
}
7076

@@ -320,6 +326,106 @@ async fn cluster_list(
320326
Ok(())
321327
}
322328

329+
async fn list_indexes_for_cluster(
330+
cluster_id: Option<String>,
331+
format: ConfigOutputFormat,
332+
) -> Result<()> {
333+
let cluster_id = resolve_cluster_id_for_indexes(cluster_id)?;
334+
let credentials = require_auth().await?;
335+
let client = reqwest::Client::new();
336+
let indexes = fetch_indexes_for_cluster(
337+
&client,
338+
&cloud_base_url(),
339+
&credentials.helix_admin_key,
340+
cluster_id.as_str(),
341+
)
342+
.await?;
343+
344+
if format == ConfigOutputFormat::Json {
345+
return print_json(&indexes);
346+
}
347+
348+
print_cluster_indexes(&cluster_id, &indexes);
349+
Ok(())
350+
}
351+
352+
fn resolve_cluster_id_for_indexes(cluster_id: Option<String>) -> Result<String> {
353+
if let Some(cluster_id) = cluster_id {
354+
let cluster_id = cluster_id.trim();
355+
if !cluster_id.is_empty() {
356+
return Ok(cluster_id.to_string());
357+
}
358+
}
359+
360+
let project = ProjectContext::find_and_load(None).wrap_err(
361+
"Provide --cluster-id, or run inside a Helix project with an Enterprise instance.",
362+
)?;
363+
364+
let mut enterprise_instances = project
365+
.config
366+
.enterprise
367+
.keys()
368+
.map(|name| (name.clone(), "Enterprise".to_string()))
369+
.collect::<Vec<_>>();
370+
enterprise_instances.sort_by(|a, b| a.0.cmp(&b.0));
371+
372+
let instance_name = match enterprise_instances.len() {
373+
0 => return Err(eyre!("No Enterprise instances found in helix.toml")),
374+
1 => enterprise_instances[0].0.clone(),
375+
_ if prompts::is_interactive() => prompts::select_instance(
376+
&enterprise_instances,
377+
"List indexes for which Enterprise instance?",
378+
)?,
379+
_ => {
380+
let available = enterprise_instances
381+
.into_iter()
382+
.map(|(name, _)| name)
383+
.collect::<Vec<_>>()
384+
.join(", ");
385+
return Err(eyre!(
386+
"No Enterprise instance specified. Available Enterprise instances: {available}. Pass --cluster-id to select one."
387+
));
388+
}
389+
};
390+
391+
project
392+
.config
393+
.enterprise
394+
.get(&instance_name)
395+
.map(|config| config.cluster_id.clone())
396+
.ok_or_else(|| eyre!("Enterprise instance '{instance_name}' was not found"))
397+
}
398+
399+
fn print_cluster_indexes(cluster_id: &str, indexes: &CliClusterIndexes) {
400+
println!("{}", "Cluster indexes".bold());
401+
println!(" Cluster: {cluster_id}");
402+
print_index_group("Vector indexes", &indexes.vector_indexes);
403+
print_index_group("Equality indexes", &indexes.equality_indexes);
404+
print_index_group("Range indexes", &indexes.range_indexes);
405+
}
406+
407+
fn print_index_group(title: &str, indexes: &[CliClusterIndex]) {
408+
println!(" {title}:");
409+
if indexes.is_empty() {
410+
println!(" (none)");
411+
return;
412+
}
413+
414+
for index in indexes {
415+
let name = if index.index_name.trim().is_empty() {
416+
"<unnamed>"
417+
} else {
418+
index.index_name.as_str()
419+
};
420+
match index.index_type.as_deref() {
421+
Some(index_type) if !index_type.trim().is_empty() => {
422+
println!(" {name} ({index_type})");
423+
}
424+
_ => println!(" {name}"),
425+
}
426+
}
427+
}
428+
323429
fn print_enterprise_clusters(clusters: &[CliEnterpriseCluster]) {
324430
println!("{}", "Enterprise clusters".bold());
325431
for cluster in clusters {

helix-cli/src/config.rs

Lines changed: 69 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,17 @@ impl InstanceInfo<'_> {
342342

343343
impl HelixConfig {
344344
pub fn from_file(path: &Path) -> Result<Self, ConfigError> {
345+
Self::from_file_inner(path, true)
346+
}
347+
348+
/// Like [`from_file`](Self::from_file), but tolerates a `helix.toml` that defines zero
349+
/// instances. Used by `helix add`, whose whole job is to add the first instance back —
350+
/// it would otherwise be locked out by the "at least one instance" check.
351+
pub fn from_file_allow_no_instances(path: &Path) -> Result<Self, ConfigError> {
352+
Self::from_file_inner(path, false)
353+
}
354+
355+
fn from_file_inner(path: &Path, require_instances: bool) -> Result<Self, ConfigError> {
345356
let content = fs::read_to_string(path).map_err(|source| ConfigError::ReadHelixConfig {
346357
path: path.to_path_buf(),
347358
source,
@@ -353,7 +364,7 @@ impl HelixConfig {
353364
source,
354365
})?;
355366

356-
config.validate(path)?;
367+
config.validate(path, require_instances)?;
357368
Ok(config)
358369
}
359370

@@ -367,7 +378,7 @@ impl HelixConfig {
367378
Ok(())
368379
}
369380

370-
fn validate(&self, path: &Path) -> Result<(), ConfigError> {
381+
fn validate(&self, path: &Path, require_instances: bool) -> Result<(), ConfigError> {
371382
let relative_path = std::env::current_dir()
372383
.ok()
373384
.and_then(|cwd| path.strip_prefix(&cwd).ok())
@@ -380,7 +391,7 @@ impl HelixConfig {
380391
});
381392
}
382393

383-
if self.local.is_empty() && self.enterprise.is_empty() {
394+
if require_instances && self.local.is_empty() && self.enterprise.is_empty() {
384395
return Err(ConfigError::MissingInstances {
385396
path: relative_path,
386397
});
@@ -509,6 +520,61 @@ tag = "latest"
509520
assert_eq!(local.storage, LocalStorageMode::Memory);
510521
}
511522

523+
#[test]
524+
fn zero_instance_config_rejected_by_default_but_allowed_leniently() {
525+
let config: HelixConfig = toml::from_str(
526+
r#"
527+
[project]
528+
name = "demo"
529+
"#,
530+
)
531+
.expect("config with no instances should still deserialize");
532+
533+
let path = Path::new("helix.toml");
534+
// Default validation (used by every command except `add`) rejects it.
535+
assert!(matches!(
536+
config.validate(path, true),
537+
Err(ConfigError::MissingInstances { .. })
538+
));
539+
// Lenient validation (used by `helix add`) accepts it so the first
540+
// instance can be re-added after the last one was deleted.
541+
assert!(config.validate(path, false).is_ok());
542+
}
543+
544+
#[test]
545+
fn lenient_validation_still_enforces_other_checks() {
546+
let path = Path::new("helix.toml");
547+
548+
// Empty project name is rejected even leniently.
549+
let empty_name: HelixConfig = toml::from_str(
550+
r#"
551+
[project]
552+
name = " "
553+
"#,
554+
)
555+
.unwrap();
556+
assert!(matches!(
557+
empty_name.validate(path, false),
558+
Err(ConfigError::EmptyProjectName { .. })
559+
));
560+
561+
// Enterprise instance without a cluster_id is rejected even leniently.
562+
let no_cluster: HelixConfig = toml::from_str(
563+
r#"
564+
[project]
565+
name = "demo"
566+
567+
[enterprise.production]
568+
cluster_id = ""
569+
"#,
570+
)
571+
.unwrap();
572+
assert!(matches!(
573+
no_cluster.validate(path, false),
574+
Err(ConfigError::MissingClusterId { .. })
575+
));
576+
}
577+
512578
#[test]
513579
fn local_config_can_use_disk_storage() {
514580
let config: HelixConfig = toml::from_str(

helix-cli/src/enterprise_cloud.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use eyre::{Result, eyre};
22
use reqwest::Client;
33
use serde::{Deserialize, Serialize, de::DeserializeOwned};
4+
use std::collections::BTreeMap;
45
use std::sync::LazyLock;
56

67
const DEFAULT_CLOUD_AUTHORITY: &str = "cloud.helix-db.com";
@@ -62,6 +63,26 @@ pub struct CliWorkspaceClusters {
6263
pub enterprise: Vec<CliEnterpriseCluster>,
6364
}
6465

66+
#[derive(Debug, Clone, Serialize, Deserialize)]
67+
pub struct CliClusterIndex {
68+
#[serde(default, rename = "name", alias = "index_name")]
69+
pub index_name: String,
70+
#[serde(default, rename = "type", alias = "index_type")]
71+
pub index_type: Option<String>,
72+
#[serde(flatten)]
73+
pub extra: BTreeMap<String, serde_json::Value>,
74+
}
75+
76+
#[derive(Debug, Clone, Serialize, Deserialize)]
77+
pub struct CliClusterIndexes {
78+
#[serde(default)]
79+
pub vector_indexes: Vec<CliClusterIndex>,
80+
#[serde(default)]
81+
pub equality_indexes: Vec<CliClusterIndex>,
82+
#[serde(default)]
83+
pub range_indexes: Vec<CliClusterIndex>,
84+
}
85+
6586
#[derive(Debug, Clone, Serialize, Deserialize)]
6687
pub struct CliEnterpriseCluster {
6788
pub cluster_id: String,
@@ -258,6 +279,21 @@ pub async fn fetch_workspace_clusters(
258279
.await
259280
}
260281

282+
pub async fn fetch_indexes_for_cluster(
283+
client: &Client,
284+
base_url: &str,
285+
api_key: &str,
286+
cluster_id: &str,
287+
) -> Result<CliClusterIndexes> {
288+
get_json(
289+
client,
290+
format!("{base_url}/api/cli/enterprise-clusters/{cluster_id}/indexes"),
291+
api_key,
292+
"fetch cluster indexes",
293+
)
294+
.await
295+
}
296+
261297
pub async fn fetch_enterprise_cluster_project(
262298
client: &Client,
263299
base_url: &str,

helix-cli/src/errors.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,8 @@ impl ConfigError {
299299
ConfigError::MissingInstances { path } => CliError::new(format!(
300300
"at least one instance must be defined in {}",
301301
path.display()
302-
)),
302+
))
303+
.with_hint("add one with `helix add local --name dev` (or `helix add enterprise`)"),
303304
ConfigError::EmptyInstanceName { path } => CliError::new(format!(
304305
"instance name cannot be empty in {}",
305306
path.display()

helix-cli/src/lib.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,4 +236,14 @@ pub enum ClusterConfigAction {
236236
#[arg(long, value_enum, default_value_t = ConfigOutputFormat::Human)]
237237
format: ConfigOutputFormat,
238238
},
239+
240+
/// List indexes in an Enterprise cluster
241+
#[command(alias = "indices")]
242+
Indexes {
243+
/// Enterprise cluster ID; defaults to the current project's Enterprise instance
244+
#[arg(long, value_name = "CLUSTER_ID")]
245+
cluster_id: Option<String>,
246+
#[arg(long, value_enum, default_value_t = ConfigOutputFormat::Human)]
247+
format: ConfigOutputFormat,
248+
},
239249
}

helix-cli/src/main.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,6 +1112,22 @@ mod tests {
11121112
}
11131113
}
11141114

1115+
#[test]
1116+
fn root_cluster_indexes_command_parses() {
1117+
let cli = Cli::parse_from(["helix", "cluster", "indexes", "--cluster-id", "ent_123"]);
1118+
1119+
match cli.command {
1120+
Some(Commands::Cluster {
1121+
action:
1122+
Some(ClusterConfigAction::Indexes {
1123+
cluster_id,
1124+
format: _,
1125+
}),
1126+
}) => assert_eq!(cluster_id.as_deref(), Some("ent_123")),
1127+
_ => panic!("expected cluster indexes command"),
1128+
}
1129+
}
1130+
11151131
#[test]
11161132
fn status_accepts_optional_instance() {
11171133
let cli = Cli::parse_from(["helix", "status", "qa"]);

0 commit comments

Comments
 (0)