diff --git a/README.md b/README.md index fc7c1989..0d33a51f 100644 --- a/README.md +++ b/README.md @@ -282,6 +282,8 @@ When a project-scoped `server stop` omits the name, it selects an existing `defa An omitted `server remove` is deliberately more conservative: it removes only an existing `default` server. It never infers a custom name, even when there is only one. If `default` does not exist, the command reports whether custom ClickHouse servers are available and directs you to `server list` before you pass a name explicitly. +Project-scoped `server list`, `stop`, and `remove` use `.clickhouse` under the canonical current directory only. They do not search parent directories, including when the current directory is reached through a symlink or has its own nested `.clickhouse`. Lookup and state errors print that canonical project directory, direct you to change to the intended project directory for stopped servers, and suggest `clickhousectl local server list --global` for finding running servers across projects. + **Server naming:** Without a name, the first server is called "default". If "default" is already running, a random name is generated (e.g. "bold-crane"). Pass a name positionally for stable identities you can start/stop repeatedly. **Ports:** Defaults are HTTP 8123 and TCP 9000. If these are already in use, free ports are automatically assigned and shown in the output. Use `--http-port` and `--tcp-port` to set explicit ports. @@ -924,13 +926,14 @@ When a dispatched `local` command fails in explicit `--json` or detected-agent m { "error": { "code": "server_not_found", - "message": "Server 'missing' not found", - "command": "clickhousectl local server list" + "message": "Server 'missing' not found\nProject directory used for lookup: \"/work/app\"\nOnly this exact directory's `.clickhouse` is searched; parent `.clickhouse` directories are not searched.\nRun `clickhousectl local server list --global` to find running servers. For stopped servers, change to the intended project directory and run `clickhousectl local server list`.", + "command": "clickhousectl local server list --global", + "project": "/work/app" } } ``` -`code` is a stable, bounded value. `command` is fixed CLI guidance and never contains user input. Opaque download, filesystem, startup, and fallback diagnostics are not copied into the JSON message. +`code` is a stable, bounded value. `command` is fixed CLI guidance and never contains user input. Project-scoped server lookup and state errors also include the canonical `project` directory; that path is emitted only in command output and is never added to telemetry. Opaque download, filesystem, startup, and fallback diagnostics are not copied into the JSON message. | Local runtime code | Meaning | | ---------------------- | -------------------------------------------- | diff --git a/crates/clickhousectl/src/error.rs b/crates/clickhousectl/src/error.rs index 05ad2ec1..bcd01231 100644 --- a/crates/clickhousectl/src/error.rs +++ b/crates/clickhousectl/src/error.rs @@ -202,16 +202,26 @@ pub enum Error { DefaultServerNotFoundForRemove, #[error( - "No removable 'default' ClickHouse server exists. Run `clickhousectl local server list`, then pass a custom server name explicitly with `clickhousectl local server remove `." + "No removable 'default' ClickHouse server exists. Run `clickhousectl local server list`, then retry with an explicit custom server name." )] ServerNameRequiredForRemove, #[error("Server '{0}' is already running")] ServerAlreadyRunning(String), - #[error("Server '{0}' is running; stop it first with `clickhousectl local server stop {0}`")] + #[error( + "Server '{0}' is running and cannot be removed. Run `clickhousectl local server list`, then stop it by name before retrying." + )] ServerRunningCannotRemove(String), + #[error("{message}")] + ProjectServerScope { + message: String, + project: String, + #[source] + source: Box, + }, + #[error("{0}")] Cloud(String), @@ -275,6 +285,55 @@ impl Error { _ => 1, } } + + pub fn with_project_server_scope(self, project: String) -> Self { + if !matches!( + &self, + Error::ServerNotFound(_) + | Error::ServerNotRunning(_) + | Error::ServerMetadataRead { .. } + | Error::ServerMetadataPermission { .. } + | Error::ServerMetadataParse { .. } + | Error::ServerMetadataWrite { .. } + | Error::ServerNameRequiredForStop + | Error::DefaultServerNotFoundForRemove + | Error::ServerNameRequiredForRemove + | Error::ServerRunningCannotRemove(_) + ) { + return self; + } + + self.into_project_server_scope(project) + } + + pub fn with_project_server_list_scope(self, project: String) -> Self { + if matches!(&self, Error::Io(_)) { + return self.into_project_server_scope(project); + } + + self.with_project_server_scope(project) + } + + fn into_project_server_scope(self, project: String) -> Self { + let message = Self::project_server_scope_message(&self.to_string(), &project); + Error::ProjectServerScope { + message, + project, + source: Box::new(self), + } + } + + pub(crate) fn project_server_scope_message(message: &str, project: &str) -> String { + format!( + "{}\nProject directory used for lookup: {project:?}\n\ + Only this exact directory's `.clickhouse` is searched; parent `.clickhouse` \ + directories are not searched.\n\ + Run `clickhousectl local server list --global` to find running servers. For stopped \ + servers, change to the intended project directory and run \ + `clickhousectl local server list`.", + message + ) + } } #[cfg(test)] @@ -332,6 +391,48 @@ mod tests { assert_eq!(Error::AuthRequired("nope".into()).exit_code(), 4); } + #[test] + fn project_server_scope_wraps_only_lookup_and_state_errors() { + let project = "/tmp/project".to_string(); + let error = + Error::ServerNotFound("default".into()).with_project_server_scope(project.clone()); + + assert!(matches!( + error, + Error::ProjectServerScope { + ref project, + ref source, + .. + } if project == "/tmp/project" && matches!(source.as_ref(), Error::ServerNotFound(_)) + )); + assert_eq!( + Error::InvalidServerName("../private".into()) + .with_project_server_scope(project) + .to_string(), + "Invalid server name '../private': must not contain path separators or '..'" + ); + } + + #[test] + fn server_list_scopes_io_without_widening_other_project_commands() { + let project = "/tmp/project".to_string(); + let unscoped = Error::Io(std::io::Error::other("read failed")) + .with_project_server_scope(project.clone()); + assert!(matches!(unscoped, Error::Io(_))); + + let scoped = + Error::Io(std::io::Error::other("read failed")).with_project_server_list_scope(project); + assert!(matches!( + scoped, + Error::ProjectServerScope { + ref project, + ref source, + .. + } if project == "/tmp/project" + && matches!(source.as_ref(), Error::Io(source) if source.to_string() == "read failed") + )); + } + #[test] fn typed_local_boundaries_preserve_human_error_text() { assert_eq!( diff --git a/crates/clickhousectl/src/init.rs b/crates/clickhousectl/src/init.rs index 248de5ea..9d215cf2 100644 --- a/crates/clickhousectl/src/init.rs +++ b/crates/clickhousectl/src/init.rs @@ -7,6 +7,10 @@ pub fn local_dir() -> PathBuf { .join(".clickhouse") } +pub fn canonical_project_dir() -> Result { + Ok(std::env::current_dir()?.canonicalize()?) +} + pub fn project_dir() -> PathBuf { std::env::current_dir() .expect("failed to get current directory") diff --git a/crates/clickhousectl/src/local/cli.rs b/crates/clickhousectl/src/local/cli.rs index add16f9f..a0c7d4c6 100644 --- a/crates/clickhousectl/src/local/cli.rs +++ b/crates/clickhousectl/src/local/cli.rs @@ -195,6 +195,8 @@ CONTEXT FOR AGENTS: Manage named local server instances. Project-scoped `server list` and `server stop-all` include both ClickHouse processes and Docker-backed Postgres containers; other commands here manage ClickHouse. + Project-scoped lookup uses only the canonical current directory; parent `.clickhouse` + directories are not searched. Each server has its own data directory. Data is stored in .clickhouse/servers//data/ and persists between restarts. Typical: `clickhousectl local server start` (starts \"default\"), `clickhousectl local server start test`. @@ -301,6 +303,8 @@ CONTEXT FOR AGENTS: #[command(after_help = "\ CONTEXT FOR AGENTS: Shows all named ClickHouse server instances and their status. + The default view reads only the canonical current directory's `.clickhouse`; it does not + search parent project directories. Use `server list --global` to find running servers elsewhere. Processes that exited unexpectedly are retained and shown as stopped. Running ClickHouse entries also show their PID, version, and ports. Related: `clickhousectl local server start` to start a server, `clickhousectl local server stop [name]` to stop one.")] @@ -319,6 +323,10 @@ CONTEXT FOR AGENTS: Use `clickhousectl local server list` to find names. The server's data and metadata are preserved so it remains visible in `server list`. Restart with `clickhousectl local server start `. + Idempotent: a server that exists but is already stopped exits 0 (no error). + An unknown server name still errors so typos are caught. + Project lookup uses only the canonical current directory; parent `.clickhouse` directories + are not searched. Use `server list --global` to find running servers in other projects. Related: `clickhousectl local server list` to see servers.")] Stop { /// Server name; omitted selection prefers "default", then a sole known ClickHouse server @@ -365,6 +373,8 @@ CONTEXT FOR AGENTS: This is irreversible — all data for this server instance will be lost. Without a name, removes \"default\" only when it exists. It never guesses a custom server; use `server list`, then pass a custom name positionally. + Project lookup uses only the canonical current directory; parent `.clickhouse` directories + are not searched. Change to the intended project directory before retrying. Related: `clickhousectl local server stop [name]` to stop first, `clickhousectl local server list` to see servers.")] Remove { /// Server name; when omitted, only an existing "default" is removed diff --git a/crates/clickhousectl/src/local/mod.rs b/crates/clickhousectl/src/local/mod.rs index 79ef6a19..8502bbb0 100644 --- a/crates/clickhousectl/src/local/mod.rs +++ b/crates/clickhousectl/src/local/mod.rs @@ -738,7 +738,9 @@ async fn run_server_commands(command: ServerCommands, json: bool) -> Result<()> if global { list_servers_global(json) } else { + let project = canonical_project_dir()?; list_servers_local(json) + .map_err(|error| error.with_project_server_list_scope(project)) } } ServerCommands::Stop { @@ -752,7 +754,9 @@ async fn run_server_commands(command: ServerCommands, json: bool) -> Result<()> let name = name.unwrap_or_else(|| "default".to_string()); stop_server_global(&name, project.as_deref(), json) } else { + let project = canonical_project_dir()?; stop_server_local(name, json) + .map_err(|error| error.with_project_server_scope(project)) } } ServerCommands::StopAll { global } => { @@ -769,10 +773,18 @@ async fn run_server_commands(command: ServerCommands, json: bool) -> Result<()> password, database, } => dotenv_server(name.as_deref(), local, user, password, database, json), - ServerCommands::Remove { name, name_flag } => remove_server_local(name.or(name_flag), json), + ServerCommands::Remove { name, name_flag } => { + let project = canonical_project_dir()?; + remove_server_local(name.or(name_flag), json) + .map_err(|error| error.with_project_server_scope(project)) + } } } +fn canonical_project_dir() -> Result { + Ok(init::canonical_project_dir()?.display().to_string()) +} + fn stop_server_local(name: Option, json: bool) -> Result<()> { let name = match name { Some(name) => name, diff --git a/crates/clickhousectl/src/local/output.rs b/crates/clickhousectl/src/local/output.rs index 56b04ed5..c6c8e5ec 100644 --- a/crates/clickhousectl/src/local/output.rs +++ b/crates/clickhousectl/src/local/output.rs @@ -41,10 +41,24 @@ pub struct LocalErrorBody { pub code: LocalErrorCode, pub message: String, pub command: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + pub project: Option, } impl LocalErrorOutput { pub fn from_error(error: &Error) -> Self { + if let Error::ProjectServerScope { + project, source, .. + } = error + { + let mut output = Self::from_error(source); + output.error.message = + Error::project_server_scope_message(&output.error.message, project); + output.error.command = "clickhousectl local server list --global"; + output.error.project = Some(project.clone()); + return output; + } + let error = match error { Error::PostgresStartupRollback { primary, .. } if matches!( @@ -179,6 +193,7 @@ impl LocalErrorOutput { code, message, command, + project: None, }, } } @@ -926,6 +941,54 @@ mod tests { } } + #[test] + fn project_scoped_errors_preserve_codes_and_expose_the_lookup_directory() { + let output = LocalErrorOutput::from_error( + &Error::ServerRunningCannotRemove("default".into()) + .with_project_server_scope("/tmp/project".into()), + ); + + assert_eq!(output.error.code, LocalErrorCode::ServerRunning); + assert_eq!(output.error.project.as_deref(), Some("/tmp/project")); + assert_eq!( + output.error.command, + "clickhousectl local server list --global" + ); + assert!( + output + .error + .message + .contains("parent `.clickhouse` directories are not searched") + ); + } + + #[test] + fn project_scoped_io_errors_keep_structured_diagnostics_opaque() { + let output = LocalErrorOutput::from_error( + &Error::Io(std::io::Error::other( + "/secret/raw/path: filesystem diagnostics", + )) + .with_project_server_list_scope("/tmp/project".into()), + ); + + assert_eq!(output.error.code, LocalErrorCode::IoError); + assert_eq!(output.error.project.as_deref(), Some("/tmp/project")); + assert!( + output.error.message.starts_with( + "Local filesystem operation failed\nProject directory used for lookup:" + ) + ); + assert!( + output + .error + .message + .contains("parent `.clickhouse` directories are not searched") + ); + assert!(!output.error.message.contains("/secret/raw/path")); + assert!(!output.error.message.contains("filesystem diagnostics")); + assert!(!output.error.message.contains("IO error:")); + } + #[test] fn engine_specific_port_errors_share_exact_structured_output() { for (error, message) in [ diff --git a/crates/clickhousectl/tests/local_server_metadata_test.rs b/crates/clickhousectl/tests/local_server_metadata_test.rs index 3ae570d1..891d8807 100644 --- a/crates/clickhousectl/tests/local_server_metadata_test.rs +++ b/crates/clickhousectl/tests/local_server_metadata_test.rs @@ -57,7 +57,7 @@ fn valid_metadata(project: &Path) -> Vec { .unwrap() } -fn assert_json_error(output: &Output, code: &str, message_fragment: &str) { +fn assert_json_error(output: &Output, project: &Path, code: &str, message_fragment: &str) { assert_eq!( output.status.code(), Some(1), @@ -68,7 +68,12 @@ fn assert_json_error(output: &Output, code: &str, message_fragment: &str) { assert!(output.stdout.is_empty()); let body: Value = serde_json::from_slice(&output.stderr).expect("parse structured error"); assert_eq!(body["error"]["code"], code); - assert_eq!(body["error"]["command"], "clickhousectl local server list"); + let project = project.canonicalize().expect("canonical project"); + assert_eq!( + body["error"]["command"], + "clickhousectl local server list --global" + ); + assert_eq!(body["error"]["project"], project.display().to_string()); assert!( body["error"]["message"] .as_str() @@ -76,6 +81,13 @@ fn assert_json_error(output: &Output, code: &str, message_fragment: &str) { .contains(message_fragment), "{body}" ); + assert!( + body["error"]["message"] + .as_str() + .unwrap() + .contains("parent `.clickhouse` directories are not searched"), + "{body}" + ); } fn assert_lock_error(output: &Output, operation: &str, path: &Path) { @@ -157,6 +169,7 @@ fn selected_partial_json_and_invalid_utf8_are_parse_errors() { let json = run(project.path(), home.path(), true); assert_json_error( &json, + project.path(), "server_metadata_invalid", "Metadata for server 'default'", ); @@ -182,6 +195,7 @@ fn selected_metadata_read_failure_is_not_reported_as_stopped() { let json = run(project.path(), home.path(), true); assert_json_error( &json, + project.path(), "server_metadata_read", "Could not read metadata for server 'default'", ); @@ -209,6 +223,7 @@ fn selected_metadata_permission_failure_has_its_own_action() { assert_json_error( &json, + project.path(), "server_metadata_permission", "Permission denied accessing metadata for server 'default'", ); diff --git a/crates/clickhousectl/tests/local_server_project_scope_test.rs b/crates/clickhousectl/tests/local_server_project_scope_test.rs new file mode 100644 index 00000000..c2416aaf --- /dev/null +++ b/crates/clickhousectl/tests/local_server_project_scope_test.rs @@ -0,0 +1,321 @@ +//! Exact project-scope diagnostics for local server state (issue #477). + +use serde_json::Value; +use std::os::unix::fs::{PermissionsExt, symlink}; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +const VERSION: &str = "25.12.9.61"; + +fn clickhousectl_binary() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_clickhousectl")) +} + +fn run(project: &Path, home: &Path, args: &[&str]) -> Output { + Command::new(clickhousectl_binary()) + .env_clear() + .env("DO_NOT_TRACK", "1") + .env("HOME", home) + .current_dir(project) + .args(args) + .output() + .expect("run clickhousectl") +} + +fn install_fake_clickhouse(home: &Path) { + let binary = home + .join(".clickhouse/versions") + .join(VERSION) + .join("clickhouse"); + std::fs::create_dir_all(binary.parent().unwrap()).expect("create fake version directory"); + std::fs::write( + &binary, + b"#!/bin/sh\ntrap 'exit 0' TERM INT\nwhile :; do sleep 1; done\n", + ) + .expect("write fake ClickHouse"); + let mut permissions = std::fs::metadata(&binary).unwrap().permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(binary, permissions).expect("make fake ClickHouse executable"); +} + +fn unused_port() -> String { + std::net::TcpListener::bind(("127.0.0.1", 0)) + .expect("bind temporary port") + .local_addr() + .unwrap() + .port() + .to_string() +} + +fn start_fake_server(project: &Path, home: &Path, name: &str) -> u32 { + let output = run( + project, + home, + &[ + "local", + "--json", + "server", + "start", + name, + "--version", + VERSION, + "--http-port", + &unused_port(), + "--tcp-port", + &unused_port(), + "--no-wait", + ], + ); + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + serde_json::from_slice::(&output.stdout).unwrap()["pid"] + .as_u64() + .expect("server pid") as u32 +} + +fn create_stopped_server(project: &Path, name: &str) { + let servers = project.join(".clickhouse/servers"); + std::fs::create_dir_all(servers.join(name).join("data")) + .expect("create stopped server data directory"); + std::fs::write( + servers.join(format!("{name}.json")), + serde_json::to_vec(&serde_json::json!({ + "name": name, + "pid": 0, + "version": "", + "http_port": 0, + "tcp_port": 0, + "started_at": "1700000000", + "cwd": project.display().to_string(), + "engine": "clickhouse" + })) + .unwrap(), + ) + .expect("write stopped server metadata"); +} + +fn create_invalid_servers_directory(project: &Path) { + std::fs::create_dir_all(project.join(".clickhouse")) + .expect("create project metadata directory"); + std::fs::write(project.join(".clickhouse/servers"), b"not a directory") + .expect("create invalid servers directory"); +} + +fn assert_scoped_json_error(output: &Output, code: &str, project: &Path) -> Value { + assert_eq!( + output.status.code(), + Some(1), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(output.stdout.is_empty()); + let body: Value = serde_json::from_slice(&output.stderr).expect("parse structured error"); + let project = project.canonicalize().expect("canonical project"); + assert_eq!(body["error"]["code"], code); + assert_eq!(body["error"]["project"], project.display().to_string()); + assert_eq!( + body["error"]["command"], + "clickhousectl local server list --global" + ); + let message = body["error"]["message"].as_str().unwrap(); + assert!( + message.contains(&project.display().to_string()), + "{message}" + ); + assert!( + message.contains("parent `.clickhouse` directories are not searched"), + "{message}" + ); + assert!( + message.contains("`clickhousectl local server list --global`"), + "{message}" + ); + body +} + +fn assert_scoped_human_error(output: &Output, project: &Path) { + assert_eq!(output.status.code(), Some(1)); + assert!(output.stdout.is_empty()); + let project = project.canonicalize().expect("canonical project"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.starts_with("Error: "), "{stderr}"); + assert!(stderr.contains(&project.display().to_string()), "{stderr}"); + assert!( + stderr.contains("parent `.clickhouse` directories are not searched"), + "{stderr}" + ); + assert!( + stderr.contains("`clickhousectl local server list --global`"), + "{stderr}" + ); +} + +struct ProcessGuard { + pid: u32, + active: bool, +} + +impl ProcessGuard { + fn new(pid: u32) -> Self { + Self { pid, active: true } + } + + fn disarm(&mut self) { + self.active = false; + } +} + +impl Drop for ProcessGuard { + fn drop(&mut self) { + if self.active { + unsafe { + libc::kill(self.pid as i32, libc::SIGKILL); + } + } + } +} + +#[test] +fn root_child_and_nested_state_use_only_the_exact_project() { + let workspace = tempfile::tempdir().expect("create workspace tempdir"); + let home = tempfile::tempdir().expect("create home tempdir"); + let root = workspace.path().join("project"); + let child = root.join("child"); + std::fs::create_dir_all(&child).expect("create project child"); + install_fake_clickhouse(home.path()); + + let pid = start_fake_server(&root, home.path(), "parent-running"); + let mut process = ProcessGuard::new(pid); + create_stopped_server(&child, "nested-stopped"); + + let list = run(&child, home.path(), &["local", "--json", "server", "list"]); + assert!(list.status.success()); + let body: Value = serde_json::from_slice(&list.stdout).expect("parse list output"); + assert_eq!(body["total_servers"], 1); + assert_eq!(body["servers"][0]["name"], "nested-stopped"); + assert_eq!(body["servers"][0]["running"], false); + + let child_stop = run( + &child, + home.path(), + &["local", "--json", "server", "stop", "parent-running"], + ); + assert_scoped_json_error(&child_stop, "server_not_found", &child); + + let child_remove = run( + &child, + home.path(), + &["local", "server", "remove", "parent-running"], + ); + assert_scoped_human_error(&child_remove, &child); + + let running_remove = run( + &root, + home.path(), + &["local", "--json", "server", "remove", "parent-running"], + ); + assert_scoped_json_error(&running_remove, "server_running", &root); + + let stop = run( + &root, + home.path(), + &["local", "--json", "server", "stop", "parent-running"], + ); + assert!( + stop.status.success(), + "stderr: {}", + String::from_utf8_lossy(&stop.stderr) + ); + process.disarm(); + + let stopped = run( + &root, + home.path(), + &["local", "--json", "server", "stop", "parent-running"], + ); + assert!(stopped.status.success()); + let body: Value = serde_json::from_slice(&stopped.stdout).expect("parse stopped output"); + assert_eq!(body["already_stopped"], true); +} + +#[test] +fn symlinked_cwd_reports_the_canonical_project_directory() { + let workspace = tempfile::tempdir().expect("create workspace tempdir"); + let home = tempfile::tempdir().expect("create home tempdir"); + let project = workspace.path().join("actual-project"); + let link = workspace.path().join("project-link"); + std::fs::create_dir(&project).expect("create project"); + symlink(&project, &link).expect("create project symlink"); + + let json = run( + &link, + home.path(), + &["local", "--json", "server", "stop", "missing"], + ); + assert_scoped_json_error(&json, "server_not_found", &project); + + let human = run( + &link, + home.path(), + &["local", "server", "remove", "missing"], + ); + assert_scoped_human_error(&human, &project); + assert!(!String::from_utf8_lossy(&human.stderr).contains(&link.display().to_string())); +} + +#[test] +fn list_metadata_errors_identify_the_nested_project_scope() { + let workspace = tempfile::tempdir().expect("create workspace tempdir"); + let home = tempfile::tempdir().expect("create home tempdir"); + let parent = workspace.path().join("project"); + let nested = parent.join("nested"); + create_stopped_server(&parent, "parent-stopped"); + std::fs::create_dir_all(nested.join(".clickhouse/servers")) + .expect("create nested metadata directory"); + std::fs::write(nested.join(".clickhouse/servers/broken.json"), b"{") + .expect("write broken metadata"); + + let json = run(&nested, home.path(), &["local", "--json", "server", "list"]); + assert_scoped_json_error(&json, "server_metadata_invalid", &nested); + + let human = run(&nested, home.path(), &["local", "server", "list"]); + assert_scoped_human_error(&human, &nested); +} + +#[test] +fn invalid_servers_directory_has_project_scope_in_json_mode() { + let project = tempfile::tempdir().expect("create project tempdir"); + let home = tempfile::tempdir().expect("create home tempdir"); + create_invalid_servers_directory(project.path()); + + let output = run( + project.path(), + home.path(), + &["local", "--json", "server", "list"], + ); + let body = assert_scoped_json_error(&output, "io_error", project.path()); + let message = body["error"]["message"].as_str().unwrap(); + assert!( + message.starts_with("Local filesystem operation failed\n"), + "{body}" + ); + assert!(!message.contains("IO error:"), "{body}"); + assert!(!message.contains("Not a directory"), "{body}"); + assert!(!message.contains("os error"), "{body}"); +} + +#[test] +fn invalid_servers_directory_has_project_scope_in_human_mode() { + let project = tempfile::tempdir().expect("create project tempdir"); + let home = tempfile::tempdir().expect("create home tempdir"); + create_invalid_servers_directory(project.path()); + + let output = run(project.path(), home.path(), &["local", "server", "list"]); + assert_scoped_human_error(&output, project.path()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.starts_with("Error: IO error:"), "stderr: {stderr}"); + assert!(stderr.contains("Not a directory"), "stderr: {stderr}"); +} diff --git a/crates/clickhousectl/tests/local_server_selection_test.rs b/crates/clickhousectl/tests/local_server_selection_test.rs index 9b2e5331..5f0349de 100644 --- a/crates/clickhousectl/tests/local_server_selection_test.rs +++ b/crates/clickhousectl/tests/local_server_selection_test.rs @@ -206,7 +206,7 @@ fn start_fake_server(project: &Path, home: &Path, name: &str) -> u32 { .expect("server pid") as u32 } -fn assert_json_error(output: &Output, message: &str) { +fn assert_json_error(output: &Output, project: &Path, message: &str) { assert_eq!( output.status.code(), Some(1), @@ -216,8 +216,18 @@ fn assert_json_error(output: &Output, message: &str) { assert!(output.stdout.is_empty()); let body: Value = serde_json::from_slice(&output.stderr).expect("parse structured error"); assert_eq!(body["error"]["code"], "server_not_found"); - assert_eq!(body["error"]["message"], message); - assert_eq!(body["error"]["command"], "clickhousectl local server list"); + let project = project.canonicalize().expect("canonical project"); + let scoped_message = body["error"]["message"].as_str().unwrap(); + assert!(scoped_message.starts_with(message), "{scoped_message}"); + assert!( + scoped_message.contains("parent `.clickhouse` directories are not searched"), + "{scoped_message}" + ); + assert_eq!( + body["error"]["command"], + "clickhousectl local server list --global" + ); + assert_eq!(body["error"]["project"], project.display().to_string()); } struct ProcessGuard { @@ -386,13 +396,14 @@ fn omitted_stop_prefers_default_and_rejects_multiple_non_default_servers() { &["local", "--json", "server", "stop"], ); let message = "No server name was provided and multiple non-default ClickHouse servers exist. Pass a name or run `clickhousectl local server stop-all`; use `clickhousectl local server list` to see available servers."; - assert_json_error(&ambiguous, message); + assert_json_error(&ambiguous, project.path(), message); let human = run(project.path(), home.path(), &["local", "server", "stop"]); assert_eq!(human.status.code(), Some(1)); - assert_eq!( - String::from_utf8_lossy(&human.stderr), - format!("Error: {message}\n") + assert!( + String::from_utf8_lossy(&human.stderr).starts_with(&format!("Error: {message}\n")), + "stderr: {}", + String::from_utf8_lossy(&human.stderr) ); create_stopped_server(project.path(), "default"); @@ -418,6 +429,7 @@ fn omitted_remove_never_guesses_custom_servers() { ); assert_json_error( &empty, + empty_project.path(), "No removable 'default' ClickHouse server exists, and no custom ClickHouse servers are available. Run `clickhousectl local server list` to inspect local server state.", ); @@ -430,16 +442,17 @@ fn omitted_remove_never_guesses_custom_servers() { home.path(), &["local", "--json", "server", "remove"], ); - let message = "No removable 'default' ClickHouse server exists. Run `clickhousectl local server list`, then pass a custom server name explicitly with `clickhousectl local server remove `."; - assert_json_error(&one, message); + let message = "No removable 'default' ClickHouse server exists. Run `clickhousectl local server list`, then retry with an explicit custom server name."; + assert_json_error(&one, project.path(), message); assert!(project.path().join(".clickhouse/servers/dev/data").exists()); create_stopped_server(project.path(), "analytics"); let many = run(project.path(), home.path(), &["local", "server", "remove"]); assert_eq!(many.status.code(), Some(1)); - assert_eq!( - String::from_utf8_lossy(&many.stderr), - format!("Error: {message}\n") + assert!( + String::from_utf8_lossy(&many.stderr).starts_with(&format!("Error: {message}\n")), + "stderr: {}", + String::from_utf8_lossy(&many.stderr) ); assert!(project.path().join(".clickhouse/servers/dev/data").exists()); assert!( @@ -480,7 +493,10 @@ fn omitted_remove_refuses_running_default_and_explicit_unknown_stays_a_typo() { assert_eq!(body["error"]["code"], "server_running"); assert_eq!( body["error"]["message"], - "Server 'default' is running; stop it first with `clickhousectl local server stop default`" + format!( + "Server 'default' is running and cannot be removed. Run `clickhousectl local server list`, then stop it by name before retrying.\nProject directory used for lookup: {:?}\nOnly this exact directory's `.clickhouse` is searched; parent `.clickhouse` directories are not searched.\nRun `clickhousectl local server list --global` to find running servers. For stopped servers, change to the intended project directory and run `clickhousectl local server list`.", + project.path().canonicalize().unwrap().display().to_string() + ) ); let stop = run( @@ -496,5 +512,5 @@ fn omitted_remove_refuses_running_default_and_explicit_unknown_stays_a_typo() { home.path(), &["local", "--json", "server", "remove", "--name", "missing"], ); - assert_json_error(&unknown, "Server 'missing' not found"); + assert_json_error(&unknown, project.path(), "Server 'missing' not found"); } diff --git a/crates/clickhousectl/tests/local_server_stopped_test.rs b/crates/clickhousectl/tests/local_server_stopped_test.rs index 21cdc7f6..eb0878cc 100644 --- a/crates/clickhousectl/tests/local_server_stopped_test.rs +++ b/crates/clickhousectl/tests/local_server_stopped_test.rs @@ -270,8 +270,9 @@ fn running_server_remove_has_stop_first_error_and_start_keeps_collision_error() assert_eq!(remove.status.code(), Some(1)); let stderr = String::from_utf8_lossy(&remove.stderr); assert!(stderr.contains( - "Server 'test2' is running; stop it first with `clickhousectl local server stop test2`" + "Server 'test2' is running and cannot be removed. Run `clickhousectl local server list`, then stop it by name before retrying." )); + assert!(stderr.contains("parent `.clickhouse` directories are not searched")); assert!(!stderr.contains("already running")); let restart = run( diff --git a/crates/clickhousectl/tests/local_structured_errors_test.rs b/crates/clickhousectl/tests/local_structured_errors_test.rs index ee919d87..4e68d144 100644 --- a/crates/clickhousectl/tests/local_structured_errors_test.rs +++ b/crates/clickhousectl/tests/local_structured_errors_test.rs @@ -33,6 +33,28 @@ fn expected_error(code: &str, message: &str, command: &str) -> String { ) } +fn scoped_message(message: &str, project: &Path) -> String { + let project = project.canonicalize().expect("canonical project"); + format!( + "{message}\nProject directory used for lookup: {:?}\n\ + Only this exact directory's `.clickhouse` is searched; parent `.clickhouse` \ + directories are not searched.\n\ + Run `clickhousectl local server list --global` to find running servers. For stopped \ + servers, change to the intended project directory and run \ + `clickhousectl local server list`.", + project.display().to_string() + ) +} + +fn expected_scoped_error(code: &str, message: &str, project: &Path) -> String { + let project = project.canonicalize().expect("canonical project"); + let message = serde_json::to_string(&scoped_message(message, &project)).unwrap(); + let project = serde_json::to_string(&project.display().to_string()).unwrap(); + format!( + "{{\n \"error\": {{\n \"code\": \"{code}\",\n \"message\": {message},\n \"command\": \"clickhousectl local server list --global\",\n \"project\": {project}\n }}\n}}\n" + ) +} + fn assert_structured_error(output: &Output, expected: &str) { assert_eq!( output.status.code(), @@ -83,10 +105,10 @@ fn explicit_json_writes_exact_server_not_found_error_to_stderr() { assert_structured_error( &output, - &expected_error( + &expected_scoped_error( "server_not_found", "Server 'missing' not found", - "clickhousectl local server list", + project.path(), ), ); } @@ -150,10 +172,10 @@ fn telemetry_debug_does_not_append_to_a_structured_error() { assert_structured_error( &output, - &expected_error( + &expected_scoped_error( "server_not_found", "Server 'missing' not found", - "clickhousectl local server list", + project.path(), ), ); } @@ -170,10 +192,10 @@ fn agent_mode_writes_the_same_structured_error_without_json_flag() { assert_structured_error( &output, - &expected_error( + &expected_scoped_error( "server_not_found", "Server 'missing' not found", - "clickhousectl local server list", + project.path(), ), ); } @@ -190,7 +212,13 @@ fn human_mode_keeps_concise_error_text() { assert_eq!(output.status.code(), Some(1)); assert!(output.stdout.is_empty()); - assert_eq!(output.stderr, b"Error: Server 'missing' not found\n"); + assert_eq!( + String::from_utf8_lossy(&output.stderr), + format!( + "Error: {}\n", + scoped_message("Server 'missing' not found", project.path()) + ) + ); } #[test]