Skip to content

Explain managed client project scope errors - #558

Open
sdairs wants to merge 1 commit into
stack/473-deliberate-server-selectionfrom
stack/467-client-project-scope-errors
Open

Explain managed client project scope errors#558
sdairs wants to merge 1 commit into
stack/473-deliberate-server-selectionfrom
stack/467-client-project-scope-errors

Conversation

@sdairs

Copy link
Copy Markdown
Collaborator

Summary

  • add managed-client-specific missing, stopped, and selected-binary diagnostics with canonical exact-project scope
  • emit stable typed JSON context and safe recovery guidance for explicit JSON and agent output
  • document exact-directory lookup and keep server names, paths, versions, and raw errors out of telemetry

Tests

  • cargo test -p clickhousectl --test local_client_project_scope_errors_test
  • cargo test -p clickhousectl --features telemetry --test telemetry_test managed_client_failure_details_never_reach_telemetry
  • cargo test -p clickhousectl -- --test-threads=1
  • cargo clippy -p clickhousectl --all-targets -- -D warnings
  • cargo fmt --all --check
  • cargo build -p clickhousectl

Closes#467

@sdairs

Copy link
Copy Markdown
CollaboratorAuthor

Carry-over review from PR #518 (old PR for #467). This PR is better on its primary targets (clearer human text naming mode + exact dir + "parent projects are not searched" + selection-appropriate recovery; selection-aware start vs start <name> guidance; exact full-output equality subprocess tests incl. a symlinked-CWD case and an explicit no-interpolation assertion; a real E2E telemetry test stronger than #518's in-process one). But it regresses the one defect sdairs personally signed off as fixed on #518. Required:

  1. BLOCKER — lock/metadata failures escape unannotated (regression of sdairs' confirmed fix on Expose project scope in managed local client errors #518). On Expose project scope in managed local client errors #518, both bots raised that ServerLock::acquire wasn't passed through with_managed_client_scope, and sdairs confirmed: "Fixed in c12947bServerLock::acquire now maps through with_managed_client_scope, and the focused subprocess test covers an invalid .locks path in both JSON and human modes." In this PR, the managed branch (crates/clickhousectl/src/local/mod.rs:~300-302) does lock_metadata()?, recover_current_project_servers_locked(&metadata_lock)?, and server_entry_locked(server_name, &metadata_lock)? with plain ? — no mapping to the managed error. Those calls return raw ServerMetadataRead/Utf8/Parse/Write, InvalidServerName, io errors, or a raw Error::ServerNotFound (stale-reload path), with no mode/project context and no recovery commands. A corrupt default.json (ServerMetadataParse) or unwritable .clickhouse/servers yields exactly the unhelpful error issue Make local client managed-state errors explain project scope #467 was opened to eliminate — violating criteria 1–2 on that failure class. The closed 3-variant ManagedClientErrorKind enum makes this structurally hard: there is no variant to carry a lock/metadata failure. Fix: add a fourth kind (e.g. ProjectStateUnavailable) wrapping ServerMetadata*/lock errors from those three calls, or re-introduce a scope-attachment mechanism like Expose project scope in managed local client errors #518's wrapper. Also port a subprocess test mirroring Expose project scope in managed local client errors #518's invalid_lock_directory_keeps_managed_scope_in_json_and_human_errors — there is currently no coverage for these paths at all.

  2. Decide the JSON contract churn (reviewer decision, not a blocker). This PR introduces three new managed_client_* codes and a divergent envelope (drops command, adds project_scope/server/guidance) vs Expose project scope in managed local client errors #518's "keep server_not_found + add mode/project". Nothing consumes these yet (unshipped), so it is tolerable — but choose deliberately and document in the README, since agent consumers may key on the existing server_not_found vocabulary.

  3. Minor over-engineering: single-variant enums serialized as constants (LocalProjectScopeKind::ExactCurrentProject, parent_projects_searched which can only be false, LocalClientMode::Managed) plus a 6-action guidance enum. (Expose project scope in managed local client errors #518 had a single-variant LocalClientMode::Managed too, so fairness noted — but consider trimming fields that can only ever hold one value.)

  4. Minor: entry.info.ok_or_else(...) (mod.rs:~310-312) is effectively unreachable (server_entry_locked_with always returns info: Some); the managed branch checks binary existence then the shared code re-checks it — harmless duplication, tidy if easy.

@sdairs
sdairsforce-pushed the stack/467-client-project-scope-errors branch 2 times, most recently from 9a5d079 to ee01097CompareAugust 26, 2026 19:01
@sdairs
sdairsforce-pushed the stack/467-client-project-scope-errors branch 4 times, most recently from fdedea6 to a1e5fceCompareAugust 26, 2026 19:12

@cursorcursorBot 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 a1e5fce. Configure here.

Comment threadcrates/clickhousectl/src/local/mod.rs
@sdairs
sdairsforce-pushed the stack/467-client-project-scope-errors branch from a1e5fce to 81056abCompareAugust 26, 2026 19:20
@sdairs
sdairsforce-pushed the stack/467-client-project-scope-errors branch from 81056ab to ca994c9CompareAugust 26, 2026 19:24
@sdairs
sdairsforce-pushed the stack/467-client-project-scope-errors branch from ca994c9 to 42f426dCompareAugust 26, 2026 19:28
@sdairs
sdairsforce-pushed the stack/467-client-project-scope-errors branch 2 times, most recently from 04fb9d2 to 8f20986CompareAugust 26, 2026 19:34
@sdairs
sdairsforce-pushed the stack/467-client-project-scope-errors branch 2 times, most recently from 418880e to d236960CompareAugust 26, 2026 20:10
@sdairs
sdairsforce-pushed the stack/467-client-project-scope-errors branch from d236960 to 5770fb0CompareAugust 26, 2026 20:30
@sdairs
sdairsforce-pushed the stack/467-client-project-scope-errors branch from 5770fb0 to 4d13967CompareAugust 26, 2026 20:35
@sdairs
sdairsforce-pushed the stack/467-client-project-scope-errors branch from 4d13967 to e07401bCompareAugust 26, 2026 20:38
@sdairs
sdairsforce-pushed the stack/467-client-project-scope-errors branch from e07401b to 4ebc92cCompareAugust 26, 2026 20:41
@sdairs
sdairsforce-pushed the stack/467-client-project-scope-errors branch 2 times, most recently from 2c4f5a4 to c66d412CompareAugust 26, 2026 20:49
@sdairs
sdairsforce-pushed the stack/467-client-project-scope-errors branch from c66d412 to e7370e1CompareAugust 26, 2026 20:55
Sign up for freeto 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.

1 participant

@sdairs