From c275f1d14fb5894a481ab966d9b52180a8c77d94 Mon Sep 17 00:00:00 2001
From: sdairs <sdairs@users.noreply.github.com>
Date: Fri, 31 Jul 2026 16:04:23 +0100
Subject: [PATCH] Make repeated exact installs idempotent

---
 .../src/version_manager/install.rs            |  7 +--
 .../tests/local_install_local_first_test.rs   | 60 +++++++++++++++----
 2 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/crates/clickhousectl/src/version_manager/install.rs b/crates/clickhousectl/src/version_manager/install.rs
index 8a435dce..bce6b123 100644
--- a/crates/clickhousectl/src/version_manager/install.rs
+++ b/crates/clickhousectl/src/version_manager/install.rs
@@ -9,17 +9,14 @@ use crate::version_manager::spec::VersionSpec;
 use std::os::unix::fs::PermissionsExt;
 
 /// Install a version spec, trying installed versions first before any remote call.
-/// Matches the UX of `install_resolved`'s post-resolve local check, but avoids the
-/// network round-trip when a local match exists.
+/// An installed match is a successful no-op regardless of whether the spec is
+/// exact or partial.
 pub async fn install_local_first(
     spec: &VersionSpec,
     platform: &Platform,
     force: bool,
 ) -> Result<String> {
     if !force && let Some(local) = try_resolve_local(spec) {
-        if matches!(spec, VersionSpec::Exact(_)) {
-            return Err(Error::VersionAlreadyInstalled(local));
-        }
         eprintln!("ClickHouse {} is already installed as {}", spec, local);
         eprintln!("Use --force to re-download the latest build");
         return Ok(local);
diff --git a/crates/clickhousectl/tests/local_install_local_first_test.rs b/crates/clickhousectl/tests/local_install_local_first_test.rs
index fcf02bd1..19b3135f 100644
--- a/crates/clickhousectl/tests/local_install_local_first_test.rs
+++ b/crates/clickhousectl/tests/local_install_local_first_test.rs
@@ -1,8 +1,10 @@
-//! Regression test for issue #217: `local install <concrete-spec>` must satisfy
-//! the request from already-installed versions without any remote call.
+//! Regression tests for local-first installs. Issue #217 requires
+//! `local install <concrete-spec>` to satisfy the request from already-installed
+//! versions without any remote call, while issue #339 requires exact and partial
+//! spellings of an installed version to both be successful no-ops.
 //!
 //! Strategy: spawn the binary with `HOME=<tempdir>`, pre-seed a fake installed
-//! version, run `local install 25.12 --json`, and assert that:
+//! version, run both partial and exact `local install` commands, and assert that:
 //!   - the command exits 0,
 //!   - stderr says "already installed",
 //!   - stderr does NOT say "Resolving" (which only prints on the remote path).
@@ -13,17 +15,19 @@
 
 use std::os::unix::fs::PermissionsExt;
 use std::path::PathBuf;
-use std::process::Command;
+use std::process::{Command, Output};
 
 fn clickhousectl_binary() -> PathBuf {
     PathBuf::from(env!("CARGO_BIN_EXE_clickhousectl"))
 }
 
-#[test]
-fn local_install_minor_with_existing_match_does_not_hit_network() {
+fn install_with_existing_version(installed_version: &str, requested_spec: &str) -> Output {
     let tempdir = tempfile::tempdir().expect("create tempdir");
 
-    let version_dir = tempdir.path().join(".clickhouse/versions/25.12.9.61");
+    let version_dir = tempdir
+        .path()
+        .join(".clickhouse/versions")
+        .join(installed_version);
     std::fs::create_dir_all(&version_dir).expect("create version dir");
 
     let binary = version_dir.join("clickhouse");
@@ -32,12 +36,17 @@ fn local_install_minor_with_existing_match_does_not_hit_network() {
     perms.set_mode(0o755);
     std::fs::set_permissions(&binary, perms).unwrap();
 
-    let output = Command::new(clickhousectl_binary())
+    Command::new(clickhousectl_binary())
         .env("DO_NOT_TRACK", "1")
         .env("HOME", tempdir.path())
-        .args(["local", "install", "25.12", "--json"])
+        .args(["local", "install", requested_spec, "--json"])
         .output()
-        .expect("run clickhousectl");
+        .expect("run clickhousectl")
+}
+
+#[test]
+fn local_install_minor_with_existing_match_does_not_hit_network() {
+    let output = install_with_existing_version("25.12.9.61", "25.12");
 
     let stderr = String::from_utf8_lossy(&output.stderr);
     assert!(
@@ -57,3 +66,34 @@ fn local_install_minor_with_existing_match_does_not_hit_network() {
         stderr
     );
 }
+
+#[test]
+fn local_install_exact_with_existing_match_is_a_successful_no_op() {
+    let version = "25.12.9.61";
+    let output = install_with_existing_version(version, version);
+
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        output.status.success(),
+        "expected success, got status {:?}\nstderr: {}",
+        output.status,
+        stderr
+    );
+    assert!(
+        stderr.contains(&format!(
+            "ClickHouse {version} is already installed as {version}"
+        )),
+        "expected exact-version no-op message, got: {}",
+        stderr
+    );
+    assert!(
+        stderr.contains("Use --force to re-download the latest build"),
+        "expected --force hint, got: {}",
+        stderr
+    );
+    assert!(
+        !stderr.contains("Resolving"),
+        "expected no remote-resolve message, got: {}",
+        stderr
+    );
+}