Skip to content

Expose project scope in local server errors - #559

Merged
sdairs merged 2 commits into
stack/467-client-project-scope-errorsfrom
stack/477-server-project-scope-errors
Aug 27, 2026
Merged

Expose project scope in local server errors#559
sdairs merged 2 commits into
stack/467-client-project-scope-errorsfrom
stack/477-server-project-scope-errors

Conversation

@sdairs

@sdairs sdairs commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • report the canonical exact-CWD project scope for local server stop and remove not-found errors
  • explain missing .clickhouse state for omitted-name stop and remove, including the project root where the server was started
  • add actionable human and structured guidance to empty project lists while keeping global list output distinct
  • document the no-parent-search behavior and cover nested projects, symlinks, server state, and telemetry redaction

Scope

  • Unlike superseded PR Expose project scope in local server errors #517, this intentionally limits new project-scoped errors to stop/remove paths that cannot find current-project state or a selected server. Running-server removal and selection errors with existing .clickhouse state remain self-evidently scoped and retain their existing envelopes.
  • server list metadata and I/O failures also retain the shared redacted io_error envelope, so this PR does not add project-path detail to that case.

Tests

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

Closes #477

@sdairs

sdairs commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Carry-over review from PR #517 (old PR for #477). This PR is preferred over #517: typed ProjectServerNotFound error raised at the source instead of a post-hoc string wrapper; paths kept out of JSON message (matching the README contract, where #517 embedded the canonical path in the message and hard-overwrote command); explicit telemetry-redaction test (#517 lacked one); empty-list guidance via success output. Green CI where it matters. Three items:

  1. Answer sdairs' open review thread from Expose project scope in local server errors #517 (never implemented there either, so an open design question, not a regression). On Expose project scope in local server errors #517, sdairs wrote: "what about if we check for the presence of a .clickhouse dir in the CWD when a local command is run - and if there isn't one - we can tailor the output to give the message that we dont search for parent .clickhouse dirs must be run from project root", then scoped it: "I think this belongs in this PR rather than a separate issue… keep it narrow to commands that require existing project state, such as server stop and server remove. When .clickhouse is absent from the current directory, the error should explain that clickhousectl does not search parent directories and that .clickhouse typically lives in the local project root where the server was started. It should not apply to commands such as server start that can initialize project state." Against that:

    • Narrow to stop/remove, exclude start: satisfied (ProjectServerCommand has exactly Stop/Remove).
    • State parents are not searched: satisfied (error.rs:~275, JSON parent_projects_searched: false).
    • State .clickhouse typically lives in the project root where the server was started: not satisfied — closest text is "Return to the project root and run …" and guidance "Change to the project root that owns the server". Add the "where the server was started" framing.
    • Keyed on .clickhouse absence: not satisfied — no local_dir().exists() check; same message either way. The explicit-name stop/remove cases still get the full scoped error from a .clickhouse-less dir, but two silent paths survive: omitted-name stop in a non-project dir succeeds as a noop with zero scope signal (ServerStopNoopOutput has no scope field, local/mod.rs:~910-918), and omitted-name remove errors without scope. Decide: either attach project_scope to those two outputs or explicitly close the thread as addressed-by-other-means.
  2. Update the README structured-error section. The new project_scope/server/guidance body and the now-absent command field on stop/remove server_not_found are undocumented (README still shows the old {code, message, command} example). Expose project scope in local server errors #517 did update that section.

  3. Justify the narrower error coverage vs Expose project scope in local server errors #517 (note in PR description). Expose project scope in local server errors #517 also wrapped ServerRunningCannotRemove, selection-required, and list metadata/IO errors. Here only the two not-found sites are scoped. Defensible (those other errors can only fire when .clickhouse state exists in CWD, so scope is self-evident), but list metadata errors are the one case where Expose project scope in local server errors #517 was strictly more informative — call the narrowing out explicitly.

CI note: postgres edge-cases failures are pre-existing on the stack base (#547 script drift), not this PR.

@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch from 4183900 to 29f972e Compare August 26, 2026 18:58
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch from 29f972e to 5cdfa90 Compare August 26, 2026 19:01
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch from 5cdfa90 to 71ede57 Compare August 26, 2026 19:03
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch from 71ede57 to 8084aa4 Compare August 26, 2026 19:07
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch 2 times, most recently from 8f4ca81 to f40dacf Compare August 26, 2026 19:12
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch from f40dacf to 1affd64 Compare August 26, 2026 19:20
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch 2 times, most recently from 9e5da80 to 63bf12a Compare August 26, 2026 19:28
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch from 63bf12a to 2cc5b31 Compare August 26, 2026 19:31
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch 2 times, most recently from 13692c0 to 80f7418 Compare August 26, 2026 19:36
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch 2 times, most recently from 6cd324c to bf87900 Compare August 26, 2026 20:30
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch 2 times, most recently from 436d999 to ca10996 Compare August 27, 2026 06:44
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch 2 times, most recently from 866f49f to edf8cc3 Compare August 27, 2026 06:53
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch 2 times, most recently from 0858e80 to d67ca1c Compare August 27, 2026 07:06
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch 2 times, most recently from 6439b64 to 9f04fa9 Compare August 27, 2026 07:17
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch 2 times, most recently from cf47b39 to 34afe62 Compare August 27, 2026 07:32

@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.

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 623f401. Configure here.

Comment thread crates/clickhousectl/src/local/mod.rs
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch from 623f401 to e11a95f Compare August 27, 2026 09:19
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch from e11a95f to 0268e1e Compare August 27, 2026 10:13
@sdairs
sdairs force-pushed the stack/477-server-project-scope-errors branch from 0268e1e to 3251cbb Compare August 27, 2026 10:51
@sdairs
sdairs merged commit 4211ece into main Aug 27, 2026
26 checks passed
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.

Expose project scope in local server stop/remove/remove errors

2 participants