Bare local server start bootstraps latest when nothing is set up#274
Bare local server start bootstraps latest when nothing is set up#274sdairs wants to merge 2 commits into
local server start bootstraps latest when nothing is set up#274Conversation
Closes #264. Running `clickhousectl local server start` with no installed version, no default set, and no `--version` flag now installs `latest` and starts with it, instead of erroring with `NoDefaultVersion`. - `start_server` (`local/mod.rs`): the bare-start branch matches on `get_default_version()`. `NoDefaultVersion` -> resolve `latest`, `ensure_installed_local_first`, and start, without setting a default (so unpinned users keep tracking latest on later starts). A `VersionNotFound` default (file points at a removed binary) stays an error. - `ensure_installed` (`version_manager/install.rs`): for `latest`/master builds the exact version is only known after download, so `install_resolved` was returning `VersionAlreadyInstalled` and breaking the "ensure" contract. Since bare start never sets a default, every repeat start hit this path and failed. Map `VersionAlreadyInstalled` -> `Ok(version)`. Also fixes the pre-existing `server start --version latest`-on-repeat failure. - Docs/help: README server section + `server start` help document the bootstrap; the "typical workflow" help drops the three-step install/use/start dance for a bare `server start`; `stable`-as-tag examples switched to `latest`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bare `server start` and `install/server start --version latest` track the rolling master build, so every repeat re-downloaded the full ~153MB master binary and re-ran `clickhouse --version`, even when master hadn't moved. Master exposes no readable version (the `--version` string is shared across many commits, sibling metadata files 403), but `builds.clickhouse.com` returns a stable, content-keyed `etag` on a HEAD request (~50ms, no body). Use it as a change-detection key: - New `version_manager/master.rs`: a per-platform sidecar (`~/.clickhouse/versions/.master-builds.json`) recording each installed master build's `etag` + version; a best-effort `head_info()` HEAD helper; a pure `should_reuse`/`reuse_if_unchanged` decision; and a `record()` writer. - `install_resolved`: for the master source, HEAD-and-compare before downloading. Etag match + binary present -> reuse the installed build, skip the download and version detection. Otherwise download and re-record. Handles "etag recorded but binary missing" by falling through to download. This also fixes a latent bug: because a master build's detected version string is shared across commits, the old code would re-download a moved master, detect the same version, hit `VersionAlreadyInstalled`, and discard the fresh binary — paying the full download cost yet keeping the stale build. Reuse/overwrite now key on the etag, so a changed master correctly overwrites in place. Measured: repeat `install latest` drops from ~12.4s (153MB download) to ~0.14s (single HEAD). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f9d1999. Configure here.
| // otherwise it pulls the newer master build. | ||
| let spec = version_manager::parse_version_spec("latest")?; | ||
| let platform = version_manager::platform::Platform::detect()?; | ||
| eprintln!("No version specified and no default set; installing latest..."); |
There was a problem hiding this comment.
Bootstrap message on every start
Low Severity
When server start runs without a specified version and no default is configured, it always prints a message indicating it's "installing latest." This message is misleading because it appears even when latest is already installed and up-to-date, and no actual download or new installation occurs.
Reviewed by Cursor Bugbot for commit f9d1999. Configure here.
| // (or there was no record), so overwrite the stale binary in place. | ||
| let version_dir = paths::version_dir(&exact_version)?; | ||
| if version_dir.exists() && !force { | ||
| if version_dir.exists() && !force && !is_master { |
There was a problem hiding this comment.
Master update deletes in-use binary
High Severity
When the remote master etag changes, install_resolved removes the existing versions/<version>/ tree before installing the new build, without checking whether a local server is still running that version. local remove refuses the same operation, but unpinned bare server start can trigger this path on every master move.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f9d1999. Configure here.
| // (or there was no record), so overwrite the stale binary in place. | ||
| let version_dir = paths::version_dir(&exact_version)?; | ||
| if version_dir.exists() && !force { | ||
| if version_dir.exists() && !force && !is_master { |
There was a problem hiding this comment.
🟡 Medium version_manager/install.rs:145
For master/latest installs, the post-download VersionAlreadyInstalled check is skipped (!is_master at line 145), so when the downloaded master binary reports the same --version string as an already-installed stable/release build, lines 151–155 remove_dir_all that existing versions/<exact_version>/ directory and replace it with the master binary. This silently overwrites a user's explicitly installed stable version with a master build instead of returning the existing install. The collision is real because master builds report the same numeric version string as released builds that share the commit. Consider distinguishing master installs from stable installs (e.g., a marker/sidecar in the version dir) so a master download only overwrites a prior master install, not a stable one — or fall back to VersionAlreadyInstalled when the existing dir has no master marker.
Also found in 1 other location(s)
crates/clickhousectl/src/version_manager/master.rs:139
reuse_if_unchangedonly checks thatversions/<record.version>/clickhousestill exists before reusing it. If a user later overwrites that same version directory with a different build viaclickhousectl local install --force(for example, replacing a recorded master build with a stable/package build that has the same numeric version), the stale sidecar entry still matches the old masteretagand this function will silently reuse the wrong binary on the nextlatestinstall/start instead of downloading the current master build.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @crates/clickhousectl/src/version_manager/install.rs around line 145:
For master/`latest` installs, the post-download `VersionAlreadyInstalled` check is skipped (`!is_master` at line 145), so when the downloaded master binary reports the same `--version` string as an already-installed stable/release build, lines 151–155 `remove_dir_all` that existing `versions/<exact_version>/` directory and replace it with the master binary. This silently overwrites a user's explicitly installed stable version with a master build instead of returning the existing install. The collision is real because master builds report the same numeric version string as released builds that share the commit. Consider distinguishing master installs from stable installs (e.g., a marker/sidecar in the version dir) so a master download only overwrites a prior master install, not a stable one — or fall back to `VersionAlreadyInstalled` when the existing dir has no master marker.
Also found in 1 other location(s):
- crates/clickhousectl/src/version_manager/master.rs:139 -- `reuse_if_unchanged` only checks that `versions/<record.version>/clickhouse` still exists before reusing it. If a user later overwrites that same version directory with a different build via `clickhousectl local install --force` (for example, replacing a recorded master build with a stable/package build that has the same numeric version), the stale sidecar entry still matches the old master `etag` and this function will silently reuse the wrong binary on the next `latest` install/start instead of downloading the current master build.


Closes #264.
Summary
Running
clickhousectl local server startwith no installed version, no default set, and no--versionflag now bootstraps automatically: it installslatestand starts with it, instead of failing withNoDefaultVersionand exit code 1.NoDefaultVersion) → resolvelatest, install if missing, and start. We deliberately do not set it as the default, so unpinned users keep trackinglateston later starts. A note is printed to stderr so the auto-download isn't silent.VersionNotFounddefault (file points at a removed binary) stays an error — only the no default at all case bootstraps.Changes
start_server(src/local/mod.rs) — the bare-start branch matches onget_default_version()and handlesNoDefaultVersionby bootstrappinglatest. Message:No version specified and no default set; installing latest....ensure_installed(src/version_manager/install.rs) — bug fix required for the feature. Forlatest/master builds the exact version is only known after download, soinstall_resolvedreturnedVersionAlreadyInstalled, breakingensure_installed's "return existing, don't error" contract. Because bare start never sets a default, every repeat start hits this path and would fail. Now mapsVersionAlreadyInstalled→Ok(version). This also fixes the pre-existingserver start --version latest-on-repeat failure.server starthelp document the bootstrap; the "typical workflow" help text drops the three-stepinstall stable && use stable && startdance in favour of a bareserver start;stable-as-tag examples switched tolatest(the install keyword catalog keepsstabledocumented).Skip re-downloading master when unchanged (etag change detection)
Because bare start never sets a default, every repeat re-entered the
latestpath and re-downloaded the full ~153MB master binary (then re-ranclickhouse --version), even when master hadn't moved. This PR now short-circuits that.Master exposes no readable version (the
--versionstring is shared across many commits, sibling metadata files 403), butbuilds.clickhouse.comreturns a stable, content-keyedetagon a HEAD request (~50ms, no body). We use it as a change-detection key:src/version_manager/master.rs— a per-platform sidecar (~/.clickhouse/versions/.master-builds.json) recording each installed master build'setag+ version; a best-efforthead_info()HEAD helper; a pureshould_reuse/reuse_if_unchangeddecision; and arecord()writer.install_resolved— for the master source, HEAD-and-compare before downloading. Etag match + binary present → reuse the installed build, skipping the download and version detection. Otherwise download and re-record. Handles "etag recorded but binary missing" by falling through to download. Applies to bothinstall latestandserver start --version latest/ bare start (shared path).This also fixes a latent bug: because a master build's detected version string is shared across commits, the old code would re-download a moved master, detect the same version, hit
VersionAlreadyInstalled, and discard the fresh binary — paying the full download cost yet keeping the stale build. Reuse/overwrite now key on the etag, so a changed master correctly overwrites in place.Verification
Manual end-to-end in an isolated
HOME(no installs, no default):server start→ message prints,latestdownloads, server starts, exit 0, nodefaultfile created. ✅server start→ reuses the installed build, starts, exit 0 (previously errored). ✅Error: Version <x> not found, exit 1. ✅Etag change detection, against the live CDN with the built binary:
First
install latest→ downloads master, writes the sidecar with the build'setag. ~12.4s. ✅Second
install latest→latest is up to date (master build unchanged); using <v>, single HEAD, no download. ~0.14s. ✅7 new unit tests cover the reuse decision (etag match/mismatch, missing binary, no record, failed HEAD) and sidecar round-trip.
cargo build,cargo clippy(clean),cargo test(all pass). ✅🤖 Generated with Claude Code