Wait for local server readiness

[sdairs]

Summary

• wait up to 30 seconds for ClickHouse HTTP /ping and TCP readiness before reporting a successful background start
• capture background startup output in .clickhouse/servers/<name>/server.log and point exit/timeout errors there
• stop timed-out startup processes gracefully, reject occupied explicit ports, and retain --no-wait for fire-and-forget startup
• add clap, timeout, failure-log, and delayed-readiness regression coverage

Stack

This PR is stacked on #356 and should be reviewed as the delta from codex/issue-324-list-stopped-servers.

Closes #330.

Validation

• cargo fmt --all --check
• cargo build -p clickhousectl
• cargo test -p clickhousectl
• cargo clippy -p clickhousectl --all-targets -- -D warnings

[Copilot]

Pull request overview

Ensures clickhousectl local server start only reports success for background starts once the local ClickHouse server is actually ready to accept HTTP /ping and TCP connections, and captures startup output to a per-server log for better diagnostics.

Changes:

• Add background-start readiness polling (HTTP /ping + TCP connect) with a default 30s timeout, plus graceful shutdown on timeout.
• Capture background startup stdout/stderr into .clickhouse/servers/<name>/server.log and reference it from failure/timeout errors.
• Add --no-wait escape hatch plus regression coverage (clap parsing, readiness wait, and failure log assertions).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
README.md	Documents --no-wait and the new readiness + logging behavior.
crates/clickhousectl/tests/local_server_stopped_test.rs	Updates existing regression test to use --no-wait where appropriate.
crates/clickhousectl/tests/local_server_readiness_test.rs	Adds integration regressions for delayed readiness and captured failure logs.
crates/clickhousectl/src/local/server.rs	Implements readiness polling, timeout shutdown, log path helper, and explicit-port occupancy checks.
crates/clickhousectl/src/local/mod.rs	Routes background start output to server.log and wires --no-wait / readiness flow.
crates/clickhousectl/src/local/cli.rs	Adds --no-wait flag, help text, and clap parsing/conflict tests.

Suppressed comments (1)

crates/clickhousectl/src/local/server.rs:620

• resolve_ports currently treats --tcp-port 0 as valid because binding to port 0 always succeeds. Port 0 is not a usable listen port and should be rejected explicitly (matching the existing Postgres port validation).

    let tcp = match tcp_port {
        Some(p) if is_port_available(p) => p,
        Some(p) => return Err(Error::Exec(format!("TCP port {} is already in use", p))),
        None => {

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cursor]

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 12d9c7e. Configure here.}

[cursor]

Readiness timeout stop leaves stale metadata

Medium Severity

The server's metadata is marked as running before it's fully ready. If wait_for_server_ready fails (e.g., timeout, HTTP client build error) or stop_starting_child can't terminate the process, an orphaned server may persist with 'running' metadata, blocking future starts with ServerAlreadyRunning.

Additional Locations (1)

• crates/clickhousectl/src/local/server.rs#L522-L527

 

^{Reviewed by Cursor Bugbot for commit 12d9c7e. Configure here.}

[cursor]

Timeout masks child exit status

Medium Severity

In wait_for_server_ready, the child is only reaped at the top of each loop iteration. If the process exits while TCP or HTTP probes run, the same iteration can still hit the timeout branch and report a readiness timeout instead of the exit status and log-directed exit error.

 

^{Reviewed by Cursor Bugbot for commit 12d9c7e. Configure here.}

[iskakaushik]

LGTM