Skip to content

Make omitted local server selection deliberate - #557

Open
sdairs wants to merge 1 commit into
stack/475-structured-local-errorsfrom
stack/473-deliberate-server-selection
Open

Make omitted local server selection deliberate#557
sdairs wants to merge 1 commit into
stack/475-structured-local-errorsfrom
stack/473-deliberate-server-selection

Conversation

@sdairs

@sdairs sdairs commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • select default or a sole ClickHouse server deliberately for an omitted server stop name, with a typed zero-server no-op
  • keep omitted server remove conservative and add path-safe human/JSON selection guidance
  • preserve positional and --name dispatch while marking implicit and explicit outcomes
  • document the selection policy and cover zero/one/many, running/stopped, default/custom, legacy, and persisted state

Tests

  • cargo test -p clickhousectl --test local_server_selection_test
  • cargo test -p clickhousectl --test local_server_name_compatibility_test
  • cargo test -p clickhousectl --test local_server_stopped_test
  • cargo test -p clickhousectl --test local_structured_errors_test
  • cargo test -p clickhousectl -- --test-threads=1
  • cargo fmt --all --check
  • cargo clippy -p clickhousectl --all-targets -- -D warnings

Closes #473

Comment thread crates/clickhousectl/src/local/mod.rs
Comment thread crates/clickhousectl/src/error.rs

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

❌ 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 dbeb1a8. Configure here.

Comment thread crates/clickhousectl/src/local/server.rs
@sdairs

sdairs commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Carry-over review from PR #514 (old PR for #473). This PR is preferred as the base: better dispatch modeling (ServerNameInput { Omitted, Positional, NameFlag } preserves how the name was supplied), better ordering (lock → recover → enumerate), implicit/explicit selection markers in both human and JSON output directly satisfy the acceptance criteria, and {available} counts in guidance. #514 should be closed in favor of it. But this PR regresses all five of sdairs' help-trim comments on #514 and has scope creep. Required:

  1. Re-apply sdairs' five Make omitted local server selection deliberate #514 help trims verbatim (all regress here). On Make omitted local server selection deliberate #514, sdairs left five pending inline comments on crates/clickhousectl/src/local/cli.rs, all trimming help over-engineering; he then fixed them in commit 299d3b9 ("Trim local teardown help"): removed --name prose, hid the --name flag (hide = true), removed the "Idempotent…"/"typos" lines, and added a test asserting help never contains --name. In this PR:

    • cli.rs:~422-423 and cli.rs:~468-469 still carry "The compatibility form --name dev remains accepted, but cannot be combined with a positional name." → delete.
    • cli.rs:~427-428 still carry "Idempotent: a server that exists but is already stopped exits 0 (no error)." and "An unknown server name still errors so typos are caught." → delete (sdairs marked both "remove"/"uncessary - does not assist in operating the CLI").
    • The --name flags here are NOT hidden (cli.rs:~436, cli.rs:~479, no hide = true) → hide them.
    • Port the server_teardown_help_hides_compatibility_name_flags test asserting help never contains --name.
  2. Revert omitted-name selection for stop --global. Make omitted local server selection deliberate #514 deliberately left --global stop unchanged (omitted name → literal default). This PR extends the selection policy to stop --global (mod.rs global arm), so an omitted global stop now stops a sole custom server in another project without a name — a real cross-project behavior change outside issue Make omitted local server name selection deliberate and actionable #473's scope (the issue is about project-scoped commands). Cursor Bugbot flagged "Global stop picks foreign sole server"; unanswered. Revert to fail-closed literal default, or explicitly justify the change in the PR description.

  3. Fix Postgres-remnant selection pollution (regression of a Make omitted local server selection deliberate #514 fix). On Make omitted local server selection deliberate #514, sdairs fixed: an orphaned *-pg* data dir without metadata was miscounted as ClickHouse (commit a3e2b8e, with a regression test using a fake Docker socket proving a failed fresh Postgres start can't pollute ClickHouse selection). Here list_clickhouse_server_names_locked (server.rs:~414-434) counts such dirs again, and there is no equivalent test. Port the rollback fix (if not in this stack's base) and the fake-Docker regression test; add a Postgres-only project no-op exclusion case.

  4. Document the new JSON error code. server_selection_required is emitted but the README structured-error table (head README.md:~978) still lists only the old codes; this PR's README diff doesn't touch the table. Add it (or remove the code if not kept).

  5. Minor: selection is added to all stop/remove JSON output (including explicit calls, and Postgres outputs via selection: None) — a broader output-contract change than the issue requires; confirm intentional.

CI note: local postgres edge cases fails identically on both this and #514 — pre-existing stack-base breakage from #547, not this PR.

@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch from dbeb1a8 to a02b2c5 Compare August 26, 2026 18:58
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch 2 times, most recently from a694feb to 43dfee6 Compare August 26, 2026 19:03
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch from 43dfee6 to 4e6d763 Compare August 26, 2026 19:07
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch from 4e6d763 to 3efce63 Compare August 26, 2026 19:10
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch from 3efce63 to 5335622 Compare August 26, 2026 19:12
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch from 5335622 to 2fe9959 Compare August 26, 2026 19:20
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch from 2fe9959 to 47b4710 Compare August 26, 2026 19:24
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch 2 times, most recently from 4df2ea3 to bb031c5 Compare August 26, 2026 19:31
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch 2 times, most recently from 5e68ca9 to f4287a1 Compare August 26, 2026 19:36
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch 2 times, most recently from 0c54a1a to 531052b Compare August 26, 2026 20:30
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch from 531052b to c1551e0 Compare August 26, 2026 20:35
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch from c1551e0 to 724c4aa Compare August 26, 2026 20:38
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch from ad218f3 to b429415 Compare August 26, 2026 20:44
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch from b429415 to 937b759 Compare August 26, 2026 20:49
@sdairs
sdairs force-pushed the stack/473-deliberate-server-selection branch from 937b759 to d94e6a7 Compare August 26, 2026 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make omitted local server name selection deliberate and actionable

1 participant