Skip to content

Emit structured local runtime errors - #556

Open
sdairs wants to merge 2 commits into
stack/474-server-name-compatibilityfrom
stack/475-structured-local-errors
Open

Emit structured local runtime errors#556
sdairs wants to merge 2 commits into
stack/474-server-name-compatibilityfrom
stack/475-structured-local-errors

Conversation

@sdairs

@sdairs sdairs commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • emit a stable redacted error envelope on stderr for local JSON and coding-agent runtime failures
  • defer first-run telemetry notices and suppress telemetry-debug output when a structured error owns stderr
  • add typed local codes for server state, versions, ports, startup, downloads, I/O, and bounded fallback errors
  • preserve human, clap, Cloud, telemetry privacy, exit-code, and child-process behavior
  • document the schema and stability contract

Tests

  • cargo test -p clickhousectl -- --test-threads=1
  • cargo test -p clickhousectl --test local_structured_errors_test -- --test-threads=1
  • cargo test -p clickhousectl --test telemetry_test -- --test-threads=1
  • cargo clippy -p clickhousectl --all-targets -- -D warnings
  • cargo fmt --all --check
  • cargo check -p clickhousectl --no-default-features

Closes #475

@sdairs

sdairs commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Carry-over review from PR #510 (old PR for #475). This PR's error model is better than #510's (typed engine-agnostic variants like PortInUse { kind, port } / StartupExit / StartupTimeout eliminate #510's mislabel and duplicate-variant Bugbot rounds; private DTO surface; honest optional command; rollback errors recurse into primary so the real code survives; and it fixes the one finding left unanswered on #510 — port exhaustion now maps to port_in_use for both engines). But it regresses the single-JSON-on-stderr guarantee that #510 explicitly fixed. Required:

  1. BLOCKER — first-run telemetry notice and telemetry-debug JSON corrupt the one-JSON-on-stderr contract (empirically verified on this branch). Emit structured local runtime errors in JSON mode #510 spent two fix commits on this; sdairs confirmed:

    • "Fixed in a198463. Structured local failures now defer a missing first-run telemetry notice without writing the consent marker, so stderr remains one JSON value."
    • "Fixed in 39633edfinalize_inner now suppresses only Action::Debug when a structured local error defers telemetry output… asserts stderr is exactly one JSON value."
      This PR touches no telemetry code (main.rs returns no defer flag; finalize_inner has no suppression). Verified with a fresh $HOME and the built binary: (a) first-ever run of a failing local --json command prints the JSON error object followed by the first-run telemetry notice — two documents; (b) CHCTL_TELEMETRY_DEBUG=1 on a failing run emits two JSON documents (error envelope + telemetry payload). The README claim "writes exactly one error object to stderr" is therefore false on first run and in telemetry-debug mode — precisely the fresh-agent-install scenario the issue targets. The existing test suite cannot see this: every subprocess helper does .env_clear() + DO_NOT_TRACK=1, bypassing telemetry entirely. Port Emit structured local runtime errors in JSON mode #510's defer_telemetry_notice handling and add a fresh-home, no-DO_NOT_TRACK subprocess test asserting stderr is exactly one JSON value (as Emit structured local runtime errors in JSON mode #510 had).
  2. Minor (decision, documented so probably fine): error.command became optional (skip_serializing_if) where the issue's example envelope always included it; ensure consumers keying on it handle absence.

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

@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from 092ece4 to 2d7e62f Compare August 26, 2026 18:58
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from 2d7e62f to 56b6c6b Compare August 26, 2026 19:01
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from 56b6c6b to 44d5d3a Compare August 26, 2026 19:03
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch 2 times, most recently from 4dd00ec to cb3f27c Compare August 26, 2026 19:10
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from cb3f27c to 90e3caa Compare August 26, 2026 19:12
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch 2 times, most recently from d6d2761 to fc109d0 Compare August 26, 2026 19:24
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from fc109d0 to 7566c25 Compare August 26, 2026 19:28
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch 3 times, most recently from cc04039 to 7ffe3b4 Compare August 26, 2026 19:36
Comment thread crates/clickhousectl/src/main.rs
Comment thread crates/clickhousectl/src/local/output.rs
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from 7ffe3b4 to 4a22a29 Compare August 26, 2026 20:10
Comment thread crates/clickhousectl/src/local/output.rs
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from 4a22a29 to 3aa3abe Compare August 26, 2026 20:30
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch 2 times, most recently from 948bf34 to 8606b47 Compare August 26, 2026 20:44
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from 8606b47 to 8a98f5b Compare August 26, 2026 20:49
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from 8a98f5b to 1fbe18f Compare August 26, 2026 20:55
Comment thread crates/clickhousectl/src/local/output.rs
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from 1fbe18f to c76b867 Compare August 27, 2026 06:34
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from c76b867 to 57af65c Compare August 27, 2026 06:44
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from 57af65c to 1227c2f Compare August 27, 2026 06:49
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from 1227c2f to 1833011 Compare August 27, 2026 06:53
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch 2 times, most recently from b66fa8f to ca115e1 Compare August 27, 2026 07:06
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from ca115e1 to c506753 Compare August 27, 2026 07:12
Comment thread crates/clickhousectl/src/local/output.rs
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from 3618e55 to d2c7018 Compare August 27, 2026 09:19
@sdairs
sdairs force-pushed the stack/475-structured-local-errors branch from d2c7018 to c79bf76 Compare August 27, 2026 10:13

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

} else {
version_in_use_by_running_server(version)
}
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

JSON install skips running-server check

Medium Severity

When structured_output is set, the commit callback always returns Ok(false) instead of calling version_in_use_by_running_server. JSON/agent master reinstalls then skip metadata recovery and can commit even when human mode would fail, and they never surface the running-server note.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c79bf76. Configure here.

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.

Emit structured errors in local --json and agent mode

1 participant